morluto/rea

Fork 0

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

[GH-ISSUE #5912] [Bug] Invalid Interface Builder plists retain budget and omit later valid archives #3589

New issue

Closed

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

bsakaguchi commented

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

Owner

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

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

An XML Interface Builder archive that passes representation preflight but fails plist decoding keeps its aggregate decode-budget reservation. A later valid archive can then be omitted even though the invalid archive retained no document or graph.

Observed in a focused production-boundary test with only the initial representation budget reduced from 256 MiB to 8192 bytes: a 111-byte invalid A.nib reserves 7100 bytes, fails with malformed document. First element should be <plist>, and never returns those bytes. A 200-byte valid B.nib needs 5920 bytes and is omitted with aggregate_decode_budget_exhausted. B alone decodes and produces an NSButton control named Build. Removing A restores B and the exact original graph nodes.

The real filesystem reader, preflight, reserve/release methods, XML parser, graph builder and coverage projection ran. CLI/MCP transport and the default 256 MiB threshold were not executed; their applicability is source-derived.

Reproduction

Save the following as tests/boundary/filesystem/interfaceBuilderInvalidBudget.test.ts in the source checkout with its locked dependencies installed, then run:

node node_modules/vitest/vitest.mjs run tests/boundary/filesystem/interfaceBuilderInvalidBudget.test.ts --maxWorkers=1

The test characterizes the current failure, so its assertions pass when the bug is present. Its sole behavioral substitution is the initial budget size. It creates inert XML fixtures and removes its generated bundle in finally.

import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { expect, it, vi } from "vitest";

vi.mock("../../../src/artifacts/apple/InterfaceBuilderDecodeBudget.js", async (importOriginal) => {
  const actual = await importOriginal<typeof import("../../../src/artifacts/apple/InterfaceBuilderDecodeBudget.js")>();
  return {
    ...actual,
    InterfaceBuilderDecodeBudget: class extends actual.InterfaceBuilderDecodeBudget {
      constructor(_maxBytes: number) { super(8192); }
    },
  };
});

import { analyzeInterfaceBuilderBundle } from "../../../src/artifacts/apple/InterfaceBuilderAnalysis.js";
import { InterfaceBuilderDecodeBudget, interfaceBuilderDecodeBudgetFailure } from "../../../src/artifacts/apple/InterfaceBuilderDecodeBudget.js";
import { preflightPropertyListDecode } from "../../../src/artifacts/apple/PropertyListDecodeBudget.js";

it("an invalid plist keeps its reservation and suppresses a later valid plist", async () => {
  const root = await mkdtemp(join(process.env.REA_TEST_SCRATCH ?? tmpdir(), "ib-budget-"));
  const invalid = `<bogus>${"<a/>".repeat(24)}</bogus>`;
  const valid = '<plist><dict><key>com.apple.ibtool.document.objects</key><dict><key>button</key><dict><key>customClass</key><string>NSButton</string><key>label</key><string>Build</string></dict></dict></dict></plist>';
  const estimate = (text: string) => preflightPropertyListDecode(Buffer.from(text), 8192, text, {
    numberProjection: true,
    budget: "the aggregate Interface Builder decode budget",
    fail: interfaceBuilderDecodeBudgetFailure,
  }).estimatedBytes;
  const invalidBytes = estimate(invalid);
  const validBytes = estimate(valid);
  expect(invalidBytes).toBeLessThan(8192);
  expect(validBytes).toBeLessThan(8192);
  expect(invalidBytes + validBytes).toBeGreaterThan(8192);
  const release = vi.spyOn(InterfaceBuilderDecodeBudget.prototype, "release");
  const reserve = vi.spyOn(InterfaceBuilderDecodeBudget.prototype, "reserve");
  try {
    const bundle = join(root, "Example.app");
    const resources = join(bundle, "Contents", "Resources");
    await mkdir(resources, { recursive: true });
    await writeFile(join(resources, "B.nib"), valid);
    const control = await analyzeInterfaceBuilderBundle({ bundlePath: bundle, targetSha256: "f".repeat(64) });
    expect(control.documents.map((d) => d.relative_path)).toEqual(["Contents/Resources/B.nib"]);
    expect(control.graph.nodes).toContainEqual(expect.objectContaining({ kind: "control", name: "Build", attributes: expect.objectContaining({ class_name: "NSButton" }) }));
    reserve.mockClear();
    release.mockClear();
    await writeFile(join(resources, "A.nib"), invalid);
    const result = await analyzeInterfaceBuilderBundle({ bundlePath: bundle, targetSha256: "f".repeat(64) });
    expect(result.documents).toEqual([]);
    expect(result.graph.nodes).toEqual([]);
    expect(reserve.mock.calls.map(([bytes]) => bytes)).toEqual([invalidBytes]);
    expect(release.mock.calls.map(([bytes]) => bytes)).toEqual([0]);
    expect(result.limitations.some((s) => s.includes("A.nib: malformed document. First element should be <plist>"))).toBe(true);
    expect(result.limitations).toContain("Contents/Resources/B.nib: omitted because aggregate decode budget exhausted.");
    expect(result.graph.coverage).toContainEqual(expect.objectContaining({
      facet: "archive_decode", reason: "aggregate_decode_budget_exhausted", omitted: 1,
    }));
    await rm(join(resources, "A.nib"));
    const recovered = await analyzeInterfaceBuilderBundle({ bundlePath: bundle, targetSha256: "f".repeat(64) });
    expect(recovered.documents.map((d) => d.relative_path)).toEqual(["Contents/Resources/B.nib"]);
    expect(recovered.graph.nodes).toEqual(control.graph.nodes);
    console.log(JSON.stringify({ invalidInputBytes: Buffer.byteLength(invalid), validInputBytes: Buffer.byteLength(valid), invalidBytes, validBytes, controlDocuments: control.documents.map((d) => d.relative_path), controlNodes: control.graph.nodes.map(({kind, name}) => ({kind, name})), documents: result.documents, releasedBytes: release.mock.calls.map(([bytes]) => bytes), recoveredDocuments: recovered.documents.map((d) => d.relative_path), coverage: result.graph.coverage.find((c) => c.facet === "archive_decode"), limitations: result.limitations }, null, 2));
  } finally {
    vi.restoreAllMocks();
    await rm(root, { recursive: true, force: true });
  }
});

Final execution: one focused file and one test passed, including the valid-only control, malformed-plus-valid failure and recovery after removing the malformed file. No external analysis engine or target application ran.

Expected behavior

Report A as invalid and return its representation reservation because no decoded document was retained. B should still decode when its own representation fits the remaining budget of successfully retained documents. Coverage should remain partial for the invalid archive, with one_or_more_archives_invalid and no budget omission caused by A's discarded representation.

The budget owner defines these as representation reservations and exposes release for rejected, unretained representations. The existing catch comment and boundary test already apply this rule to a post-reservation depth rejection. Generic format failures retain no representation either. Preserve the independent input-byte limit and real representation bounds.

Impact and scope

P2: valid Interface Builder documents and recognized controls can disappear from the decoded result when an earlier malformed plist consumes enough of the aggregate representation allowance. The result explicitly reports partial coverage, but the budget omission is caused by accounting for a rejected archive. There is no observed persistent state change or security-boundary crossing.

A demonstrated workaround is to inspect a disposable copy of the bundle with the invalid archive excluded; this restores the valid document and exact original graph in the fixture. Keep the original artifact for provenance. Real-app frequency and the regression introduction are unknown. The default-budget consequence follows the same accounting path, but no large default-budget fixture was executed.

REA version

rea-agents 6.7.0; source commit 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

macOS 27.0.1 arm64; Node.js v26.12.0; locked Vitest 5.0.3.

Target and artifact context

Synthetic app bundle with two XML .nib resources: a well-formed non-plist root in A.nib and an ibtool-style object dictionary containing one NSButton named Build in B.nib. No executable or native provider is needed for this production-boundary test.

Interface

Other

Earliest failure stage

Native analysis or reconstruction

Relevant output or evidence

Observed test output, reduced to relevant fields:

{
  "initialBudgetBytes": 8192,
  "invalidInputBytes": 111,
  "validInputBytes": 200,
  "invalidBytes": 7100,
  "validBytes": 5920,
  "controlDocuments": ["Contents/Resources/B.nib"],
  "controlNodes": [
    {"kind": "scene", "name": "Contents/Resources/B.nib"},
    {"kind": "control", "name": "Build"}
  ],
  "documents": [],
  "releasedBytes": [0],
  "recoveredDocuments": ["Contents/Resources/B.nib"],
  "coverage": {
    "facet": "archive_decode",
    "status": "partial",
    "reason": "aggregate_decode_budget_exhausted",
    "examined": 2,
    "omitted": 1
  }
}

Relevant limitations:

Some Interface Builder archives could not be decoded: Contents/Resources/A.nib: malformed document. First element should be <plist>
Contents/Resources/B.nib: omitted because aggregate decode budget exhausted.

The only release is release(0) when B fails preflight; A's 7100-byte reservation remains charged. The test also asserts that the failed run has no graph nodes and the recovered run has exactly the control's nodes.

Source reachability: CLI registration dispatches decode_interface_builder; the production artifact provider calls the same analyzer. The tool contract exposes that operation to MCP. These establish public reachability without claiming a transport execution.

Typed outcome or recovery expected

After repair, use the same malformed-plus-valid fixture and require B's document and Build control to remain, A's invalid-format limitation to remain, and no omitted archive attributed to aggregate decode exhaustion. The valid-only and recovered controls should continue to produce identical graph nodes. Actual over-budget archives and the 32 MiB aggregate input bound must remain enforced. No repair was made or tested here.

Analysis or suspected cause

Source-derived: XML preflight estimates text and element expansion without validating the plist root. The analyzer reserves that estimate before parsing. A non-plist root then throws a plain TypeError in the shared XML decoder. The catch releases the reservation only for InterfaceBuilderDecodeBudgetExceeded, while the generic invalid branch keeps it.

Returning the reservation for every failed plist decode that retained no representation would align both failure paths. This is an accounting issue rather than a request to raise the allowance.

Additional context

Freshness: fetched origin/main at c76edbebbe1e25c2bf821ec04d08dc851e37626c; the analyzer, budget class, shared preflight and XML decoder have no changes from the tested baseline. This is a source comparison, not an executed test on that newer revision.

Related reports about XML value-depth admission and its typed budget release, object-versus-connection limit attribution, ordinary plist depth rejection, and retained-graph paging do not cover generic malformed-archive reservation retention. Last known good version: NOT_ENOUGH_INFO; none established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5912 ### 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 An XML Interface Builder archive that passes representation preflight but fails plist decoding keeps its aggregate decode-budget reservation. A later valid archive can then be omitted even though the invalid archive retained no document or graph. Observed in a focused production-boundary test with only the initial representation budget reduced from 256 MiB to 8192 bytes: a 111-byte invalid A.nib reserves 7100 bytes, fails with `malformed document. First element should be <plist>`, and never returns those bytes. A 200-byte valid B.nib needs 5920 bytes and is omitted with `aggregate_decode_budget_exhausted`. B alone decodes and produces an NSButton control named Build. Removing A restores B and the exact original graph nodes. The real filesystem reader, preflight, reserve/release methods, XML parser, graph builder and coverage projection ran. CLI/MCP transport and the default 256 MiB threshold were not executed; their applicability is source-derived. ### Reproduction Save the following as `tests/boundary/filesystem/interfaceBuilderInvalidBudget.test.ts` in the source checkout with its locked dependencies installed, then run: ```sh node node_modules/vitest/vitest.mjs run tests/boundary/filesystem/interfaceBuilderInvalidBudget.test.ts --maxWorkers=1 ``` The test characterizes the current failure, so its assertions pass when the bug is present. Its sole behavioral substitution is the initial budget size. It creates inert XML fixtures and removes its generated bundle in `finally`. ```ts import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { expect, it, vi } from "vitest"; vi.mock("../../../src/artifacts/apple/InterfaceBuilderDecodeBudget.js", async (importOriginal) => { const actual = await importOriginal<typeof import("../../../src/artifacts/apple/InterfaceBuilderDecodeBudget.js")>(); return { ...actual, InterfaceBuilderDecodeBudget: class extends actual.InterfaceBuilderDecodeBudget { constructor(_maxBytes: number) { super(8192); } }, }; }); import { analyzeInterfaceBuilderBundle } from "../../../src/artifacts/apple/InterfaceBuilderAnalysis.js"; import { InterfaceBuilderDecodeBudget, interfaceBuilderDecodeBudgetFailure } from "../../../src/artifacts/apple/InterfaceBuilderDecodeBudget.js"; import { preflightPropertyListDecode } from "../../../src/artifacts/apple/PropertyListDecodeBudget.js"; it("an invalid plist keeps its reservation and suppresses a later valid plist", async () => { const root = await mkdtemp(join(process.env.REA_TEST_SCRATCH ?? tmpdir(), "ib-budget-")); const invalid = `<bogus>${"<a/>".repeat(24)}</bogus>`; const valid = '<plist><dict><key>com.apple.ibtool.document.objects</key><dict><key>button</key><dict><key>customClass</key><string>NSButton</string><key>label</key><string>Build</string></dict></dict></dict></plist>'; const estimate = (text: string) => preflightPropertyListDecode(Buffer.from(text), 8192, text, { numberProjection: true, budget: "the aggregate Interface Builder decode budget", fail: interfaceBuilderDecodeBudgetFailure, }).estimatedBytes; const invalidBytes = estimate(invalid); const validBytes = estimate(valid); expect(invalidBytes).toBeLessThan(8192); expect(validBytes).toBeLessThan(8192); expect(invalidBytes + validBytes).toBeGreaterThan(8192); const release = vi.spyOn(InterfaceBuilderDecodeBudget.prototype, "release"); const reserve = vi.spyOn(InterfaceBuilderDecodeBudget.prototype, "reserve"); try { const bundle = join(root, "Example.app"); const resources = join(bundle, "Contents", "Resources"); await mkdir(resources, { recursive: true }); await writeFile(join(resources, "B.nib"), valid); const control = await analyzeInterfaceBuilderBundle({ bundlePath: bundle, targetSha256: "f".repeat(64) }); expect(control.documents.map((d) => d.relative_path)).toEqual(["Contents/Resources/B.nib"]); expect(control.graph.nodes).toContainEqual(expect.objectContaining({ kind: "control", name: "Build", attributes: expect.objectContaining({ class_name: "NSButton" }) })); reserve.mockClear(); release.mockClear(); await writeFile(join(resources, "A.nib"), invalid); const result = await analyzeInterfaceBuilderBundle({ bundlePath: bundle, targetSha256: "f".repeat(64) }); expect(result.documents).toEqual([]); expect(result.graph.nodes).toEqual([]); expect(reserve.mock.calls.map(([bytes]) => bytes)).toEqual([invalidBytes]); expect(release.mock.calls.map(([bytes]) => bytes)).toEqual([0]); expect(result.limitations.some((s) => s.includes("A.nib: malformed document. First element should be <plist>"))).toBe(true); expect(result.limitations).toContain("Contents/Resources/B.nib: omitted because aggregate decode budget exhausted."); expect(result.graph.coverage).toContainEqual(expect.objectContaining({ facet: "archive_decode", reason: "aggregate_decode_budget_exhausted", omitted: 1, })); await rm(join(resources, "A.nib")); const recovered = await analyzeInterfaceBuilderBundle({ bundlePath: bundle, targetSha256: "f".repeat(64) }); expect(recovered.documents.map((d) => d.relative_path)).toEqual(["Contents/Resources/B.nib"]); expect(recovered.graph.nodes).toEqual(control.graph.nodes); console.log(JSON.stringify({ invalidInputBytes: Buffer.byteLength(invalid), validInputBytes: Buffer.byteLength(valid), invalidBytes, validBytes, controlDocuments: control.documents.map((d) => d.relative_path), controlNodes: control.graph.nodes.map(({kind, name}) => ({kind, name})), documents: result.documents, releasedBytes: release.mock.calls.map(([bytes]) => bytes), recoveredDocuments: recovered.documents.map((d) => d.relative_path), coverage: result.graph.coverage.find((c) => c.facet === "archive_decode"), limitations: result.limitations }, null, 2)); } finally { vi.restoreAllMocks(); await rm(root, { recursive: true, force: true }); } }); ``` Final execution: one focused file and one test passed, including the valid-only control, malformed-plus-valid failure and recovery after removing the malformed file. No external analysis engine or target application ran. ### Expected behavior Report A as invalid and return its representation reservation because no decoded document was retained. B should still decode when its own representation fits the remaining budget of successfully retained documents. Coverage should remain partial for the invalid archive, with `one_or_more_archives_invalid` and no budget omission caused by A's discarded representation. The [budget owner](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/apple/InterfaceBuilderDecodeBudget.ts#L38-L65) defines these as representation reservations and exposes `release` for rejected, unretained representations. The [existing catch comment](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/apple/InterfaceBuilderAnalysis.ts#L211-L218) and [boundary test](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/tests/boundary/filesystem/interfaceBuilderBundle.test.ts#L1073-L1098) already apply this rule to a post-reservation depth rejection. Generic format failures retain no representation either. Preserve the independent input-byte limit and real representation bounds. ### Impact and scope P2: valid Interface Builder documents and recognized controls can disappear from the decoded result when an earlier malformed plist consumes enough of the aggregate representation allowance. The result explicitly reports partial coverage, but the budget omission is caused by accounting for a rejected archive. There is no observed persistent state change or security-boundary crossing. A demonstrated workaround is to inspect a disposable copy of the bundle with the invalid archive excluded; this restores the valid document and exact original graph in the fixture. Keep the original artifact for provenance. Real-app frequency and the regression introduction are unknown. The default-budget consequence follows the same accounting path, but no large default-budget fixture was executed. ### REA version `rea-agents` 6.7.0; source commit `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform macOS 27.0.1 arm64; Node.js v26.12.0; locked Vitest 5.0.3. ### Target and artifact context Synthetic app bundle with two XML `.nib` resources: a well-formed non-plist root in A.nib and an ibtool-style object dictionary containing one NSButton named Build in B.nib. No executable or native provider is needed for this production-boundary test. ### Interface Other ### Earliest failure stage Native analysis or reconstruction ### Relevant output or evidence Observed test output, reduced to relevant fields: ```json { "initialBudgetBytes": 8192, "invalidInputBytes": 111, "validInputBytes": 200, "invalidBytes": 7100, "validBytes": 5920, "controlDocuments": ["Contents/Resources/B.nib"], "controlNodes": [ {"kind": "scene", "name": "Contents/Resources/B.nib"}, {"kind": "control", "name": "Build"} ], "documents": [], "releasedBytes": [0], "recoveredDocuments": ["Contents/Resources/B.nib"], "coverage": { "facet": "archive_decode", "status": "partial", "reason": "aggregate_decode_budget_exhausted", "examined": 2, "omitted": 1 } } ``` Relevant limitations: ```text Some Interface Builder archives could not be decoded: Contents/Resources/A.nib: malformed document. First element should be <plist> Contents/Resources/B.nib: omitted because aggregate decode budget exhausted. ``` The only release is `release(0)` when B fails preflight; A's 7100-byte reservation remains charged. The test also asserts that the failed run has no graph nodes and the recovered run has exactly the control's nodes. Source reachability: [CLI registration](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/artifactCommands.ts#L149-L196) dispatches `decode_interface_builder`; the [production artifact provider](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/ArtifactProvider.ts#L223-L240) calls the same analyzer. The [tool contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/contracts/artifactToolContracts.ts#L91-L108) exposes that operation to MCP. These establish public reachability without claiming a transport execution. ### Typed outcome or recovery expected After repair, use the same malformed-plus-valid fixture and require B's document and Build control to remain, A's invalid-format limitation to remain, and no omitted archive attributed to aggregate decode exhaustion. The valid-only and recovered controls should continue to produce identical graph nodes. Actual over-budget archives and the 32 MiB aggregate input bound must remain enforced. No repair was made or tested here. ### Analysis or suspected cause Source-derived: [XML preflight](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/apple/PropertyListDecodeBudget.ts#L51-L85) estimates text and element expansion without validating the plist root. The [analyzer](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/apple/InterfaceBuilderAnalysis.ts#L149-L177) reserves that estimate before parsing. A non-plist root then throws a plain TypeError in the [shared XML decoder](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/propertyListXml.ts#L13-L19). The [catch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/apple/InterfaceBuilderAnalysis.ts#L210-L223) releases the reservation only for `InterfaceBuilderDecodeBudgetExceeded`, while the generic invalid branch keeps it. Returning the reservation for every failed plist decode that retained no representation would align both failure paths. This is an accounting issue rather than a request to raise the allowance. ### Additional context Freshness: fetched `origin/main` at `c76edbebbe1e25c2bf821ec04d08dc851e37626c`; the analyzer, budget class, shared preflight and XML decoder have no changes from the tested baseline. This is a source comparison, not an executed test on that newer revision. Related reports about XML value-depth admission and its typed budget release, object-versus-connection limit attribution, ordinary plist depth rejection, and retained-graph paging do not cover generic malformed-archive reservation retention. Last known good version: NOT_ENOUGH_INFO; none established.

bsakaguchi closed this issue

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

bsakaguchi referenced this issue

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

[PR #3589] Feat/pri resources #5843

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