From 83b84572b0d0394e9fb48369edb510dd0901686a Mon Sep 17 00:00:00 2001 From: Jinal Gupta Date: Fri, 25 Sep 2026 17:38:49 +0530 Subject: [PATCH 1/2] fix(critical): programScope enrollment attachment was skipped on double invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ...emp.yml => drop-debug-temp-collection.yml} | 33 ++-- .github/workflows/tail-csfaq-logs.yml | 34 ---- .../programScope-double-invocation.test.ts | 91 +++++++++ apps/backend/src/middleware/programScope.ts | 176 +++++------------- .../src/scripts/drop-debug-temp-collection.ts | 35 ++++ apps/backend/src/scripts/read-debug-temp.ts | 36 ---- 6 files changed, 188 insertions(+), 217 deletions(-) rename .github/workflows/{read-debug-temp.yml => drop-debug-temp-collection.yml} (65%) delete mode 100644 .github/workflows/tail-csfaq-logs.yml create mode 100644 apps/backend/src/middleware/__tests__/programScope-double-invocation.test.ts create mode 100644 apps/backend/src/scripts/drop-debug-temp-collection.ts delete mode 100644 apps/backend/src/scripts/read-debug-temp.ts diff --git a/.github/workflows/read-debug-temp.yml b/.github/workflows/drop-debug-temp-collection.yml similarity index 65% rename from .github/workflows/read-debug-temp.yml rename to .github/workflows/drop-debug-temp-collection.yml index 5abadbc5..1764942a 100644 --- a/.github/workflows/read-debug-temp.yml +++ b/.github/workflows/drop-debug-temp-collection.yml @@ -1,25 +1,18 @@ -name: Read Debug Temp Collection (one-off, read-only) +name: Drop Debug Temp Collection (one-off, cleanup) -# Manual, one-off read for the 2026-09-25 live-bug investigation. -# Dumps the throwaway debug_temp_2026_09_25 collection written by -# temporary instrumentation in programScope.ts. Read-only, no writes. +# One-time cleanup for the 2026-09-25 live-bug investigation. Drops +# the throwaway debug_temp_2026_09_25 collection. Safe/idempotent. on: workflow_dispatch: - inputs: - userId: - description: "Optional userId to filter debug entries by" - required: false - default: "" - type: string env: INFISICAL_ENVIRONMENT: prod INFISICAL_SECRET_PATH: / jobs: - read: - name: SSH Read Debug Data + cleanup: + name: SSH Drop Collection runs-on: ubuntu-latest environment: production @@ -31,12 +24,9 @@ jobs: chmod 600 ~/.ssh/id_ed25519 ssh-keyscan -H ${{ secrets.CSFAQ_HOST }} >> ~/.ssh/known_hosts - - name: Read debug data - env: - FILTER_USER_ID: ${{ inputs.userId }} + - name: Drop collection run: | - FILTER_B64=$(printf '%s' "$FILTER_USER_ID" | base64) - ssh -i ~/.ssh/id_ed25519 csfaq-deploy@${{ secrets.CSFAQ_HOST }} "FILTER_B64='${FILTER_B64}' bash" << 'READ' + ssh -i ~/.ssh/id_ed25519 csfaq-deploy@${{ secrets.CSFAQ_HOST }} bash << 'CLEANUP' set -e cd /home/csfaq-prod/crowd-source-faq @@ -72,9 +62,8 @@ jobs: cd apps/backend pnpm install --frozen-lockfile - FILTER_USER_ID=$(printf '%s' "$FILTER_B64" | base64 -d) - echo "==> Reading debug_temp_2026_09_25 (filter: ${FILTER_USER_ID:-none})..." - npx tsx src/scripts/read-debug-temp.ts "$FILTER_USER_ID" + echo "==> Dropping debug_temp_2026_09_25..." + npx tsx src/scripts/drop-debug-temp-collection.ts - echo "==> Done. Read-only, no writes made." - READ + echo "==> Done." + CLEANUP diff --git a/.github/workflows/tail-csfaq-logs.yml b/.github/workflows/tail-csfaq-logs.yml deleted file mode 100644 index fc79a49a..00000000 --- a/.github/workflows/tail-csfaq-logs.yml +++ /dev/null @@ -1,34 +0,0 @@ -name: Tail CSFAQ Service Logs (one-off, read-only) - -# Manual, one-off log tail for the 2026-09-25 live-bug investigation. -# Read-only — prints recent journalctl output for csfaq.service, makes -# no writes. - -on: - workflow_dispatch: - inputs: - lines: - description: "Number of recent log lines to print" - required: false - default: "200" - type: string - -jobs: - tail: - name: SSH Tail Logs - runs-on: ubuntu-latest - environment: production - - steps: - - name: Setup SSH - run: | - mkdir -p ~/.ssh - echo "${{ secrets.CSFAQ_DEPLOY_SSH_KEY }}" | tr -d '\r' > ~/.ssh/id_ed25519 - chmod 600 ~/.ssh/id_ed25519 - ssh-keyscan -H ${{ secrets.CSFAQ_HOST }} >> ~/.ssh/known_hosts - - - name: Tail logs - env: - LOG_LINES: ${{ inputs.lines }} - run: | - ssh -i ~/.ssh/id_ed25519 csfaq-deploy@${{ secrets.CSFAQ_HOST }} "sudo journalctl -u csfaq.service -n ${LOG_LINES} --no-pager" diff --git a/apps/backend/src/middleware/__tests__/programScope-double-invocation.test.ts b/apps/backend/src/middleware/__tests__/programScope-double-invocation.test.ts new file mode 100644 index 00000000..57457a14 --- /dev/null +++ b/apps/backend/src/middleware/__tests__/programScope-double-invocation.test.ts @@ -0,0 +1,91 @@ +/** + * programScope-double-invocation.test — 2026-09-25 incident. + * + * Root cause of a real production bug: `programScope()` is mounted + * BOTH globally (bootstrap/middleware.ts, 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 "already attached, skip" early + * return then made the per-route pass — the only one that ever has a + * real user — skip re-running entirely, so req.programEnrollment + * never got attached even for a genuinely, correctly enrolled user. + * + * This test reproduces exactly that: call programScope() once with no + * req.user (simulating the global pre-auth pass), then call it again + * on the SAME req object after attaching req.user (simulating the + * per-route post-auth pass) — req.programEnrollment must end up set. + */ +import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; +import mongoose, { Types } from 'mongoose'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import type { Request, Response, NextFunction } from 'express'; + +let mongo: MongoMemoryServer; + +beforeAll(async () => { + mongo = await MongoMemoryServer.create(); + await mongoose.connect(mongo.getUri()); +}, 120_000); + +afterAll(async () => { + await mongoose.disconnect(); + await mongo.stop(); +}); + +beforeEach(async () => { + const db = mongoose.connection.db; + if (!db) throw new Error('no db'); + await db.collection('yaksha_faq_batches').deleteMany({}); + await db.collection('yaksha_program_enrollments').deleteMany({}); +}); + +const { default: Batch } = await import('../../modules/program/batch.model.js'); +const { default: ProgramEnrollment } = await import('../../modules/program/program-enrollment.model.js'); +const { programScope } = await import('../programScope.js'); + +describe('programScope — global-then-per-route double invocation', () => { + it('attaches programEnrollment on the SECOND call once req.user is available, even though programContext was already resolved by the first (userless) call', async () => { + const batch = await Batch.create({ + name: 'summership', + description: '', + startDate: new Date('2026-05-15'), + endDate: new Date('2026-12-31'), + isActive: true, + status: 'active', + }); + const userId = new Types.ObjectId(); + await ProgramEnrollment.create({ + userId, + batchId: batch._id, + programRole: 'student', + isActive: true, + }); + + const req = { + query: { batchId: String(batch._id) }, + params: {}, + body: {}, + headers: {}, + } as unknown as Request; + const res = { status: () => res, json: () => res } as unknown as Response; + const next1: NextFunction = (() => {}) as NextFunction; + + // Pass 1: simulates the global mount — runs BEFORE auth, no req.user. + await programScope()(req, res, next1); + expect(req.programContext?.batchId).toBe(String(batch._id)); + expect(req.programEnrollment).toBeUndefined(); + + // Auth middleware would run here in the real chain and set req.user. + (req as unknown as { user?: unknown }).user = { _id: userId, role: 'user' }; + + // Pass 2: simulates the per-route mount — runs AFTER auth, on the + // SAME req object, so req.programContext is already set. + const next2: NextFunction = (() => {}) as NextFunction; + await programScope()(req, res, next2); + + expect(req.programEnrollment).toBeDefined(); + expect(req.programEnrollment?.userId).toBe(String(userId)); + expect(req.programEnrollment?.batchId).toBe(String(batch._id)); + }); +}); diff --git a/apps/backend/src/middleware/programScope.ts b/apps/backend/src/middleware/programScope.ts index 2dcbcab7..05a487bd 100644 --- a/apps/backend/src/middleware/programScope.ts +++ b/apps/backend/src/middleware/programScope.ts @@ -70,118 +70,65 @@ function extractBatchId(req: Request): string | null { export function programScope(opts: { required?: boolean } = {}) { const required = opts.required ?? false; return async (req: Request, res: Response, next: NextFunction): Promise => { - // Incident debug (2026-09-25): absolute-first-line capture. If - // `req.programContext` is already truthy here, this function - // returns on the very next line — before any of our other debug - // writes further down, which would explain why enforceProgramMembership - // sees a populated programContext but programScope's own - // instrumentation never fires. try { - const mongoose = (await import('mongoose')).default; - const u = (req as Request & { user?: { _id?: string } }).user; - await mongoose.connection.db?.collection('debug_temp_2026_09_25').insertOne({ - at: new Date(), - fn: 'programScope-veryfirst', - userId: u?._id ? String(u._id) : null, - alreadyHadProgramContext: !!req.programContext, - existingProgramContext: req.programContext ?? null, - hasReqUser: !!u, - }); - } catch { - // best-effort - } - - if (req.programContext) return next(); // already attached - - const batchId = extractBatchId(req); - if (!batchId) { - if (required) { - res.status(400).json({ message: 'batchId is required for this route.' }); - return; - } - return next(); - } + // Root-cause fix (2026-09-25): this middleware is mounted BOTH + // globally (bootstrap/middleware.ts, before any route-specific + // auth middleware runs) AND again per-route after `optionalAuth`/ + // `protect`. The global pass runs with `req.user` not yet set, so + // it could resolve `req.programContext` but never had a user to + // attach `req.programEnrollment` for. The old "already attached, + // skip" 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 anyone + // whose request happened to reach a global-then-per-route double + // invocation (which is every request that reaches a route using + // this middleware). That looked exactly like "not enrolled" for + // users who very much were. + // + // Fix: resolving `req.programContext` is still skipped once it + // exists (that part of the cache is fine — the batch itself + // doesn't change mid-request), but the enrollment attachment is + // now independent of that cache and always attempted whenever a + // signed-in, non-admin user doesn't have it yet. + if (!req.programContext) { + const batchId = extractBatchId(req); + if (!batchId) { + if (required) { + res.status(400).json({ message: 'batchId is required for this route.' }); + return; + } + return next(); + } - try { - const batch = await Batch.findById(batchId).select('_id name isActive').lean(); - if (!batch) { - res.status(404).json({ message: 'Program not found.' }); - return; - } - if (!batch.isActive) { - res.status(410).json({ message: 'Program is archived or completed.' }); - return; + const batch = await Batch.findById(batchId).select('_id name isActive').lean(); + if (!batch) { + res.status(404).json({ message: 'Program not found.' }); + return; + } + if (!batch.isActive) { + res.status(410).json({ message: 'Program is archived or completed.' }); + return; + } + req.programContext = { + batchId: String(batch._id), + batchName: batch.name, + isActive: batch.isActive, + }; + setContextBatchId(String(batch._id)); } - req.programContext = { - batchId: String(batch._id), - batchName: batch.name, - isActive: batch.isActive, - }; - - setContextBatchId(String(batch._id)); - // Look up enrollment if the user is signed in. The model - // is loaded lazily so this middleware works even before the - // ProgramEnrollment model migration is run. + // Look up enrollment if the user is signed in and it isn't + // already attached. The model is loaded lazily so this + // middleware works even before the ProgramEnrollment model + // migration is run. const userId = (req as Request & { user?: { _id?: string; role?: string } }).user?._id; - - // Incident debug (2026-09-25): unconditional capture, including - // the case where userId is falsy (which would explain a 403 - // with zero writes from the block below). - try { - const mongoose = (await import('mongoose')).default; - await mongoose.connection.db?.collection('debug_temp_2026_09_25').insertOne({ - at: new Date(), - fn: 'programScope-top', - hasReqUser: !!(req as Request & { user?: unknown }).user, - userId: userId ? String(userId) : null, - userIdTruthy: !!userId, - userRole: (req as Request & { user?: { role?: string } }).user?.role ?? null, - batchId, - }); - } catch { - // best-effort - } - - if (userId && (req as Request & { user?: { role?: string } }).user?.role !== 'admin') { + const role = (req as Request & { user?: { role?: string } }).user?.role; + if (userId && role !== 'admin' && !req.programEnrollment) { try { // Dynamic import — keeps the middleware cheap when the // model isn't installed yet. const { default: ProgramEnrollment } = await import('../modules/program/program-enrollment.model.js'); - const enr = await ProgramEnrollment.findOne({ userId, batchId, isActive: true }).lean(); - - // Incident debug (2026-09-25): journalctl isn't readable by - // the deploy SSH user (no sudo password configured for it), - // so console logging is invisible to us. Write the same - // diagnostic to a throwaway DB collection instead, which we - // can read via the existing one-off scripts. Also runs a raw - // native-driver query alongside the Mongoose one to rule out - // a schema-cast mismatch. Best-effort — never let this debug - // write itself break the real request. - try { - const mongoose = (await import('mongoose')).default; - const rawMatch = mongoose.connection.db - ? await mongoose.connection.db.collection('yaksha_program_enrollments').findOne({ - userId: new mongoose.Types.ObjectId(String(userId)), - batchId: new mongoose.Types.ObjectId(String(batchId)), - isActive: true, - }) - : null; - await mongoose.connection.db?.collection('debug_temp_2026_09_25').insertOne({ - at: new Date(), - userId: String(userId), - userIdCtor: (userId as unknown as { constructor?: { name?: string } })?.constructor?.name, - batchId, - batchIdType: typeof batchId, - mongooseFound: !!enr, - mongooseResult: enr, - nativeFound: !!rawMatch, - nativeResult: rawMatch, - }); - } catch (debugErr) { - httpLog.warn(`[programScope] debug write failed: ${(debugErr as Error).message}`); - } - + const enr = await ProgramEnrollment.findOne({ userId, batchId: req.programContext.batchId, isActive: true }).lean(); if (enr) { req.programEnrollment = { userId: String(enr.userId), @@ -190,12 +137,11 @@ export function programScope(opts: { required?: boolean } = {}) { enrolledAt: enr.enrolledAt, }; } - } catch (enrErr) { + } catch { // ProgramEnrollment model doesn't exist yet (Phase 1 not // fully landed). Skip silently — global admins still // pass through, and per-program authz is enforced later // once the model + middleware chain is in place. - httpLog.warn(`[programScope] enrollment lookup threw: ${(enrErr as Error).message}`); } } @@ -255,26 +201,6 @@ export function programScope(opts: { required?: boolean } = {}) { export function enforceProgramMembership() { return async (req: Request, res: Response, next: NextFunction): Promise => { const user = (req as Request & { user?: { _id?: string; role?: string } }).user; - - // Incident debug (2026-09-25): unconditional, best-effort capture - // of everything this function saw, written to the same throwaway - // collection programScope uses. See programScope.ts for context. - try { - const mongoose = (await import('mongoose')).default; - await mongoose.connection.db?.collection('debug_temp_2026_09_25').insertOne({ - at: new Date(), - fn: 'enforceProgramMembership', - hasUser: !!user, - userId: user?._id ? String(user._id) : null, - userIdCtor: user?._id ? (user._id as unknown as { constructor?: { name?: string } })?.constructor?.name : null, - userRole: user?.role ?? null, - programContext: req.programContext ?? null, - programEnrollment: req.programEnrollment ?? null, - }); - } catch { - // best-effort, never break the real request - } - if (!user) return next(); if (user.role === 'admin') return next(); if (!req.programContext) return next(); diff --git a/apps/backend/src/scripts/drop-debug-temp-collection.ts b/apps/backend/src/scripts/drop-debug-temp-collection.ts new file mode 100644 index 00000000..9d77426f --- /dev/null +++ b/apps/backend/src/scripts/drop-debug-temp-collection.ts @@ -0,0 +1,35 @@ +/** + * One-time cleanup: drops the throwaway `debug_temp_2026_09_25` + * collection created during the 2026-09-25 live-bug investigation + * (see programScope.ts git history / project memory for context). + * Safe to run even if the collection doesn't exist. + * + * Run: npx tsx src/scripts/drop-debug-temp-collection.ts + */ + +import 'dotenv/config'; +import mongoose from 'mongoose'; + +async function main() { + const mongoUri = process.env.MONGODB_URI; + if (!mongoUri) throw new Error('MONGODB_URI is not set'); + await mongoose.connect(mongoUri); + + const db = mongoose.connection.db; + if (!db) throw new Error('no db connection'); + + const collections = await db.listCollections({ name: 'debug_temp_2026_09_25' }).toArray(); + if (collections.length === 0) { + console.log('debug_temp_2026_09_25 does not exist — nothing to do.'); + } else { + await db.collection('debug_temp_2026_09_25').drop(); + console.log('Dropped debug_temp_2026_09_25.'); + } + + await mongoose.disconnect(); +} + +main().catch((err) => { + console.error(err); + process.exit(1); +}); diff --git a/apps/backend/src/scripts/read-debug-temp.ts b/apps/backend/src/scripts/read-debug-temp.ts deleted file mode 100644 index 04694d8c..00000000 --- a/apps/backend/src/scripts/read-debug-temp.ts +++ /dev/null @@ -1,36 +0,0 @@ -/** - * Read-only: dumps the throwaway `debug_temp_2026_09_25` collection - * written by the temporary instrumentation in programScope.ts (see - * that file for context — a confirmed live case where an active, - * demonstrably-correct ProgramEnrollment row isn't found by the exact - * query that should match it). - * - * Run: npx tsx src/scripts/read-debug-temp.ts - */ - -import 'dotenv/config'; -import mongoose from 'mongoose'; - -async function main() { - const mongoUri = process.env.MONGODB_URI; - if (!mongoUri) throw new Error('MONGODB_URI is not set'); - await mongoose.connect(mongoUri); - - const db = mongoose.connection.db; - if (!db) throw new Error('no db connection'); - - const filterUserId = process.argv[2] || null; - const query = filterUserId ? { userId: filterUserId } : {}; - const docs = await db.collection('debug_temp_2026_09_25').find(query).sort({ at: -1 }).limit(20).toArray(); - console.log(`Found ${docs.length} debug entries (most recent first):\n`); - for (const d of docs) { - console.log(JSON.stringify(d, null, 2)); - } - - await mongoose.disconnect(); -} - -main().catch((err) => { - console.error(err); - process.exit(1); -}); From 667475cb84940bc7c9899b23a79841545174b287 Mon Sep 17 00:00:00 2001 From: Jinal Gupta Date: Fri, 25 Sep 2026 17:44:19 +0530 Subject: [PATCH 2/2] fix(security): rate-limit the global programScope mount + fix ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- apps/backend/src/bootstrap/middleware.ts | 17 ++++++++++++++++- .../backend/src/modules/search/search.routes.ts | 8 ++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/apps/backend/src/bootstrap/middleware.ts b/apps/backend/src/bootstrap/middleware.ts index 13b57f4c..946ec8c4 100644 --- a/apps/backend/src/bootstrap/middleware.ts +++ b/apps/backend/src/bootstrap/middleware.ts @@ -4,6 +4,7 @@ import helmet from 'helmet'; import path from 'path'; import { fileURLToPath } from 'url'; import { v4 as uuidv4 } from 'uuid'; +import rateLimit from 'express-rate-limit'; import connectDB from '../config/db.js'; import { runWithContext } from '../utils/http/requestContext.js'; import { requestLogger } from '../utils/http/requestLogger.js'; @@ -11,6 +12,20 @@ import { ingestFrontendLog } from '../utils/http/fileLogger.js'; import { publicBasePath } from '../utils/publicBasePath.js'; import { programScope } from '../middleware/programScope.js'; +// CodeQL fix (js/missing-rate-limiting): this middleware runs a DB +// lookup (Batch.findById) for every request carrying a batchId, on +// EVERY route in the app — with no rate limit at all before this fix. +// Generous cap (this is a global, cross-route gate, not a single +// endpoint's own limiter) so normal multi-tab/shared-IP browsing never +// trips it, while still bounding a scripted flood. +const globalProgramScopeLimiter = rateLimit({ + windowMs: 60 * 1000, + max: 600, + standardHeaders: true, + legacyHeaders: false, + message: { message: 'Too many requests. Please slow down.' }, +}); + // Absolute path to the `uploads/` directory. // Resolved from the source file location so it works regardless of // process.cwd() — which may not be the backend root (e.g. on a @@ -152,7 +167,7 @@ export function registerMiddleware(app: Express, config: any): void { app.use(express.json()); // 7.5. Global Program Scoping (soft) - app.use(programScope({ required: false })); + app.use(globalProgramScopeLimiter, programScope({ required: false })); // 8. Minimal Cookie parser app.use((req: Request, _res: Response, next: (e?: unknown) => void) => { diff --git a/apps/backend/src/modules/search/search.routes.ts b/apps/backend/src/modules/search/search.routes.ts index 964ed785..b70ee257 100644 --- a/apps/backend/src/modules/search/search.routes.ts +++ b/apps/backend/src/modules/search/search.routes.ts @@ -45,7 +45,11 @@ const searchLimiter = rateLimit({ }); // ── Public search ────────────────────────────────────────────────────────── -router.get('/trending', programScope({ required: false }), getTrending); +// CodeQL fix (js/missing-rate-limiting): the limiter must run BEFORE +// programScope, not after — programScope does its own DB lookup +// (Batch.findById), so putting the limiter downstream of it let a +// scripted flood hit the DB on every request regardless of the cap. +router.get('/trending', searchLimiter, programScope({ required: false }), getTrending); router.get('/suggest', suggestLimiter, getSuggest); // ── Semantic search (public — no auth required) ───────────────────────────── @@ -56,8 +60,8 @@ router.get('/suggest', suggestLimiter, getSuggest); // chars per the schema in validation.ts) and `limit` (1..50). router.post( '/', - programScope({ required: false }), searchLimiter, + programScope({ required: false }), validateBody(searchSchema), semanticSearch );