All checks were successful
check / check (push) Successful in 3m35s
The receiver has authenticated on the entrypoint UUID alone since inbound signature verification was removed in #279. The README described that as the current state; it did not say it is the decision. Restate it as the rule, so a proposal to add HMAC, a shared secret or a bearer token to the receiver is contradicted by the docs rather than merely unimplemented. The rule now appears in the intro, in its own section, and in the Authentication and Security lists, and carries the two consequences an operator has to act on: the URL is a capability to be kept out of logs and tickets, and rotation means minting a new entrypoint rather than changing a key. Also corrects one stale comment: a redirect test said the inbound signature was one "the receiver verifies", which in this repo's vocabulary names webhooker's own receiver. The endpoint that verifies it is the delivery target's.
385 lines
10 KiB
Go
385 lines
10 KiB
Go
package delivery_test
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"log/slog"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"sync"
|
|
"sync/atomic"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/google/uuid"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"sneak.berlin/go/webhooker/internal/database"
|
|
"sneak.berlin/go/webhooker/internal/delivery"
|
|
)
|
|
|
|
// The headers these tests drive stand in for the two classes the
|
|
// off-origin rule covers: an operator-configured credential and an
|
|
// inbound header the delivery path forwards. net/http withholds
|
|
// Authorization and Cookie across a host change, and nothing else.
|
|
const (
|
|
probeHeaderName = "X-Api-Key"
|
|
probeHeaderValue = "QQNEVERONTHEWIREQQ"
|
|
inboundHeaderName = "X-Hub-Signature"
|
|
inboundHeaderValue = "sha1=QQINBOUNDQQ"
|
|
)
|
|
|
|
// redirectProbe records what the last hop of a redirect chain
|
|
// actually received.
|
|
type redirectProbe struct {
|
|
mu sync.Mutex
|
|
seen http.Header
|
|
hits int
|
|
}
|
|
|
|
func (p *redirectProbe) serve(
|
|
w http.ResponseWriter, r *http.Request,
|
|
) {
|
|
p.mu.Lock()
|
|
p.seen = r.Header.Clone()
|
|
p.hits++
|
|
p.mu.Unlock()
|
|
|
|
w.WriteHeader(http.StatusOK)
|
|
}
|
|
|
|
func (p *redirectProbe) result() (http.Header, int) {
|
|
p.mu.Lock()
|
|
defer p.mu.Unlock()
|
|
|
|
return p.seen, p.hits
|
|
}
|
|
|
|
// deliverWithProbeHeaders runs one real delivery of a new task
|
|
// through the engine to targetURL, carrying both probe headers —
|
|
// probeHeaderName configured on the target, inboundHeaderName
|
|
// forwarded from the event — and returns the delivery status the
|
|
// engine recorded.
|
|
func deliverWithProbeHeaders(
|
|
t *testing.T, targetURL string,
|
|
) database.DeliveryStatus {
|
|
t.Helper()
|
|
|
|
s := newISetup(t)
|
|
|
|
event := iSeedEvent(
|
|
t, s.WebhookDB, s.WebhookID, `{"hello":"world"}`,
|
|
)
|
|
targetID := uuid.New().String()
|
|
|
|
inbound, err := json.Marshal(map[string][]string{
|
|
inboundHeaderName: {inboundHeaderValue},
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
event.Headers = string(inbound)
|
|
|
|
d := iSeedDelivery(
|
|
t, s.WebhookDB, event.ID, targetID,
|
|
database.DeliveryStatusPending,
|
|
)
|
|
|
|
cfg, err := json.Marshal(delivery.HTTPTargetConfig{
|
|
URL: targetURL,
|
|
Headers: map[string]string{
|
|
probeHeaderName: probeHeaderValue,
|
|
},
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
body := event.Body
|
|
task := iTask(
|
|
d, event, s.WebhookID, targetID,
|
|
"redirect-target", string(cfg), 0, 1, &body,
|
|
)
|
|
|
|
s.Engine.ExportProcessNewTask(context.TODO(), &task)
|
|
|
|
var updated database.Delivery
|
|
|
|
require.NoError(t, s.WebhookDB.First(
|
|
&updated, "id = ?", d.ID,
|
|
).Error)
|
|
|
|
return updated.Status
|
|
}
|
|
|
|
// A 302 to an origin the operator never configured must not carry
|
|
// the credential they configured for the one they did, nor the
|
|
// inbound header this delivery forwarded — one rule for both
|
|
// classes. The chain is still followed, so the delivery is recorded
|
|
// from the final hop.
|
|
func TestDelivery_CrossOriginRedirectDropsOriginScopedHeaders(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
var probe redirectProbe
|
|
|
|
final := httptest.NewServer(
|
|
http.HandlerFunc(probe.serve),
|
|
)
|
|
defer final.Close()
|
|
|
|
// httptest listens on loopback, so reach the second server
|
|
// under loopback's other name: the hop then differs in
|
|
// hostname as well as port and is cross-host by any reading.
|
|
finalURL, err := url.Parse(final.URL)
|
|
require.NoError(t, err)
|
|
|
|
finalURL.Host = "localhost:" + finalURL.Port()
|
|
finalURL.Path = "/moved"
|
|
|
|
origin := httptest.NewServer(http.HandlerFunc(
|
|
func(w http.ResponseWriter, r *http.Request) {
|
|
http.Redirect(
|
|
w, r, finalURL.String(),
|
|
http.StatusFound,
|
|
)
|
|
},
|
|
))
|
|
defer origin.Close()
|
|
|
|
status := deliverWithProbeHeaders(t, origin.URL)
|
|
|
|
seen, hits := probe.result()
|
|
|
|
assert.Equal(t, 1, hits,
|
|
"the redirect must still be followed",
|
|
)
|
|
assert.Empty(t, seen.Get(probeHeaderName),
|
|
"a configured credential header must not reach an "+
|
|
"origin the operator did not configure",
|
|
)
|
|
assert.Empty(t, seen.Get(inboundHeaderName),
|
|
"a forwarded inbound header must not reach an origin "+
|
|
"the operator did not configure",
|
|
)
|
|
assert.Equal(t,
|
|
database.DeliveryStatusDelivered, status,
|
|
"the final hop's 200 is the delivery's result",
|
|
)
|
|
}
|
|
|
|
// Stripping must not fire within the configured origin, or every
|
|
// destination that redirects its own path would lose its
|
|
// credential and start answering 401 — and would lose the inbound
|
|
// signature header the target endpoint verifies. webhooker's own
|
|
// receiver verifies no signature; it only forwards the header.
|
|
func TestDelivery_SameOriginRedirectKeepsOriginScopedHeaders(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
var probe redirectProbe
|
|
|
|
srv := httptest.NewServer(http.HandlerFunc(
|
|
func(w http.ResponseWriter, r *http.Request) {
|
|
if r.URL.Path == "/moved" {
|
|
probe.serve(w, r)
|
|
|
|
return
|
|
}
|
|
|
|
http.Redirect(
|
|
w, r, "/moved", http.StatusFound,
|
|
)
|
|
},
|
|
))
|
|
defer srv.Close()
|
|
|
|
status := deliverWithProbeHeaders(t, srv.URL+"/hook")
|
|
|
|
seen, hits := probe.result()
|
|
|
|
assert.Equal(t, 1, hits)
|
|
assert.Equal(t, probeHeaderValue, seen.Get(probeHeaderName),
|
|
"a redirect within the configured origin must keep "+
|
|
"the configured header",
|
|
)
|
|
assert.Equal(t,
|
|
inboundHeaderValue, seen.Get(inboundHeaderName),
|
|
"a redirect within the configured origin must keep "+
|
|
"the forwarded inbound header",
|
|
)
|
|
assert.Equal(t,
|
|
database.DeliveryStatusDelivered, status,
|
|
)
|
|
}
|
|
|
|
// The origin comparison is deliberately stricter than the one
|
|
// net/http applies to Authorization: the port counts and a
|
|
// subdomain does not inherit. Only the default-port spellings of
|
|
// one origin are the same origin.
|
|
func TestSameDeliveryOrigin(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// The configured target URL every case redirects away from.
|
|
// Destination paths differ only so that no literal repeats.
|
|
const configured = "https://h/a"
|
|
|
|
cases := map[string]struct {
|
|
origin string
|
|
dest string
|
|
want bool
|
|
}{
|
|
"other path": {configured, "https://h/b", true},
|
|
"default port spelled": {configured, "https://h:443/c", true},
|
|
"host in another case": {configured, "https://H/d", true},
|
|
"http default port": {"http://h:80/a", "http://h/e", true},
|
|
"upgrade to https": {"http://h/a", "https://h/f", true},
|
|
"downgrade to http": {configured, "http://h/g", false},
|
|
"another host": {configured, "https://i/h", false},
|
|
"a subdomain": {configured, "https://x.h/i", false},
|
|
"the parent domain": {"https://x.h/a", "https://h/j", false},
|
|
"another port": {configured, "https://h:8443/k", false},
|
|
|
|
// Hostname() unwraps an IPv6 literal's brackets, so a
|
|
// bracketed host whose last group is the origin's port
|
|
// renders identically to the origin unless the port is
|
|
// re-joined with brackets. Each dest below differs from
|
|
// its origin in address AND in port.
|
|
"ipv6 port as final group": {
|
|
"https://[2001:db8::1]:8080/a",
|
|
"https://[2001:db8::1:8080]/l",
|
|
false,
|
|
},
|
|
"ipv6 loopback port as final group": {
|
|
"https://[::1]:8080/a",
|
|
"https://[::1:8080]/m",
|
|
false,
|
|
},
|
|
"ipv6 same origin": {
|
|
"https://[2001:db8::1]:8080/a",
|
|
"https://[2001:DB8::1]:8080/n",
|
|
true,
|
|
},
|
|
}
|
|
|
|
for name, tc := range cases {
|
|
t.Run(name, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
origin, err := url.Parse(tc.origin)
|
|
require.NoError(t, err)
|
|
|
|
dest, err := url.Parse(tc.dest)
|
|
require.NoError(t, err)
|
|
|
|
assert.Equal(t, tc.want,
|
|
delivery.ExportSameDeliveryOrigin(
|
|
origin, dest,
|
|
),
|
|
)
|
|
})
|
|
}
|
|
}
|
|
|
|
// Installing a CheckRedirect discards net/http's own redirect
|
|
// limit, so the cap this policy restates is the only thing between
|
|
// a self-redirecting destination and an unbounded chain. A
|
|
// destination that always redirects must be cut off after exactly
|
|
// maxDeliveryRedirects requests, with the sentinel surfacing to the
|
|
// caller rather than a generic net/http error.
|
|
func TestRedirectPolicy_StopsAtHopCap(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
var hits atomic.Int64
|
|
|
|
srv := httptest.NewServer(http.HandlerFunc(
|
|
func(w http.ResponseWriter, r *http.Request) {
|
|
hits.Add(1)
|
|
http.Redirect(
|
|
w, r, "/loop", http.StatusFound,
|
|
)
|
|
},
|
|
))
|
|
defer srv.Close()
|
|
|
|
engine := delivery.NewTestEngine(
|
|
slog.New(slog.DiscardHandler),
|
|
&http.Client{Timeout: 10 * time.Second},
|
|
1,
|
|
)
|
|
|
|
client := engine.ExportClientForRequest(
|
|
&delivery.HTTPTargetConfig{URL: srv.URL},
|
|
[]string{probeHeaderName},
|
|
)
|
|
require.NotNil(t, client.CheckRedirect)
|
|
|
|
req, err := http.NewRequestWithContext(
|
|
context.Background(), http.MethodGet, srv.URL, http.NoBody,
|
|
)
|
|
require.NoError(t, err)
|
|
|
|
resp, doErr := client.Do(req)
|
|
if resp != nil {
|
|
_ = resp.Body.Close()
|
|
}
|
|
|
|
require.Error(t, doErr,
|
|
"an endless redirect chain must not be followed forever",
|
|
)
|
|
require.ErrorIs(t, doErr, delivery.ErrExportTooManyRedirects)
|
|
|
|
assert.Equal(t,
|
|
int64(delivery.ExportMaxDeliveryRedirects), hits.Load(),
|
|
"the chain must stop after exactly %d hops",
|
|
delivery.ExportMaxDeliveryRedirects,
|
|
)
|
|
}
|
|
|
|
// The set the redirect policy strips is whatever the delivery path
|
|
// actually put on the wire, so a header added to the forward set is
|
|
// covered without a second edit. A header the event never carried
|
|
// is not in the set, and the delivery path's own two are deliberately
|
|
// excluded: Content-Type describes the body, which a 307 carries
|
|
// across hosts, and the inbound User-Agent every real sender supplies
|
|
// is overwritten before the request goes out.
|
|
func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
inbound, err := json.Marshal(map[string][]string{
|
|
inboundHeaderName: {inboundHeaderValue},
|
|
"Content-Type": {testContentType},
|
|
"User-Agent": {"curl/8.7.1"},
|
|
"Host": {"inbound.example.com"},
|
|
})
|
|
require.NoError(t, err)
|
|
|
|
req, err := http.NewRequestWithContext(
|
|
context.Background(),
|
|
http.MethodPost,
|
|
"https://target.example.com/hook",
|
|
http.NoBody,
|
|
)
|
|
require.NoError(t, err)
|
|
|
|
names := delivery.ExportApplyRequestHeaders(
|
|
req,
|
|
&database.Event{
|
|
Headers: string(inbound),
|
|
ContentType: testContentType,
|
|
},
|
|
&delivery.HTTPTargetConfig{
|
|
Headers: map[string]string{
|
|
probeHeaderName: probeHeaderValue,
|
|
},
|
|
},
|
|
)
|
|
|
|
assert.Equal(t,
|
|
[]string{probeHeaderName, inboundHeaderName}, names,
|
|
"both header classes are reported, and only those: "+
|
|
"Host is never forwarded, Content-Type and "+
|
|
"User-Agent are the delivery path's own",
|
|
)
|
|
}
|