morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #7268] fix(electron): recover IPC registered through bound Electron methods #7800

New issue

Open

opened 2026-11-05 01:47:45 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-05 01:47:45 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7268
Author: @patrunks
Created: 10/26/2026
Status: 🔄 Open

Base: main ← Head: fix/electron-bound-ipc

📝 Commits (1)

01dab07 fix(electron): recover IPC registered through bound Electron methods

📊 Changes

4 files changed (+80 additions, -14 deletions)

View changed files

📝 src/domain/javascript/electronStaticAnalysisIdentity.test.ts (+47 -9)
📝 src/domain/javascript/electronStaticAnalysisIpc.ts (+4 -1)
📝 src/domain/javascript/javascriptModuleInterop.ts (+20 -4)
📝 src/domain/javascript/javascriptSymbolicBindings.ts (+9 -0)

📄 Description

Summary

Electron IPC handlers registered through a bound ipcMain method were silently dropped by analyze_javascript_application. On a real Electron app (215 ipcMain.handle registrations made through ipcMain.handle.bind(ipcMain) plus two guard wrappers), REA reported main_handlers: 0 and every renderer invoke as unpaired, with no unknown recorded. No related issue was filed; the trigger is described below.

Problem and expected behavior

Smallest trigger:

const electron_1 = require("electron");
const handle = electron_1.ipcMain.handle.bind(electron_1.ipcMain);
handle("ch:a", async () => 1);

Base revision: ipc.operations: 0. The registration disappears instead of remaining an explicit unknown, which violates the "unresolved identity remains unknown, not absent" invariant. The same holds for ipcRenderer.invoke.bind(ipcRenderer).

Two producers lose the identity:

javascriptModuleInterop resolves bound callables only for JavaScript intrinsics (capturedIntrinsic), so electronInvocationPath cannot see handle(...) as ipcMain.handle, and inspectIpcCall only consulted electronCalleePath.
JavaScriptSymbolicBindings turns a call on an external ….bind value into an unknown, so wrapper call-path resolution loses site-determined channels passed through protectedIpc(channel, …) → rawIpcHandle → bound handle.

Change and scope

resolveBoundIntrinsic/boundIntrinsic accept the caller-selected callable path that intrinsicInvocation already takes. A bind site whose callee is a proven Electron export is admitted with the existing stable-initializer, write-coverage and bound-argument rules. The intrinsic-only default is unchanged.
inspectIpcCall falls back to context.electronInvocationPath, as setAppPath detection already does.
The symbolic evaluator keeps external.method.bind(receiver) as external.method only when the bind supplies at most a receiver and no spread. Extra bound arguments would shift the channel position, so that case stays unknown.

Not included: receiver validation for bound methods (direct member calls do not validate receivers either), and bound arguments beyond the receiver.

Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: Electron identity for bound callables (javascriptModuleInterop, consumed by electronInvocationPath → inspectIpcCall), and symbolic call evaluation (JavaScriptSymbolicBindings, consumed by electronWrappedCallPaths). No superseded path remains.
Contribution scope fit and linked maintainer decision: not applicable (fix inside existing static JavaScript analysis).
CLI and MCP/tool-catalog contract: no schema change. More electron.ipc findings are reported.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: a bound app.setAppPath invocation is now reported in addition to its existing bind-site escape finding (2 app_path_changes instead of 1 for const change = app.setAppPath.bind(app); change("./other")). Both are justified: the bind may escape, and the invocation is now proven. The existing test expectation was updated accordingly.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, 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/domain/javascript/electronStaticAnalysisIdentity.test.ts):
- recovers IPC operations invoked through bound Electron methods: bound ipcMain.handle and ipcRenderer.invoke. On the base revision it returns [] instead of both operations (executed).
- recovers a bound IPC registration passed through guard wrappers: the exact .bind + rawIpcHandle + protectedIpc shape; the channel resolves to sites:list.
- setAppPath bound case: expectation changed from 1 to 2 findings, as described above.

Base reproduction (executed, local CLI node scripts/rea.mjs analyze-javascript-application <dir> --json on minimal directories):

Case	base `main_handlers` / `literal_channels`	this branch
direct `ipcMain.handle`	1 / 1	1 / 1
`.bind` only	0 / 0	1 / 1
`.bind` + two wrappers	0 / 0	2 / 1 (1 literal + 1 dynamic wrapper-internal registration)
real app ASAR, `--source-path dist-electron/electron/main.js`	0 / 0	216 / 215

User-visible CLI/MCP output: electron.ipc summary counts above.
Remaining proof gaps: MCP adapter not exercised on this branch (same producer as CLI). The real-app ASAR is third-party and not included. Boundary regressions: identity/alias resolution for parser output; malformed-input and lifecycle regressions are not applicable.

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/domain/javascript/electronStaticAnalysisIdentity.test.ts: 37 passed.
npx vitest run src/domain/javascript src/application/javascript: 178 files, 4804 passed, 6 skipped.
npm run check:fast: typecheck and lint, 2/2 tasks successful.
npm run compile, then the CLI runs above, using the Windows native controls extracted from the published rea-agents@6.8.0 tarball (manifest SHA-256 matched). Windows Server 2022, Node 24.22.1.
npm run check:pr: not run.

Compatibility, safety, and release

