Mask the http target's destination URL in the UI (closes #115)

An http target's destination is frequently a Slack, Discord or
Teams incoming-webhook endpoint whose path segments are the
credential — the same property that made the Slack target's
webhook URL a bearer token. The source detail page rendered it
in full, so the leak closed for slack targets stayed reachable
through a different target type.

Render it through the existing MaskURL, which reduces a URL to
scheme and host. The field accepts an arbitrary URL, so no path
segment can be assumed non-secret and none is shown.

.dockerignore

@@ -1,6 +1,3 @@
-# .ci-fingerprint is deliberately NOT excluded: it is the CI cache barrier
-# that keeps the check stages from replaying a cached pass. See the lint
-# stage of the Dockerfile.
 .git/
 bin/
 *.md

.gitea/workflows/check.yml

@@ -11,53 +11,5 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
-        with:
-          # The fingerprint step below needs history to find the last commit
-          # that touched the Docker build context.
-          fetch-depth: 0
-
-      - name: Neutralize superseded run statuses
-        # Gitea cancels the in-flight run when another commit is pushed to the
-        # same branch and records the cancellation as `failure`, so a commit
-        # that was never tested reads red. The cancellation is unconditional
-        # server-side for push events and cannot be disabled from a workflow
-        # file, so the superseding run rewrites those statuses to `skipped`.
-        # Only the exact cancellation status is touched; a real failure is
-        # left alone.
-        env:
-          GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
-        run: |
-          set -eu
-          api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
-          ctx='check / check (push)'
-          for sha in $(git rev-list --max-count=20 "${GITHUB_SHA}^" || true); do
-            latest="$(curl -sf "${api}/commits/${sha}/status" | jq -r \
-              --arg c "$ctx" \
-              '[.statuses[] | select(.context == $c)][0] // empty
-               | "\(.status)|\(.description)"')" || continue
-            [ "$latest" = 'failure|Has been cancelled' ] || continue
-            curl -sf -X POST "${api}/statuses/${sha}" \
-              -H "Authorization: token ${GITEA_TOKEN}" \
-              -H 'Content-Type: application/json' \
-              -d "$(jq -nc --arg c "$ctx" '{
-                context: $c,
-                state: "skipped",
-                description: "Superseded by a newer commit; never tested"
-              }')" >/dev/null
-            echo "neutralized superseded status on ${sha}"
-          done
-
-      - name: Fingerprint the build context
-        # `.dockerignore` keeps docs out of the build context, so a docs-only
-        # commit legitimately replays the whole image from cache and stays
-        # cheap. Every other commit writes a new fingerprint into the context,
-        # which invalidates the `COPY . .` layer of both check stages: a
-        # commit that was never linted, formatted-checked, tested and built
-        # cannot report success from cache.
-        run: |
-          set -eu
-          fp="$(git log -1 --format=%H -- . ':!*.md' ':!LICENSE' ':!.editorconfig')"
-          printf '%s\n' "${fp:-$GITHUB_SHA}" > .ci-fingerprint
-
       - name: Build Docker image (runs make check)
         run: script/cibuild

.gitignore

@@ -42,6 +42,3 @@ data/
 # Temporary files
 tmp/
 temp/ 
-
-# CI cache barrier, written into the build context by the check workflow
-.ci-fingerprint 

Dockerfile

@@ -12,11 +12,7 @@ WORKDIR /src
 COPY go.mod go.sum ./
 RUN go mod download
 
-# Copy source code. In CI the context also carries .ci-fingerprint, whose
+# Copy source code
-# value changes with every commit that touches the build context (see
-# .gitea/workflows/check.yml). That invalidates this layer, so the checks
-# below cannot report success by replaying a cached pass. Do not add it to
-# .dockerignore.
 COPY . .
 
 # Run formatting check and linter
@@ -40,8 +36,7 @@ WORKDIR /build
 COPY go.mod go.sum ./
 RUN go mod download
 
