diff --git a/docs/cli.md b/docs/cli.md index c0f8eefff..cad2255d9 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -286,6 +286,12 @@ reported separately as configured-secret exclusions. Re-import saved graphs whose pattern-based exclusions lack `pattern` before using them in source-to-bundle comparisons. +Entries that change during an import retain an unavailable-entry diagnostic +alongside readable siblings. If enumeration of an already admitted root fails, +the graph keeps its observed children and records the root failure as a +top-level limitation. That partial inventory does not establish absence among +unexamined children. + JavaScript and TypeScript import parsing requires valid UTF-8. Malformed source bytes retain their original hashes and sizes with a decoding diagnostic; REA does not infer module targets from replacement characters. diff --git a/src/application/ReferenceSourceImportEntries.ts b/src/application/ReferenceSourceImportEntries.ts index 84885a215..d60320182 100644 --- a/src/application/ReferenceSourceImportEntries.ts +++ b/src/application/ReferenceSourceImportEntries.ts @@ -419,6 +419,10 @@ export const parseReferenceSourceEntries = ( parseFailures: [], limitations: [...read.limitations], }; + if (read.rootFailure !== undefined) + output.limitations.push( + `Reference root enumeration is incomplete: ${read.rootFailure.message}`, + ); const packageMainInventory = readPackageMains(read); const context: SpecifierContext = { lookupPath: createReferenceSourcePathLookup(filePaths), diff --git a/src/reference/ReferenceSourceReader.test.ts b/src/reference/ReferenceSourceReader.test.ts index 2eef9ec31..8cb73cae9 100644 --- a/src/reference/ReferenceSourceReader.test.ts +++ b/src/reference/ReferenceSourceReader.test.ts @@ -30,6 +30,33 @@ const exclusions = ( ): TraversalState["exclusions"] => items; describe("unobserved internal symlink reconciliation", () => { + it("uses the explicit root failure only when no closer completed directory or exclusion covers the target", () => { + const entries: ReferenceSourceEntry[] = [ + internalLink("unobserved", "unseen/target.txt"), + internalLink("complete", "known/target.txt"), + internalLink("excluded", "omitted/target.txt"), + { status: "read", kind: "directory", path: "known" }, + ]; + + reconcileUnobservedSymlinkTargets( + entries, + exclusions({ path: "omitted", kind: "directory" }), + undefined, + { code: "changed", message: "Reference root changed" }, + ); + + expect(entries[0]).toMatchObject({ + status: "failed", + kind: "symlink", + code: "changed", + message: expect.stringContaining("Reference root changed"), + }); + expect(entries.slice(1, 3)).toEqual([ + internalLink("complete", "known/target.txt"), + internalLink("excluded", "omitted/target.txt"), + ]); + }); + it("retains unreadable targets below the nearest failed directory", () => { const entries: ReferenceSourceEntry[] = [ internalLink("alias.txt", "hidden/target.txt"), diff --git a/src/reference/ReferenceSourceReader.ts b/src/reference/ReferenceSourceReader.ts index 88553bee2..4af7da9de 100644 --- a/src/reference/ReferenceSourceReader.ts +++ b/src/reference/ReferenceSourceReader.ts @@ -9,6 +9,7 @@ import { traverseDirectory } from "./ReferenceSourceReaderEntries.js"; import type { ReferenceSourceReaderOptions, ReferenceSourceEntry, + ReferenceSourceRootFailure, ReferenceSourceRead, ReferenceSourceResult, TraversalState, @@ -78,12 +79,16 @@ const readSource = async ( state.entries, state.exclusions, state.signal, + state.rootFailure, ); state.entries.sort((left, right) => compareUnicodeCodePoints(left.path, right.path), ); const read: ReferenceSourceRead = { root: canonicalRoot, + ...(state.rootFailure === undefined + ? {} + : { rootFailure: state.rootFailure }), entries: state.entries, bytesRead: state.bytesRead, limitations: [PATH_RACE_LIMITATION], @@ -101,6 +106,7 @@ export const reconcileUnobservedSymlinkTargets = ( entries: ReferenceSourceEntry[], exclusions: TraversalState["exclusions"] = [], signal?: AbortSignal, + rootFailure?: ReferenceSourceRootFailure, ): void => { const failedDirectories = new Map( entries.flatMap((entry) => @@ -111,7 +117,11 @@ export const reconcileUnobservedSymlinkTargets = ( : [], ), ); - if (failedDirectories.size === 0 || isAborted(signal)) return; + if ( + (failedDirectories.size === 0 && rootFailure === undefined) || + isAborted(signal) + ) + return; const directoryEntries = entries.filter( (entry) => entry.kind === "directory", ); @@ -140,6 +150,7 @@ export const reconcileUnobservedSymlinkTargets = ( failedDirectories, excludedPaths, signal, + rootFailure, ); if (ancestor === undefined) continue; entries[index] = { @@ -161,7 +172,8 @@ const nearestFailedAncestor = ( >, excludedPaths: ReturnType, signal: AbortSignal | undefined, -): Extract | undefined => { + rootFailure: ReferenceSourceRootFailure | undefined, +): (ReferenceSourceRootFailure & { readonly path: string }) | undefined => { for ( let candidate = posix.dirname(target); candidate !== "." && candidate !== "/"; @@ -175,10 +187,7 @@ const nearestFailedAncestor = ( } if (isAborted(signal)) return undefined; if (excludedPaths.resolve(".") !== undefined) return undefined; - const observedRoot = directoryPaths.resolve("."); - return observedRoot === undefined - ? undefined - : failedDirectories.get(observedRoot); + return rootFailure === undefined ? undefined : { path: ".", ...rootFailure }; }; const traverse = async ( diff --git a/src/reference/ReferenceSourceReaderEntries.ts b/src/reference/ReferenceSourceReaderEntries.ts index 1ca0ce3e2..c7f634168 100644 --- a/src/reference/ReferenceSourceReaderEntries.ts +++ b/src/reference/ReferenceSourceReaderEntries.ts @@ -26,10 +26,29 @@ import type { PendingDirectory, ReferenceSourceEntry, ReferenceSourceExclusionKind, + ReferenceSourceFailureCode, ReferenceSourceResult, TraversalState, } from "./ReferenceSourceReaderTypes.js"; +const recordDirectoryFailure = ( + state: TraversalState, + absolute: string, + code: ReferenceSourceFailureCode, + message: string, +): void => { + if (absolute === state.root) state.rootFailure = { code, message }; + else + state.entries.push( + entryFailure( + pathFromRoot(state.root, absolute), + "directory", + code, + message, + ), + ); +}; + export const traverseDirectory = async ( state: TraversalState, current: PendingDirectory, @@ -42,14 +61,7 @@ export const traverseDirectory = async ( ); if (isAborted(state.signal)) return { ok: false, error: cancelled() }; if (!before.ok) { - state.entries.push( - entryFailure( - pathFromRoot(state.root, current.path), - "directory", - before.code, - before.message, - ), - ); + recordDirectoryFailure(state, current.path, before.code, before.message); return { ok: true, value: undefined }; } const directories: PendingDirectory[] = []; @@ -86,14 +98,7 @@ export const traverseDirectory = async ( } if (cleanup.kind === "failed") { if (result.ok && !isAborted(state.signal) && readFailure !== undefined) - state.entries.push( - entryFailure( - pathFromRoot(state.root, current.path), - "directory", - "io", - readFailure, - ), - ); + recordDirectoryFailure(state, current.path, "io", readFailure); const observation = ArtifactReaderFailure.cleanupObservation( cleanup.cause, current.path, @@ -126,14 +131,7 @@ export const traverseDirectory = async ( } if (isAborted(state.signal)) return { ok: false, error: cancelled() }; if (readFailure !== undefined) { - state.entries.push( - entryFailure( - pathFromRoot(state.root, current.path), - "directory", - "io", - readFailure, - ), - ); + recordDirectoryFailure(state, current.path, "io", readFailure); scheduleObservedDirectories(state, directories); return { ok: true, value: undefined }; } @@ -145,13 +143,11 @@ export const traverseDirectory = async ( ); if (isAborted(state.signal)) return { ok: false, error: cancelled() }; if (!after.ok || !sameFile(before.stats, after.stats)) { - state.entries.push( - entryFailure( - pathFromRoot(state.root, current.path), - "directory", - "changed", - "Directory changed while it was read", - ), + recordDirectoryFailure( + state, + current.path, + after.ok ? "changed" : after.code, + after.ok ? "Directory changed while it was read" : after.message, ); scheduleObservedDirectories(state, directories); return { ok: true, value: undefined }; @@ -356,7 +352,17 @@ const readSymlinkTarget = async ( absolute: string, path: string, ): Promise => { - const rawTarget = await readlink(absolute); + let rawTarget: string; + try { + rawTarget = await readlink(absolute); + } catch (cause: unknown) { + // POSIX readlink reports EINVAL when the observed link became a non-link. + // Keep this classification at the syscall, rather than hiding EINVAL from + // unrelated metadata or target-resolution operations. + if (cause instanceof Error && Reflect.get(cause, "code") === "EINVAL") + return changedSymlink(path); + throw cause; + } const lexicalTarget = resolve(dirname(absolute), rawTarget); if (!isPathWithinRoot(root, lexicalTarget)) return { diff --git a/src/reference/ReferenceSourceReaderTypes.ts b/src/reference/ReferenceSourceReaderTypes.ts index 85d6d7602..a9e3f470b 100644 --- a/src/reference/ReferenceSourceReaderTypes.ts +++ b/src/reference/ReferenceSourceReaderTypes.ts @@ -61,8 +61,15 @@ export type ReferenceSourceEntry = readonly size?: number; }; +/** A failure enumerating an already admitted root, separate from its children. */ +export interface ReferenceSourceRootFailure { + readonly code: ReferenceSourceFailureCode; + readonly message: string; +} + export interface ReferenceSourceRead { readonly root: string; + readonly rootFailure?: ReferenceSourceRootFailure; readonly entries: readonly ReferenceSourceEntry[]; readonly bytesRead: number; /** Node exposes no portable openat traversal; pathname identity is checked around each operation. */ @@ -98,6 +105,7 @@ export type TraversalState = { readonly kind: ReferenceSourceExclusionKind; }>; readonly pending: PendingDirectory[]; + rootFailure?: ReferenceSourceRootFailure; bytesRead: number; }; diff --git a/tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts b/tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts index 3ad4665bf..e471bceaf 100644 --- a/tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts +++ b/tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts @@ -253,10 +253,7 @@ it("preserves a directory read failure when closing that directory also fails", bytes: Buffer.from(bytes), }), ); - expect(result.error.partial?.entries).toContainEqual({ - status: "failed", - kind: "directory", - path: ".", + expect(result.error.partial?.rootFailure).toEqual({ code: "io", message: expect.stringContaining(readFailure.message), }); diff --git a/tests/process-global/referenceSourceSymlinkRace.test.ts b/tests/process-global/referenceSourceSymlinkRace.test.ts new file mode 100644 index 000000000..6229ab873 --- /dev/null +++ b/tests/process-global/referenceSourceSymlinkRace.test.ts @@ -0,0 +1,213 @@ +import { expect, it, vi } from "vitest"; +import type { PathLike } from "node:fs"; + +const race = vi.hoisted(() => ({ + path: "", + beforeReadlink: undefined as (() => Promise) | undefined, + actualErrno: undefined as string | undefined, + directory: "", + afterDirectoryClose: undefined as (() => Promise) | undefined, +})); + +vi.mock("node:fs/promises", async (importOriginal) => { + const filesystem = await importOriginal(); + return { + ...filesystem, + opendir: async (...args: Parameters) => { + const directory = await filesystem.opendir(...args); + if (String(args[0]) === race.directory) { + const close = directory.close.bind(directory); + vi.spyOn(directory, "close").mockImplementation(async () => { + await close(); + const hook = race.afterDirectoryClose; + race.afterDirectoryClose = undefined; + await hook?.(); + }); + } + return directory; + }, + readlink: async ( + path: PathLike, + ...args: Parameters extends [ + PathLike, + ...infer R, + ] + ? R + : never + ) => { + if (String(path) === race.path && race.beforeReadlink !== undefined) { + const beforeReadlink = race.beforeReadlink; + race.beforeReadlink = undefined; + await beforeReadlink(); + } + try { + return await filesystem.readlink(path, ...args); + } catch (cause: unknown) { + if (String(path) === race.path && cause instanceof Error) + race.actualErrno = Reflect.get(cause, "code"); + throw cause; + } + }, + }; +}); + +import { + mkdir, + readFile, + rename, + unlink, + writeFile, + symlink, +} from "node:fs/promises"; +import { join } from "node:path"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; +import { ArtifactResourceScope } from "../../src/artifacts/ArtifactResourceScope.js"; +import { importReferenceSource } from "../../src/application/ReferenceSourceImport.js"; +import { readReferenceSource } from "../../src/reference/ReferenceSourceReader.js"; + +it.skipIf(process.platform === "win32").each(["reader", "import"] as const)( + "keeps sibling observations through %s when a symlink becomes a regular file before readlink", + async (operation) => { + const root = await createTestTempDirectory("rea-reference-symlink-race-"); + await mkdir(join(root, "nested")); + const linkPath = join(root, "nested", "link"); + const siblingPath = join(root, "sibling.txt"); + await symlink("missing-target", linkPath); + await writeFile(siblingPath, "sibling remains observable"); + race.path = linkPath; + race.actualErrno = undefined; + race.beforeReadlink = async () => { + await unlink(linkPath); + await writeFile(linkPath, "replacement regular file"); + }; + + const resources = new ArtifactResourceScope(); + let read: Awaited> | undefined; + let imported: Awaited> | undefined; + let thrown: unknown; + try { + if (operation === "reader") + read = await readReferenceSource(root, resources); + else + imported = await importReferenceSource( + { + root, + caller: "symlink-replacement-test", + policy: { secretPatterns: [] }, + }, + resources, + ); + } catch (cause: unknown) { + thrown = cause; + } finally { + race.path = ""; + race.beforeReadlink = undefined; + await resources.close(); + } + + // The hook establishes scheduling; this assertion verifies the host's real + // readlink syscall reports the replacement as EINVAL. + expect(race.actualErrno).toBe("EINVAL"); + expect(thrown).toBeUndefined(); + expect(read?.ok ?? imported?.ok, JSON.stringify(imported)).toBe(true); + if (imported?.ok) { + expect(imported.value.inventory_state).toBe("partial"); + expect(imported.value.entries).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + path: "nested/link", + kind: "symlink", + target: null, + target_state: "unreadable", + }), + expect.objectContaining({ + path: "sibling.txt", + kind: "file", + content_state: "hashed", + sha256: expect.any(String), + }), + ]), + ); + expect( + imported.value.entries + .find(({ path }) => path === "nested/link") + ?.limitations.join(" "), + ).toContain("Symbolic link changed"); + } + if (!read?.ok) return; + expect(read.value.rootFailure).toBeUndefined(); + expect(read.value.entries.some(({ path }) => path === ".")).toBe(false); + expect(read.value.entries).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + status: "failed", + kind: "symlink", + path: "nested/link", + code: "changed", + }), + expect.objectContaining({ + status: "read", + kind: "file", + path: "sibling.txt", + }), + ]), + ); + }, +); + +it.skipIf(process.platform === "win32").each(["contents", "root"] as const)( + "retains a changed %s as a graph limitation after its directory handle closes", + async (changed) => { + const parent = await createTestTempDirectory("rea-reference-root-race-"); + const root = join(parent, "observed"); + await mkdir(root); + const source = join(root, "source.txt"); + await writeFile(source, "already observed source"); + race.directory = root; + race.afterDirectoryClose = () => + changed === "contents" + ? rename(source, join(root, "renamed.txt")) + : rename(root, join(parent, "moved")); + const resources = new ArtifactResourceScope(); + let imported: Awaited>; + try { + imported = await importReferenceSource( + { + root, + caller: "root-revalidation-test", + policy: { secretPatterns: [] }, + }, + resources, + ); + } finally { + race.directory = ""; + race.afterDirectoryClose = undefined; + await resources.close(); + } + const retainedPath = + changed === "contents" + ? join(root, "renamed.txt") + : join(parent, "moved", "source.txt"); + expect(await readFile(retainedPath, "utf8")).toBe( + "already observed source", + ); + await expect(readFile(source)).rejects.toMatchObject({ code: "ENOENT" }); + expect(imported.ok, JSON.stringify(imported)).toBe(true); + if (!imported.ok) return; + expect(imported.value.inventory_state).toBe("partial"); + expect(imported.value.limitations).toContainEqual( + expect.stringContaining( + changed === "contents" + ? "Reference root enumeration is incomplete: Reference root changed" + : "Reference root enumeration is incomplete: Directory identity could not be verified: ENOENT", + ), + ); + expect(imported.value.entries).toEqual([ + expect.objectContaining({ + path: "source.txt", + kind: "file", + content_state: "hashed", + }), + ]); + }, +);