Validate Slack target URLs at creation time (closes #68) #73

Merged
sneak merged 3 commits from issue-68-slack-url-validation into main 2026-08-07 14:03:56 +02:00
3 changed files with 129 additions and 0 deletions
Showing only changes of commit d3bf348871 - Show all commits

View File

@@ -0,0 +1,112 @@
package delivery_test
import (
"context"
"log/slog"
"net/http"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/delivery"
)
// newSSRFTestEngine builds an Engine whose shared client
// carries the SSRF-safe transport, mirroring production.
func newSSRFTestEngine() *delivery.Engine {
log := slog.New(slog.DiscardHandler)
client := &http.Client{
Timeout: 30 * time.Second,
Transport: delivery.NewSSRFSafeTransport(),
}
return delivery.NewTestEngine(log, client, 1)
}
// TestClientForConfig_TimeoutKeepsSSRFGuard asserts that a
// client returned by clientForConfig for a config with a
// per-target timeout still refuses connections to
// private/reserved addresses (the timeout must not drop the
// SSRF-safe transport).
func TestClientForConfig_TimeoutKeepsSSRFGuard(t *testing.T) {
t.Parallel()
engine := newSSRFTestEngine()
blocked := []string{
"http://127.0.0.1/hook",
"http://169.254.169.254/latest/meta-data/",
"http://[fe80::1]/hook",
}
for _, target := range blocked {
t.Run(target, func(t *testing.T) {
t.Parallel()
cfg := &delivery.HTTPTargetConfig{
URL: target,
Timeout: 5,
}
client := engine.ExportClientForConfig(cfg)
require.NotSame(t, engine.ExportClient(), client,
"a per-target timeout must yield a "+
"distinct client",
)
assert.Equal(t,
5*time.Second, client.Timeout,
"the per-target timeout must be applied",
)
assert.Same(t,
engine.ExportClient().Transport,
client.Transport,
"the SSRF-safe transport must be reused, "+
"not dropped",
)
req, err := http.NewRequestWithContext(
context.Background(),
http.MethodPost, target, nil,
)
require.NoError(t, err)
resp, doErr := client.Do(req)
if resp != nil {
_ = resp.Body.Close()
}
require.Error(t, doErr,
"request to %s must be blocked", target,
)
assert.Contains(t, doErr.Error(), "blocked",
"error must come from the SSRF guard",
)
})
}
}
// TestClientForConfig_NoTimeoutUnchanged asserts that with
// no per-target timeout the shared SSRF-safe client is
// returned unchanged.
func TestClientForConfig_NoTimeoutUnchanged(t *testing.T) {
t.Parallel()
engine := newSSRFTestEngine()
cfg := &delivery.HTTPTargetConfig{
URL: "https://example.com/hook",
}
client := engine.ExportClientForConfig(cfg)
assert.Same(t, engine.ExportClient(), client,
"without a per-target timeout the shared client "+
"must be returned unchanged",
)
}

View File

@@ -1713,10 +1713,15 @@ func (e *Engine) clientForConfig(
cfg *HTTPTargetConfig, cfg *HTTPTargetConfig,
) *http.Client { ) *http.Client {
if cfg.Timeout > 0 { if cfg.Timeout > 0 {
// Reuse the shared client's SSRF-safe transport so
// a per-target timeout does not drop the
// request-time private-IP guard. Only the timeout
// is overridden.
return &http.Client{ return &http.Client{
Timeout: time.Duration( Timeout: time.Duration(
cfg.Timeout, cfg.Timeout,
) * time.Second, ) * time.Second,
Transport: e.client.Transport,
} }
} }

View File

@@ -126,6 +126,18 @@ func (e *Engine) ExportDoHTTPRequest(
return e.doHTTPRequest(ctx, cfg, event) return e.doHTTPRequest(ctx, cfg, event)
} }
// ExportClientForConfig exposes clientForConfig.
func (e *Engine) ExportClientForConfig(
cfg *HTTPTargetConfig,
) *http.Client {
return e.clientForConfig(cfg)
}
// ExportClient returns the engine's shared HTTP client.
func (e *Engine) ExportClient() *http.Client {
return e.client
}
// ExportScheduleRetry exposes scheduleRetry. // ExportScheduleRetry exposes scheduleRetry.
func (e *Engine) ExportScheduleRetry( func (e *Engine) ExportScheduleRetry(
task Task, delay time.Duration, task Task, delay time.Duration,