-# Copy source code, including the .ci-fingerprint cache barrier described in
+# Copy source code
-# the lint stage above.
 COPY . .
 
 # Run tests and build

README.md

@@ -93,7 +93,6 @@ TTY detection, and security headers are always applied.
 | `METRICS_USERNAME`      | Basic auth username for `/metrics`  | `""`     |
 | `METRICS_PASSWORD`      | Basic auth password for `/metrics`  | `""`     |
 | `SENTRY_DSN`            | Sentry error reporting DSN          | `""`     |
-| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
 | `SESSION_IDLE_TIMEOUT`  | Idle session timeout (Go duration)  | `24h`    |
 | `RECEIVER_RATE_LIMIT`   | Receiver requests/minute per IP per entrypoint | `120` |
 | `TRUSTED_PROXIES`       | CIDRs whose forwarded headers are trusted | `""` (none) |
@@ -174,12 +173,8 @@ its value and refuses to start, rather than silently running with a
 substituted default. `PORT=eighty`, `DEBUG=ture`, and
 `RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must
 additionally be a number in the range 1–65535,
-`RECEIVER_RATE_LIMIT` must be at least 1,
+`RECEIVER_RATE_LIMIT` must be at least 1, and every entry in
-`RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker
+`TRUSTED_PROXIES` must be a CIDR block or a bare IP address.
-period, so `0s` or a negative value would crash the reaper after
-startup), and every entry in `TRUSTED_PROXIES` must be a CIDR block or
-a bare IP address. `SESSION_IDLE_TIMEOUT` is the exception: a
-non-positive value there means idle expiry is disabled, not invalid.
 
 Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
 spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
@@ -265,10 +260,9 @@ webhooker solves this by acting as a durable intermediary:
    targets simultaneously. This enables patterns like forwarding a
    GitHub webhook to both a deployment service and a Slack channel.
 
-5. **Replay** (not yet implemented) — Every received event is stored in
+5. **Replay** — Stored events can be manually redelivered for debugging
-   full, which is what manual redelivery for debugging or testing will
+   or testing, without requiring the original sender to fire the webhook
-   be built on. No redelivery exists today, in the web UI or the API;
+   again.
-   see [TODO.md](TODO.md).
 
 ### Use Cases
 
@@ -278,7 +272,6 @@ webhooker solves this by acting as a durable intermediary:
   size, and delivery performance
 - **Debugging** and introspection of webhook payloads in the web UI
 - **Replay** of webhook events for application testing and development
-  (planned; not yet implemented)
 - **Fan-out** delivery of a single webhook to multiple downstream
   targets
 - **High-availability ingestion** for delivery to less reliable backend
@@ -504,7 +497,7 @@ A programmatic access credential for API authentication.
 #### Event
 
 A captured incoming webhook request. Stores the complete HTTP request
-data for auditing and for the planned replay capability.
+data for replay and auditing.
 
 | Field          | Type   | Description |
 | -------------- | ------ | ----------- |
@@ -786,10 +779,9 @@ unknown) one while one of its deliveries is still `retrying`, both
 recovery paths above terminally mark that delivery `failed` and record a
 `DeliveryResult` naming the current target type as the reason, logging it
 at warn level. The delivery is not re-dispatched under the new type — the
-operator never asked for that delivery — and while the event itself
+operator never asked for that delivery — and the event itself remains
-remains stored in the per-webhook event database, there is no way to
+stored in the per-webhook event database, so it can be redelivered
-redeliver it: manual redelivery is planned, not implemented (see
+manually.
-[TODO.md](TODO.md)).
 
 ### Circuit Breaker (HTTP Targets with Retries)
 
@@ -1112,34 +1104,6 @@ binary is statically linked and runs on Alpine.
 `docker build .` is the CI gate — if it passes, the code is formatted,
 linted, tested, and compiled.
 
