fix(proxy): correct the databases-route claim and pin non-2xx passthrough - #90
Merged
Merged
Conversation
…ough The proxy's member allowlist carries a note that each entry must name a route Arc serves without admin auth, because the proxy injects the instance's admin token and this list is therefore the only gate. It said "GET databases (handleList, no admin)". Arc's three database listing routes now require read permission and, where the server restricts reads per database, a grant for the database being listed, so that claim is stale. The entry itself stays correct — readAuth is not adminAuth, so the routes remain member-reachable — but what a member gets back changes, and the note now says so: the proxy forwards the instance's stored token, so an instance with no stored token answers 401 where it used to answer 200, and a token Arc restricts to particular databases gets 403 on the list-everything route rather than a filtered list. Added tests pinning that the proxy forwards an upstream non-2xx status and body verbatim rather than flattening it: 401, both 403 wordings (no read permission, and the per-database denial for a named database and for the list-everything route), 404 and 200 all arrive unchanged, so a caller can tell "re-authenticate" from "you are scoped, name a database" from "no such database". A 502 stays reserved for the proxy's own failures — turning an upstream 403 into one would report a scoped token as a Launchpad fault. Also pinned that a 403 is not retried against the next resolved IP: a refusal is a decision, not a transport failure. README: the instance token has to be an admin token, and Arc answers with that token's reach.
5 tasks done
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.
Summary
Arc's three database listing routes (
GET /api/v1/databases,/:name,/:name/measurements) previously carried no middleware at all. They now require read permission and, where the server restricts reads per database, a grant for the database being listed.Launchpad's exposure is narrow and worth stating plainly, because it is smaller than it first looks:
SHOW DATABASES/SHOW TABLESonPOST /api/v1/query(arcClient.getDatabases), andcreateDatabaseis a POST, which was already admin-gated. So there is no broken picker to fix here, and I have not invented one.That leaves two real things, both done here.
1. The allowlist note was about to become wrong
MEMBER_READ_PREFIXEScarries an important note — each entry must name a route Arc serves without admin auth, because the proxy injects the admin token and this list is the only gate. It saidGET databases (handleList, no admin). That is no longer what Arc does.The entry itself stays correct (
readAuthis notadminAuth, so the routes remain member-reachable), but what a member gets back changes, and the note now says so rather than quietly describing a server that no longer exists.2. Non-2xx passthrough is now pinned
The proxy already forwards the upstream status and body verbatim — that is what lets a caller tell the new answers apart. It was untested, so it is now pinned: 401, both 403 wordings (no read permission, and the per-database denial for a named database and for the list-everything route), 404 and 200 all arrive unchanged.
A 502 stays reserved for the proxy's own failures (unreachable instance, oversized response). Turning an upstream 403 into one would report a scoped token as a Launchpad fault. Also pinned: a 403 is not retried against the next resolved IP — a refusal is a decision, not a transport failure, and retrying only doubles the load and the audit entries.
Companion to the Arc server change on
fix/rbac-measurement-where-and-databases-auth; see that branch's release notes under "RBAC read and write restrictions were unreachable, and the database listings had no authorization".Test plan
npm test— 237 passed (6 files), 8 of them newnpm run check— 0 errors, 10 warnings (unchanged frommain)non-2xx passthroughblock insrc/lib/server/arcProxy.test.ts, using the file's existing real-http.createServerupstream rather than a mock, as every other test there does