fix(viewer): restore a working auth configuration for UI and ingest - #112
Merged
Conversation
sklinglernv
commented
Aug 7, 2026
| the UI out entirely. | ||
| 3. ``Authorization: Bearer`` — remote programmatic clients (exporters). | ||
|
|
||
| An earlier revision made the token *replace* the loopback allowance, which |
Collaborator
Author
There was a problem hiding this comment.
remove historical comments (everywhere in this PR)
sklinglernv
force-pushed
the
fix/viewer-auth
branch
from
August 7, 2026 08:35
4789cb6 to
2993d7d
Compare
ba42b28 added `_require_viewer_authorization`, but made the token *replace* the loopback allowance rather than supplement it. That left no configuration in which both the browser UI and remote ingest worked: - token unset -> remote clients get 403 - token set -> loopback bypass is skipped, and since the SPA never sends an Authorization header, the UI gets 401 from everywhere, localhost included Make the check an OR over three independent credentials: loopback origin, session cookie, or bearer token. Local agents keep ingesting with no config; remote exporters keep using the bearer token. For the browser, add the bootstrap it was missing: `?token=` is exchanged for an HttpOnly SameSite=Lax cookie and stripped from the URL by redirect, so only the one-time share link carries the secret and trace deep links shared later do not. SameSite=Lax is deliberate — it still rides top-level GET navigations, so a shared link works on click, while withholding the cookie from cross-site POSTs. `start-dev` prints that share link when a token is configured. The token is percent-encoded: a '#' in it would otherwise start the URL fragment, so the browser would send only the part before it and the bootstrap would fail with a confusing 401. NOOA_VIEWER_PUBLIC_HOST overrides the printed hostname, since gethostname()/getfqdn()/AI_CANONNAME all return the short name when the qualified one comes from a DNS search domain, and a short name generally does not resolve from a colleague's machine. Also surface auth failures in the UI. Pages caught the error and rendered their empty state, so an unauthorized viewer looked exactly like a viewer with no data. `assertOk` tags 401/403 distinctly and a banner explains the fix. Adds an opt-in Host allowlist (NOOA_VIEWER_ALLOWED_HOSTS) for DNS rebinding, which is reachable via the loopback allowance. It is off by default: a version that derived the allowlist from gethostname()/getaddrinfo() rejected real traffic, 400-ing every span POST from a client using the DNS-qualified name. Note for review: this deletes test_api_routes_require_configured_bearer_token, which asserted that loopback gets 401 once a token is set. That behaviour is the bug being fixed; the replacement asserts the opposite, alongside eight other cases covering the cookie bootstrap and the Host check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Severin Klingler <sklingler@nvidia.com>
sklinglernv
force-pushed
the
fix/viewer-auth
branch
from
August 7, 2026 15:17
2993d7d to
5293d9a
Compare
rdasilveiracabral
self-requested a review
August 14, 2026 17:04
rdasilveiracabral
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ba42b28added_require_viewer_authorization, but the token replaces the loopback allowance instead of supplementing it. There is currently no configuration in which both the browser UI and remote ingest work:The second row is the surprising one: setting a token skips the loopback branch entirely, and the SPA never sends an
Authorizationheader — there is no code in the frontend that could — so the UI locks itself out even from localhost.Change
Auth is now an OR over three independent credentials (
main.py): loopback origin, session cookie, orAuthorization: Bearer. Local agents keep ingesting with zero configuration; remote exporters keep using the bearer token.Browser bootstrap.
?token=is exchanged for anHttpOnly; SameSite=Laxcookie and stripped from the URL via redirect, so only the one-time share link carries the secret — trace deep links shared afterwards are plain URLs.Laxis deliberate: it still rides top-level GET navigations, so a shared link works on click, while withholding the cookie from cross-site POSTs.start-devprints the share link when a token is set. The token is percent-encoded — a#in it would otherwise begin the URL fragment, so the browser sends only the part before it and the bootstrap fails with a confusing 401.NOOA_VIEWER_PUBLIC_HOSToverrides the printed hostname, becausegethostname()/getfqdn()/AI_CANONNAMEall return the short name when the qualified one comes from a DNS search domain.The UI now says when it is unauthorized. Pages caught the error and rendered their empty state, so an unauthorized viewer looked identical to one with no data.
assertOktags 401/403 distinctly and a banner explains the fix.DNS-rebinding guard, on by default
Making loopback always sufficient reopens a hole that
ba42b28had incidentally closed: a malicious page can publish a short-TTL record, then re-answer DNS with127.0.0.1. The browser connects here still believing the origin is the attacker's, so same-origin policy lets their script read the response — and the request arrives from loopback, needing no credential. Binding to localhost does not help; loopback is the target. The payoff is every prompt and completion, plusPOST /api/playground/inferenceon your server-side credentials.The guard rejects browser requests whose
Hostis not one of ours. Two design points, both learned the hard way:Sec-Fetch-*orOrigin. Rebinding is inherently a browser attack, so exempting programmatic clients costs no security and makes it structurally impossible for this check to reject a span export. An earlier revision applied it to every request and silently broke trace ingest.localhost, any IP literal (rebinding needs a name whose DNS the attacker controls),gethostname(), and<gethostname()>.<anything>. That last rule matters —gethostname()returns only the leading label when the qualified name comes from a DNS search domain, so exact comparison rejected real traffic.NOOA_VIEWER_ALLOWED_HOSTSadds CNAMEs/proxies;*disables.The effective policy is logged at startup so a rejection is diagnosable from the log alone.
Residual gaps, stated plainly: browsers older than Chrome 76 / Firefox 90 / Safari 16.4 do not send
Sec-Fetch-*and would skip the check, and header presence is a heuristic for "is a browser", not a guarantee. This is defense in depth, not a proof.For the reviewer
This deletes
test_api_routes_require_configured_bearer_token, which asserted loopback gets 401 once a token is set. That behaviour is the bug; the replacement asserts the opposite. Fifteen tests cover the new contract.test_cors_rejects_unconfigured_originnow pinsHost: localhost— otherwise the rebinding guard would reject it first and the assertion would hold for the wrong reason, silently no longer testing CORS.Verification
End-to-end against a live viewer, from a non-loopback address over the DNS-qualified hostname:
And the rebinding guard, authenticated so only the
Hostdiffers:A quickstart agent run over the external route landed its spans. 347 tests pass; committed
dist/reproduces byte-identically from a cleannpm cibuild.