security: an allowlisted reader must not auto-run outside the granted roots

coworker/permissions.py

@@ -8,6 +8,7 @@
 
 from __future__ import annotations
 
+import re
 import shlex
 from dataclasses import dataclass, field
 from enum import Enum
@@ -24,6 +25,24 @@
 def _has_shell_operators(command: str) -> bool:
     return any(op in command for op in _SHELL_OPERATORS)
 
+
+# `C:\dir` / `C:/dir` - a drive-qualified path, which carries no separator of its own.
+_WIN_DRIVE = re.compile(r"\A[A-Za-z]:[\\/]")
+
+
+def _looks_like_path(token: str) -> bool:
+    """Whether an argument names a filesystem location rather than a pattern, a
+    subcommand or a value. Intentionally narrow: a bare word like `status`, `install` or
+    `'*.py'` is not treated as a path, so only arguments that genuinely point somewhere
+    are scope-checked."""
+    return (
+        token in (".", "..")
+        or token.startswith(("/", "~", "./", "../", "\\"))
+        or "/" in token
+        or "\\" in token
+        or bool(_WIN_DRIVE.match(token))
+    )
+
 from .risk import (  # re-exported for back-compat (manager.py imports WRITE_TOOLS)
     SHELL_TOOL,
     WRITE_TOOLS,
@@ -150,7 +169,12 @@ def evaluate(
         # interactive / custom: allowlists.
         if is_shell:
             command = str(arguments.get("command", ""))
-            if self._command_allowed(command):
+            # Being on the allowlist says the PROGRAM is safe to run unattended. It says
+            # nothing about where that program is pointed, and `read_file` already refuses
+            # to leave the granted roots ("path escapes the workspace"), so auto-running
+            # `cat ~/.ssh/id_rsa` would hand back through the shell exactly what the file
+            # tool declines to read. Keep the same boundary on both paths.
+            if self._command_allowed(command) and not self._path_outside_roots(command):
                 return Decision(True, "command on allowlist")
             if command and command in self.session_allow_commands:
                 return Decision(True, "command allowed for session")
@@ -191,6 +215,34 @@ def _candidate(self, path: str) -> Path:
         p = Path(path).expanduser()
         return p.resolve() if p.is_absolute() else (self.workspace_root / p).resolve()
 
+    def _path_outside_roots(self, command: str) -> Optional[str]:
+        """The first argument of `command` naming a location outside every granted root,
+        or None if nothing does.
+
+        Only argument tokens that actually look like paths are considered, so patterns
+        and subcommands (`'*.py'`, `status`, `install`) are ignored. A relative path is
+        resolved against the workspace and therefore stays inside it unless it climbs
+        out with `..`; `_under_root` resolves symlinks, so a link planted in the
+        workspace does not launder a target outside it.
+        """
+        try:
+            argv = shlex.split(command)
+        except ValueError:
+            return None  # unparsable; _command_allowed already refuses these
+        for token in argv[1:]:
+            if token.startswith("-"):
+                # `--file=/etc/passwd` carries the path on the right of the `=`; a bare
+                # flag carries none.
+                _, sep, value = token.partition("=")
+                if not sep or not value:
+                    continue
+                token = value
+            if not _looks_like_path(token):
+                continue
+            if not self._under_root(token):
+                return token
+        return None
+
     def _under_root(self, path: str) -> bool:
         candidate = self._candidate(path)
         for rp, _ in self._resolved_roots():

tests/test_permissions_risk.py

@@ -131,6 +131,85 @@ def test_allowlist_prefix_is_argv_boundary(tmp_path):
     assert eng.evaluate("run_shell", {"command": "lsof"}, None).needs_user
 
 
+def _scoped_engine(tmp_path):
+    """Workspace + a second granted (read-only) root, with reader commands allowlisted."""
+    ws = tmp_path / "ws"
+    (ws / "src").mkdir(parents=True)
+    (ws / "README.md").write_text("hello\n")
+    (ws / "src" / "app.py").write_text("x = 1\n")
+    docs = tmp_path / "docs"
+    docs.mkdir()
+    (docs / "notes.md").write_text("notes\n")
+    return ws, docs, PermissionEngine(
+        workspace_root=ws,
+        allowed_commands=["ls", "cat", "pwd", "grep", "find", "head", "wc", "pytest"],
+        roots=[{"path": ws, "writable": True}, {"path": docs, "writable": False}],
+    )
+
+
+@pytest.mark.parametrize(
+    "command",
+    [
+        "cat /etc/passwd",
+        "cat ~/.ssh/id_rsa",
+        "cat ../../../etc/passwd",
+        "head -n 5 /etc/hosts",
+        "wc -l /etc/passwd",
+        "grep -r AKIA /Users",
+        "ls /",
+        "pytest --collect-only /tmp/evil_test.py",
+        "grep --file=/etc/passwd pattern",  # path hidden on the right of `=`
+    ],
+)
+def test_allowlisted_reader_does_not_auto_run_outside_the_roots(tmp_path, command):
+    # `read_file` already refuses to leave the granted roots ("path escapes the
+    # workspace"). An allowlisted `cat` must not hand back through the shell what the
+    # file tool declines to read.
+    _, _, eng = _scoped_engine(tmp_path)
+    d = eng.evaluate("run_shell", {"command": command}, None)
+    assert not d.allowed and d.needs_user, command
+
+
+def test_symlink_out_of_the_workspace_does_not_launder_the_read(tmp_path):
+    ws, _, eng = _scoped_engine(tmp_path)
+    secret = tmp_path / "secret"
+    secret.mkdir()
+    (secret / "id_rsa").write_text("SECRET\n")
+    (ws / "link").symlink_to(secret)
+    # The argument is workspace-relative, but it resolves outside the roots.
+    d = eng.evaluate("run_shell", {"command": "cat link/id_rsa"}, None)
+    assert not d.allowed and d.needs_user
+
+
+@pytest.mark.parametrize(
+    "command",
+    [
+        "ls -la",
+        "pwd",
+        "grep -n TODO README.md",
+        "grep -rn TODO src/",
+        "find . -name '*.py'",
+        "find . -type f -maxdepth 2",
+        "cat README.md",
+        "cat ./src/app.py",
+        "wc -l src/app.py",
+        "pytest -q",
+    ],
+)
+def test_reads_inside_the_roots_still_auto_run(tmp_path, command):
+    # Patterns and subcommands are not paths, and a relative path stays inside the
+    # workspace unless it climbs out, so ordinary use is untouched.
+    _, _, eng = _scoped_engine(tmp_path)
+    assert eng.evaluate("run_shell", {"command": command}, None).allowed, command
+
+
+def test_absolute_paths_inside_a_granted_root_still_auto_run(tmp_path):
+    ws, docs, eng = _scoped_engine(tmp_path)
+    assert eng.evaluate("run_shell", {"command": f"cat {ws}/README.md"}, None).allowed
+    # A second granted root is readable too, not just the primary workspace.
+    assert eng.evaluate("run_shell", {"command": f"cat {docs}/notes.md"}, None).allowed
+
+
 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.