feat(authn): require credentials and bind queries to their subject - #14
Merged
Conversation
ekalinin
force-pushed
the
feat/transport-auth
branch
from
August 17, 2026 19:26
102c3a3 to
3bedb44
Compare
ekalinin
force-pushed
the
fix/transport-contract
branch
from
August 17, 2026 19:26
120ff95 to
3f449ca
Compare
There was no authentication on any transport and no securityScheme in the OpenAPI document. Anyone who could reach the port could run arbitrary SQL under the service credentials of every configured database, stop other people's queries, read their results and reload the process. internal/authn holds static bearer tokens loaded from auth.tokens, with the value normally taken from an environment variable so it never has to sit in a ConfigMap. Comparison is constant time across all tokens with no early exit. Scopes are read (status, stats, download, list, watch), write (submit, stop) and admin (reload, can-stop); admin implies the others. The package sits outside internal/transport because the core stamps a query with the subject that submitted it and must not import a transport. REST gates every /v1 route on its scope, the Connect handler gets an interceptor that covers unary and streaming calls alike, and an unmapped RPC defaults to admin so a new method is never accidentally public. The health probes stay open; a configured auth section that resolves to no usable token is a startup failure rather than a silent pass-through. Knowing a query ID was also enough to read anyone's SQL, status, stats and result. QueryRecord now carries the submitting subject, and status, stats, download, stop and watch check it. Admin acts across subjects; a record written before subject binding has no owner and is admin-only. A foreign query answers 404, not 403, so the API does not confirm that an ID exists. Finally, /metrics and /v1/admin/* move to their own listener when server.admin_addr is set: the metric labels enumerate every configured db_id and the admin routes reload the process, so neither belongs on the public port.
ekalinin
force-pushed
the
feat/transport-auth
branch
from
August 18, 2026 09:47
3bedb44 to
cb77494
Compare
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
There was no authentication middleware on any transport and no
securitySchemein the OpenAPI document.POST /v1/admin/reloadandGET /v1/queries/{id}/resultwere open along with everything else. In practice that is unauthenticated execution of arbitrary SQL under the service credentials of every configured database, plus the ability to stop other people's queries, read their results and reload the process.Separately: knowing a
query_idwas enough to read someone else's SQL, status, stats and result.Mechanism
Static bearer tokens from
auth.tokens. The value is normally read from an environment variable (value_env), so it never has to live in the config file - in Kubernetes, in a ConfigMap. Comparison is constant time across all tokens with no early exit, so neither timing nor control flow reveals which token was close.Scopes:
read(status, stats, download, list, watch),write(submit, stop),admin(reload, can-stop,/metrics).writeimpliesread- a token that may submit a query has to be able to poll it and fetch its result, or onlymode: syncworks - andadminimplies both.internal/authnholds only the identity, the scopes and the token comparison, so the core can stamp the submitting subject onto a record without linking a transport. The chi middleware lives ininternal/transport/restand the Connect interceptor ininternal/transport/grpcconnect;go list -deps ./internal/core/manager | grep connectrpcis empty.Wiring
The
/v1subtree denies by default: the scope middleware is mounted withr.Useon the subtree rather thanr.Withper route, so a route added later cannot come out unauthenticated by omission. That matters more than the 401 it saves - a route without a gate would also read the records of every subject, because a missing identity is what "authentication is disabled" looks like from the inside. Groups raise the bar towritewhere a route needs it./metricsneedsadminwherever it is mounted: its labels enumerate every configureddb_idand the traffic volume per database, andadmin_addris optional, so the default deployment has it on the public listener. The shipped Prometheus config carries a token.Connect gets an interceptor covering both unary and streaming calls -
DownloadResultandWatchQueryare server streams that go through a different hook and would otherwise stay open. An unmapped procedure defaults toadmin, so a newly added RPC is never public by oversight.A browser cannot set the
Authorizationheader on a WebSocket handshake, which would have made/v1/wsunreachable for exactly the clients the Origin check exists for. The credential may also be offered as the subprotocol pair["dbbridge.bearer", token]; the server selects and echoes only the marker, never the token.401 and 403 go through the same JSON envelope as every other error, with the request ID, instead of
http.Errortext, and both are declared in OpenAPI. The admin router shares the public router's middleware, so admin errors carry a request ID, admin requests are logged and a panic there comes back as a 500./healthzand/readyzstay open. Anauthsection that resolves to no usable token is a startup failure: coming up with the API open while the operator believes it is protected is worse than not coming up at all.authis reported underreport.ignoredby a reload rather than silently kept, because theAuthenticatoris built once - so revoking a leaked token takes a restart, and the reload no longer claims success while the token keeps working.Binding queries to their subject
QueryRecord.Subjectis filled from the authentication context and checked in status, stats, download, stop and watch.adminsees everything. A foreign query answers 404 rather than 403, so the API does not confirm that an ID exists. Records written before subject binding have no subject and are reachable only withadmin- so turn authentication on while the instance is idle, or expect queries submitted before the switch to answer 404 to their owners untilresult_ttlexpires.The idempotency key is namespaced by the subject that chose it.
StartQueryis the one read path that does not go through the authorization check, and the key was global per database, so a caller who sent somebody else's key got their whole record back - SQL text, stats, owner and result locator - while its own SQL was never run, and could hold the key for its full TTL. I3 only has to hold inside a subject.When credentials are configured, a request that reaches the service without an identity is denied rather than treated as "authentication is off": that is the failure mode a forgotten gate would otherwise open.
Admin isolation
server.admin_addrmoves/metricsand/v1/admin/*to their own listener: the metric labels enumerate every configureddb_idand the admin routes reload the process, so neither belongs on the public port. That is network isolation, not authorization - theadminscope is still required there.Tests
Unit tests for
authn: rejection of every unusable configuration (no tokens, no value, no subject, no scopes, unknown scope, empty environment variable, duplicate value), reading a value from the environment, header parsing, the scope implications, and theAuthorizeSubjectmatrix. Procedure mapping moved togrpcconnectwith the interceptor.Transport tests: 401 without a token and with a bad one, 403 with an insufficient scope, 200 with the right one, open probes, a stream without a token; the owner reads its own query while another subject gets 404 and
admingets 200, over REST and over Connect; the same key from two subjects returns two different queries and each caller's own SQL;/metricsanswers 401/403/200 by scope; the separate admin listener still requiresadmin; a WebSocket handshake with and without a subprotocol credential; the 401 envelope carriesrequest_idandVary: Authorization; andconfigrejects an emptyauth.tokens.Verification
go test -race ./...,golangci-lint run ./...- clean. OpenAPI gainssecuritySchemes.bearerAuth, reusable 401/403 responses on every authenticated operation, and a globalsecuritywith the probes exempted.