fix(flutter): name a missing or non-file APK selection (#2727)
* fix(flutter): name a missing or non-file APK selection identify_flutter_build and inspect_dart_aot reported a missing path or a directory as "The APK could not be read as a package: <path>: ENOENT ...", which blames the package bytes for a selection mistake. Report "Selected APK does not exist" for ENOENT/ENOTDIR and "Selected APK is not a regular file" for a non-regular selection, matching the SQLite inspector's wording. The issue path and invalid_format reason are unchanged; package format failures keep their existing message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(filesystem): classify unopenable non-regular selections at admission A Unix socket cannot be opened at all: open() fails with ENXIO on Linux and EOPNOTSUPP on macOS (surfaced by libuv as "Unknown system error -102") before openRegularFile can stat the descriptor, so such a selection escaped NonRegularFileReadError and the Flutter provider still blamed the package bytes. When open fails, classify the path by its own metadata (lstat when symlinks are rejected, stat otherwise): an existing non-regular file becomes NonRegularFileReadError. Missing paths, permission failures, regular files, and the O_NOFOLLOW ELOOP rejection keep their original errors. The Flutter selection test now covers a real Unix socket. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(filesystem): reclassify only unopenable file-type errors at admission Checking the path's metadata after any open failure turned EACCES on an unreadable directory into NonRegularFileReadError, so callers reported invalid input instead of access_denied. Reclassify only errors that mean the file type cannot be opened (ENXIO, EOPNOTSUPP, ENOTSUP, matched by errno because libuv has no name for macOS EOPNOTSUPP 102); permission, symlink-rejection and resource failures keep their original errors. Run the Unix socket selection check in its own test, skipped on Windows where listen() takes named pipes, and keep the missing-path and directory checks cross-platform. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(filesystem): keep cancellation that arrives during the non-regular probe A caller abort while the post-open metadata probe ran was replaced by NonRegularFileReadError, so callers such as parseBinaryTarget reported an input-format error instead of cancellation. Recheck the signal before substituting the non-regular classification. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
c22d7cd756
commit
46f51ee6cc
|
|
@ -1,5 +1,6 @@
|
||||||
import type { BigIntStats, Stats } from "node:fs";
|
import type { BigIntStats, Stats } from "node:fs";
|
||||||
import type { FileHandle } from "node:fs/promises";
|
import type { FileHandle } from "node:fs/promises";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
import { afterEach, expect, it, vi } from "vitest";
|
import { afterEach, expect, it, vi } from "vitest";
|
||||||
|
|
||||||
import {
|
import {
|
||||||
|
|
@ -88,3 +89,36 @@ it("compares available nanosecond timestamps without rounding away source change
|
||||||
sameRegularFileState(before, { ...before, ctimeNs: before.ctimeNs + 1n }),
|
sameRegularFileState(before, { ...before, ctimeNs: before.ctimeNs + 1n }),
|
||||||
).toBe(false);
|
).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);
|
||||||
|
});
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
import { constants } from "node:fs";
|
import { constants } from "node:fs";
|
||||||
|
import { constants as osConstants } from "node:os";
|
||||||
import type { BigIntStats, Stats } from "node:fs";
|
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";
|
import { OwnedFileHandle } from "./OwnedFileHandle.js";
|
||||||
|
|
||||||
|
|
@ -66,7 +67,17 @@ export const openRegularFile = async (
|
||||||
(constants.O_NONBLOCK ?? 0) |
|
(constants.O_NONBLOCK ?? 0) |
|
||||||
(constants.O_NOCTTY ?? 0) |
|
(constants.O_NOCTTY ?? 0) |
|
||||||
(options.symlinks === "reject" ? (constants.O_NOFOLLOW ?? 0) : 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);
|
const owner = new OwnedFileHandle(handle);
|
||||||
try {
|
try {
|
||||||
options.signal?.throwIfAborted();
|
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<NonRegularFileReadError | undefined> => {
|
||||||
|
// 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<Record<string, number>> = osConstants.errno;
|
||||||
|
const UNOPENABLE_FILE_TYPE_ERRNOS: ReadonlySet<number> = 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. */
|
/** Compare one regular file's identity and mutable metadata across a read. */
|
||||||
export const sameRegularFileState = (
|
export const sameRegularFileState = (
|
||||||
expected: Stats | BigIntStats,
|
expected: Stats | BigIntStats,
|
||||||
|
|
|
||||||
|
|
@ -33,6 +33,7 @@ import {
|
||||||
import { ZipArtifactReader } from "../artifacts/ZipArtifactReader.js";
|
import { ZipArtifactReader } from "../artifacts/ZipArtifactReader.js";
|
||||||
import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
|
import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
|
||||||
import {
|
import {
|
||||||
|
NonRegularFileReadError,
|
||||||
openRegularFile,
|
openRegularFile,
|
||||||
RegularFileAdmissionFailure,
|
RegularFileAdmissionFailure,
|
||||||
sameRegularFileState,
|
sameRegularFileState,
|
||||||
|
|
@ -181,16 +182,29 @@ const mapInputFailure = (
|
||||||
{ max_metadata_read_bytes: APK_METADATA_READ_MAX_BYTES },
|
{ max_metadata_read_bytes: APK_METADATA_READ_MAX_BYTES },
|
||||||
{ cause },
|
{ cause },
|
||||||
);
|
);
|
||||||
const message = cause instanceof Error ? cause.message : String(cause);
|
|
||||||
return new AnalysisInputError(operation, { cause }, [
|
return new AnalysisInputError(operation, { cause }, [
|
||||||
{
|
{
|
||||||
path: ["path"],
|
path: ["path"],
|
||||||
reason: "invalid_format",
|
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. */
|
/** Provider-owned storage failures must never be attributed to caller input. */
|
||||||
const mapOwnedIoFailure = (
|
const mapOwnedIoFailure = (
|
||||||
operation: string,
|
operation: string,
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
import { createReadStream } from "node:fs";
|
import { createReadStream } from "node:fs";
|
||||||
import { open, readFile, rm, stat, utimes, writeFile } from "node:fs/promises";
|
import { open, readFile, rm, stat, utimes, writeFile } from "node:fs/promises";
|
||||||
|
import { createServer } from "node:net";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
import { Readable } from "node:stream";
|
import { Readable } from "node:stream";
|
||||||
import { Uint8ArrayReader, Uint8ArrayWriter, ZipWriter } from "@zip.js/zip.js";
|
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<void> => {
|
||||||
|
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<void>((resolve) => socket.listen(socketPath, resolve));
|
||||||
|
onTestFinished(
|
||||||
|
() => new Promise<void>((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)(
|
it.each(["EACCES", "EPERM"] as const)(
|
||||||
"preserves %s at the shared source admission boundary",
|
"preserves %s at the shared source admission boundary",
|
||||||
async (code) => {
|
async (code) => {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue