DOPS-1172: isolate scan evidence from remediation

[AlienResidents]

Summary

• add --read-only-local-targets so local evidence is mounted read-only in bind-mount-capable sandboxes
• add --workspace-mount for a separate writable remediation tree that grants no assessment scope
• persist both contracts across fresh and resumed runs, rejecting conflicting resume overrides
• reject overlapping host paths by filesystem identity, including aliases on case-insensitive filesystems
• fail closed when a sandbox backend cannot enforce read-only local targets
• avoid duplicate container mount destinations and document the workflow

Verification

• uv run pytest -q tests/test_hard_containment.py tests/test_inputs.py tests/test_session_entries.py tests/test_local_sources.py tests/test_cli_target_list.py tests/test_workspace_files.py tests/test_api_spec_targets.py tests/test_tui_backend_controller.py tests/test_go_tui_runtime.py -- 224 passed against upstream main 5d015df
• targeted Ruff format/lint -- passed
• targeted mypy -- passed
• local launcher containment harness -- 22 checks passed
• installed fork smoke -- read-only evidence, writable workspace, and scope isolation passed

DOPS-1172

👽 Directed by Chrispy chris@akuru.com.au
Authored by LLM gpt-5.6-sol -- Ranger

[AlienResidents]

👀 Claude Code review in progress (re-review -- prior review at stale 8594b7d is outdated; re-anchoring to current HEAD)

Running against 1c17472:

• Reading the 13 changed files at the new SHA + diff
• Re-review scoping (Retro A2): carrying forward prior findings, surfacing only consequences of new commits
• Cross-referencing JIRA (DOPS-1172)
• Validating reference URLs with WebFetch during drafting

ETA ~5-8 minutes depending on diff size and how many findings need teaching sections. I'll follow up with the full review.

_{Posted by pr-watcher pre-flight}

[greptile-apps]

Greptile Summary

The PR separates immutable local scan evidence from a writable, out-of-scope remediation workspace and preserves those contracts across fresh and resumed runs.

• Adds CLI options for read-only local targets and a distinct writable workspace mount.
• Rejects overlapping evidence and remediation paths and prevents duplicate container destinations.
• Fails closed when the selected sandbox cannot enforce read-only bind mounts.
• Updates task framing, TUI propagation, persistence, documentation, and containment tests.

Confidence Score: 5/5

The PR appears safe to merge; no concrete changed-code defect remains.

The containment flag reaches bind-mount permissions, unsupported backends fail closed, writable remediation paths remain outside assessment scope, and overlap and destination-collision guards cover the introduced mount combinations.

Important Files Changed

Filename	Overview
strix/interface/scan_setup.py	Propagates read-only target metadata, rejects filesystem-identity overlap, assigns collision-free workspace destinations, and persists the containment option.
strix/runtime/session_manager.py	Applies per-source read-only bind flags and rejects containment requests on backends that cannot enforce them.
strix/interface/cli_args.py	Adds and validates the new CLI options and restores containment state and workspace mounts during resume.
strix/core/inputs.py	Clearly distinguishes immutable evidence, writable remediation space, and authorized assessment scope in the root task.
strix/interface/tui/backend/controller.py	Carries explicitly supplied workspace mounts through interactive startup without invoking the unrelated current-directory consent flow.
strix/interface/tui/runtime.py	Includes the read-only target contract in TUI scan configuration while preserving workspace state during setup.
tests/test_hard_containment.py	Covers flag propagation, backend enforcement, mount permissions, overlap rejection, destination collisions, prompt framing, and scope isolation.

_{Reviews (1): Last reviewed commit: "fix(containment): isolate evidence from ..." | Re-trigger Greptile}

[AlienResidents]

Code Review -- DOPS-1172: isolate scan evidence from remediation

Re-review. A mid-review push landed commit 8594b7d ("fix(containment): reject resume overrides") on top of the prior anchor a1fdb98. I re-anchored the worktree to 8594b7d, re-ran the test subset (224 passed, +2 from the new parametrized test), and re-ran ruff/mypy on the newly-changed file. Per the Retro A2 re-review scoping rule, I carry forward the prior findings (about files the new commit did not touch) and introduce new findings only for consequences of 8594b7d.

