Repository navigation
feat: target the connected browser, not Chrome by default (#29) - #30
Conversation
Browser Hand assumed Google Chrome across the stack. When the extension
runs in a non-Chrome Chromium browser (e.g. "Search"), tab bootstrap failed
with HTTP 500 ("debugger.attach isn't available in Search"), doctor
recommended the wrong fix (path_b), file: URLs failed in tabs.create, and
the skill text pushed agents to fall back to `open -a "Google Chrome"`.
Implements the four asks from #29:
1. Detect the host browser. `extension/utils/browser-info.ts` reads the
product brand/version from navigator.userAgentData (preferring the real
product over the shared "Chromium" engine brand and the GREASE
placeholder), falling back to a UA-string parse. The extension registers
it with the relay on connect; the relay only ever holds one extension
connection, so the connected browser is the current target. `--browser`
lets the caller assert which browser they expect and warns on mismatch.
2. Report it. The relay exposes `browser: {name, version}` on the health
endpoint; `doctor` surfaces it. `doctor` now distinguishes
`debugger_unavailable` (this browser restricts chrome.debugger) from
`path_b_required` (needs a separate debug profile).
3. Fall back to scripting. When chrome.debugger.attach is unavailable,
TabManager marks the browser debugger-unavailable and switches the tab to
the scripting transport instead of throwing, so `open` still creates the
tab via chrome.tabs.create and continues rather than returning HTTP 500.
4. Local files. `file:` URLs fail in chrome.tabs.create, so the CLI rewrites
them to the relay's new `/local-file` route, which reads the file and
serves it over http with a content type.
Docs: SKILL.md now says "your browser with the Browser Hand extension" (not
Chrome), documents the file: and debugger_unavailable paths, and adds a rule
to never open tabs with OS commands as a fallback.
Tests: browser-info detection, debugger_unavailable classification + doctor
distinction, file: URL rewrite, local-file serving, and the TabManager
scripting fallback.
Closes #29
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c14709b8b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@claude please take the Codex P1 on this PR: local HTML documents opened by the relay ( While you're there, also look at the Codex P2 at Please don't merge this PR. Note: the failing |
|
Agent autojack-orchestrator failed after 13s Server restarted while task was running (reconciled: no live worker) Task |
|
Dispatched as task P1 ( Planned fix.
P2 ( CI. The Playwright-based jobs will be pinned to The work goes onto this PR's branch, |
Codex review on #30. P1: /local-file?path=... let any page served by the relay (same origin as the relay API) read any file the relay process could read and call every relay route. Replace it with a registration flow: POST /local-docs (CLI-only: no Origin header, JSON body) returns a 192-bit token, and documents are served from a second listener (its own origin) at /local/<token>/<relative path>, resolved with realpath strictly inside the document's directory. Traversal, encoded traversal, absolute paths, escaping symlinks, dotfiles and sibling directories sharing the prefix are all 404. Doc responses carry CSP connect-src 'self', nosniff, no-referrer, CORP same-origin and no CORS headers. Control routes (including the /cdp and /extension upgrades) reject requests whose Host or Origin is the document origin. Tokens expire after 2h idle, the registry is bounded (64, LRU) and cleared on stop(). P2: rewriteLocalFileUrl is now async: it registers with the relay and returns the capability URL, so ./style.css, img/a.png etc. resolve inside the document's directory, and the file: URL's query string and hash are kept. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codex review on #30: P1 (
|
| Question | Answer (confirmed by reading relay/src/relay.ts + running it) |
|---|---|
| What origin do local pages run on? | The relay's own origin, http://127.0.0.1:<port>. Same origin as /, /pages, /local-file. |
| Can a page fetch arbitrary paths? | Yes. readLocalFile did resolve(requested) + readFile with no root. A page in /tmp/bh-repro/report/ read /tmp/bh-repro/secret/creds.txt (a sibling directory) via fetch('/local-file?path=…') → 200 "TOP-SECRET-TOKEN=abc123". |
| Any token / Origin / Host gate? | None, on any route. There was no auth middleware, no Origin/Host check, no CORS config. The only protection was the loopback bind, and same-origin script needs no CORS. The page also read /pages (200). |
| P2 | Same page: ./style.css → relay /style.css → 404. |
Pre-fix DOM captured in headless Chromium: origin=http://127.0.0.1:19333, /local-file?path=<secret> -> 200, /pages -> 200, ./style.css -> 404.
Design
- Capability, not a path.
POST /local-docs {path}registers one document and returns a 192-bit token (crypto.randomBytes(24)). Documents are served at/local/<token>/<relative path>./local-fileis removed, and no served route accepts a filesystem path. - Containment. Reads are resolved with
realpathand must be a strict descendant of the registered document's directory. 404 for..,%2e%2e,%2f/%5c, NUL, malformed escapes, absolute paths, symlinks leaving the directory, a sibling sharing the prefix (/reportvs/report-secret), and dotfiles (.env,.git, …). A registered document that itself symlinks out of its directory is refused (400). Every failure is the same bare 404. - Origin isolation: separate listener. The relay starts a second HTTP listener (OS-assigned port,
localDocPortto pin it). Origins include the port, so documents are never same-origin with the API. I picked this over alocalhostvs127.0.0.1split because that depends on a Host check being right on every route (including WS upgrades) and breaks if the relay is bound to a non-loopback host. The doc listener has no route except/local/<token>/…. - Headers on every doc response:
Content-Security-Policy: … connect-src 'self'; form-action 'self'; base-uri 'self'; object-src 'none'; frame-ancestors 'self',X-Content-Type-Options: nosniff,Referrer-Policy: no-referrer(keeps the token out of Referer),Cross-Origin-Resource-Policy: same-origin,Cache-Control: no-store, and no CORS headers. - Control routes reject the doc origin. A middleware in front of every relay route (health,
/pages,/cdpand/extensionupgrades,/local-docs) returns 403 whenOriginorHostis the doc authority. - Registration is CLI-only: it must carry no
Originheader (browsers always send one on POST),Content-Type: application/json, and a valid absolute path to a regular file. - Lifetime: idle TTL 2 h (sliding), registry bounded to 64 with LRU eviction, cleared on
stop(). - P2 / CLI:
rewriteLocalFileUrlis nowasync: it registers with the relay and returns<doc origin>/local/<token>/<basename>plus the original?query#hash. It runs afterensureRelayConnected, since registering needs a running relay. A relay that predates this change gets a clear "restartbrowser-hand relay" error../style.css,img/a.pngand../x(within the directory) resolve against the capability URL. - CI:
c100e1ealready pinsDaemon (TypeScript + Tests)toubuntu-24.04. No other workflow installs Playwright browsers (release.ymland the otherci.ymljobs don't), so nothing else needed pinning. - Docs:
SKILL.mdlocal-file bullet,browser-hand --help, and anAGENTS.mdrelay note (never add a route that takes a path from a request).
Verification
Post-fix, same hostile page, real Chromium, relay + CLI path (rewriteLocalFileUrl) as shipped:
origin=http://127.0.0.1:53451 <- not the relay origin (19334)
search=?relay=http://127.0.0.1:19334 hash=#frag <- query/hash preserved
css applied via relative ./style.css: body color=rgb(255, 0, 0)
relative ./style.css -> 200
old route /local-file?path=<secret> -> 404
relative ../secret/creds.txt -> 404
encoded %2e%2e/secret/creds.txt -> 404
same-origin /pages (doc listener) -> 404
relay API http://127.0.0.1:19334/pages -> BLOCKED (TypeError) <- CSP connect-src 'self'
relay POST http://127.0.0.1:19334/local-docs -> BLOCKED (TypeError)
relay: node --import tsx --test "src/*.test.ts" tests 21 / pass 21 / fail 0
cli-js: node --test "test/*.test.js" tests 164 / pass 164 / fail 0
relay: tsc -p tsconfig.json --noEmit clean
prettier --check on every touched file I own clean
New coverage: relative CSS/JS/image/nested loads; traversal (raw, encoded, absolute) → 404; symlink and /report vs /report-secret prefix sibling → 404; unknown and expired tokens → 404; doc origin can't reach /, /pages, /cdp, /extension or /local-docs (Origin, Host and WS-upgrade variants, with positive controls); registration refuses Origin/text-plain/relative/missing; TTL slide, LRU bound, clear; CLI contract against a stub relay plus an end-to-end test that drives the real rewriteLocalFileUrl against a real relay. I mutation-checked the guards: disabling containment, the doc-origin guard, the CSP connect-src or the dotfile rule each makes tests fail.
Caveats worth a reviewer's eye
- A document can still read its own directory tree (minus dotfiles and symlinks out). That is the requested capability scope. Opening a report from a broad directory like
~/Downloadsexposes that directory to the page. connect-src 'self'means a report thatfetches remote JSON no longer works throughopen file:…. Scripts, styles and images fromhttps:still load.- The other control routes remain unauthenticated and reachable by any local process (pre-existing, out of scope here). DNS rebinding of those routes would need a Host allowlist, which I did not add.
- Size: the delta on top of
c100e1eis 894 changed lines (roughly half are tests). - The old query-path
/local-fileURL is gone, with no compatibility shim. node_moduleswas symlinked from the main checkout for the run; nothing under it is committed.
autohub-claude
Bot
added
the
babysit:active
|
Babysit stopped — Proposed split: 9 PRs, in landing order. Each part keeps source files with their tests and lands after the parts it needs. Parts stay under the 400-line per-PR target unless one file and its tests are larger. Babysit has not created any branch or PR. To go ahead, ask an agent to run this split with the split-to-prs skill. This comment is the plan it needs approved.
graph LR
base["main"]
P1["1. 392 lines"]
base --> P1
P2["2. 210 lines"]
base --> P2
P3["3. 387 lines"]
P1 --> P3
P2 --> P3
P4["4. 190 lines"]
base --> P4
P5["5. 71 lines"]
base --> P5
P6["6. 13 lines"]
base --> P6
P7["7. 7 lines"]
base --> P7
P8["8. 5 lines"]
base --> P8
P9["9. 5 lines"]
base --> P9
Files, dependencies and threads per partPart 1. Branches off
Part 2. Branches off
Part 3. Stacks on part 1, because
Part 4. Branches off
Part 5. Branches off
Part 6. Branches off
Part 7. Branches off
Part 8. Branches off
Part 9. Branches off
What this run did
Verification on the pushed head
Not run: a CI at the time of this comment (not waited on, per the size stop): Open advisory finding, left unfixed on purposeThe bounded local review pass (Codex, Why I did not fix it here: the requested containment still holds (nothing leaves the registered directory), the dotfile rule is an extra layer the worker added beyond the acceptance criteria, and this PR has no Review Contract or Threat Model to make it a direct breach. Under the Babysit rules that makes it a hardening decision, not an auto-fix. Untested sketch of the change: after It is not filed as a separate issue: it concerns unmerged code, and Babysit finalizer: Claude Code ( |
autohub-claude
Bot
added
babysit:blocked
|
Agent kernel-worker failed after 2089s Too large for one PR: 1280 changed lines on agent/kernel-worker/64ffceab. Split proposed, no PR opened. Task |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c12c4a552
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
autohub-claude
Bot
added
babysit:active
…hides When the host browser rejects debugger.attach, TabManager falls back to the scripting transport, so the doctor's smoke tab opens fine and classification returned tab_bootstrap_works before ever seeing the error it keyed on. The documented debugger_unavailable status was unreachable with the updated extension, and limits such as Page.navigate being rejected on scripting tabs went unreported. - extension: TabManager reports the limitation to the relay (register message) the first time it sees it; registerHostBrowser re-sends it on every connect, since ConnectionManager.send drops messages while the socket is down. - relay: keep the report per extension connection (cleared on replace/close), validated and bounded, and expose it as `debugger` on GET /. - doctor: re-read the relay after the smoke tab (the attach, and so the discovery, happens during it), classify debugger_unavailable explicitly even when the smoke succeeded, expose `debugger` in the result, and drop the misleading "Tab bootstrap failed" path_b note for this status. Tests: fake extension -> real relay -> real doctor end-to-end, classifier unit tests, and TabManager report-once / wedge-is-not-a-limitation tests. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
autohub-claude
Bot
added
babysit:waiting-ci
|
Babysit hand-off — verified green at
Label: |
|
Agent kernel-worker failed after 789s Babysit follow-on finished but label verification failed: Command failed: gh pr view 30 --json labels --jq [.labels[].name] | map(select(startswith("babysit:"))) Task |
jack-arturo
removed
the
babysit:ready
Browser Hand assumed Google Chrome across the stack. When the extension
runs in a non-Chrome Chromium browser (e.g. "Search"), tab bootstrap failed
with HTTP 500 ("debugger.attach isn't available in Search"), doctor
recommended the wrong fix (path_b), file: URLs failed in tabs.create, and
the skill text pushed agents to fall back to
open -a "Google Chrome".Implements the four asks from #29:
Detect the host browser.
extension/utils/browser-info.tsreads theproduct brand/version from navigator.userAgentData (preferring the real
product over the shared "Chromium" engine brand and the GREASE
placeholder), falling back to a UA-string parse. The extension registers
it with the relay on connect; the relay only ever holds one extension
connection, so the connected browser is the current target.
--browserlets the caller assert which browser they expect and warns on mismatch.
Report it. The relay exposes
browser: {name, version}on the healthendpoint;
doctorsurfaces it.doctornow distinguishesdebugger_unavailable(this browser restricts chrome.debugger) frompath_b_required(needs a separate debug profile).Fall back to scripting. When chrome.debugger.attach is unavailable,
TabManager marks the browser debugger-unavailable and switches the tab to
the scripting transport instead of throwing, so
openstill creates thetab via chrome.tabs.create and continues rather than returning HTTP 500.
Local files.
file:URLs fail in chrome.tabs.create, so the CLI rewritesthem to the relay's new
/local-fileroute, which reads the file andserves it over http with a content type.
Docs: SKILL.md now says "your browser with the Browser Hand extension" (not
Chrome), documents the file: and debugger_unavailable paths, and adds a rule
to never open tabs with OS commands as a fallback.
Tests: browser-info detection, debugger_unavailable classification + doctor
distinction, file: URL rewrite, local-file serving, and the TabManager
scripting fallback.
Closes #29
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Closes #29
Opened by AutoJack after the AutoHub babysit finalizer exited without creating the PR (agent-fp-480b25e580478ac264ffceab, retry attempt fede23f2).