BUG-HUNT: [security] Incomplete sanitization in `_actor_name` could lead to path traversal #3175

New issue

Open

opened 2026-04-05 07:18:12 +00:00 by freemo · 1 comment

freemo commented

2026-04-05 07:18:12 +00:00

Owner

Metadata

Branch: fix/security/actor-name-path-traversal-sanitization
Commit Message: fix(actor): harden _actor_name sanitization to prevent path traversal via whitelist
Milestone: (unassigned — see backlog note)
Parent Epic: #362

Bug Report

Severity Assessment

Impact: An attacker could potentially craft a malicious provider or model name to traverse the file system and access unauthorized files.
Likelihood: Medium. It depends on how the actor names are used in other parts of the system.
Priority: Critical

Location

File: src/cleveragents/actor/registry.py
Function/Class: ActorRegistry._actor_name
Lines: 41–69

Description

The _actor_name method in ActorRegistry sanitizes provider and model names by replacing forward slashes (/) with hyphens (-). However, it does not sanitize other potentially dangerous characters like . or ... The method's docstring explicitly states that it does not strip every character disallowed by the regex ^[a-z0-9_-]+/[a-z0-9_-]+$.

If the actor name is used to construct file paths, an attacker could craft a provider or model name with ../ to traverse the file system. For example, a model name like ../../../../etc/passwd would be sanitized to ..-..-..-..-etc-passwd, which is still a valid path traversal sequence on some systems.

Evidence

def _actor_name(self, provider_name: str, model_name: str) -> str:
    """Build a canonical ``namespace/identifier`` actor name.

    Provider and model names are sanitised to handle the most common
    violations (embedded slashes from providers like OpenRouter whose
    default model contains ``/``):

    * Forward slashes are replaced with ``-``.
    * The result is lowercased.

    .. note::

        This does **not** strip every character disallowed by
        ``^[a-z0-9_-]+/[a-z0-9_-]+$`` (e.g. spaces, dots, ``@``).
        In practice provider and model identifiers from configured
        providers never contain those characters.
    """
    sanitized_provider = provider_name.replace("/", "-").lower()
    sanitized_model = model_name.replace("/", "-").lower()
    return f"{sanitized_provider}/{sanitized_model}"

Expected Behavior

The _actor_name method should strip all characters that are not allowed in a file path, or it should use a safe method to construct the actor name that is not vulnerable to path traversal.

Suggested Fix

Use a whitelist of allowed characters (e.g., [a-zA-Z0-9_-]) and remove any character that is not in the whitelist. Alternatively, use a library like werkzeug.utils.secure_filename to generate a safe filename.

Subtasks

Audit all call sites of _actor_name to determine if the result is used in file path construction
Replace the partial slash-replacement sanitization with a strict whitelist regex (^[a-z0-9_-]+$) applied to each component
Raise ValueError with a descriptive message if a provider or model name contains characters outside the whitelist
Update the docstring to reflect the new strict sanitization contract
Add Behave unit tests covering: valid names, names with slashes, names with dots, names with .., names with @, names with spaces
Verify no regression in existing actor registry tests

Definition of Done

