From ae2d2956b3e52159344ae569dff1b23b335fae7b Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 7 Nov 2026 01:59:09 -0600 Subject: [PATCH] Reject relative local Runner paths before native admission (#19776) ## Thinking Path > - Paperclip manages AI agents and their work. > - Heartbeat selects the runtime that will execute a task. > - A new local Native run uses the controller's Runner binary setting. > - A relative binary path is invalid. The resolver already rejects it, but that check occurs after Native admission. > - This pull request checks the same setting before it creates Native execution authority. > - Remote providers and admitted Native recovery keep their existing behavior. ## Linked Issues or Issue Description **What happened?** A relative `PAPERCLIP_RUNNER_BINARY` setting can reach local Native admission before the binary resolver rejects it. The run has transferred preparation ownership even though this setting cannot start the local Runner. **Expected behavior** Reject this invalid setting before creating a completion contract or Native coordinator. Keep the existing preparation owner and the run row unchanged. **Steps to reproduce** Set `PAPERCLIP_RUNNER_BINARY=relative-runner`. Select a new local `paperclip_runner` Native run. The selection must report the existing absolute-path error before Native admission. Refs: #19466 and #19340. Those merged changes repair Retry authority and saved-workspace recovery. This change only moves local path validation before new admission. ## What Changed - Add a pure absolute-path check beside the existing binary resolver. Both use the actual controller environment setting. - Run that check before a new local Runner admission. Skip remote execution, API-only providers, and an already admitted Native run. - Test the rejection, unchanged preparation ownership, controller setting selection, remote behavior, and immutable recovery. Keep the existing resolver checks. ## Verification - The full binary test file passed: 32 tests. - The full runtime-selection file passed against a dedicated PostgreSQL database: 62 tests. It exercises real admission transactions, contracts, coordinator rows, and instruction-copy controls. It replaces context materialization and remote workspace synchronization at their existing test boundaries. - After normal integration with current master, the normal Shared/SDK, database, eval-kernel and Runner builds passed, together with all seven normal application TypeScript programs. The original recovery-preparation case passed again on this integrated revision against a fresh dedicated database (1 passed; 61 filtered). The earlier full 32-test binary and 62-test selection results carry only where their source and executed boundaries are unchanged. - The first private selection wrapper failed during module resolution, before any case or database use. A TypeScript wrapper with the same body recovered that setup failure. This was not a product regression. - Reproduce with `pnpm --filter @paperclipai/server exec vitest run src/services/native-runtime/native-codex-runner.test.ts --maxWorkers=1 --no-file-parallelism`. - Set `PAPERCLIP_RUNTIME_SELECTION_TEST_DATABASE_URL` to a dedicated disposable test database before running `pnpm --filter @paperclipai/server exec vitest run src/services/heartbeat/runtime-selection.test.ts --maxWorkers=1 --no-file-parallelism`. ## Risks - This check validates path syntax only. An absolute path can still fail the existing file-access or artifact checks later. - A wrong branch condition could reject remote or existing Native work. The tests cover these exclusions. - This does not prove that a real provider can start or that a deployed service is healthy. It changes no database schema or retry authority. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with source inspection and code-execution tools. The host does not expose the context-window size. ## 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 --- .../heartbeat/runtime-selection.test.ts | 48 +++++++++++++++++++ .../services/heartbeat/runtime-selection.ts | 8 ++++ .../native-codex-runner.test.ts | 10 ++++ .../native-runtime/native-codex-runner.ts | 16 +++++-- 4 files changed, 79 insertions(+), 3 deletions(-) diff --git a/server/src/services/heartbeat/runtime-selection.test.ts b/server/src/services/heartbeat/runtime-selection.test.ts index 1da386ff3e9..51415004914 100644 --- a/server/src/services/heartbeat/runtime-selection.test.ts +++ b/server/src/services/heartbeat/runtime-selection.test.ts @@ -95,6 +95,53 @@ describe.skipIf(!support.supported)("heartbeat runtime selection boundary", () = const coordinators = (id: string) => db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, id)); const sandbox = { kind: "remote" as const, transport: "sandbox" as const, remoteCwd: "/workspace", reusableLeaseConfigured: false }; + it("rejects invalid local Runner configuration before native authority and retains the preparation owner", async () => { + const f = await fixture(); + await claimPreparation(f); + vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-invalid-dot-retry-controller"); + f.input.runtimeEnv = { PAPERCLIP_RUNNER_BINARY: join(tmpdir(), "ignored-injected-runnerd") }; + const before = await read(f.run.id); + await expect(selectHeartbeatRuntime(db, f.input)).rejects.toThrow("PAPERCLIP_RUNNER_BINARY must be an absolute path"); + expect(await read(f.run.id)).toEqual(before); + expect(await coordinators(f.run.id)).toEqual([]); + expect(await db.select().from(completionContracts)).toHaveLength(0); + expect(f.input.stopControllerLease).not.toHaveBeenCalled(); + expect(buildNativeRuntimeContext).not.toHaveBeenCalled(); + }); + + it("ignores an injected runtime Runner value that the local backend does not consume", async () => { + const f = await fixture(); + vi.stubEnv("PAPERCLIP_RUNNER_BINARY", join(tmpdir(), "configuration-only-controller-runnerd")); + f.input.runtimeEnv = { PAPERCLIP_RUNNER_BINARY: "relative-ignored-injected-runnerd" }; + expect((await selectHeartbeatRuntime(db, f.input)).selected).toBe(true); + expect((await read(f.run.id)).runtimeMode).toBe("native"); + expect(await coordinators(f.run.id)).toHaveLength(1); + }); + + it("does not apply local Runner configuration to remote native selection", async () => { + const f = await fixture(); + vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-unused-remote-controller"); + f.input.workspace.executionTarget = sandbox; + expect((await selectHeartbeatRuntime(db, f.input)).selected).toBe(true); + expect((await read(f.run.id)).runtimeMode).toBe("native"); + expect(await coordinators(f.run.id)).toHaveLength(1); + }); + + it("preserves immutable native recovery when current local Runner configuration is invalid", async () => { + const f = await fixture(); + const selected = await selectHeartbeatRuntime(db, f.input); + if (!selected.selected || !selected.nativeExecution) throw new Error("Expected native selection"); + const saved = await read(f.run.id); + f.input.run = saved; + f.input.session.persistedRunnerProfile = saved.runnerProfileJson ?? {}; + f.input.session.persistedNativeExecutionInput = selected.nativeExecution; + vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-unused-recovery-controller"); + const resumed = await selectHeartbeatRuntime(db, f.input); + expect(resumed).toMatchObject({ selected: true, nativeExecution: selected.nativeExecution, nativeRunnerInstanceId: selected.nativeRunnerInstanceId }); + expect((await read(f.run.id)).runnerProfileJson).toEqual(saved.runnerProfileJson); + expect(await coordinators(f.run.id)).toHaveLength(1); + }); + async function connectorFixture(durable = true) { const f = await fixture(), { input } = f; const home = await mkdtemp(join(tmpdir(), "selection-connector-assets-")); @@ -334,6 +381,7 @@ describe.skipIf(!support.supported)("heartbeat runtime selection boundary", () = it.each([false, true])("persists the Cursor provider API placement and completion authority without a Computer lease (ephemeral: %s)", async ephemeral => { vi.stubEnv("PAPERCLIP_EPHEMERAL_SERVER", String(ephemeral)); const { input, run, issue } = await fixture(); + vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-unused-api-controller"); input.agent = { ...input.agent, adapterConfig: { provider: "cursor_cloud", model: "caller-model", repoUrl: "https://github.com/example/qualification" } }; await db.update(agents).set({ adapterConfig: input.agent.adapterConfig }).where(eq(agents.id, input.agent.id)); input.workspace.activeEnvironmentLease = null; diff --git a/server/src/services/heartbeat/runtime-selection.ts b/server/src/services/heartbeat/runtime-selection.ts index 2cd2ab9bfd2..790563214aa 100644 --- a/server/src/services/heartbeat/runtime-selection.ts +++ b/server/src/services/heartbeat/runtime-selection.ts @@ -1,3 +1,4 @@ +import { assertPaperclipRunnerBinaryConfiguration } from "../native-runtime/native-codex-runner.js"; import { readNativeWorkspaceMigration, migratedNativeExecutionCwd } from "../native-runtime/native-workspace-migration.js"; import { resolveProviderApiPlacement } from "../native-runtime/provider-api-placement.js"; import { readClaimedNativeStopRecovery } from "../native-runtime/native-cancellation-request.js"; @@ -309,6 +310,13 @@ export async function selectHeartbeatRuntime(db: Db, input: HeartbeatRuntimeSele if (!issueRef) { throw new Error("native_runtime_ineligible: issue is required"); } + if (run.runtimeMode !== "native" && agent.adapterType === "paperclip_runner" + && executionTarget?.kind !== "remote" + && nativeRuntimeResolution.profile.backend !== "cursor_cloud_api" + && nativeRuntimeResolution.profile.backend !== "claude_managed_agents_api" + && nativeRuntimeResolution.profile.backend !== "aws_agentcore_harness_api") { + assertPaperclipRunnerBinaryConfiguration(); + } const nativeExecutionWorkspaceId = persistedNativeExecutionInput?.provider.kind === "cursor_cloud" // Older Cursor runs keep their admitted binding identity. Their task diff --git a/server/src/services/native-runtime/native-codex-runner.test.ts b/server/src/services/native-runtime/native-codex-runner.test.ts index f5341c8a73e..c7a5a9ad02e 100644 --- a/server/src/services/native-runtime/native-codex-runner.test.ts +++ b/server/src/services/native-runtime/native-codex-runner.test.ts @@ -9,6 +9,7 @@ import { buildNativeRunnerArguments, buildNativeRunnerPreparePayload, resolvePaperclipRunnerBinary, + assertPaperclipRunnerBinaryConfiguration, resolveRemotePaperclipRunnerBinary, } from "./native-codex-runner.js"; @@ -225,3 +226,12 @@ describe("buildNativeRunnerPreparePayload", () => { }); }); }); + +it("validates only local Runner configuration without requiring or probing an artifact", () => { + expect(() => assertPaperclipRunnerBinaryConfiguration("relative-runnerd")).toThrow("PAPERCLIP_RUNNER_BINARY must be an absolute path"); + vi.stubEnv("PAPERCLIP_RUNNER_BINARY", "relative-controller-runnerd"); + expect(() => assertPaperclipRunnerBinaryConfiguration()).toThrow("PAPERCLIP_RUNNER_BINARY must be an absolute path"); + expect(() => resolvePaperclipRunnerBinary()).toThrow("PAPERCLIP_RUNNER_BINARY must be an absolute path"); + expect(() => assertPaperclipRunnerBinaryConfiguration("")).not.toThrow(); + expect(() => assertPaperclipRunnerBinaryConfiguration(join(tmpdir(), "not-present-configuration-only-runnerd"))).not.toThrow(); +}); diff --git a/server/src/services/native-runtime/native-codex-runner.ts b/server/src/services/native-runtime/native-codex-runner.ts index f8e22050f29..9e184e2a202 100644 --- a/server/src/services/native-runtime/native-codex-runner.ts +++ b/server/src/services/native-runtime/native-codex-runner.ts @@ -171,12 +171,22 @@ function executableName(): string { return process.platform === "win32" ? "paperclip-runnerd.exe" : "paperclip-runnerd"; } -export function resolvePaperclipRunnerBinary( - configuredPath = process.env.PAPERCLIP_RUNNER_BINARY, -): string { +function controllerPaperclipRunnerBinaryConfiguration(): string | undefined { + return process.env.PAPERCLIP_RUNNER_BINARY; +} + +export function assertPaperclipRunnerBinaryConfiguration( + configuredPath = controllerPaperclipRunnerBinaryConfiguration(), +): void { if (configuredPath && !isAbsolute(configuredPath)) { throw new Error("PAPERCLIP_RUNNER_BINARY must be an absolute path"); } +} + +export function resolvePaperclipRunnerBinary( + configuredPath = controllerPaperclipRunnerBinaryConfiguration(), +): string { + assertPaperclipRunnerBinaryConfiguration(configuredPath); if (configuredPath) { try { accessSync(configuredPath, constants.R_OK | (process.platform === "win32" ? 0 : constants.X_OK));