Anchor SHA: 8594b7d (re-anchored from a1fdb98 after the mid-review push; verified in worktree)
Prior review SHA: a1fdb98
Base: main (review comparison base: main -- branch was 0 commits behind, up-to-date)
Files reviewed: 12 (prior) + 2 re-read at new SHA (strix/interface/cli_args.py, tests/test_cli_target_list.py)
Verification: uv run pytest -q (full suite at a1fdb98) = 1703 passed; the PR's claimed subset at 8594b7d = 224 passed (was 222; +2 from the new parametrized test). ruff check + ruff format --check clean. mypy clean on all 8 changed source files.

Severity	Count
Critical	0
Warning	0
Suggestion	2
Nitpick	2

Verdict: Comment -- 0 Critical, 0 Warning, 2 Suggestion, 2 Nitpick. (Per the Step 6.5 matrix, 0 Critical + 0 Warning + Suggestion >= 1 defaults to Comment. "Approve with Warnings" is never valid; here there are no Warnings to downgrade.)

---

Summary

This is a well-executed hard-containment slice. The contract is coherent end-to-end:

• --read-only-local-targets stamps details.read_only = True on every local_code target at build_targets_info time (strix/interface/scan_setup.py:128-130), and that flag flows through collect_local_sources (strix/interface/utils.py:1320-1324) into build_bind_mounts (strix/runtime/session_manager.py:63-75), which now sets read_only: bool(src.get("read_only", False)) on the bind mount.
• --workspace-mount is validated through the same check_mountable_dir guard as targets (strix/interface/cli_args.py:359-363), attached as a non-target writable source, and given a deduplicated container subdir (scan_setup.py:243-258).
• The fail-closed gate at session_manager.py:303-307 raises RuntimeError when a manifest-only backend is asked to honour read_only it cannot enforce -- correct and tested (test_read_only_target_rejects_manifest_only_backend).
• The overlap rejection (_reject_workspace_overlap, scan_setup.py:216-234) checks both directions (workspace-inside-evidence and evidence-inside-workspace) using Path.samefile with an OSError fallback to string equality -- I verified at runtime that samefile raises OSError on a missing path, so the fallback is sound and handles case-insensitive-filesystem aliases as the ticket requires.
• Resume persistence: read_only_local_targets is written to run.json (scan_setup.py:311-314) and restored (cli_args.py:460-463); the per-target details.read_only survives in targets_info itself. Tested by test_resume_rejects_overlapping_workspace_without_traceback.
• Root-task framing (_render_local_code_target, strix/core/inputs.py:108-124) correctly distinguishes read-only evidence ("immutable evidence and must not be modified") from writable targets, and the working-directory block no longer claims "No scan target was set" when targets exist.
• The TUI controller (controller.py:97) picks up a CLI-provided workspace_mount at construction and skips the live-view confirmation prompt when one is present -- correct, because a CLI-supplied path is already user-confirmed. Tested by test_target_less_start_uses_explicit_cli_workspace_without_prompt.

The metadata mounts (.git/.agents/.codex) in _metadata_mounts (session_manager.py:239-255) are hardcoded read_only: True regardless of the source's read_only flag, so a read-only evidence tree gets read-only metadata mounts too -- no containment gap there. Coherent.

New commit 8594b7d ("reject resume overrides"): adds a guard at cli_args.py:379-383 that rejects --resume combined with a fresh --workspace-mount or --read-only-local-targets flag, instead of silently overriding the fresh flag with the persisted record (which _load_resume_state does at lines 463 and 481). This is a sound tightening -- the ticket's acceptance criterion is "Fresh and resumed runs use the same containment contract," and silently accepting-then-ignoring a fresh containment flag on resume would be confusing. The guard is placed correctly (after the --target/--target-list resume check, before _load_resume_state), and the parametrized test covers both flags. The existing test_resume_restores_a_target_less_workspace_mount still passes because it relies on the persisted record without passing a fresh flag, so the new guard does not regress the resume-restore flow.

Suggestions

1. docs/usage/cli.mdx [S1 Suggestion, carry-forward from a1fdb98] -- The CLI reference is missing <ParamField> entries for --read-only-local-targets and --workspace-mount.

   The PR documents the two new flags in docs/quickstart.mdx (a usage example), but docs/usage/cli.mdx is the canonical CLI reference and systematically documents every flag with a <ParamField> block (e.g. --workspace-file at line 40). A user looking up flags in the reference won't find the new ones. Adding two <ParamField> entries (next to --workspace-file, since they're conceptually adjacent) would close the gap and match the existing documentation pattern.

