security: backport v0.8.3 hardening to main

cmd/ember/main.go

@@ -279,11 +279,14 @@ func run() error {
 		sum = ollamaSum
 	}
 
-	fetcher := feed.NewFetcher(30 * time.Second)
-	// Block redirects to private/internal addresses on every feed fetch.
-	fetcher.Client.CheckRedirect = feed.RedirectGuard(func(rawURL string) error {
+	// Block redirects to private/internal addresses on every feed fetch — the
+	// guard is baked into the fetcher at construction.
+	fetcher := feed.NewFetcher(30*time.Second, func(rawURL string) error {
 		return urlcheck.Check(ctx, rawURL, cfg.AllowPrivateURLs)
 	})
+	// IP-pinning transport closes the DNS-rebind window between the pre-flight
+	// urlcheck and the actual dial (the redirect guard only covers 3xx hops).
+	fetcher.Client.Transport = urlcheck.GuardedTransport(cfg.AllowPrivateURLs)
 	p := poller.New(st, fetcher, sum, poller.Config{
 		Tick:                        cfg.PollTick,
 		Concurrency:                 cfg.PollConcurrency,

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/go-shiori/go-readability v0.0.0-20251205110129-5db1dc9836f0
 	github.com/go-webauthn/webauthn v0.17.4
 	github.com/gorilla/securecookie v1.1.2
+	github.com/microcosm-cc/bluemonday v1.0.27
 	github.com/mmcdole/gofeed v1.3.0
 	github.com/pressly/goose/v3 v3.27.1
 	golang.org/x/crypto v0.52.0
@@ -18,6 +19,7 @@ require (
 	github.com/PuerkitoBio/goquery v1.8.0 // indirect
 	github.com/andybalholm/cascadia v1.3.3 // indirect
 	github.com/araddon/dateparse v0.0.0-20210429162001-6b43995a97de // indirect
+	github.com/aymerick/douceur v0.2.0 // indirect
 	github.com/dustin/go-humanize v1.0.1 // indirect
 	github.com/fxamacker/cbor/v2 v2.9.2 // indirect
 	github.com/go-shiori/dom v0.0.0-20230515143342-73569d674e1c // indirect
@@ -27,6 +29,7 @@ require (
 	github.com/golang-jwt/jwt/v5 v5.3.1 // indirect
 	github.com/google/go-tpm v0.9.8 // indirect
 	github.com/google/uuid v1.6.0 // indirect
+	github.com/gorilla/css v1.0.1 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/mattn/go-isatty v0.0.21 // indirect
 	github.com/mfridman/interpolate v0.0.2 // indirect

go.sum

@@ -5,6 +5,8 @@ github.com/andybalholm/cascadia v1.3.3 h1:AG2YHrzJIm4BZ19iwJ/DAua6Btl3IwJX+VI4kk
 github.com/andybalholm/cascadia v1.3.3/go.mod h1:xNd9bqTn98Ln4DwST8/nG+H0yuB8Hmgu1YHNnWw0GeA=
 github.com/araddon/dateparse v0.0.0-20210429162001-6b43995a97de h1:FxWPpzIjnTlhPwqqXc4/vE0f7GvRjuAsbW+HOIe8KnA=
 github.com/araddon/dateparse v0.0.0-20210429162001-6b43995a97de/go.mod h1:DCaWoUhZrYW9p1lxo/cm8EmUOOzAPSEZNGF2DK1dJgw=
+github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk=
+github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -40,6 +42,8 @@ github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e h1:ijClszYn+mADRFY17k
 github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e/go.mod h1:boTsfXsheKC2y+lKOCMpSfarhxDeIzfZG1jqGcPl3cA=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/gorilla/css v1.0.1 h1:ntNaBIghp6JmvWnxbZKANoLyuXTPZ4cAMlo6RyhlbO8=
+github.com/gorilla/css v1.0.1/go.mod h1:BvnYkspnSzMmwRK+b8/xgNPLiIuNZr6vbZBTPQ2A3b0=
 github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA=
 github.com/gorilla/securecookie v1.1.2/go.mod h1:NfCASbcHqRSY+3a8tlWJwsQap2VX5pwzwo4h3eOamfo=
 github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
@@ -51,6 +55,8 @@ github.com/mattn/go-isatty v0.0.21/go.mod h1:ZXfXG4SQHsB/w3ZeOYbR0PrPwLy+n6xiMrJ
 github.com/mattn/go-runewidth v0.0.10/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk=
 github.com/mfridman/interpolate v0.0.2 h1:pnuTK7MQIxxFz1Gr+rjSIx9u7qVjf5VOoM/u6BbAxPY=
 github.com/mfridman/interpolate v0.0.2/go.mod h1:p+7uk6oE07mpE/Ik1b8EckO0O4ZXiGAfshKBWLUM9Xg=
+github.com/microcosm-cc/bluemonday v1.0.27 h1:MpEUotklkwCSLeH+Qdx1VJgNqLlpY2KXwXFM08ygZfk=
+github.com/microcosm-cc/bluemonday v1.0.27/go.mod h1:jFi9vgW+H7c3V0lb6nR74Ib/DIB5OBs92Dimizgw2cA=
 github.com/mmcdole/gofeed v1.3.0 h1:5yn+HeqlcvjMeAI4gu6T+crm7d0anY85+M+v6fIFNG4=
 github.com/mmcdole/gofeed v1.3.0/go.mod h1:9TGv2LcJhdXePDzxiuMnukhV2/zb6VtnZt1mS+SjkLE=
 github.com/mmcdole/goxpp v1.1.1-0.20240225020742-a0c311522b23 h1:Zr92CAlFhy2gL+V1F+EyIuzbQNbSgP4xhTODZtrXUtk=

internal/api/feed_handlers.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"log/slog"
 	"net/http"
 	"time"
 
@@ -342,7 +343,12 @@ func (d *Dependencies) handleTTRSSAPIImport(w http.ResponseWriter, r *http.Reque
 		ImportArchived: req.ImportArchived,
 	})
 	if err != nil {
-		writeError(w, http.StatusBadGateway, "import_failed", err.Error())
+		// Log the full error server-side for diagnosis; return a generic
+		// message. Raw net/http / DNS / TLS errors carry the resolved endpoint,
+		// internal hostnames, and TLS detail that shouldn't reach the client.
+		slog.Default().Warn("ttrss api import failed", "url", req.URL, "err", err)
+		writeError(w, http.StatusBadGateway, "import_failed",
+			"could not import from TT-RSS — check the URL/credentials and the server logs.")
 		return
 	}
 	writeData(w, http.StatusOK, res, nil)

internal/api/filter_handlers.go

@@ -9,6 +9,10 @@ import (
 	"github.com/brandonhon/ember/internal/store"
 )
 
+// maxFiltersPerUser bounds how many filters one account can create; see
+// handleCreateFilter.
+const maxFiltersPerUser = 200
+
 type filterReq struct {
 	Name      string `json:"name"`
 	MatchJSON string `json:"match_json"`
@@ -44,6 +48,18 @@ func (d *Dependencies) handleCreateFilter(w http.ResponseWriter, r *http.Request
 		writeError(w, http.StatusBadRequest, "bad_request", err.Error())
 		return
 	}
+	// Cap filters per user: each filter's regex is compiled and cached for the
+	// process lifetime, so an unbounded count is a memory-DoS vector. 200 is
+	// far beyond any realistic use.
+	existing, err := d.Store.ListFilters(r.Context(), u.ID)
+	if mapStoreError(w, err) {
+		return
+	}
+	if len(existing) >= maxFiltersPerUser {
+		writeError(w, http.StatusBadRequest, "filter_limit",
+			"filter limit reached (max 200 per user)")
+		return
+	}
 	enabled := true
 	if req.Enabled != nil {
 		enabled = *req.Enabled

internal/api/llm_handlers.go

@@ -4,13 +4,21 @@ import (
 	"context"
 	"log/slog"
 	"net/http"
+	"regexp"
 	"strconv"
 	"time"
 
 	"github.com/brandonhon/ember/internal/summarize"
 	"github.com/brandonhon/ember/internal/sysinfo"
 )
 
+// validModelName bounds an Ollama model reference to its documented shape
+// ([registry/][namespace/]name[:tag]) and printable ASCII. Even though these
+// endpoints are admin-only, validating the name stops a compromised/rogue admin
+// from coaxing the Ollama daemon into pulling from an arbitrary registry or
+// dereferencing path-traversal components.
+var validModelName = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._:/-]{0,255}$`)
+
 func floatToStr(f float64) string { return strconv.FormatFloat(f, 'f', -1, 64) }
 func intToStr(i int) string       { return strconv.Itoa(i) }
 
@@ -72,6 +80,10 @@ func (d *Dependencies) handleSetLLMModel(w http.ResponseWriter, r *http.Request)
 		writeError(w, http.StatusBadRequest, "bad_request", "model required")
 		return
 	}
+	if !validModelName.MatchString(req.Model) {
+		writeError(w, http.StatusBadRequest, "bad_request", "invalid model name")
+		return
+	}
 	if err := d.Store.PutAppSetting(r.Context(), "ollama_model", req.Model); err != nil {
 		internalError(w, "internal", err)
 		return
@@ -141,6 +153,10 @@ func (d *Dependencies) handleDeleteLLMModel(w http.ResponseWriter, r *http.Reque
 		writeError(w, http.StatusBadRequest, "bad_request", "model required")
 		return
 	}
+	if !validModelName.MatchString(req.Model) {
+		writeError(w, http.StatusBadRequest, "bad_request", "invalid model name")
+		return
+	}
 	if req.Model == d.Ollama.Model() {
 		writeError(w, http.StatusConflict, "active_model", "cannot delete the active model — switch first")
 		return
@@ -170,6 +186,10 @@ func (d *Dependencies) handlePullLLMModel(w http.ResponseWriter, r *http.Request
 		writeError(w, http.StatusBadRequest, "bad_request", "model required")
 		return
 	}
+	if !validModelName.MatchString(req.Model) {
+		writeError(w, http.StatusBadRequest, "bad_request", "invalid model name")
+		return
+	}
 	// Override per-connection deadlines so the response can stay open for
 	// the duration of the pull. http.NewResponseController is the modern
 	// way to do this; errors mean the server doesn't support it (very old

internal/api/render.go

@@ -57,6 +57,8 @@ func mapStoreError(w http.ResponseWriter, err error) bool {
 		writeError(w, http.StatusForbidden, "forbidden", "forbidden")
 	case errors.Is(err, store.ErrConflict):
 		writeError(w, http.StatusConflict, "conflict", "resource already exists")
+	case errors.Is(err, store.ErrInvalidQuery):
+		writeError(w, http.StatusBadRequest, "bad_request", "invalid search query")
 	default:
 		slog.Default().Error("store error", "err", err)
 		writeError(w, http.StatusInternalServerError, "internal", "internal error")

internal/api/user_handlers.go

@@ -32,7 +32,7 @@ type adminUserView struct {
 }
 
 func (d *Dependencies) handleListUsers(w http.ResponseWriter, r *http.Request) {
-	users, err := d.Store.ListUsers(r.Context())
+	users, err := d.Store.ListUsersPublic(r.Context())
 	if mapStoreError(w, err) {
 		return
 	}

internal/auth/auth.go

@@ -176,6 +176,15 @@ func (a *Auth) VerifyPassword(plain, encoded string) error {
 	return nil
 }
 
+// equalizeTiming runs a throwaway argon2id derivation with the live cost
+// params so the user-not-found path costs the same as a real VerifyPassword.
+// The result is discarded — only the elapsed time matters.
+func (a *Auth) equalizeTiming(password string) {
+	var salt [16]byte // fixed; the derivation is never compared, only timed
+	_ = argon2.IDKey([]byte(password), salt[:],
+		a.Params.Iterations, a.Params.Memory, a.Params.Parallelism, a.Params.KeyLength)
+}
+
 func decodeArgon2id(encoded string) (Params, []byte, []byte, error) {
 	parts := strings.Split(encoded, "$")
 	if len(parts) != 6 || parts[1] != "argon2id" {
@@ -367,6 +376,11 @@ func (a *Auth) BootstrapAdmin(ctx context.Context, username, password string) (m
 	if username == "" || password == "" {
 		return models.User{}, false, errors.New("auth: bootstrap admin requires EMBER_ADMIN_USER and EMBER_ADMIN_PASSWORD")
 	}
+	// Enforce the same 8-char floor as the create-user/change-password paths so
+	// the first-run admin can't be seeded with a trivially weak password.
+	if len(password) < 8 {
+		return models.User{}, false, errors.New("auth: EMBER_ADMIN_PASSWORD must be at least 8 characters")
+	}
 	hash, err := a.HashPassword(password)
 	if err != nil {
 		return models.User{}, false, err
@@ -390,6 +404,10 @@ func (a *Auth) BootstrapAdmin(ctx context.Context, username, password string) (m
 func (a *Auth) Login(ctx context.Context, w http.ResponseWriter, r *http.Request, username, password string) (models.User, error) {
 	u, err := a.Store.GetUserByUsername(ctx, username)
 	if errors.Is(err, store.ErrNotFound) {
+		// Equalize timing with the found-user path: without a throwaway argon2
+		// derivation, a missing username returns in ~1ms while a real one takes
+		// ~100ms, leaking account existence via a timing side channel.
+		a.equalizeTiming(password)
 		return models.User{}, ErrInvalidCredentials
 	}
 	if err != nil {

internal/auth/auth_test.go

@@ -294,6 +294,12 @@ func TestBootstrapAdmin(t *testing.T) {
 	if _, _, err := a2.BootstrapAdmin(ctx, "", ""); err == nil {
 		t.Error("empty creds should fail")
 	}
+
+	// Sub-8-char password on first run → error (no admin created).
+	a3 := newAuth(t)
+	if _, created, err := a3.BootstrapAdmin(ctx, "root", "short7!"); err == nil || created {
+		t.Errorf("short password should fail: created=%v err=%v", created, err)
+	}
 }
 
 func TestLogin(t *testing.T) {

internal/feed/discover.go

@@ -70,8 +70,16 @@ func Discover(ctx context.Context, c *http.Client, target string, validate func(
 	}
 
 	if href := findAlternateInHTML(body); href != "" {
-		abs, _ := resolveRef(parsedTarget, href)
-		return abs, nil
+		if abs, rerr := resolveRef(parsedTarget, href); rerr == nil && abs != "" {
+			// The discovered link crosses the same trust boundary as the
+			// target; validate before returning it (the caller fetches it).
+			// On reject/unresolvable, fall through to the fallback probes —
+			// matching DiscoverAll's drop-and-continue rather than handing back
+			// an unchecked URL.
+			if verr := validate(abs); verr == nil {
+				return abs, nil
+			}
+		}
 	}
 
 	for _, p := range DiscoveryFallbacks {

internal/feed/fetch.go

@@ -46,13 +46,21 @@ func RedirectGuard(validate func(rawURL string) error) func(*http.Request, []*ht
 	}
 }
 
-// NewFetcher returns a Fetcher with sensible defaults.
-func NewFetcher(timeout time.Duration) *Fetcher {
+// NewFetcher returns a Fetcher with sensible defaults and an SSRF redirect
+// guard wired in at construction, so the safe default is the only option — a
+// caller can't accidentally build an unguarded fetcher. validate is invoked on
+// every redirect hop's next URL (pass urlcheck.Check); nil still enforces the
+// 10-redirect cap but performs no address check, so production must supply a
+// real validator.
+func NewFetcher(timeout time.Duration, validate func(rawURL string) error) *Fetcher {
 	if timeout <= 0 {
 		timeout = 30 * time.Second
 	}
 	return &Fetcher{
-		Client:    &http.Client{Timeout: timeout},
+		Client: &http.Client{
+			Timeout:       timeout,
+			CheckRedirect: RedirectGuard(validate),
+		},
 		UserAgent: DefaultUserAgent,
 	}
 }

internal/feed/fetch_test.go

@@ -35,7 +35,7 @@ func TestFetch_ConditionalGET_304(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	f := NewFetcher(5 * time.Second)
+	f := NewFetcher(5*time.Second, nil)
 	res, err := f.Fetch(context.Background(), srv.URL, etag, lastMod)
 	if err != nil {
 		t.Fatal(err)
@@ -65,7 +65,7 @@ func TestFetch_200ReturnsBody(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	f := NewFetcher(5 * time.Second)
+	f := NewFetcher(5*time.Second, nil)
 	res, err := f.Fetch(context.Background(), srv.URL, "", "")
 	if err != nil {
 		t.Fatal(err)
@@ -87,7 +87,7 @@ func TestFetch_5xxIsError(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	f := NewFetcher(5 * time.Second)
+	f := NewFetcher(5*time.Second, nil)
 	_, err := f.Fetch(context.Background(), srv.URL, "", "")
 	if err == nil {
 		t.Fatal("expected error on 500")
@@ -100,7 +100,7 @@ func TestFetch_Timeout(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	f := NewFetcher(50 * time.Millisecond)
+	f := NewFetcher(50*time.Millisecond, nil)
 	_, err := f.Fetch(context.Background(), srv.URL, "", "")
 	if err == nil {
 		t.Fatal("expected timeout error")

internal/feed/parse.go

@@ -63,14 +63,20 @@ func normalizeItem(it *gofeed.Item, feedID int64, base *url.URL) models.Article
 		a.GUID = strings.TrimSpace(it.Link)
 	}
 	a.Title = strings.TrimSpace(it.Title)
-	a.URL = resolveLink(base, it.Link)
+	// Guard the article link: it is rendered as an href ("Original" button), so
+	// drop javascript:/data: and other non-web schemes. GUID fell back to the
+	// raw link above, so dedup is unaffected when this clears a bad URL.
+	a.URL = SafeHTTPURL(resolveLink(base, it.Link))
 
 	switch {
 	case it.Content != "":
 		a.ContentHTML = it.Content
 	case it.Description != "":
 		a.ContentHTML = it.Description
 	}
+	// Feed bodies are rendered via {@html}; sanitize before deriving text and
+	// storing so stored HTML is render-safe regardless of the CSP.
+	a.ContentHTML = SanitizeHTML(a.ContentHTML)
 	a.ContentText = htmlToText(a.ContentHTML)
 
 	if it.Author != nil {

internal/feed/readability.go

@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
-	"time"
 
 	"github.com/go-shiori/go-readability"
 )
@@ -23,7 +22,10 @@ type Readable struct {
 // readability-extracted view.
 func ExtractFromURL(ctx context.Context, c *http.Client, target string) (Readable, error) {
 	if c == nil {
-		c = &http.Client{Timeout: 30 * time.Second}
+		// Require a caller-supplied client: the SSRF guard (redirect + dial)
+		// lives on it, so a nil client would be an unguarded fetch. Callers
+		// build a guarded client (see poller.enrichWithReadability).
+		return Readable{}, errors.New("readability: nil http client (SSRF guard required)")
 	}
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil)
 	if err != nil {
@@ -45,7 +47,7 @@ func ExtractFromURL(ctx context.Context, c *http.Client, target string) (Readabl
 	}
 	return Readable{
 		Title:    strings.TrimSpace(art.Title),
-		HTML:     art.Content,
+		HTML:     SanitizeHTML(art.Content),
 		Text:     strings.TrimSpace(art.TextContent),
 		ImageURL: art.Image,
 	}, nil
@@ -61,7 +63,7 @@ func extractFromHTML(body, target string) (Readable, error) {
 	}
 	return Readable{
 		Title:    strings.TrimSpace(art.Title),
-		HTML:     art.Content,
+		HTML:     SanitizeHTML(art.Content),
 		Text:     strings.TrimSpace(art.TextContent),
 		ImageURL: art.Image,
 	}, nil