From e52ae1cde2439a38f88f1a608ef08e47a03ea8da Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 18:16:02 +0800 Subject: [PATCH] fix(environment): validate build paths and read dry-run tarballs --- .../environment/environmentBuild.test.ts | 28 +++++++++++++++++++ .../environment/environmentBundleWrite.ts | 21 +++++++++----- .../environment/environmentExport.ts | 10 +++++-- src/cli/environmentCommands.ts | 14 ++++------ .../boundary/cli/environmentBuildCli.test.ts | 11 +++++++- 5 files changed, 65 insertions(+), 19 deletions(-) diff --git a/src/application/environment/environmentBuild.test.ts b/src/application/environment/environmentBuild.test.ts index ffea4d2bf1..b780a8d5b3 100644 --- a/src/application/environment/environmentBuild.test.ts +++ b/src/application/environment/environmentBuild.test.ts @@ -542,6 +542,34 @@ describe("buildEnvironment credential names", () => { }); describe("buildEnvironment dry-run package tarball", () => { + it("refuses a tarball whose descriptor stats successfully but fails to read", async () => { + const root = await createTestTempDirectory("rea-environment-build-read-"); + const tarball = join(root, "rea-agents.tgz"); + await writeFile(tarball, "packed"); + const input = await buildInput(root, undefined, { + dryRun: true, + packageTarball: tarball, + }); + const prototype = await fileHandlePrototype(tarball); + const failure = Object.assign( + new Error("controlled tarball read failure"), + { code: "EIO" }, + ); + const read = vi.spyOn(prototype, "read").mockRejectedValue(failure); + try { + const planned = await buildEnvironment(input); + expect(read).toHaveBeenCalled(); + if (planned.ok) throw new Error("Expected a tarball read refusal"); + expect(projectAnalysisError(planned.error)).toMatchObject({ + code: "invalid_request", + details: { issues: [{ path: ["package_tarball"] }] }, + }); + expect(await readdir(root)).not.toContain("bundle"); + } finally { + read.mockRestore(); + } + }); + it.each([false, true])( "retries a failed tarball close and retains its owner when the failure persists: %s", async (persistent) => { diff --git a/src/application/environment/environmentBundleWrite.ts b/src/application/environment/environmentBundleWrite.ts index 1c7f48d965..9c111d3585 100644 --- a/src/application/environment/environmentBundleWrite.ts +++ b/src/application/environment/environmentBundleWrite.ts @@ -101,13 +101,10 @@ export interface BundleTarball { readonly size: number; } -/** Hash and copy only the instance retained before destination admission. */ -const writeTarball = async ( - tree: SafeOutputTree, - directory: string, - relative: string, +/** Read and hash the admitted instance, checking its complete length without retaining its contents. */ +export const bundleTarballDigest = async ( source: BundleTarball, -): Promise => { +): Promise => { const hash = createHash("sha256"); let observed = 0; for await (const chunk of tarballChunks(source)) { @@ -117,7 +114,17 @@ const writeTarball = async ( hash.update(bytes); } if (observed !== source.size) throw new RegularFileChangedError(source.path); - const sha256 = hash.digest("hex"); + return hash.digest("hex"); +}; + +/** Hash and copy only the instance retained before destination admission. */ +const writeTarball = async ( + tree: SafeOutputTree, + directory: string, + relative: string, + source: BundleTarball, +): Promise => { + const sha256 = await bundleTarballDigest(source); const written = await writingBundleFile(directory, relative, () => tree.write(relative, Readable.from(tarballChunks(source)), { bytes: source.size, diff --git a/src/application/environment/environmentExport.ts b/src/application/environment/environmentExport.ts index 0da6cc1c15..f08d55b730 100644 --- a/src/application/environment/environmentExport.ts +++ b/src/application/environment/environmentExport.ts @@ -24,6 +24,7 @@ import { RegularFileCleanupFailure } from "../../filesystem/OwnedRegularFile.js" import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; import { BundleFileHostFailure, + bundleTarballDigest, packageTarballRefusal, withBundleTarball, writeEnvironmentBundle, @@ -390,13 +391,16 @@ const readerFailureError = ( return error; }; -/** The package tarball's size, admitted and released as the export admits and releases it. */ +/** The package tarball's size after the same complete read and integrity check export uses. */ export const packageTarballBytes = async ( path: string, ): Promise> => { try { return ok( - await withBundleTarball(path, (tarball) => Promise.resolve(tarball.size)), + await withBundleTarball(path, async (tarball) => { + await bundleTarballDigest(tarball); + return tarball.size; + }), ); } catch (cause: unknown) { if (cause instanceof AnalysisError) return err(cause); @@ -404,6 +408,8 @@ export const packageTarballBytes = async ( return err(changedArtifact(cause)); if (cause instanceof ArtifactReaderFailure) return err(readerFailureError(cause, undefined)); + if (cause instanceof BundleFileHostFailure) + return err(packageTarballRefusal(path, cause)); if (systemCode(cause) !== undefined) return err(packageTarballRefusal(path, cause)); throw cause; diff --git a/src/cli/environmentCommands.ts b/src/cli/environmentCommands.ts index c998c7e12f..cf26e81bcd 100644 --- a/src/cli/environmentCommands.ts +++ b/src/cli/environmentCommands.ts @@ -486,13 +486,10 @@ const registerBuildCommand = ( description: "Build an agent environment bundle from a recorded capture in one offline step: inspect a HAR (or read capture Evidence), project the environment manifest, derive the state, report which tools the environment answers from state, which replay and why each write is withheld, then write the bundle environment-export writes", args: z.object({ - capture: z - .string() - .describe( - "A .har file, or capture Evidence JSON from inspect-web-network-capture (HAR) or capture-browser-scenario", - ), - directory: z - .string() + capture: localPathStringSchema.describe( + "A .har file, or capture Evidence JSON from inspect-web-network-capture (HAR) or capture-browser-scenario", + ), + directory: localPathStringSchema .optional() .describe( "Bundle directory to create; must be absent or empty, nothing is overwritten; omit only with --dry-run", @@ -515,8 +512,7 @@ const registerBuildCommand = ( .boolean() .default(false) .describe("Apply only the four format rules, the views' own default"), - packageTarball: z - .string() + packageTarball: localPathStringSchema .optional() .describe( "Packed rea-agents tarball (npm pack) copied into the env folder and installed by the Dockerfile instead of the registry package pinned to this version", diff --git a/tests/boundary/cli/environmentBuildCli.test.ts b/tests/boundary/cli/environmentBuildCli.test.ts index 1f9a7a43f7..206ec83960 100644 --- a/tests/boundary/cli/environmentBuildCli.test.ts +++ b/tests/boundary/cli/environmentBuildCli.test.ts @@ -1,4 +1,4 @@ -import { readdir, readFile, writeFile } from "node:fs/promises"; +import { mkdir, readdir, readFile, writeFile } from "node:fs/promises"; import { join, sep } from "node:path"; import { expect } from "vitest"; @@ -174,7 +174,16 @@ cliTest( code: "invalid_request", details: { issues: [{ path: ["directory"] }] }, }); + const emptyWorkingDirectory = join(root, "empty-cwd"); + await mkdir(emptyWorkingDirectory); + const emptyDirectory = await cli.run({ + arguments: ["environment-build", har, "", "--json"], + cwd: emptyWorkingDirectory, + }); + expect(emptyDirectory.exitCode).toBe(1); + expect(await readdir(emptyWorkingDirectory)).toEqual([]); expect((await readdir(root)).sort()).toEqual([ + "empty-cwd", "grafana.har", "grafana.saz", ]);