diff --git a/.server-changes/vercel-sync-reserved-env-vars.md b/.server-changes/vercel-sync-reserved-env-vars.md new file mode 100644 index 0000000000..554a5ace0a --- /dev/null +++ b/.server-changes/vercel-sync-reserved-env-vars.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Fix Vercel env var sync and onboarding preview leaking reserved `TRIGGER_*` keys. diff --git a/apps/webapp/app/models/vercelIntegration.server.ts b/apps/webapp/app/models/vercelIntegration.server.ts index 5acaea50af..b75446a806 100644 --- a/apps/webapp/app/models/vercelIntegration.server.ts +++ b/apps/webapp/app/models/vercelIntegration.server.ts @@ -27,6 +27,7 @@ import { envTypeToVercelTarget, } from "~/v3/vercel/vercelProjectIntegrationSchema"; import { EnvironmentVariablesRepository } from "~/v3/environmentVariables/environmentVariablesRepository.server"; +import { isReservedForExternalSync } from "~/v3/environmentVariableRules.server"; import { callVercelWithRecovery, wrapVercelCallWithRecovery, @@ -1350,10 +1351,9 @@ export class VercelIntegrationRepository { for (const mapping of envMapping) { const iterResult = await ResultAsync.fromPromise( (async () => { - // Build filter to avoid decrypting vars that will be filtered out anyway - const excludeKeys = new Set(["TRIGGER_SECRET_KEY", "TRIGGER_VERSION"]); + // Exclude reserved keys before decrypting (a reserved-only batch gets rejected). const shouldIncludeKey = (key: string) => - !excludeKeys.has(key) && + !isReservedForExternalSync(key) && shouldSyncEnvVar(params.syncEnvVarsMapping, key, mapping.triggerEnvType as TriggerEnvironmentType); const envVarsResult = await this.getVercelEnvironmentVariableValues( @@ -1399,7 +1399,7 @@ export class VercelIntegrationRepository { if (envVar.isSecret) { return false; } - if (envVar.key === "TRIGGER_SECRET_KEY" || envVar.key === "TRIGGER_VERSION") { + if (isReservedForExternalSync(envVar.key)) { return false; } return shouldSyncEnvVar( diff --git a/apps/webapp/app/presenters/v3/VercelSettingsPresenter.server.ts b/apps/webapp/app/presenters/v3/VercelSettingsPresenter.server.ts index ea2462816e..39c542871a 100644 --- a/apps/webapp/app/presenters/v3/VercelSettingsPresenter.server.ts +++ b/apps/webapp/app/presenters/v3/VercelSettingsPresenter.server.ts @@ -10,6 +10,7 @@ import { } from "~/models/vercelIntegration.server"; import { type GitHubAppInstallation } from "~/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github"; import { EnvironmentVariablesRepository } from "~/v3/environmentVariables/environmentVariablesRepository.server"; +import { isReservedForExternalSync } from "~/v3/environmentVariableRules.server"; import { VercelProjectIntegrationDataSchema, VercelProjectIntegrationData, @@ -567,12 +568,11 @@ export class VercelSettingsPresenter extends BasePresenter { const projectEnvVars = projectEnvVarsResult.isOk() ? projectEnvVarsResult.value : []; const sharedEnvVars = sharedEnvVarsResult.isOk() ? sharedEnvVarsResult.value : []; - // Filter out TRIGGER_SECRET_KEY and TRIGGER_VERSION (managed by Trigger.dev) and merge project + shared env vars - const excludedKeys = new Set(["TRIGGER_SECRET_KEY", "TRIGGER_VERSION"]); + // Hide platform-managed reserved keys from the onboarding preview. const projectEnvVarKeys = new Set(projectEnvVars.map((v) => v.key)); const mergedEnvVars: VercelEnvironmentVariable[] = [ ...projectEnvVars - .filter((v) => !excludedKeys.has(v.key)) + .filter((v) => !isReservedForExternalSync(v.key)) .map((v) => { const envVar = { ...v }; if (vercelEnvironmentId && (v as any).customEnvironmentIds?.includes(vercelEnvironmentId)) { @@ -581,7 +581,7 @@ export class VercelSettingsPresenter extends BasePresenter { return envVar; }), ...sharedEnvVars - .filter((v) => !projectEnvVarKeys.has(v.key) && !excludedKeys.has(v.key)) + .filter((v) => !projectEnvVarKeys.has(v.key) && !isReservedForExternalSync(v.key)) .map((v) => { const envVar = { id: v.id, diff --git a/apps/webapp/app/v3/environmentVariableRules.server.ts b/apps/webapp/app/v3/environmentVariableRules.server.ts index fce17d0364..7e6dc2a93d 100644 --- a/apps/webapp/app/v3/environmentVariableRules.server.ts +++ b/apps/webapp/app/v3/environmentVariableRules.server.ts @@ -10,29 +10,37 @@ const blacklistedVariables: VariableRule[] = [ { type: "exact", key: "TRIGGER_API_URL" }, ]; +const additionalExternalSyncReservedKeys = ["TRIGGER_VERSION", "TRIGGER_PREVIEW_BRANCH"]; + +export function isBlacklistedVariable(key: string): boolean { + const whitelisted = blacklistedVariables.find((bv) => bv.type === "whitelist" && bv.key === key); + if (whitelisted) { + return false; + } + + const exact = blacklistedVariables.find((bv) => bv.type === "exact" && bv.key === key); + if (exact) { + return true; + } + + const prefix = blacklistedVariables.find( + (bv) => bv.type === "prefix" && key.startsWith(bv.prefix) + ); + if (prefix) { + return true; + } + + return false; +} + +// Keys that must never be synced from an external integration (e.g. Vercel). Superset of +// the repository blacklist so submitting a reserved key doesn't get the whole batch rejected. +export function isReservedForExternalSync(key: string): boolean { + return isBlacklistedVariable(key) || additionalExternalSyncReservedKeys.includes(key); +} + export function removeBlacklistedVariables( variables: EnvironmentVariable[] ): EnvironmentVariable[] { - return variables.filter((v) => { - const whitelisted = blacklistedVariables.find( - (bv) => bv.type === "whitelist" && bv.key === v.key - ); - if (whitelisted) { - return true; - } - - const exact = blacklistedVariables.find((bv) => bv.type === "exact" && bv.key === v.key); - if (exact) { - return false; - } - - const prefix = blacklistedVariables.find( - (bv) => bv.type === "prefix" && v.key.startsWith(bv.prefix) - ); - if (prefix) { - return false; - } - - return true; - }); + return variables.filter((v) => !isBlacklistedVariable(v.key)); } diff --git a/apps/webapp/test/environmentVariableRules.test.ts b/apps/webapp/test/environmentVariableRules.test.ts index 39e3d9892e..0a035218dd 100644 --- a/apps/webapp/test/environmentVariableRules.test.ts +++ b/apps/webapp/test/environmentVariableRules.test.ts @@ -1,6 +1,10 @@ import { describe, it, expect } from "vitest"; import type { EnvironmentVariable } from "../app/v3/environmentVariables/repository"; -import { removeBlacklistedVariables } from "~/v3/environmentVariableRules.server"; +import { + isBlacklistedVariable, + isReservedForExternalSync, + removeBlacklistedVariables, +} from "~/v3/environmentVariableRules.server"; describe("removeBlacklistedVariables", () => { it("should remove exact match blacklisted variables", () => { @@ -68,3 +72,32 @@ describe("removeBlacklistedVariables", () => { ]); }); }); + +describe("isBlacklistedVariable", () => { + it("blacklists the platform-managed keys", () => { + expect(isBlacklistedVariable("TRIGGER_SECRET_KEY")).toBe(true); + expect(isBlacklistedVariable("TRIGGER_API_URL")).toBe(true); + }); + + it("allows ordinary user keys", () => { + expect(isBlacklistedVariable("DATABASE_URL")).toBe(false); + expect(isBlacklistedVariable("MY_API_KEY")).toBe(false); + }); +}); + +describe("isReservedForExternalSync", () => { + it("reserves every key the repository would reject", () => { + expect(isReservedForExternalSync("TRIGGER_SECRET_KEY")).toBe(true); + expect(isReservedForExternalSync("TRIGGER_API_URL")).toBe(true); + }); + + it("reserves deploy-managed keys that are not blacklisted", () => { + expect(isReservedForExternalSync("TRIGGER_VERSION")).toBe(true); + expect(isReservedForExternalSync("TRIGGER_PREVIEW_BRANCH")).toBe(true); + }); + + it("does not reserve ordinary user keys", () => { + expect(isReservedForExternalSync("DATABASE_URL")).toBe(false); + expect(isReservedForExternalSync("MY_API_KEY")).toBe(false); + }); +});