Skip to content

Fail the merge gate and quarantine on a write-root escape #443

Description

@hadamrd

Problem

The merge gate (runner/merge_gate.py) today refuses a PR for a closed source issue (apply_issue_closed_gate, #65) and for a weak mutation oracle (apply_mutation_survivor_gate, #381). It does not check whether the worker's diff stayed inside the filesystem sandbox it was leased.

We lease each worker a CapabilityPolicy whose filesystem.write_roots scopes Write/Edit to its worktree (worker_worktree.py:_fs_allow_entries). But that lease is enforced only at the Claude settings layer — and Bash(*) is granted whenever any write root exists, because Bash cannot be path-scoped in settings. So a worker can bash-write a file outside its worktree, and on an otherwise-green run that escape sails through the merge gate into mergeable state.

Concrete example: worker for issue #X is leased write_roots=("/tmp/forge-x/wt-loop-512",). Its agent runs echo pwned >> /home/u/forge-loop/src/forge_loop/runner/tick.py. CI is green, critic approves, the merge gate never inspects the diff's paths — auto-merge fires. A Bash escape just advanced into durable cognition.

Acceptance criteria

  • A pure function exists that, given a worker's collected changed paths and the leased write_roots, returns the non-empty tuple of paths that fall outside every write root (empty tuple for a clean diff). Path comparison is normalized (abs paths, trailing-slash insensitive), mirroring worktree_sweep's normalization.
  • A merge-gate enforcer step, wired into runner/merge_gate.py following the existing apply_*_gate shape, feeds the worker's changed paths into that function against the saga's leased CapabilityPolicy.
  • When the violation tuple is non-empty: the gate FAILS (branch never advances to merged — disable_pr_auto_merge, status flipped mergedopen like the other gates), the worktree is moved to quarantine by reusing existing primitives (worker_worktree.quarantine_if_blocking and/or tasks/store.mark_quarantined), a violation-naming PR comment is posted, and a distinct merge_refused_write_root_escape (name TBD) event is emitted with the offending paths.
  • When the violation tuple is empty (diff entirely within write_roots), the gate is a pass-through: current merge behavior is byte-for-byte unchanged.
  • The gate is injectable/testable: changed-path collection goes through a seam (a "git runner" callable or equivalent) so a test can drive it with a fake — no real git, no real subprocess.
  • No new quarantine machinery is introduced; the change reuses what already exists.

Test matrix

Unit (pure violation function), in tests/:

  • in-bounds path → empty tuple (pass).
  • out-of-bounds path → tuple contains exactly that path.
  • normalization: a path equal-but-for-trailing-slash / relative-vs-abs to a write root is in-bounds (no false positive).
  • empty write_roots (no lease) → assert the chosen fail-safe stance (recommend: any changed path is a violation, i.e. closed-by-default), matching the gate's "conservative on uncertainty" convention.

Integration (the gate), in tests/test_runner_merge_gate.py:

  • Happy path: fake git runner reports a diff entirely under a write_roots=(worktree,) policy → gate passes, auto-merge NOT disabled, status stays merged, no quarantine call, no event.
  • Adversarial / sad path: fake git runner reports a path outside the worktree → gate refuses (auto-merge disabled, status mergedopen), quarantine_* primitive invoked on the worktree, merge_refused_write_root_escape event emitted carrying the offending path, PR comment posted.
  • Best-effort side effects: a raising pr_comment / disable_pr_auto_merge does NOT prevent the event + status flip from firing (match apply_mutation_survivor_gate's contextlib.suppress pattern).

Out of scope

File pointers

  • src/forge_loop/runner/merge_gate.py — add the new gate function modeled on apply_mutation_survivor_gate (Protocol slice for the gh client, check_* per-outcome + apply_* over the list, event + status flip).
  • src/forge_loop/sandbox/policy.pyFilesystemScope.write_roots lives here; strong candidate home for the pure write_root_violations(changed_paths, write_roots) helper.
  • src/forge_loop/worker_worktree.py — reuse quarantine_if_blocking(wt: Path) (line ~196).
  • src/forge_loop/tasks/store.py — reuse mark_quarantined(task_id, *, reason) (line ~568); tasks/saga.py has TaskState.QUARANTINED.
  • src/forge_loop/runner/tick.py_run_merge_gate (line ~1072) is the production wiring site; thread the worktree + leased policy in alongside the existing gate calls.
  • src/forge_loop/worker.pyWorkerOutcome (line ~52) does NOT currently carry the worktree path or leased policy; (investigate) how the gate obtains the saga's leased CapabilityPolicy and worktree for a given outcome (thread one field through, or look it up from the saga store by issue/task id).
  • Changed-path collection (worker_changed_paths-style helper): (investigate) whether a git-diff path collector already exists; if not, add one behind an injectable seam so the test can supply a fake git runner.
  • tests/test_runner_merge_gate.py — add the integration tests next to the existing gate tests.

Worker note

AC is wide — it spans merge_gate.py, policy.py, the quarantine primitives in two modules, the tick.py wiring, and a new diff-collection seam, with an open (investigate) on how the gate reaches the leased policy. You are at high risk of running out of turns before pushing. Apply COMMIT DISCIPLINE (wip-commit every ~20 turns / ~5 file-edits) aggressively. Land the pure write_root_violations function + its unit tests FIRST (self-contained), commit, then build the gate and wiring. Run the EXIT CHECKLIST even if the wiring feels incomplete.

Original report

Part of epic: "Enforce leased filesystem write-roots against the worker's actual diff". An enforcer step wired into runner/merge_gate.py feeds worker_changed_paths into write_root_violations against the saga's leased CapabilityPolicy; non-empty violations FAIL the gate (branch never merges) and quarantine the worktree by reusing the existing primitive in tasks/store.py + worker_worktree.py — no new machinery. A clean diff leaves merge behavior unchanged. Test drives the gate with a fake git runner + a policy whose write_roots is the worktree, asserting block+quarantine out-of-bounds and pass-through in-bounds. ~140 net LOC incl. tests.

Customer story: A maintainer deciding whether an autonomous patch is semantically trustworthy needs the merge gate to reject and quarantine a patch that wrote outside the worker's leased sandbox, so a Bash escape cannot advance into mergeable state on a green run.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions