All checks were successful
check / check (push) Successful in 2m44s
The 8 KB render cap from #135 left storage untouched but no route served the rest, so a body over the cap was reachable only with filesystem access to the SQLite files — in a product whose purpose is storing webhooks so they can be inspected. GET /source/{sourceID}/logs/{eventID}/body serves the whole body to the webhook's owner, as application/octet-stream with an attachment disposition and nosniff. Those are a security control, not formatting: the bytes come from the public receiver and are handed back inside the operator's authenticated origin, and the existing CSP would not stop a stored HTML payload executing there. The truncation marker links to it only when a body was actually cut. Accepted deviation, documented rather than glossed: #157's definition of done asks the route to stream from the row. It buffers whole instead, because database/sql exposes no incremental handle on a SQLite BLOB and substr range reads re-materialise the entire column per call — an earlier revision chunked at 64 KiB and was 11-15x slower for a worse bound. Three independent reviewers confirmed no streaming path exists. Independently reviewed three times. Two earlier revisions each asserted a memory bound the code did not have; the final reviewer measured 2.057x at the ingest cap and pinned the two overlapping allocations from source — the driver's column buffer and database/sql's convertAssign clone — confirming the stated "roughly two bodies, and 2x is a floor not a ceiling" is now accurate, since SQLite's own materialisation sits outside the Go heap.
575 lines
15 KiB
Go
575 lines
15 KiB
Go
package server_test
|
|
|
|
import (
|
|
"context"
|
|
"html"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"regexp"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"go.uber.org/fx"
|
|
"go.uber.org/fx/fxtest"
|
|
"gorm.io/gorm/clause"
|
|
"sneak.berlin/go/webhooker/internal/config"
|
|
"sneak.berlin/go/webhooker/internal/database"
|
|
"sneak.berlin/go/webhooker/internal/delivery"
|
|
"sneak.berlin/go/webhooker/internal/globals"
|
|
"sneak.berlin/go/webhooker/internal/handlers"
|
|
"sneak.berlin/go/webhooker/internal/healthcheck"
|
|
"sneak.berlin/go/webhooker/internal/logger"
|
|
"sneak.berlin/go/webhooker/internal/middleware"
|
|
"sneak.berlin/go/webhooker/internal/server"
|
|
"sneak.berlin/go/webhooker/internal/session"
|
|
"sneak.berlin/go/webhooker/static"
|
|
)
|
|
|
|
// csrfCookieName is the cookie gorilla/csrf issues when it runs. Its
|
|
// presence or absence on a response is how these tests tell whether
|
|
// the CSRF middleware executed.
|
|
const csrfCookieName = "_gorilla_csrf"
|
|
|
|
type noopNotifier struct{}
|
|
|
|
func (n *noopNotifier) Notify([]delivery.Task) {}
|
|
|
|
// noopEvictor satisfies handlers.New's delivery.WebhookEvictor
|
|
// dependency. These tests never delete a webhook, so there is
|
|
// nothing to record.
|
|
type noopEvictor struct{}
|
|
|
|
func (e *noopEvictor) EvictWebhook(string) {}
|
|
|
|
// testEnv is the real router from routes.go plus the collaborators
|
|
// tests need to seed users and forge sessions.
|
|
type testEnv struct {
|
|
router http.Handler
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
}
|
|
|
|
// newTestEnv wires the dependency graph with fx and builds the
|
|
// production route tree, so middleware registration order is
|
|
// exercised exactly as it ships.
|
|
func newTestEnv(t *testing.T) *testEnv {
|
|
t.Helper()
|
|
|
|
var (
|
|
log *logger.Logger
|
|
cfg *config.Config
|
|
mw *middleware.Middleware
|
|
hnd *handlers.Handlers
|
|
sess *session.Session
|
|
db *database.Database
|
|
dbMgr *database.WebhookDBManager
|
|
)
|
|
|
|
app := fxtest.New(
|
|
t,
|
|
fx.Provide(
|
|
globals.New,
|
|
logger.New,
|
|
func() *config.Config {
|
|
return &config.Config{
|
|
DataDir: t.TempDir(),
|
|
Environment: config.EnvironmentDev,
|
|
}
|
|
},
|
|
database.New,
|
|
database.NewWebhookDBManager,
|
|
healthcheck.New,
|
|
session.New,
|
|
func() delivery.Notifier { return &noopNotifier{} },
|
|
func() delivery.WebhookEvictor { return &noopEvictor{} },
|
|
middleware.New,
|
|
handlers.New,
|
|
),
|
|
fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db, &dbMgr),
|
|
)
|
|
app.RequireStart()
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
return &testEnv{
|
|
router: server.NewRouterForTest(log.Get(), cfg, mw, hnd),
|
|
sess: sess,
|
|
db: db,
|
|
dbMgr: dbMgr,
|
|
}
|
|
}
|
|
|
|
// oversizeValue returns a form value one byte past the route-group
|
|
// body cap, so an encoded form containing it is guaranteed oversize.
|
|
func oversizeValue() string {
|
|
return strings.Repeat("a", int(server.MaxFormBodySizeForTest)+1)
|
|
}
|
|
|
|
// csrfCookieSet reports whether the response issued a gorilla/csrf
|
|
// cookie, which only happens if the CSRF middleware ran.
|
|
func csrfCookieSet(w *httptest.ResponseRecorder) bool {
|
|
for _, c := range w.Result().Cookies() {
|
|
if c.Name == csrfCookieName {
|
|
return true
|
|
}
|
|
}
|
|
|
|
return false
|
|
}
|
|
|
|
// get issues a GET through the router with the supplied cookies.
|
|
func (e *testEnv) get(
|
|
path string,
|
|
cookies []*http.Cookie,
|
|
) *httptest.ResponseRecorder {
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(), http.MethodGet, path, nil,
|
|
)
|
|
|
|
for _, c := range cookies {
|
|
req.AddCookie(c)
|
|
}
|
|
|
|
w := httptest.NewRecorder()
|
|
e.router.ServeHTTP(w, req)
|
|
|
|
return w
|
|
}
|
|
|
|
// post issues a urlencoded form POST through the router. The body is
|
|
// a strings.Reader, so the request carries an accurate
|
|
// Content-Length — the signal MaxBodySize checks up front.
|
|
func (e *testEnv) post(
|
|
path string,
|
|
form url.Values,
|
|
cookies []*http.Cookie,
|
|
) *httptest.ResponseRecorder {
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(), http.MethodPost, path,
|
|
strings.NewReader(form.Encode()),
|
|
)
|
|
req.Header.Set(
|
|
"Content-Type", "application/x-www-form-urlencoded",
|
|
)
|
|
|
|
for _, c := range cookies {
|
|
req.AddCookie(c)
|
|
}
|
|
|
|
w := httptest.NewRecorder()
|
|
e.router.ServeHTTP(w, req)
|
|
|
|
return w
|
|
}
|
|
|
|
// csrfFrom renders the page at path and returns the CSRF token from
|
|
// its form together with every cookie needed for the follow-up POST.
|
|
func (e *testEnv) csrfFrom(
|
|
t *testing.T,
|
|
path string,
|
|
cookies []*http.Cookie,
|
|
) (string, []*http.Cookie) {
|
|
t.Helper()
|
|
|
|
w := e.get(path, cookies)
|
|
require.Equal(t, http.StatusOK, w.Code)
|
|
|
|
pattern := regexp.MustCompile(
|
|
`name="csrf_token" value="([^"]+)"`,
|
|
)
|
|
|
|
match := pattern.FindStringSubmatch(w.Body.String())
|
|
require.Len(t, match, 2, "form must embed a CSRF token")
|
|
|
|
// html/template escapes "+" and "=" in attribute values, and
|
|
// gorilla/csrf tokens are standard base64, so the value read
|
|
// out of the markup has to be unescaped before it is submitted.
|
|
token := html.UnescapeString(match[1])
|
|
|
|
combined := make([]*http.Cookie, 0, len(cookies))
|
|
combined = append(combined, cookies...)
|
|
combined = append(combined, w.Result().Cookies()...)
|
|
|
|
return token, combined
|
|
}
|
|
|
|
// authCookies forges an authenticated session for the given user.
|
|
func (e *testEnv) authCookies(
|
|
t *testing.T,
|
|
userID, username string,
|
|
) []*http.Cookie {
|
|
t.Helper()
|
|
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(), http.MethodGet, "/setup", nil,
|
|
)
|
|
w := httptest.NewRecorder()
|
|
|
|
s, err := e.sess.Get(req)
|
|
require.NoError(t, err)
|
|
|
|
e.sess.SetUser(s, userID, username)
|
|
require.NoError(t, e.sess.Save(req, w, s))
|
|
|
|
cookies := w.Result().Cookies()
|
|
require.NotEmpty(t, cookies, "session cookie should be set")
|
|
|
|
return cookies
|
|
}
|
|
|
|
// seedUser creates a user with the given password and returns the
|
|
// stored hash so tests can assert whether it later changed.
|
|
func (e *testEnv) seedUser(
|
|
t *testing.T,
|
|
username, password string,
|
|
) (string, string) {
|
|
t.Helper()
|
|
|
|
hash, err := database.HashPassword(password)
|
|
require.NoError(t, err)
|
|
|
|
user := &database.User{Username: username, Password: hash}
|
|
require.NoError(t, e.db.DB().Create(user).Error)
|
|
|
|
return user.ID, hash
|
|
}
|
|
|
|
// seedWebhook creates a webhook owned by the given user.
|
|
func (e *testEnv) seedWebhook(
|
|
t *testing.T,
|
|
userID string,
|
|
) *database.Webhook {
|
|
t.Helper()
|
|
|
|
wh := &database.Webhook{UserID: userID, Name: "routed"}
|
|
|
|
require.NoError(
|
|
t,
|
|
e.db.DB().Omit(clause.Associations).Create(wh).Error,
|
|
)
|
|
|
|
return wh
|
|
}
|
|
|
|
// seedEvent records one event with the given body in a webhook's
|
|
// own database.
|
|
func (e *testEnv) seedEvent(
|
|
t *testing.T,
|
|
webhookID, body string,
|
|
) *database.Event {
|
|
t.Helper()
|
|
|
|
webhookDB, err := e.dbMgr.GetDB(webhookID)
|
|
require.NoError(t, err)
|
|
|
|
event := &database.Event{
|
|
WebhookID: webhookID,
|
|
Method: http.MethodPost,
|
|
Body: body,
|
|
ContentType: "application/octet-stream",
|
|
}
|
|
|
|
require.NoError(
|
|
t,
|
|
webhookDB.Omit(clause.Associations).Create(event).Error,
|
|
)
|
|
|
|
return event
|
|
}
|
|
|
|
// storedHash reads the current password hash for a username.
|
|
func (e *testEnv) storedHash(t *testing.T, username string) string {
|
|
t.Helper()
|
|
|
|
var user database.User
|
|
|
|
require.NoError(t,
|
|
e.db.DB().Where("username = ?", username).
|
|
First(&user).Error,
|
|
)
|
|
|
|
return user.Password
|
|
}
|
|
|
|
// --- /s static group ---
|
|
|
|
// TestStaticServesEveryMethod pins what the static mount actually
|
|
// answers. chi's Mount registers the handler for all methods and
|
|
// http.FileServer only special-cases HEAD (by suppressing the body),
|
|
// so a POST or a DELETE to an asset is served the file rather than
|
|
// refused. The README documents this; the test is what keeps the two
|
|
// from drifting.
|
|
func TestStaticServesEveryMethod(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := newTestEnv(t)
|
|
|
|
body, err := static.Static.ReadFile("js/app.js")
|
|
require.NoError(t, err)
|
|
require.NotEmpty(t, body)
|
|
|
|
for _, method := range []string{
|
|
http.MethodGet,
|
|
http.MethodHead,
|
|
http.MethodPost,
|
|
http.MethodPut,
|
|
http.MethodDelete,
|
|
} {
|
|
t.Run(method, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(), method,
|
|
"/s/js/app.js", nil,
|
|
)
|
|
w := httptest.NewRecorder()
|
|
env.router.ServeHTTP(w, req)
|
|
|
|
assert.Equal(t, http.StatusOK, w.Code,
|
|
"static mount answers every method")
|
|
|
|
if method == http.MethodHead {
|
|
assert.Empty(t, w.Body.Bytes(),
|
|
"HEAD must not carry a body")
|
|
|
|
return
|
|
}
|
|
|
|
assert.Equal(t, body, w.Body.Bytes(),
|
|
"the asset itself is returned")
|
|
})
|
|
}
|
|
}
|
|
|
|
// --- /pages group ---
|
|
|
|
// TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs
|
|
// ahead of gorilla/csrf: the response is a clean 413 and no CSRF
|
|
// cookie was issued, so neither the CSRF middleware nor the login
|
|
// handler ran.
|
|
func TestPagesLogin_OversizeBody_RejectedBeforeCSRF(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := newTestEnv(t)
|
|
|
|
form := url.Values{}
|
|
form.Set("username", oversizeValue())
|
|
form.Set("password", "irrelevant")
|
|
|
|
w := env.post("/pages/login", form, nil)
|
|
|
|
assert.Equal(
|
|
t, http.StatusRequestEntityTooLarge, w.Code,
|
|
)
|
|
assert.False(
|
|
t, csrfCookieSet(w),
|
|
"CSRF middleware must not run for an oversized body",
|
|
)
|
|
}
|
|
|
|
// TestPagesLogin_UnderLimit_NoToken_CSRFRejects is the control for
|
|
// the test above: an identically shaped but under-limit POST does
|
|
// reach gorilla/csrf, which rejects it and issues its cookie. Without
|
|
// this, the missing-cookie assertion above would prove nothing.
|
|
func TestPagesLogin_UnderLimit_NoToken_CSRFRejects(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := newTestEnv(t)
|
|
|
|
form := url.Values{}
|
|
form.Set("username", "someone")
|
|
form.Set("password", "irrelevant")
|
|
|
|
w := env.post("/pages/login", form, nil)
|
|
|
|
assert.Equal(t, http.StatusForbidden, w.Code)
|
|
assert.True(
|
|
t, csrfCookieSet(w),
|
|
"CSRF middleware should run for an under-limit body",
|
|
)
|
|
}
|
|
|
|
// TestPagesLogin_UnderLimit_ValidToken_ReachesHandler proves the
|
|
// reorder did not break CSRF token handling: a token harvested from
|
|
// the rendered login form is still accepted and the request lands in
|
|
// the handler.
|
|
func TestPagesLogin_UnderLimit_ValidToken_ReachesHandler(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
env := newTestEnv(t)
|
|
|
|
token, cookies := env.csrfFrom(t, "/pages/login", nil)
|
|
|
|
form := url.Values{}
|
|
form.Set("csrf_token", token)
|
|
form.Set("username", "nosuchuser")
|
|
form.Set("password", "wrongpassword")
|
|
|
|
w := env.post("/pages/login", form, cookies)
|
|
|
|
assert.Equal(t, http.StatusUnauthorized, w.Code)
|
|
assert.Contains(
|
|
t, w.Body.String(), "Invalid username or password",
|
|
"request should reach the login handler",
|
|
)
|
|
}
|
|
|
|
// --- /user/{username} group ---
|
|
|
|
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
|
|
// covers the route that previously had no middleware body cap at
|
|
// all. The request carries a valid session and a valid CSRF token,
|
|
// so the only thing that can stop it is the size cap; the unchanged
|
|
// password hash is the observable proof the handler never ran.
|
|
func TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
env := newTestEnv(t)
|
|
|
|
userID, originalHash := env.seedUser(t, "pwuser", "oldpassword")
|
|
cookies := env.authCookies(t, userID, "pwuser")
|
|
token, cookies := env.csrfFrom(t, "/user/pwuser/", cookies)
|
|
|
|
form := url.Values{}
|
|
form.Set("csrf_token", token)
|
|
form.Set("current_password", "oldpassword")
|
|
form.Set("new_password", oversizeValue())
|
|
form.Set("confirm_password", oversizeValue())
|
|
|
|
w := env.post("/user/pwuser/password", form, cookies)
|
|
|
|
assert.Equal(
|
|
t, http.StatusRequestEntityTooLarge, w.Code,
|
|
)
|
|
assert.Equal(
|
|
t, originalHash, env.storedHash(t, "pwuser"),
|
|
"handler must not run, so the password must be unchanged",
|
|
)
|
|
}
|
|
|
|
// TestPasswordChange_UnderLimit_Succeeds proves that adding the cap
|
|
// to the /user/{username} group did not break the route it guards.
|
|
func TestPasswordChange_UnderLimit_Succeeds(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := newTestEnv(t)
|
|
|
|
userID, originalHash := env.seedUser(t, "okuser", "oldpassword")
|
|
cookies := env.authCookies(t, userID, "okuser")
|
|
token, cookies := env.csrfFrom(t, "/user/okuser/", cookies)
|
|
|
|
form := url.Values{}
|
|
form.Set("csrf_token", token)
|
|
form.Set("current_password", "oldpassword")
|
|
form.Set("new_password", "brandnewpassword")
|
|
form.Set("confirm_password", "brandnewpassword")
|
|
|
|
w := env.post("/user/okuser/password", form, cookies)
|
|
|
|
assert.Equal(t, http.StatusOK, w.Code)
|
|
assert.NotEqual(
|
|
t, originalHash, env.storedHash(t, "okuser"),
|
|
"an under-limit password change should still apply",
|
|
)
|
|
}
|
|
|
|
// --- /source/{sourceID} group ---
|
|
|
|
// TestSourceLogs_TruncationLinkDownloadsTheBody walks the whole
|
|
// feature the way a user does: render the event log page through
|
|
// the production router, take the download URL out of the markup
|
|
// the template emitted, and fetch that URL through the router
|
|
// again. Nothing here is hand-written, so a typo in either the
|
|
// route pattern or the template href fails this test — the
|
|
// handler-level tests cannot catch that, because they forge
|
|
// their own route context and assert a URL string they wrote
|
|
// themselves.
|
|
func TestSourceLogs_TruncationLinkDownloadsTheBody(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := newTestEnv(t)
|
|
|
|
userID, _ := env.seedUser(t, "loguser", "somepassword")
|
|
cookies := env.authCookies(t, userID, "loguser")
|
|
|
|
// Comfortably over the event log page's render cap, so the
|
|
// page truncates the body and renders the download link at
|
|
// all. The exact cap is the handlers package's business and
|
|
// is pinned by its own tests; this only needs to exceed it.
|
|
stored := strings.Repeat("Z", 64*1024)
|
|
|
|
wh := env.seedWebhook(t, userID)
|
|
env.seedEvent(t, wh.ID, stored)
|
|
|
|
page := env.get("/source/"+wh.ID+"/logs", cookies)
|
|
require.Equal(t, http.StatusOK, page.Code)
|
|
|
|
link := regexp.MustCompile(
|
|
`href="(/source/[^"]+/body)"`,
|
|
).FindStringSubmatch(page.Body.String())
|
|
require.Len(
|
|
t, link, 2,
|
|
"truncated body should render a download link",
|
|
)
|
|
|
|
w := env.get(html.UnescapeString(link[1]), cookies)
|
|
|
|
require.Equal(
|
|
t, http.StatusOK, w.Code,
|
|
"the link the page emits must be a live route",
|
|
)
|
|
assert.Equal(t, stored, w.Body.String())
|
|
assert.Equal(
|
|
t, strconv.Itoa(len(stored)),
|
|
w.Header().Get("Content-Length"),
|
|
)
|
|
assert.Equal(
|
|
t, "application/octet-stream",
|
|
w.Header().Get("Content-Type"),
|
|
)
|
|
assert.Contains(
|
|
t, w.Header().Get("Content-Disposition"), "attachment",
|
|
)
|
|
assert.Equal(
|
|
t, "nosniff", w.Header().Get("X-Content-Type-Options"),
|
|
)
|
|
}
|
|
|
|
// TestSourceLogsBody_OtherUser404s pins that the download route
|
|
// as registered is behind the auth the group provides and the
|
|
// ownership check the handler applies: another logged-in user
|
|
// asking the real router for the same URL gets a 404, and an
|
|
// unauthenticated request never reaches the handler at all.
|
|
func TestSourceLogsBody_OtherUser404s(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := newTestEnv(t)
|
|
|
|
ownerID, _ := env.seedUser(t, "owner", "somepassword")
|
|
wh := env.seedWebhook(t, ownerID)
|
|
|
|
const payload = "OWNERS-PAYLOAD-77c1"
|
|
|
|
evt := env.seedEvent(t, wh.ID, payload)
|
|
path := "/source/" + wh.ID + "/logs/" + evt.ID + "/body"
|
|
|
|
intruderID, _ := env.seedUser(t, "intruder", "somepassword")
|
|
intruder := env.authCookies(t, intruderID, "intruder")
|
|
|
|
w := env.get(path, intruder)
|
|
assert.Equal(t, http.StatusNotFound, w.Code)
|
|
assert.NotContains(t, w.Body.String(), payload)
|
|
|
|
anon := env.get(path, nil)
|
|
assert.Equal(t, http.StatusSeeOther, anon.Code)
|
|
assert.Equal(t, "/pages/login", anon.Header().Get("Location"))
|
|
}
|