fix(adapter-utils): scrub lease timing vars from the wrapper child env

The wrapper deleted only two of its four own session variables from
the spawned child's environment, so the child still inherited the
lease refresh and TTL overrides. T10 asserted an empty leaked-key
list but never set those two variables, so it passed without proving
the scrub. Delete all four variables in both wrapper variants and
extend T10 to set all four through the harness on both variants.

The lease refresh script ran `mv` even when the exclusive create
failed, because the two commands were separate statements with no
`&&`. Join them so the rename runs only after the create succeeds,
and extend T29 to assert that chained shape.

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Priya RamanandPaperclip committed 2026-08-26 23:19:00 +00:00
1 parent 97879de951
commit b7ec2ff90b
2 files changed
+53 -29

No files matched your search

@@ -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<WrapperFrame[]> => (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);
+21 -14
View File
@@ -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.