fix(api): test-suite deadlock + 3 CodeQL alerts on main - #71
Merged
Merged
Conversation
Three independent fixes batched into one PR because the test-suite fix is what unblocks merging to main without flaking, and the security alerts are the next things blocking a clean main. Test deadlock — tests/helpers/db.ts Symptom: random CI failures of apps-api-ci.yml. The integration tests share one postgres instance, and cleanDatabase() was a single "TRUNCATE ... RESTART IDENTITY CASCADE" statement. TRUNCATE acquires AccessExclusiveLock on every listed table; concurrent SELECTs from another test worker take AccessShareLock on those same tables. With the suite running files in parallel, the two lock orders collide and postgres aborts one side with deadlock_detected. Fix: rewrite cleanDatabase() as a transaction that takes a pg_advisory_xact_lock (serialising cleanup calls across all workers) and DELETEs each table in child-first dependency order. DELETE takes RowExclusiveLock, which does NOT conflict with the SELECT's AccessShareLock — so a mid-cleanup worker no longer deadlocks with a mid-test worker. Trade-off is a few ms of extra wall-clock per cleanup, well under the budget for a CI suite that never spuriously fails. CodeQL js/path-injection (x2) — templates/email/preview.ts The dev-only email preview server reads req.url-derived paths via fs.existsSync and fs.readFileSync. The previous path.join + startsWith(PREVIEW_DIR) guard was correct in practice, but path.join doesn't canonicalise '..' segments and CodeQL can't see the prefix check as sufficient. Switch to path.resolve (which normalises traversals), strip leading slashes / query / fragment from the URL, and re-validate after the directory/extension fallback — mostly to make the contract explicit at every read site. CodeQL js/redos — scripts/codegen/new-resource.ts Codegen scaffolder's usersBlock regex used (?:[^}]*\n)*? — a quantifier-inside-a-quantifier shape that backtracks pathologically on input with many bare newlines. Replaced with [\s\S]*? (a single lazy quantifier = linear scan). Captures are renumbered accordingly.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…jection Previous attempt (path.resolve + startsWith) was correct in practice but the static analyzer doesn't trust prefix checks — every fs.* call that took a string derived from req.url stayed flagged. Switch to the pattern CodeQL recognises as safe: walk PREVIEW_DIR at server start with fs.readdirSync, build a Map<urlPath, absolutePath>, and look up each request against it. req.url is now an untrusted KEY into the map; the value handed to fs.readFileSync is an absolute path that originated from filesystem enumeration, not from user input. There is no flow from req.url to readFileSync for the taint analyzer to follow. Functional behavior preserved: "/" still serves index.html, both "/auth/login" and "/auth/login.html" still resolve to the same file (map miss on the bare route falls through to "route.html").
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
2 participants
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.
Three independent fixes batched into one PR because the test-suite fix is what unblocks merging to main without flaking, and the security alerts are the next things blocking a clean main.
Test deadlock — tests/helpers/db.ts
Symptom: random CI failures of apps-api-ci.yml. The integration tests share one postgres instance, and cleanDatabase() was a single "TRUNCATE ... RESTART IDENTITY CASCADE" statement. TRUNCATE acquires AccessExclusiveLock on every listed table; concurrent SELECTs from another test worker take AccessShareLock on those same tables. With the suite running files in parallel, the two lock orders collide and postgres aborts one side with deadlock_detected.
Fix: rewrite cleanDatabase() as a transaction that takes a pg_advisory_xact_lock (serialising cleanup calls across all workers) and DELETEs each table in child-first dependency order. DELETE takes RowExclusiveLock, which does NOT conflict with the SELECT's AccessShareLock — so a mid-cleanup worker no longer deadlocks with a mid-test worker. Trade-off is a few ms of extra wall-clock per cleanup, well under the budget for a CI suite that never spuriously fails.
CodeQL js/path-injection (x2) — templates/email/preview.ts
The dev-only email preview server reads req.url-derived paths via fs.existsSync and fs.readFileSync. The previous path.join + startsWith(PREVIEW_DIR) guard was correct in practice, but path.join doesn't canonicalise '..' segments and CodeQL can't see the prefix check as sufficient. Switch to path.resolve (which normalises traversals), strip leading slashes / query / fragment from the URL, and re-validate after the directory/extension fallback — mostly to make the contract explicit at every read site.
CodeQL js/redos — scripts/codegen/new-resource.ts
Codegen scaffolder's usersBlock regex used (?:[^}]\n)? — a quantifier-inside-a-quantifier shape that backtracks pathologically on input with many bare newlines. Replaced with [\s\S]*? (a single lazy quantifier = linear scan). Captures are renumbered accordingly.
Summary
Test plan
bun run check(orbun run check:fullfor the cross-app pass) from the repo rootcd infra/compose/compose && ./dev.sh upApp merge bars
cd apps/api && bun run validatecd apps/ui && bun run validatecd apps/docs && bun run build:cibun run check(from repo root)Conventions
any, no blindas, no!.env.example(+ SECURITY.md when relevant)Screenshots