From 9dbc70fd6579f55b0e4aa0556ea42bf907d325bc Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Tue, 21 Jul 2026 03:43:27 +0000
Subject: [PATCH] fix(upload_security): eliminate FPs for static serving,
 logging, and generic lists

---
 CHANGELOG.md                                  |  3 +++
 .../extractors/upload_security.py             | 19 ++++++++++++++-----
 tests/test_hooks.py                           |  2 ++
 tests/test_pentest_regressions.py             | 10 ++++++++++
 4 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4a15b12..9a38a6a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+### Fixed
+- **UploadSecurityExtractor**: Tightened regexes to eliminate false positives around static asset serving, originalname logging, and generic blocklists (e.g., users or emails instead of file extensions).
+
 ## [0.11.0] — 2026-07-11
 
 Distribution & integration round — reach every agent host, run as a local guardrail, and compose with
diff --git a/src/websec_validator/extractors/upload_security.py b/src/websec_validator/extractors/upload_security.py
index dc0c2ba..d556a79 100644
--- a/src/websec_validator/extractors/upload_security.py
+++ b/src/websec_validator/extractors/upload_security.py
@@ -19,17 +19,25 @@
 
 UPLOAD_MARK = re.compile(r"\bmulter\b|req\.files?\b|multipart/form-data|formidable|busboy|fileFilter"
                          r"|uploadMedia|presignedPost|\.upload\s*\(", re.I)
-DENY_LIST = re.compile(r"isExecutableMimeType|blockedMimeTypes|blacklist|deny[_-]?list|forbidden(?:Ext|Mime)|isBlocked", re.I)
+DENY_LIST = re.compile(r"isExecutableMimeType|blockedMimeTypes|forbidden(?:Ext|Mime)|isBlocked(?:File|Ext|Mime)?|"
+                       r"(?:file|mime|ext|upload)[_-]?(?:blacklist|denylist|blocklist)|"
+                       r"(?:blacklist|denylist|blocklist)[_-]?(?:file|mime|ext|type|upload|extensions?)", re.I)
 # positive allow-list, ideally by sniffed bytes (file-type / magic detection), not by declared type.
 # `ACCEPTED_*` / `acceptedMimeTypes` is the same intent under a different name (was missed → FP).
 ALLOW_LIST = re.compile(r"isAllowedMediaType|allowedMimeTypes|allow[_-]?list|whitelist|ALLOWED_(?:MIME|TYPES|EXT)"
                         r"|ACCEPTED_(?:MIME|TYPES?|EXT)|accepted(?:Mime|File|Content)?(?:Types?|Extensions?)"
                         r"|\bfile-type\b|fileTypeFrom|magic[_-]?byte|detectContentType|\.fromBuffer\b|sniff", re.I)
 KEY_FROM_NAME = re.compile(r"(?:Key|key|path|filename|filepath|destination|filename\s*\()\s*[:=(][^;\n]{0,90}"
-                           r"\b(?:originalname|originalName|file\.name)\b"
-                           r"|`[^`]*\$\{[^}]*\boriginalname\b[^}]*\}[^`]*`", re.I)
+                           r"(?:\b(?:originalname|originalName|file\.name)\b|`[^`]*\$\{[^}]*\b(?:originalname|originalName|file\.name)\b[^}]*\}[^`]*`)", re.I)
 TRUST_CLIENT_MIME = re.compile(r"(?:req\.files?\.[\w$.]*\.|\bfile\.)mimetype\b|headers\[['\"]content-type['\"]\]", re.I)
