diff --git a/src/artifacts/RegularFileAdmission.test.ts b/src/artifacts/RegularFileAdmission.test.ts index 17926799a..97f61fc34 100644 --- a/src/artifacts/RegularFileAdmission.test.ts +++ b/src/artifacts/RegularFileAdmission.test.ts @@ -1,5 +1,6 @@ import type { BigIntStats, Stats } from "node:fs"; import type { FileHandle } from "node:fs/promises"; +import { tmpdir } from "node:os"; import { afterEach, expect, it, vi } from "vitest"; import { @@ -88,3 +89,36 @@ it("compares available nanosecond timestamps without rounding away source change sameRegularFileState(before, { ...before, ctimeNs: before.ctimeNs + 1n }), ).toBe(false); }); + +it.each([ + ["EACCES", -13, "keeps"], + ["ENXIO", -6, "reclassifies"], +] as const)( + "%s from opening a non-regular path: admission %s the failure", + async (code, errno, outcome) => { + const failure = Object.assign(new Error(`open ${code}`), { code, errno }); + mockOpen.mockRejectedValue(failure); + // The selected path itself is a directory that stat can still describe. + const cause = await openRegularFile(tmpdir(), { symlinks: "follow" }).catch( + (rejected: unknown) => rejected, + ); + if (outcome === "keeps") expect(cause).toBe(failure); + else expect(cause).toBeInstanceOf(NonRegularFileReadError); + }, +); + +it("keeps cancellation that arrives during the non-regular metadata probe", async () => { + const controller = new AbortController(); + const cancellation = new Error("cancelled during probe"); + mockOpen.mockImplementation(async () => { + // The probe stats the path after this rejection; abort before it settles. + queueMicrotask(() => controller.abort(cancellation)); + throw Object.assign(new Error("open ENXIO"), { code: "ENXIO", errno: -6 }); + }); + await expect( + openRegularFile(tmpdir(), { + symlinks: "follow", + signal: controller.signal, + }), + ).rejects.toBe(cancellation); +}); diff --git a/src/filesystem/RegularFile.ts b/src/filesystem/RegularFile.ts index 4942e2e46..47fdfa1d8 100644 --- a/src/filesystem/RegularFile.ts +++ b/src/filesystem/RegularFile.ts @@ -1,6 +1,7 @@ import { constants } from "node:fs"; +import { constants as osConstants } from "node:os"; import type { BigIntStats, Stats } from "node:fs"; -import { lstat, open, type FileHandle } from "node:fs/promises"; +import { lstat, open, stat, type FileHandle } from "node:fs/promises"; import { OwnedFileHandle } from "./OwnedFileHandle.js"; @@ -66,7 +67,17 @@ export const openRegularFile = async ( (constants.O_NONBLOCK ?? 0) | (constants.O_NOCTTY ?? 0) | (options.symlinks === "reject" ? (constants.O_NOFOLLOW ?? 0) : 0), - ); + ).catch(async (cause: unknown) => { + const nonRegular = await nonRegularOpenFailure( + path, + options.symlinks, + cause, + ); + if (nonRegular === undefined) throw cause; + // Cancellation during the metadata probe outranks the reclassification. + options.signal?.throwIfAborted(); + throw nonRegular; + }); const owner = new OwnedFileHandle(handle); try { options.signal?.throwIfAborted(); @@ -95,6 +106,41 @@ export const openRegularFile = async ( } }; +/** + * Some non-regular files cannot be opened at all: a Unix socket fails with + * ENXIO on Linux and EOPNOTSUPP on macOS. Confirm such a failure against the + * path's own metadata before reporting a non-regular selection. + */ +const nonRegularOpenFailure = async ( + path: string, + symlinks: "follow" | "reject", + cause: unknown, +): Promise => { + // Only errors meaning "this file type cannot be opened" are reclassified; + // permission, symlink-rejection and resource failures keep their codes. + if (!isUnopenableFileTypeError(cause)) return undefined; + const metadata = await (symlinks === "reject" ? lstat : stat)(path).catch( + () => undefined, + ); + if (metadata === undefined || metadata.isFile()) return undefined; + return new NonRegularFileReadError(path, metadata.isDirectory()); +}; + +// Match by errno: libuv has no name for macOS EOPNOTSUPP (102) and reports +// "Unknown system error -102" as its code. +const errnos: Partial> = osConstants.errno; +const UNOPENABLE_FILE_TYPE_ERRNOS: ReadonlySet = new Set( + [errnos.ENXIO, errnos.EOPNOTSUPP, errnos.ENOTSUP].flatMap((errno) => + errno === undefined ? [] : [-errno], + ), +); + +const isUnopenableFileTypeError = (cause: unknown): boolean => + cause instanceof Error && + "errno" in cause && + typeof cause.errno === "number" && + UNOPENABLE_FILE_TYPE_ERRNOS.has(cause.errno); + /** Compare one regular file's identity and mutable metadata across a read. */ export const sameRegularFileState = ( expected: Stats | BigIntStats, diff --git a/src/flutter/FlutterBuildProvider.ts b/src/flutter/FlutterBuildProvider.ts index 9a6e6fe50..ea5d56025 100644 --- a/src/flutter/FlutterBuildProvider.ts +++ b/src/flutter/FlutterBuildProvider.ts @@ -33,6 +33,7 @@ import { import { ZipArtifactReader } from "../artifacts/ZipArtifactReader.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { + NonRegularFileReadError, openRegularFile, RegularFileAdmissionFailure, sameRegularFileState, @@ -181,16 +182,29 @@ const mapInputFailure = ( { max_metadata_read_bytes: APK_METADATA_READ_MAX_BYTES }, { cause }, ); - const message = cause instanceof Error ? cause.message : String(cause); return new AnalysisInputError(operation, { cause }, [ { path: ["path"], reason: "invalid_format", - message: `The APK could not be read as a package: ${path}: ${message}`, + message: inputFailureMessage(path, cause), }, ]); }; +/** Name a missing or non-file selection instead of blaming package bytes. */ +const inputFailureMessage = (path: string, cause: unknown): string => { + if ( + cause instanceof Error && + "code" in cause && + (cause.code === "ENOENT" || cause.code === "ENOTDIR") + ) + return `Selected APK does not exist: ${path}`; + if (cause instanceof NonRegularFileReadError) + return `Selected APK is not a regular file: ${path}`; + const message = cause instanceof Error ? cause.message : String(cause); + return `The APK could not be read as a package: ${path}: ${message}`; +}; + /** Provider-owned storage failures must never be attributed to caller input. */ const mapOwnedIoFailure = ( operation: string, diff --git a/tests/boundary/filesystem/flutterBuildProvider.test.ts b/tests/boundary/filesystem/flutterBuildProvider.test.ts index 4a1c8248f..68e0328c4 100644 --- a/tests/boundary/filesystem/flutterBuildProvider.test.ts +++ b/tests/boundary/filesystem/flutterBuildProvider.test.ts @@ -1,5 +1,6 @@ import { createReadStream } from "node:fs"; import { open, readFile, rm, stat, utimes, writeFile } from "node:fs/promises"; +import { createServer } from "node:net"; import { join } from "node:path"; import { Readable } from "node:stream"; import { Uint8ArrayReader, Uint8ArrayWriter, ZipWriter } from "@zip.js/zip.js"; @@ -1171,6 +1172,69 @@ it.each(["identify_flutter_build", "inspect_dart_aot"] as const)( }, ); +const expectSelectionIssue = async ( + provider: FlutterBuildProvider, + path: string, + message: string, +): Promise => { + for (const operation of [ + "identify_flutter_build", + "inspect_dart_aot", + ] as const) { + const result = await provider.execute({ operation, input: { path } }); + if (result.ok) throw new Error("A non-file APK selection must fail"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "invalid_request", + details: { + issues: [ + { + path: ["path"], + reason: "invalid_format", + message: `${message}: ${path}`, + }, + ], + }, + }); + } +}; + +it("names a missing or non-file APK selection instead of blaming package bytes", async () => { + const root = await createTestTempDirectory("rea-flutter-selection-"); + const provider = new FlutterBuildProvider(); + onTestFinished(() => provider.close()); + await expectSelectionIssue( + provider, + join(root, "absent.apk"), + "Selected APK does not exist", + ); + await expectSelectionIssue( + provider, + root, + "Selected APK is not a regular file", + ); +}); + +// Windows listens on named pipes rather than filesystem sockets. +it.skipIf(process.platform === "win32")( + "names an unopenable Unix socket APK selection as a non-file", + async () => { + const root = await createTestTempDirectory("rea-flutter-socket-"); + const socketPath = join(root, "s.sock"); + const socket = createServer(); + await new Promise((resolve) => socket.listen(socketPath, resolve)); + onTestFinished( + () => new Promise((resolve) => socket.close(() => resolve())), + ); + const provider = new FlutterBuildProvider(); + onTestFinished(() => provider.close()); + await expectSelectionIssue( + provider, + socketPath, + "Selected APK is not a regular file", + ); + }, +); + it.each(["EACCES", "EPERM"] as const)( "preserves %s at the shared source admission boundary", async (code) => {