diff --git a/src/artifacts/ArtifactResourceScope.ts b/src/artifacts/ArtifactResourceScope.ts index 59fdcba367..7c0bcf2c2b 100644 --- a/src/artifacts/ArtifactResourceScope.ts +++ b/src/artifacts/ArtifactResourceScope.ts @@ -6,6 +6,7 @@ import type { OwnedStagedOutput } from "../filesystem/OwnedStagedOutput.js"; import type { ArtifactReader } from "./ArtifactReader.js"; import { ArtifactReaderFailure } from "./ArtifactReader.js"; import type { SafeOutputTree } from "./SafeOutputTree.js"; +import type { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; /** One artifact resource and its format-specific cleanup owner. */ export type ArtifactResourceOwner = @@ -38,6 +39,11 @@ export type ArtifactResourceOwner = readonly kind: "staged-output"; readonly resource: string; readonly output: OwnedStagedOutput; + } + | { + readonly kind: "private-runtime-root"; + readonly resource: string; + readonly root: Pick; }; export type ArtifactCleanupAttempt = @@ -161,5 +167,8 @@ const cleanup = async (owner: ArtifactResourceOwner): Promise => { return; case "staged-output": await owner.output.close(); + return; + case "private-runtime-root": + await owner.root.close(); } }; diff --git a/src/native/NativeMacOSProvider.ts b/src/native/NativeMacOSProvider.ts index 2ae6a7cb66..1d1c2a36c4 100644 --- a/src/native/NativeMacOSProvider.ts +++ b/src/native/NativeMacOSProvider.ts @@ -84,16 +84,24 @@ import { inspectNativeMacho, } from "./NativeMachoInspection.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import type { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; import { createNativeUiHelperRuntime, type NativeUiHelperRuntime, } from "./NativeUiHelperRuntime.js"; +type SignatureRuntimeRootFactory = () => Promise< + Pick +>; + /** Read-only semantic provider composed from Xcode command-line utilities. */ export class NativeMacOSProvider implements AnalysisProvider { readonly #capabilities: readonly CapabilityDescriptor[]; readonly #environment: NodeJS.ProcessEnv; readonly #tracerFactory: () => NativeCallTracer; + readonly #privateRuntimeRootFactory: SignatureRuntimeRootFactory | undefined; constructor( environment: Readonly, @@ -102,10 +110,12 @@ export class NativeMacOSProvider implements AnalysisProvider { ), platform: NodeJS.Platform = process.platform, tracerFactory?: () => NativeCallTracer, + privateRuntimeRootFactory?: SignatureRuntimeRootFactory, ) { this.#environment = snapshotEnvironment(environment, platform); this.#tracerFactory = tracerFactory ?? (() => new LldbCallTracer(this.#environment)); + this.#privateRuntimeRootFactory = privateRuntimeRootFactory; this.#capabilities = nativeMacOSCapabilities(platform); } @@ -123,6 +133,7 @@ export class NativeMacOSProvider implements AnalysisProvider { this.runner, this.#tracerFactory(), this.#environment, + this.#privateRuntimeRootFactory, ); } } @@ -130,6 +141,7 @@ export class NativeMacOSProvider implements AnalysisProvider { class NativeMacOSClient implements AnalysisClient { readonly #pendingNativeUiRuntimes = new Set(); readonly #pendingCommands = new Set>(); + readonly #signatureResources = new ArtifactResourceScope(); #nativeUiTail: Promise = Promise.resolve(); #closed = false; @@ -138,6 +150,9 @@ class NativeMacOSClient implements AnalysisClient { private readonly runner: NativeCommandRunner, private readonly tracer: NativeCallTracer, private readonly environment: Readonly, + private readonly privateRuntimeRootFactory: + | SignatureRuntimeRootFactory + | undefined, ) {} async execute( @@ -270,6 +285,21 @@ class NativeMacOSClient implements AnalysisClient { const uiCleanupFailures = await this.#serializeNativeUi(() => this.#retryNativeUiCleanup(), ); + let signatureCleanup: AnalysisError | undefined; + try { + await this.#signatureResources.close(); + } catch (cause: unknown) { + const observation = ArtifactReaderFailure.cleanupObservation( + cause, + "signature-snapshot", + ); + signatureCleanup = new ProviderCleanupError( + IDENTITY.id, + observation.resources, + { reason: observation.reason }, + { operation: "close_binary", cause }, + ); + } const commandCleanup = await this.#retryNativeCommandCleanup(); let result = await this.tracer.close(); const cleanupErrors = [ @@ -277,6 +307,7 @@ class NativeMacOSClient implements AnalysisClient { ? [] : [this.#nativeUiCleanupFailure(uiCleanupFailures, "close_binary")]), ...(commandCleanup === undefined ? [] : [commandCleanup]), + ...(signatureCleanup === undefined ? [] : [signatureCleanup]), ]; for (const cleanupError of cleanupErrors) result = result.ok @@ -532,8 +563,10 @@ class NativeMacOSClient implements AnalysisClient { ): Promise> { return withSignatureTarget( this.target, + this.#signatureResources, (binding) => this.#inspectBoundSignature(binding, signal), signal, + this.privateRuntimeRootFactory, ); } diff --git a/src/native/SignatureTargetBinding.test.ts b/src/native/SignatureTargetBinding.test.ts new file mode 100644 index 0000000000..a5d91f2531 --- /dev/null +++ b/src/native/SignatureTargetBinding.test.ts @@ -0,0 +1,198 @@ +import { createHash } from "node:crypto"; +import { mkdtemp, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, expect, it } from "vitest"; + +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { ok } from "../domain/result.js"; +import type { NativeCommandRunner } from "./CommandRunner.js"; +import type { NativeCallTracer } from "./LldbCallTracer.js"; +import { NativeMacOSProvider } from "./NativeMacOSProvider.js"; +import { withSignatureTarget } from "./SignatureTargetBinding.js"; + +const roots: string[] = []; +const targets: string[] = []; +const unusedRunner: NativeCommandRunner = { + async run() { + throw new Error("Changed target must be rejected before a command runs"); + }, +}; +const unusedTracer: NativeCallTracer = { + async trace() { + throw new Error("Signature inspection must not start a trace"); + }, + async close() { + return ok(null); + }, +}; + +afterEach(async () => { + await Promise.all( + [...roots, ...targets].map((path) => + rm(path, { recursive: true, force: true }), + ), + ); + roots.length = 0; + targets.length = 0; +}); + +const createTarget = async ( + bytes: string, + digest = bytes, +): Promise => { + const directory = await mkdtemp(join(tmpdir(), "rea-signature-target-")); + targets.push(directory); + const path = join(directory, "target.bin"); + await writeFile(path, bytes); + return { + path, + sha256: createHash("sha256").update(digest).digest("hex"), + kind: "executable", + format: "mach-o", + architecture: "arm64", + availableArchitectures: ["arm64"], + }; +}; + +const createRoot = async (failedAttempts = 0) => { + const path = await mkdtemp(join(tmpdir(), "rea-signature-root-")); + roots.push(path); + let attempts = 0; + return { + path, + async close() { + if (attempts++ < failedAttempts) { + throw new Error("injected root cleanup failure"); + } + await rm(path, { recursive: true, force: true }); + }, + }; +}; + +it("retains a snapshot root after cleanup fails alongside a digest mismatch", async () => { + const target = await createTarget("observed", "expected"); + const resources = new ArtifactResourceScope(); + const result = await withSignatureTarget( + target, + resources, + async () => ok("unused"), + undefined, + () => createRoot(1), + ); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupResources).toEqual(expect.arrayContaining(roots)); + expect(projectAnalysisError(result.error)).toMatchObject({ + details: { diagnostics: { primary_error: { code: "artifact_changed" } } }, + }); + + await resources.close(); + await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" }); +}); + +it("returns a retained cleanup failure when the next call cannot retry its root", async () => { + const target = await createTarget("observed", "expected"); + const resources = new ArtifactResourceScope(); + const createFailingRoot = async () => { + const path = await mkdtemp(join(tmpdir(), "rea-signature-root-sticky-")); + roots.push(path); + return { + path, + close: async () => { + throw new Error("still blocked"); + }, + }; + }; + const first = await withSignatureTarget( + target, + resources, + async () => ok("unused"), + undefined, + createFailingRoot, + ); + expect(first.ok).toBe(false); + const second = await withSignatureTarget( + target, + resources, + async () => ok("must not run"), + undefined, + createRoot, + ); + expect(second.ok).toBe(false); + if (second.ok) return; + expect(second.error.cleanupIncomplete).toBe(true); + expect(second.error.cleanupResources).toEqual(expect.arrayContaining(roots)); +}); + +it("retries a retained signature root when the provider client closes", async () => { + const target = await createTarget("observed", "expected"); + const provider = new NativeMacOSProvider( + {}, + unusedRunner, + "darwin", + () => unusedTracer, + () => createRoot(2), + ); + const client = provider.createClient(target); + const result = await client.execute("inspect_signature", {}); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupResources).toEqual(expect.arrayContaining(roots)); + const firstClose = await client.close(); + expect(firstClose).toMatchObject({ + ok: false, + error: { cleanupIncomplete: true, cleanupResources: roots }, + }); + expect((await stat(roots[0]!)).isDirectory()).toBe(true); + await expect(client.close()).resolves.toMatchObject({ + ok: true, + value: null, + }); + await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" }); +}); + +it("waits for an in-flight signature binding before provider close", async () => { + const target = await createTarget("observed", "expected"); + let rootFactoryEntered!: () => void; + let releaseRootFactory!: () => void; + const entered = new Promise( + (resolve) => (rootFactoryEntered = resolve), + ); + const gate = new Promise((resolve) => (releaseRootFactory = resolve)); + const provider = new NativeMacOSProvider( + {}, + unusedRunner, + "darwin", + () => unusedTracer, + async () => { + rootFactoryEntered(); + await gate; + return createRoot(); + }, + ); + const client = provider.createClient(target); + const operation = client.execute("inspect_signature", {}); + await entered; + + let closeSettled = false; + const closing = client.close().finally(() => { + closeSettled = true; + }); + await Promise.resolve(); + expect(closeSettled).toBe(false); + releaseRootFactory(); + await expect(operation).resolves.toMatchObject({ + ok: false, + error: { _tag: "AnalysisArtifactChangedError" }, + }); + await expect(closing).resolves.toMatchObject({ ok: true, value: null }); + expect(closeSettled).toBe(true); + await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" }); +}); diff --git a/src/native/SignatureTargetBinding.ts b/src/native/SignatureTargetBinding.ts index ac94a5f067..29c8651e61 100644 --- a/src/native/SignatureTargetBinding.ts +++ b/src/native/SignatureTargetBinding.ts @@ -1,7 +1,6 @@ import { createHash } from "node:crypto"; import { constants, type BigIntStats } from "node:fs"; -import { lstat, mkdtemp, open, rm, type FileHandle } from "node:fs/promises"; -import { tmpdir } from "node:os"; +import { lstat, open, type FileHandle } from "node:fs/promises"; import { basename, join } from "node:path"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; @@ -9,10 +8,17 @@ import { AnalysisAccessDeniedError, AnalysisArtifactChangedError, AnalysisCancelledError, + AnalysisOutputError, AnalysisResourceConstraintError, } from "../domain/analysisErrorCore.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { err, ok, type Result } from "../domain/result.js"; +import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { NATIVE_MACOS_PROVIDER_IDENTITY } from "./NativeMacOSProviderMetadata.js"; /** Binding of commands to the selected executable's registered content/version. */ @@ -68,41 +74,126 @@ const signatureReadFailure = ( /** Inspect a digest-verified private copy, releasing it on every exit path. */ export const withSignatureTarget = async ( target: BinaryTarget, + resources: ArtifactResourceScope, inspect: ( binding: SignatureTargetBinding, ) => Promise>, signal?: AbortSignal, + createRoot: () => Promise> = () => + PrivateRuntimeRoot.create({ prefix: "rea-signature-" }), ): Promise> => { - let directory: string; try { - signal?.throwIfAborted(); - directory = await mkdtemp(join(tmpdir(), "rea-signature-")); + return await resources.run(async () => { + let root: Pick; + try { + signal?.throwIfAborted(); + root = await createRoot(); + } catch (cause: unknown) { + return err( + signal?.aborted + ? new AnalysisCancelledError("inspect_signature") + : snapshotFailure(target, "create", cause), + ); + } + const owner = { + kind: "private-runtime-root" as const, + root, + resource: root.path, + }; + let result: Result; + try { + const binding = await bindSignatureTarget( + target, + join(root.path, basename(target.path)), + resources, + signal, + ); + if (!binding.ok) result = binding; + else { + try { + result = await inspect(binding.value); + } catch (cause: unknown) { + result = err( + cause instanceof AnalysisError + ? cause + : new AnalysisOutputError( + "inspect_signature", + "Native output parsing failed", + { cause }, + ), + ); + } + } + } catch (cause: unknown) { + result = err( + cause instanceof AnalysisError + ? cause + : signatureReadFailure(target, cause), + ); + } + const cleanup = await resources.release(owner); + if (cleanup.kind === "failed") + return combineCleanupFailure( + target, + result, + [root.path], + cleanup.cause, + "cleanup", + ); + return result; + }); } catch (cause: unknown) { + if (cause instanceof AnalysisError) return err(cause); + const retained = + cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined + ? cause.cleanup + : undefined; + if (retained !== undefined) + return combineCleanupFailure( + target, + err( + signal?.aborted + ? new AnalysisCancelledError("inspect_signature") + : signatureReadFailure(target, cause), + ), + retained.resources, + cause, + "retry", + ); return err( signal?.aborted ? new AnalysisCancelledError("inspect_signature") - : snapshotFailure(target, "create", cause), + : signatureReadFailure(target, cause), ); } - let result: Result; - let cleanup: Result; - try { - const binding = await bindSignatureTarget( - target, - join(directory, basename(target.path)), - signal, - ); - result = binding.ok ? await inspect(binding.value) : binding; - } finally { - try { - await rm(directory, { recursive: true, force: true }); - cleanup = ok(undefined); - } catch (cause: unknown) { - cleanup = err(snapshotFailure(target, "cleanup", cause)); - } - } - // Cleanup cannot replace the operation's original actionable failure. - return result.ok && !cleanup.ok ? cleanup : result; +}; + +const combineCleanupFailure = ( + target: BinaryTarget, + primary: Result, + resources: readonly string[], + cause: unknown, + phase: string, +): Result => { + const cleanup = new ProviderCleanupError( + NATIVE_MACOS_PROVIDER_IDENTITY.id, + resources, + { + reason: cause instanceof Error ? cause.message : String(cause), + phase: `signature-snapshot-${phase}`, + path: target.path, + }, + { operation: "inspect_signature", cause }, + ); + return primary.ok + ? err(cleanup) + : err( + analysisErrorWithCleanupFailure( + primary.error, + cleanup, + "inspect_signature", + ), + ); }; const snapshotFailure = ( @@ -180,22 +271,28 @@ const copySignatureBytes = async ( const bindSignatureTarget = async ( target: BinaryTarget, snapshotPath: string, + resources: ArtifactResourceScope, signal?: AbortSignal, ): Promise> => { - try { - signal?.throwIfAborted(); - const before = await lstat(target.path, { bigint: true }); - signal?.throwIfAborted(); - if (!before.isFile() || before.isSymbolicLink()) - return changedTarget( - target, - `Signature target is no longer a regular file: ${target.path}`, - ); - const file = await open( - target.path, - constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK, - ); + let sourceOwner: OwnedFileHandle | undefined; + let snapshotOwner: OwnedFileHandle | undefined; + const perform = async (): Promise< + Result + > => { try { + signal?.throwIfAborted(); + const before = await lstat(target.path, { bigint: true }); + signal?.throwIfAborted(); + if (!before.isFile() || before.isSymbolicLink()) + return changedTarget( + target, + `Signature target is no longer a regular file: ${target.path}`, + ); + const file = await open( + target.path, + constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK, + ); + sourceOwner = new OwnedFileHandle(file); const opened = await file.stat({ bigint: true }); signal?.throwIfAborted(); if (identity(opened) !== identity(before)) @@ -206,44 +303,71 @@ const bindSignatureTarget = async ( const snapshot = await snapshotIO(target, "create-file", () => open(snapshotPath, "wx", 0o600), ); - try { - const observed = await copySignatureBytes( + snapshotOwner = new OwnedFileHandle(snapshot); + const observed = await copySignatureBytes( + target, + { file, stat: opened }, + snapshot, + signal, + ); + if (observed !== target.sha256) + return changedTarget( target, - { file, stat: opened }, - snapshot, - signal, + `Signature target digest changed: ${target.path}; expected ${target.sha256}, observed ${observed}`, ); - if (observed !== target.sha256) - return changedTarget( - target, - `Signature target digest changed: ${target.path}; expected ${target.sha256}, observed ${observed}`, - ); - const after = await file.stat({ bigint: true }); - const current = await lstat(target.path, { bigint: true }); - signal?.throwIfAborted(); - if ( - identity(after) !== identity(opened) || - identity(current) !== identity(opened) - ) - return changedTarget( - target, - `Signature target changed while establishing its version: ${target.path}`, - ); - await snapshotIO(target, "seal", () => snapshot.chmod(0o400)); - return ok({ identity: identity(opened), snapshotPath }); - } finally { - await snapshotIO(target, "close", () => snapshot.close()); - } - } finally { - await file.close(); + const after = await file.stat({ bigint: true }); + const current = await lstat(target.path, { bigint: true }); + signal?.throwIfAborted(); + if ( + identity(after) !== identity(opened) || + identity(current) !== identity(opened) + ) + return changedTarget( + target, + `Signature target changed while establishing its version: ${target.path}`, + ); + await snapshotIO(target, "seal", () => snapshot.chmod(0o400)); + return ok({ identity: identity(opened), snapshotPath }); + } catch (cause: unknown) { + return err( + signal?.aborted + ? new AnalysisCancelledError("inspect_signature") + : signatureReadFailure(target, cause), + ); } - } catch (cause: unknown) { - return err( - signal?.aborted - ? new AnalysisCancelledError("inspect_signature") - : signatureReadFailure(target, cause), + }; + let result = await perform(); + const cleanupFailures: { + readonly path: string; + readonly cause: unknown; + }[] = []; + for (const [owner, path] of [ + [snapshotOwner, snapshotPath], + [sourceOwner, target.path], + ] as const) { + if (owner === undefined) continue; + const cleanup = await resources.release({ + kind: "file-handle", + handle: owner, + resource: path, + }); + if (cleanup.kind === "failed") + cleanupFailures.push({ path, cause: cleanup.cause }); + } + if (cleanupFailures.length > 0) { + const cause = new AggregateError( + cleanupFailures.map(({ cause: failure }) => failure), + "Signature snapshot file handles could not be closed", + ); + result = combineCleanupFailure( + target, + result, + cleanupFailures.map(({ path }) => path), + cause, + "close", ); } + return result; }; /** Reject target drift across signature command captures. */ diff --git a/tests/boundary/providers/native/nativeSignatureTargetBinding.test.ts b/tests/boundary/providers/native/nativeSignatureTargetBinding.test.ts index 25b9e07303..b622e2d970 100644 --- a/tests/boundary/providers/native/nativeSignatureTargetBinding.test.ts +++ b/tests/boundary/providers/native/nativeSignatureTargetBinding.test.ts @@ -15,6 +15,7 @@ import { import { dirname, join } from "node:path"; import { describe, expect, it } from "vitest"; import { NativeMacOSProvider } from "../../../../src/native/NativeMacOSProvider.js"; +import { ArtifactResourceScope } from "../../../../src/artifacts/ArtifactResourceScope.js"; import { withSignatureTarget, verifySignatureTarget, @@ -277,6 +278,7 @@ it("returns typed cancellation at binding entry and final verification", async ( await expect( withSignatureTarget( target, + new ArtifactResourceScope(), async () => { throw new Error("Cancelled acquisition must not invoke inspection"); }, @@ -286,14 +288,18 @@ it("returns typed cancellation at binding entry and final verification", async ( ok: false, error: { _tag: "AnalysisCancelledError" }, }); - await withSignatureTarget(target, async (binding) => { - const result = await verifySignatureTarget(target, binding, signal); - expect(result).toMatchObject({ - ok: false, - error: { _tag: "AnalysisCancelledError" }, - }); - return result; - }); + await withSignatureTarget( + target, + new ArtifactResourceScope(), + async (binding) => { + const result = await verifySignatureTarget(target, binding, signal); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "AnalysisCancelledError" }, + }); + return result; + }, + ); }); it("preserves cancellation after a capture instead of classifying it as invalid output", async () => { @@ -348,21 +354,31 @@ it("copies and hashes the same bytes across multiple snapshot chunks", async () const bytes = Buffer.alloc(128 * 1024 + 13, 42); await writeFile(path, bytes); const target = await nativeMachoTargetForFile(path); - await withSignatureTarget(target, async (binding) => { - expect(await readFile(binding.snapshotPath)).toEqual(bytes); - return verifySignatureTarget(target, binding); - }); + await withSignatureTarget( + target, + new ArtifactResourceScope(), + async (binding) => { + expect(await readFile(binding.snapshotPath)).toEqual(bytes); + return verifySignatureTarget(target, binding); + }, + ); }); it("cleans the private snapshot when inspection throws", async () => { const { target } = await fixture(); let snapshotPath: string | undefined; - await expect( - withSignatureTarget(target, async (binding) => { + const result = await withSignatureTarget( + target, + new ArtifactResourceScope(), + async (binding) => { snapshotPath = binding.snapshotPath; throw new Error("Invalid parsed output"); - }), - ).rejects.toThrow("Invalid parsed output"); + }, + ); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "AnalysisOutputError" }, + }); if (snapshotPath === undefined) throw new Error("Expected snapshot"); await expect(lstat(dirname(snapshotPath))).rejects.toMatchObject({ code: "ENOENT",