RomanNum3ral
/
paperclip
mirror of https://github.com/paperclipai/paperclip.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(adapter-utils): move the workspace-restore merge lock to an instance-scoped root and surface restore failures on the run (#12187) 

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Agent adapters restore sandbox work into project workspaces after a
run
> - The restore lock used the target workspace parent, which can reject
writes
> - The teardown then hid restore errors, so a run could report success
with lost work
> - This pull request moves the lock into an instance-scoped root and
reports safe restore failure codes
> - The benefit is reliable restore coordination and visible failure
evidence without changing run success semantics

## Linked Issues or Issue Description

Refs: #10914

## What Changed

- Move the workspace-restore merge lock into a private, instance-scoped
root.
- Derive the lock key from the canonical target path with SHA-256.
- Resolve the lock root from the caller environment and reject unsafe
root types.
- Classify restore failures with three allowlisted codes.
- Add the failure code to run result JSON without exposing a host path
or process identifier.
- Keep restore failure fail-open for the run exit code and run status.

## Verification

- Run `npx vitest run
packages/adapter-utils/src/workspace-restore-merge.test.ts`.
- Run `npx vitest run
packages/adapter-utils/src/acpx-engine/run-fault-matrix.test.ts`.
- Run the four Codex credential suites.
- Confirm the branch includes the current `master` commit and no manual
lockfile edit.
- Confirm all pull request checks and the Greptile review reach a
terminal green state.

## Risks

- The lock path changes for workspace restore and removes the
sibling-directory fallback.
- A misconfigured or inaccessible instance home can still stop lock
setup.
- Restore remains fail-open, so callers must inspect the result evidence
when a restore fails.

## Model Used

OpenAI GPT-5. The model used tool calls and code execution to validate
and route an author-provided change. The implementing engineer authored
the code.

## 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:
Nicky Leach 2026-08-25 14:32:14 -07:00 committed by GitHub
parent d866ff374e
commit 822e0aed93
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
22 changed files with 1168 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
packages/adapter-utils/src/acpx-engine/execute.test.ts
View File

