diff --git a/coworker/engine.py b/coworker/engine.py index 9e07d712..3d144571 100644 --- a/coworker/engine.py +++ b/coworker/engine.py @@ -31,9 +31,20 @@ class ApprovalOutcome(str, Enum): ONCE = "once" ALWAYS_TOOL = "always_tool" ALWAYS_COMMAND = "always_command" + # Session-wide grant for classifier-approved read-only shell commands (readonly.py). + READONLY_SESSION = "readonly_session" DENY = "deny" +def _readonly_ok(arguments: dict) -> bool: + command = str((arguments or {}).get("command", "") or "") + if not command: + return False + from .readonly import is_readonly_command + + return is_readonly_command(command) + + @dataclass class PermissionRequest: tool_name: str @@ -711,6 +722,9 @@ class TurnEngine: metadata, self.permissions.risk_overrides, ), + # True when this shell command classifies as read-only — the card + # offers "Allow read-only commands for this session" only then. + "readonly_ok": _readonly_ok(tool_call.arguments), }, ) self._audit(tool_call, stage="approval_requested", reason=decision.reason) @@ -745,6 +759,8 @@ class TurnEngine: self.permissions.allow_command_for_session( str(tool_call.arguments.get("command", "")) ) + elif outcome is ApprovalOutcome.READONLY_SESSION: + self.permissions.allow_readonly_for_session() allowed, reason = True, "approved by user" self._audit( tool_call, diff --git a/coworker/permissions.py b/coworker/permissions.py index 82477f7b..597653c0 100644 --- a/coworker/permissions.py +++ b/coworker/permissions.py @@ -88,6 +88,9 @@ class PermissionEngine: auto_allow_tools: set[str] = field(default_factory=set) session_allow_tools: set[str] = field(default_factory=set) session_allow_commands: set[str] = field(default_factory=set) + # Session-wide read-only grant (owner ask 2026-08-11): auto-allow shell commands the + # conservative classifier (coworker/readonly.py) accepts. User-elected per session. + session_readonly: bool = False # Task-scoped standing rules (§25): {tool: {allowed targets}}, seeded from the owning # ScheduledTask's target-shaped entries. Kept by reference and re-read every check, so a # rule minted mid-run ("Allow every time") applies to the run's next call too. @@ -154,6 +157,11 @@ class PermissionEngine: return Decision(True, "command on allowlist") if command and command in self.session_allow_commands: return Decision(True, "command allowed for session") + if self.session_readonly and command: + from .readonly import is_readonly_command + + if is_readonly_command(command): + return Decision(True, "read-only command (session grant)") if tool_name in self.session_allow_tools and not is_connector: return Decision(True, "tool allowed for session") @@ -185,6 +193,9 @@ class PermissionEngine: if command: self.session_allow_commands.add(command) + def allow_readonly_for_session(self) -> None: + self.session_readonly = True + # -- helpers ---------------------------------------------------------------- def _candidate(self, path: str) -> Path: # Relative paths resolve against the primary (workspace_root); absolute/`~` taken as-is. diff --git a/coworker/readonly.py b/coworker/readonly.py new file mode 100644 index 00000000..c899af46 --- /dev/null +++ b/coworker/readonly.py @@ -0,0 +1,158 @@ +"""Conservative read-only shell-command classifier for the session-scoped grant. + +"Allow read-only commands for this session" (owner ask 2026-08-11, born of approval +fatigue in security-scan sessions: ~15 hand-approvals per run) auto-allows a command only +when THIS classifier accepts it. The contract: + +- **Local filesystem reads only.** Network clients (curl/wget/ssh/nc) are deliberately + excluded even for GET — an auto-allowed network command is an exfiltration channel + under prompt injection. Interpreters (python/ruby/sh -c) and anything that can write, + execute, or mutate are excluded. +- **Pipelines are allowed** (`nl … | sed -n … | grep …`) — every stage must classify. + All other shell operators (;, &&, ||, &, redirections, substitutions) are rejected + outright. +- **Fail closed.** Unknown commands, unparseable input, path-invoked binaries, and any + doubtful flag reject. False negatives cost one manual approval; false positives cost + an unreviewed side effect — the asymmetry decides every edge case here. + +This is a user-elected convenience on top of the approval flow, not a sandbox: the +session still runs under its permission mode, and the user granted the scope explicitly. +""" + +from __future__ import annotations + +import re +import shlex + +# Commands that only read local state, with no writing flags to police. +_SIMPLE_SAFE = { + "ls", "cat", "head", "tail", "wc", "nl", "sort", "uniq", "cut", "tr", + "grep", "egrep", "fgrep", "rg", "ugrep", "file", "stat", "du", "df", + "pwd", "echo", "printf", "which", "whoami", "id", "date", "uname", + "basename", "dirname", "realpath", "readlink", "jq", "column", "diff", + "comm", "strings", "md5sum", "shasum", "sha1sum", "sha256sum", + "hexdump", "xxd", "od", "true", "false", "yamllint", "actionlint", +} + +# Git subcommands that only read. Note the per-subcommand guards below — several git +# "read" commands grow write/exec behavior through specific flags. +_GIT_SAFE = { + "status", "log", "show", "diff", "blame", "shortlog", "describe", + "rev-parse", "rev-list", "ls-files", "ls-tree", "grep", "cat-file", + "name-rev", "merge-base", "count-objects", "var", "check-ignore", +} + +_GIT_BRANCH_FLAG_OK = { + "--show-current", "--list", "-a", "-r", "-v", "-vv", "--contains", + "--merged", "--no-merged", "--all", +} + +_FIND_BAD = ("-delete", "-exec", "-execdir", "-ok", "-okdir", "-fprint", "-fls", "-fprintf") + +_ENV_ASSIGN = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*=[^;&|<>`]*$") + +# A sed script token that invokes the `w`/`W` (write-file) command: at the start, after a +# separator, or after an address. Conservative — a false hit just means one manual approval. +_SED_WRITE = re.compile(r"(^|[;{])\s*[0-9,$/ ]*[wW]\s") + + +def _stages(command: str) -> list[list[str]] | None: + """Tokenize with operators surfaced; split into pipeline stages. None = reject.""" + if not command or not command.strip(): + return None + # Substitutions can hide inside double quotes, which the tokenizer strips — check the + # raw text. Rejects a literal '$(' in a grep pattern too; that asymmetry is the point. + if "`" in command or "$(" in command or "<(" in command or ">(" in command: + return None + lex = shlex.shlex(command, posix=True, punctuation_chars=True) + lex.whitespace_split = True + try: + tokens = list(lex) + except ValueError: + return None # unbalanced quotes etc. + stages: list[list[str]] = [[]] + for tok in tokens: + if tok == "|": + stages.append([]) + elif tok in {";", "&", "&&", "||", "|&"} or (tok and set(tok) <= {">", "<", "&", "0", "1", "2"} and any(c in tok for c in "<>&")): + return None # every operator except a plain pipe rejects (incl. 2>, &>, <<) + else: + stages[-1].append(tok) + if any(not s for s in stages): + return None # empty stage ("| cmd", "cmd |") + return stages + + +def _git_ok(args: list[str]) -> bool: + # Global flags: only `-C ` and `--no-pager` pass; `-c`/`--config-env` can set + # core.pager and similar exec hooks — rejected. + i = 0 + while i < len(args): + if args[i] == "-C" and i + 1 < len(args): + i += 2 + continue + if args[i] == "--no-pager": + i += 1 + continue + break + if i >= len(args): + return False + sub, rest = args[i], args[i + 1 :] + if any(t.startswith("--output") for t in rest): + return False # git log/diff --output= writes + if sub in _GIT_SAFE: + return True + if sub == "branch": + return all(t in _GIT_BRANCH_FLAG_OK or t.startswith(("--format=", "--sort=")) for t in rest) + if sub == "tag": + return bool(rest) and all( + t in {"-l", "--list", "-n", "--contains", "--merged"} or t.startswith("-n") for t in rest + ) + if sub == "stash": + return bool(rest) and rest[0] in {"list", "show"} + if sub == "remote": + return not rest or rest[0] in {"-v", "show", "get-url"} + if sub == "config": + return any(t in {"--get", "--get-all", "--get-regexp", "--list", "-l"} for t in rest) + if sub == "reflog": + return not rest or rest[0] == "show" + return False + + +def _stage_ok(argv: list[str]) -> bool: + # Leading VAR=value assignments (LC_ALL=C grep …) are inert — skip them. + i = 0 + while i < len(argv) and _ENV_ASSIGN.match(argv[i]): + i += 1 + argv = argv[i:] + if not argv: + return False + head = argv[0] + if "/" in head: + return False # path-invoked binaries can be anything; bare names only + args = argv[1:] + if head in _SIMPLE_SAFE: + return True + if head == "env": + return not args # bare `env` prints; `env CMD` executes + if head == "command": + return bool(args) and args[0] in {"-v", "-V"} + if head == "git": + return _git_ok(args) + if head == "sed": + if any(t.startswith(("-i", "--in-place", "-f", "--file")) for t in args): + return False + return not any(_SED_WRITE.search(t) for t in args if not t.startswith("-")) + if head in {"awk", "gawk", "mawk", "nawk"}: + return not any(">" in t or "system" in t for t in args) + if head == "find": + return not any(t.startswith(_FIND_BAD) for t in args) + return False + + +def is_readonly_command(command: str) -> bool: + """True iff `command` is a single command or pure pipeline of local read-only stages.""" + stages = _stages(str(command or "")) + if stages is None: + return False + return all(_stage_ok(s) for s in stages) diff --git a/coworker/server/manager.py b/coworker/server/manager.py index 70b46ad3..d1a30cd8 100644 --- a/coworker/server/manager.py +++ b/coworker/server/manager.py @@ -98,7 +98,13 @@ def _grants_of(engine) -> dict[str, Any]: """The engine's session-scoped "Always allow" approvals, in persistable shape.""" tools = sorted(getattr(engine.permissions, "session_allow_tools", None) or ()) commands = sorted(getattr(engine.permissions, "session_allow_commands", None) or ()) - return {"tools": tools, "commands": commands} if (tools or commands) else {} + readonly = bool(getattr(engine.permissions, "session_readonly", False)) + out: dict[str, Any] = {} + if tools or commands or readonly: + out = {"tools": tools, "commands": commands} + if readonly: + out["readonly"] = True + return out def _approval_body(request) -> str: @@ -3525,6 +3531,8 @@ class SessionManager: engine.permissions.allow_tool_for_session(str(tool)) for command in grants.get("commands") or []: engine.permissions.allow_command_for_session(str(command)) + if grants.get("readonly"): + engine.permissions.allow_readonly_for_session() @staticmethod def _extra_roots_of(engine: TurnEngine) -> list[dict[str, Any]]: diff --git a/surfaces/gui/e2e/approval-card.spec.ts b/surfaces/gui/e2e/approval-card.spec.ts index d6fece38..be922dda 100644 --- a/surfaces/gui/e2e/approval-card.spec.ts +++ b/surfaces/gui/e2e/approval-card.spec.ts @@ -78,3 +78,21 @@ test("a one-paragraph digest send is clamped to a card, expandable in place", as expect((await prev.boundingBox())!.height).toBeGreaterThan(clampedHeight); await expect(prev.getByText("show less")).toBeVisible(); }); + +test("read-only session grant: offered on classified commands, resolves the card", async ({ + page, +}) => { + await page.goto("/"); + const box = page.getByPlaceholder(/Ask the coworker/); + await box.fill("please run a tool"); + await page.getByRole("button", { name: "Send" }).click(); + + // The mocked `ls` proposal carries readonly_ok → the session-wide grant is offered. + const btn = page.getByTestId("allow-readonly-session"); + await expect(btn).toBeVisible(); + await expect(btn).toHaveAttribute("title", /no network, writes, or interpreters/); + await btn.click(); + + // Grant approves the pending call; the turn proceeds like any approval. + await expect(page.getByText(/The command ran; 1 file found/)).toBeVisible(); +}); diff --git a/surfaces/gui/e2e/fixtures.ts b/surfaces/gui/e2e/fixtures.ts index 2651f40b..b6dda034 100644 --- a/surfaces/gui/e2e/fixtures.ts +++ b/surfaces/gui/e2e/fixtures.ts @@ -606,6 +606,7 @@ export async function mockApi(page: import("@playwright/test").Page) { name: "run_shell", arguments: { command: "ls" }, reason: "The coworker wants to run a command.", + readonly_ok: true, // `ls` classifies read-only server-side }); return; // suspended on the approval } diff --git a/surfaces/gui/e2e/sharing.spec.ts b/surfaces/gui/e2e/sharing.spec.ts index 85d07d8f..bdc4d29a 100644 --- a/surfaces/gui/e2e/sharing.spec.ts +++ b/surfaces/gui/e2e/sharing.spec.ts @@ -50,9 +50,13 @@ test("zip import: trust warning leads, tools collapse behind a chevron, replaces await expect(card.getByTestId("replaces-note")).toContainText("MORE capabilities"); await expect(card.getByText(/github.*(recommended).*open fix PRs/)).toBeVisible(); - // Imported coworker landed disabled in the list above, pending consent. + // Imported coworker landed disabled in the list above, pending consent — + // and the card itself carries the Enable action (no hunting back up the list). const row = page.locator(".divide-y > div").filter({ hasText: "Team Security Coworker" }); await expect(row.getByRole("checkbox", { name: "Enabled" })).not.toBeChecked(); + await card.getByTestId("consent-enable-team-sec").click(); + await expect(card.getByTestId("consent-enabled")).toContainText("it's in your coworker picker"); + await expect(row.getByRole("checkbox", { name: "Enabled" })).toBeChecked(); }); test("Export… zips an installed coworker's bundle to a chosen folder", async ({ page }) => { diff --git a/surfaces/gui/src/App.tsx b/surfaces/gui/src/App.tsx index 40662266..bf11dc57 100644 --- a/surfaces/gui/src/App.tsx +++ b/surfaces/gui/src/App.tsx @@ -710,6 +710,7 @@ export function App() { reason: d.reason, category: d.category, standingTarget: d.standing_target || undefined, + readonlyOk: !!d.readonly_ok, }, ]); break; diff --git a/surfaces/gui/src/components/ApprovalCard.test.tsx b/surfaces/gui/src/components/ApprovalCard.test.tsx index 7f4570b5..2d33bff9 100644 --- a/surfaces/gui/src/components/ApprovalCard.test.tsx +++ b/surfaces/gui/src/components/ApprovalCard.test.tsx @@ -304,3 +304,33 @@ describe("InboxItemCard — parked save_skill proposals (SKILLS-SPEC §5.2)", () expect(onResolve).toHaveBeenCalledWith("i9", "deny"); }); }); + +describe("ApprovalCard — session read-only grant", () => { + const shellApproval = (extra: Partial = {}): ApprovalItem => ({ + kind: "approval", + name: "run_shell", + args: { command: "ls -la" }, + reason: "requires approval", + ...extra, + }); + + it("offers the read-only session grant only when the server classified the command read-only", () => { + const onApprove = vi.fn(); + render(); + fireEvent.click(screen.getByTestId("allow-readonly-session")); + expect(onApprove).toHaveBeenCalledWith("readonly_session"); + // The command-scoped grant stays alongside — different scopes, both legitimate. + expect(screen.getByText("Always allow this command")).toBeTruthy(); + cleanup(); + + // Not classified read-only (a write) → the button never renders. + const onApprove2 = vi.fn(); + render( + , + ); + expect(screen.queryByTestId("allow-readonly-session")).toBeNull(); + }); +}); diff --git a/surfaces/gui/src/components/ApprovalCard.tsx b/surfaces/gui/src/components/ApprovalCard.tsx index 8f2f2541..dad38494 100644 --- a/surfaces/gui/src/components/ApprovalCard.tsx +++ b/surfaces/gui/src/components/ApprovalCard.tsx @@ -210,6 +210,20 @@ function Buttons({ Always allow this command )} + {/* Session-wide read-only grant (owner ask 2026-08-11): offered only when the + server's conservative classifier accepted THIS command — one click, then every + local-read command in the session runs without a card. Network, writes, and + anything doubtful keep asking. */} + {item.name === "run_shell" && item.readonlyOk && !item.resolved && ( + + )} + )} + Recommended mode: {c.recommended_mode}. ); diff --git a/surfaces/gui/src/types.ts b/surfaces/gui/src/types.ts index 02f81276..dd3ac4da 100644 --- a/surfaces/gui/src/types.ts +++ b/surfaces/gui/src/types.ts @@ -34,7 +34,7 @@ import type { MessageSource } from "./api"; // "always_task" persists to the owning automation's task record (standing scoped // approval, UX-DECISIONS §25) — offered only on automation-run approval cards, in-app. -export type ApprovalDecision = "once" | "deny" | "always_tool" | "always_command" | "always_task"; +export type ApprovalDecision = "once" | "deny" | "always_tool" | "always_command" | "always_task" | "readonly_session"; export interface TodoItem { content: string; @@ -116,6 +116,9 @@ export type Item = // The exact target a standing rule could pin (server-computed) — with a run // context, the card offers "Allow every time" (§25). standingTarget?: string; + // Server-classified: this shell command only reads locally, so the card may offer + // the session-wide "Allow read-only commands" grant. + readonlyOk?: boolean; resolved?: ApprovalDecision; } | { diff --git a/tests/test_readonly_grant.py b/tests/test_readonly_grant.py new file mode 100644 index 00000000..67bad7e7 --- /dev/null +++ b/tests/test_readonly_grant.py @@ -0,0 +1,119 @@ +"""The session-scoped read-only command grant (owner ask 2026-08-11). + +The classifier is deliberately fail-closed: local reads and pure pipelines only. A false +negative costs one manual approval; a false positive costs an unreviewed side effect. +""" + +from __future__ import annotations + +import pytest + +from coworker.permissions import Mode, PermissionEngine +from coworker.readonly import is_readonly_command + +ACCEPT = [ + "ls -la", + "cat README.md", + "grep -rn 'pattern' src", + "rg --json TODO", + "nl -ba tests/test_x.py | sed -n '320,345p'", + "nl -ba a.py | sed -E \"s/'[^']*'/'[REDACTED]'/g\"", + "git log --oneline -5", + "git diff main...HEAD", + "git status", + "git -C /tmp/repo log -1", + "git branch --show-current", + "git stash list", + "git config --get user.name", + "git remote -v", + "jq '.results | length' /tmp/report.json", + "find . -name '*.py'", + "LC_ALL=C grep -c uses .github/workflows/ci.yml", + "command -v semgrep", + "wc -l file.txt | sort", + "printf 'a: '", + "awk '{print $1}' data.txt", + "head -20 x | tail -5 | uniq -c", +] + +REJECT = [ + "", + "rm -rf /", + "cat a > b", # redirection + "cat a >> b", + "grep x f 2>/dev/null", # even stderr redirects + "ls; rm -rf /", # chaining + "ls && touch x", + "cat `whoami`", # substitution + "cat $(secret)", + "grep '$(x)' file", # can't tell quoted-safe apart — fail closed + "curl https://api.github.com/repos/x", # network = exfil channel, excluded + "wget http://x", + "ssh host ls", + "python3 -c 'print(1)'", # interpreters + "bash -c ls", + "sed -i 's/a/b/' f", # in-place write + "sed -n 'w /tmp/x' f", # sed write command + "sed -f script.sed f", # script file could carry w + "awk '{print > \"f\"}' x", # awk redirection + "awk 'BEGIN{system(\"rm x\")}'", + "find . -delete", + "find . -exec rm {} ;", + "git push origin main", + "git branch new-branch", # creates + "git tag v1", # creates + "git stash", # writes + "git config user.name evil", # writes + "git -c core.pager='touch x' log", # exec hook via -c + "git log --output=/tmp/f", # write via flag + "/tmp/evil/cat file", # path-invoked binary + "env FOO=1 rm x", + "tee /tmp/x", + "xargs rm", + "ls | tee /tmp/x", # every pipeline stage must classify + "ls |", # dangling pipe + "sudo cat /etc/shadow", +] + + +@pytest.mark.parametrize("cmd", ACCEPT) +def test_classifier_accepts(cmd): + assert is_readonly_command(cmd) is True, cmd + + +@pytest.mark.parametrize("cmd", REJECT) +def test_classifier_rejects(cmd): + assert is_readonly_command(cmd) is False, cmd + + +def test_engine_grant_gates_on_classifier(tmp_path): + eng = PermissionEngine(workspace_root=tmp_path, mode=Mode.INTERACTIVE) + + class Meta: + category = "shell" + risk_level = "high" + capabilities = ["exec"] + + # Before the grant: a read-only command still asks. + d = eng.evaluate("run_shell", {"command": "ls -la"}, Meta()) + assert d.needs_user + + eng.allow_readonly_for_session() + assert eng.evaluate("run_shell", {"command": "ls -la"}, Meta()).allowed + assert eng.evaluate("run_shell", {"command": "git log -1"}, Meta()).allowed + # The grant never covers writes/network — those keep asking. + assert eng.evaluate("run_shell", {"command": "rm -rf x"}, Meta()).needs_user + assert eng.evaluate("run_shell", {"command": "curl https://x"}, Meta()).needs_user + + +def test_grant_persists_via_session_grants(tmp_path): + from coworker.server.manager import _grants_of + + class FakeEngine: + class permissions: + session_allow_tools = set() + session_allow_commands = set() + session_readonly = True + + grants = _grants_of(FakeEngine) + assert grants == {"tools": [], "commands": [], "readonly": True}