From 77ed32fabc35d22faa0c4a75884e77baf1d34f00 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 03:18:32 +0800 Subject: [PATCH] fix(javascript): retain selected-root access and filesystem failure facts --- docs/javascript-artifact-reconstruction.md | 5 + .../JavaScriptApplicationService.ts | 33 +++++- .../JavaScriptArtifactReconstruction.ts | 25 ++-- ...avascriptApplicationInputSelection.test.ts | 37 ++++-- .../mcp/javascriptFilesystemErrors.test.ts | 112 ++++++++++++++++++ 5 files changed, 189 insertions(+), 23 deletions(-) create mode 100644 tests/boundary/mcp/javascriptFilesystemErrors.test.ts diff --git a/docs/javascript-artifact-reconstruction.md b/docs/javascript-artifact-reconstruction.md index bfebec6950..09658409f9 100644 --- a/docs/javascript-artifact-reconstruction.md +++ b/docs/javascript-artifact-reconstruction.md @@ -60,6 +60,11 @@ The result retains the canonical local path, root artifact digest, artifact manifest and graph commitments, JavaScript Application Graph, static Electron summary, reconstruction statistics, and explicit limitations. It does not require a live Hopper, Ghidra, browser, or Electron process. +Selected directories require read and search access. A denied root remains a +typed host access failure; omission handling for denied descendants does not +make an inaccessible selected root analyzable. Other host filesystem failures +retain the selected input, reported errno and syscall/path when supplied, and +the original diagnostic separately from artifact-format failures. The application graph selects the shallowest `package.json` outside `node_modules` as its package root, breaking equal-depth ties by Unicode diff --git a/src/application/javascript/JavaScriptApplicationService.ts b/src/application/javascript/JavaScriptApplicationService.ts index 3ca6f122cf..82c114a40d 100644 --- a/src/application/javascript/JavaScriptApplicationService.ts +++ b/src/application/javascript/JavaScriptApplicationService.ts @@ -12,7 +12,10 @@ import { type JavaScriptApplicationAnalysisResult, parseOwnedJavaScriptApplicationAnalysisSteps, } from "../../domain/javascript/javascriptApplicationAnalysis.js"; -import { AnalysisOutputError } from "../../domain/analysisErrorCore.js"; +import { + AnalysisAccessDeniedError, + AnalysisOutputError, +} from "../../domain/analysisErrorCore.js"; import { ArtifactOperationError } from "../../domain/artifactOperationError.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; import type { Evidence } from "../../domain/evidence.js"; @@ -170,8 +173,32 @@ export const analyzeJavaScriptApplicationValidated = async ( "code" in cause && typeof cause.code === "string" && FILESYSTEM_ERROR_CODES.has(cause.code) - ) - return err(new ArtifactOperationError(OPERATION, "io")); + ) { + const path: unknown = Reflect.get(cause, "path"); + const syscall: unknown = Reflect.get(cause, "syscall"); + const diagnostics = { + input_path: input.input_path, + system_code: cause.code, + ...(typeof path === "string" ? { path } : {}), + ...(typeof syscall === "string" ? { syscall } : {}), + }; + return err( + cause.code === "EACCES" || cause.code === "EPERM" + ? new AnalysisAccessDeniedError( + OPERATION, + input.input_path, + cause.code, + { cause, diagnostics }, + ) + : new ArtifactOperationError( + OPERATION, + "io", + undefined, + cause.message, + { cause, diagnostics }, + ), + ); + } return err( new ProviderAdapterError(JAVASCRIPT_APPLICATION_PROVIDER.id, OPERATION, { cause, diff --git a/src/application/javascript/JavaScriptArtifactReconstruction.ts b/src/application/javascript/JavaScriptArtifactReconstruction.ts index fe726070e8..50aaa1805c 100644 --- a/src/application/javascript/JavaScriptArtifactReconstruction.ts +++ b/src/application/javascript/JavaScriptArtifactReconstruction.ts @@ -2,7 +2,7 @@ import { DEFAULT_ASAR_HEADER_ADMISSION, type AsarHeaderAdmission, } from "../../config/asarHeaderAdmission.js"; -import { constants } from "node:fs"; +import { constants, type Stats } from "node:fs"; import { buildJavaScriptRouteInventorySteps, type JavaScriptRouteInventory, @@ -102,8 +102,8 @@ export const reconstructJavaScriptArtifact = async ( }; const input = javascriptArtifactReconstructionInputSchema.parse(rawInput); abortIfNeeded(signal); - const path = await resolveSelectedInput(input.input_path); - const format = await resolveFormat(path, input); + const { path, metadata } = await resolveSelectedInput(input.input_path); + const format = resolveFormat(path, metadata, input); const snapshot = await scanCanonicalArtifactInventoryInScope(path, { resourceScope: scope, signal, @@ -425,11 +425,18 @@ const OPERATION = "analyze_javascript_application"; * unreadable selection is a caller or host-resolution failure, not a damaged * artifact. */ -const resolveSelectedInput = async (inputPath: string): Promise => { +const resolveSelectedInput = async ( + inputPath: string, +): Promise<{ readonly path: string; readonly metadata: Stats }> => { try { const path = await realpath(inputPath); - await access(path, constants.R_OK); - return path; + const metadata = await lstat(path); + // Directory enumeration needs read access; resolving its entries also needs search access. + await access( + path, + metadata.isDirectory() ? constants.R_OK | constants.X_OK : constants.R_OK, + ); + return { path, metadata }; } catch (cause: unknown) { const code = cause instanceof Error && "code" in cause ? String(cause.code) : ""; @@ -466,11 +473,11 @@ const selectedInputResolutionMessage = ( : `Selected input path could not be resolved because symbolic-link traversal looped or exceeded the host limit (ELOOP): ${inputPath}`; /** The selected input's kind, checked against the caller's requested format. */ -const resolveFormat = async ( +const resolveFormat = ( path: string, + metadata: Stats, input: JavaScriptArtifactReconstructionInput, -): Promise<"asar" | "directory"> => { - const metadata = await lstat(path); +): "asar" | "directory" => { const observed = metadata.isDirectory() ? "directory" : metadata.isFile() && path.toLowerCase().endsWith(".asar") diff --git a/tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts b/tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts index e0af6b86da..3f8bd25b5b 100644 --- a/tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts +++ b/tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts @@ -1,7 +1,7 @@ import { chmod, mkdir, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { describe, expect, it, onTestFinished } from "vitest"; +import { describe, expect, it } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -154,17 +154,32 @@ describe("JavaScript application input selection", () => { }, ); - it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( - "reports an unreadable selected directory as a host access denial", - async () => { + it + .skipIf(process.platform === "win32" || process.getuid?.() === 0) + .each([0o000, 0o400])( + "reports denied selected-directory read/search access as a host denial (mode %s)", + async (mode) => { const { directory } = await application(); - await chmod(directory, 0o000); - onTestFinished(() => chmod(directory, 0o700)); - - expect(await analyzeFailure({ input_path: directory })).toMatchObject({ - code: "access_denied", - details: { path: directory, system_code: "EACCES" }, - }); + expect( + (await analyzeJavaScriptApplication({ input_path: directory })).ok, + ).toBe(true); + await chmod(directory, mode); + try { + expect(await analyzeFailure({ input_path: directory })).toMatchObject({ + code: "access_denied", + category: "unavailable", + details: { + path: directory, + system_code: "EACCES", + boundary: "filesystem-read", + }, + }); + } finally { + await chmod(directory, 0o700); + } + expect( + (await analyzeJavaScriptApplication({ input_path: directory })).ok, + ).toBe(true); }, ); }); diff --git a/tests/boundary/mcp/javascriptFilesystemErrors.test.ts b/tests/boundary/mcp/javascriptFilesystemErrors.test.ts new file mode 100644 index 0000000000..9020fab1cd --- /dev/null +++ b/tests/boundary/mcp/javascriptFilesystemErrors.test.ts @@ -0,0 +1,112 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { expect, it, vi } from "vitest"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +const fault = vi.hoisted(() => ({ + method: "", + path: "", + error: undefined as Error | undefined, + calls: 0, +})); +vi.mock("node:fs/promises", async (importOriginal) => { + const native = await importOriginal(); + const inject = (method: string, path: unknown): void => { + if (fault.method !== method || String(path) !== fault.path) return; + fault.calls += 1; + throw fault.error; + }; + return { + ...native, + lstat: (...args: Parameters) => { + inject("lstat", args[0]); + return native.lstat(...args); + }, + realpath: (...args: Parameters) => { + inject("realpath", args[0]); + return native.realpath(...args); + }, + access: (...args: Parameters) => { + inject("access", args[0]); + return native.access(...args); + }, + }; +}); +import { createServer } from "../../../src/server/createServer.js"; +import { createTestBinarySession } from "../../fixtures/binarySession.js"; + +it.each([ + { method: "lstat", code: "EACCES" }, + { method: "realpath", code: "EIO" }, + { method: "access", code: "EMFILE" }, +])( + "retains selected input and host facts through MCP after $method/$code refusal and accepts a readable retry", + async ({ method, code }) => { + const root = await createTestTempDirectory("rea-mcp-js-filesystem-"); + const session = createTestBinarySession(() => { + throw new Error( + "Static JavaScript analysis must not start a binary provider", + ); + }); + const server = createServer({ kind: "session", session }); + const client = new Client({ name: "javascript-filesystem", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + fault.method = method; + fault.path = root; + fault.calls = 0; + fault.error = Object.assign( + new Error(`${code}: selected ${method} failed at ${root}`), + { code, path: root, syscall: method }, + ); + const response = await client.callTool({ + name: "analyze_javascript_application", + arguments: { input_path: root, format: "directory" }, + }); + expect(fault.calls).toBe(1); + expect(response.isError).toBe(true); + const text = response.content.find((item) => item.type === "text"); + if (text?.type !== "text") + throw new Error("Expected structured text error"); + const result = JSON.parse(text.text); + if (code === "EACCES") + expect(result.error).toMatchObject({ + code: "access_denied", + category: "unavailable", + details: { + path: root, + system_code: code, + boundary: "filesystem-read", + }, + }); + else + expect(result.error).toMatchObject({ + code: "artifact_operation_failed", + details: { + reason: "io", + detail: fault.error.message, + diagnostics: { + input_path: root, + path: root, + system_code: code, + syscall: method, + }, + }, + }); + fault.method = ""; + const control = await client.callTool({ + name: "analyze_javascript_application", + arguments: { input_path: root, format: "directory" }, + }); + expect(control.isError, JSON.stringify(control)).not.toBe(true); + } finally { + fault.method = ""; + fault.error = undefined; + await client.close(); + await server.close(); + await session.close(); + } + }, +);