Slack delivery targets were only checked by the request-time dialer guard, not at creation, giving them a weaker SSRF gate than HTTP targets.
This validates the Slack incoming-webhook URL with delivery.ValidateTargetURL in the Slack target creation path (buildSlackTargetConfig), before persisting, mirroring the existing HTTP-target path. On failure the create is rejected with the same clear, non-leaking user-facing error the HTTP path uses.
Adds handlers-package tests covering both an accepted public URL and a rejected private/reserved URL. Confined to internal/handlers/; internal/delivery/ is unchanged.
Slack delivery targets were only checked by the request-time dialer guard, not at creation, giving them a weaker SSRF gate than HTTP targets.
This validates the Slack incoming-webhook URL with `delivery.ValidateTargetURL` in the Slack target creation path (`buildSlackTargetConfig`), before persisting, mirroring the existing HTTP-target path. On failure the create is rejected with the same clear, non-leaking user-facing error the HTTP path uses.
Adds handlers-package tests covering both an accepted public URL and a rejected private/reserved URL. Confined to `internal/handlers/`; `internal/delivery/` is unchanged.
Closes #68
Confined to internal/handlers/ (validator in internal/delivery/ untouched):
internal/handlers/source_management.go: buildSlackTargetConfig now takes the request and calls delivery.ValidateTargetURL(r.Context(), targetURL) before persisting, mirroring buildHTTPTargetConfig. On failure it logs a warning and returns Invalid target URL: ... with HTTP 400 (same pattern as the HTTP path; no internals leaked). Caller in buildTargetConfig updated to pass r.
internal/handlers/export_test.go: added BuildSlackTargetConfigForTest to exercise the unexported builder from the handlers_test package.
Adversarial review against the issue spec and repo policies.
internal/handlers/source_management.go: buildSlackTargetConfig now calls delivery.ValidateTargetURL(r.Context(), targetURL) before persisting — the same create-time validation the HTTP target path uses — and on failure returns 400 without leaking internals; buildTargetConfig was updated to thread the request through. Change is confined to internal/handlers/; internal/delivery/ is untouched (it calls the existing exported validator, as instructed).
Tests: accept path uses a public IP literal (expects 200 and a config containing webhookUrl); reject path uses the metadata address 169.254.169.254 (expects an error, 400, and empty config). Both use IP literals, so they do not depend on DNS and are deterministic. The green docker build . runs these, which proves the validator blocks the reserved address and accepts the public one.
make fmt clean; no AI/tooling references; commit subject closes the issue.
Verdict: meets the bar. Marking merge-ready and handing to @sneak for final review.
## Independent review — PASS (merge-ready)
Adversarial review against the issue spec and repo policies.
- `internal/handlers/source_management.go`: `buildSlackTargetConfig` now calls `delivery.ValidateTargetURL(r.Context(), targetURL)` before persisting — the same create-time validation the HTTP target path uses — and on failure returns 400 without leaking internals; `buildTargetConfig` was updated to thread the request through. Change is confined to `internal/handlers/`; `internal/delivery/` is untouched (it calls the existing exported validator, as instructed).
- Tests: accept path uses a public IP literal (expects 200 and a config containing `webhookUrl`); reject path uses the metadata address `169.254.169.254` (expects an error, 400, and empty config). Both use IP literals, so they do not depend on DNS and are deterministic. The green `docker build .` runs these, which proves the validator blocks the reserved address and accepts the public one.
- `make fmt` clean; no AI/tooling references; commit subject closes the issue.
Verdict: meets the bar. Marking merge-ready and handing to @sneak for final review.
sneak
was assigned by clawbot2026-08-07 14:00:46 +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.
Slack delivery targets were only checked by the request-time dialer guard, not at creation, giving them a weaker SSRF gate than HTTP targets.
This validates the Slack incoming-webhook URL with
delivery.ValidateTargetURLin the Slack target creation path (buildSlackTargetConfig), before persisting, mirroring the existing HTTP-target path. On failure the create is rejected with the same clear, non-leaking user-facing error the HTTP path uses.Adds handlers-package tests covering both an accepted public URL and a rejected private/reserved URL. Confined to
internal/handlers/;internal/delivery/is unchanged.Closes #68
What changed
Confined to
internal/handlers/(validator ininternal/delivery/untouched):internal/handlers/source_management.go:buildSlackTargetConfignow takes the request and callsdelivery.ValidateTargetURL(r.Context(), targetURL)before persisting, mirroringbuildHTTPTargetConfig. On failure it logs a warning and returnsInvalid target URL: ...with HTTP 400 (same pattern as the HTTP path; no internals leaked). Caller inbuildTargetConfigupdated to passr.internal/handlers/export_test.go: addedBuildSlackTargetConfigForTestto exercise the unexported builder from thehandlers_testpackage.internal/handlers/handlers_test.go: addedTestBuildSlackTargetConfig_AcceptsPublicURL(public IP literal, expects 200 + config containingwebhookUrl) andTestBuildSlackTargetConfig_RejectsReservedURL(http://169.254.169.254/latest/meta-data/, expects error + HTTP 400 + empty config).Diffstat: 3 files changed, 76 insertions(+), 1 deletion(-).
Validation
docker build .(fmt-check, lint, test, build) exited 0:Independent review — PASS (merge-ready)
Adversarial review against the issue spec and repo policies.
internal/handlers/source_management.go:buildSlackTargetConfignow callsdelivery.ValidateTargetURL(r.Context(), targetURL)before persisting — the same create-time validation the HTTP target path uses — and on failure returns 400 without leaking internals;buildTargetConfigwas updated to thread the request through. Change is confined tointernal/handlers/;internal/delivery/is untouched (it calls the existing exported validator, as instructed).webhookUrl); reject path uses the metadata address169.254.169.254(expects an error, 400, and empty config). Both use IP literals, so they do not depend on DNS and are deterministic. The greendocker build .runs these, which proves the validator blocks the reserved address and accepts the public one.make fmtclean; no AI/tooling references; commit subject closes the issue.Verdict: meets the bar. Marking merge-ready and handing to @sneak for final review.