morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #5835] [Bug] analyze_web_bundle omits its process-launch and temporary-file effects from discovery #3533

New issue

Closed

opened 2026-10-31 00:41:35 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-31 00:41:35 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 21, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5835

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Provider or capability discovery

Observed behavior

An executed MCP test called binary_session with {} and found analyze_web_bundle available, but its group contained only effects: ["mutatesSession", "accessesNetwork", "idempotent"]. Both launchesProcess and writesFilesystem were absent. A subsequent analyze_web_bundle request on the same server successfully analyzed the fake CDP browser's captured script and returned its /api/verify endpoint.

The process and file behavior is corroborated by the production source path: successful analysis of included source starts an owned Node worker and transfers the source through a private file. The test used the real analyzer and worker, with an in-memory MCP connection and the repository's fake CDP browser; it did not trace system calls or run Chrome.

Reproduction

With the baseline's dependencies installed and runtime built, use its pinned Node.js version and place this temporary test at src/browser/repro.test.ts. Run npm run test:local -- src/browser/repro.test.ts, then remove the test. During verification, HOME, XDG configuration/data/cache/state directories, and TMPDIR all pointed to isolated scratch directories.

import { expect, onTestFinished } from "vitest";
import { mcpTest } from "../../tests/support/mcp/mcpFixture.js";
import { createTestBinarySession } from "../../tests/fixtures/binarySession.js";
import { observed } from "../../tests/fixtures/analysisExecution.js";
import { startFakeCdpBrowser } from "../../tests/fixtures/fakeCdpBrowser.js";
import { ok } from "../domain/result.js";
import { CdpBrowserProvider } from "../browser/CdpBrowserProvider.js";
import { createServer } from "../server/createServer.js";
import { toolContract } from "../contracts/toolContracts.js";

mcpTest("bundle analysis discloses its temporary worker effects", async ({ mcp }) => {
  const browser = await startFakeCdpBrowser({
    commandResult: ({ method }) => method === "Debugger.getScriptSource"
      ? { scriptSource: "fetch('/api/verify');" } : undefined,
  });
  onTestFinished(() => browser.close());
  const session = createTestBinarySession(() => ({
    execute: () => Promise.resolve(observed(null)),
    close: () => Promise.resolve(ok(null)),
  }));
  onTestFinished(() => session.close());
  const client = await mcp.connect(createServer({ kind: "session", session }, {
    browserObservation: new CdpBrowserProvider(),
    availabilityPolicy: () => ({ processCaptureEnabled: false, investigationInputRoots: 0, browserObservationEnabled: true }),
  }));
  const status = await client.callTool({ name: "binary_session", arguments: {} });
  const discovery = toolContract("binary_session").outputSchema.parse(status.structuredContent);
  const group = discovery.result.tool_availability.groups.find(
    (entry) => entry.names.includes("analyze_web_bundle"),
  );
  console.log("discovery", JSON.stringify({ effects: group?.effects, annotations: group?.annotations, available: group?.available }));
  const reply = await client.callTool({ name: "analyze_web_bundle", arguments: {
    cdp_endpoint: browser.endpoint, allowed_origins: [browser.allowedOrigin],
    target_id: "allowed-page", observation_ms: 0, max_heap_mb: 128,
  } });
  expect(reply.isError, JSON.stringify(reply.content)).not.toBe(true);
  expect(reply.structuredContent).toMatchObject({ normalized_result: { observations: {
    endpoints: [expect.objectContaining({ value: "/api/verify" })],
  } } });
  console.log("analysis", JSON.stringify({ isError: reply.isError ?? false, endpoint: "/api/verify", activeConnections: browser.activeConnectionCount() }));
  expect(group?.effects).toEqual(expect.arrayContaining(["launchesProcess", "writesFilesystem"]));
}, 30_000);

The test reached successful analysis and then failed the final effect-disclosure assertion in one completed reproduction. All browser interaction was local to the fake HTTP/WebSocket server; source-map fetching was not requested.

Expected behavior

The binary_session group for analyze_web_bundle should include launchesProcess and writesFilesystem. The contract includes possible effects across supported inputs, even temporary work; omission from the closed effect array means false. See the effect-disclosure contract and the true-only array rule.

