[PR #2752] [CLOSED] fix(artifacts): recognize XML property lists by content #5364

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

📋 Pull Request Information

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

Base: main ← Head: fix/xml-plist-content-detection


📝 Commits (4)

  • f5dc48c fix(artifacts): recognize XML property lists by content
  • 56db60e fix(artifacts): share exact XML plist content classification
  • 240c88f style(artifacts): format plist recognition fixes
  • 4f18aad fix(artifacts): preserve XML filename roles and valid fixture ordering

📊 Changes

4 files changed (+160 additions, -3 deletions)

View changed files

📝 src/application/BinaryTargetResolver.ts (+28 -1)
📝 src/artifacts/inventory/ArtifactGraphConstruction.ts (+12 -2)
📝 src/domain/apple/appleArchiveFormat.ts (+36 -0)
📝 tests/boundary/filesystem/binaryTarget.test.ts (+84 -0)

📄 Description

Summary

Fixes #2751

A standalone XML NSKeyedArchiver archive was rejected as unsupported binary format unless its name ended in .plist. Binary plists are already recognized by their bplist00 signature under any name, so the same archive in binary form opened fine. Target opening now also recognizes XML property lists by content, and inspect_keyed_archive opens them regardless of extension, as docs/native-investigation.md already describes ("reads XML or binary Foundation keyed archives").

Problem and expected behavior

Smallest trigger: an XML keyed archive saved as state.archive, then rea inspect-keyed-archive state.archive. Before this change it fails with file_format_rejected / unsupported binary format; the same bytes named state.plist succeed. Violated invariant: plist detection depended on encoding (binary by content, XML by name only). Expected: the target resolves as { kind: "artifact", format: "plist" } and the reader returns archive_format: "xml-plist".

Change and scope

  • src/domain/apple/appleArchiveFormat.ts: add hasXmlPlistSignature, next to hasBinaryPlistSignature. It inspects at most 1024 leading bytes and accepts an optional UTF-8 or UTF-16 LE/BE BOM, leading whitespace, an XML declaration, and comments before <!DOCTYPE plist or a <plist root. It does not validate the body; decoding stays in KeyedArchiveReader / decodeXmlPlistText.
  • src/application/BinaryTargetResolver.ts: detectArtifactFormat checks the XML signature right after the binary plist check. Names that select another text format (.asar, .map, .js/.mjs/.cjs) keep their existing classification, and .plist names behave as before.

Not included: content detection for nested inventory entries (classifyArtifactBytes), or any change to plist decoding or limits.

Follow the contribution scope.
Scope decision: not applicable. This is an ordinary fix within the existing inspect_keyed_archive boundary.

Contract and boundary impact

  • Semantic owner and earliest changed stage: target format detection (detectArtifactFormat) during artifact opening.
  • Contribution scope fit and linked maintainer decision: routine fix within existing boundaries; not applicable.
  • CLI and MCP/tool-catalog contract: unchanged. Inputs that were rejected now open as plist artifacts.
  • Provider, bridge, target-format, or platform compatibility: there is no new format value. Files that are XML plists by content now resolve to the existing plist artifact format. Other XML, and files named as JS, source maps or ASAR, are unchanged.
  • Evidence, artifact, provenance, or reconstruction contract: unchanged; the subject format is plist, as for .plist names.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): one additional bounded read (≤1024 bytes) from the already-open handle, with a cancellation check.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • 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 and regression coverage

  • Tests added or updated: tests/boundary/filesystem/binaryTarget.test.ts.
    • XML plists named state.archive open as plist in four forms: a declaration plus doctype, a UTF-8 BOM with leading whitespace, a UTF-16LE BOM, and a bare <plist> root.
    • Non-plist XML (state.xml containing <svg/>) and a .js file containing <plist> are not classified as plist.
  • Base reproduction or other evidence (executed): on the base revision 9928f357f, all four positive cases fail and the two negative cases pass. With this change, all six pass.
  • User-visible CLI/MCP output (if applicable): rea inspect-keyed-archive /tmp/reat/kax.archive --format json now returns "subject":{"name":"kax.archive",...,"format":"plist"} with the object graph. Before, it returned {"code":"target_unavailable","category":"unsupported_target","message":"Cannot open artifact: unsupported binary format."}.
  • Remaining proof gaps: no MCP-route run (it shares the same target resolver), and not verified on macOS or Windows.

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

  • npx vitest run tests/boundary/filesystem/binaryTarget.test.ts src/artifacts/apple src/domain/apple: 12 files passed, 245 tests passed, 1 skipped.
  • Same binaryTarget.test.ts without the source change: the 4 new positive cases fail.
  • npx vitest run tests/boundary/filesystem src/artifacts src/application/javascript: 2323 passed, 7 failed. The 7 failures are in productCatalog, referenceSourceReader, releaseCheckpoint and setupSkill, and the same 7 fail on the unmodified base in this environment.
  • npm run check:fast (typecheck + lint, including module boundaries): passed.
  • npx oxfmt --check src tests: all files formatted.
  • npm run verify:test-discovery: "Discovered all 1048 test files exactly once."

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Inputs that were rejected now open; inputs that already opened classify the same way.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable (static artifact path); run on Debian 13 x64, Node 24.18.0.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the prefix read is bounded to 1024 bytes. Decoding keeps its existing size, depth and encoding limits.

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/2752 **Author:** [@michaelkillgta](https://github.com/michaelkillgta) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/xml-plist-content-detection` --- ### 📝 Commits (4) - [`f5dc48c`](https://github.com/morluto/rea/commit/f5dc48c88cfbfca41f46494035103cccf33e4735) fix(artifacts): recognize XML property lists by content - [`56db60e`](https://github.com/morluto/rea/commit/56db60e4a0a685cf822b9837f60df31036ffd7c6) fix(artifacts): share exact XML plist content classification - [`240c88f`](https://github.com/morluto/rea/commit/240c88f828bd977ca963115c2bb080cd1b1911a7) style(artifacts): format plist recognition fixes - [`4f18aad`](https://github.com/morluto/rea/commit/4f18aadba86bf708d69b34a940699af8891c72d4) fix(artifacts): preserve XML filename roles and valid fixture ordering ### 📊 Changes **4 files changed** (+160 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/application/BinaryTargetResolver.ts` (+28 -1) 📝 `src/artifacts/inventory/ArtifactGraphConstruction.ts` (+12 -2) 📝 `src/domain/apple/appleArchiveFormat.ts` (+36 -0) 📝 `tests/boundary/filesystem/binaryTarget.test.ts` (+84 -0) </details> ### 📄 Description ## Summary Fixes #2751 A standalone XML `NSKeyedArchiver` archive was rejected as `unsupported binary format` unless its name ended in `.plist`. Binary plists are already recognized by their `bplist00` signature under any name, so the same archive in binary form opened fine. Target opening now also recognizes XML property lists by content, and `inspect_keyed_archive` opens them regardless of extension, as `docs/native-investigation.md` already describes ("reads XML or binary Foundation keyed archives"). ## Problem and expected behavior Smallest trigger: an XML keyed archive saved as `state.archive`, then `rea inspect-keyed-archive state.archive`. Before this change it fails with `file_format_rejected` / `unsupported binary format`; the same bytes named `state.plist` succeed. Violated invariant: plist detection depended on encoding (binary by content, XML by name only). Expected: the target resolves as `{ kind: "artifact", format: "plist" }` and the reader returns `archive_format: "xml-plist"`. ## Change and scope - `src/domain/apple/appleArchiveFormat.ts`: add `hasXmlPlistSignature`, next to `hasBinaryPlistSignature`. It inspects at most 1024 leading bytes and accepts an optional UTF-8 or UTF-16 LE/BE BOM, leading whitespace, an XML declaration, and comments before `<!DOCTYPE plist` or a `<plist` root. It does not validate the body; decoding stays in `KeyedArchiveReader` / `decodeXmlPlistText`. - `src/application/BinaryTargetResolver.ts`: `detectArtifactFormat` checks the XML signature right after the binary plist check. Names that select another text format (`.asar`, `.map`, `.js`/`.mjs`/`.cjs`) keep their existing classification, and `.plist` names behave as before. Not included: content detection for nested inventory entries (`classifyArtifactBytes`), or any change to plist decoding or limits. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Scope decision: not applicable. This is an ordinary fix within the existing `inspect_keyed_archive` boundary. ## Contract and boundary impact - Semantic owner and earliest changed stage: target format detection (`detectArtifactFormat`) during artifact opening. - Contribution scope fit and linked maintainer decision: routine fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: unchanged. Inputs that were rejected now open as `plist` artifacts. - Provider, bridge, target-format, or platform compatibility: there is no new format value. Files that are XML plists by content now resolve to the existing `plist` artifact format. Other XML, and files named as JS, source maps or ASAR, are unchanged. - Evidence, artifact, provenance, or reconstruction contract: unchanged; the subject `format` is `plist`, as for `.plist` names. - Target selection, authority, upstream behavior, and cleanup details (if applicable): one additional bounded read (≤1024 bytes) from the already-open handle, with a cancellation check. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [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 and regression coverage - Tests added or updated: `tests/boundary/filesystem/binaryTarget.test.ts`. - XML plists named `state.archive` open as `plist` in four forms: a declaration plus doctype, a UTF-8 BOM with leading whitespace, a UTF-16LE BOM, and a bare `<plist>` root. - Non-plist XML (`state.xml` containing `<svg/>`) and a `.js` file containing `<plist>` are not classified as `plist`. - Base reproduction or other evidence (executed): on the base revision 9928f357f, all four positive cases fail and the two negative cases pass. With this change, all six pass. - User-visible CLI/MCP output (if applicable): `rea inspect-keyed-archive /tmp/reat/kax.archive --format json` now returns `"subject":{"name":"kax.archive",...,"format":"plist"}` with the object graph. Before, it returned `{"code":"target_unavailable","category":"unsupported_target","message":"Cannot open artifact: unsupported binary format."}`. - Remaining proof gaps: no MCP-route run (it shares the same target resolver), and not verified on macOS or Windows. 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 - `npx vitest run tests/boundary/filesystem/binaryTarget.test.ts src/artifacts/apple src/domain/apple`: 12 files passed, 245 tests passed, 1 skipped. - Same `binaryTarget.test.ts` without the source change: the 4 new positive cases fail. - `npx vitest run tests/boundary/filesystem src/artifacts src/application/javascript`: 2323 passed, 7 failed. The 7 failures are in `productCatalog`, `referenceSourceReader`, `releaseCheckpoint` and `setupSkill`, and the same 7 fail on the unmodified base in this environment. - `npm run check:fast` (typecheck + lint, including module boundaries): passed. - `npx oxfmt --check src tests`: all files formatted. - `npm run verify:test-discovery`: "Discovered all 1048 test files exactly once." ## Compatibility, safety, and release - Breaking changes or migration steps: none. Inputs that were rejected now open; inputs that already opened classify the same way. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (static artifact path); run on Debian 13 x64, Node 24.18.0. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the prefix read is bounded to 1024 bytes. Decoding keeps its existing size, depth and encoding limits. ## 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:03 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#5364
No description provided.