The receiver's capability URL is sent to Sentry in full #179

New Issue

clawbot · 2026-08-18T01:41:07+02:00

clawbot commented

2026-08-18 01:41:07 +02:00

Raised by the independent review of #174, which recommended filing it. That PR scrubs the Sentry request body, query string and headers, but deliberately keeps Request.URL, which the SDK builds as scheme://host/path. On the receiver route that path is /webhook/{uuid} in full.

That UUID is not an identifier, it is a write capability: anyone holding it can POST events that this service accepts and that the operator's configured targets then deliver. It is the inbound counterpart of the outbound target credential that #113, #118, #115 and #160 were each filed to keep out of somewhere it did not belong.

Why the existing rule does not cover this

#174 justified keeping the path by pointing at #146, which deliberately keeps the concrete path on 2xx and 5xx responses. The reviewer's rebuttal is convincing and I accept it: that ruling was reasoned about a log you own, where the concrete path grants a reader no capability they did not already have. Sentry is a different trust boundary with its own retention, its own access control, and its own deletion policy. The two are not equivalent, and the earlier ruling should not be read as covering this.

Milestoned 1.0.0 for consistency with the four issues above, all of which treated a credential coming to rest somewhere unintended as blocking. Only reachable when SENTRY_DSN is configured, which is optional — but an operator who configures error reporting has not thereby consented to shipping their receiver capabilities off-host.

The fix is now cheap, and #174 records why

That PR's first revision claimed the SDK gives BeforeSend no request, so the route could not be identified. That is false, and the reviewer disproved it with a scratch test rather than by reading:

http/sentryhttp.go:123-126 calls hub.RecoverWithContext(context.WithValue(r.Context(), sentry.RequestContextKey, r), err)
hub.go:344 builds &EventHint{RecoveredException: err} with no request — the part that misled the author — but
client.go:480-487 then sets hint.Context = ctx, and client.go:629-631 passes that hint to BeforeSend

Their probe, with sentryhttp inside a chi router and a panicking POST /webhook/{uuid}, reported sawHint=true sawRequest=true routePattern="/webhook/{uuid}".

So the chi route pattern is reachable from hint.Context, and Request.URL can carry the pattern rather than the concrete path.

Definition of done

Request.URL sent to Sentry carries the chi route pattern rather than a client- or capability-bearing concrete path, at least for the receiver route.
Decide deliberately whether to apply the pattern on ALL routes or only where the path is sensitive, and say which. Bear in mind #174's own reasoning for an unconditional rule: a route-conditional one leaks on any route someone forgets to add. The same logic likely applies here.
The route must still be identifiable in Sentry — an error you cannot locate is not an improvement.
Handle the case where the hint or the route pattern is absent, and make the fallback safe rather than the concrete path.
A test through the real sentryhttp path — SetRequest to ApplyToEvent to BeforeSend, as #174's tests now do — asserting a receiver UUID does not survive into the marshalled event. Verify by mutation.

Implementation requirements

Branch from next, PR based on next, single commit, title ending (closes #N).
Do not modify TODO.md (see #112).
Run make bootstrap in a fresh clone before gating — browser assets are fetched at build time, and make lint now needs Docker.
Gate on make check plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.

Raised by the independent review of https://git.eeqj.de/sneak/webhooker/pulls/174, which recommended filing it. That PR scrubs the Sentry request body, query string and headers, but deliberately keeps `Request.URL`, which the SDK builds as `scheme://host/path`. On the receiver route that path is `/webhook/{uuid}` in full. That UUID is not an identifier, it is a **write capability**: anyone holding it can POST events that this service accepts and that the operator's configured targets then deliver. It is the inbound counterpart of the outbound target credential that https://git.eeqj.de/sneak/webhooker/issues/113, https://git.eeqj.de/sneak/webhooker/issues/118, https://git.eeqj.de/sneak/webhooker/issues/115 and https://git.eeqj.de/sneak/webhooker/issues/160 were each filed to keep out of somewhere it did not belong. ## Why the existing rule does not cover this https://git.eeqj.de/sneak/webhooker/pulls/174 justified keeping the path by pointing at https://git.eeqj.de/sneak/webhooker/issues/146, which deliberately keeps the concrete path on 2xx and 5xx responses. The reviewer's rebuttal is convincing and I accept it: that ruling was reasoned about **a log you own**, where the concrete path grants a reader no capability they did not already have. Sentry is a different trust boundary with its own retention, its own access control, and its own deletion policy. The two are not equivalent, and the earlier ruling should not be read as covering this. Milestoned `1.0.0` for consistency with the four issues above, all of which treated a credential coming to rest somewhere unintended as blocking. Only reachable when `SENTRY_DSN` is configured, which is optional — but an operator who configures error reporting has not thereby consented to shipping their receiver capabilities off-host. ## The fix is now cheap, and https://git.eeqj.de/sneak/webhooker/pulls/174 records why That PR's first revision claimed the SDK gives `BeforeSend` no request, so the route could not be identified. **That is false**, and the reviewer disproved it with a scratch test rather than by reading: - `http/sentryhttp.go:123-126` calls `hub.RecoverWithContext(context.WithValue(r.Context(), sentry.RequestContextKey, r), err)` - `hub.go:344` builds `&EventHint{RecoveredException: err}` with no request — the part that misled the author — but - `client.go:480-487` then sets `hint.Context = ctx`, and `client.go:629-631` passes that hint to `BeforeSend` Their probe, with `sentryhttp` inside a chi router and a panicking `POST /webhook/{uuid}`, reported `sawHint=true sawRequest=true routePattern="/webhook/{uuid}"`. So the chi route pattern is reachable from `hint.Context`, and `Request.URL` can carry the pattern rather than the concrete path. ## Definition of done - `Request.URL` sent to Sentry carries the chi route pattern rather than a client- or capability-bearing concrete path, at least for the receiver route. - Decide deliberately whether to apply the pattern on ALL routes or only where the path is sensitive, and say which. Bear in mind https://git.eeqj.de/sneak/webhooker/pulls/174's own reasoning for an unconditional rule: a route-conditional one leaks on any route someone forgets to add. The same logic likely applies here. - The route must still be identifiable in Sentry — an error you cannot locate is not an improvement. - Handle the case where the hint or the route pattern is absent, and make the fallback safe rather than the concrete path. - A test through the real `sentryhttp` path — `SetRequest` to `ApplyToEvent` to `BeforeSend`, as https://git.eeqj.de/sneak/webhooker/pulls/174's tests now do — asserting a receiver UUID does not survive into the marshalled event. Verify by mutation. ## Implementation requirements - Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`. - Do not modify `TODO.md` (see https://git.eeqj.de/sneak/webhooker/issues/112). - Run `make bootstrap` in a fresh clone before gating — browser assets are fetched at build time, and `make lint` now needs Docker. - Gate on `make check` plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.

clawbot added this to the 1.0.0 milestone 2026-08-18 01:41:07 +02:00

clawbot self-assigned this 2026-08-18 01:41:07 +02:00

clawbot referenced this issue from a commit

2026-08-18 02:04:10 +02:00

Read form fields from the POST body only (closes #160)

clawbot referenced this issue from a commit

2026-08-18 02:20:26 +02:00

Send the chi route pattern to Sentry, not the concrete path (closes #179)

clawbot referenced a pull request that will close this issue

2026-08-18 02:21:15 +02:00

Send the chi route pattern to Sentry, not the concrete path (closes #179) #181

clawbot referenced this issue