diff --git a/doc/observability.md b/doc/observability.md index bae41823478..5065e511ecc 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -611,6 +611,16 @@ They are children of the existing pack selection/upload path and also appear in the local run timing log. Their fixed names add no paths, credentials, or content attributes. The operator endpoint gate remains unchanged. Source installs without prebuilt archives retain the existing directory upload span. +`provider_pack.directory.verify` measures expanded source verification when +directory staging is required. + +Archive installation verifies the archive bytes and then the installed pack +against the selected manifest. It does not hash the unused expanded controller +tree. Directory installation still verifies that tree before upload, including +when the remote cannot extract an available archive. The final installed-pack +verification applies to both paths. A slow controller tree scan can block the +server event loop and inflate overlapping asynchronous span durations; a copy +span alone does not establish that the disk spent all that time copying. The native provider lifecycle emits one foldable OpenTelemetry trace, linked to the server attempt described above. This is diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index 650908db374..bddb90efb6d 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -65,7 +65,7 @@ import { nativeCompletionFeedback } from "./native-completion-feedback.js"; import { hasAcknowledgedNativeReassignmentStopIntent, hasAcknowledgedNativeStopIntent } from "../acknowledged-native-stop.js"; import { stoppedCodexTurnIsTextOnly } from "./stopped-codex-turn.js"; import { prepareVerifiedRemoteProviderPack, useVerifiedPreinstalledRemoteProviderPack, shouldUseBundledAcpxImageAssets } from "./remote-provider-pack.js"; -import { readProviderPackArchive, stageProviderPackArchive } from "./provider-pack-archive.js"; +import { uploadVerifiedProviderPack } from "./provider-pack-upload.js"; import { selectRemotePiCompanion } from "./remote-pi-companion.js"; import { readNativeLocalProcessStop, PROCESS_START_REQUESTED, withReleasedNativeRunProcess } from "../native-local-process-stop.js"; import { remoteLeaseCleanupScope } from "../remote-execution-termination.js"; @@ -12740,46 +12740,24 @@ async function createRunnerdBackendWithinSessionClaim( "runner_remote_provider_artifact_incompatible: this remote transport cannot stage a provider pack; preinstall the exact manifest-matched pack", ); } - const archive = await measureNativeRunnerSpan(input.trace, "provider_pack.archive.verify", () => { - readRemoteProviderPackManifest(configuredProviderPackRoot); - return readProviderPackArchive(configuredProviderPackRoot, expectedProviderPackManifest!.digest); - }, - ); - const escapedPackRoot = stagedRemoteProviderPackRoot.replaceAll( - "'", - "'\\''", - ); - const cleared = await measureNativeRunnerSpan(input.trace, "provider_pack.clear_stale", () => remoteCommandRunner.execute({ - command: "sh", - args: ["-c", `rm -rf '${escapedPackRoot}'`], + await uploadVerifiedProviderPack({ + root: configuredProviderPackRoot, + expectedDigest: expectedProviderPackManifest!.digest, + runner: remoteCommandRunner, cwd: remoteTarget.remoteCwd, - bypassSession: true, - timeoutMs: 10_000, - })); - if (cleared.exitCode !== 0 || cleared.timedOut) { - throw new Error( - "runner_remote_provider_artifact_incompatible: stale provider pack could not be replaced", - ); - } - await measureNativeRunnerSpan(input.trace, "provider_pack.upload", async () => { - if (archive && await stageProviderPackArchive({ - archive, - runner: remoteCommandRunner, - targetPath: stagedRemoteProviderPackRoot, - reuseRoots: preinstalledProviderPackRoots, - trace: input.trace, - })) return; - await stageRemoteRunnerDirectory({ + targetPath: stagedRemoteProviderPackRoot, + reuseRoots: preinstalledProviderPackRoots, + trace: input.trace, + verifyDirectory: () => { readRemoteProviderPackManifest(configuredProviderPackRoot); }, + stageDirectory: () => stageRemoteRunnerDirectory({ target: remoteTarget, runner: remoteCommandRunner, sourcePath: configuredProviderPackRoot, targetPath: stagedRemoteProviderPackRoot, mode: 0o700, - }); + }), + verifyUploaded: () => verifyRemoteProviderPack(stagedRemoteProviderPackRoot), }); - await measureNativeRunnerSpan(input.trace, "provider_pack.verify_uploaded", () => - verifyRemoteProviderPack(stagedRemoteProviderPackRoot), - ); activeRemoteProviderPackRoot = stagedRemoteProviderPackRoot; }, }); diff --git a/server/src/services/native-runtime/provider-pack-archive.test.ts b/server/src/services/native-runtime/provider-pack-archive.test.ts index 8e150542c44..c27b8098d9d 100644 --- a/server/src/services/native-runtime/provider-pack-archive.test.ts +++ b/server/src/services/native-runtime/provider-pack-archive.test.ts @@ -7,6 +7,7 @@ import { promisify } from "node:util"; import { afterEach, expect, it, vi } from "vitest"; import type { CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime"; import { readProviderPackArchive, stageProviderPackArchive } from "./provider-pack-archive.js"; +import { uploadVerifiedProviderPack } from "./provider-pack-upload.js"; // @ts-expect-error Build-time script has no TypeScript declaration. import { createProviderPackArchive, createProviderPackLayers } from "../../../../scripts/create-provider-pack-archive.mjs"; @@ -211,6 +212,54 @@ it("keeps the directory fallback when the remote has no zstd", async () => { expect(remote.syncIn).not.toHaveBeenCalled(); }); +it.skipIf(!tar || !hasZstd).each(["v1", "v2"])("installs verified %s archive bytes without reading the unused expanded pack", async version => { + const { root, pack } = version === "v1" ? await fixture() : await layeredFixture(); + const remote = await runner(root); + const targetPath = path.join(root, "uploaded"); + const expected = await fs.readFile(path.join(pack, "bin/tool")); + // Only the archive is uploaded. The expanded tree is a different source, + // and deliberately no longer matches the bytes sealed in that archive. + await fs.writeFile(path.join(pack, "bin/tool"), "unused expanded bytes"); + const verifyDirectory = vi.fn(() => { throw Error("expanded_directory_was_read"); }); + const stageDirectory = vi.fn(async () => { throw Error("directory_was_uploaded"); }); + const verifyUploaded = vi.fn(async () => { + expect(await fs.readFile(path.join(targetPath, "bin/tool"))).toEqual(expected); + expect(JSON.parse(await fs.readFile(path.join(targetPath, "provider-pack.json"), "utf8")).digest).toBe(digest); + }); + await uploadVerifiedProviderPack({ root: pack, expectedDigest: digest, runner: remote, cwd: root, + targetPath, reuseRoots: [], verifyDirectory, stageDirectory, verifyUploaded }); + expect(verifyDirectory).not.toHaveBeenCalled(); + expect(stageDirectory).not.toHaveBeenCalled(); + expect(verifyUploaded).toHaveBeenCalledOnce(); +}); + +it.each(["absent", "unsupported"])("verifies directory bytes before fallback when the archive is %s", async mode => { + const { root, pack } = await fixture(); + if (mode === "absent") await fs.unlink(`${pack}.archive.json`); + const remote = { syncIn: vi.fn(), execute: vi.fn().mockResolvedValue({ exitCode: 0, timedOut: false, stdout: "", stderr: "" }) }; + // The capability probe fails; clearing the old pack succeeds. + remote.execute.mockResolvedValueOnce({ exitCode: 0, timedOut: false }); + remote.execute.mockResolvedValue({ exitCode: 127, timedOut: false }); + const stageDirectory = vi.fn(), verifyUploaded = vi.fn(); + await expect(uploadVerifiedProviderPack({ root: pack, expectedDigest: digest, runner: remote, cwd: root, + targetPath: path.join(root, "uploaded"), reuseRoots: [], verifyDirectory: () => { throw Error("directory_digest_mismatch"); }, + stageDirectory, verifyUploaded })).rejects.toThrow("directory_digest_mismatch"); + expect(remote.syncIn).not.toHaveBeenCalled(); + expect(stageDirectory).not.toHaveBeenCalled(); + expect(verifyUploaded).not.toHaveBeenCalled(); +}); + +it.skipIf(!tar || !hasZstd)("rejects an installed pack that fails final verification", async () => { + const { root, pack } = await fixture(); + const remote = await runner(root), targetPath = path.join(root, "uploaded"); + const verifyUploaded = vi.fn(async () => { throw Error("installed_digest_mismatch"); }); + await expect(uploadVerifiedProviderPack({ root: pack, expectedDigest: digest, runner: remote, cwd: root, + targetPath, reuseRoots: [], verifyDirectory: () => {}, stageDirectory: async () => {}, verifyUploaded })) + .rejects.toThrow("installed_digest_mismatch"); + expect(remote.syncIn).toHaveBeenCalledOnce(); + expect(verifyUploaded).toHaveBeenCalledOnce(); +}); + it.skipIf(!tar || !hasZstd).each([ ["v1", "bsd-tar"], ["v2", "bsd-tar"], ["v1", "older-gnu-tar"], ["v2", "older-gnu-tar"], diff --git a/server/src/services/native-runtime/provider-pack-upload.ts b/server/src/services/native-runtime/provider-pack-upload.ts new file mode 100644 index 00000000000..aff0ca42564 --- /dev/null +++ b/server/src/services/native-runtime/provider-pack-upload.ts @@ -0,0 +1,44 @@ +import type { CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime"; +import type { NativeRunTrace } from "./native-run-trace.js"; +import { readProviderPackArchive, stageProviderPackArchive } from "./provider-pack-archive.js"; + +const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`; + +/** Upload a pack and verify the installed bytes before exposing it to a provider. */ +export async function uploadVerifiedProviderPack(input: { + root: string; + expectedDigest: string; + runner: CommandManagedRuntimeRunner; + cwd: string; + targetPath: string; + reuseRoots: readonly string[]; + trace?: NativeRunTrace; + verifyDirectory: () => void; + stageDirectory: () => Promise; + verifyUploaded: () => Promise; +}): Promise { + const measure = (name: string, fn: () => Promise) => input.trace ? input.trace.measure(name, fn) : fn(); + const verifyDirectory = () => measure("provider_pack.directory.verify", async () => input.verifyDirectory()); + // Archive installation reads the sealed archives, not the expanded tree. + // Hash the directory only when it is actually the upload source. The + // installed pack still passes full verification against the selected identity. + const archive = await measure("provider_pack.archive.verify", () => + readProviderPackArchive(input.root, input.expectedDigest)); + if (!archive) await verifyDirectory(); + const cleared = await measure("provider_pack.clear_stale", () => input.runner.execute({ + command: "sh", args: ["-c", `rm -rf ${quote(input.targetPath)}`], + cwd: input.cwd, bypassSession: true, timeoutMs: 10_000, + })); + if (cleared.exitCode !== 0 || cleared.timedOut) { + throw new Error("runner_remote_provider_artifact_incompatible: stale provider pack could not be replaced"); + } + await measure("provider_pack.upload", async () => { + if (archive && await stageProviderPackArchive({ + archive, runner: input.runner, targetPath: input.targetPath, + reuseRoots: input.reuseRoots, trace: input.trace, + })) return; + if (archive) await verifyDirectory(); + await input.stageDirectory(); + }); + await measure("provider_pack.verify_uploaded", input.verifyUploaded); +}