fix(trigger): publish worker env from Secrets Manager instead of the build machine - #7547
Open
waleedlatif1 wants to merge 3 commits into
Open
fix(trigger): publish worker env from Secrets Manager instead of the build machine#7547waleedlatif1 wants to merge 3 commits into
waleedlatif1 wants to merge 3 commits into
Conversation
…build machine
`syncEnvVars` sourced the `FUNCTION_EXECUTION_ENV` vars from `process.env` on
whatever machine ran the deploy, and dropped each one silently when unset. No
deploy path sets them: the CI job exports only `TRIGGER_ACCESS_TOKEN` and
`TRIGGER_PROJECT_ID`, and a build server reaches build-time env only through the
`TRIGGER_BUILD_` prefix, which appears nowhere in this repo. So 14 of the 16
entries had never reached a worker, and every worker variable was being entered
by hand instead.
Sourcing from the build's ambient env was also unsafe rather than merely inert:
the layer applies with `override: true`, so a local `deploy --env prod` would
have published the developer's own `.env` into production.
The list now resolves from the same `/{env}/sim/env-vars` secret the app
container boots from, through the same `@sim/runtime-secrets` reader, so the two
runtimes cannot drift and a new worker variable needs only a key in the secret
and an entry in `WORKER_SECRET_KEYS`.
`TRIGGER_DEV_ENABLED` is dropped from the list. `syncEnvVars` strips every
`TRIGGER_`-prefixed key before building its layer, so that entry had no effect
for the life of the config; `assertSyncableKeys` now fails config evaluation
rather than letting another one look published. Run dispatch does not read it —
the `init` hook marks the run process directly.
Resolution never throws: `syncEnvVars` swallows a callback rejection and then
publishes nothing, so a failed lookup degrades to the environment-independent
constants and leaves the Trigger.dev environment as the previous deploy left it.
A key absent from the secret is left untouched rather than blanked, since
several are legitimately unset.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017BBuD7nBqVURAah8t6jfND
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryThis PR publishes Trigger.dev worker configuration from the same AWS Secrets Manager entry used by the application runtime instead of inheriting values from the deployment machine.
Confidence Score: 5/5The PR appears safe to merge; the previous findings are resolved and no actionable new failure remains. The latest changes correctly route strict-sync errors through the shared logger and align both documentation locations with the implemented clearing behavior. All previous threads were resolved, including withdrawal of the deployment-credential sequencing concern, and the current code fixes the credential-revocation hole.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/core/config/trigger-env-sync.ts | Resolves worker variables from environment-specific Secrets Manager entries, clears omitted authoritative values, and supports strict deployment failure. |
| apps/sim/trigger.config.ts | Replaces deployment-machine environment reads with asynchronous secret-backed worker environment synchronization and repository-standard logging. |
| packages/runtime-secrets/src/index.ts | Extracts the existing fetch, retry, parse, and validation path into a reusable fetchSecretMap function. |
| .github/workflows/ci.yml | Grants the development deployment job OIDC permissions and configures AWS credentials for secret lookup. |
| apps/sim/lib/core/config/trigger-env-sync.test.ts | Covers secret mapping, clearing semantics, strict failures, fallback behavior, and unsyncable-key validation. |
| packages/runtime-secrets/src/index.test.ts | Verifies reusable secret retrieval, secret-ID forwarding, shape validation, and isolation from process.env. |
| .claude/rules/sim-architecture.md | Documents the shared secret source, worker-key workflow, prefix restriction, and authoritative clearing semantics. |
Reviews (3): Last reviewed commit: "fix(trigger): use the shared logger and ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Two review findings on the worker env sync. Skipping a key the authoritative secret no longer carries left the worker's previous value in place, so deleting a compromised `E2B_API_KEY`, `DAYTONA_API_ KEY` or `REDIS_URL` from Secrets Manager did not revoke it in the worker: the app stopped loading the credential while runs kept using it. Absent keys are now published as `''`. Every key in the list is read as a truthiness or `||` check and none uses `??`, so `''` is indistinguishable from unset at the read sites. Nothing is cleared when the read failed or the environment is unmapped — without a successful read there is no authority to clear against. A failed lookup still degrades to the constants by default, because the build credentials do not exist on the staging and prod deploy paths yet and failing there would break every deploy the moment this lands. `SIM_TRIGGER_ENV_SYNC_ REQUIRED` makes that case fail instead, so the rollout can finish and then close the hole for good. `syncEnvVars` swallows a rejected callback and continues having published nothing, so rejecting cannot fail a deploy on its own — the config turns the error into a non-zero exit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BBuD7nBqVURAah8t6jfND
Collaborator
Author
Collaborator
Author
|
@cubic-dev-ai review this PR |
Contributor
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
Review follow-ups. The required-sync failure path used `console.error`, which the repo's logging rule forbids outside the CLI package; it now goes through `createLogger`. The architecture rule and `normalizeSecretValue` both still described the preserve-on-absence behavior that the previous commit replaced, so they told a maintainer the opposite of what the resolver does when a key leaves the secret. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BBuD7nBqVURAah8t6jfND
Collaborator
Author
Collaborator
Author
|
@cubic-dev-ai review this PR |
Contributor
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
syncEnvVarsread theFUNCTION_EXECUTION_ENVvars fromprocess.envon whatever machine ran the deploy, and dropped each one silently when unset. No deploy path sets them — the CI job exports onlyTRIGGER_ACCESS_TOKENandTRIGGER_PROJECT_ID, and a build server reaches build-time env only through theTRIGGER_BUILD_prefix, which appears nowhere in this repo. 14 of the 16 entries had never reached a worker, so worker variables were being entered by hand.override: true, so a localdeploy --env prodwould have published the developer's own.envinto production./{env}/sim/env-varssecret the app container boots from, through the same@sim/runtime-secretsreader, so the two runtimes can't drift. Adding a worker variable is now a key in the secret plus an entry inWORKER_SECRET_KEYS— nothing typed into the dashboard.''rather than skipped, so deleting a compromised credential actually revokes it in the worker instead of leaving the previous value behind. Every key in the list is read as a truthiness or||check and none uses??, so''is indistinguishable from unset at the read sites. Nothing is cleared when the read failed — there's no authority to clear against.TRIGGER_DEV_ENABLED:syncEnvVarsstrips everyTRIGGER_-prefixed key before building its layer, so it had no effect for the life of the config.assertSyncableKeysnow fails config evaluation rather than letting another entry look published. Run dispatch doesn't read it — theinithook marks the run process directly.SIM_TRIGGER_ENV_SYNC_REQUIREDmakes it fail the deploy instead. Raising alone can't do that —syncEnvVarscatches a rejected callback and continues having published nothing — so the config converts the error to a non-zero exit.fetchSecretMapfromloadRuntimeSecretsso container boot and the deploy-time sync share one reader; boot behavior is unchanged and its existing tests still pass.Rollout. These credentials live in Trigger.dev environment settings, so no diff can carry them: merge → set
TRIGGER_BUILD_AWS_ACCESS_KEY_ID/_SECRET_ACCESS_KEY/_REGIONin the staging and prod Trigger.dev environments → setSIM_TRIGGER_ENV_SYNC_REQUIREDso a silent failure becomes impossible. Until step two, staging and prod log the read failure and keep whatever the environment already holds — no regression from today, just no sync yet. Dev is covered by this PR.Type of Change
Testing
TRIGGER_guard, error fallback, strict mode); verified they fail when the clearing fix, the empty guard, the strict guard and the error fallback are each revertedfetchSecretMap; all 7 pre-existingloadRuntimeSecretstests unchanged and passingtrigger.config.tsagainst live Secrets Manager: 15 vars published for staging (3 as secrets, 3 cleared), correct secret per environment,devtarget still skipped, noTRIGGER_-prefixed key survivingbun run lint,bun run check:audits(45/45),bun run docs-manifest:check,bun run type-checkall cleanChecklist
🤖 Generated with Claude Code
https://claude.ai/code/session_017BBuD7nBqVURAah8t6jfND