1 Commits
Author SHA1 Message Date
sneak 86224d4982 Default WEBHOOKER_ENVIRONMENT to prod
check / check (push) Successful in 4m2s
An unset WEBHOOKER_ENVIRONMENT now resolves to prod rather than dev, so
an operator who forgets the variable is not silently permissive. The
only behaviour dev still changes is the CORS middleware, which answers
every origin with Access-Control-Allow-Origin: *; that is now off
unless dev is set explicitly. Cookie Secure and CSRF strictness are
decided per request from the transport and are unaffected.

Updated resolveEnvironment and its comment, the README configuration
table and prose, and tests covering the default and the explicit dev.
Comments that justified behaviour by the old dev default (the
TRUSTED_PROXIES warning, a CSRF test) were corrected; that warning
still fires in every environment when TRUSTED_PROXIES is empty.

Model: opus-4-8
2026-09-21 13:04:56 +00:00
7 changed files with 30 additions and 111 deletions

No files matched your search

+21 -22
View File
@@ -44,9 +44,9 @@ make bootstrap
# Run all checks (test, lint, format check) # Run all checks (test, lint, format check)
make check make check
# Run the server from the clone. DATA_DIR defaults to # Run in development mode. DATA_DIR defaults to /var/lib/webhooker in
# /var/lib/webhooker in every environment, so set it (in .env or the # every environment, so set it (in .env or the shell) to a writable
# shell) to a writable directory. # directory when running from a clone.
DATA_DIR=./data make dev DATA_DIR=./data make dev
# Build Docker image # Build Docker image
@@ -753,18 +753,17 @@ reports.
serves the admin login form and the unauthenticated receiver with serves the admin login form and the unauthenticated receiver with
no TLS at all, and the proxy in front of it changes nothing about no TLS at all, and the proxy in front of it changes nothing about
that. that.
2. **Make sure the environment is not `dev` (leave 2. **Set `WEBHOOKER_ENVIRONMENT=prod`, and make sure the proxy sends
`WEBHOOKER_ENVIRONMENT` unset or set it to `prod`), and make sure `X-Forwarded-Proto`.** These are two requirements, not one. The
the proxy sends `X-Forwarded-Proto`.** These are two requirements, environment setting decides CORS and nothing else: `dev` answers
not one. The environment setting decides CORS and nothing else: every origin with `Access-Control-Allow-Origin: *` (without
`dev` answers every origin with `Access-Control-Allow-Origin: *` credentials), which a server-rendered production deployment has no
(without credentials), which a server-rendered production use for, and `prod` — the default — disables it. Cookie `Secure`
deployment has no use for, and `prod` — the default — disables it. and the strict Origin/Referer mode are **not** tied to it — they
Cookie `Secure` and the strict Origin/Referer mode are **not** tied are decided per request from the transport, which behind a proxy
to it — they are decided per request from the transport, which means the `X-Forwarded-Proto` header. The block below sets it;
behind a proxy means the `X-Forwarded-Proto` header. The block below without it every request is read as plaintext and cookies ship
sets it; without it every request is read as plaintext and cookies without `Secure`. See [Configuration](#configuration).
ship without `Secure`. See [Configuration](#configuration).
3. **Set `TRUSTED_PROXIES` to the proxy's address.** Unset, every rate 3. **Set `TRUSTED_PROXIES` to the proxy's address.** Unset, every rate
limiter keys on the connecting peer, which behind a proxy is the limiter keys on the connecting peer, which behind a proxy is the
proxy on every request: all clients collapse into one global bucket proxy on every request: all clients collapse into one global bucket
@@ -1509,7 +1508,7 @@ events should be forwarded.
| `type` | TargetType | One of: `http`, `slack`, `database`, `log` | | `type` | TargetType | One of: `http`, `slack`, `database`, `log` |
| `active` | boolean | Whether deliveries are enabled (default: true) | | `active` | boolean | Whether deliveries are enabled (default: true) |
| `config` | JSON text | Type-specific configuration | | `config` | JSON text | Type-specific configuration |
| `max_retries` | integer | Total delivery attempts for `http` and `slack` targets, not retries on top of the first: 0 is a single fire-and-forget attempt with no retries and no circuit breaker, and a value of N makes N attempts in all, with exponential backoff and a per-target circuit breaker. Ignored by `database` and `log` targets | | `max_retries` | integer | Maximum retry attempts for `http` and `slack` targets (0 = fire-and-forget, >0 = retries with backoff and a circuit breaker). Ignored by `database` and `log` targets |
| `max_queue_size` | integer | Stored and shown on the target's detail view, but not enforced anywhere yet: nothing in the delivery engine consults it. Queue depth is set by the two fixed 10,000-entry channels | | `max_queue_size` | integer | Stored and shown on the target's detail view, but not enforced anywhere yet: nothing in the delivery engine consults it. Queue depth is set by the two fixed 10,000-entry channels |
**Relations:** Belongs to Webhook. Has many Deliveries. **Relations:** Belongs to Webhook. Has many Deliveries.
@@ -1517,12 +1516,12 @@ events should be forwarded.
**Target types:** **Target types:**
- **`http`** — Forward the event as an HTTP POST to a configured URL. - **`http`** — Forward the event as an HTTP POST to a configured URL.
`max_retries` is the total number of delivery attempts, not retries on Behavior depends on `max_retries`: when `max_retries` is 0 (the
top of the first: when `max_retries` is 0 (the default), the target default), the target operates in fire-and-forget mode — a single
operates in fire-and-forget mode, a single attempt with no retries and attempt with no retries and no circuit breaker. When `max_retries` is
no circuit breaker; a value of N makes up to N attempts in all, greater than 0, failed deliveries are retried with exponential backoff
retrying failed deliveries with exponential backoff and protecting them up to `max_retries` attempts, protected by a per-target circuit
with a per-target circuit breaker. breaker.
- **`slack`** — Post the event as a formatted message to a - **`slack`** — Post the event as a formatted message to a
Slack-compatible incoming webhook URL (`webhookUrl` in `config`). It Slack-compatible incoming webhook URL (`webhookUrl` in `config`). It
is built on the same HTTP core as `http` and honours `max_retries` is built on the same HTTP core as `http` and honours `max_retries`
-77
View File
@@ -300,80 +300,3 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
"the page must render to completion, not abort partway", "the page must render to completion, not abort partway",
) )
} }
// maxRetriesHelp is the wording both target forms must carry. The
// delivery core makes max_retries attempts in total, not that many
// retries on top of a first try (a fresh delivery starts at attempt 1
// and target_http gives up once the attempt number reaches
// max_retries), and 0 is special-cased to a single fire-and-forget
// attempt with no circuit breaker.
const maxRetriesHelp = "This is the total number of delivery attempts, " +
"not retries on top of the first: a value of 3 makes three attempts " +
"in all. 0 means a single attempt with no retries and no circuit " +
"breaker."
// TestTargetFormMaxRetriesCopyMatchesBehaviour pins the max_retries
// help text on both the create form (the add-target form on the webhook
// detail page) and the edit form, so the copy cannot drift back to
// calling the number a retry count.
func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
var sess *session.Session
app := newTestApp(t, &h, &sess)
app.RequireStart()
t.Cleanup(app.RequireStop)
webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
webhook.ID = testWebhookID
entrypoint := database.Entrypoint{Path: "abc123"}
entrypoint.ID = "ep-1"
createBody := renderPage(
t, h, sess, "source_detail.html", map[string]any{
dataKeyWebhook: webhook,
"Entrypoints": handlers.NewEntrypointViews(
[]database.Entrypoint{entrypoint},
),
"Targets": delivery.NewTargetViews(nil),
"Events": []database.Event{},
"BaseURL": "https://hooks.example.com",
},
)
assert.Contains(
t, createBody, maxRetriesHelp,
"the add-target form must explain max_retries as total attempts",
)
// A slack target exercises the same max_retries field while needing
// only Config.URL from the edit template, so the test data stays
// minimal. The Target key mirrors the field names the template reads
// off the handler's view value.
editBody := renderPage(
t, h, sess, "target_edit.html", map[string]any{
dataKeyWebhook: webhook,
"Target": map[string]any{
"ID": "tg-1",
"Name": "t",
"Type": "slack",
"Active": true,
"MaxRetries": 3,
"Config": map[string]any{
"URL": "https://hooks.slack.com/services/x",
},
},
dataKeyError: "",
},
)
assert.Contains(
t, editBody, maxRetriesHelp,
"the target edit form must explain max_retries as total attempts",
)
}
+1 -1
View File
@@ -5,7 +5,7 @@
// several packages, by hand, and the answers disagreed. The session // several packages, by hand, and the answers disagreed. The session
// cookie's Secure attribute was decided at startup from the configured // cookie's Secure attribute was decided at startup from the configured
// environment while the CSRF cookie's was decided per-request, so a // environment while the CSRF cookie's was decided per-request, so a
// deployment behind a TLS proxy in the dev environment emitted one // deployment behind a TLS proxy in the default environment emitted one
// Secure cookie and one non-Secure cookie on the same response. // Secure cookie and one non-Secure cookie on the same response.
// Everything kept working, which is exactly why nobody noticed. // Everything kept working, which is exactly why nobody noticed.
// //
+2 -2
View File
@@ -146,8 +146,8 @@ func newStore(key []byte) *sessions.CookieStore {
// //
// This is decided per-request, not once at startup. Deciding it at // This is decided per-request, not once at startup. Deciding it at
// startup from the configured environment is what this replaces, and // startup from the configured environment is what this replaces, and
// it got the DEFAULT posture wrong: "dev" was then the environment when // it got the DEFAULT posture wrong: "dev" is the environment when
// WEBHOOKER_ENVIRONMENT was unset, so a deployment terminating TLS at a // WEBHOOKER_ENVIRONMENT is unset, so a deployment terminating TLS at a
// proxy without also setting the environment emitted the // proxy without also setting the environment emitted the
// authentication cookie with no Secure attribute -- silently, and on // authentication cookie with no Secure attribute -- silently, and on
// the same response as a CSRF cookie that did have one. // the same response as a CSRF cookie that did have one.
+2 -2
View File
@@ -990,8 +990,8 @@ func sessionCookieFrom(
// TestSave_SecureFollowsRequestTransport is the regression test for // TestSave_SecureFollowsRequestTransport is the regression test for
// the defect this replaces: Secure was fixed at startup from the // the defect this replaces: Secure was fixed at startup from the
// configured environment, and "dev" was then the environment when // configured environment, and "dev" is the environment when
// WEBHOOKER_ENVIRONMENT was unset. A deployment behind a TLS proxy in // WEBHOOKER_ENVIRONMENT is unset. A deployment behind a TLS proxy in
// that DEFAULT posture shipped the authentication cookie with no // that DEFAULT posture shipped the authentication cookie with no
// Secure attribute and said nothing about it. // Secure attribute and said nothing about it.
// //
+3 -6
View File
@@ -120,12 +120,9 @@
<label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label> <label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
<input type="number" name="timeout" min="0" max="300" :disabled="targetType !== 'http'" class="input text-sm w-24"> <input type="number" name="timeout" min="0" max="300" :disabled="targetType !== 'http'" class="input text-sm w-24">
</div> </div>
<div x-show="targetType === 'http'"> <div x-show="targetType === 'http'" class="flex gap-2 items-center">
<div class="flex gap-2 items-center"> <label class="text-sm text-gray-700">Max retries (0 = fire-and-forget):</label>
<label class="text-sm text-gray-700">Max retries:</label> <input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
<input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
</div>
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
</div> </div>
<div x-show="targetType === 'slack'"> <div x-show="targetType === 'slack'">
<input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="targetType !== 'slack'" class="input text-sm"> <input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="targetType !== 'slack'" class="input text-sm">
+1 -1
View File
@@ -69,7 +69,7 @@
<div class="form-group"> <div class="form-group">
<label for="max_retries" class="label">Max retries</label> <label for="max_retries" class="label">Max retries</label>
<input type="number" id="max_retries" name="max_retries" value="{{.Target.MaxRetries}}" min="0" max="20" class="input"> <input type="number" id="max_retries" name="max_retries" value="{{.Target.MaxRetries}}" min="0" max="20" class="input">
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p> <p class="text-xs text-gray-500 mt-1">0 is fire-and-forget: one attempt, no circuit breaker.</p>
</div> </div>
{{end}} {{end}}