From 12a5376e205c8c519a53ee06115bd279a1e121d1 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Thu, 28 May 2026 23:07:53 -0700 Subject: [PATCH] security-guidance: gate XSS pattern rules to JS-family files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- plugins/security-guidance/hooks/patterns.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/plugins/security-guidance/hooks/patterns.py b/plugins/security-guidance/hooks/patterns.py index b7f6f10a..8e749cad 100644 --- a/plugins/security-guidance/hooks/patterns.py +++ b/plugins/security-guidance/hooks/patterns.py @@ -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.", },