6 Commits
Author SHA1 Message Date
clawbot f2dfa9bfac Scope the entrypoint edit check's buttons; fit the type row at 360px
check / check (push) Waiting to run
The browser test's entrypoint edit check clicked any Cancel and Save
on the page, which now also match the add target form's hidden
buttons, so the clicks timed out. It finds them inside the edit form.

The type choice takes p-2 and flex-1 in place of w-32, so it, Next
and Cancel share one row on a 360px-wide phone with "Database" shown
in full.

Model: opus-5-5
2026-10-02 20:01:04 +00:00
sneak 68122e1f95 Give the target type choice a width the stylesheet defines
The type select used w-40, which the committed static/css/tailwind.css
has no rule for, so it took the full width and pushed Next and Cancel
onto the line below. It now uses w-32, as the old type select did, so
the type choice, Next and Cancel sit on one row.

Model: opus-5-5
2026-10-02 19:51:56 +00:00
clawbot bccafd485c Empty the add target form on Cancel; encoding failures stay a 500
The form's reason and values now come from the targetForm component,
loaded from the section's data attributes after a refusal and emptied
by Cancel, which also resets the form. Each type's fields used to be
recreated with the refused values written into the markup, so they
came back after Cancel. The browser test checks this after a refusal.

A target configuration that cannot be encoded is again a logged 500
with the generic error page, on the add and the edit path; only
refusals of submitted values come back on the form.

The README paragraph on the browser test is re-wrapped at 80 columns
and names Cancel at the type step.

Model: opus-5-5
2026-10-02 19:51:56 +00:00
clawbot 93f7a2b0f4 Targets section: one Add, then a type and Next, then its fields (closes #370)
The webhook page's targets section lists only its targets until Add is
clicked. Add shows a choice of target type with Next; Next shows only
that type's fields, with Save and Cancel. The `database` and `log`
types have no URL field, and the `slack` form gains max retries.

A refused target now shows the webhook page again with the form open on
its type, the values entered and the reason, instead of a bare text
page. Target validation returns that message rather than writing the
response; newTarget validates a whole new target for reuse by the
new-webhook page. The edit page still answers a refusal in plain text.

Model: opus-5-5
2026-10-02 19:51:51 +00:00
clawbot faf7ca1a5e Report or refuse each unusable file webhooker reads (closes #290)
check / check (push) Waiting to run
An audit of every file webhooker reads configuration or required state from found cases that carried on silently. A zero-length webhooker.db, and a missing or zero-length per-webhook database, now log the "created a new, empty database" warning naming the file; restart recovery opens every live webhook's database, checking under the manager's lock that it still exists, so a missing one is reported at start. The main database's open errors name webhooker.db, for the server and webhooker resetpw; resetpw refuses a zero-length webhooker.db. A directory in place of any database file or its -wal or -shm is refused naming it. The README says how each case is treated. Also closes #459.

Model: opus-5-5
2026-10-02 21:38:47 +02:00
clawbot 0f5f6ba6bf Let an entrypoint's description be edited in place (closes #392)
check / check (push) Waiting to run
An entrypoint's description was set when it was added and could never change, so renaming one meant deleting it and adding a new one with a new URL every sender had to be given again. Each entrypoint on the webhook page now has an Edit button, in the shared secondary style, that opens its description in place with Save and Cancel and keeps its URL. The save goes through the same login, CSRF and ownership checks as the other entrypoint actions; an empty description shows as "Entrypoint". Activate and deactivate now write only the active column, so they cannot undo an edit. Tests cover each, through the router and the browser.

Model: opus-5-5
2026-10-02 21:32:43 +02:00
29 changed files with 1586 additions and 445 deletions
+65 -27
View File
@@ -79,7 +79,8 @@ 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. A file that is there but cannot be parsed aborts startup
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
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
@@ -564,11 +565,13 @@ 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` also logs
A start that finds no `webhooker.db` in `DATA_DIR`, or a zero-length
one (which SQLite opens as an empty database), 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 `DATA_DIR` was empty, most often because its volume is not
mounted.
line means `webhooker.db` was lost: either the file was missing, most
often because the volume holding `DATA_DIR` is not mounted, or it was
zero-length, as a truncated copy leaves it.
#### Recovering a lost admin password
@@ -612,7 +615,8 @@ 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`, is an error rather than a new empty deployment —
no `webhooker.db` or a zero-length one, is an error naming the path
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.
@@ -993,6 +997,16 @@ 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.
@@ -1076,13 +1090,19 @@ 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 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.
`events-*.db` *and* every `archive-*.db`. A restore that leaves out
`webhooker.db` or an `events-*.db` is reported, not refused; one
that leaves out an `archive-*.db` or a `-wal` (step 3) is not
reported at all. 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.
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
@@ -1324,19 +1344,25 @@ markup. The CSP build runs no expressions, so every Alpine directive in
`x-data="{ open: false }"` or `@click="open = !open"`.
A browser test in `internal/server` loads the webhook page and the event log
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
under the real policy and checks that: the add entrypoint form stays hidden
until Add is clicked; for every target type, the targets section's Add shows
only a choice of type with Next and Cancel, Next shows only that type's fields
(no url field for `database` or `log`), Cancel at either step closes the form,
and saving adds the target; a refused target comes back with its form open, the
values entered and the reason, and after Cancel the next Add starts with an
empty form and no reason; the Copy button beside an entrypoint URL reads
"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, as does leaving the page and going back to it, 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.
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
@@ -1945,10 +1971,19 @@ 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 (and lazily on first access for webhooks that predate this
feature). They are managed by the `WebhookDBManager` component, which
created. 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
@@ -2013,7 +2048,9 @@ 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. An optional `expiry` in the target's config JSON (e.g.
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.
`{"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
@@ -2929,6 +2966,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,6 +4,7 @@ import (
"bytes"
"context"
"log/slog"
"os"
"path/filepath"
"strings"
"testing"
@@ -119,3 +120,26 @@ 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,
)
}
+12 -7
View File
@@ -8,7 +8,6 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"log/slog"
"os"
"path/filepath"
@@ -203,8 +202,7 @@ 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.
_, statErr := os.Stat(dbPath)
created := errors.Is(statErr, fs.ErrNotExist)
created := missingOrEmpty(dbPath)
// Opened through OpenSQLite so this handle carries the same WAL
// journaling, busy timeout, immediate-transaction locking, and pool
@@ -213,13 +211,15 @@ 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
// Then use it with GORM. Its errors are SQLite's alone and name no
// file, so the path is added to them here.
db, err := gorm.Open(sqlite.Dialector{
Conn: sqlDB,
}, &gorm.Config{
@@ -229,10 +229,11 @@ func (d *Database) connectTo(dataDir string) error {
if err != nil {
d.log.Error(
"failed to connect to database",
"path", dbPath,
"error", err,
)
return err
return fmt.Errorf("connecting to %s: %w", dbPath, err)
}
d.db = db
@@ -243,8 +244,12 @@ func (d *Database) connectTo(dataDir string) error {
d.log.Info("connected to database", "path", dbPath)
}
// Run migrations
return d.migrate()
err = d.migrate()
if err != nil {
return fmt.Errorf("migrating %s: %w", dbPath, err)
}
return nil
}
func (d *Database) migrate() error {
+25
View File
@@ -1,9 +1,15 @@
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"
@@ -100,3 +106,22 @@ 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]
// Nothing to reap if the per-webhook database has never
// been created.
// A missing database has nothing to reap. Restart recovery
// reports a lost one (see WebhookDBManager.GetDB).
if !r.dbManager.DBExists(wh.ID) {
continue
}
+23
View File
@@ -182,6 +182,29 @@ 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
+26 -2
View File
@@ -7,6 +7,7 @@ import (
"io/fs"
"net/url"
"os"
"syscall"
"time"
_ "modernc.org/sqlite" // Pure Go SQLite driver
@@ -93,7 +94,8 @@ const (
const SQLiteFilePerm fs.FileMode = 0o600
// reserveSQLiteFile puts path at SQLiteFilePerm before the driver ever
// touches it, and tightens any sidecar already on disk.
// touches it, and tightens any sidecar already on disk. A directory in
// place of any of them is an error naming it.
//
// The mode has to be settled here rather than by a chmod after opening,
// because SQLite picks it: robust_open substitutes
@@ -143,7 +145,15 @@ func reserveSQLiteFile(path string, create bool) error {
for _, p := range append(
[]string{path}, sqliteSidecarPaths(path)...,
) {
err := os.Chmod(p, SQLiteFilePerm)
// 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)
if err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("securing %s: %w", p, err)
}
@@ -152,6 +162,20 @@ 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.
+72 -21
View File
@@ -98,34 +98,37 @@ func NewWebhookDBManager(
return m, nil
}
// GetDB returns the database connection for a webhook,
// creating the database file lazily if it doesn't exist.
// 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.
func (m *WebhookDBManager) GetDB(
webhookID string,
) (*gorm.DB, error) {
// Fast path: already open
if val, ok := m.dbs.Load(webhookID); ok {
return asGormDB(val, webhookID)
}
return m.getDB(webhookID, false)
}
// 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.
// GetDBIf is GetDB, done only when check reports true. check runs under
// the lock DeleteDB holds while it removes the files, so a caller can
// confirm the webhook still exists and open its database with no delete
// in between. The handle is nil when check reports false. check must
// not call the manager.
func (m *WebhookDBManager) GetDBIf(
webhookID string, check func() (bool, error),
) (*gorm.DB, error) {
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 {
ok, err := check()
if err != nil || !ok {
return nil, err
}
m.dbs.Store(webhookID, db)
return db, nil
return m.getDBLocked(webhookID, false)
}
// asGormDB returns a value read from the cache as the database
@@ -143,12 +146,12 @@ func asGormDB(val any, webhookID string) (*gorm.DB, error) {
return db, nil
}
// CreateDB explicitly creates a new per-webhook database file
// and runs migrations.
// CreateDB creates a new webhook's database file and runs
// migrations.
func (m *WebhookDBManager) CreateDB(
webhookID string,
) error {
_, err := m.GetDB(webhookID)
_, err := m.getDB(webhookID, true)
return err
}
@@ -266,6 +269,54 @@ 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)
}
m.mu.Lock()
defer m.mu.Unlock()
return m.getDBLocked(webhookID, isNew)
}
// getDBLocked is getDB's slow path, run with m.mu held. It looks in the
// cache again first: a caller that raced another one to the lock then
// gets its handle instead of opening a second one.
func (m *WebhookDBManager) getDBLocked(
webhookID string, isNew bool,
) (*gorm.DB, error) {
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,6 +289,75 @@ 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()
+25 -5
View File
@@ -699,10 +699,9 @@ func (e *Engine) recoverInFlight(ctx context.Context) {
default:
}
if !e.dbManager.DBExists(webhookID) {
continue
}
// Opened even when its file is missing, so that a lost
// database is reported at start, not when the webhook next
// receives an event, which for a quiet webhook may be never.
e.recoverWebhookDeliveries(ctx, webhookID)
}
}
@@ -710,7 +709,24 @@ func (e *Engine) recoverInFlight(ctx context.Context) {
func (e *Engine) recoverWebhookDeliveries(
ctx context.Context, webhookID string,
) {
webhookDB, err := e.dbManager.GetDB(webhookID)
// The web interface is already serving, so the webhook may have
// been deleted since the list was read. Opening its database then
// would create the file again after the delete removed it.
stillExists := func() (bool, error) {
var count int64
err := e.database.DB().
Model(&database.Webhook{}).
Where("id = ?", webhookID).
Count(&count).Error
if err != nil {
return false, fmt.Errorf("confirming webhook exists: %w", err)
}
return count > 0, nil
}
webhookDB, err := e.dbManager.GetDBIf(webhookID, stillExists)
if err != nil {
e.log.Error(
"failed to get webhook database for recovery",
@@ -721,6 +737,10 @@ func (e *Engine) recoverWebhookDeliveries(
return
}
if webhookDB == nil {
return
}
e.recoverPendingDeliveries(
ctx, webhookDB, webhookID,
)
@@ -1,6 +1,7 @@
package delivery_test
import (
"bytes"
"context"
"encoding/json"
"fmt"
@@ -1136,6 +1137,85 @@ 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),
)
}
// TestRecoverInFlight_SkipsAWebhookDeletedAfterTheListIsRead covers a
// webhook deleted from the web interface while restart recovery runs.
// Its database file is gone, and recovery must not create it again.
func TestRecoverInFlight_SkipsAWebhookDeletedAfterTheListIsRead(
t *testing.T,
) {
t.Parallel()
mainDB := iMainDB(t)
webhookID := uuid.New().String()
iCreateWebhook(t, mainDB, webhookID, "deleted-during-recovery")
// The first query to return is recovery's read of the list of
// webhooks. Deleting the webhook right after it puts the delete
// between that read and the opening of the webhook's database.
deleted := false
require.NoError(t, mainDB.Callback().Query().After("gorm:query").
Register("delete-after-list", func(*gorm.DB) {
if deleted {
return
}
deleted = true
require.NoError(t, mainDB.Delete(
&database.Webhook{}, "id = ?", webhookID,
).Error)
}))
dbMgr := database.NewTestWebhookDBManager(t.TempDir())
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())
require.True(t, deleted)
assert.False(t, dbMgr.DBExists(webhookID))
}
// --- HTTP Config with custom headers ---
func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
+4
View File
@@ -573,6 +573,8 @@ func TestRecoverPending_TargetDeleted(t *testing.T) {
s := newISetup(t)
iCreateWebhook(t, s.MainDB, s.WebhookID, "pending-recovery")
deliveryID := tSeedDeletedTarget(
t, s, "gone-while-pending", "http://example.com/hook",
database.DeliveryStatusPending,
@@ -612,6 +614,8 @@ func TestRecoverPending_TargetDeleted_LeavesAnOwnedDeliveryAlone(
s := newISetup(t)
iCreateWebhook(t, s.MainDB, s.WebhookID, "owned-recovery")
deliveryID := tSeedDeletedTarget(
t, s, "gone-but-owned", "http://example.com/hook",
database.DeliveryStatusPending,
@@ -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)
}
+9 -12
View File
@@ -137,22 +137,20 @@ func (s *Handlers) RenderTemplateForTest(
// BuildSlackTargetConfigForTest exposes
// buildSlackTargetConfig for use in the handlers_test package.
func (s *Handlers) BuildSlackTargetConfigForTest(
w http.ResponseWriter,
r *http.Request,
ctx context.Context,
targetURL string,
) (string, error) {
return s.buildSlackTargetConfig(w, r, targetURL)
) (string, string, error) {
return s.buildSlackTargetConfig(ctx, targetURL)
}
// BuildHTTPTargetConfigForTest exposes buildHTTPTargetConfig
// for use in the handlers_test package, taking the form fields
// an HTTP target's configuration is built from.
func (s *Handlers) BuildHTTPTargetConfigForTest(
w http.ResponseWriter,
r *http.Request,
ctx context.Context,
targetURL, headers, timeout string,
) (string, error) {
return s.buildHTTPTargetConfig(w, r, targetFormInput{
) (string, string, error) {
return s.buildHTTPTargetConfig(ctx, targetFormInput{
URL: targetURL,
Headers: headers,
Timeout: timeout,
@@ -162,9 +160,8 @@ func (s *Handlers) BuildHTTPTargetConfigForTest(
// BuildDatabaseTargetConfigForTest exposes
// buildDatabaseTargetConfig for use in the handlers_test
// package.
func (s *Handlers) BuildDatabaseTargetConfigForTest(
w http.ResponseWriter,
func BuildDatabaseTargetConfigForTest(
expiry string,
) (string, error) {
return s.buildDatabaseTargetConfig(w, newRequestForTest(), expiry)
) (string, string, error) {
return buildDatabaseTargetConfig(expiry)
}
+19 -42
View File
@@ -314,16 +314,12 @@ func TestBuildSlackTargetConfig_AcceptsPublicURL(t *testing.T) {
t.Cleanup(app.RequireStop)
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, "/", nil)
w := httptest.NewRecorder()
cfg, err := h.BuildSlackTargetConfigForTest(
w, req, "http://93.184.216.34/services/T00/B00/xxx",
cfg, errMsg, err := h.BuildSlackTargetConfigForTest(
t.Context(), "http://93.184.216.34/services/T00/B00/xxx",
)
require.NoError(t, err)
assert.Equal(t, http.StatusOK, w.Code)
assert.Empty(t, errMsg)
assert.Contains(t, cfg, "webhookUrl")
}
@@ -337,17 +333,13 @@ func TestBuildSlackTargetConfig_RejectsReservedURL(t *testing.T) {
t.Cleanup(app.RequireStop)
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, "/", nil)
w := httptest.NewRecorder()
cfg, err := h.BuildSlackTargetConfigForTest(
w, req, "http://169.254.169.254/latest/meta-data/",
cfg, errMsg, err := h.BuildSlackTargetConfigForTest(
t.Context(), "http://169.254.169.254/latest/meta-data/",
)
require.Error(t, err)
require.NoError(t, err)
assert.Contains(t, errMsg, "Invalid target URL")
assert.Empty(t, cfg)
assert.Equal(t, http.StatusBadRequest, w.Code)
}
func TestRenderTemplate(t *testing.T) {
@@ -444,29 +436,22 @@ func TestRenderTemplateMidRenderErrorSendsNoPartialBody(t *testing.T) {
func TestBuildDatabaseTargetConfig_Valid(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
app := newTestApp(t, &h)
app.RequireStart()
t.Cleanup(app.RequireStop)
// Empty expiry: the keep-forever default, empty config.
w := httptest.NewRecorder()
cfg, err := h.BuildDatabaseTargetConfigForTest(w, "")
cfg, errMsg, err := handlers.BuildDatabaseTargetConfigForTest("")
require.NoError(t, err)
assert.Empty(t, errMsg)
assert.Empty(t, cfg)
// Explicit never is stored as config.
w = httptest.NewRecorder()
cfg, err = h.BuildDatabaseTargetConfigForTest(w, "never")
cfg, errMsg, err = handlers.BuildDatabaseTargetConfigForTest("never")
require.NoError(t, err)
assert.Empty(t, errMsg)
assert.JSONEq(t, `{"expiry":"never"}`, cfg)
// A positive duration is stored as config.
w = httptest.NewRecorder()
cfg, err = h.BuildDatabaseTargetConfigForTest(w, "720h")
cfg, errMsg, err = handlers.BuildDatabaseTargetConfigForTest("720h")
require.NoError(t, err)
assert.Empty(t, errMsg)
assert.JSONEq(t, `{"expiry":"720h"}`, cfg)
}
@@ -475,22 +460,14 @@ func TestBuildDatabaseTargetConfig_RejectsBadExpiry(
) {
t.Parallel()
var h *handlers.Handlers
app := newTestApp(t, &h)
app.RequireStart()
t.Cleanup(app.RequireStop)
for _, bad := range []string{"nonsense", "7d", "-5h"} {
w := httptest.NewRecorder()
cfg, err := h.BuildDatabaseTargetConfigForTest(w, bad)
cfg, errMsg, err := handlers.BuildDatabaseTargetConfigForTest(bad)
require.Error(t, err, "expiry %q", bad)
assert.Empty(t, cfg)
assert.Equal(
t, http.StatusBadRequest, w.Code,
"expiry %q should be rejected with 400", bad,
require.NoError(t, err)
assert.Contains(
t, errMsg, "Invalid archive expiry",
"expiry %q should be refused", bad,
)
assert.Empty(t, cfg)
}
}
+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
+213 -161
View File
@@ -1,6 +1,7 @@
package handlers
import (
"context"
"encoding/json"
"errors"
"fmt"
@@ -38,9 +39,6 @@ type WebhookListItem struct {
EventsUnreadable bool
}
// errMissingURL signals that a required URL was not provided.
var errMissingURL = errors.New("missing URL")
// parseRetentionDays interprets a retention_days form value. It
// returns the number of days, or, for a value it refuses, the message
// the create and edit forms show; the message is empty when the value
@@ -460,15 +458,20 @@ func (h *Handlers) HandleSourceDetail() http.HandlerFunc {
return
}
h.renderSourceDetail(w, r, webhook)
h.renderSourceDetail(w, r, webhook, targetFormInput{}, "")
}
}
// renderSourceDetail loads and renders a source detail page.
// renderSourceDetail loads and renders a source detail page. With a
// targetErr, it is the page shown again for a refused add target
// form: it answers 400, and the form opens on targetForm's type with
// its values and the message.
func (h *Handlers) renderSourceDetail(
w http.ResponseWriter,
r *http.Request,
webhook database.Webhook,
targetForm targetFormInput,
targetErr string,
) {
var entrypoints []database.Entrypoint
@@ -525,9 +528,16 @@ func (h *Handlers) renderSourceDetail(
"Events": events,
"BaseURL": baseURL,
"Stats": h.loadWebhookStats(webhook.ID, entrypoints, targets),
"TargetForm": targetForm,
"TargetError": targetErr,
}
h.renderTemplate(w, r, "source_detail.html", data)
status := http.StatusOK
if targetErr != "" {
status = http.StatusBadRequest
}
h.renderTemplateStatus(w, r, "source_detail.html", data, status)
}
// HandleSourceEdit shows the form to edit a webhook.
@@ -1396,6 +1406,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) {
@@ -1437,64 +1511,27 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
}
}
// processTargetCreate validates and creates a new target.
// processTargetCreate validates and creates a new target. A refused
// submission shows the webhook page again, with the add target form
// open on the chosen type, the values entered, and the reason.
func (h *Handlers) processTargetCreate(
w http.ResponseWriter,
r *http.Request,
webhook database.Webhook,
) {
// The body size cap is enforced by the MaxBodySize middleware,
// which runs before CSRF parses the form.
//
// Every field here is read with PostFormValue, not FormValue.
// FormValue falls back to the query string, which would let
// `POST /hook/{id}/targets?url=https://hooks.slack.com/...`
// configure a target from a value the request line carries — and
// the request line, unlike the body, is what logs, proxies,
// Referer headers and error trackers record.
name := r.PostFormValue("name")
targetType := database.TargetType(r.PostFormValue("type"))
in := targetFormInputFrom(r)
if name == "" {
http.Error(
w, "Name is required", http.StatusBadRequest,
)
return
}
if !isValidTargetType(targetType) {
http.Error(
w, "Invalid target type",
http.StatusBadRequest,
)
return
}
configJSON, err := h.buildTargetConfig(
w, r, targetType, targetFormInputFrom(r),
)
target, errMsg, err := h.newTarget(r.Context(), webhook.ID, in)
if err != nil {
h.serverError(w, r, "failed to encode target config", err)
return
}
// A new target has no stored retry count, so an absent field
// takes the fire-and-forget default. A field the operator filled
// in with something invalid is rejected rather than becoming
// that default.
maxRetries, ok := targetMaxRetries(w, r, 0)
if !ok {
return
}
if errMsg != "" {
h.renderSourceDetail(w, r, webhook, in, errMsg)
target := &database.Target{
WebhookID: webhook.ID,
Name: name,
Type: targetType,
Active: true,
Config: configJSON,
MaxRetries: maxRetries,
return
}
err = h.db.DB().Create(target).Error
@@ -1510,6 +1547,49 @@ func (h *Handlers) processTargetCreate(
)
}
// newTarget validates a new target for a webhook and returns the row
// to create, or, when it refuses the target, the message the form
// shows. An error is the server's fault, not a refusal: the accepted
// configuration could not be encoded. Every form that creates a
// target goes through here, so they all accept and refuse the same
// things.
func (h *Handlers) newTarget(
ctx context.Context,
webhookID string,
in targetFormInput,
) (*database.Target, string, error) {
if in.Name == "" {
return nil, "Name is required", nil
}
if !isValidTargetType(in.Type) {
return nil, "Invalid target type", nil
}
configJSON, errMsg, err := h.buildTargetConfig(ctx, in.Type, in)
if err != nil || errMsg != "" {
return nil, errMsg, err
}
// A new target has no stored retry count, so an absent field
// takes the fire-and-forget default. A field the operator filled
// in with something invalid is refused rather than becoming
// that default.
maxRetries, err := parseMaxRetries(in.MaxRetries, 0)
if err != nil {
return nil, "Invalid max retries: " + retriesErrorMessage(err), nil
}
return &database.Target{
WebhookID: webhookID,
Name: in.Name,
Type: in.Type,
Active: true,
Config: configJSON,
MaxRetries: maxRetries,
}, "", nil
}
// isValidTargetType checks whether the target type is supported.
func isValidTargetType(tt database.TargetType) bool {
switch tt {
@@ -1541,11 +1621,16 @@ func pageOrFirst(s string) int {
return v
}
// targetFormInput carries the raw form values describing a target's
// configuration. Both the create and the edit path fill one and hand
// it to buildTargetConfig, so neither can come to validate a
// destination differently from the other.
// targetFormInput carries the raw values of a target form. Both the
// create and the edit path fill one and hand it to buildTargetConfig,
// so neither can come to validate a destination differently from the
// other. A refused add target form is shown again from it.
type targetFormInput struct {
// Name is the target's name.
Name string
// Type is the type chosen on the add target form. The edit form
// has none: a target's stored type decides.
Type database.TargetType
// URL is the destination for an HTTP target and the webhook URL
// for a Slack target.
URL string
@@ -1554,13 +1639,15 @@ type targetFormInput struct {
Headers string
// Timeout is an HTTP target's per-request timeout in seconds.
Timeout string
// MaxRetries is an HTTP or Slack target's max_retries.
MaxRetries string
// Expiry is a database (archive) target's row expiry.
Expiry string
}
// targetFormInputFrom reads the configuration fields from a request
// body. The body size cap is enforced by the MaxBodySize middleware,
// which runs before CSRF parses the form.
// targetFormInputFrom reads a target form from a request body. The
// body size cap is enforced by the MaxBodySize middleware, which runs
// before CSRF parses the form.
//
// Every field is read with PostFormValue, not FormValue. FormValue
// falls back to the query string, which would let
@@ -1572,38 +1659,38 @@ type targetFormInput struct {
// tokens.
func targetFormInputFrom(r *http.Request) targetFormInput {
return targetFormInput{
URL: r.PostFormValue("url"),
Headers: r.PostFormValue("headers"),
Timeout: r.PostFormValue("timeout"),
Expiry: r.PostFormValue("expiry"),
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"),
}
}
// buildTargetConfig builds the JSON config string for a target from
// the submitted form values, writing its own 4xx response on
// rejection. Which fields of in apply depends on the target type.
// the submitted form values, or returns the message the form shows
// for a value it refuses. An error is the server's fault, not a
// refusal: the accepted configuration could not be encoded. Which
// fields of in apply depends on the target type; a type without a URL
// ignores any URL submitted.
func (h *Handlers) buildTargetConfig(
w http.ResponseWriter,
r *http.Request,
ctx context.Context,
targetType database.TargetType,
in targetFormInput,
) (string, error) {
) (string, string, error) {
switch targetType {
case database.TargetTypeHTTP:
return h.buildHTTPTargetConfig(w, r, in)
return h.buildHTTPTargetConfig(ctx, in)
case database.TargetTypeSlack:
return h.buildSlackTargetConfig(w, r, in.URL)
return h.buildSlackTargetConfig(ctx, in.URL)
case database.TargetTypeDatabase:
return h.buildDatabaseTargetConfig(w, r, in.Expiry)
return buildDatabaseTargetConfig(in.Expiry)
case database.TargetTypeLog:
return "", nil
return "", "", nil
default:
http.Error(
w, "Invalid target type",
http.StatusBadRequest,
)
return "", errMissingURL
return "", "Invalid target type", nil
}
}
@@ -1611,92 +1698,73 @@ func (h *Handlers) buildTargetConfig(
// SSRF-validated destination plus the optional headers and timeout
// the delivery path honours.
func (h *Handlers) buildHTTPTargetConfig(
w http.ResponseWriter,
r *http.Request,
ctx context.Context,
in targetFormInput,
) (string, error) {
err := h.validateTargetURL(
w, r, in.URL, "URL is required for HTTP targets",
) (string, string, error) {
errMsg := h.validateTargetURL(
ctx, in.URL, "URL is required for HTTP targets",
)
if err != nil {
return "", err
if errMsg != "" {
return "", errMsg, nil
}
headers, err := delivery.ParseTargetHeaders(in.Headers)
if err != nil {
http.Error(
w,
"Invalid headers: "+err.Error(),
http.StatusBadRequest,
)
return "", err
return "", fmt.Sprintf("Invalid headers: %v", err), nil
}
timeout, err := delivery.ParseTargetTimeout(in.Timeout)
if err != nil {
http.Error(
w,
"Invalid timeout: "+err.Error(),
http.StatusBadRequest,
)
return "", err
return "", fmt.Sprintf("Invalid timeout: %v", err), nil
}
return h.marshalTargetConfig(w, r, delivery.HTTPTargetConfig{
configJSON, err := marshalTargetConfig(delivery.HTTPTargetConfig{
URL: in.URL,
Headers: headers,
Timeout: timeout,
})
return configJSON, "", err
}
// buildSlackTargetConfig builds config JSON for a Slack target,
// whose whole configuration is one SSRF-validated webhook URL.
func (h *Handlers) buildSlackTargetConfig(
w http.ResponseWriter,
r *http.Request,
ctx context.Context,
targetURL string,
) (string, error) {
err := h.validateTargetURL(
w, r, targetURL,
) (string, string, error) {
errMsg := h.validateTargetURL(
ctx, targetURL,
"Webhook URL is required for Slack targets",
)
if err != nil {
return "", err
if errMsg != "" {
return "", errMsg, nil
}
return h.marshalTargetConfig(w, r, delivery.SlackTargetConfig{
configJSON, err := marshalTargetConfig(delivery.SlackTargetConfig{
WebhookURL: targetURL,
})
return configJSON, "", err
}
// validateTargetURL rejects an empty or SSRF-blocked destination,
// writing the 400 itself. missingMsg is the error shown when no URL
// is given.
// validateTargetURL refuses an empty or SSRF-blocked destination,
// returning the message the form shows, or "" when the destination
// is accepted. missingMsg is the message for no URL at all.
//
// It is the single point at which a user-supplied destination enters
// the SSRF guard, on create and on edit alike. An edit path that
// reached storage without passing through here would reopen the hole
// the guard closes.
func (h *Handlers) validateTargetURL(
w http.ResponseWriter,
r *http.Request,
ctx context.Context,
targetURL, missingMsg string,
) error {
) string {
if targetURL == "" {
http.Error(
w,
missingMsg,
http.StatusBadRequest,
)
return errMissingURL
return missingMsg
}
err := h.ssrf.ValidateTargetURL(
r.Context(), targetURL,
)
err := h.ssrf.ValidateTargetURL(ctx, targetURL)
if err != nil {
// The submitted URL can be a credential (a Slack
// incoming webhook URL is a bearer token), so the log
@@ -1722,25 +1790,16 @@ func (h *Handlers) validateTargetURL(
"egress to your own network\" in the README)."
}
http.Error(w, msg, http.StatusBadRequest)
return err
return msg
}
return nil
return ""
}
// marshalTargetConfig serialises a target configuration for storage,
// writing a 500 itself if it cannot.
func (h *Handlers) marshalTargetConfig(
w http.ResponseWriter,
r *http.Request,
cfg any,
) (string, error) {
// marshalTargetConfig serialises a target configuration for storage.
func marshalTargetConfig(cfg any) (string, error) {
configBytes, err := json.Marshal(cfg)
if err != nil {
h.serverError(w, r, "failed to encode target config", err)
return "", err
}
@@ -1748,35 +1807,24 @@ func (h *Handlers) marshalTargetConfig(
}
// buildDatabaseTargetConfig builds config JSON for a database
// (archive) target. The optional expiry (a form value read by
// the caller, which bounds the request body) is validated here,
// at creation time, so an unparseable value is rejected with a
// 400 instead of failing every subsequent delivery. An empty
// expiry yields an empty config (the keep-forever default).
func (h *Handlers) buildDatabaseTargetConfig(
w http.ResponseWriter,
r *http.Request,
expiry string,
) (string, error) {
// (archive) target. The optional expiry is validated here, at
// creation time, so an unparseable value is refused instead of
// failing every subsequent delivery. An empty expiry yields an
// empty config (the keep-forever default).
func buildDatabaseTargetConfig(expiry string) (string, string, error) {
expiry = strings.TrimSpace(expiry)
if expiry == "" {
return "", nil
return "", "", nil
}
err := delivery.ValidateArchiveExpiry(expiry)
if err != nil {
http.Error(
w,
"Invalid archive expiry: "+err.Error(),
http.StatusBadRequest,
)
return "", err
return "", fmt.Sprintf("Invalid archive expiry: %v", err), nil
}
return h.marshalTargetConfig(
w, r, map[string]any{"expiry": expiry},
)
configJSON, err := marshalTargetConfig(map[string]any{"expiry": expiry})
return configJSON, "", err
}
// HandleEntrypointDelete handles deleting an entrypoint.
@@ -1877,9 +1925,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,
+154
View File
@@ -0,0 +1,154 @@
package handlers_test
import (
"html"
"net/http"
"net/url"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
)
// TestHandleTargetCreate_EveryType adds a target of each type. Each
// submission carries a url: only the http and slack types store one.
func TestHandleTargetCreate_EveryType(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
// fields is the rest of each submission, as a query string.
cases := []struct {
targetType database.TargetType
fields string
wantConfig string
wantRetries int
}{
{
database.TargetTypeHTTP, "timeout=12&max_retries=3",
`{"url":"` + editOriginalURL + `","timeout":12}`, 3,
},
{
database.TargetTypeSlack, "max_retries=4",
`{"webhookUrl":"` + editOriginalURL + `"}`, 4,
},
{
database.TargetTypeDatabase, "expiry=720h",
`{"expiry":"720h"}`, 0,
},
{database.TargetTypeLog, "", "", 0},
}
for _, tc := range cases {
t.Run(string(tc.targetType), func(t *testing.T) {
t.Parallel()
webhook := seedWebhookWithRetention(t, env.db, 30)
form, err := url.ParseQuery(tc.fields)
require.NoError(t, err)
form.Set("name", "every-type")
form.Set("type", string(tc.targetType))
form.Set("url", editOriginalURL)
w := serveTarget(
env, http.MethodPost,
"/hook/"+webhook.ID+"/targets", form,
)
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
targets := targetsForWebhook(t, env.db, webhook.ID)
require.Len(t, targets, 1)
assert.Equal(t, tc.targetType, targets[0].Type)
assert.Equal(t, tc.wantRetries, targets[0].MaxRetries)
if tc.wantConfig == "" {
assert.Empty(t, targets[0].Config)
} else {
assert.JSONEq(t, tc.wantConfig, targets[0].Config)
}
})
}
}
// TestHandleTargetCreate_RefusedFormComesBack refuses a target of each
// type and checks that the webhook page comes back with the add target
// form open on that type, the values entered, and the reason.
func TestHandleTargetCreate_RefusedFormComesBack(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
// fields is what the operator typed, as a query string.
cases := []struct {
targetType database.TargetType
fields string
reason string
}{
{
database.TargetTypeHTTP,
"name=private&url=" + editBlockedURL +
"&timeout=12&max_retries=3",
"Invalid target URL",
},
{
database.TargetTypeSlack, "name=no-url&max_retries=4",
"Webhook URL is required for Slack targets",
},
{
database.TargetTypeDatabase, "name=archive&expiry=7d",
"Invalid archive expiry",
},
{database.TargetTypeLog, "name=", "Name is required"},
}
for _, tc := range cases {
t.Run(string(tc.targetType), func(t *testing.T) {
t.Parallel()
webhook := seedWebhookWithRetention(t, env.db, 30)
typed, err := url.ParseQuery(tc.fields)
require.NoError(t, err)
form := url.Values{}
form.Set("type", string(tc.targetType))
for field := range typed {
form.Set(field, typed.Get(field))
}
w := serveTarget(
env, http.MethodPost,
"/hook/"+webhook.ID+"/targets", form,
)
assert.Equal(t, http.StatusBadRequest, w.Code)
page := w.Body.String()
assert.Contains(
t, page, `data-type="`+string(tc.targetType)+`"`,
)
assert.Contains(t, page, html.EscapeString(tc.reason))
// Each value comes back in a data attribute of the targets
// section named after its field (max_retries as
// data-max-retries), except url, which comes back in
// data-destination; templates/source_detail.html says why.
for field := range typed {
attr := "data-" + strings.ReplaceAll(field, "_", "-")
if field == "url" {
attr = "data-destination"
}
assert.Contains(
t, page, attr+`="`+
html.EscapeString(typed.Get(field))+`"`,
)
}
assert.Empty(t, targetsForWebhook(t, env.db, webhook.ID))
})
}
}
+11 -6
View File
@@ -120,18 +120,23 @@ func (h *Handlers) applyTargetEdit(
) {
name := r.PostFormValue("name")
if name == "" {
http.Error(
w, "Name is required", http.StatusBadRequest,
)
http.Error(w, "Name is required", http.StatusBadRequest)
return
}
configJSON, err := h.buildTargetConfig(
w, r, target.Type, targetFormInputFrom(r),
configJSON, errMsg, err := h.buildTargetConfig(
r.Context(), target.Type, targetFormInputFrom(r),
)
if err != nil {
// buildTargetConfig has already written the response.
h.serverError(w, r, "failed to encode target config", err)
return
}
if errMsg != "" {
http.Error(w, errMsg, http.StatusBadRequest)
return
}
@@ -1,6 +1,7 @@
package handlers_test
import (
"html"
"net/http"
"net/url"
"testing"
@@ -43,12 +44,16 @@ func TestTargetRefusal_PrivateDestinationSaysHowToAllowIt(
form.Set("type", string(targetType))
form.Set("url", editBlockedURL)
// A refused add shows the webhook page again, where
// the hint is HTML-escaped; a refused edit answers in
// plain text.
added := serveTarget(
env, http.MethodPost, targetsPath, form,
)
assert.Equal(t, http.StatusBadRequest, added.Code)
assert.Contains(
t, added.Body.String(), privateRefusalHint,
t, added.Body.String(),
html.EscapeString(privateRefusalHint),
)
form.Set("url", editOriginalURL)
+5 -4
View File
@@ -90,13 +90,14 @@ func retriesErrorMessage(err error) string {
", or 0 for fire-and-forget"
}
// targetMaxRetries reads and validates max_retries from a target form
// targetMaxRetries reads and validates max_retries from a target edit
// submission, answering the request with a 400 and reporting false
// when the value is set but invalid.
//
// Both the create and the edit path go through here, so the two
// cannot come to disagree about what a valid retry count is. The
// wording matches the timeout control on the same submission.
// It and the create path (newTarget) both use parseMaxRetries and
// retriesErrorMessage, so the two cannot come to disagree about what a
// valid retry count is. The wording matches the timeout control on
// the same submission.
func targetMaxRetries(
w http.ResponseWriter,
r *http.Request,
+10 -1
View File
@@ -308,7 +308,7 @@ func checkDataDir(dir string) error {
dbPath := filepath.Join(dir, database.MainDBFileName)
_, err = os.Stat(dbPath)
dbInfo, err := os.Stat(dbPath)
switch {
case errors.Is(err, fs.ErrNotExist):
@@ -319,6 +319,15 @@ 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,6 +377,33 @@ 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.
+282 -86
View File
@@ -83,9 +83,39 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
page := srv.URL + "/hook/" + webhook.ID
checkAddForms(ctx, t, page)
checkTargetType(ctx, t, page+"/events")
checkAddEntrypoint(ctx, t, page)
// Each target type, with the fields its add target form submits, in
// page order. Only http and slack have a url field.
targetTypes := []struct {
name string
fields string
values map[string]string
}{
{
"http", "csrf_token name type url headers timeout max_retries",
map[string]string{"url": publicTargetURL},
},
{
"slack", "csrf_token name type url max_retries",
map[string]string{"url": publicTargetURL},
},
{
"database", "csrf_token name type expiry",
map[string]string{"expiry": "720h"},
},
{"log", "csrf_token name type", nil},
}
for _, tt := range targetTypes {
checkAddTarget(
ctx, t, page, tt.name, strings.Fields(tt.fields), tt.values,
)
}
checkRefusedTarget(ctx, t, page)
checkCopy(ctx, t, page)
checkEntrypointEdit(ctx, t, page, page+"/events")
checkEventLog(ctx, t, page+"/events", event.ID, target.Name)
checkMobileMenu(ctx, t, page)
@@ -227,115 +257,194 @@ func click(ctx context.Context, t *testing.T, xpath string) {
))
}
// checkAddForms loads a webhook page and checks that each section's add
// form stays hidden until the Add button beside its heading is clicked.
// The click looks for a button element there, so it also checks that
// Add is one.
func checkAddForms(ctx context.Context, t *testing.T, url string) {
// checkAddEntrypoint loads a webhook page and checks that the add
// entrypoint form stays hidden until the Add button beside its heading
// is clicked. The click looks for a button element there, so it also
// checks that Add is one.
func checkAddEntrypoint(ctx context.Context, t *testing.T, url string) {
t.Helper()
form := `form[action$="/entrypoints"]`
require.NoError(t, chromedp.Run(ctx, loadPage(url)))
assert.True(t, hidden(ctx, form),
"the add entrypoint form shows before Add is clicked")
click(ctx, t, `//h2[text()="Entrypoints"]/following-sibling::button`)
assert.True(t, shown(ctx, form),
"the add entrypoint form stays hidden when Add is clicked")
}
// publicTargetURL is a destination the server accepts for an http or
// slack target. It is a literal public address, so accepting it needs
// no DNS.
const publicTargetURL = "https://93.184.216.34/hook"
// The parts of the targets section's add target form the checks below
// find and click. Add is the button beside the Targets heading; each
// Cancel is found from the button beside it, since both are on the
// page at once.
const (
addTarget = `//h2[text()="Targets"]/following-sibling::button`
typeSelect = `//select[@aria-label="Target type"]`
nextButton = `//button[text()="Next"]`
cancelChoice = nextButton + `/following-sibling::button[text()="Cancel"]`
saveButton = `//form[contains(@action, "/targets")]//button[text()="Save"]`
cancelFields = saveButton + `/following-sibling::button[text()="Cancel"]`
targetName = `form[action$="/targets"] input[name="name"]`
submittedKeys = `[...new FormData(
document.querySelector('form[action$="/targets"]')).keys()]`
)
// checkAddTarget loads a webhook page and walks the add target form for
// one target type. The form shows nothing until Add is clicked; Add
// shows only the type choice; Cancel there closes it; Next shows the
// type's own fields in place of the choice, and the form then submits
// exactly fields, so a field another type uses, such as url, is absent;
// Cancel closes it again. It then adds a target of the type, filling in
// values, and checks that the section lists it with that type.
func checkAddTarget(
ctx context.Context,
t *testing.T,
url, targetType string,
fields []string,
values map[string]string,
) {
t.Helper()
require.NoError(t, chromedp.Run(ctx, loadPage(url)))
sections := []struct{ heading, form string }{
{"Entrypoints", `form[action$="/entrypoints"]`},
{"Targets", `form[action$="/targets"]`},
assert.Truef(t, hidden(ctx, typeSelect),
"%s: the type choice shows before Add is clicked", targetType)
assert.Truef(t, hidden(ctx, targetName),
"%s: the fields show before Add is clicked", targetType)
click(ctx, t, addTarget)
assert.Truef(t, shown(ctx, typeSelect),
"%s: Add does not show the type choice", targetType)
assert.Truef(t, hidden(ctx, targetName),
"%s: Add shows the fields before Next", targetType)
click(ctx, t, cancelChoice)
assert.Truef(t, hidden(ctx, typeSelect),
"%s: Cancel does not close the type choice", targetType)
chooseTargetType(ctx, t, targetType)
var submitted []string
require.NoError(t, chromedp.Run(
ctx, chromedp.Evaluate(submittedKeys, &submitted),
))
assert.Equalf(t, fields, submitted,
"%s: the form does not submit exactly the type's fields", targetType)
click(ctx, t, cancelFields)
assert.Truef(t, hidden(ctx, targetName),
"%s: Cancel does not close the fields", targetType)
assert.Truef(t, shown(ctx, addTarget),
"%s: Add does not come back after Cancel", targetType)
name := "added-" + targetType
chooseTargetType(ctx, t, targetType)
require.NoError(t, chromedp.Run(
ctx, chromedp.SetValue(targetName, name, chromedp.ByQuery),
))
for field, value := range values {
require.NoError(t, chromedp.Run(ctx, chromedp.SetValue(
`form[action$="/targets"] [name="`+field+`"]`, value,
chromedp.ByQuery,
)))
}
for _, s := range sections {
assert.Truef(
t, hidden(ctx, s.form),
"%s: the add form shows before Add is clicked", s.heading,
)
click(ctx, t, `//h2[text()="`+s.heading+
`"]/following-sibling::button`)
assert.Truef(
t, shown(ctx, s.form),
"%s: the add form stays hidden when Add is clicked", s.heading,
)
}
click(ctx, t, saveButton)
assert.Truef(t, shown(ctx, `//span[text()="`+name+
`"]/following-sibling::div/span[text()="`+targetType+`"]`),
"%s: the added target is not listed with its type", targetType)
}
// checkTargetType chooses Slack in the open add target form and checks
// what the form would then submit: one url field, the Slack one, and
// not the HTTP url, headers or timeout, which are hidden and disabled.
//
// It then opens the page at elsewhere and goes back. The browser loads
// the webhook page again and restores the form as it was left, Slack
// chosen, without a change event; the form must again show and submit
// Slack's fields, not the HTTP ones.
func checkTargetType(ctx context.Context, t *testing.T, elsewhere string) {
// chooseTargetType clicks Add, picks targetType and clicks Next, and
// checks that the type's fields then show in place of the type choice.
func chooseTargetType(ctx context.Context, t *testing.T, targetType string) {
t.Helper()
click(ctx, t, addTarget)
require.NoError(t, chromedp.Run(
ctx, chromedp.SetValue(typeSelect, targetType, chromedp.BySearch),
))
click(ctx, t, nextButton)
assert.Truef(t, shown(ctx, targetName),
"%s: Next does not show the fields", targetType)
assert.Truef(t, hidden(ctx, typeSelect),
"%s: Next leaves the type choice showing", targetType)
assert.Truef(t, hidden(ctx, addTarget),
"%s: Add still shows while the form is open", targetType)
}
// checkRefusedTarget submits an http target the server refuses, a
// loopback destination, and checks that the page comes back with the
// form open on the http fields, the values entered and the reason, and
// that after Cancel the next Add starts with an empty form and no
// reason.
func checkRefusedTarget(ctx context.Context, t *testing.T, url string) {
t.Helper()
const (
chooseSlack = `(() => {
const type = document.querySelector('select[name="type"]');
type.value = "slack";
type.dispatchEvent(new Event("change"));
})()`
chosen = `document.querySelector('select[name="type"]').value`
howLoaded = `performance.getEntriesByType("navigation")[0].type`
submitted = `[...new FormData(
document.querySelector('form[action$="/targets"]')).keys()]`
slackURL = `input[placeholder^="https://hooks.slack.com/"]`
httpURL = `input[placeholder="https://example.com/webhook"]`
refusedURL = "http://127.0.0.1/hook"
urlField = `form[action$="/targets"] input[name="url"]`
reason = `//div[@class="alert-error"]`
)
slackFields := strings.Fields("csrf_token name type max_retries url")
require.NoError(t, chromedp.Run(ctx, loadPage(url)))
var fields []string
chooseTargetType(ctx, t, "http")
require.NoError(t, chromedp.Run(
ctx,
chromedp.SetValue(targetName, "refused", chromedp.ByQuery),
chromedp.SetValue(urlField, refusedURL, chromedp.ByQuery),
))
click(ctx, t, saveButton)
assert.True(t, shown(ctx, reason),
"a refused target does not show the reason")
var name, typed string
require.NoError(t, chromedp.Run(
ctx,
chromedp.Evaluate(chooseSlack, nil),
chromedp.Evaluate(submitted, &fields),
chromedp.Value(targetName, &name, chromedp.ByQuery),
chromedp.Value(urlField, &typed, chromedp.ByQuery),
))
assert.Equal(
t, slackFields, fields,
"with Slack chosen, the HTTP fields must not be submitted",
)
assert.Equal(t, "refused", name,
"a refused target does not keep the name entered")
assert.Equal(t, refusedURL, typed,
"a refused target does not keep the url entered")
assert.True(t, shown(ctx, targetName),
"a refused target does not come back with the form open")
assert.True(t, hidden(ctx, typeSelect),
"a refused target comes back on the type choice")
var loaded, restored string
click(ctx, t, cancelFields)
chooseTargetType(ctx, t, "http")
assert.True(t, hidden(ctx, reason),
"after Cancel, the next Add still shows the reason")
// Going back waits for the load event, after which the browser has
// restored the form.
require.NoError(t, chromedp.Run(
ctx,
loadPage(elsewhere),
chromedp.NavigateBack(),
chromedp.WaitNotPresent("[x-cloak]", chromedp.ByQuery),
chromedp.Evaluate(howLoaded, &loaded),
chromedp.Evaluate(chosen, &restored),
chromedp.Value(targetName, &name, chromedp.ByQuery),
chromedp.Value(urlField, &typed, chromedp.ByQuery),
))
// A page the browser kept in memory and showed again as it was
// would prove nothing here.
require.Equal(
t, "back_forward", loaded,
"going back, the browser did not load the page again",
)
require.Equal(
t, "slack", restored,
"going back, the browser did not restore the chosen type",
)
click(ctx, t, `//h2[text()="Targets"]/following-sibling::button`)
assert.True(t, shown(ctx, slackURL),
"going back with Slack chosen, the Slack fields are not shown")
assert.True(t, hidden(ctx, httpURL),
"going back with Slack chosen, the HTTP fields are shown")
require.NoError(t, chromedp.Run(
ctx, chromedp.Evaluate(submitted, &fields),
))
assert.Equal(
t, slackFields, fields,
"going back with Slack chosen, the HTTP fields must not be submitted",
)
assert.Empty(t, name, "after Cancel, the next Add keeps the name entered")
assert.Empty(t, typed, "after Cancel, the next Add keeps the url entered")
}
// checkCopy loads a webhook page and checks that the Copy control beside
@@ -363,6 +472,93 @@ 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; that after typing, opening the page at elsewhere and going
// back, Edit again opens the form on the saved description; and that
// Save changes the description the page shows.
func checkEntrypointEdit(
ctx context.Context, t *testing.T, url, elsewhere string,
) {
t.Helper()
// Cancel and Save are found inside the edit form, since the add
// target form has buttons of the same names.
const (
editForm = `form[action$="/edit"]`
input = editForm + ` input[name="description"]`
cancelEdit = `//form[contains(@action, "/edit")]/button[text()="Cancel"]`
saveEdit = `//form[contains(@action, "/edit")]/button[text()="Save"]`
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, cancelEdit)
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")
var loaded string
require.NoError(t, chromedp.Run(
ctx,
chromedp.SendKeys(input, "draft", chromedp.ByQuery),
loadPage(elsewhere),
chromedp.NavigateBack(),
chromedp.WaitNotPresent("[x-cloak]", chromedp.ByQuery),
chromedp.Evaluate(
`performance.getEntriesByType("navigation")[0].type`, &loaded,
),
))
require.Equal(
t, "back_forward", loaded,
"going back, the browser did not load the page again",
)
click(ctx, t, edit)
require.NoError(t, chromedp.Run(
ctx, chromedp.Value(input, &typed, chromedp.ByQuery),
))
assert.Empty(t, typed, "going back puts what was typed back in the form")
require.NoError(t, chromedp.Run(
ctx, chromedp.SendKeys(input, "Billing sender", chromedp.ByQuery),
))
click(ctx, t, saveEdit)
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
+57 -21
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,
@@ -87,24 +88,65 @@ document.addEventListener("alpine:init", function () {
};
});
// The add target form. Only the chosen type's fields show, and the
// others are disabled so that the form does not submit them.
// The targets section's add target form, in three steps: closed,
// choosing a type, then filling in that type's fields. targetType
// is empty until Next takes it from the type select.
//
// The type is read from the type select when Alpine starts, when the
// select changes, and on pageshow. Going back to the page, the
// browser restores the type chosen before without a change event,
// in some browsers only after Alpine has started, but always before
// pageshow.
// The reason and the fields' values come from the properties below
// rather than from the markup, because each type's fields are made
// afresh from the markup whenever that type is chosen. A refused
// submission comes back with its type, reason and values in the
// section's data attributes, and starts on that type's fields with
// them. Cancel empties these properties and resets the form, which
// holds whatever was typed, so the next Add starts with an empty
// form and no reason.
window.Alpine.data("targetForm", function () {
return {
choosing: false,
targetType: "",
reason: "",
name: "",
url: "",
headers: "",
timeout: "",
maxRetries: "",
expiry: "",
init() {
this.readType();
const refused = this.$root.dataset;
this.targetType = refused.type;
this.reason = refused.reason;
this.name = refused.name;
this.url = refused.destination;
this.headers = refused.headers;
this.timeout = refused.timeout;
this.maxRetries = refused.maxRetries;
this.expiry = refused.expiry;
},
readType() {
this.targetType = this.$root.querySelector(
'select[name="type"]'
).value;
add() {
this.choosing = true;
},
next() {
this.targetType = this.$refs.type.value;
this.choosing = false;
},
cancel() {
this.choosing = false;
this.targetType = "";
this.reason = "";
this.name = "";
this.url = "";
this.headers = "";
this.timeout = "";
this.maxRetries = "";
this.expiry = "";
this.$refs.form.reset();
},
get filling() {
return this.targetType !== "";
},
get closed() {
return !this.choosing && !this.filling;
},
get isHttp() {
return this.targetType === "http";
@@ -115,14 +157,8 @@ document.addEventListener("alpine:init", function () {
get isDatabase() {
return this.targetType === "database";
},
get notHttp() {
return !this.isHttp;
},
get notSlack() {
return !this.isSlack;
},
get notDatabase() {
return !this.isDatabase;
get isLog() {
return this.targetType === "log";
},
};
});
+108 -45
View File
@@ -54,15 +54,29 @@
<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, and
Cancel resets what was typed. With
autocomplete="off", going back to the page
does not put unsaved text back either, so
the form always opens on the saved
description. -->
<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}}" autocomplete="off" 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}}">
@@ -90,11 +104,23 @@
</div>
</div>
<!-- Targets -->
<div class="card" x-data="collapsible">
<!-- Targets. The data attributes carry a refused add target
submission's type, reason and values back to the form. The
URL is data-destination, not data-url: html/template treats
an attribute named like a URL as a link and would rewrite
a refused ftp: or javascript: value. -->
<div class="card" x-data="targetForm"
data-type="{{.TargetForm.Type}}"
data-reason="{{.TargetError}}"
data-name="{{.TargetForm.Name}}"
data-destination="{{.TargetForm.URL}}"
data-headers="{{.TargetForm.Headers}}"
data-timeout="{{.TargetForm.Timeout}}"
data-max-retries="{{.TargetForm.MaxRetries}}"
data-expiry="{{.TargetForm.Expiry}}">
<div class="p-4 border-b border-gray-200 flex justify-between items-center">
<h2 class="text-lg font-medium text-gray-900">Targets</h2>
<button type="button" @click="toggle" class="btn-small">
<button type="button" @click="add" x-show="closed" class="btn-small">
<svg class="w-3 h-3 mr-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 4v16m8-8H4"/>
</svg>
@@ -102,48 +128,85 @@
</button>
</div>
<!-- Add target form -->
<div x-show="open" x-cloak class="p-4 bg-gray-50 border-b border-gray-200">
<form method="POST" action="/hook/{{.Webhook.ID}}/targets" x-data="targetForm" @pageshow.window="readType" class="space-y-3">
<input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
<div class="flex gap-2">
<input type="text" name="name" placeholder="Target name" required class="input text-sm flex-1">
<select name="type" @change="readType" class="input text-sm w-32">
<option value="http">HTTP</option>
<option value="slack">Slack</option>
<option value="database">Database</option>
<option value="log">Log</option>
</select>
</div>
<div x-show="isHttp">
<input type="url" name="url" placeholder="https://example.com/webhook" :disabled="notHttp" class="input text-sm">
</div>
<div x-show="isHttp">
<textarea name="headers" rows="3" placeholder="Authorization: Bearer ..." :disabled="notHttp" class="input text-sm"></textarea>
<p class="text-xs text-gray-500 mt-1">Optional request headers, one <code>Name: value</code> per line, sent with every delivery.</p>
</div>
<div x-show="isHttp" class="flex gap-2 items-center">
<label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
<input type="number" name="timeout" min="0" max="300" :disabled="notHttp" class="input text-sm w-24">
</div>
<div x-show="isHttp">
<div class="flex gap-2 items-center">
<label class="text-sm text-gray-700">Max retries:</label>
<input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
<!-- Add target form. Add shows the type choice; Next replaces
it with the chosen type's fields. Each type's fields,
and the hidden type field submitted with them, exist
only while that type is chosen. A refused submission
comes back open on its type, with the values entered;
Cancel empties the form. The type choice's p-2, narrower
than an input's own padding, keeps it, Next and Cancel on
one row on a 360px-wide phone. -->
<form method="POST" action="/hook/{{.Webhook.ID}}/targets" x-ref="form">
<input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
<div x-show="choosing" x-cloak class="p-4 bg-gray-50 border-b border-gray-200 flex flex-wrap gap-2">
<select x-ref="type" aria-label="Target type" class="input text-sm p-2 flex-1">
<option value="http">HTTP</option>
<option value="slack">Slack</option>
<option value="database">Database</option>
<option value="log">Log</option>
</select>
<button type="button" @click="next" class="btn-primary text-sm">Next</button>
<button type="button" @click="cancel" class="btn-secondary text-sm">Cancel</button>
</div>
<div x-show="filling" x-cloak class="p-4 bg-gray-50 border-b border-gray-200 space-y-3">
<div x-show="reason" x-text="reason" class="alert-error"></div>
<input type="text" name="name" :value="name" placeholder="Target name" required class="input text-sm">
<template x-if="isHttp">
<div class="space-y-3">
<input type="hidden" name="type" value="http">
<input type="url" name="url" :value="url" placeholder="https://example.com/webhook" class="input text-sm">
<div>
<textarea name="headers" rows="3" :value="headers" placeholder="Authorization: Bearer ..." class="input text-sm"></textarea>
<p class="text-xs text-gray-500 mt-1">Optional request headers, one <code>Name: value</code> per line, sent with every delivery.</p>
</div>
<div class="flex gap-2 items-center">
<label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
<input type="number" name="timeout" :value="timeout" min="0" max="300" class="input text-sm w-24">
</div>
<div>
<div class="flex gap-2 items-center">
<label class="text-sm text-gray-700">Max retries:</label>
<input type="number" name="max_retries" :value="maxRetries" placeholder="0" min="0" max="20" class="input text-sm w-24">
</div>
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
</div>
</div>
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
</template>
<template x-if="isSlack">
<div class="space-y-3">
<input type="hidden" name="type" value="slack">
<div>
<input type="url" name="url" :value="url" placeholder="https://hooks.slack.com/services/..." class="input text-sm">
<p class="text-xs text-gray-500 mt-1">Slack or Mattermost incoming webhook URL. Payloads are pretty-printed in code blocks.</p>
</div>
<div>
<div class="flex gap-2 items-center">
<label class="text-sm text-gray-700">Max retries:</label>
<input type="number" name="max_retries" :value="maxRetries" placeholder="0" min="0" max="20" class="input text-sm w-24">
</div>
<p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
</div>
</div>
</template>
<template x-if="isDatabase">
<div>
<input type="hidden" name="type" value="database">
<input type="text" name="expiry" :value="expiry" placeholder="never" class="input text-sm">
<p class="text-xs text-gray-500 mt-1">Archive expiry: "never" (default) keeps rows forever, or a duration like "720h" prunes older rows.</p>
</div>
</template>
<template x-if="isLog">
<div>
<input type="hidden" name="type" value="log">
<p class="text-xs text-gray-500">A log target writes each event to the application log. It has no settings beyond its name.</p>
</div>
</template>
<div class="flex gap-2">
<button type="submit" class="btn-primary text-sm">Save</button>
<button type="button" @click="cancel" class="btn-secondary text-sm">Cancel</button>
</div>
<div x-show="isSlack">
<input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="notSlack" class="input text-sm">
<p class="text-xs text-gray-500 mt-1">Slack or Mattermost incoming webhook URL. Payloads are pretty-printed in code blocks.</p>
</div>
<div x-show="isDatabase">
<input type="text" name="expiry" placeholder="never" :disabled="notDatabase" class="input text-sm">
<p class="text-xs text-gray-500 mt-1">Archive expiry: "never" (default) keeps rows forever, or a duration like "720h" prunes older rows.</p>
</div>
<button type="submit" class="btn-primary text-sm">Add Target</button>
</form>
</div>
</div>
</form>
<div class="divide-y divide-gray-100">
{{range .Targets}}