diff --git a/changelog.d/tsk-lzyw3v-claude-code-guard-wrappers.md b/changelog.d/tsk-lzyw3v-claude-code-guard-wrappers.md new file mode 100644 index 000000000..0bad9da5c --- /dev/null +++ b/changelog.d/tsk-lzyw3v-claude-code-guard-wrappers.md @@ -0,0 +1,6 @@ +### Fixed +- Wrapper programs (`env`, `nice`, `nohup`, `timeout`, `xargs`, `sudo`, `doas`, `exec`, `command`, `builtin`, `time`, `stdbuf`, `setsid`, `ionice`, `chroot`, `busybox`) no longer hide the real program in the shell profile: the wrapper's key is recorded and the wrapped program is also checked. +- Hard refusal for `gh` and `git push` now also applies to parsed command keys, catching escapes like `g""h pr list` and `'gh' pr list`. +- The hook's `main()` now fails closed on any exception: it prints the refusal to stderr and exits with code 2. +- Wrapper options that take a separate value (`nice -n/--adjustment`, `ionice -c/-n/-p/--class/--classdata/--pid`, `stdbuf -i/-o/-e/--input/--output/--error`, `xargs -n/-P/-I/-L/-s/-d/-a/-E/--max-args/--max-procs/--max-lines/--max-chars/--delimiter/--arg-file`, `timeout -s/--signal/-k/--kill-after`, `env -u/--unset/-C/--chdir/-S/--split-string`) no longer hide the real program; `chroot` skips its root directory operand before finding the program. +- Fixed three hard-refusal bypasses: `chroot` root operand mistaken for a wrapper (e.g. `/tmp/timeout`), `exec -a NAME` and `exec -aNAME` not skipping the alias value, and `env -S/--split-string` not parsing the split string for nested commands (fail closed on `$`, backslash, or unparseable). \ No newline at end of file diff --git a/changelog.d/tsk-r3knpi-claude-code-guard-flag-clusters.md b/changelog.d/tsk-r3knpi-claude-code-guard-flag-clusters.md new file mode 100644 index 000000000..480b68033 --- /dev/null +++ b/changelog.d/tsk-r3knpi-claude-code-guard-flag-clusters.md @@ -0,0 +1,3 @@ +### Fixed + +- Fixed bypass of hard refusal for clustered short flags (e.g., `exec -la NAME`, `sudo -Eu USER`, `chroot --userspec/--groups`) by enhancing the wrapper option parsing in `tinyagentos/claude_code_guard.py`. Added short option letter sets and extended chroot wrapper options to properly handle value-taking clustered flags and GNU long options with separate values. diff --git a/tests/test_claude_code_shell_profile.py b/tests/test_claude_code_shell_profile.py index 09879965e..b0a62c315 100644 --- a/tests/test_claude_code_shell_profile.py +++ b/tests/test_claude_code_shell_profile.py @@ -296,6 +296,239 @@ def test_control_commands_are_allowed(): assert refusal is None, cmd +# --------------------------------------------------------------------------- +# Wrapper programs, gh escaping, and fail-closed main() (tsk-lzyw3v) +# --------------------------------------------------------------------------- + +def test_wrapper_programs_expose_real_program(): + from tinyagentos.claude_code_guard import commands + # env + cmds = commands("env curl x") + assert "env" in cmds and "curl" in cmds, cmds + # timeout: first non-option is duration, then program + cmds = commands("timeout 60 curl x") + assert "curl" in cmds, cmds + # sudo -u bob curl x + cmds = commands("sudo -u bob curl x") + assert "curl" in cmds, cmds + # xargs curl + cmds = commands("xargs curl") + assert "curl" in cmds, cmds + + +def test_wrapper_option_values_do_not_hide_program(): + from tinyagentos.claude_code_guard import commands + cases = [ + ("nice -n 10 curl x", "10"), + ("ionice -c 3 curl x", "3"), + ("stdbuf -o 0 curl x", "0"), + ("xargs -n 1 curl", "1"), + ("timeout -s KILL 60 curl x", "60"), + ("chroot /newroot curl x", "newroot"), + ("chroot / curl x", "/"), + ] + for cmd, bad in cases: + cmds = commands(cmd) + assert "curl" in cmds, f"curl missing in {cmd!r}: {cmds}" + assert bad not in cmds, f"option value {bad!r} leaked as key in {cmd!r}: {cmds}" + + +def test_hard_refusal_sees_program_behind_wrapper_values(): + from tinyagentos.claude_code_guard import refusal + cases = [ + 'chroot / g""h pr merge', + 'chroot /newroot g""h pr merge', + 'nice -n 10 g""h pr merge', + 'timeout -s KILL 60 g""h pr merge', + 'ionice -c 3 g""h pr merge', + ] + for cmd in cases: + payload = {"tool_name": "Bash", "tool_input": {"command": cmd}} + msg = refusal(payload) + assert msg is not None, f"should refuse but got None for {cmd!r}" + # positive control: sudo -u already fixed + payload = {"tool_name": "Bash", "tool_input": {"command": 'sudo -u x g""h pr merge'}} + assert refusal(payload) is not None + + +def test_hard_refusal_chroot_root_operand_bypass(): + from tinyagentos.claude_code_guard import refusal + # chroot root operand parsed as a wrapper: /tmp/timeout has basename "timeout" + payload = {"tool_name": "Bash", "tool_input": {"command": 'chroot /tmp/timeout g""h pr merge'}} + assert refusal(payload) is not None + + +def test_hard_refusal_exec_a_option_bypass(): + from tinyagentos.claude_code_guard import refusal + # exec -a NAME: -a takes a value but exec not in _WRAPPER_OPTS_WITH_VALUE + payload = {"tool_name": "Bash", "tool_input": {"command": 'exec -a masked g""h pr merge'}} + assert refusal(payload) is not None + # exec -aNAME attached form + payload = {"tool_name": "Bash", "tool_input": {"command": 'exec -amasked g""h pr merge'}} + assert refusal(payload) is not None + + +def test_hard_refusal_env_split_string_bypass(): + from tinyagentos.claude_code_guard import refusal + # env -S 'g""h pr merge' (separate value) + payload = {"tool_name": "Bash", "tool_input": {"command": 'env -S \'g""h pr merge\''}} + assert refusal(payload) is not None + # env -Sg""h pr merge (attached value) + payload = {"tool_name": "Bash", "tool_input": {"command": 'env -Sg""h pr merge'}} + assert refusal(payload) is not None + # env --split-string='g""h pr merge' (long form attached) + payload = {"tool_name": "Bash", "tool_input": {"command": 'env --split-string=\'g""h pr merge\''}} + assert refusal(payload) is not None + # env -S with env var expansion (fail closed) + payload = {"tool_name": "Bash", "tool_input": {"command": r"env -S '${X} pr merge'"}} + assert refusal(payload) is not None + + +def test_hard_refusal_wrapper_bypass_positive_controls(): + from tinyagentos.claude_code_guard import refusal + # chroot with normal root dir should still allow ls + payload = {"tool_name": "Bash", "tool_input": {"command": "chroot /srv/root ls -la"}} + assert refusal(payload) is None + # env -S with normal command should still allow ls + payload = {"tool_name": "Bash", "tool_input": {"command": "env -S 'ls -la'"}} + assert refusal(payload) is None + + +def test_hard_gh_refusal_on_parsed_keys(): + from tinyagentos.claude_code_guard import refusal + # g""h and 'gh' should be refused same as gh + payload_gh = {"hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": {"command": "gh pr list"}} + payload_g_empty = {"hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": {"command": 'g""h pr list'}} + payload_g_single = {"hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": {"command": "'gh' pr list"}} + msg_gh = refusal(payload_gh) + msg_g_empty = refusal(payload_g_empty) + msg_g_single = refusal(payload_g_single) + assert msg_gh is not None + assert msg_g_empty == msg_gh + assert msg_g_single == msg_gh + # git push also + payload_git = {"hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": {"command": "git push origin main"}} + assert refusal(payload_git) == msg_gh + + +# --------------------------------------------------------------------------- +# Wrapper bypass vulnerabilities that must be REFUSED (tsk-r3knpi) +# --------------------------------------------------------------------------- + +def test_bypass_exec_clustered_short_flag(): + from tinyagentos.claude_code_guard import refusal + # These clustered short flags should be REFUSED but are currently ALLOWED + cases = [ + 'exec -la masked g""h pr merge', + 'exec -ca masked g""h pr merge', + ] + for cmd in cases: + payload = {"tool_name": "Bash", "tool_input": {"command": cmd}} + msg = refusal(payload) + assert msg is not None, f"should refuse but got None for {cmd!r}" + + +def test_bypass_sudo_clustered_short_flag(): + from tinyagentos.claude_code_guard import refusal + # These clustered short flags should be REFUSED but are currently ALLOWED + cases = [ + 'sudo -Eu root g""h pr merge', + 'sudo -nu root g""h pr merge', + ] + for cmd in cases: + payload = {"tool_name": "Bash", "tool_input": {"command": cmd}} + msg = refusal(payload) + assert msg is not None, f"should refuse but got None for {cmd!r}" + + +def test_bypass_chroot_attached_forms(): + from tinyagentos.claude_code_guard import refusal + # These attached forms should be REFUSED but are currently ALLOWED + cases = [ + 'chroot --userspec=root:root /tmp/x g""h pr merge', + 'chroot --groups=g /tmp/x g""h pr merge', + ] + for cmd in cases: + payload = {"tool_name": "Bash", "tool_input": {"command": cmd}} + msg = refusal(payload) + assert msg is not None, f"should refuse but got None for {cmd!r}" + + +# --------------------------------------------------------------------------- +# Positive controls that must stay ALLOWED (benign) +# --------------------------------------------------------------------------- + +def test_benign_exec_clustered_short_flag(): + from tinyagentos.claude_code_guard import refusal + # These benign cases should stay ALLOWED + cases = [ + 'exec -la foo ls', + 'exec -ca foo ls', + ] + for cmd in cases: + payload = {"tool_name": "Bash", "tool_input": {"command": cmd}} + assert refusal(payload) is None, f"should allow but got refusal for {cmd!r}" + + +def test_benign_sudo_clustered_short_flag(): + from tinyagentos.claude_code_guard import refusal + # These benign cases should stay ALLOWED + cases = [ + 'sudo -Eu root ls', + 'sudo -nu root ls', + ] + for cmd in cases: + payload = {"tool_name": "Bash", "tool_input": {"command": cmd}} + assert refusal(payload) is None, f"should allow but got refusal for {cmd!r}" + + +def test_benign_chroot_attached_forms(): + from tinyagentos.claude_code_guard import refusal + # These benign attached forms should stay ALLOWED + cases = [ + 'chroot --userspec=root:root /tmp/x ls', + 'chroot --groups=g /tmp/x ls', + ] + for cmd in cases: + payload = {"tool_name": "Bash", "tool_input": {"command": cmd}} + assert refusal(payload) is None, f"should allow but got refusal for {cmd!r}" + + +def test_benign_env_split_string(): + from tinyagentos.claude_code_guard import refusal + # env -S 'ls -la' should stay ALLOWED + payload = {"tool_name": "Bash", "tool_input": {"command": "env -S 'ls -la'"}} + assert refusal(payload) is None + + +def test_benign_ls(): + from tinyagentos.claude_code_guard import refusal + # ls -la should stay ALLOWED + payload = {"tool_name": "Bash", "tool_input": {"command": "ls -la"}} + assert refusal(payload) is None + + +def test_main_fails_closed_on_exception(tmp_path, monkeypatch): + from tinyagentos.claude_code_guard import main + import json + import sys + from io import StringIO + # Monkeypatch the rules loader path to raise + def raise_runtime(*args, **kwargs): + raise RuntimeError("boom") + monkeypatch.setattr("builtins.open", raise_runtime) + # Prepare stdin with a payload that triggers profile check + payload = json.dumps({"tool_name": "Bash", "tool_input": {"command": "ls"}}) + old_stdin = sys.stdin + sys.stdin = StringIO(payload) + try: + # main with --rules pointing to a file (will hit the patched open) + rc = main(["--rules", "/nonexistent/rules.json"]) + finally: + sys.stdin = old_stdin + assert rc == 2 + + # --------------------------------------------------------------------------- # Profile store, asks -> Decisions, answers -> rules, settings routes # --------------------------------------------------------------------------- diff --git a/tinyagentos/claude_code_guard.py b/tinyagentos/claude_code_guard.py index a361adc83..def6971f4 100644 --- a/tinyagentos/claude_code_guard.py +++ b/tinyagentos/claude_code_guard.py @@ -23,6 +23,13 @@ import shlex import sys +# Short option letters for wrapper programs that take a value (next word) +_WRAPPER_SHORT_OPTS_WITH_VALUE = { + "exec": frozenset({"a"}), + "sudo": frozenset({"u", "g", "C", "h", "p", "r", "t", "T", "U", "D"}), + "doas": frozenset({"u", "C"}), +} + REFUSAL = ("Refused by taOS: the agent may not run `git push` or `gh` from the shell. " "GitHub writes go through the taOS tool, which asks the user to confirm.") @@ -43,6 +50,30 @@ "apt", "apt-get", "brew", "cargo", "dnf", "docker", "git", "go", "kubectl", "make", "npm", "npx", "pip", "pip3", "pnpm", "podman", "snap", "systemctl", "uv", "yarn", }) +# Wrapper programs that prefix another program. After a wrapper we keep +# expecting a program: skip its option words, NAME=value words (env), and for +# timeout the first non-option word (the duration); for sudo/doas skip the +# value of -u/-g/-C. The wrapper's own key is still added. +WRAPPER_PROGRAMS = frozenset({ + "env", "nice", "nohup", "timeout", "xargs", "sudo", "doas", "exec", + "command", "builtin", "time", "stdbuf", "setsid", "ionice", "chroot", "busybox", +}) +# Options of wrapper programs that take a separate value (next word). +_WRAPPER_OPTS_WITH_VALUE = { + "nice": frozenset({"-n", "--adjustment"}), + "ionice": frozenset({"-c", "-n", "-p", "--class", "--classdata", "--pid"}), + "stdbuf": frozenset({"-i", "-o", "-e", "--input", "--output", "--error"}), + "xargs": frozenset({ + "-n", "-P", "-I", "-L", "-s", "-d", "-a", "-E", + "--max-args", "--max-procs", "--max-lines", "--max-chars", + "--delimiter", "--arg-file", + }), + "timeout": frozenset({"-s", "--signal", "-k", "--kill-after"}), + "env": frozenset({"-u", "--unset", "-C", "--chdir", "-S", "--split-string"}), + "exec": frozenset({"-a"}), + "doas": frozenset({"-u", "-C", "--user", "--chdir"}), + "chroot": frozenset({"--userspec", "--groups"}), +} # Options of those programs that take a separate value (``git -C dir status``). _OPTIONS_WITH_VALUE = frozenset({ "-C", "-c", "--config-env", @@ -136,6 +167,11 @@ def add(key: str) -> None: prog: str | None = None # the program of the current simple command uniq_pos = 0 # positional argument count for uniq (2nd is output file) uniq_end_opts = False # -- seen for uniq, everything after is positional + # Wrapper state: when we are inside a wrapper program, what kind of skipping to do + wrapper: str | None = None # current wrapper program name + wrapper_skip_duration = False # for timeout: skip the first non-option (duration) + wrapper_skip_root = False # for chroot: skip the first non-option (root dir) + env_split_string_pending = False # for env -S/--split-string: next token is the split string for tok in tokens: if tok and set(tok) <= _OPERATOR_CHARS: if set(tok) & set("<>") and not set(tok) & set(";|()"): @@ -148,6 +184,11 @@ def add(key: str) -> None: if sub_of: add(sub_of) sub_of = None + if wrapper: + wrapper = None + wrapper_skip_duration = False + wrapper_skip_root = False + env_split_string_pending = False expect = True prog = None uniq_pos = 0 @@ -161,6 +202,19 @@ def add(key: str) -> None: add(OUTPUT_REDIRECT) continue if skip_next: + if env_split_string_pending: + env_split_string_pending = False + # Parse the split string recursively + if "$" in tok or "\\" in tok: + add("env -S") + else: + sub_keys = commands(tok) + if sub_keys is None: + add("env -S") + else: + for k in sub_keys: + add(k) + continue skip_next = False continue if sub_of: @@ -226,7 +280,7 @@ def add(key: str) -> None: skip_next = True # else the rest of tok is the attached value break - continue + continue # general --output handling (git diff, etc.) if tok == "--output" or tok.startswith("--output="): add(OUTPUT_REDIRECT) @@ -235,6 +289,123 @@ def add(key: str) -> None: continue continue name = os.path.basename(tok) or tok + # chroot: skip the first non-option (root dir) after options - must run BEFORE + # WRAPPER_PROGRAMS check so a root like /tmp/timeout is not mistaken for the + # timeout wrapper. + if wrapper == "chroot" and wrapper_skip_root: + if not tok.startswith("-"): + wrapper_skip_root = False + continue + # Handle wrapper programs: they prefix another program + if name in WRAPPER_PROGRAMS: + add(name) + wrapper = name + # For timeout, the first non-option after options is the duration (skip it) + if name == "timeout": + wrapper_skip_duration = True + # For chroot, the first non-option after options is the root dir (skip it) + if name == "chroot": + wrapper_skip_root = True + else: + wrapper_skip_root = False + # For sudo/doas, we need to skip values of -u, -g, -C + # For env, NAME=value assignments are handled by _ASSIGNMENT check above + # For other wrappers, just skip options (words starting with -) + expect = True # keep expecting the real program + prog = name + uniq_pos = 0 + uniq_end_opts = False + continue + # If we are in a wrapper context, skip its options + if wrapper: + # sudo/doas: skip -u user, -g group, -C dir (and their long forms) + if wrapper in ("sudo", "doas"): + if tok in ("-u", "-g", "-C", "--user", "--group", "--chdir"): + skip_next = True + continue + if tok.startswith(("-u", "-g", "-C")) and len(tok) > 2: + # -uuser, -ggroup, -Cdir (attached value) + continue + # exec: skip -a NAME (and -aNAME attached form) + if wrapper == "exec": + if tok == "-a": + skip_next = True + continue + if tok.startswith("-a") and len(tok) > 2: + # -aNAME (attached value) + continue + # Clustered short options that take a value (last letter in the option) + # For a token like -la, where 'a' takes a value, skip the next word + if wrapper in _WRAPPER_SHORT_OPTS_WITH_VALUE: + if tok.startswith("-") and not tok.startswith("--"): + # Check if this is a short option (single dash) and last letter takes a value + last_char = tok[-1] + if last_char in _WRAPPER_SHORT_OPTS_WITH_VALUE[wrapper]: + # Check that earlier letters in the token are not also value-taking + # For sudo -Cu: 'u' takes a value, so -Cu should NOT skip next word + # For exec -la: 'l' is not in value-taking set, 'a' is last, so -la skips next word + earlier_letters = tok[1:-1] + if not any(letter in _WRAPPER_SHORT_OPTS_WITH_VALUE[wrapper] for letter in earlier_letters): + skip_next = True + continue + # timeout: skip the first non-option (duration) after options + if wrapper == "timeout" and wrapper_skip_duration: + if not tok.startswith("-"): + wrapper_skip_duration = False + continue + # chroot: skip the first non-option (root dir) after options + if wrapper == "chroot" and wrapper_skip_root: + if not tok.startswith("-"): + wrapper_skip_root = False + continue + # env: handle -S/--split-string by parsing the split string recursively + if wrapper == "env": + split_string = None + if tok == "-S" or tok == "--split-string": + skip_next = True + env_split_string_pending = True + continue + if tok.startswith("-S") and len(tok) > 2: + # -S attached form + split_string = tok[2:] + elif tok.startswith("--split-string="): + # --split-string= attached form + split_string = tok.split("=", 1)[1] + if split_string is not None: + # Parse the split string recursively + # Fail closed if it contains $ (env var expansion), backslash, or doesn't parse + if "$" in split_string or "\\" in split_string: + add("env -S") + else: + sub_keys = commands(split_string) + if sub_keys is None: + add("env -S") + else: + for k in sub_keys: + add(k) + continue + # Wrapper-specific options that take a separate value + if wrapper in _WRAPPER_OPTS_WITH_VALUE: + if tok in _WRAPPER_OPTS_WITH_VALUE[wrapper]: + skip_next = True + continue + # Handle attached forms like --userspec=VALUE, --groups=VALUE + # These should not set skip_next for the next word + if tok.startswith("-"): + # Check for attached value form (--opt=value) + for opt in _WRAPPER_OPTS_WITH_VALUE[wrapper]: + if opt.startswith("-") and tok.startswith(opt): + # This is an attached form, don't skip_next + break + # Other wrappers: skip options (words starting with -) + if tok.startswith("-"): + continue + # This token is the real program + wrapper = None + wrapper_skip_duration = False + wrapper_skip_root = False + env_split_string_pending = False + # Fall through to process as normal program expect = False prog = name uniq_pos = 0 @@ -287,8 +458,15 @@ def refusal(payload: object) -> str | None: command = tool_input.get("command") if isinstance(tool_input, dict) else None if not isinstance(command, str): return REFUSAL + # Hard refusal on raw text (catches gh/git push in simple cases) if _GH.search(command) or _GIT_PUSH.search(command): return REFUSAL + # Hard refusal on parsed command keys (catches g""h, 'gh', etc.) + keys = commands(command) + if keys: + for key in keys: + if key == "gh" or key.startswith("gh ") or key == "git push" or key == "env -S": + return REFUSAL return None @@ -297,35 +475,40 @@ def _arg(argv: list[str], flag: str) -> str | None: def main(argv: list[str] | None = None) -> int: - argv = sys.argv[1:] if argv is None else argv try: - payload = json.loads(sys.stdin.read()) - except ValueError: - payload = None - message = refusal(payload) - rules_path = _arg(argv, "--rules") - if message is None and rules_path and payload.get("tool_name") == "Bash": - command = payload["tool_input"]["command"] + argv = sys.argv[1:] if argv is None else argv try: - with open(rules_path, encoding="utf-8") as f: - rules = json.load(f) - allow, deny = set(rules.get("allow") or ()), set(rules.get("deny") or ()) - except (OSError, ValueError, AttributeError, TypeError): - print("Refused by taOS: the shell profile could not be read.", file=sys.stderr) - return 2 - message, unknown = profile_refusal(command, allow, deny) - asks_path = _arg(argv, "--asks") - if unknown and asks_path: + payload = json.loads(sys.stdin.read()) + except ValueError: + payload = None + message = refusal(payload) + rules_path = _arg(argv, "--rules") + if message is None and rules_path and payload.get("tool_name") == "Bash": + command = payload["tool_input"]["command"] try: - with open(asks_path, "a", encoding="utf-8") as f: - f.writelines(json.dumps({"program": key, "command": command}) + "\n" - for key in unknown) - except OSError: - pass - if message is None: - return 0 - print(message, file=sys.stderr) - return 2 + with open(rules_path, encoding="utf-8") as f: + rules = json.load(f) + allow, deny = set(rules.get("allow") or ()), set(rules.get("deny") or ()) + except (OSError, ValueError, AttributeError, TypeError): + print("Refused by taOS: the shell profile could not be read.", file=sys.stderr) + return 2 + message, unknown = profile_refusal(command, allow, deny) + asks_path = _arg(argv, "--asks") + if unknown and asks_path: + try: + with open(asks_path, "a", encoding="utf-8") as f: + f.writelines(json.dumps({"program": key, "command": command}) + "\n" + for key in unknown) + except OSError: + pass + if message is None: + return 0 + print(message, file=sys.stderr) + return 2 + except Exception: + # Fail closed: any unexpected error blocks the call + print(REFUSAL, file=sys.stderr) + return 2 if __name__ == "__main__":