fix(mcp): contain policy_boundary_check scan roots to the source boundary (legis-0186c23a2c)#22
Open
tachyon-beep wants to merge 4 commits into
Open
fix(mcp): contain policy_boundary_check scan roots to the source boundary (legis-0186c23a2c)#22tachyon-beep wants to merge 4 commits into
tachyon-beep wants to merge 4 commits into
Conversation
- Resolves candidate paths and fails closed (InvalidArgumentError) when any escapes the anchor; collapses .. and symlinks via resolve() - Deferred service.errors import to avoid a policy<->service cycle - Toward legis-0186c23a2c Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dary Closes legis-0186c23a2c. An MCP caller could pass an absolute or ..-bearing root/repo_root and make Legis walk/parse arbitrary Python trees visible to the server process. The tool now calls assert_within_boundary(source_root, ...) before any filesystem walk; an out-of-bounds root fails closed to an INVALID_ARGUMENT error envelope (never a scanned result / PASS). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The existing repo_root reject-test exercised containment only incidentally (its default root, repo_root/src, was also out of bounds), so dropping repo_root from assert_within_boundary would have left every MCP test green. Add a discriminating case (repo_root out-of-bounds, root explicitly in-bounds) + the symmetric scanned_root-absence assert; mutation-proven to fail if repo_root containment is removed. Final-review Important finding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Plan for legis-0186c23a2c executed in this branch (subagent-driven TDD). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes legis-0186c23a2c (Codex Security finding) — the last open north-star governance-honesty P2. The MCP
policy_boundary_checktool accepted caller-supplied scan roots (root/repo_root) that were absolute or..-bearing and outside the server's configured source boundary, then walked them — letting an MCP caller make Legis read/parse arbitrary Python trees visible to the server process (path/parse-result leakage, expensive traversal).The fix
assert_within_boundary(anchor, *candidates)— a pure containment primitive in the policy truth layer (src/legis/policy/boundary_scan.py). Itresolve()s each candidate (collapsing..and symlinks) and raisesInvalidArgumentErrorif any escapes the resolved anchor. Deferredservice.errorsimport avoids a policy↔service cycle._tool_policy_boundary_check(src/legis/mcp.py) now calls it withanchor=source_rootbefore any filesystem walk. An out-of-bounds root fails closed to anINVALID_ARGUMENTerror envelope (isError: True) — never a result, never PASS/FINDINGS/NO_ROOT. The resultoutcomeenum is unchanged.Scope
policy-boundary-checkis operator-invoked with local filesystem authority (it is the CI self-scan gate); containment there adds no security and would break legitimate operator runs, so it is deliberately out of scope. No HTTP route exists.NO_ROOT(unchanged) — only out-of-bounds roots error.Verification
../symlink-as-root; checks every candidate) + 5 MCP tests (3 reject vectors with fail-closed leak checks, 1 in-bounds-absolute regression guard, 1 discriminatingrepo_root-out/root-in case).repo_rootcontainment is mutation-proven: droppingrepo_rootfrom the containment call makes the discriminating test fail (it scans instead of erroring).main: ruff, mypy (78 files), full suite 1275 passed / 9 skipped @ 92.04% coverage (floor 88), all 7 per-package floors, SEI conformance oracle, the governance self-scan (legis policy-boundary-check --root src --repo-root .→ PASS), and governance-gate.North-star impact: open confirmed governance-honesty defects 1 → 0.
🤖 Generated with Claude Code