amadan
gate: review trivia — comment counts, the Secure cookie's cost, a stranger's 404
Nothing here changes what the gate does. registry: routeCols says 37 names, which is what it now holds, and the gate= line in the ON CONFLICT list gets its comma at the end of the previous line like every one of its neighbours. internal/gate: the package doc says the cookie is Secure and what that costs — a plain-http dev edge cannot pass its own gate except on localhost — so nobody rediscovers it as a bug. stripCookie says it re-serialises via r.Cookies(), so a cookie Go's parser drops never reaches the app and a quoted value arrives unquoted. The login page's label steps down to 0.75rem, so the type scale is 0.75 / 1 / 1.5 rather than an odd 0.8. console: the gate door gets its non-member test. A browser signed up into an account of its own POSTs a password at this account's app and gets the family's 404 — byte for byte what a nonexistent account answers, so the form cannot be used to learn that the account, the app or the host is real — and the record is untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
4 files changed,
+60
−4
internal/console/appsettings_gate_test.go+41 −0internal/gate/gate.go+15 −0internal/gate/login.html+1 −1internal/registry/registry.go+3 −3
CI cancelled — superseded by a newer push — run details
diff --git a/internal/console/appsettings_gate_test.go b/internal/console/appsettings_gate_test.go| index 8b2bd2e89..b03ddcd34 100644 |
| --- a/internal/console/appsettings_gate_test.go |
| +++ b/internal/console/appsettings_gate_test.go |
| @@ -79,6 +79,47 @@ func TestAppSettingsGateSetAndClear(t *testing.T) { |
| } |
| } |
| +// The gate door is not an oracle. A signed-in browser that belongs to no |
| +// account here gets the account family's 404 — byte for byte what an |
| +// account that does not exist answers, so a stranger cannot use the gate |
| +// form to learn that this account, this app or this host is real — and the |
| +// record is untouched. Worth pinning on THIS door in particular: it is the |
| +// one that takes a password, and a stranger who could set one would lock an |
| +// account out of its own site. |
| +func TestAppSettingsGateIsNotAnOracle(t *testing.T) { |
| + fx, token := instanceFixture(t, "code-settings-gate-stranger") |
| + enableInstances(t, fx, "titogo", sharedPoolManifest()) |
| + if status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), token, |
| + `{"host":"pw.go.tito.io","region":"ie","addrs":["127.0.0.1:7001"]}`); status != http.StatusCreated { |
| + t.Fatalf("create = %d %s", status, out) |
| + } |
| + |
| + stranger := newBrowser(t) |
| + fx.setUser("eve@keymail.dev") // signs up into an account of her own |
| + signUp(t, fx, stranger, "code-settings-gate-stranger-browser") |
| + csrf := sessionToken(t, fx, stranger) |
| + |
| + form := url.Values{"csrf": {csrf}, "host": {"pw.go.tito.io"}, "action": {"set"}, "password": {"eve was here"}} |
| + ghost, _, missing := postForm(t, stranger, fx.ts.URL+"/a/zzzzz/apps/titogo/gate", form) |
| + if ghost != http.StatusNotFound { |
| + t.Fatalf("the yardstick is not a 404: nonexistent account = %d", ghost) |
| + } |
| + status, loc, body := postForm(t, stranger, fx.u("/apps/titogo/gate"), form) |
| + if status != http.StatusNotFound { |
| + t.Fatalf("gate POST as a non-member = %d -> %q, want 404", status, loc) |
| + } |
| + if body != missing { |
| + t.Fatalf("a non-member's 404 differs from a nonexistent account's:\n%q\nvs\n%q", body, missing) |
| + } |
| + rec, err := provision.GetRecord(context.Background(), fx.rel, "bab", "titogo", "pw.go.tito.io") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if rec.Gate != "" { |
| + t.Fatalf("a stranger set a gate: %q", rec.Gate) |
| + } |
| +} |
| + |
| func TestAppSettingsGateNeedsCSRF(t *testing.T) { |
| fx, _ := instanceFixture(t, "code-settings-gate-csrf") |
| c := newBrowser(t) |
diff --git a/internal/gate/gate.go b/internal/gate/gate.go| index 217b1c0f7..58cb90c40 100644 |
| --- a/internal/gate/gate.go |
| +++ b/internal/gate/gate.go |
| @@ -8,6 +8,13 @@ |
| // applies before hashing, the cookie's shape, the attempt limiter — is |
| // owned by this package so the two console doors and the edge cannot |
| // drift from one another. |
| +// |
| +// The cookie is Secure, always. A gate that a network can read the answer |
| +// to is not a curtain, and the platform terminates TLS at the edge for |
| +// every real host — so the one place this bites is a plain-http dev edge |
| +// (`carlos edge -dev`), which cannot pass its own gate except on |
| +// localhost, where browsers make an exception for Secure. Testing a gate |
| +// off-localhost means testing it over https. |
| package gate |
| import ( |
| @@ -229,6 +236,14 @@ func clientKey(r *http.Request) string { |
| // stripCookie removes the gate cookie from the request so the app behind |
| // the curtain never sees it. Other cookies pass through in their order. |
| +// |
| +// It re-serialises from r.Cookies() rather than editing the header text, |
| +// which has two consequences worth knowing: a cookie Go's parser drops as |
| +// malformed does not reach the app either, and a quoted value arrives |
| +// unquoted, because that is what Cookie.Value holds. Both are narrower |
| +// than what the app would have received unproxied, and neither has ever |
| +// mattered for a real client — but an app that hand-parses exotic cookies |
| +// is looking at this function, not at a browser bug. |
| func stripCookie(r *http.Request) { |
| cookies := r.Cookies() |
| r.Header.Del("Cookie") |
diff --git a/internal/gate/login.html b/internal/gate/login.html| index 8269b823d..2fa757543 100644 |
| --- a/internal/gate/login.html |
| +++ b/internal/gate/login.html |
| @@ -26,7 +26,7 @@ main { width: 100%; max-width: 22rem; } |
| h1 { font-size: 1.5rem; margin-bottom: 0.5rem; } |
| p { color: var(--muted); margin-bottom: 1.25rem; } |
| p code { color: var(--ink); font-family: ui-monospace, SFMono-Regular, Menlo, Consolas, monospace; } |
| -label { display: block; font-size: 0.8rem; color: var(--muted); margin-bottom: 0.35rem; } |
| +label { display: block; font-size: 0.75rem; color: var(--muted); margin-bottom: 0.35rem; } |
| input[type=password] { |
| width: 100%; font: inherit; color: var(--ink); background: var(--bg); |
| border: 1px solid var(--rule); border-radius: 0.4rem; padding: 0.55rem 0.7rem; margin-bottom: 0.9rem; |
diff --git a/internal/registry/registry.go b/internal/registry/registry.go| index 48610ddd9..d75296270 100644 |
| --- a/internal/registry/registry.go |
| +++ b/internal/registry/registry.go |
| @@ -686,7 +686,7 @@ func (r *Registry) Accounts() ([]Account, error) { |
| } |
| // routeCols is the column list every route read and the one route write |
| -// share — 36 names, in this exact order. scanRoute's destinations, Upsert's |
| +// share — 37 names, in this exact order. scanRoute's destinations, Upsert's |
| // placeholder row and Upsert's argument list all count from here: adding a |
| // column means adding it in FOUR places, and a miscount is a silent column |
| // shift rather than an error (TestUpsertRoundTripsAllColumns is what catches |
| @@ -919,8 +919,8 @@ ON CONFLICT(host) DO UPDATE SET |
| seed=CASE WHEN excluded.seed = '' THEN routes.seed ELSE excluded.seed END, |
| unit=CASE WHEN excluded.unit = '' THEN routes.unit ELSE excluded.unit END, |
| fleet_box=CASE WHEN excluded.fleet_box = '' THEN routes.fleet_box ELSE excluded.fleet_box END, |
| - addrs=excluded.addrs, health=excluded.health, steering=excluded.steering |
| - , gate=CASE WHEN excluded.gate = '' THEN routes.gate ELSE excluded.gate END`, |
| + addrs=excluded.addrs, health=excluded.health, steering=excluded.steering, |
| + gate=CASE WHEN excluded.gate = '' THEN routes.gate ELSE excluded.gate END`, |
| t.Host, t.Slug, t.Kind, t.Socket, t.Addr, t.Status, t.LastError, |
| t.DBPath, t.Version, t.PrevVersion, t.App, t.Pool, t.Region, t.Channel, |
| b2i(t.Hibernate), b2i(t.Canary), t.Replication, t.NextDue, t.Backing, |