fix(api): bake org context into API keys via tRPC wrapper#1378
fix(api): bake org context into API keys via tRPC wrapper#1378saddlepaddle merged 1 commit intomainfrom
Conversation
API key creation now goes through a tRPC procedure that injects the user's activeOrganizationId into key metadata server-side. The MCP endpoint reads org from metadata instead of falling back to a members query. Also stops leaking the raw API key secret in the token field.
📝 WalkthroughWalkthroughThe changes refactor API-key authentication and management by introducing a new tRPC-based router for API key creation, removing direct database access in the agent authentication route, and sourcing organizationId from API key metadata instead of database lookups. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx (1)
84-88:⚠️ Potential issue | 🟡 MinorMissing user-facing error feedback on key creation failure.
The catch block logs to console but never shows a toast or other UI feedback. Users will see the "Generating..." button reset with no indication of what went wrong. The component already imports
toastfrom@superset/ui/sonner.Proposed fix
} catch (error) { console.error("[api-keys] Failed to generate API key:", error); + toast.error("Failed to generate API key. Please try again."); } finally {
🧹 Nitpick comments (2)
packages/trpc/src/router/api-key/api-key.ts (1)
6-27: WrapcreateApiKeycall with error handling and logging.If
ctx.auth.api.createApiKeythrows (network error, auth service down, invalid body, etc.), the raw error bubbles up without context. Per coding guidelines, errors should be logged with context before rethrowing. Also,result.keyis not null-checked before being returned.Proposed fix
- const result = await ctx.auth.api.createApiKey({ - headers: ctx.headers, - body: { - name: input.name, - metadata: { organizationId }, - }, - }); - - return { key: result.key }; + let result: { key?: string | null }; + try { + result = await ctx.auth.api.createApiKey({ + headers: ctx.headers, + body: { + name: input.name, + metadata: { organizationId }, + }, + }); + } catch (error) { + console.error("[apiKey/create] Failed to create API key:", error); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Failed to create API key", + }); + } + + if (!result.key) { + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "API key creation returned no key", + }); + } + + return { key: result.key };As per coding guidelines, "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly" and "Validate external API data as untrusted by handling missing fields, unknown enums, and unexpected shapes with tolerant parsing and explicit fallbacks".
apps/api/src/app/api/agent/[transport]/route.ts (1)
44-48:JSON.parsecan throw on malformed metadata strings.If
result.key.metadatais a non-JSON string,JSON.parsethrows and the outer catch logs a misleading "API key verification failed" message. Consider adding a targeted try/catch around the parse, or validating the shape more defensively.Also, the
as string | undefinedcast on Line 48 skips runtime type checking — iforganizationIdis unexpectedly a non-string truthy value, it'll pass through.Proposed fix
- const metadata = - typeof result.key.metadata === "string" - ? JSON.parse(result.key.metadata) - : result.key.metadata; - const organizationId = metadata?.organizationId as string | undefined; + let metadata: Record<string, unknown> | undefined; + try { + metadata = + typeof result.key.metadata === "string" + ? JSON.parse(result.key.metadata) + : (result.key.metadata as Record<string, unknown> | undefined); + } catch { + console.error("[mcp/auth] API key metadata is not valid JSON"); + return undefined; + } + const organizationId = + typeof metadata?.organizationId === "string" + ? metadata.organizationId + : undefined;As per coding guidelines, "Validate external API data as untrusted by handling missing fields, unknown enums, and unexpected shapes with tolerant parsing and explicit fallbacks".
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
apiKey.createtRPC procedure that injectsactiveOrganizationIdfrom the session into the API key metadata server-side — no client-side org passing neededtokenfield ("api-key"sentinel instead)!userIdnull guard on API key verificationChanges
packages/trpc/src/router/api-key/— newapiKey.createmutation wrappingauth.api.createApiKeywith org metadata injectionpackages/trpc/src/root.ts— registerapiKeyRouterapps/api/.../route.ts— read org from metadata, null guards, no secret leakapps/desktop/.../ApiKeysSettings.tsx— useapiTrpcClient.apiKey.create.mutate()instead ofauthClient.apiKey.create()Test plan
bun run lint:fix— cleanbun run typecheck— 18/18 passbun test— 1221 pass, 0 fail.mcp.json, confirm MCP tools respond with correct org contextSummary by CodeRabbit
Release Notes