From 8b5e3b734f326edb5a04e79148fe132629755ca6 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 2 Oct 2026 16:54:14 +0000 Subject: [PATCH] Let an entrypoint's description be edited in place (closes #392) Each entrypoint on the webhook page has an Edit button that shows its description as a form, with Save and Cancel, in place. Saving posts to /hook/{id}/entrypoints/{entrypointID}/edit, behind the same login, CSRF and ownership checks as activate, deactivate and delete, and writes only the description column, so the URL never changes. An empty description shows as "Entrypoint". Activate and deactivate now write only the active column, so they cannot write back an older description over an edit. Model: opus-5-5 --- README.md | 5 +- internal/handlers/notice.go | 2 + internal/handlers/source_management.go | 72 ++++++++++++- internal/server/alpine_browser_test.go | 52 ++++++++++ internal/server/routes.go | 4 + internal/server/routes_test.go | 135 +++++++++++++++++++++++++ static/js/app.js | 3 +- templates/source_detail.html | 13 ++- 8 files changed, 280 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 1f845f4..688ef9d 100644 --- a/README.md +++ b/README.md @@ -1327,7 +1327,9 @@ under the real policy and checks that: both add forms stay hidden until Add is clicked; choosing Slack in the add target form leaves the HTTP fields out of what it submits, also after leaving the page and going back to it, when the browser restores the choice; the Copy button beside an entrypoint URL reads -"Copied" once clicked; an event expands and collapses, and so do a delivery's +"Copied" once clicked; an entrypoint's Edit button shows its edit form in place +of its description, Cancel hides it and drops what was typed, and Save changes +the description; an event expands and collapses, and so do a delivery's attempts inside it; and at phone width the menu button opens and closes the mobile menu. It also fails if the browser reports a console warning or error, an uncaught exception, or anything the policy refused. `make check` and the @@ -2893,6 +2895,7 @@ returns to the page that was asked for. | `POST` | `/hook/{id}/deliveries/{deliveryID}/replay` | Replay a finished delivery: creates a new delivery for the same event against the target's current configuration (30 per minute per bucket, then `429`) | | `POST` | `/hook/{id}/events/{eventID}/resubmit` | Resubmit a stored event: creates a new event copying it and fans that out to every currently active target (30 per minute per bucket, then `429`) | | `POST` | `/hook/{id}/entrypoints` | Add entrypoint to webhook | +| `POST` | `/hook/{id}/entrypoints/{entrypointID}/edit` | Change an entrypoint's description; its URL stays the same | | `POST` | `/hook/{id}/entrypoints/{entrypointID}/delete` | Delete an entrypoint | | `POST` | `/hook/{id}/entrypoints/{entrypointID}/toggle` | Enable or disable an entrypoint | | `POST` | `/hook/{id}/targets` | Add target to webhook | diff --git a/internal/handlers/notice.go b/internal/handlers/notice.go index 72b97e5..5a20d5c 100644 --- a/internal/handlers/notice.go +++ b/internal/handlers/notice.go @@ -20,6 +20,7 @@ const ( webhookSaved noticeCode = "webhook-saved" webhookDeleted noticeCode = "webhook-deleted" entrypointAdded noticeCode = "entrypoint-added" + entrypointSaved noticeCode = "entrypoint-saved" entrypointDeleted noticeCode = "entrypoint-deleted" entrypointActivated noticeCode = "entrypoint-activated" entrypointDeactivated noticeCode = "entrypoint-deactivated" @@ -48,6 +49,7 @@ func noticeFor(r *http.Request) *notice { webhookSaved: {Text: "Webhook saved."}, webhookDeleted: {Text: "Webhook deleted."}, entrypointAdded: {Text: "Entrypoint added."}, + entrypointSaved: {Text: "Entrypoint description saved."}, entrypointDeleted: {Text: "Entrypoint deleted."}, entrypointActivated: {Text: "Entrypoint activated."}, entrypointDeactivated: {Text: "Entrypoint deactivated."}, diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go index 377eac8..9bb3330 100644 --- a/internal/handlers/source_management.go +++ b/internal/handlers/source_management.go @@ -1396,6 +1396,70 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc { } } +// HandleEntrypointEdit handles changing an entrypoint's description. +// It writes only the description column, so the entrypoint keeps its +// URL, and an activate or deactivate saved since the page was shown +// is not undone. +func (h *Handlers) HandleEntrypointEdit() http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + userID, ok := h.getUserID(r) + if !ok { + http.Redirect( + w, r, "/pages/login", http.StatusSeeOther, + ) + + return + } + + sourceID := chi.URLParam(r, "sourceID") + entrypointID := chi.URLParam(r, "entrypointID") + + var webhook database.Webhook + + err := h.db.DB().Where( + "id = ? AND user_id = ?", sourceID, userID, + ).First(&webhook).Error + if err != nil { + h.renderError(w, r, http.StatusNotFound) + + return + } + + // The body size cap is enforced by the MaxBodySize + // middleware, which runs before CSRF parses the form. + err = r.ParseForm() + if err != nil { + h.renderError(w, r, http.StatusBadRequest) + + return + } + + result := h.db.DB().Model(&database.Entrypoint{}).Where( + "id = ? AND webhook_id = ?", entrypointID, webhook.ID, + ).Update("description", r.PostFormValue("description")) + if result.Error != nil { + h.serverError( + w, r, "failed to edit entrypoint", result.Error, + ) + + return + } + + // The id came from the URL and may name another webhook's + // entrypoint, which this webhook does not have. + if result.RowsAffected == 0 { + h.renderError(w, r, http.StatusNotFound) + + return + } + + http.Redirect( + w, r, withNotice("/hook/"+webhook.ID, entrypointSaved), + http.StatusSeeOther, + ) + } +} + // HandleTargetCreate handles adding a new target to a webhook. func (h *Handlers) HandleTargetCreate() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { @@ -1877,9 +1941,13 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc { return false, err } - ep.Active = !ep.Active + // Only the active column: saving the whole row would + // write back the description read above over an edit + // saved since. + active := !ep.Active - return ep.Active, h.db.DB().Save(&ep).Error + return active, h.db.DB().Model(&ep). + Update("active", active).Error }, "failed to toggle entrypoint", entrypointActivated, entrypointDeactivated, diff --git a/internal/server/alpine_browser_test.go b/internal/server/alpine_browser_test.go index 7637cf8..301bebf 100644 --- a/internal/server/alpine_browser_test.go +++ b/internal/server/alpine_browser_test.go @@ -86,6 +86,7 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) { checkAddForms(ctx, t, page) checkTargetType(ctx, t, page+"/events") checkCopy(ctx, t, page) + checkEntrypointEdit(ctx, t, page) checkEventLog(ctx, t, page+"/events", event.ID, target.Name) checkMobileMenu(ctx, t, page) @@ -363,6 +364,57 @@ func checkCopy(ctx context.Context, t *testing.T, url string) { `clicking Copy does not show "Copied"`) } +// checkEntrypointEdit loads a webhook page whose entrypoint has no +// description, and checks that Edit shows the edit form in place of +// the description, that Cancel hides it and drops what was typed, and +// that Save changes the description the page shows. +func checkEntrypointEdit(ctx context.Context, t *testing.T, url string) { + t.Helper() + + const ( + editForm = `form[action$="/edit"]` + input = editForm + ` input[name="description"]` + description = `//span[text()="Entrypoint"]` + edit = `//button[text()="Edit"]` + ) + + require.NoError(t, chromedp.Run(ctx, loadPage(url))) + + assert.True(t, hidden(ctx, editForm), + "the edit form shows before Edit is clicked") + + click(ctx, t, edit) + assert.True(t, shown(ctx, editForm), + "clicking Edit does not show the edit form") + assert.True(t, hidden(ctx, description), + "the description stays shown beside the edit form") + + require.NoError(t, chromedp.Run( + ctx, chromedp.SendKeys(input, "draft", chromedp.ByQuery), + )) + click(ctx, t, `//button[text()="Cancel"]`) + assert.True(t, hidden(ctx, editForm), + "clicking Cancel does not hide the edit form") + assert.True(t, shown(ctx, description), + "clicking Cancel does not show the description again") + + var typed string + + click(ctx, t, edit) + require.NoError(t, chromedp.Run( + ctx, chromedp.Value(input, &typed, chromedp.ByQuery), + )) + assert.Empty(t, typed, "Cancel keeps what was typed") + + require.NoError(t, chromedp.Run( + ctx, chromedp.SendKeys(input, "Billing sender", chromedp.ByQuery), + )) + click(ctx, t, `//button[text()="Save"]`) + + assert.True(t, shown(ctx, `//span[text()="Billing sender"]`), + "saving the edit form does not change the description") +} + // checkEventLog loads the event log and checks that clicking an event's // row expands it, that in there clicking its delivery shows the // delivery's attempts and clicking again hides them, and that clicking diff --git a/internal/server/routes.go b/internal/server/routes.go index 24c63d0..c9bfe67 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -289,6 +289,10 @@ func (s *Server) setupSourceRoutes() { "/entrypoints", s.h.HandleEntrypointCreate(), ) + r.Post( + "/entrypoints/{entrypointID}/edit", + s.h.HandleEntrypointEdit(), + ) r.Post( "/entrypoints/{entrypointID}/delete", s.h.HandleEntrypointDelete(), diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index f9327c2..62a2ec4 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -12,6 +12,7 @@ import ( "strings" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/fx" @@ -398,6 +399,44 @@ func (e *testEnv) seedTarget( return tgt } +// seedEntrypoint creates an active entrypoint for a webhook. +func (e *testEnv) seedEntrypoint( + t *testing.T, + webhookID string, +) *database.Entrypoint { + t.Helper() + + ep := &database.Entrypoint{ + WebhookID: webhookID, + Path: uuid.New().String(), + Description: "Default entrypoint", + Active: true, + } + + require.NoError( + t, + e.db.DB().Omit(clause.Associations).Create(ep).Error, + ) + + return ep +} + +// storedEntrypoint reloads an entrypoint row. +func (e *testEnv) storedEntrypoint( + t *testing.T, + entrypointID string, +) database.Entrypoint { + t.Helper() + + var ep database.Entrypoint + + require.NoError( + t, e.db.DB().First(&ep, "id = ?", entrypointID).Error, + ) + + return ep +} + // seedFailedDelivery records a terminally failed delivery of an event // to a target in the webhook's own database. func (e *testEnv) seedFailedDelivery( @@ -1070,6 +1109,102 @@ func TestHook_EntrypointActions(t *testing.T) { assert.Zero(t, left, "the delete should remove the entrypoint") } +// TestHook_EntrypointEdit changes an entrypoint's description with the +// edit form on the webhook page, then empties it. The entrypoint keeps +// its URL, and with no description it shows as "Entrypoint". Without +// the CSRF token the edit is refused. +func TestHook_EntrypointEdit(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + + userID, _ := env.seedUser(t, "epeditor", "somepassword") + cookies := env.authCookies(t, userID, "epeditor") + wh := env.seedWebhook(t, userID) + ep := env.seedEntrypoint(t, wh.ID) + page := "/hook/" + wh.ID + + token, cookies := env.csrfFrom(t, page, cookies) + action := env.urlFrom( + t, page, `action="(/hook/[^/"]+/entrypoints/[^/"]+/edit)"`, + cookies, + ) + + assert.Equal( + t, http.StatusForbidden, + env.post(action, entrypointEditForm("", "no token"), cookies).Code, + "an edit without a CSRF token must be refused", + ) + + // edit submits the form with description and requires the + // redirect back to the webhook page with the notice. + edit := func(description string) { + t.Helper() + + w := env.post( + action, entrypointEditForm(token, description), cookies, + ) + env.requireNotice(t, w, page, "entrypoint-saved", + "Entrypoint description saved.", cookies) + } + + edit("Billing sender") + + stored := env.storedEntrypoint(t, ep.ID) + assert.Equal(t, "Billing sender", stored.Description) + assert.Equal(t, ep.Path, stored.Path, + "the edit must keep the entrypoint's URL") + + body := env.get(page, cookies).Body.String() + assert.Contains(t, body, ">Billing sender") + assert.Contains(t, body, "/h/"+ep.Path+"") + + edit("") + + assert.Empty(t, env.storedEntrypoint(t, ep.ID).Description) + assert.Contains(t, env.get(page, cookies).Body.String(), + ">Entrypoint", "no description shows as Entrypoint") +} + +// TestHook_EntrypointEdit_OtherUser404s has another logged-in user, with +// a CSRF token of their own, try to edit an entrypoint: through the +// owner's webhook, and through a webhook of their own. Both are 404s +// and the description stays as it was. +func TestHook_EntrypointEdit_OtherUser404s(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + + ownerID, _ := env.seedUser(t, "epowner", "somepassword") + wh := env.seedWebhook(t, ownerID) + ep := env.seedEntrypoint(t, wh.ID) + + otherID, _ := env.seedUser(t, "epother", "somepassword") + other := env.authCookies(t, otherID, "epother") + theirs := env.seedWebhook(t, otherID) + token, other := env.csrfFrom(t, "/hook/"+theirs.ID, other) + + for _, webhookID := range []string{wh.ID, theirs.ID} { + path := "/hook/" + webhookID + "/entrypoints/" + ep.ID + "/edit" + + w := env.post(path, entrypointEditForm(token, "not theirs"), other) + assert.Equal(t, http.StatusNotFound, w.Code, path) + } + + assert.Equal( + t, ep.Description, env.storedEntrypoint(t, ep.ID).Description, + "another user must not change the description", + ) +} + +// entrypointEditForm fills in the webhook page's entrypoint edit form. +func entrypointEditForm(token, description string) url.Values { + return url.Values{ + "csrf_token": {token}, + "description": {description}, + } +} + // TestHook_TargetActions adds a target with the form on the webhook // page, follows its Edit link to the target edit form and submits // it, then deactivates, activates and deletes it, every URL and token diff --git a/static/js/app.js b/static/js/app.js index 4f76da9..7268703 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -70,7 +70,8 @@ document.addEventListener("alpine:init", function () { "use strict"; // Something a click shows and hides: the mobile menu, an add form, - // an event in the event log, a delivery's attempts. + // an entrypoint's edit form, an event in the event log, a delivery's + // attempts. window.Alpine.data("collapsible", function () { return { open: false, diff --git a/templates/source_detail.html b/templates/source_detail.html index 0cb0a66..8d08659 100644 --- a/templates/source_detail.html +++ b/templates/source_detail.html @@ -54,15 +54,24 @@
{{range .Entrypoints}} -
+
- {{if .Description}}{{.Description}}{{else}}Entrypoint{{end}} + {{if .Description}}{{.Description}}{{else}}Entrypoint{{end}} + +
+ + + + +
{{if .Active}} Active {{else}} Inactive {{end}} +