P1: four config keys documented in README do not exist and now abort startup #61

New Issue

clawbot · 2026-08-09T03:43:07+02:00

clawbot commented

2026-08-09 03:43:07 +02:00

Verified against main at 61f42e6.

README.md:110-119 presents these as "Key settings" and says "See config.example.yml for all options with defaults":

access_control_allow_origin — CORS origin
upstream_fetch_timeout — timeout for origin requests
upstream_max_response_size — max origin response size
downstream_timeout — client response timeout

None of the four exist. isKnownConfigKey (internal/config/config.go:217-227) permits only: debug, maintenance_mode, port, state_dir, sentry_dsn, db_url, metrics, signing_key, allowlist_hosts, allow_http, upstream_connections_per_host, env.

Since #52/#53 landed strict unknown-key rejection, a user who follows the README does not get a silently-ignored key — the daemon refuses to start with unknown config keys: .... That is the correct behavior for an unknown key (repo policy: set-but-unparseable config must abort), but it means our own documentation is a startup-breaking trap.

The underlying values all exist, hardcoded:

CORS origin: []string{"*"} at internal/middleware/middleware.go:114
Fetch timeout / max response size: DefaultFetchTimeout / DefaultMaxResponseSize at internal/httpfetcher/httpfetcher.go:22-23; internal/handlers/handlers.go:76-80 currently overrides only AllowHTTP and MaxConnectionsPerHost
Downstream timeout: HTTPWriteTimeout at internal/server/http.go:12

access_control_allow_origin defaulting to * is itself worth a look while implementing — a wide-open CORS default on an image proxy is defensible but should be a deliberate, documented choice rather than an accident.

Definition of done

All four keys implemented: added to the Config struct, to isKnownConfigKey, parsed with the strict getters (a set-but-invalid value must abort startup naming the key and the offending value, per repo policy — no silent fallback), and plumbed to the code that currently hardcodes each value.
Durations and byte sizes parse consistently with how the rest of the config handles those types; if no precedent exists, pick one and document it in config.example.yml.
Each key is listed in config.example.yml with its default and a comment.
README's config section stays accurate (and the "all options with defaults" claim becomes true — see the separate docs issue for the remaining undocumented keys).
Failing tests first: for each key, a test that a valid value takes effect and an invalid value aborts startup naming key and value, following the table-driven pattern in internal/config/config_validation_internal_test.go.
make check green.

Scope note

If this is more comfortable as four commits, split by key — but keep it one PR, since the shared work is the strict-getter plumbing.

Verified against `main` at `61f42e6`. `README.md:110-119` presents these as "Key settings" and says "See `config.example.yml` for all options with defaults": - `access_control_allow_origin` — CORS origin - `upstream_fetch_timeout` — timeout for origin requests - `upstream_max_response_size` — max origin response size - `downstream_timeout` — client response timeout None of the four exist. `isKnownConfigKey` (`internal/config/config.go:217-227`) permits only: `debug`, `maintenance_mode`, `port`, `state_dir`, `sentry_dsn`, `db_url`, `metrics`, `signing_key`, `allowlist_hosts`, `allow_http`, `upstream_connections_per_host`, `env`. Since #52/#53 landed strict unknown-key rejection, a user who follows the README does not get a silently-ignored key — **the daemon refuses to start** with `unknown config keys: ...`. That is the correct behavior for an unknown key (repo policy: set-but-unparseable config must abort), but it means our own documentation is a startup-breaking trap. The underlying values all exist, hardcoded: - CORS origin: `[]string{"*"}` at `internal/middleware/middleware.go:114` - Fetch timeout / max response size: `DefaultFetchTimeout` / `DefaultMaxResponseSize` at `internal/httpfetcher/httpfetcher.go:22-23`; `internal/handlers/handlers.go:76-80` currently overrides only `AllowHTTP` and `MaxConnectionsPerHost` - Downstream timeout: `HTTPWriteTimeout` at `internal/server/http.go:12` `access_control_allow_origin` defaulting to `*` is itself worth a look while implementing — a wide-open CORS default on an image proxy is defensible but should be a deliberate, documented choice rather than an accident. ## Definition of done 1. All four keys implemented: added to the `Config` struct, to `isKnownConfigKey`, parsed with the **strict** getters (a set-but-invalid value must abort startup naming the key and the offending value, per repo policy — no silent fallback), and plumbed to the code that currently hardcodes each value. 2. Durations and byte sizes parse consistently with how the rest of the config handles those types; if no precedent exists, pick one and document it in `config.example.yml`. 3. Each key is listed in `config.example.yml` with its default and a comment. 4. README's config section stays accurate (and the "all options with defaults" claim becomes true — see the separate docs issue for the remaining undocumented keys). 5. Failing tests first: for each key, a test that a valid value takes effect and an invalid value aborts startup naming key and value, following the table-driven pattern in `internal/config/config_validation_internal_test.go`. 6. `make check` green. ## Scope note If this is more comfortable as four commits, split by key — but keep it one PR, since the shared work is the strict-getter plumbing.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:43:07 +02:00

clawbot referenced this issue

2026-08-09 07:04:11 +02:00

CORS wildcard is applied at the router root, covering the cookie-authenticated routes #98

clawbot referenced this issue

2026-08-09 16:10:39 +02:00

Tracking: 1.0.0 sequencing and critical path #103

Sign in to join this conversation.