From f4d5a3e13c57143d89d7156036dd467ac9d02c97 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 5 Sep 2026 17:47:25 -0700 Subject: [PATCH 1/3] fix(trigger): publish worker env from Secrets Manager instead of the build machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) Claude-Session: https://claude.ai/code/session_017BBuD7nBqVURAah8t6jfND --- .claude/rules/sim-architecture.md | 11 +- .github/workflows/ci.yml | 13 ++ .../lib/core/config/trigger-env-sync.test.ts | 130 +++++++++++++ apps/sim/lib/core/config/trigger-env-sync.ts | 184 ++++++++++++++++++ apps/sim/trigger.config.ts | 62 ++---- packages/runtime-secrets/src/index.test.ts | 29 ++- packages/runtime-secrets/src/index.ts | 24 ++- 7 files changed, 396 insertions(+), 57 deletions(-) create mode 100644 apps/sim/lib/core/config/trigger-env-sync.test.ts create mode 100644 apps/sim/lib/core/config/trigger-env-sync.ts diff --git a/.claude/rules/sim-architecture.md b/.claude/rules/sim-architecture.md index d950851a3f1..154ec469432 100644 --- a/.claude/rules/sim-architecture.md +++ b/.claude/rules/sim-architecture.md @@ -62,8 +62,15 @@ Every export of a `'use client'` module becomes a *client reference* on the serv Server code runs in two runtimes with **different environments**. The app container loads the full env from `SIM_ENV_SECRET_ID` (Secrets Manager). Trigger.dev workers — which execute workflows, so every block handler and every tool call — get their env from the Trigger.dev -dashboard; `trigger.config.ts` additionally syncs `DB_APP_NAME`, `TRIGGER_DEV_ENABLED`, and the -`FUNCTION_EXECUTION_ENV` vars. The repo cannot see what the dashboard holds. +dashboard, plus whatever `trigger.config.ts` publishes at deploy time: `DB_APP_NAME` and the +`WORKER_SECRET_KEYS` list in `lib/core/config/trigger-env-sync.ts`, read from the *same* +`/{env}/sim/env-vars` secret the app boots from. To give workers a new variable, add its key to +that list and set it in the secret. The repo still cannot see what else the dashboard holds. + +Two constraints on that list. `syncEnvVars` strips every `TRIGGER_`-prefixed key before it +publishes, so such a variable can only be set in the dashboard (`assertSyncableKeys` fails the +build rather than letting one look synced). And a key absent from the secret is left untouched +rather than blanked, so removing it from the secret does not remove it from a worker. So before replacing a worker's HTTP call to our own API with an in-process call, ask what env that work reads *on the app side*. Anything gated by a `require*Capability` helper is the sharp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 75d45ac5e46..1fd1c588b45 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -233,10 +233,23 @@ jobs: if: github.event_name == 'push' && github.ref == 'refs/heads/dev' runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-4vcpu-ubuntu-2404' || 'ubuntu-latest' }} timeout-minutes: 15 + permissions: + contents: read + id-token: write steps: - name: Checkout code uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + # syncEnvVars reads /dev/sim/env-vars from Secrets Manager during the + # build to publish the worker environment. Without these credentials the + # deploy still succeeds, but publishes only the environment-independent + # constants and leaves the worker env at whatever the previous deploy set. + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@e7f100cf4c008499ea8adda475de1042d6975c7b # v6 + with: + role-to-assume: ${{ secrets.DEV_AWS_ROLE_TO_ASSUME }} + aws-region: ${{ secrets.DEV_AWS_REGION }} + - name: Setup Bun uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 with: diff --git a/apps/sim/lib/core/config/trigger-env-sync.test.ts b/apps/sim/lib/core/config/trigger-env-sync.test.ts new file mode 100644 index 00000000000..634898c2ffe --- /dev/null +++ b/apps/sim/lib/core/config/trigger-env-sync.test.ts @@ -0,0 +1,130 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockFetchSecretMap } = vi.hoisted(() => ({ mockFetchSecretMap: vi.fn() })) + +vi.mock('@sim/runtime-secrets', () => ({ fetchSecretMap: mockFetchSecretMap })) + +import { + assertSyncableKeys, + resolveTriggerEnvVars, + SECRET_ID_BY_ENVIRONMENT, + WORKER_SECRET_KEYS, +} from '@/lib/core/config/trigger-env-sync' + +const CONSTANTS = ['DB_APP_NAME'] + +function byName(vars: { name: string; value: string; isSecret: boolean }[]) { + return new Map(vars.map((v) => [v.name, v])) +} + +describe('resolveTriggerEnvVars', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('reads the secret mapped to the environment', async () => { + mockFetchSecretMap.mockResolvedValue({}) + + await resolveTriggerEnvVars('prod') + + expect(mockFetchSecretMap).toHaveBeenCalledWith('/production/sim/env-vars') + }) + + it.each([ + ['prod', '/production/sim/env-vars'], + ['staging', '/staging/sim/env-vars'], + ['preview', '/dev/sim/env-vars'], + ])('maps %s to %s', (environment, secretId) => { + expect(SECRET_ID_BY_ENVIRONMENT[environment]).toBe(secretId) + }) + + it('publishes the constants plus every key present in the secret', async () => { + const secret = Object.fromEntries(WORKER_SECRET_KEYS.map(({ name }) => [name, `${name}-value`])) + const resolved = byName(await resolveTriggerEnvVars('staging', async () => secret)) + + for (const key of CONSTANTS) expect(resolved.has(key)).toBe(true) + for (const { name } of WORKER_SECRET_KEYS) { + expect(resolved.get(name)?.value).toBe(`${name}-value`) + } + expect(resolved.size).toBe(CONSTANTS.length + WORKER_SECRET_KEYS.length) + }) + + it('carries the secret flag through from the key table', async () => { + const resolved = byName( + await resolveTriggerEnvVars('staging', async () => ({ + REDIS_URL: 'redis://host', + SANDBOX_PROVIDER: 'e2b', + })) + ) + + expect(resolved.get('REDIS_URL')?.isSecret).toBe(true) + expect(resolved.get('SANDBOX_PROVIDER')?.isSecret).toBe(false) + expect(resolved.get('DB_APP_NAME')?.isSecret).toBe(false) + }) + + it('omits keys the secret does not carry, and keeps the ones it does', async () => { + const resolved = byName( + await resolveTriggerEnvVars('staging', async () => ({ REDIS_URL: 'redis://host' })) + ) + + expect(resolved.get('REDIS_URL')?.value).toBe('redis://host') + expect(resolved.has('E2B_API_KEY')).toBe(false) + expect(resolved.size).toBe(CONSTANTS.length + 1) + }) + + it('treats an empty value as unset so it cannot blank a working dashboard value', async () => { + const resolved = byName( + await resolveTriggerEnvVars('staging', async () => ({ REDIS_URL: '', E2B_API_KEY: null })) + ) + + expect(resolved.has('REDIS_URL')).toBe(false) + expect(resolved.has('E2B_API_KEY')).toBe(false) + }) + + it('serializes a non-string secret entry', async () => { + const resolved = byName( + await resolveTriggerEnvVars('staging', async () => ({ E2B_ENABLED: true })) + ) + + expect(resolved.get('E2B_ENABLED')?.value).toBe('true') + }) + + it('publishes constants only for an unmapped environment, without reading a secret', async () => { + const loadSecret = vi.fn() + + const resolved = await resolveTriggerEnvVars('dev', loadSecret) + + expect(loadSecret).not.toHaveBeenCalled() + expect(resolved.map((v) => v.name)).toEqual(CONSTANTS) + }) + + it('falls back to constants instead of rejecting when the secret cannot be read', async () => { + const resolved = await resolveTriggerEnvVars('prod', async () => { + throw new Error('AccessDeniedException') + }) + + expect(resolved.map((v) => v.name)).toEqual(CONSTANTS) + }) + + it('publishes no TRIGGER_-prefixed key, which the sync layer would strip silently', async () => { + const resolved = await resolveTriggerEnvVars('staging', async () => ({ + REDIS_URL: 'redis://host', + })) + + expect(resolved.filter((v) => v.name.startsWith('TRIGGER_'))).toEqual([]) + expect(WORKER_SECRET_KEYS.filter(({ name }) => name.startsWith('TRIGGER_'))).toEqual([]) + }) + + it('rejects a key the sync layer would strip, naming it', () => { + expect(() => assertSyncableKeys(['DB_APP_NAME', 'TRIGGER_DEV_ENABLED'])).toThrow( + /TRIGGER_DEV_ENABLED/ + ) + }) + + it('accepts the keys this module actually publishes', () => { + expect(() => assertSyncableKeys()).not.toThrow() + }) +}) diff --git a/apps/sim/lib/core/config/trigger-env-sync.ts b/apps/sim/lib/core/config/trigger-env-sync.ts new file mode 100644 index 00000000000..12ab72c8467 --- /dev/null +++ b/apps/sim/lib/core/config/trigger-env-sync.ts @@ -0,0 +1,184 @@ +import { createLogger } from '@sim/logger' +import { fetchSecretMap } from '@sim/runtime-secrets' +import { getErrorMessage } from '@sim/utils/errors' + +const logger = createLogger('TriggerEnvSync') + +/** + * One variable to publish into the Trigger.dev environment being deployed. + * Mirrors the shape `syncEnvVars` accepts. + */ +export interface SyncedEnvVar { + name: string + value: string + isSecret: boolean +} + +/** + * Values that are the same in every environment, so they need no secret lookup + * and are published even when the lookup fails. + * + * Nothing here may start with `TRIGGER_`: `syncEnvVars` drops every key with + * that prefix before it builds its layer, so such an entry looks published and + * never is. `TRIGGER_DEV_ENABLED` used to sit in this list for exactly that + * reason and had no effect for the life of the config; workers that need it get + * it from the Trigger.dev dashboard, and run dispatch does not read it at all + * because the `init` hook in `trigger.config.ts` marks the run process directly. + */ +const CONSTANT_ENV: readonly SyncedEnvVar[] = [ + { name: 'DB_APP_NAME', value: 'sim-trigger', isSecret: false }, +] as const + +/** Prefix `syncEnvVars` strips from any key it is handed. */ +const UNSYNCABLE_PREFIX = 'TRIGGER_' + +/** + * Environment a run needs for sandboxed work. Function block runs and the + * document compiler share one provider selection, and the doc-template + * variables decide whether a run reads a generated document through the doc + * sandbox's artifact store or the isolated-vm fallback. The app authors + * documents for whichever compiler it sees, so a worker missing the doc + * template falls back to isolated-vm and tries to run Python or Node-style + * sources as sandbox JavaScript. Reading a generated document under the doc + * sandbox means loading its compiled artifact from the copilot storage + * context, so that bucket has to be visible to the run as well. + * + * To give workers a new variable, add its key here and set it in the + * `/{env}/sim/env-vars` secret. The next deploy publishes it; nothing has to be + * entered in the Trigger.dev dashboard by hand. + */ +export const WORKER_SECRET_KEYS: readonly { name: string; secret: boolean }[] = [ + { name: 'REDIS_URL', secret: true }, + { name: 'REDIS_TLS_SERVERNAME', secret: false }, + { name: 'SANDBOX_PROVIDER', secret: false }, + { name: 'E2B_ENABLED', secret: false }, + { name: 'E2B_API_KEY', secret: true }, + { name: 'E2B_FUNCTION_TEMPLATE_ID', secret: false }, + { name: 'E2B_FUNCTION_TEMPLATE_GENERATION', secret: false }, + { name: 'MOTHERSHIP_E2B_DOC_TEMPLATE_ID', secret: false }, + { name: 'DAYTONA_API_KEY', secret: true }, + { name: 'DAYTONA_FUNCTION_SNAPSHOT_ID', secret: false }, + { name: 'DAYTONA_DOC_SNAPSHOT_ID', secret: false }, + { name: 'S3_COPILOT_BUCKET_NAME', secret: false }, + { name: 'AZURE_STORAGE_COPILOT_CONTAINER_NAME', secret: false }, + { name: 'GCS_COPILOT_BUCKET_NAME', secret: false }, +] as const + +/** + * Secret backing each Trigger.dev deploy target. `preview` is the `dev-sim` + * branch CI deploys from the `dev` branch, so it reads the dev environment's + * secret. A target absent from this map gets the constants and nothing else — + * guessing a secret would risk publishing one environment's credentials into + * another, which `syncEnvVars` would then apply with `override: true`. + */ +export const SECRET_ID_BY_ENVIRONMENT: Readonly> = { + prod: '/production/sim/env-vars', + staging: '/staging/sim/env-vars', + preview: '/dev/sim/env-vars', +} as const + +/** + * Resolves the variables to publish into one Trigger.dev environment, reading + * them from the same Secrets Manager entry the app container boots from, so the + * two runtimes cannot drift. + * + * Never throws. `syncEnvVars` swallows a callback rejection and then publishes + * *nothing*, which would silently drop the constants too, so a failed or + * incomplete lookup degrades to publishing what is known rather than aborting. + * Keys the secret does not carry are logged by name and simply not published — + * several are optional (Azure and GCS buckets on an S3 deployment, for one), so + * their absence is normal rather than an error, and leaving them out preserves + * whatever the Trigger.dev environment already held. + * + * @param environment Trigger.dev deploy target (`prod`, `staging`, `preview`). + * @param loadSecret Secret reader, injectable for tests. + */ +export async function resolveTriggerEnvVars( + environment: string, + loadSecret: (secretId: string) => Promise> = fetchSecretMap +): Promise { + const secretId = SECRET_ID_BY_ENVIRONMENT[environment] + if (!secretId) { + logger.warn( + `No secret mapped for Trigger.dev environment "${environment}"; publishing constants only` + ) + return [...CONSTANT_ENV] + } + + let entries: Record + try { + entries = await loadSecret(secretId) + } catch (error) { + logger.error( + `Failed to read ${secretId}; publishing constants only. Worker env is unchanged from the previous deploy.`, + { error: getErrorMessage(error) } + ) + return [...CONSTANT_ENV] + } + + const resolved: SyncedEnvVar[] = [...CONSTANT_ENV] + const missing: string[] = [] + + for (const { name, secret } of WORKER_SECRET_KEYS) { + const value = normalizeSecretValue(entries[name]) + if (value === undefined) { + missing.push(name) + continue + } + resolved.push({ name, value, isSecret: secret }) + } + + if (missing.length > 0) { + logger.info( + `${missing.length} worker env var(s) not set in ${secretId}; they are left untouched in Trigger.dev`, + { missing } + ) + } + + logger.info('Resolved Trigger.dev env vars', { + environment, + secretId, + published: resolved.length, + missing: missing.length, + }) + + return resolved +} + +/** + * Coerces a secret entry to an env var value, matching how container boot + * hydrates `process.env`. An absent or empty value is treated as unset so a + * blank secret entry cannot overwrite a working dashboard value with `''`. + */ +function normalizeSecretValue(value: unknown): string | undefined { + if (value === undefined || value === null) return undefined + const normalized = typeof value === 'string' ? value : JSON.stringify(value) + return normalized === '' ? undefined : normalized +} + +/** + * Guards the one silent failure this module cannot otherwise surface: a key the + * sync layer strips is indistinguishable, in the deploy log, from one it + * published. + * + * Called at module load rather than per resolve, so a key added to the wrong + * list fails config evaluation — and therefore the deploy and the test run — + * instead of reaching {@link resolveTriggerEnvVars}, whose contract is to + * degrade rather than throw. + */ +export function assertSyncableKeys( + names: readonly string[] = [ + ...CONSTANT_ENV.map((v) => v.name), + ...WORKER_SECRET_KEYS.map((k) => k.name), + ] +): void { + const stripped = names.filter((name) => name.startsWith(UNSYNCABLE_PREFIX)) + + if (stripped.length > 0) { + throw new Error( + `syncEnvVars strips ${UNSYNCABLE_PREFIX}-prefixed keys, so these can never reach a worker and must be set in the Trigger.dev dashboard: ${stripped.join(', ')}` + ) + } +} + +assertSyncableKeys() diff --git a/apps/sim/trigger.config.ts b/apps/sim/trigger.config.ts index fa4b4d76da3..cb65412db26 100644 --- a/apps/sim/trigger.config.ts +++ b/apps/sim/trigger.config.ts @@ -9,6 +9,7 @@ import { } from '@trigger.dev/build/extensions/core' import { defineConfig } from '@trigger.dev/sdk' import { env } from './lib/core/config/env' +import { resolveTriggerEnvVars } from './lib/core/config/trigger-env-sync' import { markInsideTriggerRun } from './lib/core/config/trigger-runtime' import { parseOtlpHeaders } from './lib/monitoring/otlp' @@ -26,43 +27,6 @@ if (grafanaConfigured && !grafanaFullyConfigured) { ) } -/** - * Environment a run needs for sandboxed work. Function block runs and the - * document compiler share one provider selection, and the doc-template - * variables decide whether a run reads a generated document through the doc - * sandbox's artifact store or the isolated-vm fallback. The app authors - * documents for whichever compiler it sees, so a worker missing the doc - * template falls back to isolated-vm and tries to run Python or Node-style - * sources as sandbox JavaScript. Reading a generated document under the doc - * sandbox means loading its compiled artifact from the copilot storage - * context, so that bucket has to be visible to the run as well. The values - * still have to exist in the Trigger.dev environment; syncing only keeps the - * worker's view of them aligned with the app's. - */ -const FUNCTION_EXECUTION_ENV = [ - { name: 'REDIS_URL', secret: true }, - { name: 'REDIS_TLS_SERVERNAME', secret: false }, - { name: 'SANDBOX_PROVIDER', secret: false }, - { name: 'E2B_ENABLED', secret: false }, - { name: 'E2B_API_KEY', secret: true }, - { name: 'E2B_FUNCTION_TEMPLATE_ID', secret: false }, - { name: 'E2B_FUNCTION_TEMPLATE_GENERATION', secret: false }, - { name: 'MOTHERSHIP_E2B_DOC_TEMPLATE_ID', secret: false }, - { name: 'DAYTONA_API_KEY', secret: true }, - { name: 'DAYTONA_FUNCTION_SNAPSHOT_ID', secret: false }, - { name: 'DAYTONA_DOC_SNAPSHOT_ID', secret: false }, - { name: 'S3_COPILOT_BUCKET_NAME', secret: false }, - { name: 'AZURE_STORAGE_COPILOT_CONTAINER_NAME', secret: false }, - { name: 'GCS_COPILOT_BUCKET_NAME', secret: false }, -] as const - -function getFunctionExecutionEnvVars() { - return FUNCTION_EXECUTION_ENV.flatMap(({ name, secret }) => { - const value = env[name] - return value ? [{ name, value, isSecret: secret }] : [] - }) -} - const grafanaTelemetry = grafanaFullyConfigured ? (() => { const baseUrl = grafanaEndpoint!.replace(/\/+$/, '') @@ -125,17 +89,19 @@ export default defineConfig({ 'pdfjs-dist', ], extensions: [ - syncEnvVars(() => [ - { name: 'DB_APP_NAME', value: 'sim-trigger' }, - /** - * Workers run Trigger.dev by definition, but the flag saying so was only - * set on the app container. Syncing it keeps the deployment flag honest - * inside runs; the dispatch decision itself no longer depends on it, - * because the `init` hook above marks the run process directly. - */ - { name: 'TRIGGER_DEV_ENABLED', value: 'TRUE' }, - ...getFunctionExecutionEnvVars(), - ]), + /** + * Publishes the worker environment from the same `/{env}/sim/env-vars` + * secret the app container boots from, so the two runtimes cannot drift + * and a new worker variable never has to be typed into the Trigger.dev + * dashboard. Reads Secrets Manager with the build's own AWS credentials — + * on the GitHub integration those arrive as `TRIGGER_BUILD_AWS_*`. + * + * This deliberately does not read the build machine's `process.env`: that + * made every value depend on who ran the deploy, so a local `deploy --env + * prod` would have published the developer's own `.env` into production + * (`syncEnvVars` applies its layer with `override: true`). + */ + syncEnvVars(({ environment }) => resolveTriggerEnvVars(environment)), additionalFiles({ files: [ './lib/execution/isolated-vm-worker.cjs', diff --git a/packages/runtime-secrets/src/index.test.ts b/packages/runtime-secrets/src/index.test.ts index dfd5cec1f4b..0f9dc6ca6e4 100644 --- a/packages/runtime-secrets/src/index.test.ts +++ b/packages/runtime-secrets/src/index.test.ts @@ -19,7 +19,7 @@ vi.mock('@sim/utils/helpers', () => ({ sleep: vi.fn().mockResolvedValue(undefined), })) -import { loadRuntimeSecrets } from './index' +import { fetchSecretMap, loadRuntimeSecrets } from './index' const TOUCHED = ['SIM_ENV_SECRET_ID', 'FOO', 'BAZ'] as const @@ -89,3 +89,30 @@ describe('loadRuntimeSecrets', () => { expect(mockSend).toHaveBeenCalledTimes(3) }) }) + +describe('fetchSecretMap', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('returns the parsed secret without touching process.env', async () => { + mockSend.mockResolvedValue({ SecretString: JSON.stringify({ FOO: 'bar' }) }) + + await expect(fetchSecretMap('/test/sim/env-vars')).resolves.toEqual({ FOO: 'bar' }) + expect(process.env.FOO).toBeUndefined() + }) + + it('requests the secret id it was given', async () => { + mockSend.mockResolvedValue({ SecretString: '{}' }) + + await fetchSecretMap('/production/sim/env-vars') + + expect(mockSend.mock.calls[0][0].input).toEqual({ SecretId: '/production/sim/env-vars' }) + }) + + it('throws when the secret JSON is not an object', async () => { + mockSend.mockResolvedValue({ SecretString: JSON.stringify(['a']) }) + + await expect(fetchSecretMap('/test/sim/env-vars')).rejects.toThrow(/must be a JSON object/) + }) +}) diff --git a/packages/runtime-secrets/src/index.ts b/packages/runtime-secrets/src/index.ts index 86c79e7952d..814d19157cb 100644 --- a/packages/runtime-secrets/src/index.ts +++ b/packages/runtime-secrets/src/index.ts @@ -15,6 +15,23 @@ const MAX_ATTEMPTS = 3 /** Bounds each Secrets Manager request so a stalled response can't hang boot. */ const REQUEST_TIMEOUT_MS = 5000 +/** + * Reads one `/{env}/sim/env-vars`-shaped secret and returns its parsed + * key/value map. Retries transient failures and throws on a fetch, parse, or + * shape failure so no caller can proceed on a partial view of its config. + * + * Shared by container boot ({@link loadRuntimeSecrets}) and by the Trigger.dev + * deploy-time env sync, so both runtimes resolve their configuration from the + * same secret through the same code path. + */ +export async function fetchSecretMap(secretId: string): Promise> { + const client = new SecretsManagerClient( + process.env.AWS_REGION ? { region: process.env.AWS_REGION } : {} + ) + + return parseSecretJson(await fetchSecretString(client, secretId)) +} + /** * Fetches the combined `/{env}/sim/env-vars` secret once at container boot and * hydrates `process.env`, so secrets no longer have to be fanned out into the @@ -33,12 +50,7 @@ export async function loadRuntimeSecrets(): Promise { return } - const client = new SecretsManagerClient( - process.env.AWS_REGION ? { region: process.env.AWS_REGION } : {} - ) - - const secretString = await fetchSecretString(client, secretId) - const entries = parseSecretJson(secretString) + const entries = await fetchSecretMap(secretId) let loaded = 0 let skipped = 0 From 5453ba0ce33f6cd4659e9a1e05b04d5920e1d5a7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 5 Sep 2026 17:56:11 -0700 Subject: [PATCH 2/3] fix(trigger): clear worker vars the secret drops, and allow strict sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_017BBuD7nBqVURAah8t6jfND --- .../lib/core/config/trigger-env-sync.test.ts | 43 ++++++++-- apps/sim/lib/core/config/trigger-env-sync.ts | 80 ++++++++++++------- apps/sim/trigger.config.ts | 14 +++- 3 files changed, 98 insertions(+), 39 deletions(-) diff --git a/apps/sim/lib/core/config/trigger-env-sync.test.ts b/apps/sim/lib/core/config/trigger-env-sync.test.ts index 634898c2ffe..6423355bfca 100644 --- a/apps/sim/lib/core/config/trigger-env-sync.test.ts +++ b/apps/sim/lib/core/config/trigger-env-sync.test.ts @@ -1,7 +1,7 @@ /** * @vitest-environment node */ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { mockFetchSecretMap } = vi.hoisted(() => ({ mockFetchSecretMap: vi.fn() })) @@ -11,6 +11,7 @@ import { assertSyncableKeys, resolveTriggerEnvVars, SECRET_ID_BY_ENVIRONMENT, + TriggerEnvSyncUnavailableError, WORKER_SECRET_KEYS, } from '@/lib/core/config/trigger-env-sync' @@ -23,6 +24,11 @@ function byName(vars: { name: string; value: string; isSecret: boolean }[]) { describe('resolveTriggerEnvVars', () => { beforeEach(() => { vi.clearAllMocks() + process.env.SIM_TRIGGER_ENV_SYNC_REQUIRED = undefined + }) + + afterEach(() => { + process.env.SIM_TRIGGER_ENV_SYNC_REQUIRED = undefined }) it('reads the secret mapped to the environment', async () => { @@ -65,23 +71,46 @@ describe('resolveTriggerEnvVars', () => { expect(resolved.get('DB_APP_NAME')?.isSecret).toBe(false) }) - it('omits keys the secret does not carry, and keeps the ones it does', async () => { + it('clears a key the authoritative secret no longer carries, so a revoked credential cannot survive in the worker', async () => { const resolved = byName( await resolveTriggerEnvVars('staging', async () => ({ REDIS_URL: 'redis://host' })) ) expect(resolved.get('REDIS_URL')?.value).toBe('redis://host') - expect(resolved.has('E2B_API_KEY')).toBe(false) - expect(resolved.size).toBe(CONSTANTS.length + 1) + expect(resolved.get('E2B_API_KEY')?.value).toBe('') + expect(resolved.get('DAYTONA_API_KEY')?.value).toBe('') + expect(resolved.size).toBe(CONSTANTS.length + WORKER_SECRET_KEYS.length) }) - it('treats an empty value as unset so it cannot blank a working dashboard value', async () => { + it('clears a key blanked or nulled in the secret rather than leaving the old value', async () => { const resolved = byName( await resolveTriggerEnvVars('staging', async () => ({ REDIS_URL: '', E2B_API_KEY: null })) ) - expect(resolved.has('REDIS_URL')).toBe(false) - expect(resolved.has('E2B_API_KEY')).toBe(false) + expect(resolved.get('REDIS_URL')?.value).toBe('') + expect(resolved.get('E2B_API_KEY')?.value).toBe('') + }) + + it('clears nothing when the secret could not be read, having no authority to clear against', async () => { + const resolved = await resolveTriggerEnvVars('prod', async () => { + throw new Error('AccessDeniedException') + }) + + expect(resolved.map((v) => v.name)).toEqual(CONSTANTS) + }) + + it('fails the resolve instead of publishing a partial env when sync is required', async () => { + process.env.SIM_TRIGGER_ENV_SYNC_REQUIRED = '1' + + await expect( + resolveTriggerEnvVars('prod', async () => { + throw new Error('AccessDeniedException') + }) + ).rejects.toBeInstanceOf(TriggerEnvSyncUnavailableError) + + await expect(resolveTriggerEnvVars('dev', vi.fn())).rejects.toBeInstanceOf( + TriggerEnvSyncUnavailableError + ) }) it('serializes a non-string secret entry', async () => { diff --git a/apps/sim/lib/core/config/trigger-env-sync.ts b/apps/sim/lib/core/config/trigger-env-sync.ts index 12ab72c8467..f9984f1df4c 100644 --- a/apps/sim/lib/core/config/trigger-env-sync.ts +++ b/apps/sim/lib/core/config/trigger-env-sync.ts @@ -64,6 +64,18 @@ export const WORKER_SECRET_KEYS: readonly { name: string; secret: boolean }[] = { name: 'GCS_COPILOT_BUCKET_NAME', secret: false }, ] as const +/** + * Set on a build to make an unusable secret fail the deploy instead of leaving + * the worker environment as the previous deploy left it. Off by default so the + * rollout can land before the build credentials exist; turn it on once every + * deploy path can reach Secrets Manager, and a silent lookup failure becomes + * impossible from then on. + */ +const REQUIRED_ENV = 'SIM_TRIGGER_ENV_SYNC_REQUIRED' + +/** Thrown only when {@link REQUIRED_ENV} is set. See `trigger.config.ts`. */ +export class TriggerEnvSyncUnavailableError extends Error {} + /** * Secret backing each Trigger.dev deploy target. `preview` is the `dev-sim` * branch CI deploys from the `dev` branch, so it reads the dev environment's @@ -82,13 +94,19 @@ export const SECRET_ID_BY_ENVIRONMENT: Readonly> = { * them from the same Secrets Manager entry the app container boots from, so the * two runtimes cannot drift. * - * Never throws. `syncEnvVars` swallows a callback rejection and then publishes - * *nothing*, which would silently drop the constants too, so a failed or - * incomplete lookup degrades to publishing what is known rather than aborting. - * Keys the secret does not carry are logged by name and simply not published — - * several are optional (Azure and GCS buckets on an S3 deployment, for one), so - * their absence is normal rather than an error, and leaving them out preserves - * whatever the Trigger.dev environment already held. + * The secret is authoritative for every {@link WORKER_SECRET_KEYS} entry, so a + * key it does not carry is published as `''` rather than skipped. Skipping left + * the worker's previous value in place, which meant deleting a compromised + * `E2B_API_KEY` from the secret did not revoke it in the worker — the app + * stopped loading it while runs kept using it. Every one of these keys is read + * as a truthiness or `||` check (never `??`), so `''` behaves exactly as unset. + * + * Throws only when {@link REQUIRED_ENV} is set. Otherwise a failed or unmapped + * lookup degrades to the environment-independent constants: `syncEnvVars` + * swallows a callback rejection and then publishes *nothing*, so rejecting by + * default would drop the constants too and still not fail the deploy. Nothing + * is cleared on that path — without a successful read there is no authority to + * clear against, and the Trigger.dev environment is left exactly as it was. * * @param environment Trigger.dev deploy target (`prod`, `staging`, `preview`). * @param loadSecret Secret reader, injectable for tests. @@ -99,52 +117,52 @@ export async function resolveTriggerEnvVars( ): Promise { const secretId = SECRET_ID_BY_ENVIRONMENT[environment] if (!secretId) { - logger.warn( - `No secret mapped for Trigger.dev environment "${environment}"; publishing constants only` - ) - return [...CONSTANT_ENV] + return unavailable(`No secret is mapped for Trigger.dev environment "${environment}"`) } let entries: Record try { entries = await loadSecret(secretId) } catch (error) { - logger.error( - `Failed to read ${secretId}; publishing constants only. Worker env is unchanged from the previous deploy.`, - { error: getErrorMessage(error) } - ) - return [...CONSTANT_ENV] + return unavailable(`Failed to read ${secretId}: ${getErrorMessage(error)}`) } const resolved: SyncedEnvVar[] = [...CONSTANT_ENV] - const missing: string[] = [] + const cleared: string[] = [] for (const { name, secret } of WORKER_SECRET_KEYS) { const value = normalizeSecretValue(entries[name]) - if (value === undefined) { - missing.push(name) - continue - } - resolved.push({ name, value, isSecret: secret }) - } - - if (missing.length > 0) { - logger.info( - `${missing.length} worker env var(s) not set in ${secretId}; they are left untouched in Trigger.dev`, - { missing } - ) + if (value === undefined) cleared.push(name) + resolved.push({ name, value: value ?? '', isSecret: secret }) } logger.info('Resolved Trigger.dev env vars', { environment, secretId, - published: resolved.length, - missing: missing.length, + published: resolved.length - cleared.length, + cleared, }) return resolved } +/** + * Handles a resolve that has no authoritative view of the environment, honoring + * {@link REQUIRED_ENV}. + */ +function unavailable(reason: string): SyncedEnvVar[] { + if (process.env[REQUIRED_ENV]) { + throw new TriggerEnvSyncUnavailableError( + `${reason}. ${REQUIRED_ENV} is set, so this deploy must not publish a partial worker environment.` + ) + } + + logger.error( + `${reason}. Publishing constants only; the worker environment is unchanged from the previous deploy. Set ${REQUIRED_ENV} to fail the deploy instead.` + ) + return [...CONSTANT_ENV] +} + /** * Coerces a secret entry to an env var value, matching how container boot * hydrates `process.env`. An absent or empty value is treated as unset so a diff --git a/apps/sim/trigger.config.ts b/apps/sim/trigger.config.ts index cb65412db26..49f4c8ec6e0 100644 --- a/apps/sim/trigger.config.ts +++ b/apps/sim/trigger.config.ts @@ -2,6 +2,7 @@ import { OTLPLogExporter } from '@opentelemetry/exporter-logs-otlp-http' import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-http' import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http' import { resourceFromAttributes } from '@opentelemetry/resources' +import { getErrorMessage } from '@sim/utils/errors' import { additionalFiles, additionalPackages, @@ -101,7 +102,18 @@ export default defineConfig({ * prod` would have published the developer's own `.env` into production * (`syncEnvVars` applies its layer with `override: true`). */ - syncEnvVars(({ environment }) => resolveTriggerEnvVars(environment)), + syncEnvVars(async ({ environment }) => { + try { + return await resolveTriggerEnvVars(environment) + } catch (error) { + // `syncEnvVars` catches a rejected callback, warns, and lets the + // deploy continue having published nothing — so rejecting is not a + // way to fail. Only an explicit non-zero exit is, and this path is + // reached only when SIM_TRIGGER_ENV_SYNC_REQUIRED asked for it. + console.error(getErrorMessage(error)) + process.exit(1) + } + }), additionalFiles({ files: [ './lib/execution/isolated-vm-worker.cjs', From ec69c21ed073f49bccef72765000aef28b2607ee Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 5 Sep 2026 18:02:55 -0700 Subject: [PATCH 3/3] fix(trigger): use the shared logger and correct the clearing docs 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) Claude-Session: https://claude.ai/code/session_017BBuD7nBqVURAah8t6jfND --- .claude/rules/sim-architecture.md | 6 ++++-- apps/sim/lib/core/config/trigger-env-sync.test.ts | 9 ++------- apps/sim/lib/core/config/trigger-env-sync.ts | 6 ++++-- apps/sim/trigger.config.ts | 3 ++- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.claude/rules/sim-architecture.md b/.claude/rules/sim-architecture.md index 154ec469432..f144fb86876 100644 --- a/.claude/rules/sim-architecture.md +++ b/.claude/rules/sim-architecture.md @@ -69,8 +69,10 @@ that list and set it in the secret. The repo still cannot see what else the dash Two constraints on that list. `syncEnvVars` strips every `TRIGGER_`-prefixed key before it publishes, so such a variable can only be set in the dashboard (`assertSyncableKeys` fails the -build rather than letting one look synced). And a key absent from the secret is left untouched -rather than blanked, so removing it from the secret does not remove it from a worker. +build rather than letting one look synced). And the secret is authoritative: after a successful +read, a key it does not carry — absent, `null`, or blank — is published as `''`, so deleting a +credential from the secret revokes it in the worker too. Nothing is cleared when the read failed +or the environment is unmapped, since there is then no authoritative view to clear against. So before replacing a worker's HTTP call to our own API with an in-process call, ask what env that work reads *on the app side*. Anything gated by a `require*Capability` helper is the sharp diff --git a/apps/sim/lib/core/config/trigger-env-sync.test.ts b/apps/sim/lib/core/config/trigger-env-sync.test.ts index 6423355bfca..85673cac74e 100644 --- a/apps/sim/lib/core/config/trigger-env-sync.test.ts +++ b/apps/sim/lib/core/config/trigger-env-sync.test.ts @@ -1,7 +1,7 @@ /** * @vitest-environment node */ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockFetchSecretMap } = vi.hoisted(() => ({ mockFetchSecretMap: vi.fn() })) @@ -24,11 +24,6 @@ function byName(vars: { name: string; value: string; isSecret: boolean }[]) { describe('resolveTriggerEnvVars', () => { beforeEach(() => { vi.clearAllMocks() - process.env.SIM_TRIGGER_ENV_SYNC_REQUIRED = undefined - }) - - afterEach(() => { - process.env.SIM_TRIGGER_ENV_SYNC_REQUIRED = undefined }) it('reads the secret mapped to the environment', async () => { @@ -100,7 +95,7 @@ describe('resolveTriggerEnvVars', () => { }) it('fails the resolve instead of publishing a partial env when sync is required', async () => { - process.env.SIM_TRIGGER_ENV_SYNC_REQUIRED = '1' + vi.stubEnv('SIM_TRIGGER_ENV_SYNC_REQUIRED', '1') await expect( resolveTriggerEnvVars('prod', async () => { diff --git a/apps/sim/lib/core/config/trigger-env-sync.ts b/apps/sim/lib/core/config/trigger-env-sync.ts index f9984f1df4c..1f9ab247529 100644 --- a/apps/sim/lib/core/config/trigger-env-sync.ts +++ b/apps/sim/lib/core/config/trigger-env-sync.ts @@ -165,8 +165,10 @@ function unavailable(reason: string): SyncedEnvVar[] { /** * Coerces a secret entry to an env var value, matching how container boot - * hydrates `process.env`. An absent or empty value is treated as unset so a - * blank secret entry cannot overwrite a working dashboard value with `''`. + * hydrates `process.env`. `undefined` means the secret does not configure this + * key, which the caller publishes as `''` to clear any value a previous deploy + * left in the worker. A blank entry is reported the same way as an absent one, + * so blanking a key in the secret revokes it exactly like deleting it. */ function normalizeSecretValue(value: unknown): string | undefined { if (value === undefined || value === null) return undefined diff --git a/apps/sim/trigger.config.ts b/apps/sim/trigger.config.ts index 49f4c8ec6e0..188af0f68cc 100644 --- a/apps/sim/trigger.config.ts +++ b/apps/sim/trigger.config.ts @@ -2,6 +2,7 @@ import { OTLPLogExporter } from '@opentelemetry/exporter-logs-otlp-http' import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-http' import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http' import { resourceFromAttributes } from '@opentelemetry/resources' +import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { additionalFiles, @@ -110,7 +111,7 @@ export default defineConfig({ // deploy continue having published nothing — so rejecting is not a // way to fail. Only an explicit non-zero exit is, and this path is // reached only when SIM_TRIGGER_ENV_SYNC_REQUIRED asked for it. - console.error(getErrorMessage(error)) + createLogger('TriggerConfig').error(getErrorMessage(error)) process.exit(1) } }),