fix(artifacts): bind directory reads to captured file state

Capture exact filesystem identity and nanosecond timestamps, validate descriptor/path state before and after streaming, and enforce the captured byte count. Bound growing files to their captured size plus one byte using precise positional reads; keep unrepresentable display sizes explicitly unknown.

Reuse the captured-state comparison around Apple assetutil and retain existing reader cleanup ownership. Validation: focused reader and affected consumer suites, full static/architecture checks, and a compiled directory-read probe on Bun 1.3.14 passed. Includes same-size mutation, growth, truncation, cancellation, FIFO replacement, cleanup retry, ZIP identity forgery, ASAR and Mach-O coverage.
This commit is contained in:
morluto
2026-10-12 04:45:17 +08:00
parent a82e37c806
commit 918713af37
7 changed files with 238 additions and 63 deletions
+31 -3
View File
@@ -26,11 +26,35 @@ export interface ArtifactEntry {
readonly adapterKey: string;
/** Filesystem identity captured during traversal, when supplied by an adapter. */
readonly sourceIdentity?: {
readonly device: number;
readonly inode: number;
readonly device: bigint;
readonly inode: bigint;
readonly size: bigint;
readonly mode: bigint;
readonly mtimeNs: bigint;
readonly ctimeNs: bigint;
};
}
/** Compare captured filesystem identity and mutable metadata at a read boundary. */
export const sameArtifactSourceIdentity = (
expected: ArtifactEntry["sourceIdentity"],
observed: {
readonly dev: bigint;
readonly ino: bigint;
readonly size: bigint;
readonly mode: bigint;
readonly mtimeNs: bigint;
readonly ctimeNs: bigint;
},
): boolean =>
expected !== undefined &&
observed.dev === expected.device &&
observed.ino === expected.inode &&
observed.size === expected.size &&
observed.mode === expected.mode &&
observed.mtimeNs === expected.mtimeNs &&
observed.ctimeNs === expected.ctimeNs;
/** Snapshot mutable entry metadata before the producer later validates open(). */
export const copyArtifactEntry = (entry: ArtifactEntry): ArtifactEntry => ({
...entry,
@@ -60,7 +84,11 @@ export const sameArtifactEntry = (
(limitation, index) => limitation === produced.limitations[index],
) &&
entry.sourceIdentity?.device === produced.sourceIdentity?.device &&
entry.sourceIdentity?.inode === produced.sourceIdentity?.inode;
entry.sourceIdentity?.inode === produced.sourceIdentity?.inode &&
entry.sourceIdentity?.size === produced.sourceIdentity?.size &&
entry.sourceIdentity?.mode === produced.sourceIdentity?.mode &&
entry.sourceIdentity?.mtimeNs === produced.sourceIdentity?.mtimeNs &&
entry.sourceIdentity?.ctimeNs === produced.sourceIdentity?.ctimeNs;
/** Read-only adapter over one directory, archive, or virtual container. */
export interface ArtifactReader {
+76 -20
View File
@@ -1,4 +1,4 @@
import { constants, type Stats } from "node:fs";
import { constants, type BigIntStats } from "node:fs";
import {
lstat,
open,
@@ -11,6 +11,7 @@ import { Readable } from "node:stream";
import {
ArtifactReaderFailure,
sameArtifactSourceIdentity,
type ArtifactEntry,
type ArtifactReader,
} from "./ArtifactReader.js";
@@ -95,9 +96,11 @@ export class DirectoryArtifactReader implements ArtifactReader {
if (child === null) break;
abortIfNeeded(signal);
const absolute = join(directory, child.name);
const metadata = await lstat(absolute).catch((cause: unknown) => {
throw directoryIoFailure("inspect entry", absolute, cause);
});
const metadata = await lstat(absolute, { bigint: true }).catch(
(cause: unknown) => {
throw directoryIoFailure("inspect entry", absolute, cause);
},
);
const path = relative(root, absolute).split(sep).join("/");
const entry = artifactEntry(path, absolute, metadata);
if (entry === undefined) continue;
@@ -158,18 +161,18 @@ export class DirectoryArtifactReader implements ArtifactReader {
const owner = new OwnedFileHandle(fileHandle);
this.#files.set(owner, entry.adapterKey);
try {
const observed = await owner.handle.stat().catch((cause: unknown) => {
throw directoryIoFailure(
"inspect opened entry",
entry.adapterKey,
cause,
);
});
const observed = await owner.handle
.stat({ bigint: true })
.catch((cause: unknown) => {
throw directoryIoFailure(
"inspect opened entry",
entry.adapterKey,
cause,
);
});
if (
!observed.isFile() ||
entry.sourceIdentity === undefined ||
observed.dev !== entry.sourceIdentity.device ||
observed.ino !== entry.sourceIdentity.inode
!sameArtifactSourceIdentity(entry.sourceIdentity, observed)
) {
throw new ArtifactReaderFailure(
"integrity",
@@ -178,7 +181,7 @@ export class DirectoryArtifactReader implements ArtifactReader {
}
abortIfNeeded(signal);
const source = Readable.from(
mapFileReadCancellation(owner.handle, signal),
mapStableFileRead(owner.handle, entry, signal),
{ objectMode: false },
);
source.once("close", () => {
@@ -256,7 +259,7 @@ export class DirectoryArtifactReader implements ArtifactReader {
const artifactEntry = (
path: string,
absolute: string,
metadata: Stats,
metadata: BigIntStats,
): ArtifactEntry | undefined => {
const kind = metadata.isSymbolicLink()
? "symlink"
@@ -269,7 +272,7 @@ const artifactEntry = (
return {
path,
kind,
declaredSize: kind === "file" ? metadata.size : null,
declaredSize: kind === "file" ? safeArtifactSize(metadata.size) : null,
compressedSize: null,
executable: hostExecutableMarking(absolute, metadata),
encrypted: false,
@@ -283,12 +286,24 @@ const artifactEntry = (
adapterKey: absolute,
...(kind === "file"
? {
sourceIdentity: { device: metadata.dev, inode: metadata.ino },
sourceIdentity: {
device: metadata.dev,
inode: metadata.ino,
size: metadata.size,
mode: metadata.mode,
mtimeNs: metadata.mtimeNs,
ctimeNs: metadata.ctimeNs,
},
}
: {}),
};
};
const safeArtifactSize = (size: bigint): number | null => {
if (size < 0n || size > BigInt(Number.MAX_SAFE_INTEGER)) return null;
return Number(size);
};
const directoryIoFailure = (
operation: string,
path: string,
@@ -313,16 +328,45 @@ const directoryIoFailure = (
const failureMessage = (cause: unknown): string =>
cause instanceof Error ? cause.message : String(cause);
const mapFileReadCancellation = async function* (
const mapStableFileRead = async function* (
handle: FileHandle,
entry: ArtifactEntry,
signal?: AbortSignal,
): AsyncGenerator<Buffer> {
let bytes = 0n;
try {
for await (const chunk of readFileHandleChunks(handle, { signal })) {
const expectedBytes = entry.sourceIdentity?.size;
if (expectedBytes === undefined)
throw new ArtifactReaderFailure(
"integrity",
`Directory entry has no captured file state: ${entry.path}`,
);
for await (const chunk of readFileHandleChunks(handle, {
// Include one extra byte so a growing file cannot make this read unbounded.
end: expectedBytes,
signal,
})) {
abortIfNeeded(signal);
bytes += BigInt(chunk.length);
yield chunk;
}
abortIfNeeded(signal);
const [opened, current] = await Promise.all([
handle.stat({ bigint: true }),
lstat(entry.adapterKey, { bigint: true }),
]);
abortIfNeeded(signal);
if (
!opened.isFile() ||
!current.isFile() ||
!sameArtifactSourceIdentity(entry.sourceIdentity, opened) ||
!sameArtifactSourceIdentity(entry.sourceIdentity, current) ||
bytes !== expectedBytes
)
throw new ArtifactReaderFailure(
"integrity",
`Directory entry changed during read: ${entry.path}`,
);
} catch (cause: unknown) {
if (signal?.aborted === true)
throw new ArtifactReaderFailure(
@@ -330,6 +374,18 @@ const mapFileReadCancellation = async function* (
"Directory artifact read cancelled",
{ cause },
);
if (
cause instanceof Error &&
"code" in cause &&
["ENOENT", "ENOTDIR", "ELOOP"].includes(String(cause.code))
)
throw new ArtifactReaderFailure(
"integrity",
`Directory entry changed during read: ${entry.path}`,
{ cause },
);
if (!(cause instanceof ArtifactReaderFailure))
throw directoryIoFailure("validate read entry", entry.adapterKey, cause);
throw cause;
}
};
@@ -1,7 +1,10 @@
import { createHash } from "node:crypto";
import { lstat } from "node:fs/promises";
import { DirectoryArtifactReader } from "../DirectoryArtifactReader.js";
import { ArtifactReaderFailure } from "../ArtifactReader.js";
import {
ArtifactReaderFailure,
sameArtifactSourceIdentity,
} from "../ArtifactReader.js";
import { analyzeInterfaceBuilderBundle } from "./InterfaceBuilderAnalysis.js";
import {
appleAssetCatalogInputSchema,
@@ -136,22 +139,20 @@ const inspectAssetCatalogEntry = async (input: {
}) => {
const { entry, reader, signal } = input;
const sha256 = await digestBounded(reader, entry, signal);
const before = await lstat(entry.adapterKey);
const before = await lstat(entry.adapterKey, { bigint: true });
if (
entry.sourceIdentity === undefined ||
before.dev !== entry.sourceIdentity.device ||
before.ino !== entry.sourceIdentity.inode
!before.isFile() ||
!sameArtifactSourceIdentity(entry.sourceIdentity, before)
)
throw new ArtifactReaderFailure(
"integrity",
`Asset catalog changed before inspection: ${entry.path}`,
);
const output = await input.runAssetUtil(entry.adapterKey, signal);
const after = await lstat(entry.adapterKey);
const after = await lstat(entry.adapterKey, { bigint: true });
if (
before.dev !== after.dev ||
before.ino !== after.ino ||
before.size !== after.size ||
!after.isFile() ||
!sameArtifactSourceIdentity(entry.sourceIdentity, after) ||
(await digestBounded(reader, entry, signal)) !== sha256
)
throw new ArtifactReaderFailure(
+5 -3
View File
@@ -1,4 +1,4 @@
import type { Stats } from "node:fs";
import type { BigIntStats, Stats } from "node:fs";
import { extname } from "node:path";
/** Extensions Windows launches directly through CreateProcess or cmd.exe. */
@@ -13,10 +13,12 @@ const windowsLaunchableExtensions = new Set([".bat", ".cmd", ".com", ".exe"]);
*/
export const hostExecutableMarking = (
path: string,
metadata: Pick<Stats, "mode" | "isFile">,
metadata: Pick<Stats | BigIntStats, "mode" | "isFile">,
platform: NodeJS.Platform = process.platform,
): boolean =>
platform === "win32"
? metadata.isFile() &&
windowsLaunchableExtensions.has(extname(path).toLowerCase())
: (metadata.mode & 0o111) !== 0;
: typeof metadata.mode === "bigint"
? (metadata.mode & 0o111n) !== 0n
: (metadata.mode & 0o111) !== 0;
+19 -10
View File
@@ -5,26 +5,30 @@ import { Readable } from "node:stream";
export const readFileHandleChunks = (
handle: Pick<FileHandle, "read">,
options: {
readonly start?: number;
readonly end?: number;
readonly start?: number | bigint;
readonly end?: number | bigint;
readonly signal?: AbortSignal | undefined;
} = {},
): Readable => {
const start = options.start ?? 0;
const end = options.end ?? Infinity;
const start = fileOffset(options.start ?? 0);
const unlimited = options.end === undefined || options.end === Infinity;
const end = unlimited ? undefined : fileOffset(options.end);
if (
!Number.isSafeInteger(start) ||
start < 0 ||
(end !== Infinity && (!Number.isSafeInteger(end) || end < start))
start === undefined ||
(!unlimited && end === undefined) ||
(end !== undefined && end < start)
)
throw new RangeError("Invalid file read range");
return Readable.from(
(async function* () {
let position = start;
while (position <= end) {
while (end === undefined || position <= end) {
options.signal?.throwIfAborted();
const remaining = end === undefined ? undefined : end - position + 1n;
const chunk = Buffer.allocUnsafe(
Math.min(64 * 1024, end - position + 1),
remaining === undefined || remaining > 64n * 1024n
? 64 * 1024
: Number(remaining),
);
const { bytesRead } = await handle.read(
chunk,
@@ -34,7 +38,7 @@ export const readFileHandleChunks = (
);
options.signal?.throwIfAborted();
if (bytesRead === 0) return;
position += bytesRead;
position += BigInt(bytesRead);
yield chunk.subarray(0, bytesRead);
}
})(),
@@ -44,3 +48,8 @@ export const readFileHandleChunks = (
},
);
};
const fileOffset = (value: number | bigint): bigint | undefined => {
if (typeof value === "bigint") return value >= 0n ? value : undefined;
return Number.isSafeInteger(value) && value >= 0 ? BigInt(value) : undefined;
};
@@ -61,6 +61,19 @@ it("binds ZIP opens to the emitted entry metadata", async () => {
await expect(
reader.open({ ...second, declaredSize: first.declaredSize }),
).rejects.toMatchObject({ reason: "integrity" });
await expect(
reader.open({
...second,
sourceIdentity: {
device: 1n,
inode: 2n,
size: 3n,
mode: 4n,
mtimeNs: 5n,
ctimeNs: 6n,
},
}),
).rejects.toMatchObject({ reason: "integrity" });
const originalKey = second.adapterKey;
Reflect.set(second, "adapterKey", first.adapterKey);
await expect(reader.open(second)).rejects.toMatchObject({
@@ -1,4 +1,4 @@
import { lstat, open, rename, writeFile } from "node:fs/promises";
import { lstat, open, rename, rm, utimes, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { buffer } from "node:stream/consumers";
@@ -29,27 +29,26 @@ describe("DirectoryArtifactReader.open", () => {
const { path, reader } = await fixture();
try {
const entry = await sampleEntry(reader);
const metadata = await lstat(path);
const metadata = await lstat(path, { bigint: true });
const handle = await open(path, "r");
try {
const observed = await handle.stat();
const observed = await handle.stat({ bigint: true });
expect(entry.sourceIdentity).toEqual({
device: metadata.dev,
inode: metadata.ino,
size: metadata.size,
mode: metadata.mode,
mtimeNs: metadata.mtimeNs,
ctimeNs: metadata.ctimeNs,
});
expect({ device: observed.dev, inode: observed.ino }).toEqual(
entry.sourceIdentity,
);
console.info(
JSON.stringify({
platform: process.platform,
node: process.version,
pathDevice: metadata.dev,
handleDevice: observed.dev,
pathInode: metadata.ino,
handleInode: observed.ino,
}),
);
expect({
device: observed.dev,
inode: observed.ino,
size: observed.size,
mode: observed.mode,
mtimeNs: observed.mtimeNs,
ctimeNs: observed.ctimeNs,
}).toEqual(entry.sourceIdentity);
} finally {
await handle.close();
}
@@ -75,6 +74,73 @@ describe("DirectoryArtifactReader.open", () => {
}
});
it.each(["changed in place with a new size\n", "TEST PAYLOAD\n"])(
"rejects an in-place file change after enumeration: %s",
async (replacement) => {
const { path, reader } = await fixture();
try {
const entry = await sampleEntry(reader);
const before = await lstat(path);
await writeFile(path, replacement);
await utimes(path, before.atime, new Date(before.mtimeMs + 1000));
const after = await lstat(path);
expect(after.ino).toBe(before.ino);
await expect(reader.open(entry)).rejects.toMatchObject({
reason: "integrity",
message: expect.stringContaining("changed before open"),
});
} finally {
await reader.close();
}
},
);
it.each(["truncate", "grow"])(
"rejects a same-file %s between streamed chunks within the captured byte bound",
async (mutation) => {
const root = await createTestTempDirectory("rea-dir-reader-mutate-");
const path = join(root, "sample.txt");
const originalBytes = 1024 * 1024;
await writeFile(path, Buffer.alloc(originalBytes, 0x61));
const reader = new DirectoryArtifactReader(root);
try {
const entry = await sampleEntry(reader);
const stream = await reader.open(entry);
const iterator = stream[Symbol.asyncIterator]();
const first = await iterator.next();
expect(first.done).toBe(false);
let observedBytes =
first.value === undefined ? 0 : Buffer.byteLength(first.value);
await writeFile(
path,
mutation === "truncate"
? "short replacement\n"
: Buffer.alloc(2 * originalBytes, 0x62),
);
let failure: unknown;
try {
for (;;) {
const next = await iterator.next();
if (next.done) break;
observedBytes += Buffer.byteLength(next.value);
}
} catch (cause: unknown) {
failure = cause;
}
expect(failure).toMatchObject({
reason: "integrity",
message: expect.stringContaining("changed during read"),
});
expect(observedBytes).toBeLessThanOrEqual(originalBytes + 1);
} finally {
await reader.close();
await rm(root, { recursive: true, force: true });
}
},
);
});
describe("DirectoryArtifactReader input identity", () => {
it.each(["zero", "another"] as const)(
"requires the enumerated %s device to match the opened file",
async (kind) => {
@@ -84,13 +150,13 @@ describe("DirectoryArtifactReader.open", () => {
const identity = entry.sourceIdentity;
if (identity === undefined)
throw new Error("Fixture omitted source identity");
if (kind === "zero" && identity.device === 0) {
if (kind === "zero" && identity.device === 0n) {
expect(
(await buffer(await reader.open(entry))).toString("utf8"),
).toBe("test payload\n");
return;
}
const device = kind === "zero" ? 0 : identity.device === 1 ? 2 : 1;
const device = kind === "zero" ? 0n : identity.device === 1n ? 2n : 1n;
await expect(
reader.open({ ...entry, sourceIdentity: { ...identity, device } }),
).rejects.toMatchObject({ reason: "integrity" });