Skip to content

Commit 32cb270

Browse files
authored
fix(trigger): keep secret text out of env sync errors and refuse an unknown sync target (#9672)
* 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. * 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.
1 parent f279d9e commit 32cb270

7 files changed

Lines changed: 109 additions & 21 deletions

File tree

‎apps/sim/lib/core/utils/urls.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ export function getBaseUrl(): string {
7575
* Whether this process is a Trigger.dev worker rather than the app container.
7676
*
7777
* `TRIGGER_SECRET_KEY` and `TRIGGER_DEV_ENABLED` are both present on the app too
78-
* — it needs them to dispatch — so neither discriminates. `trigger.config.ts`
79-
* syncs exactly one worker-only marker, `DB_APP_NAME='sim-trigger'`, which is
80-
* what this reads.
78+
* — it needs them to dispatch — so neither discriminates. Every Trigger.dev
79+
* deploy sets `DB_APP_NAME='sim-trigger'` (`TRIGGER_CONSTANT_VARS` in
80+
* `@sim/runtime-secrets/trigger-env`), which the app never has, so this reads it.
8181
*/
8282
function isTriggerWorkerRuntime(): boolean {
8383
return getEnv('DB_APP_NAME') === 'sim-trigger'

‎apps/sim/trigger.config.ts‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { resourceFromAttributes } from '@opentelemetry/resources'
55
import { createLogger } from '@sim/logger'
66
import {
77
diffTriggerEnv,
8+
parseTriggerEnvSyncTargets,
89
resolveTriggerEnvFromAppSecret,
910
TRIGGER_CONSTANT_VARS,
1011
} from '@sim/runtime-secrets/trigger-env'
@@ -46,7 +47,9 @@ if (grafanaConfigured && !grafanaFullyConfigured) {
4647
* sandbox means loading its compiled artifact from the copilot storage
4748
* context, so that bucket has to be visible to the run as well. The values
4849
* still have to exist in the Trigger.dev environment; syncing only keeps the
49-
* worker's view of them aligned with the app's.
50+
* worker's view of them aligned with the app's. Only an environment outside
51+
* `TRIGGER_ENV_SYNC_TARGETS` takes this list; a listed one gets these keys from
52+
* the app's env-vars secret with everything else.
5053
*/
5154
const FUNCTION_EXECUTION_ENV = [
5255
{ name: 'REDIS_URL', secret: true },
@@ -72,19 +75,37 @@ function getFunctionExecutionEnvVars() {
7275
})
7376
}
7477

78+
const syncLogger = createLogger('TriggerEnvSync')
79+
80+
/**
81+
* The environments `TRIGGER_ENV_SYNC_TARGETS` lists. Exits on a list the deploy workflows would
82+
* not match: `syncEnvVars` swallows a thrown error, and a typo must not silently keep the old path.
83+
*/
84+
function syncTargets(): Set<string> {
85+
try {
86+
return parseTriggerEnvSyncTargets(env.TRIGGER_ENV_SYNC_TARGETS)
87+
} catch (error) {
88+
syncLogger.error('Invalid TRIGGER_ENV_SYNC_TARGETS; aborting the deploy', {
89+
error: getErrorMessage(error),
90+
})
91+
process.exit(1)
92+
}
93+
}
94+
7595
/**
7696
* Mirrors the app's env-vars secret into an environment listed in
7797
* `TRIGGER_ENV_SYNC_TARGETS`, logging only counts, never names or values (CI
78-
* logs are public). Exits on failure: `syncEnvVars` catches a thrown error and
79-
* deploys without syncing, and a failed deploy leaves the running version and
80-
* its environment untouched where a silent skip would keep stale configuration.
98+
* logs are public). The CLI applies the variables when it uploads the version,
99+
* before promotion, and only adds or updates: a key removed from the secret
100+
* stays in Trigger.dev until it is deleted there. Exits on failure, because
101+
* `syncEnvVars` catches a thrown error and deploys without syncing, which would
102+
* keep stale configuration.
81103
*/
82104
async function syncFromAppSecret(environment: string, deployedEnv: Record<string, string>) {
83-
const logger = createLogger('TriggerEnvSync')
84105
try {
85106
const { secretId, vars } = await resolveTriggerEnvFromAppSecret(environment)
86107
const { added, changed } = diffTriggerEnv(vars, deployedEnv)
87-
logger.info('Syncing the Trigger.dev environment from the app env-vars secret', {
108+
syncLogger.info('Syncing the Trigger.dev environment from the app env-vars secret', {
88109
environment,
89110
secretId,
90111
total: vars.length,
@@ -93,7 +114,7 @@ async function syncFromAppSecret(environment: string, deployedEnv: Record<string
93114
})
94115
return vars
95116
} catch (error) {
96-
logger.error('Trigger.dev env sync failed; aborting the deploy', {
117+
syncLogger.error('Trigger.dev env sync failed; aborting the deploy', {
97118
environment,
98119
error: getErrorMessage(error),
99120
})
@@ -233,11 +254,13 @@ export default defineConfig({
233254
],
234255
extensions: [
235256
nodeBinary(),
236-
syncEnvVars(({ environment, env: deployedEnv }) =>
237-
env.TRIGGER_ENV_SYNC_TARGETS?.split(',').includes(environment)
238-
? syncFromAppSecret(environment, deployedEnv)
239-
: [...TRIGGER_CONSTANT_VARS, ...getFunctionExecutionEnvVars()]
240-
),
257+
syncEnvVars(({ environment, env: deployedEnv }) => {
258+
if (syncTargets().has(environment)) return syncFromAppSecret(environment, deployedEnv)
259+
syncLogger.info('Syncing only the worker constants and function execution settings', {
260+
environment,
261+
})
262+
return [...TRIGGER_CONSTANT_VARS, ...getFunctionExecutionEnvVars()]
263+
}),
241264
additionalFiles({
242265
files: [
243266
'./lib/execution/isolated-vm-worker.cjs',

‎packages/runtime-secrets/src/index.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,17 @@ describe('loadRuntimeSecrets', () => {
5050
await expect(loadRuntimeSecrets()).rejects.toThrow(/binary secrets/)
5151
expect(mockSend).toHaveBeenCalledTimes(1)
5252
})
53+
54+
it('keeps the secret text out of a malformed-JSON error, which deploy logs print', async () => {
55+
process.env.SIM_ENV_SECRET_ID = '/test/sim/env-vars'
56+
mockSend.mockResolvedValue({ SecretString: '{"FOO": postgres://user:hunter2@db/sim}' })
57+
58+
const error = await loadRuntimeSecrets().then(
59+
() => undefined,
60+
(caught: unknown) => caught
61+
)
62+
expect(error).toBeInstanceOf(Error)
63+
expect((error as Error).message).toMatch(/not valid JSON/)
64+
expect((error as Error).message).not.toMatch(/postgres|hunter2|FOO/)
65+
})
5366
})

‎packages/runtime-secrets/src/index.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ export async function loadRuntimeSecrets(): Promise<void> {
5252
/**
5353
* Reads one env-vars JSON secret as the string map a process would see: the
5454
* same values {@link loadRuntimeSecrets} writes into `process.env`, with
55-
* non-string JSON values serialized. Retries transient failures and throws on
56-
* anything else, so a caller never proceeds on a partial or missing secret.
55+
* non-string JSON values serialized. Retries a failed request up to three
56+
* times, then throws, so a caller never proceeds on a partial or missing secret.
5757
*/
5858
export async function fetchRuntimeSecrets(secretId: string): Promise<Record<string, string>> {
5959
const client = new SecretsManagerClient(
@@ -106,8 +106,9 @@ function parseSecretJson(secretString: string): Record<string, unknown> {
106106
let parsed: unknown
107107
try {
108108
parsed = JSON.parse(secretString)
109-
} catch (error) {
110-
throw new Error(`Runtime secret is not valid JSON: ${getErrorMessage(error)}`)
109+
} catch {
110+
// The parser's message quotes the text around the fault, which can be part of a value.
111+
throw new Error('Runtime secret is not valid JSON')
111112
}
112113
if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
113114
throw new Error('Runtime secret must be a JSON object of key/value pairs')
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { parseTriggerEnvSyncTargets } from './trigger-env'
3+
4+
describe('parseTriggerEnvSyncTargets', () => {
5+
it('reads an exact comma list', () => {
6+
expect([...parseTriggerEnvSyncTargets('preview,staging,prod')].sort()).toEqual([
7+
'preview',
8+
'prod',
9+
'staging',
10+
])
11+
})
12+
13+
it('treats an unset or empty gate as no targets', () => {
14+
expect(parseTriggerEnvSyncTargets(undefined).size).toBe(0)
15+
expect(parseTriggerEnvSyncTargets('').size).toBe(0)
16+
})
17+
18+
it.each([
19+
'preview, staging',
20+
'preview,,staging',
21+
'preview,',
22+
' ',
23+
'preview,Staging',
24+
'preview,production',
25+
'constructor',
26+
'preview,toString',
27+
])('refuses %j, which the deploy workflows would not grant credentials for', (value) => {
28+
expect(() => parseTriggerEnvSyncTargets(value)).toThrow(/TRIGGER_ENV_SYNC_TARGETS/)
29+
})
30+
})

‎packages/runtime-secrets/src/trigger-env.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,30 @@ export function diffTriggerEnv(vars: TriggerEnvVar[], current: Record<string, st
7474
}
7575
}
7676

77+
/**
78+
* Parses the `TRIGGER_ENV_SYNC_TARGETS` repository variable: an exact comma list of Trigger.dev
79+
* environment slugs, such as `preview,staging,prod`. The deploy workflows decide whether to grant
80+
* the secret-read credentials by matching that same exact list, so an entry they would not match
81+
* (padded, empty, or a slug with no mapped secret) fails the deploy here rather than syncing
82+
* without credentials or silently keeping the environment on its old configuration.
83+
*/
84+
export function parseTriggerEnvSyncTargets(value: string | undefined): Set<string> {
85+
if (!value) return new Set()
86+
const targets = value.split(',')
87+
const invalid = targets.filter((target) => !Object.hasOwn(APP_ENV_SECRET_IDS, target))
88+
if (invalid.length > 0) {
89+
throw new Error(
90+
`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(', ')}`
91+
)
92+
}
93+
return new Set(targets)
94+
}
95+
7796
/** Reads the app env-vars secret mapped to `environment` and builds its Trigger.dev variables. */
7897
export async function resolveTriggerEnvFromAppSecret(environment: string) {
79-
const secretId = APP_ENV_SECRET_IDS[environment]
98+
const secretId = Object.hasOwn(APP_ENV_SECRET_IDS, environment)
99+
? APP_ENV_SECRET_IDS[environment]
100+
: undefined
80101
if (!secretId) {
81102
throw new Error(`No app env-vars secret is mapped to Trigger.dev environment '${environment}'`)
82103
}

‎packages/testing/src/mocks/urls.mock.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ function getInternalApiBaseUrlImpl(): string {
4949
const internalBaseUrl = readEnv('INTERNAL_API_BASE_URL')?.trim()
5050
// Mirrors the real module: the internal URL names a route that resolves only
5151
// from inside the app container, so a Trigger.dev worker must ignore it.
52-
// `DB_APP_NAME='sim-trigger'` is the worker-only marker trigger.config.ts syncs.
52+
// `DB_APP_NAME='sim-trigger'` is the worker-only marker every Trigger.dev deploy sets.
5353
if (!internalBaseUrl || readEnv('DB_APP_NAME') === 'sim-trigger') return getBaseUrlImpl()
5454
if (!hasHttpProtocol(internalBaseUrl)) {
5555
throw new Error(

0 commit comments

Comments
 (0)