Validate Slack target URLs at creation time (closes #68) (#73)
All checks were successful
check / check (push) Successful in 4s
All checks were successful
check / check (push) Successful in 4s
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 Co-authored-by: sneak <sneak@sneak.berlin> Co-authored-by: Jeffrey Paul <sneak@noreply.example.org> Reviewed-on: #73 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
This commit was merged in pull request #73.
This commit is contained in:
@@ -12,3 +12,13 @@ func (s *Handlers) RenderTemplateForTest(
|
|||||||
) {
|
) {
|
||||||
s.renderTemplate(w, r, pageTemplate, data)
|
s.renderTemplate(w, r, pageTemplate, data)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// BuildSlackTargetConfigForTest exposes buildSlackTargetConfig
|
||||||
|
// for use in the handlers_test package.
|
||||||
|
func (s *Handlers) BuildSlackTargetConfigForTest(
|
||||||
|
w http.ResponseWriter,
|
||||||
|
r *http.Request,
|
||||||
|
targetURL string,
|
||||||
|
) (string, error) {
|
||||||
|
return s.buildSlackTargetConfig(w, r, targetURL)
|
||||||
|
}
|
||||||
|
|||||||
@@ -116,6 +116,52 @@ func TestHandleIndex_Authenticated(t *testing.T) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestBuildSlackTargetConfig_AcceptsPublicURL(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var h *handlers.Handlers
|
||||||
|
|
||||||
|
app := newTestApp(t, &h)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
context.Background(), http.MethodPost, "/", nil)
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
|
||||||
|
cfg, err := h.BuildSlackTargetConfigForTest(
|
||||||
|
w, req, "http://93.184.216.34/services/T00/B00/xxx",
|
||||||
|
)
|
||||||
|
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, http.StatusOK, w.Code)
|
||||||
|
assert.Contains(t, cfg, "webhookUrl")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestBuildSlackTargetConfig_RejectsReservedURL(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var h *handlers.Handlers
|
||||||
|
|
||||||
|
app := newTestApp(t, &h)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
context.Background(), http.MethodPost, "/", nil)
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
|
||||||
|
cfg, err := h.BuildSlackTargetConfigForTest(
|
||||||
|
w, req, "http://169.254.169.254/latest/meta-data/",
|
||||||
|
)
|
||||||
|
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.Empty(t, cfg)
|
||||||
|
assert.Equal(t, http.StatusBadRequest, w.Code)
|
||||||
|
}
|
||||||
|
|
||||||
func TestRenderTemplate(t *testing.T) {
|
func TestRenderTemplate(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -902,7 +902,7 @@ func (h *Handlers) buildTargetConfig(
|
|||||||
case database.TargetTypeHTTP:
|
case database.TargetTypeHTTP:
|
||||||
return h.buildHTTPTargetConfig(w, r, targetURL)
|
return h.buildHTTPTargetConfig(w, r, targetURL)
|
||||||
case database.TargetTypeSlack:
|
case database.TargetTypeSlack:
|
||||||
return h.buildSlackTargetConfig(w, targetURL)
|
return h.buildSlackTargetConfig(w, r, targetURL)
|
||||||
case database.TargetTypeDatabase, database.TargetTypeLog:
|
case database.TargetTypeDatabase, database.TargetTypeLog:
|
||||||
return "", nil
|
return "", nil
|
||||||
default:
|
default:
|
||||||
@@ -967,6 +967,7 @@ func (h *Handlers) buildHTTPTargetConfig(
|
|||||||
// buildSlackTargetConfig builds config JSON for a Slack target.
|
// buildSlackTargetConfig builds config JSON for a Slack target.
|
||||||
func (h *Handlers) buildSlackTargetConfig(
|
func (h *Handlers) buildSlackTargetConfig(
|
||||||
w http.ResponseWriter,
|
w http.ResponseWriter,
|
||||||
|
r *http.Request,
|
||||||
targetURL string,
|
targetURL string,
|
||||||
) (string, error) {
|
) (string, error) {
|
||||||
if targetURL == "" {
|
if targetURL == "" {
|
||||||
@@ -979,6 +980,24 @@ func (h *Handlers) buildSlackTargetConfig(
|
|||||||
return "", errMissingURL
|
return "", errMissingURL
|
||||||
}
|
}
|
||||||
|
|
||||||
|
err := delivery.ValidateTargetURL(
|
||||||
|
r.Context(), targetURL,
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
h.log.Warn(
|
||||||
|
"target URL blocked by SSRF protection",
|
||||||
|
"url", targetURL,
|
||||||
|
"error", err,
|
||||||
|
)
|
||||||
|
http.Error(
|
||||||
|
w,
|
||||||
|
"Invalid target URL: "+err.Error(),
|
||||||
|
http.StatusBadRequest,
|
||||||
|
)
|
||||||
|
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
|
||||||
cfg := map[string]any{"webhookUrl": targetURL}
|
cfg := map[string]any{"webhookUrl": targetURL}
|
||||||
|
|
||||||
configBytes, err := json.Marshal(cfg)
|
configBytes, err := json.Marshal(cfg)
|
||||||
|
|||||||
Reference in New Issue
Block a user