Skip to content

Non-blocking follow-ups from the adversarial review of the 2026-09-12 bug-fix batch #188

Description

@pat-lewczuk

An adversarial review of the 24 bug-fix PRs opened on 2026-09-12 raised twelve findings. Five were
blocking and are already fixed on their branches (F1 route-handler tests; F2 search regression;
F3 mutation-guard resourceKind; F4 results-export feature; F5 team capacity counting departed
members). The reviewer explicitly deferred the seven below as non-blocking — none of them should
hold up a merge, but they are real and recorded here so they are not lost.

Each is independent; split any of them out into its own issue when picked up.


1. scores route returns a hard 401 when a staff session has no selected organization

src/modules/judging/api/scores/route.ts:20if (!auth?.tenantId || !auth.orgId) return 401

AuthContext.orgId is typed string | null, and the sibling back-office routes in the same module do
not make that check (judging/api/finalists/route.ts:19 checks only tenantId). A tenant admin in
"all organizations" mode would get Unauthorized (401) on the Scoring Progress tab while Demo Queue
and Leaderboard keep working. PLAUSIBLE — the reviewer could not establish whether a staff session
here can actually carry orgId === null. Worth 10 minutes to confirm either way.

2. STAGE_SEQUENCE duplicates STAGE_ORDER, and its test does not pin them together

src/modules/competitions/lib/stages.ts:11, src/modules/competitions/lib/__tests__/stages.test.ts:10

stages.ts hand-copies the nine stage ids for a legitimate reason (client bundles must not import the
MikroORM-decorated data/entities.ts). The problem is the test: it asserts STAGE_SEQUENCE against
another hardcoded literal and never imports STAGE_ORDER. Insert a stage in entities.ts and the
test stays green while isAtOrAfterStage silently misclassifies. Fix: assert
[...STAGE_SEQUENCE] deep-equals STAGE_ORDER in a server-side test.

3. The email local part is still disclosed through the displayName fallback

competitions/api/portal/resolve-users/route.ts:85, search-participants/route.ts:114, participantSearch.ts:82

Both keep user.email?.split('@')[0] as the display-name fallback. Harmless in resolve-users (no
domain is returned), but search-participants returns the domain separately in maskedEmail, so
charlie.brown + c***@hackon.test reconstructs the full address — defeating the masking for exactly
the population it exists to protect. display_name is text not null, so the fallback only fires on
an empty string; PLAUSIBLE as a live leak, certain as a code path.

4. resolve-users has no caller anywhere in src/

grep -rn "resolve-users" src/ matches only the route's own console.error. The endpoint is reachable
only by direct HTTP. PR #147 fixed it correctly and it now has a route test, but nobody should budget
follow-up work against it without first deciding whether it should exist at all.

5. The portal judging page and its API gate on different features

judging/frontend/[orgSlug]/portal/judging/page.meta.ts requires portal.judging.score;
judging/api/portal/my-assignments/route.ts:22 requires portal.judging.view_assigned.

Both are granted to judge, so nothing breaks today. A custom role with only portal.judging.score
would pass the page gate, render the assignments screen, and get a 403 from the list it is built
around. The two gates should name the same feature, or the page should require both.

6. The project lookup in asset-file is not tenant-scoped

projects/api/portal/asset-file/[id]/route.ts:57em.findOne(Project, { id: attachment.recordId, deletedAt: null })

No tenantId / organizationId. The scope checks above it are conditional on the attachment's own
nullable columns, so a NULL-tenant attachment row skips both. The request still fails closed (the
TeamMember / CompetitionParticipation lookups are tenant-scoped and find nothing → 403), so this is
hardening, not a hole — but the entitlement check is currently the only thing standing between a
null-tenant attachment and a cross-tenant read.

7. search-participants decrypts the whole roster on every keystroke

competitions/api/portal/search-participants/route.ts:99-107

The name-prefix branch calls findWithDecryption with no limit, pulling every participant of the
competition and AES-GCM-decrypting email + display_name for each, then filtering in memory and
slicing to 10. In-memory filtering is genuinely unavoidable for encrypted columns, but this runs on
every debounced keystroke of the invite autocomplete — at a few thousand attendees that is a full
roster decrypt per keystroke per searching user.


Also noted, deliberately not counted against any PR

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions