From b2c6bc91d03e1779a56b88d241734233c77d344f Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 18:06:04 +0000 Subject: [PATCH 1/7] Add failing tests for referer_blocklist Both image routes must refuse a request whose Referer names a host on referer_blocklist with 403 and the JSON error, without fetching from the upstream host and whether or not the image is cached, and must serve a request with no Referer, one that does not parse, or one naming another host. Hosts match as allowlist_hosts matches them. The config tests check the list is read from the file and from PIXA_REFERER_BLOCKLIST, and that an entry that is not a host aborts startup naming the setting and the entry. These do not compile until the setting exists. Model: opus-5-5 --- internal/config/env_internal_test.go | 2 + .../config/referer_blocklist_internal_test.go | 107 +++++++++++ .../referer_blocklist_internal_test.go | 180 ++++++++++++++++++ 3 files changed, 289 insertions(+) create mode 100644 internal/config/referer_blocklist_internal_test.go create mode 100644 internal/handlers/referer_blocklist_internal_test.go diff --git a/internal/config/env_internal_test.go b/internal/config/env_internal_test.go index dd90dfa..bedbe06 100644 --- a/internal/config/env_internal_test.go +++ b/internal/config/env_internal_test.go @@ -65,6 +65,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) { t.Setenv("PIXA_METRICS_PASSWORD", "metricspass") t.Setenv("PIXA_SIGNING_KEY", validTestSigningKey) t.Setenv("PIXA_ALLOWLIST_HOSTS", "s3.sneak.cloud,.example.com") + t.Setenv("PIXA_REFERER_BLOCKLIST", "leech.example,.hotlinker.example") t.Setenv("PIXA_ALLOW_HTTP", "true") t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5") t.Setenv("PIXA_UPSTREAM_CONNECTIONS", "10") @@ -93,6 +94,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) { MetricsPassword: "metricspass", SigningKey: validTestSigningKey, AllowlistHosts: []string{testHostS3, ".example.com"}, + RefererBlocklist: []string{"leech.example", ".hotlinker.example"}, AllowHTTP: true, UpstreamConnectionsPerHost: 5, UpstreamConnections: 10, diff --git a/internal/config/referer_blocklist_internal_test.go b/internal/config/referer_blocklist_internal_test.go new file mode 100644 index 0000000..6128132 --- /dev/null +++ b/internal/config/referer_blocklist_internal_test.go @@ -0,0 +1,107 @@ +package config + +import ( + "slices" + "testing" +) + +// TestRefererBlocklistParsed loads a referer_blocklist with a host and a +// pattern starting with "." and checks both are kept in order. +func TestRefererBlocklistParsed(t *testing.T) { + t.Parallel() + + c, err := configFromYAML(t, signingKeyLine+`referer_blocklist: + - leech.example + - .hotlinker.example +`) + if err != nil { + t.Fatalf("valid referer_blocklist should load, got error: %v", err) + } + + want := []string{"leech.example", ".hotlinker.example"} + if !slices.Equal(c.RefererBlocklist, want) { + t.Errorf("RefererBlocklist = %v, want %v", c.RefererBlocklist, want) + } +} + +// TestRefererBlocklistOmittedIsEmpty checks that an omitted key blocks no +// referer. +func TestRefererBlocklistOmittedIsEmpty(t *testing.T) { + t.Parallel() + + c, err := configFromYAML(t, signingKeyLine) + if err != nil { + t.Fatalf("minimal config should be valid, got error: %v", err) + } + + if len(c.RefererBlocklist) != 0 { + t.Errorf("RefererBlocklist = %v, want empty", c.RefererBlocklist) + } +} + +// TestRefererBlocklistInvalidAbortsStartup checks that an entry that is not a +// host, or a value that is not a list of them, aborts startup with an error +// naming the key and the entry. +func TestRefererBlocklistInvalidAbortsStartup(t *testing.T) { + t.Parallel() + + runAbortCases(t, []abortCase{ + { + name: "url", + yaml: signingKeyLine + "referer_blocklist:\n - https://leech.example\n", + wantErrSubstrings: []string{ + keyRefererBlocklist, "https://leech.example", + }, + }, + { + name: "path", + yaml: signingKeyLine + "referer_blocklist:\n - leech.example/page\n", + wantErrSubstrings: []string{ + keyRefererBlocklist, "leech.example/page", + }, + }, + { + name: "dot only", + yaml: signingKeyLine + "referer_blocklist:\n - \".\"\n", + wantErrSubstrings: []string{keyRefererBlocklist, `"."`}, + }, + { + name: "empty entry", + yaml: signingKeyLine + "referer_blocklist:\n - \"\"\n", + wantErrSubstrings: []string{keyRefererBlocklist}, + }, + { + name: "entry not a string", + yaml: signingKeyLine + "referer_blocklist:\n - 42\n", + wantErrSubstrings: []string{keyRefererBlocklist, "42"}, + }, + { + name: "null", + yaml: signingKeyLine + "referer_blocklist:\n", + wantErrSubstrings: []string{keyRefererBlocklist, nullValueText}, + }, + }) +} + +// TestRefererBlocklistFromEnvironment checks that PIXA_REFERER_BLOCKLIST +// takes comma-separated entries, and that an entry in it that is not a host +// aborts startup naming the variable and the entry. +func TestRefererBlocklistFromEnvironment(t *testing.T) { + t.Setenv("PIXA_SIGNING_KEY", validTestSigningKey) + t.Setenv("PIXA_REFERER_BLOCKLIST", " leech.example , .hotlinker.example ") + + c, err := newFromSmartConfig(nil) + if err != nil { + t.Fatalf("valid PIXA_REFERER_BLOCKLIST should load, got error: %v", err) + } + + want := []string{"leech.example", ".hotlinker.example"} + if !slices.Equal(c.RefererBlocklist, want) { + t.Errorf("RefererBlocklist = %v, want %v", c.RefererBlocklist, want) + } + + t.Setenv("PIXA_REFERER_BLOCKLIST", "leech.example,https://hotlinker.example") + + _, err = newFromSmartConfig(nil) + wantStartupError(t, err, "PIXA_REFERER_BLOCKLIST", "https://hotlinker.example") +} diff --git a/internal/handlers/referer_blocklist_internal_test.go b/internal/handlers/referer_blocklist_internal_test.go new file mode 100644 index 0000000..d86a693 --- /dev/null +++ b/internal/handlers/referer_blocklist_internal_test.go @@ -0,0 +1,180 @@ +package handlers + +import ( + "context" + "log/slog" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "sneak.berlin/go/pixa/internal/allowlist" + "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/httpfetcher" + "sneak.berlin/go/pixa/internal/imgcache" +) + +// blockedReferer is a page on leech.example, which newRefererRoutes puts on +// referer_blocklist. +const blockedReferer = "https://leech.example/page.html" + +// countingFetcher passes each fetch on to the fetcher it holds and counts it. +type countingFetcher struct { + httpfetcher.Fetcher + + fetches atomic.Int32 +} + +// Fetch counts the fetch and passes it on. +func (f *countingFetcher) Fetch( + ctx context.Context, url string, +) (*httpfetcher.FetchResult, error) { + f.fetches.Add(1) + + return f.Fetcher.Fetch(ctx, url) +} + +// newRefererRoutes returns both image routes of a Handlers whose +// referer_blocklist is "leech.example" and ".hotlinker.example", the +// Handlers, and the fetcher the routes fetch through. The JPEG at photoPath +// exists on allowlistedHost and on signedHost. +func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher) { + t.Helper() + + fetcher := &countingFetcher{ + Fetcher: newPhotoFetcher(t, allowlistedHost, signedHost), + } + + cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ + StateDir: t.TempDir(), + CacheTTL: time.Hour, + NegativeTTL: 5 * time.Minute, + }) + if err != nil { + t.Fatalf("imgcache.NewCache() error = %v", err) + } + + svc, err := imgcache.NewService(&imgcache.ServiceConfig{ + Cache: cache, + Fetcher: fetcher, + SigningKey: testSigningKey, + Allowlist: []string{allowlistedHost}, + }) + if err != nil { + t.Fatalf("imgcache.NewService() error = %v", err) + } + + encGen, err := encurl.NewGenerator(testSigningKey) + if err != nil { + t.Fatalf("encurl.NewGenerator() error = %v", err) + } + + h := &Handlers{ + log: slog.New(slog.DiscardHandler), + imgSvc: svc, + encGen: encGen, + refererBlocklist: allowlist.New( + []string{"leech.example", ".hotlinker.example"}), + } + + r := chi.NewRouter() + r.Get("/v1/image/*", h.HandleImage()) + r.Get("/v1/e/{token}/*", h.HandleImageEnc()) + + return r, h, fetcher +} + +// getWithReferer sends a GET for target to routes with referer as its +// Referer header, or with none when referer is empty, and returns the +// response. +func getWithReferer( + t *testing.T, routes http.Handler, target, referer string, +) *httptest.ResponseRecorder { + t.Helper() + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil) + if referer != "" { + req.Header.Set("Referer", referer) + } + + rec := httptest.NewRecorder() + + routes.ServeHTTP(rec, req) + t.Logf("GET %s with Referer %q: %d", target, referer, rec.Code) + + return rec +} + +// TestRefererBlocklist verifies that both image routes refuse a request whose +// Referer names a host on referer_blocklist with 403 and the JSON error, +// without fetching from the upstream host, and serve a request with no +// Referer, one that does not parse, or one naming any other host. Hosts are +// matched as allowlist_hosts matches them. +func TestRefererBlocklist(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + referer string + want int + }{ + {"no referer", "", http.StatusOK}, + {"unlisted host", "https://unlisted.example/page.html", http.StatusOK}, + {"unparseable", "%zz", http.StatusOK}, + {"listed host", blockedReferer, http.StatusForbidden}, + {"subdomain of listed host", "https://www.leech.example/", http.StatusOK}, + {"subdomain of dot pattern", "https://www.hotlinker.example/a.html", + http.StatusForbidden}, + {"dot pattern without its dot", "https://hotlinker.example/", + http.StatusForbidden}, + {"host continuing past dot pattern", + "https://hotlinker.example.evil.example/", http.StatusOK}, + } + + for _, route := range []string{"/v1/image/", "/v1/e/"} { + for _, tc := range cases { + t.Run(route+" "+tc.name, func(t *testing.T) { + t.Parallel() + + routes, h, fetcher := newRefererRoutes(t) + + target := photoURL(allowlistedHost) + if route == "/v1/e/" { + target = encPhotoURL(t, h) + } + + rec := getWithReferer(t, routes, target, tc.referer) + + if tc.want == http.StatusOK { + requireServedPhoto(t, rec) + + return + } + + checkErrorBody(t, rec, http.StatusForbidden, "referer blocked") + + if n := fetcher.fetches.Load(); n != 0 { + t.Errorf("upstream fetched %d times, want 0", n) + } + }) + } + } +} + +// TestBlockedRefererRefusedWhenImageIsCached verifies that a request whose +// Referer is on referer_blocklist is refused even when the image it asks for +// is already cached, so the answer does not depend on the cache. +func TestBlockedRefererRefusedWhenImageIsCached(t *testing.T) { + t.Parallel() + + routes, h, _ := newRefererRoutes(t) + + for _, target := range []string{photoURL(allowlistedHost), encPhotoURL(t, h)} { + requireServedPhoto(t, getWithReferer(t, routes, target, "")) + + rec := getWithReferer(t, routes, target, blockedReferer) + checkErrorBody(t, rec, http.StatusForbidden, "referer blocked") + } +} -- 2.54.0 From c9a8b926db2c9efd4b5468df59afc4559da02ea5 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 18:09:16 +0000 Subject: [PATCH 2/7] Refuse image requests whose Referer is on referer_blocklist (closes #90) A new setting, referer_blocklist (PIXA_REFERER_BLOCKLIST), lists hosts written and matched as allowlist_hosts are, with the same matcher and the same entry check; a bad entry aborts startup naming the setting and the entry. Both image routes check the Referer first and answer 403 with the JSON error, so a blocked request fetches nothing and is refused whether or not the image is cached. No Referer, or one that does not parse as a URL with a host, is served; README.md and config.example.yml say this makes the list easy to get around. The CIDR-list entry reader is renamed listEntries now that host lists use it too. Model: opus-5-5 --- README.md | 26 ++++++++--- TODO.md | 13 +++++- configs/config.example.yml | 10 +++++ internal/config/config.go | 85 ++++++++++++++++++++++++++--------- internal/handlers/handlers.go | 16 ++++--- internal/handlers/image.go | 21 +++++++++ internal/handlers/imageenc.go | 4 ++ 7 files changed, 141 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 0906560..dda724b 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,8 @@ everything in this list without further settings. The reverse proxy must: Routes); - pass the `Host`, `Origin` and `Referer` headers on unchanged, as pixa refuses a form from those pages unless `Origin` or `Referer` names the host in `Host`, - and builds encrypted URLs from `Host`; + builds encrypted URLs from `Host`, and checks `Referer` against + `referer_blocklist`; - set `X-Forwarded-For` to the client's address, with `trusted_proxies` set to the address pixa sees the proxy's requests come from, so the login limit counts each client by its own address (see `trusted_proxies` under @@ -175,11 +176,12 @@ path under `/v1/` answers 200, in maintenance mode too. allowlisted (see Source Hosts). Answers: 200; 304 when `If-None-Match` matches the image's `ETag`; 400 for a URL or parameter that is not valid; 401 for a missing or wrong signature, a missing `exp` or an `exp` in the past; 403 when - the upstream host, or a host it redirects to, is `localhost`, ends in - `.localhost` or `.local`, or has an address in a blocked network (see - `blocked_networks`); 502 when the upstream answered with an error status, and - for 5 minutes after that for the same source URL; 503 when pixa is busy or in - maintenance mode; 500 for any other failure. + the request's `Referer` names a host in `referer_blocklist`, checked before + anything else; 403 when the upstream host, or a host it redirects to, is + `localhost`, ends in `.localhost` or `.local`, or has an address in a blocked + network (see `blocked_networks`); 502 when the upstream answered with an error + status, and for 5 minutes after that for the same source URL; 503 when pixa is + busy or in maintenance mode; 500 for any other failure. - `GET` or `HEAD` `/v1/e//` — an image through an encrypted URL (see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when `If-None-Match` matches the image's `ETag`; 400 for a token that does not @@ -428,6 +430,7 @@ tell. With no file, pixa uses the environment and the defaults. | `PIXA_DB_URL` | `db_url` | SQLite database URL; default `state.sqlite3` in the state directory | | `PIXA_CACHE_MAX_BYTES` | `cache_max_bytes` | Disk cache limit in bytes; `0` disables it; default 75% of (free + cached) | | `PIXA_ALLOWLIST_HOSTS` | `allowlist_hosts` | Upstream hosts served without a signature | +| `PIXA_REFERER_BLOCKLIST` | `referer_blocklist` | Hosts whose pages the image routes refuse with 403, by `Referer` | | `PIXA_BLOCKED_NETWORKS` | `blocked_networks` | CIDR ranges never fetched from, on top of the built-in ones | | `PIXA_TRUSTED_PROXIES` | `trusted_proxies` | CIDR ranges of proxies whose `X-Forwarded-For` is believed; default RFC 1918 | | `PIXA_ALLOW_HTTP` | `allow_http` | Allow plain-HTTP upstreams, for testing only; default `false` | @@ -456,6 +459,17 @@ Key settings in more detail: that has no leading zero and is not the scheme's default. Any other value, including another scheme such as a browser extension's, aborts startup - `allowlist_hosts` — list of allowed upstream hosts +- `referer_blocklist` — list of hosts whose pages may not show pixa's images, to + stop other sites hotlinking them. Entries are written and matched as for + `allowlist_hosts` (see Allowlist patterns); one that is not a bare host aborts + startup. A request to `/v1/image/` or `/v1/e/` whose `Referer` header names a + listed host is refused with 403 before anything else is done for it, so it + fetches nothing from the upstream host, and it is refused even when the image + is cached. A request with no `Referer`, or one that does not parse as a URL + with a host, is served, as many clients send none. So this is easily got + around: a site whose pages send no `Referer` (for example with + `Referrer-Policy: no-referrer`) is not stopped. It does not apply to the login + and generator pages. Default: empty - `blocked_networks` — list of CIDR ranges to refuse for SSRF protection, added to the always-enforced built-in ranges (loopback, private, link-local, CGNAT, benchmark, NAT64, and the like); an invalid CIDR diff --git a/TODO.md b/TODO.md index 18d33b4..8840ea1 100644 --- a/TODO.md +++ b/TODO.md @@ -27,7 +27,7 @@ The disk cache is now size-bounded with LRU eviction # Next Step -P2: security: referer blacklist +P2: security: per-IP rate limiting on the image routes # Completed Steps @@ -47,6 +47,16 @@ P2: security: referer blacklist for it, and the default `db_url` turns on WAL mode with `_pragma=journal_mode(WAL)`. The old default's `_journal_mode=WAL` is not a parameter the driver reads, so the database was never in WAL mode. +- 2026-10-04 referer blocklist (closes #90): `referer_blocklist` + (`PIXA_REFERER_BLOCKLIST`) lists hosts, written and matched as for + `allowlist_hosts` with the same matcher; an entry that is not a bare host + aborts startup naming the setting and the entry. Both image routes refuse a + request whose `Referer` names a listed host with 403 and a JSON error before + anything else is done for it, so it fetches nothing and is refused whether or + not the image is cached. A request with no `Referer`, or one that does not + parse as a URL with a host, is served, so the list is easily got around; + `README.md` and `config.example.yml` say so. It does not apply to the login + and generator pages. - 2026-10-04 `TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup` only passes through a periodic pass (closes #189): it slept for three eviction intervals before writing its file, and a startup pass still running @@ -561,7 +571,6 @@ P2: security: referer blacklist # Future Steps - P2: security - - per-IP rate limiting on the image routes - per-origin rate limiting - P2: HTTP response handling - Last-Modified headers diff --git a/configs/config.example.yml b/configs/config.example.yml index 098d936..3bcef54 100644 --- a/configs/config.example.yml +++ b/configs/config.example.yml @@ -53,6 +53,16 @@ allowlist_hosts: - github.com - user-images.githubusercontent.com +# Hosts whose pages may not show pixa's images, written as for +# allowlist_hosts. A request to /v1/image/ or /v1/e/ whose Referer header +# names one of them is answered 403 before anything is fetched, even when +# the image is cached. A request with no Referer, or one that does not +# parse, is served, so a site whose pages send no Referer is not stopped. +# An entry that is not a host aborts startup. (default: none) +# referer_blocklist: +# - leech.example +# - .hotlinker.example + # Additional CIDR ranges to refuse when fetching upstream, extending the # SSRF protection. These are added to the always-enforced built-in ranges # (loopback, RFC 1918 private, link-local, CGNAT, benchmark, NAT64, and diff --git a/internal/config/config.go b/internal/config/config.go index 98addb0..a68f888 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -48,6 +48,7 @@ const ( keyMetricsPassword = "metrics.password" keySigningKey = "signing_key" keyAllowlistHosts = "allowlist_hosts" + keyRefererBlocklist = "referer_blocklist" keyAllowHTTP = "allow_http" keyUpstreamConnectionsPerHost = "upstream_connections_per_host" keyUpstreamConnections = "upstream_connections" @@ -130,6 +131,10 @@ type Config struct { AllowHTTP bool // Allow non-TLS upstream (testing only) UpstreamConnectionsPerHost int // Max concurrent connections per upstream host + // RefererBlocklist holds host patterns, matched as AllowlistHosts is: the + // image routes refuse a request whose Referer names a matching host. + RefererBlocklist []string + // UpstreamConnections is the most concurrent connections to all // upstream hosts together, on top of the per-host limit. // MaxConcurrentProcessing is the most images processed at once. @@ -261,6 +266,11 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { return nil, err } + refererBlocklist, err := parseHostList(sc, keyRefererBlocklist) + if err != nil { + return nil, err + } + // parseCIDRList returns a nil slice only when the key is absent; an // explicitly empty list ([]) comes back non-nil and empty. An omitted // key takes the RFC 1918 default, while an explicit empty list is left @@ -298,9 +308,10 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { keyAccessControlAllowOrigin, DefaultAccessControlAllowOrigin), DownstreamTimeout: loader.durationVal( keyDownstreamTimeout, DefaultDownstreamTimeout), - CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0), - BlockedNetworks: blockedNetworks, - TrustedProxies: trustedProxies, + CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0), + BlockedNetworks: blockedNetworks, + TrustedProxies: trustedProxies, + RefererBlocklist: refererBlocklist, } // The default for an omitted cache_max_bytes is worked out when @@ -420,7 +431,8 @@ func isKnownConfigKey(key string) bool { keyUpstreamConnectionsPerHost, keyUpstreamConnections, keyMaxConcurrentProcessing, keyCacheMaxBytes, keyBlockedNetworks, keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout, - keyUpstreamMaxResponseSize, keyDownstreamTimeout, "env": + keyUpstreamMaxResponseSize, keyDownstreamTimeout, keyRefererBlocklist, + "env": return true } @@ -443,6 +455,7 @@ func envVarNames() map[string]string { keyMetricsPassword: "PIXA_METRICS_PASSWORD", keySigningKey: "PIXA_SIGNING_KEY", keyAllowlistHosts: "PIXA_ALLOWLIST_HOSTS", + keyRefererBlocklist: "PIXA_REFERER_BLOCKLIST", keyAllowHTTP: "PIXA_ALLOW_HTTP", keyUpstreamConnectionsPerHost: "PIXA_UPSTREAM_CONNECTIONS_PER_HOST", keyUpstreamConnections: "PIXA_UPSTREAM_CONNECTIONS", @@ -612,7 +625,7 @@ func (c *Config) validate() error { } for _, host := range c.AllowlistHosts { - err := validateAllowlistHost(host) + err := validateHostPattern(keyAllowlistHosts, host) if err != nil { return err } @@ -735,22 +748,22 @@ func (c *Config) validateConcurrencyLimits() error { return nil } -// validateAllowlistHost checks that an allowlist_hosts entry is a bare -// hostname, optionally with a leading dot for suffix matching. URLs, -// paths, and whitespace indicate a misconfigured entry. An entry with -// no hostname labels (such as ".") is rejected: the allowlist matcher -// treats a leading dot as a suffix pattern, so a bare "." would match -// any upstream host written in FQDN trailing-dot form and effectively -// disable URL signing. -func validateAllowlistHost(host string) error { +// validateHostPattern checks that an entry of the named key, allowlist_hosts +// or referer_blocklist, is a bare hostname, optionally with a leading dot for +// suffix matching. URLs, paths, and whitespace indicate a misconfigured entry. +// An entry with no hostname labels (such as ".") is rejected: the allowlist +// matcher treats a leading dot as a suffix pattern, so a bare "." would match +// any host written in FQDN trailing-dot form, and in allowlist_hosts +// effectively disable URL signing. +func validateHostPattern(key, host string) error { if strings.Contains(host, "://") || strings.ContainsAny(host, "/ \t") { return fmt.Errorf("%s: entry %q %w", - settingName(keyAllowlistHosts), host, errNotBareHostname) + settingName(key), host, errNotBareHostname) } if strings.Trim(host, ".") == "" { return fmt.Errorf("%s: entry %q %w", - settingName(keyAllowlistHosts), host, errNoHostnameLabels) + settingName(key), host, errNoHostnameLabels) } return nil @@ -1180,7 +1193,7 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) { return nil, errNullConfigValue(key) } - entries, err := cidrListEntries(raw, key) + entries, err := listEntries(raw, key) if err != nil { return nil, err } @@ -1200,11 +1213,41 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) { return prefixes, nil } -// cidrListEntries extracts the raw entries of the named CIDR-list key as -// trimmed, non-empty strings, from either a YAML list of strings or a -// comma-separated string; an empty string is an empty list, as for -// allowlist_hosts. Any other shape is a configuration error. -func cidrListEntries(raw any, key string) ([]string, error) { +// parseHostList parses the value of the named config key into host patterns, +// or returns nil if the key is omitted. It accepts a YAML list of strings or a +// comma-separated string. An explicitly null value, a wrong type, an empty +// entry, a non-string entry, or an entry validateHostPattern rejects aborts +// startup naming the key and the offending value. +func parseHostList(sc *smartconfig.Config, key string) ([]string, error) { + raw, ok := lookupValue(sc, key) + if !ok { + return nil, nil + } + + if raw == nil { + return nil, errNullConfigValue(key) + } + + entries, err := listEntries(raw, key) + if err != nil { + return nil, err + } + + for _, entry := range entries { + err := validateHostPattern(key, entry) + if err != nil { + return nil, err + } + } + + return entries, nil +} + +// listEntries extracts the raw entries of the named list key as trimmed, +// non-empty strings, from either a YAML list of strings or a comma-separated +// string; an empty string is an empty list, as for allowlist_hosts. Any other +// shape is a configuration error. +func listEntries(raw any, key string) ([]string, error) { switch val := raw.(type) { case []any: entries := make([]string, 0, len(val)) diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 489e3c3..a65f3b5 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -9,6 +9,7 @@ import ( "time" "go.uber.org/fx" + "sneak.berlin/go/pixa/internal/allowlist" "sneak.berlin/go/pixa/internal/config" "sneak.berlin/go/pixa/internal/database" "sneak.berlin/go/pixa/internal/encurl" @@ -40,6 +41,10 @@ type Handlers struct { sessMgr *session.Manager encGen *encurl.Generator csrfProtect func(http.Handler) http.Handler + + // refererBlocklist matches the hosts of referer_blocklist; its IsAllowed + // reports whether a URL's host is on that list. + refererBlocklist *allowlist.HostAllowList } // New creates a new Handlers instance. @@ -50,11 +55,12 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) { } s := &Handlers{ - log: params.Logger.Get(), - hc: params.Healthcheck, - db: params.Database, - config: params.Config, - csrfProtect: csrfProtect, + log: params.Logger.Get(), + hc: params.Healthcheck, + db: params.Database, + config: params.Config, + csrfProtect: csrfProtect, + refererBlocklist: allowlist.New(params.Config.RefererBlocklist), } lc.Append(fx.Hook{ diff --git a/internal/handlers/image.go b/internal/handlers/image.go index 1a46a58..e099799 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -21,6 +21,10 @@ import ( // /v1/image///x. func (s *Handlers) HandleImage() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + if s.refuseBlockedReferer(w, r) { + return + } + req, ok := s.parseImageRequest(w, r) if !ok { return @@ -248,6 +252,23 @@ func cacheControl(expires time.Time) string { return fmt.Sprintf("public, max-age=%d, immutable", int64(maxAge/time.Second)) } +// refuseBlockedReferer answers 403 with a JSON error when the request's Referer +// names a host on referer_blocklist, and reports whether it answered. A request +// with no Referer, or one that does not parse as a URL with a host, is not +// refused. +func (s *Handlers) refuseBlockedReferer( + w http.ResponseWriter, r *http.Request, +) bool { + referer, err := url.Parse(r.Referer()) + if err != nil || !s.refererBlocklist.IsAllowed(referer) { + return false + } + + s.respondError(w, "referer blocked", http.StatusForbidden) + + return true +} + // notModified sets the ETag header to etag and, when the request's // If-None-Match is that ETag, answers 304 Not Modified. It reports whether it // answered. An empty etag sets no header and never answers. diff --git a/internal/handlers/imageenc.go b/internal/handlers/imageenc.go index dadb6bd..a9c672f 100644 --- a/internal/handlers/imageenc.go +++ b/internal/handlers/imageenc.go @@ -22,6 +22,10 @@ import ( // browsers identify the content type. func (s *Handlers) HandleImageEnc() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + if s.refuseBlockedReferer(w, r) { + return + } + ctx := r.Context() start := time.Now() -- 2.54.0 From 5ae17b6361909cc822211c58f61cde6b8acf8526 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 18:19:59 +0000 Subject: [PATCH 3/7] Read referer_blocklist through strictLoader and fix test lint newFromSmartConfig went over the line limit, so referer_blocklist is now read through strictLoader, like the other typed settings, instead of its own parse step. The tests stop repeating string literals that goconst counts: the environment test uses other hosts, two case names change, and the handler test names each image route's URL instead of its path. Model: opus-5-5 --- internal/config/config.go | 21 ++++++++++++------- internal/config/env_internal_test.go | 4 ++-- .../config/referer_blocklist_internal_test.go | 6 +++--- .../referer_blocklist_internal_test.go | 21 ++++++++++++------- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index a68f888..be6a7c2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -266,11 +266,6 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { return nil, err } - refererBlocklist, err := parseHostList(sc, keyRefererBlocklist) - if err != nil { - return nil, err - } - // parseCIDRList returns a nil slice only when the key is absent; an // explicitly empty list ([]) comes back non-nil and empty. An omitted // key takes the RFC 1918 default, while an explicit empty list is left @@ -280,7 +275,6 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { } loader := &strictLoader{sc: sc} - c := &Config{ Debug: loader.boolVal(keyDebug, false), MaintenanceMode: loader.boolVal(keyMaintenanceMode, false), @@ -311,7 +305,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0), BlockedNetworks: blockedNetworks, TrustedProxies: trustedProxies, - RefererBlocklist: refererBlocklist, + RefererBlocklist: loader.hostListVal(keyRefererBlocklist), } // The default for an omitted cache_max_bytes is worked out when @@ -896,6 +890,19 @@ func (l *strictLoader) boolVal(key string, defaultVal bool) bool { return val } +func (l *strictLoader) hostListVal(key string) []string { + if l.err != nil { + return nil + } + + val, err := parseHostList(l.sc, key) + if err != nil { + l.err = err + } + + return val +} + // getString returns the string value for key, or defaultVal if the key // is omitted. A present value that is not a string, or is explicitly // null, is an error. diff --git a/internal/config/env_internal_test.go b/internal/config/env_internal_test.go index bedbe06..6eb79d9 100644 --- a/internal/config/env_internal_test.go +++ b/internal/config/env_internal_test.go @@ -65,7 +65,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) { t.Setenv("PIXA_METRICS_PASSWORD", "metricspass") t.Setenv("PIXA_SIGNING_KEY", validTestSigningKey) t.Setenv("PIXA_ALLOWLIST_HOSTS", "s3.sneak.cloud,.example.com") - t.Setenv("PIXA_REFERER_BLOCKLIST", "leech.example,.hotlinker.example") + t.Setenv("PIXA_REFERER_BLOCKLIST", "hotlinker.example,.leech.example") t.Setenv("PIXA_ALLOW_HTTP", "true") t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5") t.Setenv("PIXA_UPSTREAM_CONNECTIONS", "10") @@ -94,7 +94,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) { MetricsPassword: "metricspass", SigningKey: validTestSigningKey, AllowlistHosts: []string{testHostS3, ".example.com"}, - RefererBlocklist: []string{"leech.example", ".hotlinker.example"}, + RefererBlocklist: []string{"hotlinker.example", ".leech.example"}, AllowHTTP: true, UpstreamConnectionsPerHost: 5, UpstreamConnections: 10, diff --git a/internal/config/referer_blocklist_internal_test.go b/internal/config/referer_blocklist_internal_test.go index 6128132..ea81914 100644 --- a/internal/config/referer_blocklist_internal_test.go +++ b/internal/config/referer_blocklist_internal_test.go @@ -47,14 +47,14 @@ func TestRefererBlocklistInvalidAbortsStartup(t *testing.T) { runAbortCases(t, []abortCase{ { - name: "url", + name: "entry with a scheme", yaml: signingKeyLine + "referer_blocklist:\n - https://leech.example\n", wantErrSubstrings: []string{ keyRefererBlocklist, "https://leech.example", }, }, { - name: "path", + name: "entry with a path", yaml: signingKeyLine + "referer_blocklist:\n - leech.example/page\n", wantErrSubstrings: []string{ keyRefererBlocklist, "leech.example/page", @@ -76,7 +76,7 @@ func TestRefererBlocklistInvalidAbortsStartup(t *testing.T) { wantErrSubstrings: []string{keyRefererBlocklist, "42"}, }, { - name: "null", + name: "null value", yaml: signingKeyLine + "referer_blocklist:\n", wantErrSubstrings: []string{keyRefererBlocklist, nullValueText}, }, diff --git a/internal/handlers/referer_blocklist_internal_test.go b/internal/handlers/referer_blocklist_internal_test.go index d86a693..9235eb1 100644 --- a/internal/handlers/referer_blocklist_internal_test.go +++ b/internal/handlers/referer_blocklist_internal_test.go @@ -133,19 +133,24 @@ func TestRefererBlocklist(t *testing.T) { "https://hotlinker.example.evil.example/", http.StatusOK}, } - for _, route := range []string{"/v1/image/", "/v1/e/"} { + // The photo's URL on each image route. + photoURLs := map[string]func(t *testing.T, h *Handlers) string{ + "plain URL": func(t *testing.T, _ *Handlers) string { + t.Helper() + + return photoURL(allowlistedHost) + }, + "encrypted URL": encPhotoURL, + } + + for urlName, photoURLFor := range photoURLs { for _, tc := range cases { - t.Run(route+" "+tc.name, func(t *testing.T) { + t.Run(urlName+", "+tc.name, func(t *testing.T) { t.Parallel() routes, h, fetcher := newRefererRoutes(t) - target := photoURL(allowlistedHost) - if route == "/v1/e/" { - target = encPhotoURL(t, h) - } - - rec := getWithReferer(t, routes, target, tc.referer) + rec := getWithReferer(t, routes, photoURLFor(t, h), tc.referer) if tc.want == http.StatusOK { requireServedPhoto(t, rec) -- 2.54.0 From e1737f497cc921955c0677e7260d934e871ff250 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 19:01:23 +0000 Subject: [PATCH 4/7] Add failing tests for host entries that can never match A `*.` entry, an entry with a port and one with two leading dots must abort startup for referer_blocklist, and the first two for allowlist_hosts; IPv4 and IPv6 address entries must still load. Model: opus-5-5 --- .../config/config_validation_internal_test.go | 14 +++++++ .../config/referer_blocklist_internal_test.go | 40 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/internal/config/config_validation_internal_test.go b/internal/config/config_validation_internal_test.go index d3a3736..a222c03 100644 --- a/internal/config/config_validation_internal_test.go +++ b/internal/config/config_validation_internal_test.go @@ -318,6 +318,20 @@ func invalidHostAndCredentialCases() []abortCase { keyAllowlistHosts, "example.com/images", }, }, + { + name: "allowlist host with wildcard", + yaml: signingKeyLine + "allowlist_hosts:\n - \"*.example.com\"\n", + wantErrSubstrings: []string{ + keyAllowlistHosts, "*.example.com", + }, + }, + { + name: "allowlist host with port", + yaml: signingKeyLine + "allowlist_hosts:\n - example.com:8443\n", + wantErrSubstrings: []string{ + keyAllowlistHosts, "example.com:8443", + }, + }, { name: "allowlist host with whitespace", yaml: signingKeyLine + "allowlist_hosts:\n - \"exa mple.com\"\n", diff --git a/internal/config/referer_blocklist_internal_test.go b/internal/config/referer_blocklist_internal_test.go index ea81914..a198272 100644 --- a/internal/config/referer_blocklist_internal_test.go +++ b/internal/config/referer_blocklist_internal_test.go @@ -24,6 +24,25 @@ func TestRefererBlocklistParsed(t *testing.T) { } } +// TestRefererBlocklistAcceptsIPAddresses checks that IPv4 and IPv6 addresses, +// the IPv6 one written without brackets, are accepted as entries. +func TestRefererBlocklistAcceptsIPAddresses(t *testing.T) { + t.Parallel() + + c, err := configFromYAML(t, signingKeyLine+`referer_blocklist: + - 192.0.2.7 + - "2001:db8::7" +`) + if err != nil { + t.Fatalf("IP address entries should load, got error: %v", err) + } + + want := []string{"192.0.2.7", "2001:db8::7"} + if !slices.Equal(c.RefererBlocklist, want) { + t.Errorf("RefererBlocklist = %v, want %v", c.RefererBlocklist, want) + } +} + // TestRefererBlocklistOmittedIsEmpty checks that an omitted key blocks no // referer. func TestRefererBlocklistOmittedIsEmpty(t *testing.T) { @@ -60,6 +79,27 @@ func TestRefererBlocklistInvalidAbortsStartup(t *testing.T) { keyRefererBlocklist, "leech.example/page", }, }, + { + name: "wildcard entry", + yaml: signingKeyLine + "referer_blocklist:\n - \"*.leech.example\"\n", + wantErrSubstrings: []string{ + keyRefererBlocklist, "*.leech.example", + }, + }, + { + name: "entry with a port", + yaml: signingKeyLine + "referer_blocklist:\n - leech.example:8080\n", + wantErrSubstrings: []string{ + keyRefererBlocklist, "leech.example:8080", + }, + }, + { + name: "two leading dots", + yaml: signingKeyLine + "referer_blocklist:\n - ..leech.example\n", + wantErrSubstrings: []string{ + keyRefererBlocklist, "..leech.example", + }, + }, { name: "dot only", yaml: signingKeyLine + "referer_blocklist:\n - \".\"\n", -- 2.54.0 From a2effe1e27229e292e250cdc91eb3cbc6835d70e Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 19:04:34 +0000 Subject: [PATCH 5/7] Refuse host entries that are not a host name or an IP address An entry of allowlist_hosts or referer_blocklist that is neither a host name (letters, digits, hyphens and dots, with at most one leading dot) nor an IP address now aborts startup naming the setting and the entry, so a `*.` wildcard or a port no longer loads and silently matches nothing. README.md, configs/config.example.yml and the TODO.md entry now say the Referer check comes before the signature, the cache and the upstream fetch, since maintenance mode answers first; the example config says the list does not cover the login and generator pages. Model: opus-5-5 --- README.md | 27 +++++++++++++++---------- TODO.md | 22 +++++++++++---------- configs/config.example.yml | 4 +++- internal/config/config.go | 40 ++++++++++++++++++-------------------- 4 files changed, 51 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index dda724b..1f23d26 100644 --- a/README.md +++ b/README.md @@ -177,11 +177,12 @@ path under `/v1/` answers 200, in maintenance mode too. the image's `ETag`; 400 for a URL or parameter that is not valid; 401 for a missing or wrong signature, a missing `exp` or an `exp` in the past; 403 when the request's `Referer` names a host in `referer_blocklist`, checked before - anything else; 403 when the upstream host, or a host it redirects to, is - `localhost`, ends in `.localhost` or `.local`, or has an address in a blocked - network (see `blocked_networks`); 502 when the upstream answered with an error - status, and for 5 minutes after that for the same source URL; 503 when pixa is - busy or in maintenance mode; 500 for any other failure. + the signature, the cache and the upstream fetch; 403 when the upstream host, + or a host it redirects to, is `localhost`, ends in `.localhost` or `.local`, + or has an address in a blocked network (see `blocked_networks`); 502 when the + upstream answered with an error status, and for 5 minutes after that for the + same source URL; 503 when pixa is busy or in maintenance mode; 500 for any + other failure. - `GET` or `HEAD` `/v1/e//` — an image through an encrypted URL (see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when `If-None-Match` matches the image's `ETag`; 400 for a token that does not @@ -394,6 +395,11 @@ and the URL is - **Suffix match**: `.example.com` — matches `cdn.example.com`, `images.example.com`, and `example.com` +An IP address is matched exactly; write an IPv6 address without brackets. An +entry that is neither a host name (letters, digits, hyphens and dots, with at +most one leading dot) nor an IP address, such as one with a port or a `*.` +wildcard, aborts startup. + ### Configuration Every setting can be given as an environment variable, in a YAML config @@ -461,11 +467,12 @@ Key settings in more detail: - `allowlist_hosts` — list of allowed upstream hosts - `referer_blocklist` — list of hosts whose pages may not show pixa's images, to stop other sites hotlinking them. Entries are written and matched as for - `allowlist_hosts` (see Allowlist patterns); one that is not a bare host aborts - startup. A request to `/v1/image/` or `/v1/e/` whose `Referer` header names a - listed host is refused with 403 before anything else is done for it, so it - fetches nothing from the upstream host, and it is refused even when the image - is cached. A request with no `Referer`, or one that does not parse as a URL + `allowlist_hosts` (see Allowlist patterns), and an entry that is neither a + host name nor an IP address aborts startup. A request to `/v1/image/` or + `/v1/e/` whose `Referer` header names a listed host is refused with 403 before + its signature or token is checked and before the cache or the upstream host is + used, so it fetches nothing, and it is refused even when the image is cached. + A request with no `Referer`, or one that does not parse as a URL with a host, is served, as many clients send none. So this is easily got around: a site whose pages send no `Referer` (for example with `Referrer-Policy: no-referrer`) is not stopped. It does not apply to the login diff --git a/TODO.md b/TODO.md index 8840ea1..5eb1d53 100644 --- a/TODO.md +++ b/TODO.md @@ -31,6 +31,18 @@ P2: security: per-IP rate limiting on the image routes # Completed Steps +- 2026-10-04 referer blocklist (closes #90): `referer_blocklist` + (`PIXA_REFERER_BLOCKLIST`) lists hosts, written and matched as for + `allowlist_hosts` with the same matcher; an entry of either list that is + neither a host name (letters, digits, hyphens and dots, with at most one + leading dot) nor an IP address, such as one with a port or a `*.` wildcard, + aborts startup naming the setting and the entry. Both image routes refuse a + request whose `Referer` names a listed host with 403 and a JSON error before + the signature, the cache and the upstream fetch, so it fetches nothing and is + refused whether or not the image is cached. A request with no `Referer`, or + one that does not parse as a URL with a host, is served, so the list is easily + got around; `README.md` and `configs/config.example.yml` say so. It does not + apply to the login and generator pages. - 2026-10-04 fewer files in the repository root (closes #97): `config.example.yml` moved unchanged to `configs/config.example.yml`, and `README.md`, the comments in `internal/config/config.go` and the startup error @@ -47,16 +59,6 @@ P2: security: per-IP rate limiting on the image routes for it, and the default `db_url` turns on WAL mode with `_pragma=journal_mode(WAL)`. The old default's `_journal_mode=WAL` is not a parameter the driver reads, so the database was never in WAL mode. -- 2026-10-04 referer blocklist (closes #90): `referer_blocklist` - (`PIXA_REFERER_BLOCKLIST`) lists hosts, written and matched as for - `allowlist_hosts` with the same matcher; an entry that is not a bare host - aborts startup naming the setting and the entry. Both image routes refuse a - request whose `Referer` names a listed host with 403 and a JSON error before - anything else is done for it, so it fetches nothing and is refused whether or - not the image is cached. A request with no `Referer`, or one that does not - parse as a URL with a host, is served, so the list is easily got around; - `README.md` and `config.example.yml` say so. It does not apply to the login - and generator pages. - 2026-10-04 `TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup` only passes through a periodic pass (closes #189): it slept for three eviction intervals before writing its file, and a startup pass still running diff --git a/configs/config.example.yml b/configs/config.example.yml index 3bcef54..cede448 100644 --- a/configs/config.example.yml +++ b/configs/config.example.yml @@ -46,6 +46,8 @@ signing_key: "CHANGE_ME_generate_with_openssl_rand_base64_32" # Hosts that don't require signatures (default: none) # Use "." prefix for wildcard subdomain matching (e.g., ".example.com" matches "cdn.example.com") +# An entry that is neither a host name nor an IP address (IPv6 without +# brackets), such as one with a port or a "*." wildcard, aborts startup. allowlist_hosts: - s3.sneak.cloud - static.sneak.cloud @@ -58,7 +60,7 @@ allowlist_hosts: # names one of them is answered 403 before anything is fetched, even when # the image is cached. A request with no Referer, or one that does not # parse, is served, so a site whose pages send no Referer is not stopped. -# An entry that is not a host aborts startup. (default: none) +# The login and generator pages are not covered. (default: none) # referer_blocklist: # - leech.example # - .hotlinker.example diff --git a/internal/config/config.go b/internal/config/config.go index be6a7c2..0a5991e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,6 +11,7 @@ import ( "net/url" "os" "path/filepath" + "regexp" "runtime" "sort" "strconv" @@ -98,12 +99,11 @@ var ( "value is null; omit the key entirely to use the default") errValuesNull = errors.New( "value is null; omit a key entirely to use its default") - errNotBareHostname = errors.New( - "must be a bare hostname without scheme, path, or whitespace") - errNoHostnameLabels = errors.New("contains no hostname labels") - errNotADuration = errors.New("not a duration such as 30s or 2m") - errMustBePositive = errors.New("must be positive") - errNotAnOrigin = errors.New( + errNotAHost = errors.New("must be a host name such as " + + "cdn.example.com or .example.com, or an IP address") + errNotADuration = errors.New("not a duration such as 30s or 2m") + errMustBePositive = errors.New("must be positive") + errNotAnOrigin = errors.New( `not "*" or an origin such as https://example.com`) ) @@ -742,25 +742,23 @@ func (c *Config) validateConcurrencyLimits() error { return nil } +// hostNamePattern matches a host name: letters, digits, hyphens and dots, +// optionally after one leading dot. +var hostNamePattern = regexp.MustCompile(`^\.?[A-Za-z0-9-][A-Za-z0-9.-]*$`) + // validateHostPattern checks that an entry of the named key, allowlist_hosts -// or referer_blocklist, is a bare hostname, optionally with a leading dot for -// suffix matching. URLs, paths, and whitespace indicate a misconfigured entry. -// An entry with no hostname labels (such as ".") is rejected: the allowlist -// matcher treats a leading dot as a suffix pattern, so a bare "." would match -// any host written in FQDN trailing-dot form, and in allowlist_hosts -// effectively disable URL signing. +// or referer_blocklist, is an IP address or a host name, the host name +// optionally with one leading dot for suffix matching. Anything else, such as +// a URL, a port or a "*." wildcard, can never match a host, so it is refused. +// So is "." alone: the allowlist matcher would match it against any host +// written with a trailing dot, which in allowlist_hosts disables URL signing. func validateHostPattern(key, host string) error { - if strings.Contains(host, "://") || strings.ContainsAny(host, "/ \t") { - return fmt.Errorf("%s: entry %q %w", - settingName(key), host, errNotBareHostname) + _, err := netip.ParseAddr(host) + if err == nil || hostNamePattern.MatchString(host) { + return nil } - if strings.Trim(host, ".") == "" { - return fmt.Errorf("%s: entry %q %w", - settingName(key), host, errNoHostnameLabels) - } - - return nil + return fmt.Errorf("%s: entry %q %w", settingName(key), host, errNotAHost) } // loadConfigFile loads configuration from the PIXA_CONFIG_PATH env var -- 2.54.0 From 29ff9e4463bb904526d65ecb0ef7b2cfb678931c Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 19:46:10 +0000 Subject: [PATCH 6/7] Add failing tests for host names with an underscore Both allowlist_hosts and referer_blocklist should accept a host name with an underscore, such as my_bucket.example.com, since pixa fetches from such hosts and pages are served from them. Model: opus-5-5 --- .../config/config_validation_internal_test.go | 20 +++++++++++++++++++ .../config/referer_blocklist_internal_test.go | 19 ++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/internal/config/config_validation_internal_test.go b/internal/config/config_validation_internal_test.go index a222c03..abfdee7 100644 --- a/internal/config/config_validation_internal_test.go +++ b/internal/config/config_validation_internal_test.go @@ -5,6 +5,7 @@ import ( "log/slog" "os" "path/filepath" + "slices" "strings" "testing" "time" @@ -216,6 +217,25 @@ func TestCommaSeparatedAllowlistStillSupported(t *testing.T) { } } +// TestAllowlistHostsAcceptsUnderscore checks that an upstream host name with +// an underscore, which pixa can fetch from, is accepted as an entry. +func TestAllowlistHostsAcceptsUnderscore(t *testing.T) { + t.Parallel() + + c, err := configFromYAML(t, signingKeyLine+`allowlist_hosts: + - my_bucket.example.com + - .my_bucket.example.org +`) + if err != nil { + t.Fatalf("host names with an underscore should load, got error: %v", err) + } + + want := []string{"my_bucket.example.com", ".my_bucket.example.org"} + if !slices.Equal(c.AllowlistHosts, want) { + t.Errorf("AllowlistHosts = %v, want %v", c.AllowlistHosts, want) + } +} + // runAbortCases asserts that each case's config aborts startup with an // error message mentioning every expected substring. func runAbortCases(t *testing.T, cases []abortCase) { diff --git a/internal/config/referer_blocklist_internal_test.go b/internal/config/referer_blocklist_internal_test.go index a198272..43d5753 100644 --- a/internal/config/referer_blocklist_internal_test.go +++ b/internal/config/referer_blocklist_internal_test.go @@ -43,6 +43,25 @@ func TestRefererBlocklistAcceptsIPAddresses(t *testing.T) { } } +// TestRefererBlocklistAcceptsUnderscore checks that a host name with an +// underscore, which a page can be served from, is accepted as an entry. +func TestRefererBlocklistAcceptsUnderscore(t *testing.T) { + t.Parallel() + + c, err := configFromYAML(t, signingKeyLine+`referer_blocklist: + - my_site.leech.example + - .my_site.hotlinker.example +`) + if err != nil { + t.Fatalf("host names with an underscore should load, got error: %v", err) + } + + want := []string{"my_site.leech.example", ".my_site.hotlinker.example"} + if !slices.Equal(c.RefererBlocklist, want) { + t.Errorf("RefererBlocklist = %v, want %v", c.RefererBlocklist, want) + } +} + // TestRefererBlocklistOmittedIsEmpty checks that an omitted key blocks no // referer. func TestRefererBlocklistOmittedIsEmpty(t *testing.T) { -- 2.54.0 From 6259832fb30bdd39e69176fd316ae479574aee61 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 19:46:10 +0000 Subject: [PATCH 7/7] Allow underscores in allowlist_hosts and referer_blocklist host names The shared entry check refused host names with an underscore, though both lists can match them. The check's comment now says that what it refuses can never match a host name that resolves. Model: opus-5-5 --- README.md | 6 +++--- TODO.md | 18 +++++++++--------- internal/config/config.go | 9 +++++---- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 1f23d26..8597c53 100644 --- a/README.md +++ b/README.md @@ -396,9 +396,9 @@ and the URL is `images.example.com`, and `example.com` An IP address is matched exactly; write an IPv6 address without brackets. An -entry that is neither a host name (letters, digits, hyphens and dots, with at -most one leading dot) nor an IP address, such as one with a port or a `*.` -wildcard, aborts startup. +entry that is neither a host name (letters, digits, hyphens, underscores and +dots, with at most one leading dot) nor an IP address, such as one with a port +or a `*.` wildcard, aborts startup. ### Configuration diff --git a/TODO.md b/TODO.md index 5eb1d53..9307f32 100644 --- a/TODO.md +++ b/TODO.md @@ -34,15 +34,15 @@ P2: security: per-IP rate limiting on the image routes - 2026-10-04 referer blocklist (closes #90): `referer_blocklist` (`PIXA_REFERER_BLOCKLIST`) lists hosts, written and matched as for `allowlist_hosts` with the same matcher; an entry of either list that is - neither a host name (letters, digits, hyphens and dots, with at most one - leading dot) nor an IP address, such as one with a port or a `*.` wildcard, - aborts startup naming the setting and the entry. Both image routes refuse a - request whose `Referer` names a listed host with 403 and a JSON error before - the signature, the cache and the upstream fetch, so it fetches nothing and is - refused whether or not the image is cached. A request with no `Referer`, or - one that does not parse as a URL with a host, is served, so the list is easily - got around; `README.md` and `configs/config.example.yml` say so. It does not - apply to the login and generator pages. + neither a host name (letters, digits, hyphens, underscores and dots, with at + most one leading dot) nor an IP address, such as one with a port or a `*.` + wildcard, aborts startup naming the setting and the entry. Both image routes + refuse a request whose `Referer` names a listed host with 403 and a JSON error + before the signature, the cache and the upstream fetch, so it fetches nothing + and is refused whether or not the image is cached. A request with no + `Referer`, or one that does not parse as a URL with a host, is served, so the + list is easily got around; `README.md` and `configs/config.example.yml` say + so. It does not apply to the login and generator pages. - 2026-10-04 fewer files in the repository root (closes #97): `config.example.yml` moved unchanged to `configs/config.example.yml`, and `README.md`, the comments in `internal/config/config.go` and the startup error diff --git a/internal/config/config.go b/internal/config/config.go index 0a5991e..f4e0fc4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -742,14 +742,15 @@ func (c *Config) validateConcurrencyLimits() error { return nil } -// hostNamePattern matches a host name: letters, digits, hyphens and dots, -// optionally after one leading dot. -var hostNamePattern = regexp.MustCompile(`^\.?[A-Za-z0-9-][A-Za-z0-9.-]*$`) +// hostNamePattern matches a host name: letters, digits, hyphens, underscores +// and dots, optionally after one leading dot. +var hostNamePattern = regexp.MustCompile(`^\.?[A-Za-z0-9_-][A-Za-z0-9_.-]*$`) // validateHostPattern checks that an entry of the named key, allowlist_hosts // or referer_blocklist, is an IP address or a host name, the host name // optionally with one leading dot for suffix matching. Anything else, such as -// a URL, a port or a "*." wildcard, can never match a host, so it is refused. +// a URL, a port or a "*." wildcard, can never match a host name that resolves, +// so it is refused. // So is "." alone: the allowlist matcher would match it against any host // written with a trailing dot, which in allowlist_hosts disables URL signing. func validateHostPattern(key, host string) error { -- 2.54.0