fix: remove backtracking-prone regexes flagged by SonarQube - #3
Conversation
All five S5852 "super-linear runtime due to backtracking" hotspots that were holding SonarCloud's Security Hotspots Reviewed condition at 0%: - The import scanner (`import\s+.*?from\s+['"](.+?)['"]|...`) was duplicated verbatim in the related-files and file-context handlers. The lazy `.*?` next to `\s+` gives the engine ambiguous ways to split a line, and these scanners run over every line of up to 500 files from an untrusted repository. Extracted to `src/tools/import-scan.ts` with each quantifier over a character set disjoint from what follows it, so the match is linear. It now also recognises side-effect imports (`import 'p'`) and re-exports, which the old pattern missed. - `\s+as\s+` in the named-export-list parser: two `\s+` runs around a literal split whitespace ambiguously. Replaced with `\bas\b`. - `replace(/\/+$/, '')` in the OpenAI base-URL join (twice): the anchored `+` retries from every position. Replaced with a scan, behind a new exported `resolveChatCompletionsEndpoint` so the join is testable. Tests cover every changed line, including a timing guard on each former hot spot. Along the way, the file-context importer test was only asserting that the "Imported By" header rendered — leftover `mockResolvedValueOnce` values from earlier tests meant the scan never saw any files, so the whole match loop was untested. It now asserts the matches themselves (handlers.ts 77% -> 98%).
|
CI status on
The scan died in the same place as the previous run on I'm not pushing anything for this one — it isn't a code failure. That endpoint returns This needs a token rotation, which only you can do: mint a new token in SonarCloud (My Account → Security), then replace The coverage step did run before the scanner tried to authenticate, and it confirms the numbers claimed above landed — Generated by Claude Code |
1 participant
Two separate things are wrong with SonarQube here
1. The scan can't authenticate — this PR does not fix that, and it needs you.
The last run (30970722778) never reached analysis:
That endpoint answers
200unauthenticated and403for a bad bearer token, so the repo'sSONAR_TOKENsecret is present but no longer valid. The last green scan was 2026-02-11; a SonarCloud token minted around then has since expired. Fix: mint a new token in SonarCloud (My Account → Security) and replaceSONAR_TOKENunder Settings → Secrets and variables → Actions. Nothing in the repo can do this.2. The quality gate was failing on its own merits — that part is this PR.
All five hotspots are
typescript:S5852, "regex vulnerable to super-linear runtime due to backtracking":src/llm/openai.tsreplace(/\/+$/, '')src/llm/openai.tsreplace(/\/+$/, '')src/tools/file-context/handlers.tsimport\s+.*?from\s+['"](.+?)['"]|…src/tools/related-files/handlers.tssrc/tools/related-files/handlers.tssplit(/\s+as\s+/)Changes
The import scanner was the same regex copy-pasted into both handlers. The lazy
.*?sitting next to\s+gives the engine ambiguous ways to split one line — and this scans every line of up to 500 files from a repository being reviewed, i.e. attacker-influenced input. Extracted tosrc/tools/import-scan.tsasextractImportSpecifiers, with each quantifier over a character set disjoint from whatever follows it ([\s(]*then a quote,[^'"\n]+then a quote), so there is exactly one way to match and the scan is linear in line length.Two behaviour changes fall out, both improvements: it now also finds side-effect imports (
import './polyfill.js') and re-exports (export { x } from './p'), which the old pattern silently missed.\s+as\s+→\bas\b. The two\s+runs around a literal let the engine split whitespace ambiguously; the word boundaries have no such freedom and the extracted name is identical.replace(/\/+$/, '')→ a scan. An anchored+makes the engine retry the run from every position. The trailing-slash trim and the endpoint join now live in an exportedresolveChatCompletionsEndpoint, which also makes the join testable — it wasn't before.Coverage
New/changed lines are 100% covered (checked line-by-line against
coverage/lcov.info), so this lands well clear of the 80% new-code threshold.Two pre-existing gaps got closed along the way:
src/llm/openai.ts24.4% → 37.6% — there was notests/llm/openai.test.tsat all.src/tools/file-context/handlers.ts77.0% → 97.6% — the "includes importers section" test only asserted that the## Imported Byheader rendered.vi.clearAllMocks()keeps queuedmockResolvedValueOncevalues, and leftovers from earlier tests meantreaddirnever returned any files, so the entire match loop it was supposed to exercise ran zero times. The new test resets the mocks and asserts the matches themselves.Each former hot spot also gets a timing guard (20k spaces / 50k slashes must resolve in under a second) so a future rewrite can't quietly reintroduce the backtracking.
Verification
The SonarQube check on this PR will still fail on the 403 until the token is rotated.
Generated by Claude Code