Skip to content

Commit cf4aa7e

Browse files
authored
fix(webapp): Vercel env var sync rejecting batches containing only reserved keys (#3966)
Fix Vercel onboarding wizard to properly filter out reserved TRIGGER_ env vars
1 parent 63d6432 commit cf4aa7e

5 files changed

Lines changed: 78 additions & 31 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Fix Vercel env var sync and onboarding preview leaking reserved `TRIGGER_*` keys.

apps/webapp/app/models/vercelIntegration.server.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
envTypeToVercelTarget,
2828
} from "~/v3/vercel/vercelProjectIntegrationSchema";
2929
import { EnvironmentVariablesRepository } from "~/v3/environmentVariables/environmentVariablesRepository.server";
30+
import { isReservedForExternalSync } from "~/v3/environmentVariableRules.server";
3031
import {
3132
callVercelWithRecovery,
3233
wrapVercelCallWithRecovery,
@@ -1350,10 +1351,9 @@ export class VercelIntegrationRepository {
13501351
for (const mapping of envMapping) {
13511352
const iterResult = await ResultAsync.fromPromise(
13521353
(async () => {
1353-
// Build filter to avoid decrypting vars that will be filtered out anyway
1354-
const excludeKeys = new Set(["TRIGGER_SECRET_KEY", "TRIGGER_VERSION"]);
1354+
// Exclude reserved keys before decrypting (a reserved-only batch gets rejected).
13551355
const shouldIncludeKey = (key: string) =>
1356-
!excludeKeys.has(key) &&
1356+
!isReservedForExternalSync(key) &&
13571357
shouldSyncEnvVar(params.syncEnvVarsMapping, key, mapping.triggerEnvType as TriggerEnvironmentType);
13581358

13591359
const envVarsResult = await this.getVercelEnvironmentVariableValues(
@@ -1399,7 +1399,7 @@ export class VercelIntegrationRepository {
13991399
if (envVar.isSecret) {
14001400
return false;
14011401
}
1402-
if (envVar.key === "TRIGGER_SECRET_KEY" || envVar.key === "TRIGGER_VERSION") {
1402+
if (isReservedForExternalSync(envVar.key)) {
14031403
return false;
14041404
}
14051405
return shouldSyncEnvVar(

apps/webapp/app/presenters/v3/VercelSettingsPresenter.server.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
} from "~/models/vercelIntegration.server";
1111
import { type GitHubAppInstallation } from "~/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github";
1212
import { EnvironmentVariablesRepository } from "~/v3/environmentVariables/environmentVariablesRepository.server";
13+
import { isReservedForExternalSync } from "~/v3/environmentVariableRules.server";
1314
import {
1415
VercelProjectIntegrationDataSchema,
1516
VercelProjectIntegrationData,
@@ -567,12 +568,11 @@ export class VercelSettingsPresenter extends BasePresenter {
567568
const projectEnvVars = projectEnvVarsResult.isOk() ? projectEnvVarsResult.value : [];
568569
const sharedEnvVars = sharedEnvVarsResult.isOk() ? sharedEnvVarsResult.value : [];
569570

570-
// Filter out TRIGGER_SECRET_KEY and TRIGGER_VERSION (managed by Trigger.dev) and merge project + shared env vars
571-
const excludedKeys = new Set(["TRIGGER_SECRET_KEY", "TRIGGER_VERSION"]);
571+
// Hide platform-managed reserved keys from the onboarding preview.
572572
const projectEnvVarKeys = new Set(projectEnvVars.map((v) => v.key));
573573
const mergedEnvVars: VercelEnvironmentVariable[] = [
574574
...projectEnvVars
575-
.filter((v) => !excludedKeys.has(v.key))
575+
.filter((v) => !isReservedForExternalSync(v.key))
576576
.map((v) => {
577577
const envVar = { ...v };
578578
if (vercelEnvironmentId && (v as any).customEnvironmentIds?.includes(vercelEnvironmentId)) {
@@ -581,7 +581,7 @@ export class VercelSettingsPresenter extends BasePresenter {
581581
return envVar;
582582
}),
583583
...sharedEnvVars
584-
.filter((v) => !projectEnvVarKeys.has(v.key) && !excludedKeys.has(v.key))
584+
.filter((v) => !projectEnvVarKeys.has(v.key) && !isReservedForExternalSync(v.key))
585585
.map((v) => {
586586
const envVar = {
587587
id: v.id,

apps/webapp/app/v3/environmentVariableRules.server.ts

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,37 @@ const blacklistedVariables: VariableRule[] = [
1010
{ type: "exact", key: "TRIGGER_API_URL" },
1111
];
1212

13+
const additionalExternalSyncReservedKeys = ["TRIGGER_VERSION", "TRIGGER_PREVIEW_BRANCH"];
14+
15+
export function isBlacklistedVariable(key: string): boolean {
16+
const whitelisted = blacklistedVariables.find((bv) => bv.type === "whitelist" && bv.key === key);
17+
if (whitelisted) {
18+
return false;
19+
}
20+
21+
const exact = blacklistedVariables.find((bv) => bv.type === "exact" && bv.key === key);
22+
if (exact) {
23+
return true;
24+
}
25+
26+
const prefix = blacklistedVariables.find(
27+
(bv) => bv.type === "prefix" && key.startsWith(bv.prefix)
28+
);
29+
if (prefix) {
30+
return true;
31+
}
32+
33+
return false;
34+
}
35+
36+
// Keys that must never be synced from an external integration (e.g. Vercel). Superset of
37+
// the repository blacklist so submitting a reserved key doesn't get the whole batch rejected.
38+
export function isReservedForExternalSync(key: string): boolean {
39+
return isBlacklistedVariable(key) || additionalExternalSyncReservedKeys.includes(key);
40+
}
41+
1342
export function removeBlacklistedVariables(
1443
variables: EnvironmentVariable[]
1544
): EnvironmentVariable[] {
16-
return variables.filter((v) => {
17-
const whitelisted = blacklistedVariables.find(
18-
(bv) => bv.type === "whitelist" && bv.key === v.key
19-
);
20-
if (whitelisted) {
21-
return true;
22-
}
23-
24-
const exact = blacklistedVariables.find((bv) => bv.type === "exact" && bv.key === v.key);
25-
if (exact) {
26-
return false;
27-
}
28-
29-
const prefix = blacklistedVariables.find(
30-
(bv) => bv.type === "prefix" && v.key.startsWith(bv.prefix)
31-
);
32-
if (prefix) {
33-
return false;
34-
}
35-
36-
return true;
37-
});
45+
return variables.filter((v) => !isBlacklistedVariable(v.key));
3846
}

apps/webapp/test/environmentVariableRules.test.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { describe, it, expect } from "vitest";
22
import type { EnvironmentVariable } from "../app/v3/environmentVariables/repository";
3-
import { removeBlacklistedVariables } from "~/v3/environmentVariableRules.server";
3+
import {
4+
isBlacklistedVariable,
5+
isReservedForExternalSync,
6+
removeBlacklistedVariables,
7+
} from "~/v3/environmentVariableRules.server";
48

59
describe("removeBlacklistedVariables", () => {
610
it("should remove exact match blacklisted variables", () => {
@@ -68,3 +72,32 @@ describe("removeBlacklistedVariables", () => {
6872
]);
6973
});
7074
});
75+
76+
describe("isBlacklistedVariable", () => {
77+
it("blacklists the platform-managed keys", () => {
78+
expect(isBlacklistedVariable("TRIGGER_SECRET_KEY")).toBe(true);
79+
expect(isBlacklistedVariable("TRIGGER_API_URL")).toBe(true);
80+
});
81+
82+
it("allows ordinary user keys", () => {
83+
expect(isBlacklistedVariable("DATABASE_URL")).toBe(false);
84+
expect(isBlacklistedVariable("MY_API_KEY")).toBe(false);
85+
});
86+
});
87+
88+
describe("isReservedForExternalSync", () => {
89+
it("reserves every key the repository would reject", () => {
90+
expect(isReservedForExternalSync("TRIGGER_SECRET_KEY")).toBe(true);
91+
expect(isReservedForExternalSync("TRIGGER_API_URL")).toBe(true);
92+
});
93+
94+
it("reserves deploy-managed keys that are not blacklisted", () => {
95+
expect(isReservedForExternalSync("TRIGGER_VERSION")).toBe(true);
96+
expect(isReservedForExternalSync("TRIGGER_PREVIEW_BRANCH")).toBe(true);
97+
});
98+
99+
it("does not reserve ordinary user keys", () => {
100+
expect(isReservedForExternalSync("DATABASE_URL")).toBe(false);
101+
expect(isReservedForExternalSync("MY_API_KEY")).toBe(false);
102+
});
103+
});

0 commit comments

Comments
 (0)