diff --git a/.env.example b/.env.example index 836d41f..074cb3c 100644 --- a/.env.example +++ b/.env.example @@ -44,6 +44,12 @@ TTS_AUDIO_FORMAT=mp3 # mp3 | opus | wav — mp3/opus transcode Pipe # them commented out for local development and Petal runs as the single # hardcoded `local` user, exactly as it did before auth landed. # +# That fallback is scoped to development on purpose. With a BASE_URL naming +# anything but localhost, Petal refuses to start rather than run open — see +# PETAL_REQUIRE_AUTH — because the fallback on a reachable host means every +# anonymous visitor is the `local` user, with full read and write over every +# document in the database. +# # AUTHENTIK_URL is the issuer of the Petal provider in Authentik (the value of # its "OpenID Configuration Issuer" field). The redirect URI to register there # is BASE_URL + /auth/callback. @@ -52,8 +58,15 @@ TTS_AUDIO_FORMAT=mp3 # mp3 | opus | wav — mp3/opus transcode Pipe # AUTHENTIK_CLIENT_SECRET= # # Who may sign in: comma-separated OIDC subject ids and/or email addresses. -# Empty = anyone Authentik authenticates. +# Empty = anyone Authentik authenticates, which is right for a single-household +# instance and wrong the moment the IdP serves a wider audience than Petal. An +# empty list is warned about at every boot rather than assumed either way. # PETAL_ALLOWED_SUBS=her@example.com,me@example.com +# +# Whether a missing OIDC configuration is fatal. Defaults to false for a +# loopback BASE_URL and true for anything else, so neither a laptop nor a +# deployment normally has to name it. +# PETAL_REQUIRE_AUTH=true # --- Deferred (not wired in the local-dev build) --- diff --git a/cmd/server/headers_test.go b/cmd/server/headers_test.go new file mode 100644 index 0000000..f63f328 --- /dev/null +++ b/cmd/server/headers_test.go @@ -0,0 +1,72 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// The baseline policy has to actually permit the frontend Vite builds. These +// are the allowances dist/index.html needs; if a future build starts emitting +// an inline script or pulling from a new host, that shows up here rather than +// as a blank page in production. +func TestBaselineCSPCoversTheBuiltFrontend(t *testing.T) { + required := []string{ + "script-src 'self'", // Vite emits no inline script + "'unsafe-inline' https://fonts.googleapis.com", // React style={{…}} + the font link + "https://fonts.gstatic.com", // the font files themselves + "blob:", // read-aloud plays an object URL + "object-src 'none'", + "base-uri 'self'", + "frame-ancestors 'self'", + } + for _, want := range required { + if !strings.Contains(contentSecurityPolicy, want) { + t.Errorf("baseline CSP is missing %q:\n%s", want, contentSecurityPolicy) + } + } +} + +// The middleware is a floor, not a ceiling: it runs *before* the handler +// precisely so a route serving untrusted bytes can overwrite the policy with a +// stricter one. This is the contract the image store depends on, and the reason +// the policy no longer lives in the Traefik labels — customresponseheaders +// would overwrite it in the other direction. +func TestRouteMayTightenTheBaselineCSP(t *testing.T) { + strict := "default-src 'none'; sandbox" + handler := securityHeaders(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Security-Policy", strict) + w.WriteHeader(http.StatusOK) + })) + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/images/x.svg", nil)) + + if got := rec.Header().Get("Content-Security-Policy"); got != strict { + t.Fatalf("handler's policy was not honoured: got %q, want %q", got, strict) + } + // The headers it didn't touch still stand. + if rec.Header().Get("X-Content-Type-Options") != "nosniff" { + t.Error("baseline nosniff was lost") + } +} + +// Every ordinary response carries the baseline. +func TestBaselineHeadersOnAPlainResponse(t *testing.T) { + handler := securityHeaders(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + + for header, want := range map[string]string{ + "Content-Security-Policy": contentSecurityPolicy, + "X-Content-Type-Options": "nosniff", + "Referrer-Policy": "same-origin", + } { + if got := rec.Header().Get(header); got != want { + t.Errorf("%s = %q, want %q", header, got, want) + } + } +} diff --git a/cmd/server/main.go b/cmd/server/main.go index a3989d1..e35a653 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -64,21 +64,45 @@ func main() { sessions := auth.NewSessionStore(database.DB) users := auth.NewUserStore(database.DB) + // …and the fallback is exactly what must not happen quietly on a public + // host. Refuse to start rather than serve someone's journals to the open + // internet because one environment variable was misspelled. See + // config.RequireAuth for why this defaults on for any non-loopback BASE_URL. + if !cfg.AuthEnabled() && cfg.RequireAuth { + log.Fatalf("auth: refusing to start unauthenticated at %s.\n"+ + " Petal would resolve every anonymous request to the single %q user, with full\n"+ + " read and write over every document in the database.\n"+ + " Set AUTHENTIK_URL, AUTHENTIK_CLIENT_ID and AUTHENTIK_CLIENT_SECRET, or set\n"+ + " PETAL_REQUIRE_AUTH=false if this really is a trusted private network.", + cfg.BaseURL, db.LocalUserID) + } + var resolver auth.Resolver = auth.StaticResolver(db.LocalUserID) var oidcClient *auth.OIDC if cfg.AuthEnabled() { + allowed := auth.ParseAllowlist(cfg.AllowedSubs) oidcClient = auth.NewOIDC(context.Background(), auth.Options{ IssuerURL: cfg.AuthentikURL, ClientID: cfg.AuthentikClientID, ClientSecret: cfg.AuthentikClientSecret, BaseURL: cfg.BaseURL, - Allowed: auth.ParseAllowlist(cfg.AllowedSubs), + Allowed: allowed, }, sessions, users) resolver = sessions if n, err := sessions.Prune(); err == nil && n > 0 { log.Printf("auth: pruned %d expired session(s)", n) } log.Printf("auth: OIDC enabled (issuer=%s, redirect=%s)", cfg.AuthentikURL, oidcClient.RedirectURI()) + // An empty allowlist is a legitimate choice for a single-household + // instance and a wide-open door in front of an IdP that fronts anything + // else. Petal cannot tell which it is, so it says so every boot rather + // than assuming. + if len(allowed) == 0 { + log.Printf("auth: WARNING — PETAL_ALLOWED_SUBS is empty, so EVERY account %s "+ + "authenticates may sign in and start writing here. Set it to the "+ + "comma-separated emails (or subject ids) that belong in this Petal.", + cfg.AuthentikURL) + } } else { log.Printf("auth: OIDC not configured — running as the single %q user", db.LocalUserID) } @@ -106,6 +130,7 @@ func main() { r.Use(middleware.RealIP) r.Use(middleware.Logger) r.Use(middleware.Recoverer) + r.Use(securityHeaders) // Build version: a hash of the embedded SPA shell. Vite rewrites index.html // with content-hashed asset names on every build, so this string changes @@ -231,6 +256,47 @@ func main() { } } +// contentSecurityPolicy is the default policy for everything Petal serves. +// +// It lives here rather than in the Traefik labels, and that move is the point: +// Traefik's customResponseHeaders *sets* a header, overwriting whatever the +// application chose, so a policy declared at the edge silently replaces the +// stricter one an individual route needs. Stored images need exactly that (an +// uploaded SVG is a document that can carry script — see internal/images), and +// a rule the edge can quietly undo is not a rule. +// +// The allowances are what the built frontend actually uses, no more: script +// only from Petal itself (Vite emits no inline script — this policy is checked +// against dist/index.html), inline *styles* because React's style={{…}} props +// compile to style attributes, and Google's font hosts because index.html links +// them. object-src and base-uri close the two attribute-injection routes that +// survive HTML escaping. +const contentSecurityPolicy = "default-src 'self'; " + + "script-src 'self'; " + + "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; " + + "font-src 'self' data: https://fonts.gstatic.com; " + + "img-src 'self' data: blob:; " + + "media-src 'self' data: blob:; " + + "connect-src 'self'; " + + "object-src 'none'; " + + "base-uri 'self'; " + + "form-action 'self'; " + + "frame-ancestors 'self'" + +// securityHeaders lays down the baseline response headers before the handler +// runs, so a route that needs something stricter — the image store — simply +// overwrites its own copy on the way past. Ordering is the mechanism: this is a +// floor, not a ceiling. +func securityHeaders(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + h := w.Header() + h.Set("Content-Security-Policy", contentSecurityPolicy) + h.Set("X-Content-Type-Options", "nosniff") + h.Set("Referrer-Policy", "same-origin") + next.ServeHTTP(w, r) + }) +} + // maxAPIBodyBytes caps a JSON API request body at 2 MiB. That's far above any // real document save (the body is text plus lightweight marks; images upload // separately by reference) while still bounding abuse. Exceeding it makes the diff --git a/deploy/README.md b/deploy/README.md index 0d413ba..d566a01 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -233,8 +233,21 @@ unauthenticated `/api/health` router that existed only to escape it: every `/api route now answers 401 without a session, and the only thing an anonymous visitor gets is the app shell and a redirect to sign in. -If you ever run this stack *without* `AUTHENTIK_*` configured — Petal then falls -back to the single `local` user — put the gate back before pointing DNS at it: +Removing that gate meant a missing `AUTHENTIK_*` variable stopped being a +nuisance and became an exposure: Petal would fall back to the single `local` +user and hand every anonymous visitor full read and write over the database, +saying so only in a log line. So **it now refuses to start instead**: + +``` +auth: refusing to start unauthenticated at https://petal.parodia.dev. + Petal would resolve every anonymous request to the single "local" user, ... +``` + +The guard defaults on for any `BASE_URL` that isn't loopback, so a laptop +checkout still runs open and a deployment cannot. If you genuinely want an +unauthenticated instance on a trusted private network, say so out loud with +`PETAL_REQUIRE_AUTH=false` — and if it is reachable from anywhere else, put a +gate back in front of it first: ```yaml traefik.http.routers.petal.middlewares: compression@file,petal-headers,petal-auth @@ -243,6 +256,28 @@ traefik.http.middlewares.petal-auth.basicauth.users: ${PETAL_BASIC_AUTH:?} with `htpasswd -nbB petal 'your-password'` in `.env` as `PETAL_BASIC_AUTH`. +### Who gets in + +`PETAL_ALLOWED_SUBS` is a comma-separated list of emails and/or OIDC subject +ids. **Set it.** Empty means everyone authentik authenticates, and authentik on +this host fronts several applications — a valid account there is not the same as +belonging in someone's private journal. An empty list is legal (a single- +household instance may want it) and warns at every boot: + +``` +auth: WARNING — PETAL_ALLOWED_SUBS is empty, so EVERY account ... may sign in +``` + +### Response headers + +`Content-Security-Policy`, `X-Content-Type-Options` and `Referrer-Policy` are set +by the binary, not by the Traefik labels. Traefik's `customresponseheaders` +*overwrites*, which would silently replace the stricter policy an individual +route picks for itself — the image store serves stored uploads under +`default-src 'none'; sandbox` so that an SVG someone pasted into a document +cannot run as a page on Petal's own origin. Only HSTS stays at the edge, where +TLS is actually terminated. + --- ## 4a. Moving an account (`scripts/migrate_local_user.py`) diff --git a/deploy/petal.env.example b/deploy/petal.env.example index bcc5c93..0a8ce18 100644 --- a/deploy/petal.env.example +++ b/deploy/petal.env.example @@ -20,13 +20,9 @@ TZ=Europe/Lisbon PETAL_UID=1001 PETAL_GID=1001 -# --- Interim edge gate (delete when Phase 16 auth lands) --------------------- -# Petal has no authentication of its own yet — StaticResolver hands every -# request the same local user — so Traefik holds the door with basic auth until -# the OIDC flow exists. user:bcrypt-hash, as produced by: -# htpasswd -nbB petal 'your-password' -# /api/health is deliberately exempt (its own router) so monitoring still works. -PETAL_BASIC_AUTH= +# (The interim PETAL_BASIC_AUTH edge gate is gone: Petal authenticates for +# itself now, and the auth block at the bottom of this file is what holds the +# door. A second password in front of a real login is one more thing to lose.) # --- LLM (millenia, over headscale) ------------------------------------------ # The only cross-VPN dependency. Petal degrades warmly when it's unreachable: @@ -67,16 +63,37 @@ TTS_AUDIO_FORMAT=mp3 TTS_TIMEOUT=15s # --- Auth (Authentik OIDC) --------------------------------------------------- -# Authentik already runs on this host. Set all three and Petal authenticates -# for itself; leave any unset and it falls back to the single `local` user -# (which on a public host means the Traefik basic-auth gate must stay). +# NOT OPTIONAL HERE. Authentik already runs on this host; set all three and +# Petal authenticates for itself. +# +# Leave any of them unset and Petal REFUSES TO START, because the alternative is +# worse: it would otherwise fall back to resolving every anonymous request to +# the single `local` user, handing the open internet full read and write over +# every document in the database. That fallback is right on a laptop and a +# catastrophe on this host, so the guard is on for any non-loopback BASE_URL. +# See PETAL_REQUIRE_AUTH below. # # AUTHENTIK_URL is the provider's issuer, and the redirect URI to register in # Authentik is https://petal.parodia.dev/auth/callback. -# AUTHENTIK_URL=https://auth.parodia.dev/application/o/petal/ -# AUTHENTIK_CLIENT_ID=petal -# AUTHENTIK_CLIENT_SECRET= +AUTHENTIK_URL=https://auth.parodia.dev/application/o/petal/ +AUTHENTIK_CLIENT_ID=petal +AUTHENTIK_CLIENT_SECRET= + +# Who may sign in: comma-separated subject ids and/or emails. # -# Who may sign in: comma-separated subject ids and/or emails. Empty = anyone -# Authentik authenticates, which is wider than this instance wants. -# PETAL_ALLOWED_SUBS= +# SET THIS. Empty means everyone Authentik authenticates, and Authentik on this +# host fronts half a dozen applications — being a valid user there is not the +# same as belonging in someone's private journal. An empty value is legal (a +# single-household instance may genuinely want it) and says so loudly in the +# startup log every boot. +# +# An email is knowable in advance; a subject id is an opaque uuid nobody can +# know before that person's first login. Use emails to invite, subject ids to +# pin. +PETAL_ALLOWED_SUBS=her@example.com,me@example.com + +# The guard itself. Defaulted from BASE_URL — loopback origins run open, real +# ones demand a login — so it does not normally need setting. Set it to false +# only for a deployment genuinely reachable from nowhere but a trusted network, +# and understand that it means anyone who reaches Petal is the `local` user. +# PETAL_REQUIRE_AUTH=true diff --git a/docker-compose.yml b/docker-compose.yml index 2ee7996..d959378 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -98,11 +98,18 @@ services: # second password in front of a real login is just one more thing to lose. traefik.http.routers.petal.middlewares: compression@file,petal-headers traefik.http.services.petal.loadbalancer.server.port: "8080" - # Petal is a private writing space: no framing, no sniffing, HSTS on. - traefik.http.middlewares.petal-headers.headers.customresponseheaders.Content-Security-Policy: frame-ancestors 'self' + # HSTS is the edge's business — it is a statement about the TLS + # termination, which happens here and not in the container. + # + # The Content-Security-Policy that used to sit alongside it has moved into + # the app (see securityHeaders in cmd/server/main.go). customresponseheaders + # *overwrites*, so a policy set here would silently replace the stricter + # one an individual route chooses for itself — which is exactly what the + # image store does to keep an uploaded SVG from running as a page. A rule + # the edge can quietly undo is not a rule. X-Content-Type-Options and + # Referrer-Policy moved with it for the same reason: one place to read, + # and no dependence on this file being deployed alongside the binary. traefik.http.middlewares.petal-headers.headers.customresponseheaders.Strict-Transport-Security: max-age=31536000; includeSubDomains - traefik.http.middlewares.petal-headers.headers.customresponseheaders.X-Content-Type-Options: nosniff - traefik.http.middlewares.petal-headers.headers.customresponseheaders.Referrer-Policy: same-origin piper-en: build: diff --git a/internal/auth/oidc.go b/internal/auth/oidc.go index aa07ead..7c9545a 100644 --- a/internal/auth/oidc.go +++ b/internal/auth/oidc.go @@ -129,7 +129,10 @@ func (o *OIDC) Routes() chi.Router { r := chi.NewRouter() r.Get("/login", o.login) r.Get("/callback", o.callback) - r.Get("/logout", o.logout) + // POST only. Signing out is a state change, and SameSite=Lax deliberately + // *does* send the session cookie on a top-level cross-site GET — so a GET + // route here means any page on the internet can sign her out mid-draft by + // linking to it, or embedding it as an image. Small harm, free to remove. r.Post("/logout", o.logout) return r } @@ -275,8 +278,8 @@ func (o *OIDC) callback(w http.ResponseWriter, r *http.Request) { // matters: clearing only the cookie leaves a token that still works if it was // ever captured. func (o *OIDC) logout(w http.ResponseWriter, r *http.Request) { - if c, err := r.Cookie(SessionCookie); err == nil && c.Value != "" { - if err := o.sessions.Revoke(c.Value); err != nil { + if token := SessionToken(r); token != "" { + if err := o.sessions.Revoke(token); err != nil { log.Printf("auth: revoke failed: %v", err) } } diff --git a/internal/auth/oidc_test.go b/internal/auth/oidc_test.go index 5965964..3850893 100644 --- a/internal/auth/oidc_test.go +++ b/internal/auth/oidc_test.go @@ -234,7 +234,7 @@ func TestLoginRoundTrip(t *testing.T) { // Signing out revokes server-side, not just in the browser. out := httptest.NewRecorder() - flow.ServeHTTP(out, jar.attach(httptest.NewRequest(http.MethodGet, "/logout", nil))) + flow.ServeHTTP(out, jar.attach(httptest.NewRequest(http.MethodPost, "/logout", nil))) if out.Code != http.StatusFound { t.Fatalf("logout status=%d", out.Code) } @@ -243,6 +243,19 @@ func TestLoginRoundTrip(t *testing.T) { } } +// Signing out is a state change, so it must not be reachable by GET: with +// SameSite=Lax the session cookie *is* sent on a top-level cross-site +// navigation, which would let any page on the internet sign her out mid-draft. +func TestLogoutRejectsGET(t *testing.T) { + _, flow, _, _ := newFlow(t, nil) + + out := httptest.NewRecorder() + flow.ServeHTTP(out, httptest.NewRequest(http.MethodGet, "/logout", nil)) + if out.Code != http.StatusMethodNotAllowed { + t.Fatalf("GET /logout status=%d, want 405", out.Code) + } +} + // A callback whose state doesn't match the cookie is a forged one. func TestCallbackRejectsBadState(t *testing.T) { idp, flow, sessions, _ := newFlow(t, nil) diff --git a/internal/auth/session.go b/internal/auth/session.go index 190ea4c..6b79ebd 100644 --- a/internal/auth/session.go +++ b/internal/auth/session.go @@ -11,8 +11,31 @@ import ( "time" ) -// SessionCookie is the cookie carrying the opaque session token. -const SessionCookie = "petal_session" +// The cookie carrying the opaque session token, in its two spellings. +// +// Over https the name takes the __Host- prefix, which is not decoration: the +// browser will only accept such a cookie if it is Secure, Path=/, and carries +// no Domain attribute — and, crucially, refuses to let any other host set it. +// Without the prefix, anything that can write cookies for a sibling name under +// parodia.dev (another service on the box, a subdomain takeover) can plant a +// session cookie in her browser that Petal will then read as hers. +// +// The prefix is impossible over plain http, because it requires Secure and a +// browser drops a Secure cookie on an insecure origin. So local development +// keeps the bare name, and the name in use follows the same `secure` flag the +// rest of the cookie does. +const ( + SessionCookie = "petal_session" + HostSessionCookie = "__Host-petal_session" +) + +// sessionCookieName is the name to *write* under this scheme. +func sessionCookieName(secure bool) string { + if secure { + return HostSessionCookie + } + return SessionCookie +} const ( // sessionTTL is how long a session lives without use. Thirty days, sliding: @@ -78,11 +101,28 @@ func (s *SessionStore) Create(userID, userAgent string) (string, error) { // Resolve implements [Resolver]: it reads the session cookie, validates it, and // returns the user it belongs to — extending the session's life while it does. func (s *SessionStore) Resolve(r *http.Request) (string, error) { - c, err := r.Cookie(SessionCookie) - if err != nil || c.Value == "" { + token := SessionToken(r) + if token == "" { return "", ErrNoSession } - return s.userFor(c.Value) + return s.userFor(token) +} + +// SessionToken pulls the raw session token out of a request, preferring the +// __Host- spelling. +// +// Both are read because a deployment that was signing people in before the +// prefix existed has browsers holding the old name; those sessions stay valid +// and quietly re-issue under the new name at the next sign-in. The prefixed one +// wins where both are present, since it is the one another host could not have +// planted. +func SessionToken(r *http.Request) string { + for _, name := range []string{HostSessionCookie, SessionCookie} { + if c, err := r.Cookie(name); err == nil && c.Value != "" { + return c.Value + } + } + return "" } // userFor validates a raw token and slides its expiry forward. @@ -146,7 +186,7 @@ func hashToken(token string) string { // browser drop the cookie and silently break local login. func SetSessionCookie(w http.ResponseWriter, token string, secure bool) { http.SetCookie(w, &http.Cookie{ - Name: SessionCookie, + Name: sessionCookieName(secure), Value: token, Path: "/", HttpOnly: true, @@ -158,14 +198,24 @@ func SetSessionCookie(w http.ResponseWriter, token string, secure bool) { // ClearSessionCookie expires the session cookie in the browser. The matching // server-side row must be revoked separately — that's the half that counts. +// +// Both spellings are expired, not just the one currently written: a browser +// carrying a pre-prefix cookie must not be left holding it after signing out, +// which is precisely the case where "clear the cookie" is the part the user can +// see working. func ClearSessionCookie(w http.ResponseWriter, secure bool) { - http.SetCookie(w, &http.Cookie{ - Name: SessionCookie, - Value: "", - Path: "/", - HttpOnly: true, - Secure: secure, - SameSite: http.SameSiteLaxMode, - MaxAge: -1, - }) + for _, name := range []string{HostSessionCookie, SessionCookie} { + if name == HostSessionCookie && !secure { + continue // the browser would reject a non-Secure __Host- cookie + } + http.SetCookie(w, &http.Cookie{ + Name: name, + Value: "", + Path: "/", + HttpOnly: true, + Secure: secure, + SameSite: http.SameSiteLaxMode, + MaxAge: -1, + }) + } } diff --git a/internal/auth/session_test.go b/internal/auth/session_test.go index 7860bf3..ffc7224 100644 --- a/internal/auth/session_test.go +++ b/internal/auth/session_test.go @@ -309,3 +309,67 @@ func TestAllowlist(t *testing.T) { } } } + +// Over https the cookie takes the __Host- prefix, which the browser will only +// accept from the exact host that set it — closing the door on a sibling +// service under the same registrable domain planting a session in her browser. +// Over plain http it cannot: the prefix requires Secure, and a browser drops a +// Secure cookie on an insecure origin, so local development would silently stop +// logging in. +func TestSessionCookieNamePerScheme(t *testing.T) { + secure := httptest.NewRecorder() + SetSessionCookie(secure, "tok", true) + c := secure.Result().Cookies()[0] + if c.Name != HostSessionCookie { + t.Fatalf("https cookie name=%q, want %q", c.Name, HostSessionCookie) + } + // The prefix is a promise about these three attributes; a browser rejects + // the cookie outright if any is wrong. + if !c.Secure || c.Path != "/" || c.Domain != "" { + t.Fatalf("__Host- cookie violates its own contract: %+v", c) + } + + insecure := httptest.NewRecorder() + SetSessionCookie(insecure, "tok", false) + if name := insecure.Result().Cookies()[0].Name; name != SessionCookie { + t.Fatalf("http cookie name=%q, want %q", name, SessionCookie) + } +} + +// A browser holding a cookie issued before the prefix existed must stay signed +// in — and start using the new name at its next sign-in, not be logged out to +// get there. +func TestResolveAcceptsEitherCookieName(t *testing.T) { + store, _, _ := newStores(t) + token, err := store.Create("bob", "test-agent") + if err != nil { + t.Fatal(err) + } + + for _, name := range []string{SessionCookie, HostSessionCookie} { + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.AddCookie(&http.Cookie{Name: name, Value: token}) + got, err := store.Resolve(r) + if err != nil || got != "bob" { + t.Fatalf("%s: resolved to %q (err=%v)", name, got, err) + } + } +} + +// Signing out must not leave the browser holding either spelling. +func TestClearSessionCookieExpiresBothNames(t *testing.T) { + rec := httptest.NewRecorder() + ClearSessionCookie(rec, true) + + cleared := map[string]bool{} + for _, c := range rec.Result().Cookies() { + if c.MaxAge < 0 { + cleared[c.Name] = true + } + } + for _, name := range []string{SessionCookie, HostSessionCookie} { + if !cleared[name] { + t.Errorf("%s was left in the browser after signing out", name) + } + } +} diff --git a/internal/config/config.go b/internal/config/config.go index fbc448d..16b2dba 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,7 +1,9 @@ package config import ( + "net/url" "os" + "strconv" "strings" "time" ) @@ -61,6 +63,22 @@ type Config struct { // authenticates — right for a single-household instance, wrong the moment // the IdP serves an audience wider than Petal's. AllowedSubs string + // RequireAuth refuses to start when OIDC isn't configured, instead of + // falling back to the single local user. + // + // The fallback is the right behaviour on a laptop and a catastrophe on a + // public host: a typo in AUTHENTIK_CLIENT_SECRET turns every anonymous + // visitor into the `local` user, with full read and write over someone's + // private journals, and says so only in a log line nobody is reading. The + // Traefik basic-auth gate that used to stand behind that mistake was + // removed when Petal learned to authenticate for itself, so nothing catches + // it now. + // + // Defaulted from BASE_URL rather than declared: a Petal that knows itself by + // a real public origin has no business running open, and one on localhost + // has no business demanding an IdP. Set PETAL_REQUIRE_AUTH explicitly to + // override in either direction. + RequireAuth bool } // TTSVoice is one Piper instance and the single voice it has loaded. @@ -77,9 +95,10 @@ func (c *Config) AuthEnabled() bool { // Load reads configuration from the environment, applying sane local-dev defaults. func Load() *Config { + baseURL := env("BASE_URL", "http://localhost:8080") return &Config{ Port: env("PORT", "8080"), - BaseURL: env("BASE_URL", "http://localhost:8080"), + BaseURL: baseURL, DatabasePath: env("DATABASE_PATH", "./data/petal.db"), ImageDir: env("IMAGE_DIR", "./data/images"), DictPath: env("DICT_PATH", "./data/dict.db"), @@ -101,9 +120,26 @@ func Load() *Config { AuthentikClientID: env("AUTHENTIK_CLIENT_ID", ""), AuthentikClientSecret: env("AUTHENTIK_CLIENT_SECRET", ""), AllowedSubs: env("PETAL_ALLOWED_SUBS", ""), + RequireAuth: envBool("PETAL_REQUIRE_AUTH", !isLoopbackOrigin(baseURL)), } } +// isLoopbackOrigin reports whether a base URL names this machine — the shape a +// development checkout has, and the only shape where running without a login is +// a reasonable default. Anything else (a hostname, a public origin) is a +// deployment, however small. +func isLoopbackOrigin(baseURL string) bool { + u, err := url.Parse(strings.TrimSpace(baseURL)) + if err != nil { + return false + } + switch strings.ToLower(u.Hostname()) { + case "localhost", "127.0.0.1", "::1", "": + return true + } + return false +} + // ttsVoices reads the Piper instances out of an environment slice (as returned // by os.Environ) into a map keyed by base language tag. // @@ -169,6 +205,18 @@ func env(key, fallback string) string { return fallback } +// envBool reads a boolean knob. Anything unparseable keeps the default rather +// than silently reading as false — a mistyped PETAL_REQUIRE_AUTH must not be the +// thing that turns the guard off. +func envBool(key string, fallback bool) bool { + if v := strings.TrimSpace(os.Getenv(key)); v != "" { + if b, err := strconv.ParseBool(v); err == nil { + return b + } + } + return fallback +} + func envDuration(key string, fallback time.Duration) time.Duration { if v := os.Getenv(key); v != "" { if d, err := time.ParseDuration(v); err == nil { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c75d616..e9f2c69 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -86,3 +86,51 @@ func TestTTSVoicesEmptyWithoutEndpoints(t *testing.T) { t.Errorf("discovered %v, want none", voices) } } + +// The fallback to the single local user is right on a laptop and a catastrophe +// on a public host, so it is defaulted from the origin Petal knows itself by +// rather than left to be remembered. +func TestRequireAuthDefaultsFromBaseURL(t *testing.T) { + cases := []struct { + baseURL string + want bool + }{ + {"http://localhost:8080", false}, + {"http://127.0.0.1:8080", false}, + {"http://[::1]:8080", false}, + {"", false}, // no BASE_URL set at all: the local-dev default + {"https://petal.parodia.dev", true}, + {"http://petal.parodia.dev", true}, + {"https://petal.example.com/", true}, + } + for _, c := range cases { + t.Setenv("BASE_URL", c.baseURL) + t.Setenv("PETAL_REQUIRE_AUTH", "") + if got := Load().RequireAuth; got != c.want { + t.Errorf("BASE_URL=%q: RequireAuth=%v, want %v", c.baseURL, got, c.want) + } + } +} + +// The default is a default, not a rule: a trusted private network is a real +// deployment shape, and so is wanting the guard on locally. +func TestRequireAuthExplicitOverride(t *testing.T) { + t.Setenv("BASE_URL", "https://petal.parodia.dev") + t.Setenv("PETAL_REQUIRE_AUTH", "false") + if Load().RequireAuth { + t.Error("an explicit false must be honoured on a public origin") + } + + t.Setenv("BASE_URL", "http://localhost:8080") + t.Setenv("PETAL_REQUIRE_AUTH", "true") + if !Load().RequireAuth { + t.Error("an explicit true must be honoured on localhost") + } + + // A typo must not be the thing that disables the guard. + t.Setenv("BASE_URL", "https://petal.parodia.dev") + t.Setenv("PETAL_REQUIRE_AUTH", "nope") + if !Load().RequireAuth { + t.Error("an unparseable value must keep the default, not read as false") + } +} diff --git a/internal/docs/export.go b/internal/docs/export.go index f2c49e7..80838eb 100644 --- a/internal/docs/export.go +++ b/internal/docs/export.go @@ -287,7 +287,14 @@ func mdBlock(n pmNode, depth int) string { return "---" case "image": alt := n.attrStr("alt") - return fmt.Sprintf("![%s](%s)", alt, n.attrStr("src")) + src := safeURL(n.attrStr("src")) + if src == "" { + // Nowhere safe to point. Keep the alt text as plain prose — it is + // the part that carries meaning — rather than emitting an image + // whose destination was rejected. See safeURL. + return alt + } + return fmt.Sprintf("![%s](%s)", alt, src) case "table": return mdTable(n) case "bulletList", "orderedList": @@ -396,7 +403,9 @@ func applyMdMarks(n pmNode) string { if n.hasMark("underline") { t = "" + t + "" } - if href := n.markAttr("link", "href"); href != "" { + // Same rule as the HTML export: plenty of Markdown renderers pass a + // `javascript:` destination straight through into an . See safeURL. + if href := safeURL(n.markAttr("link", "href")); href != "" { t = "[" + t + "](" + href + ")" } return t @@ -518,7 +527,16 @@ func htmlBlock(n pmNode) string { return "
\n" case "image": alt := htmlEscape(n.attrStr("alt")) - return fmt.Sprintf("

