[PR #6284] [MERGED] fix(electron): bind each contextBridge helper invocation to its own API #7255

New issue

Closed

opened 2026-11-01 21:46:32 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:32 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/5871-contextbridge-helper-keys

📝 Commits (1)

691c155 fix(electron): bind each contextBridge helper invocation to its own API

📊 Changes

5 files changed (+451 additions, -49 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+5 -0)
📝 src/domain/javascript/electronStaticAnalysisValues.ts (+7 -1)
📝 src/domain/javascript/electronWrappedCallPaths.test.ts (+132 -0)
📝 src/domain/javascript/electronWrappedCallPaths.ts (+223 -48)
📝 tests/boundary/cli/electronWrappedCallPaths.test.ts (+84 -0)

📄 Description

What broke: Wrapper substitution looked up contextBridge findings only by the inner exposeInMainWorld call's range. Each helper invocation replaced the previous one and unioned its members, so the last key kept every invocation's members (#5871). Substituted members also listed only top-level keys, so nested paths were dropped (#5870).
Fix: Substituted exposures are now stored per call site and per (world, key), with members enumerated recursively under the same depth-8 bound the direct syntax pass uses. The site's syntax observation is replaced once, after traversal. A traced invocation that stays unresolved or guarded keeps the syntax observation.
Risk: The change is limited to the wrapper-substitution branch for contextBridge exposures in electronWrappedCallPaths.ts. Complete direct-syntax observations take the same path as before, and the IPC, call-path and graph projection code is unchanged.

Summary

Fixes #5871
Fixes #5870

analyze-javascript-application merged contextBridge exposures that go through a local helper. Calling expose("one", {ping}) and expose("two", {pong}) produced one API, two, with members ["ping","pong"], even though the call paths one.ping and two.pong were retained. With this change, each helper invocation binds its own key and members. The API count, the per-key members and the exposure call-path entries now agree with the direct-call control. The same producer also dropped nested paths for helper-passed objects (["nested"] and no nested.ping); that is fixed here too.

Problem and expected behavior

Smallest trigger: function expose(key, api) { contextBridge.exposeInMainWorld(key, api); } called twice with different literal keys.
Violated invariant: every exposed API key owns only the members of the object exposed under it. The member inventory keeps the bounded dotted paths that direct syntax keeps.
Both issues have one root cause: bridge() in src/domain/javascript/electronWrappedCallPaths.ts built the substituted finding from state shared by every invocation of the site, and only enumerated immediate keys of the symbolic object.

Change and scope

Substituted exposures are collected in a BridgeSite per exposure call site and keyed by (world, api key), which is the identity the direct syntax pass and the graph node use. After traversal, recordBridgeExposures() replaces the site's syntax observation in its original position.
Several invocations that supply the same key to one site describe one API covering each alternative object: members are unioned and unknown_members is the maximum. If the world ids differ, world_id becomes dynamic. This avoids a duplicate graph identity and matches how a union-valued argument is already treated.
A traced invocation (one reached through at least one local call) that leaves the key, object or world unresolved, or that is guarded, keeps the site's own unresolved syntax observation. That unknown no longer disappears behind resolved keys.
bridgeMemberInventory enumerates dotted paths through nested values that resolve to unconditional objects, records or namespaces. It uses the depth bound contextBridgeMemberPathDepth = 8, now shared with the direct collector in electronStaticAnalysisValues.ts, and shares the existing visit budget. This supersedes the old union with syntax members, which has been removed. It also keeps the depth-limit unknown that substitution used to drop.
Sites whose direct syntax is already complete (literal key, object literal, no unknown members) are detected once and skip inventory work. The old linear find over all bridge findings on each exposure is gone.
Not included:
- Helpers whose first argument is not a string literal are still not seeded, for example expose(api, "key") or exposeInIsolatedWorld helpers called with a numeric world id first. That limit predates this change, and those cases keep their unresolved syntax observation.
- The graph identity still joins api_key ?? api_key_expression. A dynamic-key observation whose expression text equals a resolved literal key at the same site would share a node.
Overlap check: open PR #2094 (broad wrapper/constant-folding feature, other author) touches this file but does not target these findings. This PR does not depend on it. Our open PR #6280 files are not touched.

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

Contract and boundary impact

Semantic owner and earliest changed stage: collectElectronWrappedCallPaths (domain, wrapper substitution) in src/domain/javascript/electronWrappedCallPaths.ts.
Contribution scope fit and linked maintainer decision: bug fix in existing static analysis; not applicable.
CLI and MCP/tool-catalog contract: no schema change. The context_bridge_apis findings, graph context-bridge-api observations and summary counts now contain correct per-key values.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: substituted findings keep resolution_basis: "wrapper-substitution" and static-inference Evidence. Direct observations are unchanged. docs/javascript-artifact-reconstruction.md now describes per-invocation exposure recording.
Target selection, authority, upstream behavior, and cleanup details (if applicable): 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:
- tests/boundary/cli/electronWrappedCallPaths.test.ts adds "binds each contextBridge helper invocation's own key and members", run through both CLI and stdio MCP. One fixture covers repeated helper calls, a nested API object, a helper chain with key and API in different parameter positions, a direct call alongside helpers, and a traced invocation with an unresolved key. The test asserts 5 APIs, per-key members, exposed_api_members: 5, and that the exposure entries match.
- src/domain/javascript/electronWrappedCallPaths.test.ts adds contextBridge helper invocations, 7 cases: site-literal object with a key parameter, repeated identical invocation, same key with alternative objects, differing isolated world ids, depth-8 parity with direct syntax, guarded invocation, and a site-literal key next to a guarded invocation. With the source fix stashed, 6 of the 7 fail on the base revision; the repeated-identical case already passed.

Base reproduction (executed, built CLI at b6007e0a5, node scripts/rea.mjs analyze-javascript-application <dir> --json, using the issue's extraction script):

before a-helper2  {"apis_count":1,"members_count":2,"apis":[{"api_key":"two","members":["ping","pong"]}],"exposure_entries":["one.ping","two.pong"]}
after  a-helper2  {"apis_count":2,"members_count":2,"apis":[{"api_key":"two","members":["pong"]},{"api_key":"one","members":["ping"]}],"exposure_entries":["one.ping","two.pong"]}
before c-nested   {"apis_count":1,"members_count":1,"apis":[{"api_key":"desktop","members":["nested"],"unknown_members":0}]}
after  c-nested   {"apis_count":1,"members_count":2,"apis":[{"api_key":"desktop","members":["nested","nested.ping"],"unknown_members":0}]}

The direct-call controls are unchanged (basis: direct-syntax, object-literal).

Additional executed counterexamples (before → after):
- 3 helper calls: 1 API with 4 members → 3 APIs with three: [a, b, b.c].
- Helper chain outer → inner, plus a direct inner call: 1 → 3 APIs.
- Mixed direct and helper calls: 2 → 3 APIs.
- Spread in the first call: unknown_members was lost (0) → kept as 1 on one only.
- Key parameter with a literal object at the site: 1 API with dynamic key → one and two.
- Traced unresolved key: the dynamic observation was dropped → kept next to one.
- Guarded helper call: kept as the dynamic observation.
- Same key twice: one API api: [a, b].
- Repeated identical call: 1 API.
- Nested object through an identifier: nested.deeper.pong and the other paths are listed.
- Nested spread: unknown_members: 1.
- 10-level object, helper and direct (spread-free direct call over the depth bound): members to depth 8 with unknown_members: 1 in both. The depth unknown was previously dropped for the direct case.
- Unused helper and non-literal-first-argument helpers: unchanged.
User-visible CLI/MCP output: shown above.
Remaining proof gaps:
- Synthetic inert fixtures only; no real Electron application or ASAR input was analyzed and no Electron runtime was launched.
- The MCP surface is covered only by the new boundary test, not by a manual run.

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

Issue repro plus 23 counterexample fixtures through the built CLI, before and after the change: results as summarized above.
npm run test:local -- src/domain/javascript/electronWrappedCallPaths.test.ts: 179 passed. With the source fix stashed, the 6 new cases fail.
npm run test:focused -- all src/domain/javascript/*.test.ts, all src/application/javascript/*.test.ts, and the Electron/JS CLI, MCP, filesystem and acceptance suites (tests/boundary/cli/electronWrappedCallPaths.test.ts, tests/boundary/cli/electronCli.test.ts, tests/boundary/mcp/electronMcp.test.ts, tests/boundary/filesystem/javascript-applications/* (2 files), tests/boundary/javascript/workerOrderAudit.test.ts, tests/acceptance/applications/{applicationWorkflowCli,electronIpcConstantChannelCli,javascriptFactoryCompletion,electronIpcListenerCallsiteCli}.test.ts): 158 files, 4060 tests passed.
npm run check:fast: passed (typecheck, lint).
npm run format:check: passed.
npm run docs:check: passed.
Performance, executed: a synthetic 319 KB preload with 1500 helper exposures (with nested objects) and 1500 direct exposures, 3 runs each with /usr/bin/time -l.
- Base: 5.89–6.01 s wall, 7.55–7.71 s user, max RSS 529–767 MB. It reported 1501 APIs because the 1500 helper exposures collapsed into one.
- Change: 5.79–5.89 s wall, 7.38–7.76 s user, max RSS 608–736 MB. It reports 3000 APIs.
- Timings overlap; no speedup is claimed. The JSON output grew 0.9% (46.6 → 47.1 MB) because of the restored facts. Agent token usage was not measured.

Compatibility, safety, and release

Breaking changes or migration steps: none. Output shape is unchanged; only incorrect per-key facts change.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript analysis on macOS arm64 only).
Package or release metadata impact: none.
Security, privacy, process, or containment review: inputs are parsed as inert source; nothing is executed and there are no new side effects.

Author readiness

Human owner: @kaoru0822-kitauji

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/6284 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/5871-contextbridge-helper-keys` --- ### 📝 Commits (1) - [`691c155`](https://github.com/morluto/rea/commit/691c15521384329b57d16da12dca95ead73bafe2) fix(electron): bind each contextBridge helper invocation to its own API ### 📊 Changes **5 files changed** (+451 additions, -49 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+5 -0) 📝 `src/domain/javascript/electronStaticAnalysisValues.ts` (+7 -1) 📝 `src/domain/javascript/electronWrappedCallPaths.test.ts` (+132 -0) 📝 `src/domain/javascript/electronWrappedCallPaths.ts` (+223 -48) 📝 `tests/boundary/cli/electronWrappedCallPaths.test.ts` (+84 -0) </details> ### 📄 Description **What broke:** Wrapper substitution looked up contextBridge findings only by the inner `exposeInMainWorld` call's range. Each helper invocation replaced the previous one and unioned its members, so the last key kept every invocation's members (#5871). Substituted members also listed only top-level keys, so nested paths were dropped (#5870). **Fix:** Substituted exposures are now stored per call site and per (world, key), with members enumerated recursively under the same depth-8 bound the direct syntax pass uses. The site's syntax observation is replaced once, after traversal. A traced invocation that stays unresolved or guarded keeps the syntax observation. **Risk:** The change is limited to the wrapper-substitution branch for contextBridge exposures in `electronWrappedCallPaths.ts`. Complete direct-syntax observations take the same path as before, and the IPC, call-path and graph projection code is unchanged. ## Summary Fixes #5871 Fixes #5870 `analyze-javascript-application` merged contextBridge exposures that go through a local helper. Calling `expose("one", {ping})` and `expose("two", {pong})` produced one API, `two`, with members `["ping","pong"]`, even though the call paths `one.ping` and `two.pong` were retained. With this change, each helper invocation binds its own key and members. The API count, the per-key members and the exposure call-path entries now agree with the direct-call control. The same producer also dropped nested paths for helper-passed objects (`["nested"]` and no `nested.ping`); that is fixed here too. ## Problem and expected behavior - Smallest trigger: `function expose(key, api) { contextBridge.exposeInMainWorld(key, api); }` called twice with different literal keys. - Violated invariant: every exposed API key owns only the members of the object exposed under it. The member inventory keeps the bounded dotted paths that direct syntax keeps. - Both issues have one root cause: `bridge()` in `src/domain/javascript/electronWrappedCallPaths.ts` built the substituted finding from state shared by every invocation of the site, and only enumerated immediate keys of the symbolic object. ## Change and scope - Substituted exposures are collected in a `BridgeSite` per exposure call site and keyed by (world, api key), which is the identity the direct syntax pass and the graph node use. After traversal, `recordBridgeExposures()` replaces the site's syntax observation in its original position. - Several invocations that supply the **same** key to one site describe one API covering each alternative object: members are unioned and `unknown_members` is the maximum. If the world ids differ, `world_id` becomes dynamic. This avoids a duplicate graph identity and matches how a union-valued argument is already treated. - A traced invocation (one reached through at least one local call) that leaves the key, object or world unresolved, or that is guarded, keeps the site's own unresolved syntax observation. That unknown no longer disappears behind resolved keys. - `bridgeMemberInventory` enumerates dotted paths through nested values that resolve to unconditional objects, records or namespaces. It uses the depth bound `contextBridgeMemberPathDepth = 8`, now shared with the direct collector in `electronStaticAnalysisValues.ts`, and shares the existing visit budget. This supersedes the old union with syntax members, which has been removed. It also keeps the depth-limit unknown that substitution used to drop. - Sites whose direct syntax is already complete (literal key, object literal, no unknown members) are detected once and skip inventory work. The old linear `find` over all bridge findings on each exposure is gone. - Not included: - Helpers whose first argument is not a string literal are still not seeded, for example `expose(api, "key")` or `exposeInIsolatedWorld` helpers called with a numeric world id first. That limit predates this change, and those cases keep their unresolved syntax observation. - The graph identity still joins `api_key ?? api_key_expression`. A dynamic-key observation whose expression text equals a resolved literal key at the same site would share a node. - Overlap check: open PR #2094 (broad wrapper/constant-folding feature, other author) touches this file but does not target these findings. This PR does not depend on it. Our open PR #6280 files are not touched. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `collectElectronWrappedCallPaths` (domain, wrapper substitution) in `src/domain/javascript/electronWrappedCallPaths.ts`. - Contribution scope fit and linked maintainer decision: bug fix in existing static analysis; not applicable. - CLI and MCP/tool-catalog contract: no schema change. The `context_bridge_apis` findings, graph `context-bridge-api` observations and summary counts now contain correct per-key values. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: substituted findings keep `resolution_basis: "wrapper-substitution"` and static-inference Evidence. Direct observations are unchanged. `docs/javascript-artifact-reconstruction.md` now describes per-invocation exposure recording. - Target selection, authority, upstream behavior, and cleanup details (if applicable): 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: - `tests/boundary/cli/electronWrappedCallPaths.test.ts` adds "binds each contextBridge helper invocation's own key and members", run through both CLI and stdio MCP. One fixture covers repeated helper calls, a nested API object, a helper chain with key and API in different parameter positions, a direct call alongside helpers, and a traced invocation with an unresolved key. The test asserts 5 APIs, per-key members, `exposed_api_members: 5`, and that the exposure entries match. - `src/domain/javascript/electronWrappedCallPaths.test.ts` adds `contextBridge helper invocations`, 7 cases: site-literal object with a key parameter, repeated identical invocation, same key with alternative objects, differing isolated world ids, depth-8 parity with direct syntax, guarded invocation, and a site-literal key next to a guarded invocation. With the source fix stashed, 6 of the 7 fail on the base revision; the repeated-identical case already passed. - Base reproduction (executed, built CLI at `b6007e0a5`, `node scripts/rea.mjs analyze-javascript-application <dir> --json`, using the issue's extraction script): ``` before a-helper2 {"apis_count":1,"members_count":2,"apis":[{"api_key":"two","members":["ping","pong"]}],"exposure_entries":["one.ping","two.pong"]} after a-helper2 {"apis_count":2,"members_count":2,"apis":[{"api_key":"two","members":["pong"]},{"api_key":"one","members":["ping"]}],"exposure_entries":["one.ping","two.pong"]} before c-nested {"apis_count":1,"members_count":1,"apis":[{"api_key":"desktop","members":["nested"],"unknown_members":0}]} after c-nested {"apis_count":1,"members_count":2,"apis":[{"api_key":"desktop","members":["nested","nested.ping"],"unknown_members":0}]} ``` The direct-call controls are unchanged (`basis: direct-syntax`, `object-literal`). - Additional executed counterexamples (before → after): - 3 helper calls: 1 API with 4 members → 3 APIs with `three: [a, b, b.c]`. - Helper chain `outer` → `inner`, plus a direct `inner` call: 1 → 3 APIs. - Mixed direct and helper calls: 2 → 3 APIs. - Spread in the first call: `unknown_members` was lost (0) → kept as 1 on `one` only. - Key parameter with a literal object at the site: 1 API with dynamic key → `one` and `two`. - Traced unresolved key: the dynamic observation was dropped → kept next to `one`. - Guarded helper call: kept as the dynamic observation. - Same key twice: one API `api: [a, b]`. - Repeated identical call: 1 API. - Nested object through an identifier: `nested.deeper.pong` and the other paths are listed. - Nested spread: `unknown_members: 1`. - 10-level object, helper and direct (spread-free direct call over the depth bound): members to depth 8 with `unknown_members: 1` in both. The depth unknown was previously dropped for the direct case. - Unused helper and non-literal-first-argument helpers: unchanged. - User-visible CLI/MCP output: shown above. - Remaining proof gaps: - Synthetic inert fixtures only; no real Electron application or ASAR input was analyzed and no Electron runtime was launched. - The MCP surface is covered only by the new boundary test, not by a manual run. 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 - Issue repro plus 23 counterexample fixtures through the built CLI, before and after the change: results as summarized above. - `npm run test:local -- src/domain/javascript/electronWrappedCallPaths.test.ts`: 179 passed. With the source fix stashed, the 6 new cases fail. - `npm run test:focused --` all `src/domain/javascript/*.test.ts`, all `src/application/javascript/*.test.ts`, and the Electron/JS CLI, MCP, filesystem and acceptance suites (`tests/boundary/cli/electronWrappedCallPaths.test.ts`, `tests/boundary/cli/electronCli.test.ts`, `tests/boundary/mcp/electronMcp.test.ts`, `tests/boundary/filesystem/javascript-applications/*` (2 files), `tests/boundary/javascript/workerOrderAudit.test.ts`, `tests/acceptance/applications/{applicationWorkflowCli,electronIpcConstantChannelCli,javascriptFactoryCompletion,electronIpcListenerCallsiteCli}.test.ts`): 158 files, 4060 tests passed. - `npm run check:fast`: passed (typecheck, lint). - `npm run format:check`: passed. - `npm run docs:check`: passed. - Performance, executed: a synthetic 319 KB preload with 1500 helper exposures (with nested objects) and 1500 direct exposures, 3 runs each with `/usr/bin/time -l`. - Base: 5.89–6.01 s wall, 7.55–7.71 s user, max RSS 529–767 MB. It reported 1501 APIs because the 1500 helper exposures collapsed into one. - Change: 5.79–5.89 s wall, 7.38–7.76 s user, max RSS 608–736 MB. It reports 3000 APIs. - Timings overlap; no speedup is claimed. The JSON output grew 0.9% (46.6 → 47.1 MB) because of the restored facts. Agent token usage was not measured. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Output shape is unchanged; only incorrect per-key facts change. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript analysis on macOS arm64 only). - Package or release metadata impact: none. - Security, privacy, process, or containment review: inputs are parsed as inert source; nothing is executed and there are no new side effects. ## Author readiness Human owner: @kaoru0822-kitauji - [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 - [ ] 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-11-01 21:46:32 +00:00