From bd3a9bbbafa5e021c7a75c2c315b1167ee3b818e Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 11 Oct 2026 10:38:08 +0800 Subject: [PATCH] fix(native): classify PE resource input failures Report missing and non-file selections as input issues, malformed PE images as format errors, and unstable reads as artifact changes. Keep existing limit, permission, cancellation, and other I/O classifications. Co-authored-by: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- src/native/pe/PeResourcesProvider.ts | 107 +++++++++++------- .../applications/peResources.test.ts | 45 +++++++- tests/boundary/filesystem/peResources.test.ts | 24 +++- 3 files changed, 131 insertions(+), 45 deletions(-) diff --git a/src/native/pe/PeResourcesProvider.ts b/src/native/pe/PeResourcesProvider.ts index 50fb13a291..4326cb3b7c 100644 --- a/src/native/pe/PeResourcesProvider.ts +++ b/src/native/pe/PeResourcesProvider.ts @@ -5,7 +5,9 @@ import type { ExecutionOptions } from "../../application/AnalysisProvider.js"; import { ArtifactOperationError } from "../../domain/artifactOperationError.js"; import { AnalysisAccessDeniedError, + AnalysisArtifactChangedError, AnalysisCancelledError, + AnalysisInputError, AnalysisResourceConstraintError, } from "../../domain/analysisErrorCore.js"; import type { InspectPeResourcesInput } from "../../domain/native/peResources.js"; @@ -37,47 +39,70 @@ export class PeResourcesProvider implements PeResourcesPort { ), ); } catch (cause) { - if (options?.signal?.aborted) - return err( - new AnalysisCancelledError("inspect_pe_resources", { cause }), - ); - if (cause instanceof ArtifactReaderFailure && cause.reason === "limit") - return err( - new AnalysisResourceConstraintError( - "inspect_pe_resources", - "memory", - cause.message, - { - max_file_bytes: input.max_file_bytes, - max_entries: input.max_entries, - }, - { cause }, - ), - ); - if ( - cause instanceof Error && - "code" in cause && - (cause.code === "EACCES" || cause.code === "EPERM") - ) - return err( - new AnalysisAccessDeniedError( - "inspect_pe_resources", - input.path, - cause.code, - { cause }, - ), - ); - return err( - new ArtifactOperationError( - "inspect_pe_resources", - cause instanceof ArtifactReaderFailure ? cause.reason : "io", - undefined, - cause instanceof Error - ? cause.message - : "PE artifact inspection failed.", - { cause }, - ), - ); + return err(peResourcesFailure(cause, input, options?.signal)); } } } + +const peResourcesFailure = ( + cause: unknown, + input: InspectPeResourcesInput, + signal: AbortSignal | undefined, +) => { + const operation = "inspect_pe_resources"; + if (signal?.aborted) return new AnalysisCancelledError(operation, { cause }); + + if (cause instanceof ArtifactReaderFailure) { + if (cause.reason === "limit") + return new AnalysisResourceConstraintError( + operation, + "memory", + cause.message, + { + max_file_bytes: input.max_file_bytes, + max_entries: input.max_entries, + }, + { cause }, + ); + if (cause.reason === "integrity") + return new AnalysisArtifactChangedError( + operation, + input.path, + cause.message, + { cause }, + ); + if (cause.reason === "path" || cause.reason === "format") + return new AnalysisInputError(operation, { cause }, [ + { + path: ["path"], + reason: "invalid_format", + message: cause.message, + }, + ]); + if (cause.reason === "cancelled") + return new AnalysisCancelledError(operation, { cause }); + } + + if (cause instanceof Error && "code" in cause) { + if (cause.code === "EACCES" || cause.code === "EPERM") + return new AnalysisAccessDeniedError(operation, input.path, cause.code, { + cause, + }); + if (cause.code === "ENOENT" || cause.code === "ENOTDIR") + return new AnalysisInputError(operation, { cause }, [ + { + path: ["path"], + reason: "invalid_value", + message: `Selected PE image could not be read (${cause.code}): ${input.path}.`, + }, + ]); + } + + return new ArtifactOperationError( + operation, + cause instanceof ArtifactReaderFailure ? cause.reason : "io", + undefined, + cause instanceof Error ? cause.message : "PE artifact inspection failed.", + { cause }, + ); +}; diff --git a/tests/acceptance/applications/peResources.test.ts b/tests/acceptance/applications/peResources.test.ts index dc40f3cbce..2af2b988be 100644 --- a/tests/acceptance/applications/peResources.test.ts +++ b/tests/acceptance/applications/peResources.test.ts @@ -8,7 +8,36 @@ import { toolContract } from "../../../src/contracts/toolContracts.js"; import { peResourcesSchema } from "../../../src/domain/native/peResources.js"; import { peResourceFixture } from "../../../src/native/pe/PeResources.fixture.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; -import { cliTest } from "../../support/cli/cliFixture.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { cliTest, type TestCli } from "../../support/cli/cliFixture.js"; + +const expectPeInputFailureAcrossAdapters = async (input: { + readonly cli: TestCli; + readonly client: Client; + readonly toolName: string; + readonly path: string; + readonly reason: "invalid_format" | "invalid_value"; +}) => { + const { cli, client, toolName, path, reason } = input; + const result = await cli.run({ + arguments: ["inspect-pe-resources", path, "--json"], + }); + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + details: { + operation: "inspect_pe_resources", + issues: [{ path: ["path"], reason }], + }, + }); + const response = await client.callTool({ + name: toolName, + arguments: { path }, + }); + expect(response.isError).toBe(true); + expect(parseMcpToolError(response)).toEqual({ error: result.json }); +}; cliTest( "inspects PE32/PE32+ resources through compiled CLI and real stdio MCP without an active target", @@ -92,6 +121,20 @@ cliTest( ).toBe(evidence.evidence_id); expect(await readFile(path)).toEqual(fixture.bytes); } + const malformedPath = join(root, "malformed.exe"); + await writeFile(malformedPath, Buffer.from("not a PE image")); + for (const [path, reason] of [ + [join(root, "missing.exe"), "invalid_value"], + [malformedPath, "invalid_format"], + ] as const) { + await expectPeInputFailureAcrossAdapters({ + cli, + client, + toolName: contract.name, + path, + reason, + }); + } const absent = peResourceFixture(); absent.bytes.fill(0, absent.directoryAt, absent.directoryAt + 8); const path = join(root, "absent.exe"); diff --git a/tests/boundary/filesystem/peResources.test.ts b/tests/boundary/filesystem/peResources.test.ts index a343feaa17..5196f39494 100644 --- a/tests/boundary/filesystem/peResources.test.ts +++ b/tests/boundary/filesystem/peResources.test.ts @@ -6,17 +6,35 @@ import { peResourceFixture } from "../../../src/native/pe/PeResources.fixture.js import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; describe("PE resource stable filesystem boundary", () => { - it("rejects symlinks, missing files and directories without inventing empty inventory", async () => { + it("reports non-file and malformed PE selections as caller input errors", async () => { const root = await createTestTempDirectory("rea-pe-resource-files-"); const path = join(root, "image.exe"); const alias = join(root, "alias.exe"); + const malformed = join(root, "malformed.exe"); await writeFile(path, peResourceFixture().bytes); + await writeFile(malformed, Buffer.from("not a PE image")); await symlink(path, alias); const service = createPeResourcesService(); - for (const candidate of [root, alias, join(root, "missing.exe")]) { + for (const [candidate, reason] of [ + [root, "invalid_format"], + [alias, "invalid_format"], + [malformed, "invalid_format"], + [join(root, "missing.exe"), "invalid_value"], + [join(path, "child.exe"), "invalid_value"], + ] as const) { const result = await service.inspect({ path: candidate }); if (result.ok) throw new Error("Expected file acquisition failure"); - expect(result.error._tag).toBe("ArtifactOperationError"); + expect(result.error).toMatchObject({ + _tag: "AnalysisInputError", + operation: "inspect_pe_resources", + issues: [ + { + path: ["path"], + reason, + message: expect.any(String), + }, + ], + }); } });