ext/openworker
2
0
Fork 0
mirror of https://github.com/andrewyng/openworker.git synced 2026-09-03 04:49:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

PR2: protect the permission system's own settings; never auto-approve

Browse Source 
in-project files that execute later

Two floors, both mode-independent.

1. The settings files (config.toml, risk_overrides.json, workspace_trust.json,
   unattended.json, coworker.db which holds the saved grants, secrets.json,
   inbox_routing.json) cannot be modified by any tool, in any mode. The check
   runs BEFORE mode, allowlists and every auto-approve path, and returns a hard
   refusal rather than an approvable prompt -- loosening requires editing the
   files out-of-band.

   The escalation this blocks: approve one ordinary-looking `python setup.py`,
   it quietly appends to risk_overrides.json, and every future session is more
   permissive. That happens in the DEFAULT interactive mode, so the protection
   cannot be a property of a sandbox or of any single mode.

   Covered for write tools (resolved path), patch/diff blobs (path extracted),
   and run_shell (command text). Shell matching is deliberately full-path only:
   an earlier draft also matched bare filenames, which hard-denied any command
   merely mentioning `secrets.json` -- caught by
   test_shell_commands_not_auto_allowed_by_default, which reads that path with
   `cat`. Shell is parser depth: it stops accidents and casual attempts, not a
   determined adversary. That needs the OS sandbox (OPE-14).

