fix(security): use relpath containment instead of startswith to prevent prefix-collision bypass #11242

Open
HAL9000 wants to merge 4 commits from feature/security-fix-relpath-pr-11217 into master
Owner

Summary

Replaces insecure str().startswith() path containment checks with proper, canonicalized path containment using os.path.relpath(), Path.is_relative_to(), and Path.relative_to() across three files. This prevents prefix-collision attacks where a malicious path like /tmp/evil could bypass an intended check for paths starting with /tmp/e.

## Summary Replaces insecure `str().startswith()` path containment checks with proper, canonicalized path containment using `os.path.relpath()`, `Path.is_relative_to()`, and `Path.relative_to()` across three files. This prevents prefix-collision attacks where a malicious path like `/tmp/evil` could bypass an intended check for paths starting with `/tmp/e`.
fix(security): use relpath containment instead of startswith to prevent prefix-collision bypass
Some checks failed
CI / push-validation (pull_request) Successful in 31s
CI / helm (pull_request) Successful in 43s
CI / build (pull_request) Successful in 1m13s
CI / lint (pull_request) Successful in 1m46s
CI / quality (pull_request) Successful in 1m51s
CI / security (pull_request) Successful in 1m51s
CI / typecheck (pull_request) Successful in 2m3s
CI / integration_tests (pull_request) Successful in 4m38s
CI / unit_tests (pull_request) Failing after 6m42s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 3s
52c7e96d6b
Replace str().startswith(root) checks with proper relpath-based
containment that prevents prefix-collision bypass attacks where a
sibling directory whose name starts with the root path would pass
an unchecked containment test.

Files changed:
- decomposition_clustering.py: Use os.path.relpath() instead of
  string-prefix matching for root-relative path computation.
- plan.py (_apply_sandbox_changes): Use Path.is_relative_to()
  instead of str.startswith() for sandbox-to-project root check.
- inline_executor.py: Use Path.relative_to() instead of
  str.startswith() for sandbox containment validation.
chore: re-trigger CI [controller]
Some checks failed
CI / lint (pull_request) Successful in 49s
CI / typecheck (pull_request) Successful in 1m10s
CI / push-validation (pull_request) Successful in 35s
CI / helm (pull_request) Successful in 40s
CI / build (pull_request) Successful in 47s
CI / quality (pull_request) Successful in 1m7s
CI / security (pull_request) Successful in 1m54s
CI / integration_tests (pull_request) Failing after 4m10s
CI / unit_tests (pull_request) Failing after 4m30s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 3s
4a71e3c368
HAL9000 force-pushed feature/security-fix-relpath-pr-11217 from 4a71e3c368
Some checks failed
CI / lint (pull_request) Successful in 49s
CI / typecheck (pull_request) Successful in 1m10s
CI / push-validation (pull_request) Successful in 35s
CI / helm (pull_request) Successful in 40s
CI / build (pull_request) Successful in 47s
CI / quality (pull_request) Successful in 1m7s
CI / security (pull_request) Successful in 1m54s
CI / integration_tests (pull_request) Failing after 4m10s
CI / unit_tests (pull_request) Failing after 4m30s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / status-check (pull_request) Failing after 3s
to 9d0c5850b3
Some checks failed
CI / build (pull_request) Successful in 35s
CI / lint (pull_request) Successful in 48s
CI / helm (pull_request) Successful in 29s
CI / quality (pull_request) Successful in 1m8s
CI / typecheck (pull_request) Successful in 1m9s
CI / security (pull_request) Successful in 1m18s
CI / push-validation (pull_request) Successful in 21s
CI / integration_tests (pull_request) Successful in 3m54s
CI / unit_tests (pull_request) Failing after 7m11s
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:09:45 +00:00
Compare
Two follow-up bugs from the relpath-containment refactor:

1. decomposition_clustering._directory_key had inverted boolean logic:
   `if not (rel == "" or not rel.startswith(".." + os.sep))` meant the
   relative path was used ONLY when it escaped the root, and discarded
   when the path was actually inside the root. Fixed to update
   `normalized` exactly when the path is inside (or equal to) root.

2. inline_executor._validate_paths split the original
   `except (OSError, ValueError)` so that ValueError from
   `Path(value).resolve()` (raised for invalid paths such as those
   containing null bytes) propagated instead of returning a graceful
   "Invalid path" error string.

