From 0b921cda88527ce782187c4222ee66d096427714 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Sat, 24 Oct 2026 19:46:55 +0000 Subject: [PATCH 1/2] Fix: exempt /dev/null from program-level output-redirect tagging The shell-redirection branch in commands() already exempts /dev/null, but every program-level output-file handler (sort -o, uniq, git --output, tee, time -o) was incorrectly tagging it. Added _is_write_target() helper and used it at all program-level output-file sites. For separate-value forms, introduced prog_redirect_pending flag to defer the decision until the next token arrives, mirroring redirect_target for shell redirects. ``` ============================= test session starts ============================== platform linux -- Python 3.13.5, pytest-9.1.1, pluggy-1.6.0 Using --randomly-seed=1061218174 rootdir: /tmp/exec-tsk-sggrai configfile: pyproject.toml plugins: asyncio-1.4.0, split-0.11.0, randomly-4.1.0, anyio-4.15.1, respx-0.23.1, xdist-3.8.0, timeout-2.4.0 asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function timeout: 120.0s timeout method: thread timeout func_only: False collected 1 item tests/test_claude_code_shell_profile.py F [100%] =================================== FAILURES =================================== __________________ test_program_output_to_dev_null_not_tagged __________________ def test_program_output_to_dev_null_not_tagged(): from tinyagentos.claude_code_guard import commands # These should NOT have output-redirect > assert commands("sort -o /dev/null f") == ["sort"] E AssertionError: assert ['sort', 'output-redirect'] == ["sort"] E E Left contains one more item: 'output-redirect' E Use -v to get more diff tests/test_claude_code_shell_profile.py:1173: AssertionError =========================== 1 failed in 0.41s =============================== ``` --- ...rai-shell-guard-dev-null-program-writes.md | 2 + tests/test_claude_code_shell_profile.py | 21 +++++ tinyagentos/claude_code_guard.py | 82 +++++++++++++------ 3 files changed, 81 insertions(+), 24 deletions(-) create mode 100644 changelog.d/tsk-sggrai-shell-guard-dev-null-program-writes.md diff --git a/changelog.d/tsk-sggrai-shell-guard-dev-null-program-writes.md b/changelog.d/tsk-sggrai-shell-guard-dev-null-program-writes.md new file mode 100644 index 000000000..3496ad871 --- /dev/null +++ b/changelog.d/tsk-sggrai-shell-guard-dev-null-program-writes.md @@ -0,0 +1,2 @@ +### Fixed +- Program-level output-file handlers (sort -o, uniq, git --output, tee, time -o) no longer tag `/dev/null` as `output-redirect`, matching the shell redirection behavior. Only the exact string `/dev/null` is exempt; `/dev/null/x`, `./dev/null`, `/dev/nul` and `$NULL` remain tagged. \ No newline at end of file diff --git a/tests/test_claude_code_shell_profile.py b/tests/test_claude_code_shell_profile.py index 6fb2b5ac8..ae4438ae2 100644 --- a/tests/test_claude_code_shell_profile.py +++ b/tests/test_claude_code_shell_profile.py @@ -1161,3 +1161,24 @@ def test_time_clustered_output_option_profile_refusal(): # Control: non-clustered form works correctly msg, _ = profile_refusal("time -a echo ok", allow={"time", "echo"}, deny={"gh"}) assert msg is None + + +# --------------------------------------------------------------------------- +# tsk-sggrai: program-level output to /dev/null must not tag output-redirect +# --------------------------------------------------------------------------- + +def test_program_output_to_dev_null_not_tagged(): + from tinyagentos.claude_code_guard import commands + # These should NOT have output-redirect + assert commands("sort -o /dev/null f") == ["sort"] + assert commands("uniq f /dev/null") == ["uniq"] + assert commands("git diff --output=/dev/null") == ["git diff"] + assert commands("tee /dev/null") == ["tee"] + assert commands("time -o /dev/null echo ok") == ["time", "echo"] + assert commands("time --output=/dev/null echo ok") == ["time", "echo"] + # Controls that MUST still have output-redirect + assert "output-redirect" in commands("sort -o /dev/null/x f") + assert "output-redirect" in commands("tee ./dev/null") + assert "output-redirect" in commands("tee /dev/nul") + assert commands("tee -") == ["tee", "output-redirect"] + assert commands("echo ok > /tmp/x") == ["echo", "output-redirect"] diff --git a/tinyagentos/claude_code_guard.py b/tinyagentos/claude_code_guard.py index 5524bbf0e..12b31d52c 100644 --- a/tinyagentos/claude_code_guard.py +++ b/tinyagentos/claude_code_guard.py @@ -100,6 +100,13 @@ _DUP_TARGET = re.compile(r"[0-9]+-?|-") +def _is_write_target(path: str) -> bool: + """Return True if the path is a write target that should be tagged. + The exact string /dev/null is exempt; everything else is a write target. + """ + return path != "/dev/null" + + def _has_command_substitution_in_double_quotes(text: str) -> bool: """Return True if $(...), ${...} or backtick occurs inside double quotes. @@ -167,6 +174,7 @@ def add(key: str) -> None: expect = True skip_next = False redirect_target = False # the next word is the target of an output redirection + prog_redirect_pending = False # next word is a program-level output file (sort -o, time -o, etc.) sub_of: str | None = None # a program still waiting for its subcommand prog: str | None = None # the program of the current simple command uniq_pos = 0 # positional argument count for uniq (2nd is output file) @@ -201,12 +209,19 @@ def add(key: str) -> None: tee_end_opts = False skip_next = False redirect_target = False + prog_redirect_pending = False continue if redirect_target: redirect_target = False if tok != "/dev/null" and not _DUP_TARGET.fullmatch(tok): add(OUTPUT_REDIRECT) continue + if prog_redirect_pending: + prog_redirect_pending = False + skip_next = False + if _is_write_target(tok): + add(OUTPUT_REDIRECT) + continue if skip_next: if env_split_string_pending: env_split_string_pending = False @@ -230,10 +245,11 @@ def add(key: str) -> None: add(GIT_CONFIG_OVERRIDE) elif tok.startswith("--exec-path"): add(GIT_CONFIG_OVERRIDE) - elif tok == "--output" or tok.startswith("--output="): - add(OUTPUT_REDIRECT) - if tok == "--output": - skip_next = True + elif tok == "--output": + prog_redirect_pending = True + elif tok.startswith("--output="): + if _is_write_target(tok.split("=", 1)[1]): + add(OUTPUT_REDIRECT) elif tok.startswith("-") and tok not in _OPTIONS_WITH_VALUE and tok not in _GIT_NO_VALUE_OPTS: # Unknown git option before subcommand: fail closed. return None @@ -252,14 +268,27 @@ def add(key: str) -> None: if len(opt) >= 2 and "compress-program".startswith(opt): return None if len(opt) >= 1 and "output".startswith(opt): - add(OUTPUT_REDIRECT) - if "=" not in tok: - skip_next = True + if "=" in tok: + if _is_write_target(tok.split("=", 1)[1]): + add(OUTPUT_REDIRECT) + else: + prog_redirect_pending = True continue - # sort clustered short option containing o (e.g. -ro, -o) + # sort clustered short option containing o (e.g. -ro, -o, -o/dev/null, -ro/dev/null) if prog == "sort" and tok.startswith("-") and not tok.startswith("--") and "o" in tok[1:]: - add(OUTPUT_REDIRECT) - skip_next = True + # Find the 'o' in the cluster; only the last value-taking option matters + # For sort, only 'o' takes a value + o_pos = tok.rfind("o") # rightmost 'o' in the cluster + if o_pos > 0: + has_attached = o_pos < len(tok) - 1 + if has_attached: + # Attached value form: -o/dev/null, -ro/dev/null + attached_value = tok[o_pos + 1:] + if _is_write_target(attached_value): + add(OUTPUT_REDIRECT) + else: + # Separate value form: -o /dev/null, -ro /dev/null + prog_redirect_pending = True continue # tee: any non-option argument (file operand) is a write if prog == "tee": @@ -271,14 +300,15 @@ def add(key: str) -> None: continue if tok.startswith("-"): continue - add(OUTPUT_REDIRECT) + if _is_write_target(tok): + add(OUTPUT_REDIRECT) continue # uniq positional argument counting: 2nd positional is output file if prog == "uniq": if uniq_end_opts or tok == "-" or not tok.startswith("-"): # positional argument (after --, bare -, or non-option) uniq_pos += 1 - if uniq_pos == 2: + if uniq_pos == 2 and _is_write_target(tok): add(OUTPUT_REDIRECT) continue if tok == "--": @@ -301,11 +331,12 @@ def add(key: str) -> None: break continue # general --output handling (git diff, etc.) - if tok == "--output" or tok.startswith("--output="): - add(OUTPUT_REDIRECT) - if tok == "--output": - skip_next = True - continue + if tok == "--output": + prog_redirect_pending = True + elif tok.startswith("--output="): + if _is_write_target(tok.split("=", 1)[1]): + add(OUTPUT_REDIRECT) + continue continue name = os.path.basename(tok) or tok # F5: command-position parameter expansion @@ -441,17 +472,18 @@ def add(key: str) -> None: if tok in _WRAPPER_OPTS_WITH_VALUE[wrapper]: if wrapper == "time" and tok in ("-o", "--output", "-f", "--format"): if tok in ("-o", "--output"): - add(OUTPUT_REDIRECT) + prog_redirect_pending = True 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("-"): + 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): - if wrapper == "time" and opt in ("-o", "--output"): - add(OUTPUT_REDIRECT) + if opt.startswith("--") and tok.startswith(opt): + if wrapper == "time" and opt == "--output": + if _is_write_target(tok.split("=", 1)[1]): + add(OUTPUT_REDIRECT) # This is an attached form, don't skip_next break # time: clustered short options (e.g. -ao, -po, -af, -apo, -ao/tmp/x, -af%e) @@ -477,13 +509,15 @@ def add(key: str) -> None: if has_attached: # Attached value form: -ao/tmp/x, -af%e if last == "o": - add(OUTPUT_REDIRECT) + attached_value = tok[i:] + if _is_write_target(attached_value): + add(OUTPUT_REDIRECT) # Don't skip_next, the value is attached continue else: # Separate value form: -ao FILE, -af %e, -apo /tmp/x if last == "o": - add(OUTPUT_REDIRECT) + prog_redirect_pending = True skip_next = True continue # Other wrappers: skip options (words starting with -) From d65877525b523ef182f6571c0517aa5507be29f4 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Sun, 25 Oct 2026 00:47:04 +0000 Subject: [PATCH 2/2] claude_code_guard: sort cluster takes the first o, time --output attached form needs = Lead finish from the pushed branch for CodeRabbit's two findings on #3950, both measured real on 0b921cda8: Major: `sort -o/tmp/foo /dev/null` returned ['sort'] because rfind("o") picked the last o inside the path and read /dev/null as the output; dev tagged it. The attached value starts after the FIRST o in the cluster. Minor: `time --outputx echo ok` raised IndexError (startswith("--output") matched, then split("=")[1]); a raising guard is a crashed PreToolUse hook. The attached form is matched on `--output=` only. Red on 0b921cda8 (python 3.11): ``` FAILED tests/test_claude_code_shell_profile.py::test_sort_cluster_attached_output_uses_first_o E AssertionError: assert ['sort'] == ['sort', 'output-redirect'] FAILED tests/test_claude_code_shell_profile.py::test_time_long_output_prefix_without_equals_does_not_raise tinyagentos/claude_code_guard.py:485: IndexError 2 failed ``` Green after: 73 passed (tests/test_claude_code_shell_profile.py, python 3.11). Docs-Reviewed: changelog.d/tsk-sggrai-shell-guard-dev-null-program-writes.md, added by 0b921cda8 in this PR, already covers the /dev/null exemption; this commit corrects the sort cluster parse and the time --output attached match, no further doc change. --- tests/test_claude_code_shell_profile.py | 18 ++++++++++++++++++ tinyagentos/claude_code_guard.py | 8 ++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/tests/test_claude_code_shell_profile.py b/tests/test_claude_code_shell_profile.py index ae4438ae2..b7daf77a4 100644 --- a/tests/test_claude_code_shell_profile.py +++ b/tests/test_claude_code_shell_profile.py @@ -1182,3 +1182,21 @@ def test_program_output_to_dev_null_not_tagged(): assert "output-redirect" in commands("tee /dev/nul") assert commands("tee -") == ["tee", "output-redirect"] assert commands("echo ok > /tmp/x") == ["echo", "output-redirect"] + + +def test_sort_cluster_attached_output_uses_first_o(): + # CodeRabbit Major on #3950: the attached value starts after the FIRST o + # in the cluster; a later o inside the path must not be read as the option. + from tinyagentos.claude_code_guard import commands + assert commands("sort -o/tmp/foo /dev/null") == ["sort", "output-redirect"] + assert commands("sort -ro/tmp/foo /dev/null") == ["sort", "output-redirect"] + assert commands("sort -o/dev/null f") == ["sort"] + assert commands("sort -ro/dev/null f") == ["sort"] + + +def test_time_long_output_prefix_without_equals_does_not_raise(): + # CodeRabbit Minor on #3950: --outputx is not the attached form, so it + # must not be split on "=" (IndexError made the guard crash). + from tinyagentos.claude_code_guard import commands + assert commands("time --outputx echo ok") == ["time", "echo"] + assert commands("time --output=/tmp/x echo ok") == ["time", "output-redirect", "echo"] diff --git a/tinyagentos/claude_code_guard.py b/tinyagentos/claude_code_guard.py index 12b31d52c..b6fc6bfa0 100644 --- a/tinyagentos/claude_code_guard.py +++ b/tinyagentos/claude_code_guard.py @@ -276,9 +276,9 @@ def add(key: str) -> None: continue # sort clustered short option containing o (e.g. -ro, -o, -o/dev/null, -ro/dev/null) if prog == "sort" and tok.startswith("-") and not tok.startswith("--") and "o" in tok[1:]: - # Find the 'o' in the cluster; only the last value-taking option matters - # For sort, only 'o' takes a value - o_pos = tok.rfind("o") # rightmost 'o' in the cluster + # The FIRST 'o' takes the value: everything after it is the attached + # path (-o/tmp/foo), so a later 'o' inside the path is not an option. + o_pos = tok.find("o", 1) if o_pos > 0: has_attached = o_pos < len(tok) - 1 if has_attached: @@ -480,7 +480,7 @@ def add(key: str) -> None: 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): + if opt.startswith("--") and tok.startswith(opt + "="): if wrapper == "time" and opt == "--output": if _is_write_target(tok.split("=", 1)[1]): add(OUTPUT_REDIRECT)