diff --git a/docs/cli.md b/docs/cli.md index ebb640a600..864ddecfbb 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -837,7 +837,10 @@ error and exit `1`, as does a listener that fails after binding. `0`, or prints a structured `invalid_request` error and exits `1` for the same pair refusals, a non-empty or file-occupied directory, a missing package tarball or a name that is not a kebab-case label or is a Windows device name, -writing nothing in each case. +writing nothing in each case. A file occupying the destination is reported as +requiring a directory; identity-change diagnostics are reserved for admitted +paths that fail later identity checks. Host permission failures identify whether +input reading or bundle writing was denied and retain the reported path. A write that fails after the first file (a permission, disk-space or pathname-length failure on a later path) propagates as the host's failure, naming its cause and, when it failed on one bundle file or the package diff --git a/src/application/environment/environmentExport.test.ts b/src/application/environment/environmentExport.test.ts index e55a847c02..37695970f6 100644 --- a/src/application/environment/environmentExport.test.ts +++ b/src/application/environment/environmentExport.test.ts @@ -1,4 +1,11 @@ -import { mkdir, readdir, readFile, stat, writeFile } from "node:fs/promises"; +import { + chmod, + mkdir, + readdir, + readFile, + stat, + writeFile, +} from "node:fs/promises"; import { join, sep } from "node:path"; import { describe, expect, it } from "vitest"; @@ -209,6 +216,15 @@ describe("exportEnvironmentBundle", () => { expect(projectAnalysisError(asFile.error).details?.issues).toEqual([ expect.objectContaining({ path: ["directory"] }), ]); + const fileRefusal = projectAnalysisError(asFile.error); + expect(fileRefusal.message).toContain("must be a directory"); + expect(fileRefusal.message).not.toContain("identity changed"); + expect(fileRefusal.details?.issues).toEqual([ + expect.objectContaining({ message: fileRefusal.message }), + ]); + expect(await readFile(join(directory, "keep.txt"), "utf8")).toBe( + "existing", + ); const missingTarball = await exportEnvironmentBundle({ ...input, directory: join(root, "fresh"), @@ -221,6 +237,49 @@ describe("exportEnvironmentBundle", () => { expect(await listFiles(root)).not.toContain("fresh/README.md"); }); + it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "distinguishes denied output writing from denied input reading and recovers with the same inputs", + async () => { + const { input, directory, files } = await prepare( + "rea-environment-export-denied-", + ); + await mkdir(directory, { mode: 0o500 }); + try { + const denied = await exportEnvironmentBundle({ ...input, directory }); + if (denied.ok) throw new Error("Expected write denial"); + expect(projectAnalysisError(denied.error)).toMatchObject({ + message: expect.stringContaining("denied writing"), + remediation: { + action: expect.stringContaining("writable destination"), + }, + details: { + diagnostics: { + boundary: "filesystem-write", + system_code: "EACCES", + }, + }, + }); + expect(await readdir(directory)).toEqual([]); + await chmod(files.manifestPath, 0); + const readDenied = await readEnvironmentExportInputs( + files.manifestPath, + files.capturePath, + ); + if (readDenied.ok) throw new Error("Expected read denial"); + expect(projectAnalysisError(readDenied.error)).toMatchObject({ + code: "access_denied", + details: { boundary: "filesystem-read" }, + }); + } finally { + await chmod(directory, 0o700); + await chmod(files.manifestPath, 0o600); + } + expect((await exportEnvironmentBundle({ ...input, directory })).ok).toBe( + true, + ); + }, + ); + it("accepts an existing empty directory", async () => { const { input, directory } = await prepare("rea-environment-export-empty-"); await mkdir(directory); diff --git a/src/application/environment/environmentExport.ts b/src/application/environment/environmentExport.ts index 336011db9d..859188afc9 100644 --- a/src/application/environment/environmentExport.ts +++ b/src/application/environment/environmentExport.ts @@ -128,26 +128,48 @@ const accessDeniedError = ( cause: unknown, path: string, diagnostics?: Readonly>, -): AnalysisAccessDeniedError | undefined => { + access: "read" | "write" = "read", +): AnalysisAccessDeniedError | AnalysisOutputError | undefined => { const seen = new Set(); let current = cause; while (current instanceof Error && !seen.has(current)) { seen.add(current); + if (current instanceof BundleFileHostFailure) + access = current.operation === "reading" ? "read" : "write"; const code = systemCode(current); if (code === "EACCES" || code === "EPERM") { - const error = new AnalysisAccessDeniedError( - ENVIRONMENT_EXPORT_OPERATION, - path, - code, - { - cause, - ...(diagnostics === undefined ? {} : { diagnostics }), - ...(cause instanceof ArtifactReaderFailure && - cause.cleanup !== undefined - ? { cleanup: cause.cleanup } - : {}), - }, - ); + const options = { + cause, + ...(diagnostics === undefined ? {} : { diagnostics }), + ...(cause instanceof ArtifactReaderFailure && + cause.cleanup !== undefined + ? { cleanup: cause.cleanup } + : {}), + }; + const error = + access === "read" + ? new AnalysisAccessDeniedError( + ENVIRONMENT_EXPORT_OPERATION, + path, + code, + options, + ) + : new AnalysisOutputError( + ENVIRONMENT_EXPORT_OPERATION, + `Host filesystem permissions denied write access: ${path}`, + { + ...options, + userMessage: `Host filesystem permissions denied writing the environment bundle at ${path}.`, + remediationAction: + "Check write access or select a writable destination. Inspect the output directory for partial output before retrying; export refuses a non-empty directory.", + diagnostics: { + ...diagnostics, + path, + system_code: code, + boundary: "filesystem-write", + }, + }, + ); bindArtifactFileCleanup(cause, error); return error; } @@ -354,6 +376,9 @@ export const exportEnvironmentBundle = async ( cause, hostFailurePath(cause) ?? input.directory, { output_directory: input.directory }, + cause instanceof BundleFileHostFailure && cause.operation === "reading" + ? "read" + : "write", ); if (denied !== undefined) return err(denied); if (cause instanceof RegularFileChangedError) diff --git a/src/artifacts/SafeOutputTreeIdentity.ts b/src/artifacts/SafeOutputTreeIdentity.ts index 5c29c2cea4..d8fc01d05e 100644 --- a/src/artifacts/SafeOutputTreeIdentity.ts +++ b/src/artifacts/SafeOutputTreeIdentity.ts @@ -52,7 +52,11 @@ export const readDirectoryIdentity = async ( path: string, ): Promise => { const metadata = await lstat(path, { bigint: true }); - if (!metadata.isDirectory()) throw replacedDirectory(path); + if (!metadata.isDirectory()) + throw new ArtifactReaderFailure( + "path", + `Output destination must be a directory: ${path}`, + ); return { dev: metadata.dev, ino: metadata.ino }; };