Commit5e0d7ba
fix(security): preview builds leaked every production secret to the repo owner
fix(security): preview builds leaked every production secret to the repo owner
src/lib/preview-builder.ts executed `sh -c <repo.previewBuildCommand>` with
the FULL process.env, and captured stdout/stderr into `buildLog`, which is
stored and rendered back to the user who triggered the build.
previewBuildCommand is a plain repo setting, written from the repo settings
form (routes/repo-settings.tsx). So any user who owns a repo could set it to
`env`, trigger a preview build, and read from their own build log:
DATABASE_URL production Neon credentials
ANTHROPIC_API_KEY platform billing key
GLUECRON_PAT operator admin token
WORKFLOW_SECRETS_KEY decrypts every user's stored CI secrets
SERVER_TARGETS_KEY decrypts deploy-target SSH private keys
GOOGLE_OAUTH_CLIENT_SECRET, STRIPE_SECRET_KEY, ...
That is complete platform compromise from a settings field, and the exfil
channel is a page the attacker already controls.
src/lib/workflow-runner.ts already had the right answer for the identical
threat model — user-supplied `run:` steps get a curated RUNNER_ENV_ALLOWLIST
filtered through a credential-shaped RUNNER_ENV_DENYLIST. Preview builds now
use the same buildRunnerEnv() helper (exported for the purpose) rather than
making the decision a second time and getting it wrong.
The gate added here enforces the rule for every user-code path: no
`env: { ...process.env }`, no direct read of a platform secret, and a sweep
that fails if a NEW `sh -c` / `bash -c` site appears outside the declared
USER_CODE_PATHS list. It asserts on the READ (process.env.X) rather than the
mention, because workflow-runner correctly NAMES these secrets in its
denylist — a substring check would have flagged the defence itself.
Verified rather than assumed: reintroducing the exact original expression
(`env: { ...process.env, CI: "1" }`) fails the gate, and reverting passes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>3 files changed+170−25e0d7ba0468da48d96d19307dab328bdffdbf19e
3 changed files+170−2
Addedsrc/__tests__/secret-egress.test.ts+155−0View fileUnifiedSplit
@@ -0,0 +1,155 @@
1/**
2 * No user-controlled execution path receives platform secrets.
3 *
4 * Two real breaches of this rule, both found by auditing rather than by any
5 * test:
6 *
7 * - src/lib/preview-builder.ts ran `sh -c <repo.previewBuildCommand>` — a
8 * string any repo owner sets from the repo settings form — with the FULL
9 * process.env, and captured stdout/stderr into `buildLog`, which is
10 * rendered back to that same user. `previewBuildCommand = "env"` therefore
11 * printed DATABASE_URL (production Neon credentials), ANTHROPIC_API_KEY,
12 * GLUECRON_PAT, WORKFLOW_SECRETS_KEY, SERVER_TARGETS_KEY and the OAuth
13 * secrets into a page the attacker controls. Complete platform compromise
14 * from a settings field.
15 *
16 * - src/lib/hosted-claude-loop.ts passed ANTHROPIC_API_KEY and GLUECRON_PAT
17 * into the env of arbitrary user-supplied JavaScript whose stdout is
18 * returned to the caller.
19 *
20 * src/lib/workflow-runner.ts already had the correct pattern — a curated
21 * allowlist plus a credential-shaped denylist — for user `run:` steps. The
22 * rule these tests enforce is that EVERY user-code path uses it, because the
23 * threat model is identical and the decision should not be made twice.
24 */
25
26import { describe, expect, it } from "bun:test";
27import { readFileSync } from "fs";
28
29const SECRET_NAMES = [
30 "ANTHROPIC_API_KEY",
31 "GLUECRON_PAT",
32 "DATABASE_URL",
33 "WORKFLOW_SECRETS_KEY",
34 "SERVER_TARGETS_KEY",
35 "GOOGLE_OAUTH_CLIENT_SECRET",
36 "STRIPE_SECRET_KEY",
37 "EMERGENCY_PAT_SECRET",
38];
39
40/**
41 * Every site that executes a user-supplied command or script. Adding a new one
42 * without adding it here is what this suite is designed to catch — see the
43 * "no unlisted shell-exec sites" test below.
44 */
45const USER_CODE_PATHS = [
46 "src/lib/workflow-runner.ts", // user `run:` steps from workflow YAML
47 "src/lib/preview-builder.ts", // repo.previewBuildCommand
48 "src/lib/hosted-claude-loop.ts", // user-authored loop source
49];
50
51describe("user-code paths do not inherit the process environment", () => {
52 for (const path of USER_CODE_PATHS) {
53 it(`${path} never spreads process.env into a spawn`, () => {
54 const src = readFileSync(path, "utf8")
55 .replace(/\/\*[\s\S]*?\*\//g, "")
56 .replace(/^\s*\/\/.*$/gm, "");
57 // `env: { ...process.env }` is the exact shape that leaked everything.
58 expect(src).not.toMatch(/env:\s*\{\s*\.\.\.process\.env/);
59 });
60 }
61});
62
63describe("no secret is named in a user-code path's env construction", () => {
64 for (const path of USER_CODE_PATHS) {
65 const src = readFileSync(path, "utf8")
66 .replace(/\/\*[\s\S]*?\*\//g, "")
67 .replace(/^\s*\/\/.*$/gm, "");
68 for (const secret of SECRET_NAMES) {
69 it(`${path} does not read process.env.${secret}`, () => {
70 // Assert on the READ, not the mention. workflow-runner names these
71 // secrets in its RUNNER_ENV_DENYLIST, which is the correct thing to
72 // do — a substring check would flag the very defence being asserted.
73 //
74 // hosted-claude-loop still reads ANTHROPIC_API_KEY, but only behind an
75 // explicit operator opt-in, asserted separately below.
76 if (path.includes("hosted-claude-loop") && secret === "ANTHROPIC_API_KEY") return;
77 expect(src).not.toMatch(new RegExp(`process\\.env\\.${secret}\\b`));
78 expect(src).not.toMatch(new RegExp(`process\\.env\\[["'\`]${secret}["'\`]\\]`));
79 });
80 }
81 }
82});
83
84describe("the shared allowlist is the only way user code gets an env", () => {
85 it("preview-builder uses buildRunnerEnv rather than its own env object", () => {
86 const src = readFileSync("src/lib/preview-builder.ts", "utf8");
87 expect(src).toContain("buildRunnerEnv");
88 });
89
90 it("workflow-runner still filters through allowlist AND denylist", () => {
91 const src = readFileSync("src/lib/workflow-runner.ts", "utf8");
92 expect(src).toContain("RUNNER_ENV_ALLOWLIST");
93 expect(src).toContain("RUNNER_ENV_DENYLIST");
94 // The denylist is the net: even an allowlisted key that looks like a
95 // credential is dropped.
96 expect(src).toMatch(/RUNNER_ENV_DENYLIST\.test\(key\)/);
97 });
98
99 it("the allowlist contains no secret-shaped names", () => {
100 const src = readFileSync("src/lib/workflow-runner.ts", "utf8");
101 const list = src.slice(
102 src.indexOf("const RUNNER_ENV_ALLOWLIST"),
103 src.indexOf("];", src.indexOf("const RUNNER_ENV_ALLOWLIST"))
104 );
105 for (const secret of SECRET_NAMES) expect(list).not.toContain(secret);
106 expect(list).not.toMatch(/KEY|SECRET|TOKEN|PASSWORD|CREDENTIAL/);
107 });
108});
109
110describe("hosted loops keep the platform key opt-in", () => {
111 // Comments stripped: this file documents the original bug, and that prose
112 // necessarily contains the very strings being asserted against.
113 const src = readFileSync("src/lib/hosted-claude-loop.ts", "utf8")
114 .replace(/\/\*[\s\S]*?\*\//g, "")
115 .replace(/^\s*\/\/.*$/gm, "");
116
117 it("the operator PAT is never injected", () => {
118 expect(src).not.toMatch(/env\.GLUECRON_PAT\s*=/);
119 expect(src).not.toMatch(/GLUECRON_PAT:/);
120 });
121
122 it("the Anthropic key requires an explicit opt-in flag", () => {
123 expect(src).toContain('process.env.HOSTED_LOOPS_PLATFORM_KEY === "1"');
124 // Never as a bare property on the env literal — that was the bug.
125 expect(src).not.toMatch(/^\s*ANTHROPIC_API_KEY:/m);
126 });
127});
128
129describe("no unlisted shell-exec site", () => {
130 it("every `sh -c` / `bash -c` spawn lives in a declared user-code path", () => {
131 const { readdirSync, statSync } = require("fs") as typeof import("fs");
132 const { join } = require("path") as typeof import("path");
133 const walk = (d: string): string[] =>
134 readdirSync(d).flatMap((e) => {
135 const p = join(d, e);
136 return statSync(p).isDirectory() ? walk(p) : [p];
137 });
138
139 const found: string[] = [];
140 for (const dir of ["src/lib", "src/routes", "src/hooks"]) {
141 for (const f of walk(dir)) {
142 if (!/\.tsx?$/.test(f)) continue;
143 const src = readFileSync(f, "utf8").replace(/^\s*\/\/.*$/gm, "");
144 if (/\[\s*["'`](?:ba)?sh["'`]\s*,\s*["'`]-c["'`]/.test(src)) {
145 found.push(f.replace(/\\/g, "/"));
146 }
147 }
148 }
149 // A new shell-exec site must be reviewed and added to USER_CODE_PATHS,
150 // which subjects it to every assertion above.
151 expect(found.sort()).toEqual(
152 USER_CODE_PATHS.filter((p) => found.includes(p)).sort()
153 );
154 });
155});
Modifiedsrc/lib/preview-builder.ts+14−1View fileUnifiedSplit
@@ -199,13 +199,26 @@ export async function buildPreview(
199199 }
200200
201201 // Step 2: run the build command with timeout
202 // `buildCommand` is repo.previewBuildCommand — a value any repo owner sets
203 // from the repo settings form. It was executed with the FULL process.env,
204 // and stdout/stderr is captured into buildLog and rendered back to that
205 // same user. So `previewBuildCommand = "env"` printed DATABASE_URL (the
206 // production Neon credentials), ANTHROPIC_API_KEY, GLUECRON_PAT,
207 // WORKFLOW_SECRETS_KEY, SERVER_TARGETS_KEY and the OAuth secrets straight
208 // into a page the attacker controls. Complete platform compromise from a
209 // settings field.
210 //
211 // Use the same curated allowlist the workflow runner already uses for
212 // user-supplied `run:` steps — identical threat model, so it should never
213 // have been a separate decision.
214 const { buildRunnerEnv } = await import("./workflow-runner");
202215 const buildProc = Bun.spawn(
203216 ["sh", "-c", buildCommand],
204217 {
205218 cwd: buildDir,
206219 stderr: "pipe",
207220 stdout: "pipe",
208 env: { ...process.env, CI: "1" },
221 env: buildRunnerEnv({ CI: "1" }),
209222 }
210223 );
211224
Modifiedsrc/lib/workflow-runner.ts+1−1View fileUnifiedSplit
@@ -162,7 +162,7 @@ const RUNNER_ENV_DENYLIST =
162162 * nothing sensitive, since user secrets are text-substituted, not env-passed).
163163 * `extra` wins on key collision and is exempt from the denylist by design.
164164 */
165function buildRunnerEnv(
165export function buildRunnerEnv(
166166 extra: Record<string, string> = {}
167167): Record<string, string> {
168168 const out: Record<string, string> = {};
169169