chore(cli): import Browse V3 baseline - #2833
Open
shrey150 wants to merge 5 commits into
Open
Conversation
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
This was referenced Aug 27, 2026
shrey150
force-pushed
the
agent/browse-v4-3-cli-baseline
branch
from
August 28, 2026 02:52
88c6350 to
1be9c24
Compare
This was referenced Aug 28, 2026
shrey150
force-pushed
the
agent/browse-v4-3-cli-baseline
branch
from
August 28, 2026 23:18
1be9c24 to
d0c0435
Compare
shrey150
changed the base branch from
agent/browse-v4-2-network-capture
to
main
August 28, 2026 23:19
This was referenced Aug 28, 2026
…-baseline # Conflicts: # pnpm-lock.yaml
shrey150
marked this pull request as ready for review
September 1, 2026 17:13
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
29 issues found across 193 files
Confidence score: 2/5
- Unsafe user-controlled values need priority:
packages/cli/src/commands/cloud/extensions/delete.tscan redirect deletion requests when IDs contain URL syntax, whilepackages/cli/src/lib/telemetry.tscan emit secret-like unknown commands. Encode resource IDs and restrict or redact telemetry fields before sending them. - Daemon state handling has several concrete failure modes across
paths.ts,doctor.ts,session-manager.ts, andlocal-cdp-discovery.ts: invalid environment values can redirect files to the working directory, failed initialization may be reported as healthy, stale attempts can poison retries, and an unrelated service may be accepted as Chrome. Validate paths and CDP identity, and key initialization state to the forwarded environment signature. packages/cli/src/lib/skills/install.tsbuffers catalog responses witharrayBuffer(), allowing a malformed or oversized file to consume excessive memory. Stream downloads with per-file and total byte limits.- CLI boundary validation still permits user-visible failures in
packages/cli/src/commands/cloud/sessions/create.ts,packages/cli/src/commands/screenshot.ts, andpackages/cli/src/lib/driver/command-cli.ts: invalid--persistcombinations fail request validation, relative paths can resolve against the daemon directory, and generic errors bypass typed sanitization. Reject invalid combinations early, resolve paths in the CLI, and use the repository’s typed errors.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/lib/driver/daemon/paths.ts">
<violation number="1" location="packages/cli/src/lib/driver/daemon/paths.ts:11">
P2: When `BROWSE_DAEMON_DIR` is exported as an empty string, `runtimeDir()` returns an invalid directory and daemon commands can resolve PID, lock, and socket files relative to the current directory. Treat an empty value as unset or reject it before constructing daemon paths.</violation>
</file>
<file name="packages/cli/src/lib/driver/doctor.ts">
<violation number="1" location="packages/cli/src/lib/driver/doctor.ts:186">
P2: When a daemon is alive but browser initialization has failed, doctor treats it as healthy and skips the target prerequisite check. Run the prerequisite check when `status` is absent or `!status.initialized` so remote credentials and auto-connect readiness are still diagnosed.</violation>
<violation number="2" location="packages/cli/src/lib/driver/doctor.ts:198">
P2: When the lock file exists but does not contain a readable PID, doctor falls through to `no active daemon` with an `ok` status. Handle `inspection.lock === "unreadable"` as a warning with the same cleanup guidance as a stale lock.</violation>
</file>
<file name="packages/cli/src/lib/driver/command-cli.ts">
<violation number="1" location="packages/cli/src/lib/driver/command-cli.ts:147">
P2: Custom agent: **Exception and error message sanitization**
These new user-facing parsing failures use generic `Error` instead of the repository’s typed error classes, violating the requirement to never raise generic `new Error()`. Throw the appropriate typed CLI/driver error and preserve a sanitized message.</violation>
</file>
<file name="packages/cli/src/lib/driver/commands/snapshot-format.ts">
<violation number="1" location="packages/cli/src/lib/driver/commands/snapshot-format.ts:59">
P2: With `/submit/g`, `regex.test` carries `lastIndex` between lines, so repeated matches are skipped. Reset `regex.lastIndex` before each test.</violation>
</file>
<file name="packages/cli/src/lib/driver/remote-binding.ts">
<violation number="1" location="packages/cli/src/lib/driver/remote-binding.ts:23">
P2: When `remote.js` exists but a dependency imported by `remote.ts` is missing, Node still throws `ERR_MODULE_NOT_FOUND`, so this condition loads `remote.disabled.js` instead. Restrict the fallback to an error whose URL is the expected `remote.js` path; otherwise rethrow the real initialization error.</violation>
</file>
<file name="packages/cli/src/lib/run-telemetry.ts">
<violation number="1" location="packages/cli/src/lib/run-telemetry.ts:23">
P2: When a multi-request command receives an HTTP response and a later request fails before receiving one, this merge retains the earlier `httpStatus`. Completion telemetry then reports a status such as `200` with `request_had_http_response: false`; clear `httpStatus` when recording a no-response completion.</violation>
</file>
<file name="packages/cli/src/commands/cloud/sessions/create.ts">
<violation number="1" location="packages/cli/src/commands/cloud/sessions/create.ts:81">
P2: When users pass `--persist` without `--context-id`, this builds `browserSettings.context` without the required `id`, so the session-create request fails validation. Reject that combination with an actionable error before sending the request.</violation>
</file>
<file name="packages/cli/src/lib/skills/catalog.ts">
<violation number="1" location="packages/cli/src/lib/skills/catalog.ts:164">
P2: When the catalog endpoint hangs before returning headers or a body, `browse skills list` and `find` never finish because this fetch has no timeout. Pass the existing `BROWSE_SKILLS_FETCH_TIMEOUT_MS` timeout signal, using its 10-second default, so these commands fail promptly.</violation>
</file>
<file name="packages/cli/README.md">
<violation number="1" location="packages/cli/README.md:26">
P2: When users follow this example, `browse skills add apartments.com` fails before catalog lookup because skill IDs must use the `<domain>/<task>` form. Show `skills find apartments.com` and install a returned skill ID instead.</violation>
<violation number="2" location="packages/cli/README.md:193">
P2: `--json` is not a driver-wide option: commands such as `browse open ... --json` reject it as unknown. Remove this row from the driver-flags table and document that driver commands already emit JSON by default.</violation>
</file>
<file name="packages/cli/src/commands/cloud/extensions/delete.ts">
<violation number="1" location="packages/cli/src/commands/cloud/extensions/delete.ts:19">
P2: When an ID contains URL syntax such as `../` or `?`, this interpolation lets `new URL` change the request path or query, so deletion can target a different endpoint instead of the extension resource. Encode the ID as a path segment before constructing the request URL.</violation>
</file>
<file name="packages/cli/src/lib/telemetry.ts">
<violation number="1" location="packages/cli/src/lib/telemetry.ts:147">
P2: When the unknown command itself is a safe-token secret such as `sk_live_...`, `suggestCommand` returns it as `attempted`, while `resolveCommandPath` only replaces colons, so this event sends the secret. Restrict attempted values to known command or alias paths, or omit or hash unknown attempts before emitting telemetry.</violation>
</file>
<file name="packages/cli/src/lib/driver/session-manager.ts">
<violation number="1" location="packages/cli/src/lib/driver/session-manager.ts:124">
P2: When a forwarded key changes while an earlier initialization is still pending, that attempt overwrites this reset and caches its old failure for 5 seconds. Version initialization attempts by forwarded-env signature and discard stale failure state so the new key retries immediately.</violation>
</file>
<file name="packages/cli/src/lib/driver/local-cdp-discovery.ts">
<violation number="1" location="packages/cli/src/lib/driver/local-cdp-discovery.ts:216">
P2: When a stale `DevToolsActivePort` file points to a port now occupied by another service or Chrome instance, `discoverLocalCdp()` accepts its synthesized URL after only a TCP connect and returns it as the first candidate. Validate the active-port endpoint with `/json/version` or a WebSocket handshake before adding it, then continue to fallback candidates when validation fails.</violation>
</file>
<file name="packages/cli/src/lib/skills/install.ts">
<violation number="1" location="packages/cli/src/lib/skills/install.ts:486">
P2: When a catalog URL returns a large file, `fetchSkillFile` buffers the entire response with `arrayBuffer()` before writing it. Enforce per-file and total byte limits while streaming the response so a malformed or compromised catalog cannot exhaust the CLI process or disk.</violation>
</file>
<file name="packages/cli/src/commands/screenshot.ts">
<violation number="1" location="packages/cli/src/commands/screenshot.ts:75">
P2: When an existing driver daemon was started from another directory, a relative `--path` writes the screenshot there instead of the caller's current directory. Resolve explicit paths in the CLI before sending them to the daemon.</violation>
</file>
<file name="packages/cli/src/lib/functions/shared.ts">
<violation number="1" location="packages/cli/src/lib/functions/shared.ts:127">
P2: When `browse functions publish` receives an entrypoint outside the current working tree or excluded by publish ignore rules, `resolveEntrypoint` accepts it but the uploaded archive omits it. Reject entrypoints that cannot be included or add the validated entrypoint to the archive.</violation>
</file>
<file name="packages/cli/src/lib/driver/network-capture.ts">
<violation number="1" location="packages/cli/src/lib/driver/network-capture.ts:58">
P2: When two `network.on` commands overlap, both pass this check before either marks the capture enabled, registering duplicate listeners and producing duplicate or mis-associated captures. Serialize enable/disable operations or reserve an in-progress enable before awaiting setup.</violation>
<violation number="2" location="packages/cli/src/lib/driver/network-capture.ts:65">
P2: After `network off` followed by `network on`, the counter restarts even though the capture directory is retained, so matching requests overwrite earlier captures. Preserve a counter above existing directories and reset it only after a confirmed clear.</violation>
</file>
<file name="packages/cli/src/lib/functions/init.ts">
<violation number="1" location="packages/cli/src/lib/functions/init.ts:94">
P2: `initFunctionsProject` creates the credentials file with default permissions, so a typical umask leaves the user’s API key world-readable despite `.gitignore`. Create `.env` with mode `0o600`.</violation>
</file>
<file name="packages/cli/src/lib/cloud/reduce-logs.ts">
<violation number="1" location="packages/cli/src/lib/cloud/reduce-logs.ts:88">
P2: When page code uses `%s` or `%o` formatting for a console error, this filter drops the complete error instead of returning it. Format the arguments before filtering, or at minimum stop excluding messages that begin with a format token.</violation>
</file>
<file name="packages/cli/src/lib/cloud/contexts-store.ts">
<violation number="1" location="packages/cli/src/lib/cloud/contexts-store.ts:153">
P2: When a stale or pre-created PID temp file already exists, `writeFile` does not apply `mode`, so `rename` can publish `contexts.json` with permissive permissions and expose stored context IDs. Create a unique exclusive temp file or chmod the temp file after writing before renaming.</violation>
</file>
<file name="packages/cli/src/lib/driver/cdp-tail.ts">
<violation number="1" location="packages/cli/src/lib/driver/cdp-tail.ts:188">
P2: When the command output is piped to a consumer that exits early, such as `browse cdp ... | head`, `stdout.write` reports `EPIPE` asynchronously. The `try/catch` misses it and Node terminates with an unhandled stream error; handle the write callback or an `stdout` error listener.</violation>
</file>
<file name="packages/cli/src/lib/cloud/api.ts">
<violation number="1" location="packages/cli/src/lib/cloud/api.ts:324">
P2: When a Browserbase session create or retrieve fails with an `APIError`, this line exposes the raw SDK message through the CLI error output. Use a generic sanitized message for `command === "sessions"` instead of propagating upstream request or session details.
(Based on your team's feedback about sanitized Browserbase session errors.) .</violation>
</file>
<file name="packages/cli/src/lib/identity.ts">
<violation number="1" location="packages/cli/src/lib/identity.ts:105">
P2: When the marker contains an embedded CR/LF or another invalid header byte, this returns it unchanged and cloud requests fail during header construction. Validate marker contents before caching them or expose a header-safe value through `peekInstallId`.</violation>
<violation number="2" location="packages/cli/src/lib/identity.ts:138">
P2: When multiple CLI processes encounter a stale empty marker, each process writes and returns a different install id, so some requests use an id different from the persisted winner. Use an interprocess ownership/locking protocol and re-read the marker before returning.</violation>
<violation number="3" location="packages/cli/src/lib/identity.ts:169">
P2: When `XDG_CONFIG_HOME` is exported as an empty string, `??` selects it and writes the install marker under the current working directory instead of the user config directory. Use a truthy fallback in both `resolveConfigDir` and `legacyInstallIdPaths`.</violation>
</file>
<file name="packages/cli/src/lib/driver/remote.disabled.ts">
<violation number="1" location="packages/cli/src/lib/driver/remote.disabled.ts:19">
P2: Custom agent: **Exception and error message sanitization**
When a user selects remote mode in a local-only build, both stub paths throw generic `Error` instances. Use a typed driver error such as `DriverError` with a stable code instead.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Import
packages/cli/**exactly from the publishedbrowse@0.9.6V3 release, without changing its source or runtime behavior.This is intentionally a provenance checkpoint, not a line-by-line feature review. The imported source is kept runnable by a root, version-scoped pnpm override that resolves its unchanged Stagehand dependency to
3.7.1. #2834 removes that override and starts the V4 migration.Exact-source provenance
browse@0.9.6(548c56407431db27823a212f53475443c7e8358d)1d49a95c0c230c346f8d50647e10303d6310fcd2b4048badce921cf54f199f96033d9a014ef977ecHEAD:packages/clitree:b4048badce921cf54f199f96033d9a014ef977ecThe tag's ignored README whitespace is retained too; formatting the import would invalidate the tree proof.
Verification
d24ad59e99is based on currentmainand is mergeable.HEAD:packages/clistill exactly equals the published V3 tree hash above after themainmerge.Stack (#2872)
--return-xpath; supported V3 parity/release checkpointReview and landing boundary
Review this PR by verifying the tree hashes, dependency pin, package manifest, and changeset—not by treating the imported V3 source as newly authored code. This head deliberately imports V3 code into the V4 repository and is not independently publishable. It lands only as the base of the complete stack.
The framework network-event schema proposal in #2832 is intentionally outside this landing stack.