From 3ca1cc546607d18df2d9fa5ce9bcc31cc0ed0ef7 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 27 Oct 2026 03:18:56 +0800 Subject: [PATCH] fix(filesystem): complete plist and limits probes Read bounded plist format probes and Linux resource-limit reports through positive short reads before classifying their contents. Preserve explicit EOF, report capacity, physical failures and descriptor cleanup ownership. Validated 66 focused tests and check:fast; nine new fault-injected cases fail against the previous implementation. Linux procfs was not verified on a real Linux host. --- src/native/NativePlistSource.ts | 13 +- src/process/LinuxResourceLimits.test.ts | 101 +++++++++++- src/process/LinuxResourceLimits.ts | 12 +- tests/process-global/shortFileReads.test.ts | 169 ++++++++++++++++++++ 4 files changed, 281 insertions(+), 14 deletions(-) diff --git a/src/native/NativePlistSource.ts b/src/native/NativePlistSource.ts index f7609d9db7..fbdb628a1a 100644 --- a/src/native/NativePlistSource.ts +++ b/src/native/NativePlistSource.ts @@ -1,4 +1,5 @@ import { absoluteCallerPath } from "../filesystem/callerPath.js"; +import { readFileHandleRange } from "../filesystem/readFileHandleRange.js"; import { stat } from "node:fs/promises"; import { basename, dirname, isAbsolute, resolve } from "node:path"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; @@ -356,17 +357,19 @@ export const plistSourceFormat = async ( if (/\bJSON\b/iu.test(description)) return ok("json"); if (!/\btext\b/iu.test(description)) return ok("unknown"); return readPlistFile(input, async (file) => { - const head = Buffer.alloc(PLIST_MARKUP_PROBE_BYTES); - const { bytesRead } = await file.read(head, 0, head.length, 0); + const head = await readFileHandleRange( + file, + 0, + PLIST_MARKUP_PROBE_BYTES, + () => assertArtifactReadActive(input.signal), + ); const encoding = head[0] === 0xff && head[1] === 0xfe ? "utf-16le" : head[0] === 0xfe && head[1] === 0xff ? "utf-16be" : "utf-8"; - const text = new TextDecoder(encoding) - .decode(head.subarray(0, bytesRead)) - .trimStart(); + const text = new TextDecoder(encoding).decode(head).trimStart(); return /^<(?:[!?]|(?:plist|dict|array|string|integer|real|data|date|true|false)[\s/>])/u.test( text, ) diff --git a/src/process/LinuxResourceLimits.test.ts b/src/process/LinuxResourceLimits.test.ts index cc09974fb6..6cf5f5787d 100644 --- a/src/process/LinuxResourceLimits.test.ts +++ b/src/process/LinuxResourceLimits.test.ts @@ -1,5 +1,5 @@ import { expect, it, onTestFinished, vi } from "vitest"; -import { writeFile } from "node:fs/promises"; +import { writeFile, type FileHandle } from "node:fs/promises"; import { join } from "node:path"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; import { ArtifactFileCleanupFailure } from "../artifacts/ArtifactFileOperation.js"; @@ -15,7 +15,16 @@ import { readLinuxResourceLimits, } from "./LinuxResourceLimits.js"; -const kernel = vi.hoisted(() => ({ path: "", denied: false, opens: 0 })); +const kernel = vi.hoisted(() => ({ + path: "", + denied: false, + opens: 0, + maximumRead: Number.POSITIVE_INFINITY, + readFailure: undefined as Error | undefined, + failAt: Number.POSITIVE_INFINITY, + reads: [] as { position: number; length: number }[], + handles: [] as FileHandle[], +})); vi.mock("node:fs/promises", async (importOriginal) => { const filesystem = await importOriginal(); return { @@ -25,6 +34,21 @@ vi.mock("node:fs/promises", async (importOriginal) => { return filesystem.open(...args); const file = await filesystem.open(kernel.path, args[1], args[2]); kernel.opens += 1; + kernel.handles.push(file); + const read = file.read.bind(file); + file.read = (async (...readArgs: unknown[]) => { + const [buffer, offset, length, position] = readArgs; + if (typeof length !== "number" || typeof position !== "number") + throw new Error("Expected positional limits report read"); + kernel.reads.push({ position, length }); + if (position >= kernel.failAt) throw kernel.readFailure; + return Reflect.apply(read, file, [ + buffer, + offset, + Math.min(length, kernel.maximumRead), + position, + ]); + }) as FileHandle["read"]; const close = file.close.bind(file); // Keep fixture ownership even when an assertion interrupts retry checks. onTestFinished(close); @@ -206,3 +230,76 @@ it("readmits analysis and resolves the original cleanup error after kernel descr expect(first.error.cleanupIncomplete).toBe(false); if (!blocked.ok) expect(blocked.error.cleanupIncomplete).toBe(false); }); + +const kernelReportFixture = async (text: string): Promise => { + const directory = await createTestTempDirectory("rea-kernel-short-read-"); + kernel.path = join(directory, "limits"); + kernel.reads = []; + kernel.handles = []; + onTestFinished(() => { + kernel.path = ""; + kernel.maximumRead = Number.POSITIVE_INFINITY; + kernel.readFailure = undefined; + kernel.failAt = Number.POSITIVE_INFINITY; + kernel.reads = []; + kernel.handles = []; + }); + await writeFile(kernel.path, text); +}; + +it("clamps all inherited ceilings after positive short report reads", async () => { + const text = report("268435456 536870912", "0 1048576", "10 20"); + await kernelReportFixture(text); + kernel.maximumRead = 7; + expect(await readLinuxResourceLimits(configured)).toEqual({ + address_space_bytes: 268435456, + file_bytes: 0, + cpu_seconds: 10, + }); + expect(kernel.reads.at(-1)?.position).toBe(Buffer.byteLength(text)); + expect(kernel.reads.length).toBeGreaterThan(2); + expect(kernel.handles.every((handle) => handle.fd === -1)).toBe(true); +}); + +it.each([16 * 1024, 16 * 1024 + 1])( + "rejects a %i-byte report even when its initial short prefix contains all required limits", + async (bytes) => { + const prefix = report("unlimited unlimited"); + await kernelReportFixture(prefix.padEnd(bytes, " ")); + kernel.maximumRead = Buffer.byteLength(prefix); + await expect(readLinuxResourceLimits(configured)).rejects.toThrow( + "Inherited Linux limits exceed the report budget.", + ); + expect(kernel.reads.length).toBeGreaterThan(1); + expect( + kernel.reads.every( + ({ position, length }) => position + length <= 16 * 1024, + ), + ).toBe(true); + expect(kernel.handles.every((handle) => handle.fd === -1)).toBe(true); + }, +); + +it("accepts a report below the fixed budget after reading through explicit EOF", async () => { + await kernelReportFixture( + report("unlimited unlimited").padEnd(16 * 1024 - 1, " "), + ); + kernel.maximumRead = 127; + expect(await readLinuxResourceLimits(configured)).toEqual(configured); + expect(kernel.reads.at(-1)).toEqual({ position: 16 * 1024 - 1, length: 1 }); + expect(kernel.handles.every((handle) => handle.fd === -1)).toBe(true); +}); + +it("preserves a later I/O failure instead of accepting a complete-looking short prefix", async () => { + const prefix = report("unlimited unlimited"); + await kernelReportFixture(prefix + "unexamined suffix"); + kernel.maximumRead = Buffer.byteLength(prefix); + kernel.failAt = Buffer.byteLength(prefix); + const cause = Object.assign(new Error("kernel report read failed"), { + code: "EIO", + }); + kernel.readFailure = cause; + await expect(readLinuxResourceLimits(configured)).rejects.toBe(cause); + expect(kernel.reads).toHaveLength(2); + expect(kernel.handles.every((handle) => handle.fd === -1)).toBe(true); +}); diff --git a/src/process/LinuxResourceLimits.ts b/src/process/LinuxResourceLimits.ts index 12da2b80cf..d05a82d0fc 100644 --- a/src/process/LinuxResourceLimits.ts +++ b/src/process/LinuxResourceLimits.ts @@ -1,4 +1,5 @@ import { open } from "node:fs/promises"; +import { readFileHandleRange } from "../filesystem/readFileHandleRange.js"; import { withArtifactFileHandle } from "../artifacts/ArtifactFileOperation.js"; import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; @@ -82,14 +83,11 @@ export const readLinuxResourceLimits = async ( resourceScope, undefined, async (handle) => { - const buffer = Buffer.alloc(16 * 1024); - const { bytesRead } = await handle.read(buffer, 0, buffer.length, 0); - if (bytesRead === buffer.length) + const reportBudget = 16 * 1024; + const buffer = await readFileHandleRange(handle, 0, reportBudget); + if (buffer.length === reportBudget) throw new Error("Inherited Linux limits exceed the report budget."); - return clampLinuxResourceLimits( - configured, - buffer.subarray(0, bytesRead).toString("utf8"), - ); + return clampLinuxResourceLimits(configured, buffer.toString("utf8")); }, ); }; diff --git a/tests/process-global/shortFileReads.test.ts b/tests/process-global/shortFileReads.test.ts index 89c3c328a9..e392cd8e2d 100644 --- a/tests/process-global/shortFileReads.test.ts +++ b/tests/process-global/shortFileReads.test.ts @@ -13,6 +13,7 @@ const fault = vi.hoisted(() => ({ | ((position: number, length: number) => void) | undefined, handles: [] as FileHandle[], + closeFailure: undefined as Error | undefined, })); vi.mock("node:fs/promises", async (importOriginal) => { const actual = await importOriginal(); @@ -22,6 +23,11 @@ vi.mock("node:fs/promises", async (importOriginal) => { const handle = await actual.open(...args); if (String(args[0]) !== fault.path) return handle; fault.handles.push(handle); + const close = handle.close.bind(handle); + handle.close = async () => { + if (fault.closeFailure !== undefined) throw fault.closeFailure; + await close(); + }; const read = handle.read.bind(handle); handle.read = (async (...args: unknown[]) => { const [buffer, offset, length, position] = args; @@ -68,6 +74,167 @@ import { OBSIDIAN_FUSE_BYTES, } from "../fixtures/electron/fuseImage.js"; import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; +import { NativeMacOSProvider } from "../../src/native/NativeMacOSProvider.js"; +import { + NativeFixtureRunner, + nativeMachoTarget, +} from "../fixtures/nativeCommands.js"; +import { inspectPlistSchema } from "../../src/domain/native/nativeInspection.js"; +import { plistSourceFormat } from "../../src/native/NativePlistSource.js"; + +const plistXml = + "Count9007199254740993DataAAE="; +const nativePlistFixture = async ( + bytes: Buffer, + description = "ASCII text", +) => { + const root = await createTestTempDirectory("rea-plist-short-reads-"); + const path = join(root, "selected.plist"); + await writeFile(path, bytes); + const commands: string[][] = []; + const runner = new NativeFixtureRunner({ + file: description, + plutil: "valid", + }); + const client = new NativeMacOSProvider( + {}, + { + run: async (tool, args, options) => { + commands.push([tool, ...args]); + if (tool === "plutil" && args[0] !== "-lint") + throw new Error("Unexpected conversion of directly readable XML"); + return runner.run(tool, args, options); + }, + }, + "darwin", + ).createClient(nativeMachoTarget(join(root, "target"))); + return { path, commands, client }; +}; + +it.each([ + ["UTF8", Buffer.from(plistXml)], + ["UTF16LE", Buffer.from("\ufeff" + plistXml, "utf16le")], + ["UTF16BE", Buffer.from("\ufeff" + plistXml, "utf16le").swap16()], +] as const)( + "preserves %s plist source facts through short reads without conversion", + async (_name, bytes) => { + const { path, commands, client } = await nativePlistFixture(bytes); + fault.path = path; + fault.maximum = 1; + try { + const result = await client.execute("inspect_plist", { path }); + if (!result.ok) throw result.error; + expect(inspectPlistSchema.parse(result.value.result)).toMatchObject({ + format: "xml", + source_path: path, + value: { + Count: { $plist_type: "integer", decimal: "9007199254740993" }, + Data: { $plist_type: "data", base64: "AAE=" }, + }, + }); + expect(commands).toEqual([ + ["file", "-b", path], + ["plutil", "-lint", "--", path], + ]); + expect(fault.reads).toBeGreaterThan(bytes.length); + expect(fault.handles.every((handle) => handle.fd === -1)).toBe(true); + } finally { + await client.close(); + } + }, +); + +it("treats true EOF in an incomplete plist markup probe as unknown", async () => { + const { path, client } = await nativePlistFixture(Buffer.from("<")); + const resources = new ArtifactResourceScope(); + fault.path = path; + fault.maximum = 1; + try { + expect( + await plistSourceFormat("ASCII text", { + selection: { path, origin: "selected" }, + resources, + signal: undefined, + }), + ).toEqual({ ok: true, value: "unknown" }); + expect(fault.reads).toBe(2); + expect(fault.handles.every((handle) => handle.fd === -1)).toBe(true); + } finally { + await resources.close(); + await client.close(); + } +}); + +it("preserves a native plist probe read denial over a simultaneous abort", async () => { + const { path, client } = await nativePlistFixture(Buffer.from(plistXml)); + const controller = new AbortController(); + const physical = Object.assign(new Error("controlled plist read denial"), { + code: "EACCES", + }); + fault.path = path; + fault.maximum = 1; + fault.beforeRead = () => { + if (fault.reads !== 2) return; + controller.abort("simultaneous abort"); + throw physical; + }; + try { + const result = await client.execute( + "inspect_plist", + { path }, + { signal: controller.signal }, + ); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected native plist read denial"); + expect(result.error).not.toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toBe(physical); + expect(fault.reads).toBe(2); + expect(fault.handles.every((handle) => handle.fd === -1)).toBe(true); + } finally { + await client.close(); + } +}); + +it.each([false, true])( + "preserves plist probe cancellation and descriptor cleanup (closeFails=%s)", + async (closeFails) => { + const { path, client } = await nativePlistFixture(Buffer.from(plistXml)); + const controller = new AbortController(); + const reason = { requested: "continued plist probe" }; + fault.path = path; + fault.maximum = 1; + fault.onRead = () => { + if (fault.reads !== 2) return; + controller.abort(reason); + if (closeFails) + fault.closeFailure = new Error("controlled plist close failure"); + }; + try { + const result = await client.execute( + "inspect_plist", + { path }, + { signal: controller.signal }, + ); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected plist probe cancellation"); + const primary = closeFails ? result.error.cleanupPrimary : result.error; + expect(primary).toBeInstanceOf(AnalysisCancelledError); + expect(primary?.cause).toBe(reason); + expect(fault.reads).toBe(2); + if (closeFails) { + expect(result.error.cleanupIncomplete).toBe(true); + expect(fault.handles.at(-1)?.fd).toBeGreaterThanOrEqual(0); + fault.closeFailure = undefined; + expect(await client.close()).toEqual({ ok: true, value: null }); + expect(result.error.cleanupIncomplete).toBe(false); + } + expect(fault.handles.every((handle) => handle.fd === -1)).toBe(true); + } finally { + fault.closeFailure = undefined; + await client.close(); + } + }, +); import { parseBinaryTarget } from "../../src/application/BinaryTargetResolver.js"; import { elf, pe, dosMz } from "../../src/domain/binaryTarget.fixture.js"; @@ -125,8 +292,10 @@ beforeEach(() => { fault.onRead = undefined; fault.beforeRead = undefined; fault.handles = []; + fault.closeFailure = undefined; }); afterEach(async () => { + fault.closeFailure = undefined; fault.onRead = undefined; fault.beforeRead = undefined; for (const handle of fault.handles)