Mask target config on the source detail page (closes #113) #114

clawbot · 2026-08-11T14:27:23+02:00

clawbot commented

2026-08-11 14:27:23 +02:00

Closes #113

templates/source_detail.html rendered each target's stored config blob verbatim. For a slack target that blob holds the incoming webhook URL, which is a bearer credential — anyone holding it can post to the channel indefinitely, and it cannot be scoped or revoked per holder. Rendering it put the credential into browser history, screenshots and any support screen share.

What changed

New internal/delivery/target_config_view.go. NewTargetViews projects []database.Target into TargetView, which has no raw config field at all — the blob is not reachable from any template, present or future. The template ranges ConfigField{Label, Value} pairs instead.
Masking lives on the config type: (*SlackTargetConfig).MaskedWebhookURL(), backed by maskURL, so it is unit-testable and cannot be bypassed from a template. It reduces the URL to scheme + host and elides the path, query and any userinfo.
Fields per target type: slack → masked webhook URL; http → destination URL, timeout, header count, max retries, max queue size; database → archive expiry; log → none.
No raw-blob fallback anywhere. An empty, unknown-type, or unparseable config renders Configuration: (unavailable).

Two deliberate calls

The whole path is elided, not just the trailing segments. The issue's example is https://hooks.slack.com/services/...; this renders https://hooks.slack.com/.... The field accepts an arbitrary URL, so no path segment can be assumed non-secret — keeping the first one would be safe for Slack's fixed /services/ prefix and unsafe for anything else. Still scheme + host + an elided path, per the definition of done.
HTTP header values are not shown, only how many are configured. They routinely carry Authorization: Bearer ...; rendering them would reintroduce the same class of leak the issue is about.

Verification

internal/handlers/source_detail_test.go — TestHandleSourceDetail_MasksSlackWebhookURL renders the real source detail page through HandleSourceDetail for a slack target and asserts the body contains none of the webhook URL's secret path segments (nor the webhookUrl key), and does contain the masked value. This is the load-bearing regression test.
TestHandleSourceDetail_RendersNamedTargetFields covers http, database and an unknown type end-to-end, asserting the header value never reaches the page and the unknown type gets the placeholder.
internal/delivery/target_config_view_test.go — unit tests for the masking (query, userinfo, no path, unparseable, empty) and the per-type field sets, including every unpresentable-config path.

make check: green, 0 issues. from the linter.

docker build --no-cache-filter=lint,builder --progress=plain . (scoped invalidation, no prune) — the lint and test layers executed rather than replaying cache:

#14 [lint 7/8] RUN make fmt-check
#14 DONE 1.3s
#15 [lint 8/8] RUN make lint
#15 94.98 0 issues.
#15 DONE 96.4s
#23 [builder  8/10] RUN make test
#23 101.2 --- PASS: TestMaskedWebhookURL (0.00s)
#23 101.2 --- PASS: TestHandleSourceDetail_MasksSlackWebhookURL (3.15s)
#23 101.2 --- PASS: TestHandleSourceDetail_RendersNamedTargetFields (3.23s)
#23 DONE 104.7s

script/cibuild then exits 0.

Scope

Rendering only — the stored config format and the delivery path are untouched. No new Tailwind classes (text-xs, text-gray-500, text-gray-700, break-all, mt-1, font-medium are all already in static/css/tailwind.css), so no CSS regeneration. TODO.md untouched.

One lint suppression, disclosed: //nolint:gosec // G101 on the test case whose URL carries fabricated user:pw@ userinfo — that case exists precisely to prove userinfo is stripped by the mask.

Noted while working, not addressed here: the linter warns that gomodguard is deprecated in favour of gomodguard_v2. It is a repo-wide config change, out of scope for this issue.

Closes https://git.eeqj.de/sneak/webhooker/issues/113 `templates/source_detail.html` rendered each target's stored config blob verbatim. For a `slack` target that blob holds the incoming webhook URL, which is a bearer credential — anyone holding it can post to the channel indefinitely, and it cannot be scoped or revoked per holder. Rendering it put the credential into browser history, screenshots and any support screen share. ## What changed - **New `internal/delivery/target_config_view.go`.** `NewTargetViews` projects `[]database.Target` into `TargetView`, which has **no raw config field at all** — the blob is not reachable from any template, present or future. The template ranges `ConfigField{Label, Value}` pairs instead. - **Masking lives on the config type**: `(*SlackTargetConfig).MaskedWebhookURL()`, backed by `maskURL`, so it is unit-testable and cannot be bypassed from a template. It reduces the URL to scheme + host and elides the path, query and any userinfo. - **Fields per target type**: `slack` → masked webhook URL; `http` → destination URL, timeout, header *count*, max retries, max queue size; `database` → archive expiry; `log` → none. - **No raw-blob fallback anywhere.** An empty, unknown-type, or unparseable config renders `Configuration: (unavailable)`. ## Two deliberate calls - **The whole path is elided, not just the trailing segments.** The issue's example is `https://hooks.slack.com/services/...`; this renders `https://hooks.slack.com/...`. The field accepts an arbitrary URL, so no path segment can be assumed non-secret — keeping the first one would be safe for Slack's fixed `/services/` prefix and unsafe for anything else. Still scheme + host + an elided path, per the definition of done. - **HTTP header values are not shown, only how many are configured.** They routinely carry `Authorization: Bearer ...`; rendering them would reintroduce the same class of leak the issue is about. ## Verification - `internal/handlers/source_detail_test.go` — `TestHandleSourceDetail_MasksSlackWebhookURL` renders the real source detail page through `HandleSourceDetail` for a `slack` target and asserts the body contains none of the webhook URL's secret path segments (nor the `webhookUrl` key), and does contain the masked value. This is the load-bearing regression test. - `TestHandleSourceDetail_RendersNamedTargetFields` covers `http`, `database` and an unknown type end-to-end, asserting the header value never reaches the page and the unknown type gets the placeholder. - `internal/delivery/target_config_view_test.go` — unit tests for the masking (query, userinfo, no path, unparseable, empty) and the per-type field sets, including every unpresentable-config path. `make check`: green, `0 issues.` from the linter. `docker build --no-cache-filter=lint,builder --progress=plain .` (scoped invalidation, no prune) — the lint and test layers executed rather than replaying cache: ``` #14 [lint 7/8] RUN make fmt-check #14 DONE 1.3s #15 [lint 8/8] RUN make lint #15 94.98 0 issues. #15 DONE 96.4s #23 [builder 8/10] RUN make test #23 101.2 --- PASS: TestMaskedWebhookURL (0.00s) #23 101.2 --- PASS: TestHandleSourceDetail_MasksSlackWebhookURL (3.15s) #23 101.2 --- PASS: TestHandleSourceDetail_RendersNamedTargetFields (3.23s) #23 DONE 104.7s ``` `script/cibuild` then exits 0. ## Scope Rendering only — the stored config format and the delivery path are untouched. No new Tailwind classes (`text-xs`, `text-gray-500`, `text-gray-700`, `break-all`, `mt-1`, `font-medium` are all already in `static/css/tailwind.css`), so no CSS regeneration. `TODO.md` untouched. One lint suppression, disclosed: `//nolint:gosec // G101` on the test case whose URL carries fabricated `user:pw@` userinfo — that case exists precisely to prove userinfo is stripped by the mask. Noted while working, not addressed here: the linter warns that `gomodguard` is deprecated in favour of `gomodguard_v2`. It is a repo-wide config change, out of scope for this issue.

clawbot added 1 commit 2026-08-11 14:27:23 +02:00

Mask target config on the source detail page (closes #113 )

check / check (push) Successful in 4m7s

Details

8605797b67

The source detail page rendered each target's stored config
blob verbatim. For a slack target that blob contains the
incoming webhook URL, which is a bearer credential: anyone
holding it can post to the channel indefinitely, and it
cannot be scoped or revoked per holder. Rendering it put the
credential into browser history, screenshots and any support
screen share.

Targets are now projected to a display-safe TargetView that
has no raw config field at all, so no template can render the
blob. Each type contributes named fields instead: slack shows
only a masked webhook URL, http shows its destination,
timeout, header count and retry settings, and database shows
its archive expiry. Header values are not shown because they
routinely carry authorization tokens.

Masking is a method on the config type,
SlackTargetConfig.MaskedWebhookURL, so it is unit-testable
and cannot be bypassed from a template. It reduces the URL to
scheme and host, eliding the path, query and any userinfo:
the field accepts an arbitrary URL, so no path segment can be
assumed non-secret. Any config that is empty, of an unknown
type, or fails to parse renders a neutral placeholder — there
is no fallback to the stored string on any path.

The stored config format and the delivery path are unchanged.

clawbot added the needs-review label 2026-08-11 14:27:26 +02:00

clawbot self-assigned this 2026-08-11 14:27:27 +02:00

clawbot commented

2026-08-11 14:34:59 +02:00

PASS — masking, the no-raw-field projection, the fail-closed paths, tests, single commit on next, make fmt/lint/test all verified; cache-defeated docker build --no-cache-filter=lint,builder executed make fmt-check (1.7s), make lint (88.6s, 0 issues.) and make test (85.4s, 0 (cached) markers), exit 0.

Disclosures:

I temporarily mutated MaskedWebhookURL to return the raw URL in my own throwaway clone to prove the regression test is load-bearing: source_detail_test.go:120-123,129 failed, as did 3 delivery tests. Reverted; my tree is clean at 8605797. Nothing was changed or pushed here.
CI on 8605797 is still pending / "Waiting to run" (runner backlog), so I could not confirm a green check — it must be green before merge. Note the repo's recent greens report "Successful in 3-4s", i.e. an all-CACHED build, so that signal is weak regardless; the cache-defeated run above is the stronger evidence.
Fuzzed maskURL with 32 adversarial inputs (no scheme, scheme-relative, opaque javascript:/data:/mailto:/unix:, empty, whitespace, NUL and newline, userinfo-only, secret in query, secret in fragment, https:///, IPv6 host, explicit port, uppercase scheme, 5000-char path): 0 leaks, 0 panics, fails closed to (unavailable) on every malformed input. Userinfo and fragment are both dropped.
The //nolint:gosec // G101 is scoped to the single "userinfo dropped" table entry. linters.default: all enables nolintlint, whose default rejects unused directives, so the green lint above proves it is necessary.
Deviation accepted: eliding every path segment (https://hooks.slack.com/...) rather than the issue's illustrative .../services/.... The normative criterion is "never the full path segments"; this is strictly more conservative.

Out of scope for this PR, pre-existing, worth separate issues (not blocking): internal/delivery/target_slack.go:128,142 and internal/delivery/target_http.go:337 put the *url.Error — which embeds the full webhook URL — into DeliveryResult.Error, persisted to the per-webhook DB (currently rendered nowhere, but tagged json:"error,omitempty"); internal/handlers/source_management.go:1019 logs the full submitted URL on SSRF rejection; internal/handlers/source_management.go:757 still places raw database.Target values (config included) into the source_logs.html data map, unrendered today but one {{.Target.Config}} from a repeat of this bug.

**PASS** — masking, the no-raw-field projection, the fail-closed paths, tests, single commit on `next`, `make fmt`/lint/test all verified; cache-defeated `docker build --no-cache-filter=lint,builder` executed `make fmt-check` (1.7s), `make lint` (88.6s, `0 issues.`) and `make test` (85.4s, 0 `(cached)` markers), exit 0. Disclosures: - I temporarily mutated `MaskedWebhookURL` to return the raw URL in my own throwaway clone to prove the regression test is load-bearing: `source_detail_test.go:120-123,129` failed, as did 3 delivery tests. Reverted; my tree is clean at `8605797`. Nothing was changed or pushed here. - CI on `8605797` is still `pending` / "Waiting to run" (runner backlog), so I could not confirm a green check — it must be green before merge. Note the repo's recent greens report "Successful in 3-4s", i.e. an all-`CACHED` build, so that signal is weak regardless; the cache-defeated run above is the stronger evidence. - Fuzzed `maskURL` with 32 adversarial inputs (no scheme, scheme-relative, opaque `javascript:`/`data:`/`mailto:`/`unix:`, empty, whitespace, NUL and newline, userinfo-only, secret in query, secret in fragment, `https:///`, IPv6 host, explicit port, uppercase scheme, 5000-char path): 0 leaks, 0 panics, fails closed to `(unavailable)` on every malformed input. Userinfo and fragment are both dropped. - The `//nolint:gosec // G101` is scoped to the single `"userinfo dropped"` table entry. `linters.default: all` enables `nolintlint`, whose default rejects unused directives, so the green lint above proves it is necessary. - Deviation accepted: eliding every path segment (`https://hooks.slack.com/...`) rather than the issue's illustrative `.../services/...`. The normative criterion is "never the full path segments"; this is strictly more conservative. Out of scope for this PR, pre-existing, worth separate issues (not blocking): `internal/delivery/target_slack.go:128,142` and `internal/delivery/target_http.go:337` put the `*url.Error` — which embeds the full webhook URL — into `DeliveryResult.Error`, persisted to the per-webhook DB (currently rendered nowhere, but tagged `json:"error,omitempty"`); `internal/handlers/source_management.go:1019` logs the full submitted URL on SSRF rejection; `internal/handlers/source_management.go:757` still places raw `database.Target` values (config included) into the `source_logs.html` data map, unrendered today but one `{{.Target.Config}}` from a repeat of this bug.

clawbot merged commit 15a61173fc into next

2026-08-11 14:37:10 +02:00

clawbot deleted branch issue-113-mask-target-config

2026-08-11 14:37:11 +02:00

clawbot referenced this pull request

2026-08-11 14:37:28 +02:00

Slack webhook credential still leaks via delivery errors, SSRF logs, and the source_logs template data #118

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/webhooker#114