2. docs/usage/cli.mdx:19 [S2 Suggestion, carry-forward from a1fdb98] -- The --target <Note> now understates the mount behaviour.

   The note currently reads: "A local directory is mounted into the sandbox live and writable, so the agent edits your real files (.git excepted). Commit or stash first." With --read-only-local-targets, a local directory can instead be mounted read-only as immutable evidence. The note should mention the read-only mode (one sentence pointing at the new flag) so the reference doesn't understate the new capability.

Nitpicks

• strix/interface/scan_setup.py:220 [N1 Nitpick, carry-forward from a1fdb98] -- _reject_workspace_overlap skips non-read-only sources via if not source.get("read_only"): continue with no inline comment. The function name and error message make the intent clear ("overlaps read-only target"), so a future reader can infer it, but a one-line comment ("# Overlap only matters for immutable evidence; a writable target overlapping a writable workspace is not a containment break") would pre-empt the "why not writable targets too?" question. Optional.

• strix/interface/cli_args.py:379-383 [N2 Nitpick] New since prior review (a1fdb98): triggered by commit 8594b7d ("reject resume overrides"). The new guard's error message names both flags -- "Cannot combine --resume with --workspace-mount or --read-only-local-targets." -- even when the user passed only one of them. A user who runs strix --resume foo --read-only-local-targets (no --workspace-mount) gets a message that mentions --workspace-mount they didn't pass. The second sentence ("Resume restores the original containment configuration") carries the rationale, and the test asserts on that sentence rather than the full message, so the assertion is robust. Optional polish: name only the actually-passed flag, or split into two message branches.

JIRA Scope Alignment

Ticket: DOPS-1172 "Prevent Strix scans from modifying source checkouts" (Task, In Progress, assignee chris@akuru.com.au) -- fetched via acli jira workitem view DOPS-1172.

This PR delivers the sandbox containment isolation slice of the ticket's scope:

• Read-only local-target mode -- --read-only-local-targets
• Separate writable remediation workspace granting no assessment scope -- --workspace-mount
• Reject evidence/workspace aliases including parent-child paths and duplicate container mount destinations -- _reject_workspace_overlap + dedup suffix loop
• Preserve the containment contract across resumed runs -- read_only_local_targets in run.json; commit 8594b7d further tightens this by rejecting fresh containment overrides on resume
• Automated tests for mount flags, overlap rejection, root-agent instructions, resume behaviour -- tests/test_hard_containment.py (411 lines, 22 tests) + test_parse_arguments_rejects_resume_with_containment_overrides

Out of scope for this PR (launcher drift detection): The ticket's scope also names "Update the local container launcher to create independent evidence and workspace clones at one pinned commit" and "Verify primary and evidence tree digests before and after each run, failing closed on drift" plus "launcher drift detection" tests. None of that launcher-side work is in this PR. The PR title ("isolate scan evidence from remediation") honestly scopes it to the sandbox containment layer, and shipping the sandbox-side containment before the launcher-side drift detection is a reasonable decomposition -- but the launcher slice remains open against the ticket's acceptance criteria. Recommend confirming with the operator whether a follow-up ticket will track the launcher work, or whether DOPS-1172 stays open until the launcher slice lands.

No scope creep, no title mismatch, no undocumented endpoint deviations.

Design-Deviation

Step 4d.1 -- n/a (no approved design linked). The ticket description and PR body reference no Confluence page, technical-designs PR, or plan markdown.

Notes

