[GH-ISSUE #2105] [Bug] inspect_keyed_archive skips XML plist depth preflight before recursive decoding #688

New issue

Closed

opened 2026-10-15 18:27:24 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:27:24 +00:00

Owner

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

Before submitting

Searched open and closed issues and PRs for XML plist nesting, keyed-archive depth, and preflight handling; no direct report or implementation found.
Used synthetic, source-owned XML without credentials or private artifacts.

Area

Evidence, artifacts, or reconstruction — Foundation keyed archives / inspect_keyed_archive.

Observed behavior

inspect_keyed_archive has a documented 128-level nesting limit, but the XML plist path performs recursive parsing before enforcing it. A deeply nested, approximately 75 KiB XML plist produces Maximum call stack size exceeded rather than a deterministic pre-decode nesting-limit diagnostic.

The existing CLI catches the RangeError and returns reason: "limit" / code: "truncated", so this is not a demonstrated process crash or limit-classification bypass. The defect is that stack exhaustion happens before the advertised depth guard, and the caller receives an implementation-level stack error instead of a well-defined limit.

The binary-plist path already has an expansion/depth preflight; the XML path does not.

Reproduction

Environment tested: Windows, Node.js v24.16.0, npm 11.13.0, REA source revision beb80a07bafc0847600eb635e0323068a386c811. Executed against the compiled public one-shot CLI.

From a built REA checkout, save the following as make-deep-plists.mjs:

import { writeFileSync } from "node:fs";

for (const depth of [200, 3000, 4000]) {
  const xml =
    '<?xml version="1.0" encoding="UTF-8"?>\n' +
    '<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">\n' +
    '<plist version="1.0">\n' +
    "<dict><key>k</key>".repeat(depth) +
    "<string>x</string>" +
    "</dict>".repeat(depth) +
    "\n</plist>\n";
  writeFileSync(`deep-${depth}.plist`, xml);
  console.log(`depth=${depth}, bytes=${Buffer.byteLength(xml)}`);
}

node make-deep-plists.mjs
node scripts/rea.mjs inspect-keyed-archive "<absolute-path-to-deep-200.plist>" --json
node scripts/rea.mjs inspect-keyed-archive "<absolute-path-to-deep-3000.plist>" --json
node scripts/rea.mjs inspect-keyed-archive "<absolute-path-to-deep-4000.plist>" --json

Observed (error details.detail, each command exits 1):

Depth	Input size	Observed diagnostic
200	5,192 bytes	`Cannot decode selected Foundation keyed archive: Plist exceeds 128 nesting levels`
3,000	75,192 bytes	`Cannot decode selected Foundation keyed archive: Maximum call stack size exceeded`
4,000	100,192 bytes	`Cannot decode selected Foundation keyed archive: Maximum call stack size exceeded`

All three report top-level code: "truncated", category: "truncated", details.reason: "limit". A narrower direct reproduction traced the 3,000-level overflow to strip in the compiled propertyListKeys.js, and the 8,000-level overflow to the pinned plist XML parser.

Fixture qualification: these are well-formed XML property lists selected through the public archive CLI, not independently validated NSKeyedArchiver object graphs. They exercise the common XML decoding boundary before keyed-archive graph classification. A valid keyed-archive-envelope regression fixture would strengthen the fix verification.

Expected behavior

Reject XML plist nesting beyond the supported depth before recursive parsing; report the normal bounded-resource error consistently for 129, 200, 3,000, or more levels. Existing in-limit XML/binary plist behavior, Evidence provenance, and __proto__ omission semantics should remain unchanged.

Source-derived root cause

src/artifacts/apple/KeyedArchiveReader.ts: decodeKeyedArchiveBytes calls estimateBinaryPlistExpansion only when binary === true, then sends XML input to parseXmlPropertyList without the XML preflight.
src/domain/propertyListKeys.ts: parseXmlPropertyList invokes plist.parse and withoutKey / strip, with recursive traversal but no local depth guard.
src/domain/apple/plistValue.ts: projectPlistValue eventually enforces the 128-level limit, but only after those recursive passes.
src/artifacts/apple/InterfaceBuilderDecodeBudget.ts: existing preflightPropertyListDecode includes an XML depth check before decoding. decode_interface_builder and app-bundle executable inspection already reuse this path.

The current main inspected subsequently at 3bdc8730bd98e073522bcb8ba86527be402c0cbe still has the XML preflight omission in KeyedArchiveReader.ts; the CLI experiment above has not been rerun on that newer SHA.

Proposed scope / acceptance

Reuse the existing XML preflight at the earliest safe decoder boundary in inspect_keyed_archive, or an equivalent shared check that rejects excessive nesting before recursion.
Preserve the caller-facing limit / truncated classification; note that the shared InterfaceBuilderDecodeBudgetExceeded type currently needs explicit translation in the keyed-archive error handler.
Add a portable XML-depth regression through the actual decoder and at least one public CLI/MCP boundary; include a valid NSKeyedArchiver envelope if practicable.
Retain binary-plist behavior, normal XML decoding, special dictionary-key handling, and digest/Evidence consistency.
Consider whether the same shared parseXmlPropertyList recursion needs a defensive boundary in other call sites; do not conflate this with unverified macOS-specific behavior.

Overlap checked

#1515 covered binary plist expansion preflight (different producer path).
Draft PR #1843 updates XML __proto__ dictionary-key formatting in propertyListKeys.ts; it does not add XML depth preflight to KeyedArchiveReader.ts. I will avoid duplicating that work and coordinate if touching the shared helper is necessary.

Contribution intent

I (@Zburgers) plan to follow up with a focused PR shortly, including a failing regression, the smallest architectural fix, and public-boundary test evidence. Happy to adjust the scope if maintainers prefer the guard at a different shared boundary.

Originally created by @Zburgers on GitHub (Oct 12, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2105 ### Before submitting - [x] Searched open and closed issues and PRs for XML plist nesting, keyed-archive depth, and preflight handling; no direct report or implementation found. - [x] Used synthetic, source-owned XML without credentials or private artifacts. ### Area Evidence, artifacts, or reconstruction — Foundation keyed archives / `inspect_keyed_archive`. ### Observed behavior `inspect_keyed_archive` has a documented **128-level nesting limit**, but the XML plist path performs recursive parsing before enforcing it. A deeply nested, approximately **75 KiB** XML plist produces `Maximum call stack size exceeded` rather than a deterministic pre-decode nesting-limit diagnostic. The existing CLI catches the `RangeError` and returns `reason: "limit"` / `code: "truncated"`, so this is **not a demonstrated process crash or limit-classification bypass**. The defect is that stack exhaustion happens *before* the advertised depth guard, and the caller receives an implementation-level stack error instead of a well-defined limit. The binary-plist path already has an expansion/depth preflight; the XML path does not. ### Reproduction **Environment tested:** Windows, Node.js `v24.16.0`, npm `11.13.0`, REA source revision `beb80a07bafc0847600eb635e0323068a386c811`. Executed against the compiled public one-shot CLI. From a built REA checkout, save the following as `make-deep-plists.mjs`: ```js import { writeFileSync } from "node:fs"; for (const depth of [200, 3000, 4000]) { const xml = '<?xml version="1.0" encoding="UTF-8"?>\n' + '<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">\n' + '<plist version="1.0">\n' + "<dict><key>k</key>".repeat(depth) + "<string>x</string>" + "</dict>".repeat(depth) + "\n</plist>\n"; writeFileSync(`deep-${depth}.plist`, xml); console.log(`depth=${depth}, bytes=${Buffer.byteLength(xml)}`); } ``` ```sh node make-deep-plists.mjs node scripts/rea.mjs inspect-keyed-archive "<absolute-path-to-deep-200.plist>" --json node scripts/rea.mjs inspect-keyed-archive "<absolute-path-to-deep-3000.plist>" --json node scripts/rea.mjs inspect-keyed-archive "<absolute-path-to-deep-4000.plist>" --json ``` **Observed** (error `details.detail`, each command exits 1): | Depth | Input size | Observed diagnostic | | ---: | ---: | --- | | 200 | 5,192 bytes | `Cannot decode selected Foundation keyed archive: Plist exceeds 128 nesting levels` | | 3,000 | 75,192 bytes | `Cannot decode selected Foundation keyed archive: Maximum call stack size exceeded` | | 4,000 | 100,192 bytes | `Cannot decode selected Foundation keyed archive: Maximum call stack size exceeded` | All three report top-level `code: "truncated"`, `category: "truncated"`, `details.reason: "limit"`. A narrower direct reproduction traced the 3,000-level overflow to `strip` in the compiled `propertyListKeys.js`, and the 8,000-level overflow to the pinned `plist` XML parser. **Fixture qualification:** these are well-formed XML property lists selected through the public archive CLI, not independently validated `NSKeyedArchiver` object graphs. They exercise the common XML decoding boundary *before* keyed-archive graph classification. A valid keyed-archive-envelope regression fixture would strengthen the fix verification. ### Expected behavior Reject XML plist nesting beyond the supported depth **before recursive parsing**; report the normal bounded-resource error consistently for 129, 200, 3,000, or more levels. Existing in-limit XML/binary plist behavior, Evidence provenance, and `__proto__` omission semantics should remain unchanged. ### Source-derived root cause - `src/artifacts/apple/KeyedArchiveReader.ts`: `decodeKeyedArchiveBytes` calls `estimateBinaryPlistExpansion` only when `binary === true`, then sends XML input to `parseXmlPropertyList` without the XML preflight. - `src/domain/propertyListKeys.ts`: `parseXmlPropertyList` invokes `plist.parse` and `withoutKey` / `strip`, with recursive traversal but no local depth guard. - `src/domain/apple/plistValue.ts`: `projectPlistValue` eventually enforces the 128-level limit, but only *after* those recursive passes. - `src/artifacts/apple/InterfaceBuilderDecodeBudget.ts`: existing `preflightPropertyListDecode` includes an XML depth check before decoding. `decode_interface_builder` and app-bundle executable inspection already reuse this path. The current `main` inspected subsequently at `3bdc8730bd98e073522bcb8ba86527be402c0cbe` still has the XML preflight omission in `KeyedArchiveReader.ts`; the CLI experiment above has **not** been rerun on that newer SHA. ### Proposed scope / acceptance 1. Reuse the existing XML preflight at the earliest safe decoder boundary in `inspect_keyed_archive`, or an equivalent shared check that rejects excessive nesting before recursion. 2. Preserve the caller-facing `limit` / `truncated` classification; note that the shared `InterfaceBuilderDecodeBudgetExceeded` type currently needs explicit translation in the keyed-archive error handler. 3. Add a portable XML-depth regression through the actual decoder and at least one public CLI/MCP boundary; include a valid `NSKeyedArchiver` envelope if practicable. 4. Retain binary-plist behavior, normal XML decoding, special dictionary-key handling, and digest/Evidence consistency. 5. Consider whether the same shared `parseXmlPropertyList` recursion needs a defensive boundary in other call sites; do not conflate this with unverified macOS-specific behavior. ### Overlap checked - #1515 covered **binary** plist expansion preflight (different producer path). - Draft PR #1843 updates XML `__proto__` dictionary-key formatting in `propertyListKeys.ts`; it **does not** add XML depth preflight to `KeyedArchiveReader.ts`. I will avoid duplicating that work and coordinate if touching the shared helper is necessary. ### Contribution intent **I (@Zburgers) plan to follow up with a focused PR shortly**, including a failing regression, the smallest architectural fix, and public-boundary test evidence. Happy to adjust the scope if maintainers prefer the guard at a different shared boundary.

bsakaguchi

2026-10-15 18:27:24 +00:00

closed this issue
added the
area: cli

bug
labels

bsakaguchi commented

2026-10-15 18:27:26 +00:00

Author

Owner

@maxiaochaos commented on GitHub (Oct 12, 2026):

I'd like to take this — align XML plist preflight with the documented 128-depth limit (same idea as the binary-plist path) so deep nests get a clear diagnostic instead of a stack overflow. Will open a PR shortly.

@maxiaochaos commented on GitHub (Oct 12, 2026): I'd like to take this — align XML plist preflight with the documented 128-depth limit (same idea as the binary-plist path) so deep nests get a clear diagnostic instead of a stack overflow. Will open a PR shortly.

bsakaguchi referenced this issue

2026-10-16 21:15:09 +00:00

[PR #688] [CLOSED] fix(dotnet): report observed method metadata changes #2168

bsakaguchi referenced this issue

2026-10-16 21:15:24 +00:00

[PR #705] [MERGED] fix: integrate 14 approved fix PRs (668,671,673,675,678-681,685-690) #2185