_actor_name rejects or strips all characters outside [a-z0-9_-] for each component
Path traversal sequences (e.g., ../, ..) cannot survive sanitization
All new and existing Behave unit tests pass
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/security/actor-name-path-traversal-sanitization` - **Commit Message**: `fix(actor): harden _actor_name sanitization to prevent path traversal via whitelist` - **Milestone**: *(unassigned — see backlog note)* - **Parent Epic**: #362 ## Bug Report ### Severity Assessment - **Impact**: An attacker could potentially craft a malicious provider or model name to traverse the file system and access unauthorized files. - **Likelihood**: Medium. It depends on how the actor names are used in other parts of the system. - **Priority**: Critical ### Location - **File**: `src/cleveragents/actor/registry.py` - **Function/Class**: `ActorRegistry._actor_name` - **Lines**: 41–69 ### Description The `_actor_name` method in `ActorRegistry` sanitizes provider and model names by replacing forward slashes (`/`) with hyphens (`-`). However, it does not sanitize other potentially dangerous characters like `.` or `..`. The method's docstring explicitly states that it does not strip every character disallowed by the regex `^[a-z0-9_-]+/[a-z0-9_-]+$`. If the actor name is used to construct file paths, an attacker could craft a provider or model name with `../` to traverse the file system. For example, a model name like `../../../../etc/passwd` would be sanitized to `..-..-..-..-etc-passwd`, which is still a valid path traversal sequence on some systems. ### Evidence ```python def _actor_name(self, provider_name: str, model_name: str) -> str: """Build a canonical ``namespace/identifier`` actor name. Provider and model names are sanitised to handle the most common violations (embedded slashes from providers like OpenRouter whose default model contains ``/``): * Forward slashes are replaced with ``-``. * The result is lowercased. .. note:: This does **not** strip every character disallowed by ``^[a-z0-9_-]+/[a-z0-9_-]+$`` (e.g. spaces, dots, ``@``). In practice provider and model identifiers from configured providers never contain those characters. """ sanitized_provider = provider_name.replace("/", "-").lower() sanitized_model = model_name.replace("/", "-").lower() return f"{sanitized_provider}/{sanitized_model}" ``` ### Expected Behavior The `_actor_name` method should strip all characters that are not allowed in a file path, or it should use a safe method to construct the actor name that is not vulnerable to path traversal. ### Suggested Fix Use a whitelist of allowed characters (e.g., `[a-zA-Z0-9_-]`) and remove any character that is not in the whitelist. Alternatively, use a library like `werkzeug.utils.secure_filename` to generate a safe filename. ## Subtasks - [ ] Audit all call sites of `_actor_name` to determine if the result is used in file path construction - [ ] Replace the partial slash-replacement sanitization with a strict whitelist regex (`^[a-z0-9_-]+$`) applied to each component - [ ] Raise `ValueError` with a descriptive message if a provider or model name contains characters outside the whitelist - [ ] Update the docstring to reflect the new strict sanitization contract - [ ] Add Behave unit tests covering: valid names, names with slashes, names with dots, names with `..`, names with `@`, names with spaces - [ ] Verify no regression in existing actor registry tests ## Definition of Done - [ ] `_actor_name` rejects or strips all characters outside `[a-z0-9_-]` for each component - [ ] Path traversal sequences (e.g., `../`, `..`) cannot survive sanitization - [ ] All new and existing Behave unit tests pass - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: ca-new-issue-creator

freemo added a new dependency

2026-04-05 07:22:43 +00:00

#362 Epic: Security & Safety Hardening

freemo added this to the v3.6.0 milestone

2026-04-05 07:29:08 +00:00

freemo commented

2026-04-05 07:30:39 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Critical — security vulnerability (path traversal) in actor name sanitization
Milestone: v3.6.0 (assigned — security hardening fits the Post-MVP Security scope)
MoSCoW: Should Have — security vulnerabilities should be addressed promptly; however, the actual exploitability depends on whether actor names are used in file path construction, which needs auditing first
Parent Epic: #362 (Security & Safety Hardening)

The first subtask (auditing call sites) should determine the actual severity. If actor names are used in file paths, this should be elevated to Must Have.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Critical — security vulnerability (path traversal) in actor name sanitization - **Milestone**: v3.6.0 (assigned — security hardening fits the Post-MVP Security scope) - **MoSCoW**: Should Have — security vulnerabilities should be addressed promptly; however, the actual exploitability depends on whether actor names are used in file path construction, which needs auditing first - **Parent Epic**: #362 (Security & Safety Hardening) The first subtask (auditing call sites) should determine the actual severity. If actor names are used in file paths, this should be elevated to Must Have. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo referenced this issue

2026-04-05 07:35:27 +00:00

[CA-AUTO] Architecture Supervisor — Session Tracker — 2026-04-05 #3180

freemo referenced this issue

2026-04-05 07:41:47 +00:00

[CA-AUTO] Project Owner — project-owner-1 — Session Tracker #2793

freemo referenced this issue

2026-04-05 09:18:23 +00:00

[CA-AUTO] System Watchdog — Session Tracker — watchdog-1 — 2026-04-05 #3197