From 2e814eb10f042608b5b9d02eeffa5b09e179570e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 4 Nov 2026 02:37:10 -0800 Subject: [PATCH 1/2] fix(trigger): keep secret text out of env sync errors and refuse an unknown sync target A malformed env-vars secret failed with the JSON parser's message, which quotes the text around the fault, and the Trigger.dev deploy prints that error to a public CI log; the error now names only the failure. TRIGGER_ENV_SYNC_TARGETS was split on commas without trimming or validation, so a typo or a space silently kept an environment on its old configuration; it is now parsed by parseTriggerEnvSyncTargets, which trims entries and refuses a name with no mapped secret (by own key, so prototype names cannot pass), and the deploy logs which path it took. Comments now say the CLI applies synced variables when it uploads the version and that the sync never deletes, describe the retry as every failed request, and point the worker marker at TRIGGER_CONSTANT_VARS. --- apps/sim/lib/core/utils/urls.ts | 6 +-- apps/sim/trigger.config.ts | 47 ++++++++++++++----- packages/runtime-secrets/src/index.test.ts | 13 +++++ packages/runtime-secrets/src/index.ts | 9 ++-- .../runtime-secrets/src/trigger-env.test.ts | 24 ++++++++++ packages/runtime-secrets/src/trigger-env.ts | 23 ++++++++- packages/testing/src/mocks/urls.mock.ts | 2 +- 7 files changed, 103 insertions(+), 21 deletions(-) create mode 100644 packages/runtime-secrets/src/trigger-env.test.ts diff --git a/apps/sim/lib/core/utils/urls.ts b/apps/sim/lib/core/utils/urls.ts index 6cea8fdd024..7283966aa82 100644 --- a/apps/sim/lib/core/utils/urls.ts +++ b/apps/sim/lib/core/utils/urls.ts @@ -75,9 +75,9 @@ export function getBaseUrl(): string { * Whether this process is a Trigger.dev worker rather than the app container. * * `TRIGGER_SECRET_KEY` and `TRIGGER_DEV_ENABLED` are both present on the app too - * — it needs them to dispatch — so neither discriminates. `trigger.config.ts` - * syncs exactly one worker-only marker, `DB_APP_NAME='sim-trigger'`, which is - * what this reads. + * — it needs them to dispatch — so neither discriminates. Every Trigger.dev + * deploy sets `DB_APP_NAME='sim-trigger'` (`TRIGGER_CONSTANT_VARS` in + * `@sim/runtime-secrets/trigger-env`), which the app never has, so this reads it. */ function isTriggerWorkerRuntime(): boolean { return getEnv('DB_APP_NAME') === 'sim-trigger' diff --git a/apps/sim/trigger.config.ts b/apps/sim/trigger.config.ts index 9d4211ac54a..e7921d6f1f4 100644 --- a/apps/sim/trigger.config.ts +++ b/apps/sim/trigger.config.ts @@ -5,6 +5,7 @@ import { resourceFromAttributes } from '@opentelemetry/resources' import { createLogger } from '@sim/logger' import { diffTriggerEnv, + parseTriggerEnvSyncTargets, resolveTriggerEnvFromAppSecret, TRIGGER_CONSTANT_VARS, } from '@sim/runtime-secrets/trigger-env' @@ -46,7 +47,9 @@ if (grafanaConfigured && !grafanaFullyConfigured) { * 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. + * worker's view of them aligned with the app's. Only an environment outside + * `TRIGGER_ENV_SYNC_TARGETS` takes this list; a listed one gets these keys from + * the app's env-vars secret with everything else. */ const FUNCTION_EXECUTION_ENV = [ { name: 'REDIS_URL', secret: true }, @@ -72,19 +75,37 @@ function getFunctionExecutionEnvVars() { }) } +const syncLogger = createLogger('TriggerEnvSync') + +/** + * The environments `TRIGGER_ENV_SYNC_TARGETS` lists. Exits on a name with no mapped secret: + * `syncEnvVars` swallows a thrown error, and a typo must not silently keep the old path. + */ +function syncTargets(): Set { + try { + return parseTriggerEnvSyncTargets(env.TRIGGER_ENV_SYNC_TARGETS) + } catch (error) { + syncLogger.error('Invalid TRIGGER_ENV_SYNC_TARGETS; aborting the deploy', { + error: getErrorMessage(error), + }) + process.exit(1) + } +} + /** * Mirrors the app's env-vars secret into an environment listed in * `TRIGGER_ENV_SYNC_TARGETS`, logging only counts, never names or values (CI - * logs are public). Exits on failure: `syncEnvVars` catches a thrown error and - * deploys without syncing, and a failed deploy leaves the running version and - * its environment untouched where a silent skip would keep stale configuration. + * logs are public). The CLI applies the variables when it uploads the version, + * before promotion, and only adds or updates: a key removed from the secret + * stays in Trigger.dev until it is deleted there. Exits on failure, because + * `syncEnvVars` catches a thrown error and deploys without syncing, which would + * keep stale configuration. */ async function syncFromAppSecret(environment: string, deployedEnv: Record) { - const logger = createLogger('TriggerEnvSync') try { const { secretId, vars } = await resolveTriggerEnvFromAppSecret(environment) const { added, changed } = diffTriggerEnv(vars, deployedEnv) - logger.info('Syncing the Trigger.dev environment from the app env-vars secret', { + syncLogger.info('Syncing the Trigger.dev environment from the app env-vars secret', { environment, secretId, total: vars.length, @@ -93,7 +114,7 @@ async function syncFromAppSecret(environment: string, deployedEnv: Record - env.TRIGGER_ENV_SYNC_TARGETS?.split(',').includes(environment) - ? syncFromAppSecret(environment, deployedEnv) - : [...TRIGGER_CONSTANT_VARS, ...getFunctionExecutionEnvVars()] - ), + syncEnvVars(({ environment, env: deployedEnv }) => { + if (syncTargets().has(environment)) return syncFromAppSecret(environment, deployedEnv) + syncLogger.info('Syncing only the worker constants and function execution settings', { + environment, + }) + return [...TRIGGER_CONSTANT_VARS, ...getFunctionExecutionEnvVars()] + }), 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 3a7cc4f99a8..50b826588db 100644 --- a/packages/runtime-secrets/src/index.test.ts +++ b/packages/runtime-secrets/src/index.test.ts @@ -50,4 +50,17 @@ describe('loadRuntimeSecrets', () => { await expect(loadRuntimeSecrets()).rejects.toThrow(/binary secrets/) expect(mockSend).toHaveBeenCalledTimes(1) }) + + it('keeps the secret text out of a malformed-JSON error, which deploy logs print', async () => { + process.env.SIM_ENV_SECRET_ID = '/test/sim/env-vars' + mockSend.mockResolvedValue({ SecretString: '{"FOO": postgres://user:hunter2@db/sim}' }) + + const error = await loadRuntimeSecrets().then( + () => undefined, + (caught: unknown) => caught + ) + expect(error).toBeInstanceOf(Error) + expect((error as Error).message).toMatch(/not valid JSON/) + expect((error as Error).message).not.toMatch(/postgres|hunter2|FOO/) + }) }) diff --git a/packages/runtime-secrets/src/index.ts b/packages/runtime-secrets/src/index.ts index abd0dec51bf..c1119e69fe0 100644 --- a/packages/runtime-secrets/src/index.ts +++ b/packages/runtime-secrets/src/index.ts @@ -52,8 +52,8 @@ export async function loadRuntimeSecrets(): Promise { /** * Reads one env-vars JSON secret as the string map a process would see: the * same values {@link loadRuntimeSecrets} writes into `process.env`, with - * non-string JSON values serialized. Retries transient failures and throws on - * anything else, so a caller never proceeds on a partial or missing secret. + * non-string JSON values serialized. Retries a failed request up to three + * times, then throws, so a caller never proceeds on a partial or missing secret. */ export async function fetchRuntimeSecrets(secretId: string): Promise> { const client = new SecretsManagerClient( @@ -106,8 +106,9 @@ function parseSecretJson(secretString: string): Record { let parsed: unknown try { parsed = JSON.parse(secretString) - } catch (error) { - throw new Error(`Runtime secret is not valid JSON: ${getErrorMessage(error)}`) + } catch { + // The parser's message quotes the text around the fault, which can be part of a value. + throw new Error('Runtime secret is not valid JSON') } if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { throw new Error('Runtime secret must be a JSON object of key/value pairs') diff --git a/packages/runtime-secrets/src/trigger-env.test.ts b/packages/runtime-secrets/src/trigger-env.test.ts new file mode 100644 index 00000000000..6c004d4c605 --- /dev/null +++ b/packages/runtime-secrets/src/trigger-env.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest' +import { parseTriggerEnvSyncTargets } from './trigger-env' + +describe('parseTriggerEnvSyncTargets', () => { + it('reads a comma list with stray whitespace and empty entries', () => { + expect([...parseTriggerEnvSyncTargets(' preview, staging ,,prod ')].sort()).toEqual([ + 'preview', + 'prod', + 'staging', + ]) + }) + + it('treats an unset or blank gate as no targets', () => { + expect(parseTriggerEnvSyncTargets(undefined).size).toBe(0) + expect(parseTriggerEnvSyncTargets(' ').size).toBe(0) + }) + + it.each(['Staging', 'production', 'constructor', 'toString'])( + 'refuses %s, which no app env-vars secret is mapped to', + (target) => { + expect(() => parseTriggerEnvSyncTargets(`preview,${target}`)).toThrow(target) + } + ) +}) diff --git a/packages/runtime-secrets/src/trigger-env.ts b/packages/runtime-secrets/src/trigger-env.ts index 46bd1d31892..d7df53b7e40 100644 --- a/packages/runtime-secrets/src/trigger-env.ts +++ b/packages/runtime-secrets/src/trigger-env.ts @@ -74,9 +74,30 @@ export function diffTriggerEnv(vars: TriggerEnvVar[], current: Record { + const targets = (value ?? '') + .split(',') + .map((target) => target.trim()) + .filter(Boolean) + const unknown = targets.filter((target) => !Object.hasOwn(APP_ENV_SECRET_IDS, target)) + if (unknown.length > 0) { + throw new Error( + `TRIGGER_ENV_SYNC_TARGETS names environments with no app env-vars secret: ${unknown.join(', ')}` + ) + } + return new Set(targets) +} + /** Reads the app env-vars secret mapped to `environment` and builds its Trigger.dev variables. */ export async function resolveTriggerEnvFromAppSecret(environment: string) { - const secretId = APP_ENV_SECRET_IDS[environment] + const secretId = Object.hasOwn(APP_ENV_SECRET_IDS, environment) + ? APP_ENV_SECRET_IDS[environment] + : undefined if (!secretId) { throw new Error(`No app env-vars secret is mapped to Trigger.dev environment '${environment}'`) } diff --git a/packages/testing/src/mocks/urls.mock.ts b/packages/testing/src/mocks/urls.mock.ts index dc21eaad33a..51746a394c6 100644 --- a/packages/testing/src/mocks/urls.mock.ts +++ b/packages/testing/src/mocks/urls.mock.ts @@ -49,7 +49,7 @@ function getInternalApiBaseUrlImpl(): string { const internalBaseUrl = readEnv('INTERNAL_API_BASE_URL')?.trim() // Mirrors the real module: the internal URL names a route that resolves only // from inside the app container, so a Trigger.dev worker must ignore it. - // `DB_APP_NAME='sim-trigger'` is the worker-only marker trigger.config.ts syncs. + // `DB_APP_NAME='sim-trigger'` is the worker-only marker every Trigger.dev deploy sets. if (!internalBaseUrl || readEnv('DB_APP_NAME') === 'sim-trigger') return getBaseUrlImpl() if (!hasHttpProtocol(internalBaseUrl)) { throw new Error( From 44bb5dc2e93097f272b97c520da453576acd724b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 4 Nov 2026 02:51:29 -0800 Subject: [PATCH 2/2] fix(trigger): accept only the exact sync-target list the deploy workflows match The workflows grant the secret-read credentials by matching TRIGGER_ENV_SYNC_TARGETS as an exact comma list, so a trimmed parser could accept a padded entry the credential step skipped and fail inside the sync instead. The parser now refuses a padded, empty, or unknown entry with a message naming the expected format. --- apps/sim/trigger.config.ts | 4 +-- .../runtime-secrets/src/trigger-env.test.ts | 26 ++++++++++++------- packages/runtime-secrets/src/trigger-env.ts | 20 +++++++------- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/apps/sim/trigger.config.ts b/apps/sim/trigger.config.ts index e7921d6f1f4..e67d099e0fc 100644 --- a/apps/sim/trigger.config.ts +++ b/apps/sim/trigger.config.ts @@ -78,8 +78,8 @@ function getFunctionExecutionEnvVars() { const syncLogger = createLogger('TriggerEnvSync') /** - * The environments `TRIGGER_ENV_SYNC_TARGETS` lists. Exits on a name with no mapped secret: - * `syncEnvVars` swallows a thrown error, and a typo must not silently keep the old path. + * The environments `TRIGGER_ENV_SYNC_TARGETS` lists. Exits on a list the deploy workflows would + * not match: `syncEnvVars` swallows a thrown error, and a typo must not silently keep the old path. */ function syncTargets(): Set { try { diff --git a/packages/runtime-secrets/src/trigger-env.test.ts b/packages/runtime-secrets/src/trigger-env.test.ts index 6c004d4c605..14432c12dfa 100644 --- a/packages/runtime-secrets/src/trigger-env.test.ts +++ b/packages/runtime-secrets/src/trigger-env.test.ts @@ -2,23 +2,29 @@ import { describe, expect, it } from 'vitest' import { parseTriggerEnvSyncTargets } from './trigger-env' describe('parseTriggerEnvSyncTargets', () => { - it('reads a comma list with stray whitespace and empty entries', () => { - expect([...parseTriggerEnvSyncTargets(' preview, staging ,,prod ')].sort()).toEqual([ + it('reads an exact comma list', () => { + expect([...parseTriggerEnvSyncTargets('preview,staging,prod')].sort()).toEqual([ 'preview', 'prod', 'staging', ]) }) - it('treats an unset or blank gate as no targets', () => { + it('treats an unset or empty gate as no targets', () => { expect(parseTriggerEnvSyncTargets(undefined).size).toBe(0) - expect(parseTriggerEnvSyncTargets(' ').size).toBe(0) + expect(parseTriggerEnvSyncTargets('').size).toBe(0) }) - it.each(['Staging', 'production', 'constructor', 'toString'])( - 'refuses %s, which no app env-vars secret is mapped to', - (target) => { - expect(() => parseTriggerEnvSyncTargets(`preview,${target}`)).toThrow(target) - } - ) + it.each([ + 'preview, staging', + 'preview,,staging', + 'preview,', + ' ', + 'preview,Staging', + 'preview,production', + 'constructor', + 'preview,toString', + ])('refuses %j, which the deploy workflows would not grant credentials for', (value) => { + expect(() => parseTriggerEnvSyncTargets(value)).toThrow(/TRIGGER_ENV_SYNC_TARGETS/) + }) }) diff --git a/packages/runtime-secrets/src/trigger-env.ts b/packages/runtime-secrets/src/trigger-env.ts index d7df53b7e40..c85d274b201 100644 --- a/packages/runtime-secrets/src/trigger-env.ts +++ b/packages/runtime-secrets/src/trigger-env.ts @@ -75,19 +75,19 @@ export function diffTriggerEnv(vars: TriggerEnvVar[], current: Record { - const targets = (value ?? '') - .split(',') - .map((target) => target.trim()) - .filter(Boolean) - const unknown = targets.filter((target) => !Object.hasOwn(APP_ENV_SECRET_IDS, target)) - if (unknown.length > 0) { + if (!value) return new Set() + const targets = value.split(',') + const invalid = targets.filter((target) => !Object.hasOwn(APP_ENV_SECRET_IDS, target)) + if (invalid.length > 0) { throw new Error( - `TRIGGER_ENV_SYNC_TARGETS names environments with no app env-vars secret: ${unknown.join(', ')}` + `TRIGGER_ENV_SYNC_TARGETS must be a comma list of ${Object.keys(APP_ENV_SECRET_IDS).join(', ')} with no spaces; refused: ${invalid.map((target) => JSON.stringify(target)).join(', ')}` ) } return new Set(targets)