Skip to content

feat(server): let a request name the tab that relays it - #207

Merged
chrischall merged 1 commit into
mainfrom
feat/via-tab
Aug 5, 2026
Merged

feat(server): let a request name the tab that relays it#207
chrischall merged 1 commit into
mainfrom
feat/via-tab

Conversation

@chrischall

Copy link
Copy Markdown
Owner

Closes #203.

The gap

request() derives the relay tab from the request's own host (ws-server.ts), and the fetch handler matches it strict-prefix (background.ts). That's correct for app hosts — the existing comment's photos.x.com / www.x.com reasoning is sound — but it assumes every host can have a tab.

API hosts can't. api.creditkarma.com serves no HTML app; a tab opened there has no content script (1 URL match, none responded). So with a profile declared for the apex and a signed-in www tab open, this was unsatisfiable:

$ fpx post-json https://api.creditkarma.com/graphql @body.json -p creditkarma
bridge error: no tab matching https://api.creditkarma.com/

Meanwhile that www tab can issue the cross-origin fetch perfectly well — CK's own web app does it for every GraphQL call.

The change

viaTab on RequestOpts / BodylessRequestOpts, --via-tab <url> on get / post-json / request. The default is untouched, so the photos.x.com case is unaffected — you only get the override by asking for it.

Verified live, same command that failed above:

$ node packages/cli/dist/index.js post-json https://api.creditkarma.com/graphql @body.json \
    -p creditkarma --via-tab https://www.creditkarma.com/ \
    -H 'ck-client-name: prime_web' -H 'ck-client-version: 2.0.31'
{"errors":[{"message":"An error occurred."}],"data":{"prime":null}}

A real GraphQL response from api.creditkarma.com, relayed through the www tab. (The error body is just the missing bearer — the point is that it reached the origin at all.)

Security

viaTab is guarded with assertUrlInDomains, exactly like the request URL. It widens which tab performs the fetch, never which origins are reachable; the declared-domain set approved at pair time stays the boundary. Malformed values throw naming the option rather than failing deep in the bridge. Both are tested.

Matching stays prefix-based, so https://www.example.com/ accepts any page on that host and a deeper path pins one specific page — tested.

Observation worth a separate issue

While tracing this I noticed handleFetchRequest gates req.init.url against declared domains but not req.init.tabUrl, and content scripts inject on <all_urls>. So a consumer calling the lower-level fetch() directly can already relay through a tab on an undeclared domain. The blast radius looks small — the request URL is still gated, so it's the page's origin being borrowed rather than its data being read — but it's an unenforced boundary, and this PR deliberately does not rely on it (hence the server-side guard). I left it alone rather than widen this PR's scope; say the word and I'll file it.

Tests

1154 pass (up from 1148). packages/server/tests/via-tab.test.ts covers the default, the override for absolute and relative paths, the declared-domain guard, subdomain/path forms, and malformed input. CLI tests cover flag parsing across all three verbs and the threading into server.request. Help text and CLAUDE.md updated.

🤖 Generated with Claude Code

https://claude.ai/code/session_012o2nXwu7tov6j7ciBEpigo

`request()` derives the relay tab from the request's own host. That is
right for app hosts — routing photos.x.com through a www.x.com tab would
be wrong — but it assumes every host CAN have a tab, and API hosts
cannot.

api.creditkarma.com serves no HTML app: a tab opened there has no content
script, so a bridged request to it is unroutable no matter what the user
opens. Meanwhile the signed-in www.creditkarma.com tab can issue that
cross-origin fetch perfectly well — CK's own web app does exactly that
for every GraphQL call. With a profile declared for the apex and a
signed-in www tab open, there was no way to satisfy:

  fpx post-json https://api.creditkarma.com/graphql @body.json -p ck
  # bridge error: no tab matching https://api.creditkarma.com/

`viaTab` (CLI: `--via-tab <url>`) names the relay explicitly, leaving the
default untouched so the photos.x.com reasoning still holds.

It is guarded with `assertUrlInDomains`, the same way the request URL is.
This widens which tab performs the fetch, never which origins can be
reached — the declared-domain set the user approved at pair time stays
the boundary. Matching is prefix-based, so `https://www.example.com/`
accepts any page on the host while a deeper path pins one.

Verified live: the reproduction above now returns a real GraphQL response
body from api.creditkarma.com, relayed through the www tab.

Closes #203

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012o2nXwu7tov6j7ciBEpigo
@github-actions github-actions Bot added the auto-review Trigger Claude + Copilot review on this PR label Aug 5, 2026
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @chrischall's task in 1m 59s —— View job


