From f8a749bc24b47e1afd102b74037f136b340c143e Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:35:51 -0700 Subject: [PATCH 1/2] fix(selfhost): authenticate the browserless readiness probe (#9487 follow-up) browserEndpointReadinessProbe dropped the endpoint's `?token=` and fetched /json/version unauthenticated, on the stated assumption that it needs no auth. It does. A browserless started with a TOKEN -- the documented way, and how the ORB runs it -- answers 401, so browser_endpoint reported false forever: readiness permanently not-ready and the container permanently unhealthy, while screenshot capture worked perfectly the whole time. Verified on the live ORB (browserless v2, Chrome 149): 401 without credentials, 200 with `Authorization: Bearer `. Send the token as a header rather than restoring it to the query string, which keeps the original privacy intent -- the token still never appears in a URL that could reach an access log or an echoed error -- while actually authenticating. The call site in server.ts dropped the init argument, so it also had to forward it; without that the header would have been built and silently discarded. --- src/selfhost/health.ts | 36 ++++++++++++++++++--- src/server.ts | 6 ++-- test/unit/selfhost-health.test.ts | 52 +++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 6 deletions(-) diff --git a/src/selfhost/health.ts b/src/selfhost/health.ts index 6c41600903..7979e62239 100644 --- a/src/selfhost/health.ts +++ b/src/selfhost/health.ts @@ -165,12 +165,13 @@ export function codexAuthReadinessProbe( */ export function browserEndpointReadinessProbe( env: Record, - fetchImpl: (url: string) => Promise<{ ok: boolean }>, + fetchImpl: (url: string, init?: { headers: Record }) => Promise<{ ok: boolean }>, cacheMs = 30_000, ): ReadinessProbe | null { const endpoint = (env.BROWSER_WS_ENDPOINT ?? "").trim(); if (!endpoint) return null; const versionUrl = browserVersionUrl(endpoint); + const auth = browserAuthHeaders(endpoint); // A configured-but-unparseable endpoint is a real misconfiguration, and one the ordinary capture path would // only surface as a per-shot render failure. Fail readiness closed rather than skipping the probe. if (versionUrl === null) return { name: "browser_endpoint", check: () => Promise.resolve(false) }; @@ -183,7 +184,7 @@ export function browserEndpointReadinessProbe( const now = Date.now(); if (cached !== undefined && now < cachedUntil) return Promise.resolve(cached); if (inFlight) return inFlight; - inFlight = fetchImpl(versionUrl) + inFlight = fetchImpl(versionUrl, auth) .then((response) => response.ok) .catch(() => false) .then((ok) => { @@ -200,8 +201,9 @@ export function browserEndpointReadinessProbe( } /** The `http(s):///json/version` sibling of a `ws(s)://` browserless endpoint, or null when the - * configured value is not a parseable ws/wss URL. Query strings (browserless carries `?token=`) are dropped: - * the version endpoint needs no auth and the token must never end up in a probe URL that could be logged. */ + * configured value is not a parseable ws/wss URL. The query string (browserless carries `?token=`) is + * dropped so the token can never end up in a probe URL that might be logged -- it travels as an + * Authorization header instead, see browserAuthHeaders. */ function browserVersionUrl(endpoint: string): string | null { try { const url = new URL(endpoint); @@ -212,6 +214,32 @@ function browserVersionUrl(endpoint: string): string | null { } } +/** + * `Authorization: Bearer ` for a browserless endpoint that carries `?token=`, or no headers when it + * does not. + * + * REGRESSION THIS FIXES: the probe originally dropped the query string and sent nothing, on the stated + * assumption that `/json/version` needs no auth. It does. A browserless started with a TOKEN answers 401 to + * an unauthenticated `/json/version` (verified against browserless v2 / Chrome 149 on the live ORB), so + * `browser_endpoint` reported false forever on every deployment that runs it the recommended way -- readiness + * permanently not-ready, container permanently unhealthy, while screenshot capture worked perfectly. A probe + * that cannot pass when the thing it probes is healthy is worse than no probe: it trains operators to ignore + * readiness. + * + * The header, not the query param, keeps the original privacy intent intact: the token still never appears in + * the URL, so it cannot leak through a logged request line, a proxy access log, or an error message that + * echoes the URL. + */ +export function browserAuthHeaders(endpoint: string): { headers: Record } | undefined { + try { + const token = new URL(endpoint).searchParams.get("token"); + return token ? { headers: { Authorization: `Bearer ${token}` } } : undefined; + } catch { + // An unparseable endpoint already fails the probe closed via browserVersionUrl; nothing to add here. + return undefined; + } +} + /** Boot-time DATA-SAFETY advisory. A single SQLite file with no acknowledged backup is a data-loss SPOF — yet * `/ready` would still answer 200, so an operator can run with zero durability believing they're healthy. Returns * the warning to log at boot (or null on Postgres, or once the operator sets `BACKUP_ACKNOWLEDGED=true` after diff --git a/src/server.ts b/src/server.ts index 0243ab12b3..8076d6f721 100644 --- a/src/server.ts +++ b/src/server.ts @@ -941,8 +941,10 @@ async function main(): Promise { // #9487/#9464: browserless readiness. A visual-capture outage used to be invisible in /ready and in // Prometheus while it silently affected gate outcomes; this makes it observable at the same place every // other optional backend is. No-op unless BROWSER_WS_ENDPOINT is configured. - const browserProbe = browserEndpointReadinessProbe(process.env, async (url) => { - const response = await fetch(url, { signal: AbortSignal.timeout(1500) }); + const browserProbe = browserEndpointReadinessProbe(process.env, async (url, init) => { + // `init` carries the Authorization header when BROWSER_WS_ENDPOINT has a `?token=`. Forwarding it is + // what makes the probe work at all against a tokened browserless, which 401s /json/version otherwise. + const response = await fetch(url, { ...init, signal: AbortSignal.timeout(1500) }); return { ok: response.ok }; }); if (browserProbe) { diff --git a/test/unit/selfhost-health.test.ts b/test/unit/selfhost-health.test.ts index 78eebaf756..731a708b3c 100644 --- a/test/unit/selfhost-health.test.ts +++ b/test/unit/selfhost-health.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it, vi } from "vitest"; import { createD1Adapter, nodeSqliteDriver } from "../../src/selfhost/d1-adapter"; import { backupAcknowledgedGaugeValue, + browserAuthHeaders, browserEndpointReadinessProbe, buildHealthBody, codexAuthReadinessProbe, @@ -543,3 +544,54 @@ describe("browserEndpointReadinessProbe (#9487)", () => { expect(calls).toBe(2); }); }); + +describe("browser_endpoint authenticates (#9487 follow-up)", () => { + const TOKENED = "ws://browserless:3000?token=secret-value"; + + it("REGRESSION: a tokened browserless answers 401 unauthenticated — the probe must send credentials", async () => { + // The live failure: browserless v2 started with a TOKEN (the documented way) 401s an unauthenticated + // /json/version, so browser_endpoint reported false forever while capture worked fine. Readiness that + // cannot pass on a healthy backend trains operators to ignore readiness. + const seen: Array<{ url: string; headers?: Record }> = []; + const probe = browserEndpointReadinessProbe({ BROWSER_WS_ENDPOINT: TOKENED }, (url, init) => { + seen.push({ url, headers: init?.headers }); + // Model the real server: 401 unless credentials are presented. + return Promise.resolve({ ok: init?.headers?.Authorization === "Bearer secret-value" }); + }); + expect(await probe?.check()).toBe(true); + expect(seen[0]?.headers).toEqual({ Authorization: "Bearer secret-value" }); + }); + + it("INVARIANT: the token never appears in the probe URL, only in the header", () => { + // The original code dropped the query string for a good reason -- a token in a URL leaks through access + // logs and error messages that echo it. The fix must keep that property while still authenticating. + const headers = browserAuthHeaders(TOKENED); + expect(headers).toEqual({ headers: { Authorization: "Bearer secret-value" } }); + const seen: string[] = []; + browserEndpointReadinessProbe({ BROWSER_WS_ENDPOINT: TOKENED }, (url) => { + seen.push(url); + return Promise.resolve({ ok: true }); + })?.check(); + expect(seen[0]).toBe("http://browserless:3000/json/version"); + expect(seen[0]).not.toContain("secret-value"); + expect(seen[0]).not.toContain("token"); + }); + + it("an endpoint with NO token sends no Authorization header", () => { + expect(browserAuthHeaders("ws://browserless:3000")).toBeUndefined(); + }); + + it("an unparseable endpoint yields no headers rather than throwing", () => { + expect(browserAuthHeaders("not a url")).toBeUndefined(); + }); + + it("wss maps to https and still carries the header", () => { + const seen: Array<{ url: string; headers?: Record }> = []; + browserEndpointReadinessProbe({ BROWSER_WS_ENDPOINT: "wss://remote.example:443?token=t" }, (url, init) => { + seen.push({ url, headers: init?.headers }); + return Promise.resolve({ ok: true }); + })?.check(); + expect(seen[0]?.url).toBe("https://remote.example/json/version"); + expect(seen[0]?.headers).toEqual({ Authorization: "Bearer t" }); + }); +}); From ee6558f23c31f6ff1a06121050e9bb1f4bada80e Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:36:09 -0700 Subject: [PATCH 2/2] fix(test): type the probe spy for exactOptionalPropertyTypes --- test/unit/selfhost-health.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/selfhost-health.test.ts b/test/unit/selfhost-health.test.ts index 731a708b3c..b552efccfd 100644 --- a/test/unit/selfhost-health.test.ts +++ b/test/unit/selfhost-health.test.ts @@ -552,7 +552,7 @@ describe("browser_endpoint authenticates (#9487 follow-up)", () => { // The live failure: browserless v2 started with a TOKEN (the documented way) 401s an unauthenticated // /json/version, so browser_endpoint reported false forever while capture worked fine. Readiness that // cannot pass on a healthy backend trains operators to ignore readiness. - const seen: Array<{ url: string; headers?: Record }> = []; + const seen: Array<{ url: string; headers: Record | undefined }> = []; const probe = browserEndpointReadinessProbe({ BROWSER_WS_ENDPOINT: TOKENED }, (url, init) => { seen.push({ url, headers: init?.headers }); // Model the real server: 401 unless credentials are presented. @@ -586,7 +586,7 @@ describe("browser_endpoint authenticates (#9487 follow-up)", () => { }); it("wss maps to https and still carries the header", () => { - const seen: Array<{ url: string; headers?: Record }> = []; + const seen: Array<{ url: string; headers: Record | undefined }> = []; browserEndpointReadinessProbe({ BROWSER_WS_ENDPOINT: "wss://remote.example:443?token=t" }, (url, init) => { seen.push({ url, headers: init?.headers }); return Promise.resolve({ ok: true });