oficina / home Public

The RP-ID guard was never actually fixed, and ten follow-ups

The fifth review's one block: B5 was reported fixed in the last commit
message and was not. The edit silently failed to match after an earlier
pass had already touched that snippet, so the guard still declared the
origin as a test-local constant and asserted it against itself — a gate
that cannot fail whatever the app configures, on the ruling the gate
exists for. It reads passkeyConfig(t).Origin now, uses HasSuffix rather
than Contains (which accepts x.oficina.works.attacker.example), and
moves to package home, because it calls a helper over the unexported
App and a guardstest-package test cannot reach one. Verified present
this time rather than asserted.

The follow-ups, all real:

app.go is modified by Tasks 1 and 2 and appeared in neither's git add,
so both intermediate commits would have failed to compile — home.New()
takes no arguments on main and Task 1 hands it a Sealer.

newTestApp was described in its finished form in the first task that
needs one, using BootSchema (Task 2), the signin dependency (Task 4)
and App.flow (Task 5). It starts with the database and the Sealer and
grows as each lands.

Its stub LookupTXT returning nil, nil would have put every sign-in in
the suite on the network: an empty TXT result falls through to
delegate, which returns the domain itself, and domainInfo then makes a
live HTTPS GET to the typed domain's /.well-known/keymail.

Task 7's allowlist names /static/webauthn.mjs and /static/passkey.js
literally, while rastrillo's asset helper fingerprints. Those two are
now routed on stable paths on purpose: a security rule that cannot name
its subject is not a rule.

sealtest imports testing from a non-test file exactly as logtest does,
so the guard that keeps logtest out of the binary now covers it too.
Nobody swept sessions or passkey_pending; both get the same
boot-plus-on-write shape as the link sweep.

loadSecrets was in the test-helpers table while being the production
loader main.go calls. scriptSrcs was used by the no-script test and
listed in no helper step. The no-script test file was in no git add.
Task 4 had two contradictory Consumes lines. And the File Structure
still placed logtest on the gate-9 branch, which merged.

Last, the two corrections this plan had only "filed": §5.1 and
AGENTS.md both still say classification goes through rastrillo/auth,
which cannot be done without its address-bearing tables. AGENTS.md's
own rule is that such disagreements are fixed in the same commit, so
they get fixed in this branch rather than left as a note.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 1 month ago 4980c884f4b6b013bd459077f62a0934bf0d29b1 parent e045ed9

1 file changed, +72 −23

docs/superpowers/plans/2026-09-03-home-signin.md +72 −23

diff --git a/docs/superpowers/plans/2026-09-03-home-signin.md b/docs/superpowers/plans/2026-09-03-home-signin.md

index 916689e..acd926b 100644

--- a/docs/superpowers/plans/2026-09-03-home-signin.md

+++ b/docs/superpowers/plans/2026-09-03-home-signin.md

@@ -86,6 +86,13 @@ helper folds case for the same reason.

§5.1 says "We still use `rastrillo/auth` for **address classification and Keymail upgrade** — the part that is hard to get right and holds nothing." **`auth` cannot be used that way.** Its classification is reachable only through `auth.Begin`, which writes `auth_links.address TEXT NOT NULL` — verified at source, and the very thing ruling 14 exists to avoid.

+**Land the correction, do not merely file it.** AGENTS.md's own rule is

+that where it disagrees with the spec the spec wins "and this file is wrong

+— fix it in the same commit". Both §5.1 and AGENTS.md's Data section say

+classification goes through `rastrillo/auth`; both are wrong for the reason

+below, and both are on `main`. Fix them in **this branch**, so the plan and

+the binding files stop disagreeing the moment it lands.

The usable seam is one level down: `github.com/keymaildev/signin` exports `Classifier` as a plain struct with exported fields (`HTTP`, `LookupTXT`, `Scheme`, `Now`, `TTL`, `NegTTL`) and a `Classify(ctx, address) Class` method. No constructor, no database, no stores. **Depend on `signin` directly** (rastrillo already requires v0.1.1). This plan does that; the spec's sentence should be corrected to match — filed, not silently diverged from.

---

@@ -104,10 +111,13 @@ mid-task.

| `capturedLink(t, app)` | Pulls the magic link out of the captured mail. |

| `signIn(t, app, addr)` | The whole ceremony: POST `/signin`, take the link, GET the confirm page, POST the form. Task 4's tests all go through it. |

| `passkeyConfig(t)` | Returns the `passkey.Config` the app builds, for Task 7's guard. |

-| `loadSecrets(t)` | `HOME_DEK`, `HOME_PEPPER` and `Flow.Key` from the environment, with the boot refusals below. |

-**Build these in Task 1 (`newTestApp`, `loadSecrets`) and Task 5

-(`signIn`), each with its own step and its own test.** An earlier draft

+`loadSecrets` is **not** in this table: it is the production loader

+`cmd/home/main.go` calls (Task 1 step 5b), not a test helper.

+**Build these in Task 1 (`newTestApp`) and Task 5 (`signIn`,

+`capturedLink`), each with its own step and its own test.** An earlier draft

listed them in this table and then had no step create any of them.

**`newTestApp` must wire a non-nil `Flow.Keymail`.** `Begin` skips

@@ -193,7 +203,7 @@ internal/home/

render.go One parsed tree per page, ui.Funcs plus an asset func.

templates/ Scriptless screens, attribute spelling

(ruling 20; the seven utilities keep class).

- logtest/ Gate 9. Already on feat/gate-9-no-content-in-logs.

+ logtest/ Gate 9. Already on main (9162ee5).

internal/guardstest/

logsinks_test.go Already landed. Forbids non-slog sinks.

noaddress_test.go Gate 2's schema half.

@@ -327,7 +337,10 @@ Test that unset, malformed and 31-byte values each error, and a valid one return

The plan commits `internal/home/sealtest` and no step ever created it.

Port `sealtest.go` from sheets-core **with its own `TestLeakedDetectsCanary`

-and WAL tests** — a detector nobody has watched detect anything is the

+and WAL tests**, and **extend `TestLogtestOnlyImportedByTests` to cover

+`sealtest` too** — one string in a slice. Like `logtest`, it imports

+`testing` from a non-test file, so importing it from production code would

+drag the testing package into the binary. — a detector nobody has watched detect anything is the

thing gate 9's review found twice.

Export **`CanaryAddress`** (a lowercase, address-shaped

@@ -379,10 +392,22 @@ so the fail-open test would pass vacuously without it. Register the

`secure_delete` connection hook here too, under a `sync.Once`, and **before**

`db.Open` (Task 2 step 5).

+**It starts small and grows.** `BootSchema` is Task 2's, the `signin`

+dependency is Task 4's and `App.flow` is Task 5's, so build it here with the

+database and the Sealer only, and let Tasks 2, 4 and 5 extend it as each

+lands. An earlier draft described the finished helper in the first task that

+needs one, which does not compile.

+**The stub `LookupTXT` must return a real error, not `nil, nil`.** An empty

+TXT result makes `signin` fall through to `delegate`, which returns the

+domain itself, and `domainInfo` then makes a live HTTPS GET to

+`https://<domain>/.well-known/keymail`. Every `signIn` in the suite would

+touch the network.

- [ ] **Step 6: Commit**

```bash

-git add internal/home/seal internal/home/sealtest internal/home/keys.go internal/home/keys_test.go internal/home/testapp_test.go cmd/home/main.go go.mod go.sum

+git add internal/home/seal internal/home/sealtest internal/home/keys.go internal/home/keys_test.go internal/home/testapp_test.go internal/home/app.go cmd/home/main.go go.mod go.sum

git commit -m "Seal values at rest, load the four secrets, and the plaintext gate that proves it"

```

@@ -629,7 +654,6 @@ Where the blindness claim is either kept or quietly broken.

not with the handlers)

**Interfaces:**

-- Consumes: `seal.Sealer`, `NewPersonRef`, `sessions.Sessions`, `mail.Sender`.

- Consumes: `seal.Sealer` (Task 1) and the database (Task 2) — and nothing else.

- Produces: `(*App).links`, a `signin.LinkStore`. **Routes nothing, and needs no `sessions` or `mail.Sender`** — the handlers are Task 5's, at `POST /signin`, `GET /signin/verify` (renders), `POST /signin/verify` (redeems) and `GET /signin/callback`.

@@ -766,7 +790,13 @@ fail is a gate written to pass — the finding that came back twice on gate 9.

Sealed rows are not a leak, but unbounded growth is a bug. Delete

`WHERE expires_at < ?` **at boot and on each `PutLink`**, the shape

-`passkey.Sweep` suggests. (Not "the wake-driven schedule §6 uses" — that

+`passkey.Sweep` suggests.

+**And sweep the other two while building the habit**, because nobody else

+does: `sessions.Sweep` (rows live 30 days) and `passkey.Sweep`

+(`passkey_pending`, `passkey_challenges`). Same boot-plus-on-write shape.

+None of these leak — they are keyed on person refs — but a table nothing

+ever deletes from is a bug waiting for a big enough database. (Not "the wake-driven schedule §6 uses" — that

sentence is about *instances*, and AGENTS.md says **the home does not

hibernate**, so there are no wakes to hang it on. A ticker would work here

and is simply not worth a goroutine for this.)

@@ -1123,6 +1153,14 @@ GET /static/webauthn.mjs webauthn.JS(), text/javascript

GET /static/passkey.js the app glue

```

+**Both on stable, unfingerprinted paths.** Everything else may go through

+rastrillo's fingerprinting asset helper; these two may not, because Task 7's

+allowlist has to name them, and a rule that cannot name its subject is not a

+rule.

+```

`ConfirmPath` is `/passkey/confirm` (its default, named rather than

defaulted). `RequireFresh`'s `maxAge` for the codes page is **5 minutes** —

it must be short, because the page shows ten codes once, and it must exceed

@@ -1169,7 +1207,8 @@ git commit -m "Passkeys as an optional second factor, gated on both sign-in path

**Files:**

- Create: `internal/home/render.go`, `internal/home/templates/*.html`

-- Create: `internal/guardstest/rpid_test.go`

+- Create: `internal/home/rpid_test.go` — **package `home`, not `guardstest`**: it reads the app's own config through a helper over the unexported `App`, which a `guards` test cannot reach.

+- Create: `internal/home/noscript_test.go`

- Create: `internal/home/static/tokens.css`, `internal/home/static/theme.css` — vendored from rastrillo. **Under `internal/home/`, not the repo root:** `go:embed` cannot reach a parent directory, sheets-core embeds `all:static` from inside its package, and `rastrillo new` scaffolds `internal/<pkg>/static/` with a `TestVendoredAssetsMatchTheLibrary` that reads it there.

**Interfaces:**

@@ -1210,22 +1249,25 @@ ignored `LegacyRPID` entirely.

// accepted RP ID — for assertions only; registration never takes it

// (webauthn.go rpMatches, allowLegacy). Set to "oficina.works" it

// collapses the split §4 exists to make.

-const credentialOrigin = "https://id.oficina.works"

+// This test lives in package home, NOT internal/guardstest: it reads the

+// config the APP builds, via a helper over the unexported App, and a

+// guards-package test cannot call one.

func TestRPIDIsExactlyTheCredentialHost(t *testing.T) {

- u, err := url.Parse(credentialOrigin)

+ cfg := passkeyConfig(t) // what the app configures, never a literal

+ u, err := url.Parse(cfg.Origin)

if err != nil {

- t.Fatalf("parse %q: %v", credentialOrigin, err)

+ t.Fatalf("parse Origin %q: %v", cfg.Origin, err)

}

if u.Hostname() != "id.oficina.works" {

- t.Errorf("RP ID is %q, want id.oficina.works (rulings 13, 16)", u.Hostname())

+ t.Errorf("RP ID is %q, want id.oficina.works: passkey derives the RP ID "+

+ "from Origin's hostname, so widening Origin widens it (rulings 13, 16)",

+ u.Hostname())

}

- // Wherever a passkey.Config is built, LegacyRPID must be empty, or a

- // full host with a comment naming the §5.7 drill that is running.

- // Until Task 6 lands there is no passkey.Config; assert the Origin half

- // alone and let this half start as a skipped subtest with the reason

- // written down, never as an absent assertion.

- if cfg := passkeyConfig(t); cfg.LegacyRPID != "" && !strings.Contains(cfg.LegacyRPID, ".oficina.works") {

+ // LegacyRPID must be empty, or a full host, with a comment naming the

+ // §5.7 drill that is running. HasSuffix, not Contains: Contains accepts

+ // "x.oficina.works.attacker.example".

+ if cfg.LegacyRPID != "" && !strings.HasSuffix(cfg.LegacyRPID, ".oficina.works") {

t.Errorf("LegacyRPID %q widens the RP ID (§4, ruling 2)", cfg.LegacyRPID)

}

@@ -1253,6 +1295,12 @@ any script at all on those screens. Constrain it:

```go

// scriptAllowed is ruling 21's allowlist, by TEMPLATE and by SRC. Naming

// the screens is not enough — that would admit any script at all on them.

+// Paths, not fingerprints. rastrillo's NewAssets.Path fingerprints

+// (/static/passkey.<hash>.js), so if these templates used the asset func

+// the allowlist would never match and the carve-out screens would fail

+// their own sweep. Task 6 routes these two on STABLE paths for exactly

+// that reason — they are the only two assets that need to be nameable in

+// a security rule.

var scriptAllowed = map[string][]string{

"passkey_confirm.html": {"/static/webauthn.mjs", "/static/passkey.js"},

"passkey_enrol.html": {"/static/webauthn.mjs", "/static/passkey.js"},

@@ -1295,8 +1343,9 @@ func TestConfirmPageRecoveryFormSurvivesScriptStripping(t *testing.T)

- [ ] **Step 2a: Build the test helpers this task's guards need**

-`templateNames(t)`, `renderTo(t, w, name, data)`, `sampleData(name)` and

-`passkeyConfig(t)` — all four are used by the two guards above and created

+`templateNames(t)`, `renderTo(t, w, name, data)`, `sampleData(name)`,

+`scriptSrcs(t, b)` (parse the HTML and return each `<script>`'s `src`, empty

+string for an inline one) and `passkeyConfig(t)` — all five are used by the two guards above and created

by no step in an earlier draft. `passkeyConfig` must return the config the

**app** builds, not one the test declares, or the RP-ID guard asserts a

constant against itself.

@@ -1304,7 +1353,7 @@ constant against itself.

- [ ] **Step 3: Commit**

```bash

-git add internal/home/render.go internal/home/templates internal/home/static internal/guardstest/rpid_test.go

+git add internal/home/render.go internal/home/templates internal/home/static internal/home/rpid_test.go internal/home/noscript_test.go

git commit -m "Scriptless sign-in screens, and the guard that the RP ID is a full host"

```