From aaf1bbbd4e595ca6af8728f1974499e1bbb28865 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 28 Sep 2026 19:51:14 +0000 Subject: [PATCH] Add the four settings the README documented but pixa lacked (closes #61) access_control_allow_origin, upstream_fetch_timeout, upstream_max_response_size and downstream_timeout were in the README but unknown to pixa, so a config that followed it aborted startup. Each is now a setting with its PIXA_ variable, defaulting to the value that was fixed in the code: *, 30s, 50 MiB and 60s. Durations are positive Go duration strings; the size is a positive whole number of bytes; the origin is * or one scheme and host. An invalid value aborts startup naming the key and value. downstream_timeout replaces HTTPWriteTimeout for the server's write timeout and the per-request timeout. Model: opus-5-5 --- README.md | 20 ++++- TODO.md | 10 ++- config.example.yml | 19 +++++ internal/config/config.go | 119 +++++++++++++++++++++++++++++- internal/handlers/handlers.go | 2 + internal/middleware/middleware.go | 2 +- internal/server/http.go | 3 +- internal/server/routes.go | 2 +- 8 files changed, 167 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 4102390..af1b0e1 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,10 @@ variables set by the file's `env:` section are checked the same way. | `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` | | `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20` | +| `PIXA_UPSTREAM_FETCH_TIMEOUT` | `upstream_fetch_timeout` | Time allowed for one fetch from an upstream host; default `30s` | +| `PIXA_UPSTREAM_MAX_RESPONSE_SIZE` | `upstream_max_response_size` | Largest upstream response accepted, in bytes; default 50 MiB | +| `PIXA_DOWNSTREAM_TIMEOUT` | `downstream_timeout` | Time allowed for answering one client request; default `60s` | +| `PIXA_ACCESS_CONTROL_ALLOW_ORIGIN` | `access_control_allow_origin` | CORS origin allowed to read responses: `*` or one origin; default `*` | | `PIXA_METRICS_USERNAME` | `metrics.username` | Username for `/metrics`, which is served only when both are set | | `PIXA_METRICS_PASSWORD` | `metrics.password` | Password for `/metrics`; set together with the username | | `PIXA_SENTRY_DSN` | `sentry_dsn` | Sentry DSN for error reporting; empty disables it | @@ -243,7 +247,10 @@ variables set by the file's `env:` section are checked the same way. Key settings in more detail: -- `access_control_allow_origin` — CORS origin +- `access_control_allow_origin` — the origin a browser lets read pixa's + responses, sent as the CORS `Access-Control-Allow-Origin` header: `*`, the + default, is any site; otherwise one origin, scheme and host only, such as + `https://example.com`. Anything else aborts startup - `allowlist_hosts` — list of allowed upstream hosts - `blocked_networks` — list of CIDR ranges to refuse for SSRF protection, added to the always-enforced built-in ranges (loopback, private, @@ -269,9 +276,14 @@ Key settings in more detail: the host's addresses is seen with that address. To be sure which address it is, set this to `[]` (or `PIXA_TRUSTED_PROXIES` to empty), send a request through the proxy, and read `remoteIP` in pixa's request log line for it -- `upstream_fetch_timeout` — timeout for origin requests -- `upstream_max_response_size` — max origin response size -- `downstream_timeout` — client response timeout +- `upstream_fetch_timeout` — time allowed for one fetch from an upstream + host, as a duration such as `30s` (the default) or `2m` +- `upstream_max_response_size` — largest upstream response accepted, in + bytes; default `52428800` (50 MiB). It also limits the image data pixa + decodes +- `downstream_timeout` — time allowed for answering one client request, as a + duration; default `60s`. The upstream fetch counts toward it, so keep it + longer than `upstream_fetch_timeout` - `signing_key` — HMAC secret for URL signatures - `cache_max_bytes` — disk cache size limit in bytes; `0` disables the disk cache entirely; omitted defaults to 75% of the free space on diff --git a/TODO.md b/TODO.md index 8f07fa8..6234c46 100644 --- a/TODO.md +++ b/TODO.md @@ -57,6 +57,15 @@ exhaustion that is sooner, never negative; an allowlisted host's URL that has an `exp` follows it too; `immutable` stays, as freshness now ends at the expiry; documented in `README.md`. +- 2026-09-28 add the four settings `README.md` documented but pixa did not + have, which aborted startup as unknown keys (closes #61): + `access_control_allow_origin` (default `*`, the CORS origin), + `upstream_fetch_timeout` (default `30s`), `upstream_max_response_size` + (default 50 MiB) and `downstream_timeout` (default `60s`, both the + server's write timeout and the per-request timeout); each has a + `PIXA_` variable; durations are positive Go duration strings, sizes a + whole number of bytes; an invalid value aborts startup naming the key + and the value; documented in `config.example.yml` and `README.md`. - 2026-09-28 cache stats report real numbers (closes #56): `Cache.Stats` counts the cached source images and processed variants (`source_content` plus `variant_content`) and takes their size from `Cache.UsageBytes`, @@ -306,7 +315,6 @@ exhaustion - X-Request-ID propagation - P2: auto format selection (format=auto based on Accept header) - P2: configuration - - add all configuration options from README - YAML config file support - P2: operational - optional Sentry error reporting diff --git a/config.example.yml b/config.example.yml index f92b752..686b45d 100644 --- a/config.example.yml +++ b/config.example.yml @@ -8,6 +8,10 @@ # this file's env: section is set while the file loads, so it overrides # both the environment the process was started with and this file's own # key. +# +# Durations are Go duration strings such as 30s or 2m and must be +# positive; a bare number has no unit and aborts startup. Sizes are a +# whole number of bytes. # Server settings port: 8080 @@ -67,6 +71,21 @@ allow_http: false # Maximum concurrent connections per upstream host (default: 20) upstream_connections_per_host: 20 +# Time allowed for one fetch from an upstream host (default: 30s) +upstream_fetch_timeout: 30s + +# Largest upstream response accepted, in bytes (default: 52428800, 50 MiB) +upstream_max_response_size: 52428800 + +# Time allowed for answering one client request, the upstream fetch +# included, so keep it longer than upstream_fetch_timeout (default: 60s) +downstream_timeout: 60s + +# The origin a browser lets read pixa's responses, sent as the CORS +# Access-Control-Allow-Origin header: "*" (the default) is any site; +# otherwise one origin, scheme and host only, such as https://example.com +access_control_allow_origin: "*" + # Maximum disk cache size in bytes. Explicit values are used exactly as # given; 0 disables the disk cache entirely (every request fetches and # processes uncached). When omitted, the default is 75% of the free diff --git a/internal/config/config.go b/internal/config/config.go index 3d948be..20bdf83 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -13,6 +13,7 @@ import ( "sort" "strconv" "strings" + "time" "git.eeqj.de/sneak/smartconfig" "go.uber.org/fx" @@ -24,6 +25,10 @@ const ( DefaultPort = 8080 DefaultStateDir = "/var/lib/pixa" DefaultUpstreamConnectionsPerHost = 20 + DefaultAccessControlAllowOrigin = "*" + DefaultUpstreamFetchTimeout = 30 * time.Second + DefaultUpstreamMaxResponseSize = 50 << 20 // 50 MiB + DefaultDownstreamTimeout = 60 * time.Second ) // Configuration key names. @@ -44,6 +49,10 @@ const ( keyCacheMaxBytes = "cache_max_bytes" keyBlockedNetworks = "blocked_networks" keyTrustedProxies = "trusted_proxies" + keyAccessControlAllowOrigin = "access_control_allow_origin" + keyUpstreamFetchTimeout = "upstream_fetch_timeout" + keyUpstreamMaxResponseSize = "upstream_max_response_size" + keyDownstreamTimeout = "downstream_timeout" ) // placeholderSigningKey is the dummy signing_key shipped in @@ -86,6 +95,10 @@ var ( 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( + `not "*" or an origin such as https://example.com`) ) // Params defines dependencies for Config. @@ -112,6 +125,23 @@ type Config struct { AllowHTTP bool // Allow non-TLS upstream (testing only) UpstreamConnectionsPerHost int // Max concurrent connections per upstream host + // UpstreamFetchTimeout is the time allowed for one fetch from an + // upstream host. UpstreamMaxResponseSize is the largest upstream + // response accepted, in bytes, and also the image processor's input + // limit. + UpstreamFetchTimeout time.Duration + UpstreamMaxResponseSize int64 + + // AccessControlAllowOrigin is the origin the CORS middleware allows + // to read responses: "*" for any, or one origin such as + // https://example.com. + AccessControlAllowOrigin string + + // DownstreamTimeout bounds how long answering one client request may + // take. It is both the HTTP server's write timeout and the deadline + // of the per-request timeout middleware. + DownstreamTimeout time.Duration + // 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. @@ -240,6 +270,14 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { AllowHTTP: loader.boolVal(keyAllowHTTP, false), UpstreamConnectionsPerHost: loader.intVal( keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost), + UpstreamFetchTimeout: loader.durationVal( + keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout), + UpstreamMaxResponseSize: loader.int64Val( + keyUpstreamMaxResponseSize, DefaultUpstreamMaxResponseSize), + AccessControlAllowOrigin: loader.stringVal( + keyAccessControlAllowOrigin, DefaultAccessControlAllowOrigin), + DownstreamTimeout: loader.durationVal( + keyDownstreamTimeout, DefaultDownstreamTimeout), CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0), BlockedNetworks: blockedNetworks, TrustedProxies: trustedProxies, @@ -354,7 +392,8 @@ func isKnownConfigKey(key string) bool { case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN, keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP, keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks, - keyTrustedProxies, "env": + keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout, + keyUpstreamMaxResponseSize, keyDownstreamTimeout, "env": return true } @@ -382,6 +421,10 @@ func envVarNames() map[string]string { keyCacheMaxBytes: "PIXA_CACHE_MAX_BYTES", keyBlockedNetworks: "PIXA_BLOCKED_NETWORKS", keyTrustedProxies: "PIXA_TRUSTED_PROXIES", + keyAccessControlAllowOrigin: "PIXA_ACCESS_CONTROL_ALLOW_ORIGIN", + keyUpstreamFetchTimeout: "PIXA_UPSTREAM_FETCH_TIMEOUT", + keyUpstreamMaxResponseSize: "PIXA_UPSTREAM_MAX_RESPONSE_SIZE", + keyDownstreamTimeout: "PIXA_DOWNSTREAM_TIMEOUT", } } @@ -535,6 +578,12 @@ func (c *Config) validate() error { settingName(keyCacheMaxBytes), c.CacheMaxBytes, errMustNotBeNegative) } + if c.UpstreamMaxResponseSize <= 0 { + return fmt.Errorf("%s: value %d %w", + settingName(keyUpstreamMaxResponseSize), c.UpstreamMaxResponseSize, + errMustBePositive) + } + for _, host := range c.AllowlistHosts { err := validateAllowlistHost(host) if err != nil { @@ -556,6 +605,25 @@ func (c *Config) validate() error { errMustBeSetTogether) } + return c.validateAccessControlAllowOrigin() +} + +// validateAccessControlAllowOrigin checks that access_control_allow_origin +// is "*" or one origin, a scheme and host with nothing after them, as +// browsers send it in the Origin header. Anything else, such as a bare +// hostname or a trailing slash, would match no request. +func (c *Config) validateAccessControlAllowOrigin() error { + origin := c.AccessControlAllowOrigin + if origin == "*" { + return nil + } + + parsed, err := url.Parse(origin) + if err != nil || parsed.Host == "" || parsed.Scheme+"://"+parsed.Host != origin { + return fmt.Errorf("%s: value %q is %w", + settingName(keyAccessControlAllowOrigin), origin, errNotAnOrigin) + } + return nil } @@ -673,6 +741,19 @@ func (l *strictLoader) int64Val(key string, defaultVal int64) int64 { return val } +func (l *strictLoader) durationVal(key string, defaultVal time.Duration) time.Duration { + if l.err != nil { + return 0 + } + + val, err := getDuration(l.sc, key, defaultVal) + if err != nil { + l.err = err + } + + return val +} + func (l *strictLoader) boolVal(key string, defaultVal bool) bool { if l.err != nil { return false @@ -794,6 +875,42 @@ func getInt64(sc *smartconfig.Config, key string, defaultVal int64) (int64, erro } } +// getDuration returns the duration value for key, or defaultVal if the +// key is omitted. A present value must be a positive Go duration string +// such as "30s" or "2m", read with time.ParseDuration; a bare number has +// no unit and is an error, as is an explicit null. +func getDuration( + sc *smartconfig.Config, key string, defaultVal time.Duration, +) (time.Duration, error) { + raw, ok := lookupValue(sc, key) + if !ok { + return defaultVal, nil + } + + if raw == nil { + return 0, errNullConfigValue(key) + } + + str, ok := raw.(string) + if !ok { + return 0, fmt.Errorf("config key %q: value %v (%T) is %w", + key, raw, raw, errNotADuration) + } + + parsed, err := time.ParseDuration(strings.TrimSpace(str)) + if err != nil { + return 0, fmt.Errorf("%s: value %q is %w", + settingName(key), str, errNotADuration) + } + + if parsed <= 0 { + return 0, fmt.Errorf("%s: value %q %w", + settingName(key), str, errMustBePositive) + } + + return parsed, nil +} + // getBool returns the boolean value for key, or defaultVal if the key // is omitted. A present value that is not a boolean (or a ParseBool-able // string), or is explicitly null, is an error; numbers are not accepted diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 09f3ffc..3da580d 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -106,6 +106,8 @@ func (s *Handlers) initImageService() error { // Create the fetcher config fetcherCfg := httpfetcher.DefaultConfig() fetcherCfg.AllowHTTP = s.config.AllowHTTP + fetcherCfg.Timeout = s.config.UpstreamFetchTimeout + fetcherCfg.MaxResponseSize = s.config.UpstreamMaxResponseSize if s.config.UpstreamConnectionsPerHost > 0 { fetcherCfg.MaxConnectionsPerHost = s.config.UpstreamConnectionsPerHost diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 30feaa5..492ba87 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -172,7 +172,7 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler { // CORS returns a CORS middleware. func (s *Middleware) CORS() func(http.Handler) http.Handler { return cors.Handler(cors.Options{ - AllowedOrigins: []string{"*"}, + AllowedOrigins: []string{s.config.AccessControlAllowOrigin}, AllowedMethods: []string{"GET", "HEAD", "OPTIONS"}, AllowedHeaders: []string{"Accept", "Authorization", "Content-Type"}, ExposedHeaders: []string{"Link"}, diff --git a/internal/server/http.go b/internal/server/http.go index bdd1238..13162e0 100644 --- a/internal/server/http.go +++ b/internal/server/http.go @@ -14,7 +14,6 @@ const ( // short, so a slowloris client dribbling headers is dropped well // before it ties up a connection for the whole ReadTimeout window. HTTPReadHeaderTimeout = 10 * time.Second - HTTPWriteTimeout = 60 * time.Second // HTTPIdleTimeout bounds how long an idle keep-alive connection is // held open, so idle connections cannot accumulate without limit on a // service targeting high concurrency. @@ -30,7 +29,7 @@ func (s *Server) newHTTPServer() *http.Server { Addr: fmt.Sprintf(":%d", s.config.Port), ReadTimeout: HTTPReadTimeout, ReadHeaderTimeout: HTTPReadHeaderTimeout, - WriteTimeout: HTTPWriteTimeout, + WriteTimeout: s.config.DownstreamTimeout, IdleTimeout: HTTPIdleTimeout, MaxHeaderBytes: HTTPMaxHeaderBytes, Handler: s, diff --git a/internal/server/routes.go b/internal/server/routes.go index 0bb2748..4623b75 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -33,7 +33,7 @@ func (s *Server) SetupRoutes() { } s.router.Use(s.mw.CORS()) - s.router.Use(middleware.Timeout(HTTPWriteTimeout)) + s.router.Use(middleware.Timeout(s.config.DownstreamTimeout)) if s.sentryEnabled { sentryHandler := sentryhttp.New(sentryhttp.Options{