cleveragents/cleveragents-core

Fork 3

UAT: `validate_path()` and `validate_sandbox_path()` use `str.startswith()` without `os.sep` suffix — path traversal prefix-collision bypass in file_tools.py, file_ops.py, and inline_executor.py #3960

New issue

Open

opened 2026-04-06 07:53:08 +00:00 by freemo · 0 comments

freemo commented

2026-04-06 07:53:08 +00:00

Owner

Metadata

Branch: fix/security-path-traversal-prefix-collision
Commit Message: fix(security): prevent path traversal prefix-collision bypass in file tools
Milestone: (none — backlog, see note below)
Parent Epic: #362

Backlog note: This issue was discovered during autonomous operation
on milestone v3.5.0. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Bug Report

What Was Tested

Path traversal protection in the built-in file tools and skill file tools.

Expected Behavior (from spec)

Per docs/specification.md §Security Model — Sandbox Isolation:

The sandbox path is derived deterministically from the plan ULID, preventing path traversal.

All file operations must be strictly confined to the sandbox root. A path like ../../etc/passwd or any path that escapes the sandbox root must be rejected.

Actual Behavior

Three path validation functions use str(target).startswith(str(root)) without appending os.sep (or "/"), creating a prefix-collision bypass:

File 1: src/cleveragents/tool/builtins/file_tools.py line 86

def validate_path(path_str: str, sandbox_root: str | None = None) -> Path:
    root = Path(sandbox_root) if sandbox_root else Path.cwd()
    root = root.resolve()
    target = (root / path_str).resolve()
    if not str(target).startswith(str(root)):  # ← BUG: no os.sep suffix
        raise ValueError(f"Path traversal detected: '{path_str}' escapes sandbox root")
    return target

File 2: src/cleveragents/skills/builtins/file_ops.py line 80

def validate_sandbox_path(path_str: str, sandbox_root: str | None = None) -> Path:
    ...
    if not str(target).startswith(str(root)):  # ← BUG: no os.sep suffix
        raise ValueError(f"Path traversal detected: '{path_str}' escapes sandbox root")

File 3: src/cleveragents/skills/inline_executor.py line 266

if not str(resolved).startswith(str(sandbox_resolved)):  # ← BUG: no os.sep suffix
    return f"Path '{value}' for key '{key}' escapes sandbox root '{sandbox_path}'"

Attack Scenario

If the sandbox root is /tmp/ca-sandbox-foo, an attacker can access /tmp/ca-sandbox-foobar/secret because:

"/tmp/ca-sandbox-foobar/secret".startswith("/tmp/ca-sandbox-foo")  # → True (WRONG!)

This allows a tool invocation with sandbox_root="/tmp/ca-sandbox-foo" to read/write files in /tmp/ca-sandbox-foobar/ — a different plan's sandbox.

Correct Implementation

The correct pattern (already used in src/cleveragents/resource/handlers/_base.py line 187) is:

if target != root and not str(target).startswith(str(root) + os.sep):
    raise PermissionError(f"Path '{path}' escapes resource root '{location}'")

And in src/cleveragents/tool/path_mapper.py line 167:

return path.startswith(root + "/")

Steps to Reproduce

from pathlib import Path
import os

# Simulate the bug
root = Path("/tmp/ca-sandbox-foo").resolve()
# Attacker-controlled path targeting a sibling directory
malicious_path = "/tmp/ca-sandbox-foobar/secret"
target = Path(malicious_path).resolve()

# Current (buggy) check:
print(str(target).startswith(str(root)))  # True — BYPASS!

# Correct check:
print(str(target).startswith(str(root) + os.sep))  # False — BLOCKED

Code Locations

src/cleveragents/tool/builtins/file_tools.py:86 — validate_path()
src/cleveragents/skills/builtins/file_ops.py:80 — validate_sandbox_path()
src/cleveragents/skills/inline_executor.py:266 — _validate_paths()

Fix Required

Replace str(target).startswith(str(root)) with target == root or str(target).startswith(str(root) + os.sep) in all three locations. Alternatively, use Python 3.9+'s Path.is_relative_to() which handles this correctly.

Subtasks

Fix validate_path() in src/cleveragents/tool/builtins/file_tools.py
Fix validate_sandbox_path() in src/cleveragents/skills/builtins/file_ops.py
Fix path check in src/cleveragents/skills/inline_executor.py
Add BDD scenario testing the prefix-collision bypass case
Verify fix passes nox -e unit_tests

Definition of Done

