fix(security): prevent directory traversal in UKO persistence _path_for() #10449

New issue

Open

opened 2026-04-18 09:42:09 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-18 09:42:09 +00:00

Owner

Metadata

Commit Message: fix(security): prevent directory traversal in UKO persistence _path_for()
Branch: bugfix/auto3-uko-persistence-path-traversal

Background and Context

JSONFilePersistenceBackend._path_for() in src/cleveragents/application/services/uko_persistence.py constructs a file path from a caller-supplied project string using only / and : replacement. The .. sequence is not removed or rejected, so a crafted project name such as ../../etc/passwd resolves to a path outside the intended _base_dir, enabling arbitrary file read/write on the host filesystem.

Code Evidence (verified in current codebase):

File: src/cleveragents/application/services/uko_persistence.py
Lines 103-104:

def _path_for(self, project: str) -> Path:
    """Return the file path for *project*."""
    safe_name = project.replace("/", "_").replace(":", "_")  # INSUFFICIENT SANITIZATION
    return self._base_dir / f"uko_graph_{safe_name}.json"

The sanitization only replaces / and : characters. It does NOT prevent .. (dot-dot) directory traversal. An attacker who controls the project parameter can escape the intended base directory:

Input: project = "../../etc/passwd"
After replace: "....etc.passwd" (unchanged — .. contains no / or :)
Resulting path: base_dir / "uko_graph_../../etc/passwd.json" → resolves outside base_dir

Note

: This issue is tracked as part of the automated bug hunt pool. Reference tag: [AUTO-BUG-3]

Actual Behavior

# uko_persistence.py lines 103-104
def _path_for(self, project: str) -> Path:
    safe_name = project.replace("/", "_").replace(":", "_")  # only strips / and :
    return self._base_dir / f"uko_graph_{safe_name}.json"

Passing project = "../../etc/passwd" produces a path that resolves outside _base_dir.

Expected Behavior

_path_for() must reject any project name whose resolved path falls outside _base_dir, raising ValueError for invalid inputs.

Acceptance Criteria

_path_for() raises ValueError for project names containing .. or absolute path components.
The resolved path is verified to be within _base_dir before returning.
All existing UKO persistence tests continue to pass.
New TDD test (tagged @tdd_issue, @tdd_issue_<N>, @tdd_expected_fail) demonstrates the traversal and passes after the fix.

Subtasks

Write a Behave TDD scenario in features/ that passes a traversal project name and asserts ValueError is raised; tag it @tdd_issue, @tdd_issue_<N>, @tdd_expected_fail.
Add path validation in _path_for(): reject names containing .. or absolute separators.
Add a resolved-path containment check: assert resolved.is_relative_to(self._base_dir).
Remove @tdd_expected_fail tag once the fix is in place.
Verify coverage ≥97% via nox -s coverage_report.
Run nox (all default sessions) and fix any errors.

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
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
Agent: new-issue-creator

## Metadata - **Commit Message**: `fix(security): prevent directory traversal in UKO persistence _path_for()` - **Branch**: `bugfix/auto3-uko-persistence-path-traversal` ## Background and Context `JSONFilePersistenceBackend._path_for()` in `src/cleveragents/application/services/uko_persistence.py` constructs a file path from a caller-supplied `project` string using only `/` and `:` replacement. The `..` sequence is not removed or rejected, so a crafted project name such as `../../etc/passwd` resolves to a path outside the intended `_base_dir`, enabling arbitrary file read/write on the host filesystem. **Code Evidence** (verified in current codebase): File: `src/cleveragents/application/services/uko_persistence.py` Lines 103-104: ```python def _path_for(self, project: str) -> Path: """Return the file path for *project*.""" safe_name = project.replace("/", "_").replace(":", "_") # INSUFFICIENT SANITIZATION return self._base_dir / f"uko_graph_{safe_name}.json" ``` The sanitization only replaces `/` and `:` characters. It does NOT prevent `..` (dot-dot) directory traversal. An attacker who controls the `project` parameter can escape the intended base directory: - Input: `project = "../../etc/passwd"` - After replace: `"....etc.passwd"` (unchanged — `..` contains no `/` or `:`) - Resulting path: `base_dir / "uko_graph_../../etc/passwd.json"` → resolves outside `base_dir` > **Note**: This issue is tracked as part of the automated bug hunt pool. Reference tag: `[AUTO-BUG-3]` ## Actual Behavior ```python # uko_persistence.py lines 103-104 def _path_for(self, project: str) -> Path: safe_name = project.replace("/", "_").replace(":", "_") # only strips / and : return self._base_dir / f"uko_graph_{safe_name}.json" ``` Passing `project = "../../etc/passwd"` produces a path that resolves outside `_base_dir`. ## Expected Behavior `_path_for()` must reject any project name whose resolved path falls outside `_base_dir`, raising `ValueError` for invalid inputs. ## Acceptance Criteria - [ ] `_path_for()` raises `ValueError` for project names containing `..` or absolute path components. - [ ] The resolved path is verified to be within `_base_dir` before returning. - [ ] All existing UKO persistence tests continue to pass. - [ ] New TDD test (tagged `@tdd_issue`, `@tdd_issue_<N>`, `@tdd_expected_fail`) demonstrates the traversal and passes after the fix. ## Subtasks - [ ] Write a Behave TDD scenario in `features/` that passes a traversal project name and asserts `ValueError` is raised; tag it `@tdd_issue`, `@tdd_issue_<N>`, `@tdd_expected_fail`. - [ ] Add path validation in `_path_for()`: reject names containing `..` or absolute separators. - [ ] Add a resolved-path containment check: `assert resolved.is_relative_to(self._base_dir)`. - [ ] Remove `@tdd_expected_fail` tag once the fix is in place. - [ ] Verify coverage ≥97% via `nox -s coverage_report`. - [ ] Run `nox` (all default sessions) and fix any errors. ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - 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** Agent: new-issue-creator

HAL9000 added the

labels

2026-04-18 09:43:32 +00:00

HAL9000 referenced this issue