diff --git a/packages/adapter-utils/src/execution-target-stdin-race.test.ts b/packages/adapter-utils/src/execution-target-stdin-race.test.ts index d86fa0d082e..efe0b7e00e8 100644 --- a/packages/adapter-utils/src/execution-target-stdin-race.test.ts +++ b/packages/adapter-utils/src/execution-target-stdin-race.test.ts @@ -1321,21 +1321,33 @@ describe("deterministic remote process-session wrapper shutdown (PAP-5316)", () }); it("T10 the emitted wrapper strips its own session env vars from the child", async () => { - const wrapper = await startWrapperProcess({ - outputToStdout: true, - command: process.execPath, - args: [ - "-e", - "process.stdout.write(JSON.stringify(Object.keys(process.env).filter((k) => k.startsWith('PAPERCLIP_PROCESS_SESSION'))));process.exit(0)", - ], - }); - await waitFor(() => wrapper.frames.some((frame) => frame.type === "exit"), 4_000); - const text = wrapper.frames - .filter((frame) => frame.type === "data" && frame.stream === "stdout" && typeof frame.data === "string") - .map((frame) => Buffer.from(frame.data as string, "base64").toString("utf8")) - .join(""); - const leakedKeys = JSON.parse(text || "[]") as string[]; - expect(leakedKeys).toEqual([]); + for (const outputToStdout of [true, false]) { + const wrapper = await startWrapperProcess({ + outputToStdout, + command: process.execPath, + args: [ + "-e", + "process.stdout.write(JSON.stringify(Object.keys(process.env).filter((k) => k.startsWith('PAPERCLIP_PROCESS_SESSION'))));process.exit(0)", + ], + // Drive all four of the wrapper's own session variables through the + // harness, so the assertion below proves the child sees none of + // them -- not just the two the harness happens to set by default. + env: { + PAPERCLIP_PROCESS_SESSION_LEASE_REFRESH_MS: "5000", + PAPERCLIP_PROCESS_SESSION_LEASE_TTL_MS: "15000", + }, + }); + // The stdout variant reports its frames on `wrapper.frames`; the + // event-file variant writes them under `eventsDir` instead. + const getFrames = async (): Promise => (outputToStdout ? wrapper.frames : wrapper.readEventFiles()); + await waitFor(async () => (await getFrames()).some((frame) => frame.type === "exit"), 4_000); + const text = (await getFrames()) + .filter((frame) => frame.type === "data" && frame.stream === "stdout" && typeof frame.data === "string") + .map((frame) => Buffer.from(frame.data as string, "base64").toString("utf8")) + .join(""); + const leakedKeys = JSON.parse(text || "[]") as string[]; + expect(leakedKeys).toEqual([]); + } }); it("T11 each wrapper source has exactly one spawn call site and every kill call is child.kill()", () => { @@ -2216,6 +2228,11 @@ describe("deterministic remote process-session wrapper shutdown (PAP-5316)", () expect(call.bypassSession).toBe(true); expect(call.script).not.toContain("mkdir"); expect(call.script).toContain("mv "); + // The rename must run only when the exclusive create succeeds: the + // create and the `mv` sit on the same line, joined by `&&`, not as + // two separate unconditional statements. + const createAndRenameLine = call.script.split("\n").find((line) => line.includes("mv ")); + expect(createAndRenameLine).toMatch(/^: > .+&&\s*mv /); } }, 10_000); diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index 6406e48403a..73cd2329142 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -1898,14 +1898,15 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { // O_CREAT|O_EXCL: it fails on an existing file and on a symbolic // link, so it can neither follow a link nor truncate a peer's file. "set -C", - `: > ${shellQuote(tempPath)}`, - // `mv` replaces the lease path by rename. A rename does not follow a - // symbolic link at the target, so a peer that pre-creates a link at - // the fixed lease path cannot redirect this write. This command - // contains no `mkdir`: when `stop()` already removed `sessionDir`, - // the create above fails, this refresh fails, and the wrapper - // expires -- the wanted outcome, not a defect. - `mv ${shellQuote(tempPath)} ${shellQuote(leasePath)}`, + // The `&&` runs the rename only when the exclusive create above it + // succeeds. `mv` replaces the lease path by rename. A rename does not + // follow a symbolic link at the target, so a peer that pre-creates a + // link at the fixed lease path cannot redirect this write. This + // command contains no `mkdir`: when `stop()` already removed + // `sessionDir`, the create fails, the `&&` skips the rename, this + // refresh fails, and the wrapper expires -- the wanted outcome, not a + // defect. + `: > ${shellQuote(tempPath)} && mv ${shellQuote(tempPath)} ${shellQuote(leasePath)}`, ].join("\n"); try { await runner.execute({ @@ -2977,12 +2978,15 @@ if ((await isSymbolicLink(sessionDir)) || (await isSymbolicLink(stdinDir))) { process.exit(1); } -// Hardening (I3, not containment): the wrapper's own launch env carries the -// session dir and the command payload. Scrub both keys before they reach the -// spawned child, so the child never inherits a path to its own control files. +// Hardening (I3, not containment): the wrapper's own launch env carries its +// own session control state. Remove every one of the wrapper's own session +// variables before they reach the spawned child, so the child never inherits +// the wrapper's own control state. const childEnv = { ...process.env, ...(config.env || {}) }; delete childEnv.PAPERCLIP_PROCESS_SESSION_DIR; delete childEnv.PAPERCLIP_PROCESS_SESSION_COMMAND_B64; +delete childEnv.PAPERCLIP_PROCESS_SESSION_LEASE_REFRESH_MS; +delete childEnv.PAPERCLIP_PROCESS_SESSION_LEASE_TTL_MS; // I1: exactly one child process per emitted wrapper. Do not add a second // tracked child handle. @@ -3068,12 +3072,15 @@ if ((await isSymbolicLink(sessionDir)) || (await isSymbolicLink(stdinDir))) { process.exit(1); } -// Hardening (I3, not containment): the wrapper's own launch env carries the -// session dir and the command payload. Scrub both keys before they reach the -// spawned child, so the child never inherits a path to its own control files. +// Hardening (I3, not containment): the wrapper's own launch env carries its +// own session control state. Remove every one of the wrapper's own session +// variables before they reach the spawned child, so the child never inherits +// the wrapper's own control state. const childEnv = { ...process.env, ...(config.env || {}) }; delete childEnv.PAPERCLIP_PROCESS_SESSION_DIR; delete childEnv.PAPERCLIP_PROCESS_SESSION_COMMAND_B64; +delete childEnv.PAPERCLIP_PROCESS_SESSION_LEASE_REFRESH_MS; +delete childEnv.PAPERCLIP_PROCESS_SESSION_LEASE_TTL_MS; // I1: exactly one child process per emitted wrapper. Do not add a second // tracked child handle.