[PR #3533] [CLOSED] fix(browser): block and record shell.trashItem in active electron hook #5809

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3533
Author: @taljeri
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/electron-active-hook-trash-item


📝 Commits (1)

  • bd362db fix(browser): block and record shell.trashItem in active electron hook

📊 Changes

2 files changed (+75 additions, -2 deletions)

View changed files

📝 scripts/electron-active-hook-boundaries.cjs (+1 -1)
📝 tests/boundary/browser/electronActiveHook.test.ts (+74 -1)

📄 Description

Summary

In capture_electron_scenario active hook observation, patchShell intercepted shell.openExternal and shell.openPath but omitted shell.trashItem. An inspected application calling shell.trashItem during scenario capture could move host files to the OS trash without being blocked or evidenced on the capture timeline.

Fixes #3474

Problem and expected behavior

docs/electron-observation.md declares that the active hook blocks and records external shell/navigation and OS-integration effects. When an inspected application called shell.trashItem(fullPath), the call passed through uninhibited to the underlying implementation, trashing files on the host without recording any shell-attempt event.

Expected behavior: shell.trashItem must be intercepted by patchShell, reject with new Error("External shell effects are blocked"), and record a shell-attempt:trashItem:blocked timeline event.

Change and scope

  • Added trashItem to the patched methods list in patchShell in scripts/electron-active-hook-boundaries.cjs.
  • Added test coverage in tests/boundary/browser/electronActiveHook.test.ts verifying that shell.trashItem is blocked and records the expected shell-attempt:trashItem:blocked timeline event.

Contract and boundary impact

  • Semantic owner and earliest changed stage: Electron active hook boundary (scripts/electron-active-hook-boundaries.cjs)
  • Contribution scope fit and linked maintainer decision: not applicable (standard hook boundary fix)
  • CLI and MCP/tool-catalog contract: not applicable
  • Provider, bridge, target-format, or platform compatibility: Electron active hook runtime
  • Evidence, artifact, provenance, or reconstruction contract: preserves timeline evidence for blocked shell attempts
  • Target selection, authority, upstream behavior, and cleanup details: not applicable
  • Generated metadata impact: none

Expected effects:

  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/browser/electronActiveHook.test.ts
  • Base reproduction: Calling shell.trashItem in an instrumented shell object resolves without recording a shell-attempt event on base. With this change, shell.trashItem rejects and records shell-attempt:trashItem:blocked.

Validation performed

  • npx vitest run tests/boundary/browser/electronActiveHook.test.ts — 4/4 tests passed
  • npm run typecheck — 0 errors
  • npx oxlint --fix scripts/electron-active-hook-boundaries.cjs tests/boundary/browser/electronActiveHook.test.ts — 0 errors, 0 warnings

Compatibility, safety, and release

  • Breaking changes or migration steps: none
  • Security, privacy, process, or containment review: ensures inspected targets cannot trash host files during active observation

Author readiness

Human owner: @taljeri

  • 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/3533 **Author:** [@taljeri](https://github.com/taljeri) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/electron-active-hook-trash-item` --- ### 📝 Commits (1) - [`bd362db`](https://github.com/morluto/rea/commit/bd362db01774e94aa96d6438ed031ba4e91b8f29) fix(browser): block and record shell.trashItem in active electron hook ### 📊 Changes **2 files changed** (+75 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `scripts/electron-active-hook-boundaries.cjs` (+1 -1) 📝 `tests/boundary/browser/electronActiveHook.test.ts` (+74 -1) </details> ### 📄 Description ## Summary In `capture_electron_scenario` active hook observation, `patchShell` intercepted `shell.openExternal` and `shell.openPath` but omitted `shell.trashItem`. An inspected application calling `shell.trashItem` during scenario capture could move host files to the OS trash without being blocked or evidenced on the capture timeline. Fixes #3474 ## Problem and expected behavior `docs/electron-observation.md` declares that the active hook blocks and records external shell/navigation and OS-integration effects. When an inspected application called `shell.trashItem(fullPath)`, the call passed through uninhibited to the underlying implementation, trashing files on the host without recording any `shell-attempt` event. Expected behavior: `shell.trashItem` must be intercepted by `patchShell`, reject with `new Error("External shell effects are blocked")`, and record a `shell-attempt:trashItem:blocked` timeline event. ## Change and scope - Added `trashItem` to the patched methods list in `patchShell` in `scripts/electron-active-hook-boundaries.cjs`. - Added test coverage in `tests/boundary/browser/electronActiveHook.test.ts` verifying that `shell.trashItem` is blocked and records the expected `shell-attempt:trashItem:blocked` timeline event. ## Contract and boundary impact - Semantic owner and earliest changed stage: Electron active hook boundary (`scripts/electron-active-hook-boundaries.cjs`) - Contribution scope fit and linked maintainer decision: not applicable (standard hook boundary fix) - CLI and MCP/tool-catalog contract: not applicable - Provider, bridge, target-format, or platform compatibility: Electron active hook runtime - Evidence, artifact, provenance, or reconstruction contract: preserves timeline evidence for blocked shell attempts - Target selection, authority, upstream behavior, and cleanup details: not applicable - Generated metadata impact: none Expected effects: - [x] Launch or attach to a process. - [x] Inject instrumentation or run scripts/code. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/browser/electronActiveHook.test.ts` - Base reproduction: Calling `shell.trashItem` in an instrumented shell object resolves without recording a `shell-attempt` event on base. With this change, `shell.trashItem` rejects and records `shell-attempt:trashItem:blocked`. ## Validation performed - `npx vitest run tests/boundary/browser/electronActiveHook.test.ts` — 4/4 tests passed - `npm run typecheck` — 0 errors - `npx oxlint --fix scripts/electron-active-hook-boundaries.cjs tests/boundary/browser/electronActiveHook.test.ts` — 0 errors, 0 warnings ## Compatibility, safety, and release - Breaking changes or migration steps: none - Security, privacy, process, or containment review: ensures inspected targets cannot trash host files during active observation ## Author readiness Human owner: @taljeri - [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 - [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:09:35 +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#5809
No description provided.