[PR #4562] [MERGED] fix(electron): block and record synchronous shell effects in the active hook #6308

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4562
Author: @Nitin13-git
Created: 10/16/2026
Status: ✅ Merged
Merged: 10/16/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/electron-shell-sync-effects

📝 Commits (10+)

db5e4d4 fix(electron): block and record synchronous shell effects in the active hook
c75cf82 fix(ghidra): retain typed Java prerequisite outcomes
ae07d15 fix(ghidra): explain keg-only JDK admission remediation
b0ddb58 fix(ghidra): own PSP inspection finalization
f22e8e7 test(ghidra): align Java probe fixtures with raw output
bf34758 fix(electron): preserve native IPC listener behavior (#4389)
50dbd87 fix(process): preserve owning operation cancellation
bf7fad3 fix(cleanup): project resources from unresolved owners
f26973a fix(providers): own profile selection through finalization
0386b60 fix(electron): integrate synchronous shell boundary blocking

📊 Changes

65 files changed (+17973 additions, -687 deletions)

View changed files

📝 docs/electron-observation.md (+45 -0)
📝 docs/installation.md (+10 -0)
📝 docs/tool-design.md (+8 -0)
➕ docs/verification/electron-ipc-listeners-2026-10-16.json (+10377 -0)
➕ docs/verification/electron-ipc-listeners-2026-10-16.md (+592 -0)
➕ docs/verification/electron-shell-sync-2026-10-16.json (+1827 -0)
➕ docs/verification/electron-shell-sync-2026-10-16.md (+91 -0)
➕ experiments/analysis-efficiency/drawio-electron-lifecycle-oracles.py (+115 -0)
➕ experiments/analysis-efficiency/drawio-electron-lifecycle.mjs (+191 -0)
➕ experiments/analysis-efficiency/joplin-shell-boundary.mjs (+291 -0)
📝 scripts/electron-active-hook-boundaries.cjs (+15 -2)
📝 scripts/electron-active-hook.cjs (+297 -48)
📝 src/adb/AdbProvider.ts (+9 -3)
📝 src/application/AnalysisProvider.ts (+6 -0)
📝 src/application/binary/AnalysisProviderEvaluation.ts (+24 -25)
📝 src/application/binary/AnalysisProviderRegistry.test.ts (+253 -14)
📝 src/application/binary/AnalysisProviderRegistry.ts (+231 -39)
📝 src/application/binary/BinarySession.ts (+6 -6)
📝 src/application/binary/BinarySessionOpen.ts (+1 -9)
📝 src/application/binary/BinarySessionRecords.ts (+1 -36)

...and 45 more files

📄 Description

Summary

Fixes #4393: during capture_electron_scenario, shell.showItemInFolder and shell.writeShortcutLink bypassed the active boundary hook, ran their native implementations on the host, and left no shell-attempt event.

Problem and expected behavior

patchShell in scripts/electron-active-hook-boundaries.cjs wrapped only the Promise-returning openExternal and openPath. docs/electron-observation.md says the active hook blocks and records OS-integration effects. showItemInFolder (synchronous void) opens the host file manager; writeShortcutLink (synchronous boolean) creates or replaces a Windows .lnk file. Expected: neither reaches the host, both are recorded as blocked, and the target sees an ordinary refusal.

Change and scope

patchShell now takes its blocked methods and their refusal results from one table: the two existing methods keep rejecting with External shell effects are blocked; showItemInFolder returns undefined and writeShortcutLink returns false, matching Electron's synchronous contracts (never a rejected Promise or a throw).
Events are recorded through the existing recordRuntime path, so only argument shapes are retained; paths and shortcut options are not.
Not included: shell.trashItem, which #3559 already addresses (the table is where it would be added). moveToApplicationsFolder, mentioned in the issue as an analogue, is not covered here.
Scope decision: not applicable (tightens an existing blocking boundary; no new capability).

Contract and boundary impact

Semantic owner and earliest changed stage: Electron active hook boundary patches
CLI and MCP/tool-catalog contract: none
Provider, bridge, target-format, or platform compatibility: Electron targets on all platforms; writeShortcutLink exists only on Windows
Evidence, artifact, provenance, or reconstruction contract: new shell-attempt events for these methods, same schema
Target selection, authority, upstream behavior, and cleanup details: reduces host side effects from the target; no new authority
Launch or attach to a process (existing capture behavior, unchanged)

Original-author evidence and regression coverage (author-reported)

Executed on macOS arm64, Node 24.18.0:

New test in tests/boundary/browser/electronActiveHook.test.ts runs the real staged hook against a fake Electron whose shell records native calls. It asserts the native methods are never called, results are undefined and false, two blocked shell-attempt events carry only ["string"] and ["string","string","object"] shapes, the serialized snapshot contains neither the path nor the shortcut target, and events still parse against electronActiveTimelineEventSchema. It fails on the base revision.
Existing hook tests and npm run check:fast pass.
Not run: a real Electron target, verify-real-electron.mjs, Windows, full check:pr.

Maintainer verification on current main

Integrated onto main f26973a52; source/verifier 3f231735c, current verification record in electron-shell-sync-2026-10-16.md and adjacent JSON. Original contributor commit is retained.

Executed on Linux, pinned Node24.18.0/npm11.16.0, serial1CPU/8GiB:

Base boundary + new test:73pass1fail; candidate74hook +5projection +8MCP =87pass. Fake native/controlled-provider coverage, not real Windows.
npm run check:fast and npm run docs:check pass.
Unchanged real Joplin3.7.21 ASAR on explicit generic Electron44.7.0: shipped welcome PNG → actual resource menu/bridge →0to1blocked reveal event; visible attachment, no page errors, exact retained shared-capture event. Final pipeline16.989s wall /16.055812 familyCPU s /1,376,382,976 peak cgroupB, payload observation verified.
Overall real workflow still fails cleanup (#4282). Standard launch remains unsuccessful (#2974); the valid operation probe explicitly uses a test-only argv shim. An invalid NODE_OPTIONS experiment and all other failed probes remain recorded. No production launcher/fuse change, Windows/macOS runtime claim, full-gate claim, cold/warm speedup or billed-token benefit.

Current Codex review is unavailable due repository quota (existing notice); it is not claimed approved. All38current checks pass (36CI jobs plus title and Greptile). Greptile4/5 raised two inherited main findings, not changes in this six-file diff: IPC attribution/follow-up #4593 and Ghidra attribution. Both threads are resolved for scope; neither existing issue is claimed fixed.

_{🔄 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/4562 **Author:** [@Nitin13-git](https://github.com/Nitin13-git) **Created:** 10/16/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/electron-shell-sync-effects` --- ### 📝 Commits (10+) - [`db5e4d4`](https://github.com/morluto/rea/commit/db5e4d41f0e52eb78df0de66ec0163d694c19c50) fix(electron): block and record synchronous shell effects in the active hook - [`c75cf82`](https://github.com/morluto/rea/commit/c75cf827e78d9864b9b36d2695e9e5153f9e8b78) fix(ghidra): retain typed Java prerequisite outcomes - [`ae07d15`](https://github.com/morluto/rea/commit/ae07d151704bac5f0129985cedb92546d2d5411e) fix(ghidra): explain keg-only JDK admission remediation - [`b0ddb58`](https://github.com/morluto/rea/commit/b0ddb58e27b18af996f0411b0eb02a128f237a0a) fix(ghidra): own PSP inspection finalization - [`f22e8e7`](https://github.com/morluto/rea/commit/f22e8e72685c05e372c62582c7bc5fcd085b2fa6) test(ghidra): align Java probe fixtures with raw output - [`bf34758`](https://github.com/morluto/rea/commit/bf3475867ff136b9fbdbadd37ba01a4f3c642a74) fix(electron): preserve native IPC listener behavior (#4389) - [`50dbd87`](https://github.com/morluto/rea/commit/50dbd875cf17aec70c38b6ac3a2d36e6895c6459) fix(process): preserve owning operation cancellation - [`bf7fad3`](https://github.com/morluto/rea/commit/bf7fad390c26db32d39671429ba35418ff2506da) fix(cleanup): project resources from unresolved owners - [`f26973a`](https://github.com/morluto/rea/commit/f26973a52918a938631e90fdc9116f7b85a62a52) fix(providers): own profile selection through finalization - [`0386b60`](https://github.com/morluto/rea/commit/0386b60fb685a512de75f78e98ea30a7f48a6865) fix(electron): integrate synchronous shell boundary blocking ### 📊 Changes **65 files changed** (+17973 additions, -687 deletions) <details> <summary>View changed files</summary> 📝 `docs/electron-observation.md` (+45 -0) 📝 `docs/installation.md` (+10 -0) 📝 `docs/tool-design.md` (+8 -0) ➕ `docs/verification/electron-ipc-listeners-2026-10-16.json` (+10377 -0) ➕ `docs/verification/electron-ipc-listeners-2026-10-16.md` (+592 -0) ➕ `docs/verification/electron-shell-sync-2026-10-16.json` (+1827 -0) ➕ `docs/verification/electron-shell-sync-2026-10-16.md` (+91 -0) ➕ `experiments/analysis-efficiency/drawio-electron-lifecycle-oracles.py` (+115 -0) ➕ `experiments/analysis-efficiency/drawio-electron-lifecycle.mjs` (+191 -0) ➕ `experiments/analysis-efficiency/joplin-shell-boundary.mjs` (+291 -0) 📝 `scripts/electron-active-hook-boundaries.cjs` (+15 -2) 📝 `scripts/electron-active-hook.cjs` (+297 -48) 📝 `src/adb/AdbProvider.ts` (+9 -3) 📝 `src/application/AnalysisProvider.ts` (+6 -0) 📝 `src/application/binary/AnalysisProviderEvaluation.ts` (+24 -25) 📝 `src/application/binary/AnalysisProviderRegistry.test.ts` (+253 -14) 📝 `src/application/binary/AnalysisProviderRegistry.ts` (+231 -39) 📝 `src/application/binary/BinarySession.ts` (+6 -6) 📝 `src/application/binary/BinarySessionOpen.ts` (+1 -9) 📝 `src/application/binary/BinarySessionRecords.ts` (+1 -36) _...and 45 more files_ </details> ### 📄 Description ## Summary Fixes #4393: during `capture_electron_scenario`, `shell.showItemInFolder` and `shell.writeShortcutLink` bypassed the active boundary hook, ran their native implementations on the host, and left no `shell-attempt` event. ## Problem and expected behavior `patchShell` in `scripts/electron-active-hook-boundaries.cjs` wrapped only the Promise-returning `openExternal` and `openPath`. `docs/electron-observation.md` says the active hook blocks and records OS-integration effects. `showItemInFolder` (synchronous `void`) opens the host file manager; `writeShortcutLink` (synchronous `boolean`) creates or replaces a Windows `.lnk` file. Expected: neither reaches the host, both are recorded as blocked, and the target sees an ordinary refusal. ## Change and scope - `patchShell` now takes its blocked methods and their refusal results from one table: the two existing methods keep rejecting with `External shell effects are blocked`; `showItemInFolder` returns `undefined` and `writeShortcutLink` returns `false`, matching Electron's synchronous contracts (never a rejected Promise or a throw). - Events are recorded through the existing `recordRuntime` path, so only argument shapes are retained; paths and shortcut options are not. - Not included: `shell.trashItem`, which #3559 already addresses (the table is where it would be added). `moveToApplicationsFolder`, mentioned in the issue as an analogue, is not covered here. - Scope decision: not applicable (tightens an existing blocking boundary; no new capability). ## Contract and boundary impact - Semantic owner and earliest changed stage: Electron active hook boundary patches - CLI and MCP/tool-catalog contract: none - Provider, bridge, target-format, or platform compatibility: Electron targets on all platforms; `writeShortcutLink` exists only on Windows - Evidence, artifact, provenance, or reconstruction contract: new `shell-attempt` events for these methods, same schema - Target selection, authority, upstream behavior, and cleanup details: reduces host side effects from the target; no new authority - [x] Launch or attach to a process (existing capture behavior, unchanged) ## Original-author evidence and regression coverage (author-reported) Executed on macOS arm64, Node 24.18.0: - New test in `tests/boundary/browser/electronActiveHook.test.ts` runs the real staged hook against a fake Electron whose `shell` records native calls. It asserts the native methods are never called, results are `undefined` and `false`, two blocked `shell-attempt` events carry only `["string"]` and `["string","string","object"]` shapes, the serialized snapshot contains neither the path nor the shortcut target, and events still parse against `electronActiveTimelineEventSchema`. It fails on the base revision. - Existing hook tests and `npm run check:fast` pass. - Not run: a real Electron target, `verify-real-electron.mjs`, Windows, full `check:pr`. ## Maintainer verification on current main Integrated onto main `f26973a52`; source/verifier `3f231735c`, current verification record in [electron-shell-sync-2026-10-16.md](https://github.com/Nitin13-git/rea/blob/fix/electron-shell-sync-effects/docs/verification/electron-shell-sync-2026-10-16.md) and adjacent JSON. Original contributor commit is retained. Executed on Linux, pinned Node24.18.0/npm11.16.0, serial1CPU/8GiB: - Base boundary + new test:73pass1fail; candidate74hook +5projection +8MCP =87pass. Fake native/controlled-provider coverage, not real Windows. - `npm run check:fast` and `npm run docs:check` pass. - Unchanged real Joplin3.7.21 ASAR on explicit generic Electron44.7.0: shipped welcome PNG → actual resource menu/bridge →0to1blocked reveal event; visible attachment, no page errors, exact retained shared-capture event. Final pipeline16.989s wall /16.055812 familyCPU s /1,376,382,976 peak cgroupB, payload observation verified. - **Overall real workflow still fails cleanup (#4282).** Standard launch remains unsuccessful (#2974); the valid operation probe explicitly uses a test-only argv shim. An invalid NODE_OPTIONS experiment and all other failed probes remain recorded. No production launcher/fuse change, Windows/macOS runtime claim, full-gate claim, cold/warm speedup or billed-token benefit. Current Codex review is unavailable due repository quota ([existing notice](https://github.com/morluto/rea/pull/4562#issuecomment-6223117983)); it is not claimed approved. All38current checks pass (36CI jobs plus title and Greptile). Greptile4/5 raised two inherited main findings, not changes in this six-file diff: [IPC attribution/follow-up #4593](https://github.com/morluto/rea/pull/4562#discussion_r4299308536) and [Ghidra attribution](https://github.com/morluto/rea/pull/4562#discussion_r4299308822). Both threads are resolved for scope; neither existing issue is claimed fixed. --- <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:27 +00:00