From 6f96d7a3fa2a79ccf4e50344485b2a079dae75d2 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Wed, 21 Oct 2026 22:22:15 -0700 Subject: [PATCH] fix(capture): report scenario path search denial as access denied (#6244) Electron and process scenario path admission caught every realpath or stat failure as an invalid_value input issue, so a correct path beneath a directory without search permission returned invalid_request with schema-correction recovery instead of a host access denial. Admission now classifies EACCES/EPERM from path resolution and metadata reads as the existing AnalysisAccessDeniedError (selected path, system_code, filesystem-read boundary). Missing, looping or otherwise unresolvable paths and reachable files without execute permission keep their input-error semantics. Electron reports invalid selections before a denial on another field. Process PATH search still continues past a denied directory; only an explicitly selected executable reports denial. Fixes #6117 Co-authored-by: Claude Opus 5.5 --- .../ElectronActiveObservationService.test.ts | 190 ++++++++++++++++++ .../ElectronActiveObservationService.ts | 126 +++++++++--- .../ProcessScenarioRuntimeValidation.test.ts | 122 ++++++++++- .../ProcessScenarioRuntimeValidation.ts | 78 +++++-- .../cli/scenarioCliInputDiagnostics.test.ts | 41 ++++ 5 files changed, 506 insertions(+), 51 deletions(-) create mode 100644 src/application/javascript/ElectronActiveObservationService.test.ts diff --git a/src/application/javascript/ElectronActiveObservationService.test.ts b/src/application/javascript/ElectronActiveObservationService.test.ts new file mode 100644 index 0000000000..0882db0a39 --- /dev/null +++ b/src/application/javascript/ElectronActiveObservationService.test.ts @@ -0,0 +1,190 @@ +import { chmod, mkdir, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it, vi } from "vitest"; + +import { AnalysisCapabilityUnavailableError } from "../../domain/analysisErrorCore.js"; +import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; +import { electronActiveObservationInputSchema } from "../../domain/javascript/electronActiveObservation.js"; +import { err } from "../../domain/result.js"; +import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; +import type { ElectronActiveObservationPort } from "./ElectronActiveObservationPort.js"; +import { captureElectronScenario } from "./ElectronActiveObservationService.js"; + +const SKIP_PERMISSIONS = + process.platform === "win32" || process.getuid?.() === 0; + +/** A provider that records admitted input and never launches anything. */ +const recordingProvider = () => { + const capture = vi.fn(() => + Promise.resolve( + err( + new AnalysisCapabilityUnavailableError( + "test", + "capture_electron_scenario", + "recording provider", + ), + ), + ), + ); + const provider: ElectronActiveObservationPort = { + identity: () => ({ id: "test", name: "test", version: "1" }), + capture, + }; + return { provider, capture }; +}; + +const fixture = async () => { + const root = await createTestTempDirectory("rea-electron-admission-"); + const denied = join(root, "denied"); + const readable = join(root, "readable"); + await mkdir(join(denied, "root"), { recursive: true }); + await mkdir(readable); + const executable = join(denied, "electron"); + await writeFile(executable, "#!/bin/sh\n", { mode: 0o755 }); + await writeFile(join(denied, "main.js"), "module.exports = {};\n"); + await writeFile(join(readable, "main.js"), "module.exports = {};\n"); + return { root, denied, readable, executable }; +}; + +/** Remove search permission from `directory` for the duration of `run`. */ +const withoutSearchPermission = async ( + directory: string, + run: () => Promise, +): Promise => { + await chmod(directory, 0o000); + try { + return await run(); + } finally { + await chmod(directory, 0o700); + } +}; + +it + .skipIf(SKIP_PERMISSIONS) + .each(["application_path", "application_root", "executable_path"] as const)( + "reports a search-denied %s as host access denial before any provider work", + async (field) => { + const { denied, readable, executable } = await fixture(); + const selected = { + application_path: join(denied, "main.js"), + application_root: join(denied, "root"), + executable_path: executable, + }[field]; + const { provider, capture } = recordingProvider(); + const result = await withoutSearchPermission(denied, () => + captureElectronScenario( + provider, + electronActiveObservationInputSchema.parse({ + executable_path: process.execPath, + application_path: join(readable, "main.js"), + actions: [], + [field]: selected, + }), + ), + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "access_denied", + category: "unavailable", + details: { + operation: "capture_electron_scenario", + path: selected, + system_code: "EACCES", + boundary: "filesystem-read", + }, + }); + expect(capture).not.toHaveBeenCalled(); + }, +); + +it.skipIf(SKIP_PERMISSIONS)( + "reports invalid selections before a host denial on another path", + async () => { + const { root, denied } = await fixture(); + const missing = join(root, "missing.js"); + const { provider, capture } = recordingProvider(); + const result = await withoutSearchPermission(denied, () => + captureElectronScenario( + provider, + electronActiveObservationInputSchema.parse({ + executable_path: process.execPath, + application_path: missing, + application_root: join(denied, "root"), + actions: [], + }), + ), + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "invalid_request", + details: { + issues: [ + { + path: ["application_path"], + reason: "invalid_value", + message: expect.stringContaining("ENOENT"), + }, + ], + }, + }); + expect(capture).not.toHaveBeenCalled(); + }, +); + +it.skipIf(SKIP_PERMISSIONS).each([ + { label: "0644", mode: 0o644 }, + { label: "0000", mode: 0o000 }, +])( + "keeps a reachable executable without execute permission (mode $label) as invalid input", + async ({ mode }) => { + const { root, readable } = await fixture(); + const executable = join(root, "not-executable"); + await writeFile(executable, "not executable"); + await chmod(executable, mode); + const { provider, capture } = recordingProvider(); + const result = await captureElectronScenario( + provider, + electronActiveObservationInputSchema.parse({ + executable_path: executable, + application_path: join(readable, "main.js"), + actions: [], + }), + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "invalid_request", + details: { + issues: [ + { + path: ["executable_path"], + reason: "invalid_value", + message: expect.stringContaining("is not an executable file"), + }, + ], + }, + }); + expect(capture).not.toHaveBeenCalled(); + }, +); + +it("admits readable selections and hands canonical paths to the provider", async () => { + const { readable } = await fixture(); + const { provider, capture } = recordingProvider(); + const application = join(readable, "main.js"); + await captureElectronScenario( + provider, + electronActiveObservationInputSchema.parse({ + executable_path: process.execPath, + application_path: application, + actions: [], + }), + ); + expect(capture).toHaveBeenCalledOnce(); + expect(capture.mock.calls[0]?.[0]).toMatchObject({ + application_path: application, + application_root: readable, + }); +}); diff --git a/src/application/javascript/ElectronActiveObservationService.ts b/src/application/javascript/ElectronActiveObservationService.ts index faf68db803..ec8a26c557 100644 --- a/src/application/javascript/ElectronActiveObservationService.ts +++ b/src/application/javascript/ElectronActiveObservationService.ts @@ -1,5 +1,5 @@ import { dirname } from "node:path"; -import { constants } from "node:fs"; +import { constants, type Stats } from "node:fs"; import { access, realpath, stat } from "node:fs/promises"; import type { ExecutionOptions } from "../AnalysisProvider.js"; @@ -8,6 +8,7 @@ import type { ElectronActiveObservationPort, } from "./ElectronActiveObservationPort.js"; import { + AnalysisAccessDeniedError, AnalysisCapabilityUnavailableError, AnalysisInputError, type AnalysisInputIssue, @@ -21,53 +22,107 @@ import { createElectronActiveEvidence } from "./ElectronActiveEvidence.js"; const OPERATION = "capture_electron_scenario" as const; const PROVIDER_ID = "rea-playwright-electron-active"; -const resolveInputPath = async ( - field: "executable_path" | "application_path" | "application_root", +type ElectronPathField = + | "executable_path" + | "application_path" + | "application_root"; + +/** + * Why a selected path was not admitted. Host permission denial is not + * malformed caller input: the path may be correct, but REA cannot read it. + */ +type PathAdmissionFailure = + | { + readonly kind: "invalid"; + readonly issue: AnalysisInputIssue; + readonly cause: unknown; + } + | { readonly kind: "denied"; readonly error: AnalysisAccessDeniedError }; + +const hostAccessDenial = ( selected: string, -): Promise< - Result< - string, - { readonly issue: AnalysisInputIssue; readonly cause: unknown } - > -> => { + cause: unknown, +): PathAdmissionFailure | undefined => { + const code: unknown = + cause instanceof Error ? Reflect.get(cause, "code") : undefined; + return code === "EACCES" || code === "EPERM" + ? { + kind: "denied", + error: new AnalysisAccessDeniedError(OPERATION, selected, code, { + cause, + }), + } + : undefined; +}; + +const resolveInputPath = async ( + field: ElectronPathField, + selected: string, +): Promise> => { try { return ok(await realpath(selected)); } catch (cause: unknown) { - return err({ - issue: { - path: [field], - reason: "invalid_value", - message: `Cannot resolve ${field} ${selected}: ${cause instanceof Error ? cause.message : String(cause)}`, + return err( + hostAccessDenial(selected, cause) ?? { + kind: "invalid", + issue: { + path: [field], + reason: "invalid_value", + message: `Cannot resolve ${field} ${selected}: ${cause instanceof Error ? cause.message : String(cause)}`, + }, + cause, }, - cause, - }); + ); } }; -/** Resolve the executable and require a launchable regular file before any spawn. */ +/** + * Resolve the executable and require a launchable regular file before any + * spawn. Denied resolution or metadata reads are host access failures; a + * readable file without execute permission is an invalid selection. + */ const resolveExecutablePath = async ( selected: string, ): ReturnType => { const resolved = await resolveInputPath("executable_path", selected); if (!resolved.ok) return resolved; + let metadata: Stats; try { - if (!(await stat(resolved.value)).isFile()) - throw new Error("not a regular file"); - if (process.platform !== "win32") - await access(resolved.value, constants.X_OK); - return resolved; + metadata = await stat(resolved.value); } catch (cause: unknown) { - return err({ - issue: { - path: ["executable_path"], - reason: "invalid_value", - message: `executable_path ${selected} is not an executable file: ${cause instanceof Error ? cause.message : String(cause)}`, - }, - cause, - }); + return err( + hostAccessDenial(selected, cause) ?? notExecutable(selected, cause), + ); } + if (!metadata.isFile()) + return err(notExecutable(selected, new Error("not a regular file"))); + if (process.platform !== "win32") + try { + await access(resolved.value, constants.X_OK); + } catch (cause: unknown) { + return err(notExecutable(selected, cause)); + } + return resolved; }; +const notExecutable = ( + selected: string, + cause: unknown, +): PathAdmissionFailure => ({ + kind: "invalid", + issue: { + path: ["executable_path"], + reason: "invalid_value", + message: `executable_path ${selected} is not an executable file: ${cause instanceof Error ? cause.message : String(cause)}`, + }, + cause, +}); + +/** + * Admit every selected path before any provider work. Invalid selections are + * reported together as input issues; a host access denial is reported only + * when no selection is invalid, because correcting the request comes first. + */ const canonicalizeInput = async ( input: ElectronActiveObservationInput, ): Promise> => { @@ -82,16 +137,23 @@ const canonicalizeInput = async ( const failures = [executable, application, root].flatMap((result) => result.ok ? [] : [result.error], ); + const issues = failures.flatMap((failure) => + failure.kind === "invalid" ? [failure.issue] : [], + ); + const denial = failures.find((failure) => failure.kind === "denied"); + if (issues.length === 0 && denial !== undefined) return err(denial.error); return err( new AnalysisInputError( OPERATION, { cause: new AggregateError( - failures.map(({ cause }) => cause), + failures.map((failure) => + failure.kind === "invalid" ? failure.cause : failure.error, + ), "Electron paths could not be resolved", ), }, - failures.map(({ issue }) => issue), + issues, ), ); } diff --git a/src/process/capture/ProcessScenarioRuntimeValidation.test.ts b/src/process/capture/ProcessScenarioRuntimeValidation.test.ts index 7e5cb7e300..77a43a7f2f 100644 --- a/src/process/capture/ProcessScenarioRuntimeValidation.test.ts +++ b/src/process/capture/ProcessScenarioRuntimeValidation.test.ts @@ -7,10 +7,13 @@ import { writeFile, } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { delimiter, join } from "node:path"; import { expect, it } from "vitest"; -import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; +import { + AnalysisAccessDeniedError, + AnalysisInputError, +} from "../../domain/analysisErrorCore.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { resolveExecutable, @@ -172,6 +175,121 @@ it.skipIf(process.platform === "win32")( }, ); +const SKIP_PERMISSIONS = + process.platform === "win32" || process.getuid?.() === 0; + +/** Remove search permission from `directory` for the duration of `run`. */ +const withoutSearchPermission = async ( + directory: string, + run: () => Promise, +): Promise => { + await chmod(directory, 0o000); + try { + return await run(); + } finally { + await chmod(directory, 0o700); + } +}; + +const deniedFixture = async () => { + const root = await createTestTempDirectory("rea-process-denied-"); + const denied = join(root, "denied"); + await mkdir(join(denied, "cwd"), { recursive: true }); + const tool = join(denied, "tool"); + await writeFile(tool, "#!/bin/sh\n", { mode: 0o755 }); + return { root, denied, tool }; +}; + +const runtimePathOutcome = async ( + scenario: Parameters[0], +): Promise => { + try { + return await resolveProcessScenarioRuntimePaths( + parseProcessScenario(scenario), + ); + } catch (cause: unknown) { + return cause; + } +}; + +it.skipIf(SKIP_PERMISSIONS)( + "reports a search-denied working directory as host access denial", + async () => { + const { denied } = await deniedFixture(); + const selected = join(denied, "cwd"); + const failure = await withoutSearchPermission(denied, () => + runtimePathOutcome({ + executable: process.execPath, + working_directory: selected, + }), + ); + expect(failure).toBeInstanceOf(AnalysisAccessDeniedError); + if (!(failure instanceof AnalysisAccessDeniedError)) return; + expect(projectAnalysisError(failure)).toMatchObject({ + code: "access_denied", + details: { + operation: "capture_process_scenario", + path: selected, + system_code: "EACCES", + boundary: "filesystem-read", + }, + }); + }, +); + +it.skipIf(SKIP_PERMISSIONS)( + "reports a search-denied explicit executable as host access denial", + async () => { + const { root, denied, tool } = await deniedFixture(); + const failure = await withoutSearchPermission(denied, () => + runtimePathOutcome({ + executable: "denied/tool", + working_directory: root, + }), + ); + expect(failure).toBeInstanceOf(AnalysisAccessDeniedError); + if (!(failure instanceof AnalysisAccessDeniedError)) return; + expect(projectAnalysisError(failure)).toMatchObject({ + code: "access_denied", + details: { path: tool, system_code: "EACCES" }, + }); + }, +); + +it.skipIf(SKIP_PERMISSIONS)( + "continues a PATH search past a search-denied directory", + async () => { + const { root, denied } = await deniedFixture(); + const bin = join(root, "bin"); + await mkdir(bin); + const tool = join(bin, "tool"); + await writeFile(tool, "#!/bin/sh\n", { mode: 0o755 }); + await withoutSearchPermission(denied, async () => { + await expect( + resolveExecutable("tool", root, [denied, bin].join(delimiter)), + ).resolves.toBe(tool); + const failure = await runtimePathOutcome({ + executable: "tool", + working_directory: root, + environment: { PATH: denied }, + }); + expect(failure).toBeInstanceOf(AnalysisInputError); + if (!(failure instanceof AnalysisInputError)) return; + expect(projectAnalysisError(failure)).toMatchObject({ + code: "invalid_request", + details: { + issues: [ + { + path: ["executable"], + message: `executable could not be resolved: tool; checked ${join(denied, "tool")} (EACCES)`, + }, + ], + }, + }); + }); + }, +); + const runtimePathFailure = async ( scenario: Parameters[0], ): Promise => { diff --git a/src/process/capture/ProcessScenarioRuntimeValidation.ts b/src/process/capture/ProcessScenarioRuntimeValidation.ts index e8a19771a1..77a63dfb6f 100644 --- a/src/process/capture/ProcessScenarioRuntimeValidation.ts +++ b/src/process/capture/ProcessScenarioRuntimeValidation.ts @@ -3,7 +3,10 @@ import { access, realpath, stat } from "node:fs/promises"; import { delimiter, isAbsolute, join, resolve, sep } from "node:path"; import type { ProcessScenario } from "../../domain/process/processScenario.js"; -import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; +import { + AnalysisAccessDeniedError, + AnalysisInputError, +} from "../../domain/analysisErrorCore.js"; import { snapshotEnvironment } from "../snapshotEnvironment.js"; import { processCaptureCancelled } from "./ProcessCaptureError.js"; @@ -18,20 +21,26 @@ export const resolveProcessScenarioRuntimePaths = async ( try { workingDirectory = await realpath(selectedWorkingDirectory); } catch (cause: unknown) { - throw invalidScenarioPath( - "working_directory", - `working directory could not be resolved: ${selectedWorkingDirectory}${osFailure(cause)}`, - cause, + throw ( + hostAccessDenial(selectedWorkingDirectory, cause) ?? + invalidScenarioPath( + "working_directory", + `working directory could not be resolved: ${selectedWorkingDirectory}${osFailure(cause)}`, + cause, + ) ); } let workingDirectoryStat; try { workingDirectoryStat = await stat(workingDirectory); } catch (cause: unknown) { - throw invalidScenarioPath( - "working_directory", - `working directory could not be inspected: ${workingDirectory}${osFailure(cause)}`, - cause, + throw ( + hostAccessDenial(selectedWorkingDirectory, cause) ?? + invalidScenarioPath( + "working_directory", + `working directory could not be inspected: ${workingDirectory}${osFailure(cause)}`, + cause, + ) ); } if (!workingDirectoryStat.isDirectory()) @@ -57,7 +66,12 @@ export const resolveProcessScenarioRuntimePaths = async ( }; }; -/** Resolve a selected executable through the caller's effective PATH. */ +/** + * Resolve a selected executable through the caller's effective PATH. A denied + * lookup of an explicitly selected path is a host access failure; PATH search + * continues past denied directories, and an existing file without execute + * permission remains an invalid selection. + */ export const resolveExecutable = async ( command: string, workingDirectory: string, @@ -70,22 +84,34 @@ export const resolveExecutable = async ( ? workingDirectory : resolve(workingDirectory, directory), ); - const candidates = + const explicit = isAbsolute(command) || command.includes("/") || - (sep === "\\" && command.includes("\\")) - ? [resolve(workingDirectory, command)] - : directories.map((directory) => join(directory, command)); + (sep === "\\" && command.includes("\\")); + const candidates = explicit + ? [resolve(workingDirectory, command)] + : directories.map((directory) => join(directory, command)); const failures: string[] = []; for (const candidate of candidates) { + let metadata; try { - await access(candidate, fsConstants.X_OK); - if ((await stat(candidate)).isFile()) return candidate; - failures.push(`${candidate} (not a regular file)`); + metadata = await stat(candidate); } catch (cause: unknown) { + const denial = explicit ? hostAccessDenial(candidate, cause) : undefined; + if (denial !== undefined) throw denial; failures.push(`${candidate}${osFailure(cause)}`); continue; } + if (!metadata.isFile()) { + failures.push(`${candidate} (not a regular file)`); + continue; + } + try { + await access(candidate, fsConstants.X_OK); + return candidate; + } catch (cause: unknown) { + failures.push(`${candidate}${osFailure(cause)}`); + } } throw invalidScenarioPath( "executable", @@ -104,6 +130,24 @@ const invalidScenarioPath = ( [{ path: [field], reason: "invalid_value", message }], ); +/** Host permissions denied resolving a selected path; not malformed input. */ +const hostAccessDenial = ( + selected: string, + cause: unknown, +): AnalysisAccessDeniedError | undefined => { + const code = osErrorCode(cause); + return code === "EACCES" || code === "EPERM" + ? new AnalysisAccessDeniedError( + "capture_process_scenario", + selected, + code, + { + cause, + }, + ) + : undefined; +}; + const osErrorCode = (cause: unknown): string | undefined => typeof cause === "object" && cause !== null && diff --git a/tests/boundary/cli/scenarioCliInputDiagnostics.test.ts b/tests/boundary/cli/scenarioCliInputDiagnostics.test.ts index ae03568f46..80dccb290b 100644 --- a/tests/boundary/cli/scenarioCliInputDiagnostics.test.ts +++ b/tests/boundary/cli/scenarioCliInputDiagnostics.test.ts @@ -1,3 +1,4 @@ +import { chmod, mkdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { describe, expect } from "vitest"; @@ -103,3 +104,43 @@ for (const field of [ expect(JSON.stringify(result.json)).toContain(missing); }); } + +cliTest.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "reports a search-denied Electron application_path as access denied", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-electron-denied-"); + const denied = join(root, "denied"); + await mkdir(denied); + const application = join(denied, "main.js"); + await writeFile(application, "module.exports = {};\n"); + await chmod(denied, 0o000); + let result; + try { + result = await cli.run({ + arguments: [ + "capture-electron-scenario", + JSON.stringify({ + executable_path: process.execPath, + application_path: application, + actions: [], + }), + "--json", + ], + environment: { REA_LOG_LEVEL: "silent" }, + }); + } finally { + await chmod(denied, 0o700); + } + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + code: "access_denied", + category: "unavailable", + details: { + operation: "capture_electron_scenario", + path: application, + system_code: "EACCES", + boundary: "filesystem-read", + }, + }); + }, +);