Migrate Firestore access to the Admin SDK and ship default-deny rules - #56
Merged
Conversation
Server-side service modules were using the client Firestore SDK, so privileged writes reached Firestore with request.auth == null. The deployed rules were the test-mode default (open until 2026-10-10), which is the only way that could have worked - meaning the database is world-readable and world-writable today, and would start rejecting every client request once test mode expires, taking the app down. - Add firebase-admin and a server-only Admin Firestore singleton (src/lib/firebase-admin.ts), reading credentials from FIREBASE_SERVICE_ACCOUNT_PATH (local file) or FIREBASE_SERVICE_ACCOUNT_JSON (inline, for Vercel), never from a NEXT_PUBLIC_ var. - Switch questionService, commentService, eventService, userService, voteService, and searchService from the client SDK to the Admin instance. Behavior is unchanged - this is purely the SDK swap. - Add 'use server' to searchService.ts, which previously had no directive and queried Firestore straight from the browser. - Add firestore.rules (default-deny for all client access - the Admin SDK bypasses rules, so server-side reads/writes are unaffected), firestore.indexes.json (the missing composite indexes for questions on communityId+createdAt and events on communityId+dateTime), and firebase.json wiring both. - Deployed indexes and rules to v-threads. Verified with a live probe: anonymous client SDK reads on questions/events/users succeeded before the rules deploy and fail with permission-denied after. Fixes #16 Fixes #28
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Aug 9, 2026
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
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.
Fixes #16
Fixes #28
Why
The database was world-readable and world-writable. The deployed rules were still the test-mode default:
Two separate problems with that. First, anyone on the internet could read and write every document using only the
NEXT_PUBLIC_FIREBASE_*config, which ships in the client bundle by design. Second, that rule expires on 2026-10-10 — after which every client request is denied, and because all data access went through the client SDK, the app would have stopped working entirely.The rules could not simply be tightened, though. All five
'use server'service modules imported the Firebase client SDK, so server-side writes arrived withrequest.auth == null. Any rule strict enough to block an anonymous attacker was also strict enough to block the application itself. That is why this had to be an SDK migration rather than a rules change.What changed
New
src/lib/firebase-admin.ts— a server-only Admin Firestore singleton. Credentials come fromFIREBASE_SERVICE_ACCOUNT_JSON(inline, checked first) orFIREBASE_SERVICE_ACCOUNT_PATH(local file), never from aNEXT_PUBLIC_variable. Re-initialisation is guarded withgetApps().length.It deliberately does not carry
'use server': Next.js only permits async-function exports from such files, and this module exports aFirestoreinstance.All six service modules migrated from the client SDK to Admin —
questionService,commentService,eventService,userService,voteService,searchService. The Admin API differs throughout (db.collection(...)chaining,.get(),FieldValue.serverTimestamp(),doc.existsas a property rather than a method), so every call site changed.searchService.tsgained'use server'(#28). It was the one module querying Firestore straight from the browser, so it had to move server-side before rules could deny client access outright.firestore.rules,firestore.indexes.json,firebase.jsonadded so the enforcement boundary and the index definitions live in version control and deploy from the repository instead of existing only in the console.Two missing composite indexes are now defined and deployed:
questions(communityIdASC,createdAtDESC) andevents(communityIdASC,dateTimeDESC). Both were absent, so community-filtered search and events were throwingfailed-precondition— and because the services swallow errors and return[], they were failing silently as empty results.Verification
The load-bearing evidence is a before/after probe using an anonymous client with only the public config and no sign-in:
questionspermission-deniedeventspermission-denieduserspermission-deniedcommentspermission-deniedvotespermission-deniedBefore this change those reads succeeded, which confirmed the exposure was real rather than inferred from the rule text. They now fail. Meanwhile the same reads through the Admin SDK still return 8/3/10, because the Admin SDK bypasses rules by design — so the application keeps working while external access is closed.
Also checked:
npm run buildsucceeds.npx tsc --noEmitreports exactly 7 errors, identical to the pre-change baseline — 2 incommunity/[communityId]/page.tsx, 4 inSettingsContent.tsx, 1 insearchService.ts. No new type errors. (Those 7 are tracked in Re-enable TypeScript build errors and fix the 7 current failures #47;next.config.tscurrently setsignoreBuildErrors, so a green build alone would not have proven this.)/,/qna,/events,/search, both before and after the rules deploy.BUILDINGand the real query shapes stopped returningfailed-precondition— "deploy succeeded" alone is not sufficient for indexes..envfile, service-account key, or private key is committed.Deployment note
Any hosted environment needs
FIREBASE_SERVICE_ACCOUNT_JSONset to the full service-account JSON as a single inline string, with noNEXT_PUBLIC_prefix. The file-path variant is local-only.Explicitly out of scope
The diff is deliberately narrow — an SDK swap plus the directive, with no behavioural changes — so it can be reviewed as one mechanical transformation. Left untouched on purpose:
emailis still projected into search output. Add 'use server' to searchService.ts #28's suggested fix mentions this; it is not addressed here.Event.createdAttype gap (Add author and createdAt to the Event type and drop double casts #48) and itsas unknown as Eventcasts.