All three startswith() checks are replaced with the correct os.sep-suffixed pattern or Path.is_relative_to()
A BDD scenario exists that demonstrates the prefix-collision attack is blocked
nox -e unit_tests passes
nox -e typecheck passes
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/security-path-traversal-prefix-collision` - **Commit Message**: `fix(security): prevent path traversal prefix-collision bypass in file tools` - **Milestone**: _(none — backlog, see note below)_ - **Parent Epic**: #362 > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.5.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. ## Bug Report ### What Was Tested Path traversal protection in the built-in file tools and skill file tools. ### Expected Behavior (from spec) Per `docs/specification.md` §Security Model — Sandbox Isolation: > The sandbox path is derived deterministically from the plan ULID, **preventing path traversal**. All file operations must be strictly confined to the sandbox root. A path like `../../etc/passwd` or any path that escapes the sandbox root must be rejected. ### Actual Behavior Three path validation functions use `str(target).startswith(str(root))` **without** appending `os.sep` (or `"/"`), creating a **prefix-collision bypass**: **File 1: `src/cleveragents/tool/builtins/file_tools.py` line 86** ```python def validate_path(path_str: str, sandbox_root: str | None = None) -> Path: root = Path(sandbox_root) if sandbox_root else Path.cwd() root = root.resolve() target = (root / path_str).resolve() if not str(target).startswith(str(root)): # ← BUG: no os.sep suffix raise ValueError(f"Path traversal detected: '{path_str}' escapes sandbox root") return target ``` **File 2: `src/cleveragents/skills/builtins/file_ops.py` line 80** ```python def validate_sandbox_path(path_str: str, sandbox_root: str | None = None) -> Path: ... if not str(target).startswith(str(root)): # ← BUG: no os.sep suffix raise ValueError(f"Path traversal detected: '{path_str}' escapes sandbox root") ``` **File 3: `src/cleveragents/skills/inline_executor.py` line 266** ```python if not str(resolved).startswith(str(sandbox_resolved)): # ← BUG: no os.sep suffix return f"Path '{value}' for key '{key}' escapes sandbox root '{sandbox_path}'" ``` ### Attack Scenario If the sandbox root is `/tmp/ca-sandbox-foo`, an attacker can access `/tmp/ca-sandbox-foobar/secret` because: ```python "/tmp/ca-sandbox-foobar/secret".startswith("/tmp/ca-sandbox-foo") # → True (WRONG!) ``` This allows a tool invocation with `sandbox_root="/tmp/ca-sandbox-foo"` to read/write files in `/tmp/ca-sandbox-foobar/` — a different plan's sandbox. ### Correct Implementation The correct pattern (already used in `src/cleveragents/resource/handlers/_base.py` line 187) is: ```python if target != root and not str(target).startswith(str(root) + os.sep): raise PermissionError(f"Path '{path}' escapes resource root '{location}'") ``` And in `src/cleveragents/tool/path_mapper.py` line 167: ```python return path.startswith(root + "/") ``` ### Steps to Reproduce ```python from pathlib import Path import os # Simulate the bug root = Path("/tmp/ca-sandbox-foo").resolve() # Attacker-controlled path targeting a sibling directory malicious_path = "/tmp/ca-sandbox-foobar/secret" target = Path(malicious_path).resolve() # Current (buggy) check: print(str(target).startswith(str(root))) # True — BYPASS! # Correct check: print(str(target).startswith(str(root) + os.sep)) # False — BLOCKED ``` ### Code Locations - `src/cleveragents/tool/builtins/file_tools.py:86` — `validate_path()` - `src/cleveragents/skills/builtins/file_ops.py:80` — `validate_sandbox_path()` - `src/cleveragents/skills/inline_executor.py:266` — `_validate_paths()` ### Fix Required Replace `str(target).startswith(str(root))` with `target == root or str(target).startswith(str(root) + os.sep)` in all three locations. Alternatively, use Python 3.9+'s `Path.is_relative_to()` which handles this correctly. ## Subtasks - [ ] Fix `validate_path()` in `src/cleveragents/tool/builtins/file_tools.py` - [ ] Fix `validate_sandbox_path()` in `src/cleveragents/skills/builtins/file_ops.py` - [ ] Fix path check in `src/cleveragents/skills/inline_executor.py` - [ ] Add BDD scenario testing the prefix-collision bypass case - [ ] Verify fix passes `nox -e unit_tests` ## Definition of Done - [ ] All three `startswith()` checks are replaced with the correct `os.sep`-suffixed pattern or `Path.is_relative_to()` - [ ] A BDD scenario exists that demonstrates the prefix-collision attack is blocked - [ ] `nox -e unit_tests` passes - [ ] `nox -e typecheck` passes - All nox stages pass - Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added a new dependency

2026-04-06 07:53:14 +00:00

#362 Epic: Security & Safety Hardening

freemo referenced this issue

2026-04-06 07:53:30 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775

freemo added the

Points

label

2026-04-06 07:53:35 +00:00

freemo referenced this issue

2026-04-06 07:56:58 +00:00