• Self-PR: author AlienResidents == reviewer login; this review received the same full treatment as a foreign-author PR (full file reads, full test suite, all step gates).
• Mid-review push handling: the author pushed 8594b7d while the review was in progress against a1fdb98. I re-anchored the worktree, re-ran tests/ruff/mypy, carried forward prior findings, and introduced only the one new Nitpick (N2) attributable to the new commit. No first-review nits were re-litigated at the prior SHA.
• Step 4f (Codex): Codex CLI not available on this host -- independent Codex review skipped (best-effort, never blocks).
• Cross-repo impact (Step 4h): n/a. This is the usestrix/strix fork (separate org from akuru-ai); no sibling repos under ~/git/github/akuru-ai/ reference strix symbols, and the changed symbols (attach_workspace_mount, build_bind_mounts, create_or_reuse, build_root_task, _render_local_code_target, _reject_workspace_overlap) are all internal to the strix codebase.
• Blast radius (Step 4i): create_or_reuse is called from strix/core/runner.py:316 (production path); the new RuntimeError for manifest-only backends propagates up and is the intended fail-closed. All other callers are test stubs. build_root_task's signature is unchanged; only rendering changed, and existing test_inputs.py / test_api_spec_targets.py tests confirm no semantic drift in the non-read-only path. The new resume guard in 8594b7d does not affect any production caller (it fires only at argument-parsing time).
• Implementation shape (Step 4j): Verified _metadata_mounts always mounts .git/.agents/.codex read-only regardless of source's read_only -- no containment gap for read-only evidence trees. Call ordering in prepare_run (targets collected before attach_workspace_mount) and _load_resume_state (same ordering) ensures _reject_workspace_overlap runs against target sources before the workspace entry is appended -- no self-overlap possible.
• Internal coherence (Step 4k): the infer_target_type return-type widening (dict[str, str] -> dict[str, Any]) is covariant and mypy-verified; all callers unpack into target_type, target_dict and treat it as a mutable dict. No stale references.
• CODEOWNERS (Step 4e): no CODEOWNERS file changed -- n/a.

👽 Directed by Chrispy chris@akuru.com.au
Agent: pr-reviewer (GLM-5.2, 203K context)
Authored by Ranger

[AlienResidents]

Thanks for the review @AlienResidents (review:5096478536) -- working through each finding now, will respond per-finding within ~30min. Will fix what's valid, refute what isn't with evidence, and escalate any genuinely ambiguous calls to Chrispy before re-requesting review.

[AlienResidents]

Thanks for the thorough re-review, @AlienResidents! Really appreciate the carry-forward discipline -- anchoring to the mid-review push, re-running the 224-test subset at the new SHA, and carrying forward the prior findings while introducing only the one new Nitpick attributable to 8594b7d. That scoping rule is exactly right.

Here's how each finding was addressed:

#	Severity	File:Line	State	Detail
S1	Suggestion	docs/usage/cli.mdx	Fixed in 1c17472	Added <ParamField path="--read-only-local-targets"> and <ParamField path="--workspace-mount"> entries next to --workspace-file, matching the existing reference pattern
S2	Suggestion	docs/usage/cli.mdx:19	Fixed in 1c17472	Updated the --target <Note> to mention read-only mode ("or use --read-only-local-targets with a separate --workspace-mount for immutable evidence") so the reference doesn't understate the new capability
N1	Nitpick	strix/interface/scan_setup.py:221	Fixed in 1c17472	Added inline comment above the if not source.get("read_only"): continue skip ("Only immutable evidence needs an alias guard. Two writable views do not weaken the containment contract.") to pre-empt the "why not writable targets too?" question
N2	Nitpick	strix/interface/cli_args.py:381	Fixed in 1c17472	Split the single two-flag guard into two if branches so each error names only the actually-passed flag; the second sentence ("Resume restores the original containment configuration") the test asserts on is preserved verbatim

CI handling

Check	Baseline?	State	Detail
strix/supply-chain	yes (green at review SHA 8594b7d)	pass on live head 1c17472	No failures; the fix commit did not regress the supply-chain check

The Baseline? column shows whether the check was failing on the review SHA (Step 2.5 baseline). Both the baseline and the live head are green -- no regressions, no fix-loop needed.

Notes

• Concurrent run: a parallel fixer spawn (gpt-5.6-sol, same operator directive) landed commit 1c17472 ("docs(containment): complete CLI reference") covering all four findings while this run was classifying. This run re-anchored to 1c17472, verified each finding against the live code, and did not push a duplicate -- the four fixes above are from 1c17472, not from this run's (discarded) local commits.
• Anchor SHA (final): 1c17472d5b781fd7766bf09797ecb929dd77c9d0 (re-anchored from the spawn SHA 8594b7d after the concurrent push).
• Iterations: this run processed 1 review (5096478536).