The browser guide already documents owned-child analysis and private-file cleanup. This expectation concerns the operation's internal helper work, not execution of the captured application code.

Impact and scope

P2: callers receive incorrect public discovery facts about the local resources this operation uses. This is a bounded metadata defect: the reproduced analysis returned its expected endpoint, while its declared effects excluded process launch and filesystem writes. The static declaration applies to every discovery record for this tool; actual worker use depends on having included scripts that reach analysis.

No downstream approval failure, sandbox incompatibility, security-policy bypass, or incorrect analysis finding was demonstrated. The existing annotations already omit read_only and destructive; both remain false when the missing effects are added. Real Chrome, Linux, and Windows were not tested. A worker may be reused across multiple scripts, so this report does not claim one new process per script.

REA version

rea-agents 6.7.0; source baseline a5f982de8d.

Runtime and platform

Node.js v24.18.0; macOS 27.0.1 arm64; local fake CDP browser and in-memory MCP transport.

Target and artifact context

Fake CDP target allowed-page; captured JavaScript source fetch('/api/verify'); no source-map fetching or application-code execution.

Interface

MCP

Earliest failure stage

Transport or result projection

Relevant output or evidence

Trimmed output from the focused test:

discovery {"effects":["mutatesSession","accessesNetwork","idempotent"],"annotations":["idempotent","open_world"],"available":true}
analysis {"isError":false,"endpoint":"/api/verify","activeConnections":0}

- Expected
+ Received

