[PR #2161] [MERGED] fix(apple): preflight XML plist nesting before keyed archive decoding #5066

New issue

Closed

opened 2026-10-31 01:02:16 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:02:16 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2161
Author: @Zburgers
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/12/2026
Merged by: @morluto

Base: main ← Head: fix/2105-xml-plist-depth-preflight

📝 Commits (2)

7204786 fix(apple): preflight XML plist nesting before keyed archive decoding
171d2b6 fix(apple): check XML nesting against parsed element structure

📊 Changes

7 files changed (+360 additions, -9 deletions)

View changed files

📝 src/artifacts/apple/KeyedArchiveReader.test.ts (+65 -1)
📝 src/domain/apple/plistValue.ts (+6 -1)
📝 src/domain/propertyListKeys.test.ts (+113 -0)
📝 src/domain/propertyListKeys.ts (+60 -7)
➕ tests/boundary/cli/keyedArchiveXmlNestingCli.test.ts (+78 -0)
➕ tests/boundary/filesystem/keyedArchiveXmlNesting.test.ts (+29 -0)
➕ tests/fixtures/keyedArchiveXmlNesting.ts (+9 -0)

📄 Description

Closes #2105.

Problem

inspect_keyed_archive documents a 128-level nesting limit, but the XML plist path reached recursive decoding before any depth check. plist and the prototype-key traversal in parseXmlPropertyList recurse once per nesting level, so a ~75 KiB, 3,000-level XML plist exhausted the call stack and reported Maximum call stack size exceeded instead of the bounded-resource diagnostic that shallower over-limit documents (129, 200) already produced. The CLI classified both as runcated/limit, so this is not a crash or a classification bypass — the diagnostic was the defect. The binary plist path already had an expansion/depth preflight; the XML path did not.

Fix

ssertXmlPlistDepthWithinLimit in src/domain/propertyListKeys.ts runs at the top of the XML branch of parseXmlPropertyList, before the DOM substitution and before plist.parse. It is a single-pass, iterative token scan that counts value nesting the way projectPlistValue does — container members nest one level deeper, dictionary keys name members rather than nesting them, and a self-closing element is the value at its own level — and throws the projection's own RangeError("Plist exceeds 128 nesting levels") against the shared, now exported MAX_PLIST_DEPTH.

Consequences of that shape:

The guard rejects exactly the documents the projection rejected, at the same boundary: a bare 129-level chain and a valid envelope with a 127-level payload both fail before decoding, while an in-limit envelope decodes as before. Verified empirically at 126/127 (envelope) and 128/129 (bare chain).
Callers keep the limit/ runcated classification with no error-handler translation, because the error is the same RangeError type and message the projection already raised for over-limit documents.
The guard sits at the shared decoder boundary, so Interface Builder decoding, app-bundle Info.plist resolution, and native inspect_plist gain the same deterministic limit (Interface Builder already preflights at least as strictly, so its behavior is unchanged). The binary plist path keeps its own expansion preflight.

The XML token pattern mirrors the existing Interface Builder estimator. OpenStep autodetection is untouched by this guard, matching the existing preflight's XML-only depth scope.

Behavior

Before vs. after on the issue's synthetic fixtures (exit code 1 in both cases):

Depth	Before details.detail	After
129 / 200	... Plist exceeds 128 nesting levels	unchanged
3,000 / 4,000	... Maximum call stack size exceeded	... Plist exceeds 128 nesting levels

Top-level code: "truncated", category: "truncated", details.reason: "limit" are unchanged; only the 3,000+ diagnostic becomes the documented limit.

Tests

src/domain/propertyListKeys.test.ts — over-limit rejection with the limit diagnostic for dictionary and array nesting (129/3,000/4,000), in-limit decoding at exactly 128 (including a self-closing empty container chain), and opaque markup (comments, CDATA, doctype with an internal subset) not counting as nesting.
src/artifacts/apple/KeyedArchiveReader.test.ts — the issue's deep fixtures (129/200/3,000/4,000) rejected through decodeKeyedArchiveBytes with the limit error rather than a stack error; a valid NSKeyedArchiver XML envelope at the limit decodes with its leaf value preserved; one level past the limit is rejected before decoding.
ests/boundary/filesystem/keyedArchiveXmlNesting.test.ts — production boundary through inspectBundleKeyedArchive (reason limit).
ests/boundary/cli/keyedArchiveXmlNestingCli.test.ts — the issue's public reproduction through the compiled CLI: exit 1, code: "truncated", category: "truncated", details.reason: "limit".

Validation

Reproduced on Windows Node v24.16.0 before the fix (Maximum call stack size exceeded at 3,000/4,000) and confirmed the deterministic limit diagnostic after, both at module level and through the compiled CLI end to end.

pm run test:local (source tests), plus targeted dapters, domain, oundary, cli-boundary, and mcp-boundary runs for keyed-archive suites. Pre-existing environmental failures on this host (Windows native addon/IDA/setup-TTY dependent files) were confirmed identical on a pristine main checkout and are unrelated.

sc --noEmit, oxlint, oxfmt, erify:module-boundaries, knip, jscpd (gate green), and docs:check.

Not covered here

Real-provider verification (
pm run verify:keyed-archive) requires macOS/Xcode and was not run; the portable CLI boundary test covers the public path instead.
Deeply nested OpenStep ({...}/(...)) plists are not preflighted, consistent with the existing Interface Builder estimator's XML-only depth scope; the XML path is the reported defect.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2161 **Author:** [@Zburgers](https://github.com/Zburgers) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/2105-xml-plist-depth-preflight` --- ### 📝 Commits (2) - [`7204786`](https://github.com/morluto/rea/commit/7204786d605ab87da751acdd88fc3517cc121fb3) fix(apple): preflight XML plist nesting before keyed archive decoding - [`171d2b6`](https://github.com/morluto/rea/commit/171d2b63fee84a16e30a7f7d33d4370fb8765c49) fix(apple): check XML nesting against parsed element structure ### 📊 Changes **7 files changed** (+360 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/apple/KeyedArchiveReader.test.ts` (+65 -1) 📝 `src/domain/apple/plistValue.ts` (+6 -1) 📝 `src/domain/propertyListKeys.test.ts` (+113 -0) 📝 `src/domain/propertyListKeys.ts` (+60 -7) ➕ `tests/boundary/cli/keyedArchiveXmlNestingCli.test.ts` (+78 -0) ➕ `tests/boundary/filesystem/keyedArchiveXmlNesting.test.ts` (+29 -0) ➕ `tests/fixtures/keyedArchiveXmlNesting.ts` (+9 -0) </details> ### 📄 Description Closes #2105. ## Problem inspect_keyed_archive documents a 128-level nesting limit, but the XML plist path reached recursive decoding before any depth check. plist and the prototype-key traversal in parseXmlPropertyList recurse once per nesting level, so a ~75 KiB, 3,000-level XML plist exhausted the call stack and reported Maximum call stack size exceeded instead of the bounded-resource diagnostic that shallower over-limit documents (129, 200) already produced. The CLI classified both as runcated/limit, so this is not a crash or a classification bypass — the diagnostic was the defect. The binary plist path already had an expansion/depth preflight; the XML path did not. ## Fix ssertXmlPlistDepthWithinLimit in src/domain/propertyListKeys.ts runs at the top of the XML branch of parseXmlPropertyList, before the DOM substitution and before plist.parse. It is a single-pass, iterative token scan that counts value nesting the way projectPlistValue does — container members nest one level deeper, dictionary keys name members rather than nesting them, and a self-closing element is the value at its own level — and throws the projection's own RangeError("Plist exceeds 128 nesting levels") against the shared, now exported MAX_PLIST_DEPTH. Consequences of that shape: - The guard rejects exactly the documents the projection rejected, at the same boundary: a bare 129-level chain and a valid envelope with a 127-level payload both fail before decoding, while an in-limit envelope decodes as before. Verified empirically at 126/127 (envelope) and 128/129 (bare chain). - Callers keep the limit/ runcated classification with no error-handler translation, because the error is the same RangeError type and message the projection already raised for over-limit documents. - The guard sits at the shared decoder boundary, so Interface Builder decoding, app-bundle Info.plist resolution, and native inspect_plist gain the same deterministic limit (Interface Builder already preflights at least as strictly, so its behavior is unchanged). The binary plist path keeps its own expansion preflight. The XML token pattern mirrors the existing Interface Builder estimator. OpenStep autodetection is untouched by this guard, matching the existing preflight's XML-only depth scope. ## Behavior Before vs. after on the issue's synthetic fixtures (exit code 1 in both cases): | Depth | Before details.detail | After | | --- | --- | --- | | 129 / 200 | ... Plist exceeds 128 nesting levels | unchanged | | 3,000 / 4,000 | ... Maximum call stack size exceeded | ... Plist exceeds 128 nesting levels | Top-level code: "truncated", category: "truncated", details.reason: "limit" are unchanged; only the 3,000+ diagnostic becomes the documented limit. ## Tests - src/domain/propertyListKeys.test.ts — over-limit rejection with the limit diagnostic for dictionary and array nesting (129/3,000/4,000), in-limit decoding at exactly 128 (including a self-closing empty container chain), and opaque markup (comments, CDATA, doctype with an internal subset) not counting as nesting. - src/artifacts/apple/KeyedArchiveReader.test.ts — the issue's deep fixtures (129/200/3,000/4,000) rejected through decodeKeyedArchiveBytes with the limit error rather than a stack error; a valid NSKeyedArchiver XML envelope at the limit decodes with its leaf value preserved; one level past the limit is rejected before decoding. - ests/boundary/filesystem/keyedArchiveXmlNesting.test.ts — production boundary through inspectBundleKeyedArchive (reason limit). - ests/boundary/cli/keyedArchiveXmlNestingCli.test.ts — the issue's public reproduction through the compiled CLI: exit 1, code: "truncated", category: "truncated", details.reason: "limit". ## Validation - Reproduced on Windows Node v24.16.0 before the fix (Maximum call stack size exceeded at 3,000/4,000) and confirmed the deterministic limit diagnostic after, both at module level and through the compiled CLI end to end. - pm run test:local (source tests), plus targeted dapters, domain, oundary, cli-boundary, and mcp-boundary runs for keyed-archive suites. Pre-existing environmental failures on this host (Windows native addon/IDA/setup-TTY dependent files) were confirmed identical on a pristine main checkout and are unrelated. - sc --noEmit, oxlint, oxfmt, erify:module-boundaries, knip, jscpd (gate green), and docs:check. ## Not covered here - Real-provider verification ( pm run verify:keyed-archive) requires macOS/Xcode and was not run; the portable CLI boundary test covers the public path instead. - Deeply nested **OpenStep** ({...}/(...)) plists are not preflighted, consistent with the existing Interface Builder estimator's XML-only depth scope; the XML path is the reported defect. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:02:16 +00:00