Skip to content

Commit 0bc4ab3

Browse files
committed
fix(toolkit): JSON-escape-aware command extract + repo-agnostic verify (DOJ-4872)
Round-4 bot review at 3/5 confidence flagged 1 P2 + 3 P3. All four addressed. P2 — hooks/pre-tool-use-claim-verification.sh JSON command extractor - The jq-less fallback regex `"[^"]*"` stops at the first inner quote. For a bash command like `grep -rn \"Symbol\" src/`, the JSON envelope embeds it as `"command": "grep -rn \\\"Symbol\\\" src/"`, and the naive regex truncates at the first `\"`. Subsequent detector / suggester logic then sees a partial command and emits the wrong replacement. - Switched to the escape-aware JSON-string body `([^"\\]|\\.)*` which accepts any non-quote-non-backslash byte OR a backslash followed by any byte. Verified with an envelope containing `grep -rn \"Symbol\" src/` — the full command extracts cleanly and the suggester emits the right line. P3 — hooks/pre-tool-use-claim-verification.sh cross-platform quote handling - The bot flagged that the explicit single-quoted-pattern grep extractor (added in round 3) used embedded-quote shell escaping that can drift between GNU and BSD grep. Simplified by removing the explicit single- quoted branch entirely: the awk fallback already strips both `"` and `'` from the extracted token, so it covers both quote styles without embedded escapes. Net code reduction; same behavior across the four smoke-tested variants (single/double quoted, single/multi flag). P3 — commands/verify.md repo-agnostic grep path - The behavior section hardcoded `-- src/` as the path filter, which would fail with a `fatal: pathspec` error in toolkit consumers that have a different layout (`lib/`, `app/`, `packages/`, flat). Added a detect-or-fallback step: if `src/` exists on the resolved ref, use it as the path filter; otherwise pass no path filter and grep the whole tree. P3 — commands/verify.md remote-only tag resolution - The normalization step probed `git rev-parse --verify refs/tags/<tag>` which only sees LOCAL tags. A remote-only tag (e.g. `v1.31.0` just pushed by a teammate but not yet fetched locally) would fall through to the bare-branch branch and resolve to a nonexistent `origin/v1.31.0`. Added a `git ls-remote --tags --exit-code origin refs/tags/<tag>` probe between the local-tag and SHA branches; on hit, runs `git fetch origin tag <tag> --quiet` before resolving. Comment documents the failure mode. Verification: - shellcheck → 0 issues - JSON envelope with escaped quotes: `{"tool_input":{"command":"grep -rn \\\"Symbol\\\" src/"}}` → Command: `grep -rn "Symbol" src/` (was: `grep -rn `) Suggested: `git grep "Symbol" origin/<branch> -- src/` (was: garbled) - "grep -rn 'Symbol' src/" → Suggested: "Symbol" (regression) - "grep -rn \"Symbol\" src/" → Suggested: "Symbol" (regression) - "grep -i -r \"Symbol\" src/" → Suggested: "Symbol" (regression) - "ls src/foo.tsx" → Suggested: src/foo.tsx (regression) - "echo hello" → silent (regression) Created by Claude Code on behalf of @lapc506.
1 parent 012e9ca commit 0bc4ab3

2 files changed

Lines changed: 37 additions & 27 deletions

File tree

commands/verify.md

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,25 @@ pipeline handles bare branch names, `origin/<branch>`, tags, and SHAs
4242
uniformly.
4343

4444
1. **Normalize the ref.** Decide which fully-qualified ref to query, and
45-
whether a remote fetch makes sense for that ref:
45+
whether a remote fetch makes sense for that ref. Tag resolution probes
46+
the remote via `git ls-remote` first so remote-only tags (not yet
47+
fetched locally) don't fall through to the bare-branch branch and
48+
resolve to a nonexistent `origin/<tag>`:
4649

4750
```bash
4851
RAW_REF="$1"
4952
FETCH_TARGET="" # what to pass to `git fetch origin <…> --quiet`; empty = skip
5053
RESOLVED_REF="" # what to pass to git show / ls-tree / grep
5154

5255
if git rev-parse --verify --quiet "refs/tags/$RAW_REF" >/dev/null; then
53-
# Annotated/lightweight tag, e.g. v1.31.0
56+
# Local annotated/lightweight tag, e.g. v1.31.0 (already fetched).
5457
RESOLVED_REF="refs/tags/$RAW_REF"
5558
FETCH_TARGET="tag $RAW_REF"
59+
elif git ls-remote --tags --exit-code origin "refs/tags/$RAW_REF" >/dev/null 2>&1; then
60+
# Remote-only tag — refresh it locally before resolving.
61+
git fetch origin "tag $RAW_REF" --quiet
62+
RESOLVED_REF="refs/tags/$RAW_REF"
63+
FETCH_TARGET="" # already fetched in the probe step
5664
elif [[ "$RAW_REF" =~ ^[0-9a-f]{7,40}$ ]] \
5765
&& git cat-file -e "$RAW_REF" 2>/dev/null; then
5866
# Commit SHA (short or long) that already exists locally — no fetch.
@@ -100,12 +108,20 @@ uniformly.
100108
git show "$RESOLVED_REF:$ARG" | head -50
101109
```
102110

