feat(developer-api): Add API key creation flow with OAuth billing provider auth#124
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
size/XL
|
|
github-actions
Bot
added
scope/shell
|
🗃️ Database Migration Detected This PR includes changes to Prisma schema files. Please ensure:
Requesting review from the core team: @livepeer/core |
This was
linked to
issues
Feb 18, 2026
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OAuth-based billing-provider auth (start/callback/result), an in-memory session store, DB schema and resolver for Dev API projects/keys, refactors developer API key flows (validation, storage, metadata), updates frontend key management UI, and conditions PrismaPlugin to production server builds. Changes
Sequence DiagramssequenceDiagram
participant Client as Client
participant StartAPI as Start API
participant Sessions as Session Store
participant Provider as Billing Provider
participant CallbackAPI as Callback API
participant Database as Prisma DB
participant ResultAPI as Result Polling API
Client->>StartAPI: POST /auth/providers/:provider/start
StartAPI->>Sessions: Create session (loginSessionId, state)
Sessions-->>StartAPI: ack
StartAPI-->>Client: auth_url + login_session_id
Client->>Provider: Redirect to auth_url
Provider->>Client: Redirect to callback with token & state
Client->>CallbackAPI: GET /callback?token=...&state=...
CallbackAPI->>Sessions: Lookup by state
Sessions-->>CallbackAPI: session
CallbackAPI->>Provider: Exchange token -> api_key
Provider-->>CallbackAPI: api_key
CallbackAPI->>Database: Update session (status=complete, accessToken)
Database-->>CallbackAPI: updated
CallbackAPI-->>Client: Success HTML (auto-close)
loop Polling
Client->>ResultAPI: GET /result?login_session_id=...
ResultAPI->>Sessions: Fetch session
Sessions-->>ResultAPI: session state
alt status=complete & not redeemed
ResultAPI->>Database: Redeem session atomically
Database-->>ResultAPI: redeemed result + access_token
ResultAPI-->>Client: {status: complete, access_token}
else status=pending/expired/redeemed
ResultAPI-->>Client: {status: pending/expired/redeemed}
end
end
sequenceDiagram
participant Client as User (UI)
participant Frontend as Create Key Modal
participant CSRF as CSRF Service
participant Backend as Keys API
participant ProjectResolver as resolveDevApiProjectId
participant Prisma as Database
Client->>Frontend: Open Create Key, choose project/provider
Frontend->>ProjectResolver: Resolve/create project (userId, name)
ProjectResolver->>Prisma: find/create DevApiProject
Prisma-->>ProjectResolver: projectId
ProjectResolver-->>Frontend: projectId
Frontend->>CSRF: Fetch CSRF token
CSRF-->>Frontend: csrf_token
Client->>Frontend: Submit Create Key (rawApiKey, billingProviderId, projectId, label)
Frontend->>Backend: POST /developer/keys with payload + csrf
Backend->>Prisma: Validate billingProvider exists & enabled
Prisma-->>Backend: provider
Backend->>Backend: derive keyLookupId & keyPrefix from rawApiKey
Backend->>Prisma: create DevApiKey with metadata
Prisma-->>Backend: created apiKey
Backend-->>Frontend: {apiKey, rawApiKey, project, billingProvider}
Frontend-->>Client: Display secret + copy prompt
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements end-to-end API key generation in the Developer portal through a generic Billing Provider abstraction. Users select a project, authenticate via OAuth with a billing provider (Daydream), and receive a scoped API key that's shown only once. The PR migrates the old DaydreamSettings.apiKey field to a new BillingProvider model structure.
Changes:
- Introduced OAuth-based billing provider authentication flow with in-memory session management
- Added new API routes for projects, billing providers, and updated API key creation to use provider-issued keys
- Refactored DeveloperView UI with multi-step key creation modal, project-based key grouping, and billing provider management tab
- Migrated schema from
DaydreamSettings.apiKeytoBillingProvider+DevApiProject+ updatedDevApiKeystructure with two migration scripts
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/database/prisma/schema.prisma | Added BillingProvider model, removed apiKey from DaydreamSettings, updated DevApiKey with lookupId/billingProviderId/projectId |
| packages/database/prisma/migrations/migrate-api-key-lookup-id.ts | Adds keyLookupId column and backfills existing keys with random IDs |
| packages/database/prisma/migrations/migrate-remove-billing-provider-key.ts | Backfills billingProviderId from old BillingProviderKey join table |
| apps/web-next/src/app/api/v1/auth/daydream/start/route.ts | OAuth flow start endpoint that creates login session and returns auth URL |
| apps/web-next/src/app/api/v1/auth/daydream/callback/route.ts | OAuth callback that exchanges token for API key and updates session |
| apps/web-next/src/app/api/v1/auth/daydream/result/route.ts | Polling endpoint for checking OAuth session status |
| apps/web-next/src/app/api/v1/auth/daydream/_sessions.ts | In-memory session store with TTL-based cleanup |
| apps/web-next/src/app/api/v1/developer/projects/route.ts | CRUD endpoints for user projects |
| apps/web-next/src/app/api/v1/billing-providers/route.ts | Lists enabled billing providers |
| apps/web-next/src/app/api/v1/developer/keys/route.ts | Updated to accept provider-issued keys via OAuth |
| apps/web-next/src/app/api/v1/integrations/[type]/configure/route.ts | Added Daydream integration configuration (has schema mismatch bug) |
| apps/web-next/src/app/api/v1/daydream/settings/route.ts | Removed apiKey field handling |
| plugins/developer-api/frontend/src/pages/DeveloperView.tsx | Complete UI overhaul with modal flow, project filtering, and provider management |
| apps/web-next/src/app/(dashboard)/settings/page.tsx | Redirected Daydream link/unlink to Developer page, temporarily hid integrations section |
| packages/types/src/user.ts | Deprecated daydreamLinked field |
| services/base-svc/src/services/auth.ts | Set daydreamLinked to false |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
seanhanca
requested changes
Feb 19, 2026
Contributor
seanhanca
left a comment
seanhanca
left a comment
There was a problem hiding this comment.
- Daydream API key storage removed, but integrations still write apiKey
DaydreamSettings.apiKey is removed in the schema, but integrations/[type]/configure is updated to write to it:
await prisma.daydreamSettings.upsert({ where: { userId: user.id }, update: { apiKey: credentials.apiKey }, create: { userId: user.id, apiKey: credentials.apiKey, }, });
This will fail at runtime because apiKey no longer exists. You should either:
-Remove the Daydream branch from the configure route (and rely on OAuth), or
-Keep storing Daydream keys somewhere (e.g. a new table or encrypted field) if the manual configure flow remains.
- Daydream sessions always use hardcoded default key
daydream/sessions/route.ts:
async function getApiKey(_userId: string): Promise<string> { return DEFAULT_API_KEY;}
DEFAULT_API_KEY is a fixed key used for all users. Daydream-related routes (streams, whip-proxy, models, etc.) still expect per-user keys from DaydreamSettings, which no longer exists. This causes:
-All users to share a single key
-No way for users to use their own OAuth-issued keys for sessions/streams
Either:
-Wire Daydream routes to use keys from DevApiKey (which implies storing raw keys somewhere, e.g. encrypted), or
-Clearly document that this is a dev-only fallback and that production behavior is not yet implemented.
- In-memory Daydream OAuth session store is unsuited for Vercel
_sessions.ts uses an in-memory Map with globalThis. On Vercel serverless:
- Functions are short-lived
- Multiple instances may run
- Cold starts create new heaps
As a result:
-Sessions will disappear across function invocations - OAuth callback and result polling may hit different instances and never see the session
OAuth will be unreliable in production. You need persistent storage (e.g. Redis or DB-backed sessions).
- Session cleanup may leave orphaned state: entries
In LoginSessionStore.get():
if (session && Date.now() > session.expiresAt) {
this.store.delete(key);
this.store.delete(`state:${session.state}`);
return undefined;
}
private cleanup() only does:
for (const [key, session] of this.store) {
if (now > session.expiresAt) {
this.store.delete(key);
}
}
When key is state:xxx, you delete that key but not the corresponding loginSessionId. For loginSessionId keys, you never remove the state: key. Cleanup should:
Delete both loginSessionId and state:${session.state} when expiring, or
Store and iterate over sessions in a way that lets you remove both aliases.
- Migration scripts may conflict
Two migrations both touch keyPrefix:
migrate-api-key-lookup-id.ts: sets format visible + '...' (12 chars + ...)
migrate-remove-billing-provider-key.ts: sets format visible + '****************' (7 chars + asterisks)
Running both will change the format twice and may conflict. Decide on a single canonical format and assign that to one migration. - BillingProviderKey assumption in migration
migrate-remove-billing-provider-key.ts assumes a BillingProviderKey table and joins to it. That table does not appear in the current schema. For new databases this migration may:
Reference tables/columns that never existed
Fail or behave unexpectedly
Add explicit checks and safe paths for:
Greenfield deploys (no BillingProviderKey)
Existing deploys with BillingProviderKey, including the correct migration order. - billing-providers/route.ts uses incorrect import
import { prisma } from '@/lib/db';
Next.js often uses @/lib/prisma or @naap/database. Confirm that @/lib/db is the correct and existing import in this app. - Plugin backend auth uses x-user-id header
plugins/developer-api/backend relies on req.headers['x-user-id'] for projects and keys. This header must be set by a proxy or gateway. If it’s missing, the value falls back to 'anonymous', which can:
Expose or modify other users’ keys/projects
Bypass proper auth
Verify how x-user-id is populated and that the developer-api backend is never called without it in production. - Revoke does not reload list
After revoking a key:
setApiKeys(prev => prev.map(k => k.id === revokeKeyId ? { ...k, status: 'REVOKED' } : k));
The local list is updated, but it might be safer to refetch from the server to keep status and metadata in sync.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…lback/route.ts Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web-next/src/app/api/v1/auth/providers/`[providerSlug]/callback/route.ts:
- Around line 55-105: The callback currently accepts the query userId and writes
it into session.userId without validation; instead, retrieve the session
(billingProviderLoginSessions.get(...)) and compare the incoming userId to the
session's existing userId (or prefer the session-stored userId if present), and
only set session.userId = userId if they exactly match; if they don’t match (or
if session already has a different userId), return an error htmlResponse (e.g.,
'Authentication Failed' / 'User mismatch detected') and do not complete the
session or store the access token; keep references to providerSlug, state,
session, billingProviderLoginSessions and exchangeTokenForApiKey to locate the
code to change.
---
Duplicate comments:
In
`@apps/web-next/src/app/api/v1/auth/providers/`[providerSlug]/callback/route.ts:
- Around line 25-32: The fetch to `${DAYDREAM_API_BASE}/v1/api-key` can hang;
wrap the request in an AbortController with a 5–10 second timeout, pass
controller.signal into the fetch call that constructs `response`, clear the
timeout after the fetch completes, and throw or return the abort/timeout error
so the route handler propagates the failure instead of waiting indefinitely;
update the code around the `response` fetch (and any surrounding try/catch) to
handle AbortError appropriately.
- Added support for managing billing provider OAuth sessions using Prisma, replacing the in-memory session store. - Introduced new model `BillingProviderOAuthSession` in the database schema to track session details. - Updated API routes for starting, handling callbacks, and retrieving session statuses, ensuring robust session management. - Enhanced error handling for session expiration and token exchange timeouts, improving user experience during authentication flows. - Adjusted environment configuration to support OAuth callback origins for local development.
- Modified the key lookup ID generation to use a random index ID for provider-issued keys without an embedded lookup ID, enhancing indexing and display functionality. - Updated comments to clarify the purpose of the new ID generation method.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/developer-api/backend/src/server.ts (1)
371-372:⚠️ Potential issue | 🟡 MinorIn-memory fallback returns inconsistent response shape.
The Prisma path formats keys with specific fields (
id,project,billingProvider,label,modelName,gatewayName,keyPrefix,status,createdAt,lastUsedAt), but the in-memory fallback returns rawinMemoryApiKeysobjects directly. This creates inconsistent API responses depending on whether the database is available.♻️ Proposed fix to align response shapes
const keys = inMemoryApiKeys.filter((k: any) => k.userId === userId); - res.json({ keys, total: keys.length }); + const formatted = keys.map((k: any) => ({ + id: k.id, + project: k.project, + billingProvider: k.billingProvider, + label: k.label ?? null, + modelName: 'Unknown', + gatewayName: 'Unknown', + keyPrefix: k.keyPrefix, + status: k.status, + createdAt: k.createdAt, + lastUsedAt: k.lastUsedAt || null, + })); + res.json({ keys: formatted, total: formatted.length });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/developer-api/backend/src/server.ts` around lines 371 - 372, The in-memory fallback returns raw inMemoryApiKeys objects causing a mismatched response shape; update the code that builds keys (where inMemoryApiKeys is filtered) to map each matched entry to the same shape Prisma returns (include id, project, billingProvider, label, modelName, gatewayName, keyPrefix, status, createdAt, lastUsedAt), converting dates to the same format used by the Prisma path (e.g., ISO strings) and then return res.json({ keys, total: keys.length }); so consumers always see the consistent schema.
🧹 Nitpick comments (2)
plugins/developer-api/frontend/tsconfig.json (1)
5-9: Consider retaining stricter type-checking settings.Setting
noUnusedLocals: falseandnoImplicitReturns: falserelaxes type safety and may mask dead code or missing return paths. If this is intentional for development ergonomics with cross-package includes, consider enabling these in CI or a separate strict config to catch issues before merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/developer-api/frontend/tsconfig.json` around lines 5 - 9, The current tsconfig.json relaxes type safety by setting noUnusedLocals: false and noImplicitReturns: false; change this by enabling stricter checks either directly (set noUnusedLocals: true and noImplicitReturns: true) or create a separate strict config (e.g., tsconfig.strict.json) that extends this tsconfig and flips noUnusedLocals and noImplicitReturns to true, then use that strict config in CI builds — reference the tsconfig.json settings "noUnusedLocals" and "noImplicitReturns" and the project-level config extension mechanism to implement the change.plugins/developer-api/backend/src/server.ts (1)
591-600: Minor: Inconsistent status comparison between Prisma and in-memory paths.The Prisma path uses case-sensitive comparison
k.status === 'ACTIVE'(line 591), while the in-memory fallback uses case-insensitivek.status?.toUpperCase?.() === 'ACTIVE'(line 600). Consider aligning both to use the same comparison approach for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/developer-api/backend/src/server.ts` around lines 591 - 600, The Prisma branch uses a case-sensitive status check (keys.filter((k: any) => k.status === 'ACTIVE')) while the fallback uses a case-insensitive check (inMemoryApiKeys.filter(k => k.status?.toUpperCase?.() === 'ACTIVE')), causing inconsistency; update the Prisma path (the keys -> activeKeys calculation) to normalize the status the same way as the fallback (e.g., use k.status?.toUpperCase?.() === 'ACTIVE') or change the fallback to match the Prisma comparison so both activeKeys computations (keys and inMemoryApiKeys) use the same case handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web-next/src/app/api/v1/auth/providers/`[providerSlug]/callback/route.ts:
- Around line 98-123: Before calling exchangeTokenForApiKey, check the retrieved
session.expiresAt against current time and reject if expired: after
prisma.billingProviderOAuthSession.findUnique (and the providerSlug check) add
an expiry guard that returns htmlResponse('Session Expired', 'The login session
has expired or was already used. Please try again from NaaP.', true) when
Date.now() >= new Date(session.expiresAt).getTime(); optionally update the
session via prisma.billingProviderOAuthSession.update to set status to 'expired'
before returning to ensure the session cannot be replayed, and only call
exchangeTokenForApiKey and the existing update to mark status 'complete' when
the session is still valid.
In `@apps/web-next/src/app/api/v1/developer/keys/route.ts`:
- Around line 108-125: Trim/normalize billingProviderId and rawApiKey once
immediately after extracting them from body so subsequent validation and usage
(lookups, hashing, derivation) use the canonical values; specifically, replace
the current raw uses of billingProviderId and rawApiKey by assigning normalized
variables (e.g., billingProviderId = typeof billingProviderId === 'string' ?
billingProviderId.trim() : billingProviderId) and similarly for rawApiKey before
the existing typeof/trim() checks, then use those normalized variables
everywhere else in this route handler (route.ts functions that reference
billingProviderId/rawApiKey).
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 20-30: The middleware that reads and echoes
req.header('x-request-id') in the app.use block is using the incoming value
directly, which can allow header/control character injection; update the logic
in that middleware to pass the incoming header through sanitizeForLog (or an
equivalent sanitizer) before trimming/using it, e.g. compute incomingSanitized =
sanitizeForLog(incoming) and then use incomingSanitized when setting requestId,
(req as any).requestId and res.setHeader('x-request-id', requestId); keep the
fallback UUID/randomBytes logic unchanged so untrusted or empty values still
generate a safe ID.
- Around line 479-497: The create call to prisma.devApiKey.create is not
populating the DevApiKey.keyHash field; update the endpoint that builds newKey
so it computes a secure hash of the generated raw API key (e.g., bcrypt or an
HMAC/SHA-256 with a per-app salt) and include that value in the create data as
keyHash alongside keyLookupId and keyPrefix, using the same place where
keyPrefix/label/resolvedModelId are set; ensure the raw key is only returned
once to the caller and never persisted, and keep the prisma.devApiKey.create
include block unchanged.
In `@plugins/developer-api/frontend/src/pages/DeveloperView.tsx`:
- Around line 563-567: The revoke button visibility uses a case-sensitive check
on key.status and can miss lowercase 'revoked'; normalize status first (e.g.,
const normalizedStatus = key.status?.toUpperCase()) and use normalizedStatus !==
'REVOKED' when deciding to show the revoke action (also update the Badge active
check to use normalizedStatus === 'ACTIVE' to keep behavior consistent). Locate
usages of key.status in the DeveloperView component (Badge and setRevokeKeyId
button) and replace the direct checks with the normalizedStatus comparisons.
- Around line 250-395: In handleCreateKey, check the return value of window.open
(store it in a variable like popupWindow) immediately after calling it; if it is
null (popup blocked), abort the poll (call
pollAbortControllerRef.current?.abort()), set an immediate error via
setCreateError('Popup blocked. Please allow popups and try again.'), reset UI
state with setCreateStep('form') and setCreating(false), and return before
entering the polling loop so you don't wait the full timeout; also ensure any
created AbortController is cleaned up (pollAbortControllerRef.current = null)
when bailing out.
---
Outside diff comments:
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 371-372: The in-memory fallback returns raw inMemoryApiKeys
objects causing a mismatched response shape; update the code that builds keys
(where inMemoryApiKeys is filtered) to map each matched entry to the same shape
Prisma returns (include id, project, billingProvider, label, modelName,
gatewayName, keyPrefix, status, createdAt, lastUsedAt), converting dates to the
same format used by the Prisma path (e.g., ISO strings) and then return
res.json({ keys, total: keys.length }); so consumers always see the consistent
schema.
---
Duplicate comments:
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 614-637: The error handler currently returns unsanitized
requestId, method, and path in the JSON response; update the middleware (the
app.use(...) error-handling function) to sanitize those values (use the existing
sanitizeForLog or a response-safe sanitizer) before placing them into the
response object so response fields requestId, method, and path are
escaped/normalized; ensure you reuse sanitizeForLog(requestId),
sanitizeForLog(method), and sanitizeForLog(path) (or a dedicated
sanitizeForResponse helper) when building the JSON payload to prevent reflection
of raw user input.
---
Nitpick comments:
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 591-600: The Prisma branch uses a case-sensitive status check
(keys.filter((k: any) => k.status === 'ACTIVE')) while the fallback uses a
case-insensitive check (inMemoryApiKeys.filter(k => k.status?.toUpperCase?.()
=== 'ACTIVE')), causing inconsistency; update the Prisma path (the keys ->
activeKeys calculation) to normalize the status the same way as the fallback
(e.g., use k.status?.toUpperCase?.() === 'ACTIVE') or change the fallback to
match the Prisma comparison so both activeKeys computations (keys and
inMemoryApiKeys) use the same case handling.
In `@plugins/developer-api/frontend/tsconfig.json`:
- Around line 5-9: The current tsconfig.json relaxes type safety by setting
noUnusedLocals: false and noImplicitReturns: false; change this by enabling
stricter checks either directly (set noUnusedLocals: true and noImplicitReturns:
true) or create a separate strict config (e.g., tsconfig.strict.json) that
extends this tsconfig and flips noUnusedLocals and noImplicitReturns to true,
then use that strict config in CI builds — reference the tsconfig.json settings
"noUnusedLocals" and "noImplicitReturns" and the project-level config extension
mechanism to implement the change.
- Implemented logic to update the status of expired billing provider OAuth sessions in the database. - Added a response for users when their login session has expired, improving user experience during authentication flows.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/developer-api/backend/src/server.ts (1)
373-374:⚠️ Potential issue | 🟡 MinorIn-memory keys response lacks consistent formatting.
The Prisma path (lines 358-369) formats keys with specific fields (
modelName,gatewayName,status, etc.), but the in-memory fallback returns raw key objects without the same transformation. This creates inconsistent API responses between modes.♻️ Proposed fix to align in-memory response
const keys = inMemoryApiKeys.filter((k: any) => k.userId === userId); - res.json({ keys, total: keys.length }); + const formatted = keys.map((k: any) => ({ + id: k.id, + project: k.project, + billingProvider: k.billingProvider, + label: k.label ?? null, + modelName: k.modelName || 'Unknown', + gatewayName: k.gatewayName || 'Unknown', + keyPrefix: k.keyPrefix, + status: k.status, + createdAt: k.createdAt, + lastUsedAt: k.lastUsedAt || null, + })); + res.json({ keys: formatted, total: formatted.length });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/developer-api/backend/src/server.ts` around lines 373 - 374, The in-memory fallback returns raw objects (inMemoryApiKeys) causing inconsistent responses vs the Prisma-mapped keys; update the in-memory branch where keys is built (the const keys = inMemoryApiKeys.filter(...) line) to map each key into the same transformed shape used earlier (include fields like id, userId, modelName, gatewayName, status, model, maskedKey, createdAt, lastUsedAt, etc.), matching the mapping logic used in the Prisma path so the JSON response (res.json({ keys, total: keys.length })) has identical structure in both modes.
🧹 Nitpick comments (2)
plugins/developer-api/backend/src/server.ts (1)
593-602: Minor inconsistency in status comparison.The Prisma path (line 593) uses exact match
k.status === 'ACTIVE', while the in-memory fallback (line 602) uses case-insensitive comparisonk.status?.toUpperCase?.() === 'ACTIVE'. This is unlikely to cause issues since the code consistently uses uppercase'ACTIVE'and'REVOKED', but note the difference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/developer-api/backend/src/server.ts` around lines 593 - 602, The two active key counts are using different status comparisons: Prisma block uses strict equality (k.status === 'ACTIVE') while the fallback uses case-insensitive toUpperCase (inMemoryApiKeys.filter(k => k.status?.toUpperCase?.() === 'ACTIVE')). Make them consistent by normalizing the status in both places (e.g., use String(k.status).toUpperCase() === 'ACTIVE') or switch the fallback to strict equality; update the activeKeys expressions where keys and inMemoryApiKeys are computed in server.ts so both branches use the same comparison.apps/web-next/src/app/api/v1/developer/keys/route.ts (1)
149-159:gatewayIdis silently ignored whenmodelIdis not provided.If a client provides
gatewayIdwithoutmodelId, the gateway resolution is silently skipped due to theresolvedModelId &&check on line 150. This could lead to user confusion if they expect the gateway to be associated with the key.Consider either:
- Returning an error if
gatewayIdis provided withoutmodelId, or- Documenting this behavior clearly in the API contract.
Option 1: Error when gatewayId provided without modelId
+ if (gatewayId && typeof gatewayId === 'string' && gatewayId.trim() !== '' && !resolvedModelId) { + return errors.badRequest('gatewayId requires a valid modelId'); + } + let resolvedGatewayOfferId: string | undefined; if (resolvedModelId && gatewayId && typeof gatewayId === 'string' && gatewayId.trim() !== '') {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/app/api/v1/developer/keys/route.ts` around lines 149 - 159, The handler silently ignores gatewayId when resolvedModelId is not set; update the validation in route.ts so that if gatewayId is provided but resolvedModelId is falsy you return an explicit error (e.g., use errors.badRequest) rather than skipping resolution. Concretely, before the block that calls prisma.devApiGatewayOffer.findFirst check if gatewayId exists and resolvedModelId is missing and return an error message like "modelId required when gatewayId is provided"; keep the existing lookup using prisma.devApiGatewayOffer.findFirst and resolvedGatewayOfferId when both are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web-next/src/app/api/v1/developer/keys/route.ts`:
- Around line 180-198: The devApiKey.create call is missing the keyHash value
required by the schema; compute and include keyHash using the existing
hashApiKey utility when creating the API key. In the data object passed to
prisma.devApiKey.create (where userId, projectId, keyPrefix, etc. are set), add
keyHash: hashApiKey(rawApiKey) so the stored record includes the hashed secret
(consistent with the workflows service pattern) and preserves the unique indexed
keyHash field used for authentication.
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 629-638: Response body currently includes raw requestId, method,
and path; call the existing sanitizeForLog for each value before constructing
the JSON error response in the error handler that builds the
res.status(500).json(...) object so control characters are removed. Locate the
response assembly around the res.status(500).json({ success: false, error: {
code: 'INTERNAL_SERVER_ERROR', message: 'Internal server error', requestId,
method, path } }) block in server.ts and replace those raw references with
sanitized variables (e.g., sanitizedRequestId = sanitizeForLog(requestId),
sanitizedMethod = sanitizeForLog(method), sanitizedPath = sanitizeForLog(path))
and return those sanitized values in the error payload.
---
Outside diff comments:
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 373-374: The in-memory fallback returns raw objects
(inMemoryApiKeys) causing inconsistent responses vs the Prisma-mapped keys;
update the in-memory branch where keys is built (the const keys =
inMemoryApiKeys.filter(...) line) to map each key into the same transformed
shape used earlier (include fields like id, userId, modelName, gatewayName,
status, model, maskedKey, createdAt, lastUsedAt, etc.), matching the mapping
logic used in the Prisma path so the JSON response (res.json({ keys, total:
keys.length })) has identical structure in both modes.
---
Duplicate comments:
In `@apps/web-next/src/app/api/v1/developer/keys/route.ts`:
- Around line 110-127: Extract and normalize billingProviderId and rawApiKey
immediately when reading from body so downstream lookups and key derivation use
trimmed values: change the extraction of billingProviderId and rawApiKey (the
variables currently set in route.ts) to apply .trim() and coerce empty results
to undefined (e.g., normalize to trimmed strings or undefined) before the
validation checks, then keep the existing typeof/empty validations and use these
normalized variables in subsequent lookup (where billingProviderId is used) and
key derivation logic (where rawApiKey is used).
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 481-499: The DevApiKey creation currently calls
prisma.devApiKey.create (producing newKey and returning rawApiKey to the client)
but never computes or persists keyHash, so internal authentication cannot verify
keys; update the flow in the function around prisma.devApiKey.create to compute
a secure hash (e.g., bcrypt/argon2) of rawApiKey into a variable (keyHash) and
include keyHash in the data passed to prisma.devApiKey.create, ensure the
unhashed rawApiKey is only returned once and the hash is not exposed, and if the
system truly relies solely on external validation instead, either document that
clearly or remove the keyHash column from the DevApiKey schema instead of
leaving it unused.
- Around line 20-30: The middleware that reads req.header('x-request-id') and
assigns it to (req as any).requestId and res.setHeader('x-request-id', ...) must
sanitize the incoming value before use; call the existing sanitizeForLog on the
incoming header (or a safe fallback generated id) and use that sanitizedResult
when assigning (req as any).requestId and when calling res.setHeader; ensure you
still fallback to crypto.randomUUID()/crypto.randomBytes if the sanitized header
is empty.
---
Nitpick comments:
In `@apps/web-next/src/app/api/v1/developer/keys/route.ts`:
- Around line 149-159: The handler silently ignores gatewayId when
resolvedModelId is not set; update the validation in route.ts so that if
gatewayId is provided but resolvedModelId is falsy you return an explicit error
(e.g., use errors.badRequest) rather than skipping resolution. Concretely,
before the block that calls prisma.devApiGatewayOffer.findFirst check if
gatewayId exists and resolvedModelId is missing and return an error message like
"modelId required when gatewayId is provided"; keep the existing lookup using
prisma.devApiGatewayOffer.findFirst and resolvedGatewayOfferId when both are
present.
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 593-602: The two active key counts are using different status
comparisons: Prisma block uses strict equality (k.status === 'ACTIVE') while the
fallback uses case-insensitive toUpperCase (inMemoryApiKeys.filter(k =>
k.status?.toUpperCase?.() === 'ACTIVE')). Make them consistent by normalizing
the status in both places (e.g., use String(k.status).toUpperCase() ===
'ACTIVE') or switch the fallback to strict equality; update the activeKeys
expressions where keys and inMemoryApiKeys are computed in server.ts so both
branches use the same comparison.
…erView - Modified the condition for displaying the revoke button in DeveloperView to ensure it correctly checks for the 'REVOKED' status in a case-insensitive manner. - This change improves the accuracy of the UI by preventing the revoke button from appearing for revoked keys.
…andling - Updated the error response in the server to sanitize sensitive information such as requestId, method, and path before logging. - This change enhances security by preventing exposure of potentially sensitive data in error logs.
P0 fixes: - Remove dead _sessions.ts (in-memory store never imported by routes) - Encrypt OAuth access tokens at rest using AES-256-GCM derived from NEXTAUTH_SECRET; decrypt on redemption in result endpoint - Restore keyHash storage for provider-issued keys using scrypt KDF - Document default Daydream API key removal as breaking change with improved error message guiding users to configure via settings or billing provider OAuth flow P1 fixes: - Extract duplicated key utilities (parseApiKey, deriveKeyLookupId, getKeyPrefix, hashApiKey) to shared @naap/database package - Add token-encryption module to @naap/database for reuse - Revert tsconfig.json hack in developer-api backend (was compiling package sources directly instead of consuming as dependency) - Add opportunistic cleanup of expired BillingProviderOAuthSession records (runs at most every 5 minutes during result polling) - Add per-IP rate limiting (5 req/min) on billing-provider /start endpoint to prevent session flooding - Fix inaccurate return types on start/result route handlers (Promise<ReturnType<typeof success>> → Promise<NextResponse>) - Remove unnecessary (crypto as any).randomUUID cast (Node 20+) Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Code Review Fixes Applied (P0 + P1)P0 — Critical
P1 — Medium
New shared modules in
|
seanhanca
approved these changes
Feb 20, 2026
Contributor
seanhanca
left a comment
seanhanca
left a comment
There was a problem hiding this comment.
applied a couple of critical changes in the area of improving securities, and remove some dead code.
This was referenced Feb 23, 2026
5 tasks
3 tasks
3 tasks
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.
Summary
start,callback,result) with short-lived session correlation to securely bridge provider auth back into NaaP.What’s new
UI
API Server / Routes
POST /api/v1/auth/providers/:providerSlug/startGET /api/v1/auth/providers/:providerSlug/callbackGET /api/v1/auth/providers/:providerSlug/resultBillingProviderOAuthSessionand include:redeemedAt)Cache-Control: no-storeon polling responses (including token return)GET /api/v1/developer/projectsreturns_count.apiKeysPOST /api/v1/developer/projectsreturns_count.apiKeysfor consistencyGET /api/v1/developer/projects+POST /api/v1/developer/projects(with in-memory fallback)GET /api/v1/developer/billing-providers(with in-memory fallback)project,billingProvider,label,keyPrefixData / Infrastructure
BillingProviderOAuthSessionDevApiProjectnow relates toDevApiKeyviaapiKeysDevApiKeyupdated to support nullable/relational fields (projectId,billingProviderId, optionalmodelId,keyLookupId,label, optionalkeyHash, legacyprojectName)@naap/database:resolveDevApiProjectId+DevApiProjectResolutionErrorAuthorizationheaders with safe fallback behavior.createCsrfMiddleware(...): RequestHandler).Env / Config
BILLING_PROVIDER_OAUTH_CALLBACK_ORIGINDAYDREAM_AUTH_URL(defaults to Daydream sign-in URL)DAYDREAM_API_BASE(defaults tohttps://api.daydream.live)Test plan
pending→complete/expired/redeemed).Notes / Risks
apps/web-next/src/app/api/v1/auth/providers/_sessions.ts) is no longer used after the Prisma-backed session implementation, consider removing it to reduce confusion.