feat(achievements): add progress estimation for locked achievements#1938
Open
Ridanshi wants to merge 3 commits into
Open
feat(achievements): add progress estimation for locked achievements#1938Ridanshi wants to merge 3 commits into
Ridanshi wants to merge 3 commits into
Conversation
|
@Ridanshi is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
gssoc26
GSSoC Label Checklist 🏷️@Priyanshu-byte-coder — please apply the appropriate labels before merging: Difficulty (pick one):
Quality (optional):
Validation (required to score):
|
Closes Priyanshu-byte-coder#1878 Security analysis ───────────────── Authentication architecture: - NextAuth JWT session cookie (SameSite=Lax, HttpOnly, Secure in prod) used by all browser-authenticated routes - Bearer CRON_SECRET for /api/cron/*, /api/wakatime/sync, /api/sponsors/sync, /api/notifications/discord-sync - Bearer API-key for /api/local-coding/sync - HMAC x-hub-signature-256 for /api/webhooks/github Root cause: No CSRF protection existed anywhere in the codebase. All browser-session mutation routes (POST/PUT/PATCH/DELETE) accepted requests from any origin. SameSite=Lax provides partial mitigation but does not cover older browsers, top-level form submissions, or future cookie configuration changes. Protection strategy ─────────────────── Centralized Origin / Referer validation in src/lib/csrf.ts, enforced for every mutation API request at the middleware layer (src/middleware.ts). Decision table for checkCsrfOrigin(): - GET/HEAD/OPTIONS → pass (not mutations) - Authorization: Bearer present → pass (machine client, no cookie) - Origin matches trusted set → pass - Origin present, untrusted → 403 Forbidden - No Origin, Referer trusted → pass - No Origin, Referer untrusted → 403 Forbidden - No Origin, no Referer → pass (server-side / native client) Trusted origins: NEXTAUTH_URL, NEXT_PUBLIC_APP_URL, ALLOWED_ORIGINS env var. In non-production: localhost:3000 and 127.0.0.1:3000 are always trusted. Protected routes (examples) ─────────────────────────── POST/PATCH/DELETE /api/goals, /api/goals/[id] PATCH /api/user/settings DELETE /api/user/data-export (account deletion) POST /api/daily-note POST/DELETE /api/local-coding/keys POST/PATCH/DELETE /api/webhooks/custom, /api/webhooks/custom/[id] PATCH/DELETE /api/notifications, /api/notifications/[id] POST/DELETE /api/streak/freeze POST /api/rooms, /api/integrations/jira/credentials POST /api/goals/sync, /api/contact Exempted routes ─────────────── /api/webhooks/github — HMAC signature authentication /api/cron/* — Bearer CRON_SECRET /api/wakatime/sync — Bearer CRON_SECRET /api/sponsors/sync — Bearer CRON_SECRET /api/notifications/discord-sync — Bearer CRON_SECRET /api/local-coding/sync — Bearer API-key /api/auth/* — NextAuth internal endpoints Any request with Authorization: Bearer is also exempt (machine clients cannot be tricked into attaching browser session cookies). Files changed ───────────── src/lib/csrf.ts — new: checkCsrfOrigin(), isCsrfExemptPath(), isMutationMethod(); reads NEXTAUTH_URL, NEXT_PUBLIC_APP_URL, ALLOWED_ORIGINS src/middleware.ts — CSRF check runs first on all /api/* mutations; matcher extended to /api/:path*; rate limiting scoped to /api/metrics/* and /api/contact src/lib/github-fetch.ts — add GitHubAuthError and githubAuthErrorResponse for pre-existing achievement-progress route .env.example — document ALLOWED_ORIGINS Tests added (test/csrf.test.ts — 61 tests) ─────────────────────────────────────────── isMutationMethod: POST/PUT/PATCH/DELETE true; GET/HEAD/OPTIONS false isCsrfExemptPath: all exempt prefixes pass; all protected routes fail checkCsrfOrigin: - GET requests always pass - Bearer token exemption for all mutation methods - non-Bearer auth not exempt - trusted Origin passes for all mutation methods - untrusted Origin rejected for all mutation methods - Referer fallback: trusted passes, untrusted rejected, malformed rejected - Origin takes precedence over Referer (both directions) - no headers allowed (server-side / native client) - ALLOWED_ORIGINS: single, multiple, malformed entries, unlisted rejected - NEXT_PUBLIC_APP_URL trusted, unrelated origin rejected - webhook/cron/bearer integration scenarios all pass Verification ──────────── next lint → warnings only (pre-existing), no errors tsc --noEmit → clean vitest run → 61 CSRF tests pass; total suite: 1229 passed (remaining failures are pre-existing branch issues unrelated to CSRF, confirmed via git stash)
Ridanshi
force-pushed
the
feat/achievement-progress-estimator
branch
from
32f99d9 to
4e8e8c0
Compare
Owner
|
This PR has merge conflicts with git fetch origin
git rebase origin/main
git push --force-with-lease |
Upstream rewrote the in-memory rate-limiter (inline memoryBuckets Map, pruneMemoryBuckets, new getIp without external dep). This PR added auth-endpoint rate limiting (checkAuthRateLimit/isAuthSensitivePath) and CSRF protection. Both sets of changes edited the same region of src/middleware.ts causing a conflict. Resolution keeps all upstream improvements: - inline memoryBuckets Map replaces createMemoryFixedWindowRateLimiter - pruneMemoryBuckets for bounded memory growth - getIp reads headers directly (no external getClientIp) - new checkMemoryLimit using the inline bucket store And all PR functionality: - checkAuthRateLimit/isAuthSensitivePath/AUTH_LIMIT imports - checkCsrfOrigin/isCsrfExemptPath/isMutationMethod imports - export const runtime = nodejs - CSRF check block before token resolution - auth-sensitive-path rate-limit block - two-cookie getToken for Playwright compatibility - /api/:path* matcher covering CSRF and auth paths
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
2 participants
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.
Closes #1754
Implementation Summary
Adds an evidence-based progress estimator for locked GitHub achievements. Progress is only shown where real contribution data exists as a proxy. Achievements with no reliable data source display "Progress unavailable" explicitly — no fabricated percentages.
Achievement Estimation Strategy
viewer.pullRequests(states:[MERGED]).totalCountviewer.repositoryDiscussionComments(onlyAnswers:true).totalCountBoth estimable achievements are fetched in a single GitHub GraphQL round-trip.
Files Changed
New files:
src/lib/achievement-progress.ts— pure estimator functions and types (AchievementProgressInfo,AchievementMilestone,computeMilestoneProgress, per-achievement estimators,buildLockedAchievementProgress)src/app/api/metrics/achievement-progress/route.ts— GET endpoint; GraphQL fetch, 10-min cache, token-revocation handlingsrc/components/GitHubAchievementProgress.tsx— client widget with progress-bar cards; "Estimated progress" label; "Progress unavailable" fallbacktest/achievement-progress.test.ts— 48 testsModified:
src/app/dashboard/page.tsx— addsGitHubAchievementProgressto the Goals & Insights sidebarTests Added
48 tests in
test/achievement-progress.test.ts:computeMilestoneProgress— boundary cases, empty milestones, progress clampingestimatePullSharkProgress— Bronze/Silver/Gold tiers,progressDescriptionaccuracy,currentValuetrackingestimateGalaxyBrainProgress— same coverage for Galaxy Brain thresholdsgetQuickdrawProgress—dataAvailable: false, "Progress unavailable" outputgetPairExtraordinaireProgress— same coveragebuildLockedAchievementProgress— unlocked-achievement omission, null data handling, mixed batches, full-unlock empty resultVerification