Breaking changes or migration steps: none. Additional findings only, including the extra bound setAppPath finding.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript). Windows only for the CLI runs.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no execution; analysis remains inert parsing.

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/7268 **Author:** [@patrunks](https://github.com/patrunks) **Created:** 10/26/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/electron-bound-ipc` --- ### 📝 Commits (1) - [`01dab07`](https://github.com/morluto/rea/commit/01dab0776ece26483bdd7116f981f038d1464c28) fix(electron): recover IPC registered through bound Electron methods ### 📊 Changes **4 files changed** (+80 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/electronStaticAnalysisIdentity.test.ts` (+47 -9) 📝 `src/domain/javascript/electronStaticAnalysisIpc.ts` (+4 -1) 📝 `src/domain/javascript/javascriptModuleInterop.ts` (+20 -4) 📝 `src/domain/javascript/javascriptSymbolicBindings.ts` (+9 -0) </details> ### 📄 Description ## Summary Electron IPC handlers registered through a bound `ipcMain` method were silently dropped by `analyze_javascript_application`. On a real Electron app (215 `ipcMain.handle` registrations made through `ipcMain.handle.bind(ipcMain)` plus two guard wrappers), REA reported `main_handlers: 0` and every renderer invoke as unpaired, with no unknown recorded. No related issue was filed; the trigger is described below. ## Problem and expected behavior Smallest trigger: ```js const electron_1 = require("electron"); const handle = electron_1.ipcMain.handle.bind(electron_1.ipcMain); handle("ch:a", async () => 1); ``` Base revision: `ipc.operations: 0`. The registration disappears instead of remaining an explicit unknown, which violates the "unresolved identity remains unknown, not absent" invariant. The same holds for `ipcRenderer.invoke.bind(ipcRenderer)`. Two producers lose the identity: 1. `javascriptModuleInterop` resolves bound callables only for JavaScript intrinsics (`capturedIntrinsic`), so `electronInvocationPath` cannot see `handle(...)` as `ipcMain.handle`, and `inspectIpcCall` only consulted `electronCalleePath`. 2. `JavaScriptSymbolicBindings` turns a call on an external `….bind` value into an unknown, so wrapper call-path resolution loses site-determined channels passed through `protectedIpc(channel, …)` → `rawIpcHandle` → bound `handle`. ## Change and scope - `resolveBoundIntrinsic`/`boundIntrinsic` accept the caller-selected callable path that `intrinsicInvocation` already takes. A bind site whose callee is a proven Electron export is admitted with the existing stable-initializer, write-coverage and bound-argument rules. The intrinsic-only default is unchanged. - `inspectIpcCall` falls back to `context.electronInvocationPath`, as `setAppPath` detection already does. - The symbolic evaluator keeps `external.method.bind(receiver)` as `external.method` only when the bind supplies at most a receiver and no spread. Extra bound arguments would shift the channel position, so that case stays unknown. Not included: receiver validation for bound methods (direct member calls do not validate receivers either), and bound arguments beyond the receiver. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: Electron identity for bound callables (`javascriptModuleInterop`, consumed by `electronInvocationPath` → `inspectIpcCall`), and symbolic call evaluation (`JavaScriptSymbolicBindings`, consumed by `electronWrappedCallPaths`). No superseded path remains. - Contribution scope fit and linked maintainer decision: not applicable (fix inside existing static JavaScript analysis). - CLI and MCP/tool-catalog contract: no schema change. More `electron.ipc` findings are reported. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: a bound `app.setAppPath` invocation is now reported in addition to its existing bind-site escape finding (2 `app_path_changes` instead of 1 for `const change = app.setAppPath.bind(app); change("./other")`). Both are justified: the bind may escape, and the invocation is now proven. The existing test expectation was updated accordingly. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, 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/domain/javascript/electronStaticAnalysisIdentity.test.ts`): - `recovers IPC operations invoked through bound Electron methods`: bound `ipcMain.handle` and `ipcRenderer.invoke`. On the base revision it returns `[]` instead of both operations (executed). - `recovers a bound IPC registration passed through guard wrappers`: the exact `.bind` + `rawIpcHandle` + `protectedIpc` shape; the channel resolves to `sites:list`. - `setAppPath` bound case: expectation changed from 1 to 2 findings, as described above. - Base reproduction (executed, local CLI `node scripts/rea.mjs analyze-javascript-application <dir> --json` on minimal directories): | Case | base `main_handlers` / `literal_channels` | this branch | |---|---|---| | direct `ipcMain.handle` | 1 / 1 | 1 / 1 | | `.bind` only | 0 / 0 | 1 / 1 | | `.bind` + two wrappers | 0 / 0 | 2 / 1 (1 literal + 1 dynamic wrapper-internal registration) | | real app ASAR, `--source-path dist-electron/electron/main.js` | 0 / 0 | 216 / 215 | - User-visible CLI/MCP output: `electron.ipc` summary counts above. - Remaining proof gaps: MCP adapter not exercised on this branch (same producer as CLI). The real-app ASAR is third-party and not included. Boundary regressions: identity/alias resolution for parser output; malformed-input and lifecycle regressions are not applicable. 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/domain/javascript/electronStaticAnalysisIdentity.test.ts`: 37 passed. - `npx vitest run src/domain/javascript src/application/javascript`: 178 files, 4804 passed, 6 skipped. - `npm run check:fast`: typecheck and lint, 2/2 tasks successful. - `npm run compile`, then the CLI runs above, using the Windows native controls extracted from the published `rea-agents@6.8.0` tarball (manifest SHA-256 matched). Windows Server 2022, Node 24.22.1. - `npm run check:pr`: not run. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Additional findings only, including the extra bound `setAppPath` finding. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript). Windows only for the CLI runs. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no execution; analysis remains inert parsing. ## 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 added the

pull-request

label

2026-11-05 01:47:45 +00:00

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

No description provided.

Rows
Columns