max_retries is unbounded at target creation, and unparseable input silently becomes 0 #221

New Issue

clawbot · 2026-08-20T06:23:21+02:00

clawbot commented

2026-08-20 06:23:21 +02:00

Found while implementing #202, with a second defect added from its review.

Two problems in the same helper, parseNonNegativeInt (internal/handlers/source_management.go:1183-1194), which HandleTargetCreate uses for max_retries:

1. No ceiling. Any non-negative value is accepted. parseRetentionDays in the same file shows the intended shape: validate against a stated ceiling and answer 400 naming it. A typo'd extra zero gives MaxRetries of 100000 and the engine will genuinely record that many delivery_results rows, each of which the event log then loads and renders.

2. Unparseable input silently returns 0. This is the silent-fallback pattern the owner rejected on #78 — a set-but-invalid value must fail loudly, never degrade into a default the operator did not ask for. Here a garbage max_retries quietly means "never retry", which is the opposite of what someone typing a number into that field intends, and nothing tells them.

It is operator-chosen rather than sender-chosen, so it is a robustness bug rather than a 1.0 blocker — an operator can only shoot themselves.

Definition of done:

max_retries is validated at creation and edit against a stated ceiling, rejected with 400 naming the ceiling
unparseable input is rejected with 400, never coerced to 0; audit the other callers of parseNonNegativeInt for the same coercion and fix or justify each
the same validation applies on the target edit path added by #127, so the two cannot disagree
existing stored values above the ceiling still render and still deliver; this is input validation, not a migration
tests cover a form post above the ceiling and an unparseable one, each rejected with the reason named

Found while implementing https://git.eeqj.de/sneak/webhooker/issues/202, with a second defect added from its review. Two problems in the same helper, `parseNonNegativeInt` (`internal/handlers/source_management.go:1183-1194`), which `HandleTargetCreate` uses for `max_retries`: **1. No ceiling.** Any non-negative value is accepted. `parseRetentionDays` in the same file shows the intended shape: validate against a stated ceiling and answer 400 naming it. A typo'd extra zero gives `MaxRetries` of 100000 and the engine will genuinely record that many `delivery_results` rows, each of which the event log then loads and renders. **2. Unparseable input silently returns 0.** This is the silent-fallback pattern the owner rejected on https://git.eeqj.de/sneak/webhooker/pulls/78 — a set-but-invalid value must fail loudly, never degrade into a default the operator did not ask for. Here a garbage `max_retries` quietly means "never retry", which is the opposite of what someone typing a number into that field intends, and nothing tells them. It is operator-chosen rather than sender-chosen, so it is a robustness bug rather than a 1.0 blocker — an operator can only shoot themselves. Definition of done: - `max_retries` is validated at creation and edit against a stated ceiling, rejected with 400 naming the ceiling - unparseable input is rejected with 400, never coerced to 0; audit the other callers of `parseNonNegativeInt` for the same coercion and fix or justify each - the same validation applies on the target edit path added by https://git.eeqj.de/sneak/webhooker/issues/127, so the two cannot disagree - existing stored values above the ceiling still render and still deliver; this is input validation, not a migration - tests cover a form post above the ceiling and an unparseable one, each rejected with the reason named

clawbot changed title from ~~max_retries is unbounded at target creation, so one target can queue arbitrarily many delivery attempts~~ to max_retries is unbounded at target creation, and unparseable input silently becomes 0

2026-08-20 06:38:28 +02:00

clawbot referenced this issue

2026-08-20 07:05:24 +02:00

Render delivery attempt detail in the event log (closes #202) #219

clawbot referenced this issue

2026-08-20 07:14:34 +02:00

Render delivery attempt detail in the event log (closes #202) #219

clawbot referenced this issue

2026-08-20 07:23:17 +02:00

Add a target edit form and reachable header/timeout fields (closes #127) #229

Sign in to join this conversation.