morluto/rea

Fork 0

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

[GH-ISSUE #5907] [Bug] Dylib trace loses integrity reason when an admitted pathname disappears #3583

New issue

Closed

opened 2026-10-31 00:42:07 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:42:07 +00:00

Owner

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

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

Renaming a Mach-O file away while its dyld headers or digest are being read makes trace_dylib_resolution return reason: "io", message Artifact trace_dylib_resolution failed: io, and no cause. The real post-read pathname lookup failed with ENOENT, but that evidence is lost instead of identifying an input that changed during analysis.

This was executed through ArtifactProvider.execute, the production operation used by the CLI and MCP. A focused test made the race deterministic by renaming a real file after an owned descriptor read. The CLI and MCP transports were source-checked for reachability, not executed.

Reproduction

From the source checkout with locked dependencies installed, save the following as tests/process-global/dylibPathRemoval.test.ts and run:

node_modules/.bin/vitest run tests/process-global/dylibPathRemoval.test.ts --maxWorkers 1

The executed test used this content. It characterizes the current incorrect result: all three assertions pass because the two rename cases currently produce io.

import { rename, writeFile, type FileHandle } from "node:fs/promises";
import { join } from "node:path";
import { expect, it, vi } from "vitest";

const state = vi.hoisted(() => ({ path: "", opens: 0, removeAt: 0, removed: false, missingCode: "", handles: [] as FileHandle[] }));
vi.mock("node:fs/promises", async (importOriginal) => {
  const fs = await importOriginal<typeof import("node:fs/promises")>();
  return {
    ...fs,
    lstat: async (...args: Parameters<typeof fs.lstat>) => {
      try { return await fs.lstat(...args); }
      catch (error) {
        if (args[0] === state.path && error instanceof Error && "code" in error)
          state.missingCode = String(error.code);
        throw error;
      }
    },
    open: async (...args: Parameters<typeof fs.open>) => {
      const handle = await fs.open(...args);
      if (args[0] !== state.path) return handle;
      state.handles.push(handle);
      const count = ++state.opens;
      const read = handle.read.bind(handle);
      handle.read = (async (...args: unknown[]) => {
        const result = await Reflect.apply(read, handle, args);
        if (count === state.removeAt && !state.removed) {
          state.removed = true;
          await fs.rename(state.path, `${state.path}.old`);
        }
        return result;
      }) as FileHandle["read"];
      return handle;
    },
  };
});

import { ArtifactProvider } from "../../src/artifacts/ArtifactProvider.js";
import { parseBinaryTarget } from "../../src/application/BinaryTargetResolver.js";
import { machoImage } from "../../src/artifacts/apple/MachoImage.fixture.js";
import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js";

it.each([0, 1, 2])("observes the public operation with pathname removal at open %i", async (removeAt) => {
  state.path = "";
  state.opens = 0;
  state.removeAt = removeAt;
  state.removed = false;
  state.missingCode = "";
  state.handles = [];
  const root = await createTestTempDirectory("rea-dylib-removal-");
  const path = join(root, "Main");
  await writeFile(path, machoImage({}));
  const target = await parseBinaryTarget(path);
  if (!target.ok) throw target.error;
  const client = new ArtifactProvider({}).createClient(target.value);
  state.path = path;
  try {
    const result = await client.execute("trace_dylib_resolution", { roots: ["Main"] });
    if (removeAt === 0) {
      expect(result.ok).toBe(true);
      console.log(JSON.stringify({ removeAt, ok: result.ok, opens: state.opens }));
    } else {
      if (result.ok) throw new Error("Expected failed read");
      expect(state.removed).toBe(true);
      expect(state.missingCode).toBe("ENOENT");
      expect(result.error).toMatchObject({ reason: "io", message: "Artifact trace_dylib_resolution failed: io" });
      expect(result.error.cause).toBeUndefined();
      console.log(JSON.stringify({ removeAt, removed: state.removed, missingCode: state.missingCode,
        reason: result.error.reason, message: result.error.message, causeMissing: result.error.cause === undefined }));
    }
    expect(state.handles.every(handle => handle.fd === -1)).toBe(true);
  } finally {
    await client.close();
  }
});

The unchanged-file case succeeded. Each rename case reproduced once in this run: open 1 is the header read and open 2 is the digest read. The filesystem hook delegates to real filesystem operations and only schedules the rename; it does not synthesize ENOENT. All owned descriptors closed.

Expected behavior

Report integrity when the pathname disappears after the file has been admitted and read, retaining the failed path and original filesystem cause in the typed error chain.

The reader's finalization contract requires headers, magic and digest to refer to the same physical version still selected by the pathname. The existing replacement test expects integrity when a renamed pathname is replaced. The stable artifact reader already classifies post-validation disappearance as integrity and retains its cause.

Impact and scope

P2: callers lose the machine-readable distinction between input mutation and unrelated I/O, plus the underlying error evidence. The trace still fails rather than returning potentially inconsistent results. This is a bounded race; natural frequency is unknown, and no data loss or security-boundary crossing was demonstrated.

Both header and digest paths reproduced. A stable-file control succeeded. Capturing a stable copy before tracing is a possible mitigation, but a general workaround for a mutating live app bundle was not tested. Missing-before-admission behavior is outside this report.

REA version

rea-agents 6.7.0 source checkout, revision 52415975942a4b9c53a817915b313ccc404f280e. Fetched origin/main at c324d31b45fcab77e4ed7320ed60f0bf5a4ba188; the relevant file-access, reader, provider, translation and resolver files are unchanged. No runtime replay on that newer revision is claimed.

Runtime and platform

Node.js v26.12.0; macOS 27.0.1 arm64; repository-locked Vitest 5.0.3. No Apple tool, Hopper or Ghidra was required.

Target and artifact context

A synthetic Mach-O file created by the repository's machoImage({}) fixture, selected as root Main. The file is renamed after a real descriptor read, with no replacement at its original pathname.

Interface

Other

Earliest failure stage

Native analysis or reconstruction

Relevant output or evidence

Observed subsets from the focused test:

{"removeAt":0,"ok":true,"opens":2}
{"removeAt":1,"removed":true,"missingCode":"ENOENT","reason":"io","message":"Artifact trace_dylib_resolution failed: io","causeMissing":true}
{"removeAt":2,"removed":true,"missingCode":"ENOENT","reason":"io","message":"Artifact trace_dylib_resolution failed: io","causeMissing":true}

The test file passed all 3 characterization cases. This establishes the two controlled interleavings at the production provider boundary, not their natural occurrence rate. ENOTDIR, ELOOP, bundle magic enumeration, Windows, and full CLI/MCP transport behavior were not replayed.

Typed outcome or recovery expected

An ArtifactReaderFailure with reason integrity, a path-specific change explanation, and the original ENOENT as its cause, translated to an ArtifactOperationError that retains those facts. Continuing the trace successfully or introducing partial results is not required.

Analysis or suspected cause

Source-derived: the final lstat throws before the state comparison can call changed(path). The dyld failure classifier handles nonregular-file and permission errors, leaving ENOENT raw. The artifact boundary fallback constructs a generic io error without retaining that cause.

Handling disappearance at the post-admission pathname check would match the existing stable-reader contract without conflating it with a target that was absent before admission.

Additional context

Related #4495 addressed blocking admission, cancellation and descriptor cleanup; this report concerns classification after a completed read and successful cleanup. #5747 concerns absence wording for incompatible candidates, and #5696 concerns omitted traversal of platform-less dylibs. Neither describes this final pathname probe. No fix or restored-behavior run has been performed. Last known good version: NOT_ENOUGH_INFO; no earlier-version comparison was run.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5907 ### 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 Renaming a Mach-O file away while its dyld headers or digest are being read makes `trace_dylib_resolution` return `reason: "io"`, message `Artifact trace_dylib_resolution failed: io`, and no cause. The real post-read pathname lookup failed with `ENOENT`, but that evidence is lost instead of identifying an input that changed during analysis. This was executed through `ArtifactProvider.execute`, the production operation used by the CLI and MCP. A focused test made the race deterministic by renaming a real file after an owned descriptor read. The CLI and MCP transports were source-checked for reachability, not executed. ### Reproduction From the source checkout with locked dependencies installed, save the following as `tests/process-global/dylibPathRemoval.test.ts` and run: ```sh node_modules/.bin/vitest run tests/process-global/dylibPathRemoval.test.ts --maxWorkers 1 ``` The executed test used this content. It characterizes the current incorrect result: all three assertions pass because the two rename cases currently produce `io`. ```ts import { rename, writeFile, type FileHandle } from "node:fs/promises"; import { join } from "node:path"; import { expect, it, vi } from "vitest"; const state = vi.hoisted(() => ({ path: "", opens: 0, removeAt: 0, removed: false, missingCode: "", handles: [] as FileHandle[] })); vi.mock("node:fs/promises", async (importOriginal) => { const fs = await importOriginal<typeof import("node:fs/promises")>(); return { ...fs, lstat: async (...args: Parameters<typeof fs.lstat>) => { try { return await fs.lstat(...args); } catch (error) { if (args[0] === state.path && error instanceof Error && "code" in error) state.missingCode = String(error.code); throw error; } }, open: async (...args: Parameters<typeof fs.open>) => { const handle = await fs.open(...args); if (args[0] !== state.path) return handle; state.handles.push(handle); const count = ++state.opens; const read = handle.read.bind(handle); handle.read = (async (...args: unknown[]) => { const result = await Reflect.apply(read, handle, args); if (count === state.removeAt && !state.removed) { state.removed = true; await fs.rename(state.path, `${state.path}.old`); } return result; }) as FileHandle["read"]; return handle; }, }; }); import { ArtifactProvider } from "../../src/artifacts/ArtifactProvider.js"; import { parseBinaryTarget } from "../../src/application/BinaryTargetResolver.js"; import { machoImage } from "../../src/artifacts/apple/MachoImage.fixture.js"; import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; it.each([0, 1, 2])("observes the public operation with pathname removal at open %i", async (removeAt) => { state.path = ""; state.opens = 0; state.removeAt = removeAt; state.removed = false; state.missingCode = ""; state.handles = []; const root = await createTestTempDirectory("rea-dylib-removal-"); const path = join(root, "Main"); await writeFile(path, machoImage({})); const target = await parseBinaryTarget(path); if (!target.ok) throw target.error; const client = new ArtifactProvider({}).createClient(target.value); state.path = path; try { const result = await client.execute("trace_dylib_resolution", { roots: ["Main"] }); if (removeAt === 0) { expect(result.ok).toBe(true); console.log(JSON.stringify({ removeAt, ok: result.ok, opens: state.opens })); } else { if (result.ok) throw new Error("Expected failed read"); expect(state.removed).toBe(true); expect(state.missingCode).toBe("ENOENT"); expect(result.error).toMatchObject({ reason: "io", message: "Artifact trace_dylib_resolution failed: io" }); expect(result.error.cause).toBeUndefined(); console.log(JSON.stringify({ removeAt, removed: state.removed, missingCode: state.missingCode, reason: result.error.reason, message: result.error.message, causeMissing: result.error.cause === undefined })); } expect(state.handles.every(handle => handle.fd === -1)).toBe(true); } finally { await client.close(); } }); ``` The unchanged-file case succeeded. Each rename case reproduced once in this run: open 1 is the header read and open 2 is the digest read. The filesystem hook delegates to real filesystem operations and only schedules the rename; it does not synthesize `ENOENT`. All owned descriptors closed. ### Expected behavior Report `integrity` when the pathname disappears after the file has been admitted and read, retaining the failed path and original filesystem cause in the typed error chain. The [reader's finalization contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/apple/DylibFileAccess.ts#L83-L110) requires headers, magic and digest to refer to the same physical version still selected by the pathname. The [existing replacement test](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/tests/process-global/dylibFileLifetime.test.ts#L241-L250) expects `integrity` when a renamed pathname is replaced. The [stable artifact reader](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/readStableArtifact.ts#L28-L48) already classifies post-validation disappearance as integrity and retains its cause. ### Impact and scope P2: callers lose the machine-readable distinction between input mutation and unrelated I/O, plus the underlying error evidence. The trace still fails rather than returning potentially inconsistent results. This is a bounded race; natural frequency is unknown, and no data loss or security-boundary crossing was demonstrated. Both header and digest paths reproduced. A stable-file control succeeded. Capturing a stable copy before tracing is a possible mitigation, but a general workaround for a mutating live app bundle was not tested. Missing-before-admission behavior is outside this report. ### REA version `rea-agents` 6.7.0 source checkout, revision `52415975942a4b9c53a817915b313ccc404f280e`. Fetched `origin/main` at `c324d31b45fcab77e4ed7320ed60f0bf5a4ba188`; the relevant file-access, reader, provider, translation and resolver files are unchanged. No runtime replay on that newer revision is claimed. ### Runtime and platform Node.js v26.12.0; macOS 27.0.1 arm64; repository-locked Vitest 5.0.3. No Apple tool, Hopper or Ghidra was required. ### Target and artifact context A synthetic Mach-O file created by the repository's `machoImage({})` fixture, selected as root `Main`. The file is renamed after a real descriptor read, with no replacement at its original pathname. ### Interface Other ### Earliest failure stage Native analysis or reconstruction ### Relevant output or evidence Observed subsets from the focused test: ```json {"removeAt":0,"ok":true,"opens":2} {"removeAt":1,"removed":true,"missingCode":"ENOENT","reason":"io","message":"Artifact trace_dylib_resolution failed: io","causeMissing":true} {"removeAt":2,"removed":true,"missingCode":"ENOENT","reason":"io","message":"Artifact trace_dylib_resolution failed: io","causeMissing":true} ``` The test file passed all 3 characterization cases. This establishes the two controlled interleavings at the production provider boundary, not their natural occurrence rate. `ENOTDIR`, `ELOOP`, bundle magic enumeration, Windows, and full CLI/MCP transport behavior were not replayed. ### Typed outcome or recovery expected An `ArtifactReaderFailure` with reason `integrity`, a path-specific change explanation, and the original `ENOENT` as its cause, translated to an `ArtifactOperationError` that retains those facts. Continuing the trace successfully or introducing partial results is not required. ### Analysis or suspected cause Source-derived: the [final `lstat`](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/apple/DylibFileAccess.ts#L95-L109) throws before the state comparison can call `changed(path)`. The [dyld failure classifier](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/apple/DylibFileAccess.ts#L114-L146) handles nonregular-file and permission errors, leaving `ENOENT` raw. The [artifact boundary fallback](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/ArtifactOperationFailure.ts#L16-L49) constructs a generic `io` error without retaining that cause. Handling disappearance at the post-admission pathname check would match the existing stable-reader contract without conflating it with a target that was absent before admission. ### Additional context Related #4495 addressed blocking admission, cancellation and descriptor cleanup; this report concerns classification after a completed read and successful cleanup. #5747 concerns absence wording for incompatible candidates, and #5696 concerns omitted traversal of platform-less dylibs. Neither describes this final pathname probe. No fix or restored-behavior run has been performed. Last known good version: NOT_ENOUGH_INFO; no earlier-version comparison was run.

bsakaguchi closed this issue

2026-10-31 00:42:07 +00:00

bsakaguchi referenced this issue

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

[PR #3583] [CLOSED] fix(native): check every slice of a signed universal Mach-O #5837

bsakaguchi referenced this issue

2026-10-31 01:12:39 +00:00

[PR #4359] [CLOSED] fix(native): report every slice of a signed universal Mach-O signature #6202

bsakaguchi referenced this issue

2026-11-01 21:11:07 +00:00

[GH-ISSUE #5654] [Tracker] Open PRs that can't merge yet: conflicts and red CI (October 2026) #3425