@ -3019,6 +3019,7 @@ describe("ACPX engine remote managed-home seam (PR 2: per-adapter home seed)", (
stagedRuntime,
teardown: async () => {
teardownCalls += 1;
return { ok: true };
},
};
},
@ -3362,6 +3363,7 @@ describe("ACPX engine remote session-lifecycle re-staging (PR 3: stage once / re
stagedRuntime,
teardown: async () => {
teardownCalls += 1;
return { ok: true };
},
disposeStaged: async () => {
disposeCalls += 1;
@ -3406,6 +3408,7 @@ describe("ACPX engine remote session-lifecycle re-staging (PR 3: stage once / re
stagedRuntime: await input.stage([]),
teardown: async () => {
teardownCalls += 1;
return { ok: true };
},
disposeStaged: async () => {
disposeCalls += 1;
@ -4675,6 +4678,7 @@ describe("ACPX engine sandbox-start spans (opt-in root + child parenting)", () =
teardownExecFired = true;
issueSandboxExecFromStore(traceContext);
}
return { ok: true };
},
};
},

78
packages/adapter-utils/src/acpx-engine/execute.ts
View File

@ -37,6 +37,11 @@ import {
} from "@paperclipai/adapter-utils/execution-target";
import type { DuplexLossReason } from "../duplex-observability.js";
import { DUPLEX_CHANNEL_LOST_ERROR_CODE } from "../bridge-transport-contract.js";
import type { WorkspaceRestoreFailureCode, WorkspaceRestoreOutcome } from "../workspace-restore-merge.js";
import {
classifyWorkspaceRestoreFailure,
describeWorkspaceRestoreFailure,
} from "../workspace-restore-merge.js";
import {
DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE,
applyPaperclipWorkspaceEnv,
@ -218,7 +223,7 @@ export interface StagedRuntimeCacheEntry {
* in-sandbox credential, not a stale snapshot. It never removes the staged
* in-sandbox home, so re-running it on each reuse can't invalidate this entry.
*/
teardown: (() => Promise<void>) | null;
teardown: (() => Promise<WorkspaceRestoreOutcome>) | null;
/**
* The seam's one-time host-side staged-resource cleanup (e.g. remove the
* staged home temp dir), or null. Fired ONLY when this entry is dropped —
@ -315,7 +320,7 @@ export interface AcpxRemoteManagedHomeResult {
* cached staged runtime across resumes never destroys resources a later run
* still needs.
*/
teardown?: () => Promise<void>;
teardown?: () => Promise<WorkspaceRestoreOutcome>;
/**
* One-time cleanup of host-side staged resources (e.g. the curated staged
* home temp dir). Split out from {@link teardown} so it fires ONLY when the
@ -420,7 +425,7 @@ interface AcpxPreparedRuntime {
// exit path by the settlement `syncBack` step; it never removes staged temp, so
// it is safe on every compatible resume. Null for local runs, the runner-less
// fallback, and adapters with no seam.
remoteManagedHomeTeardown: (() => Promise<void>) | null;
remoteManagedHomeTeardown: (() => Promise<WorkspaceRestoreOutcome>) | null;
// One-time host-side staged-resource cleanup from the seam (remove staged temp
// dirs). Fired ONLY when the staged runtime is dropped (failed/cancelled/timed
// -out turn, incompatible re-stage, idle eviction), not on a clean turn that
@ -2013,7 +2018,7 @@ async function buildRuntime(input: {
remoteExecutionIdentity,
});
let stagedRuntime: PreparedAdapterExecutionTargetRuntime | null = null;
let remoteManagedHomeTeardown: (() => Promise<void>) | null = null;
let remoteManagedHomeTeardown: (() => Promise<WorkspaceRestoreOutcome>) | null = null;
let remoteStagingDispose: (() => Promise<void>) | null = null;
let remoteStagingEnvDelta: Record<string, string> | null = null;
let sessionStagingLeaseRelease: (() => void) | null = null;
@ -2364,14 +2369,22 @@ async function stopRunTransport(prepared: AcpxPreparedRuntime): Promise<void> {
// dirs. The seam logs and swallows its own failures — an unclean-teardown
// copy-back miss is the accepted, loud `refresh_token_reused` residual on the
// next host Codex use, never silent HOST-credential corruption — so a teardown
// fault never masks or fails the run result here.
async function syncBackManagedHome(prepared: AcpxPreparedRuntime): Promise<void> {
if (prepared.remoteManagedHomeTeardown) {
await prepared.remoteManagedHomeTeardown().catch(() => {});
// fault never masks or fails the run result here. It still returns the restore
// outcome, so the caller can record a failure on the run record; the run's exit
// code and status stay exactly what the turn produced.
// The per-session staging lease does NOT release here. The settlement releases
// it last, in its own `finally`, so a same-session second run cannot re-stage
// until this run fully settles and the caller observes the result.
async function syncBackManagedHome(prepared: AcpxPreparedRuntime): Promise<WorkspaceRestoreOutcome> {
if (!prepared.remoteManagedHomeTeardown) {
return { ok: true };
}
// The per-session staging lease does NOT release here. The settlement releases
// it last, in its own `finally`, so a same-session second run cannot re-stage
// until this run fully settles and the caller observes the result.
// The teardown closure already catches and logs its own error (fail-soft);
// this `.catch` is defense in depth for the case where it rejects anyway, so
// a teardown fault can never propagate out of settlement.
return await prepared
.remoteManagedHomeTeardown()
.catch((): WorkspaceRestoreOutcome => ({ ok: false, code: "restore_failed" }));
}
/** How the settlement `endSession` step releases the runtime a run acquired. */
@ -3442,8 +3455,29 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) {
let referencedProjectStagingFailuresField:
| { referencedProjectStagingFailures: Array<{ projectId: string; error: string }> }
| Record<string, never> = {};
// Set only when the settlement sync-back reports a failed workspace
// restore. `reproduceResult` merges this into the returned result's
// `resultJson`, so a clean run adds no new key. The run's exit code and
// status stay exactly what the turn produced — this is a signal, not an
// outcome change.
let workspaceRestoreFailureField:
| { workspaceRestoreFailure: WorkspaceRestoreFailureCode }
| Record<string, never> = {};
// The one settlement step name whose error can be the same workspace-
// restore failure the adapter teardown closure already classifies (a
// caught error from `syncBackManagedHome`). The closure already
// sanitizes its own `onLog` line; this is a second, independent layer,
// so a defect in that closure (or a future call site that forgets to
// sanitize) still cannot put a raw `Error.message` — and the host path
// or process id it can carry — on the run log.
const SYNC_BACK_SETTLEMENT_STEP = "settlement-sync_back";
const recordTeardownError = async (step: string, teardownErr: unknown) => {
const reason = teardownErr instanceof Error ? teardownErr.message : String(teardownErr);
const reason =
step === SYNC_BACK_SETTLEMENT_STEP
? describeWorkspaceRestoreFailure(classifyWorkspaceRestoreFailure(teardownErr))
: teardownErr instanceof Error
? teardownErr.message
: String(teardownErr);
await ctx
.onLog("stderr", `[paperclip] ACPX teardown step "${step}" failed: ${reason}\n`)
.catch(() => {});
@ -4445,7 +4479,10 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) {
// per-task restore spans parent to `sandbox.syncBack`.
syncBack: () => timedPhase("sync_back", async () => {
await runRuntimeSpan("sandbox.syncBack", async () => {
await syncBackManagedHome(prepared);
const restoreOutcome = await syncBackManagedHome(prepared);
if (!restoreOutcome.ok) {
workspaceRestoreFailureField = { workspaceRestoreFailure: restoreOutcome.code };
}
});
}),
// The staging lease releases as the run's final act, AFTER the coordinator
@ -4490,7 +4527,20 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) {
if (!capturedResult) {
throw new Error("run coordinator reproduced a result before the run recorded one");
}
return capturedResult;
// The sync-back settlement step runs before this reproduces the result
// (settlement precedes reproduction), so a failed workspace restore is
// already recorded by the time we get here. Merge it into `resultJson`
// only on a failure — a clean restore adds no new key.
if (!("workspaceRestoreFailure" in workspaceRestoreFailureField)) {
return capturedResult;
}
return {
...capturedResult,
resultJson: {
...(capturedResult.resultJson ?? {}),
...workspaceRestoreFailureField,
},
};
},
};
return await runAttempt(plan);

10
packages/adapter-utils/src/acpx-engine/run-contracts.ts
View File

@ -18,6 +18,7 @@ import type {
AdapterExecutionTargetProcessSessionBridgeHandle,
PreparedAdapterExecutionTargetRuntime,
} from "@paperclipai/adapter-utils/execution-target";
import type { WorkspaceRestoreOutcome } from "../workspace-restore-merge.js";
// ---------------------------------------------------------------------------
// Resource identity
@ -92,8 +93,13 @@ export interface StagedRuntimeResource {
* `AcpxPreparedRuntime.remoteManagedHomeTeardown` and `remoteStagingEnvDelta`.
*/
export interface ManagedHomeResource {
/** Per-run copy-back hook. Runs the auth copy-back on every exit path. */
readonly teardown: () => Promise<void>;
/**
* Per-run copy-back hook. Runs the auth copy-back and the workspace restore
* on every exit path. Resolves to the workspace-restore outcome — never
* rejects — so the run coordinator can surface a failed restore on the run
* record without changing the run's exit code or status.
*/
readonly teardown: () => Promise<WorkspaceRestoreOutcome>;
/** The env keys the seam mutated on this run. */
readonly envDelta: Record<string, string>;
}

76
packages/adapter-utils/src/acpx-engine/run-fault-matrix.test.ts
View File

@ -8,6 +8,7 @@ import {
startAdapterExecutionTargetProcessSessionBridge,
} from "@paperclipai/adapter-utils/execution-target";
import { runChildProcess } from "../server-utils.js";
import { classifyWorkspaceRestoreFailure } from "../workspace-restore-merge.js";
// The composed fault matrix.
//
@ -213,7 +214,22 @@ function managedHomeSeed(options: { teardownRejects?: boolean } = {}): AcpxEngin
? async () => {
throw new Error("copy-back boom");
}
: async () => {},
: async () => ({ ok: true as const }),
};
};
}
// Same seam, but the teardown catches its own restore error and classifies it
// with the real classifier — the way the three real adapter teardown closures
// do — instead of letting it reject the promise.
function managedHomeSeedWithClassifiedTeardownFailure(
error: NodeJS.ErrnoException,
): AcpxEngineExecutorOptions["prepareRemoteManagedHome"] {
return async (input) => {
const stagedRuntime = await input.stage([]);
return {
stagedRuntime,
teardown: async () => ({ ok: false as const, code: classifyWorkspaceRestoreFailure(error) }),
};
};
}
@ -689,7 +705,8 @@ describe("composed ACPX run fault matrix", () => {
});
// Case 14 — a sync-back copy-back that rejects during settlement never changes
// the result or the report.
// the run's exit code or status, and it surfaces on the result as one
// allowlisted `workspaceRestoreFailure` code — never the raw error message.
it("case_14_sync_back_failure_does_not_change_result", async () => {
const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox();
stubBridges();
@ -710,12 +727,67 @@ describe("composed ACPX run fault matrix", () => {
expect(result.exitCode).toBe(0);
expect(result.resultJson?.status).toBe("completed");
expect(result.resultJson?.workspaceRestoreFailure).toBe("restore_failed");
expect(JSON.stringify(result.resultJson)).not.toContain("copy-back boom");
assertDispositionReport(capture.last(), {
acquired: SANDBOX_WITH_MANAGED_HOME,
transferred: "staged_runtime",
});
});
// Case 14b — a clean sync-back adds no `workspaceRestoreFailure` key at all.
it("case_14b_clean_sync_back_adds_no_workspace_restore_failure_key", async () => {
const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox();
stubBridges();
const execute = createAcpxEngineExecutor({
warmHandles: new Map(),
stagedRuntimes: new Map(),
stagingLocks: new Map(),
createRuntime: () => completedRuntime(),
prepareRemoteManagedHome: managedHomeSeed(),
});
const result = await execute({
runId: "fault-sync-back-clean",
...remoteArgs(stateDir, localCwd, executionTarget),
} as never);
expect(result.exitCode).toBe(0);
expect(result.resultJson).not.toHaveProperty("workspaceRestoreFailure");
});
// Case 14c — an EACCES teardown error (the reported lock-mkdir bug) reaches
// the run result as the allowlisted restore_permission_denied code, with no
// filesystem path and no process id in resultJson, and the run's exit code
// and status stay exactly what the turn produced.
it("case_14c_eacces_teardown_reaches_result_as_restore_permission_denied", async () => {
const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox();
stubBridges();
const eaccesError: NodeJS.ErrnoException = new Error(
`EACCES: permission denied, mkdir '/srv/telemetry-backend.paperclip-restore.lock' (pid ${process.pid})`,
);
eaccesError.code = "EACCES";
const execute = createAcpxEngineExecutor({
warmHandles: new Map(),
stagedRuntimes: new Map(),
stagingLocks: new Map(),
createRuntime: () => completedRuntime(),
prepareRemoteManagedHome: managedHomeSeedWithClassifiedTeardownFailure(eaccesError),
});
const result = await execute({
runId: "fault-sync-back-eacces",
...remoteArgs(stateDir, localCwd, executionTarget),
} as never);
expect(result.exitCode).toBe(0);
expect(result.resultJson?.status).toBe("completed");
expect(result.resultJson?.workspaceRestoreFailure).toBe("restore_permission_denied");
const serializedResult = JSON.stringify(result.resultJson);
expect(serializedResult).not.toContain("/srv/telemetry-backend");
expect(serializedResult).not.toContain(String(process.pid));
});
// Case 15 — a concurrent double settlement is a structural error: the second
// claim of the ledger throws a LedgerStateError.
it("case_15_concurrent_double_settlement_throws_ledger_state_error", async () => {

8
packages/adapter-utils/src/acpx-engine/run-site-sandbox.test.ts
View File

@ -78,7 +78,11 @@ function makeSite(overrides: Partial<SandboxRunSiteOptions> = {}) {
const stagedRuntime = await stage([]);
// Repoint a managed-home env var, so the site captures the delta.
env.CODEX_HOME = "/remote/home";
return { stagedRuntime, teardown: async () => {}, dispose: async () => {} } satisfies ManagedHomeSeamResult;
return {
stagedRuntime,
teardown: async () => ({ ok: true }),
dispose: async () => {},
} satisfies ManagedHomeSeamResult;
},
disposeFreshStagedRuntime: async () => {},
measureStageStep: (run) => run(),
@ -246,7 +250,7 @@ describe("sandbox run site", () => {
{
stagedRuntime: cachedRuntime,
envDelta: { CODEX_HOME: "/remote/home" },
teardown: async () => {},
teardown: async () => ({ ok: true }),
dispose: async () => {},
lastUsedAt: 500,
},

7
packages/adapter-utils/src/acpx-engine/run-site-sandbox.ts
View File

@ -41,6 +41,7 @@ import type {
SitePlan,
} from "./run-contracts.js";
import { createSessionReuseStore } from "./session-reuse-store.js";
import type { WorkspaceRestoreOutcome } from "../workspace-restore-merge.js";
/**
* One staged in-sandbox runtime the store keeps for the next compatible resume.
@ -52,7 +53,7 @@ import { createSessionReuseStore } from "./session-reuse-store.js";
export interface StagedRuntimeStoreEntry {
stagedRuntime: PreparedAdapterExecutionTargetRuntime;
envDelta: Record<string, string>;
teardown: (() => Promise<void>) | null;
teardown: (() => Promise<WorkspaceRestoreOutcome>) | null;
dispose: (() => Promise<void>) | null;
lastUsedAt: number;
}
@ -63,7 +64,7 @@ export interface StagedRuntimeStoreEntry {
*/
export interface StagedWorkspace {
readonly stagedRuntime: PreparedAdapterExecutionTargetRuntime;
readonly teardown: (() => Promise<void>) | null;
readonly teardown: (() => Promise<WorkspaceRestoreOutcome>) | null;
readonly dispose: (() => Promise<void>) | null;
readonly envDelta: Record<string, string>;
/** True when the run reused an already-staged runtime rather than staging fresh. */
@ -79,7 +80,7 @@ export type StageWorkspace = (assets: AdapterManagedRuntimeAsset[]) => Promise<P
*/
export interface ManagedHomeSeamResult {
readonly stagedRuntime: PreparedAdapterExecutionTargetRuntime;
readonly teardown: (() => Promise<void>) | null;
readonly teardown: (() => Promise<WorkspaceRestoreOutcome>) | null;
readonly dispose: (() => Promise<void>) | null;
}

2
packages/adapter-utils/src/acpx-engine/settlement-characterization.test.ts
View File

@ -334,6 +334,7 @@ describe("ACP settlement — Layer A: engine teardown orchestration", () => {
// The lease is still held while the sync-back runs; it releases only
// afterward, in the cleanupRemoteBridges finally.
leaseSizeDuringSyncBack = stagingLocks.size;
return { ok: true };
},
};
},
@ -1046,6 +1047,7 @@ describe("ACP settlement — Layer C: per-adapter sync-back teardown fires once
stagedRuntime,
teardown: async () => {
teardownCalls += 1;
return { ok: true };
},
};
},

458
packages/adapter-utils/src/workspace-restore-merge.test.ts
View File

@ -1,10 +1,20 @@
import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import { createHash } from "node:crypto";
import { promises as fsPromises } from "node:fs";
import { chmod, lstat, mkdir, mkdtemp, readFile, readdir, realpath, rm, stat, symlink, writeFile } from "node:fs/promises";
import net from "node:net";
import os from "node:os";
import path from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import { captureDirectorySnapshot, mergeDirectoryWithBaseline } from "./workspace-restore-merge.js";
import { resolvePaperclipInstanceRootForAdapter } from "./server-utils.js";
import {
captureDirectorySnapshot,
classifyWorkspaceRestoreFailure,
describeWorkspaceRestoreFailure,
mergeDirectoryWithBaseline,
withDirectoryMergeLock,
WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE,
} from "./workspace-restore-merge.js";
describe("workspace restore merge", () => {
const cleanupDirs: string[] = [];
@ -81,4 +91,446 @@ describe("workspace restore merge", () => {
await new Promise<void>((resolve) => server.close(() => resolve()));
}
});
describe("classifyWorkspaceRestoreFailure", () => {
it("maps an EACCES error to restore_permission_denied", () => {
const error: NodeJS.ErrnoException = new Error("permission denied");
error.code = "EACCES";
expect(classifyWorkspaceRestoreFailure(error)).toBe("restore_permission_denied");
});
it("maps an EPERM error to restore_permission_denied", () => {
const error: NodeJS.ErrnoException = new Error("operation not permitted");
error.code = "EPERM";
expect(classifyWorkspaceRestoreFailure(error)).toBe("restore_permission_denied");
});
it("maps the lock-timeout code to restore_lock_timeout", () => {
const error: NodeJS.ErrnoException = new Error("Timed out waiting for workspace restore lock at /some/path");
error.code = WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE;
expect(classifyWorkspaceRestoreFailure(error)).toBe("restore_lock_timeout");
});
it("maps an unrecognized error, a string, and null to the default restore_failed code", () => {
expect(classifyWorkspaceRestoreFailure(new Error("some other failure"))).toBe("restore_failed");
expect(classifyWorkspaceRestoreFailure("a plain string")).toBe("restore_failed");
expect(classifyWorkspaceRestoreFailure(null)).toBe("restore_failed");
});
});
describe("describeWorkspaceRestoreFailure", () => {
it("returns one fixed diagnostic line per allowlisted code, and no other text", () => {
expect(describeWorkspaceRestoreFailure("restore_permission_denied")).toBe(
"the restore could not write to the workspace (permission denied)",
);
expect(describeWorkspaceRestoreFailure("restore_lock_timeout")).toBe(
"the restore timed out waiting for the workspace merge lock",
);
expect(describeWorkspaceRestoreFailure("restore_failed")).toBe("the restore failed");
});
it("never reflects a sentinel host path or process id, however the caught error is classified", () => {
const sentinelPath = "/srv/telemetry-backend";
const sentinelPid = String(process.pid);
const error: NodeJS.ErrnoException = new Error(
`EACCES: permission denied, mkdir '${sentinelPath}.paperclip-restore.lock' (pid ${sentinelPid})`,
);
error.code = "EACCES";
const line = describeWorkspaceRestoreFailure(classifyWorkspaceRestoreFailure(error));
expect(line).not.toContain(sentinelPath);
expect(line).not.toContain(sentinelPid);
expect(line).not.toContain(error.message);
});
});
describe("instance-scoped directory merge lock", () => {
// Points PAPERCLIP_HOME (and, where noted, PAPERCLIP_INSTANCE_ID) at a
// temporary directory so the lock root never touches the real Paperclip
// instance, then restores the previous values. Mirrors the save-and-restore
// pattern in acpx-engine/execute.test.ts.
let previousHome: string | undefined;
let previousInstanceId: string | undefined;
function useTempPaperclipHome(homeDir: string, instanceId: string): void {
previousHome = process.env.PAPERCLIP_HOME;
previousInstanceId = process.env.PAPERCLIP_INSTANCE_ID;
process.env.PAPERCLIP_HOME = homeDir;
process.env.PAPERCLIP_INSTANCE_ID = instanceId;
}
afterEach(() => {
if (previousHome === undefined) delete process.env.PAPERCLIP_HOME;
else process.env.PAPERCLIP_HOME = previousHome;
if (previousInstanceId === undefined) delete process.env.PAPERCLIP_INSTANCE_ID;
else process.env.PAPERCLIP_INSTANCE_ID = previousInstanceId;
previousHome = undefined;
previousInstanceId = undefined;
});
it.skipIf(process.platform === "win32")(
"restores successfully when the parent directory of the target is not writable",
async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
useTempPaperclipHome(path.join(rootDir, "paperclip-home"), "test-instance");
// The old lock sat beside the target, so it needed mkdir rights in the
// target's parent. The new lock root lives under PAPERCLIP_HOME instead,
// so a read-only parent must no longer block a restore.
const readOnlyParent = path.join(rootDir, "read-only-parent");
const targetDir = path.join(readOnlyParent, "target");
const sourceDir = path.join(rootDir, "source");
await mkdir(targetDir, { recursive: true });
await mkdir(sourceDir, { recursive: true });
const baseline = await captureDirectorySnapshot(targetDir, { exclude: [] });
await writeFile(path.join(sourceDir, "new-file.md"), "new content\n", "utf8");
await chmod(readOnlyParent, 0o500);
try {
await mergeDirectoryWithBaseline({ baseline, sourceDir, targetDir });
} finally {
// Restore write access so the outer afterEach can remove rootDir.
await chmod(readOnlyParent, 0o700).catch(() => undefined);
}
await expect(readFile(path.join(targetDir, "new-file.md"), "utf8")).resolves.toBe("new content\n");
},
);
it.skipIf(process.platform === "win32")(
"acquires the same lock for two alias paths that resolve to one canonical target",
async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const paperclipHome = path.join(rootDir, "paperclip-home");
useTempPaperclipHome(paperclipHome, "test-instance");
const targetDir = path.join(rootDir, "target");
const aliasDir = path.join(rootDir, "target-alias");
await mkdir(targetDir, { recursive: true });
await symlink(targetDir, aliasDir);
const lockRootDir = path.join(paperclipHome, "instances", "test-instance", "locks", "directory-merge");
let lockNameViaTarget = "";
await withDirectoryMergeLock(targetDir, async () => {
const entries = await readdir(lockRootDir);
lockNameViaTarget = entries[0] ?? "";
});
let lockNameViaAlias = "";
await withDirectoryMergeLock(aliasDir, async () => {
const entries = await readdir(lockRootDir);
lockNameViaAlias = entries[0] ?? "";
});
expect(lockNameViaTarget).not.toBe("");
expect(lockNameViaAlias).toBe(lockNameViaTarget);
},
);
it("rejects a lock root that already exists as a symlink", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const paperclipHome = path.join(rootDir, "paperclip-home");
useTempPaperclipHome(paperclipHome, "test-instance");
const locksDir = path.join(paperclipHome, "instances", "test-instance", "locks");
const decoyDir = path.join(rootDir, "decoy");
await mkdir(locksDir, { recursive: true });
await mkdir(decoyDir, { recursive: true });
await symlink(decoyDir, path.join(locksDir, "directory-merge"));
const targetDir = path.join(rootDir, "target");
await mkdir(targetDir, { recursive: true });
await expect(withDirectoryMergeLock(targetDir, async () => undefined)).rejects.toThrow(
/not a plain directory/,
);
});
it("rejects a lock root that already exists as a non-directory", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const paperclipHome = path.join(rootDir, "paperclip-home");
useTempPaperclipHome(paperclipHome, "test-instance");
const locksDir = path.join(paperclipHome, "instances", "test-instance", "locks");
await mkdir(locksDir, { recursive: true });
await writeFile(path.join(locksDir, "directory-merge"), "not a directory\n", "utf8");
const targetDir = path.join(rootDir, "target");
await mkdir(targetDir, { recursive: true });
await expect(withDirectoryMergeLock(targetDir, async () => undefined)).rejects.toThrow(
/not a plain directory/,
);
});
it("closes the create/validate TOCTOU window: rejects a lock root a racing writer swapped for a symlink during creation", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const paperclipHome = path.join(rootDir, "paperclip-home");
useTempPaperclipHome(paperclipHome, "test-instance");
const targetDir = path.join(rootDir, "target");
await mkdir(targetDir, { recursive: true });
const decoyDir = path.join(rootDir, "decoy");
await mkdir(decoyDir, { recursive: true });
// Pre-create the lock root's parent, so the mock below only has to
// reproduce what `fs.mkdir({ recursive: true })` does to the leaf path.
await mkdir(path.join(paperclipHome, "instances", "test-instance", "locks"), { recursive: true });
// Real `fs.mkdir({ recursive: true })` does not fail on a leaf that
// already exists as a symlink to a real directory. This stub reproduces
// exactly that: it plants a symlink to the attacker-controlled decoy
// directory in the window between the resolver's own "does the root
// exist yet" check and its own `mkdir` call, then resolves the way a
// real `mkdir` would (silently) — proving the resolver must validate
// what `mkdir` actually left behind, not trust that the call resolved.
const mkdirSpy = vi.spyOn(fsPromises, "mkdir").mockImplementationOnce(async (dirPath) => {
await symlink(decoyDir, dirPath as string);
return undefined;
});
try {
await expect(withDirectoryMergeLock(targetDir, async () => undefined)).rejects.toThrow(
/not a plain directory/,
);
} finally {
mkdirSpy.mockRestore();
}
});
it("creates the lock root at mode 0o700 and removes the lock directory after release", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const paperclipHome = path.join(rootDir, "paperclip-home");
useTempPaperclipHome(paperclipHome, "test-instance");
const targetDir = path.join(rootDir, "target");
await mkdir(targetDir, { recursive: true });
const lockRootDir = path.join(paperclipHome, "instances", "test-instance", "locks", "directory-merge");
let entriesDuringLock: string[] = [];
await withDirectoryMergeLock(targetDir, async () => {
entriesDuringLock = await readdir(lockRootDir);
});
expect((await stat(lockRootDir)).mode & 0o777).toBe(0o700);
expect(entriesDuringLock).toHaveLength(1);
await expect(readdir(lockRootDir)).resolves.toHaveLength(0);
});
it("classifies the real lock-timeout error by its stable code, never by the message text", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const paperclipHome = path.join(rootDir, "paperclip-home");
useTempPaperclipHome(paperclipHome, "test-instance");
const targetDir = path.join(rootDir, "target");
await mkdir(targetDir, { recursive: true });
// Pre-create the lock directory a live process holds, so `isLockStale`
// never reports it stale and the retry loop can only leave through the
// deadline check. The owner pid is this test process, which stays alive.
const canonicalTargetDir = await realpath(targetDir);
const lockKey = createHash("sha256").update(canonicalTargetDir).digest("hex");
const lockRootDir = path.join(paperclipHome, "instances", "test-instance", "locks", "directory-merge");
const heldLockDir = path.join(lockRootDir, `${lockKey}.lock`);
await mkdir(heldLockDir, { recursive: true });
await writeFile(
path.join(heldLockDir, "owner.json"),
`${JSON.stringify({ pid: process.pid, createdAt: new Date().toISOString() })}\n`,
"utf8",
);
// Reach the real deadline without a real 30-second wait: the first
// `Date.now()` call computes the deadline (unchanged), and every call
// after reports a time far past it, so the retry loop's own deadline
// check — not a mocked message or a shortened constant — throws.
const realNow = Date.now();
const dateNowSpy = vi
.spyOn(Date, "now")
.mockImplementationOnce(() => realNow)
.mockImplementation(() => Number.MAX_SAFE_INTEGER);
let caughtError: NodeJS.ErrnoException | undefined;
try {
await withDirectoryMergeLock(targetDir, async () => undefined);
} catch (error) {
caughtError = error as NodeJS.ErrnoException;
} finally {
dateNowSpy.mockRestore();
}
expect(caughtError).toBeInstanceOf(Error);
expect(caughtError?.code).toBe(WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE);
// The classifier reads only `code`; prove the message text carries no
// trace of the classified outcome, so a message-text match could not
// have produced this result.
expect(caughtError?.message).not.toContain("restore_lock_timeout");
expect(classifyWorkspaceRestoreFailure(caughtError)).toBe("restore_lock_timeout");
});
it.skipIf(process.platform === "win32")(
"serializes two concurrent writers that address one target through different aliases",
async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
useTempPaperclipHome(path.join(rootDir, "paperclip-home"), "test-instance");
const targetDir = path.join(rootDir, "target");
const aliasDir = path.join(rootDir, "target-alias");
await mkdir(targetDir, { recursive: true });
await symlink(targetDir, aliasDir);
let active = false;
let overlapCount = 0;
let completedCount = 0;
const runWriter = (dir: string) =>
withDirectoryMergeLock(dir, async () => {
if (active) overlapCount += 1;
active = true;
await new Promise((resolve) => setTimeout(resolve, 30));
active = false;
completedCount += 1;
});
await Promise.all([runWriter(targetDir), runWriter(aliasDir)]);
expect(overlapCount).toBe(0);
expect(completedCount).toBe(2);
},
);
});
describe("caller-provided env for the lock root", () => {
// These tests never touch `process.env`. They prove `withDirectoryMergeLock`
// resolves the lock root from a caller's own `env` object — the shape every
// environment-parameterized Codex credential call site holds — instead of
// always reading `process.env`.
it("two callers that pass the same env with a temporary PAPERCLIP_HOME take the same lock under that home", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const explicitHome = path.join(rootDir, "explicit-home");
const env: NodeJS.ProcessEnv = { PAPERCLIP_HOME: explicitHome, PAPERCLIP_INSTANCE_ID: "test-instance" };
const targetDir = path.join(rootDir, "target");
await mkdir(targetDir, { recursive: true });
const lockRootDir = path.join(explicitHome, "instances", "test-instance", "locks", "directory-merge");
let lockNameFirstCaller = "";
await withDirectoryMergeLock(
targetDir,
async () => {
const entries = await readdir(lockRootDir);
lockNameFirstCaller = entries[0] ?? "";
},
env,
);
let lockNameSecondCaller = "";
await withDirectoryMergeLock(
targetDir,
async () => {
const entries = await readdir(lockRootDir);
lockNameSecondCaller = entries[0] ?? "";
},
env,
);
expect(lockNameFirstCaller).not.toBe("");
expect(lockNameSecondCaller).toBe(lockNameFirstCaller);
expect(lockRootDir.startsWith(explicitHome + path.sep)).toBe(true);
});
it("does not write a lock entry under process.env.PAPERCLIP_HOME when the caller passes its own env", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const explicitHome = path.join(rootDir, "explicit-home");
const env: NodeJS.ProcessEnv = { PAPERCLIP_HOME: explicitHome, PAPERCLIP_INSTANCE_ID: "test-instance" };
const targetDir = path.join(rootDir, "target");
await mkdir(targetDir, { recursive: true });
const canonicalTargetDir = await realpath(targetDir);
const lockKey = createHash("sha256").update(canonicalTargetDir).digest("hex");
// Resolved with no `env` argument, so it reads `process.env` exactly the way
// the real instance root does — unaffected by the explicit `env` above.
const realInstanceRoot = resolvePaperclipInstanceRootForAdapter();
const realLockPath = path.join(realInstanceRoot, "locks", "directory-merge", `${lockKey}.lock`);
await withDirectoryMergeLock(targetDir, async () => undefined, env);
await expect(lstat(realLockPath)).rejects.toThrow();
const explicitLockRootDir = path.join(explicitHome, "instances", "test-instance", "locks", "directory-merge");
await expect(stat(explicitLockRootDir)).resolves.toBeTruthy();
});
it("resolves the lock root under the default instance id when the caller env sets PAPERCLIP_HOME but not PAPERCLIP_INSTANCE_ID, ignoring process.env.PAPERCLIP_INSTANCE_ID", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const explicitHome = path.join(rootDir, "explicit-home");
const env: NodeJS.ProcessEnv = { PAPERCLIP_HOME: explicitHome };
const previousInstanceId = process.env.PAPERCLIP_INSTANCE_ID;
process.env.PAPERCLIP_INSTANCE_ID = "wrong-instance";
try {
const targetDir = path.join(rootDir, "target");
await mkdir(targetDir, { recursive: true });
// The independent, no-caller-env resolution of "PAPERCLIP_HOME set,
// PAPERCLIP_INSTANCE_ID unset" — the expected default instance id.
const expectedInstanceRoot = resolvePaperclipInstanceRootForAdapter({ homeDir: explicitHome, env: {} });
const expectedLockRootDir = path.join(expectedInstanceRoot, "locks", "directory-merge");
const wrongInstanceLockRootDir = path.join(explicitHome, "instances", "wrong-instance", "locks", "directory-merge");
await withDirectoryMergeLock(targetDir, async () => undefined, env);
await expect(stat(expectedLockRootDir)).resolves.toBeTruthy();
await expect(stat(wrongInstanceLockRootDir)).rejects.toThrow();
} finally {
if (previousInstanceId === undefined) delete process.env.PAPERCLIP_INSTANCE_ID;
else process.env.PAPERCLIP_INSTANCE_ID = previousInstanceId;
}
});
it("does not read process.env.PAPERCLIP_HOME when the caller env sets neither variable", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-restore-merge-"));
cleanupDirs.push(rootDir);
const fakeProcessHome = path.join(rootDir, "process-home");
const fallbackOsHome = path.join(rootDir, "os-home");
await mkdir(fallbackOsHome, { recursive: true });
const previousHome = process.env.PAPERCLIP_HOME;
process.env.PAPERCLIP_HOME = fakeProcessHome;
// Stand in for the real host home directory, so the "no env at all"
// fallback lands under a temp dir instead of the real ~/.paperclip.
const homedirSpy = vi.spyOn(os, "homedir").mockReturnValue(fallbackOsHome);
try {
const targetDir = path.join(rootDir, "target");
await mkdir(targetDir, { recursive: true });
// The independent, no-caller-env resolution of "neither variable set" —
// the expected fallback root under the mocked home directory.
const expectedInstanceRoot = resolvePaperclipInstanceRootForAdapter({ env: {} });
const expectedLockRootDir = path.join(expectedInstanceRoot, "locks", "directory-merge");
await withDirectoryMergeLock(targetDir, async () => undefined, {});
await expect(stat(fakeProcessHome)).rejects.toThrow();
await expect(stat(expectedLockRootDir)).resolves.toBeTruthy();
} finally {
homedirSpy.mockRestore();
if (previousHome === undefined) delete process.env.PAPERCLIP_HOME;
else process.env.PAPERCLIP_HOME = previousHome;
}
});
});
});