- ArrayContaining [
-   "launchesProcess",
-   "writesFilesystem",
+ [
+   "mutatesSession",
+   "accessesNetwork",
+   "idempotent",
  ]

Test Files 1 failed (1)
Tests 1 failed (1)

Source corroboration:

Typed outcome or recovery expected

No response

Analysis or suspected cause

Source-derived: analyze_web_bundle shares browserEvidence, which sets session mutation and network access only. Give this tool an effect declaration that additionally includes its owned worker and temporary filesystem work. No runtime behavior change is needed to address the reported mismatch. The historical introduction point was not established.

Additional context

Read-only searches of open and closed issues and PRs found no matching report. The signature-inspection metadata report concerns a different tool. The closed web-bundle containment report concerns in-process heap exhaustion, not this remaining disclosure error. No fix or post-fix verification was performed.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5835 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Provider or capability discovery ### Observed behavior An executed MCP test called `binary_session` with `{}` and found `analyze_web_bundle` available, but its group contained only `effects: ["mutatesSession", "accessesNetwork", "idempotent"]`. Both `launchesProcess` and `writesFilesystem` were absent. A subsequent `analyze_web_bundle` request on the same server successfully analyzed the fake CDP browser's captured script and returned its `/api/verify` endpoint. The process and file behavior is corroborated by the production source path: successful analysis of included source starts an owned Node worker and transfers the source through a private file. The test used the real analyzer and worker, with an in-memory MCP connection and the repository's fake CDP browser; it did not trace system calls or run Chrome. ### Reproduction With the baseline's dependencies installed and runtime built, use its pinned Node.js version and place this temporary test at `src/browser/repro.test.ts`. Run `npm run test:local -- src/browser/repro.test.ts`, then remove the test. During verification, HOME, XDG configuration/data/cache/state directories, and TMPDIR all pointed to isolated scratch directories. ```ts import { expect, onTestFinished } from "vitest"; import { mcpTest } from "../../tests/support/mcp/mcpFixture.js"; import { createTestBinarySession } from "../../tests/fixtures/binarySession.js"; import { observed } from "../../tests/fixtures/analysisExecution.js"; import { startFakeCdpBrowser } from "../../tests/fixtures/fakeCdpBrowser.js"; import { ok } from "../domain/result.js"; import { CdpBrowserProvider } from "../browser/CdpBrowserProvider.js"; import { createServer } from "../server/createServer.js"; import { toolContract } from "../contracts/toolContracts.js"; mcpTest("bundle analysis discloses its temporary worker effects", async ({ mcp }) => { const browser = await startFakeCdpBrowser({ commandResult: ({ method }) => method === "Debugger.getScriptSource" ? { scriptSource: "fetch('/api/verify');" } : undefined, }); onTestFinished(() => browser.close()); const session = createTestBinarySession(() => ({ execute: () => Promise.resolve(observed(null)), close: () => Promise.resolve(ok(null)), })); onTestFinished(() => session.close()); const client = await mcp.connect(createServer({ kind: "session", session }, { browserObservation: new CdpBrowserProvider(), availabilityPolicy: () => ({ processCaptureEnabled: false, investigationInputRoots: 0, browserObservationEnabled: true }), })); const status = await client.callTool({ name: "binary_session", arguments: {} }); const discovery = toolContract("binary_session").outputSchema.parse(status.structuredContent); const group = discovery.result.tool_availability.groups.find( (entry) => entry.names.includes("analyze_web_bundle"), ); console.log("discovery", JSON.stringify({ effects: group?.effects, annotations: group?.annotations, available: group?.available })); const reply = await client.callTool({ name: "analyze_web_bundle", arguments: { cdp_endpoint: browser.endpoint, allowed_origins: [browser.allowedOrigin], target_id: "allowed-page", observation_ms: 0, max_heap_mb: 128, } }); expect(reply.isError, JSON.stringify(reply.content)).not.toBe(true); expect(reply.structuredContent).toMatchObject({ normalized_result: { observations: { endpoints: [expect.objectContaining({ value: "/api/verify" })], } } }); console.log("analysis", JSON.stringify({ isError: reply.isError ?? false, endpoint: "/api/verify", activeConnections: browser.activeConnectionCount() })); expect(group?.effects).toEqual(expect.arrayContaining(["launchesProcess", "writesFilesystem"])); }, 30_000); ``` The test reached successful analysis and then failed the final effect-disclosure assertion in one completed reproduction. All browser interaction was local to the fake HTTP/WebSocket server; source-map fetching was not requested. ### Expected behavior The `binary_session` group for `analyze_web_bundle` should include `launchesProcess` and `writesFilesystem`. The contract includes possible effects across supported inputs, even temporary work; omission from the closed effect array means false. See [the effect-disclosure contract](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/mcp-contracts.md#L741-L755) and [the true-only array rule](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/mcp-contracts.md#L493-L495). The browser guide already documents [owned-child analysis and private-file cleanup](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/browser-observation.md#L396-L413). This expectation concerns the operation's internal helper work, not execution of the captured application code. ### Impact and scope P2: callers receive incorrect public discovery facts about the local resources this operation uses. This is a bounded metadata defect: the reproduced analysis returned its expected endpoint, while its declared effects excluded process launch and filesystem writes. The static declaration applies to every discovery record for this tool; actual worker use depends on having included scripts that reach analysis. No downstream approval failure, sandbox incompatibility, security-policy bypass, or incorrect analysis finding was demonstrated. The existing annotations already omit `read_only` and `destructive`; both remain false when the missing effects are added. Real Chrome, Linux, and Windows were not tested. A worker may be reused across multiple scripts, so this report does not claim one new process per script. ### REA version rea-agents 6.7.0; source baseline a5f982de8dbe4b092cc20ab89d492b7e7028b758. ### Runtime and platform Node.js v24.18.0; macOS 27.0.1 arm64; local fake CDP browser and in-memory MCP transport. ### Target and artifact context Fake CDP target allowed-page; captured JavaScript source fetch('/api/verify'); no source-map fetching or application-code execution. ### Interface MCP ### Earliest failure stage Transport or result projection ### Relevant output or evidence Trimmed output from the focused test: ```text discovery {"effects":["mutatesSession","accessesNetwork","idempotent"],"annotations":["idempotent","open_world"],"available":true} analysis {"isError":false,"endpoint":"/api/verify","activeConnections":0} - Expected + Received - ArrayContaining [ - "launchesProcess", - "writesFilesystem", + [ + "mutatesSession", + "accessesNetwork", + "idempotent", ] Test Files 1 failed (1) Tests 1 failed (1) ``` Source corroboration: - [The browser effect alias defaults filesystem and process effects to false](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/contracts/toolEffects.ts#L32-L51), and [analyze_web_bundle uses that alias](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/contracts/toolEffects.ts#L619-L623). - [The registered MCP handler invokes the application workflow](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/server/registerBrowserTools.ts#L91-L104), which [checks browser availability and cancellation before calling the provider](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/BrowserObservationService.ts#L93-L107). - [The browser provider reserves, captures, and delegates to the analyzer](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/browser/CdpBrowserProvider.ts#L186-L218). The analyzer [constructs the real worker by default](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/browser/IsolatedWebBundleAnalyzer.ts#L42-L45) and [passes included scripts to it](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/browser/IsolatedWebBundleAnalyzer.ts#L171-L182). - [Each analyzed source is transferred through a private source file](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/browser/WebBundleAnalysisWorker.ts#L281-L296), using [exclusive creation and file writes](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/browser/WebBundleAnalysisWorker.ts#L467-L497). [Worker startup selects process.execPath](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/browser/WebBundleAnalysisWorker.ts#L500-L524); [the owned runtime acquires a private root and launches the child](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/process/OwnedWorkerRuntime.ts#L303-L312). - [Discovery copies the canonical effects](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/CapabilityInventory.ts#L253-L272) and [includes only their true members in each group](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/CapabilityInventory.ts#L313-L339). ### Typed outcome or recovery expected _No response_ ### Analysis or suspected cause Source-derived: `analyze_web_bundle` shares `browserEvidence`, which sets session mutation and network access only. Give this tool an effect declaration that additionally includes its owned worker and temporary filesystem work. No runtime behavior change is needed to address the reported mismatch. The historical introduction point was not established. ### Additional context Read-only searches of open and closed issues and PRs found no matching report. [The signature-inspection metadata report](https://github.com/morluto/rea/issues/5423) concerns a different tool. [The closed web-bundle containment report](https://github.com/morluto/rea/issues/5132) concerns in-process heap exhaustion, not this remaining disclosure error. No fix or post-fix verification was performed.

bsakaguchi closed this issue

2026-10-31 00:41:35 +00:00

bsakaguchi commented

2026-10-31 00:41:36 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 21, 2026):

