release: prepare npm v0.5.0 — npm-only, CLI + skill first - #17
Merged
Conversation
Make BetterWright npm-only and CLI-first: - Remove the Python package, PyPI publish workflow, and all dual-language machinery (worker sync, policy/prompt parity, wheel checks). The policy parity vectors live on as a JS conformance suite. - Add `betterwright skill` (+ `--claude` for SKILL.md frontmatter): a paste-ready agent skill so any agent with a shell tool — Claude Code, Codex, Hermes, custom frameworks — drives the browser through the CLI with no server or SDK. - Open network defaults: private network and loopback reachable by default; cloud-metadata floor stays. New --block-private-network/--block-loopback flags and BETTERWRIGHT_BLOCK_* env vars re-harden. - Ship the MCP stdio server in the npm package (optional SDK peer dep). - Restructure README and SETUP.md around CLI + skill as the primary integration path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughBetterWright 0.5.0 shifts integration toward the Node CLI, skill output, and MCP stdio server. It updates network defaults and transport enforcement, centralizes worker result handling, removes Python packaging and publishing, and revises documentation, CI, release checks, and package validation. ChangesNode runtime and MCP integration
Network enforcement and worker transport
JavaScript-first migration and release updates
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (5)
docs/agent-prompt.md (1)
29-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the
bw.prefix for consistency.For clarity and consistency with the surrounding text and the actual JS API, include the
bw.prefix beforegenerateAndFillCredential.💡 Proposed refactor
- trusted host-side fill (`bw.fillCredential` / `generateAndFillCredential`); + trusted host-side fill (`bw.fillCredential` / `bw.generateAndFillCredential`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/agent-prompt.md` at line 29, Update the API reference in the trusted host-side fill text to use the fully qualified `bw.generateAndFillCredential` name, matching the existing `bw.fillCredential` notation and the actual JavaScript API.README.md (1)
138-138: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
npx -yto prevent potential interactive hangs in MCP configurations.Using
npxwithout the-yflag in an MCP client configuration can cause the server handshake to hang indefinitely if the package is not found globally or ifnpxattempts to prompt for confirmation. Adding-yensures robust, non-interactive execution across all environments.
README.md#L138-L138: Update the Claude config example toclaude mcp add betterwright -- npx -y betterwright mcp.SETUP.md#L176-L176: Update the Claude config example toclaude mcp add betterwright -- npx -y betterwright mcp.SETUP.md#L186-L188: Update the JSON arguments array to"args": ["-y", "betterwright", "mcp"].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 138, Update the Claude MCP command in README.md lines 138-138 and SETUP.md lines 176-176 to invoke npx with the -y flag; also update the JSON arguments array in SETUP.md lines 186-188 to include "-y" before "betterwright" and "mcp".types/mcp-server.d.ts (1)
24-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType this parameter as
RunResult<unknown>.Accepting
unknownpermits calls such ascontentForResult(null), although the implementation immediately dereferences result fields.Proposed type correction
-import type { DownloadPolicy, HeadlessMode } from "./common.js"; +import type { DownloadPolicy, HeadlessMode, RunResult } from "./common.js"; -export function contentForResult(result: unknown): Promise<McpContentBlock[]>; +export function contentForResult( + result: RunResult<unknown>, +): Promise<McpContentBlock[]>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@types/mcp-server.d.ts` around lines 24 - 25, Update the contentForResult parameter type from unknown to RunResult<unknown> in the MCP server declarations, preserving its Promise<McpContentBlock[]> return type. Ensure callers must provide a valid run result rather than nullable or arbitrary values.scripts/check-package.mjs (1)
60-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep Python paths forbidden in the npm tarball.
Removing Python support should strengthen, not remove, this package invariant. Retaining
pythoncatches accidental publication of retired sources in future releases.- .filter((name) => /(^|\/)(node_modules|tests|artifacts|\.betterwright)(\/|$)/.test(name)); + .filter((name) => /(^|\/)(node_modules|tests|artifacts|python|\.betterwright)(\/|$)/.test(name));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-package.mjs` at line 60, Update the exclusion filter in the package-checking logic to retain python as a forbidden path alongside node_modules, tests, artifacts, and .betterwright. Preserve the existing path-boundary matching behavior so Python directories or files anywhere in the npm tarball are still detected.tests/fixtures/policy-vectors.json (1)
605-609: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSuppress the known test-vector secret finding narrowly.
Checkov flags this AWS-shaped test value as
CKV_SECRET_2. Add an exact fixture-level allowlist/suppression rather than weakening repository-wide secret scanning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/fixtures/policy-vectors.json` around lines 605 - 609, Add a narrowly scoped, fixture-level suppression for the AWS-shaped value in the “aws access key in query” test vector so Checkov ignores only this known test secret. Preserve repository-wide secret scanning and avoid broad patterns or global configuration changes.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/credentials.md`:
- Around line 70-95: Update the credential-storage link text in
docs/getting-started.md to describe the vault as a pluggable backend used by
trusted host-side code, replacing the stale “encrypted storage” wording while
preserving the existing link destination.
In `@docs/getting-started.md`:
- Around line 12-17: Update the getting-started instructions so the Node.js
installation warning appears before the npx betterwright doctor command. Tell
users to install Node.js first, then rerun setup followed by doctor; keep the
existing CloakBrowser recovery guidance unchanged.
In `@docs/network-policy.md`:
- Around line 60-66: Update the allowHosts description in the network policy
table to state that matching hosts are allowed unless blocked by the metadata
floor. Keep the existing exact/parent-domain matching and port-pinning behavior,
and preserve the documented evaluation order.
- Around line 24-28: Update the network policy image caption or alt text near
the private-network description to state that private networks and loopback are
allowed by default, removing the stale claim that the policy blocks private
addresses.
In `@examples/README.md`:
- Around line 3-8: Update the examples README command block to include the
required browser setup step by adding npx betterwright setup between npm install
betterwright and node examples/javascript/quickstart.mjs, keeping the
prerequisite text consistent.
In `@package.json`:
- Around line 91-98: Constrain the `@modelcontextprotocol/sdk` entry in
peerDependencies to the v1 range by changing its version requirement to >=1.13.0
<2; keep the existing optional peerDependenciesMeta configuration unchanged.
In `@src/guard-proxy.mjs`:
- Around line 22-24: Update transportUrl and its callers to stop deriving the
scheme from port 80 versus other ports. Use the existing host/port
transport-policy operation directly so HTTP on nonstandard ports remains HTTP
and scheme-sensitive policies receive the actual transport decision.
- Around line 78-93: Update the resolved-address validation loop in the proxy
connection flow to preserve the original hostname allowlist authorization when
calling guardUrl for each candidate. Ensure allowlisted hostnames remain
permitted even when resolving to private or loopback literals, while metadata
addresses continue to be rejected unconditionally; use the existing attribution
or decision context rather than weakening general address checks.
In `@src/mcp-server.mjs`:
- Around line 209-214: Centralize parsing of BETTERWRIGHT_BROWSER and use the
shared parser in both sites: update src/mcp-server.mjs lines 209-214 to validate
the environment value and pass the parsed result through BetterWright’s browser
option, and update src/doctor.mjs lines 117-122 to reuse that parser and report
unsupported values as not ready instead of treating them as Cloak.
In `@src/policy.mjs`:
- Around line 141-147: Canonicalize hostnames by removing a terminal DNS dot
before the metadata-floor classification used by the policy checks, so dotted
forms such as metadata.google.internal. remain blocked under the open defaults.
Update the relevant hostname classification logic near allowPrivateNetwork and
allowLoopback, and add a conformance vector covering the trailing-dot metadata
hostname.
In `@src/worker.mjs`:
- Around line 1839-1840: Make buildEnvelope’s pages resolution non-throwing by
safely handling failures from summarizeSessionPages(session), especially on the
execute failure path. Preserve the existing pages value when available and use a
safe fallback when summarization fails, ensuring the error envelope is still
returned to the client.
- Around line 105-118: The MAX_ACTIVE_SECRETS eviction in trackSecret must not
remove secrets while the associated browser page or session remains capable of
exposing them. Tie eviction to page/session disposal, or restart and invalidate
that browser state before removing the oldest secret; preserve redaction for all
still-live browser state.
---
Nitpick comments:
In `@docs/agent-prompt.md`:
- Line 29: Update the API reference in the trusted host-side fill text to use
the fully qualified `bw.generateAndFillCredential` name, matching the existing
`bw.fillCredential` notation and the actual JavaScript API.
In `@README.md`:
- Line 138: Update the Claude MCP command in README.md lines 138-138 and
SETUP.md lines 176-176 to invoke npx with the -y flag; also update the JSON
arguments array in SETUP.md lines 186-188 to include "-y" before "betterwright"
and "mcp".
In `@scripts/check-package.mjs`:
- Line 60: Update the exclusion filter in the package-checking logic to retain
python as a forbidden path alongside node_modules, tests, artifacts, and
.betterwright. Preserve the existing path-boundary matching behavior so Python
directories or files anywhere in the npm tarball are still detected.
In `@tests/fixtures/policy-vectors.json`:
- Around line 605-609: Add a narrowly scoped, fixture-level suppression for the
AWS-shaped value in the “aws access key in query” test vector so Checkov ignores
only this known test secret. Preserve repository-wide secret scanning and avoid
broad patterns or global configuration changes.
In `@types/mcp-server.d.ts`:
- Around line 24-25: Update the contentForResult parameter type from unknown to
RunResult<unknown> in the MCP server declarations, preserving its
Promise<McpContentBlock[]> return type. Ensure callers must provide a valid run
result rather than nullable or arbitrary values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 126a96ed-0a3e-4aeb-8ab8-53a2e282f1e3
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (72)
.github/workflows/ci.yml.github/workflows/publish-npm.yml.github/workflows/publish-pypi.ymlCHANGELOG.mdCONTRIBUTING.mdREADME.mdSETUP.mdbin/betterwright.mjsdocs/agent-prompt.mddocs/architecture.mddocs/attach-mode.mddocs/browser-api.mddocs/credentials.mddocs/getting-started.mddocs/javascript.mddocs/network-policy.mddocs/python.mdexamples/README.mdexamples/python/local_dev.pyexamples/python/login_with_vault.pyexamples/python/onepassword_attach.pyexamples/python/quickstart.pyexamples/python/signup_with_generated_password.pyexamples/python/solve_captcha.pypackage.jsonpython/LICENSEpython/README.mdpython/pyproject.tomlpython/src/betterwright/__init__.pypython/src/betterwright/__main__.pypython/src/betterwright/_display.pypython/src/betterwright/_home.pypython/src/betterwright/_worker/challenges.mjspython/src/betterwright/_worker/cloak.mjspython/src/betterwright/_worker/downloads.mjspython/src/betterwright/_worker/human.mjspython/src/betterwright/_worker/snapshot.mjspython/src/betterwright/_worker/worker.mjspython/src/betterwright/bridge.pypython/src/betterwright/chrome.pypython/src/betterwright/cli.pypython/src/betterwright/client.pypython/src/betterwright/integrations/__init__.pypython/src/betterwright/integrations/mcp_server.pypython/src/betterwright/policy.pypython/src/betterwright/prompt.pypython/src/betterwright/runtime.pypython/src/betterwright/vault.pypython/tests/test_artifacts_and_display.pypython/tests/test_browser_integration.pypython/tests/test_chrome.pypython/tests/test_credential_fill.pypython/tests/test_mcp_download_approval.pypython/tests/test_policy.pypython/tests/test_prompt.pypython/tests/test_vault.pyscripts/check-package.mjsscripts/check-versions.mjsscripts/sync-worker.mjssrc/client.mjssrc/doctor.mjssrc/guard-proxy.mjssrc/mcp-server.mjssrc/policy.mjssrc/prompt.mjssrc/worker.mjstests/fixtures/policy-vectors.jsontests/node/browser.test.mjstests/node/mcp-server.test.mjstests/node/policy-vectors.test.mjstests/node/policy.test.mjstypes/mcp-server.d.ts
💤 Files with no reviewable changes (39)
- scripts/sync-worker.mjs
- python/src/betterwright/integrations/init.py
- python/tests/test_mcp_download_approval.py
- python/tests/test_prompt.py
- examples/python/onepassword_attach.py
- examples/python/solve_captcha.py
- python/pyproject.toml
- .github/workflows/publish-pypi.yml
- python/src/betterwright/_home.py
- docs/python.md
- python/tests/test_browser_integration.py
- python/src/betterwright/runtime.py
- python/src/betterwright/_worker/downloads.mjs
- python/src/betterwright/_display.py
- python/src/betterwright/main.py
- python/src/betterwright/cli.py
- python/src/betterwright/init.py
- python/tests/test_policy.py
- python/src/betterwright/chrome.py
- python/src/betterwright/policy.py
- python/README.md
- python/src/betterwright/prompt.py
- python/src/betterwright/vault.py
- python/tests/test_vault.py
- examples/python/quickstart.py
- examples/python/login_with_vault.py
- python/tests/test_artifacts_and_display.py
- examples/python/local_dev.py
- python/LICENSE
- python/src/betterwright/client.py
- python/src/betterwright/_worker/challenges.mjs
- python/src/betterwright/_worker/human.mjs
- python/src/betterwright/_worker/snapshot.mjs
- python/tests/test_credential_fill.py
- python/tests/test_chrome.py
- python/src/betterwright/_worker/cloak.mjs
- examples/python/signup_with_generated_password.py
- python/src/betterwright/bridge.py
- python/src/betterwright/integrations/mcp_server.py
Comment on lines
+70
to
+95
| ## Providing a vault | ||
|
|
||
| Pass a `vault=` instance to `BetterWright(...)` to share one store, or | ||
| `vault=False` to disable the model-facing management helpers entirely. Trusted | ||
| host code can call `fetch_for_fill`, `reveal`, or `generate` directly, but must | ||
| not return those secret-bearing results to model-authored code — prefer | ||
| `bw.fill_credential(...)`, which keeps the secret inside the worker. | ||
| BetterWright has no built-in credential store. The `vault` option is a | ||
| pluggable backend: any object exposing | ||
| `handleRequest(action, payload, origin)` (and optionally `redact`) can back the | ||
| management helpers and the same `fillCredential` path, so a source such as a | ||
| 1Password CLI/SDK backend can be dropped in without changing the fill code. | ||
|
|
||
| ```js | ||
| new BetterWright({ | ||
| vault: { | ||
| async handleRequest(action, payload, origin) { /* list|save|update|remove|fill|generate */ }, | ||
| redact(value) { return value; }, // optional: scrub secrets from output | ||
| }, | ||
| }); | ||
| ``` | ||
|
|
||
| The `vault=` object is a pluggable backend: any object exposing | ||
| `handle_request(action, payload, origin)` (and optionally `redact`) can back the | ||
| same `fill_credential` path, so an alternate source such as a 1Password CLI/SDK | ||
| backend can be dropped in without changing the fill code. | ||
| Every request carries the canonical `http(s)` origin of the current page | ||
| (`scheme://host[:port]`, default ports and user-info stripped); scope records | ||
| to it so one origin cannot read another's credential. If the backend provides | ||
| `redact`, every value it has handled is scrubbed from `run()` output as a final | ||
| safety net — redaction is not treated as authorization and is not used to make | ||
| DOM filling safe. Trusted host code must not return secret-bearing vault | ||
| results to model-authored code — prefer `bw.fillCredential(...)`, which keeps | ||
| the secret inside the worker. Omit `vault` to run without credential | ||
| management helpers entirely. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update the stale credential-storage wording.
This section now says BetterWright has no built-in store and requires a caller-supplied vault, but docs/getting-started.md still labels this as “encrypted storage.” Update that link text to describe the pluggable backend and trusted host-side use.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/credentials.md` around lines 70 - 95, Update the credential-storage link
text in docs/getting-started.md to describe the vault as a pluggable backend
used by trusted host-side code, replacing the stale “encrypted storage” wording
while preserving the existing link destination.
Comment on lines
+12
to
+17
| npx betterwright doctor # prints what resolved; should end with "BetterWright is ready." | ||
| ``` | ||
|
|
||
| If `doctor` reports Node missing, install it from <https://nodejs.org> and rerun | ||
| `setup`. If a JavaScript install reports CloakBrowser missing, rerun | ||
| `npx betterwright setup`; Python installs use `betterwright setup`. | ||
| `setup`. If `doctor` reports CloakBrowser missing, rerun | ||
| `npx betterwright setup`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' docs/getting-started.md
printf '\n--- SEARCH ---\n'
rg -n "betterwright doctor|betterwright setup|doctor" -S .Repository: CuriosityOS/betterwright
Length of output: 1934
Move the Node.js warning before doctor. npx betterwright doctor itself requires Node/npm, so users can’t get that message without first installing Node.js. Tell them to install Node.js first, then rerun setup and doctor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/getting-started.md` around lines 12 - 17, Update the getting-started
instructions so the Node.js installation warning appears before the npx
betterwright doctor command. Tell users to install Node.js first, then rerun
setup followed by doctor; keep the existing CloakBrowser recovery guidance
unchanged.
Comment on lines
+24
to
+28
| - **Allows the public internet, private networks, and loopback** — RFC 1918 | ||
| ranges, `127.0.0.0/8`, `localhost`, IPv6 loopback/unique-local, link-local, | ||
| carrier-grade NAT, and `*.internal`/`*.local`/`*.lan` hosts are reachable, so | ||
| an agent can drive local dev servers, a home router, or an intranet host | ||
| without extra configuration. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update the stale policy image caption.
The default posture now allows private networks and loopback, but the image description at Line [3] still says the policy “blocks private” addresses. Update the caption/alt text so the documentation does not contradict the new default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/network-policy.md` around lines 24 - 28, Update the network policy image
caption or alt text near the private-network description to state that private
networks and loopback are allowed by default, removing the stale claim that the
policy blocks private addresses.
Comment on lines
+60
to
66
| | `allowHosts` | Always allow these hosts. An entry matches a host exactly or as a parent domain (`example.com` also matches `sub.example.com`); add `:port` to pin a port. | | ||
| | `blockHosts` | Always block these hosts, evaluated before allowlists. | | ||
| | `blockSecretBearingUrls` | Refuse URLs that look like they carry a key/token. Default `true`. | | ||
| | `custom` | A hook, `custom(url, details)`, returning a decision or `null`, evaluated last. | | ||
|
|
||
| Evaluation order is: scheme check → `blockHosts` → `allowHosts` → metadata | ||
| floor → private-network rules → `custom`. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Qualify the allowHosts guarantee with the metadata floor.
“Always allow these hosts” contradicts the stated rule that metadata endpoints can never be allowlisted. Document this as “allow unless blocked by the metadata floor” to avoid implying that an allowlist can bypass the cloud-credential protection.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/network-policy.md` around lines 60 - 66, Update the allowHosts
description in the network policy table to state that matching hosts are allowed
unless blocked by the metadata floor. Keep the existing exact/parent-domain
matching and port-pinning behavior, and preserve the documented evaluation
order.
Comment on lines
3
to
8
| Runnable scripts. Each needs the runtime installed first (`betterwright setup`). | ||
|
|
||
| ### Python | ||
|
|
||
| ```bash | ||
| pip install betterwright | ||
| python examples/python/quickstart.py | ||
| ``` | ||
|
|
||
| - [`quickstart.py`](python/quickstart.py) — navigate, read, and capture proof. | ||
| - [`login_with_vault.py`](python/login_with_vault.py) — store and fill a | ||
| credential without the password reaching your code. | ||
| - [`signup_with_generated_password.py`](python/signup_with_generated_password.py) — | ||
| sign up with a generated password and a confirm-password field. | ||
| - [`onepassword_attach.py`](python/onepassword_attach.py) — log in with your own | ||
| 1Password extension in an attached (auto-launched) real Chrome. | ||
| - [`local_dev.py`](python/local_dev.py) — drive a `localhost` dev server. | ||
| - [`solve_captcha.py`](python/solve_captcha.py) — native checkbox and text-challenge helpers. | ||
|
|
||
| ### JavaScript | ||
|
|
||
| ```bash | ||
| npm install betterwright | ||
| node examples/javascript/quickstart.mjs | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the required browser setup command.
The text says examples require betterwright setup, but the copy-paste block only runs npm install and the script. Add npx betterwright setup before node examples/javascript/quickstart.mjs, or remove the setup prerequisite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/README.md` around lines 3 - 8, Update the examples README command
block to include the required browser setup step by adding npx betterwright
setup between npm install betterwright and node
examples/javascript/quickstart.mjs, keeping the prerequisite text consistent.
Comment on lines
+78
to
+93
| // Validate every answer, then connect to one of these exact literals. This | ||
| // closes both redirect-hop and DNS-rebinding gaps: Chromium never performs a | ||
| // second target lookup outside this guarded worker. | ||
| for (const candidate of addresses) { | ||
| const decision = await guardUrl( | ||
| transportUrl(candidate.address, port), | ||
| { | ||
| method: "CONNECT", | ||
| resourceType: "transport-address", | ||
| resolvedFrom: host, | ||
| }, | ||
| attribution, | ||
| ); | ||
| if (!decision?.allowed) throw proxyBlockedError(decision?.reason); | ||
| } | ||
| return addresses; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve hostname allowlist semantics during resolved-address checks.
Under strict mode, an allowlisted hostname resolving to 10.x, loopback, or another private literal passes the first check but fails the candidate check. This makes allowHosts: ["service.internal"] ineffective through the forced proxy. Carry the original allowlist authorization into resolution while still unconditionally rejecting metadata addresses.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/guard-proxy.mjs` around lines 78 - 93, Update the resolved-address
validation loop in the proxy connection flow to preserve the original hostname
allowlist authorization when calling guardUrl for each candidate. Ensure
allowlisted hostnames remain permitted even when resolving to private or
loopback literals, while metadata addresses continue to be rejected
unconditionally; use the existing attribution or decision context rather than
weakening general address checks.
Comment on lines
+209
to
+214
| const browser = new BetterWright({ | ||
| policy: policyFromEnv(env), | ||
| headless: headlessFromEnv(env), | ||
| connectOverCdp: String(env.BETTERWRIGHT_CONNECT_OVER_CDP || "").trim() || undefined, | ||
| downloadPolicy, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Centralize and propagate BETTERWRIGHT_BROWSER.
The MCP runtime and diagnostics currently interpret this setting differently.
src/mcp-server.mjs#L209-L214: validate the environment value and pass it through thebrowseroption.src/doctor.mjs#L117-L122: reuse the same parser and mark unsupported values as not ready rather than treating them as Cloak.
📍 Affects 2 files
src/mcp-server.mjs#L209-L214(this comment)src/doctor.mjs#L117-L122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mcp-server.mjs` around lines 209 - 214, Centralize parsing of
BETTERWRIGHT_BROWSER and use the shared parser in both sites: update
src/mcp-server.mjs lines 209-214 to validate the environment value and pass the
parsed result through BetterWright’s browser option, and update src/doctor.mjs
lines 117-122 to reuse that parser and report unsupported values as not ready
instead of treating them as Cloak.
Comment on lines
+141
to
+147
| // Private networks and loopback are reachable by default; agents commonly | ||
| // drive local dev servers, routers, and intranet hosts. Pass | ||
| // `allowPrivateNetwork: false` / `allowLoopback: false` for a hardened | ||
| // deployment. The cloud-metadata floor below is NOT governed by these and | ||
| // stays blocked regardless. | ||
| this.allowPrivateNetwork = options.allowPrivateNetwork !== false; | ||
| this.allowLoopback = options.allowLoopback !== false; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Canonicalize metadata hostnames before enabling open defaults.
A trailing DNS dot bypasses the metadata floor: metadata.google.internal. matches neither the metadata set nor .internal, so the new default-open policy allows it—especially dangerous in attach mode without the transport proxy. Strip a terminal dot before classification and add a conformance vector.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/policy.mjs` around lines 141 - 147, Canonicalize hostnames by removing a
terminal DNS dot before the metadata-floor classification used by the policy
checks, so dotted forms such as metadata.google.internal. remain blocked under
the open defaults. Update the relevant hostname classification logic near
allowPrivateNetwork and allowLoopback, and add a conformance vector covering the
trailing-dot metadata hostname.
Comment on lines
+105
to
+118
| // Secrets are kept beyond the run that used them because later runs can still | ||
| // echo a previously typed value (console, DOM dumps). The cap only bounds | ||
| // memory and per-redaction cost in long-lived workers; eviction is oldest-first | ||
| // (Set iterates in insertion order), with re-tracked secrets refreshed to | ||
| // newest. | ||
| function trackSecret(value) { | ||
| const secret = String(value ?? ""); | ||
| if (secret.length < 4) return; | ||
| activeSecrets.delete(secret); | ||
| activeSecrets.add(secret); | ||
| if (activeSecrets.size > MAX_ACTIVE_SECRETS) { | ||
| const oldest = activeSecrets.values().next().value; | ||
| activeSecrets.delete(oldest); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not evict secrets while browser state can still expose them.
After 201 distinct secrets, an older value can remain in the DOM, console, or queued events while no longer being redacted. Tie eviction to page/session disposal, or restart and invalidate browser state when the retention bound is reached.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/worker.mjs` around lines 105 - 118, The MAX_ACTIVE_SECRETS eviction in
trackSecret must not remove secrets while the associated browser page or session
remains capable of exposing them. Tie eviction to page/session disposal, or
restart and invalidate that browser state before removing the oldest secret;
preserve redaction for all still-live browser state.
Comment on lines
+1839
to
+1840
| pages: pages ?? (await summarizeSessionPages(session)), | ||
| durationMs: Math.round((performance.now() - started) * 10) / 10, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep error-envelope construction non-throwing.
The execute failure path calls summarizeSessionPages() through buildEnvelope; if the original failure involved a crashed or closed browser, summarization can throw and prevent any result from reaching the client. Catch summary failures or pass a safely computed pages fallback.
Proposed fix
- pages: pages ?? (await summarizeSessionPages(session)),
+ pages:
+ pages ??
+ (await summarizeSessionPages(session).catch(() => [])),Also applies to: 2863-2873
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/worker.mjs` around lines 1839 - 1840, Make buildEnvelope’s pages
resolution non-throwing by safely handling failures from
summarizeSessionPages(session), especially on the execute failure path. Preserve
the existing pages value when available and use a safe fallback when
summarization fails, ensuring the error envelope is still returned to the
client.
This was referenced Jul 16, 2026
Merged
CuriosityOS
added a commit
that referenced
this pull request
Jul 25, 2026
release: prepare npm v0.5.0 — npm-only, CLI + skill first
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
python/tree, publish-pypi workflow, and all dual-language machinery (worker byte-sync, policy/prompt parity tests, wheel smoke test). The shared policy vectors are kept as a JS conformance suite (tests/node/policy-vectors.test.mjs).betterwright skill(andskill --claude): prints a paste-ready agent skill (CLI usage + operator guidance) so any agent with a shell tool integrates without MCP, a server, or an SDK — Claude Code, Codex, Hermes, custom agents.--block-private-network/--block-loopbackandBETTERWRIGHT_BLOCK_*env vars re-harden.@modelcontextprotocol/sdkpeer dep.Verification
npm run release:checkgreen (versions, lint, unit, types, package smoke — 37-file tarball installs, imports, typechecks, CLI doctor runs).BETTERWRIGHT_REQUIRE_BROWSER=1.run(~0.8 s including navigation),replstate persistence across snippets, proof screenshot rendered and visually verified.browsercall returns Example Domain.pi --extension ./src/pi-extension.mjscompleted a browse+screenshot task.betterwright skilloutput in its system prompt drove the browser via bash successfully.🤖 Generated with Claude Code
Summary by CodeRabbit
betterwright skillwith optional Claude-compatible output.betterwright doctorreadiness reporting and improved CLI-first workflows.