Limit each client's requests to the image routes (closes #236)

The new setting image_requests_per_minute (PIXA_IMAGE_REQUESTS_PER_MINUTE),
default 600, caps the GET and HEAD requests one client sends to /v1/image/
and /v1/e/ per minute; the next is answered 429 with Retry-After. It uses
the login limit's RateLimit middleware, so clients are told apart the same
way, and sits in the image routes' group behind the CORS middleware, so a
preflight is not counted. 0 turns it off; a negative or non-integer value
stops startup.

TestEnvironmentSetsEveryKey gains the new variable, under the owner's
standing approval in #64. To stay
within the linters' limits, the sentry_dsn check moves unchanged into
validateSentryDSN, and the cache_max_bytes explicit flag is set in one
line.

Model: opus-5-5

internal/config/config.go

@@ -34,6 +34,7 @@ const (
 	DefaultUpstreamFetchTimeout       = 30 * time.Second
 	DefaultUpstreamMaxResponseSize    = 50 << 20 // 50 MiB
 	DefaultDownstreamTimeout          = 60 * time.Second
+	DefaultImageRequestsPerMinute     = 600
 )
 
 // Configuration key names.
@@ -61,6 +62,7 @@ const (
 	keyUpstreamFetchTimeout       = "upstream_fetch_timeout"
 	keyUpstreamMaxResponseSize    = "upstream_max_response_size"
 	keyDownstreamTimeout          = "downstream_timeout"
+	keyImageRequestsPerMinute     = "image_requests_per_minute"
 )
 
 // placeholderSigningKey is the dummy signing_key shipped in
@@ -158,6 +160,11 @@ type Config struct {
 	// of the per-request timeout middleware.
 	DownstreamTimeout time.Duration
 
+	// ImageRequestsPerMinute is how many requests one client may make to
+	// the image routes per minute; the next is refused with 429 Too Many
+	// Requests. 0 turns the limit off.
+	ImageRequestsPerMinute int
+
 	// BlockedNetworks are operator-supplied CIDR ranges to refuse in
 	// addition to the built-in SSRF blocklist. Enforced by the upstream
 	// fetcher's dialer; the built-in ranges always apply.
@@ -302,6 +309,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 			keyAccessControlAllowOrigin, DefaultAccessControlAllowOrigin),
 		DownstreamTimeout: loader.durationVal(
 			keyDownstreamTimeout, DefaultDownstreamTimeout),
+		ImageRequestsPerMinute: loader.intVal(
+			keyImageRequestsPerMinute, DefaultImageRequestsPerMinute),
 		CacheMaxBytes:    loader.int64Val(keyCacheMaxBytes, 0),
 		BlockedNetworks:  blockedNetworks,
 		TrustedProxies:   trustedProxies,
@@ -311,9 +320,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 	// The default for an omitted cache_max_bytes is worked out when
 	// the cache opens; here we only record whether the operator set
 	// the key explicitly.
-	if _, present := lookupValue(sc, keyCacheMaxBytes); present {
-		c.CacheMaxBytesExplicit = true
-	}
+	_, c.CacheMaxBytesExplicit = lookupValue(sc, keyCacheMaxBytes)
 
 	// Build DBURL from StateDir if not explicitly set. The derived URL
 	// is a default: it applies only when db_url is omitted, never to an
@@ -426,7 +433,7 @@ func isKnownConfigKey(key string) bool {
 		keyMaxConcurrentProcessing, keyCacheMaxBytes, keyBlockedNetworks,
 		keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout,
 		keyUpstreamMaxResponseSize, keyDownstreamTimeout, keyRefererBlocklist,
-		"env":
+		keyImageRequestsPerMinute, "env":
 		return true
 	}
 
@@ -461,6 +468,7 @@ func envVarNames() map[string]string {
 		keyUpstreamFetchTimeout:       "PIXA_UPSTREAM_FETCH_TIMEOUT",
 		keyUpstreamMaxResponseSize:    "PIXA_UPSTREAM_MAX_RESPONSE_SIZE",
 		keyDownstreamTimeout:          "PIXA_DOWNSTREAM_TIMEOUT",
+		keyImageRequestsPerMinute:     "PIXA_IMAGE_REQUESTS_PER_MINUTE",
 	}
 }
 
@@ -613,6 +621,14 @@ func (c *Config) validate() error {
 			settingName(keyCacheMaxBytes), c.CacheMaxBytes, errMustNotBeNegative)
 	}
 
+	// Zero is valid (it turns the limit off); only negative values are
+	// rejected.
+	if c.ImageRequestsPerMinute < 0 {
+		return fmt.Errorf("%s: value %d %w",
+			settingName(keyImageRequestsPerMinute), c.ImageRequestsPerMinute,
+			errMustNotBeNegative)
+	}
+
 	err = c.validateUpstreamMaxResponseSize()
 	if err != nil {
 		return err
@@ -625,12 +641,9 @@ func (c *Config) validate() error {
 		}
 	}
 
