Skip to content

Commit 2e814eb

Browse files
committed
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.
1 parent 5e57170 commit 2e814eb

7 files changed

Lines changed: 103 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 name with no mapped secret:
82+
* `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: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { parseTriggerEnvSyncTargets } from './trigger-env'
3+
4+
describe('parseTriggerEnvSyncTargets', () => {
5+
it('reads a comma list with stray whitespace and empty entries', () => {
6+
expect([...parseTriggerEnvSyncTargets(' preview, staging ,,prod ')].sort()).toEqual([
7+
'preview',
8+
'prod',
9+
'staging',
10+
])
11+
})
12+
13+
it('treats an unset or blank gate as no targets', () => {
14+
expect(parseTriggerEnvSyncTargets(undefined).size).toBe(0)
15+
expect(parseTriggerEnvSyncTargets(' ').size).toBe(0)
16+
})
17+
18+
it.each(['Staging', 'production', 'constructor', 'toString'])(
19+
'refuses %s, which no app env-vars secret is mapped to',
20+
(target) => {
21+
expect(() => parseTriggerEnvSyncTargets(`preview,${target}`)).toThrow(target)
22+
}
23+
)
24+
})

‎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: a comma list of Trigger.dev
79+
* environment slugs. Refuses any slug without a mapped secret rather than ignoring it, so a typo
80+
* fails the deploy instead of silently keeping the environment on its old configuration.
81+
*/
82+
export function parseTriggerEnvSyncTargets(value: string | undefined): Set<string> {
83+
const targets = (value ?? '')
84+
.split(',')
85+
.map((target) => target.trim())
86+
.filter(Boolean)
87+
const unknown = targets.filter((target) => !Object.hasOwn(APP_ENV_SECRET_IDS, target))
88+
if (unknown.length > 0) {
89+
throw new Error(
90+
`TRIGGER_ENV_SYNC_TARGETS names environments with no app env-vars secret: ${unknown.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)