fix: remove SUPABASE_SECRET_KEY fallback from encryptionKey()

[bmersereau]

Summary

• Removes process.env.SUPABASE_SECRET_KEY from the fallback chain in encryptionKey() in backend/src/lib/userApiKeys.ts
• USER_API_KEYS_ENCRYPTION_SECRET (or its alias API_KEYS_ENCRYPTION_SECRET) is now required; the function throws if neither is set
• Previously, a missing dedicated encryption secret silently fell back to the JWT verification secret, violating secret isolation (PR fix: require dedicated secrets for download signing and API key encryption #74 / issue [Security][Critical] SUPABASE_SECRET_KEY triple-reuse — single key leak = total system compromise #66)

Closes #98
Closes #114
Closes #117
Closes #118

Changes

• backend/src/lib/userApiKeys.ts — remove SUPABASE_SECRET_KEY from encryptionKey() fallback chain
• backend/src/lib/__tests__/encryptionKey.test.ts — tests that SUPABASE_SECRET_KEY fallback is gone and dedicated secret is required; vi.mock hoisted to module top-level
• backend/package.json / backend/package-lock.json — add vitest dev dependency and "test": "vitest run" script
• backend/vitest.config.ts — include filter scoping tests to src/ to exclude compiled dist/ artifacts

Test plan

• Unit tests added and passing (3/3)
• Backend build passes

[bmersereau]

PR Review

Summary

Removes the SUPABASE_SECRET_KEY fallback from encryptionKey() in userApiKeys.ts, closing a secret-isolation vulnerability where the JWT verification key could silently double as the AES-256-GCM encryption key when USER_API_KEYS_ENCRYPTION_SECRET was not set. The fix is correct and the test suite explicitly verifies the removal. vi.mock is correctly hoisted at module top-level (not inside describe/it).

Findings

• [severity:low] vi.resetModules() in beforeEach re-instantiates the module under test on each dynamic import(), which is correct. However, vi.resetModules() combined with a top-level vi.mock depends on Vitest's mock-registry persistence across module resets — this currently works as expected (tests pass), but it's a subtle interaction worth noting in a comment so future maintainers don't remove the top-level vi.mock.
• [severity:nit] beforeEach cleans USER_API_KEYS_ENCRYPTION_SECRET, API_KEYS_ENCRYPTION_SECRET, and SUPABASE_SECRET_KEY from the environment. There is no corresponding afterEach to restore them. If env vars were set in CI before this test runs, the beforeEach delete may interfere with subsequent test files. Low risk since isolate: true is set, but worth adding afterEach cleanup for hygiene.
• [severity:nit] The API_KEYS_ENCRYPTION_SECRET fallback is retained (only SUPABASE_SECRET_KEY is removed). If API_KEYS_ENCRYPTION_SECRET is a deprecated alias for USER_API_KEYS_ENCRYPTION_SECRET, a comment or deprecation notice would help operators know which to use.

Verdict

Approve with nits

What I verified

• Tests: pass (3/3)
• vitest.config.ts has include filter: pass ("src/**/__tests__/**/*.test.ts")
• package.json has "test": "vitest run": pass
• vi.mock is at module top-level (not inside describe/it): pass
• SUPABASE_SECRET_KEY removed from fallback chain in userApiKeys.ts: pass
• Test explicitly covers SUPABASE_SECRET_KEY-only case now throwing: pass

[bmersereau]

PR Review

Summary

Removes SUPABASE_SECRET_KEY from the encryptionKey() fallback chain in userApiKeys.ts. Three tests verify the SUPABASE_SECRET_KEY fallback is gone and that a dedicated secret is required. vi.mock is correctly hoisted to module top-level (issue #118 resolved).

Findings

• [severity:praise] vi.mock at module top-level, before describe — hoisting semantics correct ✓
• [severity:praise] API_KEYS_ENCRYPTION_SECRET alias retained for backward-compat with existing deployments that set the old name
• [severity:nit] Test name "throws when only SUPABASE_SECRET_KEY is set (fallback removed)" is slightly verbose but clear

Specific checks

• "test": "vitest run" in package.json ✓
• vitest.config.ts include filter present ✓
• vi.mock is top-level ✓ (issue #118)
• Tests pass: 3/3 ✓

Verdict

Approve — clean.

[willchen96]

Thanks Beau, this has been fixed in a more targeted PR #137 without the test script