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

clawbot · 2026-08-12T11:41:40+02:00

clawbot commented

2026-08-12 11:41:40 +02:00

Speculative, ahead of your decision on
#115. Merging it decides
option 1; closing it decides against. Title keeps the WIP: prefix so
it cannot land by accident.

Option chosen

Option 1 — mask unconditionally, reusing the existing MaskURL from
internal/delivery/target_config_view.go, which renders
scheme://host/.... It is the rule already applied to the slack target,
so both target types now behave identically, and it needs no new masker
and no heuristic.

Rejected:

Option 2 — mask only credential-shaped URLs (known webhook hosts,
or high-entropy path segments). A heuristic that can be wrong in both
directions: it leaks a self-hosted Mattermost or a bespoke endpoint it
does not recognise, and it hides a plain /hook path an operator
wanted to read.
Option 3 — render in full, on the grounds that the page is
authenticated. It keeps a written-down credential on screen, so it
survives into screenshots, screen shares and browser-rendered PDFs
regardless of who is logged in.

Correction to the issue's usability mitigation

The issue argues the cost is acceptable because "the full value stays
available in the edit form". That is not true in the code today.
internal/server/routes.go registers POST /source/{sourceID}/targets,
.../targets/{targetID}/toggle and .../targets/{targetID}/delete —
there is no target edit route, and templates/source_edit.html edits
only the webhook's name, description and retention. A target's
destination URL is write-once: after this change it is not recoverable
anywhere in the UI, only from the database.

So the real cost of option 1 is higher than the issue states. It still
looks like the right call — an unrecoverable credential is the normal
outcome for a secret, and the operator holds the value at the source
(Slack, Discord, Teams) — but the decision should be made knowing that,
and a target edit form is the thing that would repay the usability debt.
Not filed as an issue: the absent edit form is your existing design, not
a defect, so it is a question for you rather than a work order.

What changed

internal/delivery/target_config_view.go — httpConfigFields renders
MaskURL(cfg.URL) for the Destination URL field. No new masker; no
other field changed.
Tests updated for the new expected value, plus two new ones.

Sibling-path audit

Prompted by #118, which found
this credential leaking through three sibling paths. Audited every place
target data reaches an operator:

Path	Renders	Verdict
`GET /source/{id}` (`templates/source_detail.html`)	`TargetView.Config` fields	leaked; fixed here
`GET /source/{id}/logs` (`templates/source_logs.html`)	`.Target.Name` and `.Status` only	clean
`GET /sources` (`templates/sources_list.html`)	target count only	clean
`GET /source/{id}/edit`	webhook fields only, no target data	clean
delivery errors persisted to `DeliveryResult.Error` and logged	masked	already fixed by #118
`POST /source/{id}/targets` SSRF rejection (response body + log line)	`ValidateTargetURL` reports scheme/host/hostname only; the `url.Parse` branch is already run through `maskURLError`, and the log line through `MaskURL`	clean
`/metrics`	no URL-valued labels	clean

Only the one path. TargetView has no raw config field, so no template
can reach the stored blob.

Verification

Mutation-verified. With MaskURL(cfg.URL) reverted to cfg.URL and
nothing else changed, make test exits 2 and the new handler test fails
on the secret path segments at source_detail_test.go:164:

--- FAIL: TestHandleSourceDetail_MasksHTTPDestinationURL (1.52s)
--- FAIL: TestNewTargetViews_HTTPMasksDestinationURL (0.00s)

New tests:

TestHandleSourceDetail_MasksHTTPDestinationURL — the done-criterion.
Seeds an http target whose destination is a secret-bearing incoming
webhook URL, runs the real handler, asserts the rendered page contains
none of the path segments and does show https://hooks.slack.com/....
TestNewTargetViews_HTTPMasksDestinationURL — same rule at the view
layer.

Gates, on the rebased head:

make check exits 0 (tests, lint, fmt-check), re-run after the rebase
onto next at d19e336.
Containerized, cache defeated on both check-running stages:
docker build --no-cache-filter=lint --no-cache-filter=builder .
exits 0. Not a cached no-op — make lint executed in the lint stage
(0 issues. after 61s) and make test executed in the builder
stage, with every package reporting a real duration and no (cached):
ok internal/delivery 6.870s, ok internal/handlers 6.781s, and both
new tests logging --- PASS in that run. A scoped --no-cache-filter
rather than a prune, per the shared-host rule.

