Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3b1029f65c |
@@ -1235,34 +1235,36 @@ markup. The CSP build runs no expressions, so every Alpine directive in
|
||||
`static/js/app.js`: `x-data="collapsible"` and `@click="toggle"`, never
|
||||
`x-data="{ open: false }"` or `@click="open = !open"`.
|
||||
|
||||
A browser test in `internal/server` loads the webhook page, its edit pages and
|
||||
the event log under the real policy and checks that: the add entrypoint form
|
||||
stays hidden until Add is clicked; for every target type, the targets section's
|
||||
Add shows only a choice of type with Next and Cancel, Next shows only that
|
||||
type's fields (no url field for `database` or `log`), Cancel at either step
|
||||
closes the form, and saving adds the target; a refused target comes back with
|
||||
its form open, the values entered and the reason, and after Cancel the next Add
|
||||
starts with an empty form and no reason; a refused save on the target edit page
|
||||
and on the webhook edit page comes back with the reason and every value entered;
|
||||
the Copy button beside an entrypoint URL reads "Copied" once clicked; an
|
||||
entrypoint's Edit button shows its edit form in place of its description and
|
||||
hides until the form closes, Cancel hides the form and drops what was typed, as
|
||||
does leaving the page and going back to it, and Save changes the description; of
|
||||
the recent events on the webhook page only the newest starts expanded, each
|
||||
expands and collapses, and Open leads to the event's own page; of the events in
|
||||
the event log only the newest starts expanded, and an event there expands and
|
||||
collapses when its row's caret or its ID is clicked, and from the keyboard, but
|
||||
not when its ID is selected with the mouse, and a delivery's attempts inside it
|
||||
expand and collapse; and at phone width the menu button opens and closes the
|
||||
mobile menu, and neither the webhook page nor the event log, with a delivery's
|
||||
attempts open, scrolls sideways or cuts anything off at the page's or a card's
|
||||
edge. It also fails if the browser reports a console warning or error, an
|
||||
uncaught exception, or anything the policy refused. `make check` and the image
|
||||
build lint it but do not run it, and `make test` leaves it out (its file is
|
||||
built only with the `browser` build tag). Run it with `make test-browser` after
|
||||
changing `templates/` or `static/js/`: that builds `Dockerfile.browser`, which
|
||||
runs the test in a digest-pinned headless browser image, so the host needs no
|
||||
browser.
|
||||
A browser test in `internal/server` loads the webhook page, its edit pages, the
|
||||
event log, the webhook list, the settings page and the profile page under the
|
||||
real policy and checks that: the add entrypoint form stays hidden until Add is
|
||||
clicked; for every target type, the targets section's Add shows only a choice of
|
||||
type with Next and Cancel, Next shows only that type's fields (no url field for
|
||||
`database` or `log`), Cancel at either step closes the form, and saving adds the
|
||||
target; a refused target comes back with its form open, the values entered and
|
||||
the reason, and after Cancel the next Add starts with an empty form and no
|
||||
reason; a refused save on the target edit page and on the webhook edit page
|
||||
comes back with the reason and every value entered; the Copy button beside an
|
||||
entrypoint URL reads "Copied" once clicked; an entrypoint's Edit button shows
|
||||
its edit form in place of its description and hides until the form closes,
|
||||
Cancel hides the form and drops what was typed, as does leaving the page and
|
||||
going back to it, and Save changes the description; of the recent events on the
|
||||
webhook page only the newest starts expanded, each expands and collapses, and
|
||||
Open leads to the event's own page; of the events in the event log only the
|
||||
newest starts expanded, and an event there expands and collapses when its row's
|
||||
caret or its ID is clicked, and from the keyboard, but not when its ID is
|
||||
selected with the mouse, and a delivery's attempts inside it expand and
|
||||
collapse; at phone width the menu button opens and closes the mobile menu, and
|
||||
neither the webhook page nor the event log, with a delivery's attempts open,
|
||||
scrolls sideways or cuts anything off at the page's or a card's edge; and a
|
||||
target's Delete asks first, naming the target, sends nothing when the answer is
|
||||
no and deletes the target when it is yes. It also fails if the browser reports a
|
||||
console warning or error, an uncaught exception, or anything the policy refused.
|
||||
`make check` and the image build lint it but do not run it, and `make test`
|
||||
leaves it out (its file is built only with the `browser` build tag). Run it with
|
||||
`make test-browser` after changing `templates/` or `static/js/`: that builds
|
||||
`Dockerfile.browser`, which runs the test in a digest-pinned headless browser
|
||||
image, so the host needs no browser.
|
||||
|
||||
The package's tarball is committed as `3p/alpinejs-csp-3.14.9.tgz`, byte for
|
||||
byte as the npm registry publishes it. It is a dependency, not this repo's build
|
||||
@@ -1295,7 +1297,9 @@ and commit the regenerated file with the change. Tailwind takes classes only
|
||||
from the files that `input.css` names in its `@source` lines; a class written in
|
||||
any other file is not generated until that file is named there too. `make check`
|
||||
and the image build fail when the committed file differs from what `make css`
|
||||
generates. `static/css/style.css` is hand-written and is not generated.
|
||||
generates. `static/css/style.css` is hand-written and is not generated. The
|
||||
pages' Content-Security-Policy refuses inline styles, so a template styles an
|
||||
element with a class, never a `style` attribute.
|
||||
|
||||
`make css` runs the Tailwind standalone CLI in Docker, at the version and sha256
|
||||
pinned in the Dockerfile's stylesheet stages; it is never installed on the host.
|
||||
@@ -1317,22 +1321,20 @@ webhooker solves this by acting as a durable intermediary:
|
||||
nothing is lost if downstream targets are unavailable.
|
||||
|
||||
2. **Guaranteed delivery** — Events are queued for delivery to each configured
|
||||
target. Failed deliveries are retried with configurable backoff. Delivery
|
||||
attempts are logged with status codes, response bodies, and timing.
|
||||
target. Failed deliveries are retried with configurable backoff. Every
|
||||
delivery attempt is logged with status codes, response bodies, and timing.
|
||||
|
||||
**That guarantee is at-least-once, not exactly-once.** When a send reaches
|
||||
its target but the write recording that outcome fails, the delivery is
|
||||
deliberately left in a recoverable state rather than marked done — losing a
|
||||
delivery is the worse failure — so the pending sweep picks it up about
|
||||
fifteen minutes later, or the next restart does, and the target receives a
|
||||
payload it already got. A send still in flight when the service stops is
|
||||
left the same way: its attempt is not recorded, and the next start sends it
|
||||
again (see Shutdown). webhooker adds no delivery identifier of its own to an
|
||||
outbound request, so **make your receiver idempotent** against whatever the
|
||||
payload itself carries.
|
||||
payload it already got. webhooker adds no delivery identifier of its own to
|
||||
an outbound request, so **make your receiver idempotent** against whatever
|
||||
the payload itself carries.
|
||||
|
||||
3. **Observability** — Full request/response logging for every webhook received
|
||||
and for delivery attempts. Prometheus metrics expose volume, latency, and
|
||||
and every delivery attempted. Prometheus metrics expose volume, latency, and
|
||||
error rates. The web UI provides real-time visibility into event flow.
|
||||
|
||||
4. **Fan-out** — A single incoming webhook can be delivered to multiple targets
|
||||
@@ -1496,35 +1498,47 @@ the rotation chosen below that (none, monthly, daily or hourly). Both are
|
||||
validated as on the add target form, and the webhook and its targets are created
|
||||
together or not at all.
|
||||
|
||||
| Field | Type | Description |
|
||||
| ---------------- | ------- | --------------------------------------------- |
|
||||
| `id` | UUID | Primary key |
|
||||
| `user_id` | UUID | Foreign key → User |
|
||||
| `name` | string | Human-readable name |
|
||||
| `description` | string | Optional description |
|
||||
| `retention_days` | integer | Days to retain events; 0 means retain forever |
|
||||
| Field | Type | Description |
|
||||
| ---------------- | ------- | ----------------------------------------------------------- |
|
||||
| `id` | UUID | Primary key |
|
||||
| `user_id` | UUID | Foreign key → User |
|
||||
| `name` | string | Human-readable name |
|
||||
| `description` | string | Optional description |
|
||||
| `retention_days` | integer | Days to retain events (default: 30; 0 means retain forever) |
|
||||
|
||||
**Relations:** Belongs to User. Has many Entrypoints. Has many Targets.
|
||||
|
||||
The `retention_days` field controls how long event data is kept in the webhook's
|
||||
dedicated database before automatic cleanup.
|
||||
|
||||
The value is stored as entered. Setting `retention_days` to `0` means "retain
|
||||
events forever": the retention reaper skips the webhook entirely, and the web UI
|
||||
displays its retention as "forever" rather than as a day count.
|
||||
Setting `retention_days` to `0` means "retain events forever". Because the
|
||||
column carries a default of 30, a literal zero cannot survive an insert, so a
|
||||
zero is rewritten on save to a sentinel of `365 * 1000` days
|
||||
(`database.RetentionForeverDays`). The retention reaper recognises that sentinel
|
||||
and skips the webhook entirely, and the web UI displays such a webhook's
|
||||
retention as "forever" rather than as a day count.
|
||||
|
||||
The new webhook and edit forms accept `0`, or a whole number of days from `1` up
|
||||
to `database.MaxFiniteRetentionDays` (106751 days, about 292 years). Anything
|
||||
else, a negative value or one above that cap included, is rejected with a 400. A
|
||||
blank field gives the default of 30 days (`database.DefaultRetentionDays`) on
|
||||
the new webhook form, and leaves the stored value unchanged on the edit form.
|
||||
Submitted `retention_days` values therefore fall into three bands, not two:
|
||||
|
||||
- `1` up to `database.MaxFiniteRetentionDays` (106751 days, about 292 years) is
|
||||
accepted as a finite retention.
|
||||
- Above that ceiling but below the retain-forever sentinel of 365000
|
||||
(`database.RetentionForeverDays`) is rejected with a 400. This is the band the
|
||||
cap exists for.
|
||||
- `0`, and `365000` or above, are accepted and mean retain forever, collapsing
|
||||
to the sentinel — `0` in `Webhook.BeforeSave`, the large values in
|
||||
`parseRetentionDays`. The large values are not out of range: the edit form
|
||||
pre-fills the sentinel for a retain-forever webhook, so submitting that form
|
||||
back unchanged has to keep meaning "forever".
|
||||
|
||||
A negative value is in none of the three: `parseRetentionDays` rejects it with a
|
||||
400 before `BeforeSave` ever sees it.
|
||||
|
||||
The cap is not arbitrary: the reaper computes its cutoff as a `time.Duration`,
|
||||
an int64 nanosecond count, and a longer period overflows it. An overflowed
|
||||
cutoff lands in the future, where it matches every row, so the sweep would
|
||||
delete every event the webhook has instead of none. The reaper also clamps the
|
||||
value it reads to the cap, and reads a negative value as forever, so no stored
|
||||
value can trigger that either.
|
||||
value it reads to the cap, so no stored value can trigger that either.
|
||||
|
||||
#### Entrypoint
|
||||
|
||||
@@ -1758,7 +1772,7 @@ is what bounds it.
|
||||
#### DeliveryResult
|
||||
|
||||
The result of a single delivery attempt. Every attempt (including retries) is
|
||||
individually logged, except one cut off by a stop (see Shutdown).
|
||||
individually logged for full observability.
|
||||
|
||||
| Field | Type | Description |
|
||||
| --------------- | ------- | ----------------------------------------- |
|
||||
@@ -2196,16 +2210,12 @@ This means:
|
||||
buffer rather than spawning more goroutines.
|
||||
- **Independent results** — each worker records its own delivery result in the
|
||||
per-webhook database without coordination.
|
||||
- **Graceful shutdown** — the stop hook cancels the workers' context. A worker
|
||||
may still take tasks already queued before it exits. An `http` or `slack`
|
||||
request in flight is cut off, and one started after the cancel fails at once.
|
||||
Neither is recorded or counted in the metrics, and the next start sends the
|
||||
delivery again (see Shutdown). A `database` or `log` task runs as normal. The
|
||||
stop hook waits for the pool via `lifecycle.WaitForShutdown`, which bounds
|
||||
that wait by fx's stop timeout rather than blocking forever on a wedged
|
||||
worker. On timeout it logs at `ERROR` and returns an error, and the goroutines
|
||||
that did not finish are still running — an unclean shutdown is reported rather
|
||||
than hidden.
|
||||
- **Graceful shutdown** — cancel the context, workers finish their current task
|
||||
and exit. The stop hook waits for the pool via `lifecycle.WaitForShutdown`,
|
||||
which bounds that wait by fx's stop timeout rather than blocking forever on a
|
||||
wedged worker. On timeout it logs at `ERROR` and returns an error, and the
|
||||
goroutines that did not finish are still running — an unclean shutdown is
|
||||
reported rather than hidden.
|
||||
|
||||
**Recovery paths:**
|
||||
|
||||
@@ -2311,7 +2321,7 @@ getting anywhere.
|
||||
| Metric | Type | Meaning |
|
||||
| -------------------------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `webhooker_events_received_total` | counter | Events received and durably stored. Compare against the delivery counters on one dashboard |
|
||||
| `webhooker_delivery_attempts_total` | counter | Delivery attempts actually dispatched to a target. A delivery a circuit breaker refused is not one: it is counted as a retry instead, but only when the refusal moves it into `retrying`. An attempt cut off by a stop is not counted (see Shutdown) |
|
||||
| `webhooker_delivery_attempts_total` | counter | Delivery attempts actually dispatched to a target. A delivery a circuit breaker refused is not one: it is counted as a retry instead, but only when the refusal moves it into `retrying` |
|
||||
| `webhooker_deliveries_succeeded_total` | counter | Deliveries that reached `delivered` |
|
||||
| `webhooker_deliveries_failed_total` | counter | Deliveries that failed terminally and will not be retried |
|
||||
| `webhooker_delivery_retries_total` | counter | Deliveries put back into `retrying` |
|
||||
@@ -3070,8 +3080,8 @@ webhooker/
|
||||
│ ├── static.go # //go:embed directive
|
||||
│ ├── css/input.css # Tailwind input, source for tailwind.css (make css)
|
||||
│ ├── css/tailwind.css # Generated stylesheet the pages load
|
||||
│ ├── css/style.css # Hand-written, loaded after tailwind.css: btn-small, the pointer cursor for input.css's buttons, the webhook list cards' focus outline
|
||||
│ ├── js/app.js # Copy-to-clipboard, and the Alpine.js components
|
||||
│ ├── css/style.css # Hand-written, loaded after tailwind.css: btn-small, the pointer cursor for input.css's buttons, the webhook list cards' focus outline, the rule hiding x-cloak elements
|
||||
│ ├── js/app.js # Copy-to-clipboard, the delete prompts, and the Alpine.js components
|
||||
│ └── js/alpine.min.js # Alpine.js CSP build, extracted from 3p/ by make assets, not committed
|
||||
├── templates/ # Go HTML templates (base, login, sources, etc.)
|
||||
├── script/ # Scripts to Rule Them All entrypoints
|
||||
@@ -3125,8 +3135,9 @@ Applied to all routes in this order:
|
||||
|
||||
1. **RequestID** — Generate unique request IDs (chi built-in)
|
||||
2. **SecurityHeaders** — Production security headers on every response (HSTS
|
||||
unless `HSTS_MAX_AGE` is `0`, X-Content-Type-Options, X-Frame-Options, CSP,
|
||||
Referrer-Policy, Permissions-Policy)
|
||||
unless `HSTS_MAX_AGE` is `0`, X-Content-Type-Options, X-Frame-Options, CSP
|
||||
`default-src 'self'; script-src 'self'; style-src 'self'`, Referrer-Policy,
|
||||
Permissions-Policy)
|
||||
3. **Logging** — Structured request logging (method, URL, status, latency,
|
||||
remote IP, client IP, user agent, request ID)
|
||||
4. **Metrics** — Prometheus HTTP metrics (if `METRICS_USERNAME` and
|
||||
@@ -3211,6 +3222,12 @@ credential check, see [The login endpoint](#the-login-endpoint).
|
||||
- Production security headers on all responses: HSTS (unless `HSTS_MAX_AGE` is
|
||||
`0`), X-Content-Type-Options (`nosniff`), X-Frame-Options (`DENY`),
|
||||
Content-Security-Policy, Referrer-Policy, and Permissions-Policy
|
||||
- The Content-Security-Policy is
|
||||
`default-src 'self'; script-src 'self'; style-src 'self'`: a page takes
|
||||
scripts and styles only from files webhooker serves, and the browser runs no
|
||||
inline script or event handler and ignores any `<style>` element or `style`
|
||||
attribute in a page. The pages need none: their scripts are in `static/js/`
|
||||
and their styles in `static/css/`
|
||||
- Request body size limits (1 MB) on all form POST endpoints, enforced by
|
||||
middleware that runs before CSRF parses the form
|
||||
- **CSRF protection** via [gorilla/csrf](https://github.com/gorilla/csrf) on all
|
||||
@@ -3278,8 +3295,7 @@ stop-hook log:
|
||||
3. `server` — the HTTP drain, bounded by `server.ShutdownTimeout` (**3
|
||||
seconds**) and by what the hooks before it left, then a Sentry flush if
|
||||
`SENTRY_DSN` is set
|
||||
4. `delivery.Engine` — cuts off any `http` or `slack` request in flight, waits
|
||||
for its workers, then closes the archive databases
|
||||
4. `delivery.Engine` — waits for its workers, then closes the archive databases
|
||||
5. `healthcheck`
|
||||
6. `WebhookDBManager`
|
||||
7. the database close
|
||||
@@ -3290,12 +3306,6 @@ by the stop context, so a wedge there consumes the 5 seconds before the HTTP
|
||||
server hook is ever entered. The hooks after the server are microsecond-scale in
|
||||
normal operation.
|
||||
|
||||
The delivery engine cancels a delivery request in flight. That attempt is not
|
||||
recorded or counted in the metrics, and the delivery keeps its `pending` or
|
||||
`retrying` status, so startup recovery sends it again on the next start (see
|
||||
Recovery paths under Bounded Worker Pool). If the receiver already had it, it
|
||||
receives it twice: the at-least-once duplicate described under Rationale.
|
||||
|
||||
The HTTP drain budget is deliberately **shorter** than the sequence budget. Were
|
||||
the two equal, a drain that used its whole budget would exhaust the sequence
|
||||
budget at the instant it finished, and every later hook — the delivery engine,
|
||||
|
||||
@@ -4,13 +4,27 @@ import (
|
||||
"math"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
const (
|
||||
// DefaultRetentionDays is the event retention period the new
|
||||
// webhook form applies when its retention field is left blank.
|
||||
// DefaultRetentionDays is the event retention period applied to a
|
||||
// webhook created without an explicit retention value. It is the
|
||||
// single source of truth for that policy and must stay in sync
|
||||
// with the `gorm:"default:30"` column default on
|
||||
// Webhook.RetentionDays below; a struct tag cannot reference a
|
||||
// constant, so a test asserts the two agree.
|
||||
DefaultRetentionDays = 30
|
||||
|
||||
// RetentionForeverDays is the sentinel RetentionDays value meaning
|
||||
// "retain events forever". Users express that intent as 0, which
|
||||
// Webhook.BeforeSave rewrites to this value: the column default
|
||||
// substitutes DefaultRetentionDays for a zero value at insert
|
||||
// time, so a zero can never survive a round trip to the database.
|
||||
// Nothing outside this file may hardcode the number.
|
||||
RetentionForeverDays = 365 * 1000
|
||||
|
||||
// MaxFiniteRetentionDays is the largest finite retention period the
|
||||
// reaper's cutoff arithmetic can represent, and therefore the
|
||||
// largest one a caller may request. It is derived from that
|
||||
@@ -24,7 +38,9 @@ const (
|
||||
// turns the cutoff into a timestamp in the far future that matches
|
||||
// every row in the webhook's database. That is why this bound is
|
||||
// enforced on input and why retentionCutoff saturates underneath
|
||||
// it.
|
||||
// it. Note that RetentionForeverDays deliberately sits above this
|
||||
// ceiling: such webhooks are skipped before any cutoff is
|
||||
// computed, and never reach the arithmetic at all.
|
||||
MaxFiniteRetentionDays = int(
|
||||
math.MaxInt64 / int64(time.Hour) / hoursPerDay,
|
||||
)
|
||||
@@ -32,10 +48,12 @@ const (
|
||||
|
||||
// Webhook represents a webhook processing unit that groups entrypoints and targets
|
||||
//
|
||||
// Every method below takes a pointer receiver, so handlers put a
|
||||
// *Webhook into template data: html/template cannot call a pointer
|
||||
// method on a value held in a map, because a map element is not
|
||||
// addressable.
|
||||
// Every method below takes a pointer receiver. BeforeSave has to,
|
||||
// because it mutates the record and GORM only invokes hooks declared
|
||||
// that way; the display helpers follow suit so the receiver kinds do
|
||||
// not mix. Handlers therefore put a *Webhook into template data:
|
||||
// html/template cannot call a pointer method on a value held in a map,
|
||||
// because a map element is not addressable.
|
||||
type Webhook struct {
|
||||
BaseModel
|
||||
|
||||
@@ -43,9 +61,10 @@ type Webhook struct {
|
||||
Name string `gorm:"not null" json:"name"`
|
||||
Description string `json:"description"`
|
||||
|
||||
// RetentionDays is the number of days to retain events, and 0
|
||||
// means retain them forever.
|
||||
RetentionDays int `json:"retentionDays"`
|
||||
// RetentionDays is the number of days to retain events. A value of
|
||||
// RetentionForeverDays means retain forever. The column default
|
||||
// must equal DefaultRetentionDays.
|
||||
RetentionDays int `gorm:"default:30" json:"retentionDays"`
|
||||
|
||||
// Relations. No model marshals the record it belongs to:
|
||||
// User.Webhooks leads back here, and the JSON could loop.
|
||||
@@ -54,17 +73,36 @@ type Webhook struct {
|
||||
Targets []Target `json:"targets,omitempty"`
|
||||
}
|
||||
|
||||
// retainsForever reports whether a RetentionDays value means "keep
|
||||
// events indefinitely", which 0 does. It is the single definition of
|
||||
// that question, shared by Webhook.RetainsForever and by the reaper's
|
||||
// BeforeSave normalises RetentionDays on every insert and update. A
|
||||
// non-positive value is the user's way of asking for "retain forever",
|
||||
// which is stored as the RetentionForeverDays sentinel.
|
||||
//
|
||||
// This has to happen in a hook rather than at the call sites. GORM
|
||||
// substitutes the column default (DefaultRetentionDays) for a zero
|
||||
// value while building the insert statement, which runs after
|
||||
// BeforeSave; rewriting any later than this loses that race and the
|
||||
// row lands at 30 days. Living on the model also means a future call
|
||||
// site, such as a REST API or a fixture, cannot bypass it.
|
||||
func (w *Webhook) BeforeSave(_ *gorm.DB) error {
|
||||
if w.RetentionDays <= 0 {
|
||||
w.RetentionDays = RetentionForeverDays
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// retainsForever reports whether a stored RetentionDays value means
|
||||
// "keep events indefinitely". It is the single definition of that
|
||||
// question, shared by Webhook.RetainsForever and by the reaper's
|
||||
// cutoff computation so the two cannot disagree about which webhooks
|
||||
// are exempt from reaping.
|
||||
//
|
||||
// A negative value, which the forms refuse, is read the same way: as a
|
||||
// day count it would put the cutoff in the future, where it matches
|
||||
// every event the webhook has.
|
||||
// It accepts the RetentionForeverDays sentinel written by BeforeSave
|
||||
// and, defensively, any non-positive value, which BeforeSave treats
|
||||
// as a request to retain forever.
|
||||
func retainsForever(retentionDays int) bool {
|
||||
return retentionDays <= 0
|
||||
return retentionDays <= 0 ||
|
||||
retentionDays >= RetentionForeverDays
|
||||
}
|
||||
|
||||
// RetainsForever reports whether this webhook's events are kept
|
||||
@@ -81,8 +119,7 @@ func (w *Webhook) RetentionCutoff(now time.Time) (time.Time, bool) {
|
||||
}
|
||||
|
||||
// RetentionLabel returns the webhook's retention policy as display
|
||||
// text: "forever" rather than "0 days" for a webhook that keeps its
|
||||
// events.
|
||||
// text, so that no template has to know about the sentinel value.
|
||||
func (w *Webhook) RetentionLabel() string {
|
||||
if w.RetainsForever() {
|
||||
return "forever"
|
||||
|
||||
@@ -2,6 +2,8 @@ package database_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"reflect"
|
||||
"strconv"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -45,7 +47,8 @@ func storedRetention(t *testing.T, db *gorm.DB, id string) int {
|
||||
}
|
||||
|
||||
// newWebhookWithRetention creates a webhook through the ordinary Create
|
||||
// path, as the new webhook form does.
|
||||
// path, so the BeforeSave hook and the GORM column default both apply
|
||||
// exactly as they do in production.
|
||||
func newWebhookWithRetention(
|
||||
t *testing.T,
|
||||
db *gorm.DB,
|
||||
@@ -64,7 +67,7 @@ func newWebhookWithRetention(
|
||||
return wh.ID
|
||||
}
|
||||
|
||||
func TestWebhookRetentionDays_ZeroIsStoredAsZero(t *testing.T) {
|
||||
func TestWebhookBeforeSave_ZeroBecomesForeverSentinel(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := startedTestDB(t)
|
||||
@@ -72,10 +75,31 @@ func TestWebhookRetentionDays_ZeroIsStoredAsZero(t *testing.T) {
|
||||
wh := &database.Webhook{RetentionDays: 0}
|
||||
id := newWebhookWithRetention(t, db, wh)
|
||||
|
||||
assert.Equal(t, 0, storedRetention(t, db, id))
|
||||
assert.Equal(
|
||||
t,
|
||||
database.RetentionForeverDays,
|
||||
storedRetention(t, db, id),
|
||||
"a zero retention must be stored as the sentinel, "+
|
||||
"not replaced by the column default",
|
||||
)
|
||||
}
|
||||
|
||||
func TestWebhookRetentionDays_DayCountIsStored(t *testing.T) {
|
||||
func TestWebhookBeforeSave_NegativeBecomesForeverSentinel(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := startedTestDB(t)
|
||||
|
||||
wh := &database.Webhook{RetentionDays: -5}
|
||||
id := newWebhookWithRetention(t, db, wh)
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
database.RetentionForeverDays,
|
||||
storedRetention(t, db, id),
|
||||
)
|
||||
}
|
||||
|
||||
func TestWebhookBeforeSave_PositiveIsPreserved(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := startedTestDB(t)
|
||||
@@ -86,9 +110,10 @@ func TestWebhookRetentionDays_DayCountIsStored(t *testing.T) {
|
||||
assert.Equal(t, 7, storedRetention(t, db, id))
|
||||
}
|
||||
|
||||
// TestWebhookRetentionDays_UpdateToZeroIsStoredAsZero changes a day
|
||||
// count to 0 through the same Save call the edit handler makes.
|
||||
func TestWebhookRetentionDays_UpdateToZeroIsStoredAsZero(t *testing.T) {
|
||||
// TestWebhookBeforeSave_UpdateToZeroBecomesSentinel proves the hook
|
||||
// fires on update as well as insert, via the same Save call the edit
|
||||
// handler makes.
|
||||
func TestWebhookBeforeSave_UpdateToZeroBecomesSentinel(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := startedTestDB(t)
|
||||
@@ -100,7 +125,28 @@ func TestWebhookRetentionDays_UpdateToZeroIsStoredAsZero(t *testing.T) {
|
||||
wh.RetentionDays = 0
|
||||
require.NoError(t, db.Omit(clause.Associations).Save(wh).Error)
|
||||
|
||||
assert.Equal(t, 0, storedRetention(t, db, id))
|
||||
assert.Equal(
|
||||
t,
|
||||
database.RetentionForeverDays,
|
||||
storedRetention(t, db, id),
|
||||
)
|
||||
}
|
||||
|
||||
// TestWebhookRetentionColumnDefaultMatchesConstant guards the one place
|
||||
// the default lives twice: a struct tag cannot reference a constant, so
|
||||
// this asserts the tag and DefaultRetentionDays agree.
|
||||
func TestWebhookRetentionColumnDefaultMatchesConstant(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
field, ok := reflect.TypeFor[database.Webhook]().
|
||||
FieldByName("RetentionDays")
|
||||
require.True(t, ok, "Webhook.RetentionDays must exist")
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
"default:"+strconv.Itoa(database.DefaultRetentionDays),
|
||||
field.Tag.Get("gorm"),
|
||||
)
|
||||
}
|
||||
|
||||
// TestMaxFiniteRetentionDaysIsTheOverflowCeiling asserts that the
|
||||
@@ -130,6 +176,14 @@ func TestMaxFiniteRetentionDaysIsTheOverflowCeiling(t *testing.T) {
|
||||
time.Duration(overCeiling*hoursPerDay)*time.Hour,
|
||||
"one day past the ceiling must overflow",
|
||||
)
|
||||
|
||||
assert.Less(
|
||||
t,
|
||||
database.MaxFiniteRetentionDays,
|
||||
database.RetentionForeverDays,
|
||||
"the sentinel sits above the ceiling and is only safe "+
|
||||
"because retain-forever webhooks skip the arithmetic",
|
||||
)
|
||||
}
|
||||
|
||||
func TestWebhookRetainsForeverAndLabel(t *testing.T) {
|
||||
@@ -141,8 +195,16 @@ func TestWebhookRetainsForeverAndLabel(t *testing.T) {
|
||||
forever bool
|
||||
label string
|
||||
}{
|
||||
{"zero", 0, true, testForeverLabel},
|
||||
{"negative", -1, true, testForeverLabel},
|
||||
{
|
||||
"sentinel",
|
||||
database.RetentionForeverDays, true, testForeverLabel,
|
||||
},
|
||||
{
|
||||
"above sentinel",
|
||||
database.RetentionForeverDays + 1, true, testForeverLabel,
|
||||
},
|
||||
{"legacy zero", 0, true, testForeverLabel},
|
||||
{"legacy negative", -1, true, testForeverLabel},
|
||||
{"default", database.DefaultRetentionDays, false, "30 days"},
|
||||
{"one day", 1, false, "1 day"},
|
||||
}
|
||||
|
||||
@@ -243,8 +243,7 @@ func (r *RetentionReaper) reapWebhook(
|
||||
|
||||
// retentionCutoff returns the timestamp before which a webhook's
|
||||
// events have expired, and whether any cutoff applies at all. It
|
||||
// reports false for a retention of 0, which means retain forever, so
|
||||
// no DELETE is issued.
|
||||
// reports false for a retain-forever policy, so no DELETE is issued.
|
||||
//
|
||||
// The day count is clamped to MaxFiniteRetentionDays first. This is
|
||||
// defense in depth rather than decoration: a time.Duration is an int64
|
||||
|
||||
@@ -85,6 +85,41 @@ func createWebhook(
|
||||
db.Omit(clause.Associations).Create(wh).Error,
|
||||
)
|
||||
|
||||
// Webhook.BeforeSave rewrites a non-positive RetentionDays to the
|
||||
// retain-forever sentinel, and the column's GORM default would
|
||||
// otherwise substitute 30. Force the requested value with a
|
||||
// column-level update so tests can plant legacy rows that predate
|
||||
// the sentinel and still carry a literal 0 or negative value.
|
||||
require.NoError(
|
||||
t,
|
||||
db.Model(wh).
|
||||
Update("retention_days", retentionDays).Error,
|
||||
)
|
||||
|
||||
return wh.ID
|
||||
}
|
||||
|
||||
// createWebhookNormally inserts a webhook through the ordinary Create
|
||||
// path, with no column-level forcing, so Webhook.BeforeSave applies
|
||||
// exactly as it does in production. Passing 0 therefore yields a row
|
||||
// holding the RetentionForeverDays sentinel.
|
||||
func createWebhookNormally(
|
||||
t *testing.T,
|
||||
db *gorm.DB,
|
||||
retentionDays int,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
wh := &database.Webhook{
|
||||
UserID: uuid.New().String(),
|
||||
Name: testWebhookName,
|
||||
RetentionDays: retentionDays,
|
||||
}
|
||||
require.NoError(
|
||||
t,
|
||||
db.Omit(clause.Associations).Create(wh).Error,
|
||||
)
|
||||
|
||||
return wh.ID
|
||||
}
|
||||
|
||||
@@ -246,19 +281,19 @@ func TestRetentionReaper_ReapsExpiredKeepsRecent(t *testing.T) {
|
||||
assertChainPresent(t, db, recent)
|
||||
}
|
||||
|
||||
// TestRetentionReaper_SkipsZeroReapsFiniteInSameSweep covers the
|
||||
// end-to-end retain-forever path: a webhook created with a retention
|
||||
// of 0 reads back 0, and the reaper leaves its ancient events alone
|
||||
// while still reaping a finite-retention webhook in the very same
|
||||
// sweep.
|
||||
func TestRetentionReaper_SkipsZeroReapsFiniteInSameSweep(
|
||||
// TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep covers the
|
||||
// end-to-end retain-forever path: a webhook created the normal way with
|
||||
// a requested retention of 0 lands on the RetentionForeverDays
|
||||
// sentinel, and the reaper leaves its ancient events alone while still
|
||||
// reaping a finite-retention webhook in the very same sweep.
|
||||
func TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupRetentionTest(t)
|
||||
|
||||
foreverID := createWebhook(t, env.mainDB.DB(), 0)
|
||||
foreverID := createWebhookNormally(t, env.mainDB.DB(), 0)
|
||||
|
||||
var stored database.Webhook
|
||||
|
||||
@@ -267,12 +302,14 @@ func TestRetentionReaper_SkipsZeroReapsFiniteInSameSweep(
|
||||
env.mainDB.DB().Where("id = ?", foreverID).
|
||||
First(&stored).Error,
|
||||
)
|
||||
require.Zero(
|
||||
t, stored.RetentionDays,
|
||||
"a retention of 0 must be stored as 0",
|
||||
require.Equal(
|
||||
t,
|
||||
database.RetentionForeverDays,
|
||||
stored.RetentionDays,
|
||||
"a requested retention of 0 must persist as the sentinel",
|
||||
)
|
||||
|
||||
finiteID := createWebhook(t, env.mainDB.DB(), 30)
|
||||
finiteID := createWebhookNormally(t, env.mainDB.DB(), 30)
|
||||
|
||||
foreverDB, err := env.mgr.GetDB(foreverID)
|
||||
require.NoError(t, err)
|
||||
@@ -298,10 +335,14 @@ func TestRetentionReaper_SkipsZeroReapsFiniteInSameSweep(
|
||||
// negative, so subtracting it moves the cutoff into the far future,
|
||||
// where "created_at < cutoff" matches every row: an event created a
|
||||
// moment ago, and its delivery and delivery result, were all deleted
|
||||
// on the first sweep. 200000 is inside that band.
|
||||
// on the first sweep. 200000 is inside that band and below the
|
||||
// retain-forever sentinel, so it is treated as a finite policy and
|
||||
// really does reach the arithmetic.
|
||||
//
|
||||
// The form refuses such a value; the point of the test is that a row
|
||||
// written by any other path still cannot trigger the wipe.
|
||||
// The row is planted at the column level because such a value can no
|
||||
// longer be submitted through the form; the point of the test is that
|
||||
// a row from an older version, or a future call site, still cannot
|
||||
// trigger the wipe.
|
||||
func TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents(
|
||||
t *testing.T,
|
||||
) {
|
||||
@@ -317,6 +358,12 @@ func TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents(
|
||||
database.MaxFiniteRetentionDays,
|
||||
"the test value must exceed what the cutoff can represent",
|
||||
)
|
||||
require.Less(
|
||||
t,
|
||||
overflowingRetentionDays,
|
||||
database.RetentionForeverDays,
|
||||
"the test value must not be treated as retain-forever",
|
||||
)
|
||||
|
||||
webhookID := createWebhook(
|
||||
t, env.mainDB.DB(), overflowingRetentionDays,
|
||||
@@ -332,15 +379,14 @@ func TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents(
|
||||
assertChainPresent(t, db, fresh)
|
||||
}
|
||||
|
||||
// TestRetentionReaper_RetainsForeverWhenNegative checks that a negative
|
||||
// retention, which the forms refuse, keeps every event rather than
|
||||
// putting the cutoff in the future, where it would match them all.
|
||||
func TestRetentionReaper_RetainsForeverWhenNegative(t *testing.T) {
|
||||
func TestRetentionReaper_RetainsForeverWhenNonPositive(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupRetentionTest(t)
|
||||
|
||||
webhookID := createWebhook(t, env.mainDB.DB(), -1)
|
||||
// A legacy row written before the sentinel existed still carries a
|
||||
// literal 0; the <= 0 guard must keep honouring it.
|
||||
webhookID := createWebhook(t, env.mainDB.DB(), 0)
|
||||
|
||||
db, err := env.mgr.GetDB(webhookID)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -46,7 +46,7 @@ CREATE TABLE webhooks (
|
||||
user_id uuid NOT NULL,
|
||||
name text NOT NULL,
|
||||
description text,
|
||||
retention_days integer,
|
||||
retention_days integer DEFAULT 30,
|
||||
PRIMARY KEY (id),
|
||||
CONSTRAINT fk_users_webhooks FOREIGN KEY (user_id) REFERENCES users (id)
|
||||
);
|
||||
|
||||
@@ -2,16 +2,11 @@ package delivery_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/fx"
|
||||
"gorm.io/gorm/clause"
|
||||
@@ -277,154 +272,6 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
|
||||
requireStopHookExpires(t, lc.hooks[0], "delivery engine")
|
||||
}
|
||||
|
||||
// seedHeldDelivery gives the setup's webhook an http target with
|
||||
// maxRetries, pointed at a receiver that keeps every request open until
|
||||
// the client gives up on it, and seeds one delivery to that target at
|
||||
// status. It returns the task that sends attempt attemptNum of the
|
||||
// delivery, and a channel that is closed when a request reaches the
|
||||
// receiver.
|
||||
func seedHeldDelivery(
|
||||
t *testing.T,
|
||||
s iSetup,
|
||||
maxRetries int,
|
||||
status database.DeliveryStatus,
|
||||
attemptNum int,
|
||||
) (delivery.Task, <-chan struct{}) {
|
||||
t.Helper()
|
||||
|
||||
arrived := make(chan struct{})
|
||||
|
||||
var once sync.Once
|
||||
|
||||
ts := httptest.NewServer(http.HandlerFunc(
|
||||
func(_ http.ResponseWriter, r *http.Request) {
|
||||
// The server notices the client hanging up only once the
|
||||
// body has been read; until then r.Context() never ends.
|
||||
_, _ = io.Copy(io.Discard, r.Body)
|
||||
|
||||
once.Do(func() { close(arrived) })
|
||||
|
||||
<-r.Context().Done()
|
||||
},
|
||||
))
|
||||
t.Cleanup(ts.Close)
|
||||
|
||||
cfg := iHTTPConfig(ts.URL)
|
||||
targetID := uuid.New().String()
|
||||
|
||||
iCreateWebhook(t, s.MainDB, s.WebhookID, "hook")
|
||||
iCreateTarget(
|
||||
t, s.MainDB, targetID, s.WebhookID, "held",
|
||||
database.TargetTypeHTTP, cfg, maxRetries,
|
||||
)
|
||||
|
||||
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"held":true}`)
|
||||
d := iSeedDelivery(t, s.WebhookDB, event.ID, targetID, status)
|
||||
|
||||
bodyStr := event.Body
|
||||
|
||||
return iTask(
|
||||
d, event, s.WebhookID, targetID, "held", cfg,
|
||||
maxRetries, attemptNum, &bodyStr,
|
||||
), arrived
|
||||
}
|
||||
|
||||
// stopDuringAttempt waits until the receiver holds an attempt open, then
|
||||
// stops the engine through its real stop hook.
|
||||
func stopDuringAttempt(
|
||||
t *testing.T, lc *recordingLifecycle, arrived <-chan struct{},
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
select {
|
||||
case <-arrived:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("the delivery attempt never reached the receiver")
|
||||
}
|
||||
|
||||
require.NoError(t, lc.hooks[0].OnStop(context.Background()))
|
||||
}
|
||||
|
||||
// requireRequeued runs restart recovery on the stopped engine and
|
||||
// requires it to queue task's attempt again on ch.
|
||||
func requireRequeued(
|
||||
t *testing.T, s iSetup, ch <-chan delivery.Task, task delivery.Task,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
s.Engine.ExportRecoverWebhookDeliveries(
|
||||
context.Background(), s.WebhookID,
|
||||
)
|
||||
|
||||
select {
|
||||
case got := <-ch:
|
||||
assert.Equal(t, task.DeliveryID, got.DeliveryID)
|
||||
assert.Equal(t, task.AttemptNum, got.AttemptNum)
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("restart recovery did not queue the delivery again")
|
||||
}
|
||||
}
|
||||
|
||||
// TestEngine_StopLeavesFireAndForgetAttemptPending is the regression
|
||||
// test for a stop that turned an attempt in flight into a terminal
|
||||
// failure. Stopping the engine cancels the request; for a target with
|
||||
// no retries that was the delivery's only attempt, recorded as failed
|
||||
// with "context canceled", and nothing ever sent it again. The
|
||||
// delivery must instead stay pending, with no result and no count in
|
||||
// the attempt metrics, for restart recovery to send.
|
||||
func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := newISetup(t)
|
||||
reg := mIsolate(t, s)
|
||||
lc := startEngineViaHook(t, s.Engine)
|
||||
|
||||
task, arrived := seedHeldDelivery(
|
||||
t, s, 0, database.DeliveryStatusPending, 1,
|
||||
)
|
||||
|
||||
s.Engine.Notify([]delivery.Task{task})
|
||||
stopDuringAttempt(t, lc, arrived)
|
||||
|
||||
iAssertStatus(
|
||||
t, s.WebhookDB, task.DeliveryID, database.DeliveryStatusPending,
|
||||
)
|
||||
assert.Empty(t, iResults(t, s.WebhookDB, task.DeliveryID))
|
||||
assert.Zero(t, mCounter(t, reg, mAttempts, mTypeHTTP))
|
||||
assert.Zero(t, mHTTPDurations(t, reg))
|
||||
|
||||
requireRequeued(t, s, s.Engine.ExportDeliveryCh(), task)
|
||||
}
|
||||
|
||||
// TestEngine_StopLeavesLastRetryAttemptRetrying is the same regression
|
||||
// for a target with retries, on its last attempt: the cancelled attempt
|
||||
// used to exhaust the retries and end the delivery failed. It must stay
|
||||
// retrying with only its earlier attempt recorded, and restart recovery
|
||||
// must send that last attempt again.
|
||||
func TestEngine_StopLeavesLastRetryAttemptRetrying(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const maxRetries = 2
|
||||
|
||||
s := newISetup(t)
|
||||
lc := startEngineViaHook(t, s.Engine)
|
||||
|
||||
task, arrived := seedHeldDelivery(
|
||||
t, s, maxRetries, database.DeliveryStatusRetrying, maxRetries,
|
||||
)
|
||||
iSeedFailedResult(t, s.WebhookDB, task.DeliveryID)
|
||||
|
||||
s.Engine.ExportScheduleRetry(task, 0)
|
||||
stopDuringAttempt(t, lc, arrived)
|
||||
|
||||
iAssertStatus(
|
||||
t, s.WebhookDB, task.DeliveryID, database.DeliveryStatusRetrying,
|
||||
)
|
||||
assert.Len(t, iResults(t, s.WebhookDB, task.DeliveryID), 1)
|
||||
|
||||
requireRequeued(t, s, s.Engine.ExportRetryCh(), task)
|
||||
}
|
||||
|
||||
// deliverToArchive gives the setup's webhook a database target,
|
||||
// runs one delivery to it through the running engine, and returns
|
||||
// the target's ID and archive file path. The archive writer holds
|
||||
|
||||
@@ -32,8 +32,7 @@ type Scheduler interface {
|
||||
// Engine.observeAttempt, alongside the DeliveryResult it records for
|
||||
// it. Deliver is also entered for attempts that never happen — an
|
||||
// open circuit breaker refuses one — so the count cannot be taken
|
||||
// from around this call. An attempt cut off by a stop is neither
|
||||
// counted nor recorded.
|
||||
// from around this call.
|
||||
type Target interface {
|
||||
Deliver(
|
||||
ctx context.Context,
|
||||
|
||||
@@ -57,11 +57,7 @@ type httpCore struct {
|
||||
// recorded and no circuit breaker is consulted. A positive
|
||||
// maxRetries gates the attempt on the circuit breaker and
|
||||
// schedules a backed-off retry on failure.
|
||||
//
|
||||
// ctx is the context attempt sends its request with. Stopping the
|
||||
// engine cancels it; see interruptedByStop.
|
||||
func (c *httpCore) deliver(
|
||||
ctx context.Context,
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
@@ -70,55 +66,21 @@ func (c *httpCore) deliver(
|
||||
attempt func() attemptResult,
|
||||
) {
|
||||
if maxRetries == 0 {
|
||||
c.fireAndForget(ctx, webhookDB, d, attempt())
|
||||
c.fireAndForget(webhookDB, d, attempt())
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
c.withRetry(
|
||||
ctx, webhookDB, d, task, sched, maxRetries, attempt,
|
||||
webhookDB, d, task, sched, maxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
// interruptedByStop reports whether an attempt failed because the
|
||||
// engine is stopping, which cancels the context its request is made
|
||||
// with. Such an attempt is not recorded: no DeliveryResult, no circuit
|
||||
// breaker outcome, no status change, no count in the attempt metrics.
|
||||
// The delivery stays pending or retrying, and startup recovery sends
|
||||
// it again on the next start.
|
||||
//
|
||||
// A target's own timeout leaves that context alone, so its attempt is
|
||||
// still recorded as the failure it is.
|
||||
func (c *httpCore) interruptedByStop(
|
||||
ctx context.Context,
|
||||
d *database.Delivery,
|
||||
res attemptResult,
|
||||
) bool {
|
||||
if res.success || ctx.Err() == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
c.eng.log.Warn(
|
||||
"delivery attempt interrupted by shutdown; not recorded, "+
|
||||
"it is sent again on the next start",
|
||||
"delivery_id", d.ID,
|
||||
"event_id", d.EventID,
|
||||
"target_id", d.TargetID,
|
||||
)
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
func (c *httpCore) fireAndForget(
|
||||
ctx context.Context,
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
res attemptResult,
|
||||
) {
|
||||
if c.interruptedByStop(ctx, d, res) {
|
||||
return
|
||||
}
|
||||
|
||||
c.eng.observeAttempt(d.Target.Type, res.elapsed())
|
||||
|
||||
err := c.eng.recordResult(
|
||||
@@ -148,7 +110,6 @@ func (c *httpCore) fireAndForget(
|
||||
}
|
||||
|
||||
func (c *httpCore) withRetry(
|
||||
ctx context.Context,
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
@@ -170,10 +131,6 @@ func (c *httpCore) withRetry(
|
||||
|
||||
res := attempt()
|
||||
|
||||
if c.interruptedByStop(ctx, d, res) {
|
||||
return
|
||||
}
|
||||
|
||||
c.eng.observeAttempt(d.Target.Type, res.elapsed())
|
||||
|
||||
err := c.eng.recordResult(
|
||||
@@ -438,7 +395,7 @@ func (t *httpTarget) Deliver(
|
||||
}
|
||||
|
||||
t.deliver(
|
||||
ctx, webhookDB, d, task, sched,
|
||||
webhookDB, d, task, sched,
|
||||
d.Target.MaxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -82,7 +82,7 @@ func (t *slackTarget) Deliver(
|
||||
}
|
||||
|
||||
t.deliver(
|
||||
ctx, webhookDB, d, task, sched,
|
||||
webhookDB, d, task, sched,
|
||||
d.Target.MaxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -158,8 +158,7 @@ func TestHandleSourceLogs_RendersFailedAttempt(t *testing.T) {
|
||||
// TestHandleSourceLogs_EscapesResponseBody proves the
|
||||
// response body is treated as the untrusted remote content it
|
||||
// is. The remote chooses these bytes and the page is rendered
|
||||
// inside the operator's authenticated origin, where the
|
||||
// application's own CSP allows inline script from 'self'.
|
||||
// inside the operator's authenticated origin.
|
||||
func TestHandleSourceLogs_EscapesResponseBody(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -35,11 +35,10 @@ const eventBodyQuery = "SELECT cast(body as blob) " +
|
||||
// response is deliberately not renderable. Content-Disposition
|
||||
// makes the browser download rather than display it, and the
|
||||
// octet-stream type plus nosniff stop it being interpreted as
|
||||
// HTML or script. Without those a stored payload would execute
|
||||
// as the logged-in operator. The application's CSP does not
|
||||
// help here: script-src allows 'unsafe-inline' from 'self', so
|
||||
// a document served from this origin could run its own inline
|
||||
// script.
|
||||
// HTML or script. Without those a stored payload would be shown
|
||||
// as a page of the operator's own origin. The application's CSP
|
||||
// is a second defence behind them: it allows no inline script,
|
||||
// so such a page could not run script of its own.
|
||||
func (h *Handlers) HandleEventBodyDownload() http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
webhook, ok := h.ownedWebhook(w, r)
|
||||
|
||||
@@ -84,7 +84,7 @@ func TestHandleEventDetail_ShowsEventWholeWithDeliveries(t *testing.T) {
|
||||
assert.Contains(t, page, strconv.Itoa(len(body))+" bytes")
|
||||
assert.Contains(t, page, "{\n "pad": "xxx")
|
||||
assert.Contains(t, page, ""tail": ""+sentinel+""\n}")
|
||||
assert.Contains(t, page, `style="max-height: 32rem; overflow-y: auto"`)
|
||||
assert.Contains(t, page, `break-all max-h-[32rem] overflow-y-auto"`)
|
||||
assert.NotContains(t, page, "Showing the first")
|
||||
assert.Contains(t, page, target.Name)
|
||||
assert.Contains(t, page, ">failed</span>")
|
||||
|
||||
@@ -16,16 +16,21 @@ import (
|
||||
//
|
||||
// An empty value yields fallback, which lets the create path apply the
|
||||
// default and the edit path leave the stored value unchanged. A value
|
||||
// of 0 means retain events forever and is returned as it is. Anything
|
||||
// unparseable or negative is refused rather than silently given a
|
||||
// default.
|
||||
// of 0 is returned as 0 and is rewritten to the retain-forever
|
||||
// sentinel by database.Webhook's BeforeSave hook. Anything unparseable
|
||||
// or negative is refused rather than silently given a default.
|
||||
//
|
||||
// The upper bound is not cosmetic. The reaper computes its cutoff as a
|
||||
// time.Duration, an int64 nanosecond count, so a day count above
|
||||
// database.MaxFiniteRetentionDays overflows, puts the cutoff in the
|
||||
// future, and deletes every event the webhook has. A value above that
|
||||
// ceiling is therefore refused, and the message names the ceiling
|
||||
// rather than implying the input was not a number.
|
||||
// future, and deletes every event the webhook has. A finite value
|
||||
// above that ceiling is therefore refused, and the message names the
|
||||
// ceiling rather than implying the input was not a number.
|
||||
//
|
||||
// A value at or above the retain-forever sentinel is not out of range:
|
||||
// it is what the edit form pre-fills for a retain-forever webhook, so
|
||||
// submitting the form back unchanged has to keep meaning "forever"
|
||||
// rather than being rejected.
|
||||
func parseRetentionDays(raw string, fallback int) (int, string) {
|
||||
raw = strings.TrimSpace(raw)
|
||||
if raw == "" {
|
||||
@@ -38,6 +43,10 @@ func parseRetentionDays(raw string, fallback int) (int, string) {
|
||||
"retain events forever."
|
||||
}
|
||||
|
||||
if v >= database.RetentionForeverDays {
|
||||
return database.RetentionForeverDays, ""
|
||||
}
|
||||
|
||||
if v > database.MaxFiniteRetentionDays {
|
||||
return 0, "Retention must be at most " +
|
||||
strconv.Itoa(database.MaxFiniteRetentionDays) +
|
||||
|
||||
@@ -273,7 +273,7 @@ func TestHandleSourceDetail_FitsWideAndNarrowWindows(t *testing.T) {
|
||||
|
||||
assert.Contains(
|
||||
t, body,
|
||||
`<div class="mx-auto px-6 py-8" style="max-width: 108rem"`,
|
||||
`<div class="max-w-[108rem] mx-auto px-6 py-8">`,
|
||||
)
|
||||
assert.Contains(
|
||||
t, body,
|
||||
@@ -286,8 +286,8 @@ func TestHandleSourceDetail_FitsWideAndNarrowWindows(t *testing.T) {
|
||||
// target and says what deleting it loses, that the webhook's gives its
|
||||
// number of stored events (5 received, 2 removed by retention, so 3,
|
||||
// the statistics pane's "Within retention" figure), and that an
|
||||
// entrypoint with no description is named by its URL. The template
|
||||
// writes the slashes after http: as \/, which the browser reads as /.
|
||||
// entrypoint with no description is named by its URL. Each prompt is
|
||||
// its form's data-confirm attribute, where is a line break.
|
||||
func TestHandleSourceDetail_DeletePromptsNameWhatIsLost(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -325,29 +325,28 @@ func TestHandleSourceDetail_DeletePromptsNameWhatIsLost(t *testing.T) {
|
||||
body := renderSourceDetailPage(t, h, sess, wh.ID)
|
||||
|
||||
assert.Contains(t, body,
|
||||
`Delete webhook "delete-me"?\n\n`+
|
||||
`data-confirm="Delete webhook "delete-me"? `+
|
||||
`This deletes its stored events (3) and their deliveries. `+
|
||||
`Any archive files it wrote are kept.`)
|
||||
`Any archive files it wrote are kept."`)
|
||||
assert.Contains(t, body,
|
||||
`Delete entrypoint "Stripe"?\n\n`+
|
||||
`data-confirm="Delete entrypoint "Stripe"? `+
|
||||
`Senders using its URL get an error from now on, `+
|
||||
`and the URL cannot be restored.`)
|
||||
`and the URL cannot be restored."`)
|
||||
assert.Contains(t, body,
|
||||
`Delete entrypoint "http:\/\/example.com/h/`+
|
||||
unnamed.Path+`"?`)
|
||||
`data-confirm="Delete entrypoint "http://example.com/h/`+
|
||||
unnamed.Path+`"? `)
|
||||
assert.Contains(t, body,
|
||||
`Delete target "t-log"?\n\n`+
|
||||
`data-confirm="Delete target "t-log"? `+
|
||||
`Nothing more is delivered to it. `+
|
||||
`Its past deliveries stay in the event log.`)
|
||||
`Its past deliveries stay in the event log."`)
|
||||
}
|
||||
|
||||
// TestHandleSourceDetail_DeletePromptKeepsQuotesInName checks that a
|
||||
// webhook name with quotes, a backslash, a closing script tag and a
|
||||
// newline reaches its delete prompt escaped for the script, which the
|
||||
// browser reads back as the name typed: each quote and angle bracket
|
||||
// as a \u escape, the slash as \/, the newline as \n and the backslash
|
||||
// doubled. An unescaped newline would break the prompt's script, and
|
||||
// the form would then submit without asking.
|
||||
// newline reaches its delete prompt escaped for the attribute, which
|
||||
// the browser reads back as the name typed: each quote and angle
|
||||
// bracket as a character reference, the rest as typed. An unescaped
|
||||
// double quote would end the attribute and cut the prompt short.
|
||||
func TestHandleSourceDetail_DeletePromptKeepsQuotesInName(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -373,6 +372,6 @@ func TestHandleSourceDetail_DeletePromptKeepsQuotesInName(t *testing.T) {
|
||||
body := renderSourceDetailPage(t, h, sess, wh.ID)
|
||||
|
||||
assert.Contains(t, body,
|
||||
"Delete webhook "Bob\\u0027s \\u0022best\\u0022 \\\\ hook"+
|
||||
"\\u003c\\/script\\u003e\\nline two"?")
|
||||
"Delete webhook "Bob's "best" \\ hook"+
|
||||
"</script>\nline two"?")
|
||||
}
|
||||
|
||||
@@ -137,7 +137,8 @@ func onlyWebhook(
|
||||
}
|
||||
|
||||
// seedWebhookWithRetention inserts a webhook owned by the test user
|
||||
// with the given retention.
|
||||
// with an exact stored retention value, bypassing Webhook.BeforeSave
|
||||
// via a column-level update so that legacy rows can be planted too.
|
||||
func seedWebhookWithRetention(
|
||||
t *testing.T,
|
||||
db *database.Database,
|
||||
@@ -154,6 +155,13 @@ func seedWebhookWithRetention(
|
||||
t,
|
||||
db.DB().Omit(clause.Associations).Create(wh).Error,
|
||||
)
|
||||
require.NoError(
|
||||
t,
|
||||
db.DB().Model(wh).
|
||||
Update("retention_days", retentionDays).Error,
|
||||
)
|
||||
|
||||
wh.RetentionDays = retentionDays
|
||||
|
||||
return *wh
|
||||
}
|
||||
@@ -217,9 +225,10 @@ func setupSourceTest(t *testing.T) *sourceTestEnv {
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever checks
|
||||
// that the create form's 0 reaches the database as 0, which means
|
||||
// retain forever.
|
||||
// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever is the core
|
||||
// regression test for the bug: the create form's 0 must reach the
|
||||
// database as the retain-forever sentinel rather than being replaced by
|
||||
// the column's default of 30.
|
||||
func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
|
||||
t *testing.T,
|
||||
) {
|
||||
@@ -232,55 +241,30 @@ func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
wh := onlyWebhook(t, env.db)
|
||||
assert.Equal(t, 0, storedRetentionDays(t, env.db, wh.ID))
|
||||
assert.Equal(
|
||||
t,
|
||||
database.RetentionForeverDays,
|
||||
storedRetentionDays(t, env.db, wh.ID),
|
||||
)
|
||||
assert.True(t, wh.RetainsForever())
|
||||
}
|
||||
|
||||
func TestHandleSourceCreateSubmit_DayCountIsStored(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
days := "45"
|
||||
|
||||
w := submitCreate(t, env.handlers, env.cookies, "finite", &days)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
wh := onlyWebhook(t, env.db)
|
||||
assert.Equal(t, 45, storedRetentionDays(t, env.db, wh.ID))
|
||||
}
|
||||
|
||||
// TestHandleSourceCreateSubmit_BlankRetentionUsesDefault covers a
|
||||
// retention field left empty and one missing from the form: both
|
||||
// give the default, not forever.
|
||||
func TestHandleSourceCreateSubmit_BlankRetentionUsesDefault(
|
||||
func TestHandleSourceCreateSubmit_OmittedRetentionUsesDefault(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
blank := ""
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for name, retention := range map[string]*string{
|
||||
"blank": &blank,
|
||||
"omitted": nil,
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
w := submitCreate(t, env.handlers, env.cookies, "defaulted", nil)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
w := submitCreate(
|
||||
t, env.handlers, env.cookies, "defaulted", retention,
|
||||
)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
wh := onlyWebhook(t, env.db)
|
||||
assert.Equal(
|
||||
t,
|
||||
database.DefaultRetentionDays,
|
||||
storedRetentionDays(t, env.db, wh.ID),
|
||||
)
|
||||
})
|
||||
}
|
||||
wh := onlyWebhook(t, env.db)
|
||||
assert.Equal(
|
||||
t,
|
||||
database.DefaultRetentionDays,
|
||||
storedRetentionDays(t, env.db, wh.ID),
|
||||
)
|
||||
}
|
||||
|
||||
// TestHandleSourceCreate_PrefillsDefaultFromConstant keeps the create
|
||||
@@ -306,7 +290,7 @@ func TestHandleSourceCreate_PrefillsDefaultFromConstant(t *testing.T) {
|
||||
)
|
||||
assert.NotContains(
|
||||
t, body, `max="365"`,
|
||||
"a max of 365 would block any longer retention",
|
||||
"a max below the sentinel would block retain-forever",
|
||||
)
|
||||
assert.Contains(t, body, `min="0"`)
|
||||
}
|
||||
@@ -384,6 +368,40 @@ func TestHandleSourceCreateSubmit_OverflowingRetentionIsRejected(
|
||||
)
|
||||
}
|
||||
|
||||
// TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever guards the
|
||||
// boundary between "too large to represent" and "retain forever": the
|
||||
// sentinel is above MaxFiniteRetentionDays, but it is the value the
|
||||
// edit form pre-fills, so it must be accepted rather than rejected as
|
||||
// out of range. A value above the sentinel is stored as the sentinel.
|
||||
func TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
for _, days := range []int{
|
||||
database.RetentionForeverDays,
|
||||
database.RetentionForeverDays + 1,
|
||||
} {
|
||||
raw := strconv.Itoa(days)
|
||||
|
||||
t.Run(raw, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
w := submitCreate(t, env.handlers, env.cookies, "forever", &raw)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
wh := onlyWebhook(t, env.db)
|
||||
assert.Equal(
|
||||
t,
|
||||
database.RetentionForeverDays,
|
||||
storedRetentionDays(t, env.db, wh.ID),
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleSourceCreateSubmit_RejectedFormKeepsUserInput checks that a
|
||||
// validation failure hands the user's typing back, matching what the
|
||||
// edit form already does. Losing a long description to a mistyped
|
||||
@@ -462,7 +480,11 @@ func TestHandleSourceEditSubmit_ZeroRetentionPersistsForever(
|
||||
w := submitEdit(t, env, wh, "0")
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
assert.Equal(t, 0, storedRetentionDays(t, env.db, wh.ID))
|
||||
assert.Equal(
|
||||
t,
|
||||
database.RetentionForeverDays,
|
||||
storedRetentionDays(t, env.db, wh.ID),
|
||||
)
|
||||
}
|
||||
|
||||
func TestHandleSourceEditSubmit_InvalidRetentionIsRejected(
|
||||
@@ -845,15 +867,18 @@ func TestHandleSourceEditSubmit_ArchiveNameTaken(t *testing.T) {
|
||||
assert.Equal(t, oldName, stored.Name)
|
||||
}
|
||||
|
||||
// TestSourceEditForm_ForeverWebhookRoundTrips renders the edit form for
|
||||
// a retain-forever webhook, confirms it pre-fills 0, then submits that
|
||||
// pre-filled value straight back and confirms the retention policy
|
||||
// survives untouched.
|
||||
// TestSourceEditForm_ForeverWebhookRoundTrips walks the exact path that
|
||||
// the removed max="365" cap used to break: render the edit form for a
|
||||
// retain-forever webhook, confirm the pre-filled sentinel is not capped
|
||||
// by browser validation, then submit that pre-filled value straight
|
||||
// back and confirm the retention policy survives untouched.
|
||||
func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
wh := seedWebhookWithRetention(t, env.db, 0)
|
||||
wh := seedWebhookWithRetention(
|
||||
t, env.db, database.RetentionForeverDays,
|
||||
)
|
||||
|
||||
req := getRequest(
|
||||
t, "/hook/"+wh.ID+"/edit", env.cookies,
|
||||
@@ -864,12 +889,17 @@ func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
|
||||
|
||||
require.Equal(t, http.StatusOK, w.Code)
|
||||
|
||||
sentinel := strconv.Itoa(database.RetentionForeverDays)
|
||||
body := w.Body.String()
|
||||
|
||||
assert.Contains(
|
||||
t, body, `name="retention_days" value="0"`,
|
||||
t, body, `value="`+sentinel+`"`,
|
||||
"the edit form pre-fills the stored retention",
|
||||
)
|
||||
assert.NotContains(
|
||||
t, body, `max="365"`,
|
||||
"a max below the sentinel would block saving any edit",
|
||||
)
|
||||
// "Currently forever." is the rendered RetentionLabel, not the
|
||||
// static hint below the input, which says "Enter 0 to retain events
|
||||
// forever." A bare Contains of "forever" would pass for any
|
||||
@@ -880,20 +910,29 @@ func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
|
||||
)
|
||||
|
||||
// Submit the pre-filled value back, exactly as a browser would.
|
||||
post := submitEdit(t, env, wh, "0")
|
||||
post := submitEdit(t, env, wh, sentinel)
|
||||
require.Equal(t, http.StatusSeeOther, post.Code)
|
||||
|
||||
assert.Equal(t, 0, storedRetentionDays(t, env.db, wh.ID))
|
||||
assert.Equal(
|
||||
t,
|
||||
database.RetentionForeverDays,
|
||||
storedRetentionDays(t, env.db, wh.ID),
|
||||
)
|
||||
}
|
||||
|
||||
// TestSourceListAndDetail_ShowForeverNotZeroDays checks that a
|
||||
// retention of 0 is shown as forever, never as a day count, on either
|
||||
// read-only view.
|
||||
func TestSourceListAndDetail_ShowForeverNotZeroDays(t *testing.T) {
|
||||
// TestSourceListAndDetail_ShowForeverNotTheSentinelNumber checks that
|
||||
// the retain-forever value is never rendered to the user as a raw day
|
||||
// count on either read-only view.
|
||||
func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
wh := seedWebhookWithRetention(t, env.db, 0)
|
||||
wh := seedWebhookWithRetention(
|
||||
t, env.db, database.RetentionForeverDays,
|
||||
)
|
||||
sentinel := strconv.Itoa(database.RetentionForeverDays)
|
||||
|
||||
listW := httptest.NewRecorder()
|
||||
env.handlers.HandleSourceList().ServeHTTP(
|
||||
@@ -902,7 +941,7 @@ func TestSourceListAndDetail_ShowForeverNotZeroDays(t *testing.T) {
|
||||
|
||||
require.Equal(t, http.StatusOK, listW.Code)
|
||||
assert.Contains(t, listW.Body.String(), "Retention: forever")
|
||||
assert.NotContains(t, listW.Body.String(), "Retention: 0 days")
|
||||
assert.NotContains(t, listW.Body.String(), sentinel)
|
||||
|
||||
detailW := httptest.NewRecorder()
|
||||
env.handlers.HandleSourceDetail().ServeHTTP(
|
||||
@@ -915,5 +954,5 @@ func TestSourceListAndDetail_ShowForeverNotZeroDays(t *testing.T) {
|
||||
|
||||
require.Equal(t, http.StatusOK, detailW.Code)
|
||||
assert.Contains(t, detailW.Body.String(), "Retention: forever")
|
||||
assert.NotContains(t, detailW.Body.String(), "Retention: 0 days")
|
||||
assert.NotContains(t, detailW.Body.String(), sentinel)
|
||||
}
|
||||
|
||||
@@ -184,7 +184,8 @@ func TestEventLogPageIsCalledFullEventLog(t *testing.T) {
|
||||
// TestCreateFormRetentionCopyMatchesBehaviour pins the create form's
|
||||
// retention copy to what the code does: the reaper permanently deletes
|
||||
// events past the cutoff, an empty field falls back to
|
||||
// DefaultRetentionDays, and 0 means retain forever.
|
||||
// DefaultRetentionDays, and 0 is rewritten to the retain-forever
|
||||
// sentinel by Webhook.BeforeSave.
|
||||
func TestCreateFormRetentionCopyMatchesBehaviour(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -252,7 +253,10 @@ func TestEditFormRetentionCopyMatchesBehaviour(t *testing.T) {
|
||||
"blank means unchanged, not forever",
|
||||
)
|
||||
|
||||
forever := &database.Webhook{Name: "wh", RetentionDays: 0}
|
||||
forever := &database.Webhook{
|
||||
Name: "wh",
|
||||
RetentionDays: database.RetentionForeverDays,
|
||||
}
|
||||
forever.ID = "wh-2"
|
||||
|
||||
foreverBody := renderPage(
|
||||
|
||||
@@ -500,7 +500,7 @@ func TestWebhookStats_PaneShowsRetentionPeriod(t *testing.T) {
|
||||
want string
|
||||
}{
|
||||
{30, "30 days"},
|
||||
{0, "forever"},
|
||||
{database.RetentionForeverDays, "forever"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
||||
@@ -538,7 +538,9 @@ func (s *Middleware) MetricsAuth() func(http.Handler) http.Handler {
|
||||
// SecurityHeaders returns middleware that sets production security
|
||||
// headers on every response: HSTS (unless HSTS_MAX_AGE is zero),
|
||||
// X-Content-Type-Options, X-Frame-Options, CSP, Referrer-Policy, and
|
||||
// Permissions-Policy.
|
||||
// Permissions-Policy. The CSP takes scripts and styles only from
|
||||
// files on this origin, so a template may hold no inline script,
|
||||
// event handler or style attribute.
|
||||
func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
|
||||
maxAge := s.params.Config.HSTSMaxAge
|
||||
hsts := "max-age=" + strconv.FormatInt(int64(maxAge/time.Second), 10) +
|
||||
@@ -560,8 +562,8 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
|
||||
w.Header().Set(
|
||||
"Content-Security-Policy",
|
||||
"default-src 'self'; "+
|
||||
"script-src 'self' 'unsafe-inline'; "+
|
||||
"style-src 'self' 'unsafe-inline'",
|
||||
"script-src 'self'; "+
|
||||
"style-src 'self'",
|
||||
)
|
||||
w.Header().Set(
|
||||
"Referrer-Policy",
|
||||
|
||||
@@ -747,10 +747,10 @@ func TestNoCache_SetsHeaders(t *testing.T) {
|
||||
|
||||
// --- SecurityHeaders Middleware Tests ---
|
||||
|
||||
// hstsHeaders serves one request through SecurityHeaders built over
|
||||
// a Config with the given HSTSMaxAge, and returns every
|
||||
// Strict-Transport-Security value on the response.
|
||||
func hstsHeaders(t *testing.T, maxAge time.Duration) []string {
|
||||
// securityHeaders serves one request through SecurityHeaders built
|
||||
// over a Config with the given HSTSMaxAge, and returns the response's
|
||||
// headers.
|
||||
func securityHeaders(t *testing.T, maxAge time.Duration) http.Header {
|
||||
t.Helper()
|
||||
|
||||
cfg := &config.Config{HSTSMaxAge: maxAge}
|
||||
@@ -772,7 +772,7 @@ func hstsHeaders(t *testing.T, maxAge time.Duration) []string {
|
||||
|
||||
handler.ServeHTTP(w, req)
|
||||
|
||||
return w.Header().Values("Strict-Transport-Security")
|
||||
return w.Header()
|
||||
}
|
||||
|
||||
func TestSecurityHeaders_HSTS(t *testing.T) {
|
||||
@@ -804,11 +804,28 @@ func TestSecurityHeaders_HSTS(t *testing.T) {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(t, tt.expected, hstsHeaders(t, tt.maxAge))
|
||||
assert.Equal(
|
||||
t, tt.expected,
|
||||
securityHeaders(t, tt.maxAge).Values(
|
||||
"Strict-Transport-Security",
|
||||
),
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestSecurityHeaders_CSP pins the Content-Security-Policy: a page
|
||||
// takes its scripts and styles only from files on its own origin,
|
||||
// never from an inline script, event handler or style.
|
||||
func TestSecurityHeaders_CSP(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(
|
||||
t, "default-src 'self'; script-src 'self'; style-src 'self'",
|
||||
securityHeaders(t, 0).Get("Content-Security-Policy"),
|
||||
)
|
||||
}
|
||||
|
||||
// --- MaxBodySize Middleware Tests ---
|
||||
|
||||
const testBodyLimit int64 = 64
|
||||
|
||||
@@ -60,8 +60,8 @@ const (
|
||||
// TestAlpineRunsUnderTheSecurityPolicy loads the webhook page and the
|
||||
// event log in a headless browser, served by the real router and so
|
||||
// under the real Content-Security-Policy, and checks that the pages'
|
||||
// Alpine.js directives and the copy control work, and that a target's
|
||||
// row shows its delivery counts.
|
||||
// Alpine.js directives, the copy control and the delete prompts work,
|
||||
// and that a target's row shows its delivery counts.
|
||||
func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -99,6 +99,9 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
|
||||
checkEventLog(ctx, t, page+"/events", event.ID, older.ID, target.Name)
|
||||
checkMobileMenu(ctx, t, page)
|
||||
checkPhoneWidth(ctx, t, page, page+"/events", target.Name)
|
||||
checkOtherPages(ctx, t,
|
||||
srv.URL+"/hooks", srv.URL+"/settings", srv.URL+"/user/browser")
|
||||
checkDeletePrompt(ctx, t, page, target)
|
||||
|
||||
assert.Empty(t, problems(), "the browser reported problems")
|
||||
}
|
||||
@@ -1388,3 +1391,75 @@ func checkPhoneWidth(
|
||||
assert.False(t, sideways, "the event log scrolls sideways on a phone")
|
||||
assert.Empty(t, cutOff, "the event log cuts these off on a phone")
|
||||
}
|
||||
|
||||
// checkOtherPages loads the pages at urls, which no other check opens,
|
||||
// so that anything the browser reports going wrong on them fails the
|
||||
// test too.
|
||||
func checkOtherPages(ctx context.Context, t *testing.T, urls ...string) {
|
||||
t.Helper()
|
||||
|
||||
for _, url := range urls {
|
||||
require.NoError(t, chromedp.Run(ctx, loadPage(url)))
|
||||
}
|
||||
}
|
||||
|
||||
// answerPrompt returns a script that stands in for the user at the
|
||||
// browser's prompt: it makes window.confirm record the question in
|
||||
// window.asked and give answer. It also records in window.sent whether
|
||||
// a form is sent once its submit event is handled, from a listener on
|
||||
// window, which the event reaches after the listener static/js/app.js
|
||||
// adds to document.
|
||||
func answerPrompt(answer bool) string {
|
||||
return fmt.Sprintf(`
|
||||
window.asked = "";
|
||||
window.sent = false;
|
||||
window.confirm = (question) => {
|
||||
window.asked = question;
|
||||
return %t;
|
||||
};
|
||||
window.addEventListener("submit", (event) => {
|
||||
window.sent = !event.defaultPrevented;
|
||||
});`, answer)
|
||||
}
|
||||
|
||||
// checkDeletePrompt loads a webhook page and clicks Delete beside
|
||||
// target. It checks that the browser asks first, naming the target,
|
||||
// that answering no leaves the form unsent, and that answering yes
|
||||
// sends it and the target is deleted.
|
||||
func checkDeletePrompt(
|
||||
ctx context.Context, t *testing.T, url string, target *database.Target,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
form := `form[action$="/targets/` + target.ID + `/delete"]`
|
||||
question := `Delete target "` + target.Name + `"?` + "\n\n" +
|
||||
"Nothing more is delivered to it. " +
|
||||
"Its past deliveries stay in the event log."
|
||||
|
||||
var (
|
||||
asked string
|
||||
sent bool
|
||||
listed bool
|
||||
)
|
||||
|
||||
require.NoError(t, chromedp.Run(
|
||||
ctx, loadPage(url), chromedp.Evaluate(answerPrompt(false), nil),
|
||||
))
|
||||
click(ctx, t, form+" button")
|
||||
require.NoError(t, chromedp.Run(
|
||||
ctx,
|
||||
chromedp.Evaluate(`window.asked`, &asked),
|
||||
chromedp.Evaluate(`window.sent`, &sent),
|
||||
))
|
||||
assert.Equal(t, question, asked, "Delete does not ask about the target")
|
||||
assert.False(t, sent, "answering no still sends the delete form")
|
||||
|
||||
require.NoError(t, chromedp.Run(
|
||||
ctx, chromedp.Evaluate(answerPrompt(true), nil),
|
||||
))
|
||||
clickAndLoad(ctx, t, form+" button")
|
||||
require.NoError(t, chromedp.Run(ctx, chromedp.Evaluate(
|
||||
`document.querySelector('`+form+`') !== null`, &listed,
|
||||
)))
|
||||
assert.False(t, listed, "answering yes does not delete the target")
|
||||
}
|
||||
|
||||
+11
-3
@@ -1,7 +1,7 @@
|
||||
/*
|
||||
* The two shared styles for the controls a user clicks. Every page loads
|
||||
* this file after tailwind.css. It is plain CSS: make css does not build
|
||||
* it.
|
||||
* The two shared styles for the controls a user clicks, and the rule that
|
||||
* hides an element marked x-cloak. Every page loads this file after
|
||||
* tailwind.css. It is plain CSS: make css does not build it.
|
||||
*
|
||||
* A button is btn-primary, btn-secondary or btn-danger, from input.css.
|
||||
* A secondary or inline action, such as Copy beside an entrypoint URL or
|
||||
@@ -46,3 +46,11 @@
|
||||
outline-offset: 2px;
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* Alpine.js removes x-cloak from an element once it has started, so
|
||||
* something Alpine hides is not seen on the page while it loads.
|
||||
*/
|
||||
[x-cloak] {
|
||||
display: none !important;
|
||||
}
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -58,6 +58,18 @@
|
||||
}
|
||||
})();
|
||||
|
||||
// Asks before a form with a data-confirm attribute is sent, such as a
|
||||
// delete form on the webhook page. The browser's own prompt shows the
|
||||
// attribute's text, and the form is sent only if the user accepts.
|
||||
document.addEventListener("submit", function (event) {
|
||||
"use strict";
|
||||
|
||||
const message = event.target.getAttribute("data-confirm");
|
||||
if (message !== null && !window.confirm(message)) {
|
||||
event.preventDefault();
|
||||
}
|
||||
});
|
||||
|
||||
// Alpine.js components.
|
||||
//
|
||||
// The pages' Content-Security-Policy forbids eval, so the UI loads
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
{{if .Binary}}
|
||||
<p class="text-xs text-gray-500">This body is binary ({{.Size}} bytes) and is not shown. <a href="{{.EventURL}}/body" class="btn-small">Download the body</a></p>
|
||||
{{else if .Text}}
|
||||
<pre class="rounded-md border border-gray-200 bg-white p-2 text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all"{{if .Scroll}} style="max-height: 32rem; overflow-y: auto"{{end}}>{{.Text}}</pre>
|
||||
<pre class="rounded-md border border-gray-200 bg-white p-2 text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all{{if .Scroll}} max-h-[32rem] overflow-y-auto{{end}}">{{.Text}}</pre>
|
||||
{{if .Cut}}
|
||||
<p class="mt-2 text-xs text-gray-500">Showing the first {{.ShownBytes}} of {{.Size}} bytes, unformatted. <a href="{{.EventURL}}" class="btn-small">Show the whole body</a> <a href="{{.EventURL}}/body" class="btn-small">Download the body</a></p>
|
||||
{{end}}
|
||||
|
||||
@@ -4,6 +4,5 @@
|
||||
<title>{{block "title" .}}Webhooker{{end}}</title>
|
||||
<link rel="stylesheet" href="/s/css/tailwind.css">
|
||||
<link rel="stylesheet" href="/s/css/style.css">
|
||||
<style>[x-cloak] { display: none !important; }</style>
|
||||
{{block "head" .}}{{end}}
|
||||
{{end}}
|
||||
|
||||
@@ -13,13 +13,11 @@
|
||||
<div class="p-4">
|
||||
<!-- A value too wide to sit beside its name moves to the
|
||||
next line, where a list breaks only at the spaces
|
||||
between its entries. overflow-wrap: anywhere breaks
|
||||
inside a value only when it alone is wider than the
|
||||
line; an inline style, because the committed
|
||||
tailwind.css has no class for it. -->
|
||||
between its entries. wrap-anywhere breaks inside a
|
||||
value only when it alone is wider than the line. -->
|
||||
<div class="flex flex-wrap justify-between items-start gap-4">
|
||||
<code class="text-sm font-medium text-gray-900">{{.Name}}</code>
|
||||
<code class="text-sm text-gray-900" style="overflow-wrap: anywhere">{{.Value}}</code>
|
||||
<code class="text-sm text-gray-900 wrap-anywhere">{{.Value}}</code>
|
||||
</div>
|
||||
<p class="text-sm text-gray-500 mt-1">{{.Description}}</p>
|
||||
</div>
|
||||
|
||||
@@ -5,12 +5,10 @@
|
||||
{{define "content"}}
|
||||
<!-- 108rem, half again the 72rem (max-w-6xl) of the webhook list, the
|
||||
event log, the navbar and the footer, so an entrypoint URL fits on
|
||||
one line. An inline style, because the committed tailwind.css has
|
||||
no class this wide. wrap-anywhere goes only on names and
|
||||
descriptions: a row too wide for a phone must still run past the
|
||||
edge, where the browser test sees it, rather than break its
|
||||
controls mid-word. -->
|
||||
<div class="mx-auto px-6 py-8" style="max-width: 108rem">
|
||||
one line. wrap-anywhere goes only on names and descriptions: a
|
||||
row too wide for a phone must still run past the edge, where the
|
||||
browser test sees it, rather than break its controls mid-word. -->
|
||||
<div class="max-w-[108rem] mx-auto px-6 py-8">
|
||||
<div class="mb-6">
|
||||
<a href="/hooks" class="btn-small">← Back to webhooks</a>
|
||||
<div class="flex flex-wrap justify-between items-center gap-2 mt-2">
|
||||
@@ -23,11 +21,12 @@
|
||||
<div class="flex gap-2">
|
||||
<a href="/hook/{{.Webhook.ID}}/events" class="btn-secondary">Full Event Log</a>
|
||||
<a href="/hook/{{.Webhook.ID}}/edit" class="btn-secondary">Edit</a>
|
||||
<!-- The delete prompts are the browser's own, so they
|
||||
work without the page's scripts. The template
|
||||
escapes each name for the script, so a quote or a
|
||||
backslash in it shows as typed. -->
|
||||
<form method="POST" action="/hook/{{.Webhook.ID}}/delete" onsubmit="return confirm('Delete webhook "{{.Webhook.Name}}"?\n\nThis deletes its stored events{{with .Stats}} ({{.WithinRetention.Events}}){{end}} and their deliveries. Any archive files it wrote are kept.')">
|
||||
<!-- Before a delete form is sent, static/js/app.js
|
||||
asks in the browser's own prompt, with the form's
|
||||
data-confirm text; is a line break. The
|
||||
template escapes each name for the attribute, so a
|
||||
quote in it shows as typed. -->
|
||||
<form method="POST" action="/hook/{{.Webhook.ID}}/delete" data-confirm="Delete webhook "{{.Webhook.Name}}"? This deletes its stored events{{with .Stats}} ({{.WithinRetention.Events}}){{end}} and their deliveries. Any archive files it wrote are kept.">
|
||||
<input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
|
||||
<button type="submit" class="btn-danger">Delete</button>
|
||||
</form>
|
||||
@@ -90,7 +89,7 @@
|
||||
{{if .Active}}Deactivate{{else}}Activate{{end}}
|
||||
</button>
|
||||
</form>
|
||||
<form method="POST" action="/hook/{{$.Webhook.ID}}/entrypoints/{{.ID}}/delete" onsubmit="return confirm('Delete entrypoint "{{if .Description}}{{.Description}}{{else}}{{$.BaseURL}}/h/{{.Path}}{{end}}"?\n\nSenders using its URL get an error from now on, and the URL cannot be restored.')" class="inline">
|
||||
<form method="POST" action="/hook/{{$.Webhook.ID}}/entrypoints/{{.ID}}/delete" data-confirm="Delete entrypoint "{{if .Description}}{{.Description}}{{else}}{{$.BaseURL}}/h/{{.Path}}{{end}}"? Senders using its URL get an error from now on, and the URL cannot be restored." class="inline">
|
||||
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
|
||||
<button type="submit" class="btn-small text-red-600" title="Delete">Delete</button>
|
||||
</form>
|
||||
@@ -278,7 +277,7 @@
|
||||
{{if .Active}}Deactivate{{else}}Activate{{end}}
|
||||
</button>
|
||||
</form>
|
||||
<form method="POST" action="/hook/{{$.Webhook.ID}}/targets/{{.ID}}/delete" onsubmit="return confirm('Delete target "{{.Name}}"?\n\nNothing more is delivered to it. Its past deliveries stay in the event log.')" class="inline">
|
||||
<form method="POST" action="/hook/{{$.Webhook.ID}}/targets/{{.ID}}/delete" data-confirm="Delete target "{{.Name}}"? Nothing more is delivered to it. Its past deliveries stay in the event log." class="inline">
|
||||
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
|
||||
<button type="submit" class="btn-small text-red-600" title="Delete">Delete</button>
|
||||
</form>
|
||||
|
||||
Reference in New Issue
Block a user