mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(reference)!: preserve symlink races and root failure facts
Classify replacement EINVAL at the readlink syscall so the reader and importer retain sibling observations. Give an admitted root's enumeration failure its own typed field, use it for unresolved symlink coverage, and project it as a graph limitation instead of an invalid dot-path child entry. Keep admission, cancellation and cleanup failures distinct. Closes #5135 Closes #5136 BREAKING CHANGE: Partial reference-source reader results report failed root enumeration in rootFailure instead of a failed directory child at path ".". Historical source graphs preserve the root failure as a top-level limitation.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -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<typeof createReferenceSourcePathLookup>,
|
||||
signal: AbortSignal | undefined,
|
||||
): Extract<ReferenceSourceEntry, { status: "failed" }> | 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 (
|
||||
|
||||
@@ -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<ReferenceSourceEntry> => {
|
||||
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 {
|
||||
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
|
||||
@@ -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),
|
||||
});
|
||||
|
||||
@@ -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<void>) | undefined,
|
||||
actualErrno: undefined as string | undefined,
|
||||
directory: "",
|
||||
afterDirectoryClose: undefined as (() => Promise<void>) | undefined,
|
||||
}));
|
||||
|
||||
vi.mock("node:fs/promises", async (importOriginal) => {
|
||||
const filesystem = await importOriginal<typeof import("node:fs/promises")>();
|
||||
return {
|
||||
...filesystem,
|
||||
opendir: async (...args: Parameters<typeof filesystem.opendir>) => {
|
||||
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<typeof filesystem.readlink> 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<ReturnType<typeof readReferenceSource>> | undefined;
|
||||
let imported: Awaited<ReturnType<typeof importReferenceSource>> | 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<ReturnType<typeof importReferenceSource>>;
|
||||
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",
|
||||
}),
|
||||
]);
|
||||
},
|
||||
);
|
||||
Reference in New Issue
Block a user