Speculative, ahead of your decision on https://git.eeqj.de/sneak/webhooker/issues/115. Merging it decides option 1; closing it decides against. Title keeps the `WIP: ` prefix so it cannot land by accident. ## Option chosen **Option 1 — mask unconditionally**, reusing the existing `MaskURL` from `internal/delivery/target_config_view.go`, which renders `scheme://host/...`. It is the rule already applied to the slack target, so both target types now behave identically, and it needs no new masker and no heuristic. Rejected: - **Option 2 — mask only credential-shaped URLs** (known webhook hosts, or high-entropy path segments). A heuristic that can be wrong in both directions: it leaks a self-hosted Mattermost or a bespoke endpoint it does not recognise, and it hides a plain `/hook` path an operator wanted to read. - **Option 3 — render in full**, on the grounds that the page is authenticated. It keeps a written-down credential on screen, so it survives into screenshots, screen shares and browser-rendered PDFs regardless of who is logged in. ## Correction to the issue's usability mitigation The issue argues the cost is acceptable because "the full value stays available in the edit form". **That is not true in the code today.** `internal/server/routes.go` registers `POST /source/{sourceID}/targets`, `.../targets/{targetID}/toggle` and `.../targets/{targetID}/delete` — there is no target edit route, and `templates/source_edit.html` edits only the webhook's name, description and retention. A target's destination URL is write-once: after this change it is not recoverable anywhere in the UI, only from the database. So the real cost of option 1 is higher than the issue states. It still looks like the right call — an unrecoverable credential is the normal outcome for a secret, and the operator holds the value at the source (Slack, Discord, Teams) — but the decision should be made knowing that, and a target edit form is the thing that would repay the usability debt. Not filed as an issue: the absent edit form is your existing design, not a defect, so it is a question for you rather than a work order. ## What changed - `internal/delivery/target_config_view.go` — `httpConfigFields` renders `MaskURL(cfg.URL)` for the `Destination URL` field. No new masker; no other field changed. - Tests updated for the new expected value, plus two new ones. ## Sibling-path audit Prompted by https://git.eeqj.de/sneak/webhooker/issues/118, which found this credential leaking through three sibling paths. Audited every place target data reaches an operator: | Path | Renders | Verdict | | --- | --- | --- | | `GET /source/{id}` (`templates/source_detail.html`) | `TargetView.Config` fields | leaked; fixed here | | `GET /source/{id}/logs` (`templates/source_logs.html`) | `.Target.Name` and `.Status` only | clean | | `GET /sources` (`templates/sources_list.html`) | target count only | clean | | `GET /source/{id}/edit` | webhook fields only, no target data | clean | | delivery errors persisted to `DeliveryResult.Error` and logged | masked | already fixed by https://git.eeqj.de/sneak/webhooker/issues/118 | | `POST /source/{id}/targets` SSRF rejection (response body + log line) | `ValidateTargetURL` reports scheme/host/hostname only; the `url.Parse` branch is already run through `maskURLError`, and the log line through `MaskURL` | clean | | `/metrics` | no URL-valued labels | clean | Only the one path. `TargetView` has no raw config field, so no template can reach the stored blob. ## Verification Mutation-verified. With `MaskURL(cfg.URL)` reverted to `cfg.URL` and nothing else changed, `make test` exits 2 and the new handler test fails on the secret path segments at `source_detail_test.go:164`: ``` --- FAIL: TestHandleSourceDetail_MasksHTTPDestinationURL (1.52s) --- FAIL: TestNewTargetViews_HTTPMasksDestinationURL (0.00s) ``` New tests: - `TestHandleSourceDetail_MasksHTTPDestinationURL` — the done-criterion. Seeds an `http` target whose destination is a secret-bearing incoming webhook URL, runs the real handler, asserts the rendered page contains none of the path segments and does show `https://hooks.slack.com/...`. - `TestNewTargetViews_HTTPMasksDestinationURL` — same rule at the view layer. Gates, on the rebased head: - `make check` exits 0 (tests, lint, fmt-check), re-run after the rebase onto `next` at `d19e336`. - Containerized, cache defeated on both check-running stages: `docker build --no-cache-filter=lint --no-cache-filter=builder .` exits 0. Not a cached no-op — `make lint` executed in the `lint` stage (`0 issues.` after 61s) and `make test` executed in the `builder` stage, with every package reporting a real duration and no `(cached)`: `ok internal/delivery 6.870s`, `ok internal/handlers 6.781s`, and both new tests logging `--- PASS` in that run. A scoped `--no-cache-filter` rather than a prune, per the shared-host rule.

clawbot added the needs-review label 2026-08-12 11:41:40 +02:00

clawbot added 1 commit 2026-08-12 11:41:40 +02:00

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

check / check (push) Successful in 4m17s

Details

5c0ea2b44f

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.

clawbot self-assigned this 2026-08-12 11:41:44 +02:00

clawbot referenced this pull request

2026-08-12 11:42:26 +02:00

No target edit form: a target's destination URL, headers and timeout are write-once #127

clawbot referenced this pull request

2026-08-12 11:42:34 +02:00

HTTP target destination URL is rendered in full and can itself be a bearer credential #115