Compare commits

..

1 Commits

Author SHA1 Message Date
Mohamed Hegazy
12a5376e20 security-guidance: gate XSS pattern rules to JS-family files
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>
2026-05-28 23:07:53 -07:00
3 changed files with 21 additions and 139 deletions

View File

@@ -32,8 +32,6 @@ BUILD_FAILED = 3 # venv create or pip install raised/timed out
# 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
HOOK_PY_INCOMPATIBLE = 6 # hook interpreter is <3.10 — SDK syntax can't load
# here no matter how the venv was built. See #2071.
def _sdk_on_syspath() -> bool:
@@ -64,29 +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.
"""
# Honesty check (fixes the misleading NOOP_VENV in #2071): the SDK
# requires Python >=3.10 and uses 3.10+ syntax (match statements,
# PEP 604 unions). On a 3.9 hook interpreter we CANNOT import it no
# matter how the venv was built — llm.py runs in this same interpreter
# and the syntax-level import will SyntaxError. macOS ships 3.9.6 as
# the default `python3` and `/usr/bin` precedes Homebrew in PATH, so
# this case is the default state for a large share of macOS users.
#
# sg-python.sh now prefers python3.10+ binaries so most users won't
# reach this branch; the fallback to 3.9 is preserved for the
# pattern-warning hooks that don't need the SDK. Reporting
# HOOK_PY_INCOMPATIBLE here:
# (a) avoids 30-60s of wasted pip install,
# (b) avoids the lie where the venv_py probe says NOOP_VENV but the
# consumer import fails, and
# (c) gives telemetry a clean bucket to size the affected fleet.
if sys.version_info < (3, 10):
return (
HOOK_PY_INCOMPATIBLE,
"hook_py",
f"py_{sys.version_info[0]}.{sys.version_info[1]}",
)
if _sdk_on_syspath():
return NOOP_SYSTEM, "", ""
@@ -220,56 +195,6 @@ def main() -> tuple[int, str, str]:
sentinel.unlink(missing_ok=True)
def _maybe_emit_user_notice(outcome: int, pv: int) -> str | None:
"""Return a one-time user-visible notice when the agentic reviewer is
in a persistent broken state on this machine, or None if we've already
shown the notice for this plugin version (or shouldn't show one).
The marker file is plugin-version-keyed: a future plugin update can
re-notify if behavior changes (e.g. we ship out-of-process SDK in v3
and want to tell affected users it's fixed). Failures to write the
marker degrade to "skip the notice this session" so we don't spam
every SessionStart on a read-only home dir.
Currently only HOOK_PY_INCOMPATIBLE qualifies. BUILD_FAILED is
intentionally excluded — it covers transient causes (network failure,
pip registry hiccup, in-flight rebuild) where the next session may
succeed and a permanent notice would mislead.
"""
if outcome != HOOK_PY_INCOMPATIBLE:
return None
try:
state_dir = Path(
os.environ.get("SECURITY_WARNINGS_STATE_DIR")
or os.path.expanduser("~/.claude/security")
)
marker = state_dir / f".agentic_unavailable_notice_v{pv or 0}"
if marker.exists():
return None
state_dir.mkdir(parents=True, exist_ok=True)
# Write timestamp + Python version so the marker is self-documenting
# if a user goes looking. O_EXCL would be racier with no real win
# (two concurrent SessionStarts both showing the notice once is fine).
marker.write_text(
f"{time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime())} "
f"py={sys.version_info[0]}.{sys.version_info[1]}\n"
)
except OSError:
return None
return (
f"⚠ security-guidance plugin: the cross-file commit reviewer "
f"(layer 3 of 3 — catches IDOR, auth-bypass, cross-file SSRF) "
f"is unavailable in this environment. It requires Python ≥3.10, "
f"but the hook is running on "
f"{sys.version_info[0]}.{sys.version_info[1]}.\n\n"
f"Pattern checks and the single-shot LLM diff review are still "
f"active. To enable the deeper reviewer, install Python 3.10+ "
f"(e.g. `brew install python` on macOS) and restart Claude Code.\n\n"
f"This notice is shown once per plugin version. "
f"See: github.com/anthropics/claude-plugins-official/issues/2071"
)
if __name__ == "__main__":
# Tell the harness this is async — venv create + pip install can take
# 30-60s on a cold cache, well past the default sync hook timeout.
@@ -306,18 +231,4 @@ if __name__ == "__main__":
pv = _plugin_version_int()
if pv:
metrics["pv"] = pv
response: dict[str, object] = {"metrics": metrics}
# One-time user-visible notice when the agentic reviewer is dead on
# arrival. Uses hookSpecificOutput.additionalContext (SessionStart's
# supported channel for surfacing text to both the model and the user)
# plus systemMessage as a belt-and-suspenders. Marker-file-gated so
# this fires exactly once per plugin version per install — see
# _maybe_emit_user_notice.
notice = _maybe_emit_user_notice(outcome, pv)
if notice:
response["hookSpecificOutput"] = {
"hookEventName": "SessionStart",
"additionalContext": notice,
}
response["systemMessage"] = notice
print(json.dumps(response), flush=True)
print(json.dumps({"metrics": metrics}), flush=True)

View File

@@ -94,6 +94,9 @@ Only use exec() if you absolutely need shell features and the input is guarantee
},
{
"ruleName": "new_function_injection",
# JS-only construct: gate to JS/TS files so docs/.md and other prose
# mentioning "new Function" don't trip the warning.
"path_filter": lambda p: p.endswith(_JS_EXTS),
"substrings": ["new Function"],
"reminder": "\u26a0\ufe0f Security Warning: Using new Function() with string interpolation is a CODE INJECTION vulnerability. If any variable is concatenated or interpolated into the function body string, an attacker controlling that variable can execute arbitrary code. Use safe alternatives: for property access use obj[key] or array.reduce((o, k) => o[k], root); for computation use a safe expression parser. NEVER interpolate untrusted strings into new Function() bodies.",
},
@@ -107,16 +110,24 @@ Only use exec() if you absolutely need shell features and the input is guarantee
},
{
"ruleName": "react_dangerously_set_html",
# JS/TS-only (React); gate so .md docs / .py / .go files don't trip.
"path_filter": lambda p: p.endswith(_JS_EXTS),
"substrings": ["dangerouslySetInnerHTML"],
"reminder": "⚠️ Security Warning: dangerouslySetInnerHTML can lead to XSS vulnerabilities if used with untrusted content. Ensure all content is properly sanitized using an HTML sanitizer library like DOMPurify, or use safe alternatives.",
},
{
"ruleName": "document_write_xss",
# Browser DOM API: only meaningful in JS/TS source.
"path_filter": lambda p: p.endswith(_JS_EXTS),
"substrings": ["document.write"],
"reminder": "⚠️ Security Warning: document.write() can be exploited for XSS attacks and has performance issues. Use DOM manipulation methods like createElement() and appendChild() instead.",
},
{
"ruleName": "innerHTML_xss",
# Browser DOM API: only meaningful in JS/TS source. Closes FPs like
# docs/example HTML, playground/self-contained skills that hardcode
# innerHTML strings with zero user input (#410).
"path_filter": lambda p: p.endswith(_JS_EXTS),
"substrings": [".innerHTML =", ".innerHTML="],
"reminder": "⚠️ Security Warning: Setting innerHTML with untrusted content can lead to XSS vulnerabilities. Use textContent for plain text or safe DOM methods for HTML content. If you need HTML support, consider using an HTML sanitizer library such as DOMPurify.",
},
@@ -217,11 +228,15 @@ Additionally, validate user inputs:
},
{
"ruleName": "outerHTML_xss",
# Browser DOM API: only meaningful in JS/TS source.
"path_filter": lambda p: p.endswith(_JS_EXTS),
"substrings": [".outerHTML =", ".outerHTML="],
"reminder": "⚠️ Security Warning: Use textContent or sanitize with DOMPurify. outerHTML assignment is an XSS sink equivalent to innerHTML.",
},
{
"ruleName": "insertAdjacentHTML_xss",
# Browser DOM API: only meaningful in JS/TS source.
"path_filter": lambda p: p.endswith(_JS_EXTS),
"substrings": [".insertAdjacentHTML("],
"reminder": "⚠️ Security Warning: Use insertAdjacentText() or sanitize with DOMPurify. insertAdjacentHTML is an XSS sink.",
},

View File

@@ -47,65 +47,21 @@ fi
probe() {
# $1..N: the interpreter command (may be multi-word like `py -3`)
# Writes "<major>.<minor>" to stdout and exits 0 iff at least Python 3.
"$@" -c 'import sys; print(f"{sys.version_info[0]}.{sys.version_info[1]}")' 2>/dev/null
# Probe writes the major version to stdout and exits 0 iff it's >=3.
"$@" -c 'import sys; print(sys.version_info[0])' 2>/dev/null
}
# True iff arg is a "M.m" version string >= 3.10. claude_agent_sdk requires
# Python >= 3.10; below that, pip install fails ("No matching distribution")
# and the LLM-powered review (Stop / commit / push) silently no-ops while
# pattern checks (PostToolUse regex) keep working. macOS ships 3.9.6 as the
# default `python3` on current versions, so this guard matters in practice.
# See anthropics/claude-plugins-official#2071.
is_sdk_compatible() {
case "$1" in
3.1[0-9]|3.[2-9][0-9]|[4-9].*|[1-9][0-9].*) return 0 ;;
*) return 1 ;;
esac
}
# Pass 1 — try minor-versioned binaries in descending order. These are only
# present if the user explicitly installed them (Homebrew / python.org / pyenv),
# so picking one here always upgrades over the system `python3`. Highest
# available wins; the user doesn't have to PATH-prefer it.
for cmd in "python3.13" "python3.12" "python3.11" "python3.10"; do
v=$(probe "$cmd") || continue
if is_sdk_compatible "$v"; then
exec "$cmd" "$@"
fi
done
# Pass 2 — bare interpreters, but only if SDK-compatible. Covers Linux distros
# that ship 3.10+ as the default `python3`, and Windows where `python` /
# `py -3` resolves to the user's python.org install.
for cmd in "python3" "python" "py -3"; do
# Word-split intentionally so `py -3` works
# shellcheck disable=SC2086
v=$(probe $cmd) || continue
if is_sdk_compatible "$v"; then
if [ "$v" = "3" ]; then
# shellcheck disable=SC2086
exec $cmd "$@"
fi
done
# Pass 3 — fallback to any Python 3, even <3.10. Pattern-based checks
# (PostToolUse regex on Edit/Write) only need 3.6+ and are useful on their
# own; the SDK-dependent paths will detect the version mismatch and degrade
# inside the Python code. Without this fallback, the entire plugin would
# stop working on default macOS, which is a regression vs today.
for cmd in "python3" "python" "py -3"; do
# shellcheck disable=SC2086
v=$(probe $cmd) || continue
# Accept anything that successfully reported a "M.m" string.
case "$v" in
[0-9]*.[0-9]*)
# shellcheck disable=SC2086
exec $cmd "$@"
;;
esac
done
echo "security-guidance: no working Python 3 interpreter found." >&2
echo " tried: python3.13, python3.12, python3.11, python3.10, python3, python, py -3" >&2
echo " tried: python3, python, py -3" >&2
echo " on Windows, install Python from https://python.org (NOT the Microsoft Store)" >&2
echo " on macOS, install Python 3.10+ via Homebrew (\`brew install python\`)" >&2
exit 1