[PR #2759] [CLOSED] fix(apple): detect XML keyed archives by content #5370

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2759
Author: @HeadmasterEggy
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: codex/fix-xml-keyed-archive-extension

📝 Commits (10+)

7766505 fix(apple): detect XML keyed archives by content
3014750 fix(apple): tolerate split XML probe characters
3b07851 fix(apple): align XML plist inventory classification
10f1e54 fix(apple): bound XML plist prolog detection
c3ea2ca fix(apple): scan XML plist prologs linearly
9369894 fix(apple): share bounded incremental XML archive classification
8b0a1f4 fix(apple): preserve byte buffer types across XML probe chunks
c52e244 fix(artifacts): preserve entitlement occurrence roles
62909b7 fix(apple): preserve cancellation during XML probing
42007c0 fix(artifacts): preserve binary plist classification precedence

📊 Changes

7 files changed (+231 additions, -21 deletions)

View changed files

📝 src/application/BinaryTargetResolver.ts (+3 -2)
📝 src/artifacts/inventory/ArtifactGraphConstruction.test.ts (+2 -2)
📝 src/artifacts/inventory/ArtifactGraphConstruction.ts (+1 -2)
📝 src/artifacts/inventory/classify.ts (+15 -2)
📝 tests/boundary/filesystem/artifactInventoryRegularRoot.test.ts (+32 -0)
📝 tests/boundary/filesystem/binaryTarget.test.ts (+152 -13)
📝 tests/boundary/filesystem/targetDiagnostics.test.ts (+26 -0)

📄 Description

Summary

Fixes #2751. Also preserves cancellation semantics when the bounded XML probe is cancelled during its final size check.

Problem and expected behavior

inspect_keyed_archive rejects a valid XML NSKeyedArchiver when its filename is not .plist, although the same document opens when named .plist. Binary plists already use content detection independent of extension. XML plist content should receive the same treatment where no known artifact suffix has selected another format.

Change and scope

The shared target resolver now recognizes a bounded XML plist prolog after checking binary signatures and known named artifact formats. The recognizer supports UTF-8 and BOM-marked UTF-16, optional XML declarations/comments, and plist DOCTYPE or root tags. It identifies the format only; the existing plist reader remains responsible for decoding and validation.

Boundary	Impact
Semantic owner and earliest changed stage	`appleArchiveFormat` signature helper; `BinaryTargetResolver` format classification
Contribution scope fit and linked maintainer decision	Existing local format-inspection bug; no new provider or capability
CLI and MCP/tool-catalog contract	No schema or tool contract changes; both surfaces use the corrected shared resolver
Provider, bridge, target-format, or platform compatibility	XML property lists with alternate suffixes become selectable; recognized `.asar`, `.js`, `.map`, and `.plist` names retain their current classifications
Evidence, artifact, provenance, or reconstruction contract	No change; existing reader preserves the selected file digest and archive path
Target selection, authority, upstream behavior, and cleanup details	Reads only the caller-selected local file through existing analysis; no target execution or mutation
Generated metadata, package, or installation impact	None

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence

Tests added or updated: target-format classification regression, a cancellation-during-final-stat boundary case, built CLI inspection, and stdio MCP open_binary plus inspect_keyed_archive flow.
Base reproduction or other evidence: issue #2751 documents the .archive versus .plist behavior. The base resolver recognizes binary plist magic and named .plist, but has no XML-content detection; the regression tests exercise the fixed case.
User-visible CLI/MCP output: both return a successful keyed-archive result with archive_path: "Model.archive", archive_format: "xml-plist", and the decoded leaf object.
Remaining proof gaps: no real-world archive corpus was run; tests use a source-generated XML archive.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:focused -- tests/boundary/filesystem/binaryTarget.test.ts — 65 tests passed, including cancellation during the final probe stat().
Earlier focused CLI and MCP workflow coverage — 55 tests passed.
npm run check:fast — passed.
git diff --check — passed.
Push pre-check reran npm run check:fast — passed.

Merge Danger

Door: two-way; reverting restores the previous format classification.
Blast Radius: limited to classification of XML plist content without a recognized named format.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: local file classification only; parsing and existing resource limits are unchanged.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/2759 **Author:** [@HeadmasterEggy](https://github.com/HeadmasterEggy) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/fix-xml-keyed-archive-extension` --- ### 📝 Commits (10+) - [`7766505`](https://github.com/morluto/rea/commit/77665054bfc9724ce3e5b6ad7ba763e98fd0441e) fix(apple): detect XML keyed archives by content - [`3014750`](https://github.com/morluto/rea/commit/3014750e8e827305b1f7e883e94a4b34704a6907) fix(apple): tolerate split XML probe characters - [`3b07851`](https://github.com/morluto/rea/commit/3b07851aa61076de28691f09081e76b81bda8582) fix(apple): align XML plist inventory classification - [`10f1e54`](https://github.com/morluto/rea/commit/10f1e5498c0a38085d591547886aae2549af6952) fix(apple): bound XML plist prolog detection - [`c3ea2ca`](https://github.com/morluto/rea/commit/c3ea2ca6ccfa60513a92f469630b3e608dbbda87) fix(apple): scan XML plist prologs linearly - [`9369894`](https://github.com/morluto/rea/commit/93698945d4bc429335149ea0d9ebfd7c132067f5) fix(apple): share bounded incremental XML archive classification - [`8b0a1f4`](https://github.com/morluto/rea/commit/8b0a1f45ccd44dde0bae000282050ef04967723e) fix(apple): preserve byte buffer types across XML probe chunks - [`c52e244`](https://github.com/morluto/rea/commit/c52e2441985ad8568c4f431302dd208b7e919f68) fix(artifacts): preserve entitlement occurrence roles - [`62909b7`](https://github.com/morluto/rea/commit/62909b78b27b2761491216cf0539727ea0a5256f) fix(apple): preserve cancellation during XML probing - [`42007c0`](https://github.com/morluto/rea/commit/42007c0d9d25ebfffb362531fa35b86ecfba8b95) fix(artifacts): preserve binary plist classification precedence ### 📊 Changes **7 files changed** (+231 additions, -21 deletions) <details> <summary>View changed files</summary> 📝 `src/application/BinaryTargetResolver.ts` (+3 -2) 📝 `src/artifacts/inventory/ArtifactGraphConstruction.test.ts` (+2 -2) 📝 `src/artifacts/inventory/ArtifactGraphConstruction.ts` (+1 -2) 📝 `src/artifacts/inventory/classify.ts` (+15 -2) 📝 `tests/boundary/filesystem/artifactInventoryRegularRoot.test.ts` (+32 -0) 📝 `tests/boundary/filesystem/binaryTarget.test.ts` (+152 -13) 📝 `tests/boundary/filesystem/targetDiagnostics.test.ts` (+26 -0) </details> ### 📄 Description ## Summary Fixes #2751. Also preserves cancellation semantics when the bounded XML probe is cancelled during its final size check. ## Problem and expected behavior `inspect_keyed_archive` rejects a valid XML NSKeyedArchiver when its filename is not `.plist`, although the same document opens when named `.plist`. Binary plists already use content detection independent of extension. XML plist content should receive the same treatment where no known artifact suffix has selected another format. ## Change and scope The shared target resolver now recognizes a bounded XML plist prolog after checking binary signatures and known named artifact formats. The recognizer supports UTF-8 and BOM-marked UTF-16, optional XML declarations/comments, and plist DOCTYPE or root tags. It identifies the format only; the existing plist reader remains responsible for decoding and validation. | Boundary | Impact | | --- | --- | | Semantic owner and earliest changed stage | `appleArchiveFormat` signature helper; `BinaryTargetResolver` format classification | | Contribution scope fit and linked maintainer decision | Existing local format-inspection bug; no new provider or capability | | CLI and MCP/tool-catalog contract | No schema or tool contract changes; both surfaces use the corrected shared resolver | | Provider, bridge, target-format, or platform compatibility | XML property lists with alternate suffixes become selectable; recognized `.asar`, `.js`, `.map`, and `.plist` names retain their current classifications | | Evidence, artifact, provenance, or reconstruction contract | No change; existing reader preserves the selected file digest and archive path | | Target selection, authority, upstream behavior, and cleanup details | Reads only the caller-selected local file through existing analysis; no target execution or mutation | | Generated metadata, package, or installation impact | None | Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence - Tests added or updated: target-format classification regression, a cancellation-during-final-stat boundary case, built CLI inspection, and stdio MCP `open_binary` plus `inspect_keyed_archive` flow. - Base reproduction or other evidence: issue #2751 documents the `.archive` versus `.plist` behavior. The base resolver recognizes binary plist magic and named `.plist`, but has no XML-content detection; the regression tests exercise the fixed case. - User-visible CLI/MCP output: both return a successful keyed-archive result with `archive_path: "Model.archive"`, `archive_format: "xml-plist"`, and the decoded `leaf` object. - Remaining proof gaps: no real-world archive corpus was run; tests use a source-generated XML archive. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- tests/boundary/filesystem/binaryTarget.test.ts` — 65 tests passed, including cancellation during the final probe `stat()`. - Earlier focused CLI and MCP workflow coverage — 55 tests passed. - `npm run check:fast` — passed. - `git diff --check` — passed. - Push pre-check reran `npm run check:fast` — passed. ## Merge Danger - Door: two-way; reverting restores the previous format classification. - Blast Radius: limited to classification of XML plist content without a recognized named format. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: local file classification only; parsing and existing resource limits are unchanged. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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