fix(wasm): refuse unretained summary delivery before provider work (#5274)

* fix(wasm): refuse unretained summary delivery before provider work

inspect_wasm_artifact with detail summary ran the WASM provider before
checking whether the server retains Evidence. On a fixed server without
session retention a successful provider run was followed by
capability_unavailable, and a failing provider run replaced that
diagnostic with the provider error, hiding the guidance to select
detail complete.

Check retention right after input validation, as the EVM, pcap and
network-capture handlers do, so the refusal precedes any provider work
and keeps its typed diagnostic. Complete delivery is unchanged. The
fixed-server boundary test now injects a vi.fn provider and asserts it
stays idle for refused summaries while complete delivery still runs it.

Fixes #5100

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(wasm): validate caller paths before the summary retention preflight

Review follow-up for #5274. The retention preflight ran ahead of the
semantic input checks, so a relative path, glue path or candidate path
with detail summary on a fixed server returned capability_unavailable
instead of its field-specific invalid_request.

WasmArtifactService now exposes that parse-and-absolute-path step as
validate(); inspect() uses the same method, so the checks stay at the
owning boundary, and the MCP handler validates before deciding on
retention. The composition descriptor and server typing expose validate.

The fixed-server boundary test sends relative path and glue_paths with
detail summary and asserts invalid_request with both issue paths and no
provider call; it failed on 418a66d7d with capability_unavailable.

docs/mcp-contracts.md now claims the before-analysis refusal only for
inspect_artifact, inspect_wasm_artifact, inspect_macho and
decode_interface_builder, which enforce it; inspect_binary_layout and
analyze_javascript_application still analyze first and are left to a
separate change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* refactor(wasm): inspect validated artifact input without reparsing it

Follow-up for #5274, matching the review outcome on #5288. The MCP
handler validated the request and then called inspect(), which ran the
same validator again, so every call parsed and cloned its input twice.

WasmArtifactService now separates the raw entry point from execution:
inspect() validates raw input once and delegates to the new
inspectValidated(), which runs the inspection for input that validate()
already admitted. The handler validates, runs the retention preflight,
then calls inspectValidated(); the composition descriptor and server
typing expose it. The CLI keeps the raw inspect() entry point; behavior
and diagnostics are unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(wasm): keep cancellation ahead of raw artifact input validation

Follow-up for #5274, matching the review outcome on #5288. Separating
validate() from inspectValidated() moved the raw inspect() entry point's
validation ahead of its abort check, so a direct call with an
already-aborted signal and malformed input returned AnalysisInputError
instead of the AnalysisCancelledError the pre-change service gave.

The raw entry point now checks the signal before validating, as before,
while inspectValidated() keeps its own guard for the MCP handler path.
A unit test pins the precedence with no provider call.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
authored and GitHub committed 2026-10-23 02:24:45 +08:00
1 parent 9c81191b1d
commit 8ecbb74b00
4 files changed
+91 -15

No files matched your search

+3 -1
View File
@@ -314,7 +314,9 @@ that complete Evidence in the session and returns the derived
`normalized_result.parent_evidence_id` and `evidence_links` name the retained
analysis, so later views and application workflows read it without repeating
analysis or moving the complete record through the response. A server without
session retention refuses summary detail with `capability_unavailable`.
session retention refuses summary detail with `capability_unavailable`;
`inspect_artifact`, `inspect_wasm_artifact`, `inspect_macho` and
`decode_interface_builder` refuse before any analysis starts.
For `inspect_artifact`, omission also keeps the original request parameters and
complete output shape. Delivery selection never reaches the inventory engine;
the complete inspection and its atomic inventory Evidence remain retained.
@@ -191,3 +191,25 @@ it("preserves malformed-output failure when cancellation arrives with it", async
error: { _tag: "AnalysisOutputError" },
});
});
it("gives cancellation precedence over malformed raw input", async () => {
const controller = new AbortController();
controller.abort();
let calls = 0;
const service = new WasmArtifactService({
close: () => Promise.resolve(),
inspect: () => {
calls++;
return Promise.resolve(ok(wasmArtifactExecution()));
},
});
const malformed = { path: "relative.wasm" };
expect(
await service.inspect(malformed, { signal: controller.signal }),
).toMatchObject({ ok: false, error: { _tag: "AnalysisCancelledError" } });
expect(await service.inspect(malformed)).toMatchObject({
ok: false,
error: { _tag: "AnalysisInputError" },
});
expect(calls).toBe(0);
});
+10 -5
View File
@@ -7,7 +7,10 @@ import type { EvidenceMcpServer } from "./EvidenceMcpServer.js";
import type { WasmArtifactService } from "../application/wasm/WasmArtifactService.js";
import type { EvidenceWriter } from "../application/investigation/InvestigationRecordPort.js";
import type { EvidenceLookup } from "../application/EvidenceInputResolver.js";
import { summarizeRetainedAnalysis } from "../application/analysisView/AnalysisViewService.js";
import {
requireAnalysisViewRetention,
summarizeRetainedAnalysis,
} from "../application/analysisView/AnalysisViewService.js";
import { analysisInputErrorFromIssues } from "../domain/inputIssueProjection.js";
import { err } from "../domain/result.js";
import { wasmArtifactModule } from "../composition/wasm.js";
@@ -75,12 +78,14 @@ export const registerWasmTools = (
),
);
const { detail, ...input } = parsed.data;
const retention = { recordEvidence, evidenceById };
if (detail === "summary") {
const ready = requireAnalysisViewRetention(contract.name, retention);
if (!ready.ok) return ready;
}
const inspected = await service.inspect(input, options);
return detail === "summary" && inspected.ok
? summarizeRetainedAnalysis(inspected.value, {
recordEvidence,
evidenceById,
})
? summarizeRetainedAnalysis(inspected.value, retention)
: inspected;
},
logger,
+56 -9
View File
@@ -14,7 +14,8 @@ import { wasmArtifactSummaryViewResultSchema } from "../../../src/domain/analysi
import { createHash } from "node:crypto";
import { z } from "zod";
import { createServer } from "../../../src/server/createServer.js";
import { ok } from "../../../src/domain/result.js";
import { err, ok } from "../../../src/domain/result.js";
import { AnalysisInputError } from "../../../src/domain/analysisErrorCore.js";
import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js";
import { createTestBinarySession } from "../../fixtures/binarySession.js";
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
@@ -627,11 +628,14 @@ it("rejects absent or inconsistent WASM source identity before deriving portable
await harness.client.ping();
});
it("requires retention for WASM summary delivery while keeping fixed-server complete delivery", async () => {
it("rejects malformed paths, refuses WASM summary delivery before provider work when retention is unavailable, and keeps fixed-server complete delivery", async () => {
const value = wasmArtifactFixture();
const inspect = vi.fn<WasmArtifactService["provider"]["inspect"]>(() =>
Promise.resolve(ok(wasmArtifactExecution(value))),
);
const service = new WasmArtifactService({
close: () => Promise.resolve(),
inspect: () => Promise.resolve(ok(wasmArtifactExecution(value))),
inspect,
});
const session = createTestBinarySession(() => {
throw new Error("No deep provider");
@@ -645,9 +649,10 @@ it("requires retention for WASM summary delivery while keeping fixed-server comp
try {
await server.connect(b);
await client.connect(a);
const summaryInput = { path: value.artifact.path, detail: "summary" };
const summary = await client.callTool({
name: "inspect_wasm_artifact",
arguments: { path: value.artifact.path, detail: "summary" },
arguments: summaryInput,
});
expect(parseMcpToolError(summary)).toMatchObject({
error: {
@@ -655,14 +660,56 @@ it("requires retention for WASM summary delivery while keeping fixed-server comp
message: expect.stringContaining("does not retain Evidence"),
},
});
const complete = await client.callTool({
expect(inspect).not.toHaveBeenCalled();
const malformed = await client.callTool({
name: "inspect_wasm_artifact",
arguments: { path: value.artifact.path },
arguments: {
path: "relative.wasm",
glue_paths: ["relative.js"],
detail: "summary",
},
});
expect(complete.isError).not.toBe(true);
expect(parseEvidence(complete.structuredContent).normalized_result).toEqual(
value,
const rejected = parseMcpToolError(malformed).error;
expect(rejected.code).toBe("invalid_request");
expect(
z.object({ issues: z.array(z.unknown()) }).parse(rejected.details).issues,
).toEqual(
[["path"], ["glue_paths", 0]].map((path) => ({
path,
reason: "invalid_format",
expected: "regex",
message: "must be an absolute local filesystem path",
})),
);
expect(inspect).not.toHaveBeenCalled();
for (const input of [
{ path: value.artifact.path },
{ path: value.artifact.path, detail: "complete" },
]) {
const complete = await client.callTool({
name: "inspect_wasm_artifact",
arguments: input,
});
expect(complete.isError).not.toBe(true);
expect(
parseEvidence(complete.structuredContent).normalized_result,
).toEqual(value);
}
expect(inspect).toHaveBeenCalledTimes(2);
inspect.mockImplementation(() =>
Promise.resolve(err(new AnalysisInputError("inspect_wasm_artifact"))),
);
const refused = await client.callTool({
name: "inspect_wasm_artifact",
arguments: summaryInput,
});
expect(parseMcpToolError(refused)).toMatchObject({
error: {
code: "capability_unavailable",
message: expect.stringContaining("Select detail complete"),
},
});
expect(inspect).toHaveBeenCalledTimes(2);
await client.ping();
} finally {
await client.close();