Skip to content

Commit e39eeeb

Browse files
matt-aitkenmatt-aitken
committed
Deduplicate and blacklist some env vars
1 parent e338061 commit e39eeeb

File tree

3 files changed

+247
-20
lines changed

3 files changed

+247
-20
lines changed

‎apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts‎

Lines changed: 71 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,19 @@
1-
import {
2-
Prisma,
3-
PrismaClient,
4-
RuntimeEnvironment,
5-
RuntimeEnvironmentType,
6-
} from "@trigger.dev/database";
1+
import { Prisma, type PrismaClient, type RuntimeEnvironmentType } from "@trigger.dev/database";
72
import { z } from "zod";
8-
import { environmentFullTitle, environmentTitle } from "~/components/environments/EnvironmentLabel";
3+
import { environmentFullTitle } from "~/components/environments/EnvironmentLabel";
94
import { $transaction, prisma } from "~/db.server";
105
import { env } from "~/env.server";
116
import { getSecretStore } from "~/services/secrets/secretStore.server";
127
import { generateFriendlyId } from "../friendlyIdentifiers";
138
import {
14-
CreateResult,
15-
DeleteEnvironmentVariable,
16-
DeleteEnvironmentVariableValue,
17-
EnvironmentVariable,
18-
EnvironmentVariableWithSecret,
19-
ProjectEnvironmentVariable,
20-
Repository,
21-
Result,
9+
type CreateResult,
10+
type DeleteEnvironmentVariable,
11+
type DeleteEnvironmentVariableValue,
12+
type EnvironmentVariable,
13+
type EnvironmentVariableWithSecret,
14+
type ProjectEnvironmentVariable,
15+
type Repository,
16+
type Result,
2217
} from "./repository";
2318

