[davidstrouk]

What

The built-in scanner graded this repo F / 0 on 17 findings, every one a false positive. Both root causes are fixed; a self-scan now returns 0 findings, score 100, grade A.

Root cause 1 — no test/fixture exclusion (13 of 17)

walk() had no exclusion for tests, fixtures, specs or mocks, so apps/web/tests/fixtures/vulnerable-app/{app.js,config.js} — files deliberately full of fake AWS keys, GitHub tokens and eval() — were walked as real source.

The exclusion matches the path relative to the scan root, not the absolute path. security.test.ts passes the fixture directory in as the scan root, so an absolute-path filter would make walk() return nothing there and take those tests red. walk() lost the root on recursion, so it is now threaded through as walk(dir, root = dir).

examples? is deliberately not excluded — example directories do leak real keys.

Root cause 2 — the scanner detected its own documentation (4 of 17)

Not the regex literals: the human-readable strings. title: 'Dangerous eval() usage' contains the literal text eval(), which /\beval\s*\(/ matches.

This is a general false-positive class that also fires on linters, security tooling, OWASP material and any XSS tutorial, so it's fixed generally rather than special-cased. New strip-code.ts blanks comments and string literals before the code-construct patterns run — space-for-space with newlines preserved, so line and column numbers are unchanged. ${...} interpolations stay visible because they are real code.

Two deliberate carve-outs

• The SQL-injection pattern is exempt (matchesStringContent: true). The fixture's SQL is const query = `SELECT * FROM users WHERE id = ${userId}` — the keyword lives inside a template literal, so blanking strings would break detection. It runs against comment-stripped-only text instead.
• secrets.ts is untouched for the same reason: a hardcoded key lives inside a string literal.

Stripping is applied only to .js/.jsx/.mjs/.cjs/.ts/.tsx — the syntax the stripper actually understands. Other extensions match raw, as before.

Why both fixes were needed

packages/shared/src/scoring.ts deducts 25 per critical, 15 per high, floor 0; getGrade() returns F below 50.

	findings	score	grade
before	17	0	F
walk fix only	4 (2 critical + 2 high)	20	F
both fixes	0	100	A

Tests

walk.test.ts is new (3 cases, including the scan-root trap in both directions). security.test.ts goes 4 → 10 cases, adding a regression that runs securityCheck against the repo root and asserts zero findings — that pins both bugs at once.

The line-number-preservation test was confirmed to actually fail by temporarily mutating the stripper to collapse newlines, then restored.

pnpm turbo test    54 pass (api 10, web 44) — was 45
tsc --noEmit       clean
biome              clean on changed files

Not in scope

Follow-ups are written up locally in docs/plans/HANDOFF-scanner-followups.md (gitignored). The short version: production scans are delegated to the Railway API (betterleaks/opengrep/trivy) whenever SCANNER_BACKEND_URL is set, so this fix only changes the public grade if the built-in path is the one running; the SQL-injection pattern fires on ordinary English (sudo sbuild-update ... ${DIST} grades critical); and scores are bimodal — every scan in the DB is exactly 100 or exactly 0.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BYMoc47De5t8omQ3wPTZnV

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a90fa1190a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Parse regex literals before stripping string delimiters

When valid JavaScript contains quote or backtick characters inside a regex literal, this branch treats them as the start of a string/template and blanks executable code that follows. For example, after const markdown = /([^]+)/g;, the stripper remains in template mode and removes a later eval(input)`, so the newly filtered security scan silently misses a real finding. Regex literals need to be recognized or parsed before applying these delimiter transitions.

Useful? React with 👍 / 👎.

[davidstrouk]

Fixed in 9810834. The stripper now reads a regex literal before the quote rules, so a backtick inside one does not start template mode. A / is division when the previous token is a value, so total / count stays code.

[chatgpt-codex-connector]

Continue scanning test paths for exposed secrets

When a repository contains a real credential under tests/, e2e/, mocks/, or a *.test.* file, this unconditional exclusion prevents the shared walk() caller in secretsCheck from examining it. Secret validity does not depend on whether the file is test material, so a committed live AWS key, GitHub token, or webhook in one of these paths remains exploitable while the scan reports no finding; apply this exclusion only to checks for intentionally unsafe code, or otherwise retain secret scanning for these paths.

Useful? React with 👍 / 👎.

[davidstrouk]

Fixed in bc5dc82. walk() now takes skipTestPaths. The code-pattern checks keep the exclusion, and secretsCheck turns it off, so a real key under tests/ is a finding again. The check filters on the value instead: it drops a match that carries a placeholder tell, which means a word such as example, a run of six identical characters, or a sequential run such as abcdef.

[chatgpt-codex-connector]

Preserve executable source strings during filtering

When a JavaScript string is passed to an API that evaluates it as code, blanking every string hides real dangerous constructs rather than documentation. For example, setTimeout("document.write(location.hash)", 0) previously matched the document.write rule and actually executes that source in browsers, but this selection now scans only the blanked codeLines for that rule and reports nothing; executable-string contexts need to remain detectable.

Useful? React with 👍 / 👎.

[davidstrouk]

This is a deliberate trade. The blanked strings remove a false-positive class that fires on every linter repo and every security repo. setTimeout with a string argument is legacy and rare. The correct fix is a new rule that matches an evaluated string argument, not raw string text for every pattern. I logged it as a follow-up.

[chatgpt-codex-connector]

Recognize underscored fixture and mock directories

When fixtures or mocks are colocated under the common Jest-style __fixtures__ or __mocks__ directories, this expression does not match those path segments, so walk() still scans their deliberately fake credentials and unsafe calls. The regex explicitly handles __tests__, making the omission of the corresponding underscored fixture and mock conventions likely accidental and leaving the false-positive class this change targets unresolved for those repositories.

Useful? React with 👍 / 👎.

[davidstrouk]

Codex review processed. Three findings fixed, one declined.

Fixed — regex literals (9810834). A regex literal holding a backtick put the stripper into template mode and blanked the rest of the file, so a later eval(input) went unreported. Verified as a real false negative before the fix. The stripper now reads regex literals before the quote rules, handles / inside a character class, and ends an unterminated literal at a newline. A / reads as division when the previous token is a value, so total / count stays code.

Fixed — underscored fixture dirs (9810834). The pattern matched __tests__ but not __mocks__ or __fixtures__. It now accepts the underscored form of every name, plus specs?.

Fixed — secrets in test paths (bc5dc82). This one was right and mattered most. The exclusion applied to every check, so a real key committed under tests/ went unreported. walk() now takes skipTestPaths: the code-pattern checks keep the exclusion, secretsCheck drops it and filters on the value instead. A match is ignored when it carries a placeholder tell, which means a word such as example, a run of six identical characters, or a sequential run such as abcdef. The self-scan stays at zero and a real key under tests/ is a finding again.

Declined — strings passed to evaluating APIs. Correct, and a real narrow regression: setTimeout("document.write(...)", 0) no longer matches. Blanking strings removes a false-positive class that fires on every linter repo and security repo, and string-argument setTimeout is legacy and rare. The right fix is a new rule that matches the call shape rather than the string contents, so it survives the stripper. Logged as a follow-up.

61 tests pass. Self-scan reports 0 findings, score 100, grade A.