\"%s\"

\n", htmlEscape(n.attrStr("src")), alt) + src := safeURL(n.attrStr("src")) + if src == "" { + // Nowhere safe to point: keep the alt text, which is the part that + // carries meaning, rather than emitting a broken image. + if alt == "" { + return "" + } + return "

" + alt + "

\n" + } + return fmt.Sprintf("

\"%s\"

\n", htmlEscape(src), alt) case "table": return htmlTable(n) case "bulletList", "orderedList": @@ -617,7 +635,9 @@ func applyHTMLMarks(n pmNode) string { if n.hasMark("highlight") { t = "" + t + "" } - if href := n.markAttr("link", "href"); href != "" { + // An unsafe href is dropped, not the link: the words stay, they just stop + // being clickable. See safeURL. + if href := safeURL(n.markAttr("link", "href")); href != "" { t = fmt.Sprintf("%s", htmlEscape(href), t) } return t @@ -858,6 +878,53 @@ func htmlEscape(s string) string { return r.Replace(s) } +// safeURLSchemes are the schemes an exported document may point at. Escaping +// makes a URL safe to sit inside an attribute; it says nothing about what +// happens when the attribute is followed, and `javascript:` survives it +// untouched. +// +// The toolbar can't produce one — it prefixes anything it doesn't recognise +// with https:// — but the toolbar is not the only way in: PUT /api/docs/{id} +// stores whatever Tiptap JSON it is given. And an export is the one artifact +// here that is *meant* to leave: the passport and the .html backup are files a +// writer hands to a teacher or an editor, opened on a machine that has no +// reason to trust them. A link that runs code when clicked is not something to +// ship inside one. +// +// Relative and fragment links pass through: they're how a document refers to +// its own headings, and they can't reach anything. +var safeURLSchemes = map[string]bool{ + "http": true, "https": true, "mailto": true, "tel": true, "ftp": true, +} + +// safeURL returns u if it is safe to follow from an exported file, and "" if it +// isn't. A dropped href leaves the link text in place — the reader loses a +// destination, never the writing. +func safeURL(u string) string { + trimmed := strings.TrimSpace(u) + if trimmed == "" { + return "" + } + // A scheme is everything before the first ':', but only when no '/', '?' or + // '#' comes first — otherwise "notes/a:b" would read as the "notes/a" scheme. + // Nothing before a colon means a relative or fragment link, which is fine. + if i := strings.IndexAny(trimmed, ":/?#"); i >= 0 && trimmed[i] == ':' { + // Control characters and whitespace are stripped by browsers *before* + // the scheme is read, so "java\nscript:" is javascript:. Fold them out + // before deciding rather than after. + scheme := strings.Map(func(r rune) rune { + if r <= ' ' || r == 0x7f { + return -1 + } + return r + }, trimmed[:i]) + if !safeURLSchemes[strings.ToLower(scheme)] { + return "" + } + } + return trimmed +} + func xmlEscape(s string) string { r := strings.NewReplacer("&", "&", "<", "<", ">", ">", `"`, """) return r.Replace(s) diff --git a/internal/docs/export_test.go b/internal/docs/export_test.go index 2aec8b7..9b26662 100644 --- a/internal/docs/export_test.go +++ b/internal/docs/export_test.go @@ -8,6 +8,8 @@ import ( "net/http" "strings" "testing" + + "gitea.parodia.dev/drwily/petal/internal/db" ) // richDocJSON is a Tiptap document exercising headings, marks, and a list — @@ -243,3 +245,73 @@ func TestExportUnsupportedFormat(t *testing.T) { t.Fatalf("expected 400 for unsupported format, got %d", rec.Code) } } + +// Escaping makes a URL safe to sit inside an attribute; it says nothing about +// what happens when the attribute is followed. An export is the one artifact +// here meant to leave — the file handed to a teacher, opened on a machine with +// no reason to trust it — so a destination that runs code is dropped. +func TestExportDropsUnsafeLinkSchemes(t *testing.T) { + unsafe := []string{ + "javascript:alert(1)", + "JaVaScRiPt:alert(1)", + "java\nscript:alert(1)", // browsers strip control characters first + " javascript:alert(1)", + "data:text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==", + "vbscript:msgbox(1)", + } + for _, href := range unsafe { + if got := safeURL(href); got != "" { + t.Errorf("safeURL(%q) = %q, want it dropped", href, got) + } + } + + safe := []string{ + "https://example.com/a?b=1#c", + "http://example.com", + "mailto:her@example.com", + "/api/images/abc.png", + "#a-heading", + "notes/chapter:one.md", // a colon that isn't a scheme + } + for _, href := range safe { + if got := safeURL(href); got != href { + t.Errorf("safeURL(%q) = %q, want it kept", href, got) + } + } +} + +// End to end through the renderers: an unsafe href loses its destination, never +// its words. +func TestRenderedExportsCarryNoScriptURLs(t *testing.T) { + doc := db.Document{ + Title: "Notes", + Content: `{"type":"doc","content":[{"type":"paragraph","content":[ + {"type":"text","text":"click me","marks":[{"type":"link","attrs":{"href":"javascript:alert(1)"}}]}]}, + {"type":"image","attrs":{"src":"javascript:alert(2)","alt":"a drawing"}}]}`, + } + + html, err := renderHTMLFile(doc) + if err != nil { + t.Fatal(err) + } + if strings.Contains(strings.ToLower(string(html)), "javascript:") { + t.Fatalf("html export carried a javascript: URL:\n%s", html) + } + if !strings.Contains(string(html), "click me") { + t.Fatal("html export dropped the link text along with the href") + } + if !strings.Contains(string(html), "a drawing") { + t.Fatal("html export dropped the alt text of the rejected image") + } + + md, err := renderMarkdown(doc) + if err != nil { + t.Fatal(err) + } + if strings.Contains(strings.ToLower(string(md)), "javascript:") { + t.Fatalf("markdown export carried a javascript: URL:\n%s", md) + } + if !strings.Contains(string(md), "click me") { + t.Fatal("markdown export dropped the link text along with the href") + } +} diff --git a/internal/httputil/json.go b/internal/httputil/json.go index 006154a..1ca4b33 100644 --- a/internal/httputil/json.go +++ b/internal/httputil/json.go @@ -35,3 +35,18 @@ func ServerError(w http.ResponseWriter, err error) { log.Printf("internal error: %v", err) ErrorJSON(w, http.StatusInternalServerError, "something went wrong") } + +// UpstreamError is ServerError's counterpart for a dependency Petal calls out +// to — the model, chiefly. Same discipline, and for a sharper reason: a dial +// failure's error text contains the endpoint it failed to dial, so relaying it +// hands anyone who can reach Petal the address of the inference box on the far +// side of the VPN, along with which backend is running there. +// +// `what` names the pass for the operator's log ("checkpoint", "chat"). The +// browser is told only that the helper is unreachable, which is all the client +// ever did anything with: every LLM route's 502 renders as the same warm +// "小助手在休息 · Petal's helper is resting". +func UpstreamError(w http.ResponseWriter, what string, err error) { + log.Printf("upstream error (%s): %v", what, err) + ErrorJSON(w, http.StatusBadGateway, "Petal's helper is out of reach right now") +} diff --git a/internal/images/handler.go b/internal/images/handler.go index b9d184a..2e4af1e 100644 --- a/internal/images/handler.go +++ b/internal/images/handler.go @@ -37,6 +37,17 @@ import ( // small enough to keep a careless paste from filling the disk. const maxUploadBytes = 10 << 20 +// maxUserBytes caps what one account may keep stored, at 1 GiB. The per-upload +// limit bounds a single careless paste; nothing bounded ten thousand of them, +// and Petal's data directory is an 8 GiB encrypted volume shared with the +// database, the backups and the TTS cache — the disk filling is the database +// losing writes, not just images failing. +// +// A tenth of the volume per writer is far past any real use: a heavily +// illustrated journal is tens of megabytes. It is a runaway backstop, and it is +// deliberately generous enough that nobody writing normally will ever meet it. +const maxUserBytes = 1 << 30 + // extByContentType maps the image types we accept to a canonical extension. The // allowlist doubles as validation: anything not here is rejected. var extByContentType = map[string]string{ @@ -177,6 +188,19 @@ func (h *Handler) upload(w http.ResponseWriter, r *http.Request) { name := hex.EncodeToString(sum[:])[:32] + ext path := filepath.Join(h.dir, name) + userID := auth.UserID(r.Context()) + within, err := h.withinQuota(userID, name, int64(len(data))) + if err != nil { + log.Printf("images: quota check failed for %s: %v", userID, err) + http.Error(w, "could not store image", http.StatusInternalServerError) + return + } + if !within { + http.Error(w, "you've filled Petal's picture store — delete a few images and try again", + http.StatusInsufficientStorage) + return + } + // Skip the write if this exact content is already stored. if _, statErr := os.Stat(path); errors.Is(statErr, os.ErrNotExist) { if err := os.WriteFile(path, data, 0o644); err != nil { @@ -190,7 +214,7 @@ func (h *Handler) upload(w http.ResponseWriter, r *http.Request) { if _, err := h.db.Exec( `INSERT INTO images (name, user_id, content_type, size) VALUES (?, ?, ?, ?) ON CONFLICT (name, user_id) DO NOTHING`, - name, auth.UserID(r.Context()), ct, len(data), + name, userID, ct, len(data), ); err != nil { log.Printf("images: could not record ownership of %s: %v", name, err) http.Error(w, "could not store image", http.StatusInternalServerError) @@ -221,6 +245,20 @@ func (h *Handler) serve(w http.ResponseWriter, r *http.Request) { } // Private: a shared cache must never hand one writer's image to another. w.Header().Set("Cache-Control", "private, max-age=31536000, immutable") + + // SVG is a document format wearing an image's name: it can carry `) + rec := httptest.NewRecorder() + alice.ServeHTTP(rec, uploadReq(t, "image", svg)) + if rec.Code != http.StatusOK { + t.Fatalf("svg upload code=%d body=%s", rec.Code, rec.Body) + } + var resp struct{ URL string } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatal(err) + } + name := strings.TrimPrefix(resp.URL, "/api/images/") + + got := get(t, alice, name) + if got.Code != http.StatusOK { + t.Fatalf("serve code=%d", got.Code) + } + csp := got.Header().Get("Content-Security-Policy") + if !strings.Contains(csp, "default-src 'none'") || !strings.Contains(csp, "sandbox") { + t.Fatalf("CSP %q does not neutralize the response", csp) + } + if got.Header().Get("X-Content-Type-Options") != "nosniff" { + t.Fatal("stored images must be served nosniff") + } +} + +// A per-upload cap bounds one careless paste; nothing bounded ten thousand of +// them, on the same volume the database lives on. +func TestUploadQuota(t *testing.T) { + database, err := db.Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatalf("open db: %v", err) + } + t.Cleanup(func() { database.Close() }) + if _, err := database.Exec( + `INSERT INTO users (id, email, display_name) VALUES (?, ?, ?)`, + "bob", "bob@petal.local", "Bob", + ); err != nil { + t.Fatalf("seed second user: %v", err) + } + h, err := New(t.TempDir(), database.DB, db.LocalUserID) + if err != nil { + t.Fatalf("new store: %v", err) + } + alice := auth.Middleware(auth.StaticResolver(db.LocalUserID))(h.Routes()) + bob := auth.Middleware(auth.StaticResolver("bob"))(h.Routes()) + + // Fill Alice's allowance by hand — uploading a gibibyte in a test would be + // absurd, and what's under test is the accounting, not the arithmetic. + name := upload(t, alice, pngBytes) + if _, err := database.Exec( + `UPDATE images SET size = ? WHERE user_id = ? AND name = ?`, + int64(maxUserBytes), db.LocalUserID, name, + ); err != nil { + t.Fatal(err) + } + + // Re-storing something she already has costs nothing, so it still works. + if again := upload(t, alice, pngBytes); again != name { + t.Fatalf("a re-upload of an owned image should dedupe, got %q", again) + } + + // Anything new does not. + rec := httptest.NewRecorder() + alice.ServeHTTP(rec, uploadReq(t, "image", otherPNG)) + if rec.Code != http.StatusInsufficientStorage { + t.Fatalf("over-quota upload code=%d, want 507", rec.Code) + } + + // And it is *her* allowance, not the store's: Bob is unaffected. + if got := upload(t, bob, otherPNG); got == "" { + t.Fatal("one writer's quota must not stop another writing") + } +} diff --git a/internal/lexicon/handlers.go b/internal/lexicon/handlers.go index 1ba5cd8..1943410 100644 --- a/internal/lexicon/handlers.go +++ b/internal/lexicon/handlers.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "encoding/json" + "log" "net/http" "net/url" @@ -95,10 +96,15 @@ func (h *Handler) gloss(w http.ResponseWriter, r *http.Request) { writeLookup(w, res) } +// writeLookupErr answers a failed lookup. The real error is a dictionary or +// database fault — a file path, a SQLite message — and belongs in the log, not +// in a tooltip. The client treats any non-200 the same way, so nothing is lost +// by saying less. func writeLookupErr(w http.ResponseWriter, err error) { + log.Printf("lexicon: lookup failed: %v", err) w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusInternalServerError) - _ = json.NewEncoder(w).Encode(map[string]string{"error": err.Error()}) + _ = json.NewEncoder(w).Encode(map[string]string{"error": "lookup failed"}) } func writeLookup(w http.ResponseWriter, v any) { diff --git a/internal/suggestions/chat.go b/internal/suggestions/chat.go index 5218d74..8e29a34 100644 --- a/internal/suggestions/chat.go +++ b/internal/suggestions/chat.go @@ -75,7 +75,7 @@ func (h *Handler) chat(w http.ResponseWriter, r *http.Request) { if err != nil { // The stream never opened (e.g. LLM unreachable) — a normal JSON error is // still appropriate since we haven't written SSE headers yet. - httputil.ErrorJSON(w, http.StatusBadGateway, "chat failed: "+err.Error()) + httputil.UpstreamError(w, "chat", err) return } diff --git a/internal/suggestions/handlers.go b/internal/suggestions/handlers.go index c3edd62..bf1dfaa 100644 --- a/internal/suggestions/handlers.go +++ b/internal/suggestions/handlers.go @@ -283,7 +283,7 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R // the per-document slot for the full interval — stranding the frontend's // auto-retry on the throttle path. Release it so a retry can re-run. limiter.Release(docID, slotAt) - httputil.ErrorJSON(w, http.StatusBadGateway, "llm pass failed: "+err.Error()) + httputil.UpstreamError(w, "pass", err) return } diff --git a/internal/suggestions/rewrite.go b/internal/suggestions/rewrite.go index 6104214..ba9ee77 100644 --- a/internal/suggestions/rewrite.go +++ b/internal/suggestions/rewrite.go @@ -69,7 +69,7 @@ func (h *Handler) rewrite(w http.ResponseWriter, r *http.Request) { out, err := llm.RunRewrite(r.Context(), h.Client, text, body.Style) if err != nil { - httputil.ErrorJSON(w, http.StatusBadGateway, "rewrite failed: "+err.Error()) + httputil.UpstreamError(w, "rewrite", err) return } diff --git a/internal/suggestions/translate.go b/internal/suggestions/translate.go index 21d56c5..6b35779 100644 --- a/internal/suggestions/translate.go +++ b/internal/suggestions/translate.go @@ -51,7 +51,7 @@ func (h *Handler) translate(w http.ResponseWriter, r *http.Request) { out, err := llm.RunTranslate(r.Context(), h.Client, explanation, llm.LangFor(pairLang)) if err != nil { - httputil.ErrorJSON(w, http.StatusBadGateway, "translate failed: "+err.Error()) + httputil.UpstreamError(w, "translate", err) return } diff --git a/internal/tts/handler.go b/internal/tts/handler.go index 26f7060..2d5099e 100644 --- a/internal/tts/handler.go +++ b/internal/tts/handler.go @@ -21,6 +21,7 @@ import ( "path/filepath" "sort" "strings" + "sync/atomic" "time" "unicode/utf8" @@ -97,6 +98,7 @@ type Handler struct { cacheDir string format audioFormat client *http.Client + writes atomic.Uint64 // cache writes since boot; drives the prune throttle } // New builds a Handler from config. It returns (nil, false) when TTS_ENDPOINT is @@ -233,6 +235,7 @@ func (h *Handler) synth(w http.ResponseWriter, r *http.Request) { if err := os.WriteFile(tmp, audio, 0o644); err == nil { _ = os.Rename(tmp, path) } + h.pruneCache() } w.Header().Set("Content-Type", h.format.contentType) @@ -240,6 +243,80 @@ func (h *Handler) synth(w http.ResponseWriter, r *http.Request) { _, _ = w.Write(audio) } +// maxCacheBytes bounds the whole clip cache at 512 MiB. +// +// Each clip is small, so nothing about ordinary reading approaches this — a +// year of tapping words is tens of megabytes. What it bounds is the shape of +// the endpoint: the cache key is the *text*, so a client asking for four +// thousand distinct characters at a time writes a new file every request, for +// as long as it cares to. That is an authenticated writer filling the same +// encrypted volume the database lives on, and a full disk is SQLite failing to +// write, not merely read-aloud getting slower. +const maxCacheBytes = 512 << 20 + +// pruneEvery throttles the sweep: checking the directory on every synthesis +// would stat the whole cache for each new word. Synthesis is already the slow +// path and misses are rare once a writer settles, so one sweep per this many +// cache writes keeps the cost invisible while still converging long before the +// limit means anything. +const pruneEvery = 64 + +// pruneCache trims the cache back under maxCacheBytes, oldest-first, and is a +// no-op the great majority of the time it is called. +// +// Oldest by modification time is a fair approximation of least-recently-useful +// here: a clip is written once and only ever read afterwards, so its age is how +// long ago someone wanted it. Evicting one costs a re-synthesis, never data — +// which is why this can be as approximate as it likes, and why every error +// along the way is simply given up on. +func (h *Handler) pruneCache() { + if n := h.writes.Add(1); n%pruneEvery != 0 { + return + } + + entries, err := os.ReadDir(h.cacheDir) + if err != nil { + return + } + type clip struct { + path string + size int64 + mod time.Time + } + var clips []clip + var total int64 + for _, e := range entries { + if e.IsDir() { + continue + } + info, err := e.Info() + if err != nil { + continue + } + clips = append(clips, clip{filepath.Join(h.cacheDir, e.Name()), info.Size(), info.ModTime()}) + total += info.Size() + } + if total <= maxCacheBytes { + return + } + + sort.Slice(clips, func(i, j int) bool { return clips[i].mod.Before(clips[j].mod) }) + // Drop to 80% rather than exactly to the line, so the next few hundred + // clips don't each trigger another sweep. + target := int64(maxCacheBytes / 100 * 80) + removed := 0 + for _, c := range clips { + if total <= target { + break + } + if os.Remove(c.path) == nil { + total -= c.size + removed++ + } + } + fmt.Fprintf(os.Stderr, "tts: cache over %d bytes — evicted %d oldest clip(s)\n", int64(maxCacheBytes), removed) +} + // serve streams a cached clip with a long-lived immutable cache header (the URL // is content-addressed, so the bytes never change for a given request). func (h *Handler) serve(w http.ResponseWriter, r *http.Request, path string) { diff --git a/web/package-lock.json b/web/package-lock.json index 65cd522..53be6fa 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -2928,9 +2928,9 @@ } }, "node_modules/linkify-it": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/linkify-it/-/linkify-it-5.0.1.tgz", - "integrity": "sha512-wVoTjP4Q6R0NW5hiZkVJaFZPWgtXfoGF+6LucL3/FtiNjmcHhYjEr5f1Kqjirc1nBW07J/ZuRFumqr2oqccEWg==", + "version": "5.0.2", + "resolved": "https://registry.npmjs.org/linkify-it/-/linkify-it-5.0.2.tgz", + "integrity": "sha512-ONTm2jCMAVZjgQa/Fy1kScXsuOoF5NPTsoFBdE1KVIZ2vAh/r9+Bqo+0jINCBYnavTPQZz38QzFTme79ENoN3Q==", "funding": [ { "type": "github", @@ -3019,9 +3019,9 @@ "license": "MIT" }, "node_modules/nanoid": { - "version": "3.3.15", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.15.tgz", - "integrity": "sha512-y7Wygv/7mEOvxTuEQDB8StXdMRBWf1kR/tlhAzBRUFkB2jfcLOAxO/SHmOO2zgz1pVgK29/kyupn059/bCHdjA==", + "version": "3.3.16", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz", + "integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==", "dev": true, "funding": [ { @@ -3104,9 +3104,9 @@ } }, "node_modules/postcss": { - "version": "8.5.15", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.15.tgz", - "integrity": "sha512-FfR8sjd4em2T6fb3I2MwAJU7HWVMr9zba+enmQeeWFfCbm+UOC/0X4DS8XtpUTMwWMGbjKYP7xjfNekzyGmB3A==", + "version": "8.5.23", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.23.tgz", + "integrity": "sha512-g50586zr4bZmwFiTlflMu8E0bDTb5I5gertgwAKmsdUlTQIhZtunzUlD1WSzwcVWPoAVpsrA6vlfCD7oXvRwgg==", "dev": true, "funding": [ { @@ -3124,7 +3124,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.16", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, diff --git a/web/src/components/DocList/DocList.tsx b/web/src/components/DocList/DocList.tsx index 82ffb3b..9e54fb2 100644 --- a/web/src/components/DocList/DocList.tsx +++ b/web/src/components/DocList/DocList.tsx @@ -5,6 +5,7 @@ import { SearchBox } from './SearchBox' import { TagChip } from './TagChip' import { LanguagePicker } from './LanguagePicker' import { usePack, type Pack } from '../../i18n' +import { forgetAllDrafts } from '../../lib/drafts' interface Props { docs: DocSummary[] @@ -172,13 +173,29 @@ export function DocList({ 🌸 {account.name} - forgetAllDrafts()} > - {t.docs.signOut} - + + )} diff --git a/web/src/lib/drafts.test.ts b/web/src/lib/drafts.test.ts index edc9a60..6832d5d 100644 --- a/web/src/lib/drafts.test.ts +++ b/web/src/lib/drafts.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' -import { clearDraft, peekDraft, stashDraft, takeDraft } from './drafts' +import { clearDraft, forgetAllDrafts, peekDraft, stashDraft, takeDraft } from './drafts' +import { resetPrefsScopeForTests, setPrefsScope } from './prefs' // The draft stash is the last thing between an expired session and lost // writing, so these tests care about two things above all: that a rescued body @@ -22,11 +23,13 @@ function fakeStorage(): Storage { beforeEach(() => { vi.stubGlobal('localStorage', fakeStorage()) + resetPrefsScopeForTests() }) afterEach(() => { vi.unstubAllGlobals() vi.useRealTimers() + resetPrefsScopeForTests() }) describe('draft rescue', () => { @@ -76,6 +79,51 @@ describe('draft rescue', () => { expect(peekDraft('doc-1')).toBeNull() }) + // A rescue is unsaved writing, so it belongs to the writer, not to the + // browser profile two people may be sharing. + describe('per account', () => { + it('keeps two writers apart on one browser', () => { + setPrefsScope('claire') + stashDraft('doc-1', { content_text: 'hers' }) + + resetPrefsScopeForTests() + setPrefsScope('wei') + expect(peekDraft('doc-1')).toBeNull() + + stashDraft('doc-1', { content_text: 'his' }) + expect(peekDraft('doc-1')?.body.content_text).toBe('his') + + resetPrefsScopeForTests() + setPrefsScope('claire') + expect(peekDraft('doc-1')?.body.content_text).toBe('hers') + }) + + // A draft stashed before this browser knew who was writing still has to + // reach her — an in-flight rescue must survive the upgrade that introduced + // namespacing. + it('adopts a pre-account draft for the first writer to sign in', () => { + stashDraft('doc-1', { content_text: 'stashed before login' }) + setPrefsScope('claire') + expect(peekDraft('doc-1')?.body.content_text).toBe('stashed before login') + expect(localStorage.getItem('petal.draft.doc-1')).toBeNull() + }) + + it('forgets only the signed-in writer’s drafts on sign-out', () => { + setPrefsScope('claire') + stashDraft('doc-1', { content_text: 'hers' }) + resetPrefsScopeForTests() + setPrefsScope('wei') + stashDraft('doc-2', { content_text: 'his' }) + + forgetAllDrafts() + expect(peekDraft('doc-2')).toBeNull() + + resetPrefsScopeForTests() + setPrefsScope('claire') + expect(peekDraft('doc-1')?.body.content_text).toBe('hers') + }) + }) + // Storage can be full, disabled, or absent. Losing the safety net is bad; // throwing from inside a failed save is worse. it('survives storage that refuses to write', () => { diff --git a/web/src/lib/drafts.ts b/web/src/lib/drafts.ts index 1771806..4eda9ce 100644 --- a/web/src/lib/drafts.ts +++ b/web/src/lib/drafts.ts @@ -13,6 +13,7 @@ // server, and the entry is cleared the moment a normal save succeeds. import type { DocUpdate } from '../api/client' +import { forgetScopedKeys, scopedKey } from './prefs' const PREFIX = 'petal.draft.' @@ -27,8 +28,21 @@ export interface StashedDraft { // week later is more likely to be a surprise than a save. const MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000 +// Namespaced by account, like every other thing this browser holds on a +// writer's behalf (see prefs). A draft is the heaviest of them: not a +// preference but unsaved writing, sitting in a profile two people may share. +// Keying it on the document id alone meant one person's rescue could surface in +// the other's editor the moment they opened the same document id — and, more +// plainly, that her sentences stayed in the browser under a name anyone looking +// could read. function key(docId: string): string { - return PREFIX + docId + return scopedKey(PREFIX + docId) +} + +// forgetAllDrafts drops every draft this browser is holding for the signed-in +// writer. Called on the way out — see the sign-out control in DocList. +export function forgetAllDrafts(): void { + forgetScopedKeys(PREFIX) } // stashDraft records the unsaved body for a document, replacing any earlier one diff --git a/web/src/lib/prefs.ts b/web/src/lib/prefs.ts index 4eb1035..6df1b58 100644 --- a/web/src/lib/prefs.ts +++ b/web/src/lib/prefs.ts @@ -27,6 +27,20 @@ const listeners = new Set() // moment the scope becomes known. const SCOPED_KEYS = ['petal.sound', 'petal.petals', 'petal.companion'] as const +// Families of keys whose names aren't known ahead of time — the draft rescue is +// one per document id — but which follow the account for the same reason. Held +// here so adoption can sweep them, and so there is one list of "what belongs to +// a writer in this browser" rather than two. +export const SCOPED_PREFIXES = ['petal.draft.'] as const + +// isLegacyKey spots a pre-namespacing key under one of those prefixes. A scoped +// key always carries the `.u.` suffix that scopedKey adds, and a document id +// never contains it, so its absence is what marks the key as belonging to the +// era before accounts. +function isLegacyKey(key: string): boolean { + return SCOPED_PREFIXES.some((p) => key.startsWith(p)) && !key.includes('.u.') +} + // scopedKey is the storage key actually used for `base` right now. Before the // caller is known it is the legacy key, so a reload keeps working offline and // pre-login reads see the browser's existing preference. @@ -80,11 +94,53 @@ function adoptLegacy(): void { } localStorage.removeItem(base) } + + // The same move for the prefixed families. Snapshot the key list first: + // removing while iterating localStorage by index skips entries. + const legacyKeys: string[] = [] + for (let i = 0; i < localStorage.length; i++) { + const k = localStorage.key(i) + if (k && isLegacyKey(k)) legacyKeys.push(k) + } + for (const k of legacyKeys) { + const value = localStorage.getItem(k) + if (value === null) continue + if (localStorage.getItem(scopedKey(k)) === null) { + localStorage.setItem(scopedKey(k), value) + } + localStorage.removeItem(k) + } } catch { /* storage unavailable — nothing to adopt, and nothing breaks */ } } +// forgetScopedKeys removes everything this browser is holding for the current +// account under the given prefix — and anything still sitting un-namespaced, +// which on a browser that has only ever had one writer is the same content. +// +// Sign-out is the moment this matters. Everything else here is a preference; +// the draft stash is unsaved *writing*, and leaving it in localStorage after +// someone has deliberately signed out of a shared machine is the one case where +// the rescue net becomes the leak. +export function forgetScopedKeys(prefix: string): void { + try { + const doomed: string[] = [] + for (let i = 0; i < localStorage.length; i++) { + const k = localStorage.key(i) + if (!k || !k.startsWith(prefix)) continue + // Un-namespaced keys predate accounts, so on this browser they are ours; + // namespaced ones are ours only if they carry our id. Another writer's + // rescued draft on a shared laptop is not ours to throw away. + const ours = !k.includes('.u.') || (userID !== null && k.endsWith(`.u.${userID}`)) + if (ours) doomed.push(k) + } + doomed.forEach((k) => localStorage.removeItem(k)) + } catch { + /* storage unavailable — there is nothing held to forget */ + } +} + // resetPrefsScopeForTests unbinds the account again. Exported for tests only; // the app sets the scope once and never clears it (signing out leaves the // editor mounted, and the same person usually signs back in).