morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #5684] [Bug] environment-export reports false cleanup failure when admitted tarball grows #3435

New issue

Closed

opened 2026-10-31 00:40:26 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:40:26 +00:00

Owner

Originally created by @morluto on GitHub (Oct 20, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5684

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Evidence, artifacts, or reconstruction

Observed behavior

At source revision 60956c8c56475e7aaab9559f780908851677ced6, a controlled call to the public application API exportEnvironmentBundle admits a 2 MiB package tarball. A hook grows that same inode by 1 MiB after the hash pass and just before the copy pass. REA correctly refuses the copy with Extracted content exceeds the inventoried size, but also returns cleanup failure File handle was invalidated before this owner confirmed its close (ArtifactFileCleanupFailure). The cleanup error is false: the read stream invalidated the descriptor before the retained file owner confirmed its close.

Reproduction

Run from the checkout with the pinned Node 24.18.0 and installed locked dependencies. The exact executed script imports the .ts API with --import tsx.

Create a temporary directory and synthetic valid manifest/evidence using writeEnvironmentServeFiles and readEnvironmentExportInputs from tests/fixtures/environmentServe.ts.
Write a 2 MiB package.tgz.
Temporarily wrap SafeOutputTree.prototype.write. When its destination is envs/grafana-example/rea-agents.tgz, append 1 MiB to package.tgz before delegating to the original writer. Restore the method in finally.
Call exportEnvironmentBundle with the admitted tarball path.

Save this script as growth-repro.mjs in a disposable copy of the checkout. Run it with the checkout root as the working directory so the imports resolve against that source:

node --version # expected v24.18.0
node --import tsx /path/to/growth-repro.mjs

import { appendFile, mkdtemp, rm, stat, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { pathToFileURL } from "node:url";

const root = process.cwd();
const load = (path) => import(pathToFileURL(join(root, path)).href);
const [{ SafeOutputTree }, serve, exportApi] = await Promise.all([
  load("src/artifacts/SafeOutputTree.ts"),
  load("tests/fixtures/environmentServe.ts"),
  load("src/application/environment/environmentExport.ts"),
]);
const work = await mkdtemp(join(tmpdir(), "rea-env-growth-"));
const original = SafeOutputTree.prototype.write;
let changed = false;
try {
  const files = await serve.writeEnvironmentServeFiles(work);
  const inputs = await exportApi.readEnvironmentExportInputs(
    files.manifestPath,
    files.capturePath,
  );
  if (!inputs.ok) throw inputs.error;
  const tarball = join(work, "package.tgz");
  await writeFile(tarball, Buffer.alloc(2 * 1024 * 1024, 0x41));
  SafeOutputTree.prototype.write = async function (...args) {
    if (!changed && String(args[0]).endsWith("rea-agents.tgz")) {
      changed = true;
      await appendFile(tarball, Buffer.alloc(1024 * 1024, 0x42));
    }
    return original.apply(this, args);
  };
  const result = await exportApi.exportEnvironmentBundle({
    ...inputs.value,
    source: serve.environmentServeSource(),
    directory: join(work, "bundle"),
    packageTarball: tarball,
  });
  console.log(JSON.stringify({
    ok: result.ok,
    error: result.ok ? null : {
      tag: result.error._tag,
      operation: result.error.operation,
      reason: result.error.reason,
      message: result.error.message,
      cleanup: result.error.cleanup,
      cause: result.error.cause?.constructor?.name,
    },
    sourceBytes: (await stat(tarball)).size,
    changed,
  }, null, 2));
} finally {
  SafeOutputTree.prototype.write = original;
  await rm(work, { recursive: true, force: true });
}

This controlled script was run once on the unpatched revision. Script SHA-256: e99adda725e0b10fb64396ad426ed6a4157a6057c015fb49cced9b1535111226; raw output SHA-256: cf2ce9a242830e5ef6994a64e379bdc2d19b738fb53d84307cfa4361882b8473.

Expected behavior

Keep the expected integrity refusal for the changed tarball, while leaving descriptor closure to the retained file owner. Do not report cleanup failure when the borrowed stream ends early; preserve genuine close failures if the owner actually cannot close the file.

Impact and scope

Observed impact is an inaccurate cleanup failure attached to a correctly detected tarball integrity refusal. This can make callers treat a controlled changed-input refusal as unresolved cleanup. The reproduction exercises the exported application API with a controlled writer hook; it does not establish incidence through the CLI, MCP, a provider, or an external caller, nor does it establish a physical descriptor leak or ordinary-use frequency. Other platforms were not tested.

REA version

Source revision 60956c8c56475e7aaab9559f780908851677ced6 (package version 6.7.0).

Runtime and platform

Node.js v24.18.0; npm 11.16.0; macOS 26.3 arm64.

Target and artifact context

Synthetic environment manifest/evidence and an ordinary 2 MiB regular-file package tarball; no external target or provider.

Interface

Other (public application API)

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed result from the command above:

{
  "ok": false,
  "error": {
    "tag": "AnalysisOutputError",
    "operation": "environment-export",
    "reason": "Extracted content exceeds the inventoried size: envs/grafana-example/rea-agents.tgz",
    "cleanup": {
      "reason": "File handle was invalidated before this owner confirmed its close",
      "resources": ["<temporary>/package.tgz"]
    },
    "cause": "ArtifactFileCleanupFailure"
  },
  "sourceBytes": 3145728,
  "changed": true
}

The output proves that growth was detected and that the result also contains an owner-close failure. The source-derived path is environmentBundleWrite.ts at the tested revision, where both tarball passes use FileHandle.createReadStream; OwnedFileHandle.close records an invalidated descriptor as an unconfirmed close. These observations do not establish broader runtime incidence.

Typed outcome or recovery expected

Return the size/integrity refusal without a cleanup observation when the retained owner can close its descriptor. Keep exact ownership and retry reporting when a real close failure occurs.

Analysis or suspected cause

Source-derived reasoning: writeTarball hashes and then copies from the retained FileHandle, and the copy stream is consumed by SafeOutputTree.write. In the controlled early-copy refusal, the stream leaves the underlying descriptor invalid before the surrounding withRegularFile owner confirms close. The shared readFileHandleChunks provides positional reads while leaving descriptor cleanup to its owner. A proposed patch using it for both tarball passes applied cleanly to this revision; its regression test and post-fix control passed, but no fix has been integrated or submitted.

Additional context

Live read-only dedupe on 2026-10-21 found no matching environment tarball ownership report. Searches for the exact invalidated-owner message returned #4381, #4610 and #4619; these concern managed hashing cancellation, earlier cleanup classification and firmware snapshot growth respectively. They share lifecycle vocabulary or low-level ownership context but are not this environment-export boundary. Merged PR #4446 fixed managed-hash cancellation with bounded reads through its owner; it does not cover this environment writer. Open PR #5581 (4868dd9fbf29ae981f25ef33110a1223de793849) and issue #5522 concern classifying later output-write failures, not source descriptor ownership; the growth cleanup failure can coexist with that classification behavior.

Originally created by @morluto on GitHub (Oct 20, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5684 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Evidence, artifacts, or reconstruction ### Observed behavior At source revision `60956c8c56475e7aaab9559f780908851677ced6`, a controlled call to the public application API `exportEnvironmentBundle` admits a 2 MiB package tarball. A hook grows that same inode by 1 MiB after the hash pass and just before the copy pass. REA correctly refuses the copy with `Extracted content exceeds the inventoried size`, but also returns cleanup failure `File handle was invalidated before this owner confirmed its close` (`ArtifactFileCleanupFailure`). The cleanup error is false: the read stream invalidated the descriptor before the retained file owner confirmed its close. ### Reproduction Run from the checkout with the pinned Node 24.18.0 and installed locked dependencies. The exact executed script imports the `.ts` API with `--import tsx`. 1. Create a temporary directory and synthetic valid manifest/evidence using `writeEnvironmentServeFiles` and `readEnvironmentExportInputs` from `tests/fixtures/environmentServe.ts`. 2. Write a 2 MiB `package.tgz`. 3. Temporarily wrap `SafeOutputTree.prototype.write`. When its destination is `envs/grafana-example/rea-agents.tgz`, append 1 MiB to `package.tgz` before delegating to the original writer. Restore the method in `finally`. 4. Call `exportEnvironmentBundle` with the admitted tarball path. Save this script as `growth-repro.mjs` in a disposable copy of the checkout. Run it with the checkout root as the working directory so the imports resolve against that source: ```sh node --version # expected v24.18.0 node --import tsx /path/to/growth-repro.mjs ``` ```js import { appendFile, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { pathToFileURL } from "node:url"; const root = process.cwd(); const load = (path) => import(pathToFileURL(join(root, path)).href); const [{ SafeOutputTree }, serve, exportApi] = await Promise.all([ load("src/artifacts/SafeOutputTree.ts"), load("tests/fixtures/environmentServe.ts"), load("src/application/environment/environmentExport.ts"), ]); const work = await mkdtemp(join(tmpdir(), "rea-env-growth-")); const original = SafeOutputTree.prototype.write; let changed = false; try { const files = await serve.writeEnvironmentServeFiles(work); const inputs = await exportApi.readEnvironmentExportInputs( files.manifestPath, files.capturePath, ); if (!inputs.ok) throw inputs.error; const tarball = join(work, "package.tgz"); await writeFile(tarball, Buffer.alloc(2 * 1024 * 1024, 0x41)); SafeOutputTree.prototype.write = async function (...args) { if (!changed && String(args[0]).endsWith("rea-agents.tgz")) { changed = true; await appendFile(tarball, Buffer.alloc(1024 * 1024, 0x42)); } return original.apply(this, args); }; const result = await exportApi.exportEnvironmentBundle({ ...inputs.value, source: serve.environmentServeSource(), directory: join(work, "bundle"), packageTarball: tarball, }); console.log(JSON.stringify({ ok: result.ok, error: result.ok ? null : { tag: result.error._tag, operation: result.error.operation, reason: result.error.reason, message: result.error.message, cleanup: result.error.cleanup, cause: result.error.cause?.constructor?.name, }, sourceBytes: (await stat(tarball)).size, changed, }, null, 2)); } finally { SafeOutputTree.prototype.write = original; await rm(work, { recursive: true, force: true }); } ``` This controlled script was run once on the unpatched revision. Script SHA-256: `e99adda725e0b10fb64396ad426ed6a4157a6057c015fb49cced9b1535111226`; raw output SHA-256: `cf2ce9a242830e5ef6994a64e379bdc2d19b738fb53d84307cfa4361882b8473`. ### Expected behavior Keep the expected integrity refusal for the changed tarball, while leaving descriptor closure to the retained file owner. Do not report cleanup failure when the borrowed stream ends early; preserve genuine close failures if the owner actually cannot close the file. ### Impact and scope Observed impact is an inaccurate cleanup failure attached to a correctly detected tarball integrity refusal. This can make callers treat a controlled changed-input refusal as unresolved cleanup. The reproduction exercises the exported application API with a controlled writer hook; it does not establish incidence through the CLI, MCP, a provider, or an external caller, nor does it establish a physical descriptor leak or ordinary-use frequency. Other platforms were not tested. ### REA version Source revision `60956c8c56475e7aaab9559f780908851677ced6` (package version `6.7.0`). ### Runtime and platform Node.js v24.18.0; npm 11.16.0; macOS 26.3 arm64. ### Target and artifact context Synthetic environment manifest/evidence and an ordinary 2 MiB regular-file package tarball; no external target or provider. ### Interface Other (public application API) ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed result from the command above: ```json { "ok": false, "error": { "tag": "AnalysisOutputError", "operation": "environment-export", "reason": "Extracted content exceeds the inventoried size: envs/grafana-example/rea-agents.tgz", "cleanup": { "reason": "File handle was invalidated before this owner confirmed its close", "resources": ["<temporary>/package.tgz"] }, "cause": "ArtifactFileCleanupFailure" }, "sourceBytes": 3145728, "changed": true } ``` The output proves that growth was detected and that the result also contains an owner-close failure. The source-derived path is [`environmentBundleWrite.ts` at the tested revision](https://github.com/morluto/rea/blob/60956c8c56475e7aaab9559f780908851677ced6/src/application/environment/environmentBundleWrite.ts#L46-L72), where both tarball passes use `FileHandle.createReadStream`; [`OwnedFileHandle.close`](https://github.com/morluto/rea/blob/60956c8c56475e7aaab9559f780908851677ced6/src/filesystem/OwnedFileHandle.ts#L17-L28) records an invalidated descriptor as an unconfirmed close. These observations do not establish broader runtime incidence. ### Typed outcome or recovery expected Return the size/integrity refusal without a cleanup observation when the retained owner can close its descriptor. Keep exact ownership and retry reporting when a real close failure occurs. ### Analysis or suspected cause Source-derived reasoning: `writeTarball` hashes and then copies from the retained `FileHandle`, and the copy stream is consumed by `SafeOutputTree.write`. In the controlled early-copy refusal, the stream leaves the underlying descriptor invalid before the surrounding `withRegularFile` owner confirms close. The shared [`readFileHandleChunks`](https://github.com/morluto/rea/blob/60956c8c56475e7aaab9559f780908851677ced6/src/filesystem/readFileHandleChunks.ts#L32-L45) provides positional reads while leaving descriptor cleanup to its owner. A proposed patch using it for both tarball passes applied cleanly to this revision; its regression test and post-fix control passed, but no fix has been integrated or submitted. ### Additional context Live read-only dedupe on 2026-10-21 found no matching environment tarball ownership report. Searches for the exact invalidated-owner message returned #4381, #4610 and #4619; these concern managed hashing cancellation, earlier cleanup classification and firmware snapshot growth respectively. They share lifecycle vocabulary or low-level ownership context but are not this environment-export boundary. Merged PR #4446 fixed managed-hash cancellation with bounded reads through its owner; it does not cover this environment writer. Open PR #5581 (`4868dd9fbf29ae981f25ef33110a1223de793849`) and issue #5522 concern classifying later output-write failures, not source descriptor ownership; the growth cleanup failure can coexist with that classification behavior.

bsakaguchi

2026-10-31 00:40:26 +00:00

closed this issue
added the
bug

triage
labels

bsakaguchi referenced this issue

2026-10-31 01:09:06 +00:00

[PR #3440] [CLOSED] fix(security): pin CI actions, protect Hopper xauth and downloads, flag old apktool #5743

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#3435

No description provided.

Rows
Columns