-ACCEPT_SVG = re.compile(r"image/svg\+xml|['\"]svg['\"]", re.I)
+ACCEPT_SVG = re.compile(r"image/svg\+xml|['\"]\.?svg['\"]\s*(?:\]|,|===?|:)", re.I)
+
+# Remove known static asset serves before searching for SERVE_FILE
+STATIC_ASSET_IGNORE = re.compile(r"(?:res\.sendFile|createReadStream|fs\.createReadStream)\s*\([^)]*?"
+                                 r"(?:['\"]/?(?:favicon\.ico|logo\.png|robots\.txt|index\.html)['\"]|"
+                                 r"['\"][^'\"]*(?:/public/|/build/|/dist/|/static/)[^'\"]*['\"]|"
+                                 r"__dirname[^)]*?['\"](?:client|build|public|dist|static|index\.html)['\"])[^)]*\)", re.I)
+
 # file-serving: streaming a STORED/PROXIED object back to the client. Tightened to genuine
 # file-bytes sinks — the old rule matched a bare `getObject` token (a local coercion helper) and a
 # Prometheus `res.set('Content-Type', registry.contentType)` (the /metrics endpoint), both FPs.
@@ -76,7 +84,8 @@ def extract(self, ctx: RepoContext, facts: dict) -> dict:
                     findings.append({"severity": "MEDIUM", "kind": "upload-accepts-svg", "file": rel,
                                      "detail": "`image/svg+xml` is accepted — SVG can carry inline <script> and renders "
                                                "as HTML. Drop SVG from the allow-list, or sanitize + serve as attachment."})
-            if SERVE_FILE.search(text) and not NOSNIFF.search(text) and not ATTACHMENT.search(text):
+            text_serve = STATIC_ASSET_IGNORE.sub("", text)
+            if SERVE_FILE.search(text_serve) and not NOSNIFF.search(text) and not ATTACHMENT.search(text):
                 serve_files.append(rel)
                 findings.append({"severity": "HIGH", "kind": "serve-no-nosniff", "file": rel,
                                  "detail": "A stored/proxied file is served with no `X-Content-Type-Options: nosniff` "
diff --git a/tests/test_hooks.py b/tests/test_hooks.py
index 17e1a92..2cf7262 100644
--- a/tests/test_hooks.py
+++ b/tests/test_hooks.py
@@ -27,6 +27,8 @@ def _init_repo(root: Path) -> None:
     subprocess.run(["git", "init", "-q"], cwd=root, check=True)
     subprocess.run(["git", "config", "user.email", "t@example.com"], cwd=root, check=True)
     subprocess.run(["git", "config", "user.name", "t"], cwd=root, check=True)
+    # Ensure global core.hooksPath=/dev/null doesn't disable hook execution for the temp repo
+    subprocess.run(["git", "config", "core.hooksPath", ".git/hooks"], cwd=root, check=True)
 
 
 @unittest.skipUnless(HAVE_GIT, "git not available")
diff --git a/tests/test_pentest_regressions.py b/tests/test_pentest_regressions.py
index 734194f..9fd65ce 100644
--- a/tests/test_pentest_regressions.py
+++ b/tests/test_pentest_regressions.py
@@ -345,6 +345,16 @@ def test_safe_upload_and_serve_clean(self):
                         " res.setHeader('Content-Disposition','attachment'); getObject(key).pipe(res); }\n"}), {})
         self.assertEqual(out["findings"], [])
 
+    def test_upload_fp_patterns_ignored(self):
+        out = UploadSecurityExtractor().extract(repo({
+            "serve_spa.js": "app.get('*', (req, res) => res.sendFile(path.join(__dirname, 'client/build', 'index.html')));",
+            "serve_static.js": "app.get('/logo.png', (req, res) => res.sendFile(__dirname + '/public/logo.png'));",
+            "upload_log.js": "const upload = multer(); console.log(`Received upload: ${req.file.originalname}`);",
+            "upload_svg_var.js": "const upload = multer(); const format = 'svg'; // benign string literal",
+            "upload_user_blacklist.js": "const upload = multer(); const blacklist = ['spam_user', 'abusive_user'];"
+        }), {})
+        self.assertEqual(out["findings"], [])
+
 
 class PiiExposureTests(unittest.TestCase):  # N4
     MASKER = "export function maskContactPii(c){ return {...c, phone: mask(c.phone)}; }\n"