103-
- Otherwise, treat it as a regex/pattern to grep across `src/`:
111+
- Otherwise, treat it as a regex/pattern to grep across the repo. Use
112+
`src/` as the path filter when it exists at the repo root; otherwise
113+
grep the whole tree so the command stays repo-agnostic (toolkit
114+
consumers may have `lib/`, `app/`, `packages/`, or a flat layout):
104115

105116
```bash
106-
git grep -nF -- "$ARG" "$RESOLVED_REF" -- src/
117+
if git ls-tree --name-only "$RESOLVED_REF" -- src 2>/dev/null | grep -q .; then
118+
PATH_FILTER=(-- src/)
119+
else
120+
PATH_FILTER=()
121+
fi
122+
git grep -nF -- "$ARG" "$RESOLVED_REF" "${PATH_FILTER[@]}"
107123
# If the user passes a regex (contains regex metachars), drop -F:
108-
git grep -nE -- "$ARG" "$RESOLVED_REF" -- src/
124+
git grep -nE -- "$ARG" "$RESOLVED_REF" "${PATH_FILTER[@]}"
109125
```
110126

111127
6. **Emit a single verdict line** in this exact shape so it can be pasted

hooks/pre-tool-use-claim-verification.sh

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,13 @@ esac
7676

7777
# Fallback command extraction if jq is missing or input is shaped oddly.
7878
if [ -z "$COMMAND" ] && [ -n "$INPUT_RAW" ]; then
79-
# Tolerant single-line extract — does not need jq.
79+
# Tolerant single-line extract — does not need jq. Escape-aware: the
80+
# `([^"\\]|\\.)*` body of the JSON string accepts any non-quote-non-
81+
# backslash char, OR a backslash followed by any char (so embedded
82+
# `\"` sequences inside a bash command like `grep -rn \"Symbol\" src/`
83+
# don't truncate the extraction at the first inner quote).
8084
COMMAND="$(printf '%s' "$INPUT_RAW" \
81-
| grep -oE '"command"[[:space:]]*:[[:space:]]*"[^"]*"' \
85+
| grep -oE '"command"[[:space:]]*:[[:space:]]*"([^"\\]|\\.)*"' \
8286
| head -1 \
8387
| sed -E 's/^"command"[[:space:]]*:[[:space:]]*"(.*)"$/\1/')"
8488
fi
@@ -165,27 +169,17 @@ if printf '%s' "$COMMAND" | grep -qE '(^|[;&|][[:space:]]*)ls([[:space:]]|$)'; t
165169
SUGGESTED="git fetch origin <branch> --quiet && git ls-tree origin/<branch> -- <path>"
166170
fi
167171
elif printf '%s' "$COMMAND" | grep -qE 'grep[[:space:]]+-[rRnliN]+'; then
168-
# First try the double-quoted form: `grep -rn "Symbol" src/`.
172+
# Single extractor handles both quote styles + multi-flag forms.
173+
# Nested awk loop: locate the `-FLAGS` token, then walk forward
174+
# skipping additional flag tokens before emitting the first non-flag
175+
# one. Strip both " and ' so the unquoted token comes out clean
176+
# regardless of which quote style the original command used.
177+
# Cross-platform note: no escaped quotes inside the shell quoting
178+
# (the bot flagged literal-backslash drift between GNU and BSD grep
179+
# implementations in round 4).
169180
pattern_arg="$(printf '%s' "$COMMAND" \
170-
| grep -oE 'grep[[:space:]]+-[rRnliN]+[[:space:]]+"[^"]+"' \
171-
| head -1 \
172-
| sed -E 's/^grep[[:space:]]+-[rRnliN]+[[:space:]]+"(.+)"$/\1/')"
173-
# Then the single-quoted form: `grep -rn 'Symbol' src/`.
174-
if [ -z "$pattern_arg" ]; then
175-
pattern_arg="$(printf '%s' "$COMMAND" \
176-
| grep -oE "grep[[:space:]]+-[rRnliN]+[[:space:]]+'[^']+'" \
177-
| head -1 \
178-
| sed -E "s/^grep[[:space:]]+-[rRnliN]+[[:space:]]+'(.+)'$/\\1/")"
179-
fi
180-
if [ -z "$pattern_arg" ]; then
181-
# Nested awk loop: skip subsequent flag tokens so multi-flag forms
182-
# like `grep -i -r "Symbol" src/` don't mis-extract `-r` as the pattern.
183-
# Strip both " and ' so unquoted-token output is clean regardless of
184-
# whether the original command used double or single quotes.
185-
pattern_arg="$(printf '%s' "$COMMAND" \
186-
| awk '{for(i=1;i<=NF;i++) if ($i ~ /^-[rRnliN]+$/) {for(j=i+1;j<=NF;j++) if ($j !~ /^-/) {print $j; exit}}}' \
187-
| tr -d "\"'")"
188-
fi
181+
| awk '{for(i=1;i<=NF;i++) if ($i ~ /^-[rRnliN]+$/) {for(j=i+1;j<=NF;j++) if ($j !~ /^-/) {print $j; exit}}}' \
182+
| tr -d "\"'")"
189183
if [ -n "$pattern_arg" ]; then
190184
SUGGESTED="git fetch origin <branch> --quiet && git grep \"$pattern_arg\" origin/<branch> -- src/"
191185
else

0 commit comments

Comments
 (0)