Repository navigation
fix(critical): programScope enrollment attachment skipped on double invocation - #263
Merged
Merged
Conversation
…le invocation Root cause of the Lakshya Aran "not enrolled" incident, found via the DB-based debug instrumentation added over the last several commits (now removed). programScope() is mounted BOTH globally (bootstrap/middleware.ts, line 155, before any route-specific auth middleware runs) AND again per-route after optionalAuth/protect. The global pass runs with no req.user yet, so it could resolve req.programContext but never had a user to attach req.programEnrollment for. The old "if (req.programContext) return next()" early return then made the per-route pass — the only one that ever has a real user — skip re-running entirely, so req.programEnrollment silently never got attached for ANY signed-in user on ANY route using this middleware (which is every route this whole incident has been about). enforceProgramMembership then fell through to the fail-open check, found the user had SOME enrollment (just not attached to the request), and blocked with "You are not enrolled in this program" — for demonstrably, correctly enrolled users. Fix: batch-context resolution is still cached once resolved (the batch itself doesn't change mid-request), but enrollment attachment is now independent of that cache and always attempted whenever a signed-in, non-admin user doesn't have req.programEnrollment set yet — regardless of which pass (global or per-route) is running. This means the entire cross-cohort fix from earlier today (#246, #247) was less effective than believed: the authorization logic was correct, but this pre-existing structural bug prevented it from ever correctly attaching a real enrollment on the first (global) pass, and skipped retrying on the second (per-route, authenticated) pass. It's likely this bug — not missing enrollment data — explains a meaningful share of the "not enrolled" reports today, on top of the genuinely-missing-data cases already fixed. Added programScope-double-invocation.test.ts, which reproduces the exact global-then-per-route sequence and asserts req.programEnrollment ends up attached. Removed all debug/journalctl-workaround scaffolding (tail-csfaq-logs.yml, read-debug-temp.yml + .ts) added while diagnosing this; added a one-off cleanup script+workflow to drop the throwaway debug_temp_2026_09_25 collection they wrote to. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CodeQL flagged 3 findings on this PR: - bootstrap/middleware.ts:155 — the global programScope() mount does a DB lookup (Batch.findById) on every request carrying a batchId, on every route in the app, with zero rate limiting. Added a generous (600/min) cap ahead of it — high enough that normal shared-IP/ multi-tab browsing never trips it, low enough to bound a scripted flood. - search.routes.ts /trending and POST / — both had programScope() running BEFORE their rate limiter, so a flood could still hit the DB via programScope's own lookup regardless of the limiter downstream. Reordered so the limiter runs first on both routes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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
Root cause found for the Lakshya Aran "not enrolled" incident (and likely a meaningful share of today's other "not enrolled" reports), via the DB-based debug instrumentation added over the last several PRs (now removed).
programScope()is mounted both globally (bootstrap/middleware.ts:155, before any route-specific auth middleware runs) and again per-route afteroptionalAuth/protect. The global pass runs with noreq.useryet, so it could resolvereq.programContextbut never had a user to attachreq.programEnrollmentfor. The oldif (req.programContext) return next()early return then made the per-route pass — the only one that ever has a real user — skip re-running entirely, soreq.programEnrollmentsilently never got attached for any signed-in user on any route using this middleware.enforceProgramMembership()then fell through to the fail-open check, found the user had some enrollment (just not attached to the request), and blocked with "You are not enrolled in this program" — for demonstrably, correctly enrolled users.Fix: batch-context resolution is still cached once resolved (the batch itself doesn't change mid-request), but enrollment attachment is now independent of that cache and always attempted whenever a signed-in, non-admin user doesn't have
req.programEnrollmentset yet — regardless of which pass is running.This means the cross-cohort fix from earlier today (#246/#247) was less effective than believed — the authorization logic itself was correct, but this pre-existing structural bug prevented it from reliably attaching real enrollments at all.
Also in this PR:
programScope-double-invocation.test.ts— reproduces the exact global-then-per-route sequence and asserts enrollment ends up attached.tail-csfaq-logs.yml,read-debug-temp.yml+.ts).debug_temp_2026_09_25collection they wrote to.Test plan
tsc --noEmitclean🤖 Generated with Claude Code