Skip to content

Commit b85ea65

Browse files
committed
fix(compound): readonly segments count as covered in compound allow matching
`go test ./... | tail -50` with user rule `^go` used to fall through to overlay because `tail -50` had no allow rule. But tail IS readonly. Fix: before calling match_all_segments on compound Bash commands, pre-filter out segments that are readonly-auto-allowed (match readonly regex + paths inside cwd/allowed_dirs). The filter is gated on has_redirect=false so `cat file > /tmp/out && ls` still falls through when has_redirect would have blocked the readonly step. Also adds regression tests: - readonly segment covers for compound allow - filter respects has_redirect guard - 2>&1 fd duplication does not trigger has_redirect
1 parent e6e4cb1 commit b85ea65

2 files changed

Lines changed: 99 additions & 2 deletions

File tree

hooks/handlers/pre-tool-use.sh

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -538,10 +538,42 @@ ORDERED_COUNT="$(jq -r 'if type == "array" then length else 0 end' <<<"$ORDERED"
538538
MATCHED="" # "allow" | "ask" | ""
539539
if [ "$ORDERED_COUNT" -gt 0 ]; then
540540
if [ "$TOOL_NAME" = "Bash" ] && [ "$BASH_SEGMENT_COUNT" -gt 1 ]; then
541-
# Compound Bash command: use per-segment matching algorithm.
541+
# Compound Bash command: pre-filter readonly-auto-allowed segments
542+
# before per-segment matching. A segment is "covered" if it is either
543+
# readonly-auto-allowed OR matches an allow/ask rule. Filtering readonly
544+
# segments out lets match_all_segments make its all-or-nothing decision
545+
# on the remaining non-readonly segments.
546+
#
547+
# Example: `go test ./... | tail -50` with user rule `^go` and `tail`
548+
# in readonly list -> without filter, tail has no rule and command
549+
# falls through to overlay. With filter, tail is dropped, go matches
550+
# its rule, command is allowed.
551+
#
552+
# Guard: only filter when the ORIGINAL command has no output redirects.
553+
# If `cat file > /tmp/x` were filtered as readonly, we would mask the
554+
# write. has_redirect blocks this case by keeping the full segment list.
555+
_FILTERED_SEGMENTS=()
556+
if ! has_redirect "$BASH_CMD"; then
557+
for _seg in "${BASH_SEGMENTS[@]}"; do
558+
if is_readonly_command "$_seg" \
559+
&& readonly_paths_allowed "$_seg" "$CC_CWD" "$ALLOWED_DIRS_JSON"; then
560+
continue
561+
fi
562+
_FILTERED_SEGMENTS+=("$_seg")
563+
done
564+
else
565+
_FILTERED_SEGMENTS=("${BASH_SEGMENTS[@]}")
566+
fi
567+
568+
# If filtering left 0 segments, all were readonly and step 5b should
569+
# have handled it. Defensive fallback: use the original segments.
570+
if [ "${#_FILTERED_SEGMENTS[@]}" -eq 0 ]; then
571+
_FILTERED_SEGMENTS=("${BASH_SEGMENTS[@]}")
572+
fi
573+
542574
_MAS_RESULT=""
543575
_mas_rc=0
544-
_MAS_RESULT="$(match_all_segments "$ORDERED" "$TOOL_NAME" "${BASH_SEGMENTS[@]}" 2>/dev/null)" || _mas_rc=$?
576+
_MAS_RESULT="$(match_all_segments "$ORDERED" "$TOOL_NAME" "${_FILTERED_SEGMENTS[@]}" 2>/dev/null)" || _mas_rc=$?
545577
if [ "$_mas_rc" -eq 2 ]; then
546578
printf '[passthru] compound allow/ask rule regex error; passing through\n' >&2
547579
emit_passthrough

tests/hook_handler.bats

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1671,6 +1671,71 @@ EOF
16711671
[ "$decision" = "ask" ]
16721672
}
16731673

