From 82f1e03524b4db8c84e1059098be8ccdd611ed19 Mon Sep 17 00:00:00 2001 From: vthwang Date: Fri, 31 Jul 2026 12:56:26 -0700 Subject: [PATCH 01/10] docs: design for connecting a vta_only to another in-farm stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lets a vta_only session point at any full_stack this farm provisioned instead of only the platform stack. The owner mints a share code and hands out a connection bundle; the consumer pastes it on create. Specification only — no code yet. Decisions that shape it: - In-farm only. step_dids_grant_farm already enrols vtafarm-api as an admin in every full_stack's daemon ACL, so those stacks are valid targets for free. A stack we did not provision would 401 on the first DID upload, so it stays out of scope. - The bundle is evidence, not configuration. The three values a session is built from are read from the provider's own row; the pasted text only names it and proves authorisation. Nothing a user types becomes a URL this server connects to, which is what keeps SSRF and the self-asserted-audience relay unreachable. - No revoke and no delete guard. A provider teardown destroys nothing of the consumer's, so there is no data loss for a hard gate to protect — only a surprise, which a confirmation dialog handles. The two had to go together: a 409 without revoke pins a stack forever. - Mediation is open to every DID, so the share code is the only gate. - A validate route, because a confirmation card rendered from the pasted JSON would show a green tick for a bundle whose code is garbage. Signed-off-by: vthwang --- docs/custom-stack-connection-design.md | 797 +++++++++++++++++++++++++ 1 file changed, 797 insertions(+) create mode 100644 docs/custom-stack-connection-design.md diff --git a/docs/custom-stack-connection-design.md b/docs/custom-stack-connection-design.md new file mode 100644 index 0000000..8cdd1e6 --- /dev/null +++ b/docs/custom-stack-connection-design.md @@ -0,0 +1,797 @@ +# Custom Stack Connection — Design + +Lets a **`vta_only`** session connect to a `full_stack` other than the platform +stack — provided that stack is **one this farm provisioned**. + +Two halves, useless apart: + +- **Share** — a `full_stack` owner mints a connection bundle for their stack and + hands it to somebody out of band — §4. +- **Connect** — the `vta_only` create form grows a **Customize** path that takes + that bundle instead of silently using the platform stack — §5. + +> **Status: specification. Nothing is built.** §12 tracks it. +> §9 is the section to read first — it is where this design can go wrong. + +Companion: [`vtafarm/docs/custom-stack-connection-frontend.md`](../../vtafarm/docs/custom-stack-connection-frontend.md). + +Prerequisite reading: [`vta-setup-design.md`](vta-setup-design.md) §"DID hosting +credentials". + +--- + +## 1. Scope + +**Stacks outside this farm are not supported.** Not deferred behind a flag, not +half-built — the API has no code path for them and the bundle format does not +pretend otherwise. §11.1 records what it would take. + +| In scope | Out of scope | +| --- | --- | +| `vta_only` → any `full_stack` **row in this farm's database** | any daemon this farm did not provision — §11.1 | +| A share code, minted and rotated by the stack's owner | a directory of stacks accepting connections | +| Warning a provider what they are about to break, and telling the consumer afterwards | revoking a single connection, or blocking a delete — §7.4 | +| — | migrating a live `vta_only` between stacks — §11.3 | +| `vta_only` on the managed zone | `vta_only` on a custom domain (still excluded, `custom-domain-design.md` §18) | +| `full_stack` as provider | `vta_only` as provider — it deploys neither mediator nor daemon | + +--- + +## 2. What a `vta_only` is wired to, and who may touch it + +Three values, three columns that already exist (migration 000023). **The schema +does not need to change to hold a non-platform target** — which is the whole +reason this feature is small. + +| Value | Column | Where it lands | +| --- | --- | --- | +| Mediator DID | `mediator_did` | `[messaging] did` in the VTA's `config.toml` (`templates.go`) | +| DID resolution URL | `did_hosting_server_url` | `vta_did_url = /-vta` → `[vta_did] url` | +| DID control URL | `did_hosting_control_url` | every `didhosting.Factory.For(...)` call | + +`did_hosting_control_url` is the one that carries authority: it is not data we +render into a file, it is a URL **this server makes authenticated requests to**, +signing them with vtafarm-api's own private key. + +| Call | When | ACL role on the target | +| --- | --- | --- | +| `New()` → `GET /api/server-info` | first use of a URL | none | +| `RegisterDid(path, didLog)` | `runSetup`, after `vta setup` | **admin** | +| `CreateAcl(vtaDid, "service", …)` | `runProvision` | **admin** | +| `ServerDid()` → `did-mgmt servers add --id control` | `runProvision` | none (cached) | +| `DeleteDid` / `DeleteAcl` | teardown | **admin** | + +So connecting to a stack means **vtafarm-api must hold an admin ACL entry on +that stack's DID-hosting daemon**. + +### 2.1 Why that is already true — and why it is exactly the scope boundary + +`step_dids_grant_farm` (`orchestrator_fullstack.go:299`) runs +`did-hosting-daemon add-acl --did --role admin --label vtafarm` +as an offline Job on the dids PVC, for **every** `full_stack` session — not just +the platform stack. `vta-setup-design.md` justifies it as "the farm operates +these deployments and manages the `did.jsonl` documents they serve". + +That was written about operating customers' stacks. It also, unplanned, makes +every customer `full_stack` a **valid target at zero cost**: one keypair, +vtafarm-api's own, authenticates against all of them. + +The set of stacks we hold admin on is exactly the set of `full_stack` rows in +our database. §1's boundary is not a policy choice layered on top of the +design — it *is* the design. A stack we did not provision would 401 on the +first upload, and §9.1 explains why that failure is currently silent. + +--- + +## 3. The one rule that makes this safe + +> **The bundle is evidence, not configuration.** + +The three values a consumer session is built from are read from the **provider's +row in our own database**. The pasted bundle is used only to (a) name which row, +(b) prove the sharer authorised it, and (c) show the recipient what they are +joining before they commit. + +Nothing the user types ever becomes a URL this server connects to. + +Everything downstream follows from that sentence: + +- **No SSRF.** `did_hosting_control_url` still only ever comes out of our own + DB, exactly as today. A pasted `http://169.254.169.254/` reaches no socket; it + fails a string comparison. +- **No credential relay.** The `aud` of every id_token we sign still comes from + a daemon we provisioned. The self-asserted-audience hole (§9.4) stays closed + by construction rather than by a check we have to remember. +- **No stale-value class of bug.** A rebuilt stack is a *different row* with a + different share code, so an old bundle fails to resolve instead of + provisioning against a daemon that no longer exists. + +A design that instead took the URLs from the bundle would need every mitigation +in §9.4 to be correct forever. This one needs none of them. + +--- + +## 4. Share: the connection bundle + +### 4.1 The share code is the grant + +One nullable column on the provider's row: + +```sql +share_code TEXT NULL -- NULL = this stack is not shared +``` + +| Action | Effect | +| --- | --- | +| enable sharing | mint a random code | +| rotate | mint a new one — outstanding bundles stop working, live connections keep running | +| disable | `NULL` — no new connections; live ones keep running | + +One column rather than a `shared_use_enabled` boolean *plus* a code, because the +two would always have to agree and the code alone already answers both +questions. It is a capability, not a password: stored in plaintext, displayed to +its owner, never hashed — there is nothing to protect it *from* that rotation +does not handle better. + +**Rotation is the reason this is not just a public toggle.** A bare +"anyone may connect" flag would let anybody enumerate stack names and attach; a +code means the owner chose each recipient, and can un-choose all of them in one +click without touching anyone already connected. + +### 4.1.1 Format + +16 characters of Crockford base32, grouped in fours — +`K7M2-9XQP-4B8W-3NRT`. 75 bits of entropy plus a **check symbol** as the last +character (Crockford's own `~*$=U` scheme). + +Crockford rather than raw base32 because §2.2 offers this code to be read aloud, +and a format meant for oral transmission without a checksum is half-designed. +Three properties, all of which the paste side depends on: + +- **Normalise before comparing.** Strip dashes and whitespace, uppercase, and + fold the ambiguous glyphs — `I`, `L` → `1`, `O` → `0`. `k7m2-9xqp…`, + `K7M2 9XQP…` and `K7MZ…`-with-a-typed-`l` all reach the same comparison. +- **The check symbol catches a typo locally**, before any request. That makes + "you mistyped this" a different, instant answer from "this code is wrong" + (§5.2) — different problems needing different actions from the user. +- **Compare in constant time** after normalising. It is a credential. + +The check symbol is not security; an attacker computes it as easily as we do. +It exists so that the overwhelmingly common failure — a hand-copied character — +is diagnosed as itself. + +The **platform stack has no code and needs none.** It is reached by the default +path, which sends no bundle at all. That also means nobody can accidentally +paste their way onto it through the Customize form — the two paths stay +distinct. + +### 4.2 What the bundle contains + +```json +{ + "v": 1, + "kind": "vtafarm.stack-connection", + "farm": "firstperson.dev", + "stack": "alice", + "code": "k7m2-9xqp-4b8w-3nrt", + "mediator_did": "did:webvh:…:mediator-alice.firstperson.dev", + "did_hosting_server_url": "https://dids-alice.firstperson.dev", + "did_hosting_did": "did:webvh:…:dids-alice.firstperson.dev" +} +``` + +Two fields do the work; the rest is for humans and for §5.2's confirmation card. + +| Field | Role | +| --- | --- | +| `stack` | the provider's `vta_name` — its public identifier everywhere else in this API (`CLAUDE.md`, "a session is addressed by its name"), so the lookup key is the one users already see | +| `code` | the grant (§4.1) | +| `farm` | lets the paste side say "this bundle isn't from this farm" instead of "stack not found" | +| `v` / `kind` | so pasting a DID, a URL or a JWT fails with a sentence rather than a 500 five steps later | +| `mediator_did`, `did_hosting_server_url`, `did_hosting_did` | **displayed and compared, never used** (§3). Compared so a bundle showing values the stack no longer has is refused rather than silently connecting to different ones than the recipient was shown | + +Every value already exists on the provider's row and is already returned by +`GET /setup/:id` (`collected.mediator_did`, `collected.did_hosting_did`, +`urls.dids`). **The bundle invents no data** — only the code is new, and it is +minted by an explicit act. + +### 4.3 Where it is offered + +| Surface | Route | Who | +| --- | --- | --- | +| Own `full_stack` detail | `GET /api/v1/setup/:id` → `connection` object | the owner | +| Admin session detail | `GET /api/v1/admin/setup-sessions/:id` | admins, for support | + +Populated only once `status = 'running'`, `mediator_did <> ''` and +`did_hosting_did <> ''` — the same readiness rule `resolveSharedInfra` already +applies to the platform stack. Absent before that, so the UI cannot offer a +bundle that would fail on arrival. + +--- + +## 5. Connect: the Customize path + +`POST /api/v1/setup` grows one optional field: + +```jsonc +{ + "mode": "vta_only", + "vta_name": "myvta", + "vta_image": "ghcr.io/…", + "connection": { /* the §4.2 bundle, verbatim */ } // optional +} +``` + +Absent → today's behaviour exactly. Present → the provider is the named stack +instead of the platform one. + +The field is an **object, not a string to parse**. The frontend owns "the user +pasted something"; the backend owns "is this stack usable". That split is what +lets the confirmation card (§5.2) exist at all. + +### 5.1 `resolveProvider` — one function, two entry points + +Rather than bolting a second path beside `resolveSharedInfra`, both collapse +into one lookup that differs only in which row it finds: + +```go +// ref == nil → the platform stack (today's default, semantics unchanged) +// ref != nil → the stack the bundle names +func (h *SetupHandler) resolveProvider(ref *stackRef) (v sharedInfra, provider *model.SetupSession, reason, detail string) +``` + +This *reduces* the number of code paths that can wire a session to a mediator, +rather than adding one. The platform branch keeps its exact current reasons +(`platform_stack_missing` / `platform_stack_not_ready` / +`shared_infra_unconfigured`) and its exact fail-open behaviour on a DB error, so +nothing about the existing path changes observably. + +The bundle branch, in two tiers. **Everything before the code is verified must +answer identically**, or the endpoint becomes a directory of which stacks exist +and which are shared — precisely what §11.2 says there will not be. + +**Tier 1 — is this bundle usable at all.** One reason for every outcome: + +| Check | | +| --- | --- | +| `kind` / `v` recognised, fields present, `code` well-formed (§4.1.1) | 400 `bad_bundle` | +| `farm` matches `CLUSTER_DOMAIN` | 422 `wrong_farm` | +| `SELECT … WHERE vta_name = ? AND mode = 'full_stack'` **and** `share_code` non-NULL **and** constant-time equal | **403 `invalid_bundle`** | + +No such stack, a stack that never shared, a stack that turned sharing off, a +rotated code, and a hand-mangled code all produce the same 403. They are the +same fact from the holder's point of view — *this bundle does not currently open +anything* — and the only honest next step for all five is the same one: ask the +owner for a current bundle. + +`bad_bundle` and `wrong_farm` stay distinct because neither requires knowing +anything about our data to determine. + +**Tier 2 — the caller holds a valid code**, so specificity costs nothing: + +| Check | Failure | +| --- | --- | +| `status = 'running'`, `mediator_did <> ''`, `did_hosting_did <> ''` | 409 `stack_not_running` | +| bundle's three display values equal the row's | 422 `stack_changed` | +| connection count below the cap (§6.3) | 409 `stack_at_connection_limit` | + +Then, and only then, `sharedInfra` is built **from the provider row** — +`provider.MediatorDid`, `provider.DidsURL()`, `provider.DidsURL()` — and the +existing create path continues untouched. + +No check reaches the network. Reachability is proven a moment later by +`Factory.For()` fetching `/api/server-info` from a host we provisioned. + +### 5.2 The code has to be checkable before the form is filled in + +`POST /api/v1/setup/connection/validate` — user auth, rate-limited via the +existing `middleware.RateLimit`, runs §5.1 and creates nothing. + +| Outcome | Body | +| --- | --- | +| passes | `{"stack": "alice", "mediator_did": …, "did_hosting_server_url": …, "connections_used": 2, "connections_max": 10}` | +| fails | the same `reason` + `detail` `POST /setup` would have returned | + +This exists because of a flaw in the obvious design, and it is worth naming. +The bundle is JSON; a frontend can parse it and render "connecting to **alice**, +mediator `did:webvh:…`" the moment it is pasted. But every one of those values +came **out of the pasted text**. A confirmation card built that way is +self-asserted — it shows a green tick for a bundle whose code is pure garbage, +and the user discovers the truth only after naming their agent, picking an image +and pressing Create. + +The validate route makes the confirmation card render **values this server read +from its own database**, which is the only version of that card worth showing. +The check that matters — §5.1 tier 1 — happens at paste time, where a wrong code +costs one field to fix rather than a whole form. + +Three notes: + +- **It is an oracle, and that is fine.** 75 bits of entropy behind an + authenticated, rate-limited route is not brute-forceable, and tier 1's single + reason means the oracle answers exactly one question: *does the bundle I was + given work.* That is the question it exists to answer. +- **It is not authoritative.** `POST /setup` re-runs §5.1 in full. A stack can + stop running, rotate its code or fill up between the two calls, so validate is + a courtesy and create is the gate. Never skip the create-time check because + validate passed. +- **This reverses an earlier decision.** A "test this bundle" *button* was + rejected as a worse copy of create. That reasoning was wrong once the + confirmation card was in the design: the card is not a test the user opts into, + it is a claim the UI makes unprompted, and it must not be a claim the server + never checked. + +### 5.3 A bundle is one stack's worth of values + +Taking the mediator from stack A and the DID host from stack B is refused +implicitly: the bundle names one stack and every value comes from that row. +Splitting them is technically possible over public HTTPS, but the dids daemon +bakes `[identity] mediator_did` into its own recipe at provisioning time, so the +pair is meaningful. Keeping it atomic also keeps §7's dependency tracking to a +single nullable column instead of a join table. + +--- + +## 6. Schema + +```sql +-- migrations/000025_stack_connection.up.sql + +-- Provider side. NULL means "not shared" — one column rather than a boolean +-- plus a code, because the two would always have to agree and the code alone +-- answers both questions (design §4.1). +ALTER TABLE setup_sessions ADD COLUMN share_code TEXT NULL; + +-- Consumer side. Neither column is needed to RUN the session — the three +-- snapshotted values in mediator_did / did_hosting_*_url already do that. +-- They exist for the three things a snapshot cannot answer: finding dependents +-- cheaply (design §7), naming the provider in the UI, and letting support +-- answer "why is this agent dead" without correlating URLs by eye. +ALTER TABLE setup_sessions + ADD COLUMN connection_source TEXT NOT NULL DEFAULT 'platform' + CHECK (connection_source IN ('platform', 'in_farm')), + ADD COLUMN provider_session_id BIGINT NULL + REFERENCES setup_sessions(id) ON DELETE SET NULL; + +CREATE INDEX setup_sessions_provider_idx + ON setup_sessions (provider_session_id) + WHERE provider_session_id IS NOT NULL; +``` + +### 6.1 `ON DELETE SET NULL` is the whole orphan mechanism + +Not a fallback — **the** mechanism. §7 blocks nothing and writes nothing at +delete time; when a provider row goes, Postgres nulls every dependent's +`provider_session_id` in the same transaction, and + +```sql +connection_source = 'in_farm' AND provider_session_id IS NULL +``` + +is exactly and permanently "the stack this agent connected to is gone". The UI +reads it (§8), nothing has to have been running at the moment of deletion, and +there is no reconciler to drift. + +`RESTRICT` would mean §7.4's rejected design. A plain `NULL`-able column with no +FK would mean writing the orphan marker by hand in every delete path — user +delete, admin delete, cascade from a user deletion — and getting it wrong in the +one nobody tested. + +### 6.2 Why the CHECK admits only two values + +`'external'` is not reserved in the constraint. Adding it later is a one-line +`ALTER`, and leaving it out now means the schema states the same scope §1 does +instead of hinting at a path the code cannot take. + +### 6.3 A cap on connections per stack + +`MAX_STACK_CONNECTIONS`, default `10`, `0` = unlimited. + +Crude, and deliberately so. It is not a capacity model — §9.2 explains why there +isn't one — it is a bound on how much of somebody else's storage and message +volume a single share code can commit before a human notices. + +It carries more weight than it looks, because §7.4 leaves a provider **no way to +remove one connection**. Rotating the code stops new ones; the cap is what +limits how many arrived before they thought to. An admin can raise it globally; +a per-stack override is §11.4. + +### 6.4 No new session status + +An orphaned consumer stays `running`, because it is: the Deployment, Service and +Ingress are in the consumer's own namespace and nothing in a provider teardown +touches them. The pod serves; it just cannot resolve its own DID or reach a +mediator. + +A `disconnected` status was considered and dropped. It would have to be written +by whichever code path deleted the provider, which is exactly the synchronous +marking §6.1 removes — and it would claim the pod had stopped, which is a +different and larger lie than a `running` badge next to an explicit "the stack +this agent connected to no longer exists" (§8). + +So no change to `STATUS_META`, no change to the `SetupStatus` union, and no +`Orchestrator.Resume` query to re-check. + +--- + +## 7. Lifecycle + +Two operations, one of which needs no code at all. + +### 7.1 The consumer deletes itself + +Unchanged, and already correct. `teardownSession` calls `DeleteDid` / +`DeleteAcl` through `session.DidHostingControlURL` — the daemon the DID was +actually uploaded to — and treats failures as warnings. The "snapshotted, not +looked up" rule was written for platform-stack rebuilds; it is what makes a +third-party provider work without a line of new code. + +One change: those warnings stop being rare. A consumer whose provider is gone +fails both calls on every delete. They belong in the delete response as a +non-fatal note, not buried in `log.Printf` — the user is entitled to know their +DID may still be published somewhere. + +### 7.2 The provider deletes itself — allowed, with a warning + +Unchanged behaviour: the delete goes through. Its dependents keep running in +their own namespaces, degraded — the mediator and daemon are gone, so the VTA +can no longer resolve its own DID or route a message, but nothing about the +consumer's Deployment, Service, Ingress, PVC or Vault seed is touched. + +The only change is at the UI layer: because `GET /setup/:id` reports +`connections[]` (§8), the delete confirmation can name what it is about to +break. That is the whole mitigation, and it is a confirmation, not a gate. + +`ON DELETE SET NULL` (§6.1) marks the orphans as a side effect of the delete +itself. No dependent is written to, no status changes, no code runs. + +### 7.3 The consumer finds out on its next page load + +There is no notification channel (the one under discussion for uptime monitoring +is a different thing), so the signal is a query, not an event: +`connection_source = 'in_farm' AND provider_session_id IS NULL` (§6.1). The +agent's detail page renders it as "the stack this agent connected to no longer +exists"; the user deletes the agent when they are ready. + +Deleting an orphaned consumer works: `teardownSession` reaches for a daemon that +is gone, both calls fail, and both are warnings (§7.1). Everything in the +consumer's own namespace is cleaned up normally. + +### 7.4 Rejected: a delete guard, and per-connection revoke + +An earlier draft had `DELETE /setup/:id` answer **409** while connections +existed (the precedent being `DELETE /api/v1/domains/:id`), plus a +`DELETE /setup/:id/connections/:name` for the provider to eject one consumer. + +Both are out, and they had to go together. The 409 alone is a trap: a provider +who cannot remove a connection and cannot delete their stack while one exists is +pinned forever by a single consumer they never met. Revoke existed only to +unpin them. + +Dropping the pair rests on one fact: **a provider teardown destroys nothing of +the consumer's.** The pod keeps running, the seed stays in Vault, the namespace +stays. There is no data loss to prevent, so there is nothing for a hard gate to +protect — only a surprise to prevent, and a confirmation dialog does that. + +What this costs, stated plainly: **a provider has no way to remove one +connection.** Their levers are rotating the code (stops new arrivals, §4.1) and +deleting the stack (stops everyone). §6.3's cap is the only thing bounding how +many can arrive in between. If an abusive-consumer case ever turns up, revoke is +additive — §7.2 blocks nothing, so adding it later breaks no behaviour anyone +depends on. + +--- + +## 8. Surfacing it + +| Where | What | +| --- | --- | +| `GET /setup/:id` (consumer, `vta_only`) | `connection_source`, and when `in_farm`: the provider's name, or an explicit "gone" when `provider_session_id IS NULL` | +| `GET /setup/:id` (provider, `full_stack`) | `connection` bundle (§4.3) + `connections[]` — the dependents' names and statuses | +| `GET /admin/setup-sessions` | a provider column, so support can see the topology without a query | + +The provider's dependent list is not decoration — it is the entire mitigation +for §7.2. Deleting the stack is allowed and breaks every agent on that list, so +the list has to be visible from the page where Delete lives, and named in the +confirmation. + +Names and statuses only. The dependents belong to other users; nothing else +about them is the provider's business. + +--- + +## 9. Conflicts and risks + +Ordered by damage. Two of the four from the pre-scoping draft are gone: SSRF and +credential relay (§9.4) collapse into "not reachable" under §3, and the +third-party-ACL blocker is now the scope boundary rather than a hazard. + +### 9.1 `RegisterDid` failing is silent — a latent bug this feature makes reachable + +**Severity: high. Independent of this feature; must be fixed with it.** + +In `orchestrator.go`, the `RegisterDid` error path logs and carries on. The +session reaches `running` with a DID that was never published: green UI, dead +agent. + +Today that needs a platform stack whose ACL entry went missing — rare, and an +operator's problem. After this feature, every ordinary user can aim a session at +a stack whose daemon might be mid-restart, and hit it. + +The fix is not part of the connection flow but must ship with it: a failed DID +upload has to fail the session, or at minimum leave a visible marker on the row. +§5.1's checks reduce how often it happens; they cannot make a silent failure +acceptable. + +### 9.2 Cross-tenant coupling is real and barely mitigated + +**Severity: high. §7.4 deliberately declines to gate it.** + +An agent's liveness now depends on a resource owned by someone whose incentives +are not aligned: + +| Failure | Handled? | +| --- | --- | +| Provider deletes their stack | **warned, not blocked** — §7.2; consumer learns on next load, §7.3 | +| Provider's stack breaks, or they upgrade to an image that breaks the mediator | **no** — dependents degrade with no signal at all | +| Provider's PVC fills with DID logs they did not create | **partly** — §6.3 bounds the count, not the volume | +| Provider's mediator carries message volume they did not generate | **no** | +| Consumer misbehaves and the provider wants them gone | **no** — §7.4; the levers are rotate, or delete the stack | + +The second row is the one to keep in mind: a *deleted* provider is the case with +a clean signal, and it is the least likely failure. A provider whose mediator is +merely broken produces a consumer that looks perfectly healthy in the portal and +silently delivers nothing. Nothing in this design detects that, and the honest +statement is that liveness monitoring of a consumer's actual messaging path does +not exist for the platform stack either. + +`capacity.VtaOnly` remains correct for the consumer's *pod*, which is the only +thing landing in our cluster's model. What is unmodelled is the load on the +provider's fixed-size mediator and daemon. + +This is acceptable for a feature aimed at people sharing with people they know, +which is what §1 and §4.1 constrain it to. It would not be acceptable behind a +public directory — which is why §11.2 says there will not be one. + +### 9.3 A stale bundle + +**Severity: medium. Caught twice.** + +A rebuilt stack is a *different daemon* with a different `did_hosting_did` and an +empty ACL. Deleting and recreating a stack produces a new row with a new share +code, so an old bundle fails at §5.1 step 3. If a stack somehow keeps its code +while its values change, step 5 catches it. Belt and braces, because the failure +mode is an agent provisioned against a daemon that no longer exists. + +### 9.4 SSRF and credential relay — closed by construction + +**Severity: was high. Now not reachable.** + +Recorded because it is the reason §3 is written the way it is, and because the +first change that makes URLs bundle-sourced reopens all of it: + +1. **SSRF** — a pasted URL turning this server into a probe of link-local, cloud + metadata or in-cluster addresses. +2. **Credential relay** — `New()` takes the `server_did` a remote host *claims* + and uses it as the `aud` of tokens signed with the farm's admin key. A + hostile host claiming another daemon's DID gets a token it can replay there. + +Under §3 neither is reachable: we connect only to hosts we provisioned. Worth +doing anyway, as defence in depth and because it protects the platform path +too — pass the expected `did_hosting_did` into `Factory.For()` and refuse a +mismatched `/api/server-info`. Cheap, and it means §11.1 starts from a +`didhosting` that is already safe. + +### 9.5 Untrusted TLS + +**Severity: low under §1.** + +`CLAUDE.md` records the rule the hard way: components resolve each other's +`did:webvh` over HTTPS and reject an untrusted chain — a staging certificate +passes `tls_provision` and then crash-loops the mediator. Every in-farm stack is +covered by our wildcard or by cert-manager, so this is only a hazard for +§11.1. `didhosting.New()`'s real handshake is the pre-flight either way. + +### 9.6 Name collisions get a worse error, not a bug + +**Severity: low. The message lies; nothing breaks.** + +`setup_sessions_did_path_unique` is `(did_hosting_server_url, vta_name)` — it +already scopes per daemon, so pointing at a second daemon is what it was built +for. But `setup_sessions_vta_name_unique` is **global**, so two users still +cannot both call their agent `main`, and the error says `vta_name already in use` +with no hint that the other holder is on a stack they have never heard of. + +Do **not** relax the global index. The admin routes resolve a session by name +with no `user_id`, and the whole "a session is addressed by its name" design +rests on it — including §4.2's choice of `stack` as the bundle's lookup key. Fix +the message. + +One collision the global index catches by accident and must keep catching: +connecting a `vta_only` named `alice` to a provider whose own session is named +`alice` would mint `alice-vta` on a daemon already serving `alice-vta`. Both +rows carry `vta_name = 'alice'`, so the global index refuses it. The provider's +`alice-mediator` / `alice-vtc` paths are unindexed but can never be produced by +a `vta_only`, which only mints `-vta`. The suffix convention is doing +load-bearing work, exactly as `vta-setup-design.md` claims. + +### 9.7 The mediator accepts any VTA — resolved + +**Severity: none. Settled by product decision; no work.** + +The VTA's config carries only `[messaging] kind = "existing", did = `. +The open question was whether the mediator grants mediation to any DID that asks +or keeps its own allow-list — if the latter, provisioning would have needed a +seventh step in §5.1 and a matching teardown action. + +**Decision: mediation is open to every DID.** No allow-list, so a consumer's VTA +needs no admission on the provider's mediator and nothing has to be withdrawn +when it goes away. + +Two consequences worth keeping visible: + +- The mediator is not an access-control point for this feature. The **share + code is the only gate** (§4.1) — once someone has connected, the mediator will + keep serving them regardless of what the provider does with the code + afterwards. §7.4 already says the same thing from the other direction. +- A wrong or rotated code is refused at create time and never again. There is no + second checkpoint at runtime, which is exactly why §5.1's checks are the ones + that have to be right. + +### 9.8 Blast radius on first release + +**Severity: process.** + +`full_stack` is behind `users.beta_access`, so only beta users can *provide*. +Consuming has no gate. Putting Customize behind the same flag for its first +release keeps the set of people who can create a cross-tenant dependency equal +to the set who already understand the stack. One condition in `POST /setup`, +easy to remove later, awkward to add after the fact. + +--- + +## 10. Work breakdown + +### vtafarm-api + +| # | Change | Files | +| --- | --- | --- | +| 1 | Migration (§6) | `migrations/000025_stack_connection.{up,down}.sql` | +| 2 | Model fields, `Connection()` builder, `IsShared()` | `internal/model/setup_session.go` | +| 3 | Share code: mint, normalise, check symbol, constant-time compare (§4.1.1) | `internal/setup/sharecode.go` (new) + test | +| 4 | `resolveProvider` replacing `resolveSharedInfra` (§5.1) | `internal/handler/setup.go` | +| 5 | `connection` request field + create wiring | `internal/handler/setup.go` | +| 6 | `PUT /setup/:id/sharing` (+ admin twin) | `internal/handler/setup.go`, `setup_admin.go` | +| 7 | `POST /setup/connection/validate` (§5.2) | `internal/handler/setup.go`, `router/router.go` | +| 8 | `connection` + `connections[]` in provider responses; `connection_source` + provider name in consumer responses (§8) | `setup_fullstack.go`, `setup.go`, `admin.go` | +| 9 | Availability split (§10.1 below) | `internal/handler/setup.go` | +| 10 | Make `RegisterDid` failure visible (§9.1) | `internal/setup/orchestrator.go` | +| 11 | Expected-audience check (§9.4, defence in depth) | `internal/didhosting/{factory,client}.go` | +| 12 | **Every new route documented** | `internal/apidocs/openapi.yaml` | +| 13 | `MAX_STACK_CONNECTIONS` | `internal/config/config.go`, `.env.example` | + +Nothing in the delete path changes (§7.2), and there are two new routes (#6, +#7). §9.7 removed the mediator prerequisite entirely; §7.4 removed two endpoints, +a status, and every handler branch that would have had to write to another +user's session row. + +#3 and #4 are the two that need tests rather than review: the share code's +normalisation table (§4.1.1) and §5.1 tier 1 answering identically for all five +of its inputs. Both are the kind of thing that works when written and quietly +stops working later. + +### 10.1 The availability gate has to split — decided + +`GET /setup/availability` reports `vta_only.available: false` with reason +`platform_stack_missing` when there is no platform stack, and `POST /setup` 503s +to match. Once Customize exists that is no longer the whole truth. + +**Decision: a farm with no platform stack can still create a `vta_only`, as long +as the caller brings a bundle.** The platform stack is a default, not a +prerequisite for the mode. That is what forces the gate to split rather than +just relax: `platform_stack_missing` stays a true and useful statement about the +default path, and blocking the whole mode on it stops being one. + +```jsonc +"vta_only": { + "count": 3, + "available": false, // still means: the DEFAULT path + "reason": "platform_stack_missing", + "detail": "…", + "custom_target_allowed": true // ← new; false only when capacity is exhausted +} +``` + +`POST /setup` mirrors it: the `resolveSharedInfra` 503 applies only when no +`connection` was sent. + +### vtafarm + +See the companion frontend doc. Both repos branch as +`feat/vta-only-custom-stack` and must be reviewed together. + +### Documentation + +- `vta-setup-design.md` §"Open: a user-supplied DID host" — narrow to §11.1 and + point here. +- `CLAUDE.md` — add this doc to the `docs/` table, and extend "Shared + infrastructure comes from the platform stack", whose title stops being the + whole truth the day this ships. + +--- + +## 11. Open questions + +### 11.1 Stacks outside this farm + +Out of scope (§1). What it needs, unchanged from +`vta-setup-design.md` §"Open: a user-supplied DID host": either publish our +client DID and have the operator enroll it (no new secrets, but one identity +holds admin across every user's daemon), or mint a keypair per session and +enroll that (contained and revocable, but a private key per session, which +belongs in Vault beside the master seed). + +It would also reopen every mitigation in §9.4 and make §9.5 load-bearing, and it +would have to replace §5.1's DB lookup with something else entirely — the bundle +would become configuration again, losing §3. Treat it as a different feature +that happens to share a UI, not as a later phase of this one. + +### 11.2 There will not be a stack directory + +Not an open question so much as a decision worth recording. Browsing or +searching stacks that accept connections would turn §9.2's coupling from a +favour between two people into a marketplace with no quota model behind it. The +share code (§4.1) exists precisely so that connecting requires somebody to have +chosen you. + +### 11.3 Migrating a live agent between stacks + +Out of scope, and probably not buildable as stated: the VTA's `did:webvh` +contains its host, so moving it mints a new DID — a new identity, not a +migration. The honest answer for a user whose provider disappeared is "create a +new agent", and the UI should say that plainly rather than implying a Move +button could exist. + +### 11.4 Per-stack connection limits + +§6.3 is a global cap. A provider who wants to host thirty agents, or exactly +one, has no way to say so. A `max_connections` column would be trivial; whether +anyone wants it is unknown until the feature has users. + +### 11.5 Removing a single connection + +Rejected for now (§7.4) because there is no data loss to prevent and it existed +only to unpin a delete guard that is also gone. It becomes worth revisiting the +first time a provider actually wants a specific consumer off their stack — the +`DeleteAcl` + `DeleteDid` pair is already how teardown works, so the mechanism +exists. It is additive: nothing in §7 blocks or writes anything today, so adding +it later changes no behaviour anyone depends on. + +### 11.6 Should the platform stack become an ordinary provider? + +§5.1 already unifies the *lookup*. The remaining asymmetry is that the platform +stack has no share code and is reached by a nil ref. Collapsing that too — +giving it a code and making the default path just a preselected bundle — would +remove the last special case, at the cost of touching the one path every +existing session depends on. After this ships, not with it. + +--- + +## 12. What has shipped + +Nothing yet. + +| Item | Status | +| --- | --- | +| §9.7 mediator allow-list | ✅ resolved — open to every DID, no work | +| Migration + model (§6) | ☐ | +| Share code: mint / rotate / clear / normalise (§4.1, §4.1.1) | ☐ | +| `resolveProvider`, both tiers (§5.1) | ☐ | +| `POST /setup/connection/validate` (§5.2) | ☐ | +| `connection` on `POST /setup` (§5) | ☐ | +| `connection` + `connections[]` in responses (§8) | ☐ | +| Availability split (§10.1) | ☐ | +| `RegisterDid` failure visible (§9.1) | ☐ | +| Expected-audience check (§9.4) | ☐ | +| Frontend Share + Customize | ☐ | +| openapi.yaml | ☐ | From b874891a15e083b3482d16cac8058f95d399be10 Mon Sep 17 00:00:00 2001 From: vthwang Date: Fri, 31 Jul 2026 13:04:43 -0700 Subject: [PATCH 02/10] feat: fail a vta_only session when its DID cannot be published MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two hardening changes to the existing DID-hosting paths, ahead of letting a vta_only session point at a stack other than the platform one. Both touch flows that run today; neither adds a new concept. Publishing the DID log used to log and carry on. That produced the worst available outcome: a session marked `running`, a green badge in the portal, and an agent whose did:webvh does not resolve, so it silently delivers nothing. It was survivable while the only way to reach it was a platform stack whose ACL entry had gone missing — an operator's problem on a path operators watch. It stops being survivable once an ordinary user can aim a session at any stack in the farm. Every failure in that block now fails the session: no DID log parsed, no vta_did_url, no client for the control URL, or RegisterDid itself. The one exception is an unconfigured keypair, which stays a warning — that is a deployment-wide state rather than a property of the session, runProvision already treats it the same way, and failing on it would break every local environment that runs without one. The upload deliberately stays after the vta_setup_complete write. Resume only re-runs sessions in vta_setup_running, which is what stops the upload happening twice; registerAtomic sends force=false and errors on any non-2xx, so a replayed upload would fail and — with this change — kill an otherwise healthy session. Comment added, because moving that write later now breaks something non-obvious. The crash window it leaves is pre-existing and written up in design §9.1. Factory.For also takes an expected server DID and refuses a daemon that reports a different one. The daemon's self-reported DID becomes the `aud` of every id_token we sign with the farm's admin key, so a host claiming somebody else's DID would receive a token it could replay there. Nothing needed this while control URLs only ever named daemons we provisioned. Empty means "no expectation on record" and accepts anything, which is every vta_only row until phase 1 backfills did_hosting_did. Cache hits are checked too, or one unverified call would disarm the check for that URL permanently. Signed-off-by: vthwang --- docs/custom-stack-connection-design.md | 30 ++++++- internal/didhosting/factory.go | 36 +++++++- internal/didhosting/factory_test.go | 120 +++++++++++++++++++++++++ internal/handler/setup.go | 2 +- internal/setup/orchestrator.go | 72 ++++++++++++--- 5 files changed, 240 insertions(+), 20 deletions(-) create mode 100644 internal/didhosting/factory_test.go diff --git a/docs/custom-stack-connection-design.md b/docs/custom-stack-connection-design.md index 8cdd1e6..b8c8ba6 100644 --- a/docs/custom-stack-connection-design.md +++ b/docs/custom-stack-connection-design.md @@ -523,6 +523,24 @@ upload has to fail the session, or at minimum leave a visible marker on the row. §5.1's checks reduce how often it happens; they cannot make a silent failure acceptable. +**Shipped in phase 0.** Every failure in the upload block — no DID log parsed, +no `vta_did_url`, no client for the control URL, `RegisterDid` itself — now +calls `markFailed`. The one exception is `didHosting == nil`, which stays a +warning: that is a deployment-wide "no keypair configured" state rather than a +property of the session, `runProvision` already treats it the same way, and +failing on it would break every local environment that runs without one. + +**One gap remains, and it is pre-existing.** The upload runs *after* the row is +written to `vta_setup_complete`, so a crash in between leaves a session whose +DID was never published and which nothing retries. The ordering cannot simply be +reversed: `Resume` re-runs sessions in `vta_setup_running`, and +`registerAtomic` sends `force=false` and errors on any non-2xx, so a replayed +upload against an already-published path would fail — turning a crash-recovery +into a dead session. Closing this properly means making the upload idempotent +first (the way `CreateAcl` already treats 409 as success), which is a change to +what we assume of the daemon's contract and wants its own verification against +the daemon source. Not attempted here. + ### 9.2 Cross-tenant coupling is real and barely mitigated **Severity: high. §7.4 deliberately declines to gate it.** @@ -582,6 +600,14 @@ too — pass the expected `did_hosting_did` into `Factory.For()` and refuse a mismatched `/api/server-info`. Cheap, and it means §11.1 starts from a `didhosting` that is already safe. +**Shipped in phase 0.** `Factory.For(controlURL, expectedServerDid)` refuses a +daemon whose self-reported DID is not the expected one; `""` means "no +expectation on record" and accepts anything, which is the state of every +`vta_only` row until phase 1 backfills `did_hosting_did`. The check runs on +cache hits too — otherwise one unverified call would disarm it permanently for +that URL — and a mismatch does not evict the cached client, because a mismatch +says the *caller's* expectation is wrong, not that the client is unusable. + ### 9.5 Untrusted TLS **Severity: low under §1.** @@ -791,7 +817,7 @@ Nothing yet. | `connection` on `POST /setup` (§5) | ☐ | | `connection` + `connections[]` in responses (§8) | ☐ | | Availability split (§10.1) | ☐ | -| `RegisterDid` failure visible (§9.1) | ☐ | -| Expected-audience check (§9.4) | ☐ | +| `RegisterDid` failure visible (§9.1) | ✅ phase 0 | +| Expected-audience check (§9.4) | ✅ phase 0 | | Frontend Share + Customize | ☐ | | openapi.yaml | ☐ | diff --git a/internal/didhosting/factory.go b/internal/didhosting/factory.go index 50556be..a8f859c 100644 --- a/internal/didhosting/factory.go +++ b/internal/didhosting/factory.go @@ -56,7 +56,12 @@ func (f *Factory) ClientDid() string { // /api/server-info, so an uncached call reaches the network on every upload, // ACL write and teardown. Clients are keyed by URL and hold no per-session // state, so sharing one is safe. -func (f *Factory) For(controlURL string) (*Client, error) { +// +// expectedServerDid, when non-empty, is the DID the caller already knows this +// daemon to have — see checkAudience for why that matters. Empty means "no +// expectation on record", which is the state of every vta_only session until +// its did_hosting_did column is populated. +func (f *Factory) For(controlURL, expectedServerDid string) (*Client, error) { if f == nil { return nil, fmt.Errorf("DID hosting not configured (no client keypair)") } @@ -68,7 +73,7 @@ func (f *Factory) For(controlURL string) (*Client, error) { f.mu.Lock() defer f.mu.Unlock() if c, ok := f.byBase[base]; ok { - return c, nil + return c, checkAudience(base, c.serverDid, expectedServerDid) } c, err := New(base, f.clientDid, f.privKeyB64) if err != nil { @@ -77,6 +82,31 @@ func (f *Factory) For(controlURL string) (*Client, error) { // inherit the failure. return nil, err } + // Cached before the audience check, and the error returned alongside the + // cached entry above: a mismatch says the CALLER's expectation is wrong for + // this URL, not that the client is unusable. A later call carrying the right + // expectation must hit the cache and succeed rather than re-fetching. f.byBase[base] = c - return c, nil + return c, checkAudience(base, c.serverDid, expectedServerDid) +} + +// checkAudience refuses a daemon whose self-reported DID is not the one we +// expected to be talking to. +// +// serverDid comes from the daemon's own /api/server-info and becomes the `aud` +// of every id_token this client signs — with vtafarm-api's private key, which +// holds an admin ACL entry on every daemon the farm operates. A host that +// answers with somebody else's DID therefore receives a token minted for that +// somebody else, and can replay it there as us. +// +// Nothing has needed this while control URLs came only out of our own database +// and named daemons we provisioned. It is written down now because the moment a +// session can be pointed at a daemon on the strength of a value a user handed +// us, "the daemon says who it is" stops being a safe answer to "who am I +// signing for". +func checkAudience(base, serverDid, expected string) error { + if expected == "" || serverDid == expected { + return nil + } + return fmt.Errorf("DID hosting daemon at %s reports server DID %q, expected %q", base, serverDid, expected) } diff --git a/internal/didhosting/factory_test.go b/internal/didhosting/factory_test.go new file mode 100644 index 0000000..444d1fe --- /dev/null +++ b/internal/didhosting/factory_test.go @@ -0,0 +1,120 @@ +package didhosting + +import ( + "crypto/ed25519" + "encoding/base64" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" +) + +// testKeypair returns a did:key and the base64 seed New expects. The DID does +// not have to be a real multibase encoding — nothing in the paths under test +// resolves it. +func testKeypair() (did, privKeyB64 string) { + seed := make([]byte, ed25519.SeedSize) + for i := range seed { + seed[i] = byte(i) + } + return "did:key:z6MkTestClient", base64.StdEncoding.EncodeToString(seed) +} + +// serverInfoStub serves /api/server-info with the given DID and counts hits, so +// a test can tell a cache hit from a re-fetch. +func serverInfoStub(t *testing.T, serverDid string, hits *int32) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/server-info" { + w.WriteHeader(http.StatusNotFound) + return + } + atomic.AddInt32(hits, 1) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"server_did":"` + serverDid + `"}`)) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestForAcceptsMatchingAudience(t *testing.T) { + var hits int32 + srv := serverInfoStub(t, "did:webvh:dids.example", &hits) + did, key := testKeypair() + + c, err := NewFactory(did, key).For(srv.URL, "did:webvh:dids.example") + if err != nil { + t.Fatalf("For: %v", err) + } + if c.ServerDid() != "did:webvh:dids.example" { + t.Fatalf("ServerDid = %q", c.ServerDid()) + } +} + +// The check is the whole point of the expected-audience parameter: a daemon +// claiming somebody else's DID would otherwise receive an id_token minted for +// that somebody else, signed with the farm's admin key. +func TestForRefusesMismatchedAudience(t *testing.T) { + var hits int32 + srv := serverInfoStub(t, "did:webvh:attacker.example", &hits) + did, key := testKeypair() + + _, err := NewFactory(did, key).For(srv.URL, "did:webvh:dids.example") + if err == nil { + t.Fatal("expected a mismatch error, got nil") + } + for _, want := range []string{"did:webvh:attacker.example", "did:webvh:dids.example"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not name %q", err, want) + } + } +} + +// Empty means "no expectation on record" — the state of every vta_only session +// until did_hosting_did is populated. It must not start failing them. +func TestForWithoutExpectationAcceptsAnything(t *testing.T) { + var hits int32 + srv := serverInfoStub(t, "did:webvh:whatever.example", &hits) + did, key := testKeypair() + + if _, err := NewFactory(did, key).For(srv.URL, ""); err != nil { + t.Fatalf("For with no expectation: %v", err) + } +} + +// A cached client must still be checked. Otherwise one unverified call would +// permanently disarm the check for that URL. +func TestForChecksCachedClients(t *testing.T) { + var hits int32 + srv := serverInfoStub(t, "did:webvh:dids.example", &hits) + did, key := testKeypair() + f := NewFactory(did, key) + + if _, err := f.For(srv.URL, ""); err != nil { + t.Fatalf("priming call: %v", err) + } + if _, err := f.For(srv.URL, "did:webvh:someone-else.example"); err == nil { + t.Fatal("expected the cached client to be checked, got nil") + } + if _, err := f.For(srv.URL, "did:webvh:dids.example"); err != nil { + t.Fatalf("matching expectation after a mismatch: %v", err) + } + if got := atomic.LoadInt32(&hits); got != 1 { + t.Errorf("server-info fetched %d times, want 1 — a mismatch must not evict the cache", got) + } +} + +func TestNilFactoryFails(t *testing.T) { + var f *Factory + if _, err := f.For("https://dids.example", ""); err == nil { + t.Fatal("expected an error from a nil factory") + } +} + +func TestForRejectsEmptyURL(t *testing.T) { + did, key := testKeypair() + if _, err := NewFactory(did, key).For("", ""); err == nil { + t.Fatal("expected an error for an empty control URL") + } +} diff --git a/internal/handler/setup.go b/internal/handler/setup.go index b79a5a4..1513ea5 100644 --- a/internal/handler/setup.go +++ b/internal/handler/setup.go @@ -720,7 +720,7 @@ func (h *SetupHandler) teardownSession(c *gin.Context, session *model.SetupSessi // and deleting from it would leave this session's DID log behind on the old // one while removing somebody else's. if h.didHosting != nil && (session.VtaDidUrl != "" || session.VtaDid != "") { - dh, err := h.didHosting.For(session.DidHostingControlURL) + dh, err := h.didHosting.For(session.DidHostingControlURL, session.DIDHostingDid) if err != nil { log.Printf("[setup] warn: no DID hosting client for session %d (%q): %v", session.ID, session.DidHostingControlURL, err) diff --git a/internal/setup/orchestrator.go b/internal/setup/orchestrator.go index 6bee3d5..b438963 100644 --- a/internal/setup/orchestrator.go +++ b/internal/setup/orchestrator.go @@ -277,9 +277,45 @@ func (o *Orchestrator) runSetup(ctx context.Context, sessionID uint) { }) log.Printf("[orchestrator] session %d: setup complete, VTA DID=%s", sessionID, vtaDID) + // Publishing the DID log is not a best-effort side errand: an unpublished + // did:webvh cannot be resolved, so the agent this session is building can + // never be reached. Every failure below therefore fails the session. + // + // It used to log and carry on, which produced the worst available outcome — + // a session marked `running`, a green badge in the portal, and an agent that + // silently delivers nothing. That was survivable while the only way to hit + // it was a platform stack whose ACL entry had gone missing, i.e. an + // operator's problem on a path operators watch. Once a session can be aimed + // at any stack in the farm it becomes an ordinary user's failure mode, and a + // silent one is not acceptable there. + // + // This must stay AFTER the vta_setup_complete write above. Resume only + // re-runs sessions still in vta_setup_running, so a session that reaches + // here is never replayed through it — which is what keeps the upload from + // happening twice. registerAtomic sends force=false and errors on any + // non-2xx, so a second attempt at an already-published path would fail, and + // with the change above that failure would now kill an otherwise healthy + // session. Moving the status write later requires making the upload + // idempotent first. + // + // The gap that ordering leaves: a crash between the two lands the row in + // vta_setup_complete with nothing published and nothing retrying. That is + // pre-existing and unchanged here — see docs/custom-stack-connection-design.md + // §9.1. log.Printf("[orchestrator] session %d: did-hosting=%v didLog_len=%d vtaDidUrl=%q", sessionID, o.didHosting != nil, len(didLog), session.VtaDidUrl) - if o.didHosting != nil && didLog != "" && session.VtaDidUrl != "" { + if o.didHosting != nil { + switch { + case didLog == "": + // `vta setup` reported a DID but no did.jsonl followed it in the + // logs. Nothing to publish, and nothing that publishes it later. + o.markFailed(sessionID, "VTA setup produced no DID log to publish — the agent's DID would never resolve") + return + case session.VtaDidUrl == "": + o.markFailed(sessionID, "session has no VTA DID URL — cannot publish the agent's DID") + return + } + // Extract path from the full URL e.g. https://dids.fpp2.ic3.dev/pvta-vta → pvta-vta path := session.VtaDidUrl if u, err := url.Parse(path); err == nil { @@ -287,21 +323,29 @@ func (o *Orchestrator) runSetup(ctx context.Context, sessionID uint) { } // The daemon this session was provisioned against, not whichever one is // current — the two differ the moment the platform stack is rebuilt. - dh, err := o.didHosting.For(session.DidHostingControlURL) + dh, err := o.didHosting.For(session.DidHostingControlURL, session.DIDHostingDid) if err != nil { - log.Printf("[orchestrator] session %d: DID upload FAILED (no client for %q): %v", - sessionID, session.DidHostingControlURL, err) - } else { - log.Printf("[orchestrator] session %d: uploading DID log to hosting service (path=%s)", sessionID, path) - if err := dh.RegisterDid(ctx, path, didLog); err != nil { - log.Printf("[orchestrator] session %d: DID upload FAILED: %v", sessionID, err) - } else { - log.Printf("[orchestrator] session %d: DID log uploaded to hosting service", sessionID) + o.markFailed(sessionID, "cannot reach the DID hosting control API at "+ + session.DidHostingControlURL+": "+err.Error()) + return + } + log.Printf("[orchestrator] session %d: uploading DID log to hosting service (path=%s)", sessionID, path) + if err := dh.RegisterDid(ctx, path, didLog); err != nil { + if ctx.Err() != nil { + return } + o.markFailed(sessionID, "failed to publish the agent's DID to "+ + session.DidHostingControlURL+": "+err.Error()) + return } - } else if o.didHosting != nil { - log.Printf("[orchestrator] session %d: skipping DID upload — didLog_empty=%v vtaDidUrl_empty=%v", - sessionID, didLog == "", session.VtaDidUrl == "") + log.Printf("[orchestrator] session %d: DID log uploaded to hosting service", sessionID) + } else { + // No keypair configured at all. Left as a warning rather than a failure + // because it is a deployment-wide state, not a property of this session + // — runProvision's ACL step already treats it the same way, and failing + // here would break every local environment that runs without one. + log.Printf("[orchestrator] session %d: DID_HOSTING_DID unset — the agent's DID will not be published "+ + "and will not resolve", sessionID) } // Auto-trigger Phase 2 if admin_did was provided at session creation time. @@ -335,7 +379,7 @@ func (o *Orchestrator) runProvision(ctx context.Context, sessionID uint, adminDi var controlDid string log.Printf("[orchestrator] session %d: did-hosting configured=%v vta_did=%q", sessionID, o.didHosting != nil, session.VtaDid) if o.didHosting != nil { - dh, err := o.didHosting.For(session.DidHostingControlURL) + dh, err := o.didHosting.For(session.DidHostingControlURL, session.DIDHostingDid) if err != nil { o.markFailed(sessionID, "failed to reach DID hosting control API: "+err.Error()) return From c1de78b8b3fdf9e149a3dd21dc2fe2cd9630626a Mon Sep 17 00:00:00 2001 From: vthwang Date: Fri, 31 Jul 2026 13:10:50 -0700 Subject: [PATCH 03/10] feat: schema and share-code primitives for stack connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Groundwork for letting a vta_only session connect to a full_stack other than the platform one. Nothing user-visible: no route reads or writes any of it yet. The three values a vta_only is actually wired to are already per-session columns (000023), so the migration adds only the grant and the link: share_code on the provider, connection_source + provider_session_id on the consumer. share_code is one nullable column rather than a boolean plus a code, because the two would always have to agree and the code alone answers both questions — minting enables sharing, clearing disables it, replacing invalidates outstanding bundles, and none of the three touch a session already connected. It is stored in plaintext on purpose: it is a capability its owner displays to themselves and reads aloud, not a password, and rotation covers what hashing would without making the value unrecoverable to the person who has to share it. connection_source admits 'platform' and 'in_farm' only. A stack this farm did not provision has no ACL entry for our client DID and would 401 on the first DID upload, so the constraint states the same scope the code does; adding 'external' later is a one-line ALTER. provider_session_id is ON DELETE SET NULL, and that is the whole orphan mechanism rather than a fallback. Deleting a provider blocks on nothing, so no handler writes to its dependents — Postgres nulls the link in the same transaction and connection_source = 'in_farm' AND provider_session_id IS NULL is permanently "the stack this agent connected to is gone", with no event needing to have fired and no reconciler to drift. Also widens did_hosting_did from a full_stack output to "the DID of the daemon at did_hosting_control_url", and backfills it for existing vta_only rows. That is what arms Factory.For's audience check for sessions built before it existed. The backfill joins on the server URL rather than assuming the platform stack, so a row whose daemon no longer has a row keeps '' — no expectation on record — instead of being handed a DID that was never its daemon's. Share codes are Crockford base32 with a check symbol: the format is meant to survive being read aloud, and one without a checksum is half-designed. Normalisation folds the glyphs people substitute anyway (I/L to 1, O to 0) so every transcription of a code reaches the same constant-time comparison, and the check symbol makes "you mistyped this" a different answer from "this code does not open anything here" — two problems needing two different actions, which would otherwise collapse into the vaguest message in the flow. Verified against a throwaway Postgres 18: all 25 migrations apply, the FK and CHECK land as intended, deleting a provider leaves its consumer running with a nulled link, the backfill skips rows whose daemon is gone, and down/up round-trips cleanly. Signed-off-by: vthwang --- .env.example | 6 + docs/custom-stack-connection-design.md | 17 +- internal/config/config.go | 16 +- internal/model/setup_session.go | 67 ++++++++ internal/setup/sharecode.go | 156 ++++++++++++++++++ internal/setup/sharecode_test.go | 168 ++++++++++++++++++++ migrations/000025_stack_connection.down.sql | 13 ++ migrations/000025_stack_connection.up.sql | 81 ++++++++++ 8 files changed, 520 insertions(+), 4 deletions(-) create mode 100644 internal/setup/sharecode.go create mode 100644 internal/setup/sharecode_test.go create mode 100644 migrations/000025_stack_connection.down.sql create mode 100644 migrations/000025_stack_connection.up.sql diff --git a/.env.example b/.env.example index 225a08d..b540404 100644 --- a/.env.example +++ b/.env.example @@ -29,6 +29,12 @@ CLUSTER_INGRESS_IP= # Root domain managed by Cloudflare (subdomains will be created under this) CLUSTER_DOMAIN= +# How many VTA-only agents may connect to one shared full stack. 0 = unlimited. +# Bounds the storage and message volume other people's agents put on a +# provider's mediator and DID host — a provider can stop new connections by +# rotating their share code, but cannot remove one that already exists. +MAX_STACK_CONNECTIONS=10 + # Cloudflare (required for setup wizard) # API token needs Zone:DNS:Edit permission CLOUDFLARE_API_TOKEN= diff --git a/docs/custom-stack-connection-design.md b/docs/custom-stack-connection-design.md index b8c8ba6..147a930 100644 --- a/docs/custom-stack-connection-design.md +++ b/docs/custom-stack-connection-design.md @@ -358,6 +358,19 @@ CREATE INDEX setup_sessions_provider_idx WHERE provider_session_id IS NOT NULL; ``` +**Shipped in phase 1**, plus one backfill the table above does not show. ` +did_hosting_did` has been a `full_stack` output column (the daemon's own DID) +and is `''` on every `vta_only` row. Its meaning widens here to "the DID of the +daemon at `did_hosting_control_url`", true for both modes, which is what +`Factory.For`'s audience check (§9.4) compares against. Existing `vta_only` rows +are joined to the daemon they actually point at — matching on +`did_hosting_server_url` rather than assuming the platform stack — so a row +whose daemon no longer has a row keeps `''` and therefore keeps "no expectation +on record" rather than being handed a DID that was never its daemon's. + +The down migration does not reverse that backfill: it restores the schema, not a +snapshot of the data, and the value is correct independently of this feature. + ### 6.1 `ON DELETE SET NULL` is the whole orphan mechanism Not a fallback — **the** mechanism. §7 blocks nothing and writes nothing at @@ -810,8 +823,8 @@ Nothing yet. | Item | Status | | --- | --- | | §9.7 mediator allow-list | ✅ resolved — open to every DID, no work | -| Migration + model (§6) | ☐ | -| Share code: mint / rotate / clear / normalise (§4.1, §4.1.1) | ☐ | +| Migration + model (§6) | ✅ phase 1 | +| Share code: mint / normalise / validate / compare (§4.1, §4.1.1) | ✅ phase 1 | | `resolveProvider`, both tiers (§5.1) | ☐ | | `POST /setup/connection/validate` (§5.2) | ☐ | | `connection` on `POST /setup` (§5) | ☐ | diff --git a/internal/config/config.go b/internal/config/config.go index 133b2f9..1ec0247 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -22,6 +22,17 @@ type Config struct { // database, where every API would resume the same rows. // See docs/shared-dev-database.md. OrchestratorResume bool + // MaxStackConnections caps how many vta_only sessions may connect to one + // shared full_stack. 0 disables the cap. + // + // Crude on purpose — it is not a capacity model. The consumer's own pod is + // what lands in this cluster's capacity accounting; what is unmodelled is + // the storage and message volume it puts on somebody else's mediator and + // DID host. The cap matters more than its bluntness suggests because a + // provider has no way to remove a single connection: rotating the code + // stops new arrivals, and this is what bounds how many arrive before they + // think to. See docs/custom-stack-connection-design.md §6.3. + MaxStackConnections int DB DBConfig K8s K8sConfig Cloudflare CloudflareConfig @@ -162,8 +173,9 @@ func Load() *Config { ClusterIngressIP: getEnv("CLUSTER_INGRESS_IP", ""), ClusterDomain: getEnv("CLUSTER_DOMAIN", ""), - ACMEClusterIssuer: getEnv("ACME_CLUSTER_ISSUER", DefaultACMEIssuer), - OrchestratorResume: getEnvBool("ORCHESTRATOR_RESUME", true), + ACMEClusterIssuer: getEnv("ACME_CLUSTER_ISSUER", DefaultACMEIssuer), + OrchestratorResume: getEnvBool("ORCHESTRATOR_RESUME", true), + MaxStackConnections: getEnvInt("MAX_STACK_CONNECTIONS", 10), DB: DBConfig{ Host: getEnv("DB_HOST", "localhost"), Port: getEnv("DB_PORT", "5432"), diff --git a/internal/model/setup_session.go b/internal/model/setup_session.go index beea2e0..32690e0 100644 --- a/internal/model/setup_session.go +++ b/internal/model/setup_session.go @@ -14,6 +14,23 @@ const ( // Where a session's hostnames come from. Orthogonal to Mode: a session is // vta_only or full_stack, and independently managed, custom or platform. +// Where a vta_only session's mediator and DID host came from. Orthogonal to +// both Mode and DomainType, and meaningless for full_stack, which provisions +// its own. +// +// There is deliberately no "external" value: the farm's client DID is enrolled +// as an admin in every full_stack daemon it provisioned and in nothing else, so +// a stack this farm did not build cannot be a target. See +// docs/custom-stack-connection-design.md §1. +const ( + // ConnectionPlatform is the default and the only value any session created + // before the connection feature can have. + ConnectionPlatform = "platform" + // ConnectionInFarm means the session named another full_stack in this farm + // by pasting its owner's connection bundle. + ConnectionInFarm = "in_farm" +) + const ( // DomainManaged is the default — labels derived from the user's chosen // name in our own zone (vta-.firstperson.dev). DomainID is NULL. @@ -71,6 +88,30 @@ type SetupSession struct { // are follows from neither Mode nor DomainType alone. DidHostingServerURL string `gorm:"column:did_hosting_server_url;not null;default:''" json:"-"` DidHostingControlURL string `gorm:"column:did_hosting_control_url;not null;default:''" json:"-"` + + // ShareCode is the grant that lets somebody else's vta_only session connect + // to this stack. full_stack only; NULL means "not shared", which is also + // every session's starting state and the platform stack's permanent one — + // that stack is reached by the default path, which sends no bundle. + // + // Minting enables sharing, clearing disables it, and replacing invalidates + // every bundle already handed out. None of the three touch a session already + // connected: the code gates joining, never membership. + ShareCode *string `gorm:"column:share_code" json:"-"` + + // ConnectionSource says where this session's mediator and DID host came + // from — ConnectionPlatform or ConnectionInFarm. ProviderSessionID is the + // full_stack row it connected to, and is NULL both for platform sessions + // (which never had one) and for a session whose provider has since been + // deleted. + // + // Neither is needed to run the session; the three snapshotted values above + // do that, and stay authoritative because a did:webvh bakes its host in at + // mint time. These answer what a snapshot cannot: who the dependents of a + // stack are, what to call the provider in the UI, and — via + // ON DELETE SET NULL — whether that provider still exists at all. + ConnectionSource string `gorm:"column:connection_source;not null;default:platform" json:"connection_source"` + ProviderSessionID *uint `gorm:"column:provider_session_id" json:"-"` Portable bool `gorm:"not null;default:true" json:"portable"` PreRotationCount int `gorm:"not null;default:1" json:"pre_rotation_count"` // Image used for the vta-setup K8s Job @@ -185,6 +226,32 @@ func (s *SetupSession) IsFullStack() bool { return s.Mode == ModeFullStack } +// IsShared reports whether this stack currently accepts new connections. +// +// Only a full_stack can be shared, and only one that has finished provisioning: +// a bundle for a stack whose mediator DID or daemon DID has not landed yet +// would name values that are about to change. That readiness rule is the same +// one the platform stack has always been held to before a vta_only could be +// wired to it. +func (s *SetupSession) IsShared() bool { + return s.IsFullStack() && + s.ShareCode != nil && *s.ShareCode != "" && + s.Status == "running" && + s.MediatorDid != "" && + s.DIDHostingDid != "" +} + +// IsOrphaned reports whether this session connected to a stack that has since +// been deleted. Its pods keep running — nothing in a provider teardown touches +// the consumer's namespace — but its did:webvh no longer resolves and its +// mediator is gone, so it can neither be reached nor deliver. +// +// Derived from ON DELETE SET NULL rather than written by a delete handler, +// which is why it needs no event to have fired and cannot drift. +func (s *SetupSession) IsOrphaned() bool { + return s.ConnectionSource == ConnectionInFarm && s.ProviderSessionID == nil +} + // IsFixedLabel reports whether the session's hostnames are the four fixed // labels rather than name-derived ones. True for custom and platform domains, // which is also exactly when VtaName/VtcName carry no hostname meaning and are diff --git a/internal/setup/sharecode.go b/internal/setup/sharecode.go new file mode 100644 index 0000000..a6b781c --- /dev/null +++ b/internal/setup/sharecode.go @@ -0,0 +1,156 @@ +package setup + +import ( + "crypto/rand" + "crypto/subtle" + "fmt" + "strings" +) + +// A share code is the grant that lets somebody else's vta_only session connect +// to a full_stack. Design: docs/custom-stack-connection-design.md §4.1. +// +// Crockford base32 rather than raw base32 because this code is meant to survive +// being read aloud and retyped, not only pasted: it excludes I, L, O and U, and +// defines how the glyphs people confuse anyway should be folded back. A format +// for oral transmission without a check symbol is half-designed, so the last +// character is one. +// +// The check symbol is not security — an attacker computes it as easily as we +// do. It exists so the overwhelmingly common failure, a hand-copied character, +// is diagnosed as itself rather than landing in the same answer as "the owner +// rotated this code". +const ( + // crockfordAlphabet is the canonical 32-symbol set: digits, then A–Z minus + // I, L, O and U. + crockfordAlphabet = "0123456789ABCDEFGHJKMNPQRSTVWXYZ" + // crockfordCheckAlphabet extends it with the five check-only symbols, so a + // check value of 32–36 has a representation the data alphabet cannot + // produce. Standard Crockford. + crockfordCheckAlphabet = crockfordAlphabet + "*~$=U" + + // shareCodeDataLen is the number of random symbols before the check symbol. + // 15 × 5 bits = 75 bits, which is not brute-forceable through an + // authenticated, rate-limited route. + shareCodeDataLen = 15 + // shareCodeGroup is the display grouping — K7M2-9XQP-4B8W-3NRT. + shareCodeGroup = 4 +) + +// NewShareCode mints a share code, formatted for display. +// +// Rejection sampling rather than modulo: 256 is not a multiple of 32 in a way +// that matters here (it is, exactly), but reading one byte per symbol and +// masking to 5 bits is simpler to see as unbiased than any arithmetic on a +// larger word, and this runs once per share. +func NewShareCode() (string, error) { + buf := make([]byte, shareCodeDataLen) + if _, err := rand.Read(buf); err != nil { + return "", fmt.Errorf("generate share code: %w", err) + } + + data := make([]byte, shareCodeDataLen) + for i, b := range buf { + data[i] = crockfordAlphabet[b&0x1f] + } + + full := string(data) + string(crockfordCheckSymbol(data)) + return GroupShareCode(full), nil +} + +// GroupShareCode inserts the display dashes. Purely cosmetic — every comparison +// runs on the normalised form, which has none. +func GroupShareCode(code string) string { + var b strings.Builder + for i, r := range code { + if i > 0 && i%shareCodeGroup == 0 { + b.WriteByte('-') + } + b.WriteRune(r) + } + return b.String() +} + +// NormalizeShareCode folds a code to its canonical comparable form: dashes and +// whitespace removed, uppercased, and the ambiguous glyphs mapped the way +// Crockford specifies — I and L to 1, O to 0. +// +// It does NOT validate. A string that could not possibly be a share code +// normalises to something that will simply fail to match, which is the correct +// outcome for a credential comparison; ValidateShareCode is what turns a +// mistyped code into its own diagnosis. +func NormalizeShareCode(code string) string { + var b strings.Builder + b.Grow(len(code)) + for _, r := range strings.ToUpper(code) { + switch r { + case '-', ' ', '\t', '\n', '\r': + // Grouping and whatever whitespace survived a copy-paste. + case 'I', 'L': + b.WriteByte('1') + case 'O': + b.WriteByte('0') + default: + b.WriteRune(r) + } + } + return b.String() +} + +// ValidateShareCode reports whether code is well-formed: right length, only +// symbols from the alphabet, and a check symbol that matches its data. +// +// This is what lets "you mistyped this" be a different answer from "this code +// does not open anything here" — two problems needing two different actions +// from whoever is holding the code. Both would otherwise collapse into the +// second, which is the vaguest message in the flow. +func ValidateShareCode(code string) error { + n := NormalizeShareCode(code) + if len(n) != shareCodeDataLen+1 { + return fmt.Errorf("share code must be %d characters, got %d", shareCodeDataLen+1, len(n)) + } + + data := n[:shareCodeDataLen] + for i := 0; i < len(data); i++ { + if strings.IndexByte(crockfordAlphabet, data[i]) < 0 { + return fmt.Errorf("share code contains %q, which is not a valid character", data[i]) + } + } + if n[shareCodeDataLen] != crockfordCheckSymbol([]byte(data)) { + return fmt.Errorf("share code check character does not match — it looks mistyped") + } + return nil +} + +// ShareCodeMatches compares a supplied code against a stored one in constant +// time, after normalising both. It is a credential. +// +// A stored code that is empty never matches, so a stack with sharing off cannot +// be opened by supplying an empty code. +func ShareCodeMatches(supplied, stored string) bool { + s := NormalizeShareCode(stored) + if s == "" { + return false + } + return subtle.ConstantTimeCompare([]byte(NormalizeShareCode(supplied)), []byte(s)) == 1 +} + +// crockfordCheckSymbol computes the standard Crockford check symbol: the value +// of the data interpreted as a base-32 integer, modulo 37, rendered from the +// extended alphabet. +// +// Taken modulo as we go rather than building a 75-bit integer — 37 is prime and +// coprime with 32, so the running remainder is exact. +func crockfordCheckSymbol(data []byte) byte { + rem := 0 + for _, c := range data { + v := strings.IndexByte(crockfordAlphabet, c) + if v < 0 { + // Only reachable if a caller hands this un-normalised or invalid + // data; ValidateShareCode checks the alphabet before calling. + return 0 + } + rem = (rem*32 + v) % 37 + } + return crockfordCheckAlphabet[rem] +} diff --git a/internal/setup/sharecode_test.go b/internal/setup/sharecode_test.go new file mode 100644 index 0000000..547a2f1 --- /dev/null +++ b/internal/setup/sharecode_test.go @@ -0,0 +1,168 @@ +package setup + +import ( + "strings" + "testing" +) + +func TestNewShareCodeIsValidAndFormatted(t *testing.T) { + seen := map[string]bool{} + for i := 0; i < 200; i++ { + code, err := NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + if err := ValidateShareCode(code); err != nil { + t.Fatalf("minted code %q failed its own validation: %v", code, err) + } + if got, want := code, "XXXX-XXXX-XXXX-XXXX"; len(got) != len(want) { + t.Fatalf("code %q has length %d, want %d", got, len(got), len(want)) + } + if strings.Count(code, "-") != 3 { + t.Fatalf("code %q is not grouped in fours", code) + } + if seen[code] { + t.Fatalf("minted a duplicate code %q within 200 draws", code) + } + seen[code] = true + } +} + +// The alphabet exists to keep confusable glyphs out of a code that gets read +// aloud. If a minted code can contain them, normalisation would rewrite it into +// something that no longer matches what is stored. +func TestNewShareCodeExcludesConfusableGlyphs(t *testing.T) { + for i := 0; i < 200; i++ { + code, err := NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + data := NormalizeShareCode(code)[:shareCodeDataLen] + if idx := strings.IndexAny(data, "ILOU"); idx >= 0 { + t.Fatalf("code %q contains excluded glyph %q", code, data[idx]) + } + } +} + +func TestNormalizeShareCode(t *testing.T) { + cases := []struct{ in, want string }{ + {"K7M2-9XQP-4B8W-3NRT", "K7M29XQP4B8W3NRT"}, + {"k7m2-9xqp-4b8w-3nrt", "K7M29XQP4B8W3NRT"}, + {"K7M2 9XQP 4B8W 3NRT", "K7M29XQP4B8W3NRT"}, + {" K7M29XQP4B8W3NRT\n", "K7M29XQP4B8W3NRT"}, + // Crockford's own folding: the glyphs a human substitutes anyway. + {"I", "1"}, + {"l", "1"}, + {"O", "0"}, + {"o", "0"}, + {"", ""}, + } + for _, c := range cases { + if got := NormalizeShareCode(c.in); got != c.want { + t.Errorf("NormalizeShareCode(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +// Every form a human might produce from one code has to reach the same +// comparison, or a correct code is rejected as wrong. +func TestNormalizeIsStableAcrossTranscriptions(t *testing.T) { + code, err := NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + canonical := NormalizeShareCode(code) + + for _, variant := range []string{ + code, + strings.ToLower(code), + strings.ReplaceAll(code, "-", ""), + strings.ReplaceAll(code, "-", " "), + " " + code + " ", + } { + if got := NormalizeShareCode(variant); got != canonical { + t.Errorf("variant %q normalised to %q, want %q", variant, got, canonical) + } + } +} + +func TestValidateShareCodeRejectsBadInput(t *testing.T) { + good, err := NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + norm := NormalizeShareCode(good) + + cases := []struct{ name, code string }{ + {"empty", ""}, + {"too short", norm[:len(norm)-1]}, + {"too long", norm + "7"}, + {"invalid character", "K7M29XQP4B8W3N!"}, + } + for _, c := range cases { + if err := ValidateShareCode(c.code); err == nil { + t.Errorf("%s: expected an error for %q", c.name, c.code) + } + } +} + +// The point of the check symbol: a single mistyped character is caught locally, +// so it never reaches the server and never lands in the generic "this bundle +// does not open anything" message. +func TestValidateShareCodeCatchesSingleCharacterTypos(t *testing.T) { + code, err := NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + norm := NormalizeShareCode(code) + + caught, total := 0, 0 + for i := 0; i < shareCodeDataLen; i++ { + for _, sub := range crockfordAlphabet { + if byte(sub) == norm[i] { + continue + } + total++ + typo := norm[:i] + string(sub) + norm[i+1:] + if ValidateShareCode(typo) != nil { + caught++ + } + } + } + // Crockford's mod-37 check detects every single-symbol substitution. + if caught != total { + t.Errorf("caught %d of %d single-character typos, want all", caught, total) + } +} + +func TestShareCodeMatches(t *testing.T) { + code, err := NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + + if !ShareCodeMatches(code, code) { + t.Error("a code must match itself") + } + if !ShareCodeMatches(strings.ToLower(strings.ReplaceAll(code, "-", " ")), code) { + t.Error("a retyped code must match the stored one") + } + + other, err := NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + if ShareCodeMatches(other, code) { + t.Error("a different code must not match") + } +} + +// Sharing is off exactly when the stored code is absent. An empty supplied code +// must not open it. +func TestShareCodeMatchesRefusesEmptyStored(t *testing.T) { + for _, supplied := range []string{"", "K7M2-9XQP-4B8W-3NRT"} { + if ShareCodeMatches(supplied, "") { + t.Errorf("supplied %q matched an empty stored code", supplied) + } + } +} diff --git a/migrations/000025_stack_connection.down.sql b/migrations/000025_stack_connection.down.sql new file mode 100644 index 0000000..07d9618 --- /dev/null +++ b/migrations/000025_stack_connection.down.sql @@ -0,0 +1,13 @@ +-- The did_hosting_did backfill is deliberately not reversed. Down migrations +-- restore the schema, not a snapshot of the data, and the backfilled value is +-- correct independently of this feature: it records which daemon's DID a +-- vta_only session's control URL actually answers with. Blanking it would +-- disarm Factory.For's audience check on rows that predate the rollback for no +-- gain, and there is nothing to distinguish a backfilled value from one written +-- since. +DROP INDEX IF EXISTS setup_sessions_provider_idx; + +ALTER TABLE setup_sessions + DROP COLUMN IF EXISTS provider_session_id, + DROP COLUMN IF EXISTS connection_source, + DROP COLUMN IF EXISTS share_code; diff --git a/migrations/000025_stack_connection.up.sql b/migrations/000025_stack_connection.up.sql new file mode 100644 index 0000000..264c241 --- /dev/null +++ b/migrations/000025_stack_connection.up.sql @@ -0,0 +1,81 @@ +-- Lets a vta_only session connect to a full_stack other than the platform one, +-- provided that stack is one this farm provisioned. Design: +-- docs/custom-stack-connection-design.md. +-- +-- The three values a vta_only session is actually wired to — mediator_did, +-- did_hosting_server_url, did_hosting_control_url — are already per-session +-- columns (000023), so nothing here holds the connection itself. What is added +-- is the grant on the provider side and the link on the consumer side. + +-- ── Provider ──────────────────────────────────────────────────────────────── +-- NULL means "not shared". One nullable column rather than a boolean plus a +-- code, because the two would always have to agree and the code alone answers +-- both questions: minting one enables sharing, clearing it disables, and +-- replacing it invalidates every bundle already handed out without touching +-- anyone already connected. +-- +-- Deliberately not hashed. It is a capability its owner displays to themselves +-- and reads aloud, not a password — and rotation handles everything hashing +-- would, without making the value unrecoverable to the person who has to share +-- it. +ALTER TABLE setup_sessions ADD COLUMN share_code TEXT NULL; + +-- ── Consumer ──────────────────────────────────────────────────────────────── +-- Neither column is needed to RUN a session: the three snapshotted values above +-- already do that, and they stay authoritative because a did:webvh bakes its +-- host in at mint time. These exist for the three things a snapshot cannot +-- answer — finding a stack's dependents cheaply, naming the provider in the UI +-- instead of showing a URL, and letting support see the topology without +-- correlating URLs by eye. +-- +-- 'external' is not admitted. Stacks outside this farm are out of scope: the +-- farm's client DID is enrolled as an admin in every full_stack daemon it +-- provisioned (step_dids_grant_farm) and in nothing else, so a stack we did not +-- build would 401 on the first DID upload. Adding the value later is a one-line +-- ALTER; leaving it out now means the schema states the same scope the code +-- does. +ALTER TABLE setup_sessions + ADD COLUMN connection_source TEXT NOT NULL DEFAULT 'platform' + CHECK (connection_source IN ('platform', 'in_farm')), + ADD COLUMN provider_session_id BIGINT NULL + REFERENCES setup_sessions(id) ON DELETE SET NULL; + +-- ON DELETE SET NULL is not a fallback, it IS the orphan mechanism. Deleting a +-- provider is allowed and blocks on nothing, so no handler writes to its +-- dependents; Postgres nulls the link in the same transaction, and +-- +-- connection_source = 'in_farm' AND provider_session_id IS NULL +-- +-- is permanently "the stack this agent connected to is gone". Nothing has to +-- have been running at the moment of deletion and there is no reconciler to +-- drift. RESTRICT would instead pin a provider forever, since there is +-- deliberately no way to remove a single connection. +CREATE INDEX setup_sessions_provider_idx + ON setup_sessions (provider_session_id) + WHERE provider_session_id IS NOT NULL; + +-- ── Backfill ──────────────────────────────────────────────────────────────── +-- Every session that exists today predates the feature, so connection_source's +-- 'platform' default is already right for all of them and needs no UPDATE. +-- +-- did_hosting_did is a different matter. It has been a full_stack output column +-- (the daemon's own DID, step 3d) and is '' on every vta_only row. Its meaning +-- widens here to "the DID of the daemon at did_hosting_control_url", which is +-- true for both modes, and Factory.For now refuses a daemon reporting a DID +-- other than the one recorded. Populating it for existing vta_only rows is what +-- arms that check for sessions built before it existed — an empty value means +-- "no expectation on record" and accepts whatever the daemon claims. +-- +-- The value comes from the platform stack, which is the only daemon any +-- existing vta_only session can have been wired to. Joined on the server URL +-- rather than assumed, so a row pointing at a rebuilt or absent stack is left +-- at '' rather than given a DID that was never its daemon's. +UPDATE setup_sessions AS consumer + SET did_hosting_did = provider.did_hosting_did + FROM setup_sessions AS provider + WHERE consumer.mode = 'vta_only' + AND consumer.did_hosting_did = '' + AND consumer.did_hosting_server_url <> '' + AND provider.mode = 'full_stack' + AND provider.did_hosting_did <> '' + AND provider.did_hosting_server_url = consumer.did_hosting_server_url; From 596e6aa64144ccbf362488a848702bce865fe8d5 Mon Sep 17 00:00:00 2001 From: vthwang Date: Fri, 31 Jul 2026 13:16:13 -0700 Subject: [PATCH 04/10] refactor: resolveSharedInfra becomes resolveProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Names the function after the role rather than after the platform stack, so a provider named by a share code can join later without a second, parallel path to the same three values. Only the platform lookup exists today; the ref parameter arrives with the bundle branch. The judgement is split out of the I/O. providerInfra decides whether a candidate row is usable and touches no database, which makes it testable directly — there were no tests here at all — and means a bundle-named provider will be held to the same readiness bar rather than a second copy of it that drifts out of step. Writing those tests turned up two pre-existing bugs in the path being refactored. Both are fixed here rather than left under new code: The fail-open on a database error reached POST /setup. resolveSharedInfra returned ready=true with a zero sharedInfra when the lookup itself failed, on the stated reasoning that create re-reads the row anyway. It did not re-read — it used those empty values, so a transient DB error could persist a session with no mediator DID and a vta_did_url of "/-vta". A lookup failure is now its own reason, and the two callers part company on it: availability still fails open, because a blip must not blank the create screen, while create refuses, because there the choice is between waiting and provisioning an agent that can never deliver a message. The ServerURL == "" guard was unreachable. It tested the output of DidsURL(), which always prefixes https:// and so is never empty; a provider with no dids hostname yielded "https://.", passed, and got snapshotted onto the session permanently. It now tests the two components the hostname is built from. New sessions also snapshot the provider's daemon DID, which is what arms the audience check from phase 0 going forward — phase 1's migration only covered rows that already existed. It is deliberately not part of the readiness bar: a platform stack provisioned before that column carried this meaning would otherwise stop serving vta_only creation, and an empty value means "no expectation on record", which is exactly the old behaviour. Also repairs a doc comment orphaned by phase 1, which left the domain-type commentary sitting on top of the connection-source block. Signed-off-by: vthwang --- docs/custom-stack-connection-design.md | 29 +++++- internal/config/config.go | 16 +-- internal/handler/provider_infra_test.go | 100 +++++++++++++++++++ internal/handler/setup.go | 125 +++++++++++++++++------- internal/model/setup_session.go | 8 +- 5 files changed, 230 insertions(+), 48 deletions(-) create mode 100644 internal/handler/provider_infra_test.go diff --git a/docs/custom-stack-connection-design.md b/docs/custom-stack-connection-design.md index 147a930..2146ed0 100644 --- a/docs/custom-stack-connection-design.md +++ b/docs/custom-stack-connection-design.md @@ -243,8 +243,30 @@ func (h *SetupHandler) resolveProvider(ref *stackRef) (v sharedInfra, provider * This *reduces* the number of code paths that can wire a session to a mediator, rather than adding one. The platform branch keeps its exact current reasons (`platform_stack_missing` / `platform_stack_not_ready` / -`shared_infra_unconfigured`) and its exact fail-open behaviour on a DB error, so -nothing about the existing path changes observably. +`shared_infra_unconfigured`). + +**Shipped in phase 2**, as `resolveProvider()` — the ref parameter arrives with +the bundle branch. The refactor split the judgement out of the I/O: +`providerInfra(*SetupSession)` decides whether a candidate row is usable and is +pure, so it can be tested directly and so a bundle-named provider is held to +*the same* readiness bar rather than a second copy of it that drifts. + +Two pre-existing bugs surfaced while doing it, both fixed there: + +- **The fail-open on a DB error reached `POST /setup`.** `resolveSharedInfra` + returned `ready=true` with a zero `sharedInfra` when the lookup itself failed, + on the stated reasoning that "create re-reads the row anyway". It did not + re-read: it used those empty values, so a transient database error could + create a session with no mediator DID and a `vta_did_url` of `/-vta`. + The lookup failure is now its own reason, `provider_lookup_failed`, and the + two callers part company on it — `GET /setup/availability` still fails open, + because a blip must not blank the create screen, while `POST /setup` refuses, + because there the choice is between waiting and provisioning a dead agent. +- **The `ServerURL == ""` guard was unreachable.** It tested the output of + `DidsURL()`, which always prefixes `https://` and so is never empty; a + provider row with no dids hostname produced `https://.`, passed the check, and + got snapshotted onto the session permanently. It now tests the two components + the hostname is built from. The bundle branch, in two tiers. **Everything before the code is verified must answer identically**, or the endpoint becomes a directory of which stacks exist @@ -825,7 +847,8 @@ Nothing yet. | §9.7 mediator allow-list | ✅ resolved — open to every DID, no work | | Migration + model (§6) | ✅ phase 1 | | Share code: mint / normalise / validate / compare (§4.1, §4.1.1) | ✅ phase 1 | -| `resolveProvider`, both tiers (§5.1) | ☐ | +| `resolveProvider`, platform path (§5.1) | ✅ phase 2 | +| `resolveProvider`, bundle tiers (§5.1) | ☐ | | `POST /setup/connection/validate` (§5.2) | ☐ | | `connection` on `POST /setup` (§5) | ☐ | | `connection` + `connections[]` in responses (§8) | ☐ | diff --git a/internal/config/config.go b/internal/config/config.go index 1ec0247..e8cecc8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,14 +33,14 @@ type Config struct { // stops new arrivals, and this is what bounds how many arrive before they // think to. See docs/custom-stack-connection-design.md §6.3. MaxStackConnections int - DB DBConfig - K8s K8sConfig - Cloudflare CloudflareConfig - GHCR GHCRConfig - DidHosting DidHostingConfig - WebAuthn WebAuthnConfig - Vault VaultConfig - Monitor MonitorConfig + DB DBConfig + K8s K8sConfig + Cloudflare CloudflareConfig + GHCR GHCRConfig + DidHosting DidHostingConfig + WebAuthn WebAuthnConfig + Vault VaultConfig + Monitor MonitorConfig } // MonitorConfig configures the token-gated /api/v1/monitor/* endpoints polled diff --git a/internal/handler/provider_infra_test.go b/internal/handler/provider_infra_test.go new file mode 100644 index 0000000..0fc6169 --- /dev/null +++ b/internal/handler/provider_infra_test.go @@ -0,0 +1,100 @@ +package handler + +import ( + "testing" + + "github.com/ic3software/vtafarm-api/internal/model" +) + +// runningProvider is a full_stack row that has finished provisioning — the +// state a vta_only session may be wired to. +func runningProvider() *model.SetupSession { + return &model.SetupSession{ + Mode: model.ModeFullStack, + Status: "running", + Domain: "firstperson.dev", + DidsSubdomain: "dids-alice", + MediatorDid: "did:webvh:mediator-alice", + DIDHostingDid: "did:webvh:dids-alice", + } +} + +func TestProviderInfraFromRunningStack(t *testing.T) { + got, reason, detail := providerInfra(runningProvider()) + + if reason != "" { + t.Fatalf("reason = %q (%s), want usable", reason, detail) + } + want := sharedInfra{ + MediatorDid: "did:webvh:mediator-alice", + ServerURL: "https://dids-alice.firstperson.dev", + ControlURL: "https://dids-alice.firstperson.dev", + DaemonDid: "did:webvh:dids-alice", + } + if got != want { + t.Errorf("providerInfra() = %+v, want %+v", got, want) + } +} + +func TestProviderInfraRefusesUnusableStacks(t *testing.T) { + tests := []struct { + name string + mutate func(*model.SetupSession) + wantReason string + }{{ + name: "still provisioning", + mutate: func(s *model.SetupSession) { s.Status = "step_vta_setup" }, + wantReason: reasonPlatformNotReady, + }, { + name: "failed", + mutate: func(s *model.SetupSession) { s.Status = "failed" }, + wantReason: reasonPlatformNotReady, + }, { + // Running, but step 1b never landed. A session created here would carry + // an empty mediator DID and never deliver a message. + name: "running without a mediator DID", + mutate: func(s *model.SetupSession) { s.MediatorDid = "" }, + wantReason: reasonSharedUnconfigured, + }, { + name: "running without a dids hostname", + mutate: func(s *model.SetupSession) { s.DidsSubdomain = ""; s.Domain = "" }, + wantReason: reasonSharedUnconfigured, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + s := runningProvider() + tc.mutate(s) + + got, reason, detail := providerInfra(s) + if reason != tc.wantReason { + t.Fatalf("reason = %q, want %q", reason, tc.wantReason) + } + if detail == "" { + t.Error("a refusal must carry a sentence for the user") + } + if got != (sharedInfra{}) { + t.Errorf("refused, but returned %+v — callers must not be handed partial values", got) + } + }) + } +} + +// A daemon DID is what arms didhosting's audience check, but it is deliberately +// not part of the readiness bar: a platform stack provisioned before the column +// carried this meaning would otherwise stop serving vta_only creation. +func TestProviderInfraAcceptsMissingDaemonDid(t *testing.T) { + s := runningProvider() + s.DIDHostingDid = "" + + got, reason, detail := providerInfra(s) + if reason != "" { + t.Fatalf("reason = %q (%s), want usable", reason, detail) + } + if got.DaemonDid != "" { + t.Errorf("DaemonDid = %q, want empty — no expectation on record", got.DaemonDid) + } + if got.MediatorDid == "" || got.ServerURL == "" { + t.Error("the rest of the values must still come through") + } +} diff --git a/internal/handler/setup.go b/internal/handler/setup.go index 1513ea5..dd63003 100644 --- a/internal/handler/setup.go +++ b/internal/handler/setup.go @@ -272,8 +272,10 @@ func (h *SetupHandler) Create(c *gin.Context) { // It also yields the values the session is built from, so the gate and the // source are the same read — there is no window where the check passes and // the write then uses something else. - infra, ready, _, detail := h.resolveSharedInfra() - if !ready { + // The provider row itself is not needed until a caller can name one other + // than the platform stack; until then the three values are the whole answer. + infra, _, _, detail := h.resolveProvider() + if detail != "" { c.JSON(http.StatusServiceUnavailable, gin.H{"error": detail}) return } @@ -327,10 +329,17 @@ func (h *SetupHandler) Create(c *gin.Context) { // daemon, and this session's DID stays on the one that minted it. DidHostingServerURL: infra.ServerURL, DidHostingControlURL: infra.ControlURL, - VtaImage: req.VtaImage, - AdminDid: req.AdminDid, - Portable: portable, - PreRotationCount: preRotationCount, + // Which daemon that URL is expected to be, so didhosting.Factory.For can + // refuse a host answering with a different DID. Phase 1's migration + // backfilled this for rows created before the column meant anything to + // vta_only; this is what populates it going forward. "" when the + // provider never recorded one, which keeps the previous behaviour of + // accepting whatever the daemon claims. + DIDHostingDid: infra.DaemonDid, + VtaImage: req.VtaImage, + AdminDid: req.AdminDid, + Portable: portable, + PreRotationCount: preRotationCount, } // A single insert: there is no random id left to collide, so the retry loop // that used to wrap this went with unique_id. @@ -403,6 +412,11 @@ const ( reasonPlatformMissing = "platform_stack_missing" reasonPlatformNotReady = "platform_stack_not_ready" reasonSharedUnconfigured = "shared_infra_unconfigured" + // reasonProviderUnknown means the lookup itself failed — a database error, + // not a statement about the stack. The two callers must treat it + // differently, which is why it is a reason rather than a bare error: see + // resolveProvider. + reasonProviderUnknown = "provider_lookup_failed" ) // sharedInfra is what a vta_only session is wired to — read from the platform @@ -421,30 +435,45 @@ type sharedInfra struct { MediatorDid string ServerURL string ControlURL string + // DaemonDid is the DID the daemon at ControlURL reports as its own, taken + // from the provider's row rather than from the daemon itself. Snapshotted + // onto the consumer so didhosting.Factory.For can refuse a host answering + // with somebody else's DID — the token it would receive is signed with the + // farm's admin key and replayable wherever that DID is enrolled. + // + // Not part of the readiness gate below. A provider that never recorded one + // yields "", which means "no expectation on record" and behaves exactly as + // this did before the field existed — deliberately, so a platform stack + // built before the column was populated does not suddenly refuse to serve. + DaemonDid string } -// resolveSharedInfra reports whether the mediator and DID host that every -// vta_only session points at are actually usable, and returns their values. +// resolveProvider finds the stack a vta_only session will be wired to, and +// reports whether it is usable. // -// That shared infrastructure IS the platform stack (design §3.3) — a vta_only -// agent is only the VTA, wired to a mediator and DID-hosting daemon it does not -// run itself. Creating one before those exist produces an agent that can never -// deliver a message. +// Today that is always the platform stack (design §3.3) — a vta_only agent is +// only the VTA, pointed at a mediator and DID-hosting daemon it does not run +// itself, so creating one before those exist produces an agent that can never +// deliver a message. Naming this after the *role* rather than after the +// platform stack is what lets a bundle-named provider join later without a +// second, parallel path to the same values. +// +// reason is "" exactly when the returned sharedInfra is usable. The provider row +// is returned alongside it because callers need more than the three values — +// the connection has to be recorded against a row, not a URL. // // full_stack is unaffected: it provisions its own mediator and DID host. -func (h *SetupHandler) resolveSharedInfra() (v sharedInfra, ready bool, reason, detail string) { +func (h *SetupHandler) resolveProvider() (v sharedInfra, provider *model.SetupSession, reason, detail string) { const missing = "VTA-only agents need the platform stack — the shared mediator and DID hosting they connect to. " + "An admin has to create it before any VTA-only agent can be provisioned." + const unknown = "Couldn't check the platform stack just now. Please try again." domain, err := h.platformDomain() if err != nil { - // Can't tell — fail open rather than blocking every create on a - // transient DB read, the same way capacity does. Create re-reads the - // row anyway and refuses if the values it needs aren't there. - return v, true, "", "" + return v, nil, reasonProviderUnknown, unknown } if domain == nil { - return v, false, reasonPlatformMissing, missing + return v, nil, reasonPlatformMissing, missing } var session model.SetupSession @@ -452,22 +481,31 @@ func (h *SetupHandler) resolveSharedInfra() (v sharedInfra, ready bool, reason, if errors.Is(err, gorm.ErrRecordNotFound) { // The domains row outlives its session: the name is still ours, but // nothing is running on it. - return v, false, reasonPlatformMissing, missing + return v, nil, reasonPlatformMissing, missing } if err != nil { - return v, true, "", "" + return v, nil, reasonProviderUnknown, unknown } - if session.Status != "running" { - return v, false, reasonPlatformNotReady, - "The platform stack — the shared mediator and DID hosting VTA-only agents connect to — is still being set up. " + - "Try again once it's running." + + v, reason, detail = providerInfra(&session) + if reason != "" { + return sharedInfra{}, nil, reason, detail } + return v, &session, "", "" +} - v = sharedInfra{ - MediatorDid: session.MediatorDid, - // The stack's own daemon. Both roles on one host — see sharedInfra. - ServerURL: session.DidsURL(), - ControlURL: session.DidsURL(), +// providerInfra turns a candidate provider row into what a vta_only session +// wires itself to, or the reason it cannot be used. +// +// Split out from the lookup above because it is the half with all the +// judgement in it and none of the I/O, so it can be tested directly — and +// because a provider named by a share code has to be held to exactly the same +// readiness bar as the platform stack. Two copies of that bar would drift. +func providerInfra(s *model.SetupSession) (v sharedInfra, reason, detail string) { + if s.Status != "running" { + return v, reasonPlatformNotReady, + "The platform stack — the shared mediator and DID hosting VTA-only agents connect to — is still being set up. " + + "Try again once it's running." } // Running, yet its mediator DID is missing. This used to mean "an admin @@ -476,13 +514,27 @@ func (h *SetupHandler) resolveSharedInfra() (v sharedInfra, ready bool, reason, // narrow, transient one — a stack marked running whose 1b output never // landed. Kept rather than dropped because a session created here would // still carry an empty mediator DID and never deliver a message. - if v.MediatorDid == "" || v.ServerURL == "" { - return sharedInfra{}, false, reasonSharedUnconfigured, + // + // The hostname is tested through its two components rather than through + // DidsURL(). That builder always prefixes "https://", so its result is never + // empty and a row with no dids hostname used to pass this check and yield + // "https://." — a URL that resolves to nothing, is snapshotted onto the + // session forever, and fails much later. + // + // DaemonDid is deliberately not tested — see sharedInfra. + if s.MediatorDid == "" || s.DidsSubdomain == "" || s.Domain == "" { + return sharedInfra{}, reasonSharedUnconfigured, "The platform stack is running but hasn't published its mediator DID yet. " + "Try again shortly; if it persists, an admin should check the stack." } - return v, true, "", "" + return sharedInfra{ + MediatorDid: s.MediatorDid, + // The stack's own daemon. Both roles on one host — see sharedInfra. + ServerURL: s.DidsURL(), + ControlURL: s.DidsURL(), + DaemonDid: s.DIDHostingDid, + }, "", "" } // capacityAllows gates a create on remaining cluster capacity for mode. It @@ -544,7 +596,14 @@ func (h *SetupHandler) Availability(c *gin.Context) { // The shared mediator and DID host is a hard dependency of vta_only, not a // capacity question — so it overrides the fail-open above rather than // sitting alongside it. full_stack runs its own and is never gated on it. - if _, ready, reason, detail := h.resolveSharedInfra(); !ready { + // + // reasonProviderUnknown is the exception, and the two callers of + // resolveProvider part company here: a database read that failed says + // nothing about the stack, so reporting it as unavailable would blank the + // create screen on a blip. It fails open, like capacity above. POST /setup + // refuses on the same reason, because there it is the difference between + // waiting and provisioning an agent with no mediator DID at all. + if _, _, reason, detail := h.resolveProvider(); reason != "" && reason != reasonProviderUnknown { vtaOnly.Available = false vtaOnly.Reason, vtaOnly.Detail = reason, detail } diff --git a/internal/model/setup_session.go b/internal/model/setup_session.go index 32690e0..aba7078 100644 --- a/internal/model/setup_session.go +++ b/internal/model/setup_session.go @@ -12,8 +12,6 @@ const ( ModeFullStack = "full_stack" ) -// Where a session's hostnames come from. Orthogonal to Mode: a session is -// vta_only or full_stack, and independently managed, custom or platform. // Where a vta_only session's mediator and DID host came from. Orthogonal to // both Mode and DomainType, and meaningless for full_stack, which provisions // its own. @@ -31,6 +29,8 @@ const ( ConnectionInFarm = "in_farm" ) +// Where a session's hostnames come from. Orthogonal to Mode: a session is +// vta_only or full_stack, and independently managed, custom or platform. const ( // DomainManaged is the default — labels derived from the user's chosen // name in our own zone (vta-.firstperson.dev). DomainID is NULL. @@ -112,8 +112,8 @@ type SetupSession struct { // ON DELETE SET NULL — whether that provider still exists at all. ConnectionSource string `gorm:"column:connection_source;not null;default:platform" json:"connection_source"` ProviderSessionID *uint `gorm:"column:provider_session_id" json:"-"` - Portable bool `gorm:"not null;default:true" json:"portable"` - PreRotationCount int `gorm:"not null;default:1" json:"pre_rotation_count"` + Portable bool `gorm:"not null;default:true" json:"portable"` + PreRotationCount int `gorm:"not null;default:1" json:"pre_rotation_count"` // Image used for the vta-setup K8s Job VtaImage string `gorm:"not null;default:''" json:"vta_image,omitempty"` // Output populated after vta setup runs From 9f151a291cd6ff1a6f6c9da82d027fbbbbd78000 Mon Sep 17 00:00:00 2001 From: vthwang Date: Fri, 31 Jul 2026 13:21:51 -0700 Subject: [PATCH 05/10] feat: let a full_stack owner share their stack by code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provider half of stack connections. A full_stack owner mints a share code and hands out a bundle; nothing consumes it yet, so this is inert until the create path lands. PUT /setup/:id/sharing takes one action — enable, rotate or disable — rather than an enabled bool plus a rotate bool, which would make {"enabled": false, "rotate": true} mean nothing in particular. enable is idempotent: turning on something already on returns the current code instead of silently invalidating every bundle already handed out. Replacing one is what rotate is for, and it asks explicitly. All three gate joining, not membership. None of them disconnect anything, and the handler deliberately writes to no other user's row: agents already connected keep running, and the stronger lever is deleting the stack, which stops everyone. The confirms in the UI have to say so. enable and rotate require a running stack that has published its mediator and DID-hosting identifiers, so "sharing is on" never means "on, but every bundle from it is refused". The platform stack is refused outright: it is already the default for every vta_only agent, reached by a path that sends no bundle, and a second way to arrive at the same place would be a share code nobody needs but anybody could leak. The bundle carries the three display values alongside the two that matter, and buildConnectionBundle returns nil rather than a hollow struct when a stack is not shareable — the UI must not be able to offer a "copy this" for something that would be refused on arrival. Responses grow what the topology needs: full_stack rows report `shared` plus the dependent list that the delete confirmation will name, vta_only rows report where their mediator came from and whether that stack still exists. The admin session list gets the same, batched over the page so 20 rows do not become 20 round trips — support's first question about a broken agent is whose infrastructure it is on, and the answer was otherwise a URL-to-URL comparison across two queries. listConnections returns nil rather than an empty slice when its read fails. "Nothing depends on this" is the one answer that would mislead someone about to delete a stack. Both routes are in openapi.yaml; the spec parses and has no dangling refs. Signed-off-by: vthwang --- docs/custom-stack-connection-design.md | 3 +- internal/apidocs/openapi.yaml | 189 +++++++++++++++++++++++ internal/handler/setup.go | 29 ++++ internal/handler/setup_admin.go | 67 +++++++++ internal/handler/setup_fullstack.go | 11 ++ internal/handler/setup_sharing.go | 198 +++++++++++++++++++++++++ internal/handler/setup_sharing_test.go | 129 ++++++++++++++++ internal/router/router.go | 7 + 8 files changed, 632 insertions(+), 1 deletion(-) create mode 100644 internal/handler/setup_sharing.go create mode 100644 internal/handler/setup_sharing_test.go diff --git a/docs/custom-stack-connection-design.md b/docs/custom-stack-connection-design.md index 2146ed0..bf5fde0 100644 --- a/docs/custom-stack-connection-design.md +++ b/docs/custom-stack-connection-design.md @@ -851,7 +851,8 @@ Nothing yet. | `resolveProvider`, bundle tiers (§5.1) | ☐ | | `POST /setup/connection/validate` (§5.2) | ☐ | | `connection` on `POST /setup` (§5) | ☐ | -| `connection` + `connections[]` in responses (§8) | ☐ | +| Sharing toggle: `PUT /setup/:id/sharing` (+ admin twin) | ✅ phase 3 | +| `connection` + `connections[]` in responses (§8) | ✅ phase 3 | | Availability split (§10.1) | ☐ | | `RegisterDid` failure visible (§9.1) | ✅ phase 0 | | Expected-audience check (§9.4) | ✅ phase 0 | diff --git a/internal/apidocs/openapi.yaml b/internal/apidocs/openapi.yaml index ff93901..43f2fd6 100644 --- a/internal/apidocs/openapi.yaml +++ b/internal/apidocs/openapi.yaml @@ -635,6 +635,82 @@ components: type: string description: full_stack only. 3c — shown once for offline backup. + ConnectionBundle: + type: object + description: | + What a `full_stack` owner copies and hands to somebody else out of band, + and what that person pastes into the VTA-only create form. + + Only `stack` and `code` are load-bearing. The connection is resolved by + looking `stack` up in this farm's own database and checking `code` + against it; the values a session is actually built from come from that + row, never from this document. The remaining fields are shown to the + recipient so they can see whose stack they are joining before they + commit, and compared on arrival so a bundle displaying values the stack + no longer has is refused rather than quietly connecting to different + ones. + + Nothing here ever becomes a URL this server connects to. + properties: + v: + type: integer + example: 1 + kind: + type: string + example: "vtafarm.stack-connection" + farm: + type: string + description: The farm this bundle is for. A bundle from another deployment is refused. + example: "firstperson.dev" + stack: + type: string + description: The provider session's `vta_name` — its public identifier everywhere else in this API. + example: "alice" + code: + type: string + description: | + The share code, grouped for display. Crockford base32 with a check + symbol, so a mistyped character is caught before it is sent. + example: "K7M2-9XQP-4B8W-3NRT" + mediator_did: + type: string + example: "did:webvh:...:mediator-alice.firstperson.dev" + did_hosting_server_url: + type: string + example: "https://dids-alice.firstperson.dev" + did_hosting_did: + type: string + description: The DID the daemon at that URL reports as its own. + example: "did:webvh:...:dids-alice.firstperson.dev" + + StackConnection: + type: object + description: One agent connected to a stack. Name and status only — these sessions belong to other users. + properties: + vta_name: { type: string, example: "bob-vta" } + status: { type: string, example: "running" } + + SharingResponse: + type: object + properties: + shared: + type: boolean + description: Whether this stack currently accepts new connections. + connection: + allOf: + - $ref: "#/components/schemas/ConnectionBundle" + description: | + Absent — not empty — when the stack is not shareable, so the UI + cannot offer a bundle that would be refused the moment it was + pasted. + connections: + type: array + items: + $ref: "#/components/schemas/StackConnection" + description: | + Agents already connected. Unaffected by any of the three actions, + and the list the delete confirmation names. + CreateSetupResponse: type: object description: | @@ -4514,3 +4590,116 @@ paths: application/json: schema: $ref: "#/components/schemas/Error" + + /api/v1/setup/{id}/sharing: + put: + summary: Share this stack, or stop sharing it + description: | + full_stack only. Mints, replaces or clears the **share code** that lets + somebody else's VTA-only agent connect to this stack's mediator and DID + hosting. + + The code is the only gate, and it gates *joining* rather than + membership: + + | action | effect | + | --- | --- | + | `enable` | mint a code. Idempotent — an already-shared stack returns its current code rather than silently invalidating bundles already handed out | + | `rotate` | replace the code. Every bundle already shared stops working | + | `disable` | clear it. No new connections | + + **None of the three disconnect anything.** Agents already connected keep + running, and there is deliberately no way to remove one — the stronger + lever is deleting the stack, which stops everyone. See + `docs/custom-stack-connection-design.md` §7.4. + + `enable` and `rotate` require a stack that is `running` and has + published its mediator and DID-hosting identifiers, so that "sharing is + on" never means "on, but every bundle from it is refused". + + The platform stack is refused: it is already the default for every + VTA-only agent, reached by a path that sends no bundle at all. + tags: [User] + security: + - CookieAuthUser: [] + parameters: + - name: id + in: path + required: true + schema: + type: string + example: "alice" + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [action] + properties: + action: + type: string + enum: [enable, rotate, disable] + responses: + "200": + description: Sharing updated + content: + application/json: + schema: + $ref: "#/components/schemas/SharingResponse" + "400": + description: Not a full_stack session, is the platform stack, or a bad action + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "401": + description: Missing or invalid token + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "404": + description: Session not found + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "409": + description: Stack is not running, or has not published its identifiers yet + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + + /api/v1/admin/setup-sessions/{id}/sharing: + put: + summary: Share a stack, or stop sharing it (admin) + description: | + Admin-cookie twin of `PUT /setup/{id}/sharing`, reaching any user's + session. Exists for support: a stack whose owner has lost access can + still be taken out of circulation. + tags: [Admin] + security: [{ CookieAuthAdmin: [] }] + parameters: + - { name: id, in: path, required: true, schema: { type: string } } + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [action] + properties: + action: { type: string, enum: [enable, rotate, disable] } + responses: + "200": + description: Sharing updated + content: + application/json: + schema: + $ref: "#/components/schemas/SharingResponse" + "400": { description: Not shareable, content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } } + "401": { description: Missing or invalid token, content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } } + "404": { description: Session not found, content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } } + "409": { description: Stack not ready, content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } } diff --git a/internal/handler/setup.go b/internal/handler/setup.go index dd63003..f0b2a24 100644 --- a/internal/handler/setup.go +++ b/internal/handler/setup.go @@ -732,9 +732,38 @@ func (h *SetupHandler) Get(c *gin.Context) { if session.ErrorMsg != "" { resp["error_msg"] = session.ErrorMsg } + h.describeConnection(resp, &session) c.JSON(http.StatusOK, resp) } +// describeConnection adds which stack a vta_only session is wired to. +// +// The first question when an agent misbehaves is whose infrastructure it is +// on, and until now the answer was a bare mediator DID. `provider` names the +// stack; its absence on an in_farm session is not missing data but the fact +// that the stack was deleted — see model.IsOrphaned. +func (h *SetupHandler) describeConnection(resp gin.H, s *model.SetupSession) { + if s.IsFullStack() { + return + } + resp["connection_source"] = s.ConnectionSource + if s.ConnectionSource != model.ConnectionInFarm { + return + } + if s.ProviderSessionID == nil { + // The agent keeps running — nothing in a provider teardown touches the + // consumer's namespace — but its DID no longer resolves and its + // mediator is gone. Reported as a distinct fact rather than as a status, + // because nothing about this session's own pipeline failed. + resp["provider_gone"] = true + return + } + var provider model.SetupSession + if err := h.db.Select("vta_name").First(&provider, *s.ProviderSessionID).Error; err == nil { + resp["provider"] = provider.VtaName + } +} + // DELETE /api/v1/setup/:id func (h *SetupHandler) Delete(c *gin.Context) { publicID := c.Param("id") diff --git a/internal/handler/setup_admin.go b/internal/handler/setup_admin.go index e3be659..f1b1280 100644 --- a/internal/handler/setup_admin.go +++ b/internal/handler/setup_admin.go @@ -97,6 +97,51 @@ func (h *SetupHandler) AdminListSessions(c *gin.Context) { } } + // Which stack each vta_only row on this page connects to, and how many rows + // connect to each full_stack on it. Support's first question about a broken + // agent is whose infrastructure it is on, and the answer is otherwise a + // URL-to-URL comparison across two queries. + // + // Batched over the page for the same reason the owner lookup above is: 20 + // rows must not become 20 round trips. + providerNames := make(map[uint]string) + connectionCounts := make(map[uint]int64) + { + providerIDs := make([]uint, 0, len(sessions)) + fullStackIDs := make([]uint, 0, len(sessions)) + for _, s := range sessions { + if s.ProviderSessionID != nil { + providerIDs = append(providerIDs, *s.ProviderSessionID) + } + if s.IsFullStack() { + fullStackIDs = append(fullStackIDs, s.ID) + } + } + if len(providerIDs) > 0 { + var providers []model.SetupSession + if err := h.db.Select("id, vta_name").Where("id IN ?", providerIDs).Find(&providers).Error; err == nil { + for _, p := range providers { + providerNames[p.ID] = p.VtaName + } + } + } + if len(fullStackIDs) > 0 { + var counts []struct { + ProviderSessionID uint + Count int64 + } + if err := h.db.Model(&model.SetupSession{}). + Select("provider_session_id, COUNT(*) AS count"). + Where("provider_session_id IN ?", fullStackIDs). + Group("provider_session_id"). + Find(&counts).Error; err == nil { + for _, c := range counts { + connectionCounts[c.ProviderSessionID] = c.Count + } + } + } + } + type sessionItem struct { // The numeric PK, for ordering only — never an address. vta_name is what // the routes take, so there is no separate identifier field here. @@ -117,6 +162,17 @@ func (h *SetupHandler) AdminListSessions(c *gin.Context) { DidsImage string `json:"dids_image,omitempty"` VtcImage string `json:"vtc_image,omitempty"` CreatedAt string `json:"created_at"` + // vta_only: where its mediator and DID host came from. Provider names + // the stack when that is in_farm; ProviderGone says the stack was + // deleted, which is why no name is available rather than a lookup having + // failed. + ConnectionSource string `json:"connection_source,omitempty"` + Provider string `json:"provider,omitempty"` + ProviderGone bool `json:"provider_gone,omitempty"` + // full_stack: whether it currently accepts connections, and how many it + // already has. Both matter before deleting one. + Shared bool `json:"shared,omitempty"` + ConnectionCount int64 `json:"connection_count,omitempty"` } items := make([]sessionItem, len(sessions)) for i, s := range sessions { @@ -136,6 +192,17 @@ func (h *SetupHandler) AdminListSessions(c *gin.Context) { VtcImage: s.VtcImage, CreatedAt: s.CreatedAt.UTC().Format("2006-01-02T15:04:05Z"), } + if s.IsFullStack() { + items[i].Shared = s.IsShared() + items[i].ConnectionCount = connectionCounts[s.ID] + continue + } + items[i].ConnectionSource = s.ConnectionSource + if s.ProviderSessionID != nil { + items[i].Provider = providerNames[*s.ProviderSessionID] + } else { + items[i].ProviderGone = s.IsOrphaned() + } } c.JSON(http.StatusOK, gin.H{ diff --git a/internal/handler/setup_fullstack.go b/internal/handler/setup_fullstack.go index 2ed14e0..645b50f 100644 --- a/internal/handler/setup_fullstack.go +++ b/internal/handler/setup_fullstack.go @@ -256,6 +256,17 @@ func (h *SetupHandler) getFullStack(c *gin.Context, session *model.SetupSession) "vtc_image": session.VtcImage, "created_at": session.CreatedAt, "updated_at": session.UpdatedAt, + // Whether this stack is currently shared, and by what. The bundle is + // absent — not merely empty — when it is not shareable, so the UI cannot + // offer one that would be refused the moment somebody pasted it. + "shared": session.IsShared(), + // Other people's agents connected to this stack. Deleting it is allowed + // and breaks every one of them, so this list is what lets the delete + // confirmation name them. + "connections": h.listConnections(session.ID), + } + if bundle := buildConnectionBundle(session, h.clusterDomain); bundle != nil { + resp["connection"] = bundle } resp["dids_enroll_used"] = session.DidsEnrollUsed diff --git a/internal/handler/setup_sharing.go b/internal/handler/setup_sharing.go new file mode 100644 index 0000000..ac410de --- /dev/null +++ b/internal/handler/setup_sharing.go @@ -0,0 +1,198 @@ +package handler + +import ( + "net/http" + + "github.com/gin-gonic/gin" + + "github.com/ic3software/vtafarm-api/internal/model" + "github.com/ic3software/vtafarm-api/internal/setup" +) + +// The provider half of stack connections: a full_stack owner mints a share code +// and hands out the bundle a vta_only session pastes on create. Design: +// docs/custom-stack-connection-design.md §4. + +// connectionBundleKind and connectionBundleVersion tag the bundle so that +// pasting the wrong thing — a DID, a URL, a JWT, a bundle from a future +// version — fails with a sentence rather than five steps later with a 500. +const ( + connectionBundleKind = "vtafarm.stack-connection" + connectionBundleVersion = 1 +) + +// connectionBundle is what a provider hands out and a consumer pastes. +// +// Only Stack and Code are load-bearing: the connection is resolved by looking +// Stack up in this farm's own database and checking Code against it, and the +// three values a session is actually built from come from that row. The rest is +// shown to the recipient so they can see whose stack they are joining before +// they commit, and compared on arrival so a bundle displaying values the stack +// no longer has is refused rather than silently connecting to different ones. +// +// That split is the reason this feature has no SSRF surface: nothing here ever +// becomes a URL this server connects to. See design §3. +type connectionBundle struct { + Version int `json:"v"` + Kind string `json:"kind"` + Farm string `json:"farm"` + Stack string `json:"stack"` + Code string `json:"code"` + + MediatorDid string `json:"mediator_did"` + DidHostingServerURL string `json:"did_hosting_server_url"` + DidHostingDid string `json:"did_hosting_did"` +} + +// buildConnectionBundle renders a shared, ready stack as a bundle. Returns nil +// when the stack is not currently shareable, so a caller can drop the field +// rather than offer a bundle that would be refused on arrival. +func buildConnectionBundle(s *model.SetupSession, farm string) *connectionBundle { + if !s.IsShared() { + return nil + } + return &connectionBundle{ + Version: connectionBundleVersion, + Kind: connectionBundleKind, + Farm: farm, + Stack: s.VtaName, + Code: setup.GroupShareCode(*s.ShareCode), + MediatorDid: s.MediatorDid, + DidHostingServerURL: s.DidsURL(), + DidHostingDid: s.DIDHostingDid, + } +} + +// connectionSummary is one entry in a provider's dependent list: another user's +// agent connected to this stack. +// +// Name and status only. These sessions belong to other users, and the provider's +// legitimate interest is knowing what deleting their stack would break — not who +// owns it or how it is configured. +type connectionSummary struct { + VtaName string `json:"vta_name"` + Status string `json:"status"` +} + +// listConnections returns the sessions connected to a provider, oldest first. +// +// Not decoration: deleting this stack is allowed and breaks every one of them +// (design §7.2), so the list is the entire mitigation — it is what lets the UI +// name them in the delete confirmation. +func (h *SetupHandler) listConnections(providerID uint) []connectionSummary { + var rows []model.SetupSession + if err := h.db. + Where("provider_session_id = ?", providerID). + Order("created_at ASC"). + Find(&rows).Error; err != nil { + // A failed read must not blank the list into "nothing depends on this", + // which is the one answer that would mislead someone about to delete. + // nil renders as absent rather than as an empty list. + return nil + } + out := make([]connectionSummary, len(rows)) + for i, r := range rows { + out[i] = connectionSummary{VtaName: r.VtaName, Status: r.Status} + } + return out +} + +type sharingRequest struct { + // enable mints a code, disable clears it, rotate replaces it. One field + // rather than an enabled bool plus a rotate bool, which would make + // {"enabled": false, "rotate": true} mean nothing in particular. + Action string `json:"action" binding:"required,oneof=enable disable rotate"` +} + +// PUT /api/v1/setup/:id/sharing +func (h *SetupHandler) SetSharing(c *gin.Context) { + if s := h.userSession(c); s != nil { + h.setSharing(c, s) + } +} + +// AdminSetSharing — the admin-cookie twin, reaching any user's session. +func (h *SetupHandler) AdminSetSharing(c *gin.Context) { + if s := h.adminSession(c); s != nil { + h.setSharing(c, s) + } +} + +func (h *SetupHandler) setSharing(c *gin.Context, session *model.SetupSession) { + var req sharingRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + + if !session.IsFullStack() { + c.JSON(http.StatusBadRequest, gin.H{ + "error": "only a full-stack session can be shared — a VTA-only agent runs no mediator or DID host of its own"}) + return + } + // The platform stack is reached by the default path, which sends no bundle + // at all. Giving it a code would produce a second way to arrive at the same + // place, and a share code that nobody needs but anybody could leak. + if session.DomainType == model.DomainPlatform { + c.JSON(http.StatusBadRequest, gin.H{ + "error": "the platform stack is already the default for every VTA-only agent and is not shared by code"}) + return + } + + if req.Action == "disable" { + if err := h.db.Model(session).Update("share_code", nil).Error; err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to update sharing"}) + return + } + session.ShareCode = nil + // Deliberately no teardown of existing connections. The code gates + // joining, not membership: sessions already connected keep running, and + // there is no way to remove one (design §7.4). + c.JSON(http.StatusOK, gin.H{"shared": false, "connections": h.listConnections(session.ID)}) + return + } + + // enable and rotate both need a stack that can actually serve a connection. + // Checked before minting so that "sharing is on" never means "on, but any + // bundle from it is refused". + if session.Status != "running" { + c.JSON(http.StatusConflict, gin.H{ + "error": "this stack is still being set up — it can be shared once it's running"}) + return + } + if session.MediatorDid == "" || session.DIDHostingDid == "" { + c.JSON(http.StatusConflict, gin.H{ + "error": "this stack hasn't published its mediator and DID hosting identifiers yet — try again shortly"}) + return + } + + // enable is idempotent: turning on something already on returns the current + // code rather than silently invalidating every bundle already handed out. + // Replacing one is what rotate is for, and it asks explicitly. + if req.Action == "enable" && session.ShareCode != nil && *session.ShareCode != "" { + c.JSON(http.StatusOK, gin.H{ + "shared": true, + "connection": buildConnectionBundle(session, h.clusterDomain), + "connections": h.listConnections(session.ID), + }) + return + } + + code, err := setup.NewShareCode() + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to generate a share code"}) + return + } + stored := setup.NormalizeShareCode(code) + if err := h.db.Model(session).Update("share_code", stored).Error; err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to update sharing"}) + return + } + session.ShareCode = &stored + + c.JSON(http.StatusOK, gin.H{ + "shared": true, + "connection": buildConnectionBundle(session, h.clusterDomain), + "connections": h.listConnections(session.ID), + }) +} diff --git a/internal/handler/setup_sharing_test.go b/internal/handler/setup_sharing_test.go new file mode 100644 index 0000000..42c8663 --- /dev/null +++ b/internal/handler/setup_sharing_test.go @@ -0,0 +1,129 @@ +package handler + +import ( + "testing" + + "github.com/ic3software/vtafarm-api/internal/model" + "github.com/ic3software/vtafarm-api/internal/setup" +) + +func sharedStack() *model.SetupSession { + code := "K7M29XQP4B8W3NR" + return &model.SetupSession{ + Mode: model.ModeFullStack, + Status: "running", + VtaName: "alice", + Domain: "firstperson.dev", + DidsSubdomain: "dids-alice", + MediatorDid: "did:webvh:mediator-alice", + DIDHostingDid: "did:webvh:dids-alice", + ShareCode: &code, + } +} + +func TestBuildConnectionBundle(t *testing.T) { + got := buildConnectionBundle(sharedStack(), "firstperson.dev") + if got == nil { + t.Fatal("a shared, running stack must produce a bundle") + } + + want := connectionBundle{ + Version: connectionBundleVersion, + Kind: connectionBundleKind, + Farm: "firstperson.dev", + Stack: "alice", + Code: "K7M2-9XQP-4B8W-3NR", + MediatorDid: "did:webvh:mediator-alice", + DidHostingServerURL: "https://dids-alice.firstperson.dev", + DidHostingDid: "did:webvh:dids-alice", + } + if *got != want { + t.Errorf("bundle = %+v\nwant %+v", *got, want) + } +} + +// The code is stored normalised and displayed grouped. A recipient must be able +// to paste the displayed form straight back and have it match. +func TestBundleCodeRoundTripsToStored(t *testing.T) { + s := sharedStack() + bundle := buildConnectionBundle(s, "firstperson.dev") + + if !setup.ShareCodeMatches(bundle.Code, *s.ShareCode) { + t.Errorf("displayed code %q does not match stored %q", bundle.Code, *s.ShareCode) + } +} + +// Absent, not empty: a bundle that would be refused on arrival must not be +// offered at all, or the UI shows a green "copy this" for something broken. +func TestBuildConnectionBundleRefusesUnshareableStacks(t *testing.T) { + empty := "" + tests := []struct { + name string + mutate func(*model.SetupSession) + }{ + {"not shared", func(s *model.SetupSession) { s.ShareCode = nil }}, + {"share code cleared to empty", func(s *model.SetupSession) { s.ShareCode = &empty }}, + {"not running", func(s *model.SetupSession) { s.Status = "step_dids_p1" }}, + {"no mediator DID yet", func(s *model.SetupSession) { s.MediatorDid = "" }}, + {"no daemon DID yet", func(s *model.SetupSession) { s.DIDHostingDid = "" }}, + {"vta_only cannot provide", func(s *model.SetupSession) { s.Mode = model.ModeVtaOnly }}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + s := sharedStack() + tc.mutate(s) + if got := buildConnectionBundle(s, "firstperson.dev"); got != nil { + t.Errorf("expected no bundle, got %+v", *got) + } + }) + } +} + +// IsShared is the readiness bar the bundle builder and the create-time lookup +// both lean on, so its edges are worth pinning directly. +func TestIsShared(t *testing.T) { + if !sharedStack().IsShared() { + t.Error("a running, shared full_stack must report as shared") + } + + s := sharedStack() + s.ShareCode = nil + if s.IsShared() { + t.Error("a stack with no share code must not report as shared") + } +} + +// A vta_only session deploys neither a mediator nor a DID host, so it can never +// be a provider however its columns are set. +func TestVtaOnlyIsNeverShared(t *testing.T) { + s := sharedStack() + s.Mode = model.ModeVtaOnly + if s.IsShared() { + t.Error("a vta_only session must never report as shared") + } +} + +func TestIsOrphaned(t *testing.T) { + providerID := uint(7) + tests := []struct { + name string + source string + pid *uint + want bool + }{ + {"connected in farm", model.ConnectionInFarm, &providerID, false}, + {"provider deleted", model.ConnectionInFarm, nil, true}, + // A platform session never had a provider row, so a nil link there says + // nothing was ever deleted. + {"platform default", model.ConnectionPlatform, nil, false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + s := &model.SetupSession{ConnectionSource: tc.source, ProviderSessionID: tc.pid} + if got := s.IsOrphaned(); got != tc.want { + t.Errorf("IsOrphaned() = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/internal/router/router.go b/internal/router/router.go index 5d54368..6b001c7 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -160,6 +160,9 @@ func Setup( adminAuth.POST("/admin/setup-sessions/:id/dids/enroll-ack", sh.AdminAckDidsEnroll) adminAuth.POST("/admin/setup-sessions/:id/vtc/reissue-install", sh.AdminReissueVtcInstall) adminAuth.POST("/admin/setup-sessions/:id/vtc/install-ack", sh.AdminAckVtcInstall) + // Admin twin of PUT /setup/:id/sharing, for support: a stack whose owner + // has lost access to it can still be taken out of circulation. + adminAuth.PUT("/admin/setup-sessions/:id/sharing", sh.AdminSetSharing) // The farm's own flagship stack at vta.{CLUSTER_DOMAIN} and friends — // the mediator and DID host vta_only sessions point at. Created whole // (domain + DNS + session) by one action; the only route that can mint @@ -233,6 +236,10 @@ func Setup( userAuth.POST("/setup/:id/dids/enroll-ack", sh.AckDidsEnroll) userAuth.POST("/setup/:id/vtc/reissue-install", sh.ReissueVtcInstall) userAuth.POST("/setup/:id/vtc/install-ack", sh.AckVtcInstall) + // Mint, replace or clear the share code that lets someone else's + // VTA-only agent connect to this full stack. The code is the only gate: + // clearing it stops new connections and leaves existing ones running. + userAuth.PUT("/setup/:id/sharing", sh.SetSharing) } // Domains — a zone the user owns, verified on its own before any session From 30f7f25d0fa444a6c441ec6d34bb9dbef7129469 Mon Sep 17 00:00:00 2001 From: vthwang Date: Fri, 31 Jul 2026 13:28:37 -0700 Subject: [PATCH 06/10] feat: connect a vta_only agent to a stack named by a share code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The consumer half. POST /setup takes an optional connection bundle and wires the agent to the stack it names instead of the platform one; the backend is end-to-end after this. Resolution runs in two tiers, and the split is the point. Everything decided before the share code is verified answers identically — no such stack, a stack that never shared, one that turned sharing off, a rotated code, and a code that is simply wrong all return invalid_bundle with one sentence and one status. Distinguishing them would turn this into a directory of which stacks exist and which are shared, which is exactly what the share code exists to prevent, and from the holder's side they are one fact with one next step: ask for a current bundle. A failed database read lands there too, for the same reason — the alternative leaks that a name exists whenever a query happens to fail. Once a current code is proved, specificity is free, so the remaining refusals say precisely what is wrong: not running, changed since it was copied, or at its connection limit. The bundle stays evidence rather than configuration. The stack is found in this farm's own database and every value comes off that row, so a pasted URL never becomes a socket — no SSRF surface, and the self-asserted-audience relay stays closed by construction rather than by a check someone has to remember. POST /setup/connection/validate runs the same checks and creates nothing. Without it the create form's confirmation card would be rendered from the pasted JSON — showing a confident "connecting to alice" for a bundle whose code is garbage, discovered only after naming the agent, choosing an image and pressing Create. It is explicitly not authoritative: create re-runs everything, because a stack can stop running, rotate or fill up in between. connection_source is keyed on whether a bundle was sent, not on whether resolveProvider returned a row — it returns one on the platform path too, and a platform session must keep provider_session_id NULL or model.IsOrphaned would eventually read it as a provider that had been deleted. Availability splits as decided: available still describes the default path, and custom_target_allowed survives every reason except at_capacity, because the platform stack is a default rather than a prerequisite. A farm with no platform stack can still create a vta_only agent against a customer stack. A connection on a full_stack request is refused rather than ignored — that mode provisions its own mediator and DID host, and silently dropping the field would let someone believe their stack was wired to somebody else's. Tests cover every refusal that happens before the database is touched, which also asserts that malformed input never reaches a query. The tiers that need real rows are exercised in phase 8. Signed-off-by: vthwang --- docs/custom-stack-connection-design.md | 12 +- internal/apidocs/openapi.yaml | 126 +++++++++++++++- internal/cloudflare/client.go | 4 +- internal/handler/connection_resolve_test.go | 139 ++++++++++++++++++ internal/handler/setup.go | 115 ++++++++++++--- internal/handler/setup_sharing.go | 150 ++++++++++++++++++++ internal/router/router.go | 8 ++ 7 files changed, 525 insertions(+), 29 deletions(-) create mode 100644 internal/handler/connection_resolve_test.go diff --git a/docs/custom-stack-connection-design.md b/docs/custom-stack-connection-design.md index bf5fde0..4043320 100644 --- a/docs/custom-stack-connection-design.md +++ b/docs/custom-stack-connection-design.md @@ -10,7 +10,7 @@ Two halves, useless apart: - **Connect** — the `vta_only` create form grows a **Customize** path that takes that bundle instead of silently using the platform stack — §5. -> **Status: specification. Nothing is built.** §12 tracks it. +> **Status: backend complete (phases 0–4); frontend outstanding.** §12 tracks it. > §9 is the section to read first — it is where this design can go wrong. Companion: [`vtafarm/docs/custom-stack-connection-frontend.md`](../../vtafarm/docs/custom-stack-connection-frontend.md). @@ -848,13 +848,13 @@ Nothing yet. | Migration + model (§6) | ✅ phase 1 | | Share code: mint / normalise / validate / compare (§4.1, §4.1.1) | ✅ phase 1 | | `resolveProvider`, platform path (§5.1) | ✅ phase 2 | -| `resolveProvider`, bundle tiers (§5.1) | ☐ | -| `POST /setup/connection/validate` (§5.2) | ☐ | -| `connection` on `POST /setup` (§5) | ☐ | +| `resolveProvider`, bundle tiers (§5.1) | ✅ phase 4 | +| `POST /setup/connection/validate` (§5.2) | ✅ phase 4 | +| `connection` on `POST /setup` (§5) | ✅ phase 4 | | Sharing toggle: `PUT /setup/:id/sharing` (+ admin twin) | ✅ phase 3 | | `connection` + `connections[]` in responses (§8) | ✅ phase 3 | -| Availability split (§10.1) | ☐ | +| Availability split (§10.1) | ✅ phase 4 | | `RegisterDid` failure visible (§9.1) | ✅ phase 0 | | Expected-audience check (§9.4) | ✅ phase 0 | | Frontend Share + Customize | ☐ | -| openapi.yaml | ☐ | +| openapi.yaml | ✅ phases 3–4 | diff --git a/internal/apidocs/openapi.yaml b/internal/apidocs/openapi.yaml index 43f2fd6..7d14ffa 100644 --- a/internal/apidocs/openapi.yaml +++ b/internal/apidocs/openapi.yaml @@ -326,6 +326,24 @@ components: CreateSetupRequest: type: object properties: + connection: + allOf: + - $ref: "#/components/schemas/ConnectionBundle" + description: | + `vta_only` only. Points the agent at a `full_stack` in this farm + other than the platform one. **Omit for today's behaviour** — the + platform stack, unchanged. + + Sent as an object, not as the pasted text: the client owns "the user + pasted something", this owns "is that stack usable". Validate it + first with `POST /setup/connection/validate` so the user sees which + stack they are joining before filling in the rest of the form; this + route re-runs every check regardless, and returns the same `reason` + values. + + Refused with 400 on a `full_stack` request — that mode provisions + its own mediator and DID host, so there is nothing to point at, and + ignoring the field would let someone believe otherwise. mode: type: string enum: [vta_only, full_stack] @@ -751,8 +769,13 @@ components: available: type: boolean description: | - Whether this mode can be created. Covers both capacity and the - platform-stack prerequisite — read `reason` to tell them apart. + Whether this mode can be created **by its default path**. Covers both + capacity and the platform-stack prerequisite — read `reason` to tell + them apart. + + For `vta_only` this is no longer the whole story: a caller carrying + a connection bundle needs no platform stack, so read + `custom_target_allowed` alongside it. example: true reason: type: string @@ -773,6 +796,17 @@ components: detail: type: string description: A sentence to show the user. Prefer it over composing copy client-side. + custom_target_allowed: + type: boolean + description: | + `vta_only` only. Whether an agent can be created against a stack the + caller names with a connection bundle. + + Survives every `reason` except `at_capacity`, because the platform + stack is a *default*, not a prerequisite for the mode. A UI should + therefore disable the "platform stack" option rather than the whole + mode, and preselect the custom option when this is the only path + left open. paths: /health: @@ -3785,6 +3819,7 @@ paths: VTA-only agents need the platform stack — the shared mediator and DID hosting they connect to. An admin has to create it before any VTA-only agent can be provisioned. + custom_target_allowed: true full_stack: { count: 2, available: true } metrics_available: true storage_available: true @@ -4703,3 +4738,90 @@ paths: "401": { description: Missing or invalid token, content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } } "404": { description: Session not found, content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } } "409": { description: Stack not ready, content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } } + + /api/v1/setup/connection/validate: + post: + summary: Check a connection bundle without creating anything + description: | + Runs the same checks `POST /setup` runs and creates nothing, so the + create form can confirm a pasted bundle before the user has filled the + rest of it in. + + **Why this exists.** The bundle is JSON, so a frontend can parse it and + render "connecting to *alice*, mediator `did:webvh:…`" the moment it is + pasted — but every one of those values came out of the pasted text. A + confirmation built that way shows a confident green tick for a bundle + whose code is pure garbage, and the user finds out only after naming + their agent, choosing an image and pressing Create. The response here + comes from this server's own database, which is the only version of that + confirmation worth showing. + + **Not authoritative.** `POST /setup` re-runs everything: a stack can stop + running, rotate its code or reach its connection limit in between. Treat + this as a courtesy and create as the gate, and keep the failure mapping + wired to both. + + Rate-limited, since it answers a yes/no about a credential. + + Refusals arrive as `reason` plus a `detail` sentence: + + | reason | status | | + | --- | --- | --- | + | `bad_bundle` | 400 | not a bundle, incomplete, or a mistyped share code | + | `wrong_farm` | 422 | a bundle for a different VTA Farm | + | `invalid_bundle` | 403 | does not open anything here — see below | + | `stack_not_running` | 409 | the stack isn't ready | + | `stack_changed` | 422 | the stack has changed since the bundle was copied | + | `stack_at_connection_limit` | 409 | it has as many agents as it may have | + + `invalid_bundle` deliberately covers five situations — no such stack, a + stack that never shared, one that turned sharing off, a rotated code, + and a code that is simply wrong. Distinguishing them would turn this + route into a way to discover which stacks exist and which are shared. + From the holder's side they are one fact with one next step: ask for a + current bundle. **Client copy must not try to narrow it.** + tags: [User] + security: + - CookieAuthUser: [] + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/ConnectionBundle" + responses: + "200": + description: The stack this bundle names, read from the server's own records + content: + application/json: + schema: + type: object + properties: + stack: { type: string, example: "alice" } + farm: { type: string, example: "firstperson.dev" } + mediator_did: { type: string } + did_hosting_server_url: { type: string } + connections_used: + type: integer + description: Present only when a connection limit is configured. + connections_max: + type: integer + description: Present only when a connection limit is configured. + "400": + description: Not a bundle, incomplete, or a mistyped share code (`bad_bundle`) + content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } + "401": + description: Missing or invalid token + content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } + "403": + description: The bundle does not open anything here (`invalid_bundle`) + content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } + "409": + description: Stack not ready, or at its connection limit + content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } + "422": + description: Another farm's bundle, or one that is out of date + content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } + "429": + description: Too many attempts + content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } diff --git a/internal/cloudflare/client.go b/internal/cloudflare/client.go index bb6732a..bad6f47 100644 --- a/internal/cloudflare/client.go +++ b/internal/cloudflare/client.go @@ -35,9 +35,9 @@ type createRecordRequest struct { } type apiResponse[T any] struct { - Success bool `json:"success"` + Success bool `json:"success"` Errors []apiError `json:"errors"` - Result T `json:"result"` + Result T `json:"result"` } type apiError struct { diff --git a/internal/handler/connection_resolve_test.go b/internal/handler/connection_resolve_test.go new file mode 100644 index 0000000..db2abb3 --- /dev/null +++ b/internal/handler/connection_resolve_test.go @@ -0,0 +1,139 @@ +package handler + +import ( + "net/http" + "testing" + + "github.com/ic3software/vtafarm-api/internal/setup" +) + +// validBundle is a well-formed bundle for the farm the handler below serves. +// Its share code is real, so the check-symbol test passes and the tier-1 +// refusals under test are the ones actually being exercised. +func validBundle(t *testing.T) *connectionBundle { + t.Helper() + code, err := setup.NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + return &connectionBundle{ + Version: connectionBundleVersion, + Kind: connectionBundleKind, + Farm: "firstperson.dev", + Stack: "alice", + Code: code, + MediatorDid: "did:webvh:mediator-alice", + DidHostingServerURL: "https://dids-alice.firstperson.dev", + DidHostingDid: "did:webvh:dids-alice", + } +} + +// Every case here is refused before the database is touched, which is why a +// handler with a nil db is a valid fixture — and is itself worth asserting: +// malformed input must not reach a query. +func TestResolveBundleProviderRejectsBeforeAnyQuery(t *testing.T) { + h := &SetupHandler{clusterDomain: "firstperson.dev"} + + tests := []struct { + name string + mutate func(*connectionBundle) + wantReason string + }{{ + name: "not a connection bundle", + mutate: func(b *connectionBundle) { b.Kind = "something.else" }, + wantReason: reasonBadBundle, + }, { + name: "from a future version", + mutate: func(b *connectionBundle) { b.Version = 99 }, + wantReason: reasonBadBundle, + }, { + name: "no stack named", + mutate: func(b *connectionBundle) { b.Stack = "" }, + wantReason: reasonBadBundle, + }, { + name: "no code", + mutate: func(b *connectionBundle) { b.Code = "" }, + wantReason: reasonBadBundle, + }, { + // The check symbol earns its keep here: a single mistyped character is + // diagnosed as a typo instead of falling through to the deliberately + // vague invalid_bundle. + name: "mistyped code", + mutate: func(b *connectionBundle) { b.Code = mistype(b.Code) }, + wantReason: reasonBadBundle, + }, { + name: "another farm's bundle", + mutate: func(b *connectionBundle) { b.Farm = "someone-else.example" }, + wantReason: reasonWrongFarm, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + b := validBundle(t) + tc.mutate(b) + + infra, provider, reason, detail := h.resolveBundleProvider(b) + if reason != tc.wantReason { + t.Fatalf("reason = %q, want %q", reason, tc.wantReason) + } + if detail == "" { + t.Error("a refusal must carry a sentence for the user") + } + if provider != nil || infra != (sharedInfra{}) { + t.Error("a refusal must not return a provider or partial values") + } + }) + } +} + +// The farm name is compared case-insensitively — hostnames are, and a bundle +// that travelled through a mail client that title-cased it is still ours. +func TestResolveBundleProviderFarmMatchIsCaseInsensitive(t *testing.T) { + h := &SetupHandler{clusterDomain: "firstperson.dev"} + b := validBundle(t) + b.Farm = "FirstPerson.DEV" + + // Reaches the database lookup, so it must not have been refused as + // wrong_farm first. A nil db panics there, which is what we assert against. + defer func() { + if r := recover(); r == nil { + t.Error("expected the lookup to be reached (nil db panics); it was refused earlier instead") + } + }() + _, _, reason, _ := h.resolveBundleProvider(b) + t.Fatalf("expected to reach the database, got reason %q", reason) +} + +// The five situations behind invalid_bundle must not be distinguishable, or +// this becomes a way to discover which stacks exist and which are shared. The +// status has to be uniform too — a different code per case leaks just as much. +func TestConnectionRefusalStatus(t *testing.T) { + tests := []struct { + reason string + want int + }{ + {reasonBadBundle, http.StatusBadRequest}, + {reasonInvalidBundle, http.StatusForbidden}, + {reasonStackNotRunning, http.StatusConflict}, + {reasonStackAtConnLimit, http.StatusConflict}, + {reasonWrongFarm, http.StatusUnprocessableEntity}, + {reasonStackChanged, http.StatusUnprocessableEntity}, + } + for _, tc := range tests { + if got := connectionRefusalStatus(tc.reason); got != tc.want { + t.Errorf("connectionRefusalStatus(%q) = %d, want %d", tc.reason, got, tc.want) + } + } +} + +// mistype changes one data character of a share code to a different valid +// symbol, leaving the check symbol stale. +func mistype(code string) string { + n := []byte(setup.NormalizeShareCode(code)) + if n[0] == 'A' { + n[0] = 'B' + } else { + n[0] = 'A' + } + return string(n) +} diff --git a/internal/handler/setup.go b/internal/handler/setup.go index f0b2a24..f5444b9 100644 --- a/internal/handler/setup.go +++ b/internal/handler/setup.go @@ -36,6 +36,12 @@ type SetupHandler struct { orch *setup.Orchestrator ghcr *ghcr.Client // nil when not configured capacity *CapacityService + // maxStackConnections caps how many vta_only sessions may connect to one + // shared full_stack; 0 disables the cap. Not a capacity model — the + // consumer's own pod is what this cluster accounts for. It bounds what a + // single share code can commit of somebody else's storage and message + // volume, which matters because a provider cannot remove one connection. + maxStackConnections int // full_stack mode mediatorGhcr *ghcr.Client // nil when not configured @@ -54,18 +60,20 @@ func NewSetupHandler( mediatorGhcrClient *ghcr.Client, didsGhcrClient *ghcr.Client, vtcGhcrClient *ghcr.Client, + maxStackConnections int, ) *SetupHandler { return &SetupHandler{ - db: db, - cf: cf, - appEnv: appEnv, - ingressIP: ingressIP, - clusterDomain: clusterDomain, - didHosting: dhFactory, - k8s: k8sClient, - orch: orch, - ghcr: ghcrClient, - capacity: NewCapacityService(k8sClient), + db: db, + cf: cf, + appEnv: appEnv, + ingressIP: ingressIP, + clusterDomain: clusterDomain, + didHosting: dhFactory, + k8s: k8sClient, + orch: orch, + ghcr: ghcrClient, + capacity: NewCapacityService(k8sClient), + maxStackConnections: maxStackConnections, mediatorGhcr: mediatorGhcrClient, didsGhcr: didsGhcrClient, @@ -172,6 +180,13 @@ type createSetupRequest struct { DidsImage string `json:"dids_image"` VtcImage string `json:"vtc_image"` VtcName string `json:"vtc_name"` + // Connection points a vta_only session at a full_stack in this farm other + // than the platform one. Omitted → the platform stack, unchanged. + // + // An object rather than the pasted text: the frontend owns "the user pasted + // something" and this owns "is that stack usable", which is the split that + // lets the create form confirm a bundle before submitting it. + Connection *connectionBundle `json:"connection"` } // POST /api/v1/setup @@ -255,6 +270,15 @@ func (h *SetupHandler) Create(c *gin.Context) { } if req.Mode == model.ModeFullStack { + // A full_stack provisions its own mediator and DID host, so there is + // nothing for a connection bundle to point at. Refused rather than + // ignored: silently dropping it would let someone believe their new + // stack was wired to somebody else's. + if req.Connection != nil { + c.JSON(http.StatusBadRequest, gin.H{ + "error": "a full-stack session runs its own mediator and DID host — connection applies only to a VTA-only agent"}) + return + } if !user.BetaAccess { c.JSON(http.StatusForbidden, gin.H{"error": req.Mode + " mode is in beta — ask an admin to enable beta access for your account"}) return @@ -272,11 +296,19 @@ func (h *SetupHandler) Create(c *gin.Context) { // It also yields the values the session is built from, so the gate and the // source are the same read — there is no window where the check passes and // the write then uses something else. - // The provider row itself is not needed until a caller can name one other - // than the platform stack; until then the three values are the whole answer. - infra, _, _, detail := h.resolveProvider() - if detail != "" { - c.JSON(http.StatusServiceUnavailable, gin.H{"error": detail}) + // + // Re-run in full even when the frontend already called + // POST /setup/connection/validate: a stack can stop running, rotate its code + // or fill up in between. Validate is a courtesy; this is the gate. + infra, provider, reason, detail := h.resolveProvider(req.Connection) + if reason != "" { + // A refused bundle is the caller's problem and says which; a missing or + // unready platform stack is the farm's, and has always been a 503. + status := http.StatusServiceUnavailable + if req.Connection != nil { + status = connectionRefusalStatus(reason) + } + c.JSON(status, gin.H{"error": detail, "reason": reason}) return } @@ -335,12 +367,22 @@ func (h *SetupHandler) Create(c *gin.Context) { // vta_only; this is what populates it going forward. "" when the // provider never recorded one, which keeps the previous behaviour of // accepting whatever the daemon claims. - DIDHostingDid: infra.DaemonDid, + DIDHostingDid: infra.DaemonDid, + // Default; overridden just below when the caller named a stack. Keyed on + // the bundle rather than on whether resolveProvider returned a row, + // because it returns one on the platform path too — and a platform + // session must keep provider_session_id NULL, or model.IsOrphaned would + // eventually read it as a provider that had been deleted. + ConnectionSource: model.ConnectionPlatform, VtaImage: req.VtaImage, AdminDid: req.AdminDid, Portable: portable, PreRotationCount: preRotationCount, } + if req.Connection != nil { + session.ConnectionSource = model.ConnectionInFarm + session.ProviderSessionID = &provider.ID + } // A single insert: there is no random id left to collide, so the retry loop // that used to wrap this went with unique_id. if createErr := h.db.Create(&session).Error; createErr != nil { @@ -419,6 +461,23 @@ const ( reasonProviderUnknown = "provider_lookup_failed" ) +// Reasons a pasted connection bundle is refused. Split across two tiers by how +// much the caller has proved — see resolveProvider. +const ( + reasonBadBundle = "bad_bundle" + reasonWrongFarm = "wrong_farm" + // reasonInvalidBundle is deliberately one reason for five situations: no + // such stack, a stack that never shared, one that turned sharing off, a + // rotated code, and a mangled code. Distinguishing them would turn this into + // a way to discover which stacks exist and which are shared, and from the + // holder's side they are the same fact — this bundle does not currently open + // anything — with the same next step: ask for a current one. + reasonInvalidBundle = "invalid_bundle" + reasonStackNotRunning = "stack_not_running" + reasonStackChanged = "stack_changed" + reasonStackAtConnLimit = "stack_at_connection_limit" +) + // sharedInfra is what a vta_only session is wired to — read from the platform // stack that actually provides it, never from configuration. // @@ -463,7 +522,11 @@ type sharedInfra struct { // the connection has to be recorded against a row, not a URL. // // full_stack is unaffected: it provisions its own mediator and DID host. -func (h *SetupHandler) resolveProvider() (v sharedInfra, provider *model.SetupSession, reason, detail string) { +func (h *SetupHandler) resolveProvider(ref *connectionBundle) (v sharedInfra, provider *model.SetupSession, reason, detail string) { + if ref != nil { + return h.resolveBundleProvider(ref) + } + const missing = "VTA-only agents need the platform stack — the shared mediator and DID hosting they connect to. " + "An admin has to create it before any VTA-only agent can be provisioned." const unknown = "Couldn't check the platform stack just now. Please try again." @@ -563,12 +626,21 @@ func (h *SetupHandler) capacityAllows(c *gin.Context, mode capacity.Mode) bool { // metrics/Longhorn outage never wrongly blocks the UI. func (h *SetupHandler) Availability(c *gin.Context) { type modeAvail struct { - Count int `json:"count"` + Count int `json:"count"` + // Available describes the DEFAULT path — for vta_only, the platform + // stack. It is not the whole story for that mode any more, because a + // caller carrying a connection bundle needs no platform stack at all. Available bool `json:"available"` // Why it's unavailable, and a sentence to show the user. Absent when // the mode is creatable. Reason string `json:"reason,omitempty"` Detail string `json:"detail,omitempty"` + // CustomTargetAllowed says whether vta_only can be created against a + // stack the caller names, which stays true when the platform stack is + // missing and false only when the cluster itself is full. It is what + // lets the UI disable one option rather than the whole mode: the + // platform stack is a default, not a prerequisite. + CustomTargetAllowed bool `json:"custom_target_allowed,omitempty"` } // Fail open on capacity, as before: a transient metrics/Longhorn outage @@ -603,7 +675,12 @@ func (h *SetupHandler) Availability(c *gin.Context) { // create screen on a blip. It fails open, like capacity above. POST /setup // refuses on the same reason, because there it is the difference between // waiting and provisioning an agent with no mediator DID at all. - if _, _, reason, detail := h.resolveProvider(); reason != "" && reason != reasonProviderUnknown { + // + // It gates the DEFAULT path only. Connecting to a stack the caller names + // needs no platform stack, so that option survives every reason below — + // cluster capacity, decided above, is the only thing that can close it. + vtaOnly.CustomTargetAllowed = vtaOnly.Available || vtaOnly.Reason != reasonAtCapacity + if _, _, reason, detail := h.resolveProvider(nil); reason != "" && reason != reasonProviderUnknown { vtaOnly.Available = false vtaOnly.Reason, vtaOnly.Detail = reason, detail } diff --git a/internal/handler/setup_sharing.go b/internal/handler/setup_sharing.go index ac410de..f275f1f 100644 --- a/internal/handler/setup_sharing.go +++ b/internal/handler/setup_sharing.go @@ -2,6 +2,7 @@ package handler import ( "net/http" + "strings" "github.com/gin-gonic/gin" @@ -97,6 +98,155 @@ func (h *SetupHandler) listConnections(providerID uint) []connectionSummary { return out } +// resolveBundleProvider turns a pasted bundle into the stack it names, in two +// tiers. +// +// Everything decided before the share code is verified must answer identically, +// or this becomes a directory of which stacks exist and which are shared — +// exactly what the share code exists to prevent. Once the caller has proved they +// hold a current code, specificity costs nothing and every remaining refusal +// says precisely what is wrong. +// +// No check here reaches the network. The stack is found in this farm's own +// database and the values come off that row, so a pasted URL never becomes a +// socket — see design §3. +func (h *SetupHandler) resolveBundleProvider(ref *connectionBundle) (v sharedInfra, provider *model.SetupSession, reason, detail string) { + // ── Tier 1: does this bundle open anything ────────────────────────────── + if ref.Kind != connectionBundleKind || ref.Version != connectionBundleVersion { + return v, nil, reasonBadBundle, + "That doesn't look like a connection bundle. Ask for the text from the stack's Share panel." + } + if ref.Stack == "" || ref.Code == "" { + return v, nil, reasonBadBundle, + "That connection bundle is incomplete. Ask for the text from the stack's Share panel." + } + // Checked before the farm comparison so a mistyped code is diagnosed as + // itself rather than as whatever the next check happens to be. + if err := setup.ValidateShareCode(ref.Code); err != nil { + return v, nil, reasonBadBundle, + "The share code looks mistyped — check it against what you were sent." + } + if !strings.EqualFold(ref.Farm, h.clusterDomain) { + return v, nil, reasonWrongFarm, + "This bundle is for a different VTA Farm. You can only connect to stacks running here." + } + + const invalid = "This bundle doesn't open anything here. It may have been deleted, or its owner may have " + + "turned sharing off or issued a new code — ask them for a current one." + + var session model.SetupSession + err := h.db. + Where("vta_name = ? AND mode = ?", ref.Stack, model.ModeFullStack). + First(&session).Error + if err != nil { + // Includes ErrRecordNotFound. A database failure lands here too and is + // reported as "invalid" rather than as its own reason: this route is + // reachable by anyone with an account, so the alternative leaks that the + // name exists whenever the read happens to fail. + return v, nil, reasonInvalidBundle, invalid + } + if session.ShareCode == nil || !setup.ShareCodeMatches(ref.Code, *session.ShareCode) { + return v, nil, reasonInvalidBundle, invalid + } + + // ── Tier 2: the caller holds a current code ───────────────────────────── + if v, reason, detail = providerInfra(&session); reason != "" { + // providerInfra's sentences name "the platform stack", which is wrong + // for a stack somebody shared. The condition is the same; the wording + // is not. + return sharedInfra{}, nil, reasonStackNotRunning, + "That stack isn't ready right now. Ask its owner to check it, then try again." + } + + // The bundle showed the recipient three values. If the stack no longer has + // them, what they agreed to join is not what they would be joining — most + // often because the stack was rebuilt, which also means a different daemon + // with an empty ACL. + if ref.MediatorDid != session.MediatorDid || + ref.DidHostingServerURL != session.DidsURL() || + ref.DidHostingDid != session.DIDHostingDid { + return sharedInfra{}, nil, reasonStackChanged, + "This bundle is out of date — the stack has changed since it was copied. Ask for a fresh one." + } + + if h.maxStackConnections > 0 { + var connected int64 + h.db.Model(&model.SetupSession{}).Where("provider_session_id = ?", session.ID).Count(&connected) + if connected >= int64(h.maxStackConnections) { + return sharedInfra{}, nil, reasonStackAtConnLimit, + "That stack has reached its limit of connected agents. Ask an admin to raise the limit, or use a different stack." + } + } + + return v, &session, "", "" +} + +// POST /api/v1/setup/connection/validate +// +// Runs the same checks as create and creates nothing, so the create form can +// confirm a bundle at paste time. +// +// It exists because of a flaw in the obvious alternative. The bundle is JSON, so +// a frontend can parse it and render "connecting to alice, mediator did:webvh:…" +// the moment it is pasted — but every one of those values came out of the pasted +// text. A card built that way shows a confident green tick for a bundle whose +// code is pure garbage, and the user finds out only after naming their agent, +// picking an image and pressing Create. This makes the card render values this +// server read from its own database, which is the only version of it worth +// showing. +// +// Not authoritative: POST /setup re-runs everything. A stack can stop running, +// rotate its code or fill up in between, so this is a courtesy and create is the +// gate. +func (h *SetupHandler) ValidateConnection(c *gin.Context) { + var ref connectionBundle + if err := c.ShouldBindJSON(&ref); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error(), "reason": reasonBadBundle}) + return + } + + infra, provider, reason, detail := h.resolveBundleProvider(&ref) + if reason != "" { + c.JSON(connectionRefusalStatus(reason), gin.H{"error": detail, "reason": reason}) + return + } + + resp := gin.H{ + "stack": provider.VtaName, + "farm": h.clusterDomain, + "mediator_did": infra.MediatorDid, + "did_hosting_server_url": infra.ServerURL, + } + if h.maxStackConnections > 0 { + resp["connections_used"] = h.countConnections(provider.ID) + resp["connections_max"] = h.maxStackConnections + } + c.JSON(http.StatusOK, resp) +} + +// countConnections is listConnections' cheap form. +func (h *SetupHandler) countConnections(providerID uint) int64 { + var n int64 + h.db.Model(&model.SetupSession{}).Where("provider_session_id = ?", providerID).Count(&n) + return n +} + +// connectionRefusalStatus maps a refusal to its HTTP status. The reason is what +// the frontend switches on; the status is for everything else in the chain. +func connectionRefusalStatus(reason string) int { + switch reason { + case reasonBadBundle: + return http.StatusBadRequest + case reasonInvalidBundle: + return http.StatusForbidden + case reasonStackNotRunning, reasonStackAtConnLimit: + return http.StatusConflict + default: + // wrong_farm, stack_changed — well-formed, but not usable here. + return http.StatusUnprocessableEntity + } +} + type sharingRequest struct { // enable mints a code, disable clears it, rotate replaces it. One field // rather than an enabled bool plus a rotate bool, which would make diff --git a/internal/router/router.go b/internal/router/router.go index 6b001c7..0afa60b 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -58,6 +58,7 @@ func Setup( db, cfClient, cfg.AppEnv, cfg.ClusterIngressIP, cfg.ClusterDomain, dhFactory, k8sClient, orch, ghcrClient, mediatorGhcrClient, didsGhcrClient, vtcGhcrClient, + cfg.MaxStackConnections, ) v1 := r.Group("/api/v1") @@ -240,6 +241,13 @@ func Setup( // VTA-only agent connect to this full stack. The code is the only gate: // clearing it stops new connections and leaves existing ones running. userAuth.PUT("/setup/:id/sharing", sh.SetSharing) + // Check a pasted bundle without creating anything, so the create form + // can confirm which stack it names from values this server read rather + // than from the pasted text itself. Rate-limited: it answers a yes/no + // about a credential, even though 75 bits behind an authenticated route + // is not brute-forceable. + userAuth.POST("/setup/connection/validate", + middleware.RateLimit(30, time.Minute), sh.ValidateConnection) } // Domains — a zone the user owns, verified on its own before any session From 468b6e33751adea86bde057c55e7feff38788061 Mon Sep 17 00:00:00 2001 From: vthwang Date: Fri, 31 Jul 2026 13:40:56 -0700 Subject: [PATCH 07/10] feat: report stack connections on the session list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /setup carries connection_source and provider_gone for vta_only rows and connection_count for full_stack ones, so the portal can mark an orphaned agent without opening it. Needed because an orphan's status stays `running` — it is running, and nothing of its own was touched when its provider went away. Nothing else on a list row would give it away. Signed-off-by: vthwang --- internal/handler/setup.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/handler/setup.go b/internal/handler/setup.go index f5444b9..862eb94 100644 --- a/internal/handler/setup.go +++ b/internal/handler/setup.go @@ -744,6 +744,14 @@ func (h *SetupHandler) List(c *gin.Context) { ErrorMsg string `json:"error_msg,omitempty"` CreatedAt any `json:"created_at"` UpdatedAt any `json:"updated_at"` + // vta_only: where its mediator and DID host came from, and whether that + // stack still exists. On the list so an orphaned agent can be marked + // without opening it — its badge still reads `running`, because it is, + // so nothing else on the row would give it away. + ConnectionSource string `json:"connection_source,omitempty"` + ProviderGone bool `json:"provider_gone,omitempty"` + // full_stack: how many other people's agents depend on this stack. + ConnectionCount int64 `json:"connection_count,omitempty"` } result := make([]item, len(sessions)) @@ -770,8 +778,11 @@ func (h *SetupHandler) List(c *gin.Context) { "dids": "https://" + s.DidsFQDN(), "vtc": "https://" + s.VtcFQDN(), } + it.ConnectionCount = h.countConnections(s.ID) } else { it.URL = s.PublicURL() + it.ConnectionSource = s.ConnectionSource + it.ProviderGone = s.IsOrphaned() } result[i] = it } From 64d5eeba981be567c6f5a1d7c0c3a14365b0a468 Mon Sep 17 00:00:00 2001 From: vthwang Date: Fri, 31 Jul 2026 13:56:58 -0700 Subject: [PATCH 08/10] refactor: the share code is the whole handover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the JSON connection bundle with a single code. The sharer sends one value; the recipient pastes one value. Nothing was lost, because nothing else in the bundle was doing work. The stack name is redundant now that the code is globally unique — a new index enforces that, which is what lets one lookup find the stack. The three DID/URL fields were only ever compared and never used; the row they came from has always been authoritative. The farm name only turned one generic refusal into a specific one. The kind/version tags were doing a job the code's own check character already does. It also removes a hazard rather than only weight. With a bundle, a client *can* parse it and render "connecting to alice, mediator did:webvh:…" the moment it is pasted — every value is right there — and that card would show a confident tick for a bundle whose code is garbage. The validate route existed to stop that. With a bare code there is nothing to render from except the server's answer, so presenting the sender's claims as facts about a stack is structurally impossible rather than merely discouraged. The staleness comparison goes with it and is not replaced. Rebuilding a stack mints a fresh code, so an old one already fails to resolve; the three compared values were belt to that braces. One real loss, stated plainly: a code pasted at the wrong farm now gets the generic "doesn't open anything here" instead of naming the farm. Rare, and the generic answer is true. A mistyped code is still caught locally by its check character, so it never lands in that message. Verified against a real Postgres and a running API: the index enforces uniqueness while still allowing many unshared stacks; a minted code resolves, including lowercased and with its dashes stripped; a one-character typo is refused as a typo; rotating invalidates the old code; disabling closes it; the connection cap refuses at the limit; sharing a vta_only and passing a code to full_stack are both refused; and deleting a provider leaves its consumers running and marked as orphaned on both the detail and list endpoints. Signed-off-by: vthwang --- docs/custom-stack-connection-design.md | 146 +++++++-------- internal/apidocs/openapi.yaml | 130 +++++--------- internal/handler/connection_resolve_test.go | 145 +++++++-------- internal/handler/setup.go | 46 ++--- internal/handler/setup_fullstack.go | 10 +- internal/handler/setup_sharing.go | 180 ++++++++----------- internal/handler/setup_sharing_test.go | 46 ++--- migrations/000026_share_code_unique.down.sql | 1 + migrations/000026_share_code_unique.up.sql | 23 +++ 9 files changed, 323 insertions(+), 404 deletions(-) create mode 100644 migrations/000026_share_code_unique.down.sql create mode 100644 migrations/000026_share_code_unique.up.sql diff --git a/docs/custom-stack-connection-design.md b/docs/custom-stack-connection-design.md index 4043320..d9d5ce9 100644 --- a/docs/custom-stack-connection-design.md +++ b/docs/custom-stack-connection-design.md @@ -5,10 +5,10 @@ stack — provided that stack is **one this farm provisioned**. Two halves, useless apart: -- **Share** — a `full_stack` owner mints a connection bundle for their stack and - hands it to somebody out of band — §4. +- **Share** — a `full_stack` owner mints a share code and hands it to somebody + out of band — §4. - **Connect** — the `vta_only` create form grows a **Customize** path that takes - that bundle instead of silently using the platform stack — §5. + that one code instead of silently using the platform stack — §5. > **Status: backend complete (phases 0–4); frontend outstanding.** §12 tracks it. > §9 is the section to read first — it is where this design can go wrong. @@ -23,13 +23,13 @@ credentials". ## 1. Scope **Stacks outside this farm are not supported.** Not deferred behind a flag, not -half-built — the API has no code path for them and the bundle format does not +half-built — the API has no code path for them and the code format does not pretend otherwise. §11.1 records what it would take. | In scope | Out of scope | | --- | --- | | `vta_only` → any `full_stack` **row in this farm's database** | any daemon this farm did not provision — §11.1 | -| A share code, minted and rotated by the stack's owner | a directory of stacks accepting connections | +| A share code — one value, nothing alongside it — minted and rotated by the stack's owner | a directory of stacks accepting connections | | Warning a provider what they are about to break, and telling the consumer afterwards | revoking a single connection, or blocking a delete — §7.4 | | — | migrating a live `vta_only` between stacks — §11.3 | | `vta_only` on the managed zone | `vta_only` on a custom domain (still excluded, `custom-domain-design.md` §18) | @@ -85,12 +85,12 @@ first upload, and §9.1 explains why that failure is currently silent. ## 3. The one rule that makes this safe -> **The bundle is evidence, not configuration.** +> **The code is evidence, not configuration.** The three values a consumer session is built from are read from the **provider's -row in our own database**. The pasted bundle is used only to (a) name which row, -(b) prove the sharer authorised it, and (c) show the recipient what they are -joining before they commit. +row in our own database**. The pasted code does two things and no more: it names +which row, and it proves the sharer authorised it. What the recipient sees about +that stack comes from the server (§5.2), never from what they pasted. Nothing the user types ever becomes a URL this server connects to. @@ -103,15 +103,17 @@ Everything downstream follows from that sentence: a daemon we provisioned. The self-asserted-audience hole (§9.4) stays closed by construction rather than by a check we have to remember. - **No stale-value class of bug.** A rebuilt stack is a *different row* with a - different share code, so an old bundle fails to resolve instead of - provisioning against a daemon that no longer exists. + different share code, so an old code fails to resolve instead of provisioning + against a daemon that no longer exists. -A design that instead took the URLs from the bundle would need every mitigation -in §9.4 to be correct forever. This one needs none of them. +A design that instead took the URLs from the sender would need every mitigation +in §9.4 to be correct forever. This one needs none of them — and since a code +names no host at all, there is nothing a caller could type that becomes a +socket. --- -## 4. Share: the connection bundle +## 4. Share: the share code ### 4.1 The share code is the grant @@ -124,7 +126,7 @@ share_code TEXT NULL -- NULL = this stack is not shared | Action | Effect | | --- | --- | | enable sharing | mint a random code | -| rotate | mint a new one — outstanding bundles stop working, live connections keep running | +| rotate | mint a new one — outstanding codes stop working, live connections keep running | | disable | `NULL` — no new connections; live ones keep running | One column rather than a `shared_use_enabled` boolean *plus* a code, because the @@ -161,39 +163,36 @@ It exists so that the overwhelmingly common failure — a hand-copied character is diagnosed as itself. The **platform stack has no code and needs none.** It is reached by the default -path, which sends no bundle at all. That also means nobody can accidentally +path, which sends no code at all. That also means nobody can accidentally paste their way onto it through the Customize form — the two paths stay distinct. -### 4.2 What the bundle contains +### 4.2 The code is the whole handover -```json -{ - "v": 1, - "kind": "vtafarm.stack-connection", - "farm": "firstperson.dev", - "stack": "alice", - "code": "k7m2-9xqp-4b8w-3nrt", - "mediator_did": "did:webvh:…:mediator-alice.firstperson.dev", - "did_hosting_server_url": "https://dids-alice.firstperson.dev", - "did_hosting_did": "did:webvh:…:dids-alice.firstperson.dev" -} -``` +Nothing travels with it. Not a JSON document, not the stack's name, not the +mediator DID or the DID-hosting URL. -Two fields do the work; the rest is for humans and for §5.2's confirmation card. +The first cut passed a bundle carrying all of those. Removing it cost nothing, +because none of it was doing work: -| Field | Role | +| Field | Why it went | | --- | --- | -| `stack` | the provider's `vta_name` — its public identifier everywhere else in this API (`CLAUDE.md`, "a session is addressed by its name"), so the lookup key is the one users already see | -| `code` | the grant (§4.1) | -| `farm` | lets the paste side say "this bundle isn't from this farm" instead of "stack not found" | -| `v` / `kind` | so pasting a DID, a URL or a JWT fails with a sentence rather than a 500 five steps later | -| `mediator_did`, `did_hosting_server_url`, `did_hosting_did` | **displayed and compared, never used** (§3). Compared so a bundle showing values the stack no longer has is refused rather than silently connecting to different ones than the recipient was shown | - -Every value already exists on the provider's row and is already returned by -`GET /setup/:id` (`collected.mediator_did`, `collected.did_hosting_did`, -`urls.dids`). **The bundle invents no data** — only the code is new, and it is -minted by an explicit act. +| `stack` | the code is globally unique (`setup_sessions_share_code_unique`), so it identifies its stack alone | +| `mediator_did`, `did_hosting_server_url`, `did_hosting_did` | only ever compared, never used — §3 already says the row is authoritative | +| `farm` | a code from another deployment simply does not resolve | +| `v` / `kind` | the check character (§4.1.1) already rejects anything that is not a code | + +And it removed a hazard rather than only weight. With a bundle, a client *can* +parse it and render "connecting to **alice**, mediator `did:webvh:…`" the moment +it is pasted — every value is right there — which would show a confident tick +for a bundle whose code is garbage. §5.2 existed to make that not happen. With a +bare code there is nothing to render from except the server's answer, so +presenting the sender's claims as facts is **structurally impossible** rather +than merely discouraged. + +The one real loss: a code pasted at the wrong farm gets the generic +`invalid_bundle` instead of "this is for a different VTA Farm". Rare, and the +generic answer is true. ### 4.3 Where it is offered @@ -205,7 +204,7 @@ minted by an explicit act. Populated only once `status = 'running'`, `mediator_did <> ''` and `did_hosting_did <> ''` — the same readiness rule `resolveSharedInfra` already applies to the platform stack. Absent before that, so the UI cannot offer a -bundle that would fail on arrival. +code that would fail on arrival. --- @@ -218,7 +217,7 @@ bundle that would fail on arrival. "mode": "vta_only", "vta_name": "myvta", "vta_image": "ghcr.io/…", - "connection": { /* the §4.2 bundle, verbatim */ } // optional + "share_code": "K7M2-9XQP-4B8W-3NRT" // optional } ``` @@ -236,7 +235,7 @@ into one lookup that differs only in which row it finds: ```go // ref == nil → the platform stack (today's default, semantics unchanged) -// ref != nil → the stack the bundle names +// ref != nil → the stack the code opens func (h *SetupHandler) resolveProvider(ref *stackRef) (v sharedInfra, provider *model.SetupSession, reason, detail string) ``` @@ -246,9 +245,9 @@ rather than adding one. The platform branch keeps its exact current reasons `shared_infra_unconfigured`). **Shipped in phase 2**, as `resolveProvider()` — the ref parameter arrives with -the bundle branch. The refactor split the judgement out of the I/O: +the code branch. The refactor split the judgement out of the I/O: `providerInfra(*SetupSession)` decides whether a candidate row is usable and is -pure, so it can be tested directly and so a bundle-named provider is held to +pure, so it can be tested directly and so a code-named provider is held to *the same* readiness bar rather than a second copy of it that drifts. Two pre-existing bugs surfaced while doing it, both fixed there: @@ -268,11 +267,11 @@ Two pre-existing bugs surfaced while doing it, both fixed there: got snapshotted onto the session permanently. It now tests the two components the hostname is built from. -The bundle branch, in two tiers. **Everything before the code is verified must +The code branch, in two tiers. **Everything before the code is verified must answer identically**, or the endpoint becomes a directory of which stacks exist and which are shared — precisely what §11.2 says there will not be. -**Tier 1 — is this bundle usable at all.** One reason for every outcome: +**Tier 1 — is this code usable at all.** One reason for every outcome: | Check | | | --- | --- | @@ -282,9 +281,9 @@ and which are shared — precisely what §11.2 says there will not be. No such stack, a stack that never shared, a stack that turned sharing off, a rotated code, and a hand-mangled code all produce the same 403. They are the -same fact from the holder's point of view — *this bundle does not currently open +same fact from the holder's point of view — *this code does not currently open anything* — and the only honest next step for all five is the same one: ask the -owner for a current bundle. +owner for a current code. `bad_bundle` and `wrong_farm` stay distinct because neither requires knowing anything about our data to determine. @@ -294,7 +293,6 @@ anything about our data to determine. | Check | Failure | | --- | --- | | `status = 'running'`, `mediator_did <> ''`, `did_hosting_did <> ''` | 409 `stack_not_running` | -| bundle's three display values equal the row's | 422 `stack_changed` | | connection count below the cap (§6.3) | 409 `stack_at_connection_limit` | Then, and only then, `sharedInfra` is built **from the provider row** — @@ -315,12 +313,13 @@ existing `middleware.RateLimit`, runs §5.1 and creates nothing. | fails | the same `reason` + `detail` `POST /setup` would have returned | This exists because of a flaw in the obvious design, and it is worth naming. -The bundle is JSON; a frontend can parse it and render "connecting to **alice**, -mediator `did:webvh:…`" the moment it is pasted. But every one of those values -came **out of the pasted text**. A confirmation card built that way is -self-asserted — it shows a green tick for a bundle whose code is pure garbage, -and the user discovers the truth only after naming their agent, picking an image -and pressing Create. +A code carries no information — the stack's name, its mediator and its DID host +are facts this server holds and the sender never transmits — so a client has +nothing to render from except this response. That is the property worth keeping: +with a JSON bundle, a client *could* build the card from the paste and show a +confident tick for a code that is pure garbage, discovered only after naming the +agent, picking an image and pressing Create. There is now nothing to build it +from but the truth. The validate route makes the confirmation card render **values this server read from its own database**, which is the only version of that card worth showing. @@ -331,22 +330,22 @@ Three notes: - **It is an oracle, and that is fine.** 75 bits of entropy behind an authenticated, rate-limited route is not brute-forceable, and tier 1's single - reason means the oracle answers exactly one question: *does the bundle I was + reason means the oracle answers exactly one question: *does the code I was given work.* That is the question it exists to answer. - **It is not authoritative.** `POST /setup` re-runs §5.1 in full. A stack can stop running, rotate its code or fill up between the two calls, so validate is a courtesy and create is the gate. Never skip the create-time check because validate passed. -- **This reverses an earlier decision.** A "test this bundle" *button* was +- **This reverses an earlier decision.** A "test this code" *button* was rejected as a worse copy of create. That reasoning was wrong once the confirmation card was in the design: the card is not a test the user opts into, it is a claim the UI makes unprompted, and it must not be a claim the server never checked. -### 5.3 A bundle is one stack's worth of values +### 5.3 One code, one stack Taking the mediator from stack A and the DID host from stack B is refused -implicitly: the bundle names one stack and every value comes from that row. +implicitly: the code opens one stack and every value comes from that row. Splitting them is technically possible over public HTTPS, but the dids daemon bakes `[identity] mediator_did` into its own recipe at provisioning time, so the pair is meaningful. Keeping it atomic also keeps §7's dependency tracking to a @@ -522,7 +521,7 @@ depends on. | Where | What | | --- | --- | | `GET /setup/:id` (consumer, `vta_only`) | `connection_source`, and when `in_farm`: the provider's name, or an explicit "gone" when `provider_session_id IS NULL` | -| `GET /setup/:id` (provider, `full_stack`) | `connection` bundle (§4.3) + `connections[]` — the dependents' names and statuses | +| `GET /setup/:id` (provider, `full_stack`) | `share_code` (§4.3) + `connections[]` — the dependents' names and statuses | | `GET /admin/setup-sessions` | a provider column, so support can see the topology without a query | The provider's dependent list is not decoration — it is the entire mitigation @@ -606,22 +605,23 @@ This is acceptable for a feature aimed at people sharing with people they know, which is what §1 and §4.1 constrain it to. It would not be acceptable behind a public directory — which is why §11.2 says there will not be one. -### 9.3 A stale bundle +### 9.3 A stale code **Severity: medium. Caught twice.** A rebuilt stack is a *different daemon* with a different `did_hosting_did` and an empty ACL. Deleting and recreating a stack produces a new row with a new share -code, so an old bundle fails at §5.1 step 3. If a stack somehow keeps its code -while its values change, step 5 catches it. Belt and braces, because the failure -mode is an agent provisioned against a daemon that no longer exists. +code, so an old code simply fails to resolve — there is no row for it to find. +The first cut also compared three display values as a second line of defence; +dropping the bundle dropped the belt and kept the braces, which is the half that +was actually load-bearing. ### 9.4 SSRF and credential relay — closed by construction **Severity: was high. Now not reachable.** Recorded because it is the reason §3 is written the way it is, and because the -first change that makes URLs bundle-sourced reopens all of it: +first change that makes URLs sender-supplied reopens all of it: 1. **SSRF** — a pasted URL turning this server into a probe of link-local, cloud metadata or in-cluster addresses. @@ -665,7 +665,7 @@ with no hint that the other holder is on a stack they have never heard of. Do **not** relax the global index. The admin routes resolve a session by name with no `user_id`, and the whole "a session is addressed by its name" design -rests on it — including §4.2's choice of `stack` as the bundle's lookup key. Fix +rests on it. Fix the message. One collision the global index catches by accident and must keep catching: @@ -748,7 +748,7 @@ stops working later. to match. Once Customize exists that is no longer the whole truth. **Decision: a farm with no platform stack can still create a `vta_only`, as long -as the caller brings a bundle.** The platform stack is a default, not a +as the caller brings a code.** The platform stack is a default, not a prerequisite for the mode. That is what forces the gate to split rather than just relax: `platform_stack_missing` stays a true and useful statement about the default path, and blocking the whole mode on it stops being one. @@ -793,8 +793,8 @@ enroll that (contained and revocable, but a private key per session, which belongs in Vault beside the master seed). It would also reopen every mitigation in §9.4 and make §9.5 load-bearing, and it -would have to replace §5.1's DB lookup with something else entirely — the bundle -would become configuration again, losing §3. Treat it as a different feature +would have to replace §5.1's DB lookup with something else entirely — the +sender's values would become configuration again, losing §3. Treat it as a different feature that happens to share a UI, not as a later phase of this one. ### 11.2 There will not be a stack directory @@ -832,7 +832,7 @@ it later changes no behaviour anyone depends on. §5.1 already unifies the *lookup*. The remaining asymmetry is that the platform stack has no share code and is reached by a nil ref. Collapsing that too — -giving it a code and making the default path just a preselected bundle — would +giving it a code and making the default path just a preselected one — would remove the last special case, at the cost of touching the one path every existing session depends on. After this ships, not with it. @@ -848,7 +848,7 @@ Nothing yet. | Migration + model (§6) | ✅ phase 1 | | Share code: mint / normalise / validate / compare (§4.1, §4.1.1) | ✅ phase 1 | | `resolveProvider`, platform path (§5.1) | ✅ phase 2 | -| `resolveProvider`, bundle tiers (§5.1) | ✅ phase 4 | +| `resolveProvider`, code tiers (§5.1) | ✅ phase 4 | | `POST /setup/connection/validate` (§5.2) | ✅ phase 4 | | `connection` on `POST /setup` (§5) | ✅ phase 4 | | Sharing toggle: `PUT /setup/:id/sharing` (+ admin twin) | ✅ phase 3 | diff --git a/internal/apidocs/openapi.yaml b/internal/apidocs/openapi.yaml index 7d14ffa..9427975 100644 --- a/internal/apidocs/openapi.yaml +++ b/internal/apidocs/openapi.yaml @@ -326,24 +326,27 @@ components: CreateSetupRequest: type: object properties: - connection: - allOf: - - $ref: "#/components/schemas/ConnectionBundle" + share_code: + type: string description: | `vta_only` only. Points the agent at a `full_stack` in this farm other than the platform one. **Omit for today's behaviour** — the platform stack, unchanged. - Sent as an object, not as the pasted text: the client owns "the user - pasted something", this owns "is that stack usable". Validate it - first with `POST /setup/connection/validate` so the user sees which - stack they are joining before filling in the rest of the form; this - route re-runs every check regardless, and returns the same `reason` - values. + One code and nothing else. It is globally unique, so it identifies + its stack without a name alongside it, and every value the session is + built from comes off that row — there is deliberately nothing here + naming a host. + + Check it first with `POST /setup/connection/validate` so the user + sees which stack they are joining before filling in the rest of the + form; this route re-runs every check regardless, and returns the same + `reason` values. Refused with 400 on a `full_stack` request — that mode provisions its own mediator and DID host, so there is nothing to point at, and ignoring the field would let someone believe otherwise. + example: "K7M2-9XQP-4B8W-3NRT" mode: type: string enum: [vta_only, full_stack] @@ -653,54 +656,6 @@ components: type: string description: full_stack only. 3c — shown once for offline backup. - ConnectionBundle: - type: object - description: | - What a `full_stack` owner copies and hands to somebody else out of band, - and what that person pastes into the VTA-only create form. - - Only `stack` and `code` are load-bearing. The connection is resolved by - looking `stack` up in this farm's own database and checking `code` - against it; the values a session is actually built from come from that - row, never from this document. The remaining fields are shown to the - recipient so they can see whose stack they are joining before they - commit, and compared on arrival so a bundle displaying values the stack - no longer has is refused rather than quietly connecting to different - ones. - - Nothing here ever becomes a URL this server connects to. - properties: - v: - type: integer - example: 1 - kind: - type: string - example: "vtafarm.stack-connection" - farm: - type: string - description: The farm this bundle is for. A bundle from another deployment is refused. - example: "firstperson.dev" - stack: - type: string - description: The provider session's `vta_name` — its public identifier everywhere else in this API. - example: "alice" - code: - type: string - description: | - The share code, grouped for display. Crockford base32 with a check - symbol, so a mistyped character is caught before it is sent. - example: "K7M2-9XQP-4B8W-3NRT" - mediator_did: - type: string - example: "did:webvh:...:mediator-alice.firstperson.dev" - did_hosting_server_url: - type: string - example: "https://dids-alice.firstperson.dev" - did_hosting_did: - type: string - description: The DID the daemon at that URL reports as its own. - example: "did:webvh:...:dids-alice.firstperson.dev" - StackConnection: type: object description: One agent connected to a stack. Name and status only — these sessions belong to other users. @@ -714,13 +669,15 @@ components: shared: type: boolean description: Whether this stack currently accepts new connections. - connection: - allOf: - - $ref: "#/components/schemas/ConnectionBundle" + share_code: + type: string description: | + The code to hand out, grouped for display. Crockford base32 with a + check character, so a mistyped one is caught before it is sent. + Absent — not empty — when the stack is not shareable, so the UI - cannot offer a bundle that would be refused the moment it was - pasted. + cannot offer a code that would be refused the moment it was used. + example: "K7M2-9XQP-4B8W-3NRT" connections: type: array items: @@ -4744,17 +4701,16 @@ paths: summary: Check a connection bundle without creating anything description: | Runs the same checks `POST /setup` runs and creates nothing, so the - create form can confirm a pasted bundle before the user has filled the - rest of it in. - - **Why this exists.** The bundle is JSON, so a frontend can parse it and - render "connecting to *alice*, mediator `did:webvh:…`" the moment it is - pasted — but every one of those values came out of the pasted text. A - confirmation built that way shows a confident green tick for a bundle - whose code is pure garbage, and the user finds out only after naming - their agent, choosing an image and pressing Create. The response here - comes from this server's own database, which is the only version of that - confirmation worth showing. + create form can tell the user which stack a code opens before they fill + the rest of it in. + + **This is the only way that confirmation can exist.** A share code + carries no information — the stack's name, its mediator and its DID host + are all facts this server holds and the sender does not transmit — so a + client has nothing to render from except this response. That is a + property worth keeping: it makes presenting the sender's claims as facts + about a stack structurally impossible rather than merely something a + client is asked not to do. **Not authoritative.** `POST /setup` re-runs everything: a stack can stop running, rotate its code or reach its connection limit in between. Treat @@ -4767,19 +4723,17 @@ paths: | reason | status | | | --- | --- | --- | - | `bad_bundle` | 400 | not a bundle, incomplete, or a mistyped share code | - | `wrong_farm` | 422 | a bundle for a different VTA Farm | + | `bad_bundle` | 400 | empty, malformed, or a mistyped code (caught by its check character) | | `invalid_bundle` | 403 | does not open anything here — see below | | `stack_not_running` | 409 | the stack isn't ready | - | `stack_changed` | 422 | the stack has changed since the bundle was copied | | `stack_at_connection_limit` | 409 | it has as many agents as it may have | `invalid_bundle` deliberately covers five situations — no such stack, a stack that never shared, one that turned sharing off, a rotated code, - and a code that is simply wrong. Distinguishing them would turn this - route into a way to discover which stacks exist and which are shared. - From the holder's side they are one fact with one next step: ask for a - current bundle. **Client copy must not try to narrow it.** + and a code that is simply wrong. There is one lookup and one answer, so + this route cannot be used to discover which stacks exist or which are + shared. From the holder's side they are one fact with one next step: ask + for a current code. **Client copy must not try to narrow it.** tags: [User] security: - CookieAuthUser: [] @@ -4788,10 +4742,15 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/ConnectionBundle" + type: object + required: [code] + properties: + code: + type: string + example: "K7M2-9XQP-4B8W-3NRT" responses: "200": - description: The stack this bundle names, read from the server's own records + description: The stack this code opens, read from the server's own records content: application/json: schema: @@ -4808,20 +4767,17 @@ paths: type: integer description: Present only when a connection limit is configured. "400": - description: Not a bundle, incomplete, or a mistyped share code (`bad_bundle`) + description: Empty, malformed, or a mistyped code (`bad_bundle`) content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } "401": description: Missing or invalid token content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } "403": - description: The bundle does not open anything here (`invalid_bundle`) + description: The code does not open anything here (`invalid_bundle`) content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } "409": description: Stack not ready, or at its connection limit content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } - "422": - description: Another farm's bundle, or one that is out of date - content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } "429": description: Too many attempts content: { application/json: { schema: { $ref: "#/components/schemas/Error" } } } diff --git a/internal/handler/connection_resolve_test.go b/internal/handler/connection_resolve_test.go index db2abb3..5690fc1 100644 --- a/internal/handler/connection_resolve_test.go +++ b/internal/handler/connection_resolve_test.go @@ -7,74 +7,34 @@ import ( "github.com/ic3software/vtafarm-api/internal/setup" ) -// validBundle is a well-formed bundle for the farm the handler below serves. -// Its share code is real, so the check-symbol test passes and the tier-1 -// refusals under test are the ones actually being exercised. -func validBundle(t *testing.T) *connectionBundle { - t.Helper() - code, err := setup.NewShareCode() - if err != nil { - t.Fatalf("NewShareCode: %v", err) - } - return &connectionBundle{ - Version: connectionBundleVersion, - Kind: connectionBundleKind, - Farm: "firstperson.dev", - Stack: "alice", - Code: code, - MediatorDid: "did:webvh:mediator-alice", - DidHostingServerURL: "https://dids-alice.firstperson.dev", - DidHostingDid: "did:webvh:dids-alice", - } -} - // Every case here is refused before the database is touched, which is why a // handler with a nil db is a valid fixture — and is itself worth asserting: // malformed input must not reach a query. -func TestResolveBundleProviderRejectsBeforeAnyQuery(t *testing.T) { +func TestResolveShareCodeRejectsBeforeAnyQuery(t *testing.T) { h := &SetupHandler{clusterDomain: "firstperson.dev"} - tests := []struct { - name string - mutate func(*connectionBundle) - wantReason string - }{{ - name: "not a connection bundle", - mutate: func(b *connectionBundle) { b.Kind = "something.else" }, - wantReason: reasonBadBundle, - }, { - name: "from a future version", - mutate: func(b *connectionBundle) { b.Version = 99 }, - wantReason: reasonBadBundle, - }, { - name: "no stack named", - mutate: func(b *connectionBundle) { b.Stack = "" }, - wantReason: reasonBadBundle, - }, { - name: "no code", - mutate: func(b *connectionBundle) { b.Code = "" }, - wantReason: reasonBadBundle, - }, { - // The check symbol earns its keep here: a single mistyped character is - // diagnosed as a typo instead of falling through to the deliberately - // vague invalid_bundle. - name: "mistyped code", - mutate: func(b *connectionBundle) { b.Code = mistype(b.Code) }, - wantReason: reasonBadBundle, - }, { - name: "another farm's bundle", - mutate: func(b *connectionBundle) { b.Farm = "someone-else.example" }, - wantReason: reasonWrongFarm, - }} + good, err := setup.NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + + tests := []struct{ name, code string }{ + {"empty", ""}, + {"whitespace only", " "}, + {"not a code at all", "did:key:z6MkSomething"}, + {"a pasted JSON bundle", `{"v":1,"kind":"vtafarm.stack-connection"}`}, + {"too short", setup.NormalizeShareCode(good)[:10]}, + // The check character earns its keep here: one wrong glyph is diagnosed + // as a typo instead of falling through to the deliberately vague + // invalid_bundle, which is the one message a user cannot act on. + {"one character mistyped", mistype(good)}, + } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - b := validBundle(t) - tc.mutate(b) - - infra, provider, reason, detail := h.resolveBundleProvider(b) - if reason != tc.wantReason { - t.Fatalf("reason = %q, want %q", reason, tc.wantReason) + infra, provider, reason, detail := h.resolveShareCode(tc.code) + if reason != reasonBadBundle { + t.Fatalf("reason = %q, want %q", reason, reasonBadBundle) } if detail == "" { t.Error("a refusal must carry a sentence for the user") @@ -86,25 +46,34 @@ func TestResolveBundleProviderRejectsBeforeAnyQuery(t *testing.T) { } } -// The farm name is compared case-insensitively — hostnames are, and a bundle -// that travelled through a mail client that title-cased it is still ours. -func TestResolveBundleProviderFarmMatchIsCaseInsensitive(t *testing.T) { - h := &SetupHandler{clusterDomain: "firstperson.dev"} - b := validBundle(t) - b.Farm = "FirstPerson.DEV" +// A well-formed code has to reach the lookup — including in every transcription +// a person might produce, since the code is meant to survive being read aloud. +// A nil db panics there, which is what these assert against. +func TestResolveShareCodeReachesLookupForWellFormedCodes(t *testing.T) { + code, err := setup.NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } - // Reaches the database lookup, so it must not have been refused as - // wrong_farm first. A nil db panics there, which is what we assert against. - defer func() { - if r := recover(); r == nil { - t.Error("expected the lookup to be reached (nil db panics); it was refused earlier instead") - } - }() - _, _, reason, _ := h.resolveBundleProvider(b) - t.Fatalf("expected to reach the database, got reason %q", reason) + for _, variant := range []string{ + code, + lower(code), + strip(code), + } { + t.Run(variant, func(t *testing.T) { + h := &SetupHandler{clusterDomain: "firstperson.dev"} + defer func() { + if r := recover(); r == nil { + t.Error("expected the lookup to be reached (nil db panics); refused earlier instead") + } + }() + _, _, reason, _ := h.resolveShareCode(variant) + t.Fatalf("expected to reach the database, got reason %q", reason) + }) + } } -// The five situations behind invalid_bundle must not be distinguishable, or +// The five ways a code can fail to open a stack must not be distinguishable, or // this becomes a way to discover which stacks exist and which are shared. The // status has to be uniform too — a different code per case leaks just as much. func TestConnectionRefusalStatus(t *testing.T) { @@ -116,8 +85,6 @@ func TestConnectionRefusalStatus(t *testing.T) { {reasonInvalidBundle, http.StatusForbidden}, {reasonStackNotRunning, http.StatusConflict}, {reasonStackAtConnLimit, http.StatusConflict}, - {reasonWrongFarm, http.StatusUnprocessableEntity}, - {reasonStackChanged, http.StatusUnprocessableEntity}, } for _, tc := range tests { if got := connectionRefusalStatus(tc.reason); got != tc.want { @@ -127,7 +94,7 @@ func TestConnectionRefusalStatus(t *testing.T) { } // mistype changes one data character of a share code to a different valid -// symbol, leaving the check symbol stale. +// symbol, leaving the check character stale. func mistype(code string) string { n := []byte(setup.NormalizeShareCode(code)) if n[0] == 'A' { @@ -137,3 +104,23 @@ func mistype(code string) string { } return string(n) } + +func lower(s string) string { + out := []rune(s) + for i, r := range out { + if r >= 'A' && r <= 'Z' { + out[i] = r + 32 + } + } + return string(out) +} + +func strip(s string) string { + out := "" + for _, r := range s { + if r != '-' { + out += string(r) + } + } + return out +} diff --git a/internal/handler/setup.go b/internal/handler/setup.go index 862eb94..0f4f779 100644 --- a/internal/handler/setup.go +++ b/internal/handler/setup.go @@ -180,13 +180,13 @@ type createSetupRequest struct { DidsImage string `json:"dids_image"` VtcImage string `json:"vtc_image"` VtcName string `json:"vtc_name"` - // Connection points a vta_only session at a full_stack in this farm other + // ShareCode points a vta_only session at a full_stack in this farm other // than the platform one. Omitted → the platform stack, unchanged. // - // An object rather than the pasted text: the frontend owns "the user pasted - // something" and this owns "is that stack usable", which is the split that - // lets the create form confirm a bundle before submitting it. - Connection *connectionBundle `json:"connection"` + // One code and nothing else: it is globally unique, so it identifies its + // stack on its own, and every value the session is built from comes off that + // row. There is deliberately nothing here naming a host. + ShareCode string `json:"share_code"` } // POST /api/v1/setup @@ -271,12 +271,12 @@ func (h *SetupHandler) Create(c *gin.Context) { if req.Mode == model.ModeFullStack { // A full_stack provisions its own mediator and DID host, so there is - // nothing for a connection bundle to point at. Refused rather than - // ignored: silently dropping it would let someone believe their new - // stack was wired to somebody else's. - if req.Connection != nil { + // nothing for a share code to point at. Refused rather than ignored: + // silently dropping it would let someone believe their new stack was + // wired to somebody else's. + if req.ShareCode != "" { c.JSON(http.StatusBadRequest, gin.H{ - "error": "a full-stack session runs its own mediator and DID host — connection applies only to a VTA-only agent"}) + "error": "a full-stack session runs its own mediator and DID host — share_code applies only to a VTA-only agent"}) return } if !user.BetaAccess { @@ -300,12 +300,12 @@ func (h *SetupHandler) Create(c *gin.Context) { // Re-run in full even when the frontend already called // POST /setup/connection/validate: a stack can stop running, rotate its code // or fill up in between. Validate is a courtesy; this is the gate. - infra, provider, reason, detail := h.resolveProvider(req.Connection) + infra, provider, reason, detail := h.resolveProvider(req.ShareCode) if reason != "" { - // A refused bundle is the caller's problem and says which; a missing or + // A refused code is the caller's problem and says which; a missing or // unready platform stack is the farm's, and has always been a 503. status := http.StatusServiceUnavailable - if req.Connection != nil { + if req.ShareCode != "" { status = connectionRefusalStatus(reason) } c.JSON(status, gin.H{"error": detail, "reason": reason}) @@ -368,18 +368,18 @@ func (h *SetupHandler) Create(c *gin.Context) { // provider never recorded one, which keeps the previous behaviour of // accepting whatever the daemon claims. DIDHostingDid: infra.DaemonDid, - // Default; overridden just below when the caller named a stack. Keyed on - // the bundle rather than on whether resolveProvider returned a row, - // because it returns one on the platform path too — and a platform - // session must keep provider_session_id NULL, or model.IsOrphaned would - // eventually read it as a provider that had been deleted. + // Default; overridden just below when the caller supplied a code. Keyed + // on that rather than on whether resolveProvider returned a row, because + // it returns one on the platform path too — and a platform session must + // keep provider_session_id NULL, or model.IsOrphaned would eventually + // read it as a provider that had been deleted. ConnectionSource: model.ConnectionPlatform, VtaImage: req.VtaImage, AdminDid: req.AdminDid, Portable: portable, PreRotationCount: preRotationCount, } - if req.Connection != nil { + if req.ShareCode != "" { session.ConnectionSource = model.ConnectionInFarm session.ProviderSessionID = &provider.ID } @@ -522,9 +522,9 @@ type sharedInfra struct { // the connection has to be recorded against a row, not a URL. // // full_stack is unaffected: it provisions its own mediator and DID host. -func (h *SetupHandler) resolveProvider(ref *connectionBundle) (v sharedInfra, provider *model.SetupSession, reason, detail string) { - if ref != nil { - return h.resolveBundleProvider(ref) +func (h *SetupHandler) resolveProvider(shareCode string) (v sharedInfra, provider *model.SetupSession, reason, detail string) { + if shareCode != "" { + return h.resolveShareCode(shareCode) } const missing = "VTA-only agents need the platform stack — the shared mediator and DID hosting they connect to. " + @@ -680,7 +680,7 @@ func (h *SetupHandler) Availability(c *gin.Context) { // needs no platform stack, so that option survives every reason below — // cluster capacity, decided above, is the only thing that can close it. vtaOnly.CustomTargetAllowed = vtaOnly.Available || vtaOnly.Reason != reasonAtCapacity - if _, _, reason, detail := h.resolveProvider(nil); reason != "" && reason != reasonProviderUnknown { + if _, _, reason, detail := h.resolveProvider(""); reason != "" && reason != reasonProviderUnknown { vtaOnly.Available = false vtaOnly.Reason, vtaOnly.Detail = reason, detail } diff --git a/internal/handler/setup_fullstack.go b/internal/handler/setup_fullstack.go index 645b50f..338bf82 100644 --- a/internal/handler/setup_fullstack.go +++ b/internal/handler/setup_fullstack.go @@ -256,17 +256,17 @@ func (h *SetupHandler) getFullStack(c *gin.Context, session *model.SetupSession) "vtc_image": session.VtcImage, "created_at": session.CreatedAt, "updated_at": session.UpdatedAt, - // Whether this stack is currently shared, and by what. The bundle is - // absent — not merely empty — when it is not shareable, so the UI cannot - // offer one that would be refused the moment somebody pasted it. + // Whether this stack is currently shared. The code is absent — not merely + // empty — when it is not shareable, so the UI cannot offer one that would + // be refused the moment somebody used it. "shared": session.IsShared(), // Other people's agents connected to this stack. Deleting it is allowed // and breaks every one of them, so this list is what lets the delete // confirmation name them. "connections": h.listConnections(session.ID), } - if bundle := buildConnectionBundle(session, h.clusterDomain); bundle != nil { - resp["connection"] = bundle + if code := displayShareCode(session); code != "" { + resp["share_code"] = code } resp["dids_enroll_used"] = session.DidsEnrollUsed diff --git a/internal/handler/setup_sharing.go b/internal/handler/setup_sharing.go index f275f1f..a4c19c0 100644 --- a/internal/handler/setup_sharing.go +++ b/internal/handler/setup_sharing.go @@ -11,57 +11,29 @@ import ( ) // The provider half of stack connections: a full_stack owner mints a share code -// and hands out the bundle a vta_only session pastes on create. Design: -// docs/custom-stack-connection-design.md §4. - -// connectionBundleKind and connectionBundleVersion tag the bundle so that -// pasting the wrong thing — a DID, a URL, a JWT, a bundle from a future -// version — fails with a sentence rather than five steps later with a 500. -const ( - connectionBundleKind = "vtafarm.stack-connection" - connectionBundleVersion = 1 -) - -// connectionBundle is what a provider hands out and a consumer pastes. +// and hands it to somebody, who pastes that one code when creating a vta_only +// agent. Design: docs/custom-stack-connection-design.md §4. // -// Only Stack and Code are load-bearing: the connection is resolved by looking -// Stack up in this farm's own database and checking Code against it, and the -// three values a session is actually built from come from that row. The rest is -// shown to the recipient so they can see whose stack they are joining before -// they commit, and compared on arrival so a bundle displaying values the stack -// no longer has is refused rather than silently connecting to different ones. +// The code is the whole handover. An earlier cut passed a JSON bundle carrying +// the stack name, the farm and the three DID/URL values a session is built +// from — but a globally unique code already identifies its stack, those three +// values were only ever compared and never used (the row is authoritative), and +// the confirmation the recipient sees has always been rendered from this +// server's own answer rather than from the pasted text. Everything except the +// code was doing no work, and one code is what a person can read down a phone. // -// That split is the reason this feature has no SSRF surface: nothing here ever -// becomes a URL this server connects to. See design §3. -type connectionBundle struct { - Version int `json:"v"` - Kind string `json:"kind"` - Farm string `json:"farm"` - Stack string `json:"stack"` - Code string `json:"code"` - - MediatorDid string `json:"mediator_did"` - DidHostingServerURL string `json:"did_hosting_server_url"` - DidHostingDid string `json:"did_hosting_did"` -} - -// buildConnectionBundle renders a shared, ready stack as a bundle. Returns nil -// when the stack is not currently shareable, so a caller can drop the field -// rather than offer a bundle that would be refused on arrival. -func buildConnectionBundle(s *model.SetupSession, farm string) *connectionBundle { +// Dropping it also removes a hazard rather than only weight: with nothing +// pasted worth rendering, a UI *cannot* present the sender's claims as facts +// about a stack. + +// displayShareCode is the grouped form handed to a person. Returns "" when the +// stack is not currently shareable, so a caller can drop the field rather than +// offer a code that would be refused the moment it was used. +func displayShareCode(s *model.SetupSession) string { if !s.IsShared() { - return nil - } - return &connectionBundle{ - Version: connectionBundleVersion, - Kind: connectionBundleKind, - Farm: farm, - Stack: s.VtaName, - Code: setup.GroupShareCode(*s.ShareCode), - MediatorDid: s.MediatorDid, - DidHostingServerURL: s.DidsURL(), - DidHostingDid: s.DIDHostingDid, + return "" } + return setup.GroupShareCode(*s.ShareCode) } // connectionSummary is one entry in a provider's dependent list: another user's @@ -98,54 +70,50 @@ func (h *SetupHandler) listConnections(providerID uint) []connectionSummary { return out } -// resolveBundleProvider turns a pasted bundle into the stack it names, in two -// tiers. +// resolveShareCode turns a share code into the stack it opens, in two tiers. // -// Everything decided before the share code is verified must answer identically, -// or this becomes a directory of which stacks exist and which are shared — -// exactly what the share code exists to prevent. Once the caller has proved they -// hold a current code, specificity costs nothing and every remaining refusal -// says precisely what is wrong. +// Everything decided before the code is verified must answer identically, or +// this becomes a directory of which stacks exist and which are shared — exactly +// what the code exists to prevent. Once the caller has proved they hold a +// current one, specificity costs nothing and every remaining refusal says +// precisely what is wrong. // -// No check here reaches the network. The stack is found in this farm's own -// database and the values come off that row, so a pasted URL never becomes a -// socket — see design §3. -func (h *SetupHandler) resolveBundleProvider(ref *connectionBundle) (v sharedInfra, provider *model.SetupSession, reason, detail string) { - // ── Tier 1: does this bundle open anything ────────────────────────────── - if ref.Kind != connectionBundleKind || ref.Version != connectionBundleVersion { - return v, nil, reasonBadBundle, - "That doesn't look like a connection bundle. Ask for the text from the stack's Share panel." +// No check here reaches the network, and none can: a code names no host. Every +// value a session is built from comes off the row this finds, so there is +// nothing a caller could type that becomes a socket — see design §3. +func (h *SetupHandler) resolveShareCode(code string) (v sharedInfra, provider *model.SetupSession, reason, detail string) { + // ── Tier 1: does this code open anything ──────────────────────────────── + // + // Shape first, so a mistyped code is diagnosed as itself. The check + // character makes that a local, certain answer rather than a guess, and + // keeps a single hand-copied glyph out of the deliberately vague message + // below — which is the one place a user has nothing to act on. + if strings.TrimSpace(code) == "" { + return v, nil, reasonBadBundle, "Enter the share code you were given." } - if ref.Stack == "" || ref.Code == "" { + if err := setup.ValidateShareCode(code); err != nil { return v, nil, reasonBadBundle, - "That connection bundle is incomplete. Ask for the text from the stack's Share panel." - } - // Checked before the farm comparison so a mistyped code is diagnosed as - // itself rather than as whatever the next check happens to be. - if err := setup.ValidateShareCode(ref.Code); err != nil { - return v, nil, reasonBadBundle, - "The share code looks mistyped — check it against what you were sent." - } - if !strings.EqualFold(ref.Farm, h.clusterDomain) { - return v, nil, reasonWrongFarm, - "This bundle is for a different VTA Farm. You can only connect to stacks running here." + "That doesn't look like a share code — check it against what you were sent." } - const invalid = "This bundle doesn't open anything here. It may have been deleted, or its owner may have " + - "turned sharing off or issued a new code — ask them for a current one." - + const invalid = "That code doesn't open anything here. The stack may have been deleted, or its owner may " + + "have turned sharing off or issued a new code — ask them for a current one." + + // One lookup, keyed on the code alone: it is globally unique + // (setup_sessions_share_code_unique), so it identifies its stack without a + // name alongside it. + // + // This is also what makes tier 1 answer identically for every way a code can + // fail — no such stack, never shared, sharing turned off, rotated, or simply + // wrong. There is one query and one answer, so the endpoint cannot be used + // to discover which stacks exist or which are shared. A database error lands + // here too, for the same reason: its own reason would leak that a code + // matched whenever the read happened to fail. var session model.SetupSession err := h.db. - Where("vta_name = ? AND mode = ?", ref.Stack, model.ModeFullStack). + Where("share_code = ? AND mode = ?", setup.NormalizeShareCode(code), model.ModeFullStack). First(&session).Error if err != nil { - // Includes ErrRecordNotFound. A database failure lands here too and is - // reported as "invalid" rather than as its own reason: this route is - // reachable by anyone with an account, so the alternative leaks that the - // name exists whenever the read happens to fail. - return v, nil, reasonInvalidBundle, invalid - } - if session.ShareCode == nil || !setup.ShareCodeMatches(ref.Code, *session.ShareCode) { return v, nil, reasonInvalidBundle, invalid } @@ -158,16 +126,10 @@ func (h *SetupHandler) resolveBundleProvider(ref *connectionBundle) (v sharedInf "That stack isn't ready right now. Ask its owner to check it, then try again." } - // The bundle showed the recipient three values. If the stack no longer has - // them, what they agreed to join is not what they would be joining — most - // often because the stack was rebuilt, which also means a different daemon - // with an empty ACL. - if ref.MediatorDid != session.MediatorDid || - ref.DidHostingServerURL != session.DidsURL() || - ref.DidHostingDid != session.DIDHostingDid { - return sharedInfra{}, nil, reasonStackChanged, - "This bundle is out of date — the stack has changed since it was copied. Ask for a fresh one." - } + // No staleness comparison is needed. Deleting and recreating a stack mints a + // fresh code, so a code from before a rebuild simply fails to resolve above + // rather than reaching a daemon that no longer exists. The three values the + // old bundle carried were belt to this braces. if h.maxStackConnections > 0 { var connected int64 @@ -184,28 +146,28 @@ func (h *SetupHandler) resolveBundleProvider(ref *connectionBundle) (v sharedInf // POST /api/v1/setup/connection/validate // // Runs the same checks as create and creates nothing, so the create form can -// confirm a bundle at paste time. +// tell the user which stack a code opens before they fill the rest of it in. // -// It exists because of a flaw in the obvious alternative. The bundle is JSON, so -// a frontend can parse it and render "connecting to alice, mediator did:webvh:…" -// the moment it is pasted — but every one of those values came out of the pasted -// text. A card built that way shows a confident green tick for a bundle whose -// code is pure garbage, and the user finds out only after naming their agent, -// picking an image and pressing Create. This makes the card render values this -// server read from its own database, which is the only version of it worth -// showing. +// This is the only way that confirmation can exist. A share code carries no +// information — the stack's name, its mediator and its DID host are all facts +// this server holds and the sender does not transmit — so a UI has nothing to +// render from except this response. That is a property worth keeping: it makes +// presenting the sender's claims as facts about a stack structurally +// impossible, rather than merely something the client is asked not to do. // // Not authoritative: POST /setup re-runs everything. A stack can stop running, // rotate its code or fill up in between, so this is a courtesy and create is the // gate. func (h *SetupHandler) ValidateConnection(c *gin.Context) { - var ref connectionBundle - if err := c.ShouldBindJSON(&ref); err != nil { + var req struct { + Code string `json:"code"` + } + if err := c.ShouldBindJSON(&req); err != nil { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error(), "reason": reasonBadBundle}) return } - infra, provider, reason, detail := h.resolveBundleProvider(&ref) + infra, provider, reason, detail := h.resolveShareCode(req.Code) if reason != "" { c.JSON(connectionRefusalStatus(reason), gin.H{"error": detail, "reason": reason}) return @@ -322,7 +284,7 @@ func (h *SetupHandler) setSharing(c *gin.Context, session *model.SetupSession) { if req.Action == "enable" && session.ShareCode != nil && *session.ShareCode != "" { c.JSON(http.StatusOK, gin.H{ "shared": true, - "connection": buildConnectionBundle(session, h.clusterDomain), + "share_code": displayShareCode(session), "connections": h.listConnections(session.ID), }) return @@ -342,7 +304,7 @@ func (h *SetupHandler) setSharing(c *gin.Context, session *model.SetupSession) { c.JSON(http.StatusOK, gin.H{ "shared": true, - "connection": buildConnectionBundle(session, h.clusterDomain), + "share_code": displayShareCode(session), "connections": h.listConnections(session.ID), }) } diff --git a/internal/handler/setup_sharing_test.go b/internal/handler/setup_sharing_test.go index 42c8663..e4166ab 100644 --- a/internal/handler/setup_sharing_test.go +++ b/internal/handler/setup_sharing_test.go @@ -21,41 +21,31 @@ func sharedStack() *model.SetupSession { } } -func TestBuildConnectionBundle(t *testing.T) { - got := buildConnectionBundle(sharedStack(), "firstperson.dev") - if got == nil { - t.Fatal("a shared, running stack must produce a bundle") - } - - want := connectionBundle{ - Version: connectionBundleVersion, - Kind: connectionBundleKind, - Farm: "firstperson.dev", - Stack: "alice", - Code: "K7M2-9XQP-4B8W-3NR", - MediatorDid: "did:webvh:mediator-alice", - DidHostingServerURL: "https://dids-alice.firstperson.dev", - DidHostingDid: "did:webvh:dids-alice", - } - if *got != want { - t.Errorf("bundle = %+v\nwant %+v", *got, want) +func TestDisplayShareCode(t *testing.T) { + if got, want := displayShareCode(sharedStack()), "K7M2-9XQP-4B8W-3NR"; got != want { + t.Errorf("displayShareCode() = %q, want %q", got, want) } } // The code is stored normalised and displayed grouped. A recipient must be able -// to paste the displayed form straight back and have it match. -func TestBundleCodeRoundTripsToStored(t *testing.T) { +// to paste the displayed form straight back and have it match — that round trip +// is the entire handover now that nothing else travels with it. +func TestShareCodeRoundTripsToStored(t *testing.T) { s := sharedStack() - bundle := buildConnectionBundle(s, "firstperson.dev") + shown := displayShareCode(s) - if !setup.ShareCodeMatches(bundle.Code, *s.ShareCode) { - t.Errorf("displayed code %q does not match stored %q", bundle.Code, *s.ShareCode) + if !setup.ShareCodeMatches(shown, *s.ShareCode) { + t.Errorf("displayed code %q does not match stored %q", shown, *s.ShareCode) + } + if setup.NormalizeShareCode(shown) != *s.ShareCode { + t.Errorf("normalising the displayed form gives %q, want the stored %q", + setup.NormalizeShareCode(shown), *s.ShareCode) } } -// Absent, not empty: a bundle that would be refused on arrival must not be -// offered at all, or the UI shows a green "copy this" for something broken. -func TestBuildConnectionBundleRefusesUnshareableStacks(t *testing.T) { +// Absent, not empty: a code that would be refused on use must not be offered at +// all, or the UI shows a green "copy this" for something broken. +func TestDisplayShareCodeRefusesUnshareableStacks(t *testing.T) { empty := "" tests := []struct { name string @@ -73,8 +63,8 @@ func TestBuildConnectionBundleRefusesUnshareableStacks(t *testing.T) { t.Run(tc.name, func(t *testing.T) { s := sharedStack() tc.mutate(s) - if got := buildConnectionBundle(s, "firstperson.dev"); got != nil { - t.Errorf("expected no bundle, got %+v", *got) + if got := displayShareCode(s); got != "" { + t.Errorf("expected no code, got %q", got) } }) } diff --git a/migrations/000026_share_code_unique.down.sql b/migrations/000026_share_code_unique.down.sql new file mode 100644 index 0000000..e2b07dc --- /dev/null +++ b/migrations/000026_share_code_unique.down.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS setup_sessions_share_code_unique; diff --git a/migrations/000026_share_code_unique.up.sql b/migrations/000026_share_code_unique.up.sql new file mode 100644 index 0000000..13165f9 --- /dev/null +++ b/migrations/000026_share_code_unique.up.sql @@ -0,0 +1,23 @@ +-- A share code now identifies its stack on its own: the connect flow takes one +-- code and nothing else, and resolves it with +-- +-- SELECT ... FROM setup_sessions WHERE share_code = ? +-- +-- so the code has to be unique or that lookup is ambiguous. At 75 bits a +-- collision is not a practical concern; the index is here so it is not a +-- concern at all, and so the ambiguity is impossible rather than merely +-- unlikely. +-- +-- Partial, because NULL means "not shared" and any number of stacks may be in +-- that state. +-- +-- This replaces the JSON bundle the first cut used. That carried the stack +-- name, the farm, and the three DID/URL values a session is built from — but +-- those values were only ever compared, never used (the row they came from is +-- authoritative), and the confirmation the recipient sees was always rendered +-- from the server's own answer rather than from the pasted text. So everything +-- except the code was doing no work that survived contact with the design, and +-- a single code is what a person can actually read down a phone. +CREATE UNIQUE INDEX setup_sessions_share_code_unique + ON setup_sessions (share_code) + WHERE share_code IS NOT NULL; From a0ea0e98dfb134a2c05837b5a1ac4ae9663a0366 Mon Sep 17 00:00:00 2001 From: vthwang Date: Sat, 1 Aug 2026 23:50:34 -0700 Subject: [PATCH 09/10] fix: mint share codes that are always alphanumeric Crockford's check alphabet extends the 32 data symbols with `*~$=U` for remainders 32-36, so roughly one code in seven ended in punctuation -- `FDGE-K0G4-AWNF-CQS~`. Valid, and nothing downstream broke, but a code that cannot be read down a phone or typed on an arbitrary keyboard has given up the one property this format was chosen over raw base32 for. NewShareCode now rerolls until the check symbol lands inside the data alphabet: 37/32 ~ 1.16 attempts on average, 0.2 bits off the 75. ValidateShareCode is deliberately not narrowed to match. The reroll governs what we mint; codes handed out before it existed are live credentials in the database, and rejecting their check symbol would lock out their holders for a cosmetic reason. A test pins that both ways. Signed-off-by: vthwang --- docs/custom-stack-connection-design.md | 15 ++++++++- internal/apidocs/openapi.yaml | 2 ++ internal/setup/sharecode.go | 43 +++++++++++++++++--------- internal/setup/sharecode_test.go | 40 ++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 16 deletions(-) diff --git a/docs/custom-stack-connection-design.md b/docs/custom-stack-connection-design.md index d9d5ce9..3119794 100644 --- a/docs/custom-stack-connection-design.md +++ b/docs/custom-stack-connection-design.md @@ -144,7 +144,20 @@ click without touching anyone already connected. 16 characters of Crockford base32, grouped in fours — `K7M2-9XQP-4B8W-3NRT`. 75 bits of entropy plus a **check symbol** as the last -character (Crockford's own `~*$=U` scheme). +character (Crockford's own scheme). + +**Always alphanumeric.** Crockford's check alphabet extends the 32 data symbols +with `*~$=U` for remainders 32–36, so 5/37 of draws would end in punctuation — +`FDGE-K0G4-AWNF-CQS~`. Valid, and nothing downstream breaks, but a code that +cannot be read down a phone or typed on an arbitrary keyboard has given up the +one property this format was chosen for. `NewShareCode` rerolls until the check +symbol lands inside the data alphabet: 37/32 ≈ 1.16 attempts on average, 0.2 +bits off the 75. + +`ValidateShareCode` is deliberately **not** narrowed to match. The reroll +governs what we mint; codes handed out before it existed are live credentials +in the database, and rejecting their check symbol would lock out their holders +for a cosmetic reason. Crockford rather than raw base32 because §2.2 offers this code to be read aloud, and a format meant for oral transmission without a checksum is half-designed. diff --git a/internal/apidocs/openapi.yaml b/internal/apidocs/openapi.yaml index 9427975..49c5dc7 100644 --- a/internal/apidocs/openapi.yaml +++ b/internal/apidocs/openapi.yaml @@ -674,6 +674,8 @@ components: description: | The code to hand out, grouped for display. Crockford base32 with a check character, so a mistyped one is caught before it is sent. + Always 16 alphanumerics — the check character is rerolled out of + Crockford's `*~$=U` range so the code stays readable aloud. Absent — not empty — when the stack is not shareable, so the UI cannot offer a code that would be refused the moment it was used. diff --git a/internal/setup/sharecode.go b/internal/setup/sharecode.go index a6b781c..85cafd1 100644 --- a/internal/setup/sharecode.go +++ b/internal/setup/sharecode.go @@ -37,25 +37,38 @@ const ( shareCodeGroup = 4 ) -// NewShareCode mints a share code, formatted for display. +// NewShareCode mints a share code, formatted for display. Every code it returns +// is 16 alphanumerics grouped in fours. // -// Rejection sampling rather than modulo: 256 is not a multiple of 32 in a way -// that matters here (it is, exactly), but reading one byte per symbol and -// masking to 5 bits is simpler to see as unbiased than any arithmetic on a -// larger word, and this runs once per share. +// One byte per symbol masked to 5 bits, rather than arithmetic on a larger +// word: simpler to see as unbiased, and this runs once per share. +// +// The loop is what keeps a minted code alphanumeric. Crockford's check alphabet +// carries five extra symbols — `*~$=U` — for remainders 32–36, so 5/37 of +// otherwise fine codes end in punctuation. Such a code is valid and always will +// be (ValidateShareCode still accepts them, and codes minted before this loop +// existed keep working), but it is unreadable down a phone and awkward on +// keyboards that bury those glyphs, which is the entire reason this format was +// chosen over raw base32. Rerolling costs 37/32 ≈ 1.16 attempts on average and +// 0.2 bits of the 75. func NewShareCode() (string, error) { - buf := make([]byte, shareCodeDataLen) - if _, err := rand.Read(buf); err != nil { - return "", fmt.Errorf("generate share code: %w", err) - } + for { + buf := make([]byte, shareCodeDataLen) + if _, err := rand.Read(buf); err != nil { + return "", fmt.Errorf("generate share code: %w", err) + } - data := make([]byte, shareCodeDataLen) - for i, b := range buf { - data[i] = crockfordAlphabet[b&0x1f] - } + data := make([]byte, shareCodeDataLen) + for i, b := range buf { + data[i] = crockfordAlphabet[b&0x1f] + } - full := string(data) + string(crockfordCheckSymbol(data)) - return GroupShareCode(full), nil + check := crockfordCheckSymbol(data) + if strings.IndexByte(crockfordAlphabet, check) < 0 { + continue + } + return GroupShareCode(string(data) + string(check)), nil + } } // GroupShareCode inserts the display dashes. Purely cosmetic — every comparison diff --git a/internal/setup/sharecode_test.go b/internal/setup/sharecode_test.go index 547a2f1..f2d7e33 100644 --- a/internal/setup/sharecode_test.go +++ b/internal/setup/sharecode_test.go @@ -44,6 +44,46 @@ func TestNewShareCodeExcludesConfusableGlyphs(t *testing.T) { } } +// A minted code must be alphanumeric end to end. Crockford's check alphabet +// carries five punctuation symbols for remainders 32–36, so without the reroll +// in NewShareCode roughly one code in seven would end in `*`, `~`, `$` or `=` +// — valid, but not something anyone can read down a phone, which is the whole +// reason this format was picked over raw base32. +func TestNewShareCodeIsAlphanumeric(t *testing.T) { + for i := 0; i < 500; i++ { + code, err := NewShareCode() + if err != nil { + t.Fatalf("NewShareCode: %v", err) + } + for _, r := range NormalizeShareCode(code) { + if (r < '0' || r > '9') && (r < 'A' || r > 'Z') { + t.Fatalf("minted code %q contains non-alphanumeric %q", code, r) + } + } + } +} + +// The reroll narrows what we mint; it must not narrow what we accept. Codes +// handed out before it existed are live credentials in the database, and a +// validator that rejected their check symbol would lock their holders out. +func TestValidateShareCodeAcceptsLegacyCheckSymbols(t *testing.T) { + for _, a := range crockfordAlphabet { + for _, b := range crockfordAlphabet { + data := "K7M29XQP4B8W3" + string(a) + string(b) + check := crockfordCheckSymbol([]byte(data)) + if strings.IndexByte(crockfordAlphabet, check) >= 0 { + continue // an alphanumeric check symbol — not the case under test + } + code := GroupShareCode(data + string(check)) + if err := ValidateShareCode(code); err != nil { + t.Fatalf("legacy code %q rejected: %v", code, err) + } + return + } + } + t.Fatal("found no code with a punctuation check symbol to test against") +} + func TestNormalizeShareCode(t *testing.T) { cases := []struct{ in, want string }{ {"K7M2-9XQP-4B8W-3NRT", "K7M29XQP4B8W3NRT"}, From 893ce918b15391d92fce2dc798176bd7b15796f4 Mon Sep 17 00:00:00 2001 From: vthwang Date: Sat, 1 Aug 2026 23:50:42 -0700 Subject: [PATCH 10/10] feat: report the connection cap on the provider's own responses connections_max was only ever returned by the consumer's POST /setup/connection/validate, so the owner of a stack could not see how much of their own capacity was spoken for -- it is their storage and message volume being committed, and s6.3's cap is what bounds how many agents arrive before they notice. Adds it to PUT /setup/:id/sharing and to GET /setup/:id for a full_stack, both through one sharingResponse helper so the page renders identically whether it just acted or just loaded. That also collapses the four places that were assembling the same map by hand. Omitted when the cap is off, so a client renders "3 connected" rather than "3 of 0". Signed-off-by: vthwang --- internal/apidocs/openapi.yaml | 12 ++++++++++ internal/handler/setup_fullstack.go | 17 +++++-------- internal/handler/setup_sharing.go | 37 ++++++++++++++++++++--------- 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/internal/apidocs/openapi.yaml b/internal/apidocs/openapi.yaml index 49c5dc7..c08e5ee 100644 --- a/internal/apidocs/openapi.yaml +++ b/internal/apidocs/openapi.yaml @@ -687,6 +687,18 @@ components: description: | Agents already connected. Unaffected by any of the three actions, and the list the delete confirmation names. + connections_max: + type: integer + description: | + How many agents this stack may host (MAX_STACK_CONNECTIONS). + Absent when the cap is off, so a client renders "3 connected" + rather than "3 of 0". + + The provider's half of the number the consumer's + POST /setup/connection/validate has always returned: it is the + owner's storage and message volume being committed, so the count + belongs on their page. + example: 10 CreateSetupResponse: type: object diff --git a/internal/handler/setup_fullstack.go b/internal/handler/setup_fullstack.go index 338bf82..ccbb472 100644 --- a/internal/handler/setup_fullstack.go +++ b/internal/handler/setup_fullstack.go @@ -256,17 +256,12 @@ func (h *SetupHandler) getFullStack(c *gin.Context, session *model.SetupSession) "vtc_image": session.VtcImage, "created_at": session.CreatedAt, "updated_at": session.UpdatedAt, - // Whether this stack is currently shared. The code is absent — not merely - // empty — when it is not shareable, so the UI cannot offer one that would - // be refused the moment somebody used it. - "shared": session.IsShared(), - // Other people's agents connected to this stack. Deleting it is allowed - // and breaks every one of them, so this list is what lets the delete - // confirmation name them. - "connections": h.listConnections(session.ID), - } - if code := displayShareCode(session); code != "" { - resp["share_code"] = code + } + // Whether this stack is shared, its code, its dependents and the cap on + // them — the same shape PUT /sharing answers with, so the page renders + // identically whether it just acted or just loaded. + for k, v := range h.sharingResponse(session) { + resp[k] = v } resp["dids_enroll_used"] = session.DidsEnrollUsed diff --git a/internal/handler/setup_sharing.go b/internal/handler/setup_sharing.go index a4c19c0..1e1ee4f 100644 --- a/internal/handler/setup_sharing.go +++ b/internal/handler/setup_sharing.go @@ -36,6 +36,29 @@ func displayShareCode(s *model.SetupSession) string { return setup.GroupShareCode(*s.ShareCode) } +// sharingResponse is the one shape every sharing action answers with, and the +// same fields GET /setup/:id carries for a full_stack. +// +// `connections_max` is the provider's half of a number the consumer's +// ValidateConnection has always returned: §6.3's cap is what bounds how many +// agents can arrive before the owner notices, and it is their storage and +// message volume being committed, so the count belongs on their page. +// Omitted when the cap is off, so a UI renders "3 connected" rather than +// "3 of 0". +func (h *SetupHandler) sharingResponse(s *model.SetupSession) gin.H { + resp := gin.H{ + "shared": s.IsShared(), + "connections": h.listConnections(s.ID), + } + if code := displayShareCode(s); code != "" { + resp["share_code"] = code + } + if h.maxStackConnections > 0 { + resp["connections_max"] = h.maxStackConnections + } + return resp +} + // connectionSummary is one entry in a provider's dependent list: another user's // agent connected to this stack. // @@ -260,7 +283,7 @@ func (h *SetupHandler) setSharing(c *gin.Context, session *model.SetupSession) { // Deliberately no teardown of existing connections. The code gates // joining, not membership: sessions already connected keep running, and // there is no way to remove one (design §7.4). - c.JSON(http.StatusOK, gin.H{"shared": false, "connections": h.listConnections(session.ID)}) + c.JSON(http.StatusOK, h.sharingResponse(session)) return } @@ -282,11 +305,7 @@ func (h *SetupHandler) setSharing(c *gin.Context, session *model.SetupSession) { // code rather than silently invalidating every bundle already handed out. // Replacing one is what rotate is for, and it asks explicitly. if req.Action == "enable" && session.ShareCode != nil && *session.ShareCode != "" { - c.JSON(http.StatusOK, gin.H{ - "shared": true, - "share_code": displayShareCode(session), - "connections": h.listConnections(session.ID), - }) + c.JSON(http.StatusOK, h.sharingResponse(session)) return } @@ -302,9 +321,5 @@ func (h *SetupHandler) setSharing(c *gin.Context, session *model.SetupSession) { } session.ShareCode = &stored - c.JSON(http.StatusOK, gin.H{ - "shared": true, - "share_code": displayShareCode(session), - "connections": h.listConnections(session.ID), - }) + c.JSON(http.StatusOK, h.sharingResponse(session)) }