diff --git a/src/application/BinaryTargetResolver.ts b/src/application/BinaryTargetResolver.ts index 0572d3ae12..7b7f58afb0 100644 --- a/src/application/BinaryTargetResolver.ts +++ b/src/application/BinaryTargetResolver.ts @@ -90,11 +90,15 @@ export const parseBinaryTarget = async ( const hostArchitecture = options.hostArchitecture ?? process.arch; const { targetKind, formatHint, signal, architecture, entryPoint } = options; const candidate = absoluteCallerPath(input, { cwd }); + // Failures before canonicalization concern the selected path itself; later + // failures concern the canonical path opened with symbolic links rejected. + let stage: TargetFilesystemStage = "resolve"; try { throwIfTargetResolutionCancelled(signal); await access(candidate, constants.R_OK); throwIfTargetResolutionCancelled(signal); const canonical = await realpath(candidate); + stage = "read"; // Windows native admission must inspect the caller-selected reparse components. const sourcePath = process.platform === "win32" ? candidate : canonical; throwIfTargetResolutionCancelled(signal); @@ -247,24 +251,39 @@ export const parseBinaryTarget = async ( ); if (cause instanceof BinaryTargetError) return err(cause); if (!(cause instanceof Error)) throw cause; - const failure = classifyBinaryTargetFilesystemFailure(candidate, cause); + const failure = classifyTargetFilesystemFailure(candidate, cause, stage); if (failure === undefined) throw cause; return err(failure); } }; +/** + * `resolve` covers the caller-selected path before canonicalization; `read` + * covers a canonical path opened with symbolic links rejected. + */ +type TargetFilesystemStage = "resolve" | "read"; + /** Classify a filesystem failure while reading a previously selected target. */ export const classifyBinaryTargetFilesystemFailure = ( path: string, cause: unknown, +): BinaryTargetError | undefined => + classifyTargetFilesystemFailure(path, cause, "read"); + +const classifyTargetFilesystemFailure = ( + path: string, + cause: unknown, + stage: TargetFilesystemStage, ): BinaryTargetError | undefined => { if (!(cause instanceof Error)) return undefined; const code = filesystemErrorCode(cause); return code === undefined ? undefined - : new BinaryTargetError(path, targetFailureReason(code, cause.message), { - cause, - }); + : new BinaryTargetError( + path, + targetFailureReason(code, cause.message, stage), + { cause }, + ); }; const filesystemErrorCode = (cause: Error): string | undefined => { @@ -272,7 +291,11 @@ const filesystemErrorCode = (cause: Error): string | undefined => { return typeof code === "string" ? code : undefined; }; -const targetFailureReason = (code: string, detail: string): string => { +const targetFailureReason = ( + code: string, + detail: string, + stage: TargetFilesystemStage, +): string => { switch (code) { case "ENOENT": case "ENOTDIR": @@ -285,7 +308,12 @@ const targetFailureReason = (code: string, detail: string): string => { case "ENOTFILE": return "target is not a regular file"; case "ELOOP": - return `target changed while being opened: ${detail}`; + // A canonical path has no symbolic links left, so a link rejected while + // reading it appeared after resolution. Before that, ELOOP is a loop or + // an over-deep link chain in the selected path itself. + return stage === "resolve" + ? `target path could not be resolved: too many symbolic links or a symbolic link loop: ${detail}` + : `target changed while being opened: ${detail}`; default: return `target could not be read (${code}): ${detail}`; } diff --git a/tests/acceptance/artifactSymlinkLoopCli.test.ts b/tests/acceptance/artifactSymlinkLoopCli.test.ts new file mode 100644 index 0000000000..d44e122f19 --- /dev/null +++ b/tests/acceptance/artifactSymlinkLoopCli.test.ts @@ -0,0 +1,35 @@ +import { readlink, symlink } from "node:fs/promises"; +import { join } from "node:path"; +import { expect } from "vitest"; + +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; +import { cliTest } from "../support/cli/cliFixture.js"; + +cliTest.skipIf(process.platform === "win32").for([ + { command: "inspect-artifact", link: "loop.db", selected: "loop.db" }, + { command: "extract-artifact", link: "tree", selected: join("tree", "file") }, +])( + "$command reports a stable symbolic link loop as a path resolution failure", + async ({ command, link, selected }, { cli }) => { + const root = await createTestTempDirectory("rea-artifact-symlink-loop-"); + await symlink(link, join(root, link)); + const path = join(root, selected); + const result = await cli.run({ + arguments: [command, path, "--json"], + environment: { REA_LOG_LEVEL: "silent" }, + }); + expect(result.exitCode).toBe(1); + const reason = expect.stringMatching( + /^target path could not be resolved: too many symbolic links or a symbolic link loop: ELOOP/u, + ); + expect(result.json).toMatchObject({ + code: "target_unavailable", + category: "unavailable", + retryable: false, + message: expect.stringContaining("symbolic link loop"), + details: { path, reason }, + }); + expect(result.stdout).not.toContain("changed while being opened"); + expect(await readlink(join(root, link))).toBe(link); + }, +);