Store and show an event's query string, and pass it on to an HTTP target when set (closes #312)
check / check (push) Successful in 4m45s
check / check (push) Successful in 4m45s
The receiver dropped the query string of every request it received, so a sender's URL parameters were silently lost. Each event now keeps it, as sent, in a new `raw_query` column of the per-webhook `events` table; a resubmitted copy carries its original's. The event log and the event's page show it in the shared request block, the event log leaving out one over 32 KiB with a link, as for headers. The archive and log targets carry it. HTTP targets gain "Pass the query string on to this target", off by default: on, deliveries, replays and resubmits append it to the target URL, joined with `&` to one already there. The access log still hides it. Model: opus-5-5
This commit was merged in pull request #501.
This commit is contained in:
@@ -310,6 +310,7 @@ func createReplayDelivery(
|
||||
TargetConfig: target.Config,
|
||||
MaxRetries: target.MaxRetries,
|
||||
Method: event.Method,
|
||||
RawQuery: event.RawQuery,
|
||||
Headers: event.Headers,
|
||||
ContentType: event.ContentType,
|
||||
Body: replayBody(event.Body),
|
||||
|
||||
@@ -26,6 +26,9 @@ const paramDeliveryID = "deliveryID"
|
||||
// dispatches to it: the notifier is recorded, not run.
|
||||
const replayTargetURL = "http://93.184.216.34/hook"
|
||||
|
||||
// replayEventQuery is the query string a seeded event arrived with.
|
||||
const replayEventQuery = "a=1&b=2"
|
||||
|
||||
// seedFailedDelivery records an event, a terminally failed delivery of
|
||||
// it to the given target, and the attempt that failed.
|
||||
func seedFailedDelivery(
|
||||
@@ -42,6 +45,7 @@ func seedFailedDelivery(
|
||||
WebhookID: webhookID,
|
||||
EntrypointID: "entrypoint-" + webhookID,
|
||||
Method: http.MethodPost,
|
||||
RawQuery: replayEventQuery,
|
||||
Headers: `{"X-Test":["yes"]}`,
|
||||
Body: `{"replay":"me"}`,
|
||||
ContentType: contentTypeJSON,
|
||||
@@ -296,6 +300,10 @@ func assertReplayTask(
|
||||
"replay must use the target's current configuration",
|
||||
)
|
||||
assert.Equal(t, event.Method, task.Method)
|
||||
assert.Equal(
|
||||
t, replayEventQuery, task.RawQuery,
|
||||
"replay re-sends the stored query string",
|
||||
)
|
||||
assert.Equal(t, event.Headers, task.Headers)
|
||||
assert.Equal(t, event.ContentType, task.ContentType)
|
||||
assert.Equal(t, 1, task.AttemptNum)
|
||||
|
||||
@@ -324,7 +324,8 @@ func loadEventLogRows(
|
||||
var rows []eventLogRow
|
||||
|
||||
err = eventsWithStatus(webhookDB, webhookID, statuses).Select(
|
||||
eventLogColumns, maxRenderedBodyBytes, maxRenderedBodyBytes,
|
||||
eventLogColumns,
|
||||
maxRenderedBodyBytes, maxRenderedBodyBytes, maxRenderedBodyBytes,
|
||||
).Order("created_at DESC").Limit(recentEventLimit).Find(&rows).Error
|
||||
|
||||
return rows, totalEvents, err
|
||||
|
||||
@@ -17,19 +17,23 @@ import (
|
||||
// bytes rather than characters, so the cap bounds the page in
|
||||
// bytes whatever the payload's encoding. Cutting in SQLite
|
||||
// rather than in Go is the point of the projection — an
|
||||
// oversized body or set of request headers never becomes a Go
|
||||
// string at all.
|
||||
// oversized body, query string or set of request headers never
|
||||
// becomes a Go string at all.
|
||||
const eventLogColumns = "id, created_at, method, content_type, " +
|
||||
"resubmitted_from_id, entrypoint_id, " +
|
||||
"substr(cast(raw_query as blob), 1, ?) AS raw_query, " +
|
||||
"length(cast(raw_query as blob)) AS raw_query_bytes, " +
|
||||
"substr(cast(headers as blob), 1, ?) AS headers, " +
|
||||
"length(cast(headers as blob)) AS headers_bytes, " +
|
||||
"substr(cast(body as blob), 1, ?) AS body, " +
|
||||
"length(cast(body as blob)) AS body_bytes"
|
||||
|
||||
// eventColumns is eventLogColumns for the event's own page, which
|
||||
// shows the whole body and every request header.
|
||||
// shows the whole body, the whole query string and every request
|
||||
// header.
|
||||
const eventColumns = "id, created_at, method, content_type, " +
|
||||
"resubmitted_from_id, entrypoint_id, headers, " +
|
||||
"resubmitted_from_id, entrypoint_id, raw_query, " +
|
||||
"length(cast(raw_query as blob)) AS raw_query_bytes, headers, " +
|
||||
"length(cast(headers as blob)) AS headers_bytes, " +
|
||||
"cast(body as blob) AS body, " +
|
||||
"length(cast(body as blob)) AS body_bytes"
|
||||
@@ -57,6 +61,13 @@ type EventLogView struct {
|
||||
// entrypoint's secret.
|
||||
Entrypoint string
|
||||
|
||||
// RawQuery is the query string the event arrived with.
|
||||
// RawQueryCut reports one left out, RawQuery then empty, because
|
||||
// it holds more than maxRenderedBodyBytes; only the event log
|
||||
// leaves it out.
|
||||
RawQuery string
|
||||
RawQueryCut bool
|
||||
|
||||
// Headers is the event's request headers as text, one
|
||||
// "Name: value" line per value, sorted by name. HeadersCut
|
||||
// reports headers left out because they hold more than
|
||||
@@ -85,9 +96,9 @@ func (v EventLogView) ResubmittedFrom() bool {
|
||||
}
|
||||
|
||||
// eventLogRow is one row of the event log projection, or of
|
||||
// eventColumns. In the event log its headers and body columns
|
||||
// arrive already cut to the cap by SQLite, each with its true
|
||||
// size beside it.
|
||||
// eventColumns. In the event log its query string, headers and
|
||||
// body columns arrive already cut to the cap by SQLite, each with
|
||||
// its true size beside it.
|
||||
type eventLogRow struct {
|
||||
ID string
|
||||
CreatedAt time.Time
|
||||
@@ -95,6 +106,8 @@ type eventLogRow struct {
|
||||
ContentType string
|
||||
ResubmittedFromID *string
|
||||
EntrypointID string
|
||||
RawQuery string
|
||||
RawQueryBytes int64
|
||||
Headers string
|
||||
HeadersBytes int64
|
||||
Body []byte
|
||||
@@ -114,6 +127,13 @@ func (r *eventLogRow) view(
|
||||
|
||||
headers, fit := requestHeaderLines(r.Headers, maxHeaderBytes)
|
||||
|
||||
rawQuery := r.RawQuery
|
||||
rawQueryCut := r.RawQueryBytes > int64(len(rawQuery))
|
||||
|
||||
if rawQueryCut {
|
||||
rawQuery = ""
|
||||
}
|
||||
|
||||
return EventLogView{
|
||||
ID: r.ID,
|
||||
Method: r.Method,
|
||||
@@ -123,6 +143,8 @@ func (r *eventLogRow) view(
|
||||
Body: newBodyView(
|
||||
"/hook/"+webhookID+"/events/"+r.ID, r.Body, r.BodyBytes,
|
||||
),
|
||||
RawQuery: rawQuery,
|
||||
RawQueryCut: rawQueryCut,
|
||||
Headers: strings.Join(headers, "\n"),
|
||||
HeadersCut: !fit || r.HeadersBytes > int64(len(r.Headers)),
|
||||
ResubmittedFromID: from,
|
||||
|
||||
@@ -1,13 +1,16 @@
|
||||
package handlers_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi"
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
@@ -116,6 +119,7 @@ func TestEventRequest_EachEventShowsItsOwnEntrypointAndHeaders(
|
||||
t.Helper()
|
||||
|
||||
assert.Contains(t, page, arrivedAt("Billing sender"))
|
||||
assert.Contains(t, page, "No query string.")
|
||||
assert.Contains(t, page, headerBox(
|
||||
"Accept: */*",
|
||||
"User-Agent: shop/1 build\t7",
|
||||
@@ -331,3 +335,70 @@ func TestEventRequest_ManyShortHeaderLines(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleWebhook_StoresAndShowsTheQueryString posts to an
|
||||
// entrypoint's URL with a query string and proves the event stores it
|
||||
// as sent, and shows it escaped in the event log and on its own page,
|
||||
// in a box like the one the request headers show in.
|
||||
func TestHandleWebhook_StoresAndShowsTheQueryString(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
f := newRecentEventsFixture(t)
|
||||
ep := seedEntrypoint(t, f.db, f.webhook.ID)
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost,
|
||||
"/h/"+ep.Path+"?a=1&b=2", strings.NewReader("{}"),
|
||||
)
|
||||
|
||||
rctx := chi.NewRouteContext()
|
||||
rctx.URLParams.Add("uuid", ep.Path)
|
||||
|
||||
req = req.WithContext(context.WithValue(
|
||||
req.Context(), chi.RouteCtxKey, rctx,
|
||||
))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
f.h.HandleWebhook().ServeHTTP(w, req)
|
||||
|
||||
require.Equal(t, http.StatusOK, w.Code)
|
||||
|
||||
var stored database.Event
|
||||
|
||||
require.NoError(t, f.webhookDB.First(&stored).Error)
|
||||
assert.Equal(t, "a=1&b=2", stored.RawQuery)
|
||||
|
||||
page := renderSourceLogsPage(t, f.h, f.sess, f.webhook.ID)
|
||||
assert.Contains(t, page, headerBox("a=1&b=2"))
|
||||
|
||||
w = serveEventPage(t, f.h, f.sess, f.webhook.ID, stored.ID)
|
||||
require.Equal(t, http.StatusOK, w.Code)
|
||||
assert.Contains(t, w.Body.String(), headerBox("a=1&b=2"))
|
||||
}
|
||||
|
||||
// TestEventRequest_QueryStringOverTheLimit proves the event log leaves
|
||||
// out a query string that holds more than it shows of a body, and links
|
||||
// to the event's own page, which shows it whole.
|
||||
func TestEventRequest_QueryStringOverTheLimit(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
f := newRecentEventsFixture(t)
|
||||
ep := f.entrypoint(t, "Billing sender")
|
||||
event := f.eventAt(t, ep, `{}`, time.Now())
|
||||
query := "q=" + strings.Repeat("x", bodyCap)
|
||||
|
||||
require.NoError(t, f.webhookDB.Model(event).Update(
|
||||
"raw_query", query,
|
||||
).Error)
|
||||
|
||||
page := renderSourceLogsPage(t, f.h, f.sess, f.webhook.ID)
|
||||
assert.Contains(t, page, `<a href="/hook/`+f.webhook.ID+`/events/`+
|
||||
event.ID+`" class="btn-small">Show the query string</a>`)
|
||||
assert.NotContains(t, page, "q=x")
|
||||
assert.Less(t, len(page), 4*bodyCap)
|
||||
|
||||
w := serveEventPage(t, f.h, f.sess, f.webhook.ID, event.ID)
|
||||
require.Equal(t, http.StatusOK, w.Code)
|
||||
assert.Contains(t, w.Body.String(), headerBox(query))
|
||||
assert.NotContains(t, w.Body.String(), "Show the query string")
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ type resubmitSource struct {
|
||||
ID string
|
||||
EntrypointID string
|
||||
Method string
|
||||
RawQuery string
|
||||
Headers string
|
||||
ContentType string
|
||||
Body []byte
|
||||
@@ -39,7 +40,7 @@ type resubmitSource struct {
|
||||
// The cast to blob is what makes the driver hand back the stored bytes
|
||||
// rather than a string conversion, the same reason eventBodyQuery
|
||||
// casts.
|
||||
const resubmitColumns = "id, entrypoint_id, method, headers, " +
|
||||
const resubmitColumns = "id, entrypoint_id, method, raw_query, headers, " +
|
||||
"content_type, cast(body as blob) AS body"
|
||||
|
||||
// HandleEventResubmit re-injects a stored event as a new undelivered
|
||||
@@ -193,6 +194,7 @@ func (h *Handlers) queueResubmit(
|
||||
WebhookID: webhook.ID,
|
||||
EntrypointID: src.EntrypointID,
|
||||
Method: src.Method,
|
||||
RawQuery: src.RawQuery,
|
||||
HeadersJSON: src.Headers,
|
||||
ContentType: src.ContentType,
|
||||
Body: src.Body,
|
||||
|
||||
@@ -22,9 +22,13 @@ import (
|
||||
// dispatches to it: the notifier is recorded, not run.
|
||||
const resubmitTargetURL = "http://93.184.216.34/hook"
|
||||
|
||||
// resubmitEventHeaders is the stored header JSON a seeded event
|
||||
// carries, so a test can prove the copy takes it verbatim.
|
||||
const resubmitEventHeaders = `{"X-Test":["yes"],"X-Trace":["abc"]}`
|
||||
// resubmitEventHeaders and resubmitEventQuery are the stored header
|
||||
// JSON and query string a seeded event carries, so a test can prove the
|
||||
// copy takes them verbatim.
|
||||
const (
|
||||
resubmitEventHeaders = `{"X-Test":["yes"],"X-Trace":["abc"]}`
|
||||
resubmitEventQuery = "a=1&b=2"
|
||||
)
|
||||
|
||||
// seedStoredEvent records one event in a webhook's own database with
|
||||
// no deliveries at all, which is the state a captured event is in when
|
||||
@@ -43,6 +47,7 @@ func seedStoredEvent(
|
||||
WebhookID: webhookID,
|
||||
EntrypointID: "entrypoint-" + webhookID,
|
||||
Method: http.MethodPost,
|
||||
RawQuery: resubmitEventQuery,
|
||||
Headers: resubmitEventHeaders,
|
||||
Body: body,
|
||||
ContentType: contentTypeJSON,
|
||||
@@ -202,6 +207,7 @@ func assertEventCopy(
|
||||
t.Helper()
|
||||
|
||||
assert.Equal(t, original.Method, fresh.Method)
|
||||
assert.Equal(t, resubmitEventQuery, fresh.RawQuery)
|
||||
assert.Equal(t, original.Headers, fresh.Headers)
|
||||
assert.Equal(t, original.Body, fresh.Body)
|
||||
assert.Equal(t, int64(len(original.Body)), fresh.BodyBytes)
|
||||
@@ -236,6 +242,7 @@ func assertResubmitTask(
|
||||
assert.Equal(t, target.ID, task.TargetID)
|
||||
assert.Equal(t, target.Type, task.TargetType)
|
||||
assert.Equal(t, fresh.Method, task.Method)
|
||||
assert.Equal(t, fresh.RawQuery, task.RawQuery)
|
||||
assert.Equal(t, fresh.Headers, task.Headers)
|
||||
assert.Equal(t, fresh.ContentType, task.ContentType)
|
||||
assert.Equal(t, 1, task.AttemptNum)
|
||||
|
||||
@@ -173,6 +173,9 @@ type targetFormInput struct {
|
||||
Headers string
|
||||
// Timeout is an HTTP target's per-request timeout in seconds.
|
||||
Timeout string
|
||||
// ForwardQuery is an HTTP target's checkbox that passes each
|
||||
// event's query string on to it.
|
||||
ForwardQuery bool
|
||||
// MaxRetries is an HTTP or Slack target's max_retries.
|
||||
MaxRetries string
|
||||
// Expiry is a database (archive) target's row expiry.
|
||||
@@ -195,14 +198,15 @@ type targetFormInput struct {
|
||||
// tokens.
|
||||
func targetFormInputFrom(r *http.Request) targetFormInput {
|
||||
return targetFormInput{
|
||||
Name: r.PostFormValue("name"),
|
||||
Type: database.TargetType(r.PostFormValue("type")),
|
||||
URL: r.PostFormValue("url"),
|
||||
Headers: r.PostFormValue("headers"),
|
||||
Timeout: r.PostFormValue("timeout"),
|
||||
MaxRetries: r.PostFormValue("max_retries"),
|
||||
Expiry: r.PostFormValue("expiry"),
|
||||
Rotation: r.PostFormValue("rotation"),
|
||||
Name: r.PostFormValue("name"),
|
||||
Type: database.TargetType(r.PostFormValue("type")),
|
||||
URL: r.PostFormValue("url"),
|
||||
Headers: r.PostFormValue("headers"),
|
||||
Timeout: r.PostFormValue("timeout"),
|
||||
ForwardQuery: r.PostFormValue("forward_query") != "",
|
||||
MaxRetries: r.PostFormValue("max_retries"),
|
||||
Expiry: r.PostFormValue("expiry"),
|
||||
Rotation: r.PostFormValue("rotation"),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -232,8 +236,8 @@ func (h *Handlers) buildTargetConfig(
|
||||
}
|
||||
|
||||
// buildHTTPTargetConfig builds config JSON for an HTTP target: an
|
||||
// SSRF-validated destination plus the optional headers and timeout
|
||||
// the delivery path honours.
|
||||
// SSRF-validated destination plus the optional headers, timeout and
|
||||
// query string setting the delivery path honours.
|
||||
func (h *Handlers) buildHTTPTargetConfig(
|
||||
ctx context.Context,
|
||||
in targetFormInput,
|
||||
@@ -256,9 +260,10 @@ func (h *Handlers) buildHTTPTargetConfig(
|
||||
}
|
||||
|
||||
configJSON, err := marshalTargetConfig(delivery.HTTPTargetConfig{
|
||||
URL: in.URL,
|
||||
Headers: headers,
|
||||
Timeout: timeout,
|
||||
URL: in.URL,
|
||||
Headers: headers,
|
||||
Timeout: timeout,
|
||||
ForwardQuery: in.ForwardQuery,
|
||||
})
|
||||
|
||||
return configJSON, "", err
|
||||
|
||||
@@ -77,13 +77,14 @@ func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
|
||||
}
|
||||
|
||||
form := targetFormInput{
|
||||
Name: target.Name,
|
||||
URL: cfg.URL,
|
||||
Headers: cfg.Headers,
|
||||
Timeout: cfg.Timeout,
|
||||
MaxRetries: strconv.Itoa(target.MaxRetries),
|
||||
Expiry: cfg.Expiry,
|
||||
Rotation: cfg.Rotation,
|
||||
Name: target.Name,
|
||||
URL: cfg.URL,
|
||||
Headers: cfg.Headers,
|
||||
Timeout: cfg.Timeout,
|
||||
ForwardQuery: cfg.ForwardQuery,
|
||||
MaxRetries: strconv.Itoa(target.MaxRetries),
|
||||
Expiry: cfg.Expiry,
|
||||
Rotation: cfg.Rotation,
|
||||
}
|
||||
|
||||
h.renderTargetEdit(
|
||||
|
||||
@@ -442,6 +442,59 @@ func TestHandleTargetEdit_CallsTheDatabaseTypeArchive(t *testing.T) {
|
||||
assert.Contains(t, page, `class="label">Archive rotation</label>`)
|
||||
}
|
||||
|
||||
// TestHandleTarget_ForwardQuery covers the HTTP target's setting that
|
||||
// passes each event's query string on to it: the add target form
|
||||
// stores it checked, the edit form starts with it checked and turns it
|
||||
// off when saved unchecked, and both forms come back with it checked
|
||||
// when refused.
|
||||
func TestHandleTarget_ForwardQuery(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const checkbox = `name="forward_query" value="on" checked`
|
||||
|
||||
env := setupSourceTest(t)
|
||||
webhook := seedWebhookWithRetention(t, env.db, 30)
|
||||
targetsPath := "/hook/" + webhook.ID + "/targets"
|
||||
|
||||
form := url.Values{}
|
||||
form.Set("name", "forwarding")
|
||||
form.Set("type", string(database.TargetTypeHTTP))
|
||||
form.Set("url", editOriginalURL)
|
||||
form.Set("forward_query", "on")
|
||||
|
||||
w := serveTarget(env, http.MethodPost, targetsPath, form)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
||||
|
||||
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||
require.Len(t, targets, 1)
|
||||
assert.True(t, storedHTTPConfig(t, env, targets[0].ID).ForwardQuery)
|
||||
|
||||
w = serveTarget(
|
||||
env, http.MethodGet, targetsPath+"/"+targets[0].ID+"/edit", nil,
|
||||
)
|
||||
require.Equal(t, http.StatusOK, w.Code)
|
||||
assert.Contains(t, w.Body.String(), checkbox)
|
||||
|
||||
edit := editForm(editOriginalURL, "", "")
|
||||
|
||||
w = submitTargetEdit(env, webhook.ID, targets[0].ID, edit)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
||||
assert.False(t, storedHTTPConfig(t, env, targets[0].ID).ForwardQuery)
|
||||
|
||||
edit.Set("url", editBlockedURL)
|
||||
edit.Set("forward_query", "on")
|
||||
|
||||
w = submitTargetEdit(env, webhook.ID, targets[0].ID, edit)
|
||||
require.Equal(t, http.StatusBadRequest, w.Code)
|
||||
assert.Contains(t, w.Body.String(), checkbox)
|
||||
|
||||
form.Set("url", editBlockedURL)
|
||||
|
||||
w = serveTarget(env, http.MethodPost, targetsPath, form)
|
||||
require.Equal(t, http.StatusBadRequest, w.Code)
|
||||
assert.Contains(t, w.Body.String(), "data-forward-query")
|
||||
}
|
||||
|
||||
// TestHandleTargetEditSubmit_Rejects covers every submission that
|
||||
// must not reach storage.
|
||||
//
|
||||
|
||||
@@ -230,6 +230,7 @@ type eventSource struct {
|
||||
WebhookID string
|
||||
EntrypointID string
|
||||
Method string
|
||||
RawQuery string
|
||||
HeadersJSON string
|
||||
ContentType string
|
||||
Body []byte
|
||||
@@ -245,6 +246,7 @@ func (s eventSource) event() *database.Event {
|
||||
WebhookID: s.WebhookID,
|
||||
EntrypointID: s.EntrypointID,
|
||||
Method: s.Method,
|
||||
RawQuery: s.RawQuery,
|
||||
Headers: s.HeadersJSON,
|
||||
Body: string(s.Body),
|
||||
BodyBytes: int64(len(s.Body)),
|
||||
@@ -264,6 +266,7 @@ func requestEventSource(
|
||||
WebhookID: entrypoint.WebhookID,
|
||||
EntrypointID: entrypoint.ID,
|
||||
Method: r.Method,
|
||||
RawQuery: r.URL.RawQuery,
|
||||
HeadersJSON: string(headersJSON),
|
||||
ContentType: r.Header.Get("Content-Type"),
|
||||
Body: body,
|
||||
@@ -441,6 +444,7 @@ func buildDeliveryTasks(
|
||||
TargetConfig: targets[i].Config,
|
||||
MaxRetries: targets[i].MaxRetries,
|
||||
Method: event.Method,
|
||||
RawQuery: event.RawQuery,
|
||||
Headers: event.Headers,
|
||||
ContentType: event.ContentType,
|
||||
Body: bodyPtr,
|
||||
|
||||
Reference in New Issue
Block a user