[PR #3065] [CLOSED] fix(analysis-view): harden shared inline Evidence admission #5589

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3065
Author: @Zhong-fan
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/analysis-view-inline-accessors


📝 Commits (10+)

  • 4272af6 fix(analysis-view): reject inline accessors before parsing
  • b4bb702 fix(analysis-view): reject executable array members
  • a58ccd8 fix(analysis-view): bound array admission and reject executable data
  • e987630 chore: merge upstream main and preserve shared inline admission
  • b7f1c81 fix(browser): decode empty DOM snapshot string indexes
  • 7c1e29c fix(javascript): preserve scalar reads and candidate endpoint transport
  • e2f0ab6 fix(ghidra): retain completed helper diagnostics on launch failure
  • 61c15fe test: align regression assertions with current output contracts
  • ee521d6 chore: integrate CI regression fixes into analysis-view PR
  • d4731b0 chore: merge current upstream release guidance

📊 Changes

5 files changed (+372 additions, -168 deletions)

View changed files

📝 src/application/analysisView/AnalysisViewService.test.ts (+243 -17)
📝 src/application/analysisView/AnalysisViewService.ts (+7 -65)
📝 src/domain/analysisView/WebNetworkApiSurfaceAccumulator.test.ts (+25 -1)
📝 src/domain/analysisView/WebNetworkApiSurfaceAccumulator.ts (+7 -85)
➕ src/domain/analysisView/analysisViewDataAdmission.ts (+90 -0)

📄 Description

Summary

Share inline Evidence admission across analysis views so malformed local representations return typed errors before schema parsing or authentication. Reject executable traversal hooks without allocating a key list for every array element.

Fixes #2589.

Problem and expected behavior

A throwing normalized_result getter originally escaped summary/capture inspection. The upstream accessor check duplicates API-surface traversal and can still run Proxy reflection traps or enumerate an entire array before budget rejection. Array iterator/map/constructor hooks and function-valued metadata can execute during downstream validation.

Inline Evidence must be checked before parsing, without invoking caller traversal hooks. Ordinary portable Evidence must continue to authenticate and project without source mutation.

Change and scope

Use one shared admission owner for summary, capture and API-surface inline inputs. Remove the duplicate application traversal. Keep API-surface representation reservations in the existing accumulator; summary/capture use structural admission.

Reject Proxies and functions before reflection. Require standard arrays, inspect indexes incrementally, and reject own iterator/map/constructor overrides with fixed-size checks. Ignored named/symbol array metadata stays inert under the existing JSON cloning semantics. Retained Evidence lookup is unchanged.

Merged fixed main 500e32399a3bc827e17fb4adaa97db3bc0df29b5, preserving its CI-baseline fixes from #3080 and follow-ups. Superseded branch repairs were removed; the final diff is five files. No workflow, threshold or deadline changes.

This is an ordinary validation fix within the contribution scope; a new target capability or authority decision is not applicable.

Contract and boundary impact

  • Owner/earliest changed stage: borrowed inline analysis-view data admission, before parsing/authentication.
  • CLI/MCP/tool catalog: unchanged contracts; both adapters call the shared service.
  • Providers/platforms: no new provider behavior. Verified with Node24.18.0/npm11.16.0 on Windows x64 and WSL Linux, using separate locked dependencies.
  • Evidence/provenance: output, links, parent identities and original source data remain preserved. Admission does not clone, freeze or mutate borrowed input.
  • Target selection/authority/cleanup: no new interaction or ownership rule.
  • Generated metadata/package/installation: unchanged.

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

Root/nested/hidden/indexed getters; array hooks/prototypes; callable constructors; ordinary/revoked/callable Proxies; cycles, aliases, source preservation, authenticated ignored metadata and API-surface budgets. Valid-Evidence success checks are retained.

Three independent agents reviewed the final five-file diff and consumers after conflict resolution. Independent probes confirm zero getter/Proxy-trap calls, stable parent Evidence IDs and bounded array descriptor inspection. The delayed instance-field counterexample in the superseded JavaScript resolver no longer reproduces with the retained main implementation: worker specifier stays unknown, while an unrelated-write control still resolves normally.

Previous CI 38508711978 passed source/static/build and real-browser/recovery lanes, but failed three shard5 tests: browser comparison schema projection, SAZ input-limit error category and Hopper fixture process visibility. All three failures were reproduced locally at d4731b0. Current main supplies their fixes; final-head full suites pass without weakening runtime rejection, input bounds or ownership assertions.

Compiled CLI and SDK MCP tests exercise HAR decoding, retained/inline views and continued server use. Executable object fixtures cannot cross their JSON transport; source tests cover that local input boundary.

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

Validation performed

Final head: 953c4019641b8bb6763bbde95c52f4de156b45ad.

  • npm run check:ci: passed, including typecheck, lint/module boundaries, formatting, Knip, metadata, duplication/TODO checks and architecture guards.
  • Linux npm run test:fast: 504 files, 7375 passed, 8 skipped; exit0, 102.26s. Executed from an exact archive of this commit on the Linux filesystem.
  • Linux npm run test:focused -- tests/boundary/mcp/browserComparisonInput.test.ts tests/boundary/browser/sazWebCapture.test.ts tests/process-global/cleanLinuxHopperRuntime.test.ts tests/boundary/cli/analysisViewCli.test.ts tests/boundary/mcp/analysisViewMcp.test.ts tests/boundary/mcp/webNetworkCaptureMcp.test.ts: 44 passed, 1 skipped, six files; 55.97s.
  • Windows npm run test:focused -- tests/boundary/browser/cdpSvgXlinkHref.test.ts tests/boundary/cli/analysisViewCli.test.ts tests/boundary/mcp/analysisViewMcp.test.ts tests/boundary/mcp/webNetworkCaptureMcp.test.ts: 31 passed, four files; actual Chrome, compiled CLI and SDK MCP; 41.89s. Process-only TEMP/TMP point inside the checkout.
  • git diff --check: passed; tracked tree clean.
  • Windows CLI used the verified native artifact from upstream run38490448169. Its native sources/build script still match this head; SHA256 3ca4fcfe5eb39f13f0250e23de0f38c6cfd33616c664a436cd69ef3f013ac9e2.

Final-head remote CI 38515685028 completed successfully at head 953c4019641b8bb6763bbde95c52f4de156b45ad: all five test shards, Source tests, static/docs/build/native-package checks, Portable tests and CI required passed. Optional lanes skipped by the current change classifier are not claimed as executed. Historical CI results are identified above.

Compatibility, safety, and release

No migration or package changes for ordinary JSON Evidence. Executable local representations fail admission; ignored array metadata follows JSON omission semantics.

Actual Windows/Linux CLI and SDK MCP workflows passed, along with real Chrome tests. Hopper tests use controlled process fixtures; no live Hopper/Ghidra/IDA engine acceptance is claimed.

This is input validation, not arbitrary JavaScript containment or a whole-input hard heap limit. Ordinary-object key enumeration predates this change; summary/capture do not acquire an API-surface representation budget.

Before merge

  • Concrete issue and expected behavior identified.
  • Scope and authority resolved within existing boundaries.
  • Focused final diff and Conventional Commit title.
  • Related issue linked.
  • Tests cover changed behavior and meaningful failures.
  • Owning contracts updated; generated metadata unchanged.
  • Public CLI/MCP workflows exercised.
  • Final diff checked 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/3065 **Author:** [@Zhong-fan](https://github.com/Zhong-fan) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/analysis-view-inline-accessors` --- ### 📝 Commits (10+) - [`4272af6`](https://github.com/morluto/rea/commit/4272af60d8723123ccdd0aaea60a2e70b12b8a93) fix(analysis-view): reject inline accessors before parsing - [`b4bb702`](https://github.com/morluto/rea/commit/b4bb702cc00c66907e14582f1109bc9ee2eefa5c) fix(analysis-view): reject executable array members - [`a58ccd8`](https://github.com/morluto/rea/commit/a58ccd869ad48150b427754355c9642355390287) fix(analysis-view): bound array admission and reject executable data - [`e987630`](https://github.com/morluto/rea/commit/e98763004a3c06735da0105858f293430415c950) chore: merge upstream main and preserve shared inline admission - [`b7f1c81`](https://github.com/morluto/rea/commit/b7f1c81bfe7101f30e9dd23668ea9ffab1d094ab) fix(browser): decode empty DOM snapshot string indexes - [`7c1e29c`](https://github.com/morluto/rea/commit/7c1e29c5c8de5704ef3e8a75042f58abbfe2c37e) fix(javascript): preserve scalar reads and candidate endpoint transport - [`e2f0ab6`](https://github.com/morluto/rea/commit/e2f0ab669b19a081d406767abc213b6ee09b074d) fix(ghidra): retain completed helper diagnostics on launch failure - [`61c15fe`](https://github.com/morluto/rea/commit/61c15fe38b91f709fd74ba074c12b9c9a25a30c2) test: align regression assertions with current output contracts - [`ee521d6`](https://github.com/morluto/rea/commit/ee521d62a9be06186fde89008d0b26c4f21df999) chore: integrate CI regression fixes into analysis-view PR - [`d4731b0`](https://github.com/morluto/rea/commit/d4731b016cec7b37a8d071f16f653727c47c9ac4) chore: merge current upstream release guidance ### 📊 Changes **5 files changed** (+372 additions, -168 deletions) <details> <summary>View changed files</summary> 📝 `src/application/analysisView/AnalysisViewService.test.ts` (+243 -17) 📝 `src/application/analysisView/AnalysisViewService.ts` (+7 -65) 📝 `src/domain/analysisView/WebNetworkApiSurfaceAccumulator.test.ts` (+25 -1) 📝 `src/domain/analysisView/WebNetworkApiSurfaceAccumulator.ts` (+7 -85) ➕ `src/domain/analysisView/analysisViewDataAdmission.ts` (+90 -0) </details> ### 📄 Description ## Summary Share inline Evidence admission across analysis views so malformed local representations return typed errors before schema parsing or authentication. Reject executable traversal hooks without allocating a key list for every array element. Fixes #2589. ## Problem and expected behavior A throwing normalized_result getter originally escaped summary/capture inspection. The upstream accessor check duplicates API-surface traversal and can still run Proxy reflection traps or enumerate an entire array before budget rejection. Array iterator/map/constructor hooks and function-valued metadata can execute during downstream validation. Inline Evidence must be checked before parsing, without invoking caller traversal hooks. Ordinary portable Evidence must continue to authenticate and project without source mutation. ## Change and scope Use one shared admission owner for summary, capture and API-surface inline inputs. Remove the duplicate application traversal. Keep API-surface representation reservations in the existing accumulator; summary/capture use structural admission. Reject Proxies and functions before reflection. Require standard arrays, inspect indexes incrementally, and reject own iterator/map/constructor overrides with fixed-size checks. Ignored named/symbol array metadata stays inert under the existing JSON cloning semantics. Retained Evidence lookup is unchanged. Merged fixed main `500e32399a3bc827e17fb4adaa97db3bc0df29b5`, preserving its CI-baseline fixes from #3080 and follow-ups. Superseded branch repairs were removed; the final diff is five files. No workflow, threshold or deadline changes. This is an ordinary validation fix within the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope); a new target capability or authority decision is not applicable. ## Contract and boundary impact - Owner/earliest changed stage: borrowed inline analysis-view data admission, before parsing/authentication. - CLI/MCP/tool catalog: unchanged contracts; both adapters call the shared service. - Providers/platforms: no new provider behavior. Verified with Node24.18.0/npm11.16.0 on Windows x64 and WSL Linux, using separate locked dependencies. - Evidence/provenance: output, links, parent identities and original source data remain preserved. Admission does not clone, freeze or mutate borrowed input. - Target selection/authority/cleanup: no new interaction or ownership rule. - Generated metadata/package/installation: unchanged. 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 and regression coverage Root/nested/hidden/indexed getters; array hooks/prototypes; callable constructors; ordinary/revoked/callable Proxies; cycles, aliases, source preservation, authenticated ignored metadata and API-surface budgets. Valid-Evidence success checks are retained. Three independent agents reviewed the final five-file diff and consumers after conflict resolution. Independent probes confirm zero getter/Proxy-trap calls, stable parent Evidence IDs and bounded array descriptor inspection. The delayed instance-field counterexample in the superseded JavaScript resolver no longer reproduces with the retained main implementation: worker specifier stays unknown, while an unrelated-write control still resolves normally. Previous CI [38508711978](https://github.com/morluto/rea/actions/runs/38508711978) passed source/static/build and real-browser/recovery lanes, but failed three shard5 tests: browser comparison schema projection, SAZ input-limit error category and Hopper fixture process visibility. All three failures were reproduced locally at d4731b0. Current main supplies their fixes; final-head full suites pass without weakening runtime rejection, input bounds or ownership assertions. Compiled CLI and SDK MCP tests exercise HAR decoding, retained/inline views and continued server use. Executable object fixtures cannot cross their JSON transport; source tests cover that local input boundary. - [x] Observed, derived and inferred claims remain distinguishable. - [x] Artifact identity, provenance and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable or uncertain outcomes remain visible. ## Validation performed Final head: `953c4019641b8bb6763bbde95c52f4de156b45ad`. - `npm run check:ci`: passed, including typecheck, lint/module boundaries, formatting, Knip, metadata, duplication/TODO checks and architecture guards. - Linux `npm run test:fast`: **504 files, 7375 passed, 8 skipped**; exit0, 102.26s. Executed from an exact archive of this commit on the Linux filesystem. - Linux `npm run test:focused -- tests/boundary/mcp/browserComparisonInput.test.ts tests/boundary/browser/sazWebCapture.test.ts tests/process-global/cleanLinuxHopperRuntime.test.ts tests/boundary/cli/analysisViewCli.test.ts tests/boundary/mcp/analysisViewMcp.test.ts tests/boundary/mcp/webNetworkCaptureMcp.test.ts`: **44 passed, 1 skipped**, six files; 55.97s. - Windows `npm run test:focused -- tests/boundary/browser/cdpSvgXlinkHref.test.ts tests/boundary/cli/analysisViewCli.test.ts tests/boundary/mcp/analysisViewMcp.test.ts tests/boundary/mcp/webNetworkCaptureMcp.test.ts`: **31 passed**, four files; actual Chrome, compiled CLI and SDK MCP; 41.89s. Process-only TEMP/TMP point inside the checkout. - `git diff --check`: passed; tracked tree clean. - Windows CLI used the verified native artifact from upstream run38490448169. Its native sources/build script still match this head; SHA256 `3ca4fcfe5eb39f13f0250e23de0f38c6cfd33616c664a436cd69ef3f013ac9e2`. Final-head remote CI [38515685028](https://github.com/morluto/rea/actions/runs/38515685028) completed successfully at head `953c4019641b8bb6763bbde95c52f4de156b45ad`: all five test shards, Source tests, static/docs/build/native-package checks, Portable tests and CI required passed. Optional lanes skipped by the current change classifier are not claimed as executed. Historical CI results are identified above. ## Compatibility, safety, and release No migration or package changes for ordinary JSON Evidence. Executable local representations fail admission; ignored array metadata follows JSON omission semantics. Actual Windows/Linux CLI and SDK MCP workflows passed, along with real Chrome tests. Hopper tests use controlled process fixtures; no live Hopper/Ghidra/IDA engine acceptance is claimed. This is input validation, not arbitrary JavaScript containment or a whole-input hard heap limit. Ordinary-object key enumeration predates this change; summary/capture do not acquire an API-surface representation budget. ## Before merge - [x] Concrete issue and expected behavior identified. - [x] Scope and authority resolved within existing boundaries. - [x] Focused final diff and Conventional Commit title. - [x] Related issue linked. - [x] Tests cover changed behavior and meaningful failures. - [x] Owning contracts updated; generated metadata unchanged. - [x] Public CLI/MCP workflows exercised. - [x] Final diff checked 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:07:47 +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#5589
No description provided.