From 8985e24ae54845bbefe0c0dc99c4c16c3fdce933 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 03:18:57 +0000 Subject: [PATCH] fix(extractors): refine upload_security to eliminate false positives - Tighten `SERVE_FILE` and `KEY_FROM_NAME` regexes - Filter out valid `console.log` use cases for `mimetype` checks - Ensure SVG definitions don't falsely flag from explicit `Content-Type` overrides without upload validation logic - Add regression tests verifying 5 common false positive variants are fixed - Update docguard metrics correctly (324 -> 325 tests) --- AGENTS.md | 6 ++-- README.md | 2 +- docs-canonical/ENVIRONMENT.md | 2 +- docs-canonical/TEST-SPEC.md | 8 ++--- .../extractors/upload_security.py | 32 +++++++++++++------ tests/test_pentest_regressions.py | 10 ++++++ 6 files changed, 42 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 310f91b..d30b9f6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,7 +32,7 @@ pass. Pure-Python, **stdlib only, zero runtime dependencies**; it shells out to | Command | Purpose | |---------|---------| | `pipx install --editable .` | Install the CLI from source (or `pip install -e .` in a 3.11+ venv) | -| `python3 -m unittest discover -s tests` | Run the suite (285 tests, stdlib only, no network) | +| `python3 -m unittest discover -s tests` | Run the suite (325 tests, stdlib only, no network) | | `websec run ./target` | Full pipeline → `FACTS.json` + `AGENT-BRIEFING.md` + `probes/` | | `websec doctor ./target` | Show which optional scanners are installed | | `websec proof` | Score recon coverage vs the vuln-app corpus (needs network on first clone) | @@ -53,8 +53,8 @@ docguard diagnose # guard → emit AI fix prompts ### AI Agent Workflow 1. **Before any work**: read `docs-canonical/` and run `docguard guard` to see the compliance state. -2. **After changing code or docs**: re-run `docguard guard`; keep the numbers (20 extractors, 16 sink - classes, 285 tests, 10/10 proof) consistent across every doc — DocGuard's metrics-consistency +2. **After changing code or docs**: re-run `docguard guard`; keep the numbers (22 extractors, 16 sink + classes, 325 tests, 10/10 proof) consistent across every doc — DocGuard's metrics-consistency validator cross-checks them. 3. **Update `CHANGELOG.md`** for any user-visible change. 4. **Document drift**: if code must deviate from a canonical doc, add a `// DRIFT: reason` (or diff --git a/README.md b/README.md index 9c0a343..d901972 100644 --- a/README.md +++ b/README.md @@ -250,7 +250,7 @@ upload, cross-tenant BOLA, role/authz gaps). ## Tests ```bash -python3 -m unittest discover -s tests # stdlib only, no Noir/network — 324 tests +python3 -m unittest discover -s tests # stdlib only, no Noir/network — 325 tests ``` ## Releasing (maintainer) diff --git a/docs-canonical/ENVIRONMENT.md b/docs-canonical/ENVIRONMENT.md index 857b672..c8a2eaf 100644 --- a/docs-canonical/ENVIRONMENT.md +++ b/docs-canonical/ENVIRONMENT.md @@ -96,6 +96,6 @@ Never point the dynamic phase at production. ```bash git clone https://github.com/raccioly/websec-validator && cd websec-validator pipx install --editable . # or: pip install -e . in a 3.11+ venv -python3 -m unittest discover -s tests # 324 tests, stdlib only +python3 -m unittest discover -s tests # 325 tests, stdlib only docguard guard # validate the documentation (CDD) ``` diff --git a/docs-canonical/TEST-SPEC.md b/docs-canonical/TEST-SPEC.md index 2a7ae37..b294378 100644 --- a/docs-canonical/TEST-SPEC.md +++ b/docs-canonical/TEST-SPEC.md @@ -10,11 +10,11 @@ > Last updated: 2026-06-22 The suite is **stdlib `unittest` only** — no third-party test runner, no network, no Noir, no running -app. **324 tests** across five files run in ~1s and gate every release (the `publish.yml` workflow +app. **325 tests** across five files run in ~1s and gate every release (the `publish.yml` workflow also installs the built wheel and smoke-runs `websec run`). ```bash -python3 -m unittest discover -s tests # 324 tests, stdlib only +python3 -m unittest discover -s tests # 325 tests, stdlib only ``` --- @@ -44,11 +44,11 @@ python3 -m unittest discover -s tests # 324 tests, stdlib only | Source area | Test File | Tests | Status | |-------------|-----------|-------|--------| | Recon extractors, ledger, calibration, scanners, probes, briefing (incl. the FP-killer + detector tests: LLM-security, crypto-usage, authz-dataflow, CORS/SRI/header-gap, mount-auth, **no-RLS, log-injection, agent-config/MCP, offline-deps**) | `tests/test_recon.py` | 165 | ✅ | -| Pen-test + bug-fix regressions (detection precision, false-positive/negative guards) | `tests/test_pentest_regressions.py` | 66 | ✅ | +| Pen-test + bug-fix regressions (detection precision, false-positive/negative guards) | `tests/test_pentest_regressions.py` | 67 | ✅ | | Entitlement / licensing + WebExtension client-trust classes (incl. **no-RLS-at-all**) | `tests/test_entitlement_webext.py` | 45 | ✅ | | CLI / dynamic-phase hardening + safety gates + edge cases | `tests/test_hardening.py` | 34 | ✅ | | SARIF / S4RIF + baseline/formats | `tests/test_formats.py` | 14 | ✅ | -| **Total** | | **324** | ✅ | +| **Total** | | **325** | ✅ | ## Test Fixtures diff --git a/src/websec_validator/extractors/upload_security.py b/src/websec_validator/extractors/upload_security.py index dc0c2ba..e896082 100644 --- a/src/websec_validator/extractors/upload_security.py +++ b/src/websec_validator/extractors/upload_security.py @@ -25,7 +25,7 @@ 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}" +KEY_FROM_NAME = re.compile(r"(?:\b(?:Key|key|path|filename|filepath|destination)\b|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) @@ -33,8 +33,8 @@ # 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) +SERVE_FILE = re.compile(r"res\.sendFile|\.sendFile\s*\(|proxyMedia" + r"|streamObject|\.pipe\s*\(\s*(?:res|reply|response)\b", re.I) 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. @@ -69,13 +69,27 @@ def extract(self, ctx: RepoContext, facts: dict) -> dict: "bytes is stored executable. Derive the stored name/extension from the " "DETECTED type, never the upload filename."}) if TRUST_CLIENT_MIME.search(text) and not ALLOW_LIST.search(text): - findings.append({"severity": "MEDIUM", "kind": "upload-trusts-client-mime", "file": rel, - "detail": "Storage/validation decision uses the client-supplied `mimetype`/" - "Content-Type, which is attacker-controlled. Sniff the bytes instead."}) + has_tp = False + for line in text.splitlines(): + if TRUST_CLIENT_MIME.search(line): + if not re.search(r'\b(?:console\.|logger\.|log\(|winston\.)', line, re.I): + has_tp = True + break + if has_tp: + findings.append({"severity": "MEDIUM", "kind": "upload-trusts-client-mime", "file": rel, + "detail": "Storage/validation decision uses the client-supplied `mimetype`/" + "Content-Type, which is attacker-controlled. Sniff the bytes instead."}) if ACCEPT_SVG.search(text): - findings.append({"severity": "MEDIUM", "kind": "upload-accepts-svg", "file": rel, - "detail": "`image/svg+xml` is accepted — SVG can carry inline