RETENTION_SWEEP_INTERVAL is not range-checked, so a non-positive value panics two goroutines after startup reports success #140

New Issue

clawbot · 2026-08-12T12:30:00+02:00

clawbot commented

2026-08-12 12:30:00 +02:00

Found by the integration review of #111. Blocks the 1.0.0 tag: it is a crash-on-misconfiguration of exactly the kind #80 was meant to eliminate.

envDuration (internal/config/config.go:219) accepts 0s and -1h. That value feeds time.NewTicker in two places:

internal/database/retention.go:122
internal/delivery/archive_sweeper.go:130 — this second consumer is new in this milestone

time.NewTicker panics on a non-positive duration, and both calls run in goroutines with no recover. So the process logs "Configuration loaded", reports a successful startup, and then aborts.

PORT and RECEIVER_RATE_LIMIT both got lower bounds under #80. This variable was missed, and the milestone then added a second consumer of it.

Definition of done

A non-positive RETENTION_SWEEP_INTERVAL is rejected in loadFromEnv and aborts startup with an error naming the variable, consistent with how PORT and RECEIVER_RATE_LIMIT fail.
Config tests covering 0s and a negative value, asserting the failure names the variable and wraps the same sentinel error the sibling variables use.
Audit every other envDuration call site for the same gap and state the result. If another duration reaches a NewTicker, NewTimer or similar without a bound, fix it here.

Implementation requirements

Branch from next, PR based on next, single commit, title ending (closes #N).
Do not modify TODO.md.
Gate on make check plus the Docker lint path with the cache defeated (#119).

Found by the integration review of https://git.eeqj.de/sneak/webhooker/pulls/111. Blocks the 1.0.0 tag: it is a crash-on-misconfiguration of exactly the kind https://git.eeqj.de/sneak/webhooker/issues/80 was meant to eliminate. `envDuration` (`internal/config/config.go:219`) accepts `0s` and `-1h`. That value feeds `time.NewTicker` in two places: - `internal/database/retention.go:122` - `internal/delivery/archive_sweeper.go:130` — this second consumer is new in this milestone `time.NewTicker` panics on a non-positive duration, and both calls run in goroutines with no recover. So the process logs "Configuration loaded", reports a successful startup, and then aborts. `PORT` and `RECEIVER_RATE_LIMIT` both got lower bounds under https://git.eeqj.de/sneak/webhooker/issues/80. This variable was missed, and the milestone then added a second consumer of it. ## Definition of done - A non-positive `RETENTION_SWEEP_INTERVAL` is rejected in `loadFromEnv` and aborts startup with an error naming the variable, consistent with how `PORT` and `RECEIVER_RATE_LIMIT` fail. - Config tests covering `0s` and a negative value, asserting the failure names the variable and wraps the same sentinel error the sibling variables use. - Audit every other `envDuration` call site for the same gap and state the result. If another duration reaches a `NewTicker`, `NewTimer` or similar without a bound, fix it here. ## Implementation requirements - Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`. - Do not modify `TODO.md`. - Gate on `make check` plus the Docker lint path with the cache defeated (https://git.eeqj.de/sneak/webhooker/issues/119).

clawbot added this to the 1.0.0 milestone 2026-08-12 12:30:00 +02:00

clawbot self-assigned this 2026-08-12 12:30:00 +02:00

clawbot referenced this issue from a commit

2026-08-12 12:37:55 +02:00

Require a positive RETENTION_SWEEP_INTERVAL (closes #140)

clawbot referenced a pull request that will close this issue

2026-08-12 12:38:20 +02:00

Require a positive RETENTION_SWEEP_INTERVAL (closes #140) #142

~~clawbot referenced this issue 2026-08-12 12:38:21 +02:00~~

Bound the receiver rate limit per client IP across /webhook/* (closes #139) #143

clawbot commented

2026-08-12 12:38:29 +02:00

Fixed in #142.

RETENTION_SWEEP_INTERVAL now goes through a new envPositiveDuration, which wraps envDuration the way envPort wraps envPositiveInt and rejects <= 0 with an error wrapping ErrNonPositiveValue that names the variable — the same failure shape as PORT and RECEIVER_RATE_LIMIT.

Audit: there are exactly two envDuration call sites. RETENTION_SWEEP_INTERVAL (ticker in both internal/database/retention.go:122 and internal/delivery/archive_sweeper.go:130) is fixed. SESSION_IDLE_TIMEOUT reaches no ticker or timer — it becomes session.idleTimeout, guarded by if s.idleTimeout <= 0 at both use sites, where non-positive is the documented "disabled" state — so it stays on envDuration. The other NewTicker/AfterFunc/WithTimeout sites use constants or a computed backoff, none environment-configurable.

Verified: make check exit 0 with the new subtests observed running, and docker build --no-cache-filter=lint,builder exit 0 with the lint and builder stages confirmed executed rather than CACHED (0 issues., and the new subtests passing inside the container).

Fixed in https://git.eeqj.de/sneak/webhooker/pulls/142. `RETENTION_SWEEP_INTERVAL` now goes through a new `envPositiveDuration`, which wraps `envDuration` the way `envPort` wraps `envPositiveInt` and rejects `<= 0` with an error wrapping `ErrNonPositiveValue` that names the variable — the same failure shape as `PORT` and `RECEIVER_RATE_LIMIT`. Audit: there are exactly two `envDuration` call sites. `RETENTION_SWEEP_INTERVAL` (ticker in both `internal/database/retention.go:122` and `internal/delivery/archive_sweeper.go:130`) is fixed. `SESSION_IDLE_TIMEOUT` reaches no ticker or timer — it becomes `session.idleTimeout`, guarded by `if s.idleTimeout <= 0` at both use sites, where non-positive is the documented "disabled" state — so it stays on `envDuration`. The other `NewTicker`/`AfterFunc`/`WithTimeout` sites use constants or a computed backoff, none environment-configurable. Verified: `make check` exit 0 with the new subtests observed running, and `docker build --no-cache-filter=lint,builder` exit 0 with the `lint` and `builder` stages confirmed executed rather than `CACHED` (`0 issues.`, and the new subtests passing inside the container).

clawbot closed this issue

2026-08-12 12:46:41 +02:00

clawbot referenced this issue from a commit

2026-08-12 12:46:42 +02:00

Require a positive RETENTION_SWEEP_INTERVAL (closes #140)

clawbot referenced this issue

2026-08-12 13:13:51 +02:00

Correct release-blocking documentation inaccuracies (closes #141) #144

clawbot referenced this issue from a commit

2026-08-12 13:21:25 +02:00

Record the last four milestone units in TODO.md

clawbot referenced this issue