feat(webapp): add multiple environment API key management - #4390
feat(webapp): add multiple environment API key management#4390carderne wants to merge 16 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
6aba247 to
2579113
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
2579113 to
dbca394
Compare
A number passed to `expirationTime` is a Unix timestamp in seconds, not milliseconds as the JSDoc claimed. Following the old docs produced a token that effectively never expired. Also fail loudly when an additional API key reaches a local self-signing fallback. Those keys are not the environment's JWT signing material, so the token would never verify. Every endpoint that returns a public access token sets `x-trigger-jwt`, so this is unreachable today.
The API-key policy methods are capability extensions, so declare them optional on RoleBaseAccessController and normalize the surface in LazyController rather than requiring every implementation to carry them. An authorization extension is compiled against whichever core commit its base image ships, so a required additive method forces both sides to move together and turns rolling an extension back into a build failure instead of a graceful degradation. Absence fails closed: no preset catalogue, and prepareApiKeyPolicy refuses outright rather than defaulting to full access, so an extension below this contract cannot mint a credential. Keys already issued are unaffected — they authorize from the scopes persisted on their row. loader.create() now returns HostRbacController, the total surface, so host callers neither guard nor invent their own absent-extension default.
dbca394 to
646ba1e
Compare
646ba1e to
d60d5a5
Compare
Add a global, org-locked kill switch (additionalApiKeyLookupEnabled, default off) gating the additional environment API-key lookup, and bounded OTel auth telemetry (api_auth.attempts counter, api_auth.duration_ms histogram, api_auth.rollout_mode gauge) to watch the rollout. Attributes are closed enums only (resolver, credential_kind, result, lookup_path); no credentials, hashes, or tenant identifiers are recorded.
Revert authenticateApiKeyWithScope to rbac.authenticateAuthorizeBearer (the kill switch still applies via the rbac controller), and make the auth telemetry tolerate a missing resolution from test doubles. Also register the v3/apiKeys subpath in core's typesVersions for node10.
The API key policy methods are optional on the plugin-facing controller contract, so `Pick` over it yields optional members that these call sites would have to guard. Both already receive the LazyController singleton, which has substituted its fail-closed defaults, so point them at HostRbacController and keep the call sites guard-free.
Require both the global issuance switch and organization rollout flag before creating additional keys, while leaving existing credentials available for use and revocation. Show nullable creators and identify SDK v4.5.8 as the first compatible public-token version.
d60d5a5 to
ba81e5e
Compare
Record bounded outcomes for additional key creation, policy preparation, revocation, and public-token minting.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| prismaClient.organization.findUnique({ | ||
| where: { id: organizationId }, | ||
| select: { featureFlags: true }, | ||
| }), |
There was a problem hiding this comment.
🟡 API key issuance check uses a Prisma lookup the repo forbids
The organization lookup that decides whether additional API keys can be issued uses prismaClient.organization.findUnique (apps/webapp/app/services/additionalApiKeyIssuance.server.ts:13), which the repository's mandatory webapp rule prohibits in favor of findFirst.
Impact: Violates the documented Prisma query convention meant to avoid the findUnique DataLoader batching bugs.
Rule reference
apps/webapp/CLAUDE.md states under "Prisma Query Patterns": "Always use findFirst instead of findUnique." This is a newly introduced call in apps/webapp/app/services/additionalApiKeyIssuance.server.ts:12-16. The established alternative used elsewhere in the webapp is findFirst.
| prismaClient.organization.findUnique({ | |
| where: { id: organizationId }, | |
| select: { featureFlags: true }, | |
| }), | |
| prismaClient.organization.findFirst({ | |
| where: { id: organizationId }, | |
| select: { featureFlags: true }, | |
| }), |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const keyEnvironmentId = environment.parentEnvironmentId ?? environment.id; | ||
|
|
||
| const [keyEnvironment, vercelIntegration] = await Promise.all([ | ||
| this.#prismaClient.runtimeEnvironment.findUniqueOrThrow({ |
There was a problem hiding this comment.
🟡 API keys presenter uses a Prisma lookup the repo forbids
The presenter loads the key environment with runtimeEnvironment.findUniqueOrThrow (apps/webapp/app/presenters/v3/ApiKeysPresenter.server.ts:72), whereas the repository's mandatory webapp rule requires avoiding the findUnique family; the established equivalent used across presenters is findFirstOrThrow.
Impact: Violates the documented Prisma query convention intended to avoid the findUnique DataLoader batching bugs.
Rule reference
apps/webapp/CLAUDE.md "Prisma Query Patterns": "Always use findFirst instead of findUnique." Many presenters in apps/webapp/app/presenters/v3/ already use findFirstOrThrow for the throwing variant, so the fix is a drop-in replacement.
| this.#prismaClient.runtimeEnvironment.findUniqueOrThrow({ | |
| this.#prismaClient.runtimeEnvironment.findFirstOrThrow({ |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (taskFilter.length === 0) { | ||
| return anyResource([{ type: "sessions" as const }, { type: "tasks" as const }]); | ||
| } |
There was a problem hiding this comment.
🟡 Public tokens scoped only to tasks can now list every session in an environment
An unfiltered session list is now authorized for callers holding only a task-level permission (anyResource([...{ type: "tasks" }]) at apps/webapp/app/routes/api.v1.sessions.ts:58), which previously was refused, so a token intended to see one thing can read all sessions.
Impact: Holders of task-scoped public tokens gain visibility of all sessions in the environment, wider access than they were granted before.
Mechanism: the no-filter branch adds a type-level tasks alternative that the old resource list did not contain
Before this PR the resource for an unfiltered list was anyResource([{ type: "sessions" }]) only, so a JWT carrying read:tasks was rejected with 403 — this was asserted explicitly by the test at apps/webapp/test/auth-api.e2e.full.test.ts ("read:tasks (type-only) on no-filter list: 403 (filter is sessions, not tasks)"), which this PR rewrites to expect success.
The new code returns anyResource([{ type: "sessions" }, { type: "tasks" }]) when filter[taskIdentifier] is absent. checkAuth → ability.can("read", [...]) succeeds if any element matches, and buildJwtAbility matches a bare read:tasks scope against { type: "tasks" } (packages/plugins/src/rbac.ts:225-230), so the request is authorized.
The inline comment claims this "preserve[s] the legacy behavior", but the legacy behavior for the sessions route was denial — the runs route (apps/webapp/app/routes/api.v1.runs.ts:30) is the one where a type-level { type: "tasks" } element genuinely existed before. Adding the alternative here is a net widening of what a task-scoped token can read.
| if (taskFilter.length === 0) { | |
| return anyResource([{ type: "sessions" as const }, { type: "tasks" as const }]); | |
| } | |
| if (taskFilter.length === 0) { | |
| return anyResource([{ type: "sessions" as const }]); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const ApiKeySearchParams = z.object({ | ||
| showRevoked: z.preprocess((value) => value === "true" || value === true, z.boolean()).optional(), | ||
| }); | ||
|
|
There was a problem hiding this comment.
🔍 Server-only RBAC module imported into a dashboard route module scope
This dashboard route (which ships a React component) imports FULL_ACCESS_PRESET_ID from @trigger.dev/rbac at module scope and uses it inside validateCreateApiKeyPreset, a plain module-level function called only from the action. It is the only route under apps/webapp/app/routes with a UI component that imports @trigger.dev/rbac (the other importers are API-only routes). Remix strips the action export from the browser build and then relies on esbuild tree-shaking to drop validateCreateApiKeyPreset and the import. internal-packages/rbac/src/index.ts has a top-level new RoleBaseAccess() and transitively pulls in node:crypto and @trigger.dev/database via fallback.ts/bearerCredentials.ts, so if the import survives tree-shaking it will end up in the client bundle. Worth confirming with an actual client build that nothing server-only is bundled; moving validateCreateApiKeyPreset (and the constant import) into a .server.ts module would remove the risk entirely.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (taskFilter.length === 0) { | ||
| return anyResource([{ type: "runs" }, { type: "tasks" }]); | ||
| } | ||
|
|
||
| return everyResource( | ||
| taskFilter.map((id) => ({ type: "tasks", id })), | ||
| [{ type: "runs" }, { type: "tasks" }] | ||
| ); |
There was a problem hiding this comment.
🔍 Multi-task run listing is now AND-scoped, tightening existing public tokens
For a non-empty filter[taskIdentifier], authorization changes from "any listed task matches" to "every listed task must match" (with type-level runs/tasks still an alternative). The e2e test that previously asserted filter=task_a,task_b + read:tasks:task_a passes is rewritten to expect 403. This is a deliberate tightening, but it is a breaking change for any already-issued public token that lists several tasks while being scoped to only one — those requests start returning 403 immediately on deploy. Consider whether this needs a rollout gate or a release note, since public tokens minted by customer code can be long-lived.
Was this helpful? React with 👍 or 👎 to provide feedback.
| try { | ||
| const snapshot = JSON.parse(entry.payload) as { taskIdentifier?: unknown }; | ||
| if (typeof snapshot.taskIdentifier === "string") { | ||
| return { | ||
| source: "buffer", | ||
| friendlyId: input.runParam, | ||
| taskIdentifier: snapshot.taskIdentifier, | ||
| }; | ||
| } | ||
| } catch { | ||
| // A malformed snapshot is not an authorizable run resource. | ||
| } |
There was a problem hiding this comment.
🔍 Buffered runs whose snapshot lacks a task identifier now resolve as not-found for cancel/replay
resolveRunForMutation previously returned { source: "buffer" } for any buffer entry matching env+org. It now additionally requires the parsed snapshot to expose a string taskIdentifier, otherwise it falls through to the writer probe and can return null — which the route builder converts to a hard 404 before the handler runs. mollifyTrigger writes the snapshot with serialiseSnapshot (plain JSON.stringify) and readFallback.server.ts:198 reads snapshot.taskIdentifier the same way, so the field should normally be present; but any older/partial buffered entry without it would now be uncancellable and unreplayable rather than falling back to the previous behavior. A taskIdentifier-absent path that still resolves the run (with a resource the ability cannot match) would be a safer degradation.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const waitpointId = WaitpointId.toFriendlyId(result.waitpoint.id); | ||
| const $responseHeaders = await publicAccessTokenResponseHeaders({ | ||
| environment: authentication.environment, | ||
| scopes: [`write:waitpoints:${waitpointId}`], | ||
| expirationTime: "24h", | ||
| }); |
There was a problem hiding this comment.
🔍 Waitpoint/batch endpoints now always mint a signed public token, not just for browser clients
api.v1.waitpoints.tokens.ts, api.v1.tasks.$taskId.batch.ts, api.v1.tasks.batch.ts and api.v2.tasks.batch.ts previously returned only the x-trigger-jwt-claims header for non-browser callers and minted a JWT solely when x-trigger-client: browser. All of them now sign and return a token on every response (24h for waitpoints, 1h for batches). This adds an HMAC sign per request on hot paths and hands a bearer credential to server-side callers that did not ask for one — including restricted additional keys, whose minted token is signed with the environment root key and remains valid after the key is revoked (documented in the PR description). Confirm the scope/expiry choices, in particular the 24h write:waitpoints:<id> token.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Projects can create, inspect, expire, and revoke multiple API keys for each environment. Plaintext values are shown only at creation; stored credentials are hashed and the API keys page displays only an obfuscated suffix afterward.
Self-hosted installations support full-access additional keys by default. Authorization extensions can provide additional access presets and optional task selection. Additional keys can also mint scoped public access tokens through the Trigger.dev API without receiving the environment signing key.
Feature notes
_ak_keys.Deployment notes
Deploy the management UI and public-token endpoint with new key creation disabled. Enable creation for selected organizations after the authentication path and released SDK have been verified, then expand availability gradually.
Revoking an API key prevents new bearer requests and new token minting. Public tokens already minted by that key remain valid until their own expiration because they are signed by the environment signing key.
TODO
partially accept items before a validation or authorization error.
Follow-ups
_ak_key.