A security review of the whole repo. The queries were already scoped, the
OIDC flow already did state and nonce and PKCE, the session tokens were
already stored as hashes. What it found was mostly the seam between the
code and the deployment — and one place where the deployment quietly
undid the code.
The one that matters: with any AUTHENTIK_* variable missing, Petal fell
back to resolving every request to the single `local` user. That is right
on a laptop and a catastrophe on a public host, and Phase 16 removed the
Traefik basic-auth gate that used to stand behind the mistake. A typo in
the client secret would have served her journals to the open internet and
said so only in a log line nobody reads. It now refuses to start, guarded
by default for any BASE_URL that isn't loopback.
Then the one that would have been fixed and wasn't: stored images now
serve under `default-src 'none'; sandbox`, so an SVG pasted into a
document can't run as a page on Petal's own origin. Traefik's
customresponseheaders *overwrites*, so the CSP declared in the compose
labels would have silently replaced that per-route policy in production.
The whole header block moved into the binary, where a route can tighten
its own and a test can prove it; only HSTS stays at the edge, where TLS
actually terminates.
The rest, smaller:
- PETAL_ALLOWED_SUBS empty means everyone authentik authenticates, and
authentik here fronts half a dozen applications. Still legal, now
said out loud every boot, and set in both env examples.
- LLM failures relayed err.Error() to the browser, which carries the
address of the inference box on the far side of the VPN. Logged
instead; the client only ever rendered "the helper is resting".
- Exports scheme-check their links. Escaping makes a URL safe to sit
in an attribute and says nothing about following it, and an export
is the one artifact here meant to leave. Writing the test found the
markdown image src, which I'd missed reading it.
- The draft rescue is namespaced per account and cleared on sign-out.
Everything else in localStorage is a preference; this is her unsaved
writing, sitting in a profile two people share.
- /auth/logout is POST-only. With SameSite=Lax a GET route lets any
page on the internet sign her out mid-draft.
- Image uploads get a per-account allowance and the TTS cache a size
cap. Both share the encrypted volume the database is on, and a full
disk is SQLite failing to write, not a feature degrading.
- The session cookie takes the __Host- prefix over https, so nothing
else under parodia.dev can plant one. Old cookies still resolve;
nobody is signed out to get there.
- npm audit: linkify-it and postcss.
Verified: go build, go vet, the full Go suite, tsc, 195 frontend tests,
npm audit clean. The startup guard and both CSPs checked against a
running server rather than only asserted.
Claude-Session: https://claude.ai/code/session_016y6gyuHkQXPiEuW8RGQyua
325 lines
11 KiB
Go
325 lines
11 KiB
Go
package images
|
|
|
|
import (
|
|
"bytes"
|
|
"encoding/json"
|
|
"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.
|
|
var pngBytes = []byte{
|
|
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52,
|
|
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4,
|
|
0x89, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x62, 0x00, 0x01, 0x00, 0x00,
|
|
0x05, 0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae,
|
|
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
|
|
mw := multipart.NewWriter(&buf)
|
|
fw, err := mw.CreateFormFile(field, "x.png")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
fw.Write(data)
|
|
mw.Close()
|
|
req := httptest.NewRequest(http.MethodPost, "/", &buf)
|
|
req.Header.Set("Content-Type", mw.FormDataContentType())
|
|
return req
|
|
}
|
|
|
|
// 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()
|
|
h.ServeHTTP(rec, uploadReq(t, "image", data))
|
|
if rec.Code != http.StatusOK {
|
|
t.Fatalf("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)
|
|
}
|
|
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.
|
|
if again := upload(t, alice, pngBytes); again != name {
|
|
t.Fatalf("expected dedup to same name, got %q vs %q", again, name)
|
|
}
|
|
|
|
rec := get(t, alice, name)
|
|
if rec.Code != http.StatusOK {
|
|
t.Fatalf("serve code=%d", rec.Code)
|
|
}
|
|
if !bytes.Equal(rec.Body.Bytes(), pngBytes) {
|
|
t.Fatal("served bytes differ from uploaded")
|
|
}
|
|
}
|
|
|
|
// 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()
|
|
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)
|
|
}
|
|
|
|
// And an owner who no longer exists — which is what the `local` account
|
|
// becomes once it has been migrated onto a real one — must be skipped, not
|
|
// turned into a foreign-key error that takes startup down with it.
|
|
if _, err := New(dir, database.DB, "nobody-at-all"); err != nil {
|
|
t.Fatalf("backfill for a missing owner should be a no-op, got: %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) {
|
|
_, alice, _ := newStore(t)
|
|
if rec := get(t, alice, "deadbeef.png"); rec.Code != http.StatusNotFound {
|
|
t.Fatalf("expected 404, got %d", rec.Code)
|
|
}
|
|
}
|
|
|
|
// An SVG is a document, not a picture: it can carry <script>, and this route
|
|
// serves it from Petal's own origin. Rendered through an <img> that script
|
|
// never runs, but "open image in new tab" is one click away, and there it
|
|
// would — with the API of whoever opened it. Every stored image therefore
|
|
// answers with a CSP that permits nothing.
|
|
func TestStoredImagesAreServedInert(t *testing.T) {
|
|
_, alice, _ := newStore(t)
|
|
|
|
svg := []byte(`<svg xmlns="http://www.w3.org/2000/svg"><script>fetch('/api/docs')</script></svg>`)
|
|
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")
|
|
}
|
|
}
|