Fix Security Issue: Resolve TOCTOU Race Condition in User Tracking - #9036
Fix Security Issue: Resolve TOCTOU Race Condition in User Tracking#9036tamilr0727-ux wants to merge 1 commit into
Conversation
|
@tamilr0727-ux is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
This PR effectively resolves a critical race condition that could lead to security vulnerabilities by ensuring that concurrent requests are handled correctly. It aligns with previous decisions made to enhance security in the GitHub API, particularly regarding error handling and enforcing cooldowns. Great job on adding regression tests to cover these scenarios! Looking forward to your contributions. 😊 |
|
🚨 Hey @tamilr0727-ux, the CI Pipeline is failing on this PR and it has been marked as Please fix the issues before this can be reviewed. Here's how: 1. Run checks locally before pushing: npm run format:check # Check Prettier formatting
npm run lint # Run ESLint
npm run typecheck # TypeScript type check
npm run test # Run unit tests (Vitest)
npm run build # Verify production build passes2. Auto-fix common issues: npm run format # Auto-fix formatting with Prettier
npm run lint -- --fix # Auto-fix lint errors where possible3. Check the full failure log here: Once you push a fix and the CI passes, the |
Aamod007
left a comment
There was a problem hiding this comment.
This is a great security fix for the TOCTOU race condition by atomically reserving the slot before async validation.
However, the PR is currently blocked by failing CI checks. Please resolve the CI issues so we can get this merged! I am requesting changes to withhold approval until the block is resolved.
Description
Fixes a race condition in
verifyAndDeduplicate()that allows concurrent/api/track-userrequests for the same username to bypass the cooldown and perform duplicate GitHub validations and database writes.Changes
tryAcquire()inservices/github/refresh-policy.ts.Verification
✅
vitest runpasses successfully.✅ Concurrent requests cannot bypass the cooldown.
✅ Only one request can reserve a username within the cooldown window.
✅ GitHub validation and database writes are no longer unnecessarily duplicated.
✅ Existing cooldown behavior remains intact.
Fixes #9035
Pillar
Visual Preview
Can Check in the Files Changed Section,
Checklist before requesting a review:
CONTRIBUTING.mdfile.localhost:3000/api/streak?user=YOUR_USERNAME).npm run formatandnpm run lintlocally and resolved all errors (CI will fail otherwise).feat(themes): ...,fix(calculate): ...).README.mdif I added a new theme or URL parameter.