oficina / home Public

Answer the Task 1 review, and land the sealed link store

Two things in one commit, which is one more than it should be: the
review fixes and Task 4. They interleave — Task 4's store is built on
New's signature, which finding B5 changed — so splitting them would
produce two commits neither of which builds.

## The Task 1 review: CHANGES NEEDED, six blocking

The three that mattered most were all the same shape — a comment
asserting something the code did not do.

B1/B2/B3 — the boot path could not have started on the platform.
The home does not hibernate, so it runs as an ordinary unit tenant:
carlos-app@.service is `ExecStart=/opt/carlos/live/%i serve` with NO
arguments, and only the hibernate exec child is invoked with --socket
and --db. Requiring --db meant refusing every boot on the platform while
working perfectly for anyone who typed the flag locally. rastrillo's
Resolve overwrites Options.DBPath only when -db is actually passed and
joins a relative path to $STATE_DIRECTORY, so the path is now a default
in Options and the flag still wins locally. Verified end to end: the
built binary, invoked with --socket and no --db, boots, creates home.db
under $STATE_DIRECTORY and answers /healthz and / with 200; with a
secret missing it exits 1 and serves nothing.

The commit that introduced this also claimed "the previous build is what
serves" if a deploy went out without secrets. That was wrong and worth
correcting loudly: the unit is Restart=always and the activator does not
treat a wedged unit as down, so a deploy of a binary that exits 1 would
have systemd respin it and the site would 502 until a rollback. That is
what a promote does, not a deploy. Nothing in .amadan/ci.d deploys on
merge, so merging is safe; deploying is what needs the secrets first.

B4 — an all-zero key passed every check. Right length, valid base64, and
the canonical placeholder. signin.Flow refuses a zero Key per request
rather than at construction, so the app would have booted, answered
/healthz and failed every single sign-in. Refused at load now, for all
three keys, with a test per key and verified against the real binary.

B5 — "there is no zero-value escape hatch" was false. New(Secrets{})
compiled and ran. New now returns an error and refuses a nil Sealer, an
empty pepper, a zero signing key, a nil mailer and a nil database.

B6 — "adjacent columns of the same row" is Sheets' schema fact, carried
over with the port. The home's three purposes live in three tables. What
the HKDF split actually prevents here is a store handed the wrong
purpose, which no type and no round-trip test would notice.

Nits, all taken: the SMTP error text contradicted the comment above it
(HOST/FROM leak addresses into logs, PASS costs delivery — two failures,
two messages); LoadSecrets read HOME_DEK itself instead of calling
LoadSealer; "two settings" was three; sealtest scanned case-sensitively
while shipping upper-case canaries, and a team name is normalised for
reservation, so the scan folds case now; Sheets' vectors test is ported,
plus one asserting the home and Sheets derive different keys for the
same purpose name, which is the entire content of the change the port
made; the Makefile's "no database, no concurrency-bearing package" is
false twice over, so test-race lands with a CI step; the plan's
reference to the renamed containment guard.

One finding was mine: the runtime address-column check and gate 2's
static one disagreed about sealed_address, because only the static one
carried the exemption. That is the cost of two independent
implementations and also the point of them.

## Task 4: the sealed link store

- migrations/0002_signin_links.sql. No redeemed_at column, and its
  absence is the design: a row that survives redemption lets a handler
  tell "already used" from "never existed", which is an oracle, and
  marking it redeemed makes the take a read-then-write race where two
  clicks both mint a session.
- links.go. PutLink seals before it inserts — Litestream ships WAL
  frames continuously, so a plaintext write reaches the bucket before
  any delete could remove it. TakeLink is one DELETE ... RETURNING, so
  exactly one concurrent caller wins and unknown, expired and
  already-used are all "no row" by construction. expires_at is compared
  as a string, so both sides are written UTC RFC3339; a zone offset
  would sort differently and silently accept expired links.
- blindIndex is ruling 3, HMAC(pepper, lowercased address). §3.1 prefers
  "an expensive KDF rather than a bare HMAC"; that is a mitigation note
  in the threat-model section and ruling 3 is in §1's settled table, so
  the ruling wins. FILED FOR PAUL, not resolved here.
- findOrMintPerson is one upsert, so a returning person and a first-time
  person take the same path. Without it, a UNIQUE blind index and a
  fresh ref per visit would attach every session, credential and
  recovery code to an identity nobody can return to.
- The sweep runs after each put and never fails the put: sealed rows are
  not a leak, and refusing a sign-in because housekeeping failed trades
  a hygiene problem for an outage.

Gate 1 was verified to bite, as the plan insists: storing the address
unsealed fails TestPutLinkSealsTheAddress with `plaintext
"canary-8f3a@example.invalid" found in home.db-wal` — found in the WAL,
not the main file, which is exactly the reason the WAL is scanned.

Deferred with a reason: the plan's Task 4 step 5 also asks for
sessions.Sweep and passkey.Sweep on the same boot-plus-on-write shape.
Neither package is wired yet, so they land with the tasks that wire
them rather than as imports of packages nothing uses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E4uVkB6xnZJYuxTooPiUn4
Paul Campbell pushed by paul@keymail.dev e41d43ceddfce091295aec630341878d3ac480a9 parent 9209153
18 files changed, +801 −64
  • .amadan/ci.d/50-race +8 −0
  • Makefile +19 −8
  • cmd/home/main.go +34 −21
  • docs/superpowers/plans/2026-09-03-home-signin.md +2 −1
  • go.mod +1 −0
  • internal/home/app.go +36 −7
  • internal/home/db_test.go +30 −2
  • internal/home/keys.go +39 −15
  • internal/home/keys_test.go +58 −0
  • internal/home/links.go +157 −0
  • internal/home/links_test.go +266 −0
  • internal/home/migrations/0002_signin_links.sql +24 −0
  • internal/home/seal/seal.go +10 −3
  • internal/home/seal/seal_test.go +4 −3
  • internal/home/seal/vectors_test.go +79 −0
  • internal/home/sealtest/sealtest.go +10 −1
  • internal/home/sealtest/sealtest_test.go +17 −0
  • internal/home/testapp_test.go +7 −3
