fix: reduce upload_security false positives

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+### Fixed
+- **Upload Security False Positives** in `upload_security.py` where static file serving via `res.sendFile(__dirname)` or literal paths, and valid allowlist checking using variable names like `validTypes`, falsely triggered alerts. Also tightened the `image/svg+xml` check to avoid generic `'svg'` strings.
+
 ### Added
 
 - **No-Row-Level-Security detection** (`missing-rls` class, in `schemas.py` + the ledger) — committed

src/websec_validator/extractors/upload_security.py

@@ -24,17 +24,19 @@
 # `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"|(?:valid|supported)(?:Mime)?Types"
                         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)
 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", 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.
 SERVE_FILE = re.compile(r"res\.sendFile|\.sendFile\s*\(|\.getObject\s*\(|createReadStream|proxyMedia"
                         r"|streamObject|\.pipe\s*\(\s*res\b|fs\.createReadStream", re.I)
+STATIC_SERVE_CONTEXT = re.compile(r"__dirname|process\.cwd\(\)")
 NOSNIFF = re.compile(r"nosniff", re.I)
 # `Content-Disposition: attachment` fully defeats the MIME-sniff→stored-XSS vector (the browser
 # downloads instead of rendering), so a serve site that sets it is SAFE even without nosniff.
@@ -76,7 +78,19 @@ 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):
+            serve_match = SERVE_FILE.search(text)
+            is_serve_no_nosniff = False
+            if serve_match and not NOSNIFF.search(text) and not ATTACHMENT.search(text):
+                # Exclude static file serving logic for sendFile without dynamic user inputs
+                # A file is flagged if *any* serve sink is NOT purely static.
+                for m in SERVE_FILE.finditer(text):
+                    # Check context around the match for static markers
+                    context = text[max(0, m.start()-20):m.start()+100]
+                    if not STATIC_SERVE_CONTEXT.search(context):
+                        is_serve_no_nosniff = True
+                        break
+
+            if is_serve_no_nosniff:
                 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` "

tests/test_pentest_regressions.py

@@ -525,6 +525,40 @@ def test_classes_reach_ledger_with_citations(self):
 class FalsePositiveRegressionTests(unittest.TestCase):
     """Lock in the accuracy fixes found by dogfooding on real repos (68 → 11 new-group findings)."""
 
+
+    def test_upload_security_mixed_serve(self):
+        # A file with both static and dynamic serves must still be flagged
+        ext = UploadSecurityExtractor()
+        code_mixed = '''
+        app.get('/logo', (req, res) => res.sendFile(path.join(__dirname, 'logo.png')));
+        app.get('/download', (req, res) => res.sendFile(req.query.path));
+        '''
+        out_mixed = ext.extract(repo({"app.js": code_mixed}), {})
+        self.assertTrue(any(f["kind"] == "serve-no-nosniff" for f in out_mixed["findings"]))
+
+    def test_upload_security_fps(self):
+        # res.sendFile with __dirname or process.cwd() or literal string is a static file serve, not user-generated upload serve
+        ext = UploadSecurityExtractor()
+        code_static = "app.get('/logo', (req, res) => res.sendFile(path.join(__dirname, 'public/logo.png')));"
+        out_static = ext.extract(repo({"app.js": code_static}), {})
+        self.assertEqual(out_static["findings"], [])
+
+        code_static2 = "app.get('/logo', (req, res) => res.sendFile(process.cwd() + '/build/index.html'));"
+        out_static2 = ext.extract(repo({"app.js": code_static2}), {})
+        self.assertEqual(out_static2["findings"], [])
+
+        # Using variables like validTypes or supportedMimeTypes should count as an allowlist
+        code_allow = "const upload = multer(); const supportedMimeTypes = ['image/png']; if (!supportedMimeTypes.includes(req.file.mimetype)) {}"
+        out_allow = ext.extract(repo({"app.js": code_allow}), {})
+        self.assertFalse(any(f["kind"] == "upload-trusts-client-mime" for f in out_allow["findings"]))
+
+        # 'svg' string literal should not trigger if it's not actually the mimetype checking
+        code_svg = "const upload = multer(); const icon = 'svg';"
+        out_svg = ext.extract(repo({"app.js": code_svg}), {})
+        self.assertFalse(any(f["kind"] == "upload-accepts-svg" for f in out_svg["findings"]))
+
+
+
     def test_aws_sam_and_cdk_out_dirs_skipped(self):
         # regression: vendored SDK code under an AWS SAM build dir must not be scanned
         ctx = repo({"aws/.aws-sam/deps/abc/models.py": "account_number='x'\n",