Phase 16: Petal authenticates for itself

Petal is now an OIDC client in its own right rather than trusting a header
from the proxy. The Phase-0 Resolver seam was the only integration point:
main.go picks the session store when Authentik is configured and the static
local user otherwise, and no handler or query moved for either.

internal/auth gains three pieces. session.go issues an opaque cookie token
and stores only its SHA-256, so a database copy yields nothing usable; the
30-day expiry slides on every request, throttled to one write an hour, and
logout deletes the row rather than just the cookie. oidc.go runs the
authorization-code flow with state, nonce and PKCE, and discovers the
provider lazily and on retry — an Authentik outage should block new logins
without stopping Petal booting or invalidating live sessions. users.go
provisions accounts from the token's claims and gates them on an allowlist
that matches emails as well as subject ids, since a subject is an opaque
uuid that doesn't exist until someone has already logged in once.

Migration 0010 lands sessions, images and users.pair_lang together. The
images table closes the capability-URL hole the Phase-0 audit flagged: a
hash was previously enough to fetch anyone's picture. Rows are keyed
(name, user_id) so one file can have several owners and deduplication
survives; a stranger gets 404 rather than 403, the cache header drops to
private, and files already on disk are claimed at startup or every image
already pasted into a document would 404.

On the frontend a single 401 interceptor feeds a warm bilingual sign-in
overlay, drawn over a still-visible editor because nothing has been taken
away. Behind it is the part that matters: a save that comes back 401
stashes its body to localStorage before anything else and stops the
auto-save loop, and reopening that document after signing in merges the
draft back and saves it. An expired session must not cost writing.

Writing the round-trip test against a stub identity provider turned up a
real bug: the one-shot state/nonce/PKCE cookies were cleared in a defer,
which runs after the redirect has written the response header, so the
clearing Set-Cookie was silently dropped and they lingered for their full
ten minutes.

Also swaps the emoji favicon for a drawn sakura, which renders as Petal's
own rose palette everywhere instead of whatever each platform's font
decides, and doubles as the app tile in Authentik.

Migration 0010 verified against a VACUUM INTO copy of the live millenia
database: counts intact, FTS still matching, the one existing image
claimed.

Claude-Session: https://claude.ai/code/session_016y6gyuHkQXPiEuW8RGQyua

internal/images/handler.go

@@ -2,20 +2,35 @@
 // images from the editor, they're saved to disk under the configured directory,
 // and served back by hashed filename. Content addressing means the same image
 // pasted twice is stored once, and URLs are stable and cacheable forever.
+//
+// Each stored file also has one row per owner in the `images` table, and a fetch
+// joins on the caller. Before that, the store was a flat directory with no
+// database presence at all: any authenticated user holding a sha256 could fetch
+// anyone else's image. Hashes aren't guessable, so it was never an emergency —
+// but "unguessable filename" is not access control, and images pasted into a
+// private journal are exactly the content that shouldn't depend on it.
+//
+// One row per owner (rather than one owner per file) is what keeps deduplication:
+// the same picture uploaded by two people is stored once and simply has two rows.
+// The file is removed only with its last row.
 package images
 
 import (
 	"crypto/sha256"
+	"database/sql"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"io"
+	"log"
 	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/go-chi/chi/v5"
+
+	"gitea.parodia.dev/drwily/petal/internal/auth"
 )
 
 // maxUploadBytes caps a single image at 10 MiB — generous for a writing tool,
@@ -32,25 +47,81 @@ var extByContentType = map[string]string{
 	"image/svg+xml": ".svg",
 }
 
-// Handler serves the upload + fetch endpoints, backed by a directory on disk.
+// Handler serves the upload + fetch endpoints, backed by a directory on disk and
+// an ownership table.
 type Handler struct {
 	dir string
+	db  *sql.DB
 }
 
