BUG-HUNT: [Security] Potential path traversal in persona registry #1916

New issue

Open

opened 2026-04-03 00:12:21 +00:00 by freemo · 1 comment

freemo commented

2026-04-03 00:12:21 +00:00

Owner

Metadata

Branch: fix/security-path-traversal-persona-registry
Commit Message: fix(tui): sanitize persona name to prevent path traversal in PersonaRegistry.persona_path
Milestone: v3.6.0
Parent Epic: #400

Bug Report: [Security] — Potential path traversal in persona registry

Severity Assessment

Impact: A malicious user could create or delete files outside the intended persona directory, potentially leading to data corruption or denial of service.
Likelihood: Medium, as it requires a user to create a persona with a malicious name.
Priority: High

Location

File: src/cleveragents/tui/persona/registry.py
Function/Class: PersonaRegistry.persona_path
Lines: 101-105

Description

The persona_path method in the PersonaRegistry class does not sufficiently validate the persona name to prevent path traversal attacks. A malicious user could craft a persona name with ../ sequences to access parent directories. While resolve() might mitigate this, relying on it implicitly is not a secure practice.

Evidence

def persona_path(self, name: str) -> Path:
    Persona.validate_name_safe(name)
    result = (self.personas_dir / f"{name}.yaml").resolve()
    if not result.is_relative_to(self.personas_dir.resolve()):
        raise ValueError(f"Invalid persona name: {name}")
    return result

Expected Behavior

The persona name should be sanitized to remove any path traversal characters before being used to construct a file path.

Actual Behavior

The persona name is used directly to construct a file path, which could lead to a path traversal vulnerability.

Suggested Fix

Sanitize the persona name to remove any characters that could be used for path traversal, such as /, \, and ...

Subtasks

Audit Persona.validate_name_safe to determine what characters it currently rejects and identify any gaps (e.g., ../, \, null bytes, encoded traversal sequences)
Implement explicit sanitization in PersonaRegistry.persona_path to strip or reject path traversal characters (/, \, ..) from the persona name before path construction
Ensure the existing is_relative_to guard is retained as a defence-in-depth check after sanitization
Write Behave unit tests covering: valid persona names, names containing ../, names containing \, names containing null bytes, and names with URL-encoded traversal sequences
Ensure all nox stages pass (nox -s lint, nox -s typecheck, nox -s unit_tests)
Verify coverage remains >= 97% via nox -s coverage_report

Definition of Done

PersonaRegistry.persona_path explicitly sanitizes the persona name to reject any path traversal characters before constructing the file path
Persona.validate_name_safe is audited and updated if gaps are found
No path traversal attack vector remains in PersonaRegistry.persona_path
Behave unit tests cover all path validation scenarios (valid, traversal via ../, traversal via \, null bytes, encoded sequences)
All nox stages pass
Coverage >= 97%
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done

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

## Metadata - **Branch**: `fix/security-path-traversal-persona-registry` - **Commit Message**: `fix(tui): sanitize persona name to prevent path traversal in PersonaRegistry.persona_path` - **Milestone**: v3.6.0 - **Parent Epic**: #400 ## Bug Report: [Security] — Potential path traversal in persona registry ### Severity Assessment - **Impact**: A malicious user could create or delete files outside the intended persona directory, potentially leading to data corruption or denial of service. - **Likelihood**: Medium, as it requires a user to create a persona with a malicious name. - **Priority**: High ### Location - **File**: `src/cleveragents/tui/persona/registry.py` - **Function/Class**: `PersonaRegistry.persona_path` - **Lines**: 101-105 ### Description The `persona_path` method in the `PersonaRegistry` class does not sufficiently validate the persona name to prevent path traversal attacks. A malicious user could craft a persona name with `../` sequences to access parent directories. While `resolve()` might mitigate this, relying on it implicitly is not a secure practice. ### Evidence ```python def persona_path(self, name: str) -> Path: Persona.validate_name_safe(name) result = (self.personas_dir / f"{name}.yaml").resolve() if not result.is_relative_to(self.personas_dir.resolve()): raise ValueError(f"Invalid persona name: {name}") return result ``` ### Expected Behavior The persona name should be sanitized to remove any path traversal characters before being used to construct a file path. ### Actual Behavior The persona name is used directly to construct a file path, which could lead to a path traversal vulnerability. ### Suggested Fix Sanitize the persona name to remove any characters that could be used for path traversal, such as `/`, `\`, and `..`. ### Category security ## Subtasks - [ ] Audit `Persona.validate_name_safe` to determine what characters it currently rejects and identify any gaps (e.g., `../`, `\`, null bytes, encoded traversal sequences) - [ ] Implement explicit sanitization in `PersonaRegistry.persona_path` to strip or reject path traversal characters (`/`, `\`, `..`) from the persona name before path construction - [ ] Ensure the existing `is_relative_to` guard is retained as a defence-in-depth check after sanitization - [ ] Write Behave unit tests covering: valid persona names, names containing `../`, names containing `\`, names containing null bytes, and names with URL-encoded traversal sequences - [ ] Ensure all nox stages pass (`nox -s lint`, `nox -s typecheck`, `nox -s unit_tests`) - [ ] Verify coverage remains >= 97% via `nox -s coverage_report` ## Definition of Done - [ ] `PersonaRegistry.persona_path` explicitly sanitizes the persona name to reject any path traversal characters before constructing the file path - [ ] `Persona.validate_name_safe` is audited and updated if gaps are found - [ ] No path traversal attack vector remains in `PersonaRegistry.persona_path` - [ ] Behave unit tests cover all path validation scenarios (valid, traversal via `../`, traversal via `\`, null bytes, encoded sequences) - [ ] All nox stages pass - [ ] Coverage >= 97% - [ ] A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation - [ ] The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly - [ ] The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: ca-new-issue-creator

freemo added this to the v3.6.0 milestone

2026-04-03 00:12:48 +00:00

freemo added a new dependency

2026-04-03 00:15:34 +00:00

#400 Epic: Post-MVP Security

freemo commented

2026-04-03 00:19:49 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
MoSCoW: MoSCoW/Should Have — bug or error handling improvement.

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

Issue triaged by project owner: - **State**: Verified - **MoSCoW**: MoSCoW/Should Have — bug or error handling improvement. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner