mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
test(runner): own Node executable in ACPX recovery fixtures (#18196)
## Thinking Path > - Paperclip manages AI agents and controls their work. > - The runner recovery tests check durable ACPX session ownership. > - The runner must reject a qualified executable that other users can write. > - The recovery fixture passes the shared host Node executable to that guard. > - A hosted CI job rejected the fixture before it opened a session. > - This pull request gives the fixture its own Node copy with safe permissions. > - The recovery tests can then check recovery without changing the production guard. ## Linked Issues or Issue Description Refs: #18185, #14529. **What happened?** The Runner vitest 2/2 job in [run 40928584358](https://github.com/paperclipai/paperclip/actions/runs/40928584358/job/123645695206) failed five ACPX controller recovery tests. Each test failed at `session.open` with `qualified ACPX must not be group- or world-writable`. The assertion did not retain the failing artifact path or mode. The fixture passes `process.execPath` as its command and digest source. **Expected behavior** A test must supply an owned executable with the permissions that the production guard requires. It must not change the shared host executable or weaken the guard. **Steps to reproduce** Run `acpx-controller-recovery.test.ts` on a host where the shared Node executable permits group writes. The remote job above retains the actual failure. Local validation in this PR does not execute the controller or compile Rust. ## What Changed - Extract the existing Node and Homebrew `libnode` copy logic into a test-support helper. - Use that helper in the existing native distribution fixture. - Use an owned Node copy for the recovery descriptor, qualified command and command digest. - Add filesystem checks for copied bytes, permissions, library selection and unchanged source files. - Ignore only a missing adjacent library directory. Report other read errors with the directory and original cause. ## Verification - Three filesystem-only Vitest tests passed. They use synthetic writable artifacts and copy the actual host Node bytes without executing them. - TypeScript no-emit passed for the new helper and its pure test. - The new real-filesystem `ENOTDIR` control fails against the first PR head and passes after the review fix. The parent returned an incomplete closure; the successor rejects it during preparation. - `git diff --check` passed. - The first-head Build job failed in the public npm consumer check with `ETARGET` for `@aws-sdk/credential-provider-web-identity@^3.972.85`. A later single exact-version official registry metadata request returned HTTP 200 for `3.972.85`. This preserves the original failure and does not establish its cause. No dependency version or check was changed. The source review fix will receive normal fresh-head CI. - The first local test configuration had an import error. The first typecheck invocations had configuration and type-path errors. Corrected invocations passed; those setup failures are retained in the source receipt. - On the reviewed successor, remote Build passed and all five ACPX controller recovery tests passed. The other runner shard failed because the new real-Node Buffer deep-equality assertion exhausted its worker heap. The one-line follow-up uses native Buffer byte equality and keeps the same invariant. Its three filesystem controls and scoped typecheck pass; latest-head normal CI remains required. - Final head `1939f41a7e9cb4031acdf8e99df431b81f28ba03`: both required checks (`ci / verify`, `ci / e2e`) pass. Remote tests confirm 5 recovery tests, 33 native distribution tests and all 3 owned-Node tests pass. Greptile is 5/5 with no open review threads. - A server queue test timed out at 15 seconds on this head. Its actual CI checkout has identical server and dependency sources to the prior passing checkout. One authorized failed-only replay passed at the same checkout: all 105 files and 2,270 tests passed. The specific test took 5.23 seconds. Passing Build and runner jobs were carried forward. The original timeout remains retained; this does not establish its cause. - The actual recovery and native distribution tests, full typecheck, build and full suite were not run locally. Those tests can compile or execute native runtime code. Normal PR CI must verify them before delivery. ## Risks - Moving the existing fixture copy logic could affect platform-specific Node library loading. The helper retains the existing Homebrew library rule and permission modes. Remote CI remains required. - This change does not prove which artifact mode caused the retained CI failure. It removes the fixture's dependence on shared host executable permissions. - Production code, dependency versions, timeouts, retries, workflows and writable-artifact guards are unchanged. ## Model Used OpenAI GPT-6 through Codex, with source inspection and code tools. The exact serving model identifier and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
4a6f6a65eb
commit
d2c7edda16
4 files changed
+88
-10
No files matched your search
@@ -8,6 +8,7 @@ import { resolve } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { beforeAll, expect, it, vi } from "vitest";
|
||||
import { DurablePrpControlPlane, decodeDurablePrpCheckpoint, type DurablePrpCheckpoint } from "./durable-prp-control-plane.js";
|
||||
import { copyOwnedNodeRuntime } from "../test-support/owned-node-runtime.js";
|
||||
import { GENERATED_ACPX_SIDECAR_PROTOCOL_VERSION } from "../drivers/acpx/generated-sidecar-contract.js";
|
||||
import { createCapabilityRunnerdCodexTransport } from "../live/runnerd-codex-transport.js";
|
||||
|
||||
@@ -66,6 +67,7 @@ function sidecarSource(root: string) {
|
||||
|
||||
async function fixture() {
|
||||
const root = mkdtempSync(resolve(tmpdir(), "acpx-controller-recovery-"));
|
||||
const { executable: nodeCommand } = await copyOwnedNodeRuntime(resolve(root, "node"));
|
||||
const stateDirectory = resolve(root, "control-plane");
|
||||
const script = resolve(root, "sidecar.cjs");
|
||||
writeFileSync(script, sidecarSource(root), { mode: 0o700 });
|
||||
@@ -74,7 +76,7 @@ async function fixture() {
|
||||
agent: "claude", model: "scripted-model", acpxVersion: profiles.acpxVersion,
|
||||
agentServerPackage: profile.agentServerPackage, agentServerVersion: profile.agentServerVersion,
|
||||
agentRuntimePackage: profile.agentRuntimePackage, agentRuntimeVersion: profile.agentRuntimeVersion,
|
||||
commandDigest: profile.commandDigest, sidecarCommand: process.execPath, sidecarArgs: [script],
|
||||
commandDigest: profile.commandDigest, sidecarCommand: nodeCommand, sidecarArgs: [script],
|
||||
runtimeDirectory: runtime, normalizedSessionId: identity.normalizedSessionId, runId: identity.runId,
|
||||
cwd: root, instructions: "Do the supplied synthetic task.", permissionMode: "approve-reads", permissionModePinned: true,
|
||||
runtimeContext: null };
|
||||
@@ -95,7 +97,7 @@ async function fixture() {
|
||||
"--run-id",identity.runId,"--session-id",identity.normalizedSessionId,"--turn-id",identity.turnId,"--item-id",identity.itemId,
|
||||
"--runner-version",expectedRunnerVersion,"--runner-digest",expectedRunnerDigest,"--max-runtime-ms","0",
|
||||
"--reconnect-grace-ms","15000","--reconnect-delay-ms","25","--acpx-launch-authority-digest","sha256:"+"d".repeat(64),
|
||||
"--acpx-sidecar-command",process.execPath,"--acpx-sidecar-command-sha256",digest(process.execPath),
|
||||
"--acpx-sidecar-command",nodeCommand,"--acpx-sidecar-command-sha256",digest(nodeCommand),
|
||||
"--acpx-sidecar-script",script,"--acpx-sidecar-script-sha256",digest(script)];
|
||||
const startedAt = new Date().toISOString();
|
||||
const child = spawn(binary,args,{env:{PATH:process.env.PATH,PAPERCLIP_RUNNER_BOOTSTRAP_TICKET:ticket},stdio:["ignore","ignore","pipe"],detached:true});
|
||||
|
||||
@@ -8,6 +8,7 @@ import { tmpdir } from "node:os";
|
||||
import { dirname, join } from "node:path";
|
||||
import type { ChildProcess } from "node:child_process";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { copyOwnedNodeRuntime } from "../../test-support/owned-node-runtime.js";
|
||||
import { awaitVerifiedAcpxProviderExit, awaitVerifiedAcpxProviderOwnership, verifyNativeAcpxInstallation } from "./installation-integrity.js";
|
||||
import { createNativeAcpxDistributionSnapshot, readNativeAcpxDistributionEntries, parseNativeAcpxDistributionEntries, type NativeAcpxDistributionInput, type NativeAcpxDistributionEntry } from "./native-distribution-integrity.js";
|
||||
import { openCodexAcpxRuntime } from "./codex-runtime-adapter.js";
|
||||
@@ -27,14 +28,10 @@ async function fixture(options: { node?: boolean; script?: string } = {}): Promi
|
||||
const root = await mkdtemp(join(tmpdir(), "native-acpx-test-")); roots.push(root);
|
||||
const entries: NativeAcpxDistributionEntry[] = [];
|
||||
if (options.node) {
|
||||
await copyFile(process.execPath, join(root, "runtime")); await chmod(join(root, "runtime"), 0o700);
|
||||
const bytes = await readFile(join(root, "runtime")); entries.push({ path: "runtime", sha256: hash(bytes), size: bytes.length, executable: true });
|
||||
// Homebrew's test-host Node uses an @rpath libnode dylib. Include that
|
||||
// dependency in this fixture's closure instead of relying on its old path.
|
||||
const libraryRoot = join(dirname(process.execPath), "..", "lib");
|
||||
for (const name of (await readdir(libraryRoot).catch(() => [] as string[])).filter(name => /^libnode\..*\.dylib$/.test(name))) {
|
||||
await copyFile(join(libraryRoot, name), join(root, name)); await chmod(join(root, name), 0o600);
|
||||
const library = await readFile(join(root, name)); entries.push({ path: name, sha256: hash(library), size: library.length, executable: false });
|
||||
const { executable, libraries } = await copyOwnedNodeRuntime(root);
|
||||
const bytes = await readFile(executable); entries.push({ path: "runtime", sha256: hash(bytes), size: bytes.length, executable: true });
|
||||
for (const libraryPath of libraries) {
|
||||
const library = await readFile(libraryPath); entries.push({ path: libraryPath.slice(root.length + 1), sha256: hash(library), size: library.length, executable: false });
|
||||
}
|
||||
const script = options.script ?? 'console.log("qualified-entry");';
|
||||
await writeFile(join(root, "entry.cjs"), script, { mode: 0o600 });
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
import { chmod, lstat, mkdir, mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, expect, it } from "vitest";
|
||||
import { copyOwnedNodeRuntime } from "./owned-node-runtime.js";
|
||||
|
||||
const roots: string[] = [];
|
||||
afterEach(async () => {
|
||||
await Promise.all(roots.splice(0).map(root => rm(root, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
it("copies writable host artifacts into an owned closure without changing the source", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "owned-node-test-")); roots.push(root);
|
||||
const bin = join(root, "host", "bin"), lib = join(root, "host", "lib");
|
||||
await mkdir(bin, { recursive: true }); await mkdir(lib);
|
||||
const source = join(bin, "node"), library = join(lib, "libnode.137.dylib");
|
||||
await writeFile(source, "synthetic Node artifact"); await chmod(source, 0o777);
|
||||
await writeFile(library, "synthetic library"); await chmod(library, 0o666);
|
||||
await writeFile(join(lib, "unrelated.dylib"), "not in the Node closure");
|
||||
const sourceMode = (await stat(source)).mode, libraryMode = (await stat(library)).mode;
|
||||
|
||||
const owned = await copyOwnedNodeRuntime(join(root, "owned"), source);
|
||||
|
||||
expect(owned.libraries).toEqual([join(root, "owned", "libnode.137.dylib")]);
|
||||
expect(await readFile(owned.executable)).toEqual(await readFile(source));
|
||||
expect(await readFile(owned.libraries[0]!)).toEqual(await readFile(library));
|
||||
expect((await lstat(owned.executable)).isSymbolicLink()).toBe(false);
|
||||
if (process.platform !== "win32") {
|
||||
expect((await stat(owned.executable)).mode & 0o777).toBe(0o700);
|
||||
expect((await stat(owned.libraries[0]!)).mode & 0o777).toBe(0o600);
|
||||
}
|
||||
expect((await stat(source)).mode).toBe(sourceMode);
|
||||
expect((await stat(library)).mode).toBe(libraryMode);
|
||||
});
|
||||
|
||||
it("copies the real host Node bytes without executing the runtime", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "owned-node-test-")); roots.push(root);
|
||||
const owned = await copyOwnedNodeRuntime(root);
|
||||
expect((await readFile(owned.executable)).equals(await readFile(process.execPath))).toBe(true);
|
||||
expect((await lstat(owned.executable)).isFile()).toBe(true);
|
||||
if (process.platform !== "win32") expect((await stat(owned.executable)).mode & 0o022).toBe(0);
|
||||
});
|
||||
|
||||
it("rejects a broken adjacent library directory instead of returning an incomplete closure", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "owned-node-test-")); roots.push(root);
|
||||
const bin = join(root, "host", "bin"), lib = join(root, "host", "lib");
|
||||
await mkdir(bin, { recursive: true });
|
||||
const source = join(bin, "node");
|
||||
await writeFile(source, "synthetic Node artifact");
|
||||
await writeFile(lib, "a file cannot be the library directory");
|
||||
await expect(copyOwnedNodeRuntime(join(root, "owned"), source)).rejects.toMatchObject({
|
||||
message: `Cannot read Node fixture libraries at ${lib}`,
|
||||
cause: { code: "ENOTDIR" },
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,24 @@
|
||||
import { chmod, copyFile, mkdir, readdir } from "node:fs/promises";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
/** Copy the test host Node closure without changing its shared executable. */
|
||||
export async function copyOwnedNodeRuntime(directory: string, source = process.execPath) {
|
||||
await mkdir(directory, { recursive: true });
|
||||
const executable = join(directory, "runtime");
|
||||
await copyFile(source, executable);
|
||||
await chmod(executable, 0o700);
|
||||
// Homebrew Node resolves its libnode dylib next to the relocated executable.
|
||||
const libraryRoot = join(dirname(source), "..", "lib");
|
||||
const libraries: string[] = [];
|
||||
const names = await readdir(libraryRoot).catch((error: NodeJS.ErrnoException) => {
|
||||
if (error.code === "ENOENT") return [] as string[];
|
||||
throw new Error(`Cannot read Node fixture libraries at ${libraryRoot}`, { cause: error });
|
||||
});
|
||||
for (const name of names.filter(name => /^libnode\..*\.dylib$/.test(name))) {
|
||||
const library = join(directory, name);
|
||||
await copyFile(join(libraryRoot, name), library);
|
||||
await chmod(library, 0o600);
|
||||
libraries.push(library);
|
||||
}
|
||||
return { executable, libraries };
|
||||
}
|
||||
Reference in new issue
Block a user