Move webhook pages to /hook/ID and inbound URLs to /h/UUID (closes #367)
check / check (push) Successful in 4m49s
check / check (push) Successful in 4m49s
The webhook page and everything under it move from /source/ID to /hook/ID, the list and new-webhook form to /hooks and /hooks/new, and the event log from .../logs to /hook/ID/events, body download included. Entrypoint URLs move from /webhook/UUID to /h/UUID, and the webhook page shows only that form. The old paths are gone. Links, redirects, form actions, tests, comments and the README follow. Both links to the event log page, and its title and heading, now read "Full Event Log". Go identifiers and template file names are unchanged. A new route test posts to the entrypoint URL the webhook page shows and checks that the receiver rate limit applies to it. Model: opus-5-5
This commit is contained in:
@@ -119,7 +119,7 @@ func accessLogRouter(m *middleware.Middleware) *chi.Mux {
|
||||
)
|
||||
|
||||
router.HandleFunc(
|
||||
"/webhook/{uuid}",
|
||||
"/h/{uuid}",
|
||||
func(w http.ResponseWriter, r *http.Request) {
|
||||
// Stands in for the real handler: an unknown entrypoint
|
||||
// UUID 404s, a known one succeeds.
|
||||
@@ -271,11 +271,11 @@ func TestAccessLog_InventedReceiverPathsLogRoutePattern(t *testing.T) {
|
||||
assertFloodIsBounded(
|
||||
t,
|
||||
func(i int) string {
|
||||
return "/webhook/" + attackerMarker +
|
||||
return "/h/" + attackerMarker +
|
||||
strings.Repeat("x", i) + "?q=" + attackerMarker
|
||||
},
|
||||
http.StatusNotFound,
|
||||
"/webhook/{uuid}",
|
||||
"/h/{uuid}",
|
||||
)
|
||||
}
|
||||
|
||||
@@ -346,10 +346,10 @@ type sizeCase struct {
|
||||
func lineSizeCases() map[string]sizeCase {
|
||||
cases := map[string]sizeCase{
|
||||
"oversized path segment": {
|
||||
target: "/webhook/" + attackerMarker +
|
||||
target: "/h/" + attackerMarker +
|
||||
strings.Repeat("x", oversizedSegmentBytes),
|
||||
wantStatus: http.StatusNotFound,
|
||||
wantURL: "/webhook/{uuid}",
|
||||
wantURL: "/h/{uuid}",
|
||||
bound: maxLineBytes,
|
||||
},
|
||||
// /.well-known/healthcheck answers 200 to anyone and has no
|
||||
@@ -605,14 +605,14 @@ func TestAccessLog_SuccessKeepsConcretePathAndRedactsQuery(
|
||||
router := accessLogRouter(m)
|
||||
|
||||
assert.Equal(
|
||||
t, http.StatusOK, get(t, router, "/webhook/known?src=ci"),
|
||||
t, http.StatusOK, get(t, router, "/h/known?src=ci"),
|
||||
)
|
||||
|
||||
// The path resolved against a stored entrypoint, so it stays. The
|
||||
// query never does: see TestAccessLog_UnauthenticatedSuccess...
|
||||
entries := accessLogEntries(t, buf)
|
||||
require.Len(t, entries, 1)
|
||||
assert.Equal(t, "/webhook/known?(redacted)", entries[0]["url"])
|
||||
assert.Equal(t, "/h/known?(redacted)", entries[0]["url"])
|
||||
assert.NotContains(t, buf.String(), "src=ci")
|
||||
}
|
||||
|
||||
@@ -640,7 +640,7 @@ func TestAccessLog_RetainsEveryOtherField(t *testing.T) {
|
||||
assert.Equal(
|
||||
t,
|
||||
http.StatusNotFound,
|
||||
get(t, router, "/webhook/"+attackerMarker),
|
||||
get(t, router, "/h/"+attackerMarker),
|
||||
)
|
||||
|
||||
entries := accessLogEntries(t, buf)
|
||||
|
||||
@@ -41,7 +41,7 @@ func (m *Middleware) CSRF() func(http.Handler) http.Handler {
|
||||
// CSRF is registered ahead of RequireAuth on every route
|
||||
// group that uses it, so this WARN is reachable by an
|
||||
// unauthenticated client: a POST with no token to
|
||||
// /source/<any length of any text>/edit lands here. The
|
||||
// /hook/<any length of any text>/edit lands here. The
|
||||
// method and path are capped against the same budgets as
|
||||
// the access log. remote_addr is set by net/http from the
|
||||
// accepted connection rather than by the client, and
|
||||
|
||||
@@ -383,7 +383,7 @@ func TestLogLines_ClientChosenPathDoesNotSizeTheLine(t *testing.T) {
|
||||
t, newHandler,
|
||||
)
|
||||
|
||||
path := "/source/" +
|
||||
path := "/hook/" +
|
||||
oversizedPathSegment(fill) + "/edit"
|
||||
|
||||
assert.Equal(
|
||||
@@ -434,7 +434,7 @@ func TestLoginThrottle_LogLineDoesNotTrackPathSize(t *testing.T) {
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(),
|
||||
http.MethodPost,
|
||||
"/source/"+
|
||||
"/hook/"+
|
||||
oversizedPathSegment(fill)+"/login",
|
||||
nil,
|
||||
)
|
||||
@@ -499,7 +499,7 @@ func TestMaxBodySize_FloodOfOversizePathsDoesNotGrowTheLog(
|
||||
http.StatusRequestEntityTooLarge,
|
||||
postOversize(
|
||||
h,
|
||||
"/source/"+segment(i)+"/edit",
|
||||
"/hook/"+segment(i)+"/edit",
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -40,7 +40,7 @@ const unmatchedMethod = unmatchedRoute
|
||||
//
|
||||
// The pattern is what bounds the label's domain to the routes the
|
||||
// service registers. The path does not bound it at all — every byte
|
||||
// after /webhook/ is client-chosen, so labelling by path lets any
|
||||
// after /h/ is client-chosen, so labelling by path lets any
|
||||
// unauthenticated client mint permanent series at will, and publishes
|
||||
// the entrypoint UUID (the receiver's only credential) in the scrape
|
||||
// while doing it.
|
||||
|
||||
@@ -50,7 +50,7 @@ func realMethods() []string {
|
||||
// dimension varying, so any series growth a probe produces is the
|
||||
// method label's and nothing else's.
|
||||
func methodProbePath() string {
|
||||
return "/webhook/" + uuid.NewString()
|
||||
return "/h/" + uuid.NewString()
|
||||
}
|
||||
|
||||
// inventedMethods returns n distinct RFC 9110 method tokens that no
|
||||
|
||||
@@ -28,7 +28,7 @@ const (
|
||||
|
||||
// receiverRoutePattern is the one handler label every receiver
|
||||
// request must produce, however the client varies the path.
|
||||
receiverRoutePattern = "/webhook/{uuid}"
|
||||
receiverRoutePattern = "/h/{uuid}"
|
||||
|
||||
// okRoute is a static route used to pin that the response-writer
|
||||
// interceptor still reports status and size after the handler id
|
||||
@@ -143,13 +143,13 @@ func drivePaths(
|
||||
return drive(t, h, probes)
|
||||
}
|
||||
|
||||
// receiverPaths returns n distinct /webhook/ paths, each naming a
|
||||
// receiverPaths returns n distinct /h/ paths, each naming a
|
||||
// fresh UUID exactly as an unauthenticated flood would.
|
||||
func receiverPaths(n int) []string {
|
||||
paths := make([]string, 0, n)
|
||||
|
||||
for range n {
|
||||
paths = append(paths, "/webhook/"+uuid.NewString())
|
||||
paths = append(paths, "/h/"+uuid.NewString())
|
||||
}
|
||||
|
||||
return paths
|
||||
@@ -220,7 +220,7 @@ func keys(set map[string]struct{}) []string {
|
||||
|
||||
// TestMetrics_DistinctReceiverPathsMintOneLabelSet is the direct
|
||||
// assertion the issue asks for: N requests to N distinct
|
||||
// /webhook/<uuid> paths must produce exactly ONE handler label, the
|
||||
// /h/<uuid> paths must produce exactly ONE handler label, the
|
||||
// route pattern. Before the fix this produced N of them.
|
||||
func TestMetrics_DistinctReceiverPathsMintOneLabelSet(t *testing.T) {
|
||||
t.Parallel()
|
||||
@@ -250,7 +250,7 @@ func TestMetrics_DistinctReceiverPathsMintOneLabelSet(t *testing.T) {
|
||||
// The scrape must not republish the UUIDs it was driven with.
|
||||
// They are the receiver's only credential.
|
||||
for _, p := range paths {
|
||||
id := strings.TrimPrefix(p, "/webhook/")
|
||||
id := strings.TrimPrefix(p, "/h/")
|
||||
for label := range labels {
|
||||
assert.NotContains(
|
||||
t, label, id,
|
||||
@@ -354,7 +354,7 @@ func TestMetrics_UnmatchedPathsCollapseToTheSentinel(t *testing.T) {
|
||||
if i%2 == 0 {
|
||||
paths = append(paths, "/"+id)
|
||||
} else {
|
||||
paths = append(paths, "/webhook/"+id+"/"+id)
|
||||
paths = append(paths, "/h/"+id+"/"+id)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -257,7 +257,7 @@ func concreteLogURL(r *http.Request) string {
|
||||
//
|
||||
// 3xx and 4xx responses get the chi route pattern instead. Those are
|
||||
// the outcomes an unauthenticated client drives for free: 404 or 429
|
||||
// on any invented /webhook/ path, 303 to the login page on any
|
||||
// on any invented /h/ path, 303 to the login page on any
|
||||
// invented /user/ path. Logging the concrete URL there lets a flood
|
||||
// write attacker-chosen text, of attacker-chosen length, into the
|
||||
// operator's log at one line per request. The pattern comes from the
|
||||
@@ -560,7 +560,7 @@ func (s *Middleware) MaxBodySize(
|
||||
// internal/server/routes.go), so an
|
||||
// unauthenticated client reaches it with a path
|
||||
// of its own choosing and its own length —
|
||||
// POST /source/<8 KB>/edit with an oversize
|
||||
// POST /hook/<8 KB>/edit with an oversize
|
||||
// declared Content-Length costs nothing to
|
||||
// send. At WARN, on by default, that is a
|
||||
// write into the operator's log sized by the
|
||||
|
||||
@@ -640,7 +640,7 @@ func TestNoCache_SetsHeaders(t *testing.T) {
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(),
|
||||
http.MethodGet, "/sources", nil,
|
||||
http.MethodGet, "/hooks", nil,
|
||||
)
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
|
||||
@@ -63,7 +63,7 @@ const (
|
||||
|
||||
// receiverAggregateMultiplier scales the configured
|
||||
// per-entrypoint receiver limit into the aggregate limit one
|
||||
// client IP may spend across the whole /webhook/* route. Ten
|
||||
// client IP may spend across the whole /h/* route. Ten
|
||||
// entrypoints' worth lets a single sender address drive several
|
||||
// entrypoints at their full rate, while still capping what one
|
||||
// address costs the unauthenticated receiver.
|
||||
@@ -390,7 +390,7 @@ func (m *Middleware) postRateLimit(
|
||||
// It is Config.ReceiverRateLimit requests per minute.
|
||||
//
|
||||
// That limit alone bounds nothing in aggregate. The route pattern
|
||||
// /webhook/{uuid} matches any single segment, so a client that
|
||||
// /h/{uuid} matches any single segment, so a client that
|
||||
// invents a fresh path per request mints a fresh bucket per request
|
||||
// and never refills one — and every such request still reaches the
|
||||
// handler's entrypoint lookup before it 404s. The outer limit is
|
||||
|
||||
@@ -275,7 +275,7 @@ func TestReceiverRateLimit_LimitsPerIPAndPath(t *testing.T) {
|
||||
// pass.
|
||||
for i := range limit {
|
||||
w := receiverPost(
|
||||
handler, "9.9.9.9:1234", "/webhook/uuid-a",
|
||||
handler, "9.9.9.9:1234", "/h/uuid-a",
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
@@ -286,7 +286,7 @@ func TestReceiverRateLimit_LimitsPerIPAndPath(t *testing.T) {
|
||||
// The next request over the limit is rejected with a 429
|
||||
// carrying a Retry-After header.
|
||||
w := receiverPost(
|
||||
handler, "9.9.9.9:1234", "/webhook/uuid-a",
|
||||
handler, "9.9.9.9:1234", "/h/uuid-a",
|
||||
)
|
||||
assert.Equal(t, http.StatusTooManyRequests, w.Code)
|
||||
assert.NotEmpty(
|
||||
@@ -296,7 +296,7 @@ func TestReceiverRateLimit_LimitsPerIPAndPath(t *testing.T) {
|
||||
|
||||
// The same IP is not limited on a different entrypoint.
|
||||
w = receiverPost(
|
||||
handler, "9.9.9.9:1234", "/webhook/uuid-b",
|
||||
handler, "9.9.9.9:1234", "/h/uuid-b",
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
@@ -305,7 +305,7 @@ func TestReceiverRateLimit_LimitsPerIPAndPath(t *testing.T) {
|
||||
|
||||
// A different IP is not limited on the same entrypoint.
|
||||
w = receiverPost(
|
||||
handler, "8.8.8.8:1234", "/webhook/uuid-a",
|
||||
handler, "8.8.8.8:1234", "/h/uuid-a",
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
@@ -322,7 +322,7 @@ func TestReceiverRateLimit_CountsEveryMethod(t *testing.T) {
|
||||
const (
|
||||
limit = 2
|
||||
ip = "7.7.7.7:1234"
|
||||
path = "/webhook/uuid-c"
|
||||
path = "/h/uuid-c"
|
||||
)
|
||||
|
||||
handler := receiverLimitedHandler(t, limit)
|
||||
@@ -715,7 +715,7 @@ func TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths(
|
||||
// none of them shares a per-entrypoint bucket with another.
|
||||
for i := range aggregate {
|
||||
w := receiverPost(
|
||||
handler, ip, fmt.Sprintf("/webhook/invented-%d", i),
|
||||
handler, ip, fmt.Sprintf("/h/invented-%d", i),
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
@@ -724,17 +724,17 @@ func TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths(
|
||||
}
|
||||
|
||||
w := receiverPost(
|
||||
handler, ip, fmt.Sprintf("/webhook/invented-%d", aggregate),
|
||||
handler, ip, fmt.Sprintf("/h/invented-%d", aggregate),
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusTooManyRequests, w.Code,
|
||||
"a client must not be able to raise its aggregate rate "+
|
||||
"against /webhook/* by varying the path",
|
||||
"against /h/* by varying the path",
|
||||
)
|
||||
|
||||
// The aggregate limit is still per client IP: exhausting one
|
||||
// address must not throttle another.
|
||||
w = receiverPost(handler, "6.6.6.7:1234", "/webhook/invented-0")
|
||||
w = receiverPost(handler, "6.6.6.7:1234", "/h/invented-0")
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"a different client IP must not be affected",
|
||||
@@ -771,7 +771,7 @@ func TestReceiverRateLimit_RejectedRequestsCountTowardAggregate(
|
||||
// limit requests are served; the rest are rejected by the
|
||||
// per-entrypoint limiter but still count against the aggregate.
|
||||
for i := range aggregate {
|
||||
w := receiverPost(handler, ip, "/webhook/exhausted")
|
||||
w := receiverPost(handler, ip, "/h/exhausted")
|
||||
|
||||
want := http.StatusTooManyRequests
|
||||
if i < limit {
|
||||
@@ -784,7 +784,7 @@ func TestReceiverRateLimit_RejectedRequestsCountTowardAggregate(
|
||||
)
|
||||
}
|
||||
|
||||
w := receiverPost(handler, ip, "/webhook/never-used")
|
||||
w := receiverPost(handler, ip, "/h/never-used")
|
||||
assert.Equal(
|
||||
t, http.StatusTooManyRequests, w.Code,
|
||||
"requests rejected per entrypoint must still count "+
|
||||
@@ -823,7 +823,7 @@ func TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer(
|
||||
const (
|
||||
limit = 3
|
||||
peer = "203.0.113.10:44444"
|
||||
path = "/webhook/uuid-d"
|
||||
path = "/h/uuid-d"
|
||||
)
|
||||
|
||||
handler := receiverLimitedHandler(t, limit)
|
||||
|
||||
Reference in New Issue
Block a user