fix(sikkerhed): resten af gennemgangen — SSRF, tre ubegrænsede veje, og fire hemmeligheder

**H2 — SSRF gennem AI-udbyderens adresse.** `base_url` blev gemt uden kontrol og
hentet med en almindelig klient, og udbyderens svar blev givet tilbage ordret.
Det var en fungerende skanner af det interne net for enhver bruger: peg den på en
vært, læs de første tre hundrede bytes af det, der svarede. På en homelab er det
det meste af, hvad der er værd at nå.

Nu gennem `safedial`, og svarets krop går i loggen frem for i svaret. Operatøren
har stadig brug for beskeden — for en lokal model er den som regel selve
problemet — så den er der, bare kun for den, der kører instansen.

**M2 — Web Push havde tre.** Endepunktet blev gemt ukontrolleret og POSTet til på
en tidsplan, den, der ringer, selv vælger: sæt en påmindelse et minut ude, og
serveren henter adressen, hvert minut. Upserten flyttede desuden ejerskabet med
`user_id = excluded.user_id`, så enhver, der kendte en andens endepunkt, kunne
overtage det — offerets telefon ville få angriberens beskeder og ingen af sine
egne. Og `DeletePushSubscription` havde ingen ejer-kontrol.

**H3 — tre veje uden loft.** FTS-udtrykket byggede omkring halvtreds bytes pr.
ord uden grænse; en søgning på en kvart megabyte blev til to megabyte
MATCH-udtryk og seksogtyve sekunder inde i SQLite, mens den holdt på én af kun
fire til otte forbindelser. Nogle få af dem samtidig, og databasen er utilgængelig
for alle på instansen, logins inklusive. `filterquery.Parse` er kvadratisk —
halvtreds tusind led er syv sekunders CPU og seksogfyrre gigabyte allokering i én
forespørgsel — og havde hverken loft på antal eller på dybde. Og noteimporten
læste hver fil i arkivet uden grænse: 64 MB komprimeret kan blive til
seksogtres gigabyte skrevet ned på den disk, databasen ligger på.

**M1 — `comment_id` blev taget på tro.** Rettighedskontrollen så på opgaven i
adressen, mens filen blev hængt på den kommentar, formularen bad om. En viewer på
et delt projekt kan liste dets kommentarer, så hun kunne læse et id og lægge en
fil under en andens kommentar i et projekt, hun kun må læse.

**M4 — fire hemmeligheder lå i klartekst i hver sikkerhedskopi.** TOTP-koden og
AI-nøglen forsegles nu; begge konverterer sig selv næste gang de skrives.
Kalenderfeedets og mailadressens tokens gør ikke — de slås op på deres præcise
værdi og vises i fladen mere end én gang, hvilket en hash ikke kan. Det står nu
skrevet i `SECURITY.md`, som påstod det modsatte. At rette det betyder, at
adressen kun kan vises én gang, og det er en beslutning om, hvordan funktionen
virker.

Og en prøve på, at politikken overhovedet bliver sendt. Det var den, der gjorde
XSS-en i nat til indsat HTML frem for kørt script, og der var ingen prøve, der
rørte den — en foranstaltning, ingen prøve rører, kan forsvinde i en oprydning,
uden at nogen opdager det. Målt på script-src alene: `style-src 'unsafe-inline'`
står der med vilje, og en indsat stil kan gøre en side grim, ikke køre kode.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

internal/httpapi/comments_handlers.go

@@ -225,17 +225,32 @@ func (s *Server) handleUploadAttachment(w http.ResponseWriter, r *http.Request)
 
 	a := &store.Attachment{
 		TaskID:     taskID,
-		CommentID:  r.FormValue("comment_id"),
 		Filename:   safeFilename(header.Filename),
 		MimeType:   detectMime(header.Filename, header.Header.Get("Content-Type")),
 		Size:       size,
 		Path:       stored,
 		UploadedBy: user.ID,
 	}
+
 	// An attachment hangs on a task or on a comment, never both — the schema says
 	// so, and the form may have supplied a comment id.