-// New constructs a Handler, ensuring the storage directory exists.
-func New(dir string) (*Handler, error) {
+// New constructs a Handler, ensuring the storage directory exists and that every
+// file already in it has an owner.
+func New(dir string, database *sql.DB, backfillOwner string) (*Handler, error) {
 	if err := os.MkdirAll(dir, 0o755); err != nil {
 		return nil, err
 	}
-	return &Handler{dir: dir}, nil
+	h := &Handler{dir: dir, db: database}
+	if err := h.backfill(backfillOwner); err != nil {
+		return nil, err
+	}
+	return h, nil
+}
+
+// backfill claims pre-existing files for one user. Images uploaded before
+// ownership existed have no row, and a row is now what makes them fetchable —
+// so without this every picture already pasted into a document would 404.
+// Attributing them to the account that has been the only one until now is the
+// only answer the data supports. Idempotent: files that already have an owner
+// are left alone.
+func (h *Handler) backfill(owner string) error {
+	if owner == "" {
+		return nil
+	}
+	entries, err := os.ReadDir(h.dir)
+	if err != nil {
+		return err
+	}
+	claimed := 0
+	for _, e := range entries {
+		if e.IsDir() {
+			continue
+		}
+		var exists bool
+		if err := h.db.QueryRow(
+			`SELECT EXISTS(SELECT 1 FROM images WHERE name = ?)`, e.Name(),
+		).Scan(&exists); err != nil {
+			return err
+		}
+		if exists {
+			continue
+		}
+		var size int64
+		if info, err := e.Info(); err == nil {
+			size = info.Size()
+		}
+		if _, err := h.db.Exec(
+			`INSERT INTO images (name, user_id, content_type, size) VALUES (?, ?, '', ?)
+			 ON CONFLICT DO NOTHING`,
+			e.Name(), owner, size,
+		); err != nil {
+			return err
+		}
+		claimed++
+	}
+	if claimed > 0 {
+		log.Printf("images: claimed %d pre-existing image(s) for %s", claimed, owner)
+	}
+	return nil
 }
 
 // Routes mounts the image endpoints. Mount under "/images" so the full paths are
-// POST /api/images (upload) and GET /api/images/{name} (fetch).
+// POST /api/images (upload), GET /api/images/{name} (fetch) and
+// DELETE /api/images/{name} (drop your copy).
 func (h *Handler) Routes() chi.Router {
 	r := chi.NewRouter()
 	r.Post("/", h.upload)
 	r.Get("/{name}", h.serve)
+	r.Delete("/{name}", h.remove)
 	return r
 }
 
@@ -79,7 +150,7 @@ func (h *Handler) upload(w http.ResponseWriter, r *http.Request) {
 	ext, ok := extByContentType[ct]
 	if !ok {
 		if looksLikeSVG(data) {
-			ext, ok = ".svg", true
+			ct, ext, ok = "image/svg+xml", ".svg", true
 		}
 	}
 	if !ok {
@@ -99,28 +170,103 @@ func (h *Handler) upload(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// Record the caller as an owner. Re-uploading your own image is a no-op;
+	// uploading someone else's identical image adds a second row over one file.
+	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),
+	); err != nil {
+		log.Printf("images: could not record ownership of %s: %v", name, err)
+		http.Error(w, "could not store image", http.StatusInternalServerError)
+		return
+	}
+
 	w.Header().Set("Content-Type", "application/json")
 	_ = json.NewEncoder(w).Encode(map[string]string{"url": "/api/images/" + name})
 }
 
-// serve returns a stored image by its hashed filename. The filename is validated
-// to be a bare name (no path separators) so it can't escape the storage dir, and
-// served with a long-lived cache header since content-addressed URLs never change.
+// serve returns a stored image by its hashed filename, but only to someone who
+// owns it. The filename is validated to be a bare name (no path separators) so
+// it can't escape the storage dir, and served with a long-lived cache header
+// since content-addressed URLs never change.
+//
+// Someone else's image is a 404, not a 403: whether a hash exists is itself
+// information the caller has no business learning.
 func (h *Handler) serve(w http.ResponseWriter, r *http.Request) {
-	name := chi.URLParam(r, "name")
-	if name == "" || name != filepath.Base(name) || strings.ContainsAny(name, `/\`) {
+	name, ok := safeName(chi.URLParam(r, "name"))
+	if !ok || !h.owns(name, auth.UserID(r.Context())) {
 		http.NotFound(w, r)
 		return
 	}
-	path := filepath.Join(h.dir, filepath.Base(name))
+	path := filepath.Join(h.dir, name)
 	if _, err := os.Stat(path); err != nil {
 		http.NotFound(w, r)
 		return
 	}
-	w.Header().Set("Cache-Control", "public, max-age=31536000, immutable")
+	// Private: a shared cache must never hand one writer's image to another.
+	w.Header().Set("Cache-Control", "private, max-age=31536000, immutable")
 	http.ServeFile(w, r, path)
 }
 
+// remove drops the caller's claim on an image, and deletes the file itself once
+// nobody is left holding it.
+func (h *Handler) remove(w http.ResponseWriter, r *http.Request) {
+	name, ok := safeName(chi.URLParam(r, "name"))
+	if !ok {
+		http.NotFound(w, r)
+		return
+	}
+	res, err := h.db.Exec(`DELETE FROM images WHERE name = ? AND user_id = ?`,
+		name, auth.UserID(r.Context()))
+	if err != nil {
+		http.Error(w, "could not delete image", http.StatusInternalServerError)
+		return
+	}
+	if n, _ := res.RowsAffected(); n == 0 {
+		http.NotFound(w, r)
+		return
+	}
+
+	var others bool
+	if err := h.db.QueryRow(
+		`SELECT EXISTS(SELECT 1 FROM images WHERE name = ?)`, name,
+	).Scan(&others); err != nil {
+		// The row is gone either way; leaving an orphaned file behind is a
+		// wasted block, not a correctness problem.
+		log.Printf("images: could not check remaining owners of %s: %v", name, err)
+		w.WriteHeader(http.StatusNoContent)
+		return
+	}
+	if !others {
+		if err := os.Remove(filepath.Join(h.dir, name)); err != nil && !errors.Is(err, os.ErrNotExist) {
+			log.Printf("images: could not remove %s: %v", name, err)
+		}
+	}
+	w.WriteHeader(http.StatusNoContent)
+}
+
+// owns reports whether userID has a claim on a stored image.
+func (h *Handler) owns(name, userID string) bool {
+	var ok bool
+	if err := h.db.QueryRow(
+		`SELECT EXISTS(SELECT 1 FROM images WHERE name = ? AND user_id = ?)`, name, userID,
+	).Scan(&ok); err != nil {
+		log.Printf("images: ownership check failed for %s: %v", name, err)
+		return false
+	}
+	return ok
+}
+
+// safeName rejects anything that isn't a bare filename, so a request can't walk
+// out of the storage directory.
+func safeName(name string) (string, bool) {
+	if name == "" || name != filepath.Base(name) || strings.ContainsAny(name, `/\`) {
+		return "", false
+	}
+	return name, true
+}
+
 // looksLikeSVG does a cheap check for an <svg root tag near the start of the
 // file, since DetectContentType doesn't recognize SVG.
 func looksLikeSVG(data []byte) bool {

internal/images/handler_test.go

@@ -6,8 +6,13 @@ import (
 	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
+
+	"gitea.parodia.dev/drwily/petal/internal/auth"
+	"gitea.parodia.dev/drwily/petal/internal/db"
 )
 
 // a 1x1 transparent PNG.
@@ -19,6 +24,39 @@ var pngBytes = []byte{
 	0x42, 0x60, 0x82,
 }
 
+// another 1x1 PNG, differing in one pixel byte, so it hashes elsewhere.
+var otherPNG = append(append([]byte{}, pngBytes[:len(pngBytes)-8]...),
+	0x01, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44)
+
+// newStore returns a handler over a fresh directory and database, plus a router
+// per user: identical but for who the auth middleware says is calling. Two users
+// over one store is the situation that ownership exists to handle.
+func newStore(t *testing.T) (dir string, alice, bob http.Handler) {
+	t.Helper()
+	dir = t.TempDir()
+	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(dir, database.DB, db.LocalUserID)
+	if err != nil {
+		t.Fatalf("new store: %v", err)
+	}
+	mount := func(userID string) http.Handler {
+		return auth.Middleware(auth.StaticResolver(userID))(h.Routes())
+	}
+	return dir, mount(db.LocalUserID), mount("bob")
+}
+
 func uploadReq(t *testing.T, field string, data []byte) *http.Request {
 	t.Helper()
 	var buf bytes.Buffer
@@ -34,16 +72,11 @@ func uploadReq(t *testing.T, field string, data []byte) *http.Request {
 	return req
 }
 
-func TestUploadAndServe(t *testing.T) {
-	h, err := New(t.TempDir())
-	if err != nil {
-		t.Fatal(err)
-	}
-	r := h.Routes()
-
-	// Upload a PNG → expect a JSON url under /api/images/.
+// upload posts an image and returns its stored name.
+func upload(t *testing.T, h http.Handler, data []byte) string {
+	t.Helper()
 	rec := httptest.NewRecorder()
-	r.ServeHTTP(rec, uploadReq(t, "image", pngBytes))
+	h.ServeHTTP(rec, uploadReq(t, "image", data))
 	if rec.Code != http.StatusOK {
 		t.Fatalf("upload code=%d body=%s", rec.Code, rec.Body)
 	}
@@ -54,44 +87,149 @@ func TestUploadAndServe(t *testing.T) {
 	if !strings.HasPrefix(resp.URL, "/api/images/") || !strings.HasSuffix(resp.URL, ".png") {
 		t.Fatalf("unexpected url %q", resp.URL)
 	}
+	return strings.TrimPrefix(resp.URL, "/api/images/")
+}
+
+func get(t *testing.T, h http.Handler, name string) *httptest.ResponseRecorder {
+	t.Helper()
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/"+name, nil))
+	return rec
+}
+
+func TestUploadAndServe(t *testing.T) {
+	_, alice, _ := newStore(t)
+
+	name := upload(t, alice, pngBytes)
 
 	// The same content uploaded again dedupes to the same URL.
-	rec2 := httptest.NewRecorder()
-	r.ServeHTTP(rec2, uploadReq(t, "image", pngBytes))
-	var resp2 struct{ URL string }
-	json.Unmarshal(rec2.Body.Bytes(), &resp2)
-	if resp2.URL != resp.URL {
-		t.Fatalf("expected dedup to same url, got %q vs %q", resp2.URL, resp.URL)
+	if again := upload(t, alice, pngBytes); again != name {
+		t.Fatalf("expected dedup to same name, got %q vs %q", again, name)
 	}
 
-	// Fetch it back.
-	name := strings.TrimPrefix(resp.URL, "/api/images/")
-	rec3 := httptest.NewRecorder()
-	r.ServeHTTP(rec3, httptest.NewRequest(http.MethodGet, "/"+name, nil))
-	if rec3.Code != http.StatusOK {
-		t.Fatalf("serve code=%d", rec3.Code)
+	rec := get(t, alice, name)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("serve code=%d", rec.Code)
 	}
-	if !bytes.Equal(rec3.Body.Bytes(), pngBytes) {
+	if !bytes.Equal(rec.Body.Bytes(), pngBytes) {
 		t.Fatal("served bytes differ from uploaded")
 	}
 }
 
-func TestUploadRejectsNonImage(t *testing.T) {
-	h, _ := New(t.TempDir())
-	r := h.Routes()
+// The point of the ownership table: a hash is not a capability.
+func TestImageIsolation(t *testing.T) {
+	_, alice, bob := newStore(t)
+	name := upload(t, alice, pngBytes)
+
+	if rec := get(t, bob, name); rec.Code != http.StatusNotFound {
+		t.Fatalf("bob fetched alice's image: code=%d", rec.Code)
+	}
+
+	// Nor can he delete it out from under her.
 	rec := httptest.NewRecorder()
-	r.ServeHTTP(rec, uploadReq(t, "image", []byte("this is plainly not an image at all")))
+	bob.ServeHTTP(rec, httptest.NewRequest(http.MethodDelete, "/"+name, nil))
+	if rec.Code != http.StatusNotFound {
+		t.Fatalf("bob deleted alice's image: code=%d", rec.Code)
+	}
+	if got := get(t, alice, name); got.Code != http.StatusOK {
+		t.Fatalf("alice's image disappeared: code=%d", got.Code)
+	}
+}
+
+// Deduplication has to survive ownership: one file, one row each.
+func TestDedupAcrossUsers(t *testing.T) {
+	dir, alice, bob := newStore(t)
+
+	name := upload(t, alice, pngBytes)
+	if bobName := upload(t, bob, pngBytes); bobName != name {
+		t.Fatalf("expected the same stored name, got %q vs %q", bobName, name)
+	}
+
+	entries, _ := os.ReadDir(dir)
+	if len(entries) != 1 {
+		t.Fatalf("expected 1 file on disk, found %d", len(entries))
+	}
+	for _, h := range []http.Handler{alice, bob} {
+		if rec := get(t, h, name); rec.Code != http.StatusOK {
+			t.Fatalf("owner could not fetch shared image: code=%d", rec.Code)
+		}
+	}
+
+	// Alice dropping her copy must not take Bob's picture away with it.
+	rec := httptest.NewRecorder()
+	alice.ServeHTTP(rec, httptest.NewRequest(http.MethodDelete, "/"+name, nil))
+	if rec.Code != http.StatusNoContent {
+		t.Fatalf("delete code=%d", rec.Code)
+	}
+	if got := get(t, alice, name); got.Code != http.StatusNotFound {
+		t.Fatalf("alice still sees a deleted image: code=%d", got.Code)
+	}
+	if got := get(t, bob, name); got.Code != http.StatusOK {
+		t.Fatalf("bob lost his image when alice deleted hers: code=%d", got.Code)
+	}
+	if _, err := os.Stat(filepath.Join(dir, name)); err != nil {
+		t.Fatalf("file removed while still owned: %v", err)
+	}
+
+	// The last owner leaving takes the file with them.
+	rec2 := httptest.NewRecorder()
+	bob.ServeHTTP(rec2, httptest.NewRequest(http.MethodDelete, "/"+name, nil))
+	if rec2.Code != http.StatusNoContent {
+		t.Fatalf("delete code=%d", rec2.Code)
+	}
+	if _, err := os.Stat(filepath.Join(dir, name)); !os.IsNotExist(err) {
+		t.Fatalf("file survived its last owner: %v", err)
+	}
+}
+
+// Images that predate ownership must not vanish from documents that use them.
+func TestBackfillClaimsExistingFiles(t *testing.T) {
+	dir := t.TempDir()
+	database, err := db.Open(filepath.Join(t.TempDir(), "test.db"))
+	if err != nil {
+		t.Fatalf("open db: %v", err)
+	}
+	defer database.Close()
+
+	orphan := "deadbeefdeadbeefdeadbeefdeadbeef.png"
+	if err := os.WriteFile(filepath.Join(dir, orphan), pngBytes, 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	h, err := New(dir, database.DB, db.LocalUserID)
+	if err != nil {
+		t.Fatalf("new store: %v", err)
+	}
+	alice := auth.Middleware(auth.StaticResolver(db.LocalUserID))(h.Routes())
+	if rec := get(t, alice, orphan); rec.Code != http.StatusOK {
+		t.Fatalf("pre-existing image not claimed: code=%d", rec.Code)
+	}
+
+	// Re-running the backfill (i.e. a restart) must not double up or reassign.
+	if _, err := New(dir, database.DB, "bob"); err != nil {
+		t.Fatalf("second backfill: %v", err)
+	}
+	var owners int
+	if err := database.QueryRow(`SELECT COUNT(*) FROM images WHERE name = ?`, orphan).Scan(&owners); err != nil {
+		t.Fatal(err)
+	}
+	if owners != 1 {
+		t.Fatalf("expected the backfill to be idempotent, got %d owners", owners)
+	}
+}
+
+func TestUploadRejectsNonImage(t *testing.T) {
+	_, alice, _ := newStore(t)
+	rec := httptest.NewRecorder()
+	alice.ServeHTTP(rec, uploadReq(t, "image", []byte("this is plainly not an image at all")))
 	if rec.Code != http.StatusUnsupportedMediaType {
 		t.Fatalf("expected 415, got %d", rec.Code)
 	}
 }
 
 func TestServeMissing(t *testing.T) {
-	h, _ := New(t.TempDir())
-	r := h.Routes()
-	rec := httptest.NewRecorder()
-	r.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/deadbeef.png", nil))
-	if rec.Code != http.StatusNotFound {
+	_, alice, _ := newStore(t)
+	if rec := get(t, alice, "deadbeef.png"); rec.Code != http.StatusNotFound {
 		t.Fatalf("expected 404, got %d", rec.Code)
 	}
 }