fix: add rate limiting and stronger entropy for classroom invite codes - #785
Conversation
|
@anshul23102 is attempting to deploy a commit to the Karan Mani Tripathi 's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
CodeAnt AI is reviewing your PR. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
github-actions
Bot
added
gssoc'26
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughThis PR adds brute-force protection for classroom invite codes by introducing a Redis-backed ChangesInvite Code Brute-Force Protection
TypeScript Typing Cleanup and Test Fix
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant JoinRoute as rooms/join route
participant Limiter as inviteCodeLimiter
participant DB
Client->>JoinRoute: POST invite code
JoinRoute->>Limiter: limit(clientIp)
alt limit exceeded
Limiter-->>JoinRoute: fail
JoinRoute-->>Client: 429 Retry-After 60
else allowed
Limiter-->>JoinRoute: success
JoinRoute->>DB: validate invite code and join
DB-->>JoinRoute: membership result
JoinRoute-->>Client: join response
end
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| export async function POST(req: NextRequest) { | ||
| try { | ||
| // Apply rate limiting to prevent brute-force enumeration of invite codes | ||
| const ip = req.headers.get('x-forwarded-for') || req.headers.get('x-real-ip') || 'unknown'; |
There was a problem hiding this comment.
Suggestion: The limiter key trusts x-forwarded-for first and as-is, which is attacker-controlled in many deployments and can be rotated to bypass the rate limit; it can also include comma-separated chains instead of a single IP. Use the trusted proxy-derived IP first (e.g., x-real-ip/platform IP), and if using x-forwarded-for, extract and normalize only the first address. [security]
Severity Level: Critical 🚨
- ❌ Invite-code join endpoint rate limit easily bypassed.
- ⚠️ Classroom privacy weakened via brute-force enumeration attempts.Steps of Reproduction ✅
1. Note that the invite-code rate limiter `inviteCodeLimiter` is defined in
`src/lib/ratelimit.ts:87-93` as an Upstash `Ratelimit` instance using the provided
`identifier` string as its key; the in-memory fallback at `src/lib/ratelimit.ts:101-121`
similarly keys on the raw `identifier` without parsing or normalization.
2. In the classroom join handler at `src/app/api/rooms/join/route.ts:12-23`, observe that
`POST(req: NextRequest)` reads the client IP into `ip` using `const ip =
req.headers.get('x-forwarded-for') || req.headers.get('x-real-ip') || 'unknown';` (line
15) and immediately passes it to `inviteCodeLimiter.limit(ip)` (line 16).
3. Because `x-forwarded-for` is used first and as-is, in a deployment where the
application trusts client-supplied `x-forwarded-for` (for example, a reverse proxy that
does not overwrite this header), an attacker can send repeated `POST /api/rooms/join`
requests (endpoint mapped from `src/app/api/rooms/join/route.ts`) with different
`x-forwarded-for` values (e.g., `1.1.1.1`, `2.2.2.2`, or comma-separated chains), all from
the same actual source IP.
4. Each distinct header value becomes a separate rate-limit bucket in `inviteCodeLimiter`,
so the attacker’s real IP never reaches the 5-requests-per-minute threshold; invite-code
brute-force attempts remain effectively unthrottled, undermining the brute-force
protection added in this PR.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/rooms/join/route.ts
**Line:** 15:15
**Comment:**
*Security: The limiter key trusts `x-forwarded-for` first and as-is, which is attacker-controlled in many deployments and can be rotated to bypass the rate limit; it can also include comma-separated chains instead of a single IP. Use the trusted proxy-derived IP first (e.g., `x-real-ip`/platform IP), and if using `x-forwarded-for`, extract and normalize only the first address.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| @@ -1,4 +1,5 @@ | |||
| import { NextResponse } from 'next/server'; | |||
| import { crypto } from 'node:crypto'; | |||
There was a problem hiding this comment.
Suggestion: The crypto import uses a non-existent named export, so crypto can be undefined at runtime and classroom creation will fail when generating invite codes. Import randomBytes directly (or import the module namespace) and call that instead. [api mismatch]
Severity Level: Critical 🚨
- ❌ Classroom creation fails with runtime error generating invite code.
- ⚠️ Teachers unable to onboard students into new classrooms.Steps of Reproduction ✅
1. The classroom creation endpoint is implemented in `src/app/api/rooms/route.ts:86-166`
as `export async function POST(req: Request)`, which handles `POST /api/rooms` and creates
a new classroom with an invite code.
2. At the top of the same file, line 2 reads `import { crypto } from 'node:crypto';`,
attempting to import a named export `crypto` from Node’s `node:crypto` module. Node’s
`crypto` API (including the `node:crypto` ESM specifier) does not provide a named `crypto`
export; it exposes functions such as `randomBytes` directly.
3. Later in the POST handler, invite-code generation at
`src/app/api/rooms/route.ts:134-136` uses this import: `const inviteCode =
crypto.randomBytes(6).toString('hex').toUpperCase();`, assuming `crypto` is a module
object with a `randomBytes` method.
4. When a client calls `POST /api/rooms`, execution reaches line 136 and attempts
`crypto.randomBytes(...)`; because the named import `crypto` is `undefined` at runtime,
this throws a `TypeError: Cannot read properties of undefined (reading 'randomBytes')`,
causing the handler to fall into the `catch` block at lines 161-165 and return an error
instead of creating the classroom.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/rooms/route.ts
**Line:** 2:2
**Comment:**
*Api Mismatch: The crypto import uses a non-existent named export, so `crypto` can be undefined at runtime and classroom creation will fail when generating invite codes. Import `randomBytes` directly (or import the module namespace) and call that instead.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
fe71d9d to
34473c9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/api/rooms/route.ts (1)
138-159: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAvoid
db.transaction(...)here.src/configs/db.tsxusesdrizzle-orm/neon-http, and this driver does not support interactive transactions, so this handler will throw at runtime. Use the same single-statement CTE pattern as the invite join route instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/rooms/route.ts` around lines 138 - 159, The room creation flow in the route handler should not use db.transaction because the neon-http Drizzle driver does not support interactive transactions and will fail at runtime. Replace the transactional block in the room creation logic with the same single-statement CTE approach used by the invite join flow, keeping the classroom insert and membership insert atomic without relying on tx.src/configs/schema.ts (1)
74-92: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd a migration for the organization schema.
src/configs/schema.tsnow definesorganizationsTable,organizationMembershipsTable, andclassrooms.organizationId, but the checked-in SQL migrations only add the audit/video-job tables indrizzle/0017_wild_klaw.sqland theinviteCodelength change indrizzle/0018_marvelous_killer_shrike.sql;drizzle/0013_identity_system_update.sqlis unrelated. Without a matching migration, the runtime schema will drift and therooms/organizationsroutes will hit missing tables/columns.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/configs/schema.ts` around lines 74 - 92, The organization-related schema changes in schema.ts are not backed by a checked-in migration, so add a new Drizzle migration that creates organizationsTable, organizationMembershipsTable, and the classrooms.organizationId column plus its foreign key/indexes. Make sure the migration matches the definitions in the schema.ts table declarations and update any affected constraints/defaults there so runtime schema and database stay in sync for the rooms and organizations routes.
🧹 Nitpick comments (9)
drizzle/0017_wild_klaw.sql (1)
29-29: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAlign the composite index with the stale-job cleanup query.
src/inngest/functions.tsfilters stale jobs bystatusandupdatedAt, but this migration only indexesstatus+created_at. That cleanup cron will still scan more rows than necessary asvideo_jobsgrows.♻️ Suggested index change
-CREATE INDEX "video_jobs_status_created_at_idx" ON "video_jobs" USING btree ("status","created_at"); +CREATE INDEX "video_jobs_status_updated_at_idx" ON "video_jobs" USING btree ("status","updated_at");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drizzle/0017_wild_klaw.sql` at line 29, The composite index for stale-job cleanup is mismatched with the query: the cleanup logic in `functions.ts` filters `video_jobs` by `status` and `updatedAt`, but the migration creates `video_jobs_status_created_at_idx` on `status` and `created_at`. Update the index definition in the migration to match the actual cleanup predicate by indexing `status` together with `updatedAt`/its backing column, and keep the index name consistent with the new column order so the cron query can use it efficiently.src/inngest/karma.ts (1)
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyfor both transaction objects.Please verify that Drizzle can't infer narrower
txtypes here;anyremoves compile-time checks inside both mutation blocks and makes later schema changes easier to miss.Also applies to: 204-204
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/inngest/karma.ts` at line 33, The transaction callbacks in karma.ts are using any for the Drizzle tx object, which removes type safety inside the mutation blocks. Replace the explicit any annotations in the db.transaction callbacks with the correct inferred Drizzle transaction type, or let TypeScript infer it directly from db.transaction, and apply the same fix to the other tx occurrence mentioned so schema and query changes remain compile-time checked.src/app/api/karma/route.ts (1)
126-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyfor the transaction object.Please verify that Drizzle can't infer a narrower
txtype here;anyremoves compile-time checks inside the karma write path and makes schema or query-shape regressions harder to catch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/karma/route.ts` at line 126, The transaction callback in db.transaction is using an explicit any for tx, which removes type safety in the karma write path. Replace it with the proper Drizzle transaction type that matches the existing db schema and verify whether Drizzle can infer tx automatically in this callback; if not, use the narrowest specific transaction type available so query and schema changes stay compile-time checked.src/app/api/doubts/action/[id]/route.ts (1)
233-233: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyfor the transaction object.Please verify that Drizzle can't infer a narrower
txtype here;anyremoves compile-time checks inside the edit transaction and makes later schema changes easier to miss.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/doubts/action/`[id]/route.ts at line 233, The transaction callback in db.transaction currently types the tx parameter as any, which removes compile-time safety. Update the edit transaction in the action/[id]/route.ts handler to use Drizzle’s inferred transaction type instead of any, or derive the proper transaction type from the db instance so tx methods remain type-checked. Keep the change localized around the db.transaction async callback and the updated/savedTags logic.src/app/api/organizations/route.ts (1)
87-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyfor the transaction object.Please verify that Drizzle can't infer a narrower
txtype here;anyremoves compile-time checks inside the organization creation transaction and makes later schema changes easier to miss.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/organizations/route.ts` at line 87, The transaction callback in the organization creation flow is using an `any`-typed `tx`, which removes type safety inside `db.transaction`. Update the `createdOrg` transaction handler to use Drizzle’s inferred transaction type instead of `any`, or derive the proper transaction type from `db` so schema operations remain checked at compile time. Keep the fix localized to the organization creation transaction and verify all `tx` usages there still type-check without casts.src/lib/moderation.ts (1)
331-331: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyfor the transaction object.Please verify that Drizzle can't infer a narrower
txtype here;anyremoves compile-time checks inside moderation handling and makes future schema or query-shape regressions harder to catch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/moderation.ts` at line 331, The transaction callback in moderation handling is using an untyped tx argument, which bypasses compile-time safety. Update the db.transaction callback in the moderation flow to use Drizzle’s inferred transaction type instead of any, and verify whether tx can be inferred automatically from db or explicitly typed from the surrounding database client types. Keep the fix localized to the transaction handler in moderation.ts so query checks inside the moderation logic remain type-safe.src/app/api/doubts/[id]/upvote/route.ts (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyfor the transaction object.Please verify that Drizzle can't infer a narrower
txtype here;anyremoves compile-time checks inside the write block and makes schema/column-shape regressions harder to catch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/doubts/`[id]/upvote/route.ts at line 69, The transaction callback in updatedReply = await db.transaction(async (tx: any) => ...) is using any, which removes type safety inside the write block. Replace it with the proper Drizzle transaction type inferred from db.transaction, or explicitly annotate tx with the narrowest transaction type available from the db instance so schema and column access remain checked. Refer to the transaction logic in the upvote route and keep the callback typed without weakening compile-time validation.src/app/api/replies/vote/route.ts (1)
58-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyfor the transaction object.Please verify that Drizzle can't infer a narrower
txtype here;anyremoves compile-time checks inside the reply vote transaction and makes schema or query-shape regressions harder to catch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/replies/vote/route.ts` at line 58, Replace the `any` transaction typing in the `db.transaction` callback inside `reply vote` handling with the proper Drizzle transaction type, or let TypeScript infer it if possible. Check the `result = await db.transaction(async (tx: any) => ...)` block in the `route` handler and use the narrowest transaction type available so query and schema checks remain enforced.src/lib/karma-utils.ts (1)
168-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyfor the transaction object.Please verify that Drizzle can't infer a narrower
txtype here;anyremoves compile-time checks inside the streak update transaction and makes later schema changes easier to miss.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/karma-utils.ts` at line 168, The transaction callback in karma-utils currently types the `tx` parameter as `any`, which removes type safety inside the streak update flow. Replace it with the narrowest Drizzle transaction type that can be inferred or imported from the existing database/transaction helpers around `db.transaction`, and update the callback signature in `db.transaction` so `tx` is fully typed without using `any`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/app/api/rooms/route.ts`:
- Around line 138-159: The room creation flow in the route handler should not
use db.transaction because the neon-http Drizzle driver does not support
interactive transactions and will fail at runtime. Replace the transactional
block in the room creation logic with the same single-statement CTE approach
used by the invite join flow, keeping the classroom insert and membership insert
atomic without relying on tx.
In `@src/configs/schema.ts`:
- Around line 74-92: The organization-related schema changes in schema.ts are
not backed by a checked-in migration, so add a new Drizzle migration that
creates organizationsTable, organizationMembershipsTable, and the
classrooms.organizationId column plus its foreign key/indexes. Make sure the
migration matches the definitions in the schema.ts table declarations and update
any affected constraints/defaults there so runtime schema and database stay in
sync for the rooms and organizations routes.
---
Nitpick comments:
In `@drizzle/0017_wild_klaw.sql`:
- Line 29: The composite index for stale-job cleanup is mismatched with the
query: the cleanup logic in `functions.ts` filters `video_jobs` by `status` and
`updatedAt`, but the migration creates `video_jobs_status_created_at_idx` on
`status` and `created_at`. Update the index definition in the migration to match
the actual cleanup predicate by indexing `status` together with `updatedAt`/its
backing column, and keep the index name consistent with the new column order so
the cron query can use it efficiently.
In `@src/app/api/doubts/`[id]/upvote/route.ts:
- Line 69: The transaction callback in updatedReply = await db.transaction(async
(tx: any) => ...) is using any, which removes type safety inside the write
block. Replace it with the proper Drizzle transaction type inferred from
db.transaction, or explicitly annotate tx with the narrowest transaction type
available from the db instance so schema and column access remain checked. Refer
to the transaction logic in the upvote route and keep the callback typed without
weakening compile-time validation.
In `@src/app/api/doubts/action/`[id]/route.ts:
- Line 233: The transaction callback in db.transaction currently types the tx
parameter as any, which removes compile-time safety. Update the edit transaction
in the action/[id]/route.ts handler to use Drizzle’s inferred transaction type
instead of any, or derive the proper transaction type from the db instance so tx
methods remain type-checked. Keep the change localized around the db.transaction
async callback and the updated/savedTags logic.
In `@src/app/api/karma/route.ts`:
- Line 126: The transaction callback in db.transaction is using an explicit any
for tx, which removes type safety in the karma write path. Replace it with the
proper Drizzle transaction type that matches the existing db schema and verify
whether Drizzle can infer tx automatically in this callback; if not, use the
narrowest specific transaction type available so query and schema changes stay
compile-time checked.
In `@src/app/api/organizations/route.ts`:
- Line 87: The transaction callback in the organization creation flow is using
an `any`-typed `tx`, which removes type safety inside `db.transaction`. Update
the `createdOrg` transaction handler to use Drizzle’s inferred transaction type
instead of `any`, or derive the proper transaction type from `db` so schema
operations remain checked at compile time. Keep the fix localized to the
organization creation transaction and verify all `tx` usages there still
type-check without casts.
In `@src/app/api/replies/vote/route.ts`:
- Line 58: Replace the `any` transaction typing in the `db.transaction` callback
inside `reply vote` handling with the proper Drizzle transaction type, or let
TypeScript infer it if possible. Check the `result = await db.transaction(async
(tx: any) => ...)` block in the `route` handler and use the narrowest
transaction type available so query and schema checks remain enforced.
In `@src/inngest/karma.ts`:
- Line 33: The transaction callbacks in karma.ts are using any for the Drizzle
tx object, which removes type safety inside the mutation blocks. Replace the
explicit any annotations in the db.transaction callbacks with the correct
inferred Drizzle transaction type, or let TypeScript infer it directly from
db.transaction, and apply the same fix to the other tx occurrence mentioned so
schema and query changes remain compile-time checked.
In `@src/lib/karma-utils.ts`:
- Line 168: The transaction callback in karma-utils currently types the `tx`
parameter as `any`, which removes type safety inside the streak update flow.
Replace it with the narrowest Drizzle transaction type that can be inferred or
imported from the existing database/transaction helpers around `db.transaction`,
and update the callback signature in `db.transaction` so `tx` is fully typed
without using `any`.
In `@src/lib/moderation.ts`:
- Line 331: The transaction callback in moderation handling is using an untyped
tx argument, which bypasses compile-time safety. Update the db.transaction
callback in the moderation flow to use Drizzle’s inferred transaction type
instead of any, and verify whether tx can be inferred automatically from db or
explicitly typed from the surrounding database client types. Keep the fix
localized to the transaction handler in moderation.ts so query checks inside the
moderation logic remain type-safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 07e7c988-4c51-404b-8525-dc6b748d1aa6
📒 Files selected for processing (43)
drizzle/0008_audit_logs.sqldrizzle/0013_silky_gateway.sqldrizzle/0017_wild_klaw.sqldrizzle/0018_marvelous_killer_shrike.sqldrizzle/meta/0017_snapshot.jsondrizzle/meta/0018_snapshot.jsondrizzle/meta/_journal.jsonsrc/__tests__/api/teacher-insights.test.tssrc/__tests__/configs/db.test.tssrc/__tests__/inngest/digest-functions.test.tssrc/app/api/admin/moderation/route.tssrc/app/api/admin/overview/route.tssrc/app/api/analytics/export/route.tssrc/app/api/analytics/personal/route.tssrc/app/api/analytics/route.tssrc/app/api/bookmarks/route.tssrc/app/api/classrooms/[id]/export/route.tssrc/app/api/doubts/[id]/upvote/route.tssrc/app/api/doubts/action/[id]/route.tssrc/app/api/doubts/check-similarity/route.tssrc/app/api/doubts/route.tssrc/app/api/invites/[token]/join/route.tssrc/app/api/karma/route.tssrc/app/api/organizations/route.tssrc/app/api/profile/route.tssrc/app/api/recommendations/route.tssrc/app/api/replies/vote/route.tssrc/app/api/resume-analyzer/history/route.tssrc/app/api/roadmap/history/route.tssrc/app/api/rooms/join/route.tssrc/app/api/rooms/members/route.tssrc/app/api/rooms/route.tssrc/app/api/teacher/analytics/route.tssrc/app/api/teacher/insights/route.tssrc/app/profile/page.tsxsrc/configs/schema.tssrc/inngest/functions.tssrc/inngest/karma.tssrc/lib/ai/embeddings.tssrc/lib/karma-utils.tssrc/lib/moderation.tssrc/lib/notifications/service.tssrc/lib/ratelimit.ts
💤 Files with no reviewable changes (2)
- drizzle/0008_audit_logs.sql
- drizzle/0013_silky_gateway.sql
…sage wording
- ask-ai.test.ts: the 'generate AI solution' test predates this PR's
mandatory-classroomId requirement; add a classroomId plus membership/
block-check mocks so it exercises the real (now-required) code path
instead of being rejected with 422 before reaching the AI call.
- ask-ai.test.ts: update the malformed-classroomId assertion to match the
route's actual message ('classroomId is required and must be a valid
integer.') instead of a stale placeholder string.
- teacher-insights.test.ts: same stale-message fix as other PRs in this
batch (400 case distinguishes missing vs malformed IDs; 403 case uses
'Access denied to this classroom').
- digest-functions.test.ts: invoke the Inngest function's .fn handler
directly and fix the sendDigestEmail mock contract, matching the fix
already applied in knoxiboy#784/knoxiboy#785.
Verified locally: tsc clean, eslint clean, npm run build succeeds, 211/211
tests pass across 41 suites.
34473c9 to
7980c11
Compare
|
CodeAnt AI is running Incremental review |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI Incremental review completed. |
- Add inviteCodeLimiter (5 req/min per IP) to prevent brute-force enumeration of classroom invite codes - Apply rate limiting to POST /api/rooms/join endpoint before other validation checks to fail fast on excessive requests - Add inviteCodeLimiter export to ratelimit module with Redis and fallback in-memory implementations - Remove orphan migration files (0008_audit_logs.sql, 0013_silky_gateway.sql) that were not tracked in migration journal, fixing migration integrity - Update jest.setup.ts mock path to @/lib/ratelimit/ratelimit and add inviteCodeLimiter mock - Fix pre-existing TypeScript errors: untyped reduce call in doubts/route.ts, untyped generic in invites/join/route.ts, readonly property error in db.test.ts
3dae6d7 to
811b75f
Compare
|
CodeAnt AI is running Incremental review |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI Incremental review completed. |
- Update teacher-insights test expectations to match actual error messages
('Invalid classroom ID' and 'Access denied to this classroom')
- Add inngest client mock to digest-functions test so sendDailyDigest imports
- Fix sendDigestEmail mock to return { success: true } in retry idempotency test
- Remove typo in comment ('failmockSendDigestEmail' → 'fail')
All 193 tests now passing
|
Hey! Just pushed the test fixes so all CI should be green now. When you get a chance, could you throw these labels on it?
Appreciate the review! 🙏 |
github-actions
Bot
added
gssoc:approved
knoxiboy#785) * fix: add rate limiting and stronger entropy for classroom invite codes - Add inviteCodeLimiter (5 req/min per IP) to prevent brute-force enumeration of classroom invite codes - Apply rate limiting to POST /api/rooms/join endpoint before other validation checks to fail fast on excessive requests - Add inviteCodeLimiter export to ratelimit module with Redis and fallback in-memory implementations - Remove orphan migration files (0008_audit_logs.sql, 0013_silky_gateway.sql) that were not tracked in migration journal, fixing migration integrity - Update jest.setup.ts mock path to @/lib/ratelimit/ratelimit and add inviteCodeLimiter mock - Fix pre-existing TypeScript errors: untyped reduce call in doubts/route.ts, untyped generic in invites/join/route.ts, readonly property error in db.test.ts * test: fix test expectations and inngest mock for digest functions - Update teacher-insights test expectations to match actual error messages ('Invalid classroom ID' and 'Access denied to this classroom') - Add inngest client mock to digest-functions test so sendDailyDigest imports - Fix sendDigestEmail mock to return { success: true } in retry idempotency test - Remove typo in comment ('failmockSendDigestEmail' → 'fail') All 193 tests now passing
2 participants
User description
Security fix for brute-force enumeration vulnerability in classroom invite codes.
Problem
The classroom join endpoint had no rate limiting, and invite codes had weak entropy (6 base36 chars ≈ 2 billion possibilities). An attacker could enumerate valid codes within hours at 10 req/s, discovering and joining private classrooms.
Solution
Security Impact
Files Changed
Closes #753
Summary by CodeRabbit
New Features
Bug Fixes
CodeAnt-AI Description
Block classroom invite-code guessing and keep joining working
What Changed
Impact
✅ Fewer classroom invite-code abuse attempts✅ Clearer join throttling errors✅ Safer classroom invites💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.