feat(webapp): enforce scopes for environment API keys - #4389
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 |
|
| Name | Type |
|---|---|
| @trigger.dev/core | Patch |
| @trigger.dev/sdk | Patch |
| @trigger.dev/build | Patch |
| trigger.dev | Patch |
| @trigger.dev/python | Patch |
| @trigger.dev/redis-worker | Patch |
| @trigger.dev/schema-to-json | Patch |
| @internal/cache | Patch |
| @internal/clickhouse | Patch |
| @internal/llm-model-catalog | Patch |
| @trigger.dev/rbac | Patch |
| @internal/redis | Patch |
| @internal/replication | Patch |
| @internal/run-engine | Patch |
| @internal/run-store | Patch |
| @internal/schedule-engine | Patch |
| @trigger.dev/sso | Patch |
| @internal/testcontainers | Patch |
| @internal/tracing | Patch |
| @internal/tsql | Patch |
| @internal/dashboard-agent | Patch |
| @internal/sdk-compat-tests | Patch |
| @trigger.dev/react-hooks | Patch |
| @trigger.dev/rsc | Patch |
| @trigger.dev/database | Patch |
| @trigger.dev/otlp-importer | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
0e70a0b to
d20a5af
Compare
37de624 to
6b31535
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: |
6b31535 to
a8e673e
Compare
36f03ba to
51e1189
Compare
a8e673e to
24771ce
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.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| triggerAction: "trigger", | ||
| }); | ||
|
|
||
| const $responseHeaders = await responseHeaders( | ||
| batch, | ||
| authentication.environment, | ||
| triggerClient | ||
| ); | ||
| const $responseHeaders = await publicAccessTokenResponseHeaders({ | ||
| environment: authentication.environment, | ||
| scopes: [`read:batch:${batch.id}`], | ||
| expirationTime: "1h", | ||
| }); |
There was a problem hiding this comment.
🔍 Missing parent-run authorization on v1 batch trigger route
apps/webapp/app/routes/api.v1.tasks.batch.ts accepts body.parentRunId (via BatchTriggerTaskV2RequestBody) and passes body straight to BatchTriggerV3Service.call at apps/webapp/app/routes/api.v1.tasks.batch.ts:111, but the handler (api.v1.tasks.batch.ts:43) never destructures ability nor calls canWriteParentRun(...). Every other trigger/batch route touched in this PR added that check (api.v1.tasks.$taskId.trigger.ts, api.v1.tasks.$taskId.batch.ts, api.v2.tasks.batch.ts, api.v3.batches.ts). Because this route sets allowJWT: true and declares everyResource task authorization, a restricted task-scoped credential passes the route gate for its own task yet can supply an arbitrary parentRunId it does not own to link/resume. This is an authorization gap consistent with the ones the PR fixes elsewhere; reported via the security scan.
(Refers to lines 111-129)
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Environment API keys backed by the additional-key table can authenticate API requests using their stored effective scopes. Revoked and expired keys are rejected, branch environments retain their existing routing behavior, and last-used timestamps are updated on a throttled best-effort basis.
Design
API route builders receive the resolved ability and reject restricted keys on routes without an authorization declaration. Existing deployment, environment variable, queue, run, task, batch, session, and waitpoint routes declare the resources they access.
Trigger and batch responses return server-signed public access tokens, so additional keys never need access to the environment signing secret. Root-key rotation also keeps public tokens valid for the existing grace window.
Deployment notes
The schema migration must be present before this code is deployed. Because bearer resolution runs on every authenticated request, deploy the resolver with additional-key lookup disabled, verify root-key and public-token parity, then enable lookup before any additional keys can be issued.
The multi-task authorization tightening changes the result for narrowly scoped tokens that request tasks outside their grants. Observe would-deny results before enforcing that check. Request-idempotency keys are also newly isolated by environment and task, so a retry crossing the deployment boundary may execute once more before old cache entries expire.
Follow-ups
Add would-deny observability and an independent enforcement switch for multi-task authorization.Add an independent switch for server-issued batch tokens while root-key parity is verified.