fix: atomic usage limit check prevents race condition bypass

[Xenon010101]

What

Fix race condition in usageLimit middleware that allows concurrent requests to bypass the daily usage cap.

Root cause

The middleware counted existing usage logs and then called next(), while the actual log entry was created later by the controller. This gap meant concurrent requests all read the same count before any log was written, so all of them passed the check.

Changes

• usage-limit.middleware.ts: Wrapped count + create in a single $transaction with SELECT ... FOR UPDATE on the user row, serializing concurrent requests
• Removed usageLog.create() calls from 8 controllers/services that run behind usageLimit (ats, cover-letter, resume-gen, student, job-agent, dsa, upload)
• Fixed req.usageInfo.used + 1 to req.usageInfo.used since the count now includes the current request
• Updated external-application test to match new service behavior

Verification

• Concurrent requests for the same user now block on the row lock and see the correct count
• All usageLog.create calls removed from controllers that use the middleware
• External application service tests pass
• Controllers that DON'T use the middleware (mock-interview, latex-chat) keep their own creates

CI failures

N/A

Before

Concurrent requests all read count=5 before any wrote; all 10 requests passed a limit of 5.

After

Each request atomically increments the count; request #6 correctly gets 429.

Closes #1426

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Fixed potential race conditions in usage limit enforcement by implementing atomic database operations

• Refactor

  • Centralized usage tracking to middleware for consistent and reliable behavior across all features

[github-actions]

Hi @Xenon010101, thanks for contributing to InternHack! 🎉

I have automatically:

• 👤 Assigned this PR to you.
• 🏷️ Applied the gssoc:approved label.

Our workflows will now analyze your changes to classify:

• 📈 PR Difficulty: level:*
• 🧩 PR Type: type:*
• 🌟 PR Quality: quality:*

Tip

