Report browser-control CDP commands in the control telemetry category

[archandatta]

Stacked on #322 — the base is that branch, so this diff is only the new event.

Summary

• the CDP proxy emits cdp_command under control for the methods that drive the browser: input gestures, navigation, dialog handling, file selection and screenshots
• configuration and the DOM/Runtime traffic a client library issues on the caller's behalf are not reported; Runtime.evaluate and Runtime.callFunctionOn are excluded deliberately, since every locator resolution is one and including them would bury the gestures
• mouseMoved, keyUp and char are dropped, so a click reads as a press and a release rather than three frames
• payload is shape only: method, session_id, event_type, x, y, button, text_length, and key for keys that command the page (Enter, Tab, arrows, modifiers, F-keys, by allowlist). Never typed characters, never the URL — control is captured by default

Why

api_call covers browser control that arrives through the kernel-images API, but a client driving the browser over the CDP proxy — Playwright, Puppeteer, an SDK — produced nothing at all. A session whose agent works over CDP showed cdp_connect, cdp_disconnect, and nothing in between.

Hot path

Every client frame hits the transform, so classification scans for the frame's method name and does a map lookup; only an allowlisted method is unmarshalled, and a large Runtime.callFunctionOn costs the scan rather than a parse. The scan checks every "method" the frame contains and the parsed top-level method is what decides, so a nested "method" inside params can neither spoof a control command nor hide one.

Existing behavior is unchanged: message relaying, cdp_connect/cdp_disconnect and the message_count they report are untouched, and nothing is emitted for the upstream direction.

Testing

• go vet ./... clean; go test -race green across the non-e2e packages
• new: a classification table (control vs configuration vs library bookkeeping vs command results vs upstream events vs nested-method frames vs malformed), a payload-leakage test asserting text, key, URL and file-path frames leak none of those values, and a proxy-level test asserting two client gestures produce two events while the echoed upstream frames produce none
• TestUpstreamManagerDetectsChromiumAndRestart flakes about 1 in 4 runs on this package — reproduced on the unmodified base, it launches real Chromium. The new tests passed 10/10.
• e2e not run — needs Docker

---

Note

Medium Risk
Runs on every client CDP frame in the proxy hot path and emits control telemetry that is on by default; redaction mistakes could leak credentials, though tests explicitly guard typed text and URLs.

Overview
Adds cdp_command telemetry (control) for client→browser CDP traffic through the DevTools WebSocket proxy, so Playwright/Puppeteer sessions are visible between connect and disconnect.

The proxy’s message transform calls publishCdpCommand only on client-to-upstream text frames. New cdpcommand logic allowlists browser-control methods (input, navigation, screenshots, etc.), skips noisy phases (mouseMoved, keyUp, char) and library/runtime bookkeeping, and uses a byte scan before JSON parse on the hot path. Event payloads are shape-only (method, coordinates, text_length, allowlisted named_key)—never typed text, URLs, or file paths.

OpenAPI and generated oapi / category_gen register BrowserCdpCommandEvent and map cdp_command to control. Tests cover classification edge cases, secret leakage, and proxy-level emission counts.

^{Reviewed by Cursor Bugbot for commit 02ad34d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Sayan-]

Reviewed the stacked diff only (02ad34d against the base branch). Classification and the leakage test look right, and the nested-method handling is sound: a spoofed "method" inside params buys a parse and nothing more, since the parsed top-level method is what decides. I also checked that the in-VM CDP callers (lib/cdpclient, the start-URL dispatch, the monitor) all dial the upstream directly rather than through this proxy, so platform-induced navigation won't surface as agent activity. That was the thing most likely to undo the point of the change, and it holds.

Three things, none blocking.

1. Classification runs on every client frame whether or not telemetry is on

publishCdpCommand is gated only on publish == nil, and publish is always wired (cmd/api/main.go:325). The "no session, or control disabled" decision happens inside TelemetrySession.Publish, after the scan, the unmarshal and the marshal. So a session with telemetry off still pays, per frame:

dropped mouseMoved    4.4 us   440 B   12 allocs
kept click            6.2 us   728 B   19 allocs
scan only, 256 KB     3.5 us     1 B    0 allocs

(shared VM, so the allocs and the ratios are the meaningful part, not the absolute times)

mouseMoved is the highest-volume method, it is dropped after the unmarshal rather than before, and humanized cursor paths send it in bulk, so this is per-gesture garbage in the relay path for a feature that may be switched off. The HTTP middleware keeps an atomic toggle for exactly this reason. Mirroring it, set from the same reconcile point, makes the off case a single atomic load.

2. Control volume now scales with input events, against a 1024-entry ring

RingCapacity is 1024 (cmd/api/main.go:112), and the storage writer drains one envelope at a time, logging storage writer: dropped events when it falls behind. Typing is one event per character and a click is two, so a form-filling session goes from tens of control events to thousands. If the S2 producer backs up, those records are gone and the only trace is a log line inside the VM. Worth deciding whether the ring is still sized for the new rate, and whether dropped counts should be visible to a reader rather than only in the log.