2419
function secretKeyProjectPrefix(projectId: string) {
@@ -94,9 +89,7 @@ export class EnvironmentVariablesRepository implements Repository {
9489
}
9590

9691
// Remove `TRIGGER_SECRET_KEY` or `TRIGGER_API_URL`
97-
let values = options.variables.filter(
98-
(v) => v.key !== "TRIGGER_SECRET_KEY" && v.key !== "TRIGGER_API_URL"
99-
);
92+
let values = removeBlacklistedVariables(options.variables);
10093

10194
//get rid of empty variables
10295
values = values.filter((v) => v.key.trim() !== "" && v.value.trim() !== "");
@@ -610,12 +603,14 @@ export class EnvironmentVariablesRepository implements Repository {
610603
mergedSecrets.set(parsedKey, secret.value.secret);
611604
}
612605

613-
return Array.from(mergedSecrets.entries()).map(([key, value]) => {
606+
const merged = Array.from(mergedSecrets.entries()).map(([key, value]) => {
614607
return {
615608
key,
616609
value,
617610
};
618611
});
612+
613+
return removeBlacklistedVariables(merged);
619614
}
620615

621616
async getEnvironmentVariables(
@@ -833,7 +828,24 @@ export async function resolveVariablesForEnvironment(
833828
? await resolveBuiltInDevVariables(runtimeEnvironment)
834829
: await resolveBuiltInProdVariables(runtimeEnvironment, parentEnvironment);
835830

836-
return [...overridableTriggerVariables, ...projectSecrets, ...builtInVariables];
831+
return deduplicateVariableArray([
832+
...overridableTriggerVariables,
833+
...projectSecrets,
834+
...builtInVariables,
835+
]);
836+
}
837+
838+
/** Later variables override earlier ones */
839+
export function deduplicateVariableArray(variables: EnvironmentVariable[]) {
840+
const result: EnvironmentVariable[] = [];
841+
// Process array in reverse order so later variables override earlier ones
842+
for (const variable of [...variables].reverse()) {
843+
if (!result.some((v) => v.key === variable.key)) {
844+
result.push(variable);
845+
}
846+
}
847+
// Reverse back to maintain original order but with later variables taking precedence
848+
return result.reverse();
837849
}
838850

839851
async function resolveOverridableTriggerVariables(
@@ -1020,3 +1032,42 @@ async function resolveCommonBuiltInVariables(
10201032
): Promise<Array<EnvironmentVariable>> {
10211033
return [];
10221034
}
1035+
1036+
type VariableRule =
1037+
| { type: "exact"; key: string }
1038+
| { type: "prefix"; prefix: string }
1039+
| { type: "whitelist"; key: string };
1040+
1041+
const blacklistedVariables: VariableRule[] = [
1042+
{ type: "exact", key: "TRIGGER_SECRET_KEY" },
1043+
{ type: "exact", key: "TRIGGER_API_URL" },
1044+
{ type: "prefix", prefix: "OTEL_" },
1045+
{ type: "whitelist", key: "OTEL_LOG_LEVEL" },
1046+
];
1047+
1048+
export function removeBlacklistedVariables(
1049+
variables: EnvironmentVariable[]
1050+
): EnvironmentVariable[] {
1051+
return variables.filter((v) => {
1052+
const whitelisted = blacklistedVariables.find(
1053+
(bv) => bv.type === "whitelist" && bv.key === v.key
1054+
);
1055+
if (whitelisted) {
1056+
return true;
1057+
}
1058+
1059+
const exact = blacklistedVariables.find((bv) => bv.type === "exact" && bv.key === v.key);
1060+
if (exact) {
1061+
return false;
1062+
}
1063+
1064+
const prefix = blacklistedVariables.find(
1065+
(bv) => bv.type === "prefix" && v.key.startsWith(bv.prefix)
1066+
);
1067+
if (prefix) {
1068+
return false;
1069+
}
1070+
1071+
return true;
1072+
});
1073+
}

‎apps/webapp/test/environmentVariableDeduplication.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { describe, it, expect } from "vitest";
2+
import { deduplicateVariableArray } from "../app/v3/environmentVariables/environmentVariablesRepository.server";
3+
import type { EnvironmentVariable } from "../app/v3/environmentVariables/repository";
4+
5+
describe("Deduplicate variables", () => {
6+
it("should keep later variables when there are duplicates", () => {
7+
const variables: EnvironmentVariable[] = [
8+
{ key: "API_KEY", value: "old_value" },
9+
{ key: "API_KEY", value: "new_value" },
10+
];
11+
12+
const result = deduplicateVariableArray(variables);
13+
14+
expect(result).toHaveLength(1);
15+
expect(result[0]).toEqual({ key: "API_KEY", value: "new_value" });
16+
});
17+
18+
it("should preserve order of unique variables", () => {
19+
const variables: EnvironmentVariable[] = [
20+
{ key: "FIRST", value: "first" },
21+
{ key: "SECOND", value: "second" },
22+
{ key: "THIRD", value: "third" },
23+
];
24+
25+
const result = deduplicateVariableArray(variables);
26+
27+
expect(result).toHaveLength(3);
28+
expect(result[0].key).toBe("FIRST");
29+
expect(result[1].key).toBe("SECOND");
30+
expect(result[2].key).toBe("THIRD");
31+
});
32+
33+
it("should handle multiple duplicates with later values taking precedence", () => {
34+
const variables: EnvironmentVariable[] = [
35+
{ key: "DB_URL", value: "old_db" },
36+
{ key: "API_KEY", value: "old_key" },
37+
{ key: "DB_URL", value: "new_db" },
38+
{ key: "API_KEY", value: "new_key" },
39+
];
40+
41+
const result = deduplicateVariableArray(variables);
42+
43+
expect(result).toHaveLength(2);
44+
expect(result.find((v) => v.key === "DB_URL")?.value).toBe("new_db");
45+
expect(result.find((v) => v.key === "API_KEY")?.value).toBe("new_key");
46+
});
47+
48+
it("should handle empty array", () => {
49+
const result = deduplicateVariableArray([]);
50+
expect(result).toEqual([]);
51+
});
52+
53+
it("should handle array with no duplicates", () => {
54+
const variables: EnvironmentVariable[] = [
55+
{ key: "VAR1", value: "value1" },
56+
{ key: "VAR2", value: "value2" },
57+
];
58+
59+
const result = deduplicateVariableArray(variables);
60+
61+
expect(result).toHaveLength(2);
62+
expect(result).toEqual(variables);
63+
});
64+
});

‎apps/webapp/test/environmentVariableRules.test.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { describe, it, expect } from "vitest";
2+
import { removeBlacklistedVariables } from "../app/v3/environmentVariables/environmentVariablesRepository.server";
3+
import type { EnvironmentVariable } from "../app/v3/environmentVariables/repository";
4+
5+
describe("removeBlacklistedVariables", () => {
6+
it("should remove exact match blacklisted variables", () => {
7+
const variables: EnvironmentVariable[] = [
8+
{ key: "TRIGGER_SECRET_KEY", value: "secret123" },
9+
{ key: "TRIGGER_API_URL", value: "https://api.example.com" },
10+
{ key: "NORMAL_VAR", value: "normal" },
11+
];
12+
13+
const result = removeBlacklistedVariables(variables);
14+
15+
expect(result).toEqual([{ key: "NORMAL_VAR", value: "normal" }]);
16+
});
17+
18+
it("should remove variables with blacklisted prefixes", () => {
19+
const variables: EnvironmentVariable[] = [
20+
{ key: "OTEL_SERVICE_NAME", value: "my-service" },
21+
{ key: "OTEL_TRACE_SAMPLER", value: "always_on" },
22+
{ key: "NORMAL_VAR", value: "normal" },
23+
];
24+
25+
const result = removeBlacklistedVariables(variables);
26+
27+
expect(result).toEqual([{ key: "NORMAL_VAR", value: "normal" }]);
28+
});
29+
30+
it("should keep whitelisted variables even if they match a blacklisted prefix", () => {
31+
const variables: EnvironmentVariable[] = [
32+
{ key: "OTEL_LOG_LEVEL", value: "debug" },
33+
{ key: "OTEL_SERVICE_NAME", value: "my-service" },
34+
{ key: "NORMAL_VAR", value: "normal" },
35+
];
36+
37+
const result = removeBlacklistedVariables(variables);
38+
39+
expect(result).toEqual([
40+
{ key: "OTEL_LOG_LEVEL", value: "debug" },
41+
{ key: "NORMAL_VAR", value: "normal" },
42+
]);
43+
});
44+
45+
it("should handle empty input array", () => {
46+
const variables: EnvironmentVariable[] = [];
47+
48+
const result = removeBlacklistedVariables(variables);
49+
50+
expect(result).toEqual([]);
51+
});
52+
53+
it("should handle mixed case variables", () => {
54+
const variables: EnvironmentVariable[] = [
55+
{ key: "trigger_secret_key", value: "secret123" }, // Different case
56+
{ key: "OTEL_LOG_LEVEL", value: "debug" },
57+
{ key: "otel_service_name", value: "my-service" }, // Different case
58+
{ key: "NORMAL_VAR", value: "normal" },
59+
];
60+
61+
const result = removeBlacklistedVariables(variables);
62+
63+
// Should keep only the whitelisted OTEL_LOG_LEVEL and NORMAL_VAR
64+
// Note: The function is case-sensitive, so different case variables should pass through
65+
expect(result).toEqual([
66+
{ key: "trigger_secret_key", value: "secret123" },
67+
{ key: "OTEL_LOG_LEVEL", value: "debug" },
68+
{ key: "otel_service_name", value: "my-service" },
69+
{ key: "NORMAL_VAR", value: "normal" },
70+
]);
71+
});
72+
73+
it("should handle variables with empty values", () => {
74+
const variables: EnvironmentVariable[] = [
75+
{ key: "TRIGGER_SECRET_KEY", value: "" },
76+
{ key: "OTEL_SERVICE_NAME", value: "" },
77+
{ key: "OTEL_LOG_LEVEL", value: "" },
78+
{ key: "NORMAL_VAR", value: "" },
79+
];
80+
81+
const result = removeBlacklistedVariables(variables);
82+
83+
expect(result).toEqual([
84+
{ key: "OTEL_LOG_LEVEL", value: "" },
85+
{ key: "NORMAL_VAR", value: "" },
86+
]);
87+
});
88+
89+
it("should handle all types of rules in a single array", () => {
90+
const variables: EnvironmentVariable[] = [
91+
// Exact matches (should be removed)
92+
{ key: "TRIGGER_SECRET_KEY", value: "secret123" },
93+
{ key: "TRIGGER_API_URL", value: "https://api.example.com" },
94+
// Prefix matches (should be removed)
95+
{ key: "OTEL_SERVICE_NAME", value: "my-service" },
96+
{ key: "OTEL_TRACE_SAMPLER", value: "always_on" },
97+
// Whitelist exception (should be kept)
98+
{ key: "OTEL_LOG_LEVEL", value: "debug" },
99+
// Normal variables (should be kept)
100+
{ key: "NORMAL_VAR", value: "normal" },
101+
{ key: "DATABASE_URL", value: "postgres://..." },
102+
];
103+
104+
const result = removeBlacklistedVariables(variables);
105+
106+
expect(result).toEqual([
107+
{ key: "OTEL_LOG_LEVEL", value: "debug" },
108+
{ key: "NORMAL_VAR", value: "normal" },
109+
{ key: "DATABASE_URL", value: "postgres://..." },
110+
]);
111+
});
112+
});

0 commit comments

Comments
 (0)