fix: preserve process identities in pwntools cleanup errors (#2284)

Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>

src/native/pwntools/PwntoolsDecoder.ts

@@ -32,7 +32,10 @@ import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorClean
 import { ProviderAdapterError } from "../../domain/providerAdapterError.js";
 import { ProviderCleanupError } from "../../domain/providerCleanupError.js";
 import { err, ok, type Result } from "../../domain/result.js";
-import { runOwnedCommand } from "../../process/OwnedCommand.js";
+import {
+  ownedCommandResources,
+  runOwnedCommand,
+} from "../../process/OwnedCommand.js";
 import type {
   ProviderProcessSupervisor,
   OwnedProviderProcessSpawnOptions,
@@ -562,20 +565,16 @@ export class PwntoolsDecoder<
     const pending = this.#pendingCleanup;
     if (pending?.process === undefined) return undefined;
     const stopped = await pending.process.stop();
+    const ownership = pending.process.launch.ownership;
     if (stopped.status === "incomplete")
       return new ProviderCleanupError(
         this.identity.id,
-        [
-          pending.process.launch.ownership?.runId ?? "owned-decoder",
-          ...(pending.root === undefined ? [] : [pending.root.path]),
-          // The retained group may still be live; name it as OwnedCommand does.
-          ...(pending.process.launch.ownership === undefined
-            ? []
-            : [
-                `pid:${pending.process.launch.ownership.leaderPid}`,
-                `process-group:${pending.process.launch.ownership.processGroupId}`,
-              ]),
-        ],
+        ownership === undefined
+          ? [
+              "owned-decoder",
+              ...(pending.root === undefined ? [] : [pending.root.path]),
+            ]
+          : ownedCommandResources(ownership, pending.root?.path),
         {
           reason: stopped.reason,
           captured_output: {

src/process/OwnedCommand.ts

@@ -9,6 +9,7 @@ import {
 } from "./ProviderProcess.js";
 import {
   cleanupOwnedProcessGroup,
+  type OwnedProcessGroup,
   type ProcessCleanupFailure,
 } from "./ProcessOwnership.js";
 import {
@@ -34,6 +35,17 @@ export class OwnedCommandFailure extends Error {
   }
 }
 
+/** Report recorded launch identities; these labels do not authorize signaling. */
+export const ownedCommandResources = (
+  ownership: OwnedProcessGroup,
+  directory?: string,
+): readonly string[] => [
+  ownership.runId,
+  ...(directory === undefined ? [] : [directory]),
+  `pid:${ownership.leaderPid}`,
+  `process-group:${ownership.processGroupId}`,
+];
+
 /** Execute a single owned process with independent cleanup and bounded diagnostic retention. */
 export const runOwnedCommand = async (
   spawn: OwnedProviderProcessSpawnOptions,
@@ -208,12 +220,7 @@ const collectOwnedCommand = async (
           ? {}
           : { cleanupResult: stopped.cleanupResult }),
       },
-      [
-        spawn.runId,
-        ...(spawn.cwd === undefined ? [] : [spawn.cwd]),
-        `pid:${launched.ownership.leaderPid}`,
-        `process-group:${launched.ownership.processGroupId}`,
-      ],
+      ownedCommandResources(launched.ownership, spawn.cwd),
       supervisor,
     );
   if (failure !== undefined)

tests/boundary/process/ownedCommand.test.ts

@@ -257,9 +257,11 @@ it.skipIf(process.platform === "win32").each(["cancelled", "timeout"] as const)(
         failures: [expect.objectContaining({ reason: "run-token-mismatch" })],
       });
       expect(failure.cleanupResult).toEqual(cleanup);
-      expect(failure.resources).toEqual(
-        expect.arrayContaining([request.runId]),
-      );
+      expect(failure.resources).toEqual([
+        request.runId,
+        `pid:${launched.ownership.leaderPid}`,
+        `process-group:${launched.ownership.processGroupId}`,
+      ]);
       // An unresolved member does not grant authority to signal the shared group.
       const acquiredPid = launched.ownership.leaderPid;
       expect(() => process.kill(acquiredPid, 0)).not.toThrow();

tests/process-global/pwntoolsStageRetirement.test.ts

@@ -18,7 +18,10 @@ import {
   PWNTOOLS_MEMORY_FAILURE_EXIT,
   PWNTOOLS_PROVIDER_IDENTITY,
 } from "../../src/native/pwntools/PwntoolsRelease.js";
-import { cleanupOwnedProcessGroup } from "../../src/process/ProcessOwnership.js";
+import {
+  cleanupOwnedProcessGroup,
+  type OwnedProcessGroup,
+} from "../../src/process/ProcessOwnership.js";
 import { prepareProcessOwnershipInspection } from "../../src/process/ProcessOwnershipObservation.js";
 import { spawnOwnedProviderProcess } from "../../src/process/ProviderProcess.js";
 import { PrivateRuntimeRoot } from "../../src/process/PrivateRuntimeRoot.js";
@@ -232,12 +235,13 @@ it.runIf(supportedHost).each([
     const { path, root, environment, basic } = await fixture();
     let launches = 0;
     let cleanupAllowed = false;
+    let retainedOwnership: OwnedProcessGroup | undefined;
     const read = vi.spyOn(artifacts, "readStableArtifact");
     const provider = new PwntoolsRecordedCrashProvider(
       environment,
       async (spawn) => {
         if (stage === "debugger" && ++launches === 1) return basic(spawn);
-        return changingWorker(
+        const launched = await changingWorker(
           spawn,
           join(spawn.cwd ?? "", "resource.failure"),
           {
@@ -247,6 +251,8 @@ it.runIf(supportedHost).each([
           },
           () => cleanupAllowed,
         );
+        retainedOwnership = launched.ownership;
+        return launched;
       },
       async () => root,
     );
@@ -261,14 +267,14 @@ it.runIf(supportedHost).each([
     if (result.ok) throw new Error("Expected unresolved process ownership");
     const projected = projectAnalysisError(result.error);
     expect(projected.code).toBe("cleanup_incomplete");
-    expect(projected).toMatchObject({
-      details: {
-        resources: expect.arrayContaining([
-          expect.stringMatching(/^pid:\d+$/u),
-          expect.stringMatching(/^process-group:\d+$/u),
-        ]),
-      },
-    });
+    if (retainedOwnership === undefined)
+      throw new Error("Expected retained process ownership");
+    expect(projected.details?.resources).toEqual([
+      retainedOwnership.runId,
+      root.path,
+      `pid:${retainedOwnership.leaderPid}`,
+      `process-group:${retainedOwnership.processGroupId}`,
+    ]);
     if (stage === "debugger")
       expect(projected).toMatchObject({
         details: {