From 9cc27619981ae1e76ad8bf9c8d99435bbbdcf809 Mon Sep 17 00:00:00 2001 From: Rohit P Date: Fri, 24 Jul 2026 17:42:17 -0700 Subject: [PATCH] security: tighten permission handling --- coworker/agent.py | 5 +++- coworker/config.py | 49 ++++++++++++++-------------------- tests/test_config.py | 44 +++++++++++++++++++++++++++++- tests/test_permissions_risk.py | 14 +++++++--- 4 files changed, 78 insertions(+), 34 deletions(-) diff --git a/coworker/agent.py b/coworker/agent.py index d0f82a0e..456a0158 100644 --- a/coworker/agent.py +++ b/coworker/agent.py @@ -269,7 +269,10 @@ def build_engine( permissions = PermissionEngine( workspace_root=ws or (root_list[0].path if root_list else Path.cwd()), mode=mode, - allowed_commands=allowed_commands or config.allowed_commands, + # `[]` is an explicit deny-by-default override, not a request to fall back to config. + allowed_commands=( + allowed_commands if allowed_commands is not None else config.allowed_commands + ), auto_allow_tools=set(config.auto_allow), roots=root_list or None, risk_overrides=risk_overrides, diff --git a/coworker/config.py b/coworker/config.py index abfe57a6..386337ca 100644 --- a/coworker/config.py +++ b/coworker/config.py @@ -2,6 +2,9 @@ Global: /config.toml (see `secrets.state_dir`; platform-native) Workspace: /.coworker/config.toml (overrides global) + +Permission grants are deliberately global-only. A checked-out repository is untrusted +input and must not be able to grant its own shell commands or consequential tools. """ from __future__ import annotations @@ -13,28 +16,12 @@ from typing import Any, Optional from .secrets import state_dir -# Commands auto-run WITHOUT an approval prompt. Deliberately limited to read-only -# inspection: language interpreters and package managers (python/node/npm/npx) are NOT -# here — allowlisting an interpreter allowlists arbitrary code (`python3 -c "..."`), which -# defeats the point of approval gating. Users can still add them via config if they accept -# that trade-off. Matching is exact-argv-prefix and rejects shell operators — see -# PermissionEngine._command_allowed. -DEFAULT_ALLOWED_COMMANDS = [ - "ls", - "cat", - "pwd", - "echo", - "head", - "tail", - "grep", - "find", - "wc", - "git status", - "git diff", - "git log", - "git show", - "pytest", -] +# Commands auto-run WITHOUT an approval prompt. There is no generally safe executable: +# nominally read-only programs can read secrets outside the workspace, expand environment +# variables, load project-controlled config/plugins, or execute helpers (for example +# `find -exec` and pytest collection). Keep the built-in list empty. A user may explicitly +# opt into command prefixes in their user-owned global config, accepting that authority. +DEFAULT_ALLOWED_COMMANDS: list[str] = [] @dataclass @@ -87,6 +74,11 @@ _FIELDS = { "cloud_relay_ws_url", } +# These fields change what consequential actions can run without a prompt. Only the +# user-owned global config may set them; a repository's `.coworker/config.toml` may not. +_GLOBAL_ONLY_FIELDS = {"allowed_commands", "auto_allow"} +_WORKSPACE_FIELDS = _FIELDS - _GLOBAL_ONLY_FIELDS + def global_config_path() -> Path: return state_dir() / "config.toml" @@ -104,17 +96,16 @@ def load_config( workspace: Optional[str | Path] = None, *, global_path: Optional[Path] = None ) -> Config: cfg = Config() - data: dict[str, Any] = {} g = Path(global_path) if global_path is not None else global_config_path() if g.is_file(): - data.update(_read(g)) + for key, value in _read(g).items(): + if key in _FIELDS: + setattr(cfg, key, value) if workspace: w = Path(workspace).expanduser() / ".coworker" / "config.toml" if w.is_file(): - data.update(_read(w)) - - for key, value in data.items(): - if key in _FIELDS: - setattr(cfg, key, value) + for key, value in _read(w).items(): + if key in _WORKSPACE_FIELDS: + setattr(cfg, key, value) return cfg diff --git a/tests/test_config.py b/tests/test_config.py index 27648037..9070f690 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -12,7 +12,7 @@ def test_defaults_when_no_files(tmp_path): assert cfg.model == "gpt-5.6-sol" assert cfg.mode == "interactive" assert cfg.max_iterations == 150 - assert "pytest" in cfg.allowed_commands + assert cfg.allowed_commands == [] def test_global_and_workspace_override(tmp_path): @@ -31,6 +31,48 @@ def test_global_and_workspace_override(tmp_path): assert cfg.mode == "plan" # from workspace +def test_workspace_cannot_grant_its_own_permissions(tmp_path): + g = tmp_path / "global.toml" + g.write_text( + 'allowed_commands = ["git status"]\nauto_allow = ["write_file"]\n' + ) + ws = tmp_path / "ws" + (ws / ".coworker").mkdir(parents=True) + (ws / ".coworker" / "config.toml").write_text( + 'allowed_commands = ["python3"]\nauto_allow = ["run_shell"]\n' + ) + + cfg = load_config(ws, global_path=g) + assert cfg.allowed_commands == ["git status"] + assert cfg.auto_allow == ["write_file"] + + +def test_build_engine_honors_explicit_empty_command_allowlist(tmp_path): + from coworker.agent import build_code_engine + from coworker.config import global_config_path + + global_config_path().parent.mkdir(parents=True) + global_config_path().write_text('allowed_commands = ["pytest"]\n') + + class _Stub: + def complete(self, **k): # pragma: no cover + raise NotImplementedError + + def capabilities(self, m): # pragma: no cover + raise NotImplementedError + + engine = build_code_engine( + workspace=tmp_path, provider=_Stub(), allowed_commands=[] + ) + try: + decision = engine.permissions.evaluate( + "run_shell", {"command": "pytest -q"}, None + ) + assert not decision.allowed and decision.needs_user + finally: + engine.executor.close() + + def test_build_engine_respects_max_iterations(tmp_path): (tmp_path / ".coworker").mkdir() (tmp_path / ".coworker" / "config.toml").write_text("max_iterations = 3\n") diff --git a/tests/test_permissions_risk.py b/tests/test_permissions_risk.py index 98450eb0..c41e1141 100644 --- a/tests/test_permissions_risk.py +++ b/tests/test_permissions_risk.py @@ -131,13 +131,21 @@ def test_allowlist_prefix_is_argv_boundary(tmp_path): assert eng.evaluate("run_shell", {"command": "lsof"}, None).needs_user -def test_interpreters_not_auto_allowed_by_default(tmp_path): - # The default allowlist must not auto-run interpreters (arbitrary code execution). +def test_shell_commands_not_auto_allowed_by_default(tmp_path): + # There is no generally safe executable: these examples cover code execution, + # environment disclosure, reads outside the workspace, and helper execution. from coworker.config import DEFAULT_ALLOWED_COMMANDS eng = PermissionEngine( workspace_root=tmp_path, allowed_commands=list(DEFAULT_ALLOWED_COMMANDS) ) - for cmd in ("python3 -c 'import os'", "node -e 1", "npm run x", "npx foo"): + for cmd in ( + "python3 -c 'import os'", + "pytest /tmp/attacker_test.py", + "find . -exec sh -c 'echo arbitrary' {} +", + "cat ~/.config/coworker/secrets.json", + "echo $OPENAI_API_KEY", + "git status", + ): d = eng.evaluate("run_shell", {"command": cmd}, None) assert not d.allowed and d.needs_user, cmd