From 4f3b573b0922bfe04fb8246f52449629ea42c2ce Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:52:28 +0800 Subject: [PATCH] fix(native): bind signature snapshot cleanup to exact leases Retain roots and descriptors through shared cleanup leases and bind composed errors to their actual pending owners. Successful retries shrink live attribution and reveal the original typed failure. Validate mixed root and descriptor failures, partial/final release, existing target binding, and real codesign and CLI controls. No real OS close-failure or Windows qualification. Refs #4734 --- src/native/SignatureTargetBinding.test.ts | 75 +++++++++++++- src/native/SignatureTargetBinding.ts | 113 ++++++++++++++-------- 2 files changed, 147 insertions(+), 41 deletions(-) diff --git a/src/native/SignatureTargetBinding.test.ts b/src/native/SignatureTargetBinding.test.ts index a5d91f2531..171f0dd7b8 100644 --- a/src/native/SignatureTargetBinding.test.ts +++ b/src/native/SignatureTargetBinding.test.ts @@ -2,7 +2,7 @@ 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 { afterEach, expect, it, vi, onTestFinished } from "vitest"; import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; @@ -12,6 +12,8 @@ import type { NativeCommandRunner } from "./CommandRunner.js"; import type { NativeCallTracer } from "./LldbCallTracer.js"; import { NativeMacOSProvider } from "./NativeMacOSProvider.js"; import { withSignatureTarget } from "./SignatureTargetBinding.js"; +import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; +import { resolvedAnalysisError } from "../domain/analysisErrorBase.js"; const roots: string[] = []; const targets: string[] = []; @@ -92,6 +94,11 @@ it("retains a snapshot root after cleanup fails alongside a digest mismatch", as }); await resources.close(); + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.cleanupResources).toEqual([]); + expect(resolvedAnalysisError(result.error)._tag).toBe( + "AnalysisArtifactChangedError", + ); await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" }); }); @@ -155,6 +162,12 @@ it("retries a retained signature root when the provider client closes", async () ok: true, value: null, }); + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.cleanupResources).toEqual([]); + if (!firstClose.ok) { + expect(firstClose.error.cleanupIncomplete).toBe(false); + expect(firstClose.error.cleanupResources).toEqual([]); + } await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" }); }); @@ -196,3 +209,63 @@ it("waits for an in-flight signature binding before provider close", async () => expect(closeSettled).toBe(true); await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" }); }); + +it("tracks mixed snapshot descriptor and root cleanup through partial release and retry", async () => { + const target = await createTarget("observed", "expected"); + const resources = new ArtifactResourceScope(); + const original = OwnedFileHandle.prototype.close; + let blocked: OwnedFileHandle | undefined; + let attempts = 0; + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementation(function (this: OwnedFileHandle) { + blocked ??= this; + if (this === blocked && ++attempts <= 2) + return Promise.reject( + new Error("controlled snapshot descriptor close failure"), + ); + return original.call(this); + }); + onTestFinished(async () => { + close.mockRestore(); + await resources.close(); + }); + const inspect = vi.fn(async () => ok("must not run")); + const first = await withSignatureTarget( + target, + resources, + inspect, + undefined, + () => createRoot(1), + ); + expect(first.ok).toBe(false); + if (first.ok) throw new Error("Expected both retained owners"); + const snapshot = join(roots[0]!, "target.bin"); + expect(new Set(first.error.cleanupResources)).toEqual( + new Set([snapshot, roots[0]!]), + ); + const retried = await withSignatureTarget( + target, + resources, + inspect, + undefined, + createRoot, + ); + expect(retried.ok).toBe(false); + if (retried.ok) throw new Error("Expected retained descriptor retry failure"); + expect(first.error.cleanupIncomplete).toBe(true); + expect(first.error.cleanupResources).toEqual([snapshot]); + expect(retried.error.cleanupResources).toEqual([snapshot]); + expect(roots).toHaveLength(1); + expect(inspect).not.toHaveBeenCalled(); + await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" }); + await resources.close(); + expect(attempts).toBe(3); + for (const failure of [first.error, retried.error]) { + expect(failure.cleanupIncomplete).toBe(false); + expect(failure.cleanupResources).toEqual([]); + } + expect(resolvedAnalysisError(first.error)._tag).toBe( + "AnalysisArtifactChangedError", + ); +}); diff --git a/src/native/SignatureTargetBinding.ts b/src/native/SignatureTargetBinding.ts index 29c8651e61..97c5b67ceb 100644 --- a/src/native/SignatureTargetBinding.ts +++ b/src/native/SignatureTargetBinding.ts @@ -19,8 +19,22 @@ 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 { + AnalysisCleanupLease, + AnalysisCleanupLeaseGroup, +} from "../artifacts/AnalysisCleanupLease.js"; +import { bindArtifactFileCleanup } from "../artifacts/ArtifactFileOperation.js"; +import type { ArtifactResourceOwner } from "../artifacts/ArtifactResourceScope.js"; + import { NATIVE_MACOS_PROVIDER_IDENTITY } from "./NativeMacOSProviderMetadata.js"; +type SignatureOwner = Extract< + ArtifactResourceOwner, + { readonly kind: "lease" } +> & { + readonly lease: AnalysisCleanupLease; +}; + /** Binding of commands to the selected executable's registered content/version. */ export interface SignatureTargetBinding { readonly identity: string; @@ -95,9 +109,9 @@ export const withSignatureTarget = async ( : snapshotFailure(target, "create", cause), ); } - const owner = { - kind: "private-runtime-root" as const, - root, + const owner: SignatureOwner = { + kind: "lease", + lease: new AnalysisCleanupLease(() => root.close(), [root.path]), resource: root.path, }; let result: Result; @@ -132,14 +146,16 @@ export const withSignatureTarget = async ( ); } const cleanup = await resources.release(owner); - if (cleanup.kind === "failed") - return combineCleanupFailure( + if (cleanup.kind === "failed") { + const failure = signatureCleanupFailure( target, - result, [root.path], cleanup.cause, "cleanup", ); + new AnalysisCleanupLeaseGroup(failure, [owner.lease]); + return combineCleanupFailure(result, failure); + } return result; }); } catch (cause: unknown) { @@ -150,15 +166,12 @@ export const withSignatureTarget = async ( : undefined; if (retained !== undefined) return combineCleanupFailure( - target, err( signal?.aborted ? new AnalysisCancelledError("inspect_signature") : signatureReadFailure(target, cause), ), - retained.resources, - cause, - "retry", + signatureCleanupFailure(target, retained.resources, cause, "retry"), ); return err( signal?.aborted @@ -168,14 +181,28 @@ export const withSignatureTarget = async ( } }; +/** Each composed error follows the exact primary and secondary cleanup owners. */ const combineCleanupFailure = ( - target: BinaryTarget, primary: Result, + cleanup: AnalysisError, +): Result => + primary.ok + ? err(cleanup) + : err( + analysisErrorWithCleanupFailure( + primary.error, + cleanup, + "inspect_signature", + ).bindCleanupDependencies([primary.error, cleanup]), + ); + +const signatureCleanupFailure = ( + target: BinaryTarget, resources: readonly string[], cause: unknown, phase: string, -): Result => { - const cleanup = new ProviderCleanupError( +): AnalysisError => { + const failure = new ProviderCleanupError( NATIVE_MACOS_PROVIDER_IDENTITY.id, resources, { @@ -185,15 +212,8 @@ const combineCleanupFailure = ( }, { operation: "inspect_signature", cause }, ); - return primary.ok - ? err(cleanup) - : err( - analysisErrorWithCleanupFailure( - primary.error, - cleanup, - "inspect_signature", - ), - ); + bindArtifactFileCleanup(cause, failure); + return failure; }; const snapshotFailure = ( @@ -274,8 +294,8 @@ const bindSignatureTarget = async ( resources: ArtifactResourceScope, signal?: AbortSignal, ): Promise> => { - let sourceOwner: OwnedFileHandle | undefined; - let snapshotOwner: OwnedFileHandle | undefined; + let sourceOwner: SignatureOwner | undefined; + let snapshotOwner: SignatureOwner | undefined; const perform = async (): Promise< Result > => { @@ -292,7 +312,15 @@ const bindSignatureTarget = async ( target.path, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK, ); - sourceOwner = new OwnedFileHandle(file); + const sourceHandle = new OwnedFileHandle(file); + sourceOwner = { + kind: "lease", + resource: target.path, + lease: new AnalysisCleanupLease( + () => sourceHandle.close(), + [target.path], + ), + }; const opened = await file.stat({ bigint: true }); signal?.throwIfAborted(); if (identity(opened) !== identity(before)) @@ -303,7 +331,15 @@ const bindSignatureTarget = async ( const snapshot = await snapshotIO(target, "create-file", () => open(snapshotPath, "wx", 0o600), ); - snapshotOwner = new OwnedFileHandle(snapshot); + const snapshotHandle = new OwnedFileHandle(snapshot); + snapshotOwner = { + kind: "lease", + resource: snapshotPath, + lease: new AnalysisCleanupLease( + () => snapshotHandle.close(), + [snapshotPath], + ), + }; const observed = await copySignatureBytes( target, { file, stat: opened }, @@ -338,34 +374,31 @@ const bindSignatureTarget = async ( }; let result = await perform(); const cleanupFailures: { - readonly path: string; + readonly owner: SignatureOwner; readonly cause: unknown; }[] = []; - for (const [owner, path] of [ - [snapshotOwner, snapshotPath], - [sourceOwner, target.path], - ] as const) { + for (const owner of [snapshotOwner, sourceOwner]) { if (owner === undefined) continue; - const cleanup = await resources.release({ - kind: "file-handle", - handle: owner, - resource: path, - }); + const cleanup = await resources.release(owner); if (cleanup.kind === "failed") - cleanupFailures.push({ path, cause: cleanup.cause }); + cleanupFailures.push({ owner, 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( + const cleanup = signatureCleanupFailure( target, - result, - cleanupFailures.map(({ path }) => path), + cleanupFailures.map(({ owner }) => owner.resource), cause, "close", ); + new AnalysisCleanupLeaseGroup( + cleanup, + cleanupFailures.map(({ owner }) => owner.lease), + ); + result = combineCleanupFailure(result, cleanup); } return result; };