-#### CI gate honesty
-
-A layer cache lets `docker build .` exit 0 in seconds with the lint and
-test stages replayed rather than executed, which would make a green
-check meaningless. The `check` workflow therefore writes
-`.ci-fingerprint` into the build context before building. Its value is
-the hash of the last commit that touched the build context, so:
-
-- Any commit that changes code (including a squash merge whose tree
-  matches an already-built branch) gets a new fingerprint, invalidates
-  the `COPY . .` layer of both check stages, and really runs
-  `make fmt-check`, `make lint`, `make test`, and `make build`. A run
-  that reports success ran them.
-- A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
-  excludes `*.md` and `LICENSE` from the context anyway — so the image
-  replays from cache and costs seconds.
-
-The module download layer sits above `COPY . .` and stays cached either
-way.
-
-The workflow's first step covers a second way the gate lied: Gitea
-cancels an in-flight run when a newer commit lands on the same branch
-and records that cancellation as a `failure` status, marking a commit
-red that was never tested. Cancellation is unconditional server-side for
-push events, so the superseding run rewrites the exact
-`Has been cancelled` status to `skipped`. Genuine failures are never
-touched.
-
 ## TODO
 
 See [TODO.md](TODO.md).

TODO.md

@@ -1,94 +1,35 @@
 # Workflow
 
