From d5d0656dbaac2d4f25353fa1dec07867d310d1b4 Mon Sep 17 00:00:00 2001
From: prosolis <5590409+prosolis@users.noreply.github.com>
Date: Sun, 21 Jun 2026 16:21:03 -0700
Subject: [PATCH] Harden security: SSRF guard on all outbound fetches, auth
 fixes

- Route image-validation, Matrix image-download, and wayback/archive.today
  fetches through safehttp so feed-controlled URLs can't reach loopback/
  RFC1918/cloud-metadata IPs (incl. via redirects).
- Cap feed body size with safehttp.LimitedBody (16 MiB) to prevent OOM.
- Fail closed if matrix.pickle_key is unset/<16 chars; drop the hardcoded
  "pete_pickle_key" default that silently weakened E2EE-at-rest.
- Gate !post behind a matrix.admins allowlist; empty = disabled (channel
  rooms are public, so empty must not mean anyone).
- Reject /\host open-redirect bypass in post-login safeNext.
- Allowlist http(s) schemes for the Matrix link href; escape JSON embedded
  in inline <script> (prefs/sources blobs).
---
 config.example.toml            |  3 +++
 internal/config/config.go      | 13 ++++++++++---
 internal/config/config_test.go | 10 ++++++++++
 internal/ingestion/image.go    | 15 ++++++---------
 internal/ingestion/parser.go   | 25 +++++++++++++++++++++----
 internal/ingestion/wayback.go  | 20 ++++++++++++--------
 internal/matrix/client.go      |  9 +++++++--
 internal/matrix/post.go        | 27 ++++++++++++++++++++++-----
 internal/web/auth.go           |  7 +++++--
 internal/web/handlers.go       | 19 +++++++++++++++++--
 main.go                        | 14 ++++++++++++++
 11 files changed, 127 insertions(+), 35 deletions(-)

diff --git a/config.example.toml b/config.example.toml
index 1ce0e4b..71b7ce0 100644
--- a/config.example.toml
+++ b/config.example.toml
@@ -6,6 +6,9 @@ pickle_key = "${PETE_PICKLE_KEY}"
 display_name = "Pete"
 data_dir = "./data"
 admin_room = "!adminroomid:matrix.example.org"
+# Matrix user IDs allowed to run in-room commands like !post. Empty disables
+# commands entirely (channel rooms may be public, so empty != "anyone").
+admins = ["@you:matrix.example.org"]
 
 [matrix.channels]
 tech = "!techroomid:matrix.example.org"
