Files
webhooker/internal/handlers/target_edit_test.go
T
clawbot 61802bfcf4
check / check (push) Waiting to run
Add a Download button that exports a database target's archive as gzipped JSON (closes #374)
Each database target on the webhook page links to
/hook/ID/targets/TARGETID/download, which streams the target's archive
as archive-WEBHOOK-TARGET-YYYYMMDDTHHMMSSZ.json.gz: the webhook, the
target, exported_at, and archived_events, one object per row keyed by
column, a body that is not valid UTF-8 in base64 with body_encoding.
The export reads through one cursor inside a read-only transaction:
one snapshot, no write lock. A download runs for as long as the client
keeps reading, and one that fails after it has started aborts the
connection. The request limit answers 504 only when the handler has
not started its response, so a long download is logged as its 200.

Model: opus-5-5
2026-10-02 16:54:15 +00:00

739 lines
19 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, edit and download 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}/download",
env.handlers.HandleTargetDownload(),
)
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)
}
// renamedTargetName is the name the rename tests give a target.
const renamedTargetName = "Long Term"
// TestHandleTargetEditSubmit_RenamesArchive proves that renaming a
// database target renames its archive, that a save that keeps the
// name renames nothing, that a target of another type has no archive
// to rename, and that a target whose archive cannot be renamed keeps
// its stored name. When a file already has the archive's new name,
// the edit is refused with an error naming that file.
func TestHandleTargetEditSubmit_RenamesArchive(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhookWithRetention(t, env.db, 7)
archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
rename := url.Values{"name": {renamedTargetName}}
w := submitTargetEdit(env, wh.ID, archive.ID, rename)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
assert.Equal(
t,
[]archiveRename{{archive.ID, wh.Name, renamedTargetName}},
env.archives.Renames(),
)
w = submitTargetEdit(env, wh.ID, archive.ID, rename)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
assert.Len(
t, env.archives.Renames(), 1,
"a save that keeps the name renames nothing",
)
httpWebhook, httpTarget := seedHTTPTarget(t, env, "", "")
w = submitTargetEdit(
env, httpWebhook.ID, httpTarget.ID,
editForm(editOriginalURL, "", ""),
)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
assert.Len(
t, env.archives.Renames(), 1,
"an HTTP target has no archive to rename",
)
again := url.Values{"name": {"Again"}}
env.archives.FailRenames(archive.ID, errInjectedRename)
w = submitTargetEdit(env, wh.ID, archive.ID, again)
require.Equal(t, http.StatusInternalServerError, w.Code)
assert.Equal(
t, renamedTargetName, storedTarget(t, env, archive.ID).Name,
"a target whose archive was not renamed keeps its name",
)
env.archives.FailRenames(archive.ID, errNameTaken)
w = submitTargetEdit(env, wh.ID, archive.ID, again)
require.Equal(t, http.StatusConflict, w.Code)
assert.Contains(t, w.Body.String(), "archive-taken.db")
assert.Equal(
t, renamedTargetName, storedTarget(t, env, archive.ID).Name,
)
}
// TestHandleTargetEditSubmit_FailedSaveRenamesBack proves that when a
// database target's archive is renamed but the new name cannot be
// saved, the archive is renamed back to the stored name and the
// stored name stays.
func TestHandleTargetEditSubmit_FailedSaveRenamesBack(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhookWithRetention(t, env.db, 7)
archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
failSaveOnTable(t, env.db, "targets")
form := url.Values{}
form.Set("name", renamedTargetName)
w := submitTargetEdit(env, wh.ID, archive.ID, form)
require.Equal(t, http.StatusInternalServerError, w.Code)
assert.Equal(
t, archive.Name, storedTarget(t, env, archive.ID).Name,
)
assert.Equal(
t,
[]archiveRename{
{archive.ID, wh.Name, renamedTargetName},
{archive.ID, wh.Name, archive.Name},
},
env.archives.Renames(),
)
}