diff --git a/coworker/readonly.py b/coworker/readonly.py index ae75e6f77..2da3e7e3d 100644 --- a/coworker/readonly.py +++ b/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 diff --git a/tests/test_readonly_grant.py b/tests/test_readonly_grant.py index 67bad7e72..c51875e8f 100644 --- a/tests/test_readonly_grant.py +++ b/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", diff --git a/tests/test_readonly_scoping.py b/tests/test_readonly_scoping.py index 98b888bb6..9495533c3 100644 --- a/tests/test_readonly_scoping.py +++ b/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 ` is the one accepted way to leave the working directory. assert not runs(session, "git -C ~/other-private-repo log -p")