Mohamed Hegazy 1ecf3d1bac
security-guidance: purge text=True from subprocess.run + bake PYTHONUTF8=1 (#2099)
URGENT WINDOWS FIX. Sibling of #2056 / PR #2075 but covering 14 more
sites that PR #2075 missed.

The bug class: on Windows with cp1252 default encoding (typical
en-US locale), `subprocess.run(..., text=True)` decodes child stdout
AND stderr via `locale.getpreferredencoding()`. When git emits a
UTF-8 byte that's undefined in cp1252 (e.g. `0x81` from ف, present
in any path/filename/branch ref/commit message containing
Arabic/Hebrew/CJK), Python's internal `_readerthread` raises
UnicodeDecodeError. The thread crash is silent in Python 3.13+ (only
printed to stderr), but `subprocess.run` returns `stdout=None` and
the caller AttributeErrors on `.strip()`. The user sees a misleading
"WinError 267" or similar catch-all message instead of the real
decode failure.

PR #2075 fixed 6 specific helpers in `diffstate.py` / `gitutil.py`.
This commit covers the 14 survivors. Plus a defense-in-depth belt:
`PYTHONUTF8=1` exported by sg-python.sh.

This commit:

1. sg-python.sh: `export PYTHONUTF8=1` (PEP 540). No-op on
   macOS/Linux (already UTF-8). On Windows, makes Python's
   `locale.getpreferredencoding()` return UTF-8 instead of cp1252 —
   so even if a future regression slips in text=True, the decode
   succeeds. Must be set BEFORE Python starts; changing it from
   inside the interpreter has no effect.

2. gitutil.py: convert 8 subprocess.run sites from
   `capture_output=True, text=True` to `capture_output=True` +
   manual `r.stdout.decode("utf-8", errors="replace")`:
     - _git_rev_parse_head           (stdout = SHA, stderr risk)
     - _find_git_index               (stdout = PATH, primary bug site)
     - _temp_index git add           (returncode only, stderr risk)
     - _git_toplevel                 (stdout = PATH, primary bug site)
     - _git_dir                      (stdout = PATH, primary bug site)
     - _git_rev_list_range           (stdout = SHAs, stderr risk)
     - _detect_main_branch           (stdout = ref, stderr risk)
     - merge-base --is-ancestor      (returncode only, stderr risk)

3. security_reminder_hook.py: convert 6 subprocess.run sites
   (rev-parse @{u}/@{u}@{1}/local_ref, merge-base, HEAD lookup,
   reflog SHA resolution) — same pattern.

4. security_reminder_hook.py: fix the misleading log line in
   handle_user_prompt_submit. Was:
     debug_log("Failed to capture git baseline (not a git repo?)")
   Now includes the cwd in the message so the next reporter doesn't
   waste an hour grepping for the real WinError, per reporter's
   secondary finding.

