oficina / meet Public

fix: final review of slice 1

sameAnswer narrowed to spec 3.2 (address and rate outcomes pass
through); force=1 documented; per-link budget consulted before the
shared pool; budget keys evicted; go mod tidy; latestPending tiebreak;
title() logs an open failure; the two waived 13 gates written:
member-address exhaustion of the shared limiter, and an expired pending
row admitting nobody.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 1 month ago 92728a80796691142bb0220b68b5957b890df8b9 parent 82cbc1c

7 files changed, +96 −13

go.mod +1 −1
internal/meet/app.go +1 −1
internal/meet/auth.go +23 −6
internal/meet/budget.go +7 −2
internal/meet/store.go +5 −2
internal/meet/store_test.go +1 −1
internal/meettest/join_test.go +58 −0

CI passed — run details

diff --git a/go.mod b/go.mod

index 5fda04c..a20abe5 100644

--- a/go.mod

+++ b/go.mod

@@ -6,6 +6,7 @@ require (

amadan.net/rastrillo/rastrillo v0.26.0

github.com/go-chi/chi/v5 v5.3.2

golang.org/x/crypto v0.56.0

+ gorm.io/gorm v1.31.2

)

require (

@@ -20,7 +21,6 @@ require (

github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect

golang.org/x/sys v0.47.0 // indirect

golang.org/x/text v0.41.0 // indirect

- gorm.io/gorm v1.31.2 // indirect

gorm.io/plugin/dbresolver v1.6.2 // indirect

modernc.org/libc v1.74.1 // indirect

modernc.org/mathutil v1.7.1 // indirect

diff --git a/internal/meet/app.go b/internal/meet/app.go

index a08d8a2..d32cddb 100644

--- a/internal/meet/app.go

+++ b/internal/meet/app.go

@@ -51,7 +51,7 @@ func build(d *db.DB, cfg Config) (*http.ServeMux, *app, error) {

rc.Logger.Info("migrate", "applied", migrated.Applied,

"skipped", migrated.Skipped, "adopted", migrated.Adopted)

- st := &store{g: d.G, sealer: sealer, now: time.Now}

+ st := &store{g: d.G, sealer: sealer, now: time.Now, logger: rc.Logger}

a := &app{

db: d, cfg: rc, sealer: sealer, store: st, logger: rc.Logger,

linkBudget: newBudget(5, 15*time.Minute),

diff --git a/internal/meet/auth.go b/internal/meet/auth.go

index 7635394..d412ef2 100644

--- a/internal/meet/auth.go

+++ b/internal/meet/auth.go

@@ -44,10 +44,12 @@ func (a *app) authorize(address string) bool {

return live

}

-// gateBegin sits in front of rastrillo's Begin. The guarantee: every

-// answer is byte-identical, 303 Location: /signin?sent=1, whatever the

-// address's standing, so the endpoint is not a membership oracle. Task 6

-// adds the pending-join exception in front of the allowlist check.

+// gateBegin sits in front of rastrillo's Begin. Meet's own refusals are

+// byte-identical to success; rastrillo's address and rate outcomes are

+// shown; membership of a specific address is not a secret Meet keeps at

+// this form, per spec §3.2, and the timing of a sent mail is a further

+// channel the response cannot hide. Task 6 adds the pending-join

+// exception in front of the allowlist check.

func (a *app) gateBegin(next http.HandlerFunc) http.HandlerFunc {

return func(w http.ResponseWriter, r *http.Request) {

_ = r.ParseForm()

@@ -59,6 +61,11 @@ func (a *app) gateBegin(next http.HandlerFunc) http.HandlerFunc {

if r.Form == nil {

r.Form = url.Values{}

}

+ // force makes rastrillo skip its address classifier, so Begin

+ // never takes the keymail branch (a 303 to an external host

+ // chosen by the address's DNS, which would be a distinguishable

+ // outcome and an oracle); it also means keymail sign-in is

+ // disabled app-wide, deliberately.

r.Form.Set("force", "1")

next(&sameAnswer{ResponseWriter: w, app: a}, r)

}

@@ -87,7 +94,9 @@ func (a *app) mayRequestLink(w http.ResponseWriter, r *http.Request, address str

}

if !member {

now := a.store.now()

- if !a.outsiderBudget.allow("all", now) || !a.linkBudget.allow(ck.TokenHash, now) {

+ // The per-link budget is consulted first: a link already over

+ // its own budget must not keep charging the shared pool.

+ if !a.linkBudget.allow(ck.TokenHash, now) || !a.outsiderBudget.allow("all", now) {

a.logger.Warn("meet: outsider sign-in attempts over budget", "link", ck.TokenHash[:8])

return false

}

@@ -109,13 +118,21 @@ func (a *app) mayRequestLink(w http.ResponseWriter, r *http.Request, address str

// sameAnswer rewrites rastrillo's own refusal redirects to the sent

// location. It hooks WriteHeader because http.Redirect sets Location

// and then calls WriteHeader, so the Location is visible in time.

+// Meet's own refusals are byte-identical to success; rastrillo's

+// address and rate outcomes are shown; membership of a specific

+// address is not a secret Meet keeps at this form, per spec §3.2, and

+// the timing of a sent mail is a further channel the response cannot

+// hide.

type sameAnswer struct {

http.ResponseWriter

app *app

}

func (s *sameAnswer) WriteHeader(status int) {

- if loc := s.Header().Get("Location"); loc != "" && loc != sentLocation && strings.HasPrefix(loc, signinPath+"?err=") {

+ loc := s.Header().Get("Location")

+ rewrite := loc != "" && strings.HasPrefix(loc, signinPath+"?err=") &&

+ loc != signinPath+"?err=address" && loc != signinPath+"?err=rate"

+ if rewrite {

s.app.logger.Warn("meet: sign-in refused; answering as if sent", "outcome", loc)

s.Header().Set("Location", sentLocation)

}

diff --git a/internal/meet/budget.go b/internal/meet/budget.go

index 746eb0c..5fc55f6 100644

--- a/internal/meet/budget.go

+++ b/internal/meet/budget.go

@@ -8,7 +8,8 @@ import (

// budget is a sliding-window counter: at most limit events per key per

// window. It counts attempts, refused or not, because the point is to

// keep outsider traffic away from rastrillo's shared limiter (spec

-// §3.2), and an attempt that reaches Begin is what consumes that.

+// §3.2), and an attempt that reaches Begin is what consumes that. The

+// budgets are process-local and reset on every deploy or restart.

type budget struct {

mu sync.Mutex

limit int

@@ -31,8 +32,12 @@ func (b *budget) allow(key string, now time.Time) bool {

kept = append(kept, t)

}

- if len(kept) >= b.limit {

+ if len(kept) == 0 {

+ delete(b.hits, key)

+ } else {

b.hits[key] = kept

+ }

+ if len(kept) >= b.limit {

return false

}

b.hits[key] = append(kept, now)

diff --git a/internal/meet/store.go b/internal/meet/store.go

index e033444..10469f7 100644

--- a/internal/meet/store.go

+++ b/internal/meet/store.go

@@ -4,6 +4,7 @@ import (

"context"

"errors"

"fmt"

+ "log/slog"

"strings"

"time"

@@ -27,6 +28,7 @@ type store struct {

g *gorm.DB

sealer *seal.Sealer

now func() time.Time

+ logger *slog.Logger

}

func actorFor(p Person) string { return fmt.Sprintf("person:%d", p.ID) }

@@ -69,6 +71,7 @@ type meetingView struct {

func (s *store) title(m Meeting) string {

t, err := s.sealer.OpenString(seal.PurposeMeetingTitle, m.TitleSealed)

if err != nil {

+ s.logger.Error("meet: open title", "meeting_id", m.ID, "err", err)

return "(title unreadable)"

}

return t

@@ -279,7 +282,7 @@ func (s *store) latestPending(ctx context.Context, address string) (PendingJoin,

}

var p PendingJoin

err = s.g.WithContext(ctx).Where("address_fingerprint = ? AND expires_at > ?", fp, s.now().UTC()).

- Order("created_at DESC").Take(&p).Error

+ Order("created_at DESC, id DESC").Take(&p).Error

if errors.Is(err, gorm.ErrRecordNotFound) {

return PendingJoin{}, false, nil

}

@@ -301,7 +304,7 @@ func (s *store) deletePending(ctx context.Context, address string) error {

func (s *store) consumePending(ctx context.Context, p Person, pend PendingJoin, member bool) (string, error) {

dest := ""

err := s.g.WithContext(ctx).Transaction(func(tx *gorm.DB) error {

- ts := &store{g: tx, sealer: s.sealer, now: s.now}

+ ts := &store{g: tx, sealer: s.sealer, now: s.now, logger: s.logger}

if err := ts.deletePending(ctx, p.Email); err != nil {

return err

}

diff --git a/internal/meet/store_test.go b/internal/meet/store_test.go

index 1fe5b3e..ce8f125 100644

--- a/internal/meet/store_test.go

+++ b/internal/meet/store_test.go

@@ -28,7 +28,7 @@ func testStore(t *testing.T) (*store, *time.Time) {

}

sl, _ := seal.New(bytes.Repeat([]byte{9}, 32))

now := time.Date(2026, 9, 8, 10, 0, 0, 0, time.UTC)

- return &store{g: d.G, sealer: sl, now: func() time.Time { return now }}, &now

+ return &store{g: d.G, sealer: sl, now: func() time.Time { return now }, logger: logger}, &now

}

func TestCreateMeetingSealsTitleAndMintsOneLiveLink(t *testing.T) {

diff --git a/internal/meettest/join_test.go b/internal/meettest/join_test.go

index 474d9a7..a674857 100644

--- a/internal/meettest/join_test.go

+++ b/internal/meettest/join_test.go

@@ -128,6 +128,64 @@ func TestOutsiderBudgetsStopMailAtFivePerLinkAndTenOverall(t *testing.T) {

}

+func TestMemberAddressExhaustsTheSharedLimiterWithTheSameAnswer(t *testing.T) {

+ x := newHarness(t, "paul@example.com")

+ for i := 0; i < 5; i++ {

+ g := x.browser(t)

+ rec := g.post("/signin", url.Values{"address": {"paul@example.com"}})

+ if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/signin?sent=1" {

+ t.Fatalf("attempt %d: %d %s, want 303 /signin?sent=1", i, rec.Code, rec.Header().Get("Location"))

+ }

+ g := x.browser(t)

+ rec := g.post("/signin", url.Values{"address": {"paul@example.com"}})

+ if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/signin?err=rate" {

+ t.Fatalf("sixth attempt: %d %s, want 303 /signin?err=rate", rec.Code, rec.Header().Get("Location"))

+ }

+ if got := x.mail.count(); got != 5 {

+ t.Fatalf("mail count: %d, want 5", got)

+ }

+ if body := g.get("/signin?err=rate").Body.String(); !strings.Contains(body, "Too many sign-in attempts") {

+ t.Fatal("?err=rate does not render rastrillo's rate-limit message")

+ }

+func TestExpiredPendingRowAdmitsNobody(t *testing.T) {

+ x, _, tok := memberWithMeeting(t)

+ g := x.browser(t)

+ g.get("/j/" + tok)

+ rec := g.post("/signin", url.Values{"address": {"guest@example.com"}})

+ if rec.Header().Get("Location") != "/signin?sent=1" {

+ t.Fatalf("outsider's request: %d %s", rec.Code, rec.Header().Get("Location"))

+ }

+ before := x.mail.count()

+ _, link := x.mail.last()

+ x.advance(16 * time.Minute)

+ u, err := url.Parse(link)

+ if err != nil {

+ t.Fatal(err)

+ }

+ if rec := g.get(u.RequestURI()); rec.Code != http.StatusForbidden {

+ t.Fatalf("expired pending row click: %d, want 403 from Authorize", rec.Code)

+ }

+ if got := x.mail.count(); got != before {

+ t.Fatalf("mail count changed after the expired click: %d, want %d", got, before)

+ }

+ x2, _, tok2 := memberWithMeeting(t)

+ m := x2.browser(t)

+ m.get("/j/" + tok2)

+ rec = m.post("/signin", url.Values{"address": {"paul@example.com"}})

+ if rec.Header().Get("Location") != "/signin?sent=1" {

+ t.Fatalf("member's request: %d %s", rec.Code, rec.Header().Get("Location"))

+ }

+ _, link2 := x2.mail.last()

+ x2.advance(16 * time.Minute)

+ if dest := m.follow(link2); dest != "/" {

+ t.Fatalf("signed-out member after an expired pending row landed on %s, want /", dest)

+ }

func TestExpiredGrantIsRefusedEntry(t *testing.T) {

x, sqid, tok := memberWithMeeting(t)

g := x.browser(t)