Ensure your PR description references the issue it resolves (e.g. Closes #123). This allows the bot to inherit any additional labels from that issue!

Happy coding! 🚀

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR fixes a concurrency race condition in the daily usage limit enforcement by moving usage log creation from scattered downstream calls into a single atomic middleware transaction. The middleware now locks the user row, counts today's usage, and pre-creates a usage log entry within a transaction before the request proceeds, eliminating the window where concurrent requests could bypass the daily cap.

Changes

Usage Limit Race Condition Fix

Layer / File(s)	Summary
Atomic usage enforcement in middleware transaction server/src/middleware/usage-limit.middleware.ts	Replaces parallel non-atomic database queries with a single Prisma transaction that locks the user row (SELECT ... FOR UPDATE), counts today's usage for the action, computes the tier-based daily limit, and pre-creates a usageLog entry when under the limit. Returns structured results to control HTTP response flow: 401 for missing user, 429 for over-limit, or proceeds with req.usageInfo attached when capacity is available.
Controller response handlers synchronized with pre-created usage info server/src/module/ats/ats.controller.ts, server/src/module/ats/cover-letter.controller.ts, server/src/module/ats/resume-gen.controller.ts, server/src/module/student/student.controller.ts, server/src/module/job-agent/job-agent.controller.ts, server/src/module/upload/upload.controller.ts	Removes per-request prisma.usageLog.create calls from all downstream handlers. Response payloads now derive the usage.used count directly from req.usageInfo.used (set by the middleware transaction) instead of incrementing by 1, ensuring the reported usage aligns with the slot pre-reserved in the middleware.
External job application transaction and test updates server/src/module/student/student.service.ts, server/src/module/student/student.external-application.service.test.ts	Removes usageLog.create from the applyToExternalJob transaction. Test assertions now validate that the external application is created transactionally without usage logging, and introduces new post-commit side effect tests for badge awarding and milestone checking on success and failure paths.
DSA analytics engagement tracking replaces usage logging server/src/module/dsa/dsa.service.ts	Removes blocking usageLog.create with action "CODE_RUN" from code submission handling. Replaced with non-blocking fire-and-forget contentView write for engagement analytics (time spent, completion status), decoupling rate-limiting concerns from analytics tracking and preventing submission delays from analytics writes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Sachinchaurasiya360/InternHack#1317: Replaces blocking usageLog.create ("CODE_RUN") in DSA service with fire-and-forget contentView analytics, part of the same /learn tracking pipeline migration.
• Sachinchaurasiya360/InternHack#1311: Related changes to StudentController.applyToExternalJob and StudentService.applyToExternalJob around usage limit enforcement and quota accounting for job applications.

Suggested labels

level:advanced, type:bug, type:refactor, scope:backend, quality:clean, type:testing

Suggested reviewers

• Sachinchaurasiya360

Poem

🐰 Hops with glee through the middleware gate,
Where atomicity seals each request's fate—
No race, no bypass, the limit holds true,
One transaction to rule them all through! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing an atomic usage limit check to prevent race condition bypass.
Description check	✅ Passed	The PR description is comprehensive, covering what was fixed, root cause, changes made, verification steps, and links to the related issue.
Linked Issues check	✅ Passed	The PR directly addresses issue #1426 by implementing atomic usage limit checks with row locks and removing distributed log creation to prevent concurrent-request bypass.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the usage limit race condition: middleware atomicity, controller log removal, and test updates; no unrelated modifications present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

server/src/module/student/student.external-application.service.test.ts (1)

12-14: 💤 Low value

Consider removing unused usageLog mocks.

The usageLog mocks in the hoisted setup and tx object are no longer asserted in any test case since usage logging was moved to the middleware. These can be safely removed to keep the test setup minimal.

🧹 Proposed cleanup

 const mocks = vi.hoisted(() => ({
   prisma: {
     $transaction: vi.fn(),
     adminJob: {
       findUnique: vi.fn(),
     },
     externalJobApplication: {
       create: vi.fn(),
     },
-    usageLog: {
-      create: vi.fn(),
-    },
   },
   tx: {
     externalJobApplication: {
       create: vi.fn(),
     },
-    usageLog: {
-      create: vi.fn(),
-    },
   },
   badgeService: {
     checkAndAwardBadges: vi.fn().mockResolvedValue(undefined),
   },
 }));

Also applies to: 20-22

🤖 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 `@server/src/module/student/student.external-application.service.test.ts`
around lines 12 - 14, Remove the now-unused usageLog mocks from the hoisted test
setup and from the tx/mock transaction object: delete the usageLog: { create:
vi.fn() } entries and any references to usageLog inside the tx mock used in
student.external-application.service.test.ts, keeping the rest of the tx
structure intact so tests still hit the same mocked DB methods; search for
"usageLog" and remove those mock definitions in the file (including the hoisted
setup and the tx object) to keep the test fixtures minimal.

🤖 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.

Nitpick comments:
In `@server/src/module/student/student.external-application.service.test.ts`:
- Around line 12-14: Remove the now-unused usageLog mocks from the hoisted test
setup and from the tx/mock transaction object: delete the usageLog: { create:
vi.fn() } entries and any references to usageLog inside the tx mock used in
student.external-application.service.test.ts, keeping the rest of the tx
structure intact so tests still hit the same mocked DB methods; search for
"usageLog" and remove those mock definitions in the file (including the hoisted
setup and the tx object) to keep the test fixtures minimal.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 732468b9-fd9b-496c-893b-3f156ecdc5aa

📥 Commits

Reviewing files that changed from the base of the PR and between 0c176ae and 819081f.

📒 Files selected for processing (10)

• server/src/middleware/usage-limit.middleware.ts
• server/src/module/ats/ats.controller.ts
• server/src/module/ats/cover-letter.controller.ts
• server/src/module/ats/resume-gen.controller.ts
• server/src/module/dsa/dsa.service.ts
• server/src/module/job-agent/job-agent.controller.ts
• server/src/module/student/student.controller.ts
• server/src/module/student/student.external-application.service.test.ts
• server/src/module/student/student.service.ts
• server/src/module/upload/upload.controller.ts

💤 Files with no reviewable changes (4)

• server/src/module/upload/upload.controller.ts
• server/src/module/job-agent/job-agent.controller.ts
• server/src/module/student/student.service.ts
• server/src/module/dsa/dsa.service.ts