fix(trigger): clear worker vars the secret drops, and allow strict sync

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

Author: waleedlatif1

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 () => {

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<Record<string, string>> = {
  * 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<SyncedEnvVar[]> {
   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<string, unknown>
   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

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',