3. Shape gaps that cost more than they save

• Page.navigate reports no URL at all. URLs otherwise live in page and network, both opt-in, so on the default category set the stream says the agent navigated and nothing else. Origin or host only would be a large readability win for a small disclosure, and would make the event answer the question it exists to answer.
• mouseWheel deltas and Input.synthesizeScrollGesture distances aren't captured, so a scroll reports a position but not direction or amount.
• Input.dispatchTouchEvent carries its coordinates inside touchPoints[], so x and y are always absent for touch dispatches. Fine as a limitation, but probably worth saying so in the field descriptions.

[rgarcia]

reviewed — the goal is right, but i think this needs architectural and coverage changes before merging. telemetry currently runs synchronously on the critical CDP forwarding path, performs work when disabled, covers a narrow initial set of control methods, and models unrelated commands through one lossy payload type.

1. keep telemetry off the CDP forwarding path

server/lib/devtoolsproxy/proxy.go:317-327 calls publishCdpCommand from the message transform before Chromium receives the command.

The pump currently executes:

msg = transform("->", mt, msg)
upstream.Write(ctx, mt, msg)

See server/lib/wsproxy/wsproxy.go:61-76.

This means telemetry parsing, JSON marshaling, publisher locking, or a blocked publisher delays browser control. A publisher panic can terminate the process, and telemetry can report commands whose subsequent upstream write fails.

The original CDP bytes are returned unchanged, so i did not find byte corruption. The problem is synchronous work and failure coupling.

Please change this to:

1. forward the untouched message to Chromium
2. observe only after upstream.Write succeeds
3. check an atomic “telemetry active and control enabled” predicate
4. submit the owned message buffer to a size-bounded, nonblocking queue
5. parse, sanitize, and publish from an isolated worker

Queue saturation or oversized commands should drop telemetry and increment observable counters. They must never delay CDP. Capture the event timestamp immediately after the successful write so queue latency does not distort event time.

2. gate all inspection when telemetry is disabled

server/cmd/api/main.go:324-326 always wires a non-nil TelemetrySession.Publish callback into the proxy. Consequently, the nil check at server/lib/devtoolsproxy/cdpcommand.go:171-177 is not an effective production gate.

For matching commands, category filtering currently happens only after:

• scanning the frame
• unmarshaling it
• constructing the event
• marshaling the sanitized payload
• acquiring the telemetry session mutex

Audit benchmarks measured:

State	Kept click cost
No active telemetry session	4.26 µs, 18 allocations
control disabled	5.72 µs, 18 allocations
control enabled	10.58 µs, 27 allocations
1 MiB Input.insertText	~12 ms, ~1.05 MiB allocated

Please maintain an atomic predicate reflecting both active-session and control state. When false, the post-write observer should return before parsing, retaining, or queuing the frame. Keep the category check inside Publish as the final race-safe guard.

3. expand and configure the control-method inventory

server/lib/devtoolsproxy/cdpcommand.go:19-26 currently allowlists 13 methods. Reviewing the canonical protocol at ChromeDevTools/devtools-protocol@2d019e73 identifies the following 38-method candidate set for agent-driven browser control:

Domain	Methods
Input	dispatchDragEvent, dispatchKeyEvent, insertText, imeSetComposition, dispatchMouseEvent, dispatchTouchEvent, cancelDragging, emulateTouchFromMouseEvent, synthesizePinchGesture, synthesizeScrollGesture, synthesizeTapGesture
DOM	setFileInputFiles, focus, scrollIntoViewIfNeeded
Page	bringToFront, captureScreenshot, captureSnapshot, handleJavaScriptDialog, navigate, navigateToHistoryEntry, printToPDF, reload, startScreencast, stopScreencast, stopLoading, close, setWebLifecycleState
Target	activateTarget, closeTarget, createTarget, createBrowserContext, disposeBrowserContext, openDevTools
Browser	cancelDownload, close, setWindowBounds, setContentsSize
Autofill	trigger

Browser.executeBrowserCommand should remain excluded: its current command IDs operate Chrome-specific Tab Search/Glic UI. DeviceAccess, Extensions, FedCM, PWA, and Cast commands are also outside this PR’s reasonable agent-control boundary.

The telemetry configuration should support per-method exclusions:

control:
  enabled: true
  cdp:
    excluded_methods:
      - Page.captureScreenshot
      - Input.dispatchMouseEvent

Omitting excluded_methods should capture every supported method. Exclusions must affect telemetry only; the raw command must still reach Chromium.

4. preserve command cardinality; sanitize only arguments

server/lib/devtoolsproxy/cdpcommand.go:28-33 unconditionally drops the following params.type values:

• Input.dispatchMouseEvent with mouseMoved
• Input.dispatchKeyEvent with keyUp
• Input.dispatchKeyEvent with char

These are command subtypes, not redundant metadata:

• mouseMoved can contain a drag path when buttons != 0
• plain movement can represent deliberate hover behavior
• keyUp releases held keys and modifiers
• char can be the command that inserts text

Please preserve one telemetry event per successfully forwarded, configured command. Do not sample, coalesce, reorder, or hard-code subtype drops. Payload sanitation is necessary; transforming the command stream is not.

Method-level volume control should come from the control.cdp.excluded_methods telemetry configuration described above. Excluding a method should suppress only its telemetry event; every raw CDP command must still be forwarded unchanged.

The only acceptable event losses should be explicit method exclusion, bounded-queue saturation, or the telemetry-specific size limit. Those losses should be counted.

5. replace the shared god types with per-command sanitizers

server/lib/devtoolsproxy/cdpcommand.go:60-77 combines unrelated commands into cdpCommand/cdpParams. It silently discards most useful canonical arguments and permits nonsensical combinations such as Page.navigate with mouse coordinates.

The repository already has similar raw envelopes at:

• server/lib/cdpmonitor/types.go:94-104
• server/lib/cdpclient/cdpclient.go:16-26

Please share an envelope shaped like:

type cdpCommand struct {
    Method    string          `json:"method"`
    SessionID string          `json:"sessionId"`
    Params    json.RawMessage `json:"params"`
}

Dispatch Params into a command-specific canonical input type, then produce a separate sanitized output type. Each input type should link to the pinned canonical PDL definition. Unknown fields should remain privacy-safe by default, with automated protocol-drift coverage.

Useful non-sensitive arguments currently lost include:

• input modifiers, pressed-button masks, click count, wheel deltas, pointer type, pressure/tilt, touch aggregates, scroll distance, pinch scale, and tap count
• dialog accept/dismiss, screenshot dimensions/options, PDF layout, screencast options, and page lifecycle state
• navigation transition type, referrer presence/policy, reload cache behavior, and script-presence/length
• opaque frame, node, backend-node, target, browser-context, loader, window, and download identifiers
• file/drag item counts and constrained MIME categories
• autofill field/frame IDs and card versus address mode

Selectors generally are not present in these direct-control commands: Playwright resolves them through excluded DOM/Runtime bookkeeping before sending coordinates or opaque IDs.

Sensitive values must remain redacted:

• typed and composition text
• URLs, referrers, scripts, templates, and file paths
• drag contents
• autofill card and address values

Derived values such as length, count, presence, URL scheme/class, or allowlisted enum are appropriate.

Canonical CDP number maps to float64, not the current float32. Use float64 in Go and type: number, format: double in OpenAPI.

6. make the OpenAPI schema method-specific

server/openapi.yaml:2809-2844 repeats the god-type problem as one flat BrowserCdpCommandEventData. It cannot express canonical method arguments or prevent unrelated field combinations.

Please make the event data a method-discriminated union of sanitized command payloads. Each variant should:

• use a method-specific const
• contain only that command’s approved fields
• reject unknown output properties
• link to the pinned canonical input definition

Define a shared BrowserCdpCommandMethod enum for both event data and excluded_methods.

Also:

• add data to BrowserCdpCommandEvent.required; a command event without command data is not meaningful
• remove the description promising that mouseMoved, keyUp, and char are dropped
• document the one-event-per-configured-command invariant and bounded-loss exceptions
• regenerate server/lib/oapi/oapi.go and server/lib/events/category_gen.go from the source schema

7. update the tests around the actual safety invariants

server/lib/devtoolsproxy/cdpcommand_test.go:69-79 currently requires the rejected subtype-dropping behavior. Replace those cases with positive assertions for every dispatchMouseEvent and dispatchKeyEvent subtype.

The current proxy test proves that echoed upstream frames do not create duplicates, but it does not establish that telemetry cannot interfere with CDP.

Please add coverage for:

1. inactive telemetry and disabled control perform no parsing or queue work
2. blocked or panicking telemetry publication cannot delay or terminate CDP forwarding
3. failed upstream writes produce no event; successful configured commands produce exactly one
4. every sanitizer retains approved fields and excludes unique sentinels from every sensitive field
5. malformed, binary, invalid UTF-8, escaped-key, duplicate-key, oversized, and overloaded traffic preserves CDP bytes/order while telemetry may drop observably

The handwritten scanner at server/lib/devtoolsproxy/cdpcommand.go:143-168 can miss valid escaped method names such as "Input.\u0064ispatchMouseEvent". Once parsing is gated and asynchronous, use the real envelope decoder rather than a partial JSON scanner. Add fuzzing and disabled/enabled allocation benchmarks to prevent regressions.

Focused tests pass on the current commit, but they validate several behaviors described above that should change.