Security/data leak#114
Open
deepshaswat wants to merge 6 commits intomainfrom
Open
Conversation
…ategories User-reported leaks: - /api/search/accounts returned ES results to anyone (only blocked page>0) - /categories Server Action getCategoryData() returned full tree without auth - "Most Popular Categories" silently rendered nothing on the homepage Deep-dive fixes: - All apps/content Server Actions used `await SignedIn` (a React component) as auth gate which never fired. Replaced with auth() + new requireWriter() helper across analytics, author, calendar, crud-ideas, crud-posts, crud-tags, dashboard, fetch-posts, subscribers - upload-crud (makeFilePublic, deleteFileFromBucket) now require writer auth - /api/upload (apps/content) now requires writer auth - All six apps/content/api/ai/* routes now require writer auth - /api/metadata adds host allowlist + auth gate (closes SSRF) - ensureCreatorRole/syncUserRolesFromClerk now refuse if clerkId arg does not match auth().userId; getUserRolesByEmail now admin-only - creatorops verify route disables OAuth path that trusted client-supplied platformUserId (returns 501 until real handshake is implemented) - fetchReviewsAction stops returning author email/clerkId - creatorActions, /api/accounts, addAccount, refreshYoutubeData all gated - fetchPostByPostUrl restricts to status PUBLISHED - Newsletter subscribe rate limit moved from in-memory Map to Redis - Contact action gets 5/hour Redis rate limit per email - apps/content middleware returns 401/403 JSON for /api/* (was 307 redirect) Most Popular Categories fix: - Empty arrays no longer cached to Redis (was caching [] with no TTL) - Falls back to top-N categories by mapping count when no Category.popular - UI surfaces empty state instead of rendering null - New flush-popular-categories-cache script to wipe poisoned keys Tests: - 935 tests passing (was 847 before, +88 new tests) - Updated 13 test files to mock auth() since handlers now gate on it - Added new security tests: search-categories, newsletter-subscribe, contactAction.security, categoryActions.security, plus expanded search-accounts, accounts, metadata, mostPopularCategoryActions, fetchReviewsActions Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- /api/categories now requires auth (only commented-out callsite existed, but the route was reachable for anyone scraping category trees). - /api/newsletter/unsubscribe renderHtmlPage() now HTML-escapes title and message. Currently called only with static strings — defensive hardening so the helper cannot become an XSS sink later. - creatorops org GET only returns member emails to OWNER/ADMIN members of that org; regular members see id + name + own email. Was leaking every org members email to any peer. - Middleware route matchers cleaned up: removed dead /api/comments and /api/votes (no such routes), added /api/categories and /api/metadata. Tests: 936 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Findings from a five-agent deep-dive audit. Highest-impact fixes first. ## Account-takeover and role-escalation - workers user-sync upserted on email — fresh Clerk acct using victim email could overwrite victim's clerkId and inherit role/claims/links. Now keys on clerkId; user.update refuses to mutate email to one already on a different row. - worker no longer trusts Clerk publicMetadata.roles for role assignment; protected ADMIN_EMAILS is the only elevation path. New users default to USER; existing users keep DB-assigned roles on update. - workers /jobs/* now require a Pub/Sub OIDC bearer token (or a configured WORKER_DEV_TOKEN for local dev). Prior state: the IAM binding was set to allUsers so the Clerk webhook can reach the service, but no per-route auth — anyone could POST forged events. - Clerk webhook verifies Svix against the raw HTTP body (was: re-serialized JSON, which breaks signature byte-equality and is a canonical-form attack surface). Adds a 100 KiB body size cap. - workers user-sync stops persisting the entire Clerk webhook payload; only image_url is kept in webhookPayload. Logs no longer dump PII. ## Content IDOR (any WRITER could overwrite/publish anyone else's posts) - crud-posts.publishPost / updatePost / deletePost / restorePost / unpublishPost / unschedulePost / resendNewsletter now verify the caller owns the post (post.author.clerkId === session.userId) or is ADMIN. - updatePost no longer accepts data.author.id — author is immutable after creation. - createPost derives author from session, ignores client-supplied id. - sendNewsletterBroadcast re-fetches the post from DB rather than trust the caller's postData (writer can no longer broadcast arbitrary HTML under another author's name). EmailLog metadata records the actor. - crud-ideas updateIdea / deleteIdea / convertIdeaToDraft now ownership- gated; createIdea / convertIdeaToDraft attribute to session, not client. ## Author takeover - createAuthor matches by clerkId only (was: clerkId OR email). If the email already belongs to a different clerkId, refuses rather than overwrite — closes the path where a fresh Clerk account using a victim's email could inherit every Post.authorId pointing to that Author. ## Review-system fraud - ReviewValidator drops client-supplied status, verificationStatus, and authorId. Server forces status=PUBLISHED, verificationStatus=IN_PROGRESS, authorId=session — closes the "self-mark VERIFIED" badge spoof. - createReview refuses if account is suspended/deleted, and refuses duplicate reviews by the same author against the same account. ## Cron / secret hardening - Cron secret comparison uses crypto.timingSafeEqual; refuses if CRON_SECRET is unset (previous compare matched literal "Bearer undefined"). - Unsubscribe HMAC verification uses timingSafeEqual; email is lowercased before HMAC so case variants produce the same token. ## Upload / SSRF / XSS surface - /api/upload + storage.generateUploadUrl gate folderName against an allowlist (no path traversal in S3 keys) and fileType against an image-only MIME allowlist (no text/html with public-read ACL = no XSS payload host on *.digitaloceanspaces.com). - BlockNote youtube block validates iframe src to a known-safe youtube-nocookie embed URL with a verified 11-char ID — was: any URL the writer typed went into iframe src. - convertToEmbeddedUrl (review embeds) does the same canonicalization with a real URL parser; non-YouTube hosts / javascript: URIs return "". - /api/metadata host allowlist and removal of generic-URL fallback was shipped earlier; this round adds nothing there. ## Open redirects - Sign-in / sign-up pages (web + content) validate ?redirect_url= via a shared safeReturnUrl helper that only accepts paths or same-origin URLs. ## Translation actions (cost-amplifier) - translateToEnglish / batchTranslate / detectLanguage now require auth, apply a per-user 60/min Redis rate limit, and cap input at 4 KiB (single) / 16 KiB (batch). Previously unauthenticated server actions reachable from any browser. ## Linked-account access bypass - Account-detail hasAccess now requires a VERIFIED claim (was: claimedAccount OR linkedAccount). A userLinkedAccount row is just a bookmark and never proof of ownership. - unlinkAccount uses deleteMany so non-existent links are indistinguishable from successful deletes (prevents enumeration). ## Subscriber abuse - addSubscriberManually requires ADMIN (was WRITER), creates rows in PENDING (was ACTIVE), refuses to silently re-activate UNSUBSCRIBED users — closes consent-bypass spam vector. - exportSubscribersCSV and deleteSubscriber require ADMIN. - /api/newsletter/verify enforces 24h token expiry, requires status = PENDING, uses a conditional updateMany so two concurrent verify requests cannot both succeed. ## Security headers - web / content / creatorops next.config.mjs now set HSTS, X-Frame- Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy. CSP intentionally deferred — needs origin audit for Clerk + PostHog + YouTube embeds. ## Other - fetchReviewsAction filters out suspended/deleted accounts; clamps page size; extracts only image_url from webhookPayload via a typed helper. - Elasticsearch TLS verification re-enabled by default; only disabled when ELASTIC_INSECURE_TLS=true is set explicitly (logs a warning in production). ## Tests - 979 tests pass across 43 files (was 936; +43 new security gate tests). - Updated 6 test files to reflect new behavior; added IDOR matrix tests for crud-posts, takeover regression tests for createAuthor, host- allowlist tests for convertToEmbeddedUrl, rate-limit/auth tests for translation actions, and ELASTIC_INSECURE_TLS tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The sitemap.ts queries MongoDB at build time, so any environment without DB connectivity (Vercel preview without Atlas IP allowlist, local builds, CI without secrets) fails with a Prisma connect error during the static prerender phase — even though every other page builds fine. Mark the route dynamic + revalidate=3600 so it renders at request time with a 1h cache, and wrap the Prisma calls in try/catch so a transient DB blip returns the static routes alone instead of crashing the request. Tightens the Account query to also filter isDeleted=false (was suspended only). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- yarn.lock: register google-auth-library@^9.0.0 added to apps/workers for the Pub/Sub OIDC verification middleware. - .gitignore: ignore *.tsbuildinfo and untrack the five existing files that were dirtying the working tree on every type-check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
No description provided.