Verified locally on macOS Python 3.13:

  - py_compile clean on all modified files.
  - bash -n sg-python.sh clean.
  - sg-python.sh actually propagates PYTHONUTF8=1 to child Python
    (verified via probe — sys.flags.utf8_mode=1).
  - Existing 353 tests still pass — 0 regression.
  - 25 new tests in test_2099_subprocess_text_true.py:
      * 10 static-shape catchers (one per hooks/*.py file). Any
        future PR that reintroduces text=True OR encoding= in
        subprocess.run fails this check at PR time. Single source
        of truth for the regression class.
      * 2 sg-python.sh verifiers (literal export + actual
        propagation to child Python).
      * 5 macOS end-to-end against a real git repo containing
        non-cp1252 content (`ف.py` filename): _git_toplevel,
        _git_dir, _find_git_index, _git_rev_parse_head,
        _git_rev_list_range all return clean values without
        AttributeError / UnicodeDecodeError.
      * 7 round-trip bytes-decode pattern verifiers (parametrized
        over Arabic ف, Hebrew א, Japanese 案, raw 0x81, multiple
        cp1252-undefined bytes, real-world git diff headers).
      * 1 sanity check that cp1252 strict DOES raise on 0x81
        (proves the test environment can catch the bug class).

  - Full suite: 378/378 pass in 5.56s.

  - End-to-end tmux smoke test driving real claude 2.1.145 CLI:
    Made a git commit via Bash tool call. All 4 hooks fired through
    the fixed plugin path:
      11:28:16.730  Hook called with args: …/plugin/hooks/security_reminder_hook.py
      11:28:16.734  Processing: hook_event=UserPromptSubmit
      11:28:16.825  Captured git baseline: 445f7f213256
      11:28:19.923  Hook called with args: …
      11:28:19.923  Processing: hook_event=PostToolUse, tool=Bash
      11:28:19.971  Commit review: detected git commit in command
      11:28:20.020  Commit review: 1/1 sha(s) resolved, 1 files
      11:28:26.415  Hook called with args: …
      11:28:26.416  Processing: hook_event=Stop
      11:28:26.550  Stop hook: empty review set

    Confirms: PYTHONUTF8=1 export doesn't break anything; converted
    helpers (_git_rev_parse_head, _git_toplevel, _git_dir,
    _find_git_index) run end-to-end without issue on the happy path.

NOT verified end-to-end on Windows with actual non-cp1252 content
in path/filename/stderr. The static-shape catcher pins the
regression class permanently. Reporter's PYTHONUTF8=1 workaround
empirically proves the encoding-mode fix works for the affected
scenario; this commit just bakes it in.

Closes #2099.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-30 11:29:07 -07:00
..
2026-05-26 14:06:52 -07:00

security-guidance

Security review for Claude-generated code. Three layers:

  1. Pattern warnings — instant regex-based reminders on Edit/Write for ~25 known-dangerous patterns (yaml.load, torch.load(weights_only=False), pickle.load on untrusted data, raw innerHTML, hardcoded secrets, etc.).
  2. LLM diff review — when Claude finishes a turn, the plugin sends the diff to a fast LLM call (Opus 4.7 by default) and feeds high-severity findings back to Claude so it can fix them before you see the response.
  3. Agentic commit review — on git commit, an SDK-driven reviewer reads related files (Read/Grep/Glob) to trace data flow across the codebase, catching multi-file vulnerabilities pattern matching misses (IDOR, auth bypass, cross-file SSRF).

Findings cover common web-vulnerability classes — injection, XSS, SSRF, hardcoded secrets, IDOR, auth bypass, unsafe deserialization, and path traversal among others.

Install

/plugin install security-guidance@claude-plugins-official

Marketplace ships enabled by default in Claude Code — no setup beyond having the CLI itself.

Prerequisites

  • Claude Code CLI ≥ v2.1.144
  • Python 3.8+ on PATH (python3, python, or py -3 — the plugin picks the first that works)
  • A working API path (subscription, API key, or 3P provider config)

Configuration

All configuration is via environment variables. None are required for default behavior.

Selecting a model

# 1P / gateway: a canonical model id
SECURITY_REVIEW_MODEL=claude-opus-4-7   # default

# Bedrock: use the inference-profile id
SECURITY_REVIEW_MODEL=us.anthropic.claude-opus-4-7

# Vertex: use the Vertex date-tag form
SECURITY_REVIEW_MODEL=claude-opus-4-7@20260218

SECURITY_REVIEW_MODEL controls the LLM diff review. SG_AGENTIC_MODEL (same syntax) controls the agentic commit reviewer; defaults to the same model.

Enabling/disabling layers

Variable Default What it does
SECURITY_GUIDANCE_DISABLE=1 unset Kill switch — disables the entire plugin
ENABLE_PATTERN_RULES=0 on Disable layer 1 (regex pattern warnings)
ENABLE_CODE_SECURITY_REVIEW=0 on Disable all LLM reviews (Stop hook + commit/push)
ENABLE_STOP_REVIEW=0 on Disable only the Stop-hook diff review, keeping commit/push reviews. Useful for multi-agent / shared-worktree setups where another agent can move HEAD between a worker's turns
ENABLE_COMMIT_REVIEW=0 on Disable layer 3 (agentic commit review)

Higher-recall mode

SG_DUAL_OR=on   # default off

Runs two parallel review calls and unions the findings. Catches a few percentage points more vulnerabilities in our testing, at roughly 2× the API cost per review. Most users don't need it.

Org-specific policies

Drop a claude-security-guidance.md in any of:

  • ~/.claude/claude-security-guidance.md — user-wide rules
  • <project>/.claude/claude-security-guidance.md — project rules, intended to be committed
  • <project>/.claude/claude-security-guidance.local.md — local overrides, intended to be .gitignore'd

All three are loaded and concatenated into the LLM diff review's prompt in the order user → project → project-local. If the combined size exceeds the 8 KB prompt budget, the tail is truncated, so user-wide rules are kept and project-local rules are dropped first. The agentic commit reviewer (layer 3) does not currently read this file. Example:

# Acme security rules

- All SELECTs against the `customers` or `orders` tables MUST go through `db.replica`,
  never `db.primary`. Primary is for writes only.
- Background jobs must not use the user-context auth token; they get
  service-account creds from `jobs.get_service_account()`.
- Calls to `requests.get(url)` with a user-controlled `url` need
  the SSRF-allowlist wrapper at `acme.net.safe_request`.

Built-in rules cover common web-vulnerability classes without it — claude-security-guidance.md is for things specific to your codebase that the model can't infer.

Privacy and data handling

The plugin sends data to a model endpoint to perform its reviews. Specifically, each Stop-hook diff review transmits the changed file paths, the diff hunks, and the relevant file contents in the diff; each agentic commit review additionally transmits any files the reviewer pulls in via Read/Grep/Glob while tracing data flow. Your claude-security-guidance.md contents (user, project, and local) are appended to the prompt on every review, so don't put secrets in it.

Where that data goes depends on your Claude Code configuration:

  • Default (Anthropic API / subscription): sent to api.anthropic.com and handled under Anthropic's Commercial Terms and Privacy Policy.
  • LLM gateway (ANTHROPIC_BASE_URL set): sent to your gateway URL instead. The gateway operator's terms apply.
  • 3rd-party providers (Bedrock / Vertex / Foundry / Mantle): sent to your configured provider endpoint. The provider's data-handling terms apply (e.g., AWS / GCP / Azure).

The plugin writes its own debug log to ~/.claude/security/log.txt (override with SECURITY_GUIDANCE_DEBUG_LOG). The log contains diffstate metadata and finding categories — no full file contents or model prompts — and rotates at 1 MB. Nothing is uploaded.

Limitations

This is a best-effort assistive tool, not a guarantee. Treat findings as suggestions, not as a substitute for human code review, SAST/DAST, dependency scanning, or pen-testing. The reviewer can miss vulnerabilities, produce false positives, and may behave differently across codebases, languages, and model versions. No warranty is provided — use is subject to Anthropic's Commercial Terms.

Troubleshooting

Plugin doesn't seem to fire — check that ~/.claude/claude-security-guidance.md (or hook activity) shows in debug logs. Run Claude Code with --debug-file /tmp/claude/debug.txt and grep for security_reminder_hook. The plugin also writes its own log to ~/.claude/security/log.txt.

Review never finds anything — verify your API path works. On 3P providers, check SECURITY_REVIEW_MODEL is set to a provider-specific id (not a bare claude-opus-4-7). On LLM gateways, check the gateway's logs for POST /v1/messages traffic from the plugin.

Too many false positives — drop SECURITY_REVIEW_MODEL to a cheaper model (claude-sonnet-4-6) and re-evaluate; if precision is the priority, stay on Opus 4.7.

Want to silence a specific finding — add a comment to the line explaining why it's safe; the LLM reviewer treats inline justifications as exclusions. For systemic exclusions, document them in your claude-security-guidance.md.

Reporting issues

Open an issue on the security-guidance plugin repo with:

  • The Claude Code CLI version (claude --version)
  • Provider setup (1P / Bedrock / Vertex / LLM gateway / etc.)
  • A minimal repro diff
  • The relevant section of ~/.claude/security/log.txt