Design 16: subscription shares, public distribution, and snapshot fallback - #29
Merged
Merged
Conversation
…ite path A share is read on every public subscription fetch and written whenever one is created or rotated, which is the access pattern the record-level hot store exists for. Leaving it in the JSON state would put a hot read behind a file that is rewritten in full and fsynced on every unrelated write. The token joins the credentials crypto.go already seals -- it is a bearer credential for an unauthenticated public URL, the same class as the proxy-user subscription token sitting next to it. The slug is deliberately left in the clear: it already appears in reverse-proxy access logs, so sealing it would imply a secrecy it does not have. Lookup by token compares whole strings. A test asserts that neither a prefix nor a superstring resolves, because a partial match would turn a half-guessed token into a working one. Tested: go test -race -cover ./internal/store (61.4%); gofmt; go vet
… form The slug is a label, not a secret: it reaches reverse-proxy access logs and client screenshots, so the parser validates its shape but never treats a correct slug as evidence of anything. Authorization rests entirely on the token. The old single-segment form is deleted rather than kept because nothing was subscribed to it, and two shapes would mean a permanent branch here with two sets of rules to keep in agreement.
The header is caller-controlled. Keying a cache on it directly turns the cache into a memory amplifier anyone can drive; the family is all the conversion depends on anyway. The class list is data rather than control flow so the boundedness is checkable instead of re-derived from the function body.
A client poll should not boot a JavaScript VM and parse a 1.24 MB engine every time. Put ignores empty bodies: the endpoint refuses to serve one, so letting it into the cache would create a path back to the exact response that makes a client delete every node it had. InvalidateShare is beyond what the plan listed, and it is here because rotation is otherwise a lie: without it a rotated-away URL keeps being served from cache until the TTL expires, which is precisely the window rotation exists to close. Tested: go test -race ./internal/server -run 'TestSubscriptionCache|TestClassifyClientUA' (8/8)
It reads as one sentence: this plugin may produce the body of a subscription the core serves. It grants no route, no port, no listener, no response header and no access to the share's token. It is deliberately absent from hostRiskExemptForNonSystem because what the plugin returns is what a proxy client consumes, so unlike guarded egress there is no broker check that makes handing it to a sandboxed third party safe. A general http:serve capability was rejected for handing token checking and rate limiting to plugin code.
Unknown token, mismatched slug, disabled and expired all return the same nothing, and the caller turns all of them into the same 404. A response that could distinguish them would tell someone probing the endpoint which of its guesses was a real token, which is the one fact the token exists to keep. The expiry boundary is exclusive and has its own test, because off-by-one there means a share stays live for one more request than the operator asked for.
A format designed after the fact gets shaped by whatever the implementation happened to store. Records are sorted by id so two exports of the same data are byte-identical, because an export that depended on map iteration order would diff against itself and be useless for backup comparison or review. Import refuses an unknown or missing envelope format rather than guessing, so a file from another tool cannot be half-read into a state nobody intended.
Reuses proxy:admin rather than minting a scope. A subscription share is proxy subscription distribution, which is exactly what that scope already means, and a new name would have to be claimed in the shared-resource ledger and then defended against drifting into a second meaning. The share URL stays readable in the dashboard on purpose: it is copied out repeatedly, so one-time display would trade a real workflow for protection the at-rest sealing already provides. Rotation and deletion both invalidate the cached body. Without that they are lies: the cache is consulted before the token is, so a stale entry keeps serving exactly the URL the operator just retired. Rotation audits both token hashes and a test asserts no raw token appears in any metadata value. Duplicate slugs are refused at creation. Lookup is by token so a collision is not a security problem, but two shares sharing a URL's readable half is ambiguous to the person reading it. Tested: go test -race ./internal/server -run 'TestRotateShare|TestNewShareToken|TestValidateShareSource|TestRotateInvalidates' (12/12)
The core holds each subscription's last good content because the plugin has nowhere durable to keep it: it cannot reach bolt, and its runtime working directory is deleted when the runner stops it. The plugin fetches on request and hands the bytes back; the core stores them as an opaque blob and passes them in again on the next render. Refresh is lazy rather than scheduled. A fork-per-call plugin cannot schedule itself, so a timer would have to be a new core mechanism, and clients already poll on their own cadence -- laziness produces the same freshness for the requests that actually exist. Upstream syncs on a timer; this deliberately does not, and only the moment of the fetch differs. The failure behaviour is the reason the record exists: when a refresh fails and a snapshot is available, the snapshot is served and the failure is recorded on it, so the operator can see it is stale while clients keep working. With nothing to fall back on the call errors, which is what stops the caller from serving an empty body. Snapshots stay out of the JSON state file: they are provider payloads, possibly megabytes each and rewritten on every refresh, so keeping them there would make every unrelated write pay for them. They are not sealed either -- they are public content the provider already served over the network. The provider's traffic header travels with the cached body rather than in a parallel map, so a cache hit cannot pair one subscription's nodes with another's remaining-quota figures. Tested: go test -race ./internal/server -run 'TestSnapshot|TestSubscriptionCache|TestShare' green; 6 snapshot tests, 6 cache tests
Deleting the single-segment /sub/<token> route left proxySubscriptionURL still building that shape, so the rotate endpoint answered with a link that 404s. The full test suite found it; a filtered run had not. The URL now comes from a share, which is the only thing that publishes a user. That exposes a consequence worth stating in the response rather than hiding: rotating a user's sub token does not rotate public access, because the share holds the public credential. A user nobody has published gets an empty string instead of a plausible-looking address, and rotates_public_access says plainly that this operation is not the one that changes what clients can reach. Two existing expectations changed with it, both recorded inline where they live: an unapplied profile and a disabled user used to answer 200 with an empty body, and now answer non-2xx. A proxy client that receives an empty success deletes every node it had, so "the profile is not applied yet" must not arrive at the client as "you have no nodes, forget them all". The duplicate-sub-token fail-closed case is gone rather than unguarded: a share resolves by its own token, so a duplicate among user tokens can no longer make one user's URL serve another's nodes. The test now asserts the property that replaced it. The fetch audit also gains share_id, so a served subscription can be traced to a share rather than only to a token hash. Tested: go test -race -count=1 ./internal/server -timeout 40m EXIT=0 (502s)
A response that distinguishes "valid token, nothing to serve" from "no such token" has told a prober the one fact the token exists to keep. The previous implementation leaked that five ways: a JSON error body with a request id, a 429 from the rate limiter, a 405 on the wrong method, a 400 on a bad format, and a 502 on an empty render. The 400 was the sharpest. Format was validated after the token had already been resolved, so a valid token with a bad format answered differently from an invalid one -- a direct oracle for token validity. Format is now checked before resolution, so the ordering cannot leak. Every rejection returns one response with no fingerprint: no error body, no request-id header, no content type unless configured. It does not go through writeError, which exists to name errors and identify requests. The rate limiter refuses in the same voice: the limit still applies, it just stops announcing that this path is specially limited. A test asserts ten different rejection paths produce byte-identical status, body and header sets. Truth is relocated rather than discarded. Every rejection is still audited with its real reason and share id, so the operator diagnoses exactly what a prober cannot learn. Two things are deliberately absent. The server does not proxy to a decoy site: that disguise belongs at the reverse proxy, which already terminates TLS and serves the origin, and doing it here would make an unauthenticated endpoint into a request-forwarding surface while still differing in headers and timing. And timing remains distinguishable -- a valid token forks a plugin while a rejection returns immediately -- because equalising it means delaying every rejection by the worst-case render time for an imperfect result. Four existing tests asserted the distinguishable codes and were updated with the reason inline. Tested: go vet ./... clean; go test -race -cover -count=1 -timeout 45m ./... EXIT=0; internal/server 417s / 69.8%
Contributor
Author
|
Landed on Closing per rules/01 §8.5 (landed via integration rather than a PR merge). Sequencing note for whoever picks up the plugin side: this must ship as a server release before That is the TASK-0006 gate working as designed — a signed manifest must be acceptable to a released server — so the order is forced: release the server, move |
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
1 participant
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.
Core half of design-16 sub-projects 1 and 2.
What the core owns
Route, token lookup, slug comparison, rate limit, audit, Content-Type,
Subscription-Userinfoand the output cache. A source only ever produces bytes — it cannot see the token, set a header, or choose a status code.subscription:serveis a deliberately narrow new capability: this plugin may produce the body of a subscription the core serves. A generalhttp:servewas rejected for handing token checking and rate limiting to plugin code.Two rules that exist because their opposite is the tempting default
Behaviour changes needing acknowledgement
/sub/<token>is removed; the only shape is/sub/<slug>/<token>. Affordable because the deployment has zero proxy users, profiles and inbounds.rotates_public_access: falserather than a URL the rotation does not control — the previous code was still building the deleted single-segment shape, so it answered with a dead link.Verification
gofmtclean ·go vet ./...clean ·go test -race -cover -count=1 ./... -timeout 45m.Note for reviewers: an earlier run of mine was filtered through
| headand hid a failing package. The authoritative runs here are-count=1with full output captured, and theinternal/serverpackage needs an explicit-timeouton darwin because it exceeds the 10-minute default.