morluto/rea

Fork 0

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

[GH-ISSUE #7684] [Bug] Empty observation duration silently selects a zero-millisecond web capture #4606

New issue

Closed

opened 2026-10-31 00:51:15 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:51:15 +00:00

Owner

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

Before submitting

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

Area

CLI

Observed behavior

Running node scripts/rea.mjs inspect-web-page ENDPOINT allowed-page --observation-ms= --json against a loopback CDP fixture exits 0 and returns successful Evidence whose normalized_result.capture_window.observation_ms is 0. Supplying a separate empty argument has the same result. Omitting the option returns 500, and explicitly selecting --observation-ms=0 returns 0.

The negative control --observation-ms -1 fails with VALIDATION_ERROR naming observationMs. Other accepted spellings were 0x10 (16) and 1e3 (1000); these characterize the conversion path, without assuming the duration help explicitly prohibits those numeric spellings.

Reproduction

Use the baseline below, Node.js 24.22.1, and its installed locked dependencies and built runtime. From the checkout root, create an isolated environment and package-runner blockers:

WORK="$(mktemp -d)"
mkdir -p "$WORK"/{bin,home,config,cache,data,state,temp}
for runner in npx npm bun; do
  printf '#!/bin/sh\nexit 97\n' > "$WORK/bin/$runner"
  chmod +x "$WORK/bin/$runner"
done
NODE="$(node -p process.execPath)"

Save this as $WORK/reproduce.mts. It uses the repository's HTTP/WebSocket CDP fixture and the public CLI; it launches no browser:

import { spawn } from "node:child_process";
import { pathToFileURL } from "node:url";
const { startFakeCdpBrowser } = await import(
  pathToFileURL(`${process.cwd()}/tests/fixtures/fakeCdpBrowser.ts`).href
);
const browser = await startFakeCdpBrowser();
async function run(flags) {
  return await new Promise((resolve, reject) => {
    const child = spawn(process.execPath, ["scripts/rea.mjs", "inspect-web-page",
      browser.endpoint, "allowed-page", "--allowed-origins", browser.allowedOrigin,
      ...flags, "--json"], { env: process.env });
    let stdout = "", stderr = "";
    child.stdout.on("data", chunk => { stdout += chunk; });
    child.stderr.on("data", chunk => { stderr += chunk; });
    child.on("error", reject);
    child.on("close", exit => {
      const result = JSON.parse(stdout);
      resolve({ flags, exit, stderr,
        observation_ms: result.normalized_result?.capture_window?.observation_ms,
        code: result.code, fieldErrors: result.fieldErrors });
    });
  });
}
try {
  for (const flags of [["--observation-ms="], ["--observation-ms", "-1"],
    ["--observation-ms=0x10"], ["--observation-ms=1e3"],
    ["--observation-ms=0"], []]) console.log(JSON.stringify(await run(flags)));
} finally { await browser.close(); }

Run it with isolated state:

env -i HOME="$WORK/home" XDG_CONFIG_HOME="$WORK/config" \
  XDG_CACHE_HOME="$WORK/cache" XDG_DATA_HOME="$WORK/data" \
  XDG_STATE_HOME="$WORK/state" TMPDIR="$WORK/temp" \
  npm_config_prefix="$WORK/home/npm-global" \
  PATH="$WORK/bin:$(dirname "$NODE"):/usr/bin:/bin" \
  REA_LOG_LEVEL=silent NO_COLOR=1 \
  "$NODE" --import ./node_modules/tsx/dist/loader.mjs "$WORK/reproduce.mts"
rm -rf "$WORK"

All six cases reproduced twice during verification: once in the primary CLI/MCP probe and once with the reduced script above. The separate-empty-argument case was additionally checked once.

Expected behavior

Reject an explicitly empty duration with a nonzero VALIDATION_ERROR identifying observationMs. Keep omission as the documented 500 ms default and explicit numeric zero as a valid caller choice. Empty text should not silently choose a zero-length observation window.

The public help advertises a number, and the public CLI/MCP schemas advertise a nonnegative integer. The documented input-error behavior provides field-specific correction information. Source and documentation locators appear under Evidence.

Impact and scope

P2: a script that supplies an empty duration variable receives a successful zero-window result. This silently changes the capture interval instead of reporting a mistyped argument. Omitting the option when no duration is selected and supplying an explicit decimal duration are demonstrated workarounds.

Observed scope is the inspect-web-page CLI against a simulated loopback CDP target. The fixture returned some setup events even at zero; this report does not claim that all events disappear or demonstrate a missed later event. No data loss, real-browser downstream incident, or security boundary crossing was observed. The equivalent malformed string inputs were rejected through production stdio MCP.

REA version

Exact node scripts/rea.mjs --version output: 6.8.0 followed by a newline. Source baseline: 65f1c447344a423fa2785847dbcea1e9a9e2454a.

Runtime and platform

Node.js v24.22.1; macOS 27.0.1; darwin arm64. Fake CDP provider only.

Target and artifact context

The repository's fake CDP browser served HTTP and WebSocket connections on 127.0.0.1. Each REA process used isolated HOME/XDG/temp state with fixture npx/npm/bun first on PATH. No real browser or GUI was launched.

Interface

CLI

Earliest failure stage

Request parsing or validation

Relevant output or evidence

Observed public CLI results, reduced to the discriminating fields; all stderr was empty:

Duration argument	Exit	Result
`--observation-ms=`	0	`capture_window.observation_ms: 0`
`--observation-ms ""`	0	`capture_window.observation_ms: 0`
`--observation-ms -1`	1	`VALIDATION_ERROR`, field `observationMs`, code `too_small`
`--observation-ms=0x10`	0	`capture_window.observation_ms: 16`
`--observation-ms=1e3`	0	`capture_window.observation_ms: 1000`
`--observation-ms=0`	0	`capture_window.observation_ms: 0`
omitted	0	`capture_window.observation_ms: 500`

Observed inspect-web-page --schema --json duration property:

{"description":"Observation duration in milliseconds","default":500,"type":"integer","minimum":0,"maximum":9007199254740991}

Through node scripts/rea.mjs mcp and the locked stdio MCP client, actual tools/list advertised an integer observation_ms with minimum 0. Calling inspect_web_page with the same fixture endpoint, target_id allowed-page, and observation_ms set to "", "0x10", or "1e3" returned isError: true, invalid_request, and observation_ms: Invalid input: expected number, received string. Numeric -1 returned invalid_request for the minimum. Numeric 0 succeeded with capture_window.observation_ms: 0. Each MCP case ran once. This compares strings with strings; a JSON numeric exponent is already a number.

Contract and failure locators at the baseline:

docs/browser-observation.md:504: the default inspection window is 500 ms and callers choose its duration.
docs/user-visible-errors.csv:36, row 24.95: mistyped command arguments yield VALIDATION_ERROR and fieldErrors. This row defines diagnostics rather than every accepted numeric token spelling.
src/cli/options.ts:16-21: existing raw decimal-integer helper rejects empty tokens before conversion.
src/cliObservationOptions.ts:8-14,35: numeric duration declaration and page-inspection option.
src/cli/browserCommands.ts:100-129: supported command registration and forwarding of the parsed duration.
src/domain/browserObservation.ts:208-212: numeric domain input permits explicit zero.
node_modules/incur/src/Parser.ts:322-330: locked parser converts numeric option strings with Number before validation.

Typed outcome or recovery expected

An empty duration should fail before provider admission with a diagnostic naming observationMs and no success Evidence. A restoration check must retain omitted-default, explicit-zero, and ordinary positive-duration behavior. No repaired build was tested.

Analysis or suspected cause

Source-derived: the CLI duration uses a numeric schema, so Incur converts empty text into numeric zero before the nonnegative-integer checks run. The domain receives a valid zero and cannot distinguish it from an explicit caller choice. The existing raw-token integer helper validates before conversion, but these duration declarations do not use it.

The common duration helper also feeds analyze-web-bundle and inspect-electron-page. discover-webmcp-tools and observe-javascript-runtime have separate inline min-0 numeric declarations. Those paths were inspected statically, not reproduced end to end; they are possible affected scope rather than additional observed failures.

Additional context

Related #7376 concerns byte-window arguments, and #7093 / PR #7092 concern listener flags. Closed #7425 / merged PR #7505 and closed #7430 / merged PR #7493 adopt strict schemas for outline depth and Android selectors respectively. Their fixes do not change these duration options.

Those reports share a conversion mechanism, but do not describe a broken REA-wide helper whose repair closes every option. The raw-token helper already works; each affected registration must opt into the appropriate contract. This report closes when an empty observation duration is rejected while omission and explicit zero retain their meanings. Closed #6290 concerns a duration maximum, and closed #3929 / #4392 concern timer overflow, separate acceptance criteria. Open/closed issues, PRs, and comments were searched; no matching empty-duration report or general parser fix was found. The first affected release is unknown.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Empty option text returns successful Evidence with a zero observation window, silently overriding the caller's unset duration; the observed effect is bounded and no severe downstream incident was observed.
Workaround status: available
Workaround basis: Omitting the flag returned the documented 500 ms window, and explicit decimal zero selected a valid zero window on the same fixture.

Originally created by @coygeek on GitHub (Oct 28, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7684 ### 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 CLI ### Observed behavior Running `node scripts/rea.mjs inspect-web-page ENDPOINT allowed-page --observation-ms= --json` against a loopback CDP fixture exits 0 and returns successful Evidence whose `normalized_result.capture_window.observation_ms` is 0. Supplying a separate empty argument has the same result. Omitting the option returns 500, and explicitly selecting `--observation-ms=0` returns 0. The negative control `--observation-ms -1` fails with `VALIDATION_ERROR` naming `observationMs`. Other accepted spellings were `0x10` (16) and `1e3` (1000); these characterize the conversion path, without assuming the duration help explicitly prohibits those numeric spellings. ### Reproduction 1. Use the baseline below, Node.js 24.22.1, and its installed locked dependencies and built runtime. From the checkout root, create an isolated environment and package-runner blockers: ```sh WORK="$(mktemp -d)" mkdir -p "$WORK"/{bin,home,config,cache,data,state,temp} for runner in npx npm bun; do printf '#!/bin/sh\nexit 97\n' > "$WORK/bin/$runner" chmod +x "$WORK/bin/$runner" done NODE="$(node -p process.execPath)" ``` 2. Save this as `$WORK/reproduce.mts`. It uses the repository's HTTP/WebSocket CDP fixture and the public CLI; it launches no browser: ```ts import { spawn } from "node:child_process"; import { pathToFileURL } from "node:url"; const { startFakeCdpBrowser } = await import( pathToFileURL(`${process.cwd()}/tests/fixtures/fakeCdpBrowser.ts`).href ); const browser = await startFakeCdpBrowser(); async function run(flags) { return await new Promise((resolve, reject) => { const child = spawn(process.execPath, ["scripts/rea.mjs", "inspect-web-page", browser.endpoint, "allowed-page", "--allowed-origins", browser.allowedOrigin, ...flags, "--json"], { env: process.env }); let stdout = "", stderr = ""; child.stdout.on("data", chunk => { stdout += chunk; }); child.stderr.on("data", chunk => { stderr += chunk; }); child.on("error", reject); child.on("close", exit => { const result = JSON.parse(stdout); resolve({ flags, exit, stderr, observation_ms: result.normalized_result?.capture_window?.observation_ms, code: result.code, fieldErrors: result.fieldErrors }); }); }); } try { for (const flags of [["--observation-ms="], ["--observation-ms", "-1"], ["--observation-ms=0x10"], ["--observation-ms=1e3"], ["--observation-ms=0"], []]) console.log(JSON.stringify(await run(flags))); } finally { await browser.close(); } ``` 3. Run it with isolated state: ```sh env -i HOME="$WORK/home" XDG_CONFIG_HOME="$WORK/config" \ XDG_CACHE_HOME="$WORK/cache" XDG_DATA_HOME="$WORK/data" \ XDG_STATE_HOME="$WORK/state" TMPDIR="$WORK/temp" \ npm_config_prefix="$WORK/home/npm-global" \ PATH="$WORK/bin:$(dirname "$NODE"):/usr/bin:/bin" \ REA_LOG_LEVEL=silent NO_COLOR=1 \ "$NODE" --import ./node_modules/tsx/dist/loader.mjs "$WORK/reproduce.mts" rm -rf "$WORK" ``` All six cases reproduced twice during verification: once in the primary CLI/MCP probe and once with the reduced script above. The separate-empty-argument case was additionally checked once. ### Expected behavior Reject an explicitly empty duration with a nonzero `VALIDATION_ERROR` identifying `observationMs`. Keep omission as the documented 500 ms default and explicit numeric zero as a valid caller choice. Empty text should not silently choose a zero-length observation window. The public help advertises a number, and the public CLI/MCP schemas advertise a nonnegative integer. The documented input-error behavior provides field-specific correction information. Source and documentation locators appear under Evidence. ### Impact and scope P2: a script that supplies an empty duration variable receives a successful zero-window result. This silently changes the capture interval instead of reporting a mistyped argument. Omitting the option when no duration is selected and supplying an explicit decimal duration are demonstrated workarounds. Observed scope is the inspect-web-page CLI against a simulated loopback CDP target. The fixture returned some setup events even at zero; this report does not claim that all events disappear or demonstrate a missed later event. No data loss, real-browser downstream incident, or security boundary crossing was observed. The equivalent malformed string inputs were rejected through production stdio MCP. ### REA version Exact `node scripts/rea.mjs --version` output: `6.8.0` followed by a newline. Source baseline: `65f1c447344a423fa2785847dbcea1e9a9e2454a`. ### Runtime and platform Node.js v24.22.1; macOS 27.0.1; darwin arm64. Fake CDP provider only. ### Target and artifact context The repository's fake CDP browser served HTTP and WebSocket connections on 127.0.0.1. Each REA process used isolated HOME/XDG/temp state with fixture npx/npm/bun first on PATH. No real browser or GUI was launched. ### Interface CLI ### Earliest failure stage Request parsing or validation ### Relevant output or evidence Observed public CLI results, reduced to the discriminating fields; all stderr was empty: | Duration argument | Exit | Result | | --- | --- | --- | | `--observation-ms=` | 0 | `capture_window.observation_ms: 0` | | `--observation-ms ""` | 0 | `capture_window.observation_ms: 0` | | `--observation-ms -1` | 1 | `VALIDATION_ERROR`, field `observationMs`, code `too_small` | | `--observation-ms=0x10` | 0 | `capture_window.observation_ms: 16` | | `--observation-ms=1e3` | 0 | `capture_window.observation_ms: 1000` | | `--observation-ms=0` | 0 | `capture_window.observation_ms: 0` | | omitted | 0 | `capture_window.observation_ms: 500` | Observed `inspect-web-page --schema --json` duration property: ```json {"description":"Observation duration in milliseconds","default":500,"type":"integer","minimum":0,"maximum":9007199254740991} ``` Through `node scripts/rea.mjs mcp` and the locked stdio MCP client, actual tools/list advertised an integer observation_ms with minimum 0. Calling `inspect_web_page` with the same fixture endpoint, target_id `allowed-page`, and observation_ms set to `""`, `"0x10"`, or `"1e3"` returned `isError: true`, `invalid_request`, and `observation_ms: Invalid input: expected number, received string`. Numeric -1 returned invalid_request for the minimum. Numeric 0 succeeded with `capture_window.observation_ms: 0`. Each MCP case ran once. This compares strings with strings; a JSON numeric exponent is already a number. Contract and failure locators at the baseline: - `docs/browser-observation.md:504`: the default inspection window is 500 ms and callers choose its duration. - `docs/user-visible-errors.csv:36`, row 24.95: mistyped command arguments yield VALIDATION_ERROR and fieldErrors. This row defines diagnostics rather than every accepted numeric token spelling. - `src/cli/options.ts:16-21`: existing raw decimal-integer helper rejects empty tokens before conversion. - `src/cliObservationOptions.ts:8-14,35`: numeric duration declaration and page-inspection option. - `src/cli/browserCommands.ts:100-129`: supported command registration and forwarding of the parsed duration. - `src/domain/browserObservation.ts:208-212`: numeric domain input permits explicit zero. - `node_modules/incur/src/Parser.ts:322-330`: locked parser converts numeric option strings with Number before validation. ### Typed outcome or recovery expected An empty duration should fail before provider admission with a diagnostic naming observationMs and no success Evidence. A restoration check must retain omitted-default, explicit-zero, and ordinary positive-duration behavior. No repaired build was tested. ### Analysis or suspected cause Source-derived: the CLI duration uses a numeric schema, so Incur converts empty text into numeric zero before the nonnegative-integer checks run. The domain receives a valid zero and cannot distinguish it from an explicit caller choice. The existing raw-token integer helper validates before conversion, but these duration declarations do not use it. The common duration helper also feeds analyze-web-bundle and inspect-electron-page. discover-webmcp-tools and observe-javascript-runtime have separate inline min-0 numeric declarations. Those paths were inspected statically, not reproduced end to end; they are possible affected scope rather than additional observed failures. ### Additional context Related [#7376](https://github.com/morluto/rea/issues/7376) concerns byte-window arguments, and [#7093](https://github.com/morluto/rea/issues/7093) / [PR #7092](https://github.com/morluto/rea/pull/7092) concern listener flags. Closed [#7425](https://github.com/morluto/rea/issues/7425) / merged [PR #7505](https://github.com/morluto/rea/pull/7505) and closed [#7430](https://github.com/morluto/rea/issues/7430) / merged [PR #7493](https://github.com/morluto/rea/pull/7493) adopt strict schemas for outline depth and Android selectors respectively. Their fixes do not change these duration options. Those reports share a conversion mechanism, but do not describe a broken REA-wide helper whose repair closes every option. The raw-token helper already works; each affected registration must opt into the appropriate contract. This report closes when an empty observation duration is rejected while omission and explicit zero retain their meanings. Closed [#6290](https://github.com/morluto/rea/issues/6290) concerns a duration maximum, and closed [#3929](https://github.com/morluto/rea/issues/3929) / [#4392](https://github.com/morluto/rea/issues/4392) concern timer overflow, separate acceptance criteria. Open/closed issues, PRs, and comments were searched; no matching empty-duration report or general parser fix was found. The first affected release is unknown. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Empty option text returns successful Evidence with a zero observation window, silently overriding the caller's unset duration; the observed effect is bounded and no severe downstream incident was observed. - Workaround status: available - Workaround basis: Omitting the flag returned the documented 500 ms window, and explicit decimal zero selected a valid zero window on the same fixture.

bsakaguchi closed this issue

2026-10-31 00:51:16 +00:00

bsakaguchi referenced this issue

2026-10-31 01:14:08 +00:00

[PR #4759] [MERGED] fix(javascript): bind response endpoints only from writes that reach the read #6392