Security review (2026-06-01): close SEC-03 + SEC-13 inline#172
Draft
pokle wants to merge 1 commit into
Draft
Conversation
No new application source code landed since the 2026-05-25 round, so this round closes two long-standing Open findings inline: - SEC-03 (Medium, open since 2026-04-20): admin email addresses were returned on the public `GET /api/comp/:comp_id` response, exposing organisers' email addresses to scraping for phishing. The route now resolves the caller's `comp_admin` status once (reused for the existing test-comp 404 gate), then strips the `email` field from the response for non-admin / anonymous callers. A new `current_user_is_admin` boolean is added to the response so the frontend can determine admin status without probing `admins[].email`. Two new regression tests cover anonymous and authenticated-non-admin callers; the existing admin test now also asserts `current_user_is_admin === true`. - SEC-13 (Low, open since 2026-05-04): the service-worker share-target handler stashed shared files in Cache Storage keyed by an unsanitised filename and set `X-File-Name` to the raw filename. CR/LF in the name would break header parsing; `?`, `#`, `..` etc. would break cache-key round-tripping. The handler now strips control chars, falls back to `'shared-file'` for empty/missing names, and URL-encodes the cache-key path component. The analysis-page consumer is updated to `decodeURIComponent` the pathname fallback (the `X-File-Name` header remains the primary source). All tests pass: 412 engine/airscore/root + 52 auth-api + 253 competition-api (+2 new SEC-03 tests) + 21 mcp-api. `bun audit` clean.
|
Preview Deployment |
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.
Summary
Whole-repo security review for 2026-06-01. No new application source code landed since the 2026-05-25 round —
bun auditis clean and the SEC-01 / SEC-10 / SEC-11 / SEC-12 / SEC-15 / SEC-16 / SEC-17 fixes all hold byte-for-byte. With no fresh attack surface, this round closes two long-standing Open findings inline:SEC-03 (Medium, open since 2026-04-20) — Admin email addresses were returned on the public
GET /api/comp/:comp_idresponse, exposing organisers' email addresses to scraping. The route now resolvescomp_adminstatus once (combined with the existing test-comp 404 gate) and strips theemailfield for non-admin callers; a newcurrent_user_is_adminboolean is returned so the frontend can determine admin status without probingadmins[].email. Two new regression tests cover anonymous and authenticated-non-admin callers.SEC-13 (Low, open since 2026-05-04) —
web/frontend/public/sw.jsstashed shared files in Cache Storage keyed by an unsanitised filename. The handler now strips CR/LF/control chars fromX-File-Name, falls back to'shared-file'for empty names, and URL-encodes the cache-key path component. The analysis-page consumer decodes the pathname fallback (theX-File-Nameheader remains the primary source).No new
SEC-NNfindings this round.Changes
web/workers/competition-api/src/routes/comp.ts— SEC-03 fix: gateadmins[].emailoncomp_adminmembership; addcurrent_user_is_adminto the response.web/frontend/src/comp-detail.ts— usecurrent_user_is_admininstead of email probing; render admin display name whenemailis absent.web/workers/competition-api/test/comp-routes.test.ts— two new SEC-03 regression tests (anonymous + authenticated non-admin) + existing test assertscurrent_user_is_admin === true.web/frontend/public/sw.js— SEC-13 fix: sanitiseX-File-Nameand URL-encode the cache key.web/frontend/src/analysis/main.ts—decodeURIComponentthe pathname fallback for the share-target cache (header remains primary source).docs/security-review.md— appended 2026-06-01 round (commit reviewed up to:fde480c).Test plan
bun audit— clean (0 vulnerabilities)bun run typecheck:all— greenbun run test:all— 412 engine/airscore/root + 52 auth-api + 253 competition-api (+2 new SEC-03 tests) + 21 mcp-apicurl -s https://glidecomp.com/api/comp/<id> | jq '{admins, current_user_is_admin}'should show alladmins[].emailabsent andcurrent_user_is_admin: falsefor an anonymous caller.weird?name#here.igcto GlideComp's share target; inspect the cached response in DevTools —X-File-Nameshould be the original (minus control chars), the cache-key URL should be percent-encoded, the analysis page should load it.See
docs/security-review.mdfor the full 2026-06-01 round.Generated by Claude Code