157
packages/adapter-utils/src/workspace-restore-merge.ts
View File

@ -3,6 +3,7 @@ import { createReadStream } from "node:fs";
import { constants as fsConstants, promises as fs } from "node:fs";
import path from "node:path";
import { shouldExcludePath } from "./exclude-patterns.js";
import { resolvePaperclipInstanceRootForAdapter } from "./server-utils.js";
type SnapshotEntry =
| { kind: "dir" }
@ -108,6 +109,67 @@ function entriesMatch(left: SnapshotEntry | null | undefined, right: SnapshotEnt
const LOCK_STALE_MS = 30_000;
/**
* The stable `code` a lock-timeout error carries, so a caller can identify it
* without matching on the error message text (the message embeds the lock
* directory path).
*/
export const WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE = "ERR_WORKSPACE_RESTORE_LOCK_TIMEOUT";
/**
* The closed set of codes a failed workspace restore can carry off the
* sandbox. Every code is safe to store on a run record readable by any
* same-company actor: none embeds a filesystem path, a raw error message, or
* a process id.
*/
export type WorkspaceRestoreFailureCode =
| "restore_permission_denied"
| "restore_lock_timeout"
| "restore_failed";
/**
* The outcome of one workspace restore. `ok: true` on a clean restore. `ok:
* false` carries one allowlisted {@link WorkspaceRestoreFailureCode} — never a
* raw error, a path, or a process id.
*/
export type WorkspaceRestoreOutcome =
| { readonly ok: true }
| { readonly ok: false; readonly code: WorkspaceRestoreFailureCode };
/**
* Classifies a caught workspace-restore error into one allowlisted code. Maps
* `EACCES` and `EPERM` to a permission failure, the merge-lock timeout
* (matched by {@link WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE}, never by the error
* message text) to a lock-timeout failure, and every other error to a generic
* failure. Never reads or returns `Error.message`, a filesystem path, or a
* process id.
*/
export function classifyWorkspaceRestoreFailure(error: unknown): WorkspaceRestoreFailureCode {
const code = error && typeof error === "object" ? (error as NodeJS.ErrnoException).code : undefined;
if (code === "EACCES" || code === "EPERM") return "restore_permission_denied";
if (code === WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE) return "restore_lock_timeout";
return "restore_failed";
}
/**
* The fixed, allowlisted line an ACP adapter writes to the run log when a
* workspace restore fails. Every call site must pass this to `onLog` instead
* of the caught error's own message: the caught error can carry a host
* filesystem path or the lock owner's process id, and the run log is
* readable by any same-company actor. Never add the code's raw
* `Error.message` to this text.
*/
export function describeWorkspaceRestoreFailure(code: WorkspaceRestoreFailureCode): string {
switch (code) {
case "restore_permission_denied":
return "the restore could not write to the workspace (permission denied)";
case "restore_lock_timeout":
return "the restore timed out waiting for the workspace merge lock";
case "restore_failed":
return "the restore failed";
}
}
async function isLockStale(lockDir: string): Promise<boolean> {
try {
const raw = await fs.readFile(path.join(lockDir, "owner.json"), "utf8");
@ -160,20 +222,97 @@ async function acquireDirectoryMergeLock(lockDir: string): Promise<() => Promise
continue;
}
if (Date.now() >= deadline) {
throw new Error(`Timed out waiting for workspace restore lock at ${lockDir}`);
const timeoutError: NodeJS.ErrnoException = new Error(
`Timed out waiting for workspace restore lock at ${lockDir}`,
);
timeoutError.code = WORKSPACE_RESTORE_LOCK_TIMEOUT_CODE;
throw timeoutError;
}
await new Promise((resolve) => setTimeout(resolve, 50));
}
}
}
const DIRECTORY_MERGE_LOCK_ROOT_MODE = 0o700;
function nonEmpty(value: string | undefined): string | null {
return typeof value === "string" && value.trim().length > 0 ? value.trim() : null;
}
/**
* Resolves the private, instance-scoped root for every directory-merge lock:
* `<instance root>/locks/directory-merge`. Every process that can mutate one
* target directory must resolve to the same `PAPERCLIP_HOME` and
* `PAPERCLIP_INSTANCE_ID`. That shared resolution is what keeps mutual
* exclusion true for all five callers of `withDirectoryMergeLock`, including
* the three Codex credential call sites that never touch a workspace.
*
* This never falls back to `os.tmpdir()` and never places the lock beside the
* target directory: both paths funnel through this one instance-scoped root,
* so a read-only target parent (the workspace-restore bug) cannot block a
* lock acquisition.
*
* The root reads `PAPERCLIP_HOME` and `PAPERCLIP_INSTANCE_ID` from `env`, so an
* environment-parameterized caller (a Codex credential call site that builds
* its own `env` object instead of reading `process.env`) resolves its lock
* root under the same instance root as the directory it protects. This never
* reads `process.env` when the caller passes an `env`: every fallback inside
* the resolver also reads from that same `env` object. A caller that omits
* `env` gets `process.env`, which keeps the resolution unchanged for the
* workspace-restore call site.
*
* The root is validated, not trusted: `lstat` rejects a symlink and rejects
* any non-directory before use (fail closed). `fs.mkdir` does not change the
* mode of a directory that already exists, so an existing valid directory
* keeps whatever mode it already has; only a freshly created root gets mode
* `0o700`.
*
* The existence check and the `mkdir` below are two separate calls, so a
* racing writer can plant a symlink at `lockRoot` in between them. `fs.mkdir`
* with `recursive: true` does not fail on a leaf that already exists as a
* symlink to a real directory, so a successful `mkdir` call alone does not
* prove the path is a plain directory. The `lstat` after `mkdir` closes that
* window: it validates what is actually at `lockRoot` (never a `stat`, which
* would follow the symlink) before any caller treats it as the lock root.
*/
async function resolveDirectoryMergeLockRoot(env: NodeJS.ProcessEnv = process.env): Promise<string> {
const instanceRoot = resolvePaperclipInstanceRootForAdapter({
homeDir: nonEmpty(env.PAPERCLIP_HOME) ?? undefined,
instanceId: nonEmpty(env.PAPERCLIP_INSTANCE_ID) ?? undefined,
env,
});
const lockRoot = path.join(instanceRoot, "locks", "directory-merge");
const existing = await fs.lstat(lockRoot).catch((error: NodeJS.ErrnoException) => {
if (error.code === "ENOENT") return null;
throw error;
});
if (existing) {
if (existing.isSymbolicLink() || !existing.isDirectory()) {
throw new Error(`Directory merge lock root at ${lockRoot} is not a plain directory.`);
}
return lockRoot;
}
await fs.mkdir(lockRoot, { recursive: true, mode: DIRECTORY_MERGE_LOCK_ROOT_MODE });
const created = await fs.lstat(lockRoot);
if (created.isSymbolicLink() || !created.isDirectory()) {
throw new Error(`Directory merge lock root at ${lockRoot} is not a plain directory.`);
}
return lockRoot;
}
export async function withDirectoryMergeLock<T>(
targetDir: string,
fn: () => Promise<T>,
fn: (canonicalTargetDir: string) => Promise<T>,
env: NodeJS.ProcessEnv = process.env,
): Promise<T> {
const releaseLock = await acquireDirectoryMergeLock(`${targetDir}.paperclip-restore.lock`);
// Canonicalize before we hash or lock: a retargeted symlink must not let the
// lock protect one directory while the caller mutates another.
const canonicalTargetDir = await fs.realpath(targetDir);
const lockRoot = await resolveDirectoryMergeLockRoot(env);
const lockKey = createHash("sha256").update(canonicalTargetDir).digest("hex");
const releaseLock = await acquireDirectoryMergeLock(path.join(lockRoot, `${lockKey}.lock`));
try {
return await fn();
return await fn(canonicalTargetDir);
} finally {
await releaseLock();
}
@ -227,16 +366,16 @@ export async function mergeDirectoryWithBaseline(input: {
afterApply?: () => Promise<void>;
}): Promise<void> {
const source = await captureDirectorySnapshot(input.sourceDir, { exclude: input.baseline.exclude });
await withDirectoryMergeLock(input.targetDir, async () => {
await withDirectoryMergeLock(input.targetDir, async (canonicalTargetDir) => {
await input.beforeApply?.();
const current = await captureDirectorySnapshot(input.targetDir, { exclude: input.baseline.exclude });
const current = await captureDirectorySnapshot(canonicalTargetDir, { exclude: input.baseline.exclude });
const deletedLeafEntries = [...input.baseline.entries.entries()]
.filter(([relative, entry]) => entry.kind !== "dir" && !source.entries.has(relative))
.sort(([left], [right]) => right.length - left.length);
for (const [relative, baselineEntry] of deletedLeafEntries) {
if (!entriesMatch(current.entries.get(relative), baselineEntry)) continue;
await fs.rm(path.join(input.targetDir, relative), { recursive: true, force: true }).catch(() => undefined);
await fs.rm(path.join(canonicalTargetDir, relative), { recursive: true, force: true }).catch(() => undefined);
}
const deletedDirs = [...input.baseline.entries.entries()]
@ -244,7 +383,7 @@ export async function mergeDirectoryWithBaseline(input: {
.sort(([left], [right]) => right.length - left.length);
for (const [relative] of deletedDirs) {
await fs.rmdir(path.join(input.targetDir, relative)).catch(() => undefined);
await fs.rmdir(path.join(canonicalTargetDir, relative)).catch(() => undefined);
}
const changedSourceEntries = [...source.entries.entries()]
@ -252,7 +391,7 @@ export async function mergeDirectoryWithBaseline(input: {
.sort(([left], [right]) => left.localeCompare(right));
for (const [relative, entry] of changedSourceEntries) {
await copySnapshotEntry(input.sourceDir, input.targetDir, relative, entry);
await copySnapshotEntry(input.sourceDir, canonicalTargetDir, relative, entry);
}
await input.afterApply?.();

86
packages/adapters/claude-local/src/server/acp.test.ts
View File

@ -703,6 +703,92 @@ describe("claude_local ACP lane", () => {
await expect(fs.readFile(path.join(localCwd, "from-sandbox.txt"), "utf8")).resolves.toBe("synced");
});
it("test_claude_acp_teardown_restore_failure_sanitizes_the_run_log", async () => {
// Security regression for a workspace-restore write failure: the run log
// is readable by any same-company actor, so the teardown must never write
// the caught error's own message there — that message can carry the host
// workspace path. Force a real EACCES by making the workspace read-only,
// and name it with a sentinel marker so any leak is easy to spot.
const root = await makeTempRoot("paperclip-claude-acp-restore-failure-");
const localCwd = path.join(root, "SENTINEL-HOST-PATH-marker", "worktree");
const remoteCwd = path.join(root, "remote-workspace");
await fs.mkdir(localCwd, { recursive: true });
await fs.mkdir(remoteCwd, { recursive: true });
await fs.writeFile(path.join(localCwd, "hello.txt"), "hi", "utf8");
process.env.PAPERCLIP_HOME = path.join(root, "paperclip-home");
process.env.PAPERCLIP_INSTANCE_ID = "test";
// The runtime writes a new file into the in-sandbox workspace during the
// turn, so the teardown's restore has something to copy back — and a new
// file is exactly what a read-only workspace directory rejects. The
// workspace turns read-only only after the turn's own writes (settings
// seeded at startup, the sandbox-authored file) — the teardown restore
// that runs after the turn is the write this test forces to fail.
const runtime = new FakeRuntime({});
const startTurn = runtime.startTurn.bind(runtime);
runtime.startTurn = (input) => {
const turn = startTurn(input);
const remoteWorkspaceCwd = input.handle.cwd ?? remoteCwd;
return {
...turn,
result: (async () => {
await fs.writeFile(path.join(remoteWorkspaceCwd, "from-sandbox.txt"), "synced", "utf8");
await fs.chmod(localCwd, 0o500);
return await turn.result;
})(),
};
};
const execute = createClaudeAcpExecutor({
createRuntime: (options: FakeRuntimeOptions) => {
Object.assign(runtime.options, options);
return runtime as never;
},
});
const loggedLines: string[] = [];
try {
const result = await execute(
buildContext(localCwd, {
config: {
engine: "acp",
cwd: localCwd,
agentCommand: "node ./fake-acp.js",
stateDir: path.join(root, "state"),
promptTemplate: "Do the assigned work.",
},
context: {
issueId: "issue-1",
paperclipWorkspace: { cwd: localCwd, source: "project_workspace", workspaceId: "workspace-1" },
},
executionTarget: {
kind: "remote",
transport: "sandbox",
providerKey: "fake-plugin",
remoteCwd,
runner: createLocalSandboxRunner(),
} as never,
authToken: "real-run-jwt",
onLog: async (_stream, chunk) => {
loggedLines.push(chunk);
},
}),
);
// Fail-open: the restore miss never changes the run's exit code or
// status, and it surfaces as one allowlisted code — never the raw error.
expect(result.exitCode).toBe(0);
expect(result.resultJson?.workspaceRestoreFailure).toBe("restore_permission_denied");
const allLogs = loggedLines.join("");
expect(allLogs).not.toContain("SENTINEL-HOST-PATH-marker");
expect(allLogs).not.toContain(localCwd);
expect(allLogs).not.toContain("EACCES");
expect(allLogs).toContain("permission denied");
} finally {
await fs.chmod(localCwd, 0o700).catch(() => undefined);
}
});
it("remaps a workspace-relative explicit CLAUDE_CONFIG_DIR onto the in-sandbox workspace path", async () => {
const root = await makeTempRoot("paperclip-claude-acp-explicit-inworkspace-");
const localCwd = path.join(root, "worktree");

15
packages/adapters/claude-local/src/server/acp.ts
View File

@ -48,6 +48,10 @@ import {
classifyThrownErrorClass,
logSandboxProbeDiagnostic,
} from "./probe-diagnostics.js";
import {
classifyWorkspaceRestoreFailure,
describeWorkspaceRestoreFailure,
} from "@paperclipai/adapter-utils/workspace-restore-merge";
import { buildLocalAdapterTestProbeEnv } from "./probe-env.js";
import { detectClaudeLoginRequired, parseClaudeStreamJson } from "./parse.js";
import { buildClaudeProbePermissionArgs } from "./permissions.js";
@ -219,13 +223,18 @@ async function prepareClaudeRemoteManagedHome(
try {
await onLog("stdout", "[paperclip] Restoring workspace changes from the sandbox.\n");
await stagedRuntime.restoreWorkspace((line) => onLog("stdout", line));
return { ok: true };
} catch (err) {
// The run log is readable by any same-company actor, so it must never
// carry the caught error's own message: that message can hold a host
// filesystem path or a process id. Log only the fixed, allowlisted
// diagnostic for the classified code.
const code = classifyWorkspaceRestoreFailure(err);
await onLog(
"stderr",
`[paperclip] Claude ACP teardown workspace restore failed: ${
err instanceof Error ? err.message : String(err)
}\n`,
`[paperclip] Claude ACP teardown workspace restore failed: ${describeWorkspaceRestoreFailure(code)}\n`,
);
return { ok: false, code };
}
};
const envConfig = parseObject(input.config.env);

95
packages/adapters/codex-local/src/server/acp.test.ts
View File

@ -1135,6 +1135,101 @@ describe("codex_local ACP lane", () => {
expect(hostAuth.tokens.refresh_token).toBe("ref-sandbox-newer");
});
it("test_codex_acp_teardown_restore_failure_sanitizes_the_run_log", async () => {
// Security regression for a workspace-restore write failure: the run log
// is readable by any same-company actor, so the teardown must never write
// the caught error's own message there — that message can carry the host
// workspace path. Force a real EACCES by making the workspace read-only,
// and name it with a sentinel marker so any leak is easy to spot.
const runId = "run-restore-failure";
const root = await makeTempRoot("paperclip-codex-acp-restore-failure-");
const localCwd = path.join(root, "SENTINEL-HOST-PATH-marker", "worktree");
const remoteCwd = path.join(root, "remote-workspace");
const sourceHome = path.join(root, "codex-home");
const sharedHostHome = path.join(root, "shared-codex-home");
await fs.mkdir(localCwd, { recursive: true });
await fs.mkdir(remoteCwd, { recursive: true });
await fs.mkdir(sourceHome, { recursive: true });
await fs.mkdir(sharedHostHome, { recursive: true });
await fs.writeFile(path.join(localCwd, "hello.txt"), "hi", "utf8");
process.env.CODEX_HOME = sharedHostHome;
// The runtime writes a new file into the in-sandbox workspace during the
// turn, so the teardown's restore has something to copy back — and a new
// file is exactly what a read-only workspace directory rejects. The
// workspace turns read-only only after the turn's own writes, so the
// teardown restore that runs after the turn is the write this forces to
// fail.
const runtime = new FakeRuntime({});
const startTurn = runtime.startTurn.bind(runtime);
runtime.startTurn = (input) => {
const turn = startTurn(input);
const remoteWorkspaceCwd = input.handle.cwd ?? remoteCwd;
return {
...turn,
result: (async () => {
await fs.writeFile(path.join(remoteWorkspaceCwd, "from-sandbox.txt"), "synced", "utf8");
await fs.chmod(localCwd, 0o500);
return await turn.result;
})(),
};
};
const stagedRuntimes = new Map();
const execute = createCodexAcpExecutor({
createRuntime: (options: FakeRuntimeOptions) => {
Object.assign(runtime.options, options);
return runtime as never;
},
stagedRuntimes,
stagingLocks: new Map(),
});
const loggedLines: string[] = [];
try {
const result = await execute(
buildContext(localCwd, {
runId,
config: {
engine: "acp",
cwd: localCwd,
agentCommand: "node ./fake-acp.js",
stateDir: path.join(root, "state"),
env: { CODEX_HOME: sourceHome },
promptTemplate: "Do the assigned work.",
},
context: {
issueId: "issue-1",
paperclipWorkspace: { cwd: localCwd, source: "project_workspace", workspaceId: "workspace-1" },
},
executionTarget: {
kind: "remote",
transport: "sandbox",
providerKey: "fake-plugin",
remoteCwd,
runner: createLocalSandboxRunner(),
} as never,
authToken: "real-run-jwt",
onLog: async (_stream, chunk) => {
loggedLines.push(chunk);
},
}),
);
// Fail-open: the restore miss never changes the run's exit code or
// status, and it surfaces as one allowlisted code — never the raw error.
expect(result.exitCode).toBe(0);
expect(result.resultJson?.workspaceRestoreFailure).toBe("restore_permission_denied");
const allLogs = loggedLines.join("");
expect(allLogs).not.toContain("SENTINEL-HOST-PATH-marker");
expect(allLogs).not.toContain(localCwd);
expect(allLogs).not.toContain("EACCES");
expect(allLogs).toContain("permission denied");
} finally {
await fs.chmod(localCwd, 0o700).catch(() => undefined);
}
});
it("falls back to the CLI lane for a runner-less sandbox even when the ACP command is set", async () => {
setNodeVersion("v24.11.0");
// Isolate the missing bidirectional runner as the sole fallback cause:

16
packages/adapters/codex-local/src/server/acp.ts
View File

@ -35,6 +35,10 @@ import {
asString,
parseObject,
} from "@paperclipai/adapter-utils/server-utils";
import {
classifyWorkspaceRestoreFailure,
describeWorkspaceRestoreFailure,
} from "@paperclipai/adapter-utils/workspace-restore-merge";
import { normalizeCodexModel } from "../index.js";
import { classifyCodexAuthRefreshFailure } from "./parse.js";
import { copyBackCodexAuth } from "./codex-auth-copyback.js";
@ -243,17 +247,23 @@ async function prepareCodexRemoteManagedHome(
"[paperclip] Restoring workspace changes and Codex auth from the sandbox.\n",
);
await stagedRuntime.restoreWorkspace((line) => onLog("stdout", line));
return { ok: true };
} catch (err) {
// Fail-soft: a teardown copy-back miss loses this rotation and surfaces
// loudly as refresh_token_reused on the next host Codex use (re-auth
// recovers) — never silent host-credential corruption, so it must not
// mask the run result.
//
// The run log is readable by any same-company actor, so it must never
// carry the caught error's own message: that message can hold a host
// filesystem path or a process id. Log only the fixed, allowlisted
// diagnostic for the classified code.
const code = classifyWorkspaceRestoreFailure(err);
await onLog(
"stderr",
`[paperclip] Codex ACP teardown restore/copy-back failed: ${
err instanceof Error ? err.message : String(err)
}\n`,
`[paperclip] Codex ACP teardown restore/copy-back failed: ${describeWorkspaceRestoreFailure(code)}\n`,
);
return { ok: false, code };
}
},
// One-time cleanup of the HOST staged home temp dir. Fired ONLY when the

3
packages/adapters/codex-local/src/server/adapter-auth-promotion.ts
View File

@ -224,6 +224,7 @@ export async function promoteDeviceLoginCredential(
keptLine: "[paperclip] Codex device-login promotion: kept the company credential home (the login is not a seed or a strictly-newer same-identity credential).",
tempPrefix: "auth.json.promotion-home",
errorLabel: "codex device-login promotion",
env,
});
// A kept home has two very different meanings. The writer keeps the home when
@ -258,7 +259,7 @@ export async function promoteDeviceLoginCredential(
if (isCodexAuthCacheEnabled(env)) {
try {
const cacheEntryPath = await ensureCodexAuthCacheEntryDir(env, accountId, companyId);
await writeCodexAuthCacheEntry({ sandboxAuthBytes: authBytes, cacheEntryPath, log });
await writeCodexAuthCacheEntry({ sandboxAuthBytes: authBytes, cacheEntryPath, log, env });
} catch {
await log(
"[paperclip] Codex device-login promotion: the per-identity cache write failed; the company credential home is durable, so the login stays successful.",

21
packages/adapters/codex-local/src/server/codex-auth-cache.test.ts
View File

@ -204,7 +204,7 @@ describe("codex auth cache store", () => {
"acct-x": subscriptionAuth({ accountId: "acct-x", lastRefresh: NEWER, marker: "cache" }),
},
});
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined);
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined, env);
expect(outcome).toBe("vended");
const finalHost = await readFile(sharedHomeAuthPath, "utf8");
expect(finalHost).toContain("acct-x");
@ -221,7 +221,7 @@ describe("codex auth cache store", () => {
},
});
const before = await readFile(sharedHomeAuthPath, "utf8");
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined);
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined, env);
expect(outcome).toBe("kept-host");
expect(await readFile(sharedHomeAuthPath, "utf8")).toBe(before);
}
@ -235,7 +235,7 @@ describe("codex auth cache store", () => {
},
});
const before = await readFile(sharedHomeAuthPath, "utf8");
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined);
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined, env);
expect(outcome).toBe("kept-host");
expect(await readFile(sharedHomeAuthPath, "utf8")).toBe(before);
});
@ -246,7 +246,7 @@ describe("codex auth cache store", () => {
"acct-y": subscriptionAuth({ accountId: "acct-y", lastRefresh: NEWER, marker: "other" }),
},
});
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined);
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined, env);
expect(outcome).toBe("no-host-identity");
await expect(lstat(sharedHomeAuthPath)).rejects.toThrow();
});
@ -259,7 +259,7 @@ describe("codex auth cache store", () => {
},
});
const before = await readFile(sharedHomeAuthPath, "utf8");
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined);
const outcome = await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), () => undefined, env);
expect(outcome).toBe("no-host-identity");
expect(await readFile(sharedHomeAuthPath, "utf8")).toBe(before);
});
@ -272,9 +272,14 @@ describe("codex auth cache store", () => {
},
});
const logs: string[] = [];
await selectVendCredential(sharedHomeAuthPath, resolveEntry(env), (line) => {
logs.push(line);
});
await selectVendCredential(
sharedHomeAuthPath,
resolveEntry(env),
(line) => {
logs.push(line);
},
env,
);
const combined = logs.join("\n");
expect(combined).not.toContain("SENTINEL");
expect(combined).not.toContain("SECRET-ACCT");

