diff --git a/CHANGELOG.md b/CHANGELOG.md index db53af9..70bd1a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -349,6 +349,11 @@ No behavior change. approves each probe). Reduces dual-use false-positive pauses. The README hero line now leads with a defensive, ownership-asserting phrasing and the PyPI install. **No behavior change** — wording/order only; the live-fire confirmation checkpoint is intentionally preserved. +## [Unreleased] + +### Fixed +- **UploadSecurityExtractor False Positives**: Refined `KEY_FROM_NAME` regex in `upload_security.py` to prevent false positive `upload-key-from-filename` findings on safe metadata assignments, original name logging, and comments containing the original file name. Added regression tests to `FalsePositiveRegressionTests`. + ## [0.6.2] — 2026-06-12 diff --git a/src/websec_validator/extractors/upload_security.py b/src/websec_validator/extractors/upload_security.py index dc0c2ba..e3e8b69 100644 --- a/src/websec_validator/extractors/upload_security.py +++ b/src/websec_validator/extractors/upload_security.py @@ -25,9 +25,9 @@ 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) +KEY_FROM_NAME = re.compile(r"(?:Key|key|path|filename|filepath|destination)\s*[:=]\s*(?:(?!(?:\w+|['\"]\w+['\"])\s*:|\/\/|\/\*)[^;\n])*?\b(?:originalname|originalName|file\.name)\b" + r"|\b(?:path|filename|destination)\s*\(\s*(?:(?!(?:\w+|['\"]\w+['\"])\s*:|\/\/|\/\*)[^;\n])*?\b(?:originalname|originalName|file\.name)\b" + r"|(?:Key|key|path|filename|filepath|destination)\s*[:=]\s*`[^`]*\$\{[^}]*\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) # file-serving: streaming a STORED/PROXIED object back to the client. Tightened to genuine diff --git a/tests/test_pentest_regressions.py b/tests/test_pentest_regressions.py index 734194f..9775a4a 100644 --- a/tests/test_pentest_regressions.py +++ b/tests/test_pentest_regressions.py @@ -525,6 +525,29 @@ 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_key_from_filename_fps(self): + from websec_validator.extractors.upload_security import UploadSecurityExtractor + # False positives: + # 1. safe assignment to Metadata (not Key) + # 2. logging originalname + # 3. safe Key assignment with a comment containing originalname + # 4. originalname used in a totally separate path variable + snippets = { + "src/fp_metadata.js": "const upload = multer(); const params = { Key: uuid(), Metadata: { 'original-name': file.originalname } };", + "src/fp_log.js": "const upload = multer(); console.log(`Received file: ${req.file.originalname}`);", + "src/fp_comment.js": "const upload = multer(); const params = { Key: uuid(), // not file.originalname\n };", + "src/fp_other.js": "const upload = multer(); const dest = path.join('uploads', uuid()); const meta = path.join('meta', file.originalname);", + "src/fp_string.js": "const upload = multer(); const safeKey = crypto.randomUUID(); const msg = `Uploaded ${req.file.originalname} as ${safeKey}`;", + "src/fp_unrelated.js": "const upload = multer(); res.json({ success: true, originalname: req.file.originalname, savedAs: uuid() });", + "src/fp_array.js": "const upload = multer(); const uploads = []; uploads.push(file.originalname);" + } + for name, content in snippets.items(): + out = UploadSecurityExtractor().extract(repo({name: content}), {}) + self.assertFalse( + any(f["kind"] == "upload-key-from-filename" for f in out.get("findings", [])), + f"False positive triggered for {name}" + ) + 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",