{{ message }}
fix: complete CLI --contain for all write paths#1410
Merged
Conversation
Engine-backed renames already used PathGuard via stage_for_write. The write_dispatch callback path (binary and case-only renames) did not, so agents could still escape a --cwd workspace with rename ../ under --contain. Check both from and to early for all rename modes, document full write coverage, and add unit plus integration tests. Closes #1406 Closes #1409 Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Phase 4 (#1376) closed while four domain monofiles still carried that tracker. Point waivers at open #1408, document co-located MCP tests as intentional, and accept any #NNNN in the hygiene check so the living tracker can move without hardcoding a closed issue. Ref #1408 Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
4 tasks
3 tasks
SebTardif
added a commit
that referenced
this pull request
Jul 4, 2026
## Summary Multi-perspective improvement cycle (post #1410) focused on agent-facing accuracy and residual `--contain` edge cases. - **Agent rules / PATCHLOOM.md:** stop claiming the CLI has no containment; document `patchloom --cwd <ws> --contain` for CLI sandboxes (MCP+CLI and CLI-only modes) - **concepts.md:** agent-author guidance mentions CLI `--contain` - **Tests:** delete `--contain` integration coverage; create rejects absolute paths under `--contain` (MCP `AbsolutePathPolicy::Reject` parity) - **Help text:** `--contain` documents absolute path rejection - **Hygiene:** clarify misleading `doc` write-dispatch comment ## Perspectives (cycle 12) | Perspective | Outcome | |-------------|---------| | QA | delete contain tests; agent-rules stale claim fixed | | Developer | machete clean; doc comment fix | | End User | agent-rules + concepts | | Maintainer | no unused deps | | Ops/SRE | AGENTS.md `make check` matches Makefile | | Security / Adversarial | absolute-path under contain | | Spec/Contract | `--help` wording | | Others (PM, Perf, Compat, Arch, Contributor, Observability) | no-op / verified | ## Test plan - [x] `make check-fast` - [x] `cargo test --lib --all-features documents_contain create_with_contain` - [x] `cargo test --test integration --all-features test_delete_contain` --------- Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
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
Completes optional CLI workspace containment after partial land in #1407.
renameunder--contain: earlyPathGuardcheck on bothfromandtofor every rename path, including binary/case-only callback (write_dispatch), not only engine-backed text renames--contain, including binary rename escape#NNNNissue refTest plan
cargo test --lib --all-features rename::cargo test --test integration --all-features contain(filter via names)make check-fastCloses #1406
Closes #1409
Ref #1408