-One issue per unit of work, one branch and one PR per issue:
+* branch (from `main`)
-
+* do the work in Next Step
-* ensure a tracked issue exists with a definition of done
+* move Next Step to the top of Completed Steps
-* branch from `next` (never from `main`)
+* move the top item of Future Steps into Next Step
-* do the work; open a PR based on `next` (never on `main`)
+* commit (`TODO.md` changes in the same commit as the work)
-* pass an independent review, then the manager squash-merges into `next`
+* merge to `main` if the branch is not protected, otherwise open a PR
-* push; nothing stays local-only
+* push
-
-`next` is the branch for the next milestone and must stay green and
-mergeable to `main` without notice. One `next` -> `main` PR accumulates
-the milestone; releases are cut from `main` separately.
-
-Issue branches do NOT touch this file — the manager maintains it on
-`next`. Every branch editing `TODO.md` conflicts with every other
-(#112).
 
 # Status
 
-pre-1.0. No git tags exist. `main` (4f5ecb1) is a working webhook proxy
+pre-1.0. No git tags exist. main (4f5ecb1) is a working webhook proxy
 with auth, CSRF/SSRF protections, login rate limiting, Slack target,
 event retention (#63), the database archiving target (#43), the admin
 password change flow (#65), policy compliance (#6), pinned lint tooling
-(#55), and fail-loud configuration parsing (#80).
+(#55), and fail-loud configuration parsing (#80). Note: TODO.md was
-
+deliberately deleted from this repo in f9a9569 (2026-03-01, #6); its
-`next` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
+content was folded into the README TODO section, which this draft
-is closed, and it is verified green by cache-defeated container runs
+reconstructs as of 2026-07-06.
-rather than by the CI badge, which can pass without executing anything
-(#119). Note: TODO.md was deliberately deleted from this repo in f9a9569
-(2026-03-01, #6); its content was folded into the README TODO section,
-which this draft reconstructs as of 2026-07-06.
 
 # Next Step
 
-Tag 1.0.0 from `main` once the milestone PR merges, then repair the CI
+Manual event redelivery from the web UI (replay is a core promised
-gate (#119) before the next cycle's work lands — a gate that can report
+capability in the README rationale).
-success without running is the one thing every other guarantee here
-rests on.
 
 # Completed Steps
 
-- 2026-08-12 Bound the `X-Forwarded-For` scan's allocation to the hop
-  cap: the reverse walk cuts entries with `strings.LastIndexByte`
-  instead of joining and splitting, so a 1 MB header allocates 16 bytes
-  rather than 1.6 MB per request on the unauthenticated receiver.
-  Semantics proven unchanged by differential testing against the
-  previous implementation (#133)
-- 2026-08-12 Cap the `X-Forwarded-For` hop walk at 64 entries, so an
-  attacker-supplied chain cannot burn unbounded CPU in the rate-limit
-  key function; running off the end falls back to the peer address
-  (#124)
-- 2026-08-12 Gate forwarded-header trust behind a `TRUSTED_PROXIES` CIDR
-  list: all three rate limiters key on the connection's own address
-  unless the direct peer is a configured proxy, in which case
-  `X-Forwarded-For` is walked right to left for the first non-proxy hop.
-  Default trusts nothing, and a set-but-unparseable value aborts
-  startup. Before this, any client could mint a fresh bucket or drain
-  another's by rotating a spoofed header (#88)
 - 2026-08-11 Web UI cleanup: nav terminology unified on Webhooks, the
   Profile settings placeholder removed, a progressive-enhancement copy
   button for the entrypoint URL, and retention form copy that states the
   actual policy (deletion by the reaper, 0 retains forever) (#57)
-- 2026-08-11 Mask the webhook credential in delivery errors and logs:
-  Go embeds the request URL in `*url.Error`, so every transport failure
-  persisted the full Slack webhook URL into the per-webhook event
-  database via `DeliveryResult.Error`, a field a future REST API would
-  have served. `maskURLError` drops path, query and userinfo while
-  preserving the wrapped cause, so `errors.Is`/`As` and `Timeout()`
-  still work and DNS, TLS and timeout failures still read differently
-  (#118)
-- 2026-08-11 Rate-limit the public webhook receiver endpoint
-  (`RECEIVER_RATE_LIMIT`, default 120/min), keyed on client IP plus
-  entrypoint path so one entrypoint cannot exhaust another's budget;
-  over-limit requests get 429 with `Retry-After`. It was the one
-  unauthenticated, internet-facing endpoint with no limit at all (#64)
-- 2026-08-11 Enforce the body size limit before CSRF parses the form:
-  `MaxBodySize` is now first in all four form-parsing route groups, so
-  an oversized request is rejected with 413 instead of being read in
-  full by the CSRF middleware before any cap applied (#90)
-- 2026-08-11 Mask target config on the source detail page, which
-  rendered the stored blob verbatim and so exposed the Slack
-  incoming-webhook URL — a bearer credential that cannot be revoked
-  per-holder. Config reaches the template only as a `TargetView` of
-  labelled fields, and header values are rendered as a count (#113)
-- 2026-08-11 Allow `retention_days` of 0 to mean retain forever, via a
-  sentinel written in `BeforeSave` so the GORM column default cannot
-  win the race. Also bounds the reaper's cutoff arithmetic: day counts
-  above 106751 overflowed `time.Duration` and wrapped the cutoff into
-  the future, where every row matched and the sweep deleted everything
-  (#79)
 - 2026-08-09 Inactivity-based session timeout: sliding idle expiry
   (`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
   requests, with the 7-day absolute cap kept as an independent
@@ -143,9 +84,6 @@ rests on.
 
 # Future Steps
 
-- Manual event redelivery from the web UI — the "Replay" capability the
-  README describes as planned. No redelivery code exists anywhere in the
-  tree; events are stored in full, which is all it would be built on
 - Delivery status and retry management UI
 - Per-webhook rate limiting in the receiver handler (per-webhook config
   plus handler enforcement; global limits must not apply to receiver

internal/config/config.go

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

internal/config/config_test.go

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

internal/delivery/engine.go

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

internal/delivery/target_config_view.go

@@ -106,6 +106,12 @@ func slackConfigFields(configJSON string) []ConfigField {
 // and its retry settings. Header values are not shown — they
 // routinely carry authorization tokens — only how many are
 // configured.
+//
+// The destination is masked to scheme and host by the same
+// rule the Slack target uses. An HTTP target's destination is
+// commonly a Slack, Discord or Teams incoming-webhook endpoint
+// whose path segments are the credential, and the field takes
+// an arbitrary URL, so no segment can be assumed non-secret.
 func httpConfigFields(t *database.Target) []ConfigField {
 	cfg, err := parseHTTPConfig(t.Config)
 	if err != nil {
@@ -114,7 +120,7 @@ func httpConfigFields(t *database.Target) []ConfigField {
 
 	fields := []ConfigField{{
 		Label: "Destination URL",
-		Value: cfg.URL,
+		Value: MaskURL(cfg.URL),
 	}}
 
 	if cfg.Timeout > 0 {

internal/delivery/target_config_view_test.go

@@ -19,6 +19,7 @@ const (
 
 	viewExampleOrigin = "https://example.com"
 	viewExampleHook   = viewExampleOrigin + "/hook"
+	viewMaskedOrigin  = viewExampleOrigin + "/..."
 	viewUnavailable   = "(unavailable)"
 	viewExpiryNever   = "never"
 )
@@ -162,7 +163,7 @@ func TestNewTargetViews_HTTP(t *testing.T) {
 	assert.Equal(
 		t,
 		map[string]string{
-			"Destination URL": viewExampleHook,
+			"Destination URL": viewMaskedOrigin,
 			"Timeout":         "30s",
 			"Headers":         "1 configured",
 			"Max Retries":     "5",
@@ -188,13 +189,41 @@ func TestNewTargetViews_HTTPFireAndForget(t *testing.T) {
 	assert.Equal(
 		t,
 		map[string]string{
-			"Destination URL": viewExampleHook,
+			"Destination URL": viewMaskedOrigin,
 			"Max Retries":     "0 (fire-and-forget)",
 		},
 		fieldMap(view.Config),
 	)
 }
 
+// TestNewTargetViews_HTTPMasksDestinationURL proves the rule
+// holds for the http target too: an http destination is
+// routinely an incoming-webhook endpoint whose path segments
+// are the credential, so none of them is shown.
+func TestNewTargetViews_HTTPMasksDestinationURL(t *testing.T) {
+	t.Parallel()
+
+	view := viewFor(t, database.Target{
+		Type:   database.TargetTypeHTTP,
+		Config: `{"url":"` + slackWebhookURL + `"}`,
+	})
+
+	fields := fieldMap(view.Config)
+
+	assert.Equal(
+		t,
+		"https://hooks.slack.com/...",
+		fields["Destination URL"],
+	)
+
+	for _, v := range fields {
+		assert.NotContains(t, v, slackSecretPath)
+		assert.NotContains(t, v, "T00000000")
+		assert.NotContains(t, v, "B00000000")
+		assert.NotContains(t, v, "XXXXXXXXXXXXXXXXXXXXXXXX")
+	}
+}
+
 func TestNewTargetViews_Database(t *testing.T) {
 	t.Parallel()
 

internal/handlers/source_detail_test.go

@@ -131,6 +131,47 @@ func TestHandleSourceDetail_MasksSlackWebhookURL(t *testing.T) {
 	assert.Contains(t, body, "https://hooks.slack.com/...")
 }
 
+// TestHandleSourceDetail_MasksHTTPDestinationURL is the
+// regression test for the same leak reached through the http
+// target: its destination is routinely an incoming-webhook
+// endpoint whose path segments are the credential, so the
+// rendered page must not contain them.
+func TestHandleSourceDetail_MasksHTTPDestinationURL(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h    *handlers.Handlers
+		sess *session.Session
+		db   *database.Database
+	)
+
+	app := newTestApp(t, &h, &sess, &db)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	wh := seedWebhook(t, db)
+	seedConfiguredTarget(
+		t, db, wh.ID,
+		database.TargetTypeHTTP,
+		`{"url":"`+slackWebhookURL+`"}`,
+	)
+
+	body := renderSourceDetailPage(t, h, sess, wh.ID)
+
+	assert.NotContains(t, body, slackSecretPath)
+	assert.NotContains(t, body, "T00000000")
+	assert.NotContains(t, body, "B00000000")
+	assert.NotContains(
+		t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
+	)
+
+	assert.Contains(t, body, "Destination URL")
+	assert.Contains(t, body, "https://hooks.slack.com/...")
+}
+
 // TestHandleSourceDetail_RendersNamedTargetFields proves the
 // other target types render labelled fields rather than the
 // stored blob.
@@ -172,7 +213,7 @@ func TestHandleSourceDetail_RendersNamedTargetFields(
 	body := renderSourceDetailPage(t, h, sess, wh.ID)
 
 	assert.Contains(t, body, "Destination URL")
-	assert.Contains(t, body, "https://example.com/hook")
+	assert.Contains(t, body, "https://example.com/...")
 	assert.Contains(t, body, "Timeout")
 	assert.Contains(t, body, "1 configured")
 	assert.NotContains(t, body, "sekrit")

internal/middleware/export_test.go

@@ -25,11 +25,6 @@ func IPFromHostPort(hp string) string {
 	return ipFromHostPort(hp)
 }
 
-// ClientKeyForTest exposes clientKey for testing.
-func ClientKeyForTest(m *Middleware, r *http.Request) string {
-	return m.clientKey(r)
-}
-
 // IsClientTLS exposes isClientTLS for testing.
 func IsClientTLS(r *http.Request) bool {
 	return isClientTLS(r)

internal/middleware/ratelimit.go

@@ -32,13 +32,6 @@ const (
 	// receiver rate limit. The configured limit is expressed in
 	// requests per minute.
 	receiverRateInterval = 1 * time.Minute
-
-	// maxForwardedHops bounds how many X-Forwarded-For entries the
-	// chain walk examines. Real chains are one to three hops, but a
-	// client can pad the header up to MaxHeaderBytes, so without a
-	// bound every request pays a walk proportional to whatever the
-	// client sent.
-	maxForwardedHops = 64
 )
 
 // normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
@@ -77,36 +70,14 @@ func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
 // a trusted proxy is the client. A hop that cannot be read as a bare
 // address ends the walk: past it the chain is not the shape assumed
 // here, so the caller falls back to the peer address.
-//
-// Only the last maxForwardedHops entries are examined. A longer chain
-// is padding, and running out of hops falls back to the peer address
-// the same way an unreadable hop does.
-//
-// The entries are cut off the right end of each header value in place
-// rather than split out of it: the receiver is unauthenticated and a
-// client can pad the header up to MaxHeaderBytes, so splitting would
-// allocate in proportion to the padding (about 8 MB for a 1 MB
-// header) before the cap could discard any of it. Multiple header
-// values are walked in reverse for the same reason, since joining
-// them copies the whole chain.
 func (m *Middleware) forwardedClientAddr(
 	r *http.Request,
 ) (netip.Addr, bool) {
-	seen := 0
+	hops := strings.Split(
-
+		strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
-	for _, value := range slices.Backward(
+	)
-		r.Header.Values("X-Forwarded-For"),
-	) {
-		for last := false; !last && seen < maxForwardedHops; seen++ {
-			hop := value
-
-			comma := strings.LastIndexByte(value, ',')
-			if comma < 0 {
-				last = true
-			} else {
-				hop, value = value[comma+1:], value[:comma]
-			}
 
+	for _, hop := range slices.Backward(hops) {
 		hop = strings.TrimSpace(hop)
 		if hop == "" {
 			continue
@@ -121,7 +92,6 @@ func (m *Middleware) forwardedClientAddr(
 			return addr, true
 		}
 	}
-	}
 
 	return netip.Addr{}, false
 }
@@ -143,10 +113,8 @@ func (m *Middleware) clientKey(r *http.Request) string {
 	peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
 	if err != nil {
 		// Not an address we can reason about; key on the raw
-		// value, the most specific identity left. On a
+		// value rather than collapsing such peers into one
-		// Unix-socket listener every peer carries the same
+		// shared bucket.
-		// RemoteAddr and so shares one bucket, which is the
-		// fail-closed direction.
 		return r.RemoteAddr
 	}
 

internal/middleware/ratelimit_test.go

@@ -8,10 +8,7 @@ import (
 	"net/http/httptest"
 	"net/netip"
 	"os"
-	"runtime"
-	"strings"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"sneak.berlin/go/webhooker/internal/config"
@@ -571,105 +568,6 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
 	)
 }
 
-// TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer covers the
-// hop-walk cap. A client behind the trusted proxy can pad
-// X-Forwarded-For with tens of thousands of trusted-looking hops,
-// which costs a walk proportional to the padding and, once the walk
-// runs off the left end of the chain, reaches the entry the client
-// put there. Capping the walk stops both: the key falls back to the
-// peer address, so rotating the head of the chain mints no bucket,
-// and the run does not scale with the chain length.
-func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	// 50k hops is roughly 0.9 MB, within the default
-	// MaxHeaderBytes.
-	const hops = 50000
-
-	padding := strings.Repeat(", 10.0.0.2", hops-1)
-
-	start := time.Now()
-
-	assertSharedBucket(
-		t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
-		func(i int) map[string]string {
-			return map[string]string{
-				headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
-			}
-		},
-		"a padded X-Forwarded-For chain must fall back to the "+
-			"peer address, not reach the client-controlled entry "+
-			"at the head of the chain",
-	)
-
-	assert.Less(
-		t, time.Since(start), 2*time.Second,
-		"the capped walk must not scale with the chain length",
-	)
-}
-
-// TestRateLimitKey_LongChainAllocationIsBounded is the allocation
-// half of the hop cap. Capping the walk still left every request
-// paying for the whole header the client sent, because the chain was
-// split before it was capped: about 8 MB of []string for the 1 MB a
-// default MaxHeaderBytes allows, on the unauthenticated receiver.
-//
-// Bytes are the measurement, not allocation count: strings.Split of a
-// 1 MB chain is a single allocation, so testing.AllocsPerRun scores
-// it as cheap. The test is deliberately sequential — it reads
-// process-wide counters, and Go runs this package's parallel tests
-// only after the sequential ones finish.
-//
-//nolint:paralleltest // reads process-wide allocation counters
-func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
-	// 100k hops of ", 10.0.0.2" is roughly 1 MB.
-	const (
-		hops            = 100000
-		iterations      = 50
-		maxBytesPerCall = 4096
-	)
-
-	m := rateLimitMiddleware(t, &config.Config{
-		TrustedProxies: trustedProxies("10.0.0.0/8"),
-	})
-
-	req := httptest.NewRequestWithContext(
-		context.Background(), http.MethodPost, loginPath, nil,
-	)
-	req.RemoteAddr = "10.0.0.1:44444"
-	req.Header.Set(
-		headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
-	)
-
-	var before, after runtime.MemStats
-
-	var key string
-
-	runtime.ReadMemStats(&before)
-
-	for range iterations {
-		key = middleware.ClientKeyForTest(m, req)
-	}
-
-	runtime.ReadMemStats(&after)
-
-	perCall := (after.TotalAlloc - before.TotalAlloc) / iterations
-
-	assert.Less(
-		t, perCall, uint64(maxBytesPerCall),
-		"a %d-byte X-Forwarded-For must not allocate in proportion "+
-			"to its length, but cost %d bytes per call",
-		len(req.Header.Get(headerXFF)), perCall,
-	)
-
-	assert.Equal(
-		t, "10.0.0.1", key,
-		"the padded chain must still fall back to the peer address",
-	)
-}
-
 // TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
 // the receiver limiter uses the same gated key function as the
 // POST limiters.

static/js/app.js

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