morluto/rea

Fork 0

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

[GH-ISSUE #4714] [Bug] Keyed archive inspection reports recovered malformed XML as observed values #2461

New issue

Closed

opened 2026-10-17 21:05:05 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-17 21:05:05 +00:00

Owner

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

Before submitting

Searched open and closed issues and open plist PRs; #4042 and #3371 concern character-data loss, not malformed XML admission. PR #4530 addresses inherited entity lookup in a separate numeric-metadata helper.
Report contains generated inert data, without credentials or private target content.

Area

Evidence, artifacts, or reconstruction

Observed behavior

Public CLI inspect-keyed-archive returns ok: true for an XML archive containing an undeclared &unknown; entity. Object 1 has status: observed, value value &unknown;, without a malformed-source limitation. Direct production-decoder probes also accept an unquoted XML attribute and  (the latter becomes a NUL in the observed value).

Reproduction

At source revision 3cfa00a29, build a tiny XML fixture with plist 5.0.0:

build({$archiver:'NSKeyedArchiver',$version:100000,
  $objects:['$null','value'],$top:{root:{UID:1}}})

Replace <string>value</string> with <string>value &unknown;</string>, save as Archive.plist, then run:

node scripts/rea.mjs inspect-keyed-archive Archive.plist --format json --full-output

Actually executed once: exit 0, ok: true, object 1 observed as above. The three malformed direct-decoder probes each succeeded once. Generated ordinary valid archive succeeds as control.

Expected behavior

Reject malformed XML with a format error before projecting recovered values as observations. XML 1.0 requires declared entities, quoted attributes and excludes U+0000: https://www.w3.org/TR/xml/. Literal U+FFFD is valid and must remain accepted.

Impact and scope

Observed on one generated archive through the public CLI and three reduced decoder controls. Source-derived shared boundary affects keyed archives, Interface Builder plists and app executable metadata; their other adapters and live applications were not reproduced. No target execution, entity fetching or code execution claim. Frequency in real files: NOT_ENOUGH_INFO.

REA version

Source 3cfa00a29c1eb531faca9542935446e6d751ff4c.

Runtime and platform

Node 24.18.0, macOS arm64; no Ghidra/Hopper/IDA process involved.

Target and artifact context

Generated inert XML NSKeyedArchiver file, two object-table entries.

Interface / earliest failure stage

CLI / Native analysis or reconstruction (shared static artifact decoder).

Relevant output or evidence

ok: true; normalized object {id:1,kind:'scalar',status:'observed',value:'value &unknown;'}. parseXmlPropertyList uses a default recovering DOMParser for preflight and plist.parse repeats the same recovery during value decoding. Local CLI and decoder JSON transcripts retained. Context7/DeepWiki claims were checked against installed plist 5.0.0 source; the generated strict-validation claim does not match that source.

Typed outcome

Current successful evidence should instead be a typed malformed-format failure. No cleanup or native-resource failure observed.

Proposed check

Validate at the shared XML owner before depth/key/value processing; preserve line endings, literal U+FFFD, entity references, prototype-key omission and depth-limit classifications. Keep OpenStep behavior separate.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4714 ### Before submitting - [x] Searched open and closed issues and open plist PRs; #4042 and #3371 concern character-data loss, not malformed XML admission. PR #4530 addresses inherited entity lookup in a separate numeric-metadata helper. - [x] Report contains generated inert data, without credentials or private target content. ### Area Evidence, artifacts, or reconstruction ### Observed behavior Public CLI `inspect-keyed-archive` returns `ok: true` for an XML archive containing an undeclared `&unknown;` entity. Object 1 has `status: observed`, value `value &unknown;`, without a malformed-source limitation. Direct production-decoder probes also accept an unquoted XML attribute and `` (the latter becomes a NUL in the observed value). ### Reproduction At source revision `3cfa00a29`, build a tiny XML fixture with plist 5.0.0: ```js build({$archiver:'NSKeyedArchiver',$version:100000, $objects:['$null','value'],$top:{root:{UID:1}}}) ``` Replace `<string>value</string>` with `<string>value &unknown;</string>`, save as `Archive.plist`, then run: ```sh node scripts/rea.mjs inspect-keyed-archive Archive.plist --format json --full-output ``` Actually executed once: exit 0, `ok: true`, object 1 observed as above. The three malformed direct-decoder probes each succeeded once. Generated ordinary valid archive succeeds as control. ### Expected behavior Reject malformed XML with a format error before projecting recovered values as observations. XML 1.0 requires declared entities, quoted attributes and excludes U+0000: https://www.w3.org/TR/xml/. Literal U+FFFD is valid and must remain accepted. ### Impact and scope Observed on one generated archive through the public CLI and three reduced decoder controls. Source-derived shared boundary affects keyed archives, Interface Builder plists and app executable metadata; their other adapters and live applications were not reproduced. No target execution, entity fetching or code execution claim. Frequency in real files: NOT_ENOUGH_INFO. ### REA version Source `3cfa00a29c1eb531faca9542935446e6d751ff4c`. ### Runtime and platform Node 24.18.0, macOS arm64; no Ghidra/Hopper/IDA process involved. ### Target and artifact context Generated inert XML NSKeyedArchiver file, two object-table entries. ### Interface / earliest failure stage CLI / Native analysis or reconstruction (shared static artifact decoder). ### Relevant output or evidence `ok: true`; normalized object `{id:1,kind:'scalar',status:'observed',value:'value &unknown;'}`. `parseXmlPropertyList` uses a default recovering DOMParser for preflight and plist.parse repeats the same recovery during value decoding. Local CLI and decoder JSON transcripts retained. Context7/DeepWiki claims were checked against installed plist 5.0.0 source; the generated strict-validation claim does not match that source. ### Typed outcome Current successful evidence should instead be a typed malformed-format failure. No cleanup or native-resource failure observed. ### Proposed check Validate at the shared XML owner before depth/key/value processing; preserve line endings, literal U+FFFD, entity references, prototype-key omission and depth-limit classifications. Keep OpenStep behavior separate.

bsakaguchi added the

triage

bug

labels

2026-10-17 21:05:05 +00:00

bsakaguchi commented

2026-10-17 21:05:07 +00:00

Author

Owner

@morluto commented on GitHub (Oct 17, 2026):

Published c7e9e096d on main. The shared plist boundary now uses the validated XML owner before depth checks, prototype-key substitution and value decoding. Removed its independent recovering DOM parse; OpenStep handling is unchanged.

Executed validation: 46 focused checks (39 plist boundary, 2 app executable metadata, 5 new filesystem syntax controls); 5 final renamed syntax controls; 10 overlap checks (8 captured encoding variants/invalid-byte controls, 1 compiled public CLI prototype-key flow, 1 public MCP prototype-key flow); check:fast; commit/push hooks. The exact malformed-entity public CLI fixture changed from exit 0 / ok:true to exit 1 / artifact_operation_failed with reason:format and undefined-entity detail.

Research: Context7 and DeepWiki queried. DeepWiki claimed an XMLParse validation wrapper that does not exist in installed plist 5.0.0; rejected that generated claim after reading pinned dist/parse.js and executing baseline controls. XML rules checked against XML 1.0.

Cost scope: identical completed tiny production keyed-archive file reads and full projections; all six complete replies byte-identical, 255 raw cl100k tokens each. Baseline 3cfa00a29; first processes separately from two alternating warm repeats.

Phase/build	Elapsed s	CPU s	Sampled process-family peak RSS bytes
First/baseline	0.081811	0.065302	71139328
First/candidate	0.081184	0.063903	59768832
Warm median/baseline	0.080307	0.062759	58064896
Warm median/candidate	0.081264	0.064032	59760640

No performance claim. No true cold-cache, whole-agent/schema/retry, full CLI/MCP task costs or large-file costs measured. CPU uses wait4; PPID-family RSS sampling can miss transients. Heavy profiling remained stopped. No live Apple application or native engine executed.

Remaining related work: plist still owns recursive value decoding and reparses validated source, so its advisory U+FFFD warning can remain on stderr for valid text. Replacing that decoder would change established scalar/key semantics and overlap open #3426/#3475/#4054; those are not fixed by this batch. PR #4530 concerns a separate raw numeric-metadata helper and was not integrated or duplicated. No validating-DTD or external-entity support claim.

Rebased onto independent JavaScript Unicode fix 8c62a4aa3; affected parser/consumer/test source blobs are unchanged from the validated pre-rebase commit.

@morluto commented on GitHub (Oct 17, 2026): Published `c7e9e096d` on main. The shared plist boundary now uses the validated XML owner before depth checks, prototype-key substitution and value decoding. Removed its independent recovering DOM parse; OpenStep handling is unchanged. Executed validation: 46 focused checks (39 plist boundary, 2 app executable metadata, 5 new filesystem syntax controls); 5 final renamed syntax controls; 10 overlap checks (8 captured encoding variants/invalid-byte controls, 1 compiled public CLI prototype-key flow, 1 public MCP prototype-key flow); check:fast; commit/push hooks. The exact malformed-entity public CLI fixture changed from exit 0 / ok:true to exit 1 / artifact_operation_failed with reason:format and undefined-entity detail. Research: Context7 and DeepWiki queried. DeepWiki claimed an XMLParse validation wrapper that does not exist in installed plist 5.0.0; rejected that generated claim after reading pinned dist/parse.js and executing baseline controls. XML rules checked against [XML 1.0](https://www.w3.org/TR/xml/). Cost scope: identical completed tiny production keyed-archive file reads and full projections; all six complete replies byte-identical, 255 raw cl100k tokens each. Baseline 3cfa00a29; first processes separately from two alternating warm repeats. | Phase/build | Elapsed s | CPU s | Sampled process-family peak RSS bytes | | --- | ---: | ---: | ---: | | First/baseline | 0.081811 | 0.065302 | 71139328 | | First/candidate | 0.081184 | 0.063903 | 59768832 | | Warm median/baseline | 0.080307 | 0.062759 | 58064896 | | Warm median/candidate | 0.081264 | 0.064032 | 59760640 | No performance claim. No true cold-cache, whole-agent/schema/retry, full CLI/MCP task costs or large-file costs measured. CPU uses wait4; PPID-family RSS sampling can miss transients. Heavy profiling remained stopped. No live Apple application or native engine executed. Remaining related work: plist still owns recursive value decoding and reparses validated source, so its advisory U+FFFD warning can remain on stderr for valid text. Replacing that decoder would change established scalar/key semantics and overlap open #3426/#3475/#4054; those are not fixed by this batch. PR #4530 concerns a separate raw numeric-metadata helper and was not integrated or duplicated. No validating-DTD or external-entity support claim. Rebased onto independent JavaScript Unicode fix `8c62a4aa3`; affected parser/consumer/test source blobs are unchanged from the validated pre-rebase commit.

bsakaguchi commented

2026-10-17 21:05:08 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 17, 2026):

Nice fix — routing the shared plist boundary through the validated XML owner before depth checks, prototype-key substitution and value decoding closes the gap where the independent recovering DOM parse would admit an undeclared &unknown; entity and still report ok: true. The behavior change on the public CLI fixture (exit 1 / artifact_operation_failed with reason: format) is the right contract for malformed input, and leaving OpenStep handling untouched keeps the blast radius small. One small suggestion: pin the malformed-entity fixture into the regression set so future parser refactors cannot silently reintroduce the recovering-parse path.

@XinwangTao commented on GitHub (Oct 17, 2026): Nice fix — routing the shared plist boundary through the validated XML owner before depth checks, prototype-key substitution and value decoding closes the gap where the independent recovering DOM parse would admit an undeclared `&unknown;` entity and still report `ok: true`. The behavior change on the public CLI fixture (exit 1 / `artifact_operation_failed` with `reason: format`) is the right contract for malformed input, and leaving OpenStep handling untouched keeps the blast radius small. One small suggestion: pin the malformed-entity fixture into the regression set so future parser refactors cannot silently reintroduce the recovering-parse path.

bsakaguchi closed this issue

2026-10-31 00:34:14 +00:00

bsakaguchi commented

2026-10-31 00:34:16 +00:00

Author

Owner

@morluto commented on GitHub (Oct 18, 2026):

Closing as fixed on main after reconciling this report with the landed owner/consumer changes.

c7e9e096d: fix(plist): reject malformed XML before value decoding. Validation: 46 focused checks; 5 final syntax controls; 10 encoding and public CLI/MCP overlap checks; check:fast. Public malformed-entity CLI changed from success to typed format failure. Generated inert artifacts only; no real Apple application or native engine execution.

Evidence scope: this closure uses the landed source, regression coverage and validation already recorded in the linked commits/thread. No new local tests or native-provider reproduction were run during this backlog pass; previously documented platform and real-provider gaps still apply.

@morluto commented on GitHub (Oct 18, 2026):  Closing as fixed on `main` after reconciling this report with the landed owner/consumer changes. - [c7e9e096d](https://github.com/morluto/rea/commit/c7e9e096d3e2444e4c251123b8e914e9c0941f4e): fix(plist): reject malformed XML before value decoding. Validation: 46 focused checks; 5 final syntax controls; 10 encoding and public CLI/MCP overlap checks; check:fast. Public malformed-entity CLI changed from success to typed format failure. Generated inert artifacts only; no real Apple application or native engine execution. Evidence scope: this closure uses the landed source, regression coverage and validation already recorded in the linked commits/thread. No new local tests or native-provider reproduction were run during this backlog pass; previously documented platform and real-provider gaps still apply.

bsakaguchi referenced this issue

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

[PR #2461] [MERGED] fix(setup): resolve DeepSeek Harness home overrides #5244

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#2461

No description provided.

Rows
Columns