Add TLS-validation and cookie-security detectors (5 new rules)#1
Merged
Merged
Conversation
Add two new high-signal, AST-anchored detectors covering vulnerability classes the scanner previously missed entirely: - InsecureTransportDetector - BCR-TLS-001: rejectUnauthorized: false on any HTTPS/TLS options object - BCR-TLS-002: process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0" kill switch - CookieSecurityDetector (res.cookie / express-session / cookie-session) - BCR-COOKIE-001: missing/false httpOnly on session/auth cookies - BCR-COOKIE-002: missing/false secure flag (heuristic) - BCR-COOKIE-003: missing sameSite / sameSite:none (heuristic) Cookie checks are library-default-aware: a missing httpOnly is only flagged for res.cookie (insecure default), not express-session/ cookie-session (which default to httpOnly:true). Wires both detectors into the analyzer, registers the rules with CWE/OWASP metadata, and adds 14 unit tests. Full suite: 300/301 pass (the one failure is a pre-existing gitignored-fixture test unrelated to this change).
tests/phase3AndAnalyzerRegression.test.ts expects scan-scope-fixtures/project-root/ignored-by-gitignore.ts to exist on disk while being matched by the fixture-local .gitignore. The file was never committed (it is gitignored), so fresh checkouts — including CI — had filesAnalyzed=0 and the 'explicit file targets are still analyzable even when gitignored' test failed. Force-add the fixture (git add -f) so it exists in checkouts. Broad directory scans still skip it (the scanner honors the fixture .gitignore), while explicit file targets analyze it directly. Full suite now 301/301.
Make scan-time errors actionable instead of a bare count: - Add JSONReporter.remediationFor(type) mapping every RuntimeIssueType to a concrete fix (parse failure, detector crash, invalid target, write failure, fatal). - Console summary and text report now print each runtime issue as What / Where / Fix. JSON-log output gains runtimeIssueDetails[] with the same remediation per issue. Harden InsecureTransportDetector (BCR-TLS-001): also flag the falsy non-boolean variants rejectUnauthorized: 0 / '0' / 'false' / '', while leaving non-literal values (e.g. isProd) alone to avoid false positives. Adds 6 tests (TLS falsy/dynamic variants + runtime-issue remediation contract). Full suite: 307/307.
There was a problem hiding this comment.
Pull request overview
This PR expands the backend scanner’s AST-based coverage to detect insecure TLS validation and insecure session/auth cookie flags, and enhances runtime issue reporting so scan-time failures include actionable remediation.
Changes:
- Added 2 new detectors (
InsecureTransportDetector,CookieSecurityDetector) and registered 5 new rules (TLS + cookie security). - Enhanced reporting to include per-runtime-issue “What / Where / Fix” in console + text output, and detailed remediation in JSON logs.
- Added unit tests for the new detectors and for runtime-issue remediation; restored a missing scan-scope fixture.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/transportCookieDetectors.test.ts | Adds unit coverage for TLS and cookie detectors. |
| tests/runtimeIssueRemediation.test.ts | Pins the new “actionable fix per runtime issue” contract. |
| src/rules/registry.ts | Registers TLS + cookie rules with severity/CWE/OWASP metadata. |
| src/reporter.ts | Adds remediation mapping and renders runtime issues with actionable fixes (console/text/JSON-log). |
| src/detectors/insecureTransportDetector.ts | Implements detection for rejectUnauthorized: false and NODE_TLS_REJECT_UNAUTHORIZED="0". |
| src/detectors/cookieSecurityDetector.ts | Implements detection for missing/false httpOnly/secure/sameSite across supported cookie-setting APIs. |
| src/analyzer.ts | Wires the two new detectors into the analyzer pipeline. |
| scan-scope-fixtures/project-root/ignored-by-gitignore.ts | Restores a gitignored fixture used by scan-scope tests. |
| README.md | Documents new TLS + cookie-security coverage areas. |
| CHANGELOG.md | Notes new detectors/rules and improved runtime-issue reporting under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- CookieSecurityDetector: a dynamic/non-literal cookie name is no longer treated as sensitive, so res.cookie(nameVar, ...) no longer flags every dynamically-named cookie for a missing flag (false-positive class). Explicit httpOnly:false still fires regardless of name. - sameSite:false now maps to 'absent' (missing attribute, TENTATIVE) rather than 'none' (FIRM), matching Express semantics where false means 'do not set SameSite'. - transportCookieDetectors test: clean up temp files in afterAll so bcr-tlscookie-* no longer accumulate in the OS temp dir. Adds tests for the dynamic-name FP guard and sameSite:false. 310/310.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Expands the scanner to detect two vulnerability classes it previously missed entirely, and makes scan-time errors actionable. All detection is AST-anchored for high precision.
New detectors & rules
InsecureTransportDetector— disabled TLS certificate validation (MITM):BCR-TLS-001(CRITICAL, CWE-295) —rejectUnauthorized: false(and the falsy0/'0'/'false'/''variants) on any HTTPS/TLS options object. Read byhttps.request/https.Agent/tls.connectand forwarded by axios/got/node-fetch/request agents. Non-literal values (e.g.isProd) are left alone.BCR-TLS-002(CRITICAL, CWE-295) —process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"(dot or bracket access), the process-global kill switch that disables validation for every TLS connection.CookieSecurityDetector— insecure session/auth cookie flags. Understandsres.cookie(),express-session, andcookie-session:BCR-COOKIE-001(HIGH, CWE-1004) — missing/falsehttpOnly→ XSS session theft.BCR-COOKIE-002(MEDIUM, CWE-614, heuristic) — missing/falsesecure→ cleartext transmission.BCR-COOKIE-003(LOW, CWE-1275, heuristic) — missingsameSite/sameSite: 'none'→ CSRF.Cookie checks are library-default-aware: a missing
httpOnlyis only flagged forres.cookie(whose default is insecure), not forexpress-session/cookie-session(which default tohttpOnly: true).Actionable scan-error reporting
runtimeIssueDetails[]array with the same remediation.JSONReporter.remediationFor(type)maps everyRuntimeIssueTypeto an actionable next step.Changes
src/detectors/insecureTransportDetector.ts,src/detectors/cookieSecurityDetector.ts— new detectorssrc/analyzer.ts— wire both into the detector pipelinesrc/rules/registry.ts— register the 5 rules with CWE/OWASP metadatasrc/reporter.ts— actionable runtime-issue rendering +remediationFortests/transportCookieDetectors.test.ts,tests/runtimeIssueRemediation.test.ts— unit testsscan-scope-fixtures/project-root/ignored-by-gitignore.ts— restore a missing fixture that broke a pre-existing test in fresh checkoutsREADME.md,CHANGELOG.md— document the new coverageTesting
npx tsc --noEmit— clean