Confirmed — the browser effect alias defaults filesystem/process effects to false and analyze_web_bundle inherits that, yet the operation demonstrably spawns an owned worker and transfers sources through private files, so the discovery record understates its local resource use. Since the effect array is closed (omission means false), callers could wrongly assume approval/sandbox safety. Declaring launchesProcess and writesFilesystem (keeping read_only/destructive false) matches both the documented owned-child behavior and the true-only array rule.

@XinwangTao commented on GitHub (Oct 21, 2026): Confirmed — the browser effect alias defaults filesystem/process effects to false and analyze_web_bundle inherits that, yet the operation demonstrably spawns an owned worker and transfers sources through private files, so the discovery record understates its local resource use. Since the effect array is closed (omission means false), callers could wrongly assume approval/sandbox safety. Declaring launchesProcess and writesFilesystem (keeping read_only/destructive false) matches both the documented owned-child behavior and the true-only array rule.

bsakaguchi commented

2026-10-31 00:41:36 +00:00

Author

Owner

@coygeek commented on GitHub (Oct 24, 2026):

Checked the reported fix on main at d87e73f2db.

The canonical analyze_web_bundle entry now extends browserEvidence with launchesProcess and writesFilesystem. Its contract regression asserts both flags plus nonforeign/nonmutating-target behavior. Discovery copies canonical effects and true members, so the original missing flags are repaired at their producer.

Implementation: toolEffects.ts, toolEffects.test.ts.

The relevant regression checks in toolEffects.test.ts passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked the reported fix on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. The canonical analyze_web_bundle entry now extends browserEvidence with launchesProcess and writesFilesystem. Its contract regression asserts both flags plus nonforeign/nonmutating-target behavior. Discovery copies canonical effects and true members, so the original missing flags are repaired at their producer. Implementation: [toolEffects.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/contracts/toolEffects.ts#L636), [toolEffects.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/contracts/toolEffects.test.ts#L117). The relevant regression checks in [toolEffects.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/contracts/toolEffects.test.ts) passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

bsakaguchi referenced this issue

2026-10-31 01:09:35 +00:00

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

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

No description provided.

Rows
Columns