1674+
@test "compound: readonly segment covers for compound allow (go test | tail)" {
1675+
# Regression: `go test ./... | tail -50` with user rule `^go` should
1676+
# allow the whole compound. tail is readonly; with the pre-filter it
1677+
# is dropped from match_all_segments and go matches its rule.
1678+
cat > "$USER_ROOT/.claude/passthru.json" <<'EOF'
1679+
{
1680+
"version": 2,
1681+
"allow": [
1682+
{ "tool": "Bash", "match": { "command": "^go(\\s|$)" }, "reason": "allow go" }
1683+
],
1684+
"deny": []
1685+
}
1686+
EOF
1687+
run_handler '{"tool_name":"Bash","tool_input":{"command":"go test ./... -timeout 60s | tail -50"}}'
1688+
[ "$status" -eq 0 ]
1689+
decision="$(jq -r '.hookSpecificOutput.permissionDecision' <<<"$output")"
1690+
[ "$decision" = "allow" ]
1691+
reason="$(jq -r '.hookSpecificOutput.permissionDecisionReason' <<<"$output")"
1692+
[[ "$reason" == *"allow go"* ]]
1693+
}
1694+
1695+
@test "compound: readonly segment filter respects has_redirect guard" {
1696+
# `cat file > /tmp/out && ls` has an output redirect. The filter must
1697+
# NOT drop segments when has_redirect=true, otherwise we would mask
1698+
# the write. Without user rules, this should fall through to ask.
1699+
cat > "$USER_ROOT/.claude/passthru.json" <<'EOF'
1700+
{
1701+
"version": 2,
1702+
"allow": [
1703+
{ "tool": "Bash", "match": { "command": "^cat(\\s|$)" }, "reason": "allow cat" }
1704+
],
1705+
"deny": []
1706+
}
1707+
EOF
1708+
run_handler '{"tool_name":"Bash","tool_input":{"command":"cat file > /tmp/out && ls"}}'
1709+
[ "$status" -eq 0 ]
1710+
json_line="$(printf '%s\n' "$output" | grep -o '{"hookSpecificOutput".*}' | head -n1)"
1711+
[ -n "$json_line" ]
1712+
decision="$(jq -r '.hookSpecificOutput.permissionDecision' <<<"$json_line")"
1713+
# ls is readonly. Without the guard, filter would drop ls leaving cat,
1714+
# and cat matches the rule -> allow. With the guard (has_redirect=true),
1715+
# both segments stay. cat matches, but ls has no allow rule -> fall
1716+
# through to ask. This prevents masking the write via the redirect.
1717+
[ "$decision" = "ask" ]
1718+
}
1719+
1720+
@test "compound: fd duplication (2>&1) does not trigger has_redirect" {
1721+
# `go test 2>&1 | head` has no file redirect, only fd duplication.
1722+
# has_redirect returns false, readonly filter applies. head is readonly,
1723+
# filter drops it, go matches user rule -> allow.
1724+
cat > "$USER_ROOT/.claude/passthru.json" <<'EOF'
1725+
{
1726+
"version": 2,
1727+
"allow": [
1728+
{ "tool": "Bash", "match": { "command": "^go(\\s|$)" }, "reason": "allow go" }
1729+
],
1730+
"deny": []
1731+
}
1732+
EOF
1733+
run_handler '{"tool_name":"Bash","tool_input":{"command":"go test ./... 2>&1 | head"}}'
1734+
[ "$status" -eq 0 ]
1735+
decision="$(jq -r '.hookSpecificOutput.permissionDecision' <<<"$output")"
1736+
[ "$decision" = "allow" ]
1737+
}
1738+
16741739
@test "compound: allow rules covering ALL segments allows compound command" {
16751740
# Two different rules covering two different segments: both must match
16761741
# for the compound command to be allowed. Use non-readonly commands (make,

0 commit comments

Comments
 (0)