fix: security release 2026-07-08#4316
Merged
Merged
Conversation
* fix(supervisor): authenticate compute snapshot callbacks * fix(supervisor): harden compute snapshot callback token derivation Derive a domain-separated key for callback HMACs instead of using the worker secret directly, reject an empty callback secret at construction (covering the file-based secret path), tighten the worker token env to be non-empty, and document the token's binding scope and the callback channel assumptions it relies on.
* feat(core,supervisor,webapp): authenticate the runner boundary with a deployment-scoped token Supervisor mints a signed, deployment-scoped token at pod creation and injects it as the TRIGGER_DEPLOYMENT_ID value; old runners forward it unchanged. The supervisor verifies it on the workload API (HTTP routes + WS handshake), adds a runConnected ownership guard, and forwards the verified background worker id upstream. The platform checks the run belongs to that worker on start/complete/continue/snapshots-since/snapshots-latest, gated by a created-at cutoff (disabled/log/enforce). Also adds mint/verify helpers to core and an OTLP worker_id unwrap so the metric stays a short, stable id. refs TRI-11844 * refactor(core,supervisor,webapp): scope worker actions by environment Fold the tenant check into the snapshot read each worker action already performs (scope by the token's environment id) instead of a separate per-run version lookup, so enforcement adds no extra database reads on the hot path. The supervisor forwards the verified environment id only when enforcing; the platform scopes the snapshot read by it when present. The no-token created-at suppression is now a separate, default-off gate that only reads a run row when enabled and no header is present, so feature-off is identical to prior behaviour. Drop the now-unused background_worker_id claim from the token. Refs TRI-11844 * refactor(core,supervisor): mint the deployment token deterministically per deployment Drop the wall-clock issued-at and use a caller-supplied absolute expiry when minting, so the same deployment mints byte-identical tokens on every pod and every supervisor node. The runner stamps TRIGGER_DEPLOYMENT_ID verbatim as the OpenTelemetry worker.id, so a per-pod-unique token would explode that attribute's cardinality (and ride to any external exporter); a per-deployment token keeps it at one value per deployment, matching the prior friendlyId. The supervisor drives the absolute expiry via WORKLOAD_TOKEN_EXP. Refs TRI-11844 * fix(webapp): decode deployment-token worker.id in span and log telemetry ingest The worker.id a runner stamps into span/log metadata can be a deployment token; only the metrics ingest path decoded it back to the friendlyId, so spans and logs stored the raw token in customer-facing metadata. Unwrap it on the span/log path too, at the storage boundary, so the credential never lands in ClickHouse and the value stays a stable deployment id. Non-token values (bare friendlyIds, dev ids) take a zero-cost fast path. Refs TRI-11851 * chore: apply oxfmt formatting * refactor(supervisor): drop speculative start_type from token mint The deployment token is injected once at pod creation and frozen across warm starts and restores (nothing re-injects TRIGGER_DEPLOYMENT_ID), so mint only ever happens at cold start. The start_type label was always cold - a cardinality-1 dimension - and the warm/restore variants were unreachable. Drop them; add the dimension back if a future token refreshes on warm start. Refs TRI-11844 * fix(supervisor): don't leak WORKLOAD_TOKEN_SECRET in debug log; allow same-runner reconnect to rebind run * test(core): fix flaky retry-delay boundary assertion (>= minDelay) * refactor(webapp): use lru-cache for the worker_id unwrap memo Replace the plain-Map memo with an LRU. The active-deployment working set is unbounded, so an insertion-order memo can thrash to worse-than-no-cache once churn exceeds the cap; an LRU keeps the hot set and degrades gracefully. Uses lru-cache directly (the lib @internal/cache wraps; its async SWR cache is the wrong shape for this synchronous ingest hot path) - same version already in the lockfile, no new package version. Refs TRI-11851 * fix(webapp): route created-at gate run lookup through the run-store * fix(supervisor): redact TRIGGER_DEPLOYMENT_ID from relayed debug logs * fix(supervisor): import deployment token minting --------- Co-authored-by: Chris Arderne <chris@trigger.dev>
…CSRF logout (#72) * fix(webapp): require same-origin navigation for GET /logout to block CSRF logout The /logout route exposes a `loader` that runs on GET and calls `authenticator.logout(...)`. A cross-site top-level GET navigation (e.g. a victim clicking a link to https://cloud.trigger.dev/logout on an attacker's page) therefore logs the victim out. Because the `__session` cookie is `SameSite=Lax`, the browser still sends it on cross-site top-level GET navigations, so SameSite does not mitigate this (CWE-352, forced logout). Gate the loader with a same-origin check (`isSameOriginNavigation`, deny-by- default on missing headers), redirecting to `/` otherwise. Server-initiated `throw redirect("/logout")` flows (auto-logout, SSO expiry) and the in-app SideMenu logout link are same-origin and unaffected; cross-site clicks are rejected. Deliberately not converted to POST-only so the existing `throw redirect("/logout")` flows keep working. Advisory: GHSA-2cqj-rq4j-8835 * chore(webapp): shorten logout navigation comments
…icated environment (#66)
* fix(webapp): rate-limit unauthenticated OTLP ingestion endpoints Add a per-IP rate limiter to /otel/* to bound unauthenticated bulk-injection and DoS against the OTLP ingestion path. Fails open on limiter errors and when the source IP is unknown, with generous, configurable defaults so legitimate telemetry is not throttled. This is a minimal hardening step only — it does NOT authenticate the endpoints (see PR description). * fix(webapp): make OTLP per-IP rate limiter opt-in and document IP-keying caveats The per-IP limiter on /otel/* was enabled by default and keys on the source IP. Where legitimate clients share a single egress IP (NAT or a shared proxy) their telemetry collapses into one bucket and could be throttled, so a default-on limiter risks dropping telemetry that flows fine today (fail-open only covers limiter errors, not over-limit). Default OTLP_RATE_LIMIT_ENABLED to 0 so it is opt-in, and document the preconditions for enabling it safely: each client must present a distinct IP through a proxy that appends the real client IP to X-Forwarded-For, and the limit must be sized for aggregate shared-source volume. Without such a proxy the X-Forwarded-For value is client-controlled and the per-IP bound is bypassable. * fix(webapp): harden optional OTLP request rate limiting
…#58) * fix(webapp): scope GitHub App installation update to the caller's org * fix(webapp): make GitHub App install session single-use Invalidate the GitHub App installation state cookie once a callback consumes an installation_id, so a single install initiation cannot be replayed against other installation_ids.
…bundled-datastore defaults (#57) * fix(webapp): require control-plane auth secrets * fix(helm): remove weak default credentials for bundled datastores Empty the shipped postgres/clickhouse/minio/registry credential defaults so an unconfigured install fails closed instead of booting with publicly-known passwords. Add fail-closed validation guards for the bundled (deploy=true) datastores and feed CI throwaway render-time values so chart linting stays green. * fix(webapp): reject known-insecure control-plane secret defaults Add a blocklist refine to PROVIDER_SECRET, COORDINATOR_SECRET and MANAGED_WORKER_SECRET so the webapp rejects the publicly-known placeholder strings at boot, matching the value the coordinator already refuses at startup. Add the three now-required vars to the root .env.example and align the supervisor MANAGED_WORKER_SECRET example with the Docker example so local setups authenticate consistently. * fix(helm): fail closed on missing control-plane secrets and fix datastore guard guidance Blank the bundled provider/coordinator/managed-worker secret defaults and add a render-time guard so an unconfigured install fails closed instead of shipping repo-published tokens, matching the datastore fail-closed pattern. Require both postgres.auth.postgresPassword and postgres.auth.password, and point the Postgres/ClickHouse guard messages at the key paths the webapp actually consumes so operators are not sent to a crash loop. Add placeholder credentials to values-production-example.yaml and the CI lint values so the documented install paths still render, and update the README to reflect that a values file with credentials is now required. * test(webapp): provide required control-plane secrets in e2e harness The e2e webapp harness (startTestServer) spawns the production webapp bundle, which now fails closed when PROVIDER_SECRET / COORDINATOR_SECRET / MANAGED_WORKER_SECRET are unset. Supply strong test values (not the blocklisted known-insecure defaults) so the webapp boots and the E2E Tests: Webapp check passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(webapp,hosting): fail closed on empty app secrets and stop shipping working ones Extend the fail-closed convention to the two remaining app secrets and the Docker Compose self-host path: - env.server: SESSION_SECRET and MAGIC_LINK_SECRET now require .min(1), so an empty value fails the schema parse at boot rather than running with an empty secret (SESSION_SECRET is the JWT signing key) or falling through to the emailAuth runtime guard. - hosting/docker/.env.example and apps/supervisor/.env.example: ship the crypto/auth secrets empty with a short 'generate with openssl rand -hex 16' comment instead of the previous working public values. A bare 'cp .env.example .env && docker compose up' now fails closed on the first missing secret; the documented setup already generates all six. - Tests: cover empty and unset rejection for all five required secrets. Addresses SEC-387 / GHSA-pqxw-g93w-hj9x (the Docker path the earlier commits in this PR did not cover). * fix(webapp,helm): reject published application secret defaults * fix(helm): reject unsupported bundled datastore secret references * format * .env * feat(helm): auto-generate app and control-plane secrets instead of failing closed Leave secrets.* empty to have the chart generate a strong value on first install, retained across upgrades via lookup so ENCRYPTION_KEY/SESSION_SECRET are never rotated. Explicit values and existingSecret still win. Removes the now-redundant fail-closed guards for these secrets (the webapp still rejects known-insecure values at startup). * feat(hosting): add docker generate-secrets.sh to fill self-hosting secrets Fills empty required secrets in .env with openssl-random values. Safe to re-run: never overwrites an already-set value, so restarts and re-runs don't rotate ENCRYPTION_KEY/SESSION_SECRET. Docs updated to run it during setup. * ci(helm): lint and render release-helm with ci lint-values The bundled-datastore passwords are now fail-closed, so a values.yaml-only lint/template render fails. Supply ci/lint-values.yaml like the prerelease workflow already does, keeping the chart release pipeline green. * feat(webapp): allow opting out of the insecure-default secret blocklist ALLOW_INSECURE_DEFAULT_SECRETS lets a deployment boot while still using a known-published default it cannot safely rotate yet (e.g. ENCRYPTION_KEY protects existing data, SESSION_SECRET rotation logs everyone out). It only bypasses the known-insecure blocklist; the min-length and 32-byte ENCRYPTION_KEY checks still apply. A loud warning is logged at boot naming any secret still on a published default. * feat(hosting): auto-generate bundled-datastore passwords for docker self-hosting Remove the shipped weak defaults for postgres/clickhouse/minio/registry. generate-secrets.sh now fills each datastore password (openssl rand), and for the registry writes a matching bcrypt htpasswd via docker. Connection URLs are derived from the single password var by compose interpolation, so server and client always match; datastore services fail closed with a helpful message if a password is unset. Never clobbers an existing value - use --force to rotate. * docs(self-hosting): document secret auto-generation, rotation gotchas, and the insecure-default opt-out Note that generate-secrets.sh also fills the bundled datastore passwords; add ALLOW_INSECURE_DEFAULT_SECRETS to the webapp env list; add a Helm secret generation/rotation section covering upgrade retention, the GitOps regeneration caveat, and the lack of a clean ENCRYPTION_KEY migration. * feat(helm): auto-generate bundled-datastore passwords Generate the postgres/clickhouse/minio passwords once into a chart-managed datastore Secret (retained across upgrades via lookup) and point each bundled subchart at it via auth.existingSecret; the webapp reads them back through secretKeyRef and $(VAR) URL interpolation, so server and client always match. The registry password is generated and retained inside secrets.yaml (consumed at render time by htpasswd + dockerconfigjson). Drops the deploy=true datastore password guards - a bare helm install now renders and deploys with strong, unique credentials, and explicit values or an existingSecret still win. * fix(helm): start the s2 container via args, not command The s2 image ENTRYPOINT is ["./s2"] with the subcommand/flags as CMD. Setting Kubernetes command overrode the entrypoint and tried to exec "lite" directly, so the pod failed with StartError and realtime streams v2 (the default) never came up. Passing them as args preserves the entrypoint (./s2 lite ...). Verified live in kind: the s2 pod now reaches Running. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: nicktrn <55853254+nicktrn@users.noreply.github.com>
* fix(webapp): gate CLI auth-code PAT minting behind explicit consent + rate limit
The `/account/authorization-code/:code` loader minted and bound a Personal
Access Token as a pure side effect of a GET, so any authenticated browser that
merely landed on the URL (phished link, prefetch) silently issued a CLI PAT
bound to an attacker-supplied code, which the attacker then harvested from the
unauthenticated, unthrottled `/api/v1/token` endpoint.
- Move the mint out of the loader into an `action` behind an explicit
"Authorize" POST from a logged-in human. The loader now only renders a
consent screen (read-only `isAuthorizationCodeMintable`). The CLI contract is
unchanged: it never calls this route and keeps polling `/api/v1/token`, which
already returns `{ token: null }` until consent is given.
- Add IP rate limiting to `/api/v1/authorization-code` (mint) and per-code rate
limiting to `/api/v1/token` (poll). The poll limiter is keyed by the code,
not the IP, so the CLI's ~1/s poll loop isn't broken behind a shared NAT.
- Remove both endpoints from the global rate-limit allowlist.
- Shorten the unconsumed-code TTL from 10 minutes to 2.
Response shapes are unchanged; the only new behavior is a 429 on limit breach.
Addresses the auth-code half of GHSA-58mc (TRI-9770).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 92ef2c3cd68fc6c5d3e7b01ad553bae5fcf121b6)
* fix(webapp): require ADMIN role to rename or delete a project
verifyProjectMembership only proved org membership, so any MEMBER-role
invitee could rename or permanently delete a project and all its runs,
schedules, and environments. Gate the rename/delete intents on
OrgMember.role === ADMIN.
Addresses TRI-9870.
Co-authored-by: Daniel Sutton <dansutton@trigger.dev>
* fix(webapp): keep auth-code/token endpoints allowlisted (don't break CLI login)
The global apiRateLimiter keys on the Authorization header and returns 401 for
any matched-but-unauthenticated /api path BEFORE the route runs. The CLI
auth-code/token endpoints are intentionally unauthenticated, so removing them
from the allowlist 401s them and breaks CLI login outright. Restore the
allowlist entries (skipping the auth-keyed global limiter) — the dedicated
authCodeRateLimiter in the route actions still provides the throttle.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit b823a8750b570802f245cbbab3d948b509db3cef)
* fix(webapp): require ADMIN for org delete/rename and non-DEV key regen
Add an isOrgAdmin helper (legacy OrgMember.role, independent of the RBAC
ability layer so it holds in OSS deployments) and gate on it for:
- organization rename/delete (settings index action)
- regenerating non-development API keys (production/staging/preview are
org-wide; DEV self-service is unchanged)
Both previously checked only org membership, letting any MEMBER perform
them.
Addresses TRI-9870.
Co-authored-by: Daniel Sutton <dansutton@trigger.dev>
* fix(webapp): bound auth-code minting even without X-Forwarded-For + record deferral
- The mint rate limiter previously no-op'd when extractClientIp returned null
(no X-Forwarded-For, e.g. non-ALB/direct deploys), leaving minting unbounded
there. Fall back to a shared key so it's always throttled.
- Add a server-changes note documenting the auth-code/org-admin hardening and
recording the intentional inviteMembers-baseline deferral so it isn't lost.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit d532795a28ac76b52b14d6a0ef72ac401535c7e1)
* fix(webapp): restore auth-code TTL to original 10 minutes
The 2-minute TTL was imported from the third-party report's recommendation, not
grounded in a real need. The consent gate is what closes the phishing vector;
the TTL length is immaterial to it. Restore the original 10-minute window (keep
the shared constant purely to stop the mint/read paths drifting).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 70951ddb57faf9ffb96f032110fda1668ea4ed62)
* fix(cli): widen login poll window to fit the new consent click
The auth-code login page now requires an explicit "Authorize" click before a PAT
is minted (it no longer mints as a side effect of loading the page). The CLI/MCP
poll loop was sized (~60s) for the old auto-mint flow, so a human who takes
longer than ~60s to approve would get "Failed to get access token" even though
the code is valid for 10 minutes. Widen the poll to ~5 minutes — within the code
TTL and the per-code poll rate limit (~1/s).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 80c39d6f0d63c89a6b50fcf8b80c02c1801472ff)
* docs(webapp): correct stale CLI poll-count in auth-code rate-limiter comment
The CLI poll window was widened to ~5 min, so "up to ~61 times/min / 60 retries"
no longer describes it. The steady cadence is ~1/s (~60/min), still under the
100/min/code cap regardless of total poll count. Comment-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 3f87584db9c7e91474977ae9431b3b8b71c2e6d2)
* refactor(webapp): use findFirst over findUnique in auth-code lookups
Code-review cleanup: findFirst avoids the findUnique constraint on the
non-unique lookup shape used by the auth-code mint/consent path.
Co-authored-by: Daniel Sutton <dansutton@trigger.dev>
* fix(webapp): skip auth-code mint limit when there's no trustworthy client IP
The shared "no-forwarded-for" fallback collapsed every X-Forwarded-For-less
request into one 30/min bucket, letting a single client DoS login for a whole
non-ALB instance. Skip the limit when there's no client IP instead — matching
the existing magicLinkRateLimiter pattern. Our cloud is behind an ALB so the
per-IP limit always applies there; the consent gate (not this limit) is what
closes the PAT-theft vector, so leaving non-proxied self-host minting unbounded
is low-risk row churn rather than a login outage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit e5b9e109384b163b397ad879c9c0ae3b7e4d1853)
* docs: scope server-changes note to shipped fixes
* format
* fix(webapp): use conform submission.reply for ADMIN-denied 403s
The ADMIN gate returned the pre-parseWithZod `submission.error[""]` shape,
which no longer typechecks (TS2339) and would not surface in the form.
Use `submission.reply({ formErrors })` so the denial message renders and
the types line up with the parseWithZod migration on the base branch.
* chore: consolidate changeset and tighten comments
* fix(cli): retry auth-code polling on 429 instead of aborting
The login poll loop threw an AbortError for any failed token response,
which pRetry treats as fatal. A 429 from the per-code poll rate limiter
would therefore abandon the whole login ("Failed to get access token")
even though the auth code is still valid and the user may not have
approved the consent screen yet.
Expose the HTTP status on wrapZodFetch failures and, in the poll path,
treat a 429 as a retryable error so the loop backs off and keeps polling.
Covers both the CLI login command and the MCP auth flow, which share the
same getPersonalAccessToken helper.
* chore(webapp): scope changes to auth-code login
* chore: align auth-code login release note
---------
Co-authored-by: Daniel Sutton <dansutton@trigger.dev>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nv IDs) (#46) * fix(webapp): scope schedule lookups by project and environment (TRI-9865) Closes a cluster of cross-tenant IDORs in the schedule routes and services by scoping every `findFirst({friendlyId})` to the caller's projectId, and by gating the env-scoped schedule API on env-visibility so a low-trust key (e.g. dev) can't see or mutate a schedule whose instances live in a different env. Foreign environment IDs passed to CheckScheduleService now fail closed instead of being silently filtered. Three observable API changes are documented in .server-changes/tri-9865-tenant-isolation-schedules.md. Closes TRI-9865, TRI-10040, TRI-10041 (P0); TRI-9854, TRI-9869, TRI-9960, TRI-9963, TRI-9997 (P1); TRI-9859 (P2). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(webapp): consolidate schedule env-visibility into tri-state helper Address review feedback on the TRI-9865 batch: - Promote findScheduleScopedToEnvironment to a tri-state helper getScheduleEnvVisibility returning { status: 'visible' | 'hidden' | 'missing' }. PUT can disambiguate hidden (refuse, 404) from missing (fall through to upsert's create path) without re-implementing the visibility logic inline. - All three call sites (GET / PUT / DELETE in api.v1.schedules.$id) now share the single helper. - Add test coverage for the deduplicationKey branch of scheduleWhereClause (covered the sched_-prefix branch only before). - Add DeleteTaskScheduleService test asserting DECLARATIVE schedules cannot be deleted (surfaces as a service-layer error after the visibility check passes). - Rename the misleading "devEnv" test fixture to "stagingEnv" to match its actual RuntimeEnvironment type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(webapp): regression tests for schedule environment scoping Scopes schedule read/update/delete/activate to the caller's environment via a shared getScheduleEnvVisibility (visible/hidden/missing) helper. Real-Postgres tests (testcontainers) cover checkSchedule, delete, setActive, and PUT upsert. Verified RED on the shared visibility guard (cross-env detection flips); GREEN 13/13. Bundles the streamBatchItems timeout bump. * fix(webapp): reject foreign environment IDs when upserting a schedule CheckScheduleService.call previously intersected `project.environments` with the caller-supplied `environmentIds` via `.filter()`, silently dropping any ID that didn't belong to the authorized project. Downstream UpsertTaskScheduleService.#createNewSchedule then iterated the raw input and created TaskScheduleInstance rows pairing the validated projectId with whichever environmentId the caller sent — including another tenant's RuntimeEnvironment. When the schedule fires, the engine triggers against the *referenced* environment, enabling cross-tenant task execution if the victim env id is known. Replace the silent intersection with an explicit rejection: build a map of project.environments by id and throw `ServiceValidationError` on the first foreign id, so #createNewSchedule and #updateExistingSchedule never see an environmentId outside the authorized project. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(repro): remove reproducer from PR branch * test(webapp): regression test for cross-project schedule env scoping Extracts the env-scoping into a pure resolveProjectScopedEnvironments (returns a foreign-id flag or the mapped envs) and unit-tests it: all-valid resolves; foreign id rejected; foreign mixed with valid still rejected (not dropped); empty ok. Verified RED against the pre-fix silent filter-drop, GREEN with the rejection. Bundles the streamBatchItems timeout bump. * fix(webapp): enforce per-env access when creating env vars The env-vars `new` action passed user-supplied `environmentIds[]` straight to `repository.create` after a project-membership check. DEV environments are per-user (`RuntimeEnvironment.orgMember.userId`), and the dashboard loader filters other members' DEV envs out of the UI — but the action did not enforce the same filter. A project member could submit `environmentIds=[my_dev, victim_dev]` plus a key/value, and the value was injected into the victim's next task run via `resolveVariablesForEnvironment` (whose secret-store key prefix is `environmentvariable:<projectId>:<envId>:`). Now query `runtimeEnvironment.findMany` with the same OR clause `findEnvironmentBySlug` uses — non-DEV envs in the project, or DEV envs owned by the requesting user — and refuse the submission if any submitted ID is missing from the result. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(repro): remove reproducer from PR branch * test(webapp): regression test for env-var DEV-environment ownership Extracts the per-env writability rule into a pure findUnauthorizedEnvironmentId (shared env types writable by any member; DEV only by its owner; unknown id rejected) and unit-tests it. Route now fetches the candidate envs and applies the helper. Verified RED against the pre-fix no-check behaviour, GREEN with it. Bundles the streamBatchItems timeout bump. * fix(webapp): reject mixed valid/foreign environmentIds in env-var create/edit The create()/edit() guard used `environmentIds.every((v) => !inProject(v))`, which only errors when EVERY id is foreign — a single in-project id short-circuited it, so a mixed array passed and the write loop stored values/secrets against another tenant's environment. Switch to `.some` so any foreign id rejects the whole request. Adds RED→GREEN cross-tenant regression tests and bumps the streamBatchItems CI timeout. * test(webapp): isolate schedule scoping tests from the global prisma singleton The checkSchedule/deleteTaskSchedule/setActiveOnTaskSchedule regression tests imported the full services, which pull in ~/db.server; its eager global-prisma $connect() becomes an unhandled rejection in a unit-test job with no reachable global database, failing vitest even though every test passes. Make schedules.server a leaf (type-only Prisma imports) and rewrite the three tests to exercise the project/environment scoping primitives directly against the container DB (scheduleWhereClause, scheduleUniqWhereClause, resolveProjectScopedEnvironments) instead of importing the services. * fix(webapp): enforce DEV-env ownership on env-var edit and delete The per-user DEV-env write check was only on the create route; the edit/delete value actions passed a user-controlled environmentId straight to the repository, which only checks project membership — letting a member overwrite or delete an env-var value in another member's DEV environment. Gate both actions with the same findUnauthorizedEnvironmentId check the create route uses. * format * chore: consolidate server-change and tighten comments * fix(webapp): use .some for schedule env visibility and gate activate/deactivate A schedule can be bound to several environments at once, and the schedule list surfaces a schedule for any environment it has an instance in. The per-schedule visibility check required every instance to be in the caller's environment, so a multi-environment schedule was listed but returned 404 on GET/PUT/DELETE for the same key. Match the list's semantics: a schedule is visible when it has at least one instance in the caller's environment. Also apply the same environment-visibility gate to the activate and deactivate endpoints, which previously scoped only by project and let a key scoped to one environment enable/disable a schedule that runs only in another environment of the same project. * fix(webapp): reject empty-string foreign environment id instead of dropping it The foreign-id guard used a truthiness check, so an empty-string environment id (a foreign id) was falsy and fell through to the accepted path, where it was silently dropped rather than rejected. Use an explicit undefined check so any foreign id, including the empty string, is reported and rejected. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Arderne <chris@trigger.dev>
…env cancel IDOR (#70) * fix(webapp): scope deployment lookup by environment to prevent cross-env cancel IDOR The private getDeployment helper in deployment.server.ts resolved worker deployments scoped only by projectId. Because a single API key authenticates to one environment but projectId is shared across all environments of a project, a lower-trust environment key (dev/preview/CI) could cancel, progress, or request registry credentials for another environment's (e.g. prod) in-progress deployment if it knew the target's friendlyId. Scope getDeployment by environmentId instead of projectId, and update the three call sites (cancelDeployment, progressDeployment, generateRegistryCredentials) to pass the environment id. The registry-credentials platform call stays project+region scoped by design. The dashboard cancel route (resources.$projectId.deployments.$deploymentShortCode.cancel) is project-scoped by design via RBAC; it now passes the located deployment's own environmentId so its behavior is preserved. GHSA-4672-hwv6-gq62 * fix(webapp): remove unused project ID from deployment cancellation * chore(webapp): format deployment cancellation call
…65) * fix(core): block prototype-pollution via run metadata operation keys * chore: remove test, add changeset * respond to devin comments * fix(core): block prototype pollution in unflattenAttributes via dangerous key segments * fix(core): preserve safe constructor attributes * format * chore(core): combine prototype pollution changeset
…ping (#28) * fix(webapp): harden Electric sync routes against SQL injection and cross-tenant leak sync.traces.\$traceId.ts and sync.traces.runs.\$traceId.ts interpolated params.traceId raw into the Electric `where` clause, and that clause had no organizationId predicate. An authenticated user can craft a traceparent header so the runtime SDK persists a malicious traceId into their own org's row, request /sync/traces/<crafted-id>, pass the membership check on that very row, and then watch Electric stream rows from any tenant whose traceId matches the crafted SQL. Closes the vector at four layers: - OtelTraceIdSchema validates the canonical 32-lowercase-hex format at the Zod layer; non-conforming input 404s with no error echo. - buildElectricTraceWhereClause emits `"traceId"='…' AND "organizationId"='…'` so cross-tenant collision doesn't matter even if input validation is ever bypassed. Both inputs are re-checked at the function boundary. - RESERVED_ELECTRIC_SHAPE_PARAMS strips `where`/`table`/`columns` from the incoming searchParams before forwarding to Electric so callers can't override the server-set values. - validateRealtimeTags + a matching Zod allowlist in realtime.v1.runs.ts reject tag values that could break out of the `"runTags" @> ARRAY['<tag>',…]` SQL string and steer the jumpHash-derived shard key. Closes TRI-9903, TRI-9904, TRI-9905, TRI-9906 (4 P0). Also closes TRI-9913 (Shape param allowlist) and TRI-9985 (realtime tag interpolation). 19 unit tests cover the regex, the where-clause builder, the reserved- param set, and the tag allowlist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(webapp): regression tests for Electric sync SQLi + scoping hardening Centralizes the hardening in a pure electricShape.server module: OtelTraceIdSchema (32-hex), buildElectricTraceWhereClause (org-scoped, validated), and realtime tag sanitization (reject unsafe chars, escape quotes), plus reserved-param stripping. Pure test (23 cases). Verified RED with the trace-id validation and tag sanitizer neutered (12 injection tests flip), GREEN with them. Bundles the streamBatchItems timeout bump. * fix(webapp): scope TaskRun trace sync by projectId, not nullable organizationId TaskRun.organizationId is nullable and the table is far too large to backfill, so AND-scoping the trace-runs Electric shape by organizationId silently dropped legacy NULL-org runs from the trace view. Scope by the non-null projectId instead: buildElectricTraceWhereClause now takes a typed tenant column (org for the TaskEvent shape, project for TaskRun). Tenant-safe — membership is verified against the project's org and a trace's runs share one project, and the column is a fixed union, never user input. * docs: reword server-changes note to comply with release-note conventions * chore(webapp): remove electricShape test and shorten comments * fix(webapp): preserve project scoping after primary trace lookup * widen test --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Arderne <chris@trigger.dev>
* feat(supervisor,cli): source worker.id telemetry from the deployment friendlyId The runner stamps the OTel worker.id from TRIGGER_DEPLOYMENT_ID, which the supervisor may set to an opaque token. Have the supervisor also pass the plain friendlyId in TRIGGER_DEPLOYMENT_FRIENDLY_ID, and have the runner prefer it for the telemetry worker.id (falling back to TRIGGER_DEPLOYMENT_ID for older supervisors). The auth header keeps using TRIGGER_DEPLOYMENT_ID unchanged. New deploys then report a stable deployment id in telemetry with no decode - including to customers' own external exporters. Refs TRI-11974 * fix(cli): redact TRIGGER_DEPLOYMENT_ID from managed run debug logs
🦋 Changeset detectedLatest commit: fc392b1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
carderne
marked this pull request as ready for review
July 21, 2026 10:52
Contributor
WalkthroughThe pull request adds deployment-scoped workload tokens, authenticated snapshot callbacks, environment-scoped worker operations, safer metadata and SQL handling, authorization-code consent and rate limiting, and telemetry deployment identifiers. It also hardens self-hosted secret configuration, adds persistent Helm-generated credentials, updates Docker secret generation, and adjusts CI Helm rendering. Related tests, documentation, changesets, and environment examples are included. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
🧭 Helm Chart Prerelease PublishedVersion: Install: helm upgrade --install trigger \
oci://ghcr.io/triggerdotdev/charts/trigger \
--version "4.5.5-pr4316.fc392b1"
|
d-cs
approved these changes
Jul 21, 2026
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
matt-aitken
approved these changes
Jul 21, 2026
Merged
carderne
pushed a commit
that referenced
this pull request
Jul 21, 2026
## Summary 5 improvements, 9 bug fixes. ## Breaking changes - Self-hosted deployments no longer ship shared default credentials; fresh installs generate their own. If yours still uses a previously published default, set a unique value before upgrading, or set `ALLOW_INSECURE_DEFAULT_SECRETS=true` to keep booting while you migrate. ([#4316](#4316)) ## Improvements - Require explicit browser approval for CLI and MCP login, with resilient polling while approval is pending. ([#4316](#4316)) - Deployed task telemetry now reports the deployment identifier (e.g. `deployment_abc123`) in the `worker.id` attribute, instead of an opaque internal value. Upgrade to get the readable identifier in your own OpenTelemetry exporters. ([#4316](#4316)) - Prevent prototype pollution when applying run metadata operations or reconstructing nested telemetry attributes, while preserving legitimate `constructor` and `prototype` fields. ([#4316](#4316)) - Add helpers to mint and verify the deployment-scoped token used to authenticate run controllers to the platform. ([#4316](#4316)) ## Server changes These changes affect the self-hosted Docker image and Trigger.dev Cloud: - Added optional request rate limiting for telemetry ingestion endpoints. ([#4316](#4316)) - Background-worker deployment lookups are now scoped to the authenticated environment. ([#4316](#4316)) - Updating a GitHub App installation from the callback flow is now scoped to your own organization, so an installation ID belonging to another organization can no longer be used to refresh that organization's installation record. The GitHub App installation session is also now single-use, so completing an installation callback invalidates its state and it can no longer be replayed. ([#4316](#4316)) - Scope schedule and environment-variable writes to the caller's project and environment ([#4316](#4316)) - Reject compute snapshot callbacks that do not match the snapshot request that created them. ([#4316](#4316)) - Require secret-key authentication to initialize the session out (agent→client) stream, matching the append route. ([#4316](#4316)) - Live run and trace subscriptions now validate their identifiers more strictly and only return data from your own organization. ([#4316](#4316)) - Window-function names in the query compiler are now validated against the allowlist, matching how other function calls are handled. ([#4316](#4316)) - Authenticate run controllers to the platform with a signed, deployment-scoped token. ([#4316](#4316)) - Verify that worker actions (starting, completing, and continuing a run, and reading its snapshots) target a run belonging to the caller's environment. ([#4316](#4316)) <details> <summary>Raw changeset output</summary> # Releases ## @trigger.dev/build@4.5.6 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.6` ## trigger.dev@4.5.6 ### Patch Changes - Require explicit browser approval for CLI and MCP login, with resilient polling while approval is pending. ([#4316](#4316)) - Deployed task telemetry now reports the deployment identifier (e.g. `deployment_abc123`) in the `worker.id` attribute, instead of an opaque internal value. Upgrade to get the readable identifier in your own OpenTelemetry exporters. ([#4316](#4316)) - Updated dependencies: - `@trigger.dev/core@4.5.6` - `@trigger.dev/build@4.5.6` - `@trigger.dev/schema-to-json@4.5.6` ## @trigger.dev/core@4.5.6 ### Patch Changes - Prevent prototype pollution when applying run metadata operations or reconstructing nested telemetry attributes, while preserving legitimate `constructor` and `prototype` fields. ([#4316](#4316)) - Require explicit browser approval for CLI and MCP login, with resilient polling while approval is pending. ([#4316](#4316)) - Add helpers to mint and verify the deployment-scoped token used to authenticate run controllers to the platform. ([#4316](#4316)) ## @trigger.dev/python@4.5.6 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.6` - `@trigger.dev/build@4.5.6` - `@trigger.dev/sdk@4.5.6` ## @trigger.dev/react-hooks@4.5.6 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.6` ## @trigger.dev/redis-worker@4.5.6 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.6` ## @trigger.dev/rsc@4.5.6 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.6` ## @trigger.dev/schema-to-json@4.5.6 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.6` ## @trigger.dev/sdk@4.5.6 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.6` </details> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/supervisor/src/workloadManager/docker.ts (1)
65-65: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
opts.deploymentTokenleaks into supervisor logs.
optsnow carries the secretdeploymentToken(pertypes.ts), but it's still passed whole intothis.logger.verbose("create()", { opts })(line 65) and bound permanently viathis.logger.child({ opts, containerCreateOpts })(line 152), so every subsequent log line through that child logger (includinglogger.info/logger.erroron container create/pull failures) carries the raw token. This directly conflicts with this release's own goal of redacting deployment tokens from logs.🔒 Proposed fix
async create(opts: WorkloadManagerCreateOptions) { - this.logger.verbose("create()", { opts }); + this.logger.verbose("create()", { opts: { ...opts, deploymentToken: opts.deploymentToken ? "[redacted]" : undefined } });- const logger = this.logger.child({ opts, containerCreateOpts }); + const logger = this.logger.child({ + opts: { ...opts, deploymentToken: opts.deploymentToken ? "[redacted]" : undefined }, + containerCreateOpts, + });Consider a shared redaction helper for
WorkloadManagerCreateOptionsif this pattern also applies tocompute.ts/kubernetes.ts.Also applies to: 138-153
apps/webapp/app/routes/account.authorization-code.$authorizationCode/route.tsx (1)
69-86: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winSanitize the returned error message
createPersonalAccessTokenFromAuthorizationCodecan still surface Prisma/internal errors, so returnclientSafeErrorMessage(error)here instead oferror.messageto avoid leaking implementation details to the browser.apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts (1)
213-230: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winSecret value logged in plaintext on mismatch.
Both the length-mismatch and value-mismatch branches log the raw
managedWorkerSecretthe caller sent. This directly contradicts this PR's stated goal of redacting deployment tokens/secrets from logs, and persists the credential (or a near-miss of it) in log storage.🔒 Proposed fix
if (a.byteLength !== b.byteLength) { logger.error("[WorkerGroupTokenService] Managed secret length mismatch", { - managedWorkerSecret, + managedWorkerSecretLength: managedWorkerSecret.length, headers: this.sanitizeHeaders(request), }); return; } if (!timingSafeEqual(a, b)) { logger.error("[WorkerGroupTokenService] Managed secret mismatch", { - managedWorkerSecret, headers: this.sanitizeHeaders(request), }); return; }hosting/k8s/helm/templates/webapp.yaml (1)
415-427: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMissing
CLICKHOUSE_PASSWORDfallback for external ClickHouse with a plaintext password (noexistingSecret).This chain only handles
external.existingSecretand the newclickhouse.deploycase. It drops the "external host + plaintextexternal.password, no existingSecret" case thatsecrets.yamlalready stores under theclickhouse-passwordkey (lines 37-39) but that nothing here reads — unlike the parallel S3 (s3-access-key-id/s3-secret-access-key, lines 367-377) and Redis (REDIS_PASSWORD, lines 250-252) fallbacks, which both have this exact plain-value branch. Self-hosters usingclickhouse.deploy: falsewith a directly-setexternal.passwordwill get noCLICKHOUSE_PASSWORDenv var, breaking the webapp's connection to that ClickHouse instance.🐛 Proposed fix
{{- if and .Values.clickhouse.external.host .Values.clickhouse.external.existingSecret }} - name: CLICKHOUSE_PASSWORD valueFrom: secretKeyRef: name: {{ include "trigger-v4.clickhouse.external.secretName" . }} key: {{ include "trigger-v4.clickhouse.external.passwordKey" . }} + {{- else if .Values.clickhouse.external.password }} + - name: CLICKHOUSE_PASSWORD + valueFrom: + secretKeyRef: + name: {{ include "trigger-v4.secretsName" . }} + key: clickhouse-password {{- else if .Values.clickhouse.deploy }} - name: CLICKHOUSE_PASSWORD valueFrom: secretKeyRef: name: {{ include "trigger-v4.datastore.secretName" . }} key: clickhouse-admin-password {{- end }}Based on cross-referencing
hosting/k8s/helm/templates/validate-external-config.yaml, the equivalent fail-closed check that exists for S3's external-credentials path is also missing for this ClickHouse case — worth adding there too once the wiring is fixed.hosting/k8s/helm/templates/secrets.yaml (1)
4-17: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep the app secrets Secret on uninstall
lookuponly preserves these values across upgrades. Ahelm uninstallwill deletetrigger-v4-*-secrets, and reinstalling with empty values can regenerateENCRYPTION_KEY/SESSION_SECRET, orphaning encrypted data and invalidating sessions. Addhelm.sh/resource-policy: keephere to matchdatastore-secret.yaml.
🧹 Nitpick comments (3)
internal-packages/tsql/src/query/printer.ts (1)
3130-3139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding argument validation.
This is a great security fix. To maintain consistency with
visitCall, you might also want to callvalidateFunctionArgshere so that passing an invalid number of arguments to a window function returns a friendlyQueryErrorrather than a raw database error.♻️ Proposed refactor
const funcMeta = findTSQLFunction(node.name) ?? findTSQLAggregation(node.name); if (!funcMeta) { throw new QueryError(`Unknown function: ${node.name}`); } + validateFunctionArgs(node.args || [], funcMeta.minArgs, funcMeta.maxArgs, node.name); + const args = node.args ? node.args.map((a) => this.visit(a)) : [];packages/core/src/v3/workloadDeploymentToken.ts (1)
11-21: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConstrain
environment_typeto a bounded enum for OTEL cardinality safety.
environment_typeis typed as an unconstrainedz.string(), but this claim is used directly as theenv_typelabel onmintCounter/verifyCounterinapps/supervisor/src/workloadToken.ts. As per coding guidelines, "Do not use high-cardinality attributes in OTEL metrics such as... free-form strings" — tightening this schema field to the known environment-type enum would enforce the bound at the trust boundary (JWT claims are external input) rather than relying on callers always passing well-known values.♻️ Suggested schema tightening
- environment_type: z.string(), + environment_type: z.enum(["PRODUCTION", "STAGING", "DEVELOPMENT", "PREVIEW"]),Source: Coding guidelines
hosting/k8s/helm/values-production-example.yaml (1)
47-50: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse distinct placeholder values for
postgresPasswordandpassword.Both fields use the exact same placeholder text, unlike the
secretsblock above which uses distinct suffixes per secret.postgresPassword(superuser) andpassword(app user) are separate credentials in the Bitnami subchart — using identical example text risks self-hosters copying it verbatim and sharing one password across both roles.postgres: auth: # Required — no built-in default. The webapp connection string uses postgres.auth.password. - postgresPassword: "your-strong-postgres-password" - password: "your-strong-postgres-password" + postgresPassword: "your-strong-postgres-superuser-password" + password: "your-strong-postgres-app-password"
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b8539c0f-cebc-43fe-905f-7b4b7cd76c1a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (140)
.changeset/block-metadata-proto-pollution.md.changeset/cli-login-poll-window.md.changeset/worker-id-deployment-friendly-id.md.changeset/workload-deployment-token.md.env.example.github/workflows/helm-prerelease.yml.github/workflows/release-helm.yml.server-changes/otlp-ingestion-rate-limit.md.server-changes/require-webapp-auth-secrets.md.server-changes/scope-deployment-background-worker-lookup.md.server-changes/scope-github-app-update-to-org.md.server-changes/scope-schedule-and-env-var-writes.md.server-changes/secure-compute-snapshot-callbacks.md.server-changes/session-out-stream-private-auth.md.server-changes/tighten-realtime-and-trace-sync.md.server-changes/tsql-window-function-allowlist.md.server-changes/workload-token-supervisor.md.server-changes/workload-token-webapp.mdapps/supervisor/.env.exampleapps/supervisor/src/env.tsapps/supervisor/src/index.tsapps/supervisor/src/services/computeSnapshotService.test.tsapps/supervisor/src/services/computeSnapshotService.tsapps/supervisor/src/workloadManager/compute.tsapps/supervisor/src/workloadManager/docker.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/supervisor/src/workloadManager/types.tsapps/supervisor/src/workloadServer/index.tsapps/supervisor/src/workloadServer/workloadServer.auth.test.tsapps/supervisor/src/workloadServer/workloadServer.authLog.test.tsapps/supervisor/src/workloadToken.tsapps/webapp/app/entry.server.tsxapps/webapp/app/env.server.tsapps/webapp/app/models/schedules.server.tsapps/webapp/app/routes/_app.github.callback/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules.$scheduleParam/route.tsxapps/webapp/app/routes/account.authorization-code.$authorizationCode/route.tsxapps/webapp/app/routes/api.v1.authorization-code.tsapps/webapp/app/routes/api.v1.deployments.$deploymentId.cancel.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/api.v1.token.tsapps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.attempts.complete.tsapps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.attempts.start.tsapps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.continue.tsapps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.latest.tsapps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.since.$snapshotId.tsapps/webapp/app/routes/logout.tsxapps/webapp/app/routes/realtime.v1.runs.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.tsapps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.cancel.tsapps/webapp/app/routes/sync.traces.$traceId.tsapps/webapp/app/routes/sync.traces.runs.$traceId.tsapps/webapp/app/services/apiRateLimit.server.tsapps/webapp/app/services/authCodeRateLimiter.server.tsapps/webapp/app/services/gitHub.server.tsapps/webapp/app/services/otlpRateLimit.server.tsapps/webapp/app/services/personalAccessToken.server.tsapps/webapp/app/services/realtimeClient.server.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.tsapps/webapp/app/v3/electricShape.server.tsapps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.tsapps/webapp/app/v3/otlpTransform.server.tsapps/webapp/app/v3/services/checkSchedule.server.tsapps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.tsapps/webapp/app/v3/services/deleteTaskSchedule.server.tsapps/webapp/app/v3/services/deployment.server.tsapps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/resolveProjectScopedEnvironments.tsapps/webapp/app/v3/services/setActiveOnTaskSchedule.server.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.tsapps/webapp/app/v3/services/worker/workloadTokenAuthorization.server.tsapps/webapp/app/v3/workerIdUnwrap.server.tsapps/webapp/app/v3/writableEnvironments.tsapps/webapp/package.jsonapps/webapp/server.tsapps/webapp/test/authorizationCodeConsent.test.tsapps/webapp/test/checkSchedule.test.tsapps/webapp/test/deleteTaskSchedule.test.tsapps/webapp/test/env.server.test.tsapps/webapp/test/environmentVariablesRepository.test.tsapps/webapp/test/registryConfig.test.tsapps/webapp/test/resolveProjectScopedEnvironments.test.tsapps/webapp/test/schedulesPutEnvScoping.test.tsapps/webapp/test/setActiveOnTaskSchedule.test.tsapps/webapp/test/setup.tsapps/webapp/test/spanTraceRoutes.replicaLag.test.tsapps/webapp/test/workerIdUnwrap.test.tsapps/webapp/test/workloadTokenAuthorization.test.tsapps/webapp/test/workloadTokenGate.integration.test.tsapps/webapp/test/writableEnvironments.test.tsdocs/self-hosting/docker.mdxdocs/self-hosting/env/webapp.mdxdocs/self-hosting/kubernetes.mdxhosting/docker/.env.examplehosting/docker/generate-secrets.shhosting/docker/registry/auth.htpasswdhosting/docker/webapp/docker-compose.ymlhosting/k8s/helm/README.mdhosting/k8s/helm/ci/lint-values.yamlhosting/k8s/helm/templates/NOTES.txthosting/k8s/helm/templates/_helpers.tplhosting/k8s/helm/templates/datastore-secret.yamlhosting/k8s/helm/templates/electric.yamlhosting/k8s/helm/templates/s2.yamlhosting/k8s/helm/templates/secrets.yamlhosting/k8s/helm/templates/validate-external-config.yamlhosting/k8s/helm/templates/webapp.yamlhosting/k8s/helm/values-production-example.yamlhosting/k8s/helm/values.yamlinternal-packages/run-engine/src/engine/index.tsinternal-packages/run-engine/src/engine/systems/checkpointSystem.tsinternal-packages/run-engine/src/engine/systems/executionSnapshotSystem.tsinternal-packages/run-engine/src/engine/systems/runAttemptSystem.tsinternal-packages/run-store/src/PostgresRunStore.tsinternal-packages/run-store/src/runOpsStore.tsinternal-packages/run-store/src/types.tsinternal-packages/testcontainers/src/webapp.tsinternal-packages/tsql/src/query/printer.tspackages/cli-v3/src/commands/login.tspackages/cli-v3/src/entryPoints/managed/controller.tspackages/cli-v3/src/entryPoints/managed/env.tspackages/cli-v3/src/entryPoints/managed/execution.tspackages/cli-v3/src/entryPoints/managed/taskRunProcessProvider.tspackages/cli-v3/src/mcp/auth.tspackages/core/src/v3/apiClient/core.tspackages/core/src/v3/index.tspackages/core/src/v3/jwt.tspackages/core/src/v3/runEngineWorker/consts.tspackages/core/src/v3/runEngineWorker/supervisor/http.tspackages/core/src/v3/runEngineWorker/types.tspackages/core/src/v3/runMetadata/operations.tspackages/core/src/v3/schemas/common.tspackages/core/src/v3/utils/flattenAttributes.tspackages/core/src/v3/workloadDeploymentToken.test.tspackages/core/src/v3/workloadDeploymentToken.tspackages/core/test/taskExecutor.test.ts
💤 Files with no reviewable changes (1)
- hosting/docker/registry/auth.htpasswd
Comment on lines
+22
to
+39
| //this endpoint is unauthenticated (codes only allow a user to log in), so it's | ||
| //rate-limited per client IP. Keyed by X-Forwarded-For; if there's no trustworthy | ||
| //client IP we skip the limit rather than bucket everyone together. Self-hosters | ||
| //wanting per-IP limiting should front the app with a proxy that sets X-Forwarded-For. | ||
| const clientIp = extractClientIp(request.headers.get("x-forwarded-for")); | ||
| if (clientIp) { | ||
| try { | ||
| await checkAuthorizationCodeMintRateLimit(clientIp); | ||
| } catch (error) { | ||
| if (error instanceof AuthorizationCodeRateLimitError) { | ||
| return json( | ||
| { error: "Too many requests, please try again later." }, | ||
| { status: 429, headers: { "Retry-After": Math.ceil(error.retryAfter / 1000).toString() } } | ||
| ); | ||
| } | ||
| throw error; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Both new auth-code rate-limit checks fail closed on backend errors. Root cause: neither call site distinguishes "over limit" from "limiter backend errored" — any error other than AuthorizationCodeRateLimitError is rethrown, unlike otlpRateLimit.server.ts's deliberate fail-open pattern for exactly this failure mode.
apps/webapp/app/routes/api.v1.authorization-code.ts#L22-L39: in thecatcharoundcheckAuthorizationCodeMintRateLimit, log and fall through tonext/continue instead ofthrow errorfor non-rate-limit errors.apps/webapp/app/routes/api.v1.token.ts#L32-L45: in thecatcharoundcheckAuthorizationCodeTokenPollRateLimit, apply the same fail-open handling for non-rate-limit errors.
📍 Affects 2 files
apps/webapp/app/routes/api.v1.authorization-code.ts#L22-L39(this comment)apps/webapp/app/routes/api.v1.token.ts#L32-L45
Comment on lines
+1
to
+5
| import { redirect, type ActionFunction, type LoaderFunction } from "@remix-run/node"; | ||
| import { env } from "~/env.server"; | ||
| import { authenticator } from "~/services/auth.server"; | ||
| import { sanitizeRedirectPath } from "~/utils"; | ||
| import { isSameOriginNavigation } from "~/utils/sameOriginNavigation"; |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd sameOriginNavigation apps/webapp/app/utils | xargs cat -nRepository: triggerdotdev/trigger.dev
Length of output: 1150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the logout route and nearby usage to determine the actual behavior.
ast-grep outline apps/webapp/app/routes/logout.tsx --view expanded
printf '\n---\n'
cat -n apps/webapp/app/routes/logout.tsxRepository: triggerdotdev/trigger.dev
Length of output: 1842
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the route and confirm how the logout path behaves for GET vs POST.
cat -n apps/webapp/app/routes/logout.tsxRepository: triggerdotdev/trigger.dev
Length of output: 1611
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the logout route to see whether this gate affects only the GET loader
# and whether there's any alternate logout path.
cat -n apps/webapp/app/routes/logout.tsxRepository: triggerdotdev/trigger.dev
Length of output: 1611
Allow direct /logout navigations
isSameOriginNavigation() returns false when both Sec-Fetch-Site and Referer are missing, so a typed/bookmarked /logout visit gets redirected to / instead of logging out.
Comment on lines
+52
to
+61
| const workloadCreatedAtGateEnabled = env.WORKLOAD_CREATED_AT_GATE_ENABLED === "1"; | ||
| const workloadTokenCutoff = env.WORKLOAD_TOKEN_CUTOFF | ||
| ? new Date(env.WORKLOAD_TOKEN_CUTOFF) | ||
| : undefined; | ||
|
|
||
| if (workloadCreatedAtGateEnabled && !workloadTokenCutoff) { | ||
| logger.warn( | ||
| "WORKLOAD_CREATED_AT_GATE_ENABLED is set but WORKLOAD_TOKEN_CUTOFF is missing; the created-at gate stays off until a cutoff is configured" | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Malformed WORKLOAD_TOKEN_CUTOFF silently disables the created-at gate instead of failing loudly.
new Date(env.WORKLOAD_TOKEN_CUTOFF) on an unparseable string yields a truthy Invalid Date. workloadTokenCutoff stays truthy, so assertCreatedAtGate proceeds to evaluateCreatedAtGate, where any > comparison against NaN (cutoff.getTime()) is false — every run is then treated as "grandfathered"/allowed, regardless of its real createdAt. The startup warning only fires when the env var is unset, not when it's set but malformed, so this misconfiguration silently defeats the gate with no signal.
🛡️ Proposed fix
const workloadTokenCutoff = env.WORKLOAD_TOKEN_CUTOFF
? new Date(env.WORKLOAD_TOKEN_CUTOFF)
: undefined;
-if (workloadCreatedAtGateEnabled && !workloadTokenCutoff) {
+if (workloadTokenCutoff && Number.isNaN(workloadTokenCutoff.getTime())) {
+ logger.error("WORKLOAD_TOKEN_CUTOFF is not a valid date; the created-at gate stays off", {
+ value: env.WORKLOAD_TOKEN_CUTOFF,
+ });
+}
+
+if (
+ workloadCreatedAtGateEnabled &&
+ (!workloadTokenCutoff || Number.isNaN(workloadTokenCutoff.getTime()))
+) {
logger.warn(
"WORKLOAD_CREATED_AT_GATE_ENABLED is set but WORKLOAD_TOKEN_CUTOFF is missing; the created-at gate stays off until a cutoff is configured"
);
}And guard assertCreatedAtGate's early-return condition to also treat an invalid workloadTokenCutoff as "gate off":
- if (!workloadCreatedAtGateEnabled || !workloadTokenCutoff) {
+ if (
+ !workloadCreatedAtGateEnabled ||
+ !workloadTokenCutoff ||
+ Number.isNaN(workloadTokenCutoff.getTime())
+ ) {
return;
}
Comment on lines
+348
to
358
| private sanitizeHeaders(request: Request, skipHeaders = ["authorization"]) { | ||
| const sanitizedHeaders: Partial<Record<string, string>> = {}; | ||
|
|
||
| for (const [key, value] of request.headers.entries()) { | ||
| if (!skipHeaders.includes(key.toLowerCase())) { | ||
| sanitizedHeaders[key] = value; | ||
| } | ||
| } | ||
|
|
||
| return sanitizedHeaders; | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
git log --all --oneline -- '**/workerGroupTokenService.server.ts' | head -20
rg -n 'SENSITIVE_WORKER_HEADERS' -C5 $(git rev-list --all -- '**/workerGroupTokenService.server.ts' | tail -1) 2>/dev/null
rg -n 'MANAGED_SECRET|WORKER_HEADERS' -C3 --type=ts apps/webapp/app packages/core/srcRepository: triggerdotdev/trigger.dev
Length of output: 12134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '160,230p' apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
printf '\n--- sanitizeWorkerHeaders ---\n'
sed -n '1,80p' apps/webapp/app/v3/services/worker/sanitizeWorkerHeaders.tsRepository: triggerdotdev/trigger.dev
Length of output: 3134
Redact the full worker-header denylist here sanitizeHeaders should default to the same sensitive set as SENSITIVE_WORKER_HEADERS (authorization, cookie, and x-trigger-worker-managed-secret). Right now only authorization is dropped, so the managed secret and cookie can be logged on auth failures.
Comment on lines
1
to
+3
| {{- if and .Values.secrets.enabled (not .Values.secrets.existingSecret) }} | ||
| {{- $existing := (lookup "v1" "Secret" .Release.Namespace (printf "%s-secrets" (include "trigger-v4.fullname" .))) | default dict }} | ||
| {{- $existingData := (get $existing "data") | default dict }} |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether Helm RBAC/lookup requirements are documented for self-hosted k8s installs.
rg -n "lookup|resource-policy|RBAC" hosting/k8s/helm/README.md docs/self-hosting/kubernetes.mdx 2>/dev/nullRepository: triggerdotdev/trigger.dev
Length of output: 241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant template and docs with line numbers.
git ls-files hosting/k8s/helm/templates/secrets.yaml hosting/k8s/helm/README.md docs/self-hosting/kubernetes.mdx 2>/dev/null
printf '\n--- secrets.yaml ---\n'
wc -l hosting/k8s/helm/templates/secrets.yaml
sed -n '1,120p' hosting/k8s/helm/templates/secrets.yaml | cat -n
printf '\n--- README RBAC mentions ---\n'
rg -n -C 2 "Configure proper RBAC|lookup|existingSecret|secrets.enabled|helm upgrade|Helm" hosting/k8s/helm/README.md
printf '\n--- kubernetes.mdx RBAC/lookup mentions ---\n'
if [ -f docs/self-hosting/kubernetes.mdx ]; then
rg -n -C 2 "RBAC|lookup|helm|Secret|existingSecret|upgrade|install" docs/self-hosting/kubernetes.mdx
fiRepository: triggerdotdev/trigger.dev
Length of output: 13724
🌐 Web query:
Helm lookup function RBAC forbidden empty dict behavior docs
💡 Result:
In Helm, the lookup function is designed to be non-fatal, meaning it should not cause your templates to fail if it encounters an error or cannot retrieve a resource [1]. When a lookup call fails due to RBAC restrictions (e.g., a "forbidden" 403 error), it returns an empty dictionary (map) rather than terminating the rendering process [1]. This behavior is consistent with other scenarios where lookup returns no data, such as when a resource is not found (404) or when the function is executed in contexts that cannot connect to the Kubernetes API, such as standard helm template or helm install --dry-run [2][3][4][1]. Because the function returns an empty dictionary (or nil) in these cases, your templates must be written to handle this result gracefully to avoid "nil pointer" errors [4][5][6]. If you attempt to access a field directly on a failed lookup (e.g., $lookup.data.myKey), the template will fail [5]. To handle this safely, you should use template flow control, such as the with or if action, to check if the result is populated before accessing its fields [4][5][6]: {{- with lookup "v1" "Secret".Release.Namespace "my-secret" }} {{- /* Resource exists, perform operations / -}} data: {{.data.myKey }} {{- else }} {{- / Resource missing, forbidden, or lookup skipped; provide default behavior */ -}} data: defaultValue {{- end }} It is important to note that because these failures return an empty value silently, they can be difficult to debug [1][7]. If you are troubleshooting, using helm install --debug or checking logs for debug-level output (if available in your Helm version) can help verify whether the lookup is returning an empty result [7]. Top results: [4][1][5][6]
Citations:
- 1: https://github.com/alegeay/Sherpack/blob/main/docs/LOOKUP.md
- 2: lookup function causes lint to fail helm/helm#7955
- 3: lookup function not executed if --dry-run specified helm/helm-www#635
- 4: Unable to debug "lookup" function, as its disabled with
helm templatehelm/helm#8137 - 5: https://stackoverflow.com/questions/74043061/helm-chart-template-lookup-function-does-not-work
- 6: lookup causes lint to fail helm/helm#9733
- 7: Add some sort of warning (at least to debug level logging) if the lookup function returns nil helm/helm#32101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- keep annotations across helm templates ---'
rg -n -C 2 'helm.sh/resource-policy:\s*keep' hosting/k8s/helm/templates
printf '\n%s\n' '--- secrets-related retention docs ---'
rg -n -C 2 'retain|retained|uninstall|reinstall|resource-policy|keep|encrypted data|sessions' hosting/k8s/helm/README.md docs/self-hosting/kubernetes.mdxRepository: triggerdotdev/trigger.dev
Length of output: 5039
Require Secret-read access for retained secrets hosting/k8s/helm/templates/secrets.yaml:1-3 If Helm can’t read the existing Secret, lookup falls back to {} and the chart regenerates SESSION_SECRET, ENCRYPTION_KEY, and the other auto-generated values on upgrade, which can break sessions and make encrypted data unrecoverable. Document the required Secret-read RBAC for Helm upgrades or fail this path explicitly.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
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
4 participants
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.
Security release batch. Advisory details to be published with the release.