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

Closed
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 -->
Author
Owner

event occurred 2026-05-28T04:07:37.990738+00:00

📋 Estimate: tier 1.

3-file security fix replacing startswith with proper path containment (relpath/is_relative_to/relative_to). CI is failing with specific actor-related test regressions in both unit and integration gates — Unknown Actor Name Error and Actor App Unknown Name Error — indicating the path containment changes broke actor name/path resolution logic. Not flaky CI; the implementer must diagnose the regression across the actor subsystem, fix without undoing the security improvement, and validate across both test suites. Cross-file context and actor path resolution understanding required.

*event occurred 2026-05-28T04:07:37.990738+00:00* **📋 Estimate: tier 1.** 3-file security fix replacing startswith with proper path containment (relpath/is_relative_to/relative_to). CI is failing with specific actor-related test regressions in both unit and integration gates — Unknown Actor Name Error and Actor App Unknown Name Error — indicating the path containment changes broke actor name/path resolution logic. Not flaky CI; the implementer must diagnose the regression across the actor subsystem, fix without undoing the security improvement, and validate across both test suites. Cross-file context and actor path resolution understanding required. <!-- controller:fingerprint:a03b71092739c97b -->
Author
Owner

(attempt #3, tier 1)

event occurred 2026-05-28T04:12:01.850398+00:00

🔧 Implementer attempt — rebased.

Pushed 1 commit: 9d0c585.

_(attempt #3, tier 1)_ *event occurred 2026-05-28T04:12:01.850398+00:00* **🔧 Implementer attempt — `rebased`.** Pushed 1 commit: `9d0c585`. <!-- controller:fingerprint:19814f3a0f3eda93 -->
Author
Owner

(attempt #4, tier 1)

event occurred 2026-05-28T04:28:00.712692+00:00

🔧 Implementer attempt — ci-not-ready.

_(attempt #4, tier 1)_ *event occurred 2026-05-28T04:28:00.712692+00:00* **🔧 Implementer attempt — `ci-not-ready`.** <!-- controller:fingerprint:55f774579ce443f6 -->
Author
Owner

(attempt #8, tier 1)

event occurred 2026-05-28T10:42:21.506801+00:00

🔧 Implementer attempt — resolved.

Pushed 2 commits: ba1b2e2, 0aff794.

Files touched: src/cleveragents/application/services/decomposition_clustering.py, src/cleveragents/skills/inline_executor.py.

_(attempt #8, tier 1)_ *event occurred 2026-05-28T10:42:21.506801+00:00* **🔧 Implementer attempt — `resolved`.** Pushed 2 commits: `ba1b2e2`, `0aff794`. Files touched: `src/cleveragents/application/services/decomposition_clustering.py`, `src/cleveragents/skills/inline_executor.py`. <!-- controller:fingerprint:af7c5016e5b88ab1 -->
Author
Owner

(attempt #9, tier 2)

event occurred 2026-05-28T11:11:30.953087+00:00

🔧 Implementer attempt — blocked.

Blockers:

  • CI integration_tests reports 'Plan Use And Execute With Custom Actor' (robot/m2_e2e_verification.robot) FAILED on head 0aff794f, but the same test passes locally on the exact head SHA. Verified twice: (1) standalone -t 'Plan Use And Execute With Custom Actor' on robot/m2_e2e_verification.robot — PASS in 51s; (2) full pabot --processes 64 m2 suite + entire robot/ directory — 2014 tests, 2014 passed, this test PASSED in 98.7s. No Traceback or unhandled INTERNAL error is visible in the CI log excerpt; only the Robot framework FAIL summary line. The test exercises 'agents plan use' + 'agents plan execute' for a plan in strategize/queued state expecting a controlled 'not ready' rejection. The 3 files in this PR's diff (decomposition_clustering.py, plan.py, inline_executor.py) implement a relpath-containment security fix and are not exercised by this test path (plan is rejected before sandbox apply runs; decomposition_clustering._directory_key is not on the plan-use/execute path). No reproducible local failure to root-cause, and noop is not satisfiable because CI overall_state=failure. Suspected CI runner flake (act/docker environment, parallel-test contention, or external runner reaper). Recommend retrigger or inspect run 21299 job 5 raw log beyond the FAIL summary line for actual failure detail.
_(attempt #9, tier 2)_ *event occurred 2026-05-28T11:11:30.953087+00:00* **🔧 Implementer attempt — `blocked`.** Blockers: - CI integration_tests reports 'Plan Use And Execute With Custom Actor' (robot/m2_e2e_verification.robot) FAILED on head 0aff794f, but the same test passes locally on the exact head SHA. Verified twice: (1) standalone -t 'Plan Use And Execute With Custom Actor' on robot/m2_e2e_verification.robot — PASS in 51s; (2) full pabot --processes 64 m2 suite + entire robot/ directory — 2014 tests, 2014 passed, this test PASSED in 98.7s. No Traceback or unhandled INTERNAL error is visible in the CI log excerpt; only the Robot framework FAIL summary line. The test exercises 'agents plan use' + 'agents plan execute' for a plan in strategize/queued state expecting a controlled 'not ready' rejection. The 3 files in this PR's diff (decomposition_clustering.py, plan.py, inline_executor.py) implement a relpath-containment security fix and are not exercised by this test path (plan is rejected before sandbox apply runs; decomposition_clustering._directory_key is not on the plan-use/execute path). No reproducible local failure to root-cause, and noop is not satisfiable because CI overall_state=failure. Suspected CI runner flake (act/docker environment, parallel-test contention, or external runner reaper). Recommend retrigger or inspect run 21299 job 5 raw log beyond the FAIL summary line for actual failure detail. <!-- controller:fingerprint:86a44b06fc4c4637 -->
Author
Owner

Closing as redundant: the relpath-containment security fix (issue #7478) has already merged; issue #7478 is closed.

Automated canonical-selection cleanup of the controller's duplicate deferred queue.

Closing as redundant: the relpath-containment security fix (issue #7478) has already merged; issue #7478 is closed. _Automated canonical-selection cleanup of the controller's duplicate deferred queue._
HAL9000 2026-06-18 12:02:05 +00:00
  • closed this pull request
  • removed the
    State
    Paused
    label
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

Pull request closed

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.