From 4decd2e3b26f6c17e458fcdcf2dd60aef8566f3e Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 27 May 2026 14:50:51 -0700 Subject: [PATCH 1/2] Enable agentic commit reviewer on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../hooks/ensure_agent_sdk.py | 17 ++++++++--------- plugins/security-guidance/hooks/llm.py | 18 ++++++++++++------ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/plugins/security-guidance/hooks/ensure_agent_sdk.py b/plugins/security-guidance/hooks/ensure_agent_sdk.py index 4e34f176..697b5cce 100644 --- a/plugins/security-guidance/hooks/ensure_agent_sdk.py +++ b/plugins/security-guidance/hooks/ensure_agent_sdk.py @@ -28,7 +28,9 @@ NOOP_SYSTEM = 0 # claude_agent_sdk already importable in system python NOOP_VENV = 1 # venv already built and SDK imports from it BUILT = 2 # venv created + SDK pip-installed this run BUILD_FAILED = 3 # venv create or pip install raised/timed out -SKIP_WIN32 = 4 # Windows; consumer glob doesn't handle Lib/ layout +# Outcome 4 was previously SKIP_WIN32; retired now that the consumer glob in +# llm.py also matches Windows venv layout (Lib/site-packages). Don't reuse the +# value — telemetry rows from older plugin builds still emit 4. SKIP_SENTINEL = 5 # another SessionStart is currently building @@ -60,13 +62,6 @@ def main() -> tuple[int, str, str]: err_phase / err_kind are non-empty only on BUILD_FAILED — they let telemetry split bootstrap failures by root cause. """ - # Windows venv layout (Lib/site-packages, no python* subdir) isn't - # handled by the consumer's glob in security_reminder_hook.py; skip the - # bootstrap entirely rather than build a venv that's never read. - if sys.platform == "win32": - return SKIP_WIN32, "", "" - - if _sdk_on_syspath(): return NOOP_SYSTEM, "", "" @@ -75,7 +70,11 @@ def main() -> tuple[int, str, str]: or os.path.expanduser("~/.claude/security") ) venv = state_dir / "agent-sdk-venv" - venv_py = venv / "bin" / "python" + # Windows venvs put the interpreter at Scripts\python.exe; POSIX uses bin/python. + if sys.platform == "win32": + venv_py = venv / "Scripts" / "python.exe" + else: + venv_py = venv / "bin" / "python" # Another SessionStart (concurrent CC instance, same plugin) may already # be building. The sentinel lives NEXT TO the venv, not inside it — diff --git a/plugins/security-guidance/hooks/llm.py b/plugins/security-guidance/hooks/llm.py index eff56de7..11ef6ce9 100644 --- a/plugins/security-guidance/hooks/llm.py +++ b/plugins/security-guidance/hooks/llm.py @@ -298,9 +298,12 @@ def _call_claude_via_sdk(prompt, output_schema, *, max_tokens=16000, model=None) "SECURITY_WARNINGS_STATE_DIR", os.path.expanduser("~/.claude/security"), ) - for _sp in glob.glob( - os.path.join(_state_dir, "agent-sdk-venv", "lib", - "python*", "site-packages") + # POSIX venv: lib/pythonX.Y/site-packages + # Windows venv: Lib/site-packages (capital L, no pythonX.Y subdir) + _venv_root = os.path.join(_state_dir, "agent-sdk-venv") + for _sp in ( + glob.glob(os.path.join(_venv_root, "lib", "python*", "site-packages")) + + glob.glob(os.path.join(_venv_root, "Lib", "site-packages")) ): if os.path.isdir(_sp) and _sp not in sys.path: sys.path.insert(0, _sp) @@ -1094,9 +1097,12 @@ def agentic_review( "SECURITY_WARNINGS_STATE_DIR", os.path.expanduser("~/.claude/security"), ) - for _sp in glob.glob( - os.path.join(_state_dir, "agent-sdk-venv", "lib", - "python*", "site-packages") + # POSIX venv: lib/pythonX.Y/site-packages + # Windows venv: Lib/site-packages (capital L, no pythonX.Y subdir) + _venv_root = os.path.join(_state_dir, "agent-sdk-venv") + for _sp in ( + glob.glob(os.path.join(_venv_root, "lib", "python*", "site-packages")) + + glob.glob(os.path.join(_venv_root, "Lib", "site-packages")) ): if os.path.isdir(_sp) and _sp not in sys.path: sys.path.insert(0, _sp) From c11244778df46ce0cf8192377637b09f8c323351 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 27 May 2026 15:07:33 -0700 Subject: [PATCH 2/2] Address Windows verification: --prefer-binary + pywin32 bootstrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../hooks/ensure_agent_sdk.py | 12 ++- plugins/security-guidance/hooks/llm.py | 83 ++++++++++++++----- 2 files changed, 74 insertions(+), 21 deletions(-) diff --git a/plugins/security-guidance/hooks/ensure_agent_sdk.py b/plugins/security-guidance/hooks/ensure_agent_sdk.py index 697b5cce..220188e5 100644 --- a/plugins/security-guidance/hooks/ensure_agent_sdk.py +++ b/plugins/security-guidance/hooks/ensure_agent_sdk.py @@ -124,10 +124,20 @@ def main() -> tuple[int, str, str]: # the user's machine, pip's own default registry applies — that's the same # exposure the user would have running `pip install` themselves, so # we're not widening the supply-chain surface. + # + # --prefer-binary: on ARM64 Windows, pip's default resolver picks a + # `cryptography` version with no published binary wheel and tries to + # build from source, which needs Rust/Cargo (almost never present + # on user machines). The build fails and the whole bootstrap returns + # BUILD_FAILED. A binary wheel exists on PyPI 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 + # on platforms where the latest version already has a wheel. err_phase = "pip" subprocess.run( [str(venv_py), "-m", "pip", "install", "--quiet", - "--disable-pip-version-check", "claude-agent-sdk"], + "--disable-pip-version-check", "--prefer-binary", + "claude-agent-sdk"], capture_output=True, timeout=120, check=True, ) return BUILT, "", "" diff --git a/plugins/security-guidance/hooks/llm.py b/plugins/security-guidance/hooks/llm.py index 11ef6ce9..bd47aa43 100644 --- a/plugins/security-guidance/hooks/llm.py +++ b/plugins/security-guidance/hooks/llm.py @@ -31,6 +31,67 @@ from _base import debug_log, _record_usage, _PV, PROVENANCE_TAG # noqa: F401 from session_state import with_locked_state +def _inject_agent_sdk_venv_into_syspath(state_dir): + """Prepend the agent-SDK venv's site-packages to sys.path so the SDK + import below picks it up when the user's system Python doesn't have it. + + Called from two fallback sites (3P SDK + agentic_review); shared here so + Windows pywin32 handling stays in one place. + + Returns True if any path was added. + + POSIX venv layout: `agent-sdk-venv/lib/pythonX.Y/site-packages` + Windows venv layout: `agent-sdk-venv/Lib/site-packages` (capital L, no + pythonX.Y subdir). The SDK transitively imports pywin32 on Windows, and + pywin32's `.pth` files (which add `win32/`, `win32/lib/` to sys.path and + register the DLL search dir via `pywin32_bootstrap.py`) are processed + ONLY by Python's `site.py` at interpreter startup — not when we manually + insert a path here. Without the bootstrap, the SDK's + `mcp.client.stdio → mcp.os.win32.utilities → pywintypes` import chain + fails with `ModuleNotFoundError: pywintypes` and the agentic reviewer + falls back to single-shot silently. Replicate what site.py would do. + """ + venv_root = os.path.join(state_dir, "agent-sdk-venv") + candidates = ( + glob.glob(os.path.join(venv_root, "lib", "python*", "site-packages")) + + glob.glob(os.path.join(venv_root, "Lib", "site-packages")) + ) + added = False + for sp in candidates: + if not os.path.isdir(sp) or sp in sys.path: + continue + sys.path.insert(0, sp) + added = True + if sys.platform == "win32": + _bootstrap_pywin32(sp) + return added + + +def _bootstrap_pywin32(site_packages_dir): + """Manually replicate the pywin32 `.pth` bootstrap so a venv added via + `sys.path.insert()` (not site.py) can still import `pywintypes`. No-op + when the venv doesn't include pywin32. Failures are swallowed — the + SDK import below will raise its own ImportError and the caller's + fallback path handles it cleanly.""" + try: + win32 = os.path.join(site_packages_dir, "win32") + win32_lib = os.path.join(win32, "lib") + for d in (win32, win32_lib): + if os.path.isdir(d) and d not in sys.path: + sys.path.insert(0, d) + bootstrap = os.path.join(win32_lib, "pywin32_bootstrap.py") + if os.path.isfile(bootstrap): + import importlib.util + spec = importlib.util.spec_from_file_location( + "pywin32_bootstrap", bootstrap, + ) + if spec and spec.loader: + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + except Exception as e: + debug_log(f"pywin32 bootstrap failed (may break SDK import on Windows): {e}") + + # Plan Security Check Configuration ANTHROPIC_API_KEY = os.environ.get("ANTHROPIC_API_KEY", "") # OAuth access token — Claude Code passes this for /login users. @@ -298,15 +359,7 @@ def _call_claude_via_sdk(prompt, output_schema, *, max_tokens=16000, model=None) "SECURITY_WARNINGS_STATE_DIR", os.path.expanduser("~/.claude/security"), ) - # POSIX venv: lib/pythonX.Y/site-packages - # Windows venv: Lib/site-packages (capital L, no pythonX.Y subdir) - _venv_root = os.path.join(_state_dir, "agent-sdk-venv") - for _sp in ( - glob.glob(os.path.join(_venv_root, "lib", "python*", "site-packages")) - + glob.glob(os.path.join(_venv_root, "Lib", "site-packages")) - ): - if os.path.isdir(_sp) and _sp not in sys.path: - sys.path.insert(0, _sp) + _inject_agent_sdk_venv_into_syspath(_state_dir) try: import asyncio as _asyncio # noqa: F811 from claude_agent_sdk import ( # noqa: F401,F811 @@ -1092,21 +1145,11 @@ def agentic_review( # ~/.claude/security/ with the SDK installed; try that as a fallback # before giving up. The system import is attempted first so users # who DO have it never touch the venv. - _venv_tried = False _state_dir = os.environ.get( "SECURITY_WARNINGS_STATE_DIR", os.path.expanduser("~/.claude/security"), ) - # POSIX venv: lib/pythonX.Y/site-packages - # Windows venv: Lib/site-packages (capital L, no pythonX.Y subdir) - _venv_root = os.path.join(_state_dir, "agent-sdk-venv") - for _sp in ( - glob.glob(os.path.join(_venv_root, "lib", "python*", "site-packages")) - + glob.glob(os.path.join(_venv_root, "Lib", "site-packages")) - ): - if os.path.isdir(_sp) and _sp not in sys.path: - sys.path.insert(0, _sp) - _venv_tried = True + _venv_tried = _inject_agent_sdk_venv_into_syspath(_state_dir) try: import asyncio as _asyncio # noqa: F811