All checks were successful
check / check (push) Successful in 3m4s
The SSRF blocklist had no escape hatch, so the thing webhooker is mostly for — taking a public webhook and forwarding it to something on your own network — could not be configured at all. Every private address, Docker sibling and loopback service was permanently unreachable as a delivery destination. ALLOWED_EGRESS_CIDRS (default empty) names blocks that delivery targets may reach despite the default blocklist. It is an allowlist and only ever adds destinations: there is no boolean, and no value disables SSRF protection wholesale. Empty, it adds nothing and the guard permits and refuses the same addresses it did before, save the two spellings named below. A small set of addresses is refused before the allowlist is consulted, so no supplied CIDR opens one — not the exact address, not a supernet, not 0.0.0.0/0 or ::/0. alwaysBlockedNetworks in internal/delivery/ssrf.go is the authoritative list and states the membership criterion in full; it is deliberately not copied here, because a copy drifts out of date. In short: the provider fixes the address, so a host route for it collides with nothing the operator runs, and reaching it discloses credentials or user data. A publicly routable address never qualifies however well it meets both — nothing in this set can be reopened, so blocking one here would leave the operator no escape hatch at all. Those belong in blockedNetworks, which an allowlist can override. Every entry is already inside the default blocklist, which is what makes it unconditional rather than newly blocked, with two exceptions that are the one behaviour change visible when the allowlist is unset: ::a9fe:a9fe and 64:ff9b::a9fe:a9fe, the IPv4-compatible and NAT64 spellings of 169.254.169.254, were reachable before and are refused now. net.IPNet.Contains normalises only the IPv4-mapped form via To4(), so 169.254.0.0/16 never matched those two. The ten it does cover now report a metadata error rather than the generic private-range one. The policy now lives in one function, Guard.checkIP, which both target-creation validation and the delivery dialer call. The two paths previously decided separately, which is how they came to disagree about a destination. The guard is built once from config and injected via fx into both the handlers and the delivery engine, so there is a single instance and a single answer. A set-but-unparseable value aborts startup naming the variable, reusing the existing envPrefixList parser. A non-empty list is logged at startup with the blocks spelled out, not counted, so the hole is visible in the log of any deployment that has one. Tests: an allowlisted loopback CIDR both validates and delivers to a live server (and the same URL still fails without the allowlist); a private address outside the listed block stays refused on both paths; every unconditionally blocked address stays refused on both paths under an allowlist that covers it, and the set itself is pinned entry by entry; public addresses are unaffected either way; and config coverage for parsing, startup abort, and the warning's contents.
369 lines
8.4 KiB
Go
369 lines
8.4 KiB
Go
package handlers_test
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"html/template"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"sync"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"go.uber.org/fx"
|
|
"go.uber.org/fx/fxtest"
|
|
"sneak.berlin/go/webhooker/internal/config"
|
|
"sneak.berlin/go/webhooker/internal/database"
|
|
"sneak.berlin/go/webhooker/internal/delivery"
|
|
"sneak.berlin/go/webhooker/internal/globals"
|
|
"sneak.berlin/go/webhooker/internal/handlers"
|
|
"sneak.berlin/go/webhooker/internal/healthcheck"
|
|
"sneak.berlin/go/webhooker/internal/logger"
|
|
"sneak.berlin/go/webhooker/internal/middleware"
|
|
"sneak.berlin/go/webhooker/internal/session"
|
|
)
|
|
|
|
// recordingNotifier is a delivery.Notifier that records the tasks it
|
|
// was handed, so a test can prove a handler queued the delivery it
|
|
// claims to have queued — and, on the refusal paths, that it queued
|
|
// nothing.
|
|
type recordingNotifier struct {
|
|
mu sync.Mutex
|
|
tasks []delivery.Task
|
|
}
|
|
|
|
func (n *recordingNotifier) Notify(tasks []delivery.Task) {
|
|
n.mu.Lock()
|
|
defer n.mu.Unlock()
|
|
|
|
n.tasks = append(n.tasks, tasks...)
|
|
}
|
|
|
|
// Tasks returns a copy of the recorded tasks.
|
|
func (n *recordingNotifier) Tasks() []delivery.Task {
|
|
n.mu.Lock()
|
|
defer n.mu.Unlock()
|
|
|
|
out := make([]delivery.Task, len(n.tasks))
|
|
copy(out, n.tasks)
|
|
|
|
return out
|
|
}
|
|
|
|
// recordingEvictor is a delivery.WebhookEvictor that records
|
|
// the webhook ids it was asked to evict, so a test can prove
|
|
// that a deletion path reached the delivery engine.
|
|
type recordingEvictor struct {
|
|
mu sync.Mutex
|
|
evicted []string
|
|
}
|
|
|
|
func (r *recordingEvictor) EvictWebhook(webhookID string) {
|
|
r.mu.Lock()
|
|
defer r.mu.Unlock()
|
|
|
|
r.evicted = append(r.evicted, webhookID)
|
|
}
|
|
|
|
// Evicted returns a copy of the recorded webhook ids.
|
|
func (r *recordingEvictor) Evicted() []string {
|
|
r.mu.Lock()
|
|
defer r.mu.Unlock()
|
|
|
|
out := make([]string, len(r.evicted))
|
|
copy(out, r.evicted)
|
|
|
|
return out
|
|
}
|
|
|
|
func newTestApp(
|
|
t *testing.T,
|
|
targets ...any,
|
|
) *fxtest.App {
|
|
t.Helper()
|
|
|
|
return fxtest.New(
|
|
t,
|
|
fx.Provide(
|
|
globals.New,
|
|
logger.New,
|
|
func() *config.Config {
|
|
return &config.Config{
|
|
DataDir: t.TempDir(),
|
|
}
|
|
},
|
|
database.New,
|
|
database.NewWebhookDBManager,
|
|
healthcheck.New,
|
|
session.New,
|
|
func() *recordingNotifier {
|
|
return &recordingNotifier{}
|
|
},
|
|
func(n *recordingNotifier) delivery.Notifier {
|
|
return n
|
|
},
|
|
func() *recordingEvictor {
|
|
return &recordingEvictor{}
|
|
},
|
|
func(r *recordingEvictor) delivery.WebhookEvictor {
|
|
return r
|
|
},
|
|
middleware.New,
|
|
delivery.NewGuard,
|
|
handlers.New,
|
|
),
|
|
fx.Populate(targets...),
|
|
)
|
|
}
|
|
|
|
func TestHandleIndex_Unauthenticated(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.MethodGet, "/", nil)
|
|
w := httptest.NewRecorder()
|
|
|
|
handler := h.HandleIndex()
|
|
handler.ServeHTTP(w, req)
|
|
|
|
assert.Equal(t, http.StatusSeeOther, w.Code)
|
|
assert.Equal(
|
|
t, "/pages/login", w.Header().Get("Location"),
|
|
)
|
|
}
|
|
|
|
func TestHandleIndex_Authenticated(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
var h *handlers.Handlers
|
|
|
|
var sess *session.Session
|
|
|
|
app := newTestApp(t, &h, &sess)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(), http.MethodGet, "/", nil)
|
|
w := httptest.NewRecorder()
|
|
|
|
s, err := sess.Get(req)
|
|
require.NoError(t, err)
|
|
|
|
sess.SetUser(s, "test-user-id", "testuser")
|
|
|
|
err = sess.Save(req, w, s)
|
|
require.NoError(t, err)
|
|
|
|
req2 := httptest.NewRequestWithContext(
|
|
context.Background(), http.MethodGet, "/", nil)
|
|
|
|
for _, cookie := range w.Result().Cookies() {
|
|
req2.AddCookie(cookie)
|
|
}
|
|
|
|
w2 := httptest.NewRecorder()
|
|
h.HandleIndex().ServeHTTP(w2, req2)
|
|
|
|
assert.Equal(t, http.StatusSeeOther, w2.Code)
|
|
assert.Equal(
|
|
t, "/sources", w2.Header().Get("Location"),
|
|
)
|
|
}
|
|
|
|
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) {
|
|
t.Parallel()
|
|
|
|
var h *handlers.Handlers
|
|
|
|
app := newTestApp(t, &h)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(), http.MethodGet, "/", nil)
|
|
w := httptest.NewRecorder()
|
|
|
|
data := map[string]any{"Version": "1.0.0"}
|
|
|
|
h.RenderTemplateForTest(
|
|
w, req, "nonexistent.html", data,
|
|
)
|
|
|
|
assert.Equal(
|
|
t, http.StatusInternalServerError, w.Code,
|
|
)
|
|
}
|
|
|
|
// errMidRender is the failure a test template raises partway through
|
|
// rendering.
|
|
var errMidRender = errors.New("deliberate mid-render failure")
|
|
|
|
// midRenderFailure is template data whose first method renders and
|
|
// whose second fails, so the template aborts after output has
|
|
// already been produced.
|
|
type midRenderFailure struct{}
|
|
|
|
// Prefix is the output a streaming renderer would flush before the
|
|
// failure below aborts the template.
|
|
func (midRenderFailure) Prefix() string { return partialPageMarker }
|
|
|
|
// Boom aborts template execution.
|
|
func (midRenderFailure) Boom() (string, error) {
|
|
return "", errMidRender
|
|
}
|
|
|
|
// partialPageMarker is content the failing template emits before it
|
|
// aborts.
|
|
const partialPageMarker = "PARTIAL PAGE CONTENT"
|
|
|
|
// TestRenderTemplateMidRenderErrorSendsNoPartialBody proves the
|
|
// renderer does not commit output it cannot finish: a template that
|
|
// fails partway through must yield a 500 and a body carrying none of
|
|
// the content emitted before the failure. Against a renderer that
|
|
// executes straight into the ResponseWriter this fails on both
|
|
// counts, returning 200 with the prefix already flushed.
|
|
func TestRenderTemplateMidRenderErrorSendsNoPartialBody(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
var h *handlers.Handlers
|
|
|
|
app := newTestApp(t, &h)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
h.AddTemplateForTest("failing.html", template.Must(
|
|
template.New("failing").Parse(
|
|
`{{.Data.Prefix}}{{.Data.Boom}}TAIL`,
|
|
),
|
|
))
|
|
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(), http.MethodGet, "/", nil)
|
|
w := httptest.NewRecorder()
|
|
|
|
h.RenderTemplateForTest(
|
|
w, req, "failing.html", midRenderFailure{},
|
|
)
|
|
|
|
assert.Equal(
|
|
t, http.StatusInternalServerError, w.Code,
|
|
"a failed render must report a 500",
|
|
)
|
|
assert.Equal(
|
|
t, "Internal server error\n", w.Body.String(),
|
|
"the response must carry no part of the aborted page",
|
|
)
|
|
}
|
|
|
|
func TestBuildDatabaseTargetConfig_Valid(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
var h *handlers.Handlers
|
|
|
|
app := newTestApp(t, &h)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
// Empty expiry: the keep-forever default, empty config.
|
|
w := httptest.NewRecorder()
|
|
cfg, err := h.BuildDatabaseTargetConfigForTest(w, "")
|
|
require.NoError(t, err)
|
|
assert.Empty(t, cfg)
|
|
|
|
// Explicit never is stored as config.
|
|
w = httptest.NewRecorder()
|
|
cfg, err = h.BuildDatabaseTargetConfigForTest(w, "never")
|
|
require.NoError(t, err)
|
|
assert.JSONEq(t, `{"expiry":"never"}`, cfg)
|
|
|
|
// A positive duration is stored as config.
|
|
w = httptest.NewRecorder()
|
|
cfg, err = h.BuildDatabaseTargetConfigForTest(w, "720h")
|
|
require.NoError(t, err)
|
|
assert.JSONEq(t, `{"expiry":"720h"}`, cfg)
|
|
}
|
|
|
|
func TestBuildDatabaseTargetConfig_RejectsBadExpiry(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
var h *handlers.Handlers
|
|
|
|
app := newTestApp(t, &h)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
for _, bad := range []string{"nonsense", "7d", "-5h"} {
|
|
w := httptest.NewRecorder()
|
|
cfg, err := h.BuildDatabaseTargetConfigForTest(w, bad)
|
|
|
|
require.Error(t, err, "expiry %q", bad)
|
|
assert.Empty(t, cfg)
|
|
assert.Equal(
|
|
t, http.StatusBadRequest, w.Code,
|
|
"expiry %q should be rejected with 400", bad,
|
|
)
|
|
}
|
|
}
|