diff --git a/src/application/BinarySession.ts b/src/application/BinarySession.ts index fd9f3972c7..480b72adb5 100644 --- a/src/application/BinarySession.ts +++ b/src/application/BinarySession.ts @@ -422,11 +422,15 @@ export class BinarySession for (const listener of this.#availabilityListeners) { try { const notification = listener(); - if (notification !== undefined) + if (notification !== undefined) { + // best-effort cleanup: async observer notifications must not reject + // unhandled; state transitions and other listeners continue. void notification.catch(() => undefined); - } catch { + } + } catch (cause: unknown) { // External observers are best-effort notifications. Contain only the // callback failure so state transitions and other listeners continue. + void cause; } } } diff --git a/src/artifacts/DirectoryArtifactReader.ts b/src/artifacts/DirectoryArtifactReader.ts index 7ffcc8c04c..db4a37fad7 100644 --- a/src/artifacts/DirectoryArtifactReader.ts +++ b/src/artifacts/DirectoryArtifactReader.ts @@ -31,6 +31,7 @@ export class DirectoryArtifactReader implements ArtifactReader { try { for await (const child of handle) children.push(child.name); } finally { + // best-effort cleanup: directory-handle close must not mask traversal. await handle.close().catch(() => undefined); } await assertContainedDirectory(root, directory); @@ -101,6 +102,7 @@ export class DirectoryArtifactReader implements ArtifactReader { } return handle.createReadStream({ autoClose: true }); } catch (cause: unknown) { + // best-effort cleanup: file-handle close must not mask the open failure. await handle.close().catch(() => undefined); throw cause; } diff --git a/src/artifacts/NibArchive.ts b/src/artifacts/NibArchive.ts index a4f08b92f5..23d991bed1 100644 --- a/src/artifacts/NibArchive.ts +++ b/src/artifacts/NibArchive.ts @@ -372,8 +372,10 @@ const decodeUtf8 = ( return new TextDecoder("utf-8", { fatal: true }).decode( bytes.subarray(start, end), ); - } catch { - throw new TypeError(`NIBArchive ${label} is not valid UTF-8`); + } catch (cause: unknown) { + throw new TypeError(`NIBArchive ${label} is not valid UTF-8`, { + cause, + }); } }; diff --git a/src/artifacts/SafeOutputTree.ts b/src/artifacts/SafeOutputTree.ts index f45eb20a41..ee07136310 100644 --- a/src/artifacts/SafeOutputTree.ts +++ b/src/artifacts/SafeOutputTree.ts @@ -153,6 +153,7 @@ export class SafeOutputTree { ); return { relativePath: path, sha256, bytesWritten: bytes }; } catch (cause: unknown) { + // best-effort cleanup: file-handle close must not mask the write failure. await handle.close().catch(() => undefined); throw cause; } diff --git a/src/artifacts/ZipArtifactReader.ts b/src/artifacts/ZipArtifactReader.ts index 4f80c087bf..f71a3b35bc 100644 --- a/src/artifacts/ZipArtifactReader.ts +++ b/src/artifacts/ZipArtifactReader.ts @@ -95,6 +95,7 @@ export class ZipArtifactReader implements ArtifactReader { } async close(): Promise { + // best-effort cleanup: zip-reader close must not mask prior extraction state. await this.#reader.close().catch(() => undefined); await this.#source.closeHandle(); } diff --git a/src/browser/CdpConnection.ts b/src/browser/CdpConnection.ts index cb60338466..3ed5275135 100644 --- a/src/browser/CdpConnection.ts +++ b/src/browser/CdpConnection.ts @@ -6,6 +6,7 @@ import { BrowserObservationError, type BrowserObservationOperation, } from "../domain/errors.js"; +import { safeParseJson } from "../domain/safeJson.js"; export interface CdpEvent { readonly method: string; @@ -140,13 +141,12 @@ export class CdpConnection { } #receive(data: RawData): void { - let message: unknown; - try { - message = JSON.parse(rawText(data)); - } catch { + const parsed = safeParseJson(rawText(data)); + if (!parsed.ok) { this.#failPending("protocol_error"); return; } + const message: unknown = parsed.value; if (!isRecord(message)) { this.#failPending("protocol_error"); return; diff --git a/src/browser/CdpEndpoint.ts b/src/browser/CdpEndpoint.ts index b5c8a55b0d..a208f9ba72 100644 --- a/src/browser/CdpEndpoint.ts +++ b/src/browser/CdpEndpoint.ts @@ -9,6 +9,7 @@ import { type BrowserObservationOperation, } from "../domain/errors.js"; import { sanitizeBrowserUrl } from "../domain/browserObservation.js"; +import { safeParseJson } from "../domain/safeJson.js"; const endpointVersionSchema = z.object({ Browser: z.string().min(1), @@ -304,17 +305,18 @@ export const readCdpJson = async ( const chunks: Buffer[] = []; response.on("data", (chunk: Buffer) => chunks.push(chunk)); response.on("end", () => { - try { - resolve(JSON.parse(Buffer.concat(chunks).toString("utf8"))); - } catch (cause: unknown) { + const parsed = safeParseJson(Buffer.concat(chunks).toString("utf8")); + if (!parsed.ok) { reject( new BrowserObservationError( operation, "invalid_endpoint_response", - { cause }, + { cause: parsed.cause }, ), ); + return; } + resolve(parsed.value); }); response.on("error", (cause: unknown) => reject(endpointFailure(cause, operation, signal)), diff --git a/src/browser/CdpTargetSession.ts b/src/browser/CdpTargetSession.ts index 660c13012c..71542f41cb 100644 --- a/src/browser/CdpTargetSession.ts +++ b/src/browser/CdpTargetSession.ts @@ -60,8 +60,9 @@ export const closeCdpTargetSession = async ( } try { await connection.send(`${domain}.disable`, {}, sessionId, signal); - } catch { - // Cleanup continues to the detach or direct-socket close boundary. + } catch (cause: unknown) { + // best-effort cleanup: domain disable continues to detach/close boundary. + void cause; } } if (signal?.aborted === true) { @@ -76,8 +77,9 @@ export const closeCdpTargetSession = async ( undefined, signal, ); - } catch { - // Closing REA's socket is the final non-destructive cleanup boundary. + } catch (cause: unknown) { + // best-effort cleanup: closing REA's socket is the final cleanup boundary. + void cause; } await connection.close(); }; diff --git a/src/browser/PlaywrightScenarioBrowser.ts b/src/browser/PlaywrightScenarioBrowser.ts index a2303395f2..7719e1d00f 100644 --- a/src/browser/PlaywrightScenarioBrowser.ts +++ b/src/browser/PlaywrightScenarioBrowser.ts @@ -178,6 +178,8 @@ export const closePlaywrightScenarioBrowser = async ( await rm(profilePath, { recursive: true, force: true, maxRetries: 3 }); } })(); + // best-effort cleanup: the bounded execution boundary below observes the + // same cleanup promise; this early attachment only prevents unhandled rejection. void cleanup.catch(() => undefined); // The shared execution boundary rejects cancellation/timeouts instead of // returning a successful capture whose browser or profile may still exist. diff --git a/src/browser/V8InspectorProvider.ts b/src/browser/V8InspectorProvider.ts index 298b13e2b9..d846f9622d 100644 --- a/src/browser/V8InspectorProvider.ts +++ b/src/browser/V8InspectorProvider.ts @@ -201,6 +201,8 @@ export const closeInspectorConnection = async ( signal?: AbortSignal, ): Promise => { const closing = connection.close(); + // best-effort cleanup: the race below observes the close; this only prevents + // unhandled rejection when the caller abandons the close via cancellation. void closing.catch(() => undefined); // CdpConnection.close has its own one second transport bound. Do not make a // cancelled caller wait for that fallback. diff --git a/src/browser/WebSourceMapFetcher.ts b/src/browser/WebSourceMapFetcher.ts index 5e0a97ec37..7738d9fb21 100644 --- a/src/browser/WebSourceMapFetcher.ts +++ b/src/browser/WebSourceMapFetcher.ts @@ -12,6 +12,7 @@ import type { } from "../domain/webBundleAnalysis.js"; import { webSourceMapsSchema } from "../domain/webBundleAnalysis.js"; import { createWebTextArtifact } from "../domain/webContentArtifact.js"; +import { safeParseJson } from "../domain/safeJson.js"; export interface WebSourceMapRequest { readonly scriptKey: string; @@ -272,12 +273,9 @@ const originalModuleEdges = ( }; const validSourceMapEnvelope = (text: string): boolean => { - let parsed: unknown; - try { - parsed = JSON.parse(text); - } catch { - return false; - } + const parsedResult = safeParseJson(text); + if (!parsedResult.ok) return false; + const parsed: unknown = parsedResult.value; if (!isRecord(parsed) || parsed.version !== 3) return false; if (typeof parsed.mappings === "string") return validSourceMapLeaf(parsed); if (!Array.isArray(parsed.sections)) return false; diff --git a/src/cli/coreAnalysisCommands.ts b/src/cli/coreAnalysisCommands.ts index 9e6324044b..76617f7e15 100644 --- a/src/cli/coreAnalysisCommands.ts +++ b/src/cli/coreAnalysisCommands.ts @@ -351,7 +351,10 @@ const isJavaScriptApplicationPath = async (path: string): Promise => { if (lower.endsWith(".app")) return false; try { return (await stat(path)).isDirectory(); - } catch { + } catch (cause: unknown) { + // A missing or unreadable path is not a JavaScript application directory; + // the caller falls through to direct binary analysis instead. + void cause; return false; } }; diff --git a/src/cliBrowserAdvancedCommands.ts b/src/cliBrowserAdvancedCommands.ts index 9942997929..1238701198 100644 --- a/src/cliBrowserAdvancedCommands.ts +++ b/src/cliBrowserAdvancedCommands.ts @@ -18,6 +18,7 @@ import { compareWebScreenshotsInputSchema, } from "./domain/webScreenshot.js"; import type { JsonValue } from "./domain/jsonValue.js"; +import { safeParseJson } from "./domain/safeJson.js"; import type { Logger } from "./logger.js"; import { CLI_COMMANDS } from "./cliCommandNames.js"; @@ -181,11 +182,8 @@ const registerScreenshotDiff = ( }; const parseJson = (value: string): unknown => { - try { - return JSON.parse(value); - } catch { - return undefined; - } + const parsed = safeParseJson(value); + return parsed.ok ? parsed.value : undefined; }; const inputError = (operation: string): JsonValue => diff --git a/src/cliJsonInput.ts b/src/cliJsonInput.ts index ca3f2eacf6..a28f8a5783 100644 --- a/src/cliJsonInput.ts +++ b/src/cliJsonInput.ts @@ -2,6 +2,7 @@ import { readFile } from "node:fs/promises"; import { AnalysisInputError, projectAnalysisError } from "./domain/errors.js"; import type { JsonValue } from "./domain/jsonValue.js"; +import { safeParseJson } from "./domain/safeJson.js"; /** Parse inline JSON or one local JSON file for a CLI workflow. */ export const parseCliJsonInput = async ( @@ -16,27 +17,33 @@ export const parseCliJsonInput = async ( if (["{", "["].includes(value.trimStart()[0] ?? "")) return { ok: false, error: inputError(operation) }; try { - const parsed = parseJson(await readFile(value)); - return parsed === undefined - ? jsonFileError(value, operation, "invalid-json") - : { ok: true, value: parsed }; - } catch { - return jsonFileError(value, operation, "read-failed"); + const decoded = safeParseJson(await readFile(value, "utf8")); + return decoded.ok + ? { ok: true, value: decoded.value } + : jsonFileError(value, operation, "invalid-json"); + } catch (cause: unknown) { + return jsonFileError(value, operation, "read-failed", cause); } }; const parseJson = (value: string | Uint8Array): unknown => { - try { - return JSON.parse( - typeof value === "string" - ? value - : new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }).decode( - value, - ), - ); - } catch { - return undefined; + let text: string; + if (typeof value === "string") { + text = value; + } else { + try { + text = new TextDecoder("utf-8", { + fatal: true, + ignoreBOM: true, + }).decode(value); + } catch (cause: unknown) { + // Decoding failure means the bytes are not valid UTF-8 JSON input. + void cause; + return undefined; + } } + const parsed = safeParseJson(text); + return parsed.ok ? parsed.value : undefined; }; const inputError = (operation: string): JsonValue => ({ @@ -52,6 +59,7 @@ const jsonFileError = ( path: string | undefined, operation: string, reason: "invalid-json" | "read-failed", + cause?: unknown, ) => ({ ok: false as const, error: { @@ -59,7 +67,7 @@ const jsonFileError = ( ...projectAnalysisError( new AnalysisInputError( operation, - undefined, + cause === undefined ? undefined : { cause }, reason === "invalid-json" ? [{ path: [], reason: "invalid_format", expected: "JSON" }] : [], diff --git a/src/cliOutput.ts b/src/cliOutput.ts index f5cafd138d..862de8b5b0 100644 --- a/src/cliOutput.ts +++ b/src/cliOutput.ts @@ -1,3 +1,5 @@ +import { safeParseJson } from "./domain/safeJson.js"; + const SAFE_VALIDATION_MESSAGE = "REA could not read the command arguments. Run `rea --help`, correct the arguments, then try again."; const UNSUPPORTED_OUTPUT_COMBINATION_MESSAGE = @@ -94,20 +96,18 @@ export const renderCliOutputArgumentError = ( export const sanitizeCliOutput = (output: string): string => { const trimmed = output.trimStart(); if (trimmed.startsWith("{")) { - try { - const parsed: unknown = JSON.parse(trimmed); - if (!isRecord(parsed)) return output; - const error = validationError(parsed); - if (error !== undefined) { - const safeError = { - code: "VALIDATION_ERROR", - message: SAFE_VALIDATION_MESSAGE, - }; - if (error === parsed) return `${JSON.stringify(safeError)}\n`; - return `${JSON.stringify({ ...parsed, error: safeError })}\n`; - } - } catch { - return output; + const decoded = safeParseJson(trimmed); + if (!decoded.ok) return output; + const parsed: unknown = decoded.value; + if (!isRecord(parsed)) return output; + const error = validationError(parsed); + if (error !== undefined) { + const safeError = { + code: "VALIDATION_ERROR", + message: SAFE_VALIDATION_MESSAGE, + }; + if (error === parsed) return `${JSON.stringify(safeError)}\n`; + return `${JSON.stringify({ ...parsed, error: safeError })}\n`; } return output; } diff --git a/src/domain/safeJson.test.ts b/src/domain/safeJson.test.ts new file mode 100644 index 0000000000..ff198a0bcd --- /dev/null +++ b/src/domain/safeJson.test.ts @@ -0,0 +1,12 @@ +import { expect, it } from "vitest"; + +import { safeParseJson } from "./safeJson.js"; + +it("keeps the original JSON parse failure as the cause", () => { + const parsed = safeParseJson("{"); + + expect(parsed.ok).toBe(false); + if (parsed.ok) throw new Error("expected invalid JSON to fail"); + expect(parsed.error).toContain("JSON"); + expect(parsed.cause).toBeInstanceOf(SyntaxError); +}); diff --git a/src/domain/safeJson.ts b/src/domain/safeJson.ts new file mode 100644 index 0000000000..fc141e3748 --- /dev/null +++ b/src/domain/safeJson.ts @@ -0,0 +1,17 @@ +/** JSON parsing without throws at an input boundary. */ +export type SafeJsonParseResult = + | { readonly ok: true; readonly value: unknown } + | { readonly ok: false; readonly error: string; readonly cause: unknown }; + +/** Parse JSON text, returning the failure reason instead of throwing. */ +export const safeParseJson = (value: string): SafeJsonParseResult => { + try { + return { ok: true, value: JSON.parse(value) as unknown }; + } catch (cause: unknown) { + return { + ok: false, + error: cause instanceof Error ? cause.message : String(cause), + cause, + }; + } +}; diff --git a/src/ghidra/GhidraClient.ts b/src/ghidra/GhidraClient.ts index 1168dfaa21..9dd0d6d9e0 100644 --- a/src/ghidra/GhidraClient.ts +++ b/src/ghidra/GhidraClient.ts @@ -153,10 +153,19 @@ export class GhidraClient { this.#startupController = undefined; } }; - void started.then((result) => { - signal?.removeEventListener("abort", onAbort); - if (!result.ok) reset(); - }, reset); + void started.then( + (result) => { + signal?.removeEventListener("abort", onAbort); + if (!result.ok) reset(); + }, + (cause: unknown) => { + this.#logger.debug( + { error: cause instanceof Error ? cause.message : String(cause) }, + "Ghidra startup promise rejected during bookkeeping", + ); + reset(); + }, + ); return started; } @@ -388,6 +397,7 @@ export class GhidraClient { forceStop: boolean, ): Promise { try { + // best-effort cleanup: a rejected startup must not mask close/cleanup. await starting?.catch(() => undefined); await this.#cleanup(forceStop); } finally { diff --git a/src/hopper/HopperCleanup.ts b/src/hopper/HopperCleanup.ts index 40574b71b4..0ac15ab23b 100644 --- a/src/hopper/HopperCleanup.ts +++ b/src/hopper/HopperCleanup.ts @@ -264,6 +264,7 @@ const report = async ( completed: number, message: string, ): Promise => { + // best-effort cleanup: progress reporting must not fail session cleanup. await progress ?.report({ phase: "hopper_cleanup", diff --git a/src/hopper/HopperClient.ts b/src/hopper/HopperClient.ts index f9da79f890..fdb17236c4 100644 --- a/src/hopper/HopperClient.ts +++ b/src/hopper/HopperClient.ts @@ -175,7 +175,11 @@ export class HopperClient { release(); if (!result.ok) reset(); }, - () => { + (cause: unknown) => { + this.#logger.debug( + { error: cause instanceof Error ? cause.message : String(cause) }, + "Hopper startup promise rejected during bookkeeping", + ); release(); reset(); }, @@ -323,6 +327,8 @@ export class HopperClient { options: HopperClientCloseOptions, ): Promise> { try { + // best-effort cleanup: a rejected startup must not mask close/cleanup; + // #cleanup reports the authoritative close result. await starting?.catch(() => undefined); return await this.#cleanup(options); } finally { diff --git a/src/hopper/HopperTargetLease.ts b/src/hopper/HopperTargetLease.ts index 3c34c3f844..9edff5b6fb 100644 --- a/src/hopper/HopperTargetLease.ts +++ b/src/hopper/HopperTargetLease.ts @@ -8,6 +8,7 @@ import { } from "node:net"; import { join, resolve } from "node:path"; import { HopperStartError } from "../domain/errors.js"; +import { safeParseJson } from "../domain/safeJson.js"; interface HopperTargetLeaseOwner { readonly runId: string; @@ -144,22 +145,23 @@ const readOwner = ( response += chunk.toString("utf8"); const newline = response.indexOf("\n"); if (newline < 0) return; - try { - const value: unknown = JSON.parse(response.slice(0, newline)); - if ( - typeof value === "object" && - value !== null && - typeof Reflect.get(value, "runId") === "string" && - typeof Reflect.get(value, "processId") === "number" - ) - finish({ - runId: Reflect.get(value, "runId") as string, - processId: Reflect.get(value, "processId") as number, - }); - else finish(undefined); - } catch { + const parsed = safeParseJson(response.slice(0, newline)); + if (!parsed.ok) { finish(undefined); + return; } + const value: unknown = parsed.value; + if ( + typeof value === "object" && + value !== null && + typeof Reflect.get(value, "runId") === "string" && + typeof Reflect.get(value, "processId") === "number" + ) + finish({ + runId: Reflect.get(value, "runId") as string, + processId: Reflect.get(value, "processId") as number, + }); + else finish(undefined); }); socket.on("error", () => finish(undefined)); socket.on("end", () => finish(undefined)); diff --git a/src/hopper/LinuxPrivateDisplayDiagnostic.ts b/src/hopper/LinuxPrivateDisplayDiagnostic.ts index 8de08f4627..6fe4d2437b 100644 --- a/src/hopper/LinuxPrivateDisplayDiagnostic.ts +++ b/src/hopper/LinuxPrivateDisplayDiagnostic.ts @@ -4,6 +4,7 @@ import { isHopperStartupFailureCode, type HopperStartupDiagnostic, } from "../domain/hopperStartupFailure.js"; +import { safeParseJson } from "../domain/safeJson.js"; export const LINUX_PRIVATE_DISPLAY_DIAGNOSTIC_PREFIX = "REA_X11_DIAGNOSTIC="; @@ -70,13 +71,11 @@ export const parseLinuxPrivateDisplayDiagnostic = ( if (records.length !== 1) return { ok: false, reason: "diagnostic_multiple" }; const record = records[0]; if (record === undefined) return { ok: false, reason: "diagnostic_missing" }; - try { - const parsed = diagnosticSchema.safeParse( - JSON.parse(record.slice(LINUX_PRIVATE_DISPLAY_DIAGNOSTIC_PREFIX.length)), - ); - if (!parsed.success) return { ok: false, reason: "diagnostic_malformed" }; - return { ok: true, value: parsed.data }; - } catch { - return { ok: false, reason: "diagnostic_malformed" }; - } + const decoded = safeParseJson( + record.slice(LINUX_PRIVATE_DISPLAY_DIAGNOSTIC_PREFIX.length), + ); + if (!decoded.ok) return { ok: false, reason: "diagnostic_malformed" }; + const parsed = diagnosticSchema.safeParse(decoded.value); + if (!parsed.success) return { ok: false, reason: "diagnostic_malformed" }; + return { ok: true, value: parsed.data }; }; diff --git a/src/hopper/protocol.ts b/src/hopper/protocol.ts index 0b6dc3d243..9c74111e9d 100644 --- a/src/hopper/protocol.ts +++ b/src/hopper/protocol.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import { HopperProtocolError, HopperRemoteError } from "../domain/errors.js"; import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; import { err, ok, type Result } from "../domain/result.js"; +import { safeParseJson } from "../domain/safeJson.js"; const remoteErrorSchema = z .object({ @@ -82,14 +83,14 @@ export type HopperBridgeMessage = z.infer; export const parseBridgeMessageLine = ( line: string, ): Result => { - let decoded: unknown; - try { - decoded = JSON.parse(line); - } catch (cause: unknown) { + const decodedResult = safeParseJson(line); + if (!decodedResult.ok) return err( - new HopperProtocolError("Hopper returned malformed JSON", { cause }), + new HopperProtocolError("Hopper returned malformed JSON", { + cause: decodedResult.cause, + }), ); - } + const decoded: unknown = decodedResult.value; const parsed = messageSchema.safeParse(decoded); return parsed.success diff --git a/src/main.ts b/src/main.ts index 20307cb09f..0188a0aa8a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -120,7 +120,10 @@ export const runEntrypoint = async ( ): Promise => { try { setExitCode(await start()); - } catch { + } catch (cause: unknown) { + // Intentionally redact the startup cause from caller-visible output; + // unexpected failures must not leak internal details. + void cause; writeStderr(`${SERVER_START_FAILED}\n`); setExitCode(1); } diff --git a/src/main/transport.ts b/src/main/transport.ts index 3bc5d1e5b1..5235419d10 100644 --- a/src/main/transport.ts +++ b/src/main/transport.ts @@ -43,8 +43,11 @@ export const startMcpTransport = async ( let optionalProviders: OptionalProviders = {}; try { optionalProviders = await serverContext.loadOptionalProviders(); - } catch { + } catch (cause: unknown) { serverLogger.warn( + { + error: cause instanceof Error ? cause.message : String(cause), + }, "Optional MCP providers could not load; affected tools remain unavailable", ); } @@ -63,9 +66,14 @@ export const startMcpTransport = async ( }, }, ); - } catch { + } catch (cause: unknown) { await session.close(); - serverLogger.error(MCP_CONNECTION_START_FAILED); + serverLogger.error( + { + error: cause instanceof Error ? cause.message : String(cause), + }, + MCP_CONNECTION_START_FAILED, + ); dependencies.writeStderr(`${MCP_CONNECTION_START_FAILED}\n`); return { ok: false }; } diff --git a/src/native/NativeMacOSProvider.ts b/src/native/NativeMacOSProvider.ts index cb06e689f5..e92500f8ec 100644 --- a/src/native/NativeMacOSProvider.ts +++ b/src/native/NativeMacOSProvider.ts @@ -415,6 +415,7 @@ class NativeMacOSClient implements AnalysisClient { ); if (!capture.ok) return capture; const parsed = parsePlistJson(capture.value.stdout); + if (!parsed.ok) return parsed; const provenance = [classified.value, capture.value].map((item) => invocation(item, plist.value, "$PLIST"), ); @@ -424,7 +425,7 @@ class NativeMacOSClient implements AnalysisClient { : /XML|text/iu.test(classified.value.stdout) ? "xml" : "unknown", - ...parsed, + ...parsed.value, source_path: plist.value, provenance, limitations: [], diff --git a/src/native/NativeUiHelperRuntime.ts b/src/native/NativeUiHelperRuntime.ts index 6d019c03db..475704538b 100644 --- a/src/native/NativeUiHelperRuntime.ts +++ b/src/native/NativeUiHelperRuntime.ts @@ -3,6 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { execFileOutput } from "../process/ExecFileOutput.js"; +import { safeParseJson } from "../domain/safeJson.js"; import type { NativeUiHelper } from "./NativeUiObservation.js"; /** Lazily compile one owned helper per observation/scenario and remove its compiler cache. */ @@ -42,7 +43,12 @@ export const createNativeUiHelperRuntime = () => { ...(signal === undefined ? {} : { signal }), }, ); - return JSON.parse(output.stdout) as unknown; + const parsed = safeParseJson(output.stdout); + if (!parsed.ok) + throw new Error(`Native helper returned invalid JSON: ${parsed.error}`, { + cause: parsed.cause, + }); + return parsed.value; }; return { invoke, diff --git a/src/native/parsers/plist.ts b/src/native/parsers/plist.ts index 6ec7cc0e34..89244c1dd5 100644 --- a/src/native/parsers/plist.ts +++ b/src/native/parsers/plist.ts @@ -1,18 +1,43 @@ import { z } from "zod"; +import { AnalysisOutputError } from "../../domain/errors.js"; +import { err, ok, type Result } from "../../domain/result.js"; +import { safeParseJson } from "../../domain/safeJson.js"; + const plistObject = z.record(z.string(), z.unknown()); /** Parse plutil JSON output and project stable bundle metadata. */ -export const parsePlistJson = (output: string) => { +export const parsePlistJson = ( + output: string, +): Result< + { + readonly value: unknown; + readonly bundle: { + readonly identifier: string | null; + readonly executable: string | null; + readonly name: string | null; + readonly version: string | null; + readonly short_version: string | null; + }; + }, + AnalysisOutputError +> => { // plutil output can carry a UTF-8 BOM, which JSON.parse rejects outright. - const value: unknown = JSON.parse(output.replace(/^\uFEFF/u, "")); + const parsed = safeParseJson(output.replace(/^\uFEFF/u, "")); + if (!parsed.ok) + return err( + new AnalysisOutputError("inspect_plist", parsed.error, { + cause: parsed.cause, + }), + ); + const value: unknown = parsed.value; const object = plistObject.safeParse(value); const field = (name: string): string | null => { if (!object.success) return null; const candidate = object.data[name]; return typeof candidate === "string" ? candidate : null; }; - return { + return ok({ value, bundle: { identifier: field("CFBundleIdentifier"), @@ -21,5 +46,5 @@ export const parsePlistJson = (output: string) => { version: field("CFBundleVersion"), short_version: field("CFBundleShortVersionString"), }, - }; + }); }; diff --git a/src/reference/ReferenceSourceReaderEntries.ts b/src/reference/ReferenceSourceReaderEntries.ts index 0eed1206bd..a03e096a2e 100644 --- a/src/reference/ReferenceSourceReaderEntries.ts +++ b/src/reference/ReferenceSourceReaderEntries.ts @@ -289,7 +289,11 @@ const applyExclusion = ( ): { readonly ok: true; readonly value: boolean } | { readonly ok: false } => { try { return { ok: true, value: shouldExclude?.(path) === true }; - } catch { + } catch (cause: unknown) { + // Exclusion predicates are caller-supplied; a throwing predicate fails + // closed and the caller-visible `{ ok: false }` preserves the rejection + // without propagating an arbitrary predicate cause. + void cause; return { ok: false }; } }; diff --git a/src/reference/ReferenceSourceReaderFile.ts b/src/reference/ReferenceSourceReaderFile.ts index 096c7fe6a7..2e0845af8b 100644 --- a/src/reference/ReferenceSourceReaderFile.ts +++ b/src/reference/ReferenceSourceReaderFile.ts @@ -220,6 +220,7 @@ export const readStableFile = async ( if (message === undefined) throw cause; return entryFailure(path, "file", "io", message, safeSize(expected.size)); } finally { + // best-effort cleanup: file-handle close must not mask the read result. await handle?.close().catch(() => undefined); } }; diff --git a/tests/boundary/providers/native/nativeTextRobustness.test.ts b/tests/boundary/providers/native/nativeTextRobustness.test.ts index 68dd78e3f5..daea94ff99 100644 --- a/tests/boundary/providers/native/nativeTextRobustness.test.ts +++ b/tests/boundary/providers/native/nativeTextRobustness.test.ts @@ -32,5 +32,7 @@ it("accepts a byte-order mark before plist JSON", () => { const parsed = parsePlistJson( '\uFEFF{"CFBundleIdentifier":"com.owned.fixture"}', ); - expect(parsed.bundle.identifier).toBe("com.owned.fixture"); + expect(parsed.ok).toBe(true); + if (!parsed.ok) throw new Error("expected plist JSON to parse"); + expect(parsed.value.bundle.identifier).toBe("com.owned.fixture"); });