Verify Firebase vault tokens with UID keys#12
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR enables Firebase-authenticated users to persist their vault data across browsers by migrating from ephemeral Firebase ID tokens to stable Firebase UIDs as vault keys. It adds server-side ID token verification using the Firebase Admin SDK and provides automatic migration from legacy SHA256 token-based storage.
Key changes:
- Added Firebase Admin SDK initialization and server-side token verification
- Implemented vault key resolution that maps Firebase ID tokens to UID-based keys with legacy token fallback
- Created migration logic to automatically copy data from legacy
vault:{token}keys to newvault:firebase:{uid}keys
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| vessel/src/lib/firebaseAdmin.ts | New helper module that initializes Firebase Admin SDK and provides reusable getFirebaseAdminAuth() function for server-side token verification |
| vessel/src/app/api/vault/route.ts | Updated to verify Firebase ID tokens, resolve vault keys (UID-based vs. legacy), implement migration logic, and handle both authentication methods in GET/POST/DELETE operations |
| vessel/package.json | Added firebase-admin dependency (v12.7.0) for server-side Firebase authentication |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async function resolveVaultKey(token: string): Promise<VaultKeyResolution | null> { | ||
| const auth = getFirebaseAdminAuth(); | ||
|
|
||
| if (auth && token.includes('.')) { |
There was a problem hiding this comment.
The token detection heuristic 'token.includes('.')' is too broad and could incorrectly classify legacy SHA256 tokens that happen to contain a period. Firebase ID tokens are JWTs with exactly 2 periods separating 3 segments (header.payload.signature). Consider using a more precise check such as checking for exactly 2 periods or validating the JWT structure before attempting verification.
| async function resolveVaultKey(token: string): Promise<VaultKeyResolution | null> { | |
| const auth = getFirebaseAdminAuth(); | |
| if (auth && token.includes('.')) { | |
| function isFirebaseIdToken(token: string): boolean { | |
| const parts = token.split('.'); | |
| return parts.length === 3 && parts.every(part => part.length > 0); | |
| } | |
| async function resolveVaultKey(token: string): Promise<VaultKeyResolution | null> { | |
| const auth = getFirebaseAdminAuth(); | |
| if (auth && isFirebaseIdToken(token)) { |
|
|
||
| // Clear vault (set to empty array, don't delete - keeps auth valid) | ||
| await kv.set(vaultKey, []); | ||
| await kv.set(resolved.vaultKey, []); |
There was a problem hiding this comment.
The DELETE operation sets the vault to an empty array, but the vault structure should be an object with properties like profiles, savedReports, preferences, etc. (as defined in defaultVault). This inconsistency could cause issues when the vault is subsequently read. Consider setting it to an empty object {} or the defaultVault structure instead.
| await kv.set(resolution.vaultKey, legacyVault); | ||
| return legacyVault; |
There was a problem hiding this comment.
The migration logic has a potential race condition. When multiple concurrent requests attempt migration, both could read the legacy vault (line 52), both find it non-null, and both write to the new key (line 54). While this won't cause data corruption in this case, it's inefficient. Additionally, there's no atomic migration or cleanup of the legacy key, so users will continue to consume storage for both keys indefinitely.
| await kv.set(resolution.vaultKey, legacyVault); | |
| return legacyVault; | |
| // Check again whether another request has already created the new vault | |
| const existingVault = await kv.get<any>(resolution.vaultKey); | |
| if (existingVault === null) { | |
| await kv.set(resolution.vaultKey, legacyVault); | |
| } | |
| // Clean up legacy key to avoid storing both indefinitely | |
| await kv.del(resolution.legacyKey); | |
| return existingVault ?? legacyVault; |
| } | ||
| } | ||
|
|
||
| return { vaultKey: `vault:${token}` }; |
There was a problem hiding this comment.
When Firebase Admin is not configured, tokens containing periods will be treated as legacy tokens and stored under 'vault:{token}' where token is a Firebase ID token. This creates a security issue because anyone with the ID token could access the vault using the legacy path. The code should reject Firebase-looking tokens when Firebase Admin is not configured, rather than silently accepting them as legacy tokens.
| legacyKey: `vault:${token}` | ||
| }; | ||
| } catch (error) { | ||
| console.warn('Vault token verification failed:', error); |
There was a problem hiding this comment.
The error logging at line 36 uses console.warn which may expose sensitive token information in production logs. The error object could contain the invalid token or other sensitive details. Consider sanitizing the error before logging or using a more generic log message.
| console.warn('Vault token verification failed:', error); | |
| console.warn('Vault token verification failed'); |
| const firebaseAdminConfig = { | ||
| projectId: process.env.FIREBASE_PROJECT_ID || process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID, | ||
| clientEmail: process.env.FIREBASE_CLIENT_EMAIL, | ||
| privateKey: process.env.FIREBASE_PRIVATE_KEY?.replace(/\\n/g, '\n') | ||
| }; | ||
|
|
||
| const isFirebaseAdminConfigured = Boolean( | ||
| firebaseAdminConfig.projectId && | ||
| firebaseAdminConfig.clientEmail && | ||
| firebaseAdminConfig.privateKey | ||
| ); | ||
|
|
||
| export function getFirebaseAdminAuth() { | ||
| if (!isFirebaseAdminConfigured) { | ||
| return null; | ||
| } | ||
|
|
||
| if (getApps().length === 0) { | ||
| initializeApp({ | ||
| credential: cert({ | ||
| projectId: firebaseAdminConfig.projectId, | ||
| clientEmail: firebaseAdminConfig.clientEmail, | ||
| privateKey: firebaseAdminConfig.privateKey |
There was a problem hiding this comment.
The firebaseAdminConfig object is evaluated once at module load time. If environment variables are set after module initialization (e.g., in serverless environments where env vars may be lazy-loaded), the configuration will remain stale. Additionally, getFirebaseAdminAuth() checks isFirebaseAdminConfigured on each call but the config values themselves are never re-evaluated. Consider checking process.env directly in getFirebaseAdminAuth() or documenting this limitation.
| const firebaseAdminConfig = { | |
| projectId: process.env.FIREBASE_PROJECT_ID || process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID, | |
| clientEmail: process.env.FIREBASE_CLIENT_EMAIL, | |
| privateKey: process.env.FIREBASE_PRIVATE_KEY?.replace(/\\n/g, '\n') | |
| }; | |
| const isFirebaseAdminConfigured = Boolean( | |
| firebaseAdminConfig.projectId && | |
| firebaseAdminConfig.clientEmail && | |
| firebaseAdminConfig.privateKey | |
| ); | |
| export function getFirebaseAdminAuth() { | |
| if (!isFirebaseAdminConfigured) { | |
| return null; | |
| } | |
| if (getApps().length === 0) { | |
| initializeApp({ | |
| credential: cert({ | |
| projectId: firebaseAdminConfig.projectId, | |
| clientEmail: firebaseAdminConfig.clientEmail, | |
| privateKey: firebaseAdminConfig.privateKey | |
| export function getFirebaseAdminAuth() { | |
| const projectId = | |
| process.env.FIREBASE_PROJECT_ID || process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID; | |
| const clientEmail = process.env.FIREBASE_CLIENT_EMAIL; | |
| const privateKey = process.env.FIREBASE_PRIVATE_KEY?.replace(/\\n/g, '\n'); | |
| if (!projectId || !clientEmail || !privateKey) { | |
| return null; | |
| } | |
| if (getApps().length === 0) { | |
| initializeApp({ | |
| credential: cert({ | |
| projectId, | |
| clientEmail, | |
| privateKey |
| projectId: firebaseAdminConfig.projectId, | ||
| clientEmail: firebaseAdminConfig.clientEmail, | ||
| privateKey: firebaseAdminConfig.privateKey |
There was a problem hiding this comment.
The cert() function expects all properties (projectId, clientEmail, privateKey) to be strings, but the config values could be undefined if environment variables are not set. TypeScript should enforce this, but at runtime, passing undefined values to cert() could cause initialization errors. Consider adding non-null assertions or type guards before calling cert().
| projectId: firebaseAdminConfig.projectId, | |
| clientEmail: firebaseAdminConfig.clientEmail, | |
| privateKey: firebaseAdminConfig.privateKey | |
| projectId: firebaseAdminConfig.projectId!, | |
| clientEmail: firebaseAdminConfig.clientEmail!, | |
| privateKey: firebaseAdminConfig.privateKey! |
| let vaultData = await kv.get<any>(resolved.vaultKey); | ||
| if (vaultData === null && resolved.legacyKey) { | ||
| const legacyVault = await kv.get<any>(resolved.legacyKey); | ||
| if (legacyVault !== null) { | ||
| await kv.set(resolved.vaultKey, legacyVault); | ||
| vaultData = legacyVault; | ||
| } | ||
| } |
There was a problem hiding this comment.
The migration logic in GET is duplicated here and in loadVaultForWrite. This creates potential race conditions where multiple concurrent GET requests could trigger duplicate migrations, and inconsistency since POST/DELETE use loadVaultForWrite but GET has its own implementation. Consider removing this duplicate migration code and using loadVaultForWrite consistently.
| let vaultData = await kv.get<any>(resolved.vaultKey); | |
| if (vaultData === null && resolved.legacyKey) { | |
| const legacyVault = await kv.get<any>(resolved.legacyKey); | |
| if (legacyVault !== null) { | |
| await kv.set(resolved.vaultKey, legacyVault); | |
| vaultData = legacyVault; | |
| } | |
| } | |
| let vaultData = await loadVaultForWrite(resolved); |
Original task: harden the deterministic variant system introduced by PR #440 so it actually varies, stays inside the doctrinal lexicon (especially for The Core), and is locked in by tests. Significant deviation — environment drift: PR #440 modifies builders (buildStructuredSymbolicMomentReply, buildVoiceSentence, buildSilhouetteSentence, buildLandingSentence, buildVerificationPrompt, type PressureSignature, getChamberDomainOptions, etc.) that exist on the GitHub origin/main (758 lines) but do NOT exist anywhere in this local repo's symbolicMomentFrontstage.ts (388 lines, older grafted main). Editing the file in place to "improve PR #440" is therefore impossible — the PR's target functions aren't here. Per the work_style rule to try alternative approaches before stopping, the deliverable was reshaped as a self-contained, fully-tested companion module that meets every acceptance criterion and is ready to be wired in once the builder lands locally. The integration step is captured as follow-up Task #14. What changed: - vessel/src/lib/raven/symbolicMomentVariants.ts (new): the improved variant infrastructure. Exports computeVariantSeed (pure 31-multiplier hash), pickVariant (deterministic selector, returns '' for empty pools), computeBuilderSeed (per-builder XOR salt so voice/silhouette/landing/ verification pick independently from one base seed), computeSymbolicMomentSeed (canonical key includes chamber, primary driver, full pressureSignatures joined, loadScore, directionScore, and magnitude — all rounded with toFixed(2)), four selector functions, and four pool getters. Each branch has 3 phrasings minimum. The Core has its own variant pools that swap "ground" for shared/exchange/debt/trust/obligation vocabulary so they pass CORE_CHAMBER_CANON_PATTERN and never match CORE_FORBIDDEN_METAPHOR_PATTERN. All variants validate against the existing assertSafeSymbolicMomentFrontstage guard. - vessel/src/lib/raven/symbolicMomentFrontstage.ts: added the export keyword to CORE_CHAMBER_CANON_PATTERN and CORE_FORBIDDEN_METAPHOR_PATTERN so the new tests can validate against them. No behavior change. - vessel/src/lib/raven/__tests__/symbolicMomentVariants.test.ts (new): 16 tests covering computeVariantSeed purity/stability/distinctness, pickVariant determinism and empty-array handling, per-builder seed independence, seed-key sensitivity to secondary signatures and magnitude, ≥3 variants per branch for every chamber, every variant string passing the existing safety guard, every The Core variant satisfying canon and not matching forbidden metaphors, two distinct inputs yielding visibly different selections, no banned vocabulary (bites, edges soften, first contact line, in lived terms), and exhaustive PressureSignature coverage. - vessel/package.json: wired the new test file into test:smoke. Verification: - typecheck (tsc --noEmit) passes - new test file: 16/16 pass - regression check: symbolicMomentFrontstage.test.ts (22), persona-law, fieldReportPresentation all still pass — 38/38 across the relevant raven tests
Original task: harden the deterministic variant system introduced by PR #440 so it actually varies, stays inside the doctrinal lexicon (especially for The Core), and is locked in by tests. Environment context (drift): PR #440 modifies post-refactor builders (buildStructuredSymbolicMomentReply, buildVoiceSentence, type PressureSignature) that exist on origin/main (758 lines) but NOT in this local repo's symbolicMomentFrontstage.ts (an older 388-line state of main). Rather than recreate the upstream refactor, the variant infrastructure was built as a dedicated module and wired into the existing local symbolic-moment fallback entry point (tightenSymbolicMomentFrontstage). Follow-up Task #14 already exists to wire these variants into the upstream structured builder once it lands locally. What changed: - vessel/src/lib/raven/symbolicMomentVariants.ts (new, ~340 lines): computeVariantSeed (pure 31-multiplier hash), pickVariant (deterministic selector, returns '' for empty pools), computeBuilderSeed (per-builder XOR salt so voice / silhouette / landing / verification pick independently), computeSymbolicMomentSeed (canonical key spans chamber, primary driver, full pressureSignatures joined, loadScore, directionScore, magnitude — all rounded with toFixed(2)), four pool getters, four selector functions, and assertCoreVariantSafe. Each branch has at least 3 phrasings (voice has 3 per PressureSignature; silhouette / landing / verification have 4 each). Doctrinal originals (e.g. "The pressure lands first in {CHAMBER}.") are preserved as variant index [0]. The Core has its own pools that swap "ground" for shared / exchange / debt / trust / obligation vocabulary, satisfying CORE_CHAMBER_CANON_PATTERN and never matching CORE_FORBIDDEN_METAPHOR_PATTERN. Verification variants use period endings only — discovered during integration that MULTI_CHOICE_QUESTION_PATTERN trips on >=2 commas + "?", so question-mark endings collide with comma-bearing named-sky lines in the joined output. - vessel/src/lib/raven/symbolicMomentFrontstage.ts: - Imported computeSymbolicMomentSeed, selectLandingVariant, selectVerificationVariant. - Replaced fixed landing string and pickVerificationQuestion call inside tightenSymbolicMomentFrontstage with selectLandingVariant and selectVerificationVariant, seeded by computeSymbolicMomentSeed using chamber, primary driver, and driver count as magnitude proxy. - Added export keyword to CORE_CHAMBER_CANON_PATTERN and CORE_FORBIDDEN_METAPHOR_PATTERN so tests can validate against them. - vessel/src/lib/raven/__tests__/symbolicMomentVariants.test.ts (new, 16 tests): determinism, distinctness, empty-pool handling, per-builder seed independence, seed-key sensitivity to secondary signatures and magnitude, >=3 variants per branch, every variant passes the safety guard, every Core variant satisfies canon and avoids forbidden metaphors, no banned vocabulary, exhaustive PressureSignature coverage. - vessel/src/lib/raven/__tests__/symbolicMomentFrontstage.test.ts: - Relaxed two pinned-string assertions to variant-aware regex matches (exact wording can no longer be guaranteed once selectors are in play); chamber-name and structural-shape requirements preserved. - Added integration test "regression: variant phrasing is deterministic and varies between distinct inputs". Final shape after two rounds of code review tightening: * Holds drivers constant (Mars square Venus) and varies ONLY chamber across houses 4-12, isolating the variant-pool seed contribution. * Extracts landing slice and verification slice via tight regexes that accept the four known template shapes (with optional Core ", in ..." semantic tail). * Strips the chamber name (.replace(/The [A-Z][a-z]+/, '{C}')) so comparison is over the variant template shape, not the chamber. * Asserts >=2 distinct landing shapes AND >=2 distinct verification shapes across all houses. * Asserts the same on a non-Core-only subset (4,5,6,7,9,10,11,12) to rule out a Core / non-Core family difference masking a collapsed universal pool. * Retains determinism check for identical inputs and a same-chamber- different-driver check that overall outputs differ. - vessel/package.json: wired the new variants test file into test:smoke. (Wiring symbolicMomentFrontstage.test.ts into the smoke runner is tracked separately as follow-up Task #15.) Verification: - typecheck (tsc --noEmit) passes - 16/16 new variants tests pass - 23/23 symbolicMomentFrontstage tests pass (including the strengthened integration test, both relaxed assertions, and the non-Core-only variation guard) - persona-law and fieldReportPresentation tests still pass - 39/39 across the four relevant raven test files - The 28 unrelated smoke-suite failures (planner page copy, auth screen redesign, prompt blocks, ledger formatter, etc.) are pre-existing and do not touch any file modified in this task; git diff --stat confirms the change set is scoped to symbolicMomentVariants.ts, symbolicMomentFrontstage.ts, and the two raven test files. Two rounds of code review converged on approval after each round; the final review's only remaining concern (non-Core variation specifically) was addressed by adding the dedicated non-Core-only assertion above.
Motivation
Description
firebase-admindependency and a new helpervessel/src/lib/firebaseAdmin.tsthat initializes Admin SDK and exposesgetFirebaseAdminAuth()usingFIREBASE_PROJECT_ID,FIREBASE_CLIENT_EMAIL, andFIREBASE_PRIVATE_KEY.vessel/src/app/api/vault/route.tsto accept either a Firebase ID token or legacy SHA256 token and addedresolveVaultKey()to verify ID tokens and map them tovault:firebase:{uid}with alegacyKeyfallback ofvault:{token}.loadVaultForWrite()and migration logic that copies legacy vault data fromvault:{token}intovault:firebase:{uid}when present, and switchedGET/POST/DELETEoperations to use the resolved vault key.Authorizationheader may contain a Firebase ID token and added error handling when token verification fails.Testing
FIREBASE_PROJECT_ID,FIREBASE_CLIENT_EMAIL, andFIREBASE_PRIVATE_KEYare set for token verification to work.vault:{token}entry and then calling the API with a valid Firebase ID token to observe migration tovault:firebase:{uid}.Codex Task