From 73852202d42743fe5589fd9477f0d0991d87c060 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 14 Jul 2026 06:28:10 +0800 Subject: [PATCH] fix: preserve actionable artifact diagnostics (#119) --- AGENTS.md | 2 + src/cliInvestigationCommands.ts | 14 ++-- src/contracts/toolOutputSchemas.ts | 9 +++ src/domain/errors.ts | 15 +++- tests/artifactMcp.test.ts | 112 +++++++++++++++++------------ tests/cliOutput.test.ts | 30 +++++++- tests/errors.test.ts | 9 ++- 7 files changed, 136 insertions(+), 55 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 039fbdf22..233f67985 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,8 @@ Prioritize: Installers must not install or upgrade Homebrew, Node.js, npm, or other unrelated software. `rea setup` must print its planned changes and require approval before writing files or installing Hopper. +REA is a local-only tool; do not sanitize actionable local diagnostics such as artifact paths, digests, mismatch locations, or analysis metadata, while continuing to redact genuine secrets such as credentials and authorization headers. + ## Project Structure & Module Organization REA is a layered ESM TypeScript application. Dependencies flow inward: `domain` ← `contracts` ← `hopper` ← `application` ← `server` ← adapters. diff --git a/src/cliInvestigationCommands.ts b/src/cliInvestigationCommands.ts index 7fea6e73f..4c932cf1e 100644 --- a/src/cliInvestigationCommands.ts +++ b/src/cliInvestigationCommands.ts @@ -3,6 +3,7 @@ import { Cli, z } from "incur"; import { runCrossVersionInvestigation } from "./application/CrossVersionInvestigation.js"; import { logCliCommand } from "./cliLogging.js"; import { parseConfig } from "./config.js"; +import { projectAnalysisError, type AnalysisError } from "./domain/errors.js"; import type { Logger } from "./logger.js"; const investigationOptionsSchema = z.object({ @@ -74,8 +75,7 @@ const runInvestigation = async ( message: "Persistent workspace writes require explicit --yes approval.", }; const config = parseConfig(process.env); - if (!config.ok) - return { error: config.error._tag, message: config.error.message }; + if (!config.ok) return cliError(config.error); const investigated = await runCrossVersionInvestigation( { approved: true, @@ -102,8 +102,10 @@ const runInvestigation = async ( ); return investigated.ok ? investigated.value.evidence - : { - error: investigated.error._tag, - message: investigated.error.message, - }; + : cliError(investigated.error); }; + +const cliError = (error: AnalysisError) => ({ + error: "Analysis failed", + ...projectAnalysisError(error), +}); diff --git a/src/contracts/toolOutputSchemas.ts b/src/contracts/toolOutputSchemas.ts index d6321d901..6772a9925 100644 --- a/src/contracts/toolOutputSchemas.ts +++ b/src/contracts/toolOutputSchemas.ts @@ -191,6 +191,15 @@ const analysisErrorProjectionSchema = z "execution_failure", ]), message: z.string(), + details: z + .object({ + logical_path: z.string(), + declared_sha256: z.string().nullable(), + calculated_sha256: z.string().nullable(), + unpacked: z.boolean(), + }) + .strict() + .optional(), code: z .enum([ "private_display_unavailable", diff --git a/src/domain/errors.ts b/src/domain/errors.ts index b2d7048ef..3afb2a9fd 100644 --- a/src/domain/errors.ts +++ b/src/domain/errors.ts @@ -3,8 +3,7 @@ import { hopperStartupFailure, type HopperStartupFailureCode, } from "./hopperStartupFailure.js"; -export { hopperStartupFailure } from "./hopperStartupFailure.js"; -export type { HopperStartupFailureCode } from "./hopperStartupFailure.js"; +export { hopperStartupFailure, type HopperStartupFailureCode }; /** Stable tags exposed by safe analysis-error projections. */ const ANALYSIS_ERROR_TAGS = [ @@ -349,6 +348,7 @@ export interface AnalysisErrorProjection | "execution_failure"; readonly message: string; readonly code?: HopperStartupFailureCode; + readonly details?: Readonly>; } /** Project expected failures into exhaustive, secret-safe caller fields. */ @@ -358,10 +358,21 @@ export const projectAnalysisError = ( assertKnownTag(error._tag); const code = error instanceof HopperProcessError ? error.failureCode : undefined; + const artifact = + error instanceof ArtifactOperationError && error.artifactDetails; + const details = artifact + ? { + logical_path: artifact.logicalPath, + declared_sha256: artifact.declaredSha256, + calculated_sha256: artifact.calculatedSha256, + unpacked: artifact.unpacked, + } + : undefined; return { category: errorCategory(error), message: userMessage(error), ...(code === undefined ? {} : { code }), + ...(details === undefined ? {} : { details }), }; }; diff --git a/tests/artifactMcp.test.ts b/tests/artifactMcp.test.ts index 7e5c94af2..07a1c9455 100644 --- a/tests/artifactMcp.test.ts +++ b/tests/artifactMcp.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -17,51 +17,75 @@ import { createServer } from "../src/server/createServer.js"; import { observed } from "./fixtures/analysisExecution.js"; describe("artifact graph MCP integration", () => { - it("returns a safe ASAR integrity error without paths or hashes", async () => { - const root = await mkdtemp(join(tmpdir(), "rea-asar-integrity-mcp-")); - const source = join(root, "source"); - const original = "console.log('ok');\n"; - const changed = "changed();\n"; - await mkdir(source); - await writeFile(join(source, "main.js"), original); - const archive = join(root, "fixture.asar"); - await createPackageWithOptions(source, archive, { unpack: "*.js" }); - await writeFile(join(`${archive}.unpacked`, "main.js"), changed); + it.each([ + { unpacked: false, label: "embedded" }, + { unpacked: true, label: "unpacked" }, + ])( + "returns actionable local details for an $label ASAR integrity error", + async ({ unpacked }) => { + const root = await mkdtemp(join(tmpdir(), "rea-asar-integrity-mcp-")); + const source = join(root, "source"); + const original = "console.log('ok');\n"; + const changed = "console.log('no');\n"; + await mkdir(source); + await writeFile(join(source, "main.js"), original); + const archive = join(root, "fixture.asar"); + await createPackageWithOptions( + source, + archive, + unpacked ? { unpack: "*.js" } : {}, + ); + if (unpacked) { + await writeFile(join(`${archive}.unpacked`, "main.js"), changed); + } else { + const bytes = await readFile(archive); + const contentOffset = bytes.indexOf(original); + expect(contentOffset).toBeGreaterThanOrEqual(0); + bytes.write(changed, contentOffset, "utf8"); + await writeFile(archive, bytes); + } - const session = new BinarySession(new ArtifactProvider()); - const server = createServer(session, session); - const client = new Client({ name: "asar-integrity-test", version: "1" }); - const [clientTransport, serverTransport] = - InMemoryTransport.createLinkedPair(); - try { - await server.connect(serverTransport); - await client.connect(clientTransport); - const opened = await client.callTool({ - name: "open_binary", - arguments: { path: archive }, - }); - expect(opened.isError).not.toBe(true); + const session = new BinarySession(new ArtifactProvider()); + const server = createServer(session, session); + const client = new Client({ name: "asar-integrity-test", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + const opened = await client.callTool({ + name: "open_binary", + arguments: { path: archive }, + }); + expect(opened.isError).not.toBe(true); - const result = await client.callTool({ - name: "inventory_artifact", - arguments: {}, - }); - expect(result.isError).toBe(true); - expect(result.structuredContent).toEqual({ - error: { - category: "integrity_mismatch", - message: - "Artifact is invalid or has changed. Get a fresh copy and try again.", - }, - }); - } finally { - await Promise.allSettled([ - client.close(), - server.close(), - session.close(), - ]); - } - }); + const result = await client.callTool({ + name: "inventory_artifact", + arguments: {}, + }); + expect(result.isError).toBe(true); + expect(result.structuredContent).toEqual({ + error: { + category: "integrity_mismatch", + message: + "Artifact is invalid or has changed. Get a fresh copy and try again.", + details: { + logical_path: "main.js", + declared_sha256: expect.stringMatching(/^[a-f0-9]{64}$/u), + calculated_sha256: expect.stringMatching(/^[a-f0-9]{64}$/u), + unpacked, + }, + }, + }); + } finally { + await Promise.allSettled([ + client.close(), + server.close(), + session.close(), + ]); + } + }, + ); it("rejects altered payloads that reuse session Evidence IDs", async () => { const session = new BinarySession(() => ({ diff --git a/tests/cliOutput.test.ts b/tests/cliOutput.test.ts index 5ff34d1b0..a6266c124 100644 --- a/tests/cliOutput.test.ts +++ b/tests/cliOutput.test.ts @@ -1,6 +1,10 @@ import { execFile } from "node:child_process"; +import { mkdir, mkdtemp, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { promisify } from "node:util"; +import { createPackageWithOptions } from "@electron/asar"; import { describe, expect, it } from "vitest"; import { sanitizeCliOutput } from "../src/cliOutput.js"; @@ -86,5 +90,29 @@ describe("CLI output boundary", () => { stdout: 'ok: false\nerror:\n code: VALIDATION_ERROR\n message: "REA could not read the command arguments. Run `rea --help`, correct the arguments, then try again."\n', }); - }); + }, 20_000); + + it("preserves artifact diagnostics in ordinary and full JSON output", async () => { + const root = await mkdtemp(join(tmpdir(), "rea-cli-diagnostics-")); + const source = join(root, "source"); + await mkdir(source); + await writeFile(join(source, "main.js"), "console.log('ok');\n"); + const archive = join(root, "fixture.asar"); + await createPackageWithOptions(source, archive, { unpack: "*.js" }); + await writeFile(join(`${archive}.unpacked`, "main.js"), "changed();\n"); + + for (const flags of [["--json"], ["--full-output", "--json"]]) { + const { stdout } = await execFileAsync(process.execPath, [ + "scripts/rea.mjs", + ...flags, + "inventory-artifact", + archive, + ]); + const output = JSON.stringify(JSON.parse(stdout) as unknown); + expect(output).toContain('"logical_path":"main.js"'); + expect(output).toMatch(/"declared_sha256":"[a-f0-9]{64}"/u); + expect(output).toMatch(/"calculated_sha256":"[a-f0-9]{64}"/u); + expect(output).toContain('"unpacked":true'); + } + }, 20_000); }); diff --git a/tests/errors.test.ts b/tests/errors.test.ts index 121acddcf..4ae3c5df4 100644 --- a/tests/errors.test.ts +++ b/tests/errors.test.ts @@ -58,7 +58,7 @@ describe("analysis error projection", () => { } }); - it("projects every typed failure without internal details", () => { + it("projects every typed failure without secrets", () => { const secretCause = new Error("secret-token"); const byTag = { AnalysisProtocolError: new AnalysisProtocolError("protocol failed"), @@ -132,7 +132,6 @@ describe("analysis error projection", () => { "fixture", "overview", "AnalysisCapabilityUnavailableError", - "main.js", "declaredSha256", ]) expect(serialized).not.toContain(hidden); @@ -152,6 +151,12 @@ describe("analysis error projection", () => { ), ).toMatchObject({ category: "integrity_mismatch", + details: { + logical_path: "main.js", + declared_sha256: "a".repeat(64), + calculated_sha256: "b".repeat(64), + unpacked: true, + }, }); expect(projected.every(({ message }) => message.length > 0)).toBe(true); expect(