morluto/rea

Fork 0

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

[PR #6809] [CLOSED] fix(cli): keep own proto members selectable in output filters #7416

New issue

Closed

opened 2026-11-01 21:47:42 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:42 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6809
Author: @amyanger
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/6756-filter-output-own-proto

📝 Commits (1)

cf9e6d5 fix(cli): keep own proto members selectable in output filters

📊 Changes

3 files changed (+79 additions, -6 deletions)

View changed files

📝 src/cli/cliResultOutput.ts (+13 -6)
📝 tests/boundary/cli/evidenceBundleCommands.test.ts (+50 -0)
📝 tests/boundary/cli/filteredArrayOutput.test.ts (+16 -0)

📄 Description

Summary

Fixes #6756.

--filter-output silently dropped a selected own JSON member named __proto__. With the issue's bundle, inspect-evidence-bundle ... --json --filter-output 'records[0,1].normalized_result.__proto__.answer' printed {"records":[{"normalized_result":{}}]} and exited 0. It now prints {"records":[{"normalized_result":{"__proto__":{"answer":7}}}]}.

The same projection also invented a member that does not exist. --filter-output __proto__ printed {"__proto__":{}} (read from Object.prototype) for a result without that member. It now prints {}, the same as any missing key.

Problem and expected behavior

projectObjectPaths in src/cli/cliResultOutput.ts built its projection with result[key] = ... on a plain {}. For key === "__proto__" that assignment calls the inherited setter and changes the prototype, so the member never becomes an own key and JSON.stringify drops it. Reads used value[key] without an own-property check, so a missing __proto__ resolved to Object.prototype.

Invariant: a filter selects own JSON data from the command result, so filtered output is a subset of the unfiltered output. src/domain/jsonValue.ts keeps all own data members, including __proto__.

Change and scope

projectObjectPaths collects [key, value] entries and returns Object.fromEntries(entries). Object.fromEntries defines own data properties, so __proto__ stays an own key on an ordinary object.
Both projectObjectPaths and the single-key fast path in applyCliOutputFilter skip keys for which Object.hasOwn(value, key) is false. This follows src/application/javascript/JavaScriptJsonDocuments.ts, which reads JSON members with Object.hasOwn.
I tried a null-prototype result object first. Incur's Markdown table renderer calls String() on nested cell objects, which throws for a null prototype, and two existing md cases in tests/boundary/cli/filteredArrayOutput.test.ts failed. Object.fromEntries keeps an ordinary prototype, so all formats work.

Not included: Markdown escaping (#6755) and other filter grammar changes.

Contract and boundary impact

Semantic owner and earliest changed stage: CLI result output projection (src/cli/cliResultOutput.ts), after the command returns and before formatting.
Contribution scope fit and linked maintainer decision: not applicable, ordinary fix within an existing boundary.
CLI and MCP/tool-catalog contract: CLI --filter-output only. Own members, including __proto__, are selectable. Inherited members (prototype getters, Map#size, members of Object.create(proto) objects) are no longer selectable. They never appear in unfiltered --json output either. MCP is unaffected.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none. Stored Evidence is unchanged; only CLI projection of it changes.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.

Expected effects: none of the listed target effects. The change only alters how an already computed CLI result is projected.

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/cli/evidenceBundleCommands.test.ts, "selects own proto members without inventing inherited ones". It runs the compiled CLI inspect-evidence-bundle with the issue's bundle and checks four filters: the issue's selection, a parent plus sibling selection, a nested missing __proto__ compared with a nested missing ordinary key, and a top-level __proto__ compared with a top-level missing key.
- tests/boundary/cli/filteredArrayOutput.test.ts, "retains a selected own proto member in %s". It covers json, jsonl, yaml, toon and md through the existing in-process runFilteredCli harness.
Base reproduction (executed): with src/cli/cliResultOutput.ts from base 60accad2d and the new test, npm run test:focused -- tests/boundary/cli/evidenceBundleCommands.test.ts gives 1 failed, 6 passed. The failure is at line 131, the issue's selection, with received "normalized_result": {}. With base source, npm run test:focused -- tests/boundary/cli/filteredArrayOutput.test.ts gives 5 failed, 25 passed: the new case fails in all five formats.
Mutation check (executed): I reverted each part of the fix separately and ran the new test. Each reverted part failed a different assertion.
- Removing hasOwn from the fast path failed the top-level __proto__ assertion.
- Removing hasOwn from projectObjectPaths failed the nested missing __proto__ assertion.
- Replacing Object.fromEntries with assignment into {} failed the issue's selection, and all five format cases.
- A null-prototype result object in place of Object.fromEntries passes the new cases but fails two existing md cases in filteredArrayOutput.test.ts.

User-visible CLI output (executed, the issue's reproduce.mjs against the built package):

Filter	Base	Branch
`records[0,1].normalized_result.__proto__.answer`	`{"records":[{"normalized_result":{}}]}`	`{"records":[{"normalized_result":{"__proto__":{"answer":7}}}]}`
`records[0,1].normalized_result.normal.answer`	`{"records":[{"normalized_result":{"normal":{"answer":7}}}]}`	same
`records[0,1].normalized_result`	own `__proto__` and `normal`	same
`__proto__`	`{"__proto__":{}}`	`{}`
`missing`	`{}`	`{}`

Remaining proof gaps: I did not trace every command's return type. A command that returns a class instance and is filtered on a prototype getter would now get the missing-key result instead of the getter value. A review found no result type defining toJSON, and existing tests already require inherited methods (toString, constructor) to be absent from filtered output.

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

All commands ran on macOS arm64 with Node v24.22.1. Base is upstream 60accad2d. The newest main commit with completed green CI, 1635eadfc, predates a4bf6f83a (which added projectObjectPaths), so this fix cannot sit on it; I will rebase once main is green on a commit that includes that change.

npm run test:focused -- tests/boundary/cli/evidenceBundleCommands.test.ts tests/boundary/cli/filteredArrayOutput.test.ts tests/boundary/cli/cliOutput.test.ts tests/boundary/cli/filteredFailureOutput.test.ts tests/boundary/cli/jsonOutput.test.ts tests/boundary/cli/cliTokenWindowOutput.test.ts tests/boundary/cli/javascriptApplicationJsonOutput.test.ts tests/boundary/cli/projectedErrorStatus.test.ts: 8 files, 153 tests passed.
npm run check:fast: passed.
npm run test:changed -- --base 60accad2d: 3 files, 18 tests passed. This command selects only source-level projects, so the CLI boundary files are covered by the test:focused run above.
npm run check: exit 0, architecture guards passed across 1907 source files.
Independent review (separate worktree, read-only on the checkout): 9 filter-related files, 132 tests passed. It compared base and branch output for 24 values covering prototype getters, Map, Proxy, Date, Buffer, frozen objects, strings, arrays, non-enumerable own getters and key order. Only inherited members changed. Key order, including integer-like keys, was identical.
/code-review (high) on the uncommitted diff reported seven findings. Handling:
- Non-JSON formats were untested for own __proto__. Fixed: added the format case above.
- Object.hasOwn admits non-enumerable own members (an array's length, an Error's stack). Not changed: base selects them the same way (the independent review saw identical base and branch output for array length and a non-enumerable own getter), and an enumerable-only check would stop rows.length from being selectable. That is a separate behavior decision.
- constructor and toString are not covered by the new test. Not changed: before this change such selections put only a function into the projection, which JSON output omits; removing the hasOwn guard already fails the new test; existing tests in filteredArrayOutput.test.ts and javascriptApplicationJsonOutput.test.ts pin toString and constructor.
- The public CLI test is slower than the in-process harness. Kept: docs/testing.md prefers public CLI workflows, and the issue reproduces through inspect-evidence-bundle.
- Mixed slice and key siblings under one array key (rows[0,1].id,rows.length) drop the key paths, and nested slices (matrix[0,2][0,1]) project to {}. Not changed: both behave the same on base, and both are outside this fix.
Real provider: not applicable. The changed code is the CLI output layer, exercised above through the compiled public CLI.

Compatibility, safety, and release

Breaking changes or migration steps: none intended. Filtering on an inherited member of a non-JSON result now yields the missing-key result. No documented behavior or test relies on that.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the filter no longer writes through the __proto__ setter on its projection object and no longer reads Object.prototype into output.

Author readiness

Human owner: @amyanger

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.

_{🔄 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/6809 **Author:** [@amyanger](https://github.com/amyanger) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/6756-filter-output-own-proto` --- ### 📝 Commits (1) - [`cf9e6d5`](https://github.com/morluto/rea/commit/cf9e6d5f9f8f29803e8a4d0f0ef67ee2b0e617ae) fix(cli): keep own __proto__ members selectable in output filters ### 📊 Changes **3 files changed** (+79 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/cliResultOutput.ts` (+13 -6) 📝 `tests/boundary/cli/evidenceBundleCommands.test.ts` (+50 -0) 📝 `tests/boundary/cli/filteredArrayOutput.test.ts` (+16 -0) </details> ### 📄 Description ## Summary Fixes #6756. `--filter-output` silently dropped a selected own JSON member named `__proto__`. With the issue's bundle, `inspect-evidence-bundle ... --json --filter-output 'records[0,1].normalized_result.__proto__.answer'` printed `{"records":[{"normalized_result":{}}]}` and exited 0. It now prints `{"records":[{"normalized_result":{"__proto__":{"answer":7}}}]}`. The same projection also invented a member that does not exist. `--filter-output __proto__` printed `{"__proto__":{}}` (read from `Object.prototype`) for a result without that member. It now prints `{}`, the same as any missing key. ## Problem and expected behavior `projectObjectPaths` in `src/cli/cliResultOutput.ts` built its projection with `result[key] = ...` on a plain `{}`. For `key === "__proto__"` that assignment calls the inherited setter and changes the prototype, so the member never becomes an own key and `JSON.stringify` drops it. Reads used `value[key]` without an own-property check, so a missing `__proto__` resolved to `Object.prototype`. Invariant: a filter selects own JSON data from the command result, so filtered output is a subset of the unfiltered output. `src/domain/jsonValue.ts` keeps all own data members, including `__proto__`. ## Change and scope - `projectObjectPaths` collects `[key, value]` entries and returns `Object.fromEntries(entries)`. `Object.fromEntries` defines own data properties, so `__proto__` stays an own key on an ordinary object. - Both `projectObjectPaths` and the single-key fast path in `applyCliOutputFilter` skip keys for which `Object.hasOwn(value, key)` is false. This follows `src/application/javascript/JavaScriptJsonDocuments.ts`, which reads JSON members with `Object.hasOwn`. - I tried a null-prototype result object first. Incur's Markdown table renderer calls `String()` on nested cell objects, which throws for a null prototype, and two existing `md` cases in `tests/boundary/cli/filteredArrayOutput.test.ts` failed. `Object.fromEntries` keeps an ordinary prototype, so all formats work. Not included: Markdown escaping (#6755) and other filter grammar changes. ## Contract and boundary impact - Semantic owner and earliest changed stage: CLI result output projection (`src/cli/cliResultOutput.ts`), after the command returns and before formatting. - Contribution scope fit and linked maintainer decision: not applicable, ordinary fix within an existing boundary. - CLI and MCP/tool-catalog contract: CLI `--filter-output` only. Own members, including `__proto__`, are selectable. Inherited members (prototype getters, `Map#size`, members of `Object.create(proto)` objects) are no longer selectable. They never appear in unfiltered `--json` output either. MCP is unaffected. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. Stored Evidence is unchanged; only CLI projection of it changes. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: none of the listed target effects. The change only alters how an already computed CLI result is projected. - [ ] 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/cli/evidenceBundleCommands.test.ts`, "selects own __proto__ members without inventing inherited ones". It runs the compiled CLI `inspect-evidence-bundle` with the issue's bundle and checks four filters: the issue's selection, a parent plus sibling selection, a nested missing `__proto__` compared with a nested missing ordinary key, and a top-level `__proto__` compared with a top-level missing key. - `tests/boundary/cli/filteredArrayOutput.test.ts`, "retains a selected own __proto__ member in %s". It covers json, jsonl, yaml, toon and md through the existing in-process `runFilteredCli` harness. - Base reproduction (executed): with `src/cli/cliResultOutput.ts` from base `60accad2d` and the new test, `npm run test:focused -- tests/boundary/cli/evidenceBundleCommands.test.ts` gives 1 failed, 6 passed. The failure is at line 131, the issue's selection, with received `"normalized_result": {}`. With base source, `npm run test:focused -- tests/boundary/cli/filteredArrayOutput.test.ts` gives 5 failed, 25 passed: the new case fails in all five formats. - Mutation check (executed): I reverted each part of the fix separately and ran the new test. Each reverted part failed a different assertion. - Removing `hasOwn` from the fast path failed the top-level `__proto__` assertion. - Removing `hasOwn` from `projectObjectPaths` failed the nested missing `__proto__` assertion. - Replacing `Object.fromEntries` with assignment into `{}` failed the issue's selection, and all five format cases. - A null-prototype result object in place of `Object.fromEntries` passes the new cases but fails two existing `md` cases in `filteredArrayOutput.test.ts`. - User-visible CLI output (executed, the issue's `reproduce.mjs` against the built package): | Filter | Base | Branch | | --- | --- | --- | | `records[0,1].normalized_result.__proto__.answer` | `{"records":[{"normalized_result":{}}]}` | `{"records":[{"normalized_result":{"__proto__":{"answer":7}}}]}` | | `records[0,1].normalized_result.normal.answer` | `{"records":[{"normalized_result":{"normal":{"answer":7}}}]}` | same | | `records[0,1].normalized_result` | own `__proto__` and `normal` | same | | `__proto__` | `{"__proto__":{}}` | `{}` | | `missing` | `{}` | `{}` | - Remaining proof gaps: I did not trace every command's return type. A command that returns a class instance and is filtered on a prototype getter would now get the missing-key result instead of the getter value. A review found no result type defining `toJSON`, and existing tests already require inherited methods (`toString`, `constructor`) to be absent from filtered output. 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 All commands ran on macOS arm64 with Node v24.22.1. Base is upstream `60accad2d`. The newest main commit with completed green CI, `1635eadfc`, predates `a4bf6f83a` (which added `projectObjectPaths`), so this fix cannot sit on it; I will rebase once main is green on a commit that includes that change. - `npm run test:focused -- tests/boundary/cli/evidenceBundleCommands.test.ts tests/boundary/cli/filteredArrayOutput.test.ts tests/boundary/cli/cliOutput.test.ts tests/boundary/cli/filteredFailureOutput.test.ts tests/boundary/cli/jsonOutput.test.ts tests/boundary/cli/cliTokenWindowOutput.test.ts tests/boundary/cli/javascriptApplicationJsonOutput.test.ts tests/boundary/cli/projectedErrorStatus.test.ts`: 8 files, 153 tests passed. - `npm run check:fast`: passed. - `npm run test:changed -- --base 60accad2d`: 3 files, 18 tests passed. This command selects only source-level projects, so the CLI boundary files are covered by the `test:focused` run above. - `npm run check`: exit 0, architecture guards passed across 1907 source files. - Independent review (separate worktree, read-only on the checkout): 9 filter-related files, 132 tests passed. It compared base and branch output for 24 values covering prototype getters, `Map`, `Proxy`, `Date`, `Buffer`, frozen objects, strings, arrays, non-enumerable own getters and key order. Only inherited members changed. Key order, including integer-like keys, was identical. - `/code-review` (high) on the uncommitted diff reported seven findings. Handling: - Non-JSON formats were untested for own `__proto__`. Fixed: added the format case above. - `Object.hasOwn` admits non-enumerable own members (an array's `length`, an `Error`'s `stack`). Not changed: base selects them the same way (the independent review saw identical base and branch output for array `length` and a non-enumerable own getter), and an enumerable-only check would stop `rows.length` from being selectable. That is a separate behavior decision. - `constructor` and `toString` are not covered by the new test. Not changed: before this change such selections put only a function into the projection, which JSON output omits; removing the `hasOwn` guard already fails the new test; existing tests in `filteredArrayOutput.test.ts` and `javascriptApplicationJsonOutput.test.ts` pin `toString` and `constructor`. - The public CLI test is slower than the in-process harness. Kept: `docs/testing.md` prefers public CLI workflows, and the issue reproduces through `inspect-evidence-bundle`. - Mixed slice and key siblings under one array key (`rows[0,1].id,rows.length`) drop the key paths, and nested slices (`matrix[0,2][0,1]`) project to `{}`. Not changed: both behave the same on base, and both are outside this fix. - Real provider: not applicable. The changed code is the CLI output layer, exercised above through the compiled public CLI. ## Compatibility, safety, and release - Breaking changes or migration steps: none intended. Filtering on an inherited member of a non-JSON result now yields the missing-key result. No documented behavior or test relies on that. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the filter no longer writes through the `__proto__` setter on its projection object and no longer reads `Object.prototype` into output. ## Author readiness Human owner: @amyanger - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## 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-11-01 21:47:42 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns

[PR #6809] [CLOSED] fix(cli): keep own __proto__ members selectable in output filters #7416