Let me know if any of the refutations need another pass -- happy to dig deeper if any of the evidence anchors aren't landing.

processed 1 review (5096478536)

👽 Directed by Chrispy chris@akuru.com.au
Agent: pr-fixer (GLM-5.2, 203K context)
Authored by Ranger

[AlienResidents]

Code Review -- DOPS-1172: isolate scan evidence from remediation

Re-review. The prior review at 8594b7d (a Comment verdict with 2 Suggestions + 2 Nitpicks) is stale: the branch has moved to 1c17472. Per the Step 0.4 stale-review rule, the prior review no longer blocks a fresh pass. I re-anchored the worktree to 1c17472, re-ran the PR's claimed test subset and the linters, and scoped NEW findings to consequences of the 4 commits between 8594b7d and 1c17472 (Retro A2 re-review scoping). All four prior findings are now resolved; no new findings were introduced.

Anchor SHA: 1c17472 (verified in worktree at .agent/worktrees/pr-1247)
Prior review SHA: 8594b7d (stale -- branch moved)
Base: main (review comparison base: main -- branch is up-to-date; main is an ancestor of HEAD)
Files changed since prior review (4): docs/usage/cli.mdx, strix/interface/cli_args.py, strix/interface/scan_setup.py, tests/test_cli_target_list.py
Verification: uv run pytest -q tests/test_hard_containment.py tests/test_inputs.py tests/test_session_entries.py tests/test_local_sources.py tests/test_cli_target_list.py tests/test_workspace_files.py tests/test_api_spec_targets.py tests/test_tui_backend_controller.py tests/test_go_tui_runtime.py = 224 passed (matches the PR body's claim exactly). ruff check + ruff format --check clean on cli_args.py + scan_setup.py + test_cli_target_list.py. mypy clean on both changed source files. Both parametrized cases of test_parse_arguments_rejects_resume_with_containment_overrides pass individually.

Severity	Count
Critical	0
Warning	0
Suggestion	0
Nitpick	0

Verdict: Approve -- no findings this pass. Verified against 1c17472.

---

Prior findings -- all resolved

The four commits between 8594b7d and 1c17472 resolve every outstanding item from the prior review. Carrying forward per the re-review scoping rule, each is marked resolved with the change that closed it:

1. S1 (was Suggestion) -- RESOLVED. docs/usage/cli.mdx now carries <ParamField> entries for both --read-only-local-targets (lines 47-52) and --workspace-mount (lines 54-59), placed adjacent to --workspace-file as the prior review suggested. The entries document the fail-closed backend behaviour and the path-overlap constraint. Verified at docs/usage/cli.mdx:47-59 in the worktree.

2. S2 (was Suggestion) -- RESOLVED. The --target <Note> at docs/usage/cli.mdx:19 now reads "By default, a local directory is mounted into the sandbox live and writable, so the agent edits your real files (.git excepted). Commit or stash first, or use --read-only-local-targets with a separate --workspace-mount for immutable evidence." The read-only mode is now mentioned in the reference as the prior review asked.

3. N1 (was Nitpick) -- RESOLVED. _reject_workspace_overlap at strix/interface/scan_setup.py:219-220 now carries the inline comment the prior review asked for: "# Only immutable evidence needs an alias guard. Two writable views do not weaken the containment contract." The comment pre-empts the "why not writable targets too?" question.

4. N2 (was Nitpick) -- RESOLVED. The combined resume guard at strix/interface/cli_args.py:379-388 is now split into two parser.error calls, each naming only the flag the user actually passed. A user running strix --resume foo --read-only-local-targets (no --workspace-mount) now sees an error naming only --read-only-local-targets, not both flags. The test at tests/test_cli_target_list.py:88-113 was widened to assert expected_flag in error for each parametrized branch, so the assertion is robust to the split. Verified both parametrized cases pass.

New findings since prior review (8594b7d)

None. The 4 changed files are refinements (split error, inline comment, widened test assertion, two doc entries) that close the prior findings without introducing new behaviour, new exported symbols, or new boundary moves. I adversarially checked:

• Split-guard ordering: --workspace-mount is checked first (cli_args.py:379), --read-only-local-targets second (:384). A user passing both flags hits the workspace-mount error first; both errors are accurate and name only the relevant flag. No ambiguity.
• Test assertion integrity: the expected_flag variable is set per-branch (--workspace-mount for the workspace branch, --read-only-local-targets for the read_only branch) and the error message for each branch contains exactly that flag string. Confirmed by running both parametrized cases individually.
• @pytest.mark.parametrize spelling: correct (pytest uses parametrize with a z); both cases collected and passed.

JIRA Scope Alignment

Ticket: DOPS-1172 "Prevent Strix scans from modifying source checkouts" (Task, In Progress, assignee chris@akuru.com.au) -- fetched via acli jira workitem view DOPS-1172. AKM searched in parallel ("DOPS-1172 strix containment launcher drift") -- the Foundry greybox program memory corroborates the scanner-written checkout contamination incident that motivated this ticket.

This PR delivers the sandbox containment isolation slice (slice 1) of the ticket:

• Read-only local-target mode -- --read-only-local-targets
• Separate writable remediation workspace granting no assessment scope -- --workspace-mount
• Reject evidence/workspace aliases including parent-child paths and duplicate container mount destinations -- _reject_workspace_overlap + dedup suffix loop
• Preserve the containment contract across resumed runs -- read_only_local_targets in run.json; commit 8594b7d further tightened this by rejecting fresh containment overrides on resume; this SHA's split of the guard is a UX refinement of that tightening
• Automated tests -- tests/test_hard_containment.py (411 lines) + the parametrized resume-override test

Out of scope (slice 2 -- launcher drift detection): the ticket's scope also names "Update the local container launcher to create independent evidence and workspace clones at one pinned commit," "Verify primary and evidence tree digests before and after each run, failing closed on drift," and "launcher drift detection" tests. None of that launcher-side work is in this PR. The PR title ("isolate scan evidence from remediation") honestly scopes it to the sandbox containment layer. Shipping the sandbox-side containment before the launcher-side drift detection is a reasonable decomposition -- but the launcher slice remains open against the ticket's acceptance criteria. Recommend confirming with the operator whether a follow-up ticket will track the launcher work, or whether DOPS-1172 stays open until the launcher slice lands. (Carried forward from the prior review -- still accurate; no launcher work landed in this SHA.)

No scope creep, no title mismatch, no undocumented endpoint deviations.

Design-Deviation

Step 4d.1 -- n/a (no approved design linked). The ticket description and PR body reference no Confluence page, technical-designs PR, or plan markdown. (Carried forward from the prior review -- still accurate.)

Notes

• Self-PR: author AlienResidents == reviewer login; this review received the same full treatment as a foreign-author PR (full file reads, full test subset, all step gates). The zero-findings verdict is posted as --approve per the self-PR rule, with the severity table included so the fixer's parseFindings classifies it as structured_zero_findings (non-actionable).
• Mid-review push handling: no push during this review spawn; SHA stable at 1c17472 throughout.
• Step 4f (Codex): Codex CLI not available on this host -- independent Codex review skipped (best-effort, never blocks).
• Cross-repo impact (Step 4h): n/a. This is the usestrix/strix fork (separate org from akuru-ai); grep -rl across ~/git/github/akuru-ai/ for attach_workspace_mount, build_bind_mounts, _reject_workspace_overlap, read_only_local_targets returned nothing.
• Blast radius (Step 4i): the 4 changed files do not alter any exported signature (only split one parser.error into two, added an inline comment, widened a test assertion, added docs). The prior review's blast-radius analysis for the original PR's symbols (create_or_reuse, build_bind_mounts, build_root_task, etc.) still holds.
• Implementation shape (Step 4j): the split guard's ordering (workspace-mount checked before read-only-local-targets) is correct; both errors name only the relevant flag; no edge cases introduced.
• Internal coherence (Step 4k): n/a -- the 4 changed files contain no symbol renames; the inline comment is additive only.
• Intent and architectural fit (Step 4l): n/a -- the 4 changed files are refinements of existing logic, not new behaviour or boundary moves.
• CODEOWNERS (Step 4e): no CODEOWNERS file in the repo -- n/a.

👽 Directed by Chrispy chris@akuru.com.au
Agent: pr-reviewer (GLM-5.2, 203K context)
Authored by Ranger