clawbot
d2ce961fe5
check / check (push) Waiting to run
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
638 lines
16 KiB
Go
638 lines
16 KiB
Go
package handlers_test
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/go-chi/chi"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"gorm.io/gorm/clause"
|
|
"sneak.berlin/go/webhooker/internal/database"
|
|
"sneak.berlin/go/webhooker/internal/delivery"
|
|
)
|
|
|
|
// The destinations the target edit tests configure. Both are literal
|
|
// public addresses rather than hostnames so the SSRF check resolves
|
|
// nothing: with a hostname, a sandbox without DNS would reject the
|
|
// URL for the wrong reason and a test asserting rejection would pass
|
|
// even with the guard removed.
|
|
const (
|
|
editOriginalURL = "https://93.184.216.34/hooks/original"
|
|
editReplacedURL = "https://93.184.216.34/hooks/replaced"
|
|
// editBlockedURL resolves to loopback, which the SSRF guard
|
|
// refuses. It is what proves the guard runs on the edit path.
|
|
editBlockedURL = "http://127.0.0.1/hooks/internal"
|
|
)
|
|
|
|
// editAuthHeader carries a bearer credential, the case the headers
|
|
// field exists for.
|
|
const (
|
|
editBearerSecret = "QQEDITSECRETQQ"
|
|
editAuthHeader = "Authorization: Bearer " + editBearerSecret
|
|
)
|
|
|
|
// targetRouter mounts the target create and edit routes on a chi
|
|
// router so the handlers see the URL parameters they read.
|
|
func targetRouter(env *sourceTestEnv) *chi.Mux {
|
|
router := chi.NewRouter()
|
|
router.Post(
|
|
"/hook/{sourceID}/targets",
|
|
env.handlers.HandleTargetCreate(),
|
|
)
|
|
router.Get(
|
|
"/hook/{sourceID}/targets/{targetID}/edit",
|
|
env.handlers.HandleTargetEdit(),
|
|
)
|
|
router.Post(
|
|
"/hook/{sourceID}/targets/{targetID}/edit",
|
|
env.handlers.HandleTargetEditSubmit(),
|
|
)
|
|
|
|
return router
|
|
}
|
|
|
|
// serveTarget drives one request through the target routes as the
|
|
// authenticated test user.
|
|
func serveTarget(
|
|
env *sourceTestEnv,
|
|
method, path string,
|
|
form url.Values,
|
|
) *httptest.ResponseRecorder {
|
|
body := ""
|
|
if form != nil {
|
|
body = form.Encode()
|
|
}
|
|
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(), method, path,
|
|
strings.NewReader(body),
|
|
)
|
|
if form != nil {
|
|
req.Header.Set(
|
|
"Content-Type",
|
|
"application/x-www-form-urlencoded",
|
|
)
|
|
}
|
|
|
|
for _, c := range env.cookies {
|
|
req.AddCookie(c)
|
|
}
|
|
|
|
w := httptest.NewRecorder()
|
|
targetRouter(env).ServeHTTP(w, req)
|
|
|
|
return w
|
|
}
|
|
|
|
// seedHTTPTarget creates a webhook and an HTTP target on it through
|
|
// the real create handler, so every case starts from a target the
|
|
// production path produced rather than a hand-written row.
|
|
//
|
|
// Standing the fx app up is what a handler test mostly costs, and
|
|
// internal/handlers is already the slowest package in the suite, so
|
|
// the tests below share one env per test function and give each case
|
|
// its own webhook rather than its own app.
|
|
func seedHTTPTarget(
|
|
t *testing.T,
|
|
env *sourceTestEnv,
|
|
headers, timeout string,
|
|
) (database.Webhook, database.Target) {
|
|
t.Helper()
|
|
|
|
webhook := seedWebhookWithRetention(t, env.db, 30)
|
|
|
|
form := url.Values{}
|
|
form.Set("name", "original-name")
|
|
form.Set("type", string(database.TargetTypeHTTP))
|
|
form.Set("url", editOriginalURL)
|
|
form.Set("headers", headers)
|
|
form.Set("timeout", timeout)
|
|
form.Set("max_retries", "3")
|
|
|
|
w := serveTarget(
|
|
env, http.MethodPost,
|
|
"/hook/"+webhook.ID+"/targets", form,
|
|
)
|
|
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
|
|
|
targets := targetsForWebhook(t, env.db, webhook.ID)
|
|
require.Len(t, targets, 1)
|
|
|
|
return webhook, targets[0]
|
|
}
|
|
|
|
// storedTarget reloads a target row.
|
|
func storedTarget(
|
|
t *testing.T,
|
|
env *sourceTestEnv,
|
|
targetID string,
|
|
) database.Target {
|
|
t.Helper()
|
|
|
|
var target database.Target
|
|
|
|
require.NoError(
|
|
t,
|
|
env.db.DB().Where("id = ?", targetID).
|
|
First(&target).Error,
|
|
)
|
|
|
|
return target
|
|
}
|
|
|
|
// storedHTTPConfig reloads a target and parses its stored HTTP
|
|
// configuration.
|
|
func storedHTTPConfig(
|
|
t *testing.T,
|
|
env *sourceTestEnv,
|
|
targetID string,
|
|
) delivery.HTTPTargetConfig {
|
|
t.Helper()
|
|
|
|
var cfg delivery.HTTPTargetConfig
|
|
|
|
require.NoError(
|
|
t,
|
|
json.Unmarshal(
|
|
[]byte(storedTarget(t, env, targetID).Config), &cfg,
|
|
),
|
|
)
|
|
|
|
return cfg
|
|
}
|
|
|
|
// editForm is the fully populated edit submission for an HTTP
|
|
// target.
|
|
func editForm(targetURL, headers, timeout string) url.Values {
|
|
form := url.Values{}
|
|
form.Set("name", "edited-name")
|
|
form.Set("url", targetURL)
|
|
form.Set("headers", headers)
|
|
form.Set("timeout", timeout)
|
|
form.Set("max_retries", "5")
|
|
|
|
return form
|
|
}
|
|
|
|
// submitTargetEdit posts the edit form for a target.
|
|
func submitTargetEdit(
|
|
env *sourceTestEnv,
|
|
webhookID, targetID string,
|
|
form url.Values,
|
|
) *httptest.ResponseRecorder {
|
|
return serveTarget(
|
|
env, http.MethodPost,
|
|
"/hook/"+webhookID+"/targets/"+targetID+"/edit",
|
|
form,
|
|
)
|
|
}
|
|
|
|
// TestHandleTargetCreate_Configuration covers the half of the gap
|
|
// that is not about editing at all: HTTPTargetConfig has carried
|
|
// Headers and Timeout, and the delivery path has honoured them, but
|
|
// the create form wrote {"url":...} and nothing else, so a
|
|
// destination needing an Authorization header could not be
|
|
// configured through the UI at all.
|
|
func TestHandleTargetCreate_Configuration(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
|
|
t.Run("stores headers and timeout", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertCreateStoresHeadersAndTimeout(t, env)
|
|
})
|
|
|
|
t.Run("without them keeps a url-only config", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertCreateKeepsURLOnlyConfig(t, env)
|
|
})
|
|
}
|
|
|
|
func assertCreateStoresHeadersAndTimeout(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
_, target := seedHTTPTarget(
|
|
t, env, editAuthHeader+"\nX-Tenant: acme\n", "12",
|
|
)
|
|
|
|
cfg := storedHTTPConfig(t, env, target.ID)
|
|
|
|
assert.Equal(t, editOriginalURL, cfg.URL)
|
|
assert.Equal(t, 12, cfg.Timeout)
|
|
assert.Equal(
|
|
t,
|
|
map[string]string{
|
|
"Authorization": "Bearer " + editBearerSecret,
|
|
"X-Tenant": "acme",
|
|
},
|
|
cfg.Headers,
|
|
)
|
|
}
|
|
|
|
// Without the new fields the stored shape must be the same
|
|
// {"url":...} the create form wrote before they existed, so no
|
|
// existing target's configuration is rewritten by this change.
|
|
func assertCreateKeepsURLOnlyConfig(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
_, target := seedHTTPTarget(t, env, "", "")
|
|
|
|
assert.JSONEq(
|
|
t, `{"url":"`+editOriginalURL+`"}`, target.Config,
|
|
)
|
|
}
|
|
|
|
// TestHandleTargetEditSubmit_Saves is the round trip the issue asks
|
|
// for: create a target, edit it, and confirm the stored config
|
|
// changed.
|
|
func TestHandleTargetEditSubmit_Saves(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
|
|
t.Run("changes the destination URL", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertEditChangesDestination(t, env)
|
|
})
|
|
|
|
t.Run("round trips headers and timeout", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertEditRoundTripsHeadersAndTimeout(t, env)
|
|
})
|
|
|
|
t.Run("clearing them removes them", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertEditClearingRemovesThem(t, env)
|
|
})
|
|
|
|
t.Run("absent max_retries is not zeroed", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertEditKeepsAbsentMaxRetries(t, env)
|
|
})
|
|
}
|
|
|
|
func assertEditChangesDestination(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
webhook, target := seedHTTPTarget(t, env, "", "")
|
|
|
|
w := submitTargetEdit(
|
|
env, webhook.ID, target.ID,
|
|
editForm(editReplacedURL, "", ""),
|
|
)
|
|
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
|
|
|
assert.Equal(
|
|
t,
|
|
editReplacedURL,
|
|
storedHTTPConfig(t, env, target.ID).URL,
|
|
)
|
|
|
|
reloaded := storedTarget(t, env, target.ID)
|
|
assert.Equal(t, "edited-name", reloaded.Name)
|
|
assert.Equal(t, 5, reloaded.MaxRetries)
|
|
assert.Equal(
|
|
t, database.TargetTypeHTTP, reloaded.Type,
|
|
"the edit form must not change a target's type",
|
|
)
|
|
}
|
|
|
|
// The two previously unreachable fields must survive create,
|
|
// pre-fill and save.
|
|
func assertEditRoundTripsHeadersAndTimeout(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
webhook, target := seedHTTPTarget(t, env, editAuthHeader, "7")
|
|
|
|
w := submitTargetEdit(
|
|
env, webhook.ID, target.ID,
|
|
editForm(
|
|
editOriginalURL,
|
|
"Authorization: Bearer rotated\nX-Trace: on",
|
|
"21",
|
|
),
|
|
)
|
|
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
|
|
|
cfg := storedHTTPConfig(t, env, target.ID)
|
|
|
|
assert.Equal(t, 21, cfg.Timeout)
|
|
assert.Equal(
|
|
t,
|
|
map[string]string{
|
|
"Authorization": "Bearer rotated",
|
|
"X-Trace": "on",
|
|
},
|
|
cfg.Headers,
|
|
)
|
|
}
|
|
|
|
// The direction a naive "only set what was submitted" implementation
|
|
// gets wrong: an emptied field must remove the stored value, not
|
|
// leave the previous one in place.
|
|
func assertEditClearingRemovesThem(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
webhook, target := seedHTTPTarget(t, env, editAuthHeader, "7")
|
|
|
|
w := submitTargetEdit(
|
|
env, webhook.ID, target.ID,
|
|
editForm(editOriginalURL, "", ""),
|
|
)
|
|
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
|
|
|
cfg := storedHTTPConfig(t, env, target.ID)
|
|
|
|
assert.Empty(t, cfg.Headers)
|
|
assert.Zero(t, cfg.Timeout)
|
|
}
|
|
|
|
// Retries are offered only by the forms for target types that retry.
|
|
// An absent field means the form does not edit retries, not that
|
|
// they should be turned off.
|
|
func assertEditKeepsAbsentMaxRetries(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
webhook, target := seedHTTPTarget(t, env, "", "")
|
|
require.Equal(t, 3, target.MaxRetries)
|
|
|
|
form := editForm(editOriginalURL, "", "")
|
|
form.Del("max_retries")
|
|
|
|
w := submitTargetEdit(env, webhook.ID, target.ID, form)
|
|
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
|
|
|
assert.Equal(
|
|
t, 3, storedTarget(t, env, target.ID).MaxRetries,
|
|
)
|
|
}
|
|
|
|
// TestHandleTargetEdit_PrefillsTheStoredValuesUnmasked covers the
|
|
// deliberate exception to the masking rule. The operator cannot
|
|
// correct a value they cannot see, so this page — and only this page
|
|
// — renders the destination and the header values in full.
|
|
func TestHandleTargetEdit_PrefillsTheStoredValuesUnmasked(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
|
|
webhook, target := seedHTTPTarget(t, env, editAuthHeader, "7")
|
|
|
|
w := serveTarget(
|
|
env, http.MethodGet,
|
|
"/hook/"+webhook.ID+"/targets/"+target.ID+"/edit",
|
|
nil,
|
|
)
|
|
require.Equal(t, http.StatusOK, w.Code)
|
|
|
|
page := w.Body.String()
|
|
|
|
assert.Contains(t, page, editOriginalURL)
|
|
assert.Contains(t, page, "Bearer "+editBearerSecret)
|
|
assert.Contains(t, page, `value="7"`)
|
|
assert.Contains(t, page, "original-name")
|
|
}
|
|
|
|
// TestHandleTargetEditSubmit_Rejects covers every submission that
|
|
// must not reach storage.
|
|
//
|
|
// The SSRF case is the most important assertion on this change: the
|
|
// edited destination goes through the same guard the create path
|
|
// uses. An edit that stored an unvalidated URL would reopen a closed
|
|
// hole, since a target could then be created public and edited to
|
|
// point at loopback.
|
|
//
|
|
// The header and timeout cases keep input that could not be
|
|
// delivered as written out of storage: a stored value that provably
|
|
// never reaches the wire reports a configuration that did not take
|
|
// effect.
|
|
func TestHandleTargetEditSubmit_Rejects(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
|
|
t.Run("an SSRF-blocked destination", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertEditRejectsBlockedDestination(t, env)
|
|
})
|
|
|
|
t.Run("a query-string destination", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertEditIgnoresQueryString(t, env)
|
|
})
|
|
|
|
headerCases := map[string]string{
|
|
"no colon": "Authorization Bearer token",
|
|
"empty name": ": value",
|
|
"invalid name": "X Bad Name: value",
|
|
"reserved header": "User-Agent: curl/8",
|
|
"duplicate name": "X-A: one\nx-a: two",
|
|
}
|
|
|
|
for name, headers := range headerCases {
|
|
t.Run("headers: "+name, func(t *testing.T) {
|
|
t.Parallel()
|
|
assertEditRejectsHeaders(t, env, headers)
|
|
})
|
|
}
|
|
|
|
timeoutCases := map[string]string{
|
|
"not a number": "soon",
|
|
"negative": "-1",
|
|
"over ceiling": "100000",
|
|
}
|
|
|
|
for name, timeout := range timeoutCases {
|
|
t.Run("timeout: "+name, func(t *testing.T) {
|
|
t.Parallel()
|
|
assertEditRejectsTimeout(t, env, timeout)
|
|
})
|
|
}
|
|
}
|
|
|
|
func assertEditRejectsBlockedDestination(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
webhook, target := seedHTTPTarget(t, env, "", "")
|
|
|
|
w := submitTargetEdit(
|
|
env, webhook.ID, target.ID,
|
|
editForm(editBlockedURL, "", ""),
|
|
)
|
|
|
|
assert.Equal(t, http.StatusBadRequest, w.Code)
|
|
assert.Contains(t, w.Body.String(), "Invalid target URL")
|
|
assert.Equal(
|
|
t, editOriginalURL,
|
|
storedHTTPConfig(t, env, target.ID).URL,
|
|
"a rejected edit must leave the stored config alone",
|
|
)
|
|
}
|
|
|
|
// The ingress rule the create path already follows applies to the
|
|
// edit path too: reading a field with FormValue would let the request
|
|
// line carry the credential, and the request line is what logs,
|
|
// proxies and Referer headers record.
|
|
func assertEditIgnoresQueryString(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
webhook, target := seedHTTPTarget(t, env, "", "")
|
|
|
|
form := url.Values{}
|
|
form.Set("name", "edited-name")
|
|
|
|
w := serveTarget(
|
|
env, http.MethodPost,
|
|
"/hook/"+webhook.ID+"/targets/"+target.ID+
|
|
"/edit?url="+url.QueryEscape(editReplacedURL)+
|
|
"&headers="+url.QueryEscape(editAuthHeader),
|
|
form,
|
|
)
|
|
|
|
assert.Equal(t, http.StatusBadRequest, w.Code)
|
|
|
|
cfg := storedHTTPConfig(t, env, target.ID)
|
|
assert.Equal(t, editOriginalURL, cfg.URL)
|
|
assert.Empty(t, cfg.Headers)
|
|
}
|
|
|
|
func assertEditRejectsHeaders(
|
|
t *testing.T, env *sourceTestEnv, headers string,
|
|
) {
|
|
t.Helper()
|
|
|
|
webhook, target := seedHTTPTarget(t, env, "", "")
|
|
|
|
w := submitTargetEdit(
|
|
env, webhook.ID, target.ID,
|
|
editForm(editOriginalURL, headers, ""),
|
|
)
|
|
|
|
assert.Equal(t, http.StatusBadRequest, w.Code)
|
|
assert.Contains(t, w.Body.String(), "Invalid headers")
|
|
assert.Empty(
|
|
t, storedHTTPConfig(t, env, target.ID).Headers,
|
|
"a rejected header must not be stored",
|
|
)
|
|
}
|
|
|
|
func assertEditRejectsTimeout(
|
|
t *testing.T, env *sourceTestEnv, timeout string,
|
|
) {
|
|
t.Helper()
|
|
|
|
webhook, target := seedHTTPTarget(t, env, "", "9")
|
|
|
|
w := submitTargetEdit(
|
|
env, webhook.ID, target.ID,
|
|
editForm(editOriginalURL, "", timeout),
|
|
)
|
|
|
|
assert.Equal(t, http.StatusBadRequest, w.Code)
|
|
assert.Contains(t, w.Body.String(), "Invalid timeout")
|
|
assert.Equal(
|
|
t, 9, storedHTTPConfig(t, env, target.ID).Timeout,
|
|
"a rejected timeout must leave the stored one alone",
|
|
)
|
|
}
|
|
|
|
// TestHandleTargetEdit_Scoping keeps the edit routes scoped the way
|
|
// the delete and toggle routes are: ownership is decided by the
|
|
// webhook, and the target is then scoped to it.
|
|
func TestHandleTargetEdit_Scoping(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
|
|
t.Run("a target of another webhook", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertTargetOfAnotherWebhook404s(t, env)
|
|
})
|
|
|
|
t.Run("a webhook of another user", func(t *testing.T) {
|
|
t.Parallel()
|
|
assertWebhookOfAnotherUser404s(t, env)
|
|
})
|
|
}
|
|
|
|
// A target id from elsewhere must not become editable by pairing it
|
|
// with a webhook the user does own.
|
|
func assertTargetOfAnotherWebhook404s(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
mine := seedWebhookWithRetention(t, env.db, 30)
|
|
_, target := seedHTTPTarget(t, env, "", "")
|
|
|
|
get := serveTarget(
|
|
env, http.MethodGet,
|
|
"/hook/"+mine.ID+"/targets/"+target.ID+"/edit", nil,
|
|
)
|
|
assert.Equal(t, http.StatusNotFound, get.Code)
|
|
|
|
post := submitTargetEdit(
|
|
env, mine.ID, target.ID,
|
|
editForm(editReplacedURL, "", ""),
|
|
)
|
|
assert.Equal(t, http.StatusNotFound, post.Code)
|
|
|
|
assert.Equal(
|
|
t, editOriginalURL,
|
|
storedHTTPConfig(t, env, target.ID).URL,
|
|
)
|
|
}
|
|
|
|
func assertWebhookOfAnotherUser404s(
|
|
t *testing.T, env *sourceTestEnv,
|
|
) {
|
|
t.Helper()
|
|
|
|
other := &database.Webhook{
|
|
UserID: "some-other-user",
|
|
Name: "not mine",
|
|
RetentionDays: 30,
|
|
}
|
|
require.NoError(
|
|
t,
|
|
env.db.DB().Omit(clause.Associations).Create(other).Error,
|
|
)
|
|
|
|
target := seedConfiguredTarget(
|
|
t, env.db, other.ID, database.TargetTypeHTTP,
|
|
`{"url":"`+editOriginalURL+`"}`,
|
|
)
|
|
|
|
w := serveTarget(
|
|
env, http.MethodGet,
|
|
"/hook/"+other.ID+"/targets/"+target.ID+"/edit", nil,
|
|
)
|
|
|
|
assert.Equal(t, http.StatusNotFound, w.Code)
|
|
}
|