Found by the independent review of #126, which was masking the same credential on the render path. Not introduced by that PR.
internal/handlers/source_management.go:1002 reads the destination with r.FormValue("url"). r.FormValue falls back to the URL query string when the field is absent from the POST body. internal/middleware/middleware.go:123 logs "url", r.URL.String() unmasked, and the same URL reaches the Sentry request context.
So:
POST /source/{id}/targets?url=https://hooks.slack.com/services/T/B/SECRET
creates the target AND writes the full credential into the access log and to Sentry — off-host. This is the same credential class that #113, #118 and #115 were each filed to contain; those closed the render, delivery-error and log-line paths, and this is the remaining ingress path.
Milestoned 1.0.0 on that basis: the release has treated "the webhook URL must not come to rest anywhere an operator did not intend" as a blocking property three times already, and a partial fix of a credential-exposure class is the kind of gap that reads as closed when it is not. It requires an authenticated request to trigger, but the consequence is a live third-party credential written to log storage and shipped to an external error tracker.
Definition of done
The handler reads POST body fields only — r.PostFormValue — so a query-string value cannot populate a target's configuration. Audit source_management.go for every other r.FormValue call and convert each one whose value is secret-bearing or security-relevant; state in the PR which you converted and which you deliberately left.
A handler test that POSTs ?url=<secret> with an empty body asserts the access-log line contains no path segment of that URL, and asserts the target was NOT created from the query value.
Confirm whether the Sentry integration attaches the raw URL independently of the access log; if it does, that path is in scope too.
Second, cheaper item found in the same review
database.Target carries json:"config", and the /api/v1 route group exists but is empty. TargetView is the masking barrier for the HTML path only — the first handler that marshals the model directly will serialise the raw config blob, credential included. Add json:"-" to that field now, while it costs one line and no behaviour, rather than after an API handler exists. If any code genuinely needs to marshal it, say so in the PR instead.
Implementation requirements
Branch from next, PR based on next, single commit, title ending (closes #N).
Gate on make check plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.
Found by the independent review of https://git.eeqj.de/sneak/webhooker/pulls/126, which was masking the same credential on the render path. Not introduced by that PR.
`internal/handlers/source_management.go:1002` reads the destination with `r.FormValue("url")`. `r.FormValue` falls back to the URL query string when the field is absent from the POST body. `internal/middleware/middleware.go:123` logs `"url", r.URL.String()` unmasked, and the same URL reaches the Sentry request context.
So:
```
POST /source/{id}/targets?url=https://hooks.slack.com/services/T/B/SECRET
```
creates the target AND writes the full credential into the access log and to Sentry — off-host. This is the same credential class that https://git.eeqj.de/sneak/webhooker/issues/113, https://git.eeqj.de/sneak/webhooker/issues/118 and https://git.eeqj.de/sneak/webhooker/issues/115 were each filed to contain; those closed the render, delivery-error and log-line paths, and this is the remaining ingress path.
Milestoned 1.0.0 on that basis: the release has treated "the webhook URL must not come to rest anywhere an operator did not intend" as a blocking property three times already, and a partial fix of a credential-exposure class is the kind of gap that reads as closed when it is not. It requires an authenticated request to trigger, but the consequence is a live third-party credential written to log storage and shipped to an external error tracker.
## Definition of done
- The handler reads POST body fields only — `r.PostFormValue` — so a query-string value cannot populate a target's configuration. Audit `source_management.go` for every other `r.FormValue` call and convert each one whose value is secret-bearing or security-relevant; state in the PR which you converted and which you deliberately left.
- A handler test that POSTs `?url=<secret>` with an empty body asserts the access-log line contains no path segment of that URL, and asserts the target was NOT created from the query value.
- Confirm whether the Sentry integration attaches the raw URL independently of the access log; if it does, that path is in scope too.
## Second, cheaper item found in the same review
`database.Target` carries `json:"config"`, and the `/api/v1` route group exists but is empty. `TargetView` is the masking barrier for the HTML path only — the first handler that marshals the model directly will serialise the raw config blob, credential included. Add `json:"-"` to that field now, while it costs one line and no behaviour, rather than after an API handler exists. If any code genuinely needs to marshal it, say so in the PR instead.
## 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).
- 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-17 22:50:53 +02:00
clawbot
self-assigned this 2026-08-17 22:50:53 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Found by the independent review of #126, which was masking the same credential on the render path. Not introduced by that PR.
internal/handlers/source_management.go:1002reads the destination withr.FormValue("url").r.FormValuefalls back to the URL query string when the field is absent from the POST body.internal/middleware/middleware.go:123logs"url", r.URL.String()unmasked, and the same URL reaches the Sentry request context.So:
creates the target AND writes the full credential into the access log and to Sentry — off-host. This is the same credential class that #113, #118 and #115 were each filed to contain; those closed the render, delivery-error and log-line paths, and this is the remaining ingress path.
Milestoned 1.0.0 on that basis: the release has treated "the webhook URL must not come to rest anywhere an operator did not intend" as a blocking property three times already, and a partial fix of a credential-exposure class is the kind of gap that reads as closed when it is not. It requires an authenticated request to trigger, but the consequence is a live third-party credential written to log storage and shipped to an external error tracker.
Definition of done
r.PostFormValue— so a query-string value cannot populate a target's configuration. Auditsource_management.gofor every otherr.FormValuecall and convert each one whose value is secret-bearing or security-relevant; state in the PR which you converted and which you deliberately left.?url=<secret>with an empty body asserts the access-log line contains no path segment of that URL, and asserts the target was NOT created from the query value.Second, cheaper item found in the same review
database.Targetcarriesjson:"config", and the/api/v1route group exists but is empty.TargetViewis the masking barrier for the HTML path only — the first handler that marshals the model directly will serialise the raw config blob, credential included. Addjson:"-"to that field now, while it costs one line and no behaviour, rather than after an API handler exists. If any code genuinely needs to marshal it, say so in the PR instead.Implementation requirements
next, PR based onnext, single commit, title ending(closes #N).TODO.md(see #112).make checkplus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.clawbot referenced this issue2026-08-17 23:50:11 +02:00