-	if a.CommentID != "" {
-		a.TaskID = ""
+	//
+	// The comment is resolved and checked against the task in the URL. It was
+	// taken on trust, which meant the permission check above guarded the *task
+	// somebody named* while the file landed on *whatever comment they asked for*.
+	// A viewer on a shared project can list its comments, so she could read an id
+	// there and then post to a task in her own project with that id attached: the
+	// file appears under somebody else's comment, in a project she may only read,
+	// and is served to everyone in it from this origin under a name she chose.
+	// Somebody removed from a project could do the same with ids they kept.
+	if id := r.FormValue("comment_id"); id != "" {
+		owner, err := s.db.CommentTask(r.Context(), id)
+		if err != nil || owner != taskID {
+			os.Remove(filepath.Join(s.cfg.FilesDir(), stored))
+			writeError(w, http.StatusNotFound, CodeNotFound, "not found")
+			return
+		}
+		a.CommentID, a.TaskID = id, ""
 	}
 
 	if err := s.db.CreateAttachment(r.Context(), a); err != nil {

internal/httpapi/csp_header_test.go

@@ -0,0 +1,66 @@
+package httpapi
+
+import (
+	"strings"
+	"testing"
+)
+
+// Politikken skal stå i svaret, og den skal stadig være stram.
+//
+// Den er dét, der gjorde en XSS i noteeditoren til indsat HTML frem for kørt
+// script, dengang en `alt` slap uden om sin undslupning — og der var ingen prøve
+// på, at headeren overhovedet blev sendt. En sikkerhedsforanstaltning, ingen
+// prøve rører, er en, der kan forsvinde i en oprydning uden at nogen opdager det.
+func TestEveryResponseCarriesAStrictCSP(t *testing.T) {
+	ts := newTestServer(t)
+	ts.bootstrap(t)
+
+	resp, _ := ts.do(t, "GET", "/api/v1/auth/me", nil)
+	policy := resp.Header.Get("Content-Security-Policy")
+	if policy == "" {
+		t.Fatal("der er ingen Content-Security-Policy på svaret")
+	}
+
+	// Målt på script-src alene, og ikke på hele politikken.
+	//
+	// `style-src 'unsafe-inline'` står der med vilje og er forklaret i csp.go:
+	// Svelte skriver stilarter ind i dokumentet, og en indsat stil kan gøre en
+	// side grim, ikke køre kode. Det er script-src, der afgør, om indsat HTML
+	// bliver til et kørende script, og det er derfor den, prøven ser på.
+	scriptSrc := ""
+	for _, part := range strings.Split(policy, ";") {
+		if strings.HasPrefix(strings.TrimSpace(part), "script-src") {
+			scriptSrc = part
+		}
+	}
+	if scriptSrc == "" {
+		t.Fatal("der er ingen script-src i politikken")
+	}
+	for _, forbidden := range []string{"'unsafe-inline'", "'unsafe-eval'", "'unsafe-hashes'", "'strict-dynamic'"} {
+		if strings.Contains(scriptSrc, forbidden) {
+			t.Errorf("script-src indeholder %s: %q", forbidden, strings.TrimSpace(scriptSrc))
+		}
+	}
+
+	// Og de fire, der skal være der.
+	for _, want := range []string{"script-src", "object-src 'none'", "base-uri 'none'", "frame-ancestors 'none'"} {
+		if !strings.Contains(policy, want) {
+			t.Errorf("politikken mangler %q", want)
+		}
+	}
+}
+
+// Et bilag må ikke kunne blive læst som et dokument.
+func TestAttachmentsAreNotServedAsDocuments(t *testing.T) {
+	for _, mime := range []string{"image/svg+xml", "text/html", "application/xhtml+xml", "text/xml"} {
+		if inlineImage[mime] {
+			t.Errorf("%s står på listen over det, der vises inline", mime)
+		}
+	}
+	// Og listen skal faktisk kunne noget, ellers virker billeder i noter ikke.
+	for _, mime := range []string{"image/png", "image/jpeg"} {
+		if !inlineImage[mime] {
+			t.Errorf("%s mangler; billeder i noter kan ikke vises", mime)
+		}
+	}
+}

internal/httpapi/integrations_handlers.go

@@ -11,6 +11,7 @@ import (
 	"github.com/kristianwind/verdande/internal/ai"
 	"github.com/kristianwind/verdande/internal/auth"
 	"github.com/kristianwind/verdande/internal/quickadd"
+	"github.com/kristianwind/verdande/internal/safedial"
 	"github.com/kristianwind/verdande/internal/store"
 )
 
@@ -214,6 +215,24 @@ func (s *Server) handleSetAISettings(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// The address is checked where it is typed, as well as when it is dialled.
+	//
+	// safedial refuses a private destination at connect time and is the actual
+	// wall; this is so the person entering it hears about it now rather than
+	// discovering it later as a failed summary. A scheme other than http or https
+	// is refused outright — file:// and gopher:// are not providers.
+	if url := strings.TrimSpace(req.BaseURL); url != "" {
+		if !strings.HasPrefix(url, "http://") && !strings.HasPrefix(url, "https://") {
+			writeFieldErrors(w, map[string]string{"base_url": "must start with http:// or https://"})
+			return
+		}
+		if reason := safedial.CheckURL(url); reason != "" {
+			writeFieldErrors(w, map[string]string{"base_url": reason})
+			return
+		}
+		req.BaseURL = url
+	}
+
 	existing, err := s.aiConfig(r, user.ID)
 	if err != nil {
 		s.internal(w, r, "ai settings", err)

internal/httpapi/notes_handlers.go

@@ -294,6 +294,14 @@ func noteFilename(title string) string {
 // a few megabytes — and small enough that a wrong file is refused rather than read.
 const maxNoteImportBytes = 64 << 20
 
+// What the archive may become once it is unpacked, and how many entries it may
+// hold. See handleImportNotes: the upload cap bounds what arrives, and says
+// nothing at all about what it expands to.
+const (
+	maxNoteImportUnpacked = 2 << 30
+	maxNoteImportFiles    = 20_000
+)
+
 // handleImportNotes reads a zip of Markdown files and makes a note of each.
 //
 // The mirror of the export, and deliberately the same shape: whatever comes out of
@@ -332,24 +340,57 @@ func (s *Server) handleImportNotes(w http.ResponseWriter, r *http.Request) {
 	// note is read its pictures already have addresses to be rewritten to — a note
 	// and its images arrive in whatever order the archive happens to hold them, and
 	// one pass would leave half the links pointing at nothing.
+	// The archive as a whole, not just the upload.
+	//
+	// The 64 MB cap is on what arrives; deflate reaches about 1032:1, so those 64
+	// MB can become sixty-six gigabytes on the way out — written into the same
+	// volume the database lives on. The upload limit says nothing about that, and
+	// nothing else did either.
+	//
+	// Two ceilings: one per file, and one across the archive. A single enormous
+	// photograph and ten thousand small ones are different attacks with the same
+	// ending.
+	if len(zr.File) > maxNoteImportFiles {
+		writeError(w, http.StatusRequestEntityTooLarge, CodePayloadTooLarge,
+			"that archive holds too many files")
+		return
+	}
+	budget := int64(maxNoteImportUnpacked)
+
 	stored := map[string]string{}
 	for _, f := range zr.File {
 		name := f.Name
 		if f.FileInfo().IsDir() || skipFile(name) || strings.EqualFold(path.Ext(name), ".md") {
 			continue
 		}
+		// The header's own claim first, which costs nothing to check and rejects
+		// the obvious case before a byte is written. It is only a claim, so the
+		// reader below is limited as well.
+		if f.UncompressedSize64 > maxUploadBytes {
+			s.log.Warn("import: a file in the archive is too large", "file", name)
+			skipped++
+			continue
+		}
 		rc, err := f.Open()
 		if err != nil {
 			continue
 		}
-		hashed, size, err := s.storeUpload(rc)
+		hashed, size, err := s.storeUpload(io.LimitReader(rc, min(budget, maxUploadBytes)+1))
 		rc.Close()
 		if err != nil {
 			s.log.Warn("import note file", "err", err, "file", name)
 			continue
 		}
+		if size > maxUploadBytes || size > budget {
+			// Over the line once it was actually read: remove what was written
+			// rather than keeping a truncated file that would open as nothing.
+			os.Remove(filepath.Join(s.cfg.FilesDir(), hashed))
+			s.log.Warn("import: archive is larger unpacked than allowed", "file", name)
+			skipped++
+			continue
+		}
+		budget -= size
 		stored[name] = hashed
-		_ = size
 		files++
 	}
 

internal/httpapi/notes_import_limits_test.go

@@ -0,0 +1,77 @@
+package httpapi
+
+import (
+	"archive/zip"
+	"bytes"
+	"io"
+	"testing"
+)
+
+// En zip-bombe skal ikke kunne skrive gigabyte ned på den disk, databasen ligger
+// på.
+//
+// Uploaden er begrænset til 64 MB, og det siger ingenting om, hvad de 64 MB
+// bliver til: deflate når omkring 1032:1, så de kan pakkes ud til seksogtres
+// gigabyte. Prøven bygger en rigtig lille bombe — fire megabyte nuller, som
+// pakker til nogle få kilobyte — og ser efter, at loftet på det udpakkede holder.
+//
+// Skrevet imod tallene frem for imod endepunktet, fordi det, der skal holde, er
+// forholdet: en fil, hvis udpakkede størrelse overstiger loftet, må ikke læses
+// færdig.
+func TestAZipBombIsRefusedByItsUnpackedSize(t *testing.T) {
+	var buf bytes.Buffer
+	zw := zip.NewWriter(&buf)
+	w, err := zw.Create("bombe.bin")
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Nuller pakker næsten helt væk. Fire megabyte er nok til at vise forholdet
+	// uden at prøven selv bruger en gigabyte.
+	if _, err := w.Write(make([]byte, 4<<20)); err != nil {
+		t.Fatal(err)
+	}
+	if err := zw.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	packed := int64(buf.Len())
+	if packed > 64<<10 {
+		t.Fatalf("prøvens egen bombe pakkede kun til %d bytes; den viser ikke noget", packed)
+	}
+
+	zr, err := zip.NewReader(bytes.NewReader(buf.Bytes()), packed)
+	if err != nil {
+		t.Fatal(err)
+	}
+	f := zr.File[0]
+	if f.UncompressedSize64 != 4<<20 {
+		t.Fatalf("udpakket størrelse = %d", f.UncompressedSize64)
+	}
+
+	// Det, importen gør: læs højst det, budgettet tillader, plus én byte — så en
+	// fil, der er præcis på grænsen, kan skelnes fra en, der er over.
+	rc, err := f.Open()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer rc.Close()
+	budget := int64(1 << 20)
+	n, err := io.Copy(io.Discard, io.LimitReader(rc, budget+1))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n <= budget {
+		t.Fatalf("læste %d bytes af en fil på 4 MB med et budget på %d — loftet fanger den ikke", n, budget)
+	}
+}
+
+// Tallene skal stå i et forhold, der giver mening: en enkelt fil må aldrig kunne
+// bruge hele arkivets budget, og arkivet må ikke være mindre end én fil.
+func TestTheImportCeilingsAgreeWithEachOther(t *testing.T) {
+	if maxNoteImportUnpacked <= maxUploadBytes {
+		t.Error("arkivets budget er ikke større end én enkelt fil")
+	}
+	if maxNoteImportFiles < 2000 {
+		t.Error("filloftet er under det, en rigtig Apple Noter-eksport indeholder")
+	}
+}

internal/httpapi/notifications_handlers.go

@@ -2,11 +2,13 @@ package httpapi
 
 import (
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/go-chi/chi/v5"
 
 	"github.com/kristianwind/verdande/internal/push"
+	"github.com/kristianwind/verdande/internal/safedial"
 	"github.com/kristianwind/verdande/internal/store"
 )
 
@@ -170,6 +172,23 @@ func (s *Server) handleSubscribePush(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// The endpoint is an address this server will POST to, on a schedule the
+	// caller chooses — set a reminder a minute out and the push job fetches it,
+	// again every minute. Unchecked, that is a way to make the instance reach
+	// anything it can route to, on demand.
+	//
+	// https only: a push service is on the public internet and speaks TLS. Nothing
+	// legitimate needs http here, and allowing it is the difference between a
+	// blind request and a readable one.
+	if !strings.HasPrefix(req.Endpoint, "https://") {
+		writeFieldErrors(w, map[string]string{"endpoint": "must be an https address"})
+		return
+	}
+	if reason := safedial.CheckURL(req.Endpoint); reason != "" {
+		writeFieldErrors(w, map[string]string{"endpoint": reason})
+		return
+	}
+
 	err := s.db.SavePushSubscription(r.Context(), &store.PushSubscription{
 		UserID:    userFrom(r.Context()).ID,
 		Endpoint:  req.Endpoint,
@@ -189,7 +208,10 @@ func (s *Server) handleUnsubscribePush(w http.ResponseWriter, r *http.Request) {
 	if err := decodeJSON(w, r, &req); err != nil {
 		return
 	}
-	if err := s.db.DeletePushSubscription(r.Context(), req.Endpoint); err != nil {
+	// Scoped to the caller. It was not, so anybody who learned somebody else's
+	// endpoint could unsubscribe their device — a small thing to do to somebody
+	// and no reason at all to be able to.
+	if err := s.db.DeletePushSubscription(r.Context(), userFrom(r.Context()).ID, req.Endpoint); err != nil {
 		s.internal(w, r, "delete push subscription", err)
 		return
 	}