2. Files inside the workspace that run on a later, innocuous action --
   .git/hooks/**, .github/workflows/**, .vscode/tasks.json, .coworker/** --
   stay writable but never WITHOUT a human. Auto mode, custom-mode auto_allow
   and session "always allow" all lose to this. Otherwise accept-edits is a
   clean bypass: write .git/hooks/pre-commit, then run an innocuous git commit.

19 new tests, incl. every mode parametrized and a lookalike case
(docs/pre-commit.md stays ordinary). 167 permission-related tests green.
test_standing_approvals::test_blocked_run_does_not_stall_other_tasks is an
intermittent pre-existing scheduler timing flake (fails ~1 in 3 on the
unmodified tree).

Design of record: ocw-context/docs/reviewed-auto-mode.md Part 3.
This commit is contained in:
Devika Verma
2026-08-11 11:41:24 -07:00
parent a442e0e0b8
commit 48a498d5e2
2 changed files with 203 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coworker/permissions.py
+103
View File
@@ -27,6 +27,48 @@ def _has_shell_operators(command: str) -> bool:
return any(op in command for op in _SHELL_OPERATORS)
def protected_paths() -> list[Path]:
"""Files that govern the permission system itself. Nothing the agent does may write
these — in any mode, through any tool. The escalation this blocks is: approve one
ordinary-looking command, it quietly appends to the rule file, every future session is
more permissive. That happens in the DEFAULT interactive mode, so this cannot be a
property of a sandbox or of any one mode; it is a floor."""
from .secrets import state_dir
base = state_dir()
return [
base / "config.toml",
base / "risk_overrides.json",
base / "workspace_trust.json",
base / "unattended.json",
base / "coworker.db", # session records carry the saved "always allow" grants
base / "secrets.json",
base / "inbox_routing.json",
]
# Files INSIDE a workspace that execute on a later, innocuous-looking action. An edit here
# is a deferred command: writing `.git/hooks/pre-commit` and then running `git commit` runs
# it. They stay writable, but never WITHOUT a human — no auto-approve path may clear them.
_PROTECTED_IN_PROJECT = (
".git/hooks/",
".github/workflows/",
".gitlab-ci.yml",
".vscode/tasks.json",
".coworker/", # workspace policy + skills the agent would otherwise self-grant
)
def _is_protected_in_project(candidate: Path) -> bool:
posix = candidate.as_posix()
return any(
(f"/{marker}" in posix or posix.startswith(marker))
if marker.endswith("/")
else posix.endswith("/" + marker)
for marker in _PROTECTED_IN_PROJECT
)
def _host_of(url_or_domain: str) -> str:
"""The lowercased host of a URL, or a bare domain as-is. `''` when there's nothing
usable. Accepts both `https://docs.python.org/x` and `docs.python.org`."""
@@ -182,6 +224,19 @@ class PermissionEngine:
is_egress = risk is RiskClass.EGRESS
consequential = is_consequential(risk)
# SELF-PROTECTION FLOOR — runs before mode, allowlists and every auto-approve path,
# because the escalation it blocks happens in the DEFAULT mode. No verdict below can
# reach these files, and no human click in the flow can grant it either: loosening
# requires editing the files out-of-band.
if is_write or is_shell:
hit = self._touches_protected(tool_name, arguments, is_shell)
if hit is not None:
return Decision(
False,
f"refusing to modify OpenWorker's own settings: {hit}",
needs_user=False,
)
# Discuss / plan modes: read-only.
if self.mode in READ_ONLY_MODES and consequential:
return Decision(
@@ -191,6 +246,7 @@ class PermissionEngine:
# Path scoping for writes (all modes): every path the write touches must land in a
# writable root. A write whose path can't be located is not scoped-able, so it fails
# closed to approval rather than slipping through auto/custom unscoped.
needs_human_for_protected = False
if is_write:
paths, located = write_paths(tool_name, arguments)
if not located:
@@ -204,11 +260,24 @@ class PermissionEngine:
return Decision(
False, f"path is not in a writable directory: {path}"
)
# In-project files that run on a later action (git hooks, CI configs) may be
# edited, but never by an auto-approve path — a human must see it.
if _is_protected_in_project(self._candidate(path)):
needs_human_for_protected = True
# Non-consequential tools always run.
if not consequential:
return Decision(True, "low risk")
# A protected in-project target (git hooks, CI config) skips every auto-approve path
# below — including auto mode and the session/config allowlists — and asks.
if needs_human_for_protected:
return Decision(
False,
"this file runs automatically later — approval required",
needs_user=True,
)
# Full access.
if self.mode is Mode.AUTO:
return Decision(True, "full access")
@@ -289,6 +358,40 @@ class PermissionEngine:
continue
return False
def _touches_protected(
self, tool_name: str, arguments: dict[str, Any], is_shell: bool
) -> Optional[str]:
"""The protected settings path this call would modify, or None.
For writes we resolve the real target. For shell we can only inspect the command
text — parser depth, so it stops accidents and casual attempts, not a determined
adversary (that needs the OS sandbox). Cheap and worth having regardless.
Shell matching is on the FULL path only, never a bare filename: matching
`secrets.json` anywhere in a command would refuse unrelated work that merely
mentions the name. A command naming the real settings path is refused whether it
reads or writes — we cannot tell which from text, and the conservative direction is
the right one for these files.
"""
targets = [str(p) for p in protected_paths()]
if is_shell:
command = str(arguments.get("command", ""))
if not command:
return None
lowered = command.replace("\\", "/").lower()
for target in targets:
if target.replace("\\", "/").lower() in lowered:
return target
return None
paths, located = write_paths(tool_name, arguments)
if not located:
return None # unlocatable writes are already failed closed by the caller
resolved = {str(self._candidate(p)) for p in paths}
for target in targets:
if str(Path(target).resolve()) in resolved:
return target
return None
def _domain_allowed(self, url: str) -> bool:
"""True when the URL's host is an allowed egress destination — an exact match or a
subdomain of an allowed domain (so `docs.python.org` matches `python.org`, but
tests/test_self_protection.py
+100
View File
@@ -0,0 +1,100 @@
"""PR2 — the permission system protects its own settings, and in-project files that
execute later are never auto-approved.
The escalation being blocked: approve one ordinary-looking command, it quietly appends to
`risk_overrides.json`, and every future session is more permissive. That happens in the
DEFAULT interactive mode, so the protection must be a floor — not a property of a sandbox
or of any one mode. See `ocw-context/docs/reviewed-auto-mode.md` Part 3.
"""
from __future__ import annotations
import pytest
from coworker.permissions import Mode, PermissionEngine, protected_paths
from coworker.secrets import state_dir
ALL_MODES = [Mode.DISCUSS, Mode.PLAN, Mode.INTERACTIVE, Mode.CUSTOM, Mode.AUTO]
def _engine(tmp_path, mode=Mode.INTERACTIVE, **kw):
# The state dir is redirected per-test by the autouse fixture in conftest, so the
# protected paths point somewhere isolated.
return PermissionEngine(workspace_root=tmp_path, mode=mode, **kw)
# -- the settings files ---------------------------------------------------------
@pytest.mark.parametrize("mode", ALL_MODES)
def test_write_to_settings_blocked_in_every_mode(tmp_path, mode):
eng = _engine(tmp_path, mode)
target = str(state_dir() / "risk_overrides.json")
d = eng.evaluate("write_file", {"path": target, "content": "x"}, None)
assert not d.allowed
assert not d.needs_user, "must be a hard refusal, not an approval the user can grant"
@pytest.mark.parametrize("mode", ALL_MODES)
def test_shell_touching_settings_blocked_in_every_mode(tmp_path, mode):
eng = _engine(tmp_path, mode)
target = str(state_dir() / "risk_overrides.json")
d = eng.evaluate("run_shell", {"command": f'echo "{{}}" > "{target}"'}, None)
assert not d.allowed and not d.needs_user
def test_settings_write_beats_auto_mode_and_allowlists(tmp_path):
# Every auto-approve path must lose to the floor.
target = str(state_dir() / "config.toml")
auto = _engine(tmp_path, Mode.AUTO)
assert not auto.evaluate("write_file", {"path": target, "content": "x"}, None).allowed
custom = _engine(tmp_path, Mode.CUSTOM, auto_allow_tools={"write_file"})
assert not custom.evaluate("write_file", {"path": target, "content": "x"}, None).allowed
session = _engine(tmp_path)
session.allow_tool_for_session("write_file")
assert not session.evaluate("write_file", {"path": target, "content": "x"}, None).allowed
def test_settings_protected_via_patch_blob(tmp_path):
# The patch path is extracted from the blob, so this route is covered too.
eng = _engine(tmp_path, Mode.AUTO)
target = str(state_dir() / "workspace_trust.json")
patch = f"*** Begin Patch\n*** Update File: {target}\n@@\n-a\n+b\n*** End Patch"
assert not eng.evaluate("apply_patch", {"patch": patch}, None).allowed
def test_protected_paths_cover_the_grant_and_trust_stores():
names = {p.name for p in protected_paths()}
assert {"config.toml", "risk_overrides.json", "workspace_trust.json", "coworker.db"} <= names
# -- in-project files that run later --------------------------------------------
@pytest.mark.parametrize(
"rel",
[
".git/hooks/pre-commit",
".github/workflows/ci.yml",
".vscode/tasks.json",
".coworker/config.toml",
],
)
def test_protected_in_project_never_auto_approved(tmp_path, rel):
# Writable (inside the root) but must always reach a human, even in auto mode.
for mode in (Mode.AUTO, Mode.CUSTOM, Mode.INTERACTIVE):
eng = _engine(tmp_path, mode, auto_allow_tools={"write_file"})
eng.allow_tool_for_session("write_file")
d = eng.evaluate("write_file", {"path": rel, "content": "x"}, None)
assert not d.allowed, f"{rel} auto-approved in {mode.value}"
assert d.needs_user, f"{rel} should ask, not hard-deny, in {mode.value}"
def test_ordinary_project_file_still_auto_approves(tmp_path):
# The protection must not leak onto normal edits.
eng = _engine(tmp_path, Mode.AUTO)
assert eng.evaluate("write_file", {"path": "src/app.py", "content": "x"}, None).allowed
def test_lookalike_paths_are_not_protected(tmp_path):
# A file merely NAMED like a hook, outside the protected dirs, is ordinary.
eng = _engine(tmp_path, Mode.AUTO)
assert eng.evaluate("write_file", {"path": "docs/pre-commit.md", "content": "x"}, None).allowed