security: reject shell variable expansion in read-only grants

coworker/readonly.py

@@ -56,13 +56,29 @@
 _SED_WRITE = re.compile(r"(^|[;{])\s*[0-9,$/ ]*[wW]\s")
 
 
+def _has_unquoted_shell_variable(command: str) -> bool:
+    quote: str | None = None
+    escaped = False
+    for char in command:
+        if escaped:
+            escaped = False
+        elif char == "\\" and quote != "'":
+            escaped = True
+        elif char == "'" and quote != '"':
+            quote = None if quote == "'" else "'"
+        elif char == '"' and quote != "'":
+            quote = None if quote == '"' else '"'
+        elif char == "$" and quote != "'":
+            return True
+    return False
+
+
 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:
+    # Shell variables are expanded after this check; single-quoted '$' is literal.
+    if _has_unquoted_shell_variable(command) or "$(" 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

tests/test_readonly_grant.py

@@ -47,6 +47,9 @@
     "cat `whoami`",                           # substitution
     "cat $(secret)",
     "grep '$(x)' file",                       # can't tell quoted-safe apart — fail closed
+    r"cat $env:APPDATA\coworker\secrets.json",  # PowerShell variable expansion
+    r"cat $HOME/.config/coworker/secrets.json",  # POSIX variable expansion
+    r"cat ${HOME}/.config/coworker/secrets.json",  # POSIX braced expansion
     "curl https://api.github.com/repos/x",    # network = exfil channel, excluded
     "wget http://x",
     "ssh host ls",

tests/test_readonly_scoping.py

@@ -76,6 +76,18 @@ def test_openworkers_own_secrets_are_no_longer_readable(session):
     assert not runs(session, "jq . ~/.config/coworker/secrets.json")
 
 
+@pytest.mark.parametrize(
+    "command",
+    [
+        r"cat $env:APPDATA\coworker\secrets.json",
+        r"cat $HOME/.config/coworker/secrets.json",
+        r"cat ${HOME}/.config/coworker/secrets.json",
+    ],
+)
+def test_variable_expansion_cannot_hide_an_out_of_scope_read(session, command):
+    assert not runs(session, command)
+
+
 def test_another_repository_is_not_in_scope(session):
     # `git -C <dir>` is the one accepted way to leave the working directory.
     assert not runs(session, "git -C ~/other-private-repo log -p")