[PR #4105] [MERGED] fix(apple): report proto entries omitted from OpenStep property lists #6061

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:11:34 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4105
Author: @kaoru0822-kitauji
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/3892-openstep-proto-omission

📝 Commits (2)

beba661 fix(apple): report proto entries omitted from OpenStep property lists
8045be3 fix(apple): count OpenStep compound keys that coerce to proto

📊 Changes

2 files changed (+85 additions, -11 deletions)

View changed files

📝 src/domain/propertyListKeys.test.ts (+34 -0)
📝 src/domain/propertyListKeys.ts (+51 -11)

📄 Description

Summary

Fixes #3892. An OpenStep-format keyed archive with a __proto__ dictionary key lost that entry without any omission limitation. The same archive converted to XML reports 1 dictionary entry keyed __proto__ was omitted…. OpenStep inputs now report the same omission.

Problem and expected behavior

parseXmlPropertyList (src/domain/propertyListKeys.ts) sends OpenStep text to the pinned plist decoder. It then counts own __proto__ properties with omitPrototypeKeys. The decoder (parse-openstep.js) assigns obj[key] = value, so a __proto__ key never becomes an own property:

a string value is ignored;
a dictionary value replaces the object's prototype.

In both cases the count was always 0. A decoded-value walk cannot recover a string-valued entry, so the source has to be counted.

Reproduced with the issue's archive and a real /usr/bin/plutil -convert xml1 copy, through rea inspect-keyed-archive --json:

input	main	this PR
OpenStep	object 1 `{"kept":"visible"}`, `truncated: false`, no omission limitation, 2,274 bytes, 492 ms	same object, `1 dictionary entry keyed __proto__ was omitted because REA results cannot represent that key.`, 2,466 bytes, 442–455 ms (warm)
XML (plutil)	object 1 `{"kept":"visible"}`, omission limitation, 2,470 bytes, 470 ms	unchanged (473 ms)

The OpenStep output grows only by the limitation that was missing. Timing is unchanged.

Change and scope

The existing OpenStep pre-scan already walks every dictionary key to bound nesting before the recursive decoder runs. It is renamed scanOpenStep and now also returns the number of keys that decode to __proto__:
- unquoted keys compare as written;
- quoted keys without escapes compare their contents;
- quoted keys with escapes, and compound keys (a container opened in key position, such as (__proto__)), are decoded by the pinned decoder's own exported parseOpenStep and counted when String(value) is __proto__, the same coercion obj[key] performs, so REA does not reinterpret escapes or coercion. For example, "\_\_proto__" counts, while "\137_proto__" decodes to 137_proto__ and does not.
parseXmlPropertyList reports that count for OpenStep. It still returns the omitPrototypeKeys rebuild, which yields plain objects even when a key replaced a prototype.
The XML and binary paths are unchanged.

Contract and boundary impact

Semantic owner and earliest changed stage: shared property-list parsing (parseXmlPropertyList / OpenStep pre-scan), used by keyed archives and the other plist consumers.
Contribution scope fit and linked maintainer decision: a bug fix within the existing static, read-only boundary; not applicable.
CLI and MCP/tool-catalog contract: schemas unchanged. An OpenStep result with such keys gains the existing omission limitation.
Provider, bridge, target-format, or platform compatibility: OpenStep plists only.
Evidence, artifact, provenance, or reconstruction contract: omissions are now disclosed instead of silent.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: src/domain/propertyListKeys.test.ts adds one case that counts four keys decoding to __proto__: unquoted, quoted, escape-spelled, and a nested key with a dictionary value. The case also checks that a __proto__ value, an array element __proto__ and "\137_proto__" are not counted, and that the nested object keeps Object.prototype. It fails on main (count 0) and passes with the fix.
Real tooling: the XML control was produced by Apple plutil, and both files were inspected through the built CLI as tabulated.
Remaining proof gaps: no shipped OpenStep keyed archive containing __proto__ was available.

Validation performed

npx vitest run src/domain/propertyListKeys.test.ts — 38 passed; against main, 1 failed as expected.
npx vitest run src/domain src/artifacts/apple tests/boundary/filesystem — 376 files: 5,902 passed, 2 failed. The two failures (productCatalog.test.ts, setupSkill.test.ts) compare against generated artifacts that were stale in my local checkout. They pass under npm run test:focused -- tests/boundary/filesystem/productCatalog.test.ts tests/boundary/filesystem/setupSkill.test.ts (26 passed), which regenerates those artifacts, and neither involves this change.
npm run check:fast — passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: macOS plutil for the control file.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no change in how values are decoded. The rebuild still prevents prototype pollution from reaching results.

Author readiness

Human owner:

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

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.

🤖 Generated with Claude Code

_{🔄 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/4105 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/3892-openstep-proto-omission` --- ### 📝 Commits (2) - [`beba661`](https://github.com/morluto/rea/commit/beba6611a82ea0d1a0c38c12eea9357def83b893) fix(apple): report __proto__ entries omitted from OpenStep property lists - [`8045be3`](https://github.com/morluto/rea/commit/8045be3ba73e1874074609663d149f49685ee25a) fix(apple): count OpenStep compound keys that coerce to __proto__ ### 📊 Changes **2 files changed** (+85 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/propertyListKeys.test.ts` (+34 -0) 📝 `src/domain/propertyListKeys.ts` (+51 -11) </details> ### 📄 Description ## Summary Fixes #3892. An OpenStep-format keyed archive with a `__proto__` dictionary key lost that entry without any omission limitation. The same archive converted to XML reports `1 dictionary entry keyed __proto__ was omitted…`. OpenStep inputs now report the same omission. ## Problem and expected behavior `parseXmlPropertyList` (`src/domain/propertyListKeys.ts`) sends OpenStep text to the pinned `plist` decoder. It then counts own `__proto__` properties with `omitPrototypeKeys`. The decoder (`parse-openstep.js`) assigns `obj[key] = value`, so a `__proto__` key never becomes an own property: - a string value is ignored; - a dictionary value replaces the object's prototype. In both cases the count was always 0. A decoded-value walk cannot recover a string-valued entry, so the source has to be counted. Reproduced with the issue's archive and a real `/usr/bin/plutil -convert xml1` copy, through `rea inspect-keyed-archive --json`: | input | main | this PR | |---|---|---| | OpenStep | object 1 `{"kept":"visible"}`, `truncated: false`, **no omission limitation**, 2,274 bytes, 492 ms | same object, **`1 dictionary entry keyed __proto__ was omitted because REA results cannot represent that key.`**, 2,466 bytes, 442–455 ms (warm) | | XML (plutil) | object 1 `{"kept":"visible"}`, omission limitation, 2,470 bytes, 470 ms | unchanged (473 ms) | The OpenStep output grows only by the limitation that was missing. Timing is unchanged. ## Change and scope - The existing OpenStep pre-scan already walks every dictionary key to bound nesting before the recursive decoder runs. It is renamed `scanOpenStep` and now also returns the number of keys that decode to `__proto__`: - unquoted keys compare as written; - quoted keys without escapes compare their contents; - quoted keys with escapes, and compound keys (a container opened in key position, such as `(__proto__)`), are decoded by the pinned decoder's own exported `parseOpenStep` and counted when `String(value)` is `__proto__`, the same coercion `obj[key]` performs, so REA does not reinterpret escapes or coercion. For example, `"\_\_proto__"` counts, while `"\137_proto__"` decodes to `137_proto__` and does not. - `parseXmlPropertyList` reports that count for OpenStep. It still returns the `omitPrototypeKeys` rebuild, which yields plain objects even when a key replaced a prototype. - The XML and binary paths are unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: shared property-list parsing (`parseXmlPropertyList` / OpenStep pre-scan), used by keyed archives and the other plist consumers. - Contribution scope fit and linked maintainer decision: a bug fix within the existing static, read-only boundary; not applicable. - CLI and MCP/tool-catalog contract: schemas unchanged. An OpenStep result with such keys gains the existing omission limitation. - Provider, bridge, target-format, or platform compatibility: OpenStep plists only. - Evidence, artifact, provenance, or reconstruction contract: omissions are now disclosed instead of silent. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `src/domain/propertyListKeys.test.ts` adds one case that counts four keys decoding to `__proto__`: unquoted, quoted, escape-spelled, and a nested key with a dictionary value. The case also checks that a `__proto__` *value*, an array element `__proto__` and `"\137_proto__"` are not counted, and that the nested object keeps `Object.prototype`. It fails on main (count 0) and passes with the fix. - Real tooling: the XML control was produced by Apple `plutil`, and both files were inspected through the built CLI as tabulated. - Remaining proof gaps: no shipped OpenStep keyed archive containing `__proto__` was available. ## Validation performed - `npx vitest run src/domain/propertyListKeys.test.ts` — 38 passed; against main, 1 failed as expected. - `npx vitest run src/domain src/artifacts/apple tests/boundary/filesystem` — 376 files: 5,902 passed, 2 failed. The two failures (`productCatalog.test.ts`, `setupSkill.test.ts`) compare against generated artifacts that were stale in my local checkout. They pass under `npm run test:focused -- tests/boundary/filesystem/productCatalog.test.ts tests/boundary/filesystem/setupSkill.test.ts` (26 passed), which regenerates those artifacts, and neither involves this change. - `npm run check:fast` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS `plutil` for the control file. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no change in how values are decoded. The rebuild still prevents prototype pollution from reaching results. ## Author readiness Human owner:  - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:11:34 +00:00