12 Commits
Author SHA1 Message Date
clawbot 2a8ce05335 Give the login rate limit tests the default request timeout (closes #61)
check / check (push) Successful in 3m14s
newTestServer left downstream_timeout at zero, so every request in those
tests ran with an already expired per-request timeout. It now uses
config.DefaultDownstreamTimeout, as a real config would. Approved by the
owner on the issue.

Model: opus-5-5
2026-09-29 06:10:58 +00:00
clawbot d46e42b747 State the origin rule in README.md and config.example.yml (closes #61)
check / check (push) Successful in 2m47s
Both now say what access_control_allow_origin accepts, instead of
"exactly as the browser sends it", and that another scheme, such as a
browser extension's, aborts startup.

Model: opus-5-5
2026-09-29 00:19:28 +00:00
clawbot 2055b557e3 Test that an origin with an upper case scheme aborts startup (closes #61)
Model: opus-5-5
2026-09-29 00:19:28 +00:00
clawbot facf84299c Accept only an origin written exactly as a browser sends it (closes #61)
access_control_allow_origin is now "*", or http or https, a host that is
an IP address as net/netip writes it (IPv6 in brackets) or a lowercase
host name whose last part contains a letter, and an optional port 1-65535
with no leading zero that is not the scheme's default. The value must
equal the origin rebuilt from those parts; anything else aborts startup
naming the key, its variable and the value. README.md and
config.example.yml say an origin is scheme, host and optional port,
exactly as the browser sends it.

Model: opus-5-5
2026-09-29 00:19:28 +00:00
clawbot af5f98b867 Test that origins a browser does not send abort startup (closes #61)
Adds the review's examples to invalidSizeAndOriginCases: a scheme's
default port, a port with a leading zero, hosts a browser reads as an
IPv4 address or refuses, an IPv6 address not in its shortest form,
plus a scheme other than http or https and a host name in upper case.

Model: opus-5-5
2026-09-29 00:19:18 +00:00
clawbot 84ec9a6080 Check the origin's host and port; cap the size at 1 GiB (closes #61)
access_control_allow_origin now needs a host name (ASCII letters,
digits, hyphens, dots) or an IP address, and a port, when given, from 1
to 65535, read with net/url, net/netip and strconv. Two hosts, an empty
port, no host, a bad port or a non-ASCII host name abort startup, as a
* inside the value already did.

upstream_max_response_size above 1 GiB aborts startup: the image
processor reads one byte past the limit, which wrapped negative at the
largest 64-bit value, and a response is held whole in memory.
config.example.yml states the maximum.

Model: opus-5-5
2026-09-29 00:19:18 +00:00
clawbot 256fc71511 Test origin host and port checks and a 1 GiB size maximum (closes #61)
Failing cases: access_control_allow_origin with two hosts, an empty
port, no host, port 0, port 99999 or a non-ASCII host name, and
upstream_max_response_size above 1 GiB, up to the largest 64-bit
integer, where the image processor's limit wraps negative. Each must
abort startup naming the key and the value. The bad origins are now one
list so the table fits the linter's function length limit; the size
test uses 1 GiB to show the maximum itself is accepted, and IPv4 and
IPv6 origins are shown to be accepted.

Model: opus-5-5
2026-09-29 00:19:07 +00:00
clawbot 44b3885f41 Reword the comment on TestCORSAnswersWithConfiguredOrigin (closes #61)
The comment was not a readable sentence; it is now one plain sentence
saying what the test checks.

Model: opus-5-5
2026-09-29 00:19:07 +00:00
clawbot 3fdb7faab1 Refuse an origin with a * inside at startup (closes #61)
access_control_allow_origin accepted values such as https://* or
https://*example.com, which the CORS middleware reads as a pattern that
lets other sites read responses. Any value that contains * and is not
exactly * now aborts startup naming the key, its variable and the value.

Model: opus-5-5
2026-09-29 00:19:07 +00:00
clawbot 7a969bb225 Test that an origin with a * inside aborts startup (closes #61)
Failing cases for access_control_allow_origin set to https://*,
https://*.example.com and https://*example.com. The CORS middleware
reads a * inside an origin as a pattern, so these would let other sites
read responses; each must abort startup naming the key and the value.

Model: opus-5-5
2026-09-29 00:19:07 +00:00
clawbot 2cd328d2da 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
2026-09-29 00:19:07 +00:00
clawbot 5fcbad8515 Test four README settings pixa does not have yet (closes #61)
Failing tests for access_control_allow_origin, upstream_fetch_timeout,
upstream_max_response_size and downstream_timeout: their defaults,
valid values from the file and the environment, invalid values aborting
startup naming the key or variable and the value, the CORS middleware
answering with the configured origin, and the server's write timeout
coming from downstream_timeout. They do not compile until the settings
exist.

Model: opus-5-5
2026-09-29 00:18:54 +00:00
13 changed files with 571 additions and 18 deletions
+20 -4
View File
@@ -212,6 +212,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_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_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_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_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_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 | | `PIXA_SENTRY_DSN` | `sentry_dsn` | Sentry DSN for error reporting; empty disables it |
@@ -220,7 +224,14 @@ variables set by the file's `env:` section are checked the same way.
Key settings in more detail: 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 `http` or `https` origin such as
`https://example.com`, whose host is a lowercase host name (letters,
digits, hyphens and dots, with a letter in its last part) or an IP address
(IPv6 in brackets, in its shortest form), with an optional port 1-65535
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 - `allowlist_hosts` — list of allowed upstream hosts
- `blocked_networks` — list of CIDR ranges to refuse for SSRF protection, - `blocked_networks` — list of CIDR ranges to refuse for SSRF protection,
added to the always-enforced built-in ranges (loopback, private, added to the always-enforced built-in ranges (loopback, private,
@@ -239,9 +250,14 @@ Key settings in more detail:
one, and an explicit list replaces the default. An invalid CIDR aborts one, and an explicit list replaces the default. An invalid CIDR aborts
startup. Set this to your proxy's address range if it is not already startup. Set this to your proxy's address range if it is not already
covered by the defaults covered by the defaults
- `upstream_fetch_timeout` — timeout for origin requests - `upstream_fetch_timeout` — time allowed for one fetch from an upstream
- `upstream_max_response_size` — max origin response size host, as a duration such as `30s` (the default) or `2m`
- `downstream_timeout` — client response timeout - `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 - `signing_key` — HMAC secret for URL signatures
- `cache_max_bytes` — disk cache size limit in bytes; `0` disables the - `cache_max_bytes` — disk cache size limit in bytes; `0` disables the
disk cache entirely; omitted defaults to 75% of the free space on disk cache entirely; omitted defaults to 75% of the free space on
+11 -1
View File
@@ -30,6 +30,17 @@ exhaustion
# Completed Steps # Completed Steps
- 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, the size a
whole number of bytes up to 1 GiB, the origin `*` or one `http` or
`https` origin as `README.md` describes it; an invalid value
aborts startup naming the key and the value; documented in
`config.example.yml` and `README.md`.
- 2026-09-28 strip metadata from processed images (closes #82): every output is - 2026-09-28 strip metadata from processed images (closes #82): every output is
exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC
profile; the image is first turned upright with `AutoRotate` (before sizes are profile; the image is first turned upright with `AutoRotate` (before sizes are
@@ -267,7 +278,6 @@ exhaustion
- X-Request-ID propagation - X-Request-ID propagation
- P2: auto format selection (format=auto based on Accept header) - P2: auto format selection (format=auto based on Accept header)
- P2: configuration - P2: configuration
- add all configuration options from README
- YAML config file support - YAML config file support
- P2: operational - P2: operational
- optional Sentry error reporting - optional Sentry error reporting
+25
View File
@@ -8,6 +8,10 @@
# this file's env: section is set while the file loads, so it overrides # 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 # both the environment the process was started with and this file's own
# key. # 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 # Server settings
port: 8080 port: 8080
@@ -61,6 +65,27 @@ allow_http: false
# Maximum concurrent connections per upstream host (default: 20) # Maximum concurrent connections per upstream host (default: 20)
upstream_connections_per_host: 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, at most 1073741824
# (1 GiB) (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 http or https origin such as https://example.com, whose
# host is a lowercase host name (letters, digits, hyphens and dots, with a
# letter in its last part) or an IP address (IPv6 in brackets, in its
# shortest form), with an optional port 1-65535 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.
access_control_allow_origin: "*"
# Maximum disk cache size in bytes. Explicit values are used exactly as # Maximum disk cache size in bytes. Explicit values are used exactly as
# given; 0 disables the disk cache entirely (every request fetches and # given; 0 disables the disk cache entirely (every request fetches and
# processes uncached). When omitted, the default is 75% of the free # processes uncached). When omitted, the default is 75% of the free
+175 -1
View File
@@ -13,6 +13,7 @@ import (
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
"time"
"git.eeqj.de/sneak/smartconfig" "git.eeqj.de/sneak/smartconfig"
"go.uber.org/fx" "go.uber.org/fx"
@@ -24,6 +25,10 @@ const (
DefaultPort = 8080 DefaultPort = 8080
DefaultStateDir = "/var/lib/pixa" DefaultStateDir = "/var/lib/pixa"
DefaultUpstreamConnectionsPerHost = 20 DefaultUpstreamConnectionsPerHost = 20
DefaultAccessControlAllowOrigin = "*"
DefaultUpstreamFetchTimeout = 30 * time.Second
DefaultUpstreamMaxResponseSize = 50 << 20 // 50 MiB
DefaultDownstreamTimeout = 60 * time.Second
) )
// Configuration key names. // Configuration key names.
@@ -44,6 +49,10 @@ const (
keyCacheMaxBytes = "cache_max_bytes" keyCacheMaxBytes = "cache_max_bytes"
keyBlockedNetworks = "blocked_networks" keyBlockedNetworks = "blocked_networks"
keyTrustedProxies = "trusted_proxies" 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 // placeholderSigningKey is the dummy signing_key shipped in
@@ -69,6 +78,7 @@ var (
errEmptyEntry = errors.New("contains an empty entry") errEmptyEntry = errors.New("contains an empty entry")
errNotAValidURL = errors.New("not a valid URL") errNotAValidURL = errors.New("not a valid URL")
errPortOutOfRange = errors.New("outside the valid port range") errPortOutOfRange = errors.New("outside the valid port range")
errSizeOutOfRange = errors.New("outside the accepted range")
errTooFewConnections = errors.New("must be at least 1") errTooFewConnections = errors.New("must be at least 1")
errValueTooShort = errors.New("value too short") errValueTooShort = errors.New("value too short")
errPlaceholderKey = errors.New( errPlaceholderKey = errors.New(
@@ -86,6 +96,10 @@ var (
errNotBareHostname = errors.New( errNotBareHostname = errors.New(
"must be a bare hostname without scheme, path, or whitespace") "must be a bare hostname without scheme, path, or whitespace")
errNoHostnameLabels = errors.New("contains no hostname labels") 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. // Params defines dependencies for Config.
@@ -112,6 +126,23 @@ type Config struct {
AllowHTTP bool // Allow non-TLS upstream (testing only) AllowHTTP bool // Allow non-TLS upstream (testing only)
UpstreamConnectionsPerHost int // Max concurrent connections per upstream host 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 // BlockedNetworks are operator-supplied CIDR ranges to refuse in
// addition to the built-in SSRF blocklist. Enforced by the upstream // addition to the built-in SSRF blocklist. Enforced by the upstream
// fetcher's dialer; the built-in ranges always apply. // fetcher's dialer; the built-in ranges always apply.
@@ -240,6 +271,14 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
AllowHTTP: loader.boolVal(keyAllowHTTP, false), AllowHTTP: loader.boolVal(keyAllowHTTP, false),
UpstreamConnectionsPerHost: loader.intVal( UpstreamConnectionsPerHost: loader.intVal(
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost), 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), CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
BlockedNetworks: blockedNetworks, BlockedNetworks: blockedNetworks,
TrustedProxies: trustedProxies, TrustedProxies: trustedProxies,
@@ -354,7 +393,8 @@ func isKnownConfigKey(key string) bool {
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN, case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP, keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks, keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks,
keyTrustedProxies, "env": keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout,
keyUpstreamMaxResponseSize, keyDownstreamTimeout, "env":
return true return true
} }
@@ -382,6 +422,10 @@ func envVarNames() map[string]string {
keyCacheMaxBytes: "PIXA_CACHE_MAX_BYTES", keyCacheMaxBytes: "PIXA_CACHE_MAX_BYTES",
keyBlockedNetworks: "PIXA_BLOCKED_NETWORKS", keyBlockedNetworks: "PIXA_BLOCKED_NETWORKS",
keyTrustedProxies: "PIXA_TRUSTED_PROXIES", 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 +579,11 @@ func (c *Config) validate() error {
settingName(keyCacheMaxBytes), c.CacheMaxBytes, errMustNotBeNegative) settingName(keyCacheMaxBytes), c.CacheMaxBytes, errMustNotBeNegative)
} }
err = c.validateUpstreamMaxResponseSize()
if err != nil {
return err
}
for _, host := range c.AllowlistHosts { for _, host := range c.AllowlistHosts {
err := validateAllowlistHost(host) err := validateAllowlistHost(host)
if err != nil { if err != nil {
@@ -556,6 +605,82 @@ func (c *Config) validate() error {
errMustBeSetTogether) errMustBeSetTogether)
} }
return c.validateAccessControlAllowOrigin()
}
// 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
// must not overflow.
func (c *Config) validateUpstreamMaxResponseSize() error {
const maxUpstreamMaxResponseSize = 1 << 30 // 1 GiB
if c.UpstreamMaxResponseSize < 1 ||
c.UpstreamMaxResponseSize > maxUpstreamMaxResponseSize {
return fmt.Errorf("%s: value %d is %w 1-%d",
settingName(keyUpstreamMaxResponseSize), c.UpstreamMaxResponseSize,
errSizeOutOfRange, maxUpstreamMaxResponseSize)
}
return nil
}
// validateAccessControlAllowOrigin accepts "*" or an origin exactly as a browser
// sends it: http or https, an IP address as netip writes it or a lowercase name
// with a letter in its last part, and an optional port 1-65535, not the default.
func (c *Config) validateAccessControlAllowOrigin() error {
origin := c.AccessControlAllowOrigin
if origin == "*" {
return nil
}
errOrigin := fmt.Errorf("%s: value %q is %w",
settingName(keyAccessControlAllowOrigin), origin, errNotAnOrigin)
parsed, err := url.Parse(origin)
if err != nil {
return errOrigin
}
defaultPort := map[string]string{"http": "80", "https": "443"}[parsed.Scheme]
if defaultPort == "" {
return errOrigin
}
const letters = "abcdefghijklmnopqrstuvwxyz"
host := parsed.Hostname()
lastPart := host[strings.LastIndex(host, ".")+1:]
addr, err := netip.ParseAddr(host)
switch {
case err == nil && addr.Is6():
host = "[" + addr.String() + "]"
case err == nil:
host = addr.String()
case strings.Trim(host, letters+"0123456789-.") != "": // a character other than these
return errOrigin
case !strings.ContainsAny(lastPart, letters):
return errOrigin
}
// The value must be exactly the origin rebuilt from its parts.
rebuilt := parsed.Scheme + "://" + host
port := parsed.Port()
if port != "" {
_, err := strconv.ParseUint(port, 10, 16)
if err != nil || port[0] == '0' || port == defaultPort {
return errOrigin
}
rebuilt += ":" + port
}
if rebuilt != origin {
return errOrigin
}
return nil return nil
} }
@@ -673,6 +798,19 @@ func (l *strictLoader) int64Val(key string, defaultVal int64) int64 {
return val 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 { func (l *strictLoader) boolVal(key string, defaultVal bool) bool {
if l.err != nil { if l.err != nil {
return false return false
@@ -794,6 +932,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 // 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 // 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 // string), or is explicitly null, is an error; numbers are not accepted
@@ -6,6 +6,7 @@ import (
"path/filepath" "path/filepath"
"strings" "strings"
"testing" "testing"
"time"
"git.eeqj.de/sneak/smartconfig" "git.eeqj.de/sneak/smartconfig"
) )
@@ -599,3 +600,244 @@ func TestEnsureStateDirFailsOnUncreatablePath(t *testing.T) {
t.Errorf("error %q does not name the offending key state_dir", err.Error()) t.Errorf("error %q does not name the offending key state_dir", err.Error())
} }
} }
// TestOmittedOriginTimeoutsAndSizeUseDefaults checks that the CORS
// origin, the upstream fetch timeout, the upstream response size limit
// and the downstream timeout default to the values pixa used before they
// could be configured.
func TestOmittedOriginTimeoutsAndSizeUseDefaults(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.AccessControlAllowOrigin != "*" {
t.Errorf("AccessControlAllowOrigin = %q, want *", c.AccessControlAllowOrigin)
}
if c.UpstreamFetchTimeout != 30*time.Second {
t.Errorf("UpstreamFetchTimeout = %v, want 30s", c.UpstreamFetchTimeout)
}
if c.UpstreamMaxResponseSize != 50<<20 {
t.Errorf("UpstreamMaxResponseSize = %d, want %d (50 MiB)",
c.UpstreamMaxResponseSize, 50<<20)
}
if c.DownstreamTimeout != 60*time.Second {
t.Errorf("DownstreamTimeout = %v, want 60s", c.DownstreamTimeout)
}
}
// TestExplicitOriginTimeoutsAndSizeAreUsed checks that valid values for
// the CORS origin, the two timeouts and the response size limit are used
// as given. The size is the largest accepted, 1 GiB.
func TestExplicitOriginTimeoutsAndSizeAreUsed(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine+`
access_control_allow_origin: https://app.example.com
upstream_fetch_timeout: 10s
upstream_max_response_size: 1073741824
downstream_timeout: 2m
`)
if err != nil {
t.Fatalf("valid config should load, got error: %v", err)
}
if c.AccessControlAllowOrigin != "https://app.example.com" {
t.Errorf("AccessControlAllowOrigin = %q, want https://app.example.com",
c.AccessControlAllowOrigin)
}
if c.UpstreamFetchTimeout != 10*time.Second {
t.Errorf("UpstreamFetchTimeout = %v, want 10s", c.UpstreamFetchTimeout)
}
if c.UpstreamMaxResponseSize != 1073741824 {
t.Errorf("UpstreamMaxResponseSize = %d, want 1073741824",
c.UpstreamMaxResponseSize)
}
if c.DownstreamTimeout != 2*time.Minute {
t.Errorf("DownstreamTimeout = %v, want 2m", c.DownstreamTimeout)
}
}
// TestOriginWithPortOrAnyOriginIsAccepted checks the other accepted forms
// of access_control_allow_origin: "*", an origin with a port, and origins
// whose host is an IPv4 or IPv6 address.
func TestOriginWithPortOrAnyOriginIsAccepted(t *testing.T) {
t.Parallel()
for _, origin := range []string{
"*", "http://localhost:3000", "http://192.0.2.1", "http://[2001:db8::1]:8080",
} {
c, err := configFromYAML(t, signingKeyLine+
"access_control_allow_origin: \""+origin+"\"\n")
if err != nil {
t.Fatalf("origin %q should be accepted, got error: %v", origin, err)
}
if c.AccessControlAllowOrigin != origin {
t.Errorf("AccessControlAllowOrigin = %q, want %q",
c.AccessControlAllowOrigin, origin)
}
}
}
// invalidTimeoutCases are configs where upstream_fetch_timeout or
// downstream_timeout is not a positive Go duration string; each must
// abort startup naming the key and the value.
func invalidTimeoutCases() []abortCase {
return []abortCase{
{
name: "upstream_fetch_timeout not a duration",
yaml: signingKeyLine + "upstream_fetch_timeout: soon\n",
wantErrSubstrings: []string{keyUpstreamFetchTimeout, "soon"},
},
{
name: "upstream_fetch_timeout number without a unit",
yaml: signingKeyLine + "upstream_fetch_timeout: 45\n",
wantErrSubstrings: []string{keyUpstreamFetchTimeout, "45"},
},
{
name: "upstream_fetch_timeout zero",
yaml: signingKeyLine + "upstream_fetch_timeout: 0s\n",
wantErrSubstrings: []string{keyUpstreamFetchTimeout, "0s"},
},
{
name: "upstream_fetch_timeout negative",
yaml: signingKeyLine + "upstream_fetch_timeout: -5s\n",
wantErrSubstrings: []string{keyUpstreamFetchTimeout, "-5s"},
},
{
name: "upstream_fetch_timeout null",
yaml: signingKeyLine + "upstream_fetch_timeout: null\n",
wantErrSubstrings: []string{keyUpstreamFetchTimeout, nullValueText},
},
{
name: "downstream_timeout not a duration",
yaml: signingKeyLine + "downstream_timeout: 1 minute\n",
wantErrSubstrings: []string{keyDownstreamTimeout, "1 minute"},
},
{
name: "downstream_timeout zero",
yaml: signingKeyLine + "downstream_timeout: 0s\n",
wantErrSubstrings: []string{keyDownstreamTimeout, "0s"},
},
{
name: "downstream_timeout negative",
yaml: signingKeyLine + "downstream_timeout: -1m\n",
wantErrSubstrings: []string{keyDownstreamTimeout, "-1m"},
},
{
name: "downstream_timeout null",
yaml: signingKeyLine + "downstream_timeout:\n",
wantErrSubstrings: []string{keyDownstreamTimeout, nullValueText},
},
}
}
// invalidSizeAndOriginCases are configs where upstream_max_response_size
// is not a whole number of bytes from 1 to 1 GiB, or
// access_control_allow_origin is neither "*" nor an origin; each must
// abort startup naming the key and the value.
func invalidSizeAndOriginCases() []abortCase {
badOrigins := []string{
"", // empty
"example.com", // no scheme
"https://example.com/images", // a path
"https://example.com/", // a trailing slash
// The CORS middleware reads a * inside an origin as a pattern
// that lets other sites read responses.
"https://*",
"https://*.example.com",
"https://*example.com",
"https://a.com,b.com", // two hosts
"https://example.com:", // an empty port
"https://:8443", // no host
"https://example.com:0", // a port below 1
"https://example.com:99999", // a port above 65535
"https://exämple.com", // a host name that is not ASCII
"https://example.com:443", // the default port for https
"http://example.com:80", // the default port for http
"https://example.com:08080", // a port with a leading zero
"https://01.2.3.4", // an IPv4 address with a leading zero
"https://10.0.0", // an IPv4 address with three parts
"https://192.168.1.256", // an IPv4 address part above 255
"https://example.123", // a host name whose last part is a number
"https://[0:0:0:0:0:0:0:1]", // an IPv6 address not in its shortest form
"file://example.com", // a scheme other than http or https
"HTTPS://example.com", // a scheme in upper case
"https://Example.com", // a host name in upper case
}
cases := make([]abortCase, 0, len(badOrigins))
for _, origin := range badOrigins {
cases = append(cases, abortCase{
name: "access_control_allow_origin " + origin,
yaml: signingKeyLine +
"access_control_allow_origin: \"" + origin + "\"\n",
wantErrSubstrings: []string{keyAccessControlAllowOrigin, origin},
})
}
return append(cases, []abortCase{
{
name: "access_control_allow_origin null",
yaml: signingKeyLine + "access_control_allow_origin: null\n",
wantErrSubstrings: []string{keyAccessControlAllowOrigin, nullValueText},
},
{
name: "upstream_max_response_size with a unit",
yaml: signingKeyLine + "upstream_max_response_size: 50MB\n",
wantErrSubstrings: []string{keyUpstreamMaxResponseSize, "50MB"},
},
{
name: "upstream_max_response_size fractional",
yaml: signingKeyLine + "upstream_max_response_size: 1.5\n",
wantErrSubstrings: []string{keyUpstreamMaxResponseSize, "1.5"},
},
{
name: "upstream_max_response_size zero",
yaml: signingKeyLine + "upstream_max_response_size: 0\n",
wantErrSubstrings: []string{keyUpstreamMaxResponseSize, "0"},
},
{
name: "upstream_max_response_size negative",
yaml: signingKeyLine + "upstream_max_response_size: -1\n",
wantErrSubstrings: []string{keyUpstreamMaxResponseSize, "-1"},
},
{
name: "upstream_max_response_size null",
yaml: signingKeyLine + "upstream_max_response_size: null\n",
wantErrSubstrings: []string{keyUpstreamMaxResponseSize, nullValueText},
},
{
name: "upstream_max_response_size above 1 GiB",
yaml: signingKeyLine + "upstream_max_response_size: 1073741825\n",
wantErrSubstrings: []string{keyUpstreamMaxResponseSize, "1073741825"},
},
{
name: "upstream_max_response_size largest 64-bit integer",
yaml: signingKeyLine +
"upstream_max_response_size: 9223372036854775807\n",
wantErrSubstrings: []string{
keyUpstreamMaxResponseSize, "9223372036854775807",
},
},
}...)
}
// TestInvalidOriginTimeoutOrSizeAbortsStartup verifies the
// no-silent-fallback rule for the CORS origin, the two timeouts and the
// response size limit: a value that does not parse or is out of range
// aborts startup naming the key and the value.
func TestInvalidOriginTimeoutOrSizeAbortsStartup(t *testing.T) {
t.Parallel()
runAbortCases(t, append(invalidTimeoutCases(), invalidSizeAndOriginCases()...))
}
+33
View File
@@ -8,6 +8,7 @@ import (
"slices" "slices"
"strings" "strings"
"testing" "testing"
"time"
"sneak.berlin/go/pixa/internal/globals" "sneak.berlin/go/pixa/internal/globals"
"sneak.berlin/go/pixa/internal/logger" "sneak.berlin/go/pixa/internal/logger"
@@ -69,6 +70,10 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
t.Setenv("PIXA_CACHE_MAX_BYTES", "1024") t.Setenv("PIXA_CACHE_MAX_BYTES", "1024")
t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24") t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24")
t.Setenv("PIXA_TRUSTED_PROXIES", "192.0.2.0/24") t.Setenv("PIXA_TRUSTED_PROXIES", "192.0.2.0/24")
t.Setenv("PIXA_ACCESS_CONTROL_ALLOW_ORIGIN", "https://app.example.com")
t.Setenv("PIXA_UPSTREAM_FETCH_TIMEOUT", "10s")
t.Setenv("PIXA_UPSTREAM_MAX_RESPONSE_SIZE", "1048576")
t.Setenv("PIXA_DOWNSTREAM_TIMEOUT", "2m")
c, err := newFromSmartConfig(nil) c, err := newFromSmartConfig(nil)
if err != nil { if err != nil {
@@ -92,6 +97,10 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
cacheMaxBytesExplicit: true, cacheMaxBytesExplicit: true,
BlockedNetworks: []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")}, BlockedNetworks: []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")},
TrustedProxies: []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}, TrustedProxies: []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")},
AccessControlAllowOrigin: "https://app.example.com",
UpstreamFetchTimeout: 10 * time.Second,
UpstreamMaxResponseSize: 1048576,
DownstreamTimeout: 2 * time.Minute,
} }
if !reflect.DeepEqual(*c, want) { if !reflect.DeepEqual(*c, want) {
@@ -280,6 +289,30 @@ func TestInvalidDebugFromEnvironmentAbortsStartup(t *testing.T) {
wantStartupError(t, err, "PIXA_DEBUG", "maybe") wantStartupError(t, err, "PIXA_DEBUG", "maybe")
} }
// TestInvalidOriginTimeoutOrSizeFromEnvironmentAbortsStartup checks that
// an invalid CORS origin, timeout or response size limit in its variable
// aborts startup naming the variable and the value.
func TestInvalidOriginTimeoutOrSizeFromEnvironmentAbortsStartup(t *testing.T) {
cases := []struct {
variable string
value string
}{
{"PIXA_ACCESS_CONTROL_ALLOW_ORIGIN", "example.com"},
{"PIXA_UPSTREAM_FETCH_TIMEOUT", "soon"},
{"PIXA_UPSTREAM_MAX_RESPONSE_SIZE", "50MB"},
{"PIXA_DOWNSTREAM_TIMEOUT", "0s"},
}
for _, tc := range cases {
t.Run(tc.variable, func(t *testing.T) {
t.Setenv(tc.variable, tc.value)
_, err := configFromYAML(t, signingKeyLine)
wantStartupError(t, err, tc.variable, tc.value)
})
}
}
// TestConfigFileAloneBehavesAsBefore checks that with no variables set // TestConfigFileAloneBehavesAsBefore checks that with no variables set
// (TestMain unsets them) the config file's values are used and omitted // (TestMain unsets them) the config file's values are used and omitted
// keys take their defaults. // keys take their defaults.
+2
View File
@@ -106,6 +106,8 @@ func (s *Handlers) initImageService() error {
// Create the fetcher config // Create the fetcher config
fetcherCfg := httpfetcher.DefaultConfig() fetcherCfg := httpfetcher.DefaultConfig()
fetcherCfg.AllowHTTP = s.config.AllowHTTP fetcherCfg.AllowHTTP = s.config.AllowHTTP
fetcherCfg.Timeout = s.config.UpstreamFetchTimeout
fetcherCfg.MaxResponseSize = s.config.UpstreamMaxResponseSize
if s.config.UpstreamConnectionsPerHost > 0 { if s.config.UpstreamConnectionsPerHost > 0 {
fetcherCfg.MaxConnectionsPerHost = s.config.UpstreamConnectionsPerHost fetcherCfg.MaxConnectionsPerHost = s.config.UpstreamConnectionsPerHost
+1 -1
View File
@@ -172,7 +172,7 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
// CORS returns a CORS middleware. // CORS returns a CORS middleware.
func (s *Middleware) CORS() func(http.Handler) http.Handler { func (s *Middleware) CORS() func(http.Handler) http.Handler {
return cors.Handler(cors.Options{ return cors.Handler(cors.Options{
AllowedOrigins: []string{"*"}, AllowedOrigins: []string{s.config.AccessControlAllowOrigin},
AllowedMethods: []string{"GET", "HEAD", "OPTIONS"}, AllowedMethods: []string{"GET", "HEAD", "OPTIONS"},
AllowedHeaders: []string{"Accept", "Authorization", "Content-Type"}, AllowedHeaders: []string{"Accept", "Authorization", "Content-Type"},
ExposedHeaders: []string{"Link"}, ExposedHeaders: []string{"Link"},
@@ -9,6 +9,53 @@ import (
"sneak.berlin/go/pixa/internal/config" "sneak.berlin/go/pixa/internal/config"
) )
// TestCORSAnswersWithConfiguredOrigin checks that the CORS middleware
// answers with access_control_allow_origin, where "*" lets any origin read
// responses and a single origin lets only that origin read them.
func TestCORSAnswersWithConfiguredOrigin(t *testing.T) {
t.Parallel()
const appOrigin = "https://app.example.com"
cases := []struct {
configured string
requestOrigin string
want string
}{
{"*", "https://any.example.com", "*"},
{appOrigin, appOrigin, appOrigin},
{appOrigin, "https://other.example.com", ""},
}
testHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})
for _, tc := range cases {
mw := &Middleware{
log: slog.Default(),
config: &config.Config{AccessControlAllowOrigin: tc.configured},
}
handler := mw.CORS()(testHandler)
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/v1/image/example.com/a.jpg/1x1.png", nil)
req.Header.Set("Origin", tc.requestOrigin)
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
got := rec.Header().Get("Access-Control-Allow-Origin")
if got != tc.want {
t.Errorf("configured %q, request from %q: "+
"Access-Control-Allow-Origin = %q, want %q",
tc.configured, tc.requestOrigin, got, tc.want)
}
}
}
func TestSecurityHeaders(t *testing.T) { func TestSecurityHeaders(t *testing.T) {
t.Parallel() t.Parallel()
+1 -2
View File
@@ -14,7 +14,6 @@ const (
// short, so a slowloris client dribbling headers is dropped well // short, so a slowloris client dribbling headers is dropped well
// before it ties up a connection for the whole ReadTimeout window. // before it ties up a connection for the whole ReadTimeout window.
HTTPReadHeaderTimeout = 10 * time.Second HTTPReadHeaderTimeout = 10 * time.Second
HTTPWriteTimeout = 60 * time.Second
// HTTPIdleTimeout bounds how long an idle keep-alive connection is // HTTPIdleTimeout bounds how long an idle keep-alive connection is
// held open, so idle connections cannot accumulate without limit on a // held open, so idle connections cannot accumulate without limit on a
// service targeting high concurrency. // service targeting high concurrency.
@@ -30,7 +29,7 @@ func (s *Server) newHTTPServer() *http.Server {
Addr: fmt.Sprintf(":%d", s.config.Port), Addr: fmt.Sprintf(":%d", s.config.Port),
ReadTimeout: HTTPReadTimeout, ReadTimeout: HTTPReadTimeout,
ReadHeaderTimeout: HTTPReadHeaderTimeout, ReadHeaderTimeout: HTTPReadHeaderTimeout,
WriteTimeout: HTTPWriteTimeout, WriteTimeout: s.config.DownstreamTimeout,
IdleTimeout: HTTPIdleTimeout, IdleTimeout: HTTPIdleTimeout,
MaxHeaderBytes: HTTPMaxHeaderBytes, MaxHeaderBytes: HTTPMaxHeaderBytes,
Handler: s, Handler: s,
+7 -3
View File
@@ -11,11 +11,15 @@ import (
// carries every hardening timeout wired onto it, including the slowloris // carries every hardening timeout wired onto it, including the slowloris
// defense (ReadHeaderTimeout) and the keep-alive bound (IdleTimeout). This // defense (ReadHeaderTimeout) and the keep-alive bound (IdleTimeout). This
// guards against a field being defined but never set on the server, so // guards against a field being defined but never set on the server, so
// each assertion compares the server field to its constant. // each assertion compares the server field to its constant, or, for
// WriteTimeout, to downstream_timeout from the config.
func TestNewHTTPServerTimeouts(t *testing.T) { func TestNewHTTPServerTimeouts(t *testing.T) {
t.Parallel() t.Parallel()
s := &Server{config: &config.Config{Port: 8080}} s := &Server{config: &config.Config{
Port: 8080,
DownstreamTimeout: 45 * time.Second,
}}
srv := s.newHTTPServer() srv := s.newHTTPServer()
@@ -26,7 +30,7 @@ func TestNewHTTPServerTimeouts(t *testing.T) {
}{ }{
{"ReadTimeout", srv.ReadTimeout, HTTPReadTimeout}, {"ReadTimeout", srv.ReadTimeout, HTTPReadTimeout},
{"ReadHeaderTimeout", srv.ReadHeaderTimeout, HTTPReadHeaderTimeout}, {"ReadHeaderTimeout", srv.ReadHeaderTimeout, HTTPReadHeaderTimeout},
{"WriteTimeout", srv.WriteTimeout, HTTPWriteTimeout}, {"WriteTimeout", srv.WriteTimeout, 45 * time.Second},
{"IdleTimeout", srv.IdleTimeout, HTTPIdleTimeout}, {"IdleTimeout", srv.IdleTimeout, HTTPIdleTimeout},
} }
@@ -51,11 +51,12 @@ func newTestServer(t *testing.T) *Server {
stateDir := t.TempDir() stateDir := t.TempDir()
cfg := &config.Config{ cfg := &config.Config{
Debug: true, Debug: true,
SigningKey: testSigningKey, SigningKey: testSigningKey,
StateDir: stateDir, StateDir: stateDir,
DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"), DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"),
TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}, TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
DownstreamTimeout: config.DefaultDownstreamTimeout,
} }
lc := fxtest.NewLifecycle(t) lc := fxtest.NewLifecycle(t)
+1 -1
View File
@@ -33,7 +33,7 @@ func (s *Server) SetupRoutes() {
} }
s.router.Use(s.mw.CORS()) s.router.Use(s.mw.CORS())
s.router.Use(middleware.Timeout(HTTPWriteTimeout)) s.router.Use(middleware.Timeout(s.config.DownstreamTimeout))
if s.sentryEnabled { if s.sentryEnabled {
sentryHandler := sentryhttp.New(sentryhttp.Options{ sentryHandler := sentryhttp.New(sentryhttp.Options{