Reviewing PR #207

  • Read CLAUDE.md for conventions
  • Read PR diff
  • Review server-side changes (ws-server.ts, via-tab.test.ts)
  • Review CLI changes (args.ts, main.ts, verbs/fetch.ts, tests)
  • Post findings and verdict

Summary

Clean, well-scoped addition. viaTab on RequestOpts/BodylessRequestOpts is threaded correctly from CLI flag → CommandrunFetchFetchproxyServer.request, validated as a URL and gated through the existing assertUrlInDomains boundary (packages/server/src/ws-server.ts:1797-1806) before overriding tabUrl. Default behavior (https://{host}/) is untouched when viaTab is omitted, matching the PR's stated intent. Tests cover the default, override (absolute + relative paths), off-domain rejection, subdomain/path prefix matching, and malformed input on both the server and CLI sides.

The 1.12.0+ doc annotations (ws-server.ts:389, :577) match the pre-existing convention already on main (e.g. FetchproxyScopeError docs at :519, :547 from PR #205) of annotating with the next unreleased version pre-emptively — not a new inconsistency introduced here.

Note: I was unable to run npm test/npm run typecheck in this environment (tool approval required for non-read-only Bash), so test-suite-green status is based on static review of the new/changed test files, not an actual run.

Nits

  • 🟡 --via-tab isn't validated at the CLI layer the way the request URL is (assertUrlOnProfile in packages/cli/src/verbs/fetch.ts:52, called from runFetch at :75, throws a UsageError → exit 1). A malformed or off-domain --via-tab instead surfaces from the server as a generic thrown Error, which mapBridgeError (packages/cli/src/bridge-errors.ts) falls through to the other bucket and exit code 2 (bridge) rather than 1 (usage). Minor UX inconsistency, not a functional bug — the value is still safely rejected either way.

Observation (out of scope, author already flagged)

The PR body itself notes handleFetchRequest gates req.init.url against declared domains but not req.init.tabUrl on the extension side, deliberately not relied upon here since the server-side guard covers it. Worth a follow-up issue as the author suggests, not a blocker for this PR.

Verdict: pass

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

✅ Auto-review verdict: pass — Well-scoped, correctly wired, and well-tested addition of a viaTab/--via-tab override for relay-tab selection; only a minor CLI-vs-server exit-code inconsistency on malformed input.
📋 Tracking follow-ups: #209

@chrischall chrischall added the ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green label Aug 5, 2026
@chrischall
chrischall enabled auto-merge (squash) August 5, 2026 22:10
@chrischall
chrischall merged commit c5d3f4d into main Aug 5, 2026
11 checks passed
@chrischall
chrischall deleted the feat/via-tab branch August 5, 2026 22:11
@chrischall

Copy link
Copy Markdown
Owner Author

Correcting the evidence in this PR

The verification I gave was weaker than it looked, and I should have caught that before opening it. Two gaps:

  1. No control. I showed one successful --via-tab call returning a body, and asserted that proved the relay. It didn't — with no matching failure run in the same bridge state, the success wasn't attributable to the flag.
  2. The motivation was asserted, not tested. I claimed an API host "serves no HTML app" so its tab can never relay. My only supporting run had also reported a downed service worker and had explicitly told me to refresh the page and retry — and I never did. It could have been service-worker eviction.

Both have now been tested properly. The conclusion is unchanged, but the reasoning is different and sharper than what the PR body says.

Control, back to back, same bridge state

$ node packages/cli/dist/index.js post-json https://api.creditkarma.com/graphql @body.json -p creditkarma
bridge error (protocol): no tab matching https://api.creditkarma.com/ — open a tab on that host…

$ node … --via-tab https://www.creditkarma.com/
HTTP 400 https://api.creditkarma.com/graphql

Same command, same seconds apart, one flag different: unroutable → real origin response.

Why the API-host tab genuinely cannot work

I opened a tab on https://api.creditkarma.com/, reloaded it, and retried without the flag. It failed twice with 1 URL match, none responded. Then I inspected the tab:

{"url":"chrome-error://chromewebdata/",
 "title":"api.creditkarma.com",
 "bodyStart":"This api.creditkarma.com page can't be found\nNo webpage was found for the web address: https://api.creditkarma.com/"}

https://api.creditkarma.com/ 404s, so Chrome renders its own error document at chrome-error://chromewebdata/, and Chrome does not inject content scripts into chrome-error:// pages — the <all_urls> match never applies. Meanwhile chrome.tabs.query still reports the tab's URL as the requested https URL, which is precisely why the extension says "1 URL match, none responded": the URL matches, the document behind it can't host a relay.

So no amount of refreshing fixes it, and the "refresh the page to inject the content script" advice in that error is unactionable for this class of host.

What that changes

Nothing about the code — the guard, the default, and the tests all stand. But the PR body's framing ("serves no HTML app") is imprecise; the operative fact is 404 → chrome-error:// document → no content script, permanently. Any host that doesn't serve a real document at the path you'd open is in the same position, including one that 404s at / while serving a working API underneath — which is the common case.

Worth folding that mechanism into the viaTab doc comment; happy to do it in the follow-up that addresses #209.

🤖 Generated with Claude Code

chrischall added a commit that referenced this pull request Aug 5, 2026
…210)

Closes #209 — the nit auto-review raised on #207.

## The inconsistency

`runFetch` validates the request URL against the profile before
connecting, so a typo is exit 1 with guidance. `--via-tab` skipped that,
so the same class of mistake travelled to the server guard and came back
as exit 2 ("bridge error") — after making the user wait on a connection
to be told their flag was malformed.

Both now fail identically, before `listen()`:

```
$ fpx get https://api.x.com/v1 -p x --via-tab 'not a url'
fpx: not a valid URL: "not a url"                                    # exit 1

$ fpx get https://api.x.com/v1 -p x --via-tab https://evil.example/
evil.example is not on this profile's declared domains (…)           # exit 1
```

Verified by exit code, and the tests assert `listen()` was never called
— a typo shouldn't cost a bridge round-trip.

The server-side guard stays. It's the real boundary for library callers;
this just stops the CLI from routing a usage error through it.

## Also: corrects the `viaTab` doc comment

#207 said API hosts "serve no HTML app". That's imprecise, and it
doesn't explain why the extension's own advice for the failure —
*"Refresh the page in your browser to inject the content script"* —
can't work.

The actual mechanism, confirmed by opening such a tab and inspecting it:

```json
{"url":"chrome-error://chromewebdata/",
 "bodyStart":"This api.creditkarma.com page can't be found…"}
```

The host 404s at `/`, so Chrome renders its **own** document at
`chrome-error://chromewebdata/`, and Chrome never injects content
scripts into `chrome-error://` pages regardless of the `<all_urls>`
match. `chrome.tabs.query` still reports the tab's URL as the requested
https one — which is exactly why the failure reads "1 URL match, none
responded". No amount of reloading fixes it.

Full reproduction and the missing control are in the correction comment
on #207. Short version: my original evidence for that PR was weaker than
it looked — one success with no matching failure run, and a motivation
I'd asserted rather than tested. The conclusion held, but I should have
run the control before opening it.

## Tests

1157 pass (up from 1154). Three new CLI tests: malformed relay tab,
off-domain relay tab, and the accepted case — the first two also
asserting no connection attempt.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_012o2nXwu7tov6j7ciBEpigo

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
chrischall added a commit that referenced this pull request Aug 6, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.0.0](v1.11.0...v2.0.0)
(2026-08-06)


### ⚠ BREAKING CHANGES

* **protocol:** bind the ephemeral key into the ready signature
([#222](#222))

### Features

* **protocol:** add write_cookies, the one verb that can repair a
rotated session
([#211](#211))
([b2557c2](b2557c2))
* **protocol:** bind the ephemeral key into the ready signature
([#222](#222))
([c13aeed](c13aeed))
* **server:** let a request name the tab that relays it
([#207](#207))
([c5d3f4d](c5d3f4d))
* **server:** pin the extension's identity, and verify it on the peer
path ([#213](#213))
([0eeced7](0eeced7))


### Bug Fixes

* **cli:** let a real filesystem error be itself, not "no extension pin"
([#221](#221))
([c87a864](c87a864)),
closes [#220](#220)
* **cli:** validate --via-tab before connecting, like the request URL
([#210](#210))
([959fcc5](959fcc5))
* **extension:** reattach the write_cookies doc block, and name the
writable cookies as writable
([#215](#215))
([2730c4a](2730c4a))
* **extension:** use the guarded caps local for the cookie heading
([#217](#217))
([f95c832](f95c832))
* **server:** release only our own extension claim, and stop guessing
scoped names
([#219](#219))
([3d90a64](3d90a64)),
closes [#218](#218)
* **server:** type no-tab rejections so they stop reading as version
mismatches ([#205](#205))
([dc30bd9](dc30bd9))


### Refactor

* **server:** drop the concatBytes imports the signature change orphaned
([#224](#224))
([4985ba7](4985ba7)),
closes [#223](#223)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review Trigger Claude + Copilot review on this PR ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fetches to API-only subdomains are unroutable: tabUrl is host-exact with no override

1 participant