Compare commits

Author SHA1 Message Date
sneak ed81db137e Implement the log delivery target (closes #70)
check / check (push) Successful in 4s
2026-08-07 19:57:16 +07:00
clawbotandsneak 752d6beead Validate Slack target URLs at creation time (closes #68) (#73)
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>
2026-08-07 14:03:56 +02:00
clawbotandsneak b1f43c9520 Keep the SSRF-safe transport in clientForConfig (closes #69) (#74)
check / check (push) Superseded by a newer commit; never tested
`clientForConfig()` in `internal/delivery/engine.go` built a fresh `http.Client` without a Transport when a per-target timeout was configured, dropping the request-time private-IP guard for that path.

It now reuses the shared client's SSRF-safe transport (`e.client.Transport`, the same `NewSSRFSafeTransport` instance), overriding only the `Timeout`. Behaviour is unchanged when no per-target timeout is set (the shared client is returned as before), so no engine code path makes an outbound target request with a client lacking the SSRF-safe transport.

Adds a delivery-package test proving a client from `clientForConfig()` with a per-target timeout still refuses private/reserved/link-local destinations, that the timeout is applied, that the SSRF-safe transport is reused (not duplicated), and that the no-timeout path returns the shared client unchanged.

Confined to `internal/delivery/` only; handlers and server code untouched.

Closes #69

Co-authored-by: sneak <sneak@sneak.berlin>
Co-authored-by: Jeffrey Paul <sneak@noreply.example.org>
Reviewed-on: #74
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-08-07 14:03:38 +02:00
5 changed files with 165 additions and 1 deletions
+3
View File
@@ -1006,8 +1006,11 @@ func (e *Engine) deliverLog(
"webhook event delivered to log target", "webhook event delivered to log target",
"delivery_id", d.ID, "delivery_id", d.ID,
"event_id", d.EventID, "event_id", d.EventID,
"webhook_id", d.Event.WebhookID,
"entrypoint_id", d.Event.EntrypointID,
"target_id", d.TargetID, "target_id", d.TargetID,
"target_name", d.Target.Name, "target_name", d.Target.Name,
"outcome", database.DeliveryStatusDelivered,
"method", d.Event.Method, "method", d.Event.Method,
"content_type", d.Event.ContentType, "content_type", d.Event.ContentType,
"body_length", len(d.Event.Body), "body_length", len(d.Event.Body),
+86
View File
@@ -1,6 +1,7 @@
package delivery_test package delivery_test
import ( import (
"bytes"
"context" "context"
"database/sql" "database/sql"
"encoding/json" "encoding/json"
@@ -435,6 +436,91 @@ func TestDeliverLog_ImmediateSuccess(t *testing.T) {
assert.True(t, result.Success) assert.True(t, result.Success)
} }
func TestDeliverLog_StructuredLogFields(t *testing.T) {
t.Parallel()
db := testWebhookDB(t)
var logBuf bytes.Buffer
e := delivery.NewTestEngine(
slog.New(slog.NewTextHandler(
&logBuf,
&slog.HandlerOptions{Level: slog.LevelDebug},
)),
&http.Client{Timeout: 5 * time.Second},
1,
)
event := seedEvent(t, db, `{"log":"structured"}`)
dlv := seedDelivery(
t, db, event.ID, uuid.New().String(),
database.DeliveryStatusPending,
)
d := &database.Delivery{
EventID: event.ID,
TargetID: dlv.TargetID,
Status: database.DeliveryStatusPending,
Event: event,
Target: database.Target{
Name: "structured-log",
Type: database.TargetTypeLog,
},
}
d.ID = dlv.ID
e.ExportDeliverLog(db, d)
// The delivery is marked delivered and a success
// DeliveryResult with no HTTP status is recorded,
// mirroring the other target types' bookkeeping.
var updated database.Delivery
require.NoError(t, db.First(
&updated, "id = ?", dlv.ID,
).Error)
assert.Equal(t,
database.DeliveryStatusDelivered, updated.Status,
"log target should immediately succeed",
)
var result database.DeliveryResult
require.NoError(t, db.Where(
"delivery_id = ?", dlv.ID,
).First(&result).Error)
assert.True(t, result.Success)
assert.Equal(t, 0, result.StatusCode,
"log target should not have an HTTP status",
)
assertLogFields(t, logBuf.String(), event, "structured-log")
}
// assertLogFields checks that a log target's structured
// log line carries the required fields: event id,
// webhook/entrypoint, target name, and outcome.
func assertLogFields(
t *testing.T,
logged string,
event database.Event,
targetName string,
) {
t.Helper()
assert.Contains(t, logged, "event_id="+event.ID)
assert.Contains(t, logged, "webhook_id="+event.WebhookID)
assert.Contains(t,
logged, "entrypoint_id="+event.EntrypointID,
)
assert.Contains(t, logged, "target_name="+targetName)
assert.Contains(t, logged, "outcome=delivered")
}
func TestDeliverHTTP_WithRetries_Success(t *testing.T) { func TestDeliverHTTP_WithRetries_Success(t *testing.T) {
t.Parallel() t.Parallel()
+10
View File
@@ -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)
}
+46
View File
@@ -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()
+20 -1
View File
@@ -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)