From e2d4c0dabbcf1e9e0ee0249706eff906192e4188 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:26:16 +0800 Subject: [PATCH] fix(process): preserve environment snapshots and final cancellation --- .../analysis/JavaScriptAnalysisWorker.ts | 8 ++ src/pcap/PcapCommand.ts | 1 - ...riptWorkerFinalizationCancellation.test.ts | 99 +++++++++++++++++++ .../workerFailureComposition.test.ts | 23 ++++- .../cliSelectedEnvironment.test.ts | 49 +++++++++ 5 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 tests/boundary/javascript/javascriptWorkerFinalizationCancellation.test.ts diff --git a/src/javascript/analysis/JavaScriptAnalysisWorker.ts b/src/javascript/analysis/JavaScriptAnalysisWorker.ts index 2efa87f177..c75c5703d0 100644 --- a/src/javascript/analysis/JavaScriptAnalysisWorker.ts +++ b/src/javascript/analysis/JavaScriptAnalysisWorker.ts @@ -550,6 +550,14 @@ export class JavaScriptAnalysisWorker { }); } } + if (result.ok && signal?.aborted === true) { + const error = new AnalysisCancelledError(OPERATION, { + cause: signal.reason, + capturedOutput: this.#captured(), + }); + this.#lastFailure = error; + result = err({ error, javascript, module, projection }); + } if (this.#applicationProjectionFailure !== undefined) return result.ok ? ok({ diff --git a/src/pcap/PcapCommand.ts b/src/pcap/PcapCommand.ts index b7ac8d50a8..bcebcb2655 100644 --- a/src/pcap/PcapCommand.ts +++ b/src/pcap/PcapCommand.ts @@ -84,7 +84,6 @@ export const runPcapCommand = async (options: { if (signal?.aborted) throw new AnalysisCancelledError(operation, { cause: signal.reason }); const hostEnvironment = { - ...process.env, ...options.environment, LC_ALL: "C", LANG: "C", diff --git a/tests/boundary/javascript/javascriptWorkerFinalizationCancellation.test.ts b/tests/boundary/javascript/javascriptWorkerFinalizationCancellation.test.ts new file mode 100644 index 0000000000..f4b0ae5f5f --- /dev/null +++ b/tests/boundary/javascript/javascriptWorkerFinalizationCancellation.test.ts @@ -0,0 +1,99 @@ +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +import { expect, it } from "vitest"; + +it("retains completed worker facts when cancellation arrives during final job cleanup", async () => { + const loaderUrl = new URL( + "../../../scripts/lib/verifier-source-loader.mjs", + import.meta.url, + ).href; + const workerUrl = new URL( + "../../../src/javascript/analysis/JavaScriptAnalysisWorker.ts", + import.meta.url, + ).href; + const processUrl = new URL( + "../../../src/process/ProviderProcess.ts", + import.meta.url, + ).href; + // Isolate the filesystem seam in a Node process. Both analyses use the real + // analyzer and remove their actual files before the cancellation is delivered. + const script = ` + import fs from 'node:fs'; + import { createHash } from 'node:crypto'; + import { syncBuiltinESMExports } from 'node:module'; + const { importVerifierSource } = await import(${JSON.stringify(loaderUrl)}); + const controller = new AbortController(); + const reason = new Error('cancel during final worker cleanup'); + const nativeRm = fs.promises.rm; + let cancelOnCleanup = false; + let completedRemovals = 0; + fs.promises.rm = async (path, options) => { + await nativeRm(path, options); + if (String(path).includes('rea-javascript-analysis-') && String(path).endsWith('.records')) { + completedRemovals += 1; + if (cancelOnCleanup) controller.abort(reason); + } + }; + syncBuiltinESMExports(); + const { JavaScriptAnalysisWorker } = await importVerifierSource(new URL(${JSON.stringify(workerUrl)})); + const { spawnOwnedProviderProcess } = await importVerifierSource(new URL(${JSON.stringify(processUrl)})); + const value = 'export const observed = 7;'; + const source = path => ({ path, sha256: createHash('sha256').update(value).digest('hex'), + container_sha256: '1'.repeat(64), bytes: Buffer.byteLength(value), inventory_artifact_id: 'fixture-source', + kind: 'javascript', unpacked: false, text: { included: true, value } }); + let root; + let child; + let launches = 0; + const worker = new JavaScriptAnalysisWorker({ heapMb: 128, timeoutMs: 5000 }, async request => { + root = request.cwd; + const spawned = await spawnOwnedProviderProcess(request); + child = spawned.process; + launches += 1; + return spawned; + }); + let observed; + try { + const control = await worker.analyze(source('control.mjs'), { nodes: 100, unknowns: 100 }); + const controlReusable = worker.canReuse(); + cancelOnCleanup = true; + const cancelled = await worker.analyze(source('cancelled.mjs'), { nodes: 100, unknowns: 100 }, controller.signal); + const facts = cancelled.ok ? cancelled.value : cancelled.error; + observed = { controlOk: control.ok, controlReusable, completedRemovals, launches, + cancelled: !cancelled.ok && cancelled.error.error._tag === 'AnalysisCancelledError', + exactReason: !cancelled.ok && cancelled.error.error.cause === reason, + parseStatus: facts.javascript?.parse_status, hasModule: facts.module !== null, + hasProjection: facts.projection !== null, reusable: worker.canReuse() }; + } finally { + fs.promises.rm = nativeRm; + syncBuiltinESMExports(); + await worker.close(); + } + const rootRemoved = await fs.promises.stat(root).then(() => false, cause => { + if (cause.code !== 'ENOENT') throw cause; + return true; + }); + process.stdout.write(JSON.stringify({ ...observed, rootRemoved, + childExited: child.exitCode !== null || child.signalCode !== null })); + `; + const { stdout } = await promisify(execFile)( + process.execPath, + ["--input-type=module", "--eval", script], + { timeout: 20_000 }, + ); + const observed: unknown = JSON.parse(stdout); + expect(observed).toEqual({ + controlOk: true, + controlReusable: true, + completedRemovals: 2, + launches: 1, + cancelled: true, + exactReason: true, + parseStatus: "complete", + hasModule: true, + hasProjection: true, + reusable: false, + rootRemoved: true, + childExited: true, + }); +}); diff --git a/tests/boundary/javascript/workerFailureComposition.test.ts b/tests/boundary/javascript/workerFailureComposition.test.ts index e7864567a3..accc7b2c6d 100644 --- a/tests/boundary/javascript/workerFailureComposition.test.ts +++ b/tests/boundary/javascript/workerFailureComposition.test.ts @@ -5,6 +5,7 @@ const faults = vi.hoisted(() => ({ lowHeadroom: false, sourceRemoval: false, removals: 0, + onSourceRemoval: undefined as (() => void) | undefined, })); vi.mock("node:v8", async (original) => { @@ -35,6 +36,7 @@ vi.mock("node:fs/promises", async (original) => { String(args[0]).endsWith(".source") ) { faults.removals += 1; + faults.onSourceRemoval?.(); throw Object.assign( new Error("controlled source-file removal failure"), { code: "EACCES" }, @@ -71,6 +73,7 @@ afterEach(() => { faults.lowHeadroom = false; faults.sourceRemoval = false; faults.removals = 0; + faults.onSourceRemoval = undefined; }); it.each(["io", "output"] as const)( @@ -118,6 +121,9 @@ it.each([false, true])( async (removalFailure) => { faults.lowHeadroom = true; faults.sourceRemoval = removalFailure; + const controller = new AbortController(); + const reason = new Error("cancel during denied source-file cleanup"); + faults.onSourceRemoval = () => controller.abort(reason); const sourceMapUrls = [ { declared_url: "app.map", @@ -144,8 +150,13 @@ it.each([false, true])( ], }), ); + let cleanupFailure: ProviderCleanupError | undefined; try { - const result = await worker.analyze(file, { nodes: 100, unknowns: 100 }); + const result = await worker.analyze( + file, + { nodes: 100, unknowns: 100 }, + controller.signal, + ); expect(result.ok).toBe(!removalFailure); const retained = result.ok ? result.value : result.error; expect(retained.javascript?.source_map_urls).toEqual(sourceMapUrls); @@ -156,11 +167,21 @@ it.each([false, true])( if (!result.ok) { expect(result.error.error).toBeInstanceOf(ProviderCleanupError); expect(result.error.error.cleanupIncomplete).toBe(true); + expect(worker.canReuse()).toBe(false); + if (result.error.error instanceof ProviderCleanupError) + cleanupFailure = result.error.error; } expect(faults.removals).toBe(removalFailure ? 1 : 0); + expect(controller.signal.aborted).toBe(removalFailure); + expect(controller.signal.reason).toBe( + removalFailure ? reason : undefined, + ); } finally { faults.sourceRemoval = false; await worker.close(); } + expect(cleanupFailure?.cleanupIncomplete).toBe( + removalFailure ? false : undefined, + ); }, ); diff --git a/tests/process-global/cliSelectedEnvironment.test.ts b/tests/process-global/cliSelectedEnvironment.test.ts index e9f450e45a..879ef2b4ed 100644 --- a/tests/process-global/cliSelectedEnvironment.test.ts +++ b/tests/process-global/cliSelectedEnvironment.test.ts @@ -96,3 +96,52 @@ describe("CLI selected environment", () => { expect(serialized).not.toContain("value-must-not-be-captured"); }); }); + +// Real owned probes exercise CLI environment admission; these scripts do not +// establish native Wireshark behavior. +it.skipIf(process.platform === "win32").each([ + { name: "missing", selected: {}, expected: "absent" }, + { + name: "unset", + selected: { REA_PCAP_ENV_PROFILE: undefined }, + expected: "absent", + }, + { name: "empty", selected: { REA_PCAP_ENV_PROFILE: "" }, expected: "" }, + { + name: "selected", + selected: { REA_PCAP_ENV_PROFILE: "selected-profile" }, + expected: "selected-profile", + }, +])( + "preserves $name PCAP settings after the ambient environment changes", + async ({ selected, expected }) => { + const root = await createTestTempDirectory("rea-cli-pcap-environment-"); + const tshark = join(root, "tshark"), + capinfos = join(root, "capinfos"); + for (const [path, label] of [ + [tshark, "TShark"], + [capinfos, "Capinfos"], + ] as const) + await writeFile( + path, + `#!/bin/sh\nprintf '${label} (Wireshark) 4.6.6-%s\\n' "` + + "${REA_PCAP_ENV_PROFILE-absent}" + + '"\n', + { mode: 0o755 }, + ); + const cli = createCli({ + REA_TSHARK_COMMAND: tshark, + REA_CAPINFOS_COMMAND: capinfos, + ...selected, + }); + vi.stubEnv("REA_PCAP_ENV_PROFILE", "late-ambient-profile"); + const evidence = parseEvidence(await serve(cli, ["inspect-pcap-client"])); + expect(evidence.normalized_result).toMatchObject({ + client: { + tshark_version: `TShark (Wireshark) 4.6.6-${expected}`, + capinfos_version: `Capinfos (Wireshark) 4.6.6-${expected}`, + }, + }); + expect(evidence.environment).toBeNull(); + }, +);