From bfc04d68ed08c18f3fba805509c0417b7c185ffc Mon Sep 17 00:00:00 2001 From: Chris Hall Date: Wed, 5 Aug 2026 18:17:58 -0400 Subject: [PATCH] fix(cli): validate --via-tab before connecting, like the request URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runFetch` checks the request URL against the profile up front, so a typo is exit 1 with guidance. `--via-tab` skipped that, so the same class of mistake reached the server guard instead and came back as exit 2 — a "bridge error" for what is plainly a usage error, after making the user wait on a connection to be told so. Both now fail the same way, before `listen()`: $ fpx get https://api.x.com/v1 -p x --via-tab 'not a url' # exit 1 $ fpx get https://api.x.com/v1 -p x --via-tab https://evil.example/ # exit 1 Also corrects the `viaTab` doc comment. It said API hosts "serve no HTML app", which is imprecise and does not explain why the extension's own advice for the failure ("refresh the page to inject the content script") is unactionable. The real mechanism, confirmed by inspecting such a tab: the host 404s at `/`, so Chrome renders its own document at `chrome-error://chromewebdata/`, and Chrome never injects content scripts into `chrome-error://` pages. `chrome.tabs.query` still reports the tab's URL as the requested https one — hence "1 URL match, none responded" — and no amount of reloading changes it. Closes #209 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012o2nXwu7tov6j7ciBEpigo --- packages/cli/src/verbs/fetch.ts | 5 +++++ packages/cli/tests/fetch.test.ts | 36 +++++++++++++++++++++++++++++++- packages/server/src/ws-server.ts | 19 ++++++++++++----- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/verbs/fetch.ts b/packages/cli/src/verbs/fetch.ts index 39bbc50..241e593 100644 --- a/packages/cli/src/verbs/fetch.ts +++ b/packages/cli/src/verbs/fetch.ts @@ -73,6 +73,11 @@ export async function runFetch( makeServer: VerbServerFactory = defaultServerFactory, ): Promise { const domain = assertUrlOnProfile(cmd.url, profile); + // Check the relay tab the same way and at the same time as the request URL. + // The server guards it too, but that guard only fires after the bridge is + // up, turning a typo into exit 2 ("bridge error") when it is plainly a usage + // error — and making the user wait on a connection to be told so (#209). + if (cmd.viaTab !== undefined) assertUrlOnProfile(cmd.viaTab, profile); const server = makeServer({ ...serverOptsFor(cmd.profile, profile, VERSION), onPairCode: pairCodePrinter(io), diff --git a/packages/cli/tests/fetch.test.ts b/packages/cli/tests/fetch.test.ts index 3c07db7..da623d4 100644 --- a/packages/cli/tests/fetch.test.ts +++ b/packages/cli/tests/fetch.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi } from 'vitest'; import { runFetch, type VerbServer } from '../src/verbs/fetch.js'; import { emptyProfile } from '../src/profiles.js'; -import { EXIT, type Io } from '../src/output.js'; +import { EXIT, UsageError, type Io } from '../src/output.js'; import { FetchproxySessionNotReadyError } from '@fetchproxy/server'; function memIo(): Io & { outs: string[]; errs: string[] } { @@ -131,3 +131,37 @@ describe('runFetch — --via-tab', () => { ); }); }); + +describe('runFetch — --via-tab is validated like the request URL', () => { + // The request URL is checked against the profile before connecting, so a + // typo is exit 1 with guidance. --via-tab skipped that check, so the same + // class of mistake travelled to the bridge and came back as exit 2 — a + // "bridge error" for what is purely a usage error (#209). + it('rejects a malformed relay tab as a usage error', async () => { + const server = stubServer(); + await expect( + runFetch({ ...CMD, viaTab: 'not a url' }, PROFILE, memIo(), () => server), + ).rejects.toThrow(UsageError); + // Must fail before connecting — no bridge round-trip for a typo. + expect(server.listen).not.toHaveBeenCalled(); + }); + + it('rejects an off-domain relay tab as a usage error', async () => { + const server = stubServer(); + await expect( + runFetch({ ...CMD, viaTab: 'https://evil.example/' }, PROFILE, memIo(), () => server), + ).rejects.toThrow(UsageError); + expect(server.listen).not.toHaveBeenCalled(); + }); + + it('accepts a relay tab on a declared domain', async () => { + const server = stubServer(); + const code = await runFetch( + { ...CMD, viaTab: 'https://www.tripadvisor.com/' }, + PROFILE, + memIo(), + () => server, + ); + expect(code).toBe(EXIT.OK); + }); +}); diff --git a/packages/server/src/ws-server.ts b/packages/server/src/ws-server.ts index a1a18bf..5d7eca7 100644 --- a/packages/server/src/ws-server.ts +++ b/packages/server/src/ws-server.ts @@ -392,11 +392,20 @@ export interface RequestOpts { * By default the relay tab is `https://{host-of-the-request}/`, which 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.example.com` typically serves no HTML app, so a tab opened - * there has no content script and the request is unroutable however hard the - * user tries. Meanwhile the signed-in `www.example.com` tab can issue that - * cross-origin fetch perfectly well — which is exactly what the site's own - * web app does. + * cannot. + * + * The mechanism is worth knowing, because the extension's own advice for + * this failure ("refresh the page to inject the content script") cannot + * work. An API host typically 404s at `/`, so Chrome renders its OWN + * document at `chrome-error://chromewebdata/` — and Chrome never injects + * content scripts into `chrome-error://` pages, `` match or not. + * `chrome.tabs.query` still reports the tab's URL as the requested https + * one, which is why the failure reads "1 URL match, none responded": the URL + * matches, the document behind it cannot host a relay, and no amount of + * reloading changes that. + * + * Meanwhile the signed-in `www.example.com` tab can issue that cross-origin + * fetch perfectly well — which is exactly what the site's own web app does. * * Naming the relay explicitly keeps the safe default intact while unblocking * that case. The value is matched against open tabs by prefix, so