16
packages/adapters/codex-local/src/server/codex-auth-cache.ts
View File

@ -220,6 +220,8 @@ export async function writeCodexAuthCacheEntry(input: {
sandboxAuthBytes: Buffer;
cacheEntryPath: string;
log: (line: string) => void | Promise<void>;
/** Environment for the merge lock root. Defaults to `process.env`. */
env?: NodeJS.ProcessEnv;
}): Promise<WriteCodexAuthCacheEntryOutcome> {
const outcome = await writeCredentialSeedOrNewer({
sourceBytes: input.sandboxAuthBytes,
@ -231,6 +233,7 @@ export async function writeCodexAuthCacheEntry(input: {
"[paperclip] Codex auth cache: kept the cache slot (source is not a strictly-newer same-identity subscription credential).",
tempPrefix: "auth.json.cache-source",
errorLabel: "codex auth cache",
env: input.env,
});
return outcome === "written" ? "written" : "kept-slot";
}
@ -258,6 +261,7 @@ export async function selectVendCredential(
sharedHomeAuthPath: string,
resolveCacheEntryPath: (accountId: string) => string,
log: (line: string) => void | Promise<void>,
env: NodeJS.ProcessEnv = process.env,
): Promise<VendCodexAuthOutcome> {
const hostBytes = await readFile(sharedHomeAuthPath).catch((error: NodeJS.ErrnoException) => {
if (error.code === "ENOENT") return null;
@ -322,7 +326,7 @@ export async function selectVendCredential(
await handle.close().catch(() => undefined);
await rm(stagedTempPath, { force: true }).catch(() => undefined);
}
});
}, env);
}
/**
@ -345,9 +349,13 @@ export async function clearCodexAuthCacheEntry(
throw error;
});
if (!existing) return;
await withDirectoryMergeLock(entryDir, async () => {
await rm(entryDir, { recursive: true, force: true });
});
await withDirectoryMergeLock(
entryDir,
async () => {
await rm(entryDir, { recursive: true, force: true });
},
env,
);
}
/**

72
packages/adapters/codex-local/src/server/codex-auth-copyback.ts
View File

@ -96,43 +96,47 @@ export async function copyBackCodexAuth(input: CopyBackCodexAuthInput): Promise<
const hostDir = path.dirname(hostAuthPath);
await mkdir(hostDir, { recursive: true });
const hostOutcome = await withDirectoryMergeLock(hostDir, async () => {
// Stage on the same filesystem as the host target so both the predicate read
// and the final rename stay device-local (rename across devices is not
// atomic and would fail with EXDEV).
const stagedTempPath = path.join(hostDir, `.auth.json.copyback-${process.pid}-${randomUUID()}.tmp`);
// `wx` + explicit mode create the temp private (0600) and fail if it somehow
// already exists, so we never write through a pre-existing symlink.
const handle = await open(stagedTempPath, "wx", 0o600);
try {
await handle.writeFile(sandboxAuthBytes);
await handle.close();
const hostOutcome = await withDirectoryMergeLock(
hostDir,
async () => {
// Stage on the same filesystem as the host target so both the predicate read
// and the final rename stay device-local (rename across devices is not
// atomic and would fail with EXDEV).
const stagedTempPath = path.join(hostDir, `.auth.json.copyback-${process.pid}-${randomUUID()}.tmp`);
// `wx` + explicit mode create the temp private (0600) and fail if it somehow
// already exists, so we never write through a pre-existing symlink.
const handle = await open(stagedTempPath, "wx", 0o600);
try {
await handle.writeFile(sandboxAuthBytes);
await handle.close();
const decision = await decideCodexAuthMerge(stagedTempPath, hostAuthPath, {
errorLabel: "codex auth copy-back",
});
if (decision === USE_SOURCE_EXIT) {
// Atomic same-directory swap; rename preserves the temp's 0600 mode.
await rename(stagedTempPath, hostAuthPath);
await log(
"[paperclip] Codex auth copy-back: sandbox credential is strictly newer for the same subscription identity; installed to the host at mode 0600.",
);
return "copied";
}
const decision = await decideCodexAuthMerge(stagedTempPath, hostAuthPath, {
errorLabel: "codex auth copy-back",
});
if (decision === USE_SOURCE_EXIT) {
// Atomic same-directory swap; rename preserves the temp's 0600 mode.
await rename(stagedTempPath, hostAuthPath);
await log(
"[paperclip] Codex auth copy-back: sandbox credential is strictly newer for the same subscription identity; installed to the host at mode 0600.",
"[paperclip] Codex auth copy-back: host credential kept (sandbox copy is not a strictly-newer same-identity subscription credential).",
);
return "copied";
return "kept-host";
} finally {
// Close is idempotent-safe to skip after an explicit close; the temp is the
// thing that must never linger. On the copy path rename already consumed it
// (force makes the removal a no-op); on every other path this deletes the
// staged credential bytes.
await handle.close().catch(() => undefined);
await rm(stagedTempPath, { force: true }).catch(() => undefined);
}
await log(
"[paperclip] Codex auth copy-back: host credential kept (sandbox copy is not a strictly-newer same-identity subscription credential).",
);
return "kept-host";
} finally {
// Close is idempotent-safe to skip after an explicit close; the temp is the
// thing that must never linger. On the copy path rename already consumed it
// (force makes the removal a no-op); on every other path this deletes the
// staged credential bytes.
await handle.close().catch(() => undefined);
await rm(stagedTempPath, { force: true }).catch(() => undefined);
}
});
},
env,
);
// Additive cache write. Independent of the host default overwrite above: it
// runs on its own directory lock, keys the slot by the real sandbox
@ -150,7 +154,7 @@ export async function copyBackCodexAuth(input: CopyBackCodexAuthInput): Promise<
const sandboxAccountId = readSubscriptionAccountId(sandboxAuthBytes);
if (sandboxAccountId) {
const cacheEntryPath = await resolveCacheEntryPath(sandboxAccountId);
await writeCodexAuthCacheEntry({ sandboxAuthBytes, cacheEntryPath, log });
await writeCodexAuthCacheEntry({ sandboxAuthBytes, cacheEntryPath, log, env });
}
} catch (error) {
// Log only the errno code, never the error message. The message embeds the

58
packages/adapters/codex-local/src/server/codex-auth-seed-write.ts
View File

@ -33,6 +33,8 @@ export interface WriteCredentialSeedOrNewerInput {
tempPrefix: string;
/** The caller name that prefixes a predicate error. */
errorLabel: string;
/** Environment for the merge lock root. Defaults to `process.env`. */
env?: NodeJS.ProcessEnv;
}
/**
@ -46,31 +48,35 @@ export async function writeCredentialSeedOrNewer(
input: WriteCredentialSeedOrNewerInput,
): Promise<WriteCredentialSeedOrNewerOutcome> {
const destinationDir = path.dirname(input.destinationPath);
return withDirectoryMergeLock(destinationDir, async () => {
const stagedTempPath = path.join(
destinationDir,
`.${input.tempPrefix}-${process.pid}-${randomUUID()}.tmp`,
);
// `wx` + explicit mode create the temp private (0600) and fail if it already
// exists, so the writer never writes through a pre-existing symlink.
const handle = await open(stagedTempPath, "wx", 0o600);
try {
await handle.writeFile(input.sourceBytes);
await handle.close();
const decision = await decideCodexAuthMerge(stagedTempPath, input.destinationPath, {
seedIfDestAbsent: input.seedIfDestAbsent,
errorLabel: input.errorLabel,
});
if (decision === USE_SOURCE_EXIT) {
await rename(stagedTempPath, input.destinationPath);
await input.log(input.writtenLine);
return "written";
return withDirectoryMergeLock(
destinationDir,
async () => {
const stagedTempPath = path.join(
destinationDir,
`.${input.tempPrefix}-${process.pid}-${randomUUID()}.tmp`,
);
// `wx` + explicit mode create the temp private (0600) and fail if it already
// exists, so the writer never writes through a pre-existing symlink.
const handle = await open(stagedTempPath, "wx", 0o600);
try {
await handle.writeFile(input.sourceBytes);
await handle.close();
const decision = await decideCodexAuthMerge(stagedTempPath, input.destinationPath, {
seedIfDestAbsent: input.seedIfDestAbsent,
errorLabel: input.errorLabel,
});
if (decision === USE_SOURCE_EXIT) {
await rename(stagedTempPath, input.destinationPath);
await input.log(input.writtenLine);
return "written";
}
await input.log(input.keptLine);
return "kept";
} finally {
await handle.close().catch(() => undefined);
await rm(stagedTempPath, { force: true }).catch(() => undefined);
}
await input.log(input.keptLine);
return "kept";
} finally {
await handle.close().catch(() => undefined);
await rm(stagedTempPath, { force: true }).catch(() => undefined);
}
});
},
input.env,
);
}

1
packages/adapters/codex-local/src/server/device-login-export.ts
View File

@ -247,6 +247,7 @@ export async function installDeviceLoginCredential(
sandboxAuthBytes,
cacheEntryPath: authPath,
log,
env,
});
return outcome === "written" ? "seeded" : "kept";
}

3
packages/adapters/codex-local/src/server/execute.ts
View File

@ -665,6 +665,9 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
// freshest same-identity copy. The off-switch (default on) skips the vend.
if (isCodexAuthCacheEnabled(process.env)) {
const sharedHomeAuthPath = path.join(resolveSharedCodexHomeDir(process.env), "auth.json");
// This caller reads `process.env` directly and holds no separate `env`
// object, so `selectVendCredential` falls back to its own `process.env`
// default for the merge lock root.
await selectVendCredential(
sharedHomeAuthPath,
(accountId) => resolveCodexAuthCacheEntryPath(process.env, accountId, agent.companyId),

84
packages/adapters/gemini-local/src/server/acp.test.ts
View File

@ -613,6 +613,90 @@ describe("gemini_local ACP lane", () => {
await expect(fs.readFile(path.join(localCwd, "from-sandbox.txt"), "utf8")).resolves.toBe("synced");
});
it("test_gemini_acp_teardown_restore_failure_sanitizes_the_run_log", async () => {
// Security regression for a workspace-restore write failure: the run log
// is readable by any same-company actor, so the teardown must never write
// the caught error's own message there — that message can carry the host
// workspace path. Force a real EACCES by making the workspace read-only,
// and name it with a sentinel marker so any leak is easy to spot.
const root = await makeTempRoot("paperclip-gemini-acp-restore-failure-");
const localCwd = path.join(root, "SENTINEL-HOST-PATH-marker", "worktree");
const remoteCwd = path.join(root, "remote-workspace");
await fs.mkdir(localCwd, { recursive: true });
await fs.mkdir(remoteCwd, { recursive: true });
await fs.writeFile(path.join(localCwd, "hello.txt"), "hi", "utf8");
// The runtime writes a new file into the in-sandbox workspace during the
// turn, so the teardown's restore has something to copy back — and a new
// file is exactly what a read-only workspace directory rejects. The
// workspace turns read-only only after the turn's own writes, so the
// teardown restore that runs after the turn is the write this forces to
// fail.
const runtime = new FakeRuntime({});
const startTurn = runtime.startTurn.bind(runtime);
runtime.startTurn = (input) => {
const turn = startTurn(input);
const remoteWorkspaceCwd = input.handle.cwd ?? remoteCwd;
return {
...turn,
result: (async () => {
await fs.writeFile(path.join(remoteWorkspaceCwd, "from-sandbox.txt"), "synced", "utf8");
await fs.chmod(localCwd, 0o500);
return await turn.result;
})(),
};
};
const execute = createGeminiAcpExecutor({
createRuntime: (options) => {
Object.assign(runtime.options, options);
return runtime as never;
},
});
const loggedLines: string[] = [];
try {
const result = await execute(
buildContext(localCwd, {
config: {
engine: "acp",
cwd: localCwd,
agentCommand: "node ./fake-acp.js",
stateDir: path.join(root, "state"),
promptTemplate: "Do the assigned work.",
},
context: {
issueId: "issue-1",
paperclipWorkspace: { cwd: localCwd, source: "project_workspace", workspaceId: "workspace-1" },
},
executionTarget: {
kind: "remote",
transport: "sandbox",
providerKey: "fake-plugin",
remoteCwd,
runner: createLocalSandboxRunner(),
} as never,
authToken: "real-run-jwt",
onLog: async (_stream, chunk) => {
loggedLines.push(chunk);
},
}),
);
// Fail-open: the restore miss never changes the run's exit code or
// status, and it surfaces as one allowlisted code — never the raw error.
expect(result.exitCode).toBe(0);
expect(result.resultJson?.workspaceRestoreFailure).toBe("restore_permission_denied");
const allLogs = loggedLines.join("");
expect(allLogs).not.toContain("SENTINEL-HOST-PATH-marker");
expect(allLogs).not.toContain(localCwd);
expect(allLogs).not.toContain("EACCES");
expect(allLogs).toContain("permission denied");
} finally {
await fs.chmod(localCwd, 0o700).catch(() => undefined);
}
});
it("does not persist an api-key auth selector from a host-only credential", async () => {
const root = await makeTempRoot("paperclip-gemini-acp-hostkey-");
const localCwd = path.join(root, "worktree");

15
packages/adapters/gemini-local/src/server/acp.ts
View File

@ -31,6 +31,10 @@ import {
asString,
parseObject,
} from "@paperclipai/adapter-utils/server-utils";
import {
classifyWorkspaceRestoreFailure,
describeWorkspaceRestoreFailure,
} from "@paperclipai/adapter-utils/workspace-restore-merge";
import { DEFAULT_GEMINI_LOCAL_MODEL } from "../index.js";
const moduleDir = path.dirname(fileURLToPath(import.meta.url));
@ -166,13 +170,18 @@ async function prepareGeminiRemoteManagedHome(
try {
await onLog("stdout", "[paperclip] Restoring workspace changes from the sandbox.\n");
await stagedRuntime.restoreWorkspace((line) => onLog("stdout", line));
return { ok: true };
} catch (err) {
// The run log is readable by any same-company actor, so it must never
// carry the caught error's own message: that message can hold a host
// filesystem path or a process id. Log only the fixed, allowlisted
// diagnostic for the classified code.
const code = classifyWorkspaceRestoreFailure(err);
await onLog(
"stderr",
`[paperclip] Gemini ACP teardown workspace restore failed: ${
err instanceof Error ? err.message : String(err)
}\n`,
`[paperclip] Gemini ACP teardown workspace restore failed: ${describeWorkspaceRestoreFailure(code)}\n`,
);
return { ok: false, code };
}
};
const geminiSkillsHome = resolveGeminiSkillsHome(input.config);