-	if c.SentryDSN != "" {
-		parsed, err := url.Parse(c.SentryDSN)
-		if err != nil || parsed.Scheme == "" || parsed.Host == "" {
-			return fmt.Errorf("%s: value %q is %w",
-				settingName(keySentryDSN), c.SentryDSN, errNotAValidURL)
-		}
+	err = c.validateSentryDSN()
+	if err != nil {
+		return err
 	}
 
 	if (c.MetricsUsername == "") != (c.MetricsPassword == "") {
@@ -642,6 +655,22 @@ func (c *Config) validate() error {
 	return c.validateAccessControlAllowOrigin()
 }
 
+// validateSentryDSN checks that sentry_dsn, when set, is a URL with a scheme
+// and a host.
+func (c *Config) validateSentryDSN() error {
+	if c.SentryDSN == "" {
+		return nil
+	}
+
+	parsed, err := url.Parse(c.SentryDSN)
+	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
+		return fmt.Errorf("%s: value %q is %w",
+			settingName(keySentryDSN), c.SentryDSN, errNotAValidURL)
+	}
+
+	return nil
+}
+
 // validateUpstreamMaxResponseSize checks that upstream_max_response_size
 // is from 1 byte to 1 GiB. An upstream response is read whole into
 // memory, and the image processor reads one byte past this limit, which

internal/config/env_internal_test.go

@@ -70,6 +70,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
 	t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5")
 	t.Setenv("PIXA_UPSTREAM_CONNECTIONS", "10")
 	t.Setenv("PIXA_MAX_CONCURRENT_PROCESSING", "3")
+	t.Setenv("PIXA_IMAGE_REQUESTS_PER_MINUTE", "30")
 	t.Setenv("PIXA_CACHE_MAX_BYTES", "1024")
 	t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24")
 	t.Setenv("PIXA_TRUSTED_PROXIES", "192.0.2.0/24")
@@ -99,6 +100,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
 		UpstreamConnectionsPerHost: 5,
 		UpstreamConnections:        10,
 		MaxConcurrentProcessing:    3,
+		ImageRequestsPerMinute:     30,
 		CacheMaxBytes:              1024,
 		CacheMaxBytesExplicit:      true,
 		BlockedNetworks:            []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")},

internal/config/image_requests_per_minute_internal_test.go

@@ -0,0 +1,107 @@
+package config
+
+import "testing"
+
+// testImageRequestsPerMinuteVar is the variable that sets
+// image_requests_per_minute.
+const testImageRequestsPerMinuteVar = "PIXA_IMAGE_REQUESTS_PER_MINUTE"
+
+// TestImageRequestsPerMinuteValues checks that an omitted
+// image_requests_per_minute is 600, and that 0, which turns the limit off,
+// and any larger whole number are used as given.
+func TestImageRequestsPerMinuteValues(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 c.ImageRequestsPerMinute != 600 {
+		t.Errorf("ImageRequestsPerMinute = %d, want 600", c.ImageRequestsPerMinute)
+	}
+
+	explicit := []struct {
+		value string
+		want  int
+	}{
+		{"0", 0},
+		{"30", 30},
+	}
+
+	for _, tc := range explicit {
+		c, err := configFromYAML(t,
+			signingKeyLine+"image_requests_per_minute: "+tc.value+"\n")
+		if err != nil {
+			t.Fatalf("image_requests_per_minute: %s should load, got error: %v",
+				tc.value, err)
+		}
+
+		if c.ImageRequestsPerMinute != tc.want {
+			t.Errorf("ImageRequestsPerMinute = %d, want %d",
+				c.ImageRequestsPerMinute, tc.want)
+		}
+	}
+}
+
+// TestInvalidImageRequestsPerMinuteAbortsStartup checks that a value that is
+// negative, not a whole number, or null aborts startup naming the key and
+// the value, and the variable too where the value could have come from it.
+func TestInvalidImageRequestsPerMinuteAbortsStartup(t *testing.T) {
+	t.Parallel()
+
+	key := keyImageRequestsPerMinute
+
+	runAbortCases(t, []abortCase{
+		{
+			name: "negative",
+			yaml: signingKeyLine + key + ": -1\n",
+			wantErrSubstrings: []string{
+				key, testImageRequestsPerMinuteVar, "value -1",
+			},
+		},
+		{
+			name: "not a number",
+			yaml: signingKeyLine + key + ": plenty\n",
+			wantErrSubstrings: []string{
+				key, testImageRequestsPerMinuteVar, "plenty",
+			},
+		},
+		{
+			name:              "fractional",
+			yaml:              signingKeyLine + key + ": 2.5\n",
+			wantErrSubstrings: []string{key, "2.5"},
+		},
+		{
+			name:              "explicit null",
+			yaml:              signingKeyLine + key + ": null\n",
+			wantErrSubstrings: []string{key, nullValueText},
+		},
+	})
+}
+
+// TestImageRequestsPerMinuteFromEnvironment checks that the variable sets the
+// limit over the config file, and that an invalid value in it aborts startup
+// naming the variable and the value.
+func TestImageRequestsPerMinuteFromEnvironment(t *testing.T) {
+	t.Setenv(testImageRequestsPerMinuteVar, "0")
+
+	c, err := configFromYAML(t, signingKeyLine+"image_requests_per_minute: 30\n")
+	if err != nil {
+		t.Fatalf("limit from the environment should load: %v", err)
+	}
+
+	if c.ImageRequestsPerMinute != 0 {
+		t.Errorf("ImageRequestsPerMinute = %d, want 0 from the environment",
+			c.ImageRequestsPerMinute)
+	}
+
+	for _, value := range []string{"-5", "ten"} {
+		t.Run(value, func(t *testing.T) {
+			t.Setenv(testImageRequestsPerMinuteVar, value)
+
+			_, err := configFromYAML(t, signingKeyLine)
+			wantStartupError(t, err, testImageRequestsPerMinuteVar, value)
+		})
+	}
+}