diff --git a/internal/config/config.go b/internal/config/config.go
index 4550c19..8df1e65 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -50,6 +50,10 @@ type MatrixConfig struct {
 	DataDir     string            `toml:"data_dir"`
 	AdminRoom   string            `toml:"admin_room"`
 	Channels    map[string]string `toml:"channels"`
+	// Admins is the allowlist of Matrix user IDs permitted to run in-room
+	// commands like !post. Empty means commands are disabled — the channel
+	// rooms are public, so an empty list must NOT mean "anyone".
+	Admins []string `toml:"admins"`
 }
 
 type PostingConfig struct {
@@ -130,6 +134,12 @@ func (c *Config) validate() error {
 	if c.Matrix.Password == "" {
 		return fmt.Errorf("matrix.password is required")
 	}
+	// pickle_key encrypts the E2EE crypto store (olm/megolm + cross-signing
+	// keys) at rest. Never fall back to a hardcoded default: a key baked into
+	// the source is no protection if crypto.db ever leaks.
+	if len(c.Matrix.PickleKey) < 16 {
+		return fmt.Errorf("matrix.pickle_key must be set to a strong secret (>=16 chars); it encrypts the E2EE crypto store at rest")
+	}
 	if len(c.Matrix.Channels) == 0 {
 		return fmt.Errorf("matrix.channels must have at least one entry")
 	}
@@ -184,9 +194,6 @@ func (c *Config) applyDefaults() {
 	if c.Matrix.DisplayName == "" {
 		c.Matrix.DisplayName = "Pete"
 	}
-	if c.Matrix.PickleKey == "" {
-		c.Matrix.PickleKey = "pete_pickle_key"
-	}
 	if c.Posting.MinIntervalSeconds == 0 {
 		c.Posting.MinIntervalSeconds = 300
 	}
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 57c95e5..6c6b57e 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -12,6 +12,7 @@ const validMatrix = `
 homeserver = "https://matrix.example.org"
 user_id = "@pete:example.org"
 password = "testpass"
+pickle_key = "test_pickle_key_1234"
 [matrix.channels]
 tech = "!tech:example.org"
 politics = "!politics:example.org"
@@ -57,6 +58,7 @@ func TestEnvVarExpansion(t *testing.T) {
 homeserver = "https://matrix.example.org"
 user_id = "@pete:example.org"
 password = "${TEST_PETE_PASS}"
+pickle_key = "test_pickle_key_1234"
 [matrix.channels]
 tech = "!tech:example.org"
 
@@ -114,6 +116,7 @@ func TestValidationErrors(t *testing.T) {
 [matrix]
 user_id = "@p:h"
 password = "pw"
+pickle_key = "test_pickle_key_1234"
 [matrix.channels]
 tech = "!t:e"
 [storage]
@@ -123,6 +126,7 @@ db_path = "/tmp/t.db"
 [matrix]
 homeserver = "https://h"
 password = "pw"
+pickle_key = "test_pickle_key_1234"
 [matrix.channels]
 tech = "!t:e"
 [storage]
@@ -142,6 +146,7 @@ db_path = "/tmp/t.db"
 homeserver = "https://h"
 user_id = "@p:h"
 password = "pw"
+pickle_key = "test_pickle_key_1234"
 [storage]
 db_path = "/tmp/t.db"
 `},
@@ -150,6 +155,7 @@ db_path = "/tmp/t.db"
 homeserver = "https://h"
 user_id = "@p:h"
 password = "pw"
+pickle_key = "test_pickle_key_1234"
 [matrix.channels]
 tech = "!t:e"
 [storage]
@@ -159,6 +165,7 @@ tech = "!t:e"
 homeserver = "https://h"
 user_id = "@p:h"
 password = "pw"
+pickle_key = "test_pickle_key_1234"
 [matrix.channels]
 tech = "!t:e"
 [storage]
@@ -176,6 +183,7 @@ enabled = true
 homeserver = "https://h"
 user_id = "@p:h"
 password = "pw"
+pickle_key = "test_pickle_key_1234"
 [matrix.channels]
 tech = "!t:e"
 [storage]
@@ -192,6 +200,7 @@ enabled = true
 homeserver = "https://h"
 user_id = "@p:h"
 password = "pw"
+pickle_key = "test_pickle_key_1234"
 [matrix.channels]
 tech = "!t:e"
 [storage]
@@ -225,6 +234,7 @@ func TestDisabledSourceSkipsDirectRouteCheck(t *testing.T) {
 homeserver = "https://h"
 user_id = "@p:h"
 password = "pw"
+pickle_key = "test_pickle_key_1234"
 [matrix.channels]
 tech = "!t:e"
 [storage]
diff --git a/internal/ingestion/image.go b/internal/ingestion/image.go
index 3f036db..7ebd515 100644
--- a/internal/ingestion/image.go
+++ b/internal/ingestion/image.go
@@ -7,6 +7,8 @@ import (
 	"strconv"
 	"strings"
 	"time"
+
+	"pete/internal/safehttp"
 )
 
 // NormalizeImageURL rewrites known CDN thumbnail URLs to a higher-resolution
@@ -36,15 +38,10 @@ func NormalizeImageURL(raw string) string {
 	return u.String()
 }
 
-var imageClient = &http.Client{
-	Timeout: 10 * time.Second,
-	CheckRedirect: func(req *http.Request, via []*http.Request) error {
-		if len(via) >= 3 {
-			return http.ErrUseLastResponse
-		}
-		return nil
-	},
-}
+// imageClient validates feed-supplied image URLs. It routes through safehttp
+// so a hostile feed can't steer the HEAD probe at loopback / RFC1918 / cloud
+// metadata IPs — the dial-time guard also re-checks every redirect target.
+var imageClient = safehttp.NewClient(10 * time.Second)
 
 // ValidateImageURL checks that an image URL returns a valid image response.
 // Returns false (with no error) on any failure — story posts without image.
diff --git a/internal/ingestion/parser.go b/internal/ingestion/parser.go
index 19c7f55..2872432 100644
--- a/internal/ingestion/parser.go
+++ b/internal/ingestion/parser.go
@@ -2,8 +2,10 @@ package ingestion
 
 import (
 	"context"
+	"fmt"
 	"html"
 	"log/slog"
+	"net/http"
 	"regexp"
 	"strconv"
 	"strings"
@@ -17,6 +19,11 @@ import (
 
 const userAgent = "Pete/1.0 (newsbot; +https://github.com/reala-misaki/pete)"
 
+// maxFeedBytes caps how much of a feed body we'll buffer into the parser, so a
+// hostile feed streaming an endless body within the request timeout can't OOM
+// the process. Generous headroom — real RSS/Atom feeds are well under this.
+const maxFeedBytes = 16 << 20
+
 var feedClient = safehttp.NewClient(30 * time.Second)
 
 // FeedItem represents a parsed RSS item ready for routing.
@@ -44,11 +51,21 @@ func FetchFeed(feedURL string) ([]FeedItem, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 
-	fp := gofeed.NewParser()
-	fp.Client = feedClient
-	fp.UserAgent = userAgent
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, feedURL, nil)
+	if err != nil {
+		return nil, err
+	}
+	req.Header.Set("User-Agent", userAgent)
+	resp, err := feedClient.Do(req)
+	if err != nil {
+		return nil, err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("feed fetch %s: status %d", feedURL, resp.StatusCode)
+	}
 
-	feed, err := fp.ParseURLWithContext(feedURL, ctx)
+	feed, err := gofeed.NewParser().Parse(safehttp.LimitedBody(resp.Body, maxFeedBytes))
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/ingestion/wayback.go b/internal/ingestion/wayback.go
index f8f6cc3..a42a2c5 100644
--- a/internal/ingestion/wayback.go
+++ b/internal/ingestion/wayback.go
@@ -7,9 +7,11 @@ import (
 	"net/url"
 	"strings"
 	"time"
+
+	"pete/internal/safehttp"
 )
 
-var waybackClient = &http.Client{Timeout: 10 * time.Second}
+var waybackClient = safehttp.NewClient(10 * time.Second)
 
 // maxSnapshotAge bounds how stale a Wayback snapshot can be before we treat
 // it as useless for a freshly-published news article. The "closest" snapshot
@@ -86,14 +88,16 @@ func snapshotFreshEnough(ts string) bool {
 	return time.Since(t) <= maxSnapshotAge
 }
 
-var archiveTodayClient = &http.Client{
-	Timeout: 10 * time.Second,
-	// Don't follow the redirect — we just want the Location header pointing
-	// at the most recent capture.
-	CheckRedirect: func(*http.Request, []*http.Request) error {
+// archiveTodayClient routes through safehttp for the dial-time SSRF guard, but
+// overrides CheckRedirect so we read the 302 Location header (the most recent
+// capture) instead of following it.
+var archiveTodayClient = func() *http.Client {
+	c := safehttp.NewClient(10 * time.Second)
+	c.CheckRedirect = func(*http.Request, []*http.Request) error {
 		return http.ErrUseLastResponse
-	},
-}
+	}
+	return c
+}()
 
 // ResolveArchiveToday looks up the newest archive.today / archive.ph snapshot
 // for the given URL. archive.ph has no public availability API, but its
diff --git a/internal/matrix/client.go b/internal/matrix/client.go
index 0d4c01f..ef73eab 100644
--- a/internal/matrix/client.go
+++ b/internal/matrix/client.go
@@ -19,6 +19,7 @@ import (
 	"time"
 
 	"pete/internal/config"
+	"pete/internal/safehttp"
 
 	_ "go.mau.fi/util/dbutil/litestream" // registers "sqlite3-fk-wal" driver used by cryptohelper
 
@@ -430,14 +431,18 @@ func (c *Client) PostStory(channel string, story *PostableStory) (eventID id.Eve
 	return resp.EventID, imageSent, nil
 }
 
+// imageDownloadClient fetches feed-supplied image URLs for re-upload to Matrix.
+// It routes through safehttp so a hostile feed can't point ImageURL at internal
+// services (loopback / RFC1918 / cloud metadata) — including via redirects.
+var imageDownloadClient = safehttp.NewClient(15 * time.Second)
+
 // sendImage downloads an image, uploads it to Matrix, and sends it as m.image.
 func (c *Client) sendImage(ctx context.Context, roomID id.RoomID, imageURL string) error {
 	req, err := http.NewRequestWithContext(ctx, "GET", imageURL, nil)
 	if err != nil {
 		return fmt.Errorf("create image request: %w", err)
 	}
-	httpClient := &http.Client{Timeout: 15 * time.Second}
-	resp, err := httpClient.Do(req)
+	resp, err := imageDownloadClient.Do(req)
 	if err != nil {
 		return fmt.Errorf("download image: %w", err)
 	}
diff --git a/internal/matrix/post.go b/internal/matrix/post.go
index d1f0642..8639aa0 100644
--- a/internal/matrix/post.go
+++ b/internal/matrix/post.go
@@ -3,9 +3,21 @@ package matrix
 import (
 	"fmt"
 	"html"
+	"net/url"
 	"strings"
 )
 
+// safeHref returns raw if it is an http(s) URL, else "". Keeps feed-supplied
+// links from injecting javascript:/data: schemes into the formatted_body href
+// (html.EscapeString blocks attribute breakout but not the scheme itself).
+func safeHref(raw string) string {
+	u, err := url.Parse(strings.TrimSpace(raw))
+	if err != nil || (u.Scheme != "http" && u.Scheme != "https") {
+		return ""
+	}
+	return raw
+}
+
 // PostableStory contains the data needed to format and send a story.
 type PostableStory struct {
 	ImageURL   string
@@ -30,11 +42,16 @@ func formatPost(s *PostableStory) (plain, htmlBody string) {
 
 	// HTML body
 	var htmlParts []string
-	htmlParts = append(htmlParts, fmt.Sprintf(
-		`<strong><a href="%s">%s</a></strong>`,
-		html.EscapeString(s.ArticleURL),
-		html.EscapeString(s.Headline),
-	))
+	if href := safeHref(s.ArticleURL); href != "" {
+		htmlParts = append(htmlParts, fmt.Sprintf(
+			`<strong><a href="%s">%s</a></strong>`,
+			html.EscapeString(href),
+			html.EscapeString(s.Headline),
+		))
+	} else {
+		// Non-http(s) link: render the headline without an anchor.
+		htmlParts = append(htmlParts, fmt.Sprintf(`<strong>%s</strong>`, html.EscapeString(s.Headline)))
+	}
 	if s.Lede != "" {
 		htmlParts = append(htmlParts, html.EscapeString(s.Lede))
 	}
diff --git a/internal/web/auth.go b/internal/web/auth.go
index 13918d0..b4d9b6e 100644
--- a/internal/web/auth.go
+++ b/internal/web/auth.go
@@ -261,9 +261,12 @@ func (a *Authenticator) handleLogout(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, "/", http.StatusFound)
 }
 
-// safeNext keeps post-login redirects on-site (no open redirects).
+// safeNext keeps post-login redirects on-site (no open redirects). It rejects
+// "//host" and "/\host" — browsers normalize the backslash to a slash, so the
+// latter would otherwise resolve to a protocol-relative off-site URL.
 func safeNext(next string) string {
-	if next == "" || !strings.HasPrefix(next, "/") || strings.HasPrefix(next, "//") {
+	if next == "" || !strings.HasPrefix(next, "/") ||
+		strings.HasPrefix(next, "//") || strings.HasPrefix(next, "/\\") {
 		return "/"
 	}
 	return next
diff --git a/internal/web/handlers.go b/internal/web/handlers.go
index cff5b5d..d54a9a8 100644
--- a/internal/web/handlers.go
+++ b/internal/web/handlers.go
@@ -82,6 +82,21 @@ type channelStat struct {
 	Total         int
 }
 
+// jsForScript marks already-valid JSON as safe to embed in an inline <script>,
+// after neutralizing the byte sequences that could break out of that context
+// (</script>, HTML comments, and the JS line separators U+2028/U+2029). Without
+// this, a stored prefs blob containing "</script>" could inject markup.
+func jsForScript(b []byte) template.JS {
+	r := strings.NewReplacer(
+		"<", "\\u003c",
+		">", "\\u003e",
+		"&", "\\u0026",
+		"
", "\\u2028",
+		"
", "\\u2029",
+	)
+	return template.JS(r.Replace(string(b)))
+}
+
 func (s *Server) base(r *http.Request) pageData {
 	srcJSON, err := json.Marshal(s.sources)
 	if err != nil {
@@ -91,7 +106,7 @@ func (s *Server) base(r *http.Request) pageData {
 		SiteTitle:   s.cfg.SiteTitle,
 		Channels:    channels,
 		Weather:     currentWeather(time.Now()),
-		AllSources:  template.JS(srcJSON),
+		AllSources:  jsForScript(srcJSON),
 		AuthEnabled: s.auth != nil,
 		UserPrefs:   template.JS("null"),
 		Path:        r.URL.Path,
@@ -100,7 +115,7 @@ func (s *Server) base(r *http.Request) pageData {
 		if u := s.auth.userFromRequest(r); u != nil {
 			d.User = u
 			if blob, err := storage.GetUserPrefs(u.Sub); err == nil && blob != "" {
-				d.UserPrefs = template.JS(blob)
+				d.UserPrefs = jsForScript([]byte(blob))
 			}
 		}
 	}
diff --git a/main.go b/main.go
index cf5d7f9..d432874 100644
--- a/main.go
+++ b/main.go
@@ -88,11 +88,25 @@ func main() {
 	// Wire reaction handler
 	mx.SetReactionHandler(poster.HandleReaction)
 
+	// Allowlist of users permitted to run in-room commands. The channel rooms
+	// are public, so commands are gated to configured admins; an empty list
+	// disables them entirely (fail closed) rather than allowing anyone.
+	admins := make(map[id.UserID]bool, len(cfg.Matrix.Admins))
+	for _, a := range cfg.Matrix.Admins {
+		if a = strings.TrimSpace(a); a != "" {
+			admins[id.UserID(a)] = true
+		}
+	}
+
 	// Wire !post command: force-publish next queued story for the room's channel.
 	mx.SetMessageHandler(func(channel string, roomID id.RoomID, eventID id.EventID, sender id.UserID, body string) {
 		if strings.TrimSpace(body) != "!post" {
 			return
 		}
+		if !admins[sender] {
+			slog.Warn("!post ignored: sender not in matrix.admins allowlist", "channel", channel, "sender", sender)
+			return
+		}
 		slog.Info("!post requested", "channel", channel, "sender", sender)
 		if queue.ForcePost(channel) {
 			return