diff --git a/.amadan/ci.d/50-race b/.amadan/ci.d/50-race
new file mode 100755
index 0000000..3b79bd6
--- /dev/null
+++ b/.amadan/ci.d/50-race
@@ -0,0 +1,8 @@
+#!/bin/sh
+# One step of the home's CI. It runs a Makefile target rather than its
+# own copy of the commands, so CI and the local gate stay ONE
+# definition: `make test` runs these same targets in this same order,
+# and `make test-race` is exactly what CI runs here. Debugging CI is
+# running it.
+set -eu
+exec make test-race
diff --git a/Makefile b/Makefile
index 0c08b0a..14576af 100644
--- a/Makefile
+++ b/Makefile
@@ -1,8 +1,8 @@
-.PHONY: build test test-vet test-fmt test-build test-go
+.PHONY: build test test-vet test-fmt test-build test-go test-race
-# One binary, CGO_ENABLED=0, cgo-free SQLite (AGENTS.md / Deploys). This
-# skeleton opens no database yet — see cmd/home/main.go — but the
-# constraint holds regardless: nothing here may need cgo to build.
+# One binary, CGO_ENABLED=0, cgo-free SQLite (AGENTS.md / Deploys). Set
+# per-target rather than exported globally, because test-race is a
+# deliberate, documented exception — see its comment.
build:
CGO_ENABLED=0 go build ./...
@@ -14,10 +14,8 @@ build:
#
# Order is cheapest-and-fastest first: vet and gofmt are near-instant,
# build catches compile errors before time is spent on tests, plain
-# `go test` runs last. No test-migrations (no database yet) and no
-# test-race (no concurrency-bearing package yet) — see Sheets'
-# Makefile for what those look like once this app needs them.
-test: test-vet test-fmt test-build test-go
+# `go test` next, and -race — by far the slowest step — last.
+test: test-vet test-fmt test-build test-go test-race
test-vet:
CGO_ENABLED=0 go vet ./...
@@ -37,3 +35,16 @@ test-build:
test-go:
CGO_ENABLED=0 go test -count=1 ./...
+
+# The one concurrency-bearing package. internal/home's link store is
+# taken by a single DELETE ... RETURNING precisely so that two clicks on
+# one magic link mint one session, and its test races sixteen goroutines
+# at the same row to prove it. A race the detector would catch there is
+# a session minted twice.
+#
+# Deliberately NOT CGO_ENABLED=0: the race detector requires cgo (`go:
+# -race requires cgo`), so this one target runs with the ambient
+# CGO_ENABLED. Nothing it builds is shipped, so it does not violate the
+# cgo-free binary rule, which is about what gets deployed.
+test-race:
+ go test -race -count=1 ./internal/home/
diff --git a/cmd/home/main.go b/cmd/home/main.go
index 205fbb6..85d5e6b 100644
--- a/cmd/home/main.go
+++ b/cmd/home/main.go
@@ -5,28 +5,35 @@
// docs/superpowers/specs/2026-08-30-home-design.md and
// internal/home/page.go.
//
-// The platform activation contract has no $PORT: the platform (or a
-// developer) invokes this binary as
+// The platform activation contract has no $PORT: rastrillo.Resolve and
+// Serve own it end to end — activation argv, LISTEN_FDS,
+// $STATE_DIRECTORY, GET /healthz, GET /api/version, SIGTERM drain,
+// baseline security headers.
//
-// home --socket <path>
+// THE DATABASE PATH IS A DEFAULT HERE, NOT A FLAG, and that is not a
+// style choice. The home does not hibernate (AGENTS.md), so it runs as
+// an ordinary unit tenant: the platform's carlos-app@.service has
+// ExecStart=/opt/carlos/live/%i serve with NO arguments, and only the
+// hibernate exec child is invoked with --socket and --db. A binary that
+// required --db would therefore refuse every boot on the platform while
+// working perfectly for a developer who typed the flag. Resolve
+// overwrites Options.DBPath only when -db is actually passed, and joins
+// a relative path to $STATE_DIRECTORY, so "home.db" below is the
+// platform path and the flag still wins locally.
//
-// and rastrillo.Resolve/Serve own that contract end to end — activation
-// argv, LISTEN_FDS, $STATE_DIRECTORY, GET /healthz, GET /api/version,
-// SIGTERM drain, baseline security headers.
+// THIS BINARY REFUSES TO START WITHOUT ITS SECRETS: HOME_DEK,
+// HOME_PEPPER, HOME_SIGNIN_KEY, CARLOS_SMTP_PASS and the SMTP host and
+// from address. That is deliberate — every one of those absences
+// produces an app that answers /healthz and is broken or leaking.
//
-// THIS BINARY NO LONGER BOOTS ON ITS OWN. LoadSecrets refuses without
-// HOME_DEK, HOME_PEPPER, HOME_SIGNIN_KEY, CARLOS_SMTP_PASS, the SMTP
-// host and from address, and a --db path. That is deliberate — every
-// one of those absences produces an app that answers /healthz and is
-// broken or leaking — but it means a deploy of this binary must
-// provision all of them first. Until then the previous build is what
-// serves.
-//
-// It still opens no database handle. The --db path is validated here
-// and passed to rastrillo, and storage itself arrives with the schema
-// in the next step of the plan; this file then grows the same
-// Resolve -> db.Open -> migrate -> build mux -> Serve shape Sheets
-// uses.
+// What that costs, stated plainly: the unit is Restart=always, and the
+// activator deliberately does not treat a wedged unit as down. Deploying
+// this build before those secrets exist would cycle the process into a
+// binary that exits 1, systemd would respin it, and the site would 502
+// until a rollback. It is NOT true that "the old binary keeps serving" —
+// that is what a promote does, not a deploy. Nothing in .amadan/ci.d
+// deploys on merge, so merging is safe; deploying is the step that
+// needs the secrets first.
package main
import (
@@ -45,7 +52,9 @@ import (
func main() {
logger := slog.Default()
- opts, err := rastrillo.Resolve(rastrillo.Options{Logger: logger})
+ // DBPath is set here, not required as a flag — see the package
+ // comment. Relative, so Resolve joins it to $STATE_DIRECTORY.
+ opts, err := rastrillo.Resolve(rastrillo.Options{Logger: logger, DBPath: "home.db"})
if err != nil {
logger.Error("resolve", "err", err)
os.Exit(1)
@@ -85,7 +94,11 @@ func main() {
os.Exit(1)
}
- app := home.New(secrets, d)
+ app, err := home.New(secrets, d, logger)
+ if err != nil {
+ logger.Error("wire app", "err", err)
+ os.Exit(1)
+ }
// rastrillo.Options.Mux wants a *http.ServeMux; the app's own
// router is a chi.Router (an http.Handler), so it mounts under one.
diff --git a/docs/superpowers/plans/2026-09-03-home-signin.md b/docs/superpowers/plans/2026-09-03-home-signin.md
index 3cc10c1..9a9d89c 100644
--- a/docs/superpowers/plans/2026-09-03-home-signin.md
+++ b/docs/superpowers/plans/2026-09-03-home-signin.md
@@ -340,7 +340,8 @@ 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**, and **extend `TestLogtestOnlyImportedByTests` to cover
+and WAL tests**, and **extend the containment guard (now
+`TestTestOnlyPackagesNotImportedByProductionCode`) to cover
`sealtest` too** — on `main` it checks a single `const logtestImport`, so this
turns that constant into a slice of two. Like `logtest`, it imports
`testing` from a non-test file, so importing it from production code would
diff --git a/go.mod b/go.mod
index 55af060..08b036c 100644
--- a/go.mod
+++ b/go.mod
@@ -5,6 +5,7 @@ go 1.25.0
require (
github.com/carlosframework/rastrillo v0.22.0
github.com/go-chi/chi/v5 v5.3.2
+ github.com/keymaildev/signin v0.1.1
golang.org/x/crypto v0.55.0
gorm.io/plugin/dbresolver v1.6.2
modernc.org/sqlite v1.55.0
diff --git a/internal/home/app.go b/internal/home/app.go
index 918cfef..02f852c 100644
--- a/internal/home/app.go
+++ b/internal/home/app.go
@@ -12,6 +12,8 @@
package home
import (
+ "errors"
+ "log/slog"
"net/http"
"github.com/carlosframework/rastrillo/db"
@@ -26,16 +28,43 @@ import (
type App struct {
secrets Secrets
db *db.DB
+ links *linkStore
}
-// New builds an App on validated secrets.
+// New builds an App on validated secrets, and refuses anything less.
//
-// Secrets is a value, not a pointer, and there is no zero-value escape
-// hatch: an App without a Sealer would seal nothing and persist
-// addresses in the clear, which is the failure this whole step exists
-// to make impossible. Callers get their Secrets from LoadSecrets, which
-// refuses rather than degrades.
-func New(secrets Secrets, d *db.DB) *App { return &App{secrets: secrets, db: d} }
+// Secrets is a value, so a zero one always exists and can always be
+// passed; an earlier version of this comment claimed there was "no
+// zero-value escape hatch", which was false — New(Secrets{}) compiled
+// and ran, and an App with a nil Sealer would persist addresses in the
+// clear, the exact failure this step exists to prevent. The claim is now
+// true because it is checked rather than asserted.
+//
+// The checks duplicate LoadSecrets deliberately. LoadSecrets guards the
+// boot path; this guards every other way an App can be constructed,
+// which is every test and every future caller.
+func New(secrets Secrets, d *db.DB, logger *slog.Logger) (*App, error) {
+ if logger == nil {
+ logger = slog.Default()
+ }
+ switch {
+ case d == nil:
+ return nil, errors.New("home: no database")
+ case secrets.Sealer == nil:
+ return nil, errors.New("home: no Sealer, so nothing would be sealed at rest")
+ case len(secrets.Pepper) == 0:
+ return nil, errors.New("home: no pepper, so no address could be found again")
+ case secrets.SigninKey == ([32]byte{}):
+ return nil, errors.New("home: signing key is the zero value, which signin.Flow refuses")
+ case secrets.Mailer == nil:
+ return nil, errors.New("home: no mailer, so no sign-in link could be sent")
+ }
+ return &App{
+ secrets: secrets,
+ db: d,
+ links: &linkStore{db: d, sealer: secrets.Sealer, logger: logger},
+ }, nil
+}
// Router returns the mounted handler.
func (a *App) Router() http.Handler {
diff --git a/internal/home/db_test.go b/internal/home/db_test.go
index aed3685..fc95e77 100644
--- a/internal/home/db_test.go
+++ b/internal/home/db_test.go
@@ -103,9 +103,21 @@ func TestNoAddressColumnInTheBuiltDatabase(t *testing.T) {
// addressishColumn is the runtime twin of gate 2's static matcher. It is
// deliberately a separate, simpler implementation: two independent
// checks that agree are worth more than one shared helper that could be
-// wrong in one place.
+// wrong in one place. The cost of that is real — the two disagreed on
+// sealed_address the first time this ran, because only the static one
+// carried the exemption — so both exemptions are stated in both places.
+//
+// The exemption is the same one: a sealed blob is not an address, and
+// says so in its name. It is a whole word in the identifier, not a
+// substring, so a column called unsealed_address does not slip through.
func addressishColumn(col string) bool {
- for _, part := range splitIdent(col) {
+ parts := splitIdent(col)
+ for _, part := range parts {
+ if part == "sealed" {
+ return false
+ }
+ }
+ for _, part := range parts {
switch part {
case "address", "email", "mail", "recipient":
return true
@@ -135,3 +147,19 @@ func splitIdent(s string) []string {
}
return out
}
+
+// The exemption is a whole word, not a substring: the one column that
+// carries an address in this schema is allowed, and a name that merely
+// contains the letters is not.
+func TestAddressishColumnExemptionIsAWholeWord(t *testing.T) {
+ for _, col := range []string{"sealed_address", "address_sealed"} {
+ if addressishColumn(col) {
+ t.Errorf("addressishColumn(%q) = true; a sealed blob is not an address", col)
+ }
+ }
+ for _, col := range []string{"address", "owner_email", "unsealed_address", "recipient", "mail_to"} {
+ if !addressishColumn(col) {
+ t.Errorf("addressishColumn(%q) = false", col)
+ }
+ }
+}
diff --git a/internal/home/keys.go b/internal/home/keys.go
index db03f3a..232ed1c 100644
--- a/internal/home/keys.go
+++ b/internal/home/keys.go
@@ -1,6 +1,7 @@
package home
import (
+ "crypto/subtle"
"encoding/base64"
"errors"
"fmt"
@@ -78,12 +79,22 @@ func key32(name string) ([32]byte, error) {
if len(b) != 32 {
return out, fmt.Errorf("%s decodes to %d bytes, want exactly 32", name, len(b))
}
+ // An all-zero key is the canonical placeholder — the value a
+ // generated config, a truncated copy-paste or a `head -c 32
+ // /dev/zero` produces — and it is well-formed, so every other check
+ // here passes it. signin.Flow refuses a zero Key with ErrNoKey, but
+ // per request rather than at construction: the app boots, answers
+ // /healthz, and fails every single sign-in. This loader is the only
+ // place that refusal can happen at boot.
+ if subtle.ConstantTimeCompare(b, make([]byte, 32)) == 1 {
+ return out, fmt.Errorf("%s is 32 zero bytes, which is a placeholder rather than a key", name)
+ }
copy(out[:], b)
return out, nil
}
-// LoadSecrets reads the four secrets and the two settings whose absence
-// is silent, and refuses to boot on any of them.
+// LoadSecrets reads the four secrets and the three settings whose
+// absence is silent, and refuses to boot on any of them.
//
// HOME_DEK seals link state, team names and queue payloads
// HOME_PEPPER the blind index
@@ -107,15 +118,15 @@ func LoadSecrets(dbPath string, logger *slog.Logger) (Secrets, error) {
}
var s Secrets
+ var err error
- dek, err := key32("HOME_DEK")
+ // Through LoadSealer rather than beside it, so HOME_DEK is read in
+ // one place. Two readers of the same variable is how one of them
+ // ends up with a check the other does not have.
+ s.Sealer, err = LoadSealer()
if err != nil {
return Secrets{}, err
}
- s.Sealer, err = seal.New(dek[:])
- if err != nil {
- return Secrets{}, fmt.Errorf("HOME_DEK: %w", err)
- }
pepper, err := key32("HOME_PEPPER")
if err != nil {
@@ -134,9 +145,14 @@ func LoadSecrets(dbPath string, logger *slog.Logger) (Secrets, error) {
// enough: that log is storage we cannot grep or redact, so this is
// the one fallback we refuse instead of accept.
//
- // CARLOS_SMTP_PASS is a different failure and is required for a
- // different reason: it does not leak anything, it just means every
- // link fails at send time on an app that otherwise looks healthy.
+ // CARLOS_SMTP_PASS is a different failure and is refused for a
+ // different reason, so the message below distinguishes them. It
+ // leaks nothing: with HOST and FROM set, FromEnv returns the SMTP
+ // sender whether or not PASS is present. What a missing PASS costs
+ // is delivery — every link fails at send time on an app that
+ // otherwise looks healthy. (It is inert without CARLOS_SMTP_USER,
+ // which `carlos email enable` delivers alongside it; refusing on
+ // PASS is a bet that the platform sets both or neither.)
//
// Deliberately no loopback relaxation. Sheets' LoadAuthEnv allows
// one for local development, keyed on its instance origin being an
@@ -145,16 +161,24 @@ func LoadSecrets(dbPath string, logger *slog.Logger) (Secrets, error) {
// carve-out that turns off the leak guard is not a thing to invent
// in passing. Local development without SMTP is deferred until
// there is somewhere to put it.
- var missing []string
- for _, name := range []string{"CARLOS_SMTP_HOST", "CARLOS_SMTP_FROM", "CARLOS_SMTP_PASS"} {
+ var leaking, undeliverable []string
+ for _, name := range []string{"CARLOS_SMTP_HOST", "CARLOS_SMTP_FROM"} {
if strings.TrimSpace(os.Getenv(name)) == "" {
- missing = append(missing, name)
+ leaking = append(leaking, name)
}
}
- if len(missing) > 0 {
+ if strings.TrimSpace(os.Getenv("CARLOS_SMTP_PASS")) == "" {
+ undeliverable = append(undeliverable, "CARLOS_SMTP_PASS")
+ }
+ if len(leaking) > 0 {
return Secrets{}, fmt.Errorf(
"mail is not configured: %s not set, so magic links would be written to the log instead of emailed. Run `carlos email enable --app home`",
- strings.Join(missing, ", "))
+ strings.Join(leaking, ", "))
+ }
+ if len(undeliverable) > 0 {
+ return Secrets{}, fmt.Errorf(
+ "mail is not configured: %s not set, so every magic link would fail at send time. Run `carlos email enable --app home`",
+ strings.Join(undeliverable, ", "))
}
s.Mailer = mail.FromEnv("CARLOS", logger)
diff --git a/internal/home/keys_test.go b/internal/home/keys_test.go
index 8cd05f1..c66f33d 100644
--- a/internal/home/keys_test.go
+++ b/internal/home/keys_test.go
@@ -151,3 +151,61 @@ func TestLoadSecretsRefusesEmptyDBPath(t *testing.T) {
t.Fatal("LoadSecrets accepted an empty database path")
}
}
+
+// An all-zero key is well-formed: right length, valid base64, and every
+// other check passes it. It is also the canonical placeholder, and
+// signin.Flow refuses a zero Key per request rather than at
+// construction — so without this the app boots, answers /healthz, and
+// fails every single sign-in.
+func TestLoadSecretsRefusesAllZeroKeys(t *testing.T) {
+ zero := base64.StdEncoding.EncodeToString(make([]byte, 32))
+ for _, name := range []string{"HOME_DEK", "HOME_PEPPER", "HOME_SIGNIN_KEY"} {
+ t.Run(name, func(t *testing.T) {
+ setSecretEnv(t, map[string]string{name: zero})
+ _, err := LoadSecrets("/tmp/home.db", quietLogger())
+ if err == nil {
+ t.Fatalf("LoadSecrets accepted 32 zero bytes for %s", name)
+ }
+ if !strings.Contains(err.Error(), name) {
+ t.Errorf("error %q does not name %s", err, name)
+ }
+ })
+ }
+}
+
+// New guards every construction path that is not the boot path — which
+// is every test, and every caller written from here on.
+func TestNewRefusesIncompleteWiring(t *testing.T) {
+ full, _ := testSecrets(t)
+ d := newTestApp(t).db
+
+ noPepper := full
+ noPepper.Pepper = nil
+ noKey := full
+ noKey.SigninKey = [32]byte{}
+ noSealer := full
+ noSealer.Sealer = nil
+ noMailer := full
+ noMailer.Mailer = nil
+
+ for name, s := range map[string]Secrets{
+ "zero value": {},
+ "no Sealer": noSealer,
+ "no pepper": noPepper,
+ "no sign key": noKey,
+ "no mailer": noMailer,
+ } {
+ t.Run(name, func(t *testing.T) {
+ if _, err := New(s, d, quietLogger()); err == nil {
+ t.Fatal("New accepted it")
+ }
+ })
+ }
+
+ if _, err := New(full, nil, quietLogger()); err == nil {
+ t.Fatal("New accepted a nil database")
+ }
+ if _, err := New(full, d, quietLogger()); err != nil {
+ t.Fatalf("New refused complete wiring: %v", err)
+ }
+}
diff --git a/internal/home/links.go b/internal/home/links.go
new file mode 100644
index 0000000..22e2109
--- /dev/null
+++ b/internal/home/links.go
@@ -0,0 +1,157 @@
+package home
+
+import (
+ "context"
+ "crypto/hmac"
+ "crypto/sha256"
+ "database/sql"
+ "encoding/hex"
+ "errors"
+ "log/slog"
+ "strings"
+ "time"
+
+ "github.com/carlosframework/rastrillo/db"
+
+ "amadan.net/oficina/home/internal/home/seal"
+)
+
+// sealPurposeAddress is the HKDF purpose a link's address is sealed
+// under. A value sealed under one purpose cannot be opened under
+// another, which is what keeps an address and a team name — adjacent
+// columns of adjacent tables — from ever being swapped unnoticed.
+const sealPurposeAddress = "signin.address"
+
+// linkTimeFormat pins how expires_at is written AND compared.
+//
+// The comparison is a string comparison, so both sides must be written
+// the same way. A value carrying a zone offset sorts differently from a
+// Z-suffixed one of the same instant, which would silently accept
+// expired links. UTC and RFC3339 everywhere, never local time.
+const linkTimeFormat = time.RFC3339
+
+// dayFormat coarsens a person's created_at to the day.
+//
+// This is this plan's choice, not a rule inherited from the spec: §3.3's
+// coarsening is explicitly about publication rather than storage. It is
+// still right here — it is strictly less information, and
+// people.created_at has no freshness role that a time of day would
+// serve.
+const dayFormat = "2006-01-02"
+
+// blindIndex is ruling 3: HMAC(pepper, lowercased address).
+//
+// The pepper never leaves the home (ruling 4). One on every instance
+// would turn a single instance compromise into an offline dictionary
+// attack against the whole suite directory.
+//
+// A tension worth stating rather than hiding: §3.1 prefers "an expensive
+// KDF rather than a bare HMAC", pricing a dictionary run against a
+// leaked pepper. That is a mitigation note in the threat-model section;
+// ruling 3, in §1's settled table, says HMAC, and a settled ruling wins
+// over a note. Filed for Paul as a spec question, not resolved here.
+func blindIndex(pepper []byte, address string) string {
+ m := hmac.New(sha256.New, pepper)
+ m.Write([]byte(strings.ToLower(address)))
+ return hex.EncodeToString(m.Sum(nil))
+}
+
+// linkStore is the signin.LinkStore this app hands to signin.Flow. It
+// seals before it writes and takes exactly once.
+type linkStore struct {
+ db *db.DB
+ sealer *seal.Sealer
+ logger *slog.Logger
+}
+
+// PutLink seals the address and stores the link.
+//
+// Seal before the insert, never "insert then clean up": Litestream ships
+// WAL frames continuously, so a plaintext write reaches the bucket
+// before any delete could remove it.
+func (s *linkStore) PutLink(ctx context.Context, hash, address, purpose string, expires time.Time) error {
+ sealed, err := s.sealer.Seal(sealPurposeAddress, []byte(address))
+ if err != nil {
+ return err
+ }
+ _, err = s.db.Writer().ExecContext(ctx,
+ `INSERT INTO signin_links (token_hash, sealed_address, purpose, expires_at)
+ VALUES (?, ?, ?, ?)`,
+ hash, sealed, purpose, expires.UTC().Format(linkTimeFormat))
+ if err != nil {
+ return err
+ }
+
+ // Sweep after the write, and never fail the write on it. Sealed rows
+ // are not a leak; a table nothing deletes from is a bug waiting for a
+ // big enough database. Refusing a sign-in because the housekeeping
+ // failed would trade a hygiene problem for an outage.
+ if err := s.sweep(ctx, time.Now()); err != nil {
+ s.logger.Warn("sweeping expired sign-in links", "err", err)
+ }
+ return nil
+}
+
+// TakeLink consumes a link in one statement.
+//
+// Exactly one concurrent caller gets a row, so two clicks mint one
+// session. Unknown, expired and already-used are all "no row", so a
+// handler cannot tell them apart and turn the difference into an oracle.
+func (s *linkStore) TakeLink(ctx context.Context, hash, purpose string) (string, bool, error) {
+ var sealed []byte
+ err := s.db.Writer().QueryRowContext(ctx,
+ `DELETE FROM signin_links
+ WHERE token_hash = ? AND purpose = ? AND expires_at > ?
+ RETURNING sealed_address`,
+ hash, purpose, time.Now().UTC().Format(linkTimeFormat)).Scan(&sealed)
+ if errors.Is(err, sql.ErrNoRows) {
+ return "", false, nil
+ }
+ if err != nil {
+ return "", false, err
+ }
+
+ address, err := s.sealer.Open(sealPurposeAddress, sealed)
+ if err != nil {
+ return "", false, err
+ }
+ return string(address), true, nil
+}
+
+// sweep deletes links that have expired.
+func (s *linkStore) sweep(ctx context.Context, now time.Time) error {
+ _, err := s.db.Writer().ExecContext(ctx,
+ `DELETE FROM signin_links WHERE expires_at <= ?`,
+ now.UTC().Format(linkTimeFormat))
+ return err
+}
+
+// findOrMintPerson returns the person ref for an address, minting one on
+// first sight.
+//
+// One statement, so a returning person and a first-time person take the
+// same path. Without the upsert, a UNIQUE blind index and a fresh ref on
+// every visit would attach each session, credential and recovery code to
+// an identity nobody can ever return to.
+//
+// The DO UPDATE looks like a no-op and is not: SQLite's RETURNING yields
+// no row for a conflict that does nothing, so writing the index back to
+// itself is what makes the existing ref come out.
+func (a *App) findOrMintPerson(ctx context.Context, address string) (string, error) {
+ ref, err := NewPersonRef()
+ if err != nil {
+ return "", err
+ }
+ var got string
+ err = a.db.Writer().QueryRowContext(ctx,
+ `INSERT INTO people (person_ref, blind_index, created_at)
+ VALUES (?, ?, ?)
+ ON CONFLICT(blind_index) DO UPDATE SET blind_index = excluded.blind_index
+ RETURNING person_ref`,
+ ref, blindIndex(a.secrets.Pepper, address), time.Now().UTC().Format(dayFormat)).
+ Scan(&got)
+ if err != nil {
+ return "", err
+ }
+ return got, nil
+}
diff --git a/internal/home/links_test.go b/internal/home/links_test.go
new file mode 100644
index 0000000..3dd64b0
--- /dev/null
+++ b/internal/home/links_test.go
@@ -0,0 +1,266 @@
+package home
+
+import (
+ "context"
+ "errors"
+ "strings"
+ "sync"
+ "testing"
+ "time"
+
+ "github.com/keymaildev/signin"
+
+ "amadan.net/oficina/home/internal/home/sealtest"
+)
+
+// errWrongAddress is reported from a racing goroutine, where t.Errorf
+// would be called from a non-test goroutine.
+var errWrongAddress = errors.New("TakeLink returned the wrong address")
+
+// The whole point of the column. A link's address is at rest between
+// send and click; it is sealed there, and this reads the raw file, the
+// WAL and the SHM to prove it.
+func TestPutLinkSealsTheAddress(t *testing.T) {
+ app := newTestApp(t)
+ ctx := context.Background()
+
+ err := app.links.PutLink(ctx, "hash-1", sealtest.CanaryAddress, signin.PurposeSignin,
+ time.Now().Add(15*time.Minute))
+ if err != nil {
+ t.Fatalf("PutLink: %v", err)
+ }
+
+ sealtest.AssertNoPlaintext(t, app.dbPath)
+}
+
+func TestTakeLinkReturnsTheAddressOnce(t *testing.T) {
+ app := newTestApp(t)
+ ctx := context.Background()
+ const addr = "person@example.invalid"
+
+ if err := app.links.PutLink(ctx, "hash-2", addr, signin.PurposeSignin,
+ time.Now().Add(15*time.Minute)); err != nil {
+ t.Fatalf("PutLink: %v", err)
+ }
+
+ got, ok, err := app.links.TakeLink(ctx, "hash-2", signin.PurposeSignin)
+ if err != nil {
+ t.Fatalf("TakeLink: %v", err)
+ }
+ if !ok {
+ t.Fatal("first TakeLink reported ok == false")
+ }
+ if got != addr {
+ t.Fatalf("TakeLink = %q, want %q", got, addr)
+ }
+
+ // A second take must be indistinguishable from a link that never
+ // existed: the row is gone, not marked.
+ _, ok, err = app.links.TakeLink(ctx, "hash-2", signin.PurposeSignin)
+ if err != nil {
+ t.Fatalf("second TakeLink: %v", err)
+ }
+ if ok {
+ t.Fatal("second TakeLink succeeded; the link was redeemed twice")
+ }
+}
+
+// Unknown, expired and already-used must all be "no row", so that a
+// handler cannot tell them apart and turn the difference into an oracle.
+func TestTakeLinkIsIndistinguishableAcrossFailures(t *testing.T) {
+ app := newTestApp(t)
+ ctx := context.Background()
+
+ if err := app.links.PutLink(ctx, "expired", "person@example.invalid",
+ signin.PurposeSignin, time.Now().Add(-time.Minute)); err != nil {
+ t.Fatalf("PutLink: %v", err)
+ }
+ if err := app.links.PutLink(ctx, "wrong-purpose", "person@example.invalid",
+ "something-else", time.Now().Add(15*time.Minute)); err != nil {
+ t.Fatalf("PutLink: %v", err)
+ }
+
+ for _, tc := range []struct{ name, hash, purpose string }{
+ {"never existed", "no-such-hash", signin.PurposeSignin},
+ {"expired", "expired", signin.PurposeSignin},
+ {"wrong purpose", "wrong-purpose", signin.PurposeSignin},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ addr, ok, err := app.links.TakeLink(ctx, tc.hash, tc.purpose)
+ if err != nil {
+ t.Fatalf("TakeLink: %v", err)
+ }
+ if ok {
+ t.Fatal("TakeLink reported ok == true")
+ }
+ if addr != "" {
+ t.Fatalf("TakeLink returned %q alongside ok == false", addr)
+ }
+ })
+ }
+}
+
+// Two clicks on the same link, at the same moment, must mint one
+// session and not two. That is a property of the single DELETE …
+// RETURNING statement, so it is asserted rather than assumed.
+func TestConcurrentTakeLinkYieldsOneWinner(t *testing.T) {
+ app := newTestApp(t)
+ ctx := context.Background()
+ const addr = "person@example.invalid"
+
+ if err := app.links.PutLink(ctx, "race", addr, signin.PurposeSignin,
+ time.Now().Add(15*time.Minute)); err != nil {
+ t.Fatalf("PutLink: %v", err)
+ }
+
+ const racers = 16
+ var (
+ wg sync.WaitGroup
+ mu sync.Mutex
+ winners int
+ errs []error
+ )
+ start := make(chan struct{})
+ for range racers {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ <-start
+ got, ok, err := app.links.TakeLink(ctx, "race", signin.PurposeSignin)
+ mu.Lock()
+ defer mu.Unlock()
+ if err != nil {
+ errs = append(errs, err)
+ return
+ }
+ if ok {
+ winners++
+ if got != addr {
+ errs = append(errs, errWrongAddress)
+ }
+ }
+ }()
+ }
+ close(start)
+ wg.Wait()
+
+ for _, err := range errs {
+ t.Errorf("racer: %v", err)
+ }
+ if winners != 1 {
+ t.Fatalf("%d racers redeemed the link, want exactly 1", winners)
+ }
+}
+
+func TestSweepExpiredLinksRemovesOnlyExpiredOnes(t *testing.T) {
+ app := newTestApp(t)
+ ctx := context.Background()
+
+ if err := app.links.PutLink(ctx, "old", "a@example.invalid",
+ signin.PurposeSignin, time.Now().Add(-time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ if err := app.links.PutLink(ctx, "live", "b@example.invalid",
+ signin.PurposeSignin, time.Now().Add(time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+
+ if err := app.links.sweep(ctx, time.Now()); err != nil {
+ t.Fatalf("sweep: %v", err)
+ }
+
+ var n int
+ if err := app.db.Writer().QueryRow("SELECT count(*) FROM signin_links").Scan(&n); err != nil {
+ t.Fatal(err)
+ }
+ if n != 1 {
+ t.Fatalf("%d rows survived the sweep, want 1", n)
+ }
+ if _, ok, err := app.links.TakeLink(ctx, "live", signin.PurposeSignin); err != nil || !ok {
+ t.Fatalf("the live link did not survive the sweep (ok=%v err=%v)", ok, err)
+ }
+}
+
+// Ruling 3: HMAC(pepper, lowercased address). Three properties matter —
+// it folds case, it is stable, and it is pepper-dependent. The third is
+// what makes a leaked database without the pepper useless.
+func TestBlindIndex(t *testing.T) {
+ pepper := []byte("pepper-one-pepper-one-pepper-one")
+ other := []byte("pepper-two-pepper-two-pepper-two")
+
+ a := blindIndex(pepper, "Person@Example.Invalid")
+ b := blindIndex(pepper, "person@example.invalid")
+ if a != b {
+ t.Errorf("blind index does not fold case: %q vs %q", a, b)
+ }
+ if a != blindIndex(pepper, "person@example.invalid") {
+ t.Error("blind index is not stable across calls")
+ }
+ if a == blindIndex(other, "person@example.invalid") {
+ t.Error("blind index does not depend on the pepper")
+ }
+ if strings.Contains(a, "@") || strings.Contains(a, "example") {
+ t.Errorf("blind index %q carries the address", a)
+ }
+}
+
+// A returning person is the same person. With a UNIQUE blind index and
+// no upsert, every visit would mint a fresh ref and every session,
+// credential and recovery code would attach to an identity nobody can
+// return to.
+func TestFindOrMintPersonIsStable(t *testing.T) {
+ app := newTestApp(t)
+ ctx := context.Background()
+
+ first, err := app.findOrMintPerson(ctx, "Person@Example.Invalid")
+ if err != nil {
+ t.Fatalf("findOrMintPerson: %v", err)
+ }
+ if !ValidPersonRef(first) {
+ t.Fatalf("minted %q, which ValidPersonRef rejects", first)
+ }
+
+ again, err := app.findOrMintPerson(ctx, "person@example.invalid")
+ if err != nil {
+ t.Fatalf("findOrMintPerson: %v", err)
+ }
+ if again != first {
+ t.Errorf("the same address minted two refs: %q then %q", first, again)
+ }
+
+ other, err := app.findOrMintPerson(ctx, "someone-else@example.invalid")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if other == first {
+ t.Error("two addresses share one person ref")
+ }
+}
+
+// The people table must never hold a readable address either.
+func TestFindOrMintPersonStoresNoAddress(t *testing.T) {
+ app := newTestApp(t)
+ if _, err := app.findOrMintPerson(context.Background(), sealtest.CanaryAddress); err != nil {
+ t.Fatalf("findOrMintPerson: %v", err)
+ }
+ sealtest.AssertNoPlaintext(t, app.dbPath)
+}
+
+// created_at is coarsened to the day. Strictly less information, and
+// people.created_at has no freshness role that a time of day would serve.
+func TestPersonCreatedAtIsCoarsenedToTheDay(t *testing.T) {
+ app := newTestApp(t)
+ ref, err := app.findOrMintPerson(context.Background(), "person@example.invalid")
+ if err != nil {
+ t.Fatal(err)
+ }
+ var createdAt string
+ if err := app.db.Writer().
+ QueryRow("SELECT created_at FROM people WHERE person_ref = ?", ref).
+ Scan(&createdAt); err != nil {
+ t.Fatal(err)
+ }
+ if len(createdAt) != len("2006-01-02") {
+ t.Errorf("created_at = %q, want a date with no time of day", createdAt)
+ }
+}
diff --git a/internal/home/migrations/0002_signin_links.sql b/internal/home/migrations/0002_signin_links.sql
new file mode 100644
index 0000000..69a4bc9
--- /dev/null
+++ b/internal/home/migrations/0002_signin_links.sql
@@ -0,0 +1,24 @@
+-- A magic link's address is at rest between send and click. It is sealed
+-- there (sealed_address), and gate 1 reads the raw file, the WAL and the
+-- SHM to prove it.
+--
+-- There is no redeemed_at column and its absence is the design. A row
+-- that survives redemption lets a handler tell "already used" apart from
+-- "never existed", which is an oracle; and marking it redeemed makes the
+-- take a read-then-write race, where two clicks that both read a NULL
+-- both mint a session. Taking a link is therefore one DELETE ... RETURNING
+-- statement, and unknown, expired and already-used are all "no row" by
+-- construction rather than by remembering to make them so.
+--
+-- Deleting is safe here where it would not be elsewhere: the column is
+-- sealed, so the bytes Litestream keeps are bytes nobody can read. That
+-- is also why PRAGMA secure_delete is on regardless (internal/home/db.go).
+CREATE TABLE signin_links (
+ token_hash TEXT PRIMARY KEY,
+ sealed_address BLOB NOT NULL,
+ purpose TEXT NOT NULL,
+ expires_at TEXT NOT NULL
+);
+
+-- The sweep deletes by expiry, and it runs on every put.
+CREATE INDEX idx_signin_links_expires_at ON signin_links(expires_at);
diff --git a/internal/home/seal/seal.go b/internal/home/seal/seal.go
index 3f69fe8..9228590 100644
--- a/internal/home/seal/seal.go
+++ b/internal/home/seal/seal.go
@@ -6,9 +6,16 @@
// as platform config and is never written to the database it decrypts.
//
// Purpose keys are derived with HKDF so that a value sealed under one
-// purpose cannot be opened under another. That is not paranoia: these are
-// adjacent columns of the same row, and a bug that swapped them would be
-// invisible without this.
+// purpose cannot be opened under another. In the home these live in three
+// different tables — signin_links, teams, the provisioning queue — so the
+// swap this prevents is not a mistyped column name but a store handed the
+// wrong Sealer purpose, which no type checks and no test would notice
+// while both values are just opaque bytes.
+//
+// (Sheets' version of this comment says "adjacent columns of the same
+// row", which is true of its cell.input/cell.cached pair and not of
+// anything here. The sentence was carried over with the port and is
+// corrected rather than kept for symmetry.)
//
// Ported from oficina/sheets-core's internal/sheets/seal, deliberately
// unchanged apart from this doc and the HKDF info string. Two apps in one
diff --git a/internal/home/seal/seal_test.go b/internal/home/seal/seal_test.go
index b546515..6be7312 100644
--- a/internal/home/seal/seal_test.go
+++ b/internal/home/seal/seal_test.go
@@ -34,9 +34,10 @@ func TestSealOpenRoundTrip(t *testing.T) {
}
}
-// A value sealed under one purpose must not open under another. These are
-// adjacent columns of the same row; a bug that swapped them would be
-// invisible without this.
+// A value sealed under one purpose must not open under another. In the
+// home these are three different tables, so what this catches is a store
+// handed the wrong purpose — invisible to the type system, and invisible
+// to any test that only round-trips through the same purpose it wrote.
func TestWrongPurposeFails(t *testing.T) {
s, _ := New(testDEK())
ct, _ := s.Seal("signin.address", []byte("secret"))
diff --git a/internal/home/seal/vectors_test.go b/internal/home/seal/vectors_test.go
new file mode 100644
index 0000000..739d2a8
--- /dev/null
+++ b/internal/home/seal/vectors_test.go
@@ -0,0 +1,79 @@
+package seal
+
+import (
+ "crypto/sha256"
+ "encoding/hex"
+ "io"
+ "testing"
+
+ "golang.org/x/crypto/hkdf"
+)
+
+// purposeKeyWithInfo derives a key exactly as purposeKey does but with an
+// info string the caller supplies, so a test can derive Sheets' key from
+// the same DEK and compare. It is a copy of the two lines that matter
+// rather than a parameter on the real function: the info string being
+// fixed is the property under test.
+func purposeKeyWithInfo(s *Sealer, info string) ([]byte, error) {
+ key := make([]byte, 32)
+ r := hkdf.New(sha256.New, s.dek, nil, []byte(info))
+ if _, err := io.ReadFull(r, key); err != nil {
+ return nil, err
+ }
+ return key, nil
+}
+
+// Pins the HKDF derivation. If this changes, every sealed row in every
+// deployed home becomes unreadable — so it changes only with a
+// migration, never as a refactor.
+//
+// This is the one test that would catch the drift the port was careful
+// about: seal.go is a copy of Sheets' with a different info string, and
+// nothing else in this package would notice if a later edit made the two
+// derive the same key or a different one by accident.
+func TestPurposeKeyVectors(t *testing.T) {
+ s, err := New(testDEK())
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, tc := range []struct{ purpose, want string }{
+ {"signin.address", "1c677fc03245425f809a5a8ba5fda280f745ee96babd14ac8a75857ec9c7fe00"},
+ {"team.name", "8cadd5d2a5921541918563b0046844c371de29743a4b9335759da8c345bfc732"},
+ {"queue.payload", "2d6b4e963b93489c6a9515469f274516e48014d0bdb01831a9ff65086d0cb999"},
+ } {
+ key, err := s.purposeKey(tc.purpose)
+ if err != nil {
+ t.Fatal(err)
+ }
+ got := hex.EncodeToString(key)
+ if tc.want == "" {
+ t.Logf("RECORD %s = %s", tc.purpose, got)
+ continue
+ }
+ if got != tc.want {
+ t.Fatalf("%s key = %s, want %s", tc.purpose, got, tc.want)
+ }
+ }
+}
+
+// The home's keys must differ from Sheets' for the same purpose name.
+// That is the entire content of the one-line change the port made, and
+// nothing else asserts it.
+func TestHomeInfoStringIsDistinct(t *testing.T) {
+ s, err := New(testDEK())
+ if err != nil {
+ t.Fatal(err)
+ }
+ home, err := s.purposeKey("cell.input")
+ if err != nil {
+ t.Fatal(err)
+ }
+ // Derived the way Sheets does, from the same DEK.
+ sheets, err := purposeKeyWithInfo(s, "oficina/sheets:cell.input")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if hex.EncodeToString(home) == hex.EncodeToString(sheets) {
+ t.Fatal("the home and Sheets derive the same key for the same purpose")
+ }
+}
diff --git a/internal/home/sealtest/sealtest.go b/internal/home/sealtest/sealtest.go
index bfc9f1a..2efb20d 100644
--- a/internal/home/sealtest/sealtest.go
+++ b/internal/home/sealtest/sealtest.go
@@ -48,9 +48,18 @@ var Canaries = [][]byte{
// Split out from AssertNoPlaintext so the detection logic can be
// unit-tested directly, without needing a live *testing.T failure to
// observe.
+//
+// The scan folds case. CanaryAddress is already lowercase, because the
+// blind index and signin's limiter key on strings.ToLower(address); but
+// the team-name and queue canaries are not, and a team name is
+// normalised for reservation, so the copy that reaches a store may be
+// lowercased on the way. A case-sensitive scan would walk straight past
+// it — which is the same class of miss that made an upper-case address
+// canary wrong in the first place.
func leaked(b []byte) []byte {
+ lower := bytes.ToLower(b)
for _, c := range Canaries {
- if bytes.Contains(b, c) {
+ if bytes.Contains(lower, bytes.ToLower(c)) {
return c
}
}
diff --git a/internal/home/sealtest/sealtest_test.go b/internal/home/sealtest/sealtest_test.go
index f38198f..9ae3f09 100644
--- a/internal/home/sealtest/sealtest_test.go
+++ b/internal/home/sealtest/sealtest_test.go
@@ -117,3 +117,20 @@ func TestFindLeakChecksWAL(t *testing.T) {
t.Fatalf("found in %q, want leaky.db-wal", f)
}
}
+
+// A canary that only ever appears lowercased must still be caught. The
+// team-name and queue canaries are written upper-case, and a team name
+// is normalised for reservation, so the copy that reaches a store may
+// not be the copy Canaries holds.
+func TestLeakedFoldsCase(t *testing.T) {
+ for _, c := range Canaries {
+ lower := []byte(strings.ToLower(string(c)))
+ if leaked(lower) == nil {
+ t.Errorf("leaked(%q) = nil; a lowercased canary walked past the scan", lower)
+ }
+ upper := []byte(strings.ToUpper(string(c)))
+ if leaked(upper) == nil {
+ t.Errorf("leaked(%q) = nil; an uppercased canary walked past the scan", upper)
+ }
+ }
+}
diff --git a/internal/home/testapp_test.go b/internal/home/testapp_test.go
index 1e2f85e..864ca94 100644
--- a/internal/home/testapp_test.go
+++ b/internal/home/testapp_test.go
@@ -13,8 +13,8 @@ import (
)
// captureMailer is the Mailer every test App runs on. It records instead
-// of sending, and it is safe for concurrent use because mail.Sender's
-// contract requires that of implementations.
+// of sending. The mutex is what makes it safe for concurrent use;
+// mail.Sender's contract is why it has to be.
type captureMailer struct {
mu sync.Mutex
sent []sentMail
@@ -109,5 +109,9 @@ func newTestApp(t *testing.T) *testApp {
t.Fatalf("migrate.Apply: %v", err)
}
- return &testApp{App: New(secrets, d), mailer: mailer, db: d, dbPath: dbPath}
+ app, err := New(secrets, d, quietLogger())
+ if err != nil {
+ t.Fatalf("New: %v", err)
+ }
+ return &testApp{App: app, mailer: mailer, db: d, dbPath: dbPath}
}