fix(security): fix file_tools.py validate_path startswith bypass #7478 #11245

Open
HAL9000 wants to merge 3 commits from fix-file-tools-path-validation into master
Owner

Summary

This PR fixes a critical path traversal vulnerability in the validate_path function of file_tools.py.

Vulnerability

The original validate_path implementation relied on pathlib's / join operator followed by .resolve() and .relative_to() checks. This has a well-known bypass: when an absolute path is passed to pathlib's / operator, it completely discards the base path.

Example:

Path("/sandbox") / "/etc/passwd"  # returns PosixPath("/etc/passwd") - sandbox is DISCARDED!

Fix

Added defensive pre-validation on the raw input string:

  1. Empty path rejection - validates non-empty input before any pathlib operations
  2. Absolute path rejection - detects / or \ prefixed paths and rejects them early, preventing sandbox bypass
  3. Component-walk traversal detection - walks through each path component individually, checking that no intermediate step (including .. handling) escapes outside the sandbox root

Files Changed

  • src/cleveragents/tool/builtins/file_tools.py - Enhanced validate_path with pre-validation
  • tests/test_file_tools_security.py - New comprehensive security test suite (14 test methods)

Closes #7478

## Summary This PR fixes a critical path traversal vulnerability in the `validate_path` function of `file_tools.py`. ## Vulnerability The original `validate_path` implementation relied on pathlib's `/` join operator followed by `.resolve()` and `.relative_to()` checks. This has a well-known bypass: when an absolute path is passed to pathlib's `/` operator, it completely discards the base path. Example: ```python Path("/sandbox") / "/etc/passwd" # returns PosixPath("/etc/passwd") - sandbox is DISCARDED! ``` ## Fix Added defensive pre-validation on the raw input string: 1. **Empty path rejection** - validates non-empty input before any pathlib operations 2. **Absolute path rejection** - detects `/` or `\` prefixed paths and rejects them early, preventing sandbox bypass 3. **Component-walk traversal detection** - walks through each path component individually, checking that no intermediate step (including `..` handling) escapes outside the sandbox root ## Files Changed - `src/cleveragents/tool/builtins/file_tools.py` - Enhanced validate_path with pre-validation - `tests/test_file_tools_security.py` - New comprehensive security test suite (14 test methods) Closes #7478
fix: secure path validation in file_tools.py and add security tests
Some checks failed
CI / build (pull_request) Successful in 1m7s
CI / lint (pull_request) Failing after 1m30s
CI / quality (pull_request) Successful in 1m29s
CI / security (pull_request) Successful in 1m56s
CI / typecheck (pull_request) Successful in 2m2s
CI / push-validation (pull_request) Successful in 33s
CI / helm (pull_request) Successful in 36s
CI / integration_tests (pull_request) Successful in 4m48s
CI / unit_tests (pull_request) Failing after 6m16s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 3s
93a5784ddb
chore: re-trigger CI [controller]
Some checks failed
CI / push-validation (pull_request) Successful in 40s
CI / lint (pull_request) Failing after 51s
CI / helm (pull_request) Successful in 43s
CI / build (pull_request) Successful in 50s
CI / typecheck (pull_request) Successful in 1m28s
CI / quality (pull_request) Successful in 1m30s
CI / security (pull_request) Successful in 1m39s
CI / integration_tests (pull_request) Failing after 4m0s
CI / unit_tests (pull_request) Failing after 6m27s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 4s
c7a77ac409
HAL9000 added this to the v3.5.0 milestone 2026-05-28 03:51:14 +00:00
HAL9000 force-pushed fix-file-tools-path-validation from c7a77ac409
Some checks failed
CI / push-validation (pull_request) Successful in 40s
CI / lint (pull_request) Failing after 51s
CI / helm (pull_request) Successful in 43s
CI / build (pull_request) Successful in 50s
CI / typecheck (pull_request) Successful in 1m28s
CI / quality (pull_request) Successful in 1m30s
CI / security (pull_request) Successful in 1m39s
CI / integration_tests (pull_request) Failing after 4m0s
CI / unit_tests (pull_request) Failing after 6m27s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 4s
to 31c714bfdc
Some checks failed
CI / lint (pull_request) Failing after 53s
CI / build (pull_request) Successful in 31s
CI / typecheck (pull_request) Successful in 1m6s
CI / quality (pull_request) Successful in 56s
CI / security (pull_request) Successful in 1m14s
CI / helm (pull_request) Successful in 39s
CI / push-validation (pull_request) Successful in 21s
CI / integration_tests (pull_request) Successful in 3m54s
CI / unit_tests (pull_request) Successful in 5m13s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 3s
2026-05-28 04:06:20 +00:00
Compare
fix(lint): replace EN DASH with hyphen-minus in comment; apply ruff format
All checks were successful
CI / push-validation (pull_request) Successful in 34s
CI / helm (pull_request) Successful in 37s
CI / build (pull_request) Successful in 48s
CI / lint (pull_request) Successful in 1m8s
CI / typecheck (pull_request) Successful in 1m14s
CI / quality (pull_request) Successful in 1m7s
CI / security (pull_request) Successful in 1m15s
CI / integration_tests (pull_request) Successful in 3m7s
CI / unit_tests (pull_request) Successful in 4m54s
CI / docker (pull_request) Successful in 1m26s
CI / coverage (pull_request) Successful in 11m59s
CI / status-check (pull_request) Successful in 3s
d646288142
Author
Owner

[CONTROLLER-DEFER:Gate 1:linked_issue_closed]

This PR has been deferred for re-evaluation. The controller has stepped back
from processing it. To resume, a human or scope-evaluator must clear the
deferral flag AND re-add the auto/sentinel label.

Decision:

  • Gate: Gate 1
  • Reason category: linked_issue_closed
  • Canonical: #-
  • LLM confidence: deterministic
  • LLM reasoning: Linked issue(s) [7478] already closed; no LLM call needed.

To clear the deferral (SQL):
UPDATE workflows SET deferred_reason=NULL,
deferred_at=NULL,
deferred_target_workflow_id=NULL
WHERE workflow_id = 9;

INSERT INTO controller_events
  (workflow_id, ts, event_type, payload, cause, forgejo_write_pending, replay_attempts)
VALUES (9, datetime('now'), 'deferral_cleared',
        json_object('cleared_by', 'operator', 'reason', '<your reason>'),
        'operator', 0, 0);

Audit ID: 1329


Automated by the CleverAgents controller pipeline.
Identity: HAL9000 (pipeline action)

[CONTROLLER-DEFER:Gate 1:linked_issue_closed] This PR has been deferred for re-evaluation. The controller has stepped back from processing it. To resume, a human or scope-evaluator must clear the deferral flag AND re-add the auto/sentinel label. Decision: - Gate: Gate 1 - Reason category: linked_issue_closed - Canonical: #- - LLM confidence: deterministic - LLM reasoning: Linked issue(s) [7478] already closed; no LLM call needed. To clear the deferral (SQL): UPDATE workflows SET deferred_reason=NULL, deferred_at=NULL, deferred_target_workflow_id=NULL WHERE workflow_id = 9; INSERT INTO controller_events (workflow_id, ts, event_type, payload, cause, forgejo_write_pending, replay_attempts) VALUES (9, datetime('now'), 'deferral_cleared', json_object('cleared_by', 'operator', 'reason', '<your reason>'), 'operator', 0, 0); Audit ID: 1329 --- Automated by the CleverAgents controller pipeline. Identity: HAL9000 (pipeline action) <!-- controller:fingerprint:38a2e567a0026e65 -->
All checks were successful
CI / push-validation (pull_request) Successful in 34s
CI / helm (pull_request) Successful in 37s
CI / build (pull_request) Successful in 48s
Required
Details
CI / lint (pull_request) Successful in 1m8s
Required
Details
CI / typecheck (pull_request) Successful in 1m14s
Required
Details
CI / quality (pull_request) Successful in 1m7s
Required
Details
CI / security (pull_request) Successful in 1m15s
Required
Details
CI / integration_tests (pull_request) Successful in 3m7s
Required
Details
CI / unit_tests (pull_request) Successful in 4m54s
Required
Details
CI / docker (pull_request) Successful in 1m26s
Required
Details
CI / coverage (pull_request) Successful in 11m59s
Required
Details
CI / status-check (pull_request) Successful in 3s
This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix-file-tools-path-validation:fix-file-tools-path-validation
git switch fix-file-tools-path-validation
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core!11245
No description provided.