check / check (push) Waiting to run
Target.MaxRetries (max_retries) becomes Target.MaxAttempts (max_attempts), and the form field, parser and ceiling are renamed to match. The column is NOT NULL and a check constraint refuses a value below 1, so every target, whatever its type, is stored with at least 1. 1 is a single attempt with no circuit breaker; above 1 retries behind the breaker up to that many attempts. Both target forms refuse 0 and less; a blank field on the add target form gives 1, and on the edit form keeps the stored value. The target page shows the plain number, and the README's two paragraphs on 0 against 1 become one sentence. Model: opus-5-5
392 lines
10 KiB
Go
392 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), 1, 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 neither is the inbound Content-Type, because
|
|
// it is not forwarded. Two more are deliberately excluded: a
|
|
// Content-Type configured on the target 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,
|
|
"Content-Type": testContentType,
|
|
},
|
|
},
|
|
"webhooker/dev",
|
|
)
|
|
|
|
assert.Equal(t,
|
|
[]string{probeHeaderName, inboundHeaderName}, names,
|
|
"both header classes are reported, and only those: "+
|
|
"Host and the inbound Content-Type are never "+
|
|
"forwarded, User-Agent is the delivery path's own",
|
|
)
|
|
assert.NotContains(t, names, "Content-Type",
|
|
"a Content-Type configured on the target must survive "+
|
|
"a cross-origin 307/308 with the body it describes",
|
|
)
|
|
}
|