6f37d05ab6
All checks were successful
check / check (push) Successful in 3m23s
Capping the event log page at 8 KB of body per event left no
in-app way to see a larger one: storage keeps it, but no route
served it, so a payload over the cap was reachable only by an
operator with filesystem access. GitHub pull_request and
multi-commit push payloads, expanded Stripe events and Shopify
orders all routinely clear 8 KB, which is exactly when the tool
is supposed to be useful.
GET /source/{sourceID}/logs/{eventID}/body now serves one whole
body, and the truncation marker links to it when — and only
when — a body was actually cut.
The response is deliberately inert. Its bytes are chosen by
whoever can reach the public receiver and it hands them back
inside the operator's own authenticated origin, so it goes out
as application/octet-stream with Content-Disposition: attachment
and nosniff, and the filename is built from a parsed uuid rather
than from anything in the request. The application CSP is no
help on this path: script-src allows 'unsafe-inline' from
'self', so a document served from this origin could run its own
script.
The body is read in one query and held whole while it is
written. There is no cheaper bound to take. database/sql
exposes no incremental handle on a SQLite blob, and reading
byte ranges with substr does not avoid the cost either: SQLite
materialises the entire column value to evaluate each substr
call, so range reads pay for the whole body once per range
rather than once per download. Measured over a 1 MiB body,
64 KiB ranges cost 11-15x a single read to move the same bytes.
The bound is therefore the one the issue asks for: the route is
owner-authenticated and ingest is capped at 1 MB, so peak is
one body per concurrent download. Nothing goes through
renderTemplate, which buffers a whole response before writing
it.
Reading the body before the first header is written also means
an event reaped mid-request cannot produce a torn response: it
is either served whole or 404s cleanly, and both are tested.
The ownership check the log page applies is extracted as
ownedWebhook and shared with the download, so the two cannot
drift apart. A webhook owned by someone else and one that does
not exist are the same 404.
The route registration and the link the template emits are
covered end to end through the production router, so a typo in
either fails the suite rather than leaving the feature dead
behind green handler tests.
507 lines
13 KiB
Go
507 lines
13 KiB
Go
package handlers_test
|
|
|
|
import (
|
|
"context"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/go-chi/chi"
|
|
"github.com/google/uuid"
|
|
"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/handlers"
|
|
"sneak.berlin/go/webhooker/internal/session"
|
|
)
|
|
|
|
// paramEventID is the chi URL parameter the body download
|
|
// handler reads.
|
|
const paramEventID = "eventID"
|
|
|
|
// otherTestUserID owns webhooks the session user must not be
|
|
// able to read.
|
|
const otherTestUserID = "other-user-id"
|
|
|
|
// seedWebhookFor inserts a webhook owned by the given user.
|
|
func seedWebhookFor(
|
|
t *testing.T,
|
|
db *database.Database,
|
|
userID string,
|
|
) *database.Webhook {
|
|
t.Helper()
|
|
|
|
wh := &database.Webhook{
|
|
UserID: userID,
|
|
Name: "wh-" + userID,
|
|
}
|
|
|
|
require.NoError(
|
|
t,
|
|
db.DB().Omit(clause.Associations).Create(wh).Error,
|
|
)
|
|
|
|
return wh
|
|
}
|
|
|
|
// fetchEventBody runs the real download handler as the test user
|
|
// for the given source and event ids.
|
|
func fetchEventBody(
|
|
t *testing.T,
|
|
h *handlers.Handlers,
|
|
sess *session.Session,
|
|
sourceID, eventID string,
|
|
) *httptest.ResponseRecorder {
|
|
t.Helper()
|
|
|
|
// The path is escaped and the raw id goes in the route
|
|
// context, which is what chi hands a handler: the param is
|
|
// already percent-decoded by the time it is read.
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(),
|
|
http.MethodGet,
|
|
"/source/"+url.PathEscape(sourceID)+
|
|
"/logs/"+url.PathEscape(eventID)+"/body",
|
|
nil,
|
|
)
|
|
|
|
for _, c := range authenticatedCookies(
|
|
t, sess, deleteTestUserID, deleteTestUsername,
|
|
) {
|
|
req.AddCookie(c)
|
|
}
|
|
|
|
rctx := chi.NewRouteContext()
|
|
rctx.URLParams.Add(paramSourceID, sourceID)
|
|
rctx.URLParams.Add(paramEventID, eventID)
|
|
|
|
req = req.WithContext(
|
|
context.WithValue(
|
|
req.Context(), chi.RouteCtxKey, rctx,
|
|
),
|
|
)
|
|
|
|
w := httptest.NewRecorder()
|
|
h.HandleEventBodyDownload().ServeHTTP(w, req)
|
|
|
|
return w
|
|
}
|
|
|
|
// TestHandleEventBodyDownload_ServesOversizeBodyInFull is the
|
|
// capability the render cap took away: a body far above what the
|
|
// event log page will show comes back whole and byte-identical,
|
|
// with the headers that keep it from being rendered.
|
|
func TestHandleEventBodyDownload_ServesOversizeBodyInFull(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
var (
|
|
h *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
// Far above the render cap, with multibyte runes and a
|
|
// distinctive tail, so a body that the log page can only
|
|
// show a slice of comes back whole and in order.
|
|
const sentinel = "TAIL-SENTINEL-1f4a9c"
|
|
|
|
stored := strings.Repeat("A", 200*1024) +
|
|
strings.Repeat(snowman, 1000) + sentinel
|
|
|
|
wh := seedWebhook(t, db)
|
|
evt := seedEventWithBody(t, dbMgr, wh.ID, stored)
|
|
|
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
|
|
|
require.Equal(t, http.StatusOK, w.Code)
|
|
assert.Greater(t, len(stored), bodyCap)
|
|
assert.Equal(t, stored, w.Body.String())
|
|
assert.Equal(
|
|
t, strconv.Itoa(len(stored)),
|
|
w.Header().Get("Content-Length"),
|
|
)
|
|
}
|
|
|
|
// TestHandleEventBodyDownload_BodiesRoundTripByteIdentical
|
|
// covers the sizes and byte values a stored body can actually
|
|
// take: empty, one byte, either side of the render cap, and
|
|
// bytes that are not text at all. Content-Length has to equal
|
|
// the bytes written in every case, since it is derived from the
|
|
// same read that produces them.
|
|
func TestHandleEventBodyDownload_BodiesRoundTripByteIdentical(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
// A NUL, invalid UTF-8 and a multibyte rune, so nothing on
|
|
// the path can be treating the body as text.
|
|
binary := "\x00\x01\xff\xfe" + snowman + "\x00tail"
|
|
|
|
cases := map[string]string{
|
|
"empty": "",
|
|
"single byte": "x",
|
|
"one below cap": strings.Repeat("b", bodyCap-1),
|
|
"exactly cap": strings.Repeat("c", bodyCap),
|
|
"one above cap": strings.Repeat("d", bodyCap+1),
|
|
"binary": binary,
|
|
}
|
|
|
|
for name, stored := range cases {
|
|
t.Run(name, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
var (
|
|
h *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
wh := seedWebhook(t, db)
|
|
evt := seedEventWithBody(t, dbMgr, wh.ID, stored)
|
|
|
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
|
|
|
require.Equal(t, http.StatusOK, w.Code)
|
|
assert.Equal(t, stored, w.Body.String())
|
|
assert.Equal(
|
|
t, strconv.Itoa(len(stored)),
|
|
w.Header().Get("Content-Length"),
|
|
)
|
|
assert.Equal(
|
|
t, len(stored), w.Body.Len(),
|
|
"Content-Length must equal bytes written",
|
|
)
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestHandleEventBodyDownload_HeadersAreNotRenderable pins the
|
|
// response headers that stop attacker-supplied bytes executing
|
|
// in the operator's own origin. They are a security control, not
|
|
// presentation.
|
|
func TestHandleEventBodyDownload_HeadersAreNotRenderable(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
var (
|
|
h *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
wh := seedWebhook(t, db)
|
|
evt := seedEventWithBody(t, dbMgr, wh.ID, `{"small":true}`)
|
|
|
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
|
|
|
require.Equal(t, http.StatusOK, w.Code)
|
|
assert.Equal(
|
|
t, "application/octet-stream",
|
|
w.Header().Get("Content-Type"),
|
|
)
|
|
assert.Equal(
|
|
t, "nosniff",
|
|
w.Header().Get("X-Content-Type-Options"),
|
|
)
|
|
|
|
disposition := w.Header().Get("Content-Disposition")
|
|
assert.Equal(
|
|
t,
|
|
`attachment; filename="webhooker-event-`+evt.ID+`.bin"`,
|
|
disposition,
|
|
)
|
|
}
|
|
|
|
// TestHandleEventBodyDownload_ScriptBodyStaysInert proves a
|
|
// stored HTML payload is handed back as an attachment of opaque
|
|
// bytes rather than as anything a browser will execute. The
|
|
// bytes themselves are unaltered: this route reports what was
|
|
// delivered.
|
|
func TestHandleEventBodyDownload_ScriptBodyStaysInert(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
var (
|
|
h *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
const payload = `<html><script>alert(document.cookie)` +
|
|
`</script></html>`
|
|
|
|
wh := seedWebhook(t, db)
|
|
evt := seedEventWithBody(t, dbMgr, wh.ID, payload)
|
|
|
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
|
|
|
require.Equal(t, http.StatusOK, w.Code)
|
|
assert.Equal(t, payload, w.Body.String())
|
|
|
|
contentType := w.Header().Get("Content-Type")
|
|
assert.Equal(t, "application/octet-stream", contentType)
|
|
assert.NotContains(t, contentType, "html")
|
|
assert.NotContains(t, contentType, "xml")
|
|
assert.NotContains(t, contentType, "javascript")
|
|
assert.Contains(
|
|
t, w.Header().Get("Content-Disposition"), "attachment",
|
|
)
|
|
assert.Equal(
|
|
t, "nosniff",
|
|
w.Header().Get("X-Content-Type-Options"),
|
|
)
|
|
}
|
|
|
|
// TestHandleEventBodyDownload_OtherUsersEvent404s is the
|
|
// authorization test the definition of done asks for: an event
|
|
// stored under a webhook the session user does not own is not
|
|
// readable, and the miss does not distinguish itself from a
|
|
// nonexistent one.
|
|
func TestHandleEventBodyDownload_OtherUsersEvent404s(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
var (
|
|
h *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
const theirPayload = "OTHER-USERS-PAYLOAD-8b1d"
|
|
|
|
theirs := seedWebhookFor(t, db, otherTestUserID)
|
|
evt := seedEventWithBody(t, dbMgr, theirs.ID, theirPayload)
|
|
|
|
w := fetchEventBody(t, h, sess, theirs.ID, evt.ID)
|
|
|
|
assert.Equal(t, http.StatusNotFound, w.Code)
|
|
assert.NotContains(t, w.Body.String(), theirPayload)
|
|
}
|
|
|
|
// TestHandleEventBodyDownload_EventOfAnotherWebhook404s pins
|
|
// that holding a valid event id is not enough: the event has to
|
|
// belong to the webhook in the path. Both webhooks here are the
|
|
// session user's and both have event databases, so the
|
|
// ownership check cannot be what produces the 404.
|
|
//
|
|
// What does produce it is the per-webhook database file rather
|
|
// than the webhook_id predicate on the query — removing that
|
|
// predicate leaves this test green, because the sibling's event
|
|
// is in a different file. The test is kept as the behavioural
|
|
// guard the route owes; see serveEventBody for which mechanism
|
|
// is load-bearing.
|
|
func TestHandleEventBodyDownload_EventOfAnotherWebhook404s(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
var (
|
|
h *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
const other = "BELONGS-TO-THE-OTHER-WEBHOOK-3c7e"
|
|
|
|
mine := seedWebhook(t, db)
|
|
seedEventWithBody(t, dbMgr, mine.ID, `{"mine":true}`)
|
|
|
|
sibling := seedWebhook(t, db)
|
|
evt := seedEventWithBody(t, dbMgr, sibling.ID, other)
|
|
|
|
w := fetchEventBody(t, h, sess, mine.ID, evt.ID)
|
|
|
|
assert.Equal(t, http.StatusNotFound, w.Code)
|
|
assert.NotContains(t, w.Body.String(), other)
|
|
}
|
|
|
|
// TestHandleEventBodyDownload_UnknownEvent404s covers the plain
|
|
// miss, including an id that is not a uuid at all and so never
|
|
// reaches the query or the response header.
|
|
func TestHandleEventBodyDownload_UnknownEvent404s(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
var (
|
|
h *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
wh := seedWebhook(t, db)
|
|
seedEventWithBody(t, dbMgr, wh.ID, `{"mine":true}`)
|
|
|
|
for _, id := range []string{
|
|
uuid.New().String(),
|
|
`../../etc/passwd`,
|
|
"not-a-uuid",
|
|
`x"; rm -rf /`,
|
|
} {
|
|
w := fetchEventBody(t, h, sess, wh.ID, id)
|
|
|
|
assert.Equal(
|
|
t, http.StatusNotFound, w.Code,
|
|
"event id %q", id,
|
|
)
|
|
assert.Empty(
|
|
t, w.Header().Get("Content-Disposition"),
|
|
"event id %q must not reach a header", id,
|
|
)
|
|
}
|
|
}
|
|
|
|
// TestHandleEventBodyDownload_ReapedEvent404s pins what happens
|
|
// when the retention reaper takes an event out from under this
|
|
// route. The body is read in one query before any header is
|
|
// written, so a reaped event cannot produce a partial download:
|
|
// it is a clean 404 with no Content-Length and no
|
|
// Content-Disposition. Both removals the codebase performs are
|
|
// covered — the reaper hard-deletes, and a soft-deleted row is
|
|
// excluded by the query's own deleted_at predicate rather than
|
|
// by GORM's default scope, which Raw bypasses.
|
|
func TestHandleEventBodyDownload_ReapedEvent404s(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
for name, hard := range map[string]bool{
|
|
"soft deleted": false,
|
|
"hard deleted": true,
|
|
} {
|
|
t.Run(name, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
var (
|
|
h *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
const payload = "REAPED-PAYLOAD-4d2a"
|
|
|
|
wh := seedWebhook(t, db)
|
|
evt := seedEventWithBody(t, dbMgr, wh.ID, payload)
|
|
|
|
webhookDB, err := dbMgr.GetDB(wh.ID)
|
|
require.NoError(t, err)
|
|
|
|
del := webhookDB
|
|
if hard {
|
|
del = del.Unscoped()
|
|
}
|
|
|
|
require.NoError(
|
|
t,
|
|
del.Delete(&database.Event{}, "id = ?", evt.ID).
|
|
Error,
|
|
)
|
|
|
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
|
|
|
assert.Equal(t, http.StatusNotFound, w.Code)
|
|
assert.NotContains(t, w.Body.String(), payload)
|
|
assert.Empty(t, w.Header().Get("Content-Length"))
|
|
assert.Empty(
|
|
t, w.Header().Get("Content-Disposition"),
|
|
)
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestHandleSourceLogs_TruncationMarkerLinksToDownload proves
|
|
// the page tells the reader where the rest of the body is, and
|
|
// only when there is a rest to fetch.
|
|
func TestHandleSourceLogs_TruncationMarkerLinksToDownload(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
var (
|
|
h *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
big := seedWebhook(t, db)
|
|
bigEvt := seedEventWithBody(
|
|
t, dbMgr, big.ID, strings.Repeat("A", 4*bodyCap),
|
|
)
|
|
|
|
page := renderSourceLogsPage(t, h, sess, big.ID)
|
|
assert.Contains(
|
|
t, page,
|
|
"/source/"+big.ID+"/logs/"+bigEvt.ID+"/body",
|
|
)
|
|
|
|
small := seedWebhook(t, db)
|
|
smallEvt := seedEventWithBody(
|
|
t, dbMgr, small.ID, `{"kept":"whole"}`,
|
|
)
|
|
|
|
page = renderSourceLogsPage(t, h, sess, small.ID)
|
|
assert.NotContains(
|
|
t, page,
|
|
"/source/"+small.ID+"/logs/"+smallEvt.ID+"/body",
|
|
)
|
|
}
|