[PR #4721] [CLOSED] fix(electron): exclude DOM nodes with out-of-range parentIndex #6372

New issue

Closed

opened 2026-10-31 01:13:58 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:13:58 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4721
Author: @Priyanshu-WebDev-io
Created: 10/17/2026
Status: ❌ Closed

Base: main ← Head: fix/issue-4709-electron-dom-parent-validation

📝 Commits (1)

ff059ed fix(electron): exclude DOM nodes with out-of-range parentIndex

📊 Changes

2 files changed (+175 additions, -3 deletions)

View changed files

📝 src/browser/CdpElectronInspection.test.ts (+169 -1)
📝 src/browser/CdpElectronInspection.ts (+6 -2)

📄 Description

Summary

Fixes #4709.

When inspecting an Electron page's DOM snapshot (inspect_electron_page / CdpElectronInspection), a node with an invalid, malformed, or out-of-range parent index (e.g. parent < -1, parent === index, or parent >= nodeTypes.length) was previously retained with an invalid parent_index, and no invalid_protocol_value exclusion was recorded in completeness tracking.

This change validates parent index bounds during Electron DOM capture, excludes out-of-range or self-referential nodes with an invalid_protocol_value exclusion, and preserves stable document node indices and parent pointers across documents, aligning Electron DOM capture with web DOM capture semantics in CdpCaptureDocuments.

Problem and expected behavior

Problem:
In src/browser/CdpElectronInspection.ts, captureDom read parents[index] from documentNodes and computed parent_index: parent < 0 ? -1 : baseIndex + parent without checking whether parents[index] was within bounds of the document's nodes. Furthermore, baseIndex was computed using nodes.length instead of accumulated total nodes.

If a producer emitted an out-of-range parent index (such as 99999 for a document with 3 nodes) or self-referential parent (parent === index), the invalid node was retained in dom.nodes, pointing to non-existent parent indices, and completeness.snapshot().excluded reported 0 exclusions. If any node was omitted, using nodes.length for subsequent documents would shift base indices and corrupt parent-child linkages.

Expected behavior:
Any DOM node with a parent index that is undefined, < -1, equal to the node's own index, or >= nodeTypes.length must be excluded, recording an invalid_protocol_value exclusion under section dom. Valid nodes must retain their stable document indices (baseIndex + index) and accurate parent pointers.

Change and scope

Added parent index bounds validation in src/browser/CdpElectronInspection.ts:

const rawParent = parents[index];
const parent = integer(rawParent, -1);
if (
  rawParent === undefined ||
  parent < -1 ||
  parent === index ||
  parent >= nodeTypes.length
) {
  invalid(1);
  continue;
}

Updated document node indexing to use const baseIndex = total; total += nodeTypes.length; and index: baseIndex + index, ensuring stable indexing across documents regardless of node exclusions.
Exported captureDom to allow focused unit testing.
Added comprehensive unit tests in src/browser/CdpElectronInspection.test.ts covering:
- Out-of-range parent index exclusion and invalid_protocol_value recording.
- Boundary cases: -2, parent === index, 99999, and undefined.
- Multi-document index stability when earlier nodes are excluded.

Contract and boundary impact

Semantic owner and earliest changed stage: Electron browser observation adapter (CdpElectronInspection.ts), during DOM snapshot capture.
Contribution scope fit and linked maintainer decision: Bug fix within existing boundaries; not applicable.
CLI and MCP/tool-catalog contract: Unchanged (inspect_electron_page output schema preserved).
Provider, bridge, target-format, or platform compatibility: Compatible across all platforms supporting Electron inspection.
Evidence, artifact, provenance, or reconstruction contract: Corrects DOM evidence completeness tracking by recording invalid_protocol_value exclusions when invalid protocol values occur.
Target selection, authority, upstream behavior, and cleanup details (if applicable): None.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: None.

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

Tests added or updated:
- src/browser/CdpElectronInspection.test.ts:
  - excludes nodes with out-of-range parent indices and reports invalid evidence
  - excludes node when parentIndex is invalid or self-referential: -2, 2, 99999, undefined
  - preserves stable document node indices across multiple documents when earlier nodes are excluded
Base reproduction or other evidence: On the base revision, supplying a document with parentIndex: [-1, 0, 1, 99999, 2] retains 5 nodes with parent_index: 99999 and 0 exclusions recorded. With this fix, the invalid node is excluded, 4 valid nodes are retained with correct indices ([0, 1, 2, 4]) and parent indices ([-1, 0, 1, 2]), and completeness.snapshot().excluded reports { section: "dom", reason: "invalid_protocol_value", count: 1 }.
User-visible CLI/MCP output (if applicable): N/A (internal data validation).
Remaining proof gaps: None.

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 src/browser/CdpElectronInspection.test.ts — 9/9 passed.
npx vitest run src/browser/ — 35 test files, 536 passed.
npm run check:fast — typecheck and lint passed (0 errors).
npm run check — typecheck, lint:built, format:check, knip, metadata:check, architecture-guards passed.
npm run build — compilation, package metadata, and skill metadata generation succeeded.

Compatibility, safety, and release

Breaking changes or migration steps: None.
Real Hopper/Ghidra, browser, or OS coverage: Browser/Electron observation adapter.
Package or release metadata impact: None.
Security, privacy, process, or containment review: Prevents out-of-bounds pointer propagation in captured DOM structures.

Author readiness

Human owner: @Priyanshu-WebDev-io

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/4721 **Author:** [@Priyanshu-WebDev-io](https://github.com/Priyanshu-WebDev-io) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/issue-4709-electron-dom-parent-validation` --- ### 📝 Commits (1) - [`ff059ed`](https://github.com/morluto/rea/commit/ff059ed379b86cfe740bc0e2cb4dd084ace4607f) fix(electron): exclude DOM nodes with out-of-range parentIndex ### 📊 Changes **2 files changed** (+175 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/CdpElectronInspection.test.ts` (+169 -1) 📝 `src/browser/CdpElectronInspection.ts` (+6 -2) </details> ### 📄 Description ## Summary Fixes #4709. When inspecting an Electron page's DOM snapshot (`inspect_electron_page` / `CdpElectronInspection`), a node with an invalid, malformed, or out-of-range parent index (e.g. `parent < -1`, `parent === index`, or `parent >= nodeTypes.length`) was previously retained with an invalid `parent_index`, and no `invalid_protocol_value` exclusion was recorded in completeness tracking. This change validates parent index bounds during Electron DOM capture, excludes out-of-range or self-referential nodes with an `invalid_protocol_value` exclusion, and preserves stable document node indices and parent pointers across documents, aligning Electron DOM capture with web DOM capture semantics in `CdpCaptureDocuments`. ## Problem and expected behavior **Problem**: In `src/browser/CdpElectronInspection.ts`, `captureDom` read `parents[index]` from `documentNodes` and computed `parent_index: parent < 0 ? -1 : baseIndex + parent` without checking whether `parents[index]` was within bounds of the document's nodes. Furthermore, `baseIndex` was computed using `nodes.length` instead of accumulated total nodes. If a producer emitted an out-of-range parent index (such as `99999` for a document with 3 nodes) or self-referential parent (`parent === index`), the invalid node was retained in `dom.nodes`, pointing to non-existent parent indices, and `completeness.snapshot().excluded` reported 0 exclusions. If any node was omitted, using `nodes.length` for subsequent documents would shift base indices and corrupt parent-child linkages. **Expected behavior**: Any DOM node with a parent index that is undefined, `< -1`, equal to the node's own index, or `>= nodeTypes.length` must be excluded, recording an `invalid_protocol_value` exclusion under section `dom`. Valid nodes must retain their stable document indices (`baseIndex + index`) and accurate parent pointers. ## Change and scope - Added parent index bounds validation in `src/browser/CdpElectronInspection.ts`: ```typescript const rawParent = parents[index]; const parent = integer(rawParent, -1); if ( rawParent === undefined || parent < -1 || parent === index || parent >= nodeTypes.length ) { invalid(1); continue; } ``` - Updated document node indexing to use `const baseIndex = total; total += nodeTypes.length;` and `index: baseIndex + index`, ensuring stable indexing across documents regardless of node exclusions. - Exported `captureDom` to allow focused unit testing. - Added comprehensive unit tests in `src/browser/CdpElectronInspection.test.ts` covering: - Out-of-range parent index exclusion and `invalid_protocol_value` recording. - Boundary cases: `-2`, `parent === index`, `99999`, and `undefined`. - Multi-document index stability when earlier nodes are excluded. ## Contract and boundary impact - Semantic owner and earliest changed stage: Electron browser observation adapter (`CdpElectronInspection.ts`), during DOM snapshot capture. - Contribution scope fit and linked maintainer decision: Bug fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: Unchanged (`inspect_electron_page` output schema preserved). - Provider, bridge, target-format, or platform compatibility: Compatible across all platforms supporting Electron inspection. - Evidence, artifact, provenance, or reconstruction contract: Corrects DOM evidence completeness tracking by recording `invalid_protocol_value` exclusions when invalid protocol values occur. - Target selection, authority, upstream behavior, and cleanup details (if applicable): None. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: None. 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 - Tests added or updated: - `src/browser/CdpElectronInspection.test.ts`: - `excludes nodes with out-of-range parent indices and reports invalid evidence` - `excludes node when parentIndex is invalid or self-referential: -2, 2, 99999, undefined` - `preserves stable document node indices across multiple documents when earlier nodes are excluded` - Base reproduction or other evidence: On the base revision, supplying a document with `parentIndex: [-1, 0, 1, 99999, 2]` retains 5 nodes with `parent_index: 99999` and 0 exclusions recorded. With this fix, the invalid node is excluded, 4 valid nodes are retained with correct indices (`[0, 1, 2, 4]`) and parent indices (`[-1, 0, 1, 2]`), and `completeness.snapshot().excluded` reports `{ section: "dom", reason: "invalid_protocol_value", count: 1 }`. - User-visible CLI/MCP output (if applicable): N/A (internal data validation). - Remaining proof gaps: None. 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 src/browser/CdpElectronInspection.test.ts` — 9/9 passed. - `npx vitest run src/browser/` — 35 test files, 536 passed. - `npm run check:fast` — typecheck and lint passed (0 errors). - `npm run check` — typecheck, lint:built, format:check, knip, metadata:check, architecture-guards passed. - `npm run build` — compilation, package metadata, and skill metadata generation succeeded. ## Compatibility, safety, and release - Breaking changes or migration steps: None. - Real Hopper/Ghidra, browser, or OS coverage: Browser/Electron observation adapter. - Package or release metadata impact: None. - Security, privacy, process, or containment review: Prevents out-of-bounds pointer propagation in captured DOM structures. ## Author readiness Human owner: @Priyanshu-WebDev-io - [ ] 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 - [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:13:58 +00:00