2 Commits

Author SHA1 Message Date
clawbot
9b3baec214 Correct release-blocking documentation inaccuracies (closes #141)
Some checks failed
check / check (push) Has been cancelled
Four defects found by the integration review, each of which would have
made the README or the release notes untrue at the moment of tagging.

RETENTION_SWEEP_INTERVAL was absent from the README env table while two
other passages referred to it as documented. Enumerated every variable
read by internal/config from the source (12 in total) rather than by
eye; that was the only one missing.

TODO.md omitted five of the units landed in this milestone (#64, #79,
#90, #113, #118), two of them credential-exposure fixes, which are
precisely the entries a reader of the release notes wants to find. The
list is now derived from git log origin/main..origin/next.

The README sold Replay in the present tense as a core capability while
no redelivery code exists anywhere in the tree, and the roadmap entry
for it had been dropped without it being implemented. Every mention of
replay or redelivery in the README is now either marked planned or
already under a Planned heading: the Rationale item, the Use Cases
bullet, the Event model's field description, and the delivery-semantics
passage on target-type edits, which told an operator that a terminally
failed delivery was recoverable by hand when nothing can recover it.
The comment on failUnretryableRetry made the same claim and is
corrected with it. The roadmap entry is back in TODO.md.

The production JS asset shipped a console.log on load. The rest of the
file and every other shipped asset were checked; that was the only one
(alpine.min.js is vendored and untouched).

No behavioural change: the only non-comment, non-documentation edit is
the deleted console.log.
2026-08-12 10:56:22 +00:00
3941f0b0ff Require a positive RETENTION_SWEEP_INTERVAL (closes #140)
All checks were successful
check / check (push) Successful in 6s
A non-positive value reached time.NewTicker in the retention reaper and the archive sweeper, panicking both goroutines after startup had already reported success. envPositiveDuration now rejects it in loadFromEnv, matching how PORT and RECEIVER_RATE_LIMIT fail. SESSION_IDLE_TIMEOUT keeps treating non-positive as disabled, which is guarded at every use site.
2026-08-12 12:46:39 +02:00
6 changed files with 125 additions and 16 deletions

View File

@@ -93,6 +93,7 @@ TTY detection, and security headers are always applied.
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` | | `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` | | `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
| `SENTRY_DSN` | Sentry error reporting DSN | `""` | | `SENTRY_DSN` | Sentry error reporting DSN | `""` |
| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` | | `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` | | `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` |
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) | | `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) |
@@ -173,8 +174,12 @@ its value and refuses to start, rather than silently running with a
substituted default. `PORT=eighty`, `DEBUG=ture`, and substituted default. `PORT=eighty`, `DEBUG=ture`, and
`RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must `RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must
additionally be a number in the range 165535, additionally be a number in the range 165535,
`RECEIVER_RATE_LIMIT` must be at least 1, and every entry in `RECEIVER_RATE_LIMIT` must be at least 1,
`TRUSTED_PROXIES` must be a CIDR block or a bare IP address. `RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker
period, so `0s` or a negative value would crash the reaper after
startup), and every entry in `TRUSTED_PROXIES` must be a CIDR block or
a bare IP address. `SESSION_IDLE_TIMEOUT` is the exception: a
non-positive value there means idle expiry is disabled, not invalid.
Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`, spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
@@ -260,9 +265,10 @@ webhooker solves this by acting as a durable intermediary:
targets simultaneously. This enables patterns like forwarding a targets simultaneously. This enables patterns like forwarding a
GitHub webhook to both a deployment service and a Slack channel. GitHub webhook to both a deployment service and a Slack channel.
5. **Replay** — Stored events can be manually redelivered for debugging 5. **Replay** (not yet implemented) — Every received event is stored in
or testing, without requiring the original sender to fire the webhook full, which is what manual redelivery for debugging or testing will
again. be built on. No redelivery exists today, in the web UI or the API;
see [TODO.md](TODO.md).
### Use Cases ### Use Cases
@@ -272,6 +278,7 @@ webhooker solves this by acting as a durable intermediary:
size, and delivery performance size, and delivery performance
- **Debugging** and introspection of webhook payloads in the web UI - **Debugging** and introspection of webhook payloads in the web UI
- **Replay** of webhook events for application testing and development - **Replay** of webhook events for application testing and development
(planned; not yet implemented)
- **Fan-out** delivery of a single webhook to multiple downstream - **Fan-out** delivery of a single webhook to multiple downstream
targets targets
- **High-availability ingestion** for delivery to less reliable backend - **High-availability ingestion** for delivery to less reliable backend
@@ -497,7 +504,7 @@ A programmatic access credential for API authentication.
#### Event #### Event
A captured incoming webhook request. Stores the complete HTTP request A captured incoming webhook request. Stores the complete HTTP request
data for replay and auditing. data for auditing and for the planned replay capability.
| Field | Type | Description | | Field | Type | Description |
| -------------- | ------ | ----------- | | -------------- | ------ | ----------- |
@@ -779,9 +786,10 @@ unknown) one while one of its deliveries is still `retrying`, both
recovery paths above terminally mark that delivery `failed` and record a recovery paths above terminally mark that delivery `failed` and record a
`DeliveryResult` naming the current target type as the reason, logging it `DeliveryResult` naming the current target type as the reason, logging it
at warn level. The delivery is not re-dispatched under the new type — the at warn level. The delivery is not re-dispatched under the new type — the
operator never asked for that delivery — and the event itself remains operator never asked for that delivery — and while the event itself
stored in the per-webhook event database, so it can be redelivered remains stored in the per-webhook event database, there is no way to
manually. redeliver it: manual redelivery is planned, not implemented (see
[TODO.md](TODO.md)).
### Circuit Breaker (HTTP Targets with Retries) ### Circuit Breaker (HTTP Targets with Retries)

31
TODO.md
View File

@@ -61,6 +61,34 @@ rests on.
Profile settings placeholder removed, a progressive-enhancement copy Profile settings placeholder removed, a progressive-enhancement copy
button for the entrypoint URL, and retention form copy that states the button for the entrypoint URL, and retention form copy that states the
actual policy (deletion by the reaper, 0 retains forever) (#57) actual policy (deletion by the reaper, 0 retains forever) (#57)
- 2026-08-11 Mask the webhook credential in delivery errors and logs:
Go embeds the request URL in `*url.Error`, so every transport failure
persisted the full Slack webhook URL into the per-webhook event
database via `DeliveryResult.Error`, a field a future REST API would
have served. `maskURLError` drops path, query and userinfo while
preserving the wrapped cause, so `errors.Is`/`As` and `Timeout()`
still work and DNS, TLS and timeout failures still read differently
(#118)
- 2026-08-11 Rate-limit the public webhook receiver endpoint
(`RECEIVER_RATE_LIMIT`, default 120/min), keyed on client IP plus
entrypoint path so one entrypoint cannot exhaust another's budget;
over-limit requests get 429 with `Retry-After`. It was the one
unauthenticated, internet-facing endpoint with no limit at all (#64)
- 2026-08-11 Enforce the body size limit before CSRF parses the form:
`MaxBodySize` is now first in all four form-parsing route groups, so
an oversized request is rejected with 413 instead of being read in
full by the CSRF middleware before any cap applied (#90)
- 2026-08-11 Mask target config on the source detail page, which
rendered the stored blob verbatim and so exposed the Slack
incoming-webhook URL — a bearer credential that cannot be revoked
per-holder. Config reaches the template only as a `TargetView` of
labelled fields, and header values are rendered as a count (#113)
- 2026-08-11 Allow `retention_days` of 0 to mean retain forever, via a
sentinel written in `BeforeSave` so the GORM column default cannot
win the race. Also bounds the reaper's cutoff arithmetic: day counts
above 106751 overflowed `time.Duration` and wrapped the cutoff into
the future, where every row matched and the sweep deleted everything
(#79)
- 2026-08-09 Inactivity-based session timeout: sliding idle expiry - 2026-08-09 Inactivity-based session timeout: sliding idle expiry
(`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated (`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
requests, with the 7-day absolute cap kept as an independent requests, with the 7-day absolute cap kept as an independent
@@ -115,6 +143,9 @@ rests on.
# Future Steps # Future Steps
- Manual event redelivery from the web UI — the "Replay" capability the
README describes as planned. No redelivery code exists anywhere in the
tree; events are stored in full, which is all it would be built on
- Delivery status and retry management UI - Delivery status and retry management UI
- Per-webhook rate limiting in the receiver handler (per-webhook config - Per-webhook rate limiting in the receiver handler (per-webhook config
plus handler enforcement; global limits must not apply to receiver plus handler enforcement; global limits must not apply to receiver

View File

@@ -92,6 +92,7 @@ type Config struct {
SentryDSN string SentryDSN string
// RetentionSweepInterval is how often the retention reaper runs. // RetentionSweepInterval is how often the retention reaper runs.
// Always positive: it becomes a time.NewTicker period.
RetentionSweepInterval time.Duration RetentionSweepInterval time.Duration
// SessionIdleTimeout is the sliding inactivity window after // SessionIdleTimeout is the sliding inactivity window after
@@ -235,6 +236,34 @@ func envDuration(
return d, nil return d, nil
} }
// envPositiveDuration returns the value of the named environment
// variable parsed as a Go duration that must be greater than zero.
// Returns defaultValue if not set. A set value that is unparseable or
// non-positive is a hard error naming the key and the bad value.
//
// This is for durations that reach time.NewTicker, which panics on a
// non-positive period, in a goroutine started after startup has
// already reported success. It is deliberately not used for durations
// where non-positive means "disabled" (SESSION_IDLE_TIMEOUT).
func envPositiveDuration(
key string,
defaultValue time.Duration,
) (time.Duration, error) {
d, err := envDuration(key, defaultValue)
if err != nil {
return 0, err
}
if d <= 0 {
return 0, fmt.Errorf(
"%w: %s must be greater than zero, got %s",
ErrNonPositiveValue, key, d,
)
}
return d, nil
}
// parseCIDR parses one trusted-proxy list entry, which may be a // parseCIDR parses one trusted-proxy list entry, which may be a
// CIDR block ("10.0.0.0/8") or a bare address ("10.0.0.1", treated // CIDR block ("10.0.0.0/8") or a bare address ("10.0.0.1", treated
// as a single-host block). // as a single-host block).
@@ -346,7 +375,7 @@ func loadFromEnv() (*Config, error) {
return nil, err return nil, err
} }
retentionSweepInterval, err := envDuration( retentionSweepInterval, err := envPositiveDuration(
"RETENTION_SWEEP_INTERVAL", "RETENTION_SWEEP_INTERVAL",
defaultRetentionSweepInterval, defaultRetentionSweepInterval,
) )
@@ -354,6 +383,8 @@ func loadFromEnv() (*Config, error) {
return nil, err return nil, err
} }
// Non-positive is "disabled" here, not invalid, so this stays on
// envDuration.
sessionIdleTimeout, err := envDuration( sessionIdleTimeout, err := envDuration(
"SESSION_IDLE_TIMEOUT", "SESSION_IDLE_TIMEOUT",
defaultSessionIdleTimeout, defaultSessionIdleTimeout,

View File

@@ -139,7 +139,11 @@ func TestRetentionSweepInterval(t *testing.T) {
set bool set bool
value string value string
expectError bool expectError bool
expected time.Duration // sentinel, when set, must be wrapped by the startup
// error; every error case must additionally name the
// variable in its message.
sentinel error
expected time.Duration
}{ }{
{ {
name: caseUnsetUsesDefault, name: caseUnsetUsesDefault,
@@ -158,6 +162,24 @@ func TestRetentionSweepInterval(t *testing.T) {
value: "not-a-duration", value: "not-a-duration",
expectError: true, expectError: true,
}, },
{
// A non-positive period panics the ticker in the
// reaper and archive-sweeper goroutines, long after
// startup has reported success, so it has to fail
// here instead.
name: "zero fails startup",
set: true,
value: "0s",
expectError: true,
sentinel: config.ErrNonPositiveValue,
},
{
name: "negative fails startup",
set: true,
value: "-1h",
expectError: true,
sentinel: config.ErrNonPositiveValue,
},
} }
for _, tt := range tests { for _, tt := range tests {
@@ -175,7 +197,9 @@ func TestRetentionSweepInterval(t *testing.T) {
} }
if tt.expectError { if tt.expectError {
expectStartupError(t) expectStartupErrorFor(
t, "RETENTION_SWEEP_INTERVAL", tt.sentinel,
)
} else { } else {
testRetentionSweepIntervalSuccess(t, tt.expected) testRetentionSweepIntervalSuccess(t, tt.expected)
} }
@@ -281,6 +305,22 @@ func TestSessionIdleTimeout(t *testing.T) {
value: "not-a-duration", value: "not-a-duration",
expectError: true, expectError: true,
}, },
{
// Non-positive is "idle expiry disabled" for this
// variable, not a configuration error: unlike
// RETENTION_SWEEP_INTERVAL it never becomes a ticker
// period.
name: "zero disables idle expiry",
set: true,
value: "0s",
expected: 0,
},
{
name: "negative disables idle expiry",
set: true,
value: "-1h",
expected: -time.Hour,
},
} }
for _, tt := range tests { for _, tt := range tests {

View File

@@ -785,9 +785,9 @@ func (e *Engine) sweepSingleRetry(
// status retrying themselves. Re-dispatching under the new type // status retrying themselves. Re-dispatching under the new type
// would be a delivery the operator never asked for, and leaving // would be a delivery the operator never asked for, and leaving
// the row retrying strands it forever, so the delivery is // the row retrying strands it forever, so the delivery is
// failed with a recorded reason and can be redelivered // failed with a recorded reason. The event stays stored, but
// manually. Logged at warn, not error: this is operator-caused // nothing redelivers it today. Logged at warn, not error: this
// state, not a system fault. // is operator-caused state, not a system fault.
func (e *Engine) failUnretryableRetry( func (e *Engine) failUnretryableRetry(
webhookDB *gorm.DB, webhookDB *gorm.DB,
webhookID string, webhookID string,

View File

@@ -1,5 +1,4 @@
// Webhooker client-side JavaScript // Webhooker client-side JavaScript
console.log("Webhooker loaded");
// Copy-to-clipboard, as progressive enhancement. // Copy-to-clipboard, as progressive enhancement.
// //