Author SHA1 Message Date
sneak 1ac8102243 Let an entrypoint's description be edited in place (closes #392)
check / check (push) Waiting to run
Each entrypoint on the webhook page has an Edit button that shows its
description as a form, with Save and Cancel, in place. Edit hides while
the form is open, so the form always opens on the saved description.
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
2026-10-02 18:14:36 +00:00
22 changed files with 471 additions and 370 deletions
+27 -53
View File
@@ -79,8 +79,7 @@ directory, read once at startup before anything else looks at the
environment.
The file is optional and having none is the normal case for a
deployment. An empty file is the same as none: it has nothing in it to
apply. A file that is there but cannot be parsed aborts startup
deployment. A file that is there but cannot be parsed aborts startup
with a message naming it, because a single malformed line makes none
of the file apply: every variable in it silently reverts to its
default, which is exactly the failure [Invalid values abort
@@ -565,13 +564,11 @@ its Argon2id hash. There is no second account and no forgot-password
flow, so the banner and the reset command below are the only two ways
in.
A start that finds no `webhooker.db` in `DATA_DIR`, or a zero-length
one (which SQLite opens as an empty database), also logs
A start that finds no `webhooker.db` in `DATA_DIR` also logs
`created a new, empty database` at `WARN`, with the file's path,
shortly before the banner. On a deployment that has run before, that
line means `webhooker.db` was lost: either `DATA_DIR` was empty, most
often because its volume is not mounted, or the file was zero-length,
as a truncated copy leaves it.
line means `DATA_DIR` was empty, most often because its volume is not
mounted.
#### Recovering a lost admin password
@@ -615,8 +612,7 @@ What it will not do:
the old password, so a reset underneath it would report a change the
service does not honour.
- **Create anything.** A `DATA_DIR` that does not exist, or that holds
no `webhooker.db` or a zero-length one, is an error naming the path
rather than a new empty deployment —
no `webhooker.db`, is an error rather than a new empty deployment —
a mistyped path must not be built out and then reported as a success.
- **Create an account.** A username that does not exist is an error.
`resetpw` changes an existing account's password and nothing else.
@@ -997,16 +993,6 @@ its sidecars; a killed or crashed instance leaves them, and they must be
carried with the `.db`. An archive the service has not opened since a
crash keeps that crash's sidecars, even across a later clean stop.
A missing sidecar is therefore normal, and SQLite makes new ones, so a
`-wal` lost from a copy cannot be reported: the transactions it held
are simply gone. SQLite reads a `-wal` up to its first damaged frame,
as after a crash, and rebuilds a damaged `-shm`. A sidecar with the
wrong mode is set back to `0600` when its database is opened. A
directory in place of either is refused then, with an error naming it:
for `webhooker.db` the server and `webhooker resetpw` stop, and an event
or archive database fails as a damaged one does (see
[Database Architecture](#database-architecture)).
Configuration is **not** in `DATA_DIR` — it comes from the environment
and from a `.env` file read out of the process working directory. Back
that up with your deployment config, separately.
@@ -1089,17 +1075,13 @@ with any `-wal`/`-shm` beside it, or wait until there are none.
1. Stop the service.
2. Restore the **whole set together**: `webhooker.db` *and* every
`events-*.db` *and* every `archive-*.db`. A partial restore is
reported, not refused. Every database is opened `mode=rwc`, so a
missing `events-{uuid}.db` is **created empty**: the webhook comes
back with its configuration intact and its entire event history
gone. The first start after the restore logs
`created a new, empty database` at `WARN` for each such file, with
its path, as it does for a missing `webhooker.db`. A missing
`archive-*.db` is recreated at its target's next delivery without a
warning, since moving one away is a supported workflow. Event
databases restored without `webhooker.db` are simply orphaned;
nothing references their UUIDs.
`events-*.db` *and* every `archive-*.db`. A partial restore fails
quietly rather than loudly. Every database is opened `mode=rwc`, so a
missing `events-{uuid}.db` is **created empty** on first access
instead of erroring — the webhook comes back with its configuration
intact and its entire event history silently gone. Event databases
restored without `webhooker.db` are simply orphaned; nothing
references their UUIDs.
3. Carry any `*.db-wal` and `*.db-shm` files that are in the backup.
They are part of the database, and dropping a `-wal` silently
@@ -1345,15 +1327,17 @@ 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
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
image build lint it but do not run it, and `make test` leaves it out (its file
is built only with the `browser` build tag). Run it with `make test-browser`
after changing `templates/` or `static/js/`: that builds `Dockerfile.browser`,
which runs the test in a digest-pinned headless browser image, so the host
needs no browser.
"Copied" once clicked; an entrypoint's Edit button shows its edit form in place
of its description and hides until the form closes, Cancel hides the form 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 image build lint it but do not run it, and
`make test` leaves it out (its file is built only with the `browser` build tag).
Run it with `make test-browser` after changing `templates/` or `static/js/`:
that builds `Dockerfile.browser`, which runs the test in a digest-pinned
headless browser image, so the host needs no browser.
The package's tarball is committed as `3p/alpinejs-csp-3.14.9.tgz`, byte for
byte as the npm registry publishes it. It is a dependency, not this repo's build
@@ -1957,19 +1941,10 @@ encryption key is generated and stored, and an `admin` user is created.
the deliveries per target, kept through retention
Per-webhook databases are created automatically when a webhook is
created. They are managed by the `WebhookDBManager` component, which
created (and lazily on first access for webhooks that predate this
feature). They are managed by the `WebhookDBManager` component, which
handles connection pooling, lazy opening, migrations, and cleanup.
A per-webhook database that is missing or zero-length later means its
webhook's events and pending deliveries are gone. The next time it is
opened, an empty one is created in its place, so the webhook keeps
receiving, and `created a new, empty database` is logged at `WARN` with
the file's path. Every webhook's database is opened when the service
starts, so this appears at the latest at the first start after the
file was lost. A file there that SQLite cannot open fails that
webhook alone, with an `ERROR` naming the webhook on every access and a
500 to its senders, so one damaged file does not stop the others.
This separation provides:
- **Isolation** — a high-volume webhook won't cause lock contention or
@@ -2034,9 +2009,7 @@ After each write the archive handle is closed
and reopened, debounced to at most once per second, so an operator can
move the archive file away for offline archiving without stopping the
service; a moved or removed archive file is recreated automatically on
the next write. A zero-length archive file is written to as a new
archive: SQLite opens it as an empty database, so it holds nothing to
lose. An optional `expiry` in the target's config JSON (e.g.
the next write. An optional `expiry` in the target's config JSON (e.g.
`{"expiry":"720h"}`) is validated when the target is created — the
default (unset or the literal `never`) keeps rows forever — and rows
older than the expiry are pruned each time the archive is (re)opened. An
@@ -2922,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 |
@@ -4,7 +4,6 @@ import (
"bytes"
"context"
"log/slog"
"os"
"path/filepath"
"strings"
"testing"
@@ -120,26 +119,3 @@ func TestNewDatabase_IsLoggedWithItsPath(t *testing.T) {
t, second, created, "an existing database is not new",
)
}
// TestZeroLengthDatabase_IsLoggedAsNew covers what
// https://git.eeqj.de/sneak/webhooker/issues/290 found: SQLite opens a
// zero-length file as an empty database, so a start on one is a first
// start, and it must say so exactly as a start with no file does.
func TestZeroLengthDatabase_IsLoggedAsNew(t *testing.T) {
t.Parallel()
dir := t.TempDir()
path := filepath.Join(dir, database.MainDBFileName)
require.NoError(t, os.WriteFile(path, nil, database.SQLiteFilePerm))
var out bytes.Buffer
db, err := database.Open(dir, slog.New(slog.NewTextHandler(&out, nil)))
require.NoError(t, err)
require.NoError(t, db.Close())
assert.Contains(
t, out.String(),
`level=WARN msg="created a new, empty database" path=`+path,
)
}
+7 -12
View File
@@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"log/slog"
"os"
"path/filepath"
@@ -202,7 +203,8 @@ func (d *Database) connectTo(dataDir string) error {
// Checked before opening, which creates the file. A DATA_DIR that
// is unexpectedly empty -- its volume not mounted, say -- looks
// exactly like a first start, so a new database is a warning.
created := missingOrEmpty(dbPath)
_, statErr := os.Stat(dbPath)
created := errors.Is(statErr, fs.ErrNotExist)
// Opened through OpenSQLite so this handle carries the same WAL
// journaling, busy timeout, immediate-transaction locking, and pool
@@ -211,15 +213,13 @@ func (d *Database) connectTo(dataDir string) error {
if err != nil {
d.log.Error(
"failed to open database",
"path", dbPath,
"error", err,
)
return err
}
// Then use it with GORM. Its errors are SQLite's alone and name no
// file, so the path is added to them here.
// Then use it with GORM
db, err := gorm.Open(sqlite.Dialector{
Conn: sqlDB,
}, &gorm.Config{
@@ -229,11 +229,10 @@ func (d *Database) connectTo(dataDir string) error {
if err != nil {
d.log.Error(
"failed to connect to database",
"path", dbPath,
"error", err,
)
return fmt.Errorf("connecting to %s: %w", dbPath, err)
return err
}
d.db = db
@@ -244,12 +243,8 @@ func (d *Database) connectTo(dataDir string) error {
d.log.Info("connected to database", "path", dbPath)
}
err = d.migrate()
if err != nil {
return fmt.Errorf("migrating %s: %w", dbPath, err)
}
return nil
// Run migrations
return d.migrate()
}
func (d *Database) migrate() error {
-25
View File
@@ -1,15 +1,9 @@
package database_test
import (
"bytes"
"context"
"log/slog"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/database"
@@ -106,22 +100,3 @@ func TestDatabaseConnection(t *testing.T) {
)
}
}
// TestOpen_UnreadableDatabaseIsNamed pins
// https://git.eeqj.de/sneak/webhooker/issues/459: when SQLite cannot
// read webhooker.db, the error that stops the server and `webhooker
// resetpw` names the file, not only SQLite's own message.
func TestOpen_UnreadableDatabaseIsNamed(t *testing.T) {
t.Parallel()
dir := t.TempDir()
path := filepath.Join(dir, database.MainDBFileName)
require.NoError(t, os.WriteFile(
path, bytes.Repeat([]byte("junk"), 1024), database.SQLiteFilePerm,
))
_, err := database.Open(dir, slog.New(slog.DiscardHandler))
require.Error(t, err)
assert.Contains(t, err.Error(), path)
assert.Contains(t, err.Error(), "file is not a database")
}
+2 -2
View File
@@ -184,8 +184,8 @@ func (r *RetentionReaper) sweep(ctx context.Context) {
wh := webhooks[i]
// A missing database has nothing to reap. Restart recovery
// reports a lost one (see WebhookDBManager.GetDB).
// Nothing to reap if the per-webhook database has never
// been created.
if !r.dbManager.DBExists(wh.ID) {
continue
}
-23
View File
@@ -182,29 +182,6 @@ func TestOpenSQLiteTightensFilesLeftWorldReadable(t *testing.T) {
requireDatabaseSetOwnerOnly(t, path)
}
// TestOpenSQLiteRefusesADirectorySidecar covers a directory in place
// of -wal or -shm. Beside a -shm directory SQLite opens the database
// read-only without a word, and every write then fails naming no file,
// so the open must stop instead, naming the directory.
func TestOpenSQLiteRefusesADirectorySidecar(t *testing.T) {
t.Parallel()
for _, suffix := range []string{"-wal", "-shm"} {
t.Run(suffix, func(t *testing.T) {
t.Parallel()
path := filepath.Join(t.TempDir(), database.MainDBFileName)
require.NoError(t, os.Mkdir(path+suffix, 0o700))
_, err := database.OpenSQLite(
path, database.SQLiteModeCreate,
)
require.Error(t, err)
assert.Contains(t, err.Error(), path+suffix)
})
}
}
// TestOpenSQLiteExistingModeDoesNotCreateTheFile guards the mechanism
// the fix uses: OpenSQLite now creates the database file itself, and
// must not do so for a caller that asked for an existing database. An
+2 -26
View File
@@ -7,7 +7,6 @@ import (
"io/fs"
"net/url"
"os"
"syscall"
"time"
_ "modernc.org/sqlite" // Pure Go SQLite driver
@@ -94,8 +93,7 @@ const (
const SQLiteFilePerm fs.FileMode = 0o600
// reserveSQLiteFile puts path at SQLiteFilePerm before the driver ever
// touches it, and tightens any sidecar already on disk. A directory in
// place of any of them is an error naming it.
// touches it, and tightens any sidecar already on disk.
//
// The mode has to be settled here rather than by a chmod after opening,
// because SQLite picks it: robust_open substitutes
@@ -145,15 +143,7 @@ func reserveSQLiteFile(path string, create bool) error {
for _, p := range append(
[]string{path}, sqliteSidecarPaths(path)...,
) {
// Chmod accepts a directory, and SQLite opens a database whose
// -shm is one read-only, without a word: every write then
// fails naming no file.
info, err := os.Stat(p)
if err == nil && info.IsDir() {
return fmt.Errorf("securing %s: %w", p, syscall.EISDIR)
}
err = os.Chmod(p, SQLiteFilePerm)
err := os.Chmod(p, SQLiteFilePerm)
if err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("securing %s: %w", p, err)
}
@@ -162,20 +152,6 @@ func reserveSQLiteFile(path string, create bool) error {
return nil
}
// missingOrEmpty reports whether opening path in SQLiteModeCreate
// would start a new, empty database: the file is not there, or it is
// zero-length, which SQLite opens as an empty database. A file left at
// zero length by an interrupted first start or a truncated copy holds
// as little as a missing one, and must be reported the same way.
func missingOrEmpty(path string) bool {
info, err := os.Stat(path)
if errors.Is(err, fs.ErrNotExist) {
return true
}
return err == nil && info.Size() == 0
}
// sqliteSidecarPaths returns the files SQLite maintains beside a
// database under WAL. They carry the same rows as the database itself,
// so a fix that tightens only the main file has fixed nothing.
+28 -54
View File
@@ -98,18 +98,34 @@ func NewWebhookDBManager(
return m, nil
}
// GetDB returns the database connection for a webhook, opening it on
// first use.
//
// The file is made by CreateDB when the webhook is created. One that is
// missing or zero-length here means the webhook's events and pending
// deliveries are gone: an empty database is created in its place so
// the webhook keeps receiving, and that is logged as a warning naming
// the file, as a new main database is.
// GetDB returns the database connection for a webhook,
// creating the database file lazily if it doesn't exist.
func (m *WebhookDBManager) GetDB(
webhookID string,
) (*gorm.DB, error) {
return m.getDB(webhookID, false)
// Fast path: already open
if val, ok := m.dbs.Load(webhookID); ok {
return asGormDB(val, webhookID)
}
// Slow path: open the database under the lock, looking in the
// cache again first. A caller that raced another one here then
// waits for its handle instead of opening a second one.
m.mu.Lock()
defer m.mu.Unlock()
if val, ok := m.dbs.Load(webhookID); ok {
return asGormDB(val, webhookID)
}
db, err := m.openDB(webhookID)
if err != nil {
return nil, err
}
m.dbs.Store(webhookID, db)
return db, nil
}
// asGormDB returns a value read from the cache as the database
@@ -127,12 +143,12 @@ func asGormDB(val any, webhookID string) (*gorm.DB, error) {
return db, nil
}
// CreateDB creates a new webhook's database file and runs
// migrations.
// CreateDB explicitly creates a new per-webhook database file
// and runs migrations.
func (m *WebhookDBManager) CreateDB(
webhookID string,
) error {
_, err := m.getDB(webhookID, true)
_, err := m.GetDB(webhookID)
return err
}
@@ -250,48 +266,6 @@ func (m *WebhookDBManager) DBPath(
return m.dbPath(webhookID)
}
// getDB is GetDB, and CreateDB when isNew is true: the webhook has just
// been created, so a missing file is expected rather than lost.
func (m *WebhookDBManager) getDB(
webhookID string, isNew bool,
) (*gorm.DB, error) {
// Fast path: already open
if val, ok := m.dbs.Load(webhookID); ok {
return asGormDB(val, webhookID)
}
// Slow path: open the database under the lock, looking in the
// cache again first. A caller that raced another one here then
// waits for its handle instead of opening a second one.
m.mu.Lock()
defer m.mu.Unlock()
if val, ok := m.dbs.Load(webhookID); ok {
return asGormDB(val, webhookID)
}
// Checked before opening, which creates the file. See GetDB.
path := m.dbPath(webhookID)
replaced := !isNew && missingOrEmpty(path)
db, err := m.openDB(webhookID)
if err != nil {
return nil, err
}
if replaced {
m.log.Warn(
"created a new, empty database",
"webhook_id", webhookID,
"path", path,
)
}
m.dbs.Store(webhookID, db)
return db, nil
}
func (m *WebhookDBManager) dbPath(
webhookID string,
) string {
@@ -289,75 +289,6 @@ func TestWebhookDBManager_LazyCreation(t *testing.T) {
assert.True(t, mgr.DBExists(webhookID))
}
// A webhook's database is made by CreateDB along with the webhook. One
// that GetDB finds missing or zero-length has lost the webhook's events
// and pending deliveries, so the empty database made in its place is
// logged as a warning naming the file
// (https://git.eeqj.de/sneak/webhooker/issues/290). CreateDB, and
// reopening a database that is there, log no such warning.
func TestWebhookDBManager_LostDatabaseIsLogged(t *testing.T) {
t.Parallel()
const created = `level=WARN msg="created a new, empty database"`
open := func(
t *testing.T, prepare func(*database.WebhookDBManager, string),
) (string, string) {
t.Helper()
var logs bytes.Buffer
mgr := database.NewTestWebhookDBManagerWithLogger(
t.TempDir(),
slog.New(slog.NewTextHandler(&logs, nil)),
)
webhookID := uuid.New().String()
prepare(mgr, webhookID)
_, err := mgr.GetDB(webhookID)
require.NoError(t, err)
require.NoError(t, mgr.CloseAll())
return logs.String(),
" webhook_id=" + webhookID + " path=" + mgr.DBPath(webhookID)
}
t.Run("missing", func(t *testing.T) {
t.Parallel()
logs, fields := open(
t, func(*database.WebhookDBManager, string) {},
)
assert.Contains(t, logs, created+fields)
})
t.Run("zero-length", func(t *testing.T) {
t.Parallel()
logs, fields := open(
t, func(mgr *database.WebhookDBManager, webhookID string) {
require.NoError(t, os.WriteFile(
mgr.DBPath(webhookID), nil, database.SQLiteFilePerm,
))
},
)
assert.Contains(t, logs, created+fields)
})
t.Run("created with the webhook, then reopened", func(t *testing.T) {
t.Parallel()
logs, _ := open(
t, func(mgr *database.WebhookDBManager, webhookID string) {
require.NoError(t, mgr.CreateDB(webhookID))
require.NoError(t, mgr.CloseAll())
},
)
assert.NotContains(t, logs, created)
})
}
func TestWebhookDBManager_DeliveryWorkflow(t *testing.T) {
t.Parallel()
+4 -3
View File
@@ -699,9 +699,10 @@ func (e *Engine) recoverInFlight(ctx context.Context) {
default:
}
// Opened even when its file is missing, so that GetDB reports
// a lost database at start, not when the webhook next receives
// an event, which for a quiet webhook may be never.
if !e.dbManager.DBExists(webhookID) {
continue
}
e.recoverWebhookDeliveries(ctx, webhookID)
}
}
@@ -1,7 +1,6 @@
package delivery_test
import (
"bytes"
"context"
"encoding/json"
"fmt"
@@ -1135,40 +1134,6 @@ func TestRecoverInFlight_WithPendingDeliveries(
}
}
// TestRecoverInFlight_ReportsAMissingWebhookDatabase covers a webhook
// whose database file is gone, after a partial restore say. Restart
// recovery opens every webhook's database, so the empty one made in its
// place is reported at start, naming the file
// (https://git.eeqj.de/sneak/webhooker/issues/290).
func TestRecoverInFlight_ReportsAMissingWebhookDatabase(t *testing.T) {
t.Parallel()
mainDB := iMainDB(t)
webhookID := uuid.New().String()
iCreateWebhook(t, mainDB, webhookID, "lost-database")
var logs bytes.Buffer
dbMgr := database.NewTestWebhookDBManagerWithLogger(
t.TempDir(), slog.New(slog.NewTextHandler(&logs, nil)),
)
t.Cleanup(func() { _ = dbMgr.CloseAll() })
engine := delivery.NewTestEngineWithDB(
database.NewTestDatabase(mainDB), dbMgr,
slog.New(slog.DiscardHandler),
&http.Client{Timeout: 5 * time.Second}, 1,
)
engine.ExportRecoverInFlight(context.Background())
assert.Contains(
t, logs.String(),
`level=WARN msg="created a new, empty database" webhook_id=`+
webhookID+" path="+dbMgr.DBPath(webhookID),
)
}
// --- HTTP Config with custom headers ---
func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
@@ -0,0 +1,95 @@
package handlers_test
import (
"context"
"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"
"sneak.berlin/go/webhooker/internal/database"
)
// TestHandleEntrypointToggle_DoesNotUndoAnEdit proves that a toggle
// which loaded the entrypoint before an edit of its description was
// saved does not write the old description back over the edit. The
// edit is submitted from a callback on the toggle's own read of the
// entrypoint, so it is saved after that read and before the toggle
// writes.
func TestHandleEntrypointToggle_DoesNotUndoAnEdit(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
wh := seedWebhookWithRetention(t, env.db, 30)
ep := seedEntrypoint(t, env.db, wh.ID)
require.True(t, ep.Active)
router := chi.NewRouter()
router.Post(
"/hook/{sourceID}/entrypoints/{entrypointID}/edit",
env.handlers.HandleEntrypointEdit(),
)
router.Post(
"/hook/{sourceID}/entrypoints/{entrypointID}/toggle",
env.handlers.HandleEntrypointToggle(),
)
// post submits one of the entrypoint's forms as the test user and
// returns the response's status code.
post := func(action string, form url.Values) int {
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost,
"/hook/"+wh.ID+"/entrypoints/"+ep.ID+"/"+action,
strings.NewReader(form.Encode()),
)
req.Header.Set(
"Content-Type", "application/x-www-form-urlencoded",
)
for _, c := range env.cookies {
req.AddCookie(c)
}
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
return w.Code
}
var (
edited bool
editCode int
)
require.NoError(t, env.db.DB().Callback().Query().
After("gorm:query").
Register("test:edit_after_toggle_read", func(tx *gorm.DB) {
// Only the first read of an entrypoint, the toggle's,
// submits the edit.
if tx.Statement.Table != "entrypoints" || edited {
return
}
edited = true
editCode = post(
"edit", url.Values{"description": {"Billing sender"}},
)
}),
)
require.Equal(t, http.StatusSeeOther, post("toggle", nil))
require.Equal(t, http.StatusSeeOther, editCode)
var stored database.Entrypoint
require.NoError(
t, env.db.DB().First(&stored, "id = ?", ep.ID).Error,
)
assert.False(t, stored.Active)
assert.Equal(t, "Billing sender", stored.Description)
}
+2
View File
@@ -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."},
+5 -2
View File
@@ -86,12 +86,13 @@ var errInjectedDelete = errors.New("injected delete failure")
// save of an existing row.
var errInjectedSave = errors.New("injected save failure")
// seedEntrypoint inserts an entrypoint for a webhook.
// seedEntrypoint inserts an active entrypoint for a webhook and
// returns it.
func seedEntrypoint(
t *testing.T,
db *database.Database,
webhookID string,
) {
) *database.Entrypoint {
t.Helper()
ep := &database.Entrypoint{
@@ -104,6 +105,8 @@ func seedEntrypoint(
t,
db.DB().Omit(clause.Associations).Create(ep).Error,
)
return ep
}
// countRows counts the live (not soft-deleted) rows of a model
+70 -2
View File
@@ -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,
+1 -10
View File
@@ -308,7 +308,7 @@ func checkDataDir(dir string) error {
dbPath := filepath.Join(dir, database.MainDBFileName)
dbInfo, err := os.Stat(dbPath)
_, err = os.Stat(dbPath)
switch {
case errors.Is(err, fs.ErrNotExist):
@@ -319,15 +319,6 @@ func checkDataDir(dir string) error {
)
case err != nil:
return fmt.Errorf("checking %s: %w", dbPath, err)
case dbInfo.Size() == 0:
// SQLite opens a zero-length file as an empty database, so
// it holds no deployment either, and opening it would write
// an empty schema into it.
return fmt.Errorf(
"%w: %s is zero-length. The admin account is created by "+
"the first server start",
ErrNoDatabase, dbPath,
)
}
return nil
-27
View File
@@ -377,33 +377,6 @@ func TestMissingDatabaseCreatesNothing(t *testing.T) {
)
}
// TestZeroLengthDatabaseCreatesNothing covers a webhooker.db left at
// zero length, as a truncated copy leaves it. SQLite would open it as
// an empty database, so it is refused like a missing one and left as
// it is.
func TestZeroLengthDatabaseCreatesNothing(t *testing.T) {
dir := t.TempDir()
t.Setenv("DATA_DIR", dir)
dbPath := filepath.Join(dir, database.MainDBFileName)
require.NoError(
t, os.WriteFile(dbPath, nil, database.SQLiteFilePerm),
)
code, _, stderr := run(t, newPassword+"\n", operatorUser)
require.Equal(t, exitFailure, code)
assert.Contains(t, stderr, dbPath)
entries, err := os.ReadDir(dir)
require.NoError(t, err)
assert.Len(t, entries, 1, "nothing may be created beside it")
info, err := os.Stat(dbPath)
require.NoError(t, err)
assert.Zero(t, info.Size(), "nothing may be written into it")
}
// TestUnknownUserFails states the decision: resetpw changes an
// existing account's password and never creates an account. A typo in
// the username must say so rather than quietly adding a second user.
+57
View File
@@ -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,62 @@ 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 and hides until the form closes, so the form always
// opens on the saved 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")
assert.True(t, hidden(ctx, edit),
"Edit stays shown while the edit form is open")
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")
assert.True(t, shown(ctx, edit),
"clicking Cancel does not show Edit 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
+4
View File
@@ -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(),
+152
View File
@@ -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(
@@ -1087,6 +1126,119 @@ 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, or without a session, 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",
)
// The request without a session carries a valid CSRF token from
// the login page, so only the session check can refuse it.
anonToken, anon := env.csrfFrom(t, "/pages/login", nil)
refused := env.post(
action, entrypointEditForm(anonToken, "no session"), anon,
)
assert.Equal(t, http.StatusSeeOther, refused.Code)
assert.Equal(
t, "/pages/login", refused.Header().Get("Location"),
"an edit without a session must be refused",
)
assert.Equal(
t, ep.Description, env.storedEntrypoint(t, ep.ID).Description,
"a refused edit must not change the description",
)
// 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</span>")
assert.Contains(t, body, "/h/"+ep.Path+"</code>")
edit("")
assert.Empty(t, env.storedEntrypoint(t, ep.ID).Description)
assert.Contains(t, env.get(page, cookies).Body.String(),
">Entrypoint</span>", "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
+2 -1
View File
@@ -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,
+13 -2
View File
@@ -54,15 +54,26 @@
<div class="divide-y divide-gray-100">
{{range .Entrypoints}}
<div class="p-4">
<div class="p-4" x-data="collapsible">
<div class="flex flex-wrap items-center justify-between gap-2 mb-1">
<span class="text-sm font-medium text-gray-900">{{if .Description}}{{.Description}}{{else}}Entrypoint{{end}}</span>
<span x-show="closed" class="text-sm font-medium text-gray-900">{{if .Description}}{{.Description}}{{else}}Entrypoint{{end}}</span>
<!-- Edit shows this form in place of the
description and hides until it closes, so
the form always opens on the saved
description. Cancel resets what was typed. -->
<form x-show="open" x-cloak method="POST" action="/hook/{{$.Webhook.ID}}/entrypoints/{{.ID}}/edit" class="flex w-full gap-2">
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
<input type="text" name="description" value="{{.Description}}" placeholder="Description (optional)" class="input text-sm flex-1">
<button type="submit" class="btn-primary text-sm">Save</button>
<button type="reset" @click="toggle" class="btn-secondary text-sm">Cancel</button>
</form>
<div class="flex flex-wrap items-center gap-2">
{{if .Active}}
<span class="badge-success">Active</span>
{{else}}
<span class="badge-error">Inactive</span>
{{end}}
<button type="button" x-show="closed" @click="toggle" class="btn-small" title="Edit">Edit</button>
<form method="POST" action="/hook/{{$.Webhook.ID}}/entrypoints/{{.ID}}/toggle" class="inline">
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
<button type="submit" class="btn-small" title="{{if .Active}}Deactivate{{else}}Activate{{end}}">