Fixes anthropics/claude-plugins-official#2082 — diff feeders use git's
default quotePath setting, which C-quotes any path with a non-ASCII
byte. The downstream parsers in gitutil.parse_diff_into_files /
gitutil.extract_file_paths_from_diff match the diff header with
`re.match(r'^a/(.+?) b/(.+)$', ...)`, which only sees the raw
`a/path b/path` form. The C-quoted `"a/\303\201vila/..."` form
slips past the regex, the `continue` fires, and the file is silently
dropped from review.
Effect: a vulnerable file like `Ávila/payment.py` with
`os.system('curl ' + user_input)` never reaches the LLM reviewer.
False negative in exactly the direction the plugin exists to catch.
Sibling of #2056 / #2075: those fixed the UTF-8 decode of the
subprocess output (text=True crashed the reader thread on Windows
cp1252). This one fixes the diff-feeder commands themselves — the
name-only helpers (_git_name_only, _git_status_porcelain) already
pass core.quotePath=false for this exact reason; the diff-text
feeders were the holdouts.
Fix: add `-c core.quotePath=false` to 4 git invocations:
- gitutil._git_diff_range (push-sweep feed)
- gitutil.get_git_diff (Stop-hook feed)
- security_reminder_hook commit-review `git diff` (amend delta)
- security_reminder_hook commit-review `git show` (post-amend)
With the flag, git emits raw UTF-8 in the diff header
(`a/Ávila/payment.py`), the regex matches, and both files (the
non-ASCII vulnerable one + any ASCII control file) flow through to
review correctly.
Verified locally on macOS Python 3.13:
- py_compile clean on both files.
- Existing 45 smoke + extensibility tests still pass.
- 8 new tests in test_diff_parser_non_ascii.py (added to internal
test suite at sg-staging/tests/, not in this PR):
* 2 static-shape: gitutil._git_diff_range and get_git_diff both
contain `core.quotePath=false` in their source.
* 2 commit-review static: every subprocess.run in
handle_commit_review_posttooluse that mentions `"diff"` or
`"show"` also passes the flag. Catches the regression
class where a new diff/show call site is added without
plumbing the flag through.
* 4 end-to-end with a real git repo containing a
`Ávila/payment.py` baseline-and-edit:
- WITHOUT flag: header is C-quoted, both parsers drop the
non-ASCII file (demonstrates the bug).
- WITH flag: header is raw UTF-8, both parsers see the file.
- parse_diff_into_files (the other parse path) also keeps
the file with the flag.
- get_git_diff end-to-end produces unquoted output whose
file list includes the non-ASCII path.
- 53/53 pass total (45 existing + 8 new) in 3.41s.
NOT verified end-to-end with a real CC commit-review fire on a
non-ASCII path. The static-shape tests catch the regression and the
end-to-end git-repo tests pin parser behavior, but the actual
LLM-review-with-vuln-found path requires runtime verification against
an Anthropic-API-credentialed CC session.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixesanthropics/claude-plugins-official#2056 — on Windows, when the
worktree contains an untracked file whose name has a character undefined
in cp1252 (accented capitals like Á Í Ï Ð Ý, most CJK, emoji), the
UserPromptSubmit hook crashes:
Exception in thread Thread-5 (_readerthread):
UnicodeDecodeError: 'charmap' codec can't decode byte 0x81
Traceback (most recent call last):
File diffstate.py, line 338, in _list_untracked
for p in r.stdout.split('\\0'):
AttributeError: 'NoneType' object has no attribute 'split'
Non-blocking (UPS failures still let the prompt through) but the
baseline-untracked snapshot is silently lost, so the Stop-hook review
mis-handles pre-existing untracked files.
Root cause (reporter's diagnosis, verified):
1. core.quotePath=false makes git emit raw UTF-8 for non-ASCII filenames.
2. subprocess.run(..., text=True) decodes via
locale.getpreferredencoding(False) in strict mode — on Windows that
is cp1252, in which 0x81 / 0x8D / 0x8F / 0x90 / 0x9D are undefined.
Those bytes appear in the UTF-8 encodings of Á (C3 81), Í (C3 8D),
Ï (C3 8F), Ð (C3 90), Ý (C3 9D), and a large fraction of CJK / emoji
codepoints.
3. The decode runs in the subprocess reader thread. The thread raises
UnicodeDecodeError, threading prints 'Exception in thread Thread-N',
subprocess.run returns with stdout=None. The handler then does
None.split('\\0') -> AttributeError, which is NOT in the narrow
except (TimeoutExpired, FileNotFoundError, OSError) tuple, so it
escapes the helper, propagates out of UserPromptSubmit's
ThreadPoolExecutor.result(), and exits the hook non-zero.
This is internally inconsistent: gitutil._git_diff_range,
security_reminder_hook._reflog_amend_lookup (line ~540), and the commit
diff loop (line ~1115) already do bytes + decode utf-8/replace, with
comments explicitly noting that text=True would crash. The fix below
extends that established pattern to the helpers that were holdouts.
Affected helpers (6 total):
- diffstate._list_untracked <- reporter, hot path, CRITICAL
- diffstate.capture_git_baseline <- reporter, latent
- diffstate.get_baseline_file_content <- audit, file content read, HIGH
- gitutil._git_name_only <- reporter, latent
- gitutil._git_status_porcelain <- reporter, latent
- gitutil._git_reflog_recent_commits <- audit, embeds %gs commit msg, HIGH
For each one:
- Drop text=True from subprocess.run.
- Decode r.stdout / r.stderr as .decode('utf-8', errors='replace').
- Add ValueError to the except tuple as defense against any future
strict-decode regression (UnicodeDecodeError is a ValueError
subclass; including it explicitly degrades the helper to its
empty/None return instead of escaping out of the hook).
Verified locally on macOS Python 3.13:
- py_compile clean on both files.
- 45 existing smoke + extensibility tests still pass.
- 21 new internal tests (not in this PR — added to the team's local
test suite at staging/tests/test_unicode_decode.py):
* 18 static-shape parametrized: each of the 6 fixed helpers has
no text=True in its subprocess calls, contains errors='replace',
and lists ValueError in its except.
* Deterministic end-to-end: create real git repo + Ávila_report.txt
untracked, call _list_untracked, verify it returns
{'Ávila_report.txt': <mtime>} without crashing.
* Deterministic end-to-end: same for capture_git_baseline (verifies
the latent stderr-warning case stays valid).
* Deterministic end-to-end: get_baseline_file_content on a file
whose content has 山田太郎 + 🎉; verify the bytes round-trip
through the decode.
- 66/66 tests pass total (45 existing + 21 new).
NOT verified end-to-end on Windows — would need actual cp1252 strict
decode to fire. Reporter has the deterministic repro and will
re-verify on their Win11 / Python 3.14.x setup before merge.
Not in this PR (defense-in-depth, lower risk):
- 3 git rev-parse calls returning path output (gitutil._find_git_index,
_git_toplevel, _git_dir) could fail on Windows if cwd is in a
non-ASCII install directory. Same fix shape but unreported and
much lower probability — worth a separate follow-up if anyone
actually hits it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes#410, #2037, #2045, #1640, #1280, #1329, #1341, #255,
anthropics/claude-code#46720 (partial closes on overlap with other rules).
The plugin's substring-only XSS / browser-DOM rules
(new_function_injection, react_dangerously_set_html, document_write_xss,
innerHTML_xss, outerHTML_xss, insertAdjacentHTML_xss) fired on any file
containing the trigger substring — including:
* Markdown documentation explaining XSS sinks
* Blog posts / READMEs that name browser APIs
* Python tutorials referencing dangerouslySetInnerHTML
* Plugin skill files with example HTML strings
* .yaml / .json configs that happen to contain the literal string
* .gitignore / Dockerfile / Makefile
These constructs have no meaning outside JS/TS source. Add a
path_filter: lambda p: p.endswith(_JS_EXTS) to each so they fire only
on .js, .jsx, .ts, .tsx, .mjs, .cjs, .mts, .cts, .vue, .svelte.
Cross-checked against the existing _JS_EXTS-gated rules
(regex_exec_substring, child_process_exec, exec_substring) — same
pattern, same constant, same intent. Uses the module-level _JS_EXTS
tuple so future extension changes propagate to all 6 rules atomically.
Verified locally on macOS Python 3.13:
- py_compile clean.
- 45-test existing smoke + extensibility suite still passes.
- 151 new parametrized tests in test_xss_gate.py (added to internal
test suite this PR doesn't ship): each gated rule x every
JS-family extension accepts, x every non-JS path (.md / .py /
.yaml / .json / .txt / .html / Dockerfile / Makefile / .gitignore
/ .sh / .go / .rs / .rb) rejects. 196 tests pass total.
Doesn't address everything in the false-positive cluster — issues that
require Python-rule gating (#1114 .env.schema exec), tighter substring
scoping (#660 pickle in usernames), or hook-protocol changes (#1358
exit-2 vs warning, #1375 plain-text-vs-JSON output) need separate PRs.
This PR covers the JS-substring subset cleanly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes anthropics/claude-plugins-official#2071 — on macOS where the
default `python3` is Apple's Command Line Tools Python 3.9.6, the
plugin's agentic commit reviewer silently does not run, even when the
user has a newer Python installed.
Three compounding factors in the bug:
1. `sg-python.sh` only checks the major version (`3`), so it always
picks 3.9 even when 3.10+ is on PATH.
2. `claude_agent_sdk` requires Python >=3.10 — pip install on 3.9
returns "No matching distribution" -> bootstrap returns BUILD_FAILED.
3. Even with a hand-built 3.12 venv, `llm.py` imports the SDK
in-process into the hook's interpreter (still 3.9), which raises
SyntaxError. The existing venv-probe in `ensure_agent_sdk.py` uses
the venv's own Python (3.12) so it reports NOOP_VENV (healthy) while
the consumer fails — misleading telemetry on top of silent feature
degradation.
Per BQ telemetry, 14,073 external macOS users hit
sdk_bootstrap=BUILD_FAILED in the past 4 days (the default-macOS
cohort), out of ~86K total external installed users. Combined with
~20K other users in similar broken-bootstrap states (Windows pre-#2055,
Linux <3.10), about half the installed base has a silently-broken
agentic reviewer.
This PR implements the reporter's items #1, #3, and #4. Item #2
(running the SDK out-of-process) is deferred as a bigger refactor.
Item #1 — hooks/sg-python.sh — prefer >=3.10 binaries via 3-pass probe:
Pass 1: python3.13 / 3.12 / 3.11 / 3.10 (>=3.10 by name, highest wins)
Pass 2: bare python3 / python / py -3 (accept only if reported >=3.10)
Pass 3: bare python3 / python / py -3 (any Python 3, FALLBACK so
pattern checks still work on macOS-default 3.9 — no regression
vs today; SDK-dependent paths detect the version mismatch
inside Python and degrade cleanly via item #4)
Item #4 — ensure_agent_sdk.py — health-check honesty:
Added HOOK_PY_INCOMPATIBLE=6 outcome with short-circuit at top of main():
if sys.version_info < (3, 10):
return HOOK_PY_INCOMPATIBLE, "hook_py", f"py_{...}"
Telemetry consequences after rollout: sdk_bootstrap=6 is a new clean
bucket; some users currently miscounted in sdk_bootstrap=3 BUILD_FAILED
(wasted pip cycles) and sdk_bootstrap=1 NOOP_VENV (falsely-healthy)
move to sdk_bootstrap=6. The remaining NOOP_VENV count becomes
trustworthy.
Item #3 — ensure_agent_sdk.py — one-time user-visible notice:
When outcome == HOOK_PY_INCOMPATIBLE and a marker file at
`~/.claude/security/.agentic_unavailable_notice_v<pv>` doesn't exist,
the SessionStart response includes hookSpecificOutput.additionalContext
+ systemMessage explaining the situation. Marker file is plugin-
version-keyed so a future fix (e.g. shipping out-of-process SDK) can
bump pv and re-notify users.
BUILD_FAILED is intentionally excluded from the notice — it covers
transient causes where a permanent banner would mislead.
Verified locally on macOS Python 3.13:
- py_compile clean on both files.
- Existing 45-test smoke + extensibility suite: 45/45 PASS in 2.50s.
- Unit test of simulated 3.9 path: HOOK_PY_INCOMPATIBLE returned with
correct phase/kind; notice shown on first call, suppressed on
second, reshown on bumped pv; BUILD_FAILED correctly does NOT
trigger notice.
NOT verified: actual Python 3.9 behavior end-to-end (would need a 3.9
install). Worth a follow-up smoke test in a 3.9 venv before next
release. The unit test simulating 3.9 covers the logic but not the
runtime invocation through the shim.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The first round of this PR removed SKIP_WIN32, fixed venv_py to use
Scripts/python.exe, and added Lib/site-packages to the consumer glob —
all necessary. Windows verification (Win11 ARM64, Py 3.13, Git Bash)
showed two more blockers, both addressed here.
1. Pip dependency resolver picks unbuildable cryptography on ARM64.
Without --prefer-binary, pip picks a cryptography version with no
published ARM64 wheel and tries to build it from source. That needs
Rust/Cargo, almost never present on user machines → BUILD_FAILED
with err_kind=other:cryptography. A binary wheel exists for an
adjacent version (cryptography-46.0.3-cp311-abi3-win_arm64.whl);
--prefer-binary tells pip to pick it. Cross-platform safe (no-op
where the latest version already has a wheel).
2. pywin32 .pth files aren't processed by sys.path.insert().
With the venv built, ensure_agent_sdk.py's post-build probe passes
(it runs from venv_py, where Python's site.py at startup processes
pywin32.pth and registers win32/, win32/lib/ plus runs
pywin32_bootstrap.py to set the DLL search dir). But llm.py runs in
the hook's SYSTEM Python and adds the venv via sys.path.insert(),
which doesn't trigger site.py at all. Without the bootstrap, the
SDK's mcp.client.stdio → mcp.os.win32.utilities chain raises
ModuleNotFoundError: pywintypes and the agentic reviewer falls back
to single-shot silently — exactly the symptom this PR is trying to
fix. The probe says NOOP_VENV; the actual consumer fails. Probe and
consumer use different Pythons.
Replicate what site.py would do: after inserting site-packages,
also insert win32/ and win32/lib/, then exec pywin32_bootstrap.py.
Pulled into a shared helper _inject_agent_sdk_venv_into_syspath()
so both consumer sites (3P SDK fallback, agentic_review fallback)
call the same code — Windows handling stays in one place.
Verified on macOS (POSIX path unchanged):
- Helper end-to-end test: POSIX-layout venv detected + fake package
imports successfully via the injected path
- Windows-layout venv also detected; win32 branch correctly skipped
via sys.platform check
- Both files pass py_compile
Credit: @mhegazy verified the previous commit on Win11 ARM64 / Py 3.13
/ Git Bash, surfaced both issues end-to-end, and provided the exact
fix patterns. This commit applies them with the pywin32 part factored
into a shared helper (vs. inlining at both consumer sites).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The agentic reviewer is silently no-op on Windows today. SessionStart
bootstrap (ensure_agent_sdk.py) short-circuits with SKIP_WIN32 because
the consumer glob in llm.py only matches POSIX venv layout
(lib/pythonX.Y/site-packages). On Windows, venvs use Lib/site-packages
(capital L, no pythonX.Y subdir), so even if a venv got built the
glob wouldn't find its contents.
Result: Windows users on default installs (no system-wide
claude_agent_sdk) get layer 1 (pattern warnings) and layer 2 (single-
shot LLM diff review) but not layer 3 — the cross-file agentic review
that catches IDOR, auth-bypass, cross-file SSRF, and other things that
need to read related files. Plugin description claims layer 3 but it
silently doesn't run.
Three changes:
1. llm.py — extend the consumer glob (2 sites: 3P SDK fallback at
~L297, agentic_review fallback at ~L1090) to also match the Windows
Lib/site-packages layout, so a venv built on Windows is actually
discoverable.
2. ensure_agent_sdk.py — remove the sys.platform == 'win32' early-exit
so the SessionStart bootstrap builds the venv on Windows too.
Outcome code 4 (formerly SKIP_WIN32) is retired but not reused so
pre-fix telemetry rows still decode correctly.
3. ensure_agent_sdk.py — venv_py path now branches on sys.platform:
Windows venvs put the interpreter at Scripts\python.exe; POSIX
uses bin/python. Previously assumed POSIX, so even with the glob
fix, the post-build SDK-importability probe would fail on Windows.
Verified locally on macOS:
- glob test: both layouts now match (POSIX venv detected, simulated
Windows venv also detected via the new Lib/site-packages branch)
- both files pass py_compile
- POSIX path unchanged (sys.platform != 'win32' so old branch runs)
Not verified on Windows in this commit — needs an actual Windows
runner to confirm the venv build + SDK import + subprocess plumbing
all work end-to-end. The SDK spawns a child claude.exe; Windows
process plumbing has its own quirks (shell semantics, path escaping)
that may surface separately. Worth a controlled rollout (one-week
soak under env-var opt-in before flipping default).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes#2043. On Git Bash for Windows, Claude Code hands script paths to
the shim in POSIX form (`/c/Users/...`). We exec a Windows `python.exe`
(the `python3` Microsoft Store stub fails the probe), and Windows Python
interprets the leading `/` as the root of the current drive — `/c/...`
becomes `C:\c\Users\...` or `D:\c\Users\...` depending on which
drive the shell happens to be on, fails with ENOENT, and every
Edit/Write/MultiEdit blocks until the session restarts.
Convert absolute path args via `cygpath -w` (a Git Bash builtin) before
exec. Guarded by `command -v cygpath` so macOS/Linux fall straight
through unchanged; `cygpath -w` is idempotent on already-Windows paths
so the rare mixed-form case is safe. Only `/*` paths are converted —
Windows-form paths reaching the shim are already openable by python.exe.
Verified locally:
- cygpath absent on macOS → guard skips → POSIX behavior unchanged
- end-to-end shim invocation with a POSIX path on macOS exits 0
- stubbed cygpath -w on /c/Users/test/hook.py produces C:\Users\test\hook.py
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Paths containing spaces (common on Windows, e.g. C:\Users\Some User\...)
cause shell word-splitting when CLAUDE_PLUGIN_ROOT is unquoted, resulting
in hooks erroring with "No such file or directory" on every tool call.
Wraps the path in double quotes for all five affected hook commands.
Fixes the pattern reported in issue #57946. Closes the fix surfaced in PR #1921.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