ISSUES CLOSED: #11217
chore: worker ruff auto-fix (pre-push lint gate)
Some checks failed
CI / push-validation (pull_request) Successful in 22s
CI / build (pull_request) Successful in 33s
CI / helm (pull_request) Successful in 34s
CI / lint (pull_request) Successful in 36s
CI / quality (pull_request) Successful in 1m14s
CI / typecheck (pull_request) Successful in 1m18s
CI / security (pull_request) Successful in 1m19s
CI / integration_tests (pull_request) Failing after 2m54s
CI / unit_tests (pull_request) Successful in 4m46s
CI / docker (pull_request) Successful in 1m36s
CI / coverage (pull_request) Successful in 11m13s
CI / status-check (pull_request) Failing after 3s
0aff794f69
Author
Owner

[CONTROLLER-DEFER:Gate 1:needs_evaluation]

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: needs_evaluation
  • Canonical: #-
  • LLM confidence: medium
  • LLM reasoning: PR #11242 (anchor) and PR #11217 have identical titles and both fix the same security issue (#7478) using the relpath containment approach. The anchor's head branch name explicitly references "#11217", indicating it builds on that work. However, #11242 has 4.5x larger scope (32 additions vs 7, covering 3 files vs 1). This could represent: (1) necessary comprehensive coverage of the vulnerability across the codebase, or (2) redundant extension beyond what #11217 sufficiently addresses. Human judgment required to determine if #11242's broader file coverage is essential or unnecessary.
  • Preserved value (when applicable): PR #11242 applies the security fix to 3 files across the codebase (32 additions, 16 deletions), while PR #11217 covers only 1 file (7 additions, 1 deletion). The broader scope in the anchor may be necessary for comprehensive security coverage against prefix-collision bypass attacks across all vulnerable code paths, or may represent over-engineering if the issue is adequately addressed by PR #11217's focused approach.

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

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

Audit ID: 1354


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

[CONTROLLER-DEFER:Gate 1:needs_evaluation] 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: needs_evaluation - Canonical: #- - LLM confidence: medium - LLM reasoning: PR #11242 (anchor) and PR #11217 have identical titles and both fix the same security issue (#7478) using the relpath containment approach. The anchor's head branch name explicitly references "#11217", indicating it builds on that work. However, #11242 has 4.5x larger scope (32 additions vs 7, covering 3 files vs 1). This could represent: (1) necessary comprehensive coverage of the vulnerability across the codebase, or (2) redundant extension beyond what #11217 sufficiently addresses. Human judgment required to determine if #11242's broader file coverage is essential or unnecessary. - Preserved value (when applicable): PR #11242 applies the security fix to 3 files across the codebase (32 additions, 16 deletions), while PR #11217 covers only 1 file (7 additions, 1 deletion). The broader scope in the anchor may be necessary for comprehensive security coverage against prefix-collision bypass attacks across all vulnerable code paths, or may represent over-engineering if the issue is adequately addressed by PR #11217's focused approach. To clear the deferral (SQL): UPDATE workflows SET deferred_reason=NULL, deferred_at=NULL, deferred_target_workflow_id=NULL WHERE workflow_id = 8; INSERT INTO controller_events (workflow_id, ts, event_type, payload, cause, forgejo_write_pending, replay_attempts) VALUES (8, datetime('now'), 'deferral_cleared', json_object('cleared_by', 'operator', 'reason', '<your reason>'), 'operator', 0, 0); Audit ID: 1354 --- Automated by the CleverAgents controller pipeline. Identity: HAL9000 (pipeline action) <!-- controller:fingerprint:9ad2f404a97cfcfc -->
Some checks failed
CI / push-validation (pull_request) Successful in 22s
CI / build (pull_request) Successful in 33s
Required
Details
CI / helm (pull_request) Successful in 34s
CI / lint (pull_request) Successful in 36s
Required
Details
CI / quality (pull_request) Successful in 1m14s
Required
Details
CI / typecheck (pull_request) Successful in 1m18s
Required
Details
CI / security (pull_request) Successful in 1m19s
Required
Details
CI / integration_tests (pull_request) Failing after 2m54s
Required
Details
CI / unit_tests (pull_request) Successful in 4m46s
Required
Details
CI / docker (pull_request) Successful in 1m36s
Required
Details
CI / coverage (pull_request) Successful in 11m13s
Required
Details
CI / status-check (pull_request) Failing after 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 feature/security-fix-relpath-pr-11217:feature/security-fix-relpath-pr-11217
git switch feature/security-fix-relpath-pr-11217
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!11242
No description provided.