Skip to content

Commit a213904

Browse files
committed
tests: isolate the sandbox's docker client from the user's client config (#5714)
The docker CLI copies the `proxies` entries of its client config into every build (as build args) and every container (as env). A proxy URL can hold a user and password, and that path skipped containerEnv()'s credential check. - One `docker context inspect` with the user's DOCKER_HOST, DOCKER_CONTEXT and DOCKER_CONFIG resolves the selected endpoint. Only a local unix socket passes. A remote, ssh or relative endpoint is refused, never swapped for the default socket. - Every other docker command (info, image inspect, build, the probe, the job, ps, rm) runs with PATH, DOCKER_HOST = that endpoint and DOCKER_CONFIG = a fresh, empty private folder that lives until the launcher exits. No HOME, no other DOCKER_* value, no credential or TLS file.
1 parent c5fb22d commit a213904

2 files changed

Lines changed: 221 additions & 46 deletions

File tree

‎tests/bugbash/sandbox/launch.test.ts‎

Lines changed: 168 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -128,42 +128,77 @@ test("only `e2e run --config e2e.config.ts` with selection options is an exact-s
128128
}
129129
});
130130

131+
// The fake docker CLI. It logs each call: its args, its env names, DOCKER_HOST, DOCKER_CONFIG,
132+
// the files in that folder, and whether the env or that folder holds the synthetic MARKER.
133+
// It answers like a local Linux engine. `context` answers from DOCKER_HOST, else DOCKER_CONTEXT.
134+
// The launcher passes the fake no FAKE_* env (that is the rule under test), so launch() writes
135+
// the log path and the modes into the script.
136+
const FAKE_DOCKER = String.raw`
137+
{
138+
echo "CALL $*"
139+
echo "ENV $(env | sed 's/=.*//' | grep -vxE 'PWD|OLDPWD|SHLVL|_' | sort | tr '\n' ' ')"
140+
echo "HOST $DOCKER_HOST CONFIG $DOCKER_CONFIG"
141+
echo "FILES $(ls -A "$DOCKER_CONFIG" 2>&1 | tr '\n' ' ')"
142+
echo "MARKER $(env | grep -c MARKER) $(grep -rl MARKER "$DOCKER_CONFIG" 2>/dev/null | wc -l)"
143+
} >> "$FAKE_LOG"
144+
case "$1" in
145+
context)
146+
if [ -n "$DOCKER_HOST" ]; then echo "$DOCKER_HOST"
147+
elif [ "$DOCKER_CONTEXT" = remote ]; then echo tcp://10.0.0.1:2376
148+
elif [ "$DOCKER_CONTEXT" = other ]; then echo unix:///run/other.sock
149+
else echo unix:///var/run/docker.sock; fi ;;
150+
info) echo '{"OSType":"linux","OperatingSystem":"Ubuntu","SecurityOptions":[]}' ;;
151+
image) [ "$FAKE_IMAGE" = present ] ;;
152+
build) cat > /dev/null ;;
153+
run)
154+
src=$(printf '%s\n' "$@" | sed -n 's/^type=bind,src=\([^,]*\),dst=\/probe,readonly$/\1/p')
155+
if [ -z "$src" ]; then printf '{"end":true}\n'; exit 0; fi
156+
[ "$FAKE_PROBE" = fail ] && exit 1
157+
cat /proc/sys/kernel/random/boot_id; cat "$src/.nonce"; echo ;;
158+
ps) [ -e "$FAKE_LOG.rm" ] || echo cid123 ;;
159+
rm) touch "$FAKE_LOG.rm" ;;
160+
*) exit 1 ;;
161+
esac
162+
`;
163+
164+
interface Launch {
165+
sandbox?: boolean;
166+
cwd?: string;
167+
env?: Record<string, string>;
168+
/** The fake's same-host probe fails (default), so a sandbox job falls back to the host. */
169+
probeFails?: boolean;
170+
/** The image is missing, so the launcher builds it. */
171+
build?: boolean;
172+
}
173+
131174
/**
132-
* Runs the launcher with a fake docker and a fake e2e node, which log each call. `sandbox`: the
133-
* fake docker reports a local Linux engine, so the launcher picks the sandbox. Else the real
134-
* app AI sends the job to the host fallback.
175+
* Runs the launcher with a fake docker and a fake e2e node, which log each call. `sandbox`
176+
* (default true): the mock app AI, so the launcher picks the sandbox. Else the real app AI sends
177+
* the job to the host fallback.
135178
*/
136-
function launch(args: string[], sandbox: boolean, cwd = BUGBASH_DIR) {
179+
function launch(args: string[], options: Launch = {}) {
137180
const bin = fs.mkdtempSync(path.join(os.tmpdir(), "xbb-launch-test-"));
138181
const calls = path.join(bin, "calls.log");
139-
const fake = (name: string, body: string) =>
140-
fs.writeFileSync(path.join(bin, name), `#!/bin/sh\necho "${name} $*" >> ${calls}\n${body}\n`, {
141-
mode: 0o755,
142-
});
143-
fake(
144-
"docker",
145-
[
146-
'case "$1" in',
147-
" context) echo unix:///var/run/docker.sock ;;",
148-
' info) echo \'{"OSType":"linux","OperatingSystem":"Ubuntu","SecurityOptions":[]}\' ;;',
149-
" image) exit 0 ;;",
150-
" *) exit 1 ;;", // the same-host probe fails: the job falls back to the host fake below
151-
"esac",
152-
].join("\n")
153-
);
154-
fake("e2e-node", "exit 0");
182+
const script = FAKE_DOCKER.replaceAll("$FAKE_LOG", calls)
183+
.replaceAll("$FAKE_PROBE", options.probeFails === false ? "ok" : "fail")
184+
.replaceAll("$FAKE_IMAGE", options.build === true ? "missing" : "present");
185+
fs.writeFileSync(path.join(bin, "docker"), `#!/bin/sh${script}`, { mode: 0o755 });
186+
fs.writeFileSync(path.join(bin, "e2e-node"), `#!/bin/sh\necho "CALL e2e-node $*" >> ${calls}\n`, {
187+
mode: 0o755,
188+
});
155189
try {
156190
const r = spawnSync(
157191
process.execPath,
158192
[path.join(import.meta.dir, "launch.ts"), "--", ...args],
159193
{
160-
cwd,
194+
cwd: options.cwd ?? BUGBASH_DIR,
161195
encoding: "utf8",
162196
env: {
163197
PATH: `${bin}:${process.env.PATH ?? ""}`,
164198
HOME: process.env.HOME,
165199
E2E_NODE: path.join(bin, "e2e-node"),
166-
BUGBASH_AI_RESOLVED: sandbox ? "mock" : "real",
200+
BUGBASH_AI_RESOLVED: options.sandbox === false ? "real" : "mock",
201+
...options.env,
167202
},
168203
}
169204
);
@@ -177,23 +212,126 @@ function launch(args: string[], sandbox: boolean, cwd = BUGBASH_DIR) {
177212

178213
test("the launcher runs nothing else, in the sandbox and on the host fallback", () => {
179214
// Controls: the exact-step run reaches the docker run (sandbox) or the e2e node (host).
180-
const sandboxed = launch(OK, true);
181-
expect(sandboxed.log).toContain("docker run");
182-
const hosted = launch(OK, false);
215+
const sandboxed = launch(OK);
216+
expect(sandboxed.log).toContain("CALL run");
217+
const hosted = launch(OK, { sandbox: false });
183218
expect(hosted.log).toContain(
184-
`e2e-node ${path.resolve(BUGBASH_DIR, "../..")}/node_modules/.bin/e2e run`
219+
`CALL e2e-node ${path.resolve(BUGBASH_DIR, "../..")}/node_modules/.bin/e2e run`
185220
);
186221
for (const sandbox of [true, false])
187222
for (const args of NOT_EXACT) {
188-
const r = launch(args, sandbox);
189-
expect({ args, status: r.status, ran: r.log.match(/docker (run|build)|e2e-node/g) }).toEqual({
223+
const r = launch(args, { sandbox });
224+
expect({ args, status: r.status, ran: r.log.match(/CALL (run|build|e2e-node)/g) }).toEqual({
190225
args,
191226
status: 2,
192227
ran: null,
193228
});
194229
expect(r.stderr).toContain("exact-step repros");
195230
}
196231
// The same args from another cwd would load another e2e.config.ts.
197-
const elsewhere = launch(OK, false, path.resolve(BUGBASH_DIR, "../.."));
198-
expect([elsewhere.status, elsewhere.log.includes("e2e-node")]).toEqual([2, false]);
232+
const elsewhere = launch(OK, { sandbox: false, cwd: path.resolve(BUGBASH_DIR, "../..") });
233+
expect([elsewhere.status, elsewhere.log.includes("CALL e2e-node")]).toEqual([2, false]);
234+
});
235+
236+
/** The calls in a fake docker log, one record per call. */
237+
function dockerCalls(log: string) {
238+
return log
239+
.split("CALL ")
240+
.slice(1)
241+
.map((block) => {
242+
const line = (key: string) =>
243+
block
244+
.split("\n")
245+
.find((l) => l.startsWith(`${key} `))
246+
?.slice(key.length + 1) ?? "";
247+
const [host, , config] = line("HOST").split(" ");
248+
return {
249+
command: block.split("\n")[0],
250+
env: line("ENV").trim(),
251+
host,
252+
config,
253+
files: line("FILES").trim(),
254+
marker: line("MARKER"),
255+
};
256+
})
257+
.filter((call) => !call.command.startsWith("e2e-node"));
258+
}
259+
260+
/** A client config with an authenticated proxy: the CLI would copy it into builds and containers. */
261+
function syntheticDockerConfig(): string {
262+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "xbb-launch-test-"));
263+
const proxy = "http://user:MARKER-secret@127.0.0.1:9";
264+
const config = { proxies: { default: { httpProxy: proxy, httpsProxy: proxy } }, auths: {} };
265+
fs.writeFileSync(path.join(dir, "config.json"), JSON.stringify(config));
266+
return dir;
267+
}
268+
269+
test("docker commands never see the user's client config, HOME or other DOCKER_* values", () => {
270+
const userConfig = syntheticDockerConfig();
271+
try {
272+
const r = launch(OK, {
273+
probeFails: false,
274+
build: true,
275+
env: {
276+
DOCKER_CONFIG: userConfig,
277+
DOCKER_CERT_PATH: userConfig,
278+
DOCKER_TLS_VERIFY: "1",
279+
},
280+
});
281+
expect(r.status).toBe(0);
282+
const calls = dockerCalls(r.log);
283+
expect(calls.map((call) => call.command.split(" ")[0])).toEqual([
284+
"context", // the only call that reads the user's config: the context selection
285+
"info",
286+
"image",
287+
"build",
288+
"run", // the same-host probe
289+
"run", // the job
290+
"ps", // cleanup: found, removed, gone
291+
"rm",
292+
"ps",
293+
]);
294+
expect(calls[0].config).toBe(userConfig);
295+
const privateDirs = new Set(calls.slice(1).map((call) => call.config));
296+
expect(privateDirs.size).toBe(1);
297+
const [privateDir] = privateDirs;
298+
for (const call of calls.slice(1))
299+
expect({ ...call, command: "" }).toEqual({
300+
command: "",
301+
env: "DOCKER_CONFIG DOCKER_HOST PATH",
302+
host: "unix:///var/run/docker.sock",
303+
config: privateDir,
304+
files: "",
305+
marker: "0 0",
306+
});
307+
expect(privateDir.startsWith(os.tmpdir())).toBe(true);
308+
expect(fs.existsSync(privateDir)).toBe(false); // removed when the launcher exited
309+
expect(r.stderr).not.toContain("MARKER");
310+
} finally {
311+
fs.rmSync(userConfig, { recursive: true, force: true });
312+
}
313+
});
314+
315+
test("the selected context is resolved once; a remote one is refused, not swapped for the default", () => {
316+
const other = dockerCalls(launch(OK, { env: { DOCKER_CONTEXT: "other" } }).log);
317+
expect(other.slice(1).map((call) => [call.host, call.env])).toEqual(
318+
other.slice(1).map(() => ["unix:///run/other.sock", "DOCKER_CONFIG DOCKER_HOST PATH"])
319+
);
320+
for (const env of [
321+
{ DOCKER_CONTEXT: "remote" } as Record<string, string>,
322+
{ DOCKER_HOST: "tcp://10.0.0.1:2376" },
323+
{ DOCKER_HOST: "ssh://user@host" },
324+
{ DOCKER_HOST: "unix://run/relative.sock" },
325+
]) {
326+
const required = launch(OK, { env: { ...env, BUGBASH_SANDBOX: "require" } });
327+
expect([
328+
required.status,
329+
dockerCalls(required.log).map((c) => c.command.split(" ")[0]),
330+
]).toEqual([2, ["context"]]);
331+
expect(required.stderr).toContain("not a local socket");
332+
// auto: the exact-step run goes to the host, with no further docker command.
333+
const auto = launch(OK, { env });
334+
expect(dockerCalls(auto.log).length).toBe(1);
335+
expect(auto.log).toContain("CALL e2e-node");
336+
}
199337
});

‎tests/bugbash/sandbox/launch.ts‎

Lines changed: 53 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,53 @@ class Unusable extends Refusal {}
4848
const log = (message: string) => console.error(`sandbox ${message}`);
4949
const sha = (text: string) => crypto.createHash("sha256").update(text).digest("hex");
5050

51-
// The docker CLI gets only what it needs to find the daemon: no keys, no tokens.
52-
function dockerEnv(): NodeJS.ProcessEnv {
53-
return Object.fromEntries(
54-
Object.entries(process.env).filter(
55-
([k]) => k === "PATH" || k === "HOME" || k.startsWith("DOCKER_")
56-
)
57-
);
51+
/**
52+
* The env of every docker command after checkEndpoint(): PATH, the endpoint that the user
53+
* selected, and an empty private client config. The CLI reads no user config. Its `proxies`
54+
* entries (a proxy URL can hold a user and password) would otherwise go into every build as
55+
* build args and into every container as env, past containerEnv(). No HOME, no other DOCKER_*.
56+
*/
57+
let client: Record<string, string> | null = null;
58+
59+
function clientEnv(): Record<string, string> {
60+
if (client == null) throw new Refusal("docker: checkEndpoint() must pass first");
61+
return client;
62+
}
63+
64+
/**
65+
* The endpoint of the user's docker CLI: DOCKER_HOST, else DOCKER_CONTEXT, else the current
66+
* context in the user's client config. This is the one docker command that reads the user's
67+
* config. It reads the context only, and starts no build and no container.
68+
*/
69+
function selectedEndpoint(): { host: string } | { error: string } {
70+
const pass = ["PATH", "HOME", "DOCKER_HOST", "DOCKER_CONTEXT", "DOCKER_CONFIG"];
71+
const env: Record<string, string> = {};
72+
for (const [key, value] of Object.entries(process.env))
73+
if (pass.includes(key) && value != null) env[key] = value;
74+
const r = run(env, ["context", "inspect", "--format", "{{.Endpoints.docker.Host}}"], {
75+
timeoutMs: 10_000,
76+
});
77+
return r.ok ? { host: r.stdout } : { error: r.error };
78+
}
79+
80+
/** A fresh, empty client config folder. It lives until this launcher exits. */
81+
function privateClient(host: string): Record<string, string> {
82+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "xum-bugbash-docker-"));
83+
process.on("exit", () => fs.rmSync(dir, { recursive: true, force: true }));
84+
return { PATH: process.env.PATH ?? "", DOCKER_HOST: host, DOCKER_CONFIG: dir };
5885
}
5986

6087
function docker(args: string[], options: { timeoutMs: number; input?: string; quiet?: boolean }) {
88+
return run(clientEnv(), args, options);
89+
}
90+
91+
function run(
92+
env: Record<string, string>,
93+
args: string[],
94+
options: { timeoutMs: number; input?: string; quiet?: boolean }
95+
) {
6196
const r = spawnSync("docker", args, {
62-
env: dockerEnv(),
97+
env,
6398
encoding: "utf8",
6499
timeout: options.timeoutMs,
65100
input: options.input,
@@ -77,13 +112,14 @@ function docker(args: string[], options: { timeoutMs: number; input?: string; qu
77112
export function checkEndpoint(): string | null {
78113
if (process.platform !== "linux") return `${process.platform}: the sandbox needs Linux`;
79114
if (process.getuid?.() === 0) return "the sandbox does not run as root";
80-
const host = docker(["context", "inspect", "--format", "{{.Endpoints.docker.Host}}"], {
81-
timeoutMs: 10_000,
82-
});
83-
if (!host.ok) return `docker: ${host.error}`;
84-
if (!host.stdout.startsWith("unix://"))
85-
return `docker endpoint ${host.stdout}: not a local socket`;
86-
const info = docker(["info", "--format", "{{json .}}"], { timeoutMs: 15_000 });
115+
if (client != null) return null; // resolved and checked once per launcher
116+
const selected = selectedEndpoint();
117+
if ("error" in selected) return `docker: ${selected.error}`;
118+
// Fail closed: a remote, ssh or relative endpoint is refused, never swapped for the default.
119+
if (!/^unix:\/\/\/./.test(selected.host))
120+
return `docker endpoint ${JSON.stringify(selected.host)}: not a local socket`;
121+
const candidate = privateClient(selected.host);
122+
const info = run(candidate, ["info", "--format", "{{json .}}"], { timeoutMs: 15_000 });
87123
if (!info.ok) return `docker info: ${info.error}`;
88124
const daemon = JSON.parse(info.stdout) as {
89125
OSType?: string;
@@ -98,6 +134,7 @@ export function checkEndpoint(): string | null {
98134
return "Docker Desktop is not supported";
99135
if ((daemon.SecurityOptions ?? []).some((o) => o.includes("rootless")))
100136
return "rootless Docker is not supported";
137+
client = candidate;
101138
return null;
102139
}
103140

@@ -267,7 +304,7 @@ interface Job {
267304
async function runContainer(args: string[], job: Job & { dest: string }) {
268305
// The lifeline: this process holds the container's stdin. When it dies, the pipe closes and
269306
// entry.ts stops the job (measured: the container was gone 0.59 s after a SIGKILL).
270-
const child = spawn("docker", args, { env: dockerEnv(), stdio: ["pipe", "pipe", "inherit"] });
307+
const child = spawn("docker", args, { env: clientEnv(), stdio: ["pipe", "pipe", "inherit"] });
271308
child.stdin.on("error", () => undefined);
272309
const exported = receiveExport(child.stdout, job.dest);
273310
let stopped: string | null = null;

0 commit comments

Comments
 (0)