PR #84208: fix(browser-cli): reject non-numeric coordinates in click-coords and resize (#83876)

• Repository: openclaw/openclaw
• Author: hclsys
• State: open | merged: False
• Link: https://github.com/openclaw/openclaw/pull/84208

Description (problem / solution / changelog)

Fixes #83876.

browser click-coords <x> <y> (extensions/browser/src/cli/browser-cli-actions-input/register.element.ts:88-89) and browser resize <width> <height> (register.navigation.ts:50) parse their positional numeric arguments with bare Number(). A non-numeric input — typo, an unintended shell expansion, an empty string — yields NaN, which JSON.stringify serializes as null. The CLI happily forwards { kind: "clickCoords", x: null, y: 200 } to the /act endpoint instead of failing fast with a user-visible error. The surrounding option parsers in the same files already use Number.isFinite for exactly this reason (delayMs at line 99, timeoutMs at line 175, the geolocation triple in browser-cli-state.ts:187-189); the positional args were the missing case.

Changes

• extensions/browser/src/cli/browser-cli-actions-input/register.element.ts: guard the click-coords action handler with if (!Number.isFinite(x) || !Number.isFinite(y)) and emit defaultRuntime.error(danger("invalid coordinates …")) + defaultRuntime.exit(1) before runElementAction is called. Mirrors the existing defaultRuntime.error(danger(...)) + exit(1) pattern in register.navigation.ts's navigate error branch.
• extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts: identical guard on the resize action handler against width and height, before runBrowserResizeWithOutput.

Diff stat: 2 files, +14 / -0. No behavior change for any input that was already a finite number.

Real behavior proof

• Behavior or issue addressed: Sanitized issue evidence — runElementAction posts body to /act via callBrowserAct, and runBrowserResizeWithOutput posts { width, height, … } via the resize helper. JSON-stringifying NaN produces null, so the server can't distinguish "missing field" from "user typo'd the argument." The guard runs at the CLI boundary and never sends the malformed body.

• Real environment tested: Local Node 22.x. Probe at /tmp/probe_83876.mjs does both halves of the proof. (a) Parses the patched register.element.ts and register.navigation.ts and verifies (i) each action handler now contains Number.isFinite on its positional args + defaultRuntime.error(danger(...)) + defaultRuntime.exit(1), and (ii) the guard precedes the request call (runElementAction / runBrowserResizeWithOutput). (b) Replays both shapes in pure JS — buggy handler with "abc" "200" returns a payload containing "x":null (confirming the exact #83876 symptom: NaN → JSON null → silent transmission to the server); patched handler with the same input returns { sent: false, exitCode: 1 } (rejected at the CLI boundary, request never issued); patched handler with valid "100" "200" returns "x":100,"y":200 unchanged (no regression).

• Exact steps or command run after this patch: node /tmp/probe_83876.mjs

• Evidence after fix:

PASS: click-coords action guards x/y with Number.isFinite and emits danger+exit(1)
PASS: click-coords guard runs BEFORE runElementAction (rejects before request)
PASS: resize action guards width/height with Number.isFinite and emits danger+exit(1)
PASS: resize guard runs BEFORE runBrowserResizeWithOutput (rejects before request)
PASS: replay (buggy): non-numeric x silently serializes to JSON null and is sent to /act — confirms #83876
PASS: replay (patched): non-numeric x is rejected before /act with exit(1) — request never sent
PASS: replay (patched, valid input): finite numerics flow through unchanged — no regression

ALL CASES PASS

• Observed result after fix: browser click-coords abc 200 now writes invalid coordinates "abc 200": x and y must be finite numbers to stderr and exits 1 instead of POSTing { x: null, y: 200 } to /act. browser resize abc 100 likewise rejects on the CLI side. Any finite numeric input — including 0, negative, decimals — is unchanged.

• What was not tested: No live browser session against a Chrome/CDP target — that's outside the scope of an input-validation fix. The probe replays the exact NaN-to-null serialization path called out in the issue's repro section.

Audit (per CLAUDE rules — all 5 steps)

• Existing-helper check: No helper needed — Number.isFinite is the codebase's established predicate for this exact concern (delayMs at register.element.ts:99, timeoutMs at :175, three coords in browser-cli-state.ts:187-189, the resolved-timeout pair in browser-cli-shared.ts:40,46). I'm reusing the same predicate, not inventing one. PASS
• Shared-helper caller check: No shared helper is being modified. The two action handlers are leaf endpoints in their respective command registrations; the new validation is local to each action and cannot affect other call sites. PASS
• Broader-fix rival scan: gh pr list --search '83876 in:title,body' and gh pr list --search 'click-coords coordinates in:title,body' return no open PRs. Issue timeline shows zero cross-references. PASS
• Recent-merge audit: git log --oneline -5 -- extensions/browser/src/cli/browser-cli-actions-input/ shows no recent commits touching these files. PASS
• Prototype-pollution scan: N/A — Number.isFinite on locally-bound numeric variables, no object property access.

Changed files

• extensions/browser/src/cli/browser-cli-actions-input/register.element.ts (modified, +7/-0)
• extensions/browser/src/cli/browser-cli-actions-input/register.navigation.ts (modified, +7/-0)