Skip to content

fix(selfhost): authenticate the browserless readiness probe so it can pass on a healthy backend (#9487 follow-up) - #9784

Merged
JSONbored merged 2 commits into
mainfrom
fix/browser-readiness-token
Jul 29, 2026
Merged

fix(selfhost): authenticate the browserless readiness probe so it can pass on a healthy backend (#9487 follow-up)#9784
JSONbored merged 2 commits into
mainfrom
fix/browser-readiness-token

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

The bug

browserEndpointReadinessProbe drops the endpoint's ?token= and fetches /json/version unauthenticated:

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.

The first half of that is wrong. A browserless started with a TOKEN — the documented way, and how the ORB runs it — requires auth on /json/version too.

Verified against the live ORB (browserless v2 / Chrome 149):

/json/version                          -> 401 "Bad or missing authentication."
/json/version  Authorization: Bearer   -> 200 {"Browser":"Chrome/149.0.7827.55",...}
/json/version  x-token: <token>        -> 401

So browser_endpoint reported false permanently on any deployment running browserless the recommended way — readiness never ready, container never healthy — while screenshot capture worked perfectly throughout. A probe that cannot pass when the thing it probes is healthy is worse than no probe: it trains operators to ignore readiness.

Found by deploying orb-v3.6.0-beta.1 to the ORB, where it was the only failing readiness check.

The fix

Send the token as Authorization: Bearer <token> instead of restoring it to the query string. That keeps the original comment's privacy intent fully intact — the token still never appears in a URL, so it can't leak via a request line, a proxy access log, or an error that echoes the URL — while actually authenticating.

The call site in src/server.ts discarded the probe's init argument, so it needed fixing too; without that the header would have been constructed and silently thrown away. Worth noting the unit tests alone would not have caught that half — they exercise the probe with an injected fetchImpl, so a call site that ignores init still passes them. The test asserting the exact header, plus this call-site change, are what make it real.

Tests

5 new cases in test/unit/selfhost-health.test.ts, including a regression that models the real server (401 unless credentials are presented) and an invariant asserting the token appears in the header and never in the probe URL. 49 pass.

Note

Deploying beta.1 also surfaced a separate config issue on the ORB itself: docker-compose.yml defaults GITHUB_APP_PRIVATE_KEY_FILE to a 0-byte placeholder .pem, which #9487's (correct) empty-secret check turns into a boot crash for any operator who has that placeholder and no GitHub App. Fixed on the box via the local override; the default may deserve a follow-up so self-hosters don't hit the same wall.

…llow-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 <token>`.

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.
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Important

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏳ LoopOver is waiting…

LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting

@JSONbored JSONbored self-assigned this Jul 29, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored
JSONbored merged commit d1a4658 into main Jul 29, 2026
4 checks passed
@JSONbored
JSONbored deleted the fix/browser-readiness-token branch July 29, 2026 08:39
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.48%. Comparing base (47842ec) to head (ee6558f).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9784      +/-   ##
==========================================
- Coverage   90.30%   89.48%   -0.82%     
==========================================
  Files         913      913              
  Lines      113586   113591       +5     
  Branches    26961    26962       +1     
==========================================
- Hits       102575   101650     -925     
- Misses       9682    10851    +1169     
+ Partials     1329     1090     -239     
Flag Coverage Δ
backend 94.09% <100.00%> (-1.48%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/selfhost/health.ts 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant