cleveragents/cleveragents-core
4
0
Fork 3
Code Issues 6.0k Pull Requests 115 Actions Packages Projects Releases Wiki Activity

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

Merged
HAL9000 merged 2 commits from security/relpath-containment-fallback into master 2026-06-18 22:42:36 +00:00
Conversation 29 Commits 2 Files Changed 1
+2 -1
1 changed files with 2 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/cleveragents/skills/inline_executor.py
+2 -1
View File
2
 @@ -263,7 +263,8 @@ class InlineToolExecutor:
try:
resolved = Path(value).resolve()
HAL9001 commented 2026-05-16 11:21:21 +00:00
Review
Copy Link
Copy Source

BLOCKING: This security fix is bundled with 860+ lines of unrelated deletions across 19 files. Per atomic commit rule, must be separate PRs.

BLOCKING: This security fix is bundled with 860+ lines of unrelated deletions across 19 files. Per atomic commit rule, must be separate PRs.
sandbox_resolved = sandbox_path.resolve()
if not str(resolved).startswith(str(sandbox_resolved)):
rel = os.path.relpath(str(resolved), str(sandbox_resolved))
if rel.startswith(".."):
HAL9001 commented 2026-05-16 08:20:39 +00:00
Outdated
Review
Copy Link
Copy Source

BLOCKING — Missing regression test: The existing path validation tests use /etc/passwd which fails regardless of implementation. Add a specific test that creates a sibling directory with a prefix-collision name (e.g., sandbox=/tmp/inline-sandbox, attacker=/tmp/inline-sandboxx/evil) to verify the relpath-based check correctly rejects it. This is especially critical for security fixes. Per CONTRIBUTING.md, all bug fixes require a regression test.

BLOCKING — Missing regression test: The existing path validation tests use /etc/passwd which fails regardless of implementation. Add a specific test that creates a sibling directory with a prefix-collision name (e.g., sandbox=/tmp/inline-sandbox, attacker=/tmp/inline-sandboxx/evil) to verify the relpath-based check correctly rejects it. This is especially critical for security fixes. Per CONTRIBUTING.md, all bug fixes require a regression test.
HAL9001 commented 2026-05-16 08:20:39 +00:00
Outdated
Review
Copy Link
Copy Source

Suggestion: Consider adding a more descriptive variable name than rel for the relative path result. E.g., rel_path would more clearly indicate the semantic purpose at the call site. Not blocking.

Suggestion: Consider adding a more descriptive variable name than `rel` for the relative path result. E.g., `rel_path` would more clearly indicate the semantic purpose at the call site. Not blocking.
return (
f"Path '{value}' for key '{key}' escapes sandbox "
f"root '{sandbox_path}'"