From 4d4cc95979b62dd10fed5c16f8897bc56ee95270 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Tue, 8 Sep 2026 19:42:17 -0500 Subject: [PATCH 01/27] ci: run the backend and frontend suites on every push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing executed the test suite, so a regression could reach main with every check green simply because there were no checks. This runs the backend build, vet and tests — with CGO disabled to match the released image — plus the frontend type-check, production build and vitest suite. (cherry picked from commit 61c758c48c7d35b0339916d671c3193522e0895f) --- .github/workflows/ci.yml | 62 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..932df41 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,62 @@ +name: CI + +# Nothing ran the test suite before this workflow existed, so a regression could +# reach main with every check green simply because there were no checks. This runs +# the backend build, vet and tests plus the frontend type-check, build and tests. +# +# CGO is disabled to match the released image: the Dockerfile builds with +# CGO_ENABLED=0, and the tests do not need a database (they are unit tests). +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +jobs: + backend: + runs-on: ubuntu-latest + env: + CGO_ENABLED: "0" + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: backend/go.mod + cache-dependency-path: backend/go.sum + + - name: Build + working-directory: backend + run: go build ./... + + - name: Vet + working-directory: backend + run: go vet ./... + + - name: Test + working-directory: backend + run: go test ./... + + frontend: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: "20" + cache: npm + cache-dependency-path: frontend/package-lock.json + + - name: Install + working-directory: frontend + run: npm ci + + # `npm run build` runs the type-check and the production build together. + - name: Type-check and build + working-directory: frontend + run: npm run build + + - name: Test + working-directory: frontend + run: npm test From a783c240d9849b879e0d8995ea3e546ac7497b16 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Tue, 8 Sep 2026 21:16:49 -0500 Subject: [PATCH 02/27] fix(broadcast): keep job ownership across restarts so nothing is sent twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every startup reset jobs left in 'sending' regardless of who owned them, so starting or rolling one instance re-queued jobs another instance was still delivering, and those recipients received the same broadcast twice. Jobs now carry the worker that claimed them and a lease. Startup recovery only reclaims rows whose lease has expired, which leaves work in flight alone while still recovering genuinely abandoned jobs — including rows written before the lease existed. Covered by unit tests for the lease window and by integration tests that run against a real database when DATABASE_URL is set: one asserts that a live lease survives recovery, one that expired and pre-lease rows are reclaimed, and one that a claim stamps ownership and a deadline. (cherry picked from commit 5fef4e68692e60223ff40008e10ad8b5d6a44b52) --- CHANGELOG.md | 6 + backend/internal/db/broadcast_jobs.sql.go | 30 ++- backend/internal/db/models.go | 2 + backend/internal/service/broadcast_worker.go | 26 +- .../service/broadcast_worker_lease_test.go | 233 ++++++++++++++++++ .../internal/service/broadcast_worker_test.go | 26 ++ .../20260927123000_broadcast_jobs_lease.sql | 15 ++ backend/sqlc/queries/broadcast_jobs.sql | 15 +- docs/changelog.md | 6 + docs/guide/sending.md | 4 +- 10 files changed, 344 insertions(+), 19 deletions(-) create mode 100644 backend/internal/service/broadcast_worker_lease_test.go create mode 100644 backend/internal/service/broadcast_worker_test.go create mode 100644 backend/migrations/20260927123000_broadcast_jobs_lease.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 0679d60..2468161 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Releases are also published on [GitHub](https://github.com/arkhe-systems/senddock/releases) and the `ghcr.io/arkhe-systems/senddock` image carries the matching tag. +## [Unreleased] + +### Fixed + +- **A restart no longer re-sends a broadcast another instance is delivering.** Broadcast jobs left in `sending` were reset to `retry` on every startup regardless of who owned them, so starting or rolling one instance re-queued jobs another instance was still sending, and those recipients received the same email twice. A claimed job now carries the worker's id and a lease, and startup recovery only reclaims jobs whose lease has expired — work in flight is left alone. + ## [0.8.2] — 2026-09-01 Audience & white-label: multiple newsletters per project with per-newsletter unsubscribe, and branded unsubscribe pages. diff --git a/backend/internal/db/broadcast_jobs.sql.go b/backend/internal/db/broadcast_jobs.sql.go index f4946e5..e5be5f7 100644 --- a/backend/internal/db/broadcast_jobs.sql.go +++ b/backend/internal/db/broadcast_jobs.sql.go @@ -37,7 +37,9 @@ func (q *Queries) BulkInsertBroadcastJobs(ctx context.Context, arg BulkInsertBro const claimBroadcastJob = `-- name: ClaimBroadcastJob :one UPDATE broadcast_jobs SET status = 'sending', - attempts = attempts + 1 + attempts = attempts + 1, + worker_id = $1::uuid, + lease_expires_at = $2::timestamptz WHERE id = ( SELECT id FROM broadcast_jobs WHERE status IN ('pending', 'retry') @@ -46,11 +48,16 @@ WHERE id = ( FOR UPDATE SKIP LOCKED LIMIT 1 ) -RETURNING id, broadcast_id, project_id, subscriber_id, recipient_email, status, attempts, last_error, scheduled_at, completed_at, created_at +RETURNING id, broadcast_id, project_id, subscriber_id, recipient_email, status, attempts, last_error, scheduled_at, completed_at, created_at, worker_id, lease_expires_at ` -func (q *Queries) ClaimBroadcastJob(ctx context.Context) (BroadcastJob, error) { - row := q.db.QueryRowContext(ctx, claimBroadcastJob) +type ClaimBroadcastJobParams struct { + WorkerID uuid.UUID + LeaseExpiresAt time.Time +} + +func (q *Queries) ClaimBroadcastJob(ctx context.Context, arg ClaimBroadcastJobParams) (BroadcastJob, error) { + row := q.db.QueryRowContext(ctx, claimBroadcastJob, arg.WorkerID, arg.LeaseExpiresAt) var i BroadcastJob err := row.Scan( &i.ID, @@ -64,6 +71,8 @@ func (q *Queries) ClaimBroadcastJob(ctx context.Context) (BroadcastJob, error) { &i.ScheduledAt, &i.CompletedAt, &i.CreatedAt, + &i.WorkerID, + &i.LeaseExpiresAt, ) return i, err } @@ -157,12 +166,15 @@ func (q *Queries) MarkBroadcastJobSuppressed(ctx context.Context, id uuid.UUID) const resetStuckSendingJobs = `-- name: ResetStuckSendingJobs :execrows UPDATE broadcast_jobs SET status = 'retry', - scheduled_at = NOW() + scheduled_at = NOW(), + worker_id = NULL, + lease_expires_at = NULL WHERE status = 'sending' + AND (lease_expires_at IS NULL OR lease_expires_at < $1::timestamptz) ` -func (q *Queries) ResetStuckSendingJobs(ctx context.Context) (int64, error) { - result, err := q.db.ExecContext(ctx, resetStuckSendingJobs) +func (q *Queries) ResetStuckSendingJobs(ctx context.Context, now time.Time) (int64, error) { + result, err := q.db.ExecContext(ctx, resetStuckSendingJobs, now) if err != nil { return 0, err } @@ -173,7 +185,9 @@ const scheduleBroadcastJobRetry = `-- name: ScheduleBroadcastJobRetry :exec UPDATE broadcast_jobs SET status = 'retry', scheduled_at = $1, - last_error = $2::text + last_error = $2::text, + worker_id = NULL, + lease_expires_at = NULL WHERE id = $3 ` diff --git a/backend/internal/db/models.go b/backend/internal/db/models.go index 9ac424b..de57705 100644 --- a/backend/internal/db/models.go +++ b/backend/internal/db/models.go @@ -65,6 +65,8 @@ type BroadcastJob struct { ScheduledAt time.Time CompletedAt sql.NullTime CreatedAt time.Time + WorkerID uuid.NullUUID + LeaseExpiresAt sql.NullTime } type Campaign struct { diff --git a/backend/internal/service/broadcast_worker.go b/backend/internal/service/broadcast_worker.go index d25e915..7e6c7de 100644 --- a/backend/internal/service/broadcast_worker.go +++ b/backend/internal/service/broadcast_worker.go @@ -16,28 +16,41 @@ const ( broadcastWorkerCount = 5 broadcastWorkerIdleSleep = 2 * time.Second broadcastJobMaxAttempts = 5 + + // broadcastJobLease is how long a claimed job belongs to the worker that took it. + // Startup recovery only reclaims jobs whose lease has expired, so the lease has to + // outlive a full send attempt (connect + session timeouts). If it did not, restarting + // one instance could reclaim a job another instance is still delivering and send the + // same email twice. + broadcastJobLease = 5 * time.Minute ) type BroadcastWorker struct { queries *db.Queries emailService *EmailService + workerID uuid.UUID } func NewBroadcastWorker(queries *db.Queries, emailService *EmailService) *BroadcastWorker { - return &BroadcastWorker{queries: queries, emailService: emailService} + return &BroadcastWorker{queries: queries, emailService: emailService, workerID: uuid.New()} +} + +// leaseDeadline is when a job claimed at now stops being owned by this process. +func leaseDeadline(now time.Time) time.Time { + return now.Add(broadcastJobLease) } func (w *BroadcastWorker) Start(ctx context.Context) { - if rows, err := w.queries.ResetStuckSendingJobs(ctx); err != nil { + if rows, err := w.queries.ResetStuckSendingJobs(ctx, time.Now()); err != nil { slog.Error("broadcast worker: reset stuck jobs failed", "error", err) } else if rows > 0 { - slog.Warn("broadcast worker: reset stuck sending jobs to retry", "jobs", rows) + slog.Warn("broadcast worker: reclaimed abandoned broadcast jobs", "jobs", rows) } for i := 0; i < broadcastWorkerCount; i++ { go w.run(ctx, i) } - slog.Info("broadcast worker started", "goroutines", broadcastWorkerCount) + slog.Info("broadcast worker started", "goroutines", broadcastWorkerCount, "worker_id", w.workerID) } func (w *BroadcastWorker) run(ctx context.Context, id int) { @@ -56,7 +69,10 @@ func (w *BroadcastWorker) run(ctx context.Context, id int) { default: } - job, err := w.queries.ClaimBroadcastJob(ctx) + job, err := w.queries.ClaimBroadcastJob(ctx, db.ClaimBroadcastJobParams{ + WorkerID: w.workerID, + LeaseExpiresAt: leaseDeadline(time.Now()), + }) if errors.Is(err, sql.ErrNoRows) { select { case <-ctx.Done(): diff --git a/backend/internal/service/broadcast_worker_lease_test.go b/backend/internal/service/broadcast_worker_lease_test.go new file mode 100644 index 0000000..c6c83fa --- /dev/null +++ b/backend/internal/service/broadcast_worker_lease_test.go @@ -0,0 +1,233 @@ +package service + +import ( + "context" + "database/sql" + "os" + "testing" + "time" + + "github.com/google/uuid" + _ "github.com/lib/pq" + + "github.com/arkhe-systems/senddock/internal/db" +) + +// These tests exercise the broadcast job ownership lease against a real database. +// They skip unless DATABASE_URL points at a migrated instance, so the default test run +// stays free of external dependencies; CI passes the value explicitly when a database +// is available. + +func leaseTestDB(t *testing.T) (*sql.DB, *db.Queries) { + t.Helper() + + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + t.Skip("DATABASE_URL is not set; skipping broadcast lease integration test") + } + + conn, err := sql.Open("postgres", dsn) + if err != nil { + t.Fatalf("open database: %v", err) + } + if err := conn.Ping(); err != nil { + conn.Close() + t.Fatalf("ping database: %v", err) + } + t.Cleanup(func() { conn.Close() }) + + return conn, db.New(conn) +} + +// leaseFixtures creates the minimum graph a broadcast job needs and returns the ids. +// Deleting the workspace in cleanup cascades to everything the test inserted. +func leaseFixtures(t *testing.T, conn *sql.DB) (broadcastID, projectID, subscriberID uuid.UUID) { + t.Helper() + ctx := context.Background() + + var userID, workspaceID, templateID uuid.UUID + + if err := conn.QueryRowContext(ctx, + `INSERT INTO users (email, name) VALUES ($1, $2) RETURNING id`, + "lease-test-"+uuid.NewString()+"@example.test", "Lease Test", + ).Scan(&userID); err != nil { + t.Fatalf("insert user: %v", err) + } + + if err := conn.QueryRowContext(ctx, + `INSERT INTO workspaces (name, created_by) VALUES ($1, $2) RETURNING id`, + "lease-test", userID, + ).Scan(&workspaceID); err != nil { + t.Fatalf("insert workspace: %v", err) + } + t.Cleanup(func() { + if _, err := conn.ExecContext(ctx, `DELETE FROM workspaces WHERE id = $1`, workspaceID); err != nil { + t.Logf("cleanup workspace: %v", err) + } + }) + + if err := conn.QueryRowContext(ctx, + `INSERT INTO projects (workspace_id, user_id, name, from_email) VALUES ($1, $2, $3, $4) RETURNING id`, + workspaceID, userID, "lease-test", "sender@example.test", + ).Scan(&projectID); err != nil { + t.Fatalf("insert project: %v", err) + } + + if err := conn.QueryRowContext(ctx, + `INSERT INTO templates (project_id, name, subject) VALUES ($1, $2, $3) RETURNING id`, + projectID, "lease-test", "Lease test", + ).Scan(&templateID); err != nil { + t.Fatalf("insert template: %v", err) + } + + if err := conn.QueryRowContext(ctx, + `INSERT INTO subscribers (project_id, email) VALUES ($1, $2) RETURNING id`, + projectID, "recipient@example.test", + ).Scan(&subscriberID); err != nil { + t.Fatalf("insert subscriber: %v", err) + } + + if err := conn.QueryRowContext(ctx, + `INSERT INTO broadcasts (project_id, template_id) VALUES ($1, $2) RETURNING id`, + projectID, templateID, + ).Scan(&broadcastID); err != nil { + t.Fatalf("insert broadcast: %v", err) + } + + return broadcastID, projectID, subscriberID +} + +// insertSendingJob writes a job already in 'sending' with the given ownership, which is +// what startup recovery inspects. +func insertSendingJob(t *testing.T, conn *sql.DB, broadcastID, projectID, subscriberID uuid.UUID, recipient string, worker uuid.NullUUID, lease sql.NullTime) uuid.UUID { + t.Helper() + + var id uuid.UUID + if err := conn.QueryRowContext(context.Background(), + `INSERT INTO broadcast_jobs + (broadcast_id, project_id, subscriber_id, recipient_email, status, worker_id, lease_expires_at) + VALUES ($1, $2, $3, $4, 'sending', $5, $6) + RETURNING id`, + broadcastID, projectID, subscriberID, recipient, worker, lease, + ).Scan(&id); err != nil { + t.Fatalf("insert sending job: %v", err) + } + return id +} + +func jobOwnerAndLease(t *testing.T, conn *sql.DB, id uuid.UUID) (string, uuid.NullUUID, sql.NullTime) { + t.Helper() + + var status string + var worker uuid.NullUUID + var lease sql.NullTime + if err := conn.QueryRowContext(context.Background(), + `SELECT status, worker_id, lease_expires_at FROM broadcast_jobs WHERE id = $1`, id, + ).Scan(&status, &worker, &lease); err != nil { + t.Fatalf("read job: %v", err) + } + return status, worker, lease +} + +// A restart must not reclaim a job whose lease is still valid: that job belongs to an +// instance that is actively delivering it, and reclaiming it sends the email twice. +func TestResetStuckSendingJobsLeavesLiveLeasesAlone(t *testing.T) { + conn, q := leaseTestDB(t) + broadcastID, projectID, subscriberID := leaseFixtures(t, conn) + ctx := context.Background() + + liveWorker := uuid.New() + liveID := insertSendingJob(t, conn, broadcastID, projectID, subscriberID, + "live@example.test", + uuid.NullUUID{UUID: liveWorker, Valid: true}, + sql.NullTime{Time: time.Now().Add(time.Minute), Valid: true}) + + // Recover as if this were a fresh process starting up. + if _, err := q.ResetStuckSendingJobs(ctx, time.Now()); err != nil { + t.Fatalf("reset stuck jobs: %v", err) + } + + status, worker, lease := jobOwnerAndLease(t, conn, liveID) + if status != "sending" { + t.Fatalf("job with a live lease was reclaimed: status = %q, want %q", status, "sending") + } + if !worker.Valid || worker.UUID != liveWorker { + t.Fatalf("job with a live lease lost its owner: worker = %+v, want %s", worker, liveWorker) + } + if !lease.Valid { + t.Fatal("job with a live lease lost its deadline") + } +} + +// Abandoned work still has to be recovered: an expired lease and a row written before +// leases existed both go back to the queue. +func TestResetStuckSendingJobsReclaimsAbandonedWork(t *testing.T) { + conn, q := leaseTestDB(t) + broadcastID, projectID, subscriberID := leaseFixtures(t, conn) + ctx := context.Background() + + expiredID := insertSendingJob(t, conn, broadcastID, projectID, subscriberID, + "expired@example.test", + uuid.NullUUID{UUID: uuid.New(), Valid: true}, + sql.NullTime{Time: time.Now().Add(-time.Minute), Valid: true}) + + legacyID := insertSendingJob(t, conn, broadcastID, projectID, subscriberID, + "legacy@example.test", + uuid.NullUUID{}, sql.NullTime{}) + + reclaimed, err := q.ResetStuckSendingJobs(ctx, time.Now()) + if err != nil { + t.Fatalf("reset stuck jobs: %v", err) + } + if reclaimed != 2 { + t.Fatalf("reclaimed %d jobs, want 2 (expired lease + pre-lease row)", reclaimed) + } + + for name, id := range map[string]uuid.UUID{"expired": expiredID, "legacy": legacyID} { + status, worker, lease := jobOwnerAndLease(t, conn, id) + if status != "retry" { + t.Errorf("%s job: status = %q, want %q", name, status, "retry") + } + if worker.Valid || lease.Valid { + t.Errorf("%s job: ownership not cleared (worker=%+v lease=%+v)", name, worker, lease) + } + } +} + +// A claimed job carries its owner and deadline, which is what makes the recovery above +// able to tell abandoned work from work in flight. +func TestClaimBroadcastJobStampsOwnerAndLease(t *testing.T) { + conn, q := leaseTestDB(t) + broadcastID, projectID, subscriberID := leaseFixtures(t, conn) + ctx := context.Background() + + jobID := insertSendingJob(t, conn, broadcastID, projectID, subscriberID, + "pending@example.test", uuid.NullUUID{}, sql.NullTime{}) + if _, err := conn.ExecContext(ctx, + `UPDATE broadcast_jobs SET status = 'pending', scheduled_at = NOW() - INTERVAL '1 minute' WHERE id = $1`, + jobID); err != nil { + t.Fatalf("make job claimable: %v", err) + } + + worker := uuid.New() + deadline := time.Now().Add(broadcastJobLease) + claimed, err := q.ClaimBroadcastJob(ctx, db.ClaimBroadcastJobParams{ + WorkerID: worker, + LeaseExpiresAt: deadline, + }) + if err != nil { + t.Fatalf("claim job: %v", err) + } + if claimed.ID != jobID { + t.Fatalf("claimed job %s, want %s", claimed.ID, jobID) + } + if !claimed.WorkerID.Valid || claimed.WorkerID.UUID != worker { + t.Fatalf("claimed job owner = %+v, want %s", claimed.WorkerID, worker) + } + if !claimed.LeaseExpiresAt.Valid { + t.Fatal("claimed job carries no deadline") + } + if diff := claimed.LeaseExpiresAt.Time.Sub(deadline).Abs(); diff > time.Second { + t.Fatalf("claimed job deadline = %s, want ~%s (off by %s)", claimed.LeaseExpiresAt.Time, deadline, diff) + } +} diff --git a/backend/internal/service/broadcast_worker_test.go b/backend/internal/service/broadcast_worker_test.go new file mode 100644 index 0000000..24f8341 --- /dev/null +++ b/backend/internal/service/broadcast_worker_test.go @@ -0,0 +1,26 @@ +package service + +import ( + "testing" + "time" +) + +// Startup recovery reclaims broadcast jobs whose lease has expired. If the lease were +// shorter than a send attempt, an instance starting up could reclaim a job another +// instance is still delivering and send the same email twice — the exact failure the +// lease exists to prevent. +func TestBroadcastJobLeaseOutlivesSendAttempt(t *testing.T) { + attempt := smtpConnectTimeout + smtpSessionTimeout + if broadcastJobLease <= attempt { + t.Fatalf("broadcastJobLease (%s) must exceed a full send attempt (%s)", broadcastJobLease, attempt) + } +} + +// A claim that is immediately past its deadline would be reclaimed by the next startup, +// so the window has to run forward from the moment of the claim. +func TestLeaseDeadlineRunsForward(t *testing.T) { + now := time.Now() + if !leaseDeadline(now).After(now) { + t.Fatal("leaseDeadline must return a time after the claim") + } +} diff --git a/backend/migrations/20260927123000_broadcast_jobs_lease.sql b/backend/migrations/20260927123000_broadcast_jobs_lease.sql new file mode 100644 index 0000000..abef12a --- /dev/null +++ b/backend/migrations/20260927123000_broadcast_jobs_lease.sql @@ -0,0 +1,15 @@ +-- +goose Up +-- Ownership lease for broadcast jobs. Startup recovery resets rows left in 'sending', +-- and without an owner it also resets jobs another instance is still delivering, which +-- sends those recipients the same email twice. +ALTER TABLE broadcast_jobs ADD COLUMN worker_id UUID; +ALTER TABLE broadcast_jobs ADD COLUMN lease_expires_at TIMESTAMPTZ; + +CREATE INDEX idx_broadcast_jobs_lease_expiry + ON broadcast_jobs (lease_expires_at) + WHERE status = 'sending'; + +-- +goose Down +DROP INDEX idx_broadcast_jobs_lease_expiry; +ALTER TABLE broadcast_jobs DROP COLUMN lease_expires_at; +ALTER TABLE broadcast_jobs DROP COLUMN worker_id; diff --git a/backend/sqlc/queries/broadcast_jobs.sql b/backend/sqlc/queries/broadcast_jobs.sql index 4abbe8b..46d63c9 100644 --- a/backend/sqlc/queries/broadcast_jobs.sql +++ b/backend/sqlc/queries/broadcast_jobs.sql @@ -10,7 +10,9 @@ FROM jsonb_array_elements(@jobs::jsonb) AS j; -- name: ClaimBroadcastJob :one UPDATE broadcast_jobs SET status = 'sending', - attempts = attempts + 1 + attempts = attempts + 1, + worker_id = @worker_id::uuid, + lease_expires_at = @lease_expires_at::timestamptz WHERE id = ( SELECT id FROM broadcast_jobs WHERE status IN ('pending', 'retry') @@ -52,7 +54,9 @@ WHERE id = @id; UPDATE broadcast_jobs SET status = 'retry', scheduled_at = @scheduled_at, - last_error = @last_error::text + last_error = @last_error::text, + worker_id = NULL, + lease_expires_at = NULL WHERE id = @id; -- name: CountBroadcastJobsRemaining :one @@ -67,5 +71,8 @@ WHERE status IN ('pending', 'retry', 'sending'); -- name: ResetStuckSendingJobs :execrows UPDATE broadcast_jobs SET status = 'retry', - scheduled_at = NOW() -WHERE status = 'sending'; + scheduled_at = NOW(), + worker_id = NULL, + lease_expires_at = NULL +WHERE status = 'sending' + AND (lease_expires_at IS NULL OR lease_expires_at < @now::timestamptz); diff --git a/docs/changelog.md b/docs/changelog.md index ec7b1c2..e9c30d3 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -14,6 +14,12 @@ For the canonical source, see [`CHANGELOG.md` in the repo](https://github.com/ar Pre-1.0 minor releases may contain breaking changes — check the version's notes before upgrading. Database migrations are forward-only and applied automatically by `goose` on container startup; rollback steps are documented in [Updating](./self-hosting/updating#rolling-back). +## [Unreleased] + +### Fixed + +- **A restart no longer re-sends a broadcast another instance is delivering.** Broadcast jobs left in `sending` were reset to `retry` on every startup regardless of who owned them, so starting or rolling one instance re-queued jobs another instance was still sending, and those recipients received the same email twice. A claimed job now carries the worker's id and a lease, and startup recovery only reclaims jobs whose lease has expired — work in flight is left alone. + ## [0.8.2] — 2026-09-01 Audience & white-label: multiple newsletters per project with per-newsletter unsubscribe, and branded unsubscribe pages. diff --git a/docs/guide/sending.md b/docs/guide/sending.md index 81158e9..9fcd949 100644 --- a/docs/guide/sending.md +++ b/docs/guide/sending.md @@ -163,12 +163,12 @@ When you call `/broadcast` (or a scheduled campaign reaches its time), SendDock 1. A `broadcasts` row is created with `status=sending` and `total_recipients=N`. 2. One row per recipient is inserted into `broadcast_jobs` with `status=pending`. -3. Five worker goroutines pull jobs concurrently using `SELECT … FOR UPDATE SKIP LOCKED`, so jobs never get sent twice even with multiple workers (or multiple SendDock instances on the same database). +3. Five worker goroutines pull jobs concurrently using `SELECT … FOR UPDATE SKIP LOCKED`. Each claim also stamps the row with the worker's id and a **lease**, so a job is owned by exactly one worker at a time — across goroutines and across SendDock instances sharing the same database. 4. As each job finishes, the worker increments the broadcast's `sent_count`, `failed_count`, or `suppressed_count`. When the queue drains, the broadcast flips to `status=completed` and any linked campaign moves from `sending` to `sent` with the real counts. This design has three consequences worth knowing: -- **Server restart mid-send is safe.** Jobs that were in `sending` when the process died are rescheduled to `retry` on the next startup; nothing is lost and nothing is double-sent. +- **Server restart mid-send is safe.** On startup a worker reclaims jobs left in `sending` **whose lease has expired**, rescheduling them to `retry`. A job still inside its lease is owned by a worker that is actively delivering it, so it is left untouched: a restart never reclaims work in flight, and nothing is lost or sent twice. - **Per-recipient retries with exponential backoff.** Transient errors (DNS failures, network timeouts, SMTP 4xx) reschedule the job — backoff steps are 30s, 2m, 8m, 30m, 1h. After 5 attempts the job is marked `failed`. SMTP 5xx bounces are *not* retried (they are tagged `bounced` and the recipient is added to the suppression list immediately). - **Live progress is visible while sending.** The Campaigns list, Broadcasts tab, and the "broadcasts in flight" panel in Analytics all read counts from the live broadcast row, refreshing every 5 seconds while at least one broadcast is in flight. You see `42/213 → 87/213 → 213/213`, not a sudden jump at the end. From fa07509f4aa963c978759ae2ad05c15f78a66916 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Wed, 9 Sep 2026 18:37:06 -0500 Subject: [PATCH 03/27] docs: reconcile the roadmap with what actually shipped The roadmap advertised work that was already implemented, which misleads both contributors looking for something to pick up and users checking whether a feature exists. Fifteen items are now marked done, the analytics entries no longer claim to be Pro (they moved to Core in v0.8.0), and the API key prefix, the webhook backoff steps, the webhook event list and the licensing note are corrected. A status note states that this file describes product direction, and that active work and open bugs live in issues. (cherry picked from commit a5a60bc1b760f3ea764e77ba2a81813bb91cdb40) --- ROADMAP.md | 50 +++++++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 5d50b4e..35481df 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,5 +1,13 @@ # SendDock Roadmap +> **Estado, reconciliado con el código.** Los ítems de las fases de abajo están verificados contra el +> código, no contra lo que se creía. Lo que sigue sin marcar sigue pendiente **de verdad** — entre otros: +> validación de email antes de enviar, onboarding de cloud, recuperación de contraseña, request logging y +> reset mensual de usage. +> +> Este archivo describe **dirección de producto**. El trabajo activo y los bugs abiertos se trackean en +> [issues](https://github.com/Arkhe-Systems/senddock/issues), no acá. + ## Phase 1: Core Backend - [x] Project structure (Go modules, internal/, cmd/) - [x] HTTP server with net/http stdlib @@ -18,14 +26,14 @@ - [x] Makefile with dev commands - [x] Refresh token rotation (HttpOnly cookies, SHA-256 hashing) - [x] Logout with token invalidation -- [ ] Validate project limits per plan (cloud mode) +- [x] Validate project limits per plan (cloud mode) — enforced in `senddock-pro` (`internal/cloud/quota.go`) ## Phase 2: Frontend Foundation - [x] Vue 3 + TypeScript + Vite project setup - [x] Tailwind CSS 4 configuration - [x] API client (fetch wrapper) - [x] Auth store (Pinia) -- [ ] Update auth store to work with HttpOnly cookies (remove localStorage) +- [x] Update auth store to work with HttpOnly cookies (remove localStorage) — auth no longer touches localStorage; only the active workspace id is kept there - [x] Vue Router with auth guards - [x] Reusable UI components (AppInput, AppButton, AppAlert, AppCard) - [x] Login page @@ -56,51 +64,51 @@ - [x] Email worker with asynq (Redis-based job queue) - [x] Broadcast endpoint (send to all subscribers) - [ ] Email validation before sending -- [ ] Campaign builder UI (scheduled sends) +- [x] Campaign builder UI (scheduled sends) ## Phase 5: Email Verification & Security -- [ ] Email verification on registration (send code) -- [ ] Verification page +- [x] Email verification on registration (send code) — cloud sign-up (`/auth/verify`, `/auth/resend-verification`) +- [x] Verification page - [ ] Onboarding flow (additional user info for cloud) - [ ] Password reset flow -- [ ] Session expiration (7 days inactivity) -- [ ] Account lockout after failed attempts +- [x] Session expiration — configurable inactivity timeout (5–1440 min) set under **Instance** in the dashboard +- [x] Account lockout after failed attempts ## Phase 6: Tracking & Analytics - [x] Open tracking (pixel injection) — Core - [x] Click tracking (link rewriting + HMAC redirect) — Core - [x] Unsubscribe handling (one-click + confirmation page, RFC 8058) - [x] Analytics table + migration -- [x] Analytics endpoints (sent, failed, opened, clicked) — PRO +- [x] Analytics endpoints (sent, failed, opened, clicked) — Core (moved from Pro in v0.8.0) - [x] Logs table + migration - [x] System logs endpoint -- [x] Analytics dashboard UI with charts — PRO +- [x] Analytics dashboard UI with charts — Core (moved from Pro in v0.8.0) ## Phase 7: API Keys & Security - [x] API keys table + migration -- [x] API key generation (public pk_ / secret sdk_) +- [x] API key generation (`sk_` prefix, SHA-256 hashed secret) - [x] API key authentication middleware - [x] Per-project rate limiting on `/send`, `/send/batch` and `/broadcast` (Redis-backed) - [ ] Request logging - [x] API keys management UI ## Phase 8: Payments & Plans (Cloud mode) -- [ ] Lemon Squeezy webhook handler -- [ ] Subscription lifecycle (created, updated, cancelled, expired) -- [ ] Plan upgrade/downgrade logic -- [ ] Coupon/discount support (handled by Lemon Squeezy) +- [x] Lemon Squeezy webhook handler — `senddock-pro` (`internal/cloud/billing.go`, signature verified) +- [x] Subscription lifecycle (created, updated, cancelled, expired) +- [x] Plan upgrade/downgrade logic — per-tier checkout + billing portal +- [x] Coupon/discount support — delegated to Lemon Squeezy's hosted checkout; nothing to build - [ ] Monthly usage reset cron job - [x] Deployment mode config (cloud vs self-hosted) - [x] Feature gating based on plan (license validator + paywall states in UI) - [x] License key activation/validation against Lemon Squeezy -- [ ] Billing page UI +- [x] Billing page UI ## Phase 9: Webhooks - [x] Webhook configuration per project — Core UI/API + dispatcher (management moved from Pro to Core: webhooks are developer table stakes) - [x] Webhook dispatcher (FOR UPDATE SKIP LOCKED, batch claim) — Core -- [x] Webhook retry logic with exponential backoff (30s → 2h, 5 attempts) — Core +- [x] Webhook retry logic with exponential backoff (30s, 2m, 10m, 30m — 5 attempts) — Core - [x] Webhook signature verification (HMAC-SHA256, `X-SendDock-Signature: t=,v1=`) — Core -- [x] Webhook event types (email.sent/failed/opened/clicked, subscriber.created/unsubscribed) — Core +- [x] Webhook event types (email.sent/failed/bounced/opened/clicked, subscriber.created/unsubscribed/newsletter_unsubscribed) — Core - [x] Webhooks management UI — Core - [x] Per-webhook deliveries view — Core @@ -109,8 +117,8 @@ - [x] Docker Compose for self-hosting (app + postgres + redis) - [x] Go serves Vue static files (single binary/container) - [x] Health check endpoints for container orchestration -- [ ] Graceful shutdown handling -- [ ] Environment configuration documentation +- [x] Graceful shutdown handling — SIGINT/SIGTERM with a drain window (`cmd/server/main.go`, `pkg/app/app.go`) +- [x] Environment configuration documentation — `docs/guide/environment.md` --- @@ -139,7 +147,7 @@ Lower the friction of bringing users and existing subscriber lists into the plat What teams evaluate before committing their list. Plus the first phase of drips — linear sequences cover most automation demand on their own. - [ ] Drip automations · Phase 1: linear sequences — [#77](https://github.com/Arkhe-Systems/senddock/issues/77) — Pro -- [ ] DNS deliverability check (SPF / DKIM / DMARC) — [#64](https://github.com/Arkhe-Systems/senddock/issues/64) — Pro +- [x] DNS deliverability check (SPF / DKIM / DMARC) — [#64](https://github.com/Arkhe-Systems/senddock/issues/64) — Pro — shipped as the Deliverability tab - [ ] GDPR / data subject tools — [#46](https://github.com/Arkhe-Systems/senddock/issues/46) — Core ### Milestone: Automation parity @@ -171,4 +179,4 @@ Captured for visibility, not actively scoped. Will be promoted when there is a c --- -Items marked **Pro** or **Team** live in the private `senddock-pro` repository and are gated by `SENDDOCK_LICENSE_KEY`. They are not part of the AGPL Community edition. +Items marked **Pro** or **Team** live in the private `senddock-pro` repository and are gated by the Pro/Team license, activated from the dashboard under **Instance → License**. They are not part of the AGPL Community edition. From da4c80f89b98931cb848251522443a6f74546376 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Wed, 9 Sep 2026 20:54:33 -0500 Subject: [PATCH 04/27] fix(email): stop header injection through the subject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subjects are built from subscriber-supplied text — their name, email and custom fields are substituted into them — and that text went into the message headers untouched. A value carrying a line break could close the Subject line and start a header of its own, so a crafted value could add a Bcc to the message SendDock sends. The unsubscribe URL had the same weakness. Every value that reaches a header is now stripped of carriage returns and line feeds, at the single point where headers are built, which also covers the sender name and the recipient. Message assembly moved into buildMIME so the property is testable without going near the network. The documentation already claimed newlines were stripped. That is now true, and the sentence states which values it covers. (cherry picked from commit b8899b8cd9529c04bdeaa4923f753cd1960ef105) --- CHANGELOG.md | 4 ++ backend/internal/service/email.go | 29 ++++++--- backend/internal/service/email_mime_test.go | 72 +++++++++++++++++++++ docs/changelog.md | 4 ++ docs/guide/templates.md | 2 +- 5 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 backend/internal/service/email_mime_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 2468161..5a7bc62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ Releases are also published on [GitHub](https://github.com/arkhe-systems/senddoc - **A restart no longer re-sends a broadcast another instance is delivering.** Broadcast jobs left in `sending` were reset to `retry` on every startup regardless of who owned them, so starting or rolling one instance re-queued jobs another instance was still sending, and those recipients received the same email twice. A claimed job now carries the worker's id and a lease, and startup recovery only reclaims jobs whose lease has expired — work in flight is left alone. +### Security + +- **Headers could be injected through the subject.** Subjects are built from subscriber-supplied text — their name, email and custom fields are substituted into them — and that text was written into the message headers as-is. A value containing a line break could therefore add headers of the attacker's choosing, a `Bcc` for example, to the message SendDock sent. Carriage returns and line feeds are now stripped from every value that reaches a header: the subject, the sender name, the recipient and the unsubscribe URL. + ## [0.8.2] — 2026-09-01 Audience & white-label: multiple newsletters per project with per-newsletter unsubscribe, and branded unsubscribe pages. diff --git a/backend/internal/service/email.go b/backend/internal/service/email.go index e5daf4d..8215dab 100644 --- a/backend/internal/service/email.go +++ b/backend/internal/service/email.go @@ -1151,18 +1151,31 @@ func (s *EmailService) sendSMTPWithTimeouts(project db.Project, to, subject, htm from = fmt.Sprintf("%s <%s>", project.FromName.String, fromEmail) } - inlinedBody := inlineCSS(htmlBody) + msg := buildMIME(from, to, subject, unsubscribeURL, inlineCSS(htmlBody)) + addr := fmt.Sprintf("%s:%d", host, port) + + return deliverSMTP(host, addr, user, pass, fromEmail, to, msg, port == 465, connectTimeout, sessionTimeout) +} + +// headerSafe strips CR and LF so a value cannot end its header line early. Subjects +// carry subscriber-supplied text — their name, email and custom fields are substituted +// into them — so without this a crafted value could inject arbitrary headers into the +// message, a Bcc for example. +func headerSafe(value string) string { + return strings.NewReplacer("\r", "", "\n", "").Replace(value) +} + +// buildMIME assembles the message that goes on the wire: headers, a blank line, then the +// rendered body. Every interpolated header value is stripped of line breaks here, at the +// single point where headers are built, so no caller can bypass it. +func buildMIME(from, to, subject, unsubscribeURL, body string) []byte { headers := fmt.Sprintf("From: %s\r\nTo: %s\r\nSubject: %s\r\nMIME-Version: 1.0\r\nContent-Type: text/html; charset=UTF-8\r\n", - from, to, subject) + headerSafe(from), headerSafe(to), headerSafe(subject)) if unsubscribeURL != "" { - headers += fmt.Sprintf("List-Unsubscribe: <%s>\r\nList-Unsubscribe-Post: List-Unsubscribe=One-Click\r\n", unsubscribeURL) + headers += fmt.Sprintf("List-Unsubscribe: <%s>\r\nList-Unsubscribe-Post: List-Unsubscribe=One-Click\r\n", headerSafe(unsubscribeURL)) } - msg := headers + "\r\n" + inlinedBody - - addr := fmt.Sprintf("%s:%d", host, port) - - return deliverSMTP(host, addr, user, pass, fromEmail, to, []byte(msg), port == 465, connectTimeout, sessionTimeout) + return []byte(headers + "\r\n" + body) } const ( diff --git a/backend/internal/service/email_mime_test.go b/backend/internal/service/email_mime_test.go new file mode 100644 index 0000000..9a306a2 --- /dev/null +++ b/backend/internal/service/email_mime_test.go @@ -0,0 +1,72 @@ +package service + +import ( + "strings" + "testing" +) + +var mimeHeaderPrefixes = []string{ + "From:", + "To:", + "Subject:", + "MIME-Version:", + "Content-Type:", +} + +// A subject is built from subscriber-supplied text, so a value carrying a line break +// must not be able to close the Subject line and start a header of its own. +func TestBuildMIMERejectsHeaderInjection(t *testing.T) { + msg := string(buildMIME( + "Sender ", + "recipient@example.test", + "Hello\r\nBcc: victim@example.test\r\nX-Injected: 1", + "https://example.test/unsubscribe?t=abc\r\nBcc: victim2@example.test", + "

body

", + )) + + headerBlock, body, found := strings.Cut(msg, "\r\n\r\n") + if !found { + t.Fatalf("message has no header/body separator:\n%q", msg) + } + + want := append(append([]string{}, mimeHeaderPrefixes...), "List-Unsubscribe:", "List-Unsubscribe-Post:") + lines := strings.Split(headerBlock, "\r\n") + if len(lines) != len(want) { + t.Fatalf("message carries %d header lines, want %d — an injected header got through:\n%s", len(lines), len(want), headerBlock) + } + for i, prefix := range want { + if !strings.HasPrefix(lines[i], prefix) { + t.Errorf("header %d is %q, want a line starting with %q", i, lines[i], prefix) + } + } + if body != "

body

" { + t.Errorf("body = %q, want it untouched", body) + } +} + +// The fix strips line breaks rather than truncating, so the rest of the value survives +// as part of the same header. +func TestBuildMIMEKeepsInjectedTextInsideTheHeaderValue(t *testing.T) { + msg := string(buildMIME("from@example.test", "to@example.test", + "Hi\r\nBcc: victim@example.test", "", "

body

")) + + headerBlock, _, _ := strings.Cut(msg, "\r\n\r\n") + if !strings.Contains(headerBlock, "Subject: HiBcc: victim@example.test") { + t.Fatalf("subject value was not preserved on one line:\n%s", headerBlock) + } +} + +// The ordinary shape must not change: headers, blank line, body. +func TestBuildMIMEKeepsTheOrdinaryShape(t *testing.T) { + bare := string(buildMIME("from@example.test", "to@example.test", "Subject", "", "

hi

")) + headerBlock, body, found := strings.Cut(bare, "\r\n\r\n") + if !found { + t.Fatalf("no separator in:\n%q", bare) + } + if lines := strings.Split(headerBlock, "\r\n"); len(lines) != len(mimeHeaderPrefixes) { + t.Fatalf("message without an unsubscribe URL carries %d headers, want %d:\n%s", len(lines), len(mimeHeaderPrefixes), headerBlock) + } + if body != "

hi

" { + t.Errorf("body = %q", body) + } +} diff --git a/docs/changelog.md b/docs/changelog.md index e9c30d3..e7c6eb3 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -20,6 +20,10 @@ Pre-1.0 minor releases may contain breaking changes — check the version's note - **A restart no longer re-sends a broadcast another instance is delivering.** Broadcast jobs left in `sending` were reset to `retry` on every startup regardless of who owned them, so starting or rolling one instance re-queued jobs another instance was still sending, and those recipients received the same email twice. A claimed job now carries the worker's id and a lease, and startup recovery only reclaims jobs whose lease has expired — work in flight is left alone. +### Security + +- **Headers could be injected through the subject.** Subjects are built from subscriber-supplied text — their name, email and custom fields are substituted into them — and that text was written into the message headers as-is. A value containing a line break could therefore add headers of the attacker's choosing, a `Bcc` for example, to the message SendDock sent. Carriage returns and line feeds are now stripped from every value that reaches a header: the subject, the sender name, the recipient and the unsubscribe URL. + ## [0.8.2] — 2026-09-01 Audience & white-label: multiple newsletters per project with per-newsletter unsubscribe, and branded unsubscribe pages. diff --git a/docs/guide/templates.md b/docs/guide/templates.md index 0a6e98e..95b10ee 100644 --- a/docs/guide/templates.md +++ b/docs/guide/templates.md @@ -105,7 +105,7 @@ A rich value is **not** escaped — instead it runs through a strict HTML **sani See [Email Sending → Rich-text variables](/guide/sending#rich-text-variables) for the full workflow. -The subject line is **not** escaped (subject is plain text, not HTML), but it is also not allowed to introduce headers — newlines are stripped to prevent SMTP header injection. +The subject line is **not** escaped (subject is plain text, not HTML), but it cannot introduce headers: carriage returns and line feeds are stripped from every value that reaches a header — the subject, the sender name, the recipient and the unsubscribe URL — so a crafted value cannot inject one. ## Page templates From b96bd31f3991c9b5efb7716a80bf629dc760bcfe Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Thu, 10 Sep 2026 19:08:51 -0500 Subject: [PATCH 05/27] fix(smtp): verify certificates on outgoing mail Outgoing SMTP accepted any certificate, so an attacker positioned between the instance and the relay could read the SMTP password and the mail itself while the connection still looked encrypted. Verification is now on for both implicit TLS and STARTTLS. It is deliberately not unconditional: relays serving a self-signed or expired certificate are common in self-hosted setups, so a project can opt out under Project -> SMTP -> Allow insecure TLS. The opt-out is per project, which keeps the exception scoped to the relay that needs it instead of weakening every project on the instance, and the form says what it gives up. A failed handshake now explains itself and names that setting, so the fix is a click instead of a support thread. (cherry picked from commit 5ed600f1a3740e7ab98c856ffe4e10701defb439) --- CHANGELOG.md | 2 + backend/internal/db/models.go | 1 + backend/internal/db/projects.sql.go | 39 ++++++---- backend/internal/handler/project.go | 17 ++--- backend/internal/response/response.go | 2 + backend/internal/service/email.go | 38 +++++++++- backend/internal/service/email_tls_test.go | 72 +++++++++++++++++++ backend/internal/service/project.go | 5 +- ...260927140000_project_smtp_insecure_tls.sql | 9 +++ backend/sqlc/queries/projects.sql | 1 + docs/changelog.md | 2 + docs/guide/smtp.md | 10 +++ frontend/src/stores/projects.ts | 1 + frontend/src/views/project/SmtpSection.vue | 16 +++++ 14 files changed, 190 insertions(+), 25 deletions(-) create mode 100644 backend/internal/service/email_tls_test.go create mode 100644 backend/migrations/20260927140000_project_smtp_insecure_tls.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a7bc62..bf405c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ Releases are also published on [GitHub](https://github.com/arkhe-systems/senddoc ### Security - **Headers could be injected through the subject.** Subjects are built from subscriber-supplied text — their name, email and custom fields are substituted into them — and that text was written into the message headers as-is. A value containing a line break could therefore add headers of the attacker's choosing, a `Bcc` for example, to the message SendDock sent. Carriage returns and line feeds are now stripped from every value that reaches a header: the subject, the sender name, the recipient and the unsubscribe URL. +- **Certificates are verified on outgoing SMTP.** Every connection accepted any certificate, so an attacker able to sit between the instance and the relay could read SMTP credentials and the mail itself while the connection still looked encrypted. Verification is now on for both implicit TLS and STARTTLS. A relay with a self-signed or expired certificate can be kept working per project under **Project → SMTP → Allow insecure TLS**, and the failure message names that setting so the fix is one click away. + ## [0.8.2] — 2026-09-01 diff --git a/backend/internal/db/models.go b/backend/internal/db/models.go index de57705..77154b2 100644 --- a/backend/internal/db/models.go +++ b/backend/internal/db/models.go @@ -163,6 +163,7 @@ type Project struct { BounceImapEnabled bool WorkspaceID uuid.UUID UnsubscribeTemplateID uuid.NullUUID + SmtpAllowInsecureTls bool } type RefreshToken struct { diff --git a/backend/internal/db/projects.sql.go b/backend/internal/db/projects.sql.go index 1502749..a9df777 100644 --- a/backend/internal/db/projects.sql.go +++ b/backend/internal/db/projects.sql.go @@ -28,7 +28,7 @@ func (q *Queries) CountProjectsByUserID(ctx context.Context, userID uuid.UUID) ( const createProject = `-- name: CreateProject :one INSERT INTO projects (workspace_id, user_id, name, description, from_name, from_email) VALUES ($1, $2, $3, $4, $5, $6) -RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id +RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id, smtp_allow_insecure_tls ` type CreateProjectParams struct { @@ -75,6 +75,7 @@ func (q *Queries) CreateProject(ctx context.Context, arg CreateProjectParams) (P &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ) return i, err } @@ -96,7 +97,7 @@ func (q *Queries) DeleteProject(ctx context.Context, arg DeleteProjectParams) er } const getProjectByBounceToken = `-- name: GetProjectByBounceToken :one -SELECT id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id FROM projects WHERE id = $1 AND bounce_token = $2 +SELECT id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id, smtp_allow_insecure_tls FROM projects WHERE id = $1 AND bounce_token = $2 ` type GetProjectByBounceTokenParams struct { @@ -132,12 +133,13 @@ func (q *Queries) GetProjectByBounceToken(ctx context.Context, arg GetProjectByB &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ) return i, err } const getProjectByID = `-- name: GetProjectByID :one -SELECT p.id, p.user_id, p.name, p.from_name, p.from_email, p.smtp_host, p.smtp_port, p.smtp_user, p.smtp_password_encrypted, p.webhook_url, p.webhook_secret, p.tracking_enabled, p.created_at, p.updated_at, p.description, p.bounce_token, p.bounce_imap_host, p.bounce_imap_port, p.bounce_imap_user, p.bounce_imap_password_encrypted, p.bounce_imap_folder, p.bounce_imap_enabled, p.workspace_id, p.unsubscribe_template_id FROM projects p +SELECT p.id, p.user_id, p.name, p.from_name, p.from_email, p.smtp_host, p.smtp_port, p.smtp_user, p.smtp_password_encrypted, p.webhook_url, p.webhook_secret, p.tracking_enabled, p.created_at, p.updated_at, p.description, p.bounce_token, p.bounce_imap_host, p.bounce_imap_port, p.bounce_imap_user, p.bounce_imap_password_encrypted, p.bounce_imap_folder, p.bounce_imap_enabled, p.workspace_id, p.unsubscribe_template_id, p.smtp_allow_insecure_tls FROM projects p JOIN workspace_members m ON m.workspace_id = p.workspace_id WHERE p.id = $1 AND m.user_id = $2 ` @@ -175,12 +177,13 @@ func (q *Queries) GetProjectByID(ctx context.Context, arg GetProjectByIDParams) &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ) return i, err } const getProjectByIDOnly = `-- name: GetProjectByIDOnly :one -SELECT id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id FROM projects WHERE id = $1 +SELECT id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id, smtp_allow_insecure_tls FROM projects WHERE id = $1 ` func (q *Queries) GetProjectByIDOnly(ctx context.Context, id uuid.UUID) (Project, error) { @@ -211,6 +214,7 @@ func (q *Queries) GetProjectByIDOnly(ctx context.Context, id uuid.UUID) (Project &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ) return i, err } @@ -234,7 +238,7 @@ func (q *Queries) GetProjectMemberRole(ctx context.Context, arg GetProjectMember } const getProjectsByUserID = `-- name: GetProjectsByUserID :many -SELECT p.id, p.user_id, p.name, p.from_name, p.from_email, p.smtp_host, p.smtp_port, p.smtp_user, p.smtp_password_encrypted, p.webhook_url, p.webhook_secret, p.tracking_enabled, p.created_at, p.updated_at, p.description, p.bounce_token, p.bounce_imap_host, p.bounce_imap_port, p.bounce_imap_user, p.bounce_imap_password_encrypted, p.bounce_imap_folder, p.bounce_imap_enabled, p.workspace_id, p.unsubscribe_template_id FROM projects p +SELECT p.id, p.user_id, p.name, p.from_name, p.from_email, p.smtp_host, p.smtp_port, p.smtp_user, p.smtp_password_encrypted, p.webhook_url, p.webhook_secret, p.tracking_enabled, p.created_at, p.updated_at, p.description, p.bounce_token, p.bounce_imap_host, p.bounce_imap_port, p.bounce_imap_user, p.bounce_imap_password_encrypted, p.bounce_imap_folder, p.bounce_imap_enabled, p.workspace_id, p.unsubscribe_template_id, p.smtp_allow_insecure_tls FROM projects p JOIN workspace_members m ON m.workspace_id = p.workspace_id WHERE m.user_id = $1 ORDER BY p.created_at DESC @@ -274,6 +278,7 @@ func (q *Queries) GetProjectsByUserID(ctx context.Context, userID uuid.UUID) ([] &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ); err != nil { return nil, err } @@ -289,7 +294,7 @@ func (q *Queries) GetProjectsByUserID(ctx context.Context, userID uuid.UUID) ([] } const getProjectsByWorkspaceForUser = `-- name: GetProjectsByWorkspaceForUser :many -SELECT p.id, p.user_id, p.name, p.from_name, p.from_email, p.smtp_host, p.smtp_port, p.smtp_user, p.smtp_password_encrypted, p.webhook_url, p.webhook_secret, p.tracking_enabled, p.created_at, p.updated_at, p.description, p.bounce_token, p.bounce_imap_host, p.bounce_imap_port, p.bounce_imap_user, p.bounce_imap_password_encrypted, p.bounce_imap_folder, p.bounce_imap_enabled, p.workspace_id, p.unsubscribe_template_id FROM projects p +SELECT p.id, p.user_id, p.name, p.from_name, p.from_email, p.smtp_host, p.smtp_port, p.smtp_user, p.smtp_password_encrypted, p.webhook_url, p.webhook_secret, p.tracking_enabled, p.created_at, p.updated_at, p.description, p.bounce_token, p.bounce_imap_host, p.bounce_imap_port, p.bounce_imap_user, p.bounce_imap_password_encrypted, p.bounce_imap_folder, p.bounce_imap_enabled, p.workspace_id, p.unsubscribe_template_id, p.smtp_allow_insecure_tls FROM projects p JOIN workspace_members m ON m.workspace_id = p.workspace_id WHERE p.workspace_id = $1 AND m.user_id = $2 ORDER BY p.created_at DESC @@ -334,6 +339,7 @@ func (q *Queries) GetProjectsByWorkspaceForUser(ctx context.Context, arg GetProj &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ); err != nil { return nil, err } @@ -349,7 +355,7 @@ func (q *Queries) GetProjectsByWorkspaceForUser(ctx context.Context, arg GetProj } const listProjectsWithBounceIMAP = `-- name: ListProjectsWithBounceIMAP :many -SELECT id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id FROM projects +SELECT id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id, smtp_allow_insecure_tls FROM projects WHERE bounce_imap_enabled = TRUE AND bounce_imap_host IS NOT NULL AND bounce_imap_user IS NOT NULL @@ -390,6 +396,7 @@ func (q *Queries) ListProjectsWithBounceIMAP(ctx context.Context) ([]Project, er &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ); err != nil { return nil, err } @@ -408,7 +415,7 @@ const rotateBounceToken = `-- name: RotateBounceToken :one UPDATE projects SET bounce_token = gen_random_uuid(), updated_at = NOW() WHERE id = $1 AND workspace_id IN (SELECT wm.workspace_id FROM workspace_members wm WHERE wm.user_id = $2) -RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id +RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id, smtp_allow_insecure_tls ` type RotateBounceTokenParams struct { @@ -444,6 +451,7 @@ func (q *Queries) RotateBounceToken(ctx context.Context, arg RotateBounceTokenPa &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ) return i, err } @@ -459,7 +467,7 @@ UPDATE projects SET updated_at = NOW() WHERE id = $1 AND workspace_id IN (SELECT wm.workspace_id FROM workspace_members wm WHERE wm.user_id = $2) -RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id +RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id, smtp_allow_insecure_tls ` type UpdateBounceIMAPParams struct { @@ -510,6 +518,7 @@ func (q *Queries) UpdateBounceIMAP(ctx context.Context, arg UpdateBounceIMAPPara &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ) return i, err } @@ -521,7 +530,7 @@ UPDATE projects SET updated_at = NOW() WHERE id = $1 AND workspace_id IN (SELECT wm.workspace_id FROM workspace_members wm WHERE wm.user_id = $2) -RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id +RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id, smtp_allow_insecure_tls ` type UpdateProjectParams struct { @@ -564,6 +573,7 @@ func (q *Queries) UpdateProject(ctx context.Context, arg UpdateProjectParams) (P &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ) return i, err } @@ -576,10 +586,11 @@ UPDATE projects SET smtp_password_encrypted = $6, from_name = $7, from_email = $8, + smtp_allow_insecure_tls = $9, updated_at = NOW() WHERE id = $1 AND workspace_id IN (SELECT wm.workspace_id FROM workspace_members wm WHERE wm.user_id = $2) -RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id +RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id, smtp_allow_insecure_tls ` type UpdateProjectSMTPParams struct { @@ -591,6 +602,7 @@ type UpdateProjectSMTPParams struct { SmtpPasswordEncrypted sql.NullString FromName sql.NullString FromEmail sql.NullString + SmtpAllowInsecureTls bool } func (q *Queries) UpdateProjectSMTP(ctx context.Context, arg UpdateProjectSMTPParams) (Project, error) { @@ -603,6 +615,7 @@ func (q *Queries) UpdateProjectSMTP(ctx context.Context, arg UpdateProjectSMTPPa arg.SmtpPasswordEncrypted, arg.FromName, arg.FromEmail, + arg.SmtpAllowInsecureTls, ) var i Project err := row.Scan( @@ -630,6 +643,7 @@ func (q *Queries) UpdateProjectSMTP(ctx context.Context, arg UpdateProjectSMTPPa &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ) return i, err } @@ -640,7 +654,7 @@ UPDATE projects SET updated_at = NOW() WHERE id = $1 AND workspace_id IN (SELECT wm.workspace_id FROM workspace_members wm WHERE wm.user_id = $2) -RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id +RETURNING id, user_id, name, from_name, from_email, smtp_host, smtp_port, smtp_user, smtp_password_encrypted, webhook_url, webhook_secret, tracking_enabled, created_at, updated_at, description, bounce_token, bounce_imap_host, bounce_imap_port, bounce_imap_user, bounce_imap_password_encrypted, bounce_imap_folder, bounce_imap_enabled, workspace_id, unsubscribe_template_id, smtp_allow_insecure_tls ` type UpdateProjectUnsubscribeTemplateParams struct { @@ -677,6 +691,7 @@ func (q *Queries) UpdateProjectUnsubscribeTemplate(ctx context.Context, arg Upda &i.BounceImapEnabled, &i.WorkspaceID, &i.UnsubscribeTemplateID, + &i.SmtpAllowInsecureTls, ) return i, err } diff --git a/backend/internal/handler/project.go b/backend/internal/handler/project.go index 3723604..eae64f3 100644 --- a/backend/internal/handler/project.go +++ b/backend/internal/handler/project.go @@ -4,9 +4,9 @@ import ( "encoding/json" "net/http" - "github.com/arkhe-systems/senddock/pkg/auth" "github.com/arkhe-systems/senddock/internal/response" "github.com/arkhe-systems/senddock/internal/service" + "github.com/arkhe-systems/senddock/pkg/auth" ) type ProjectHandler struct { @@ -142,12 +142,13 @@ func (h *ProjectHandler) Get(w http.ResponseWriter, r *http.Request) { } type updateSMTPRequest struct { - SmtpHost string `json:"smtp_host"` - SmtpPort int32 `json:"smtp_port"` - SmtpUser string `json:"smtp_user"` - SmtpPassword string `json:"smtp_password"` - FromName string `json:"from_name"` - FromEmail string `json:"from_email"` + SmtpHost string `json:"smtp_host"` + SmtpPort int32 `json:"smtp_port"` + SmtpUser string `json:"smtp_user"` + SmtpPassword string `json:"smtp_password"` + SmtpAllowInsecureTls bool `json:"smtp_allow_insecure_tls"` + FromName string `json:"from_name"` + FromEmail string `json:"from_email"` } type updateUnsubscribeTemplateRequest struct { @@ -205,7 +206,7 @@ func (h *ProjectHandler) UpdateSMTP(w http.ResponseWriter, r *http.Request) { return } - project, err := h.projectService.UpdateSMTP(r.Context(), projectID, userID, req.SmtpHost, req.SmtpPort, req.SmtpUser, req.SmtpPassword, req.FromName, req.FromEmail) + project, err := h.projectService.UpdateSMTP(r.Context(), projectID, userID, req.SmtpHost, req.SmtpPort, req.SmtpUser, req.SmtpPassword, req.FromName, req.FromEmail, req.SmtpAllowInsecureTls) if err != nil { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusInternalServerError) diff --git a/backend/internal/response/response.go b/backend/internal/response/response.go index 4d9b8e6..06f7282 100644 --- a/backend/internal/response/response.go +++ b/backend/internal/response/response.go @@ -18,6 +18,7 @@ type Project struct { SmtpHost *string `json:"smtp_host"` SmtpPort *int32 `json:"smtp_port"` SmtpUser *string `json:"smtp_user"` + SmtpAllowInsecureTls bool `json:"smtp_allow_insecure_tls"` CreatedAt string `json:"created_at"` UpdatedAt string `json:"updated_at"` UnsubscribeTemplateID *string `json:"unsubscribe_template_id"` @@ -126,6 +127,7 @@ func FromProject(p db.Project) Project { SmtpHost: nullStr(p.SmtpHost), SmtpPort: nullInt32(p.SmtpPort), SmtpUser: nullStr(p.SmtpUser), + SmtpAllowInsecureTls: p.SmtpAllowInsecureTls, CreatedAt: p.CreatedAt.Format(time.RFC3339), UpdatedAt: p.UpdatedAt.Format(time.RFC3339), UnsubscribeTemplateID: nullUUIDString(p.UnsubscribeTemplateID), diff --git a/backend/internal/service/email.go b/backend/internal/service/email.go index 8215dab..c83c0e7 100644 --- a/backend/internal/service/email.go +++ b/backend/internal/service/email.go @@ -1155,7 +1155,33 @@ func (s *EmailService) sendSMTPWithTimeouts(project db.Project, to, subject, htm addr := fmt.Sprintf("%s:%d", host, port) - return deliverSMTP(host, addr, user, pass, fromEmail, to, msg, port == 465, connectTimeout, sessionTimeout) + return deliverSMTP(host, addr, user, pass, fromEmail, to, msg, port == 465, connectTimeout, sessionTimeout, project.SmtpAllowInsecureTls) +} + +// smtpTLSConfig builds the TLS settings for outgoing mail. Certificates are verified +// unless the project opted out, which exists for relays serving a self-signed or expired +// certificate. +func smtpTLSConfig(host string, allowInsecureTLS bool) *tls.Config { + config := &tls.Config{ServerName: host} + if allowInsecureTLS { + config.InsecureSkipVerify = true + } + return config +} + +func isCertificateError(err error) bool { + msg := err.Error() + return strings.Contains(msg, "x509:") || strings.Contains(msg, "failed to verify certificate") +} + +// certificateError turns a failed handshake into something an operator can act on. +// Verification is on by default now, so a self-signed relay fails with a message naming +// the project setting that resolves it instead of a bare handshake error. +func certificateError(host string, allowInsecureTLS bool, err error) error { + if allowInsecureTLS { + return fmt.Errorf("TLS handshake with %s failed: %w", host, err) + } + return fmt.Errorf("TLS certificate verification failed for %s: %w. Renew the certificate, or enable \"Allow insecure TLS\" for this project under Project -> SMTP if the relay uses a self-signed or expired one", host, err) } // headerSafe strips CR and LF so a value cannot end its header line early. Subjects @@ -1203,8 +1229,8 @@ func wrapDialError(addr string, connectTimeout time.Duration, err error) error { return fmt.Errorf("smtp connection failed: %w", err) } -func deliverSMTP(host, addr, user, pass, from, to string, msg []byte, implicitTLS bool, connectTimeout, sessionTimeout time.Duration) error { - tlsConfig := &tls.Config{ServerName: host, InsecureSkipVerify: true} +func deliverSMTP(host, addr, user, pass, from, to string, msg []byte, implicitTLS bool, connectTimeout, sessionTimeout time.Duration, allowInsecureTLS bool) error { + tlsConfig := smtpTLSConfig(host, allowInsecureTLS) dialer := &net.Dialer{Timeout: connectTimeout} var conn net.Conn @@ -1215,6 +1241,9 @@ func deliverSMTP(host, addr, user, pass, from, to string, msg []byte, implicitTL conn, err = dialer.Dial("tcp", addr) } if err != nil { + if isCertificateError(err) { + return certificateError(host, allowInsecureTLS, err) + } return wrapDialError(addr, connectTimeout, err) } defer conn.Close() @@ -1232,6 +1261,9 @@ func deliverSMTP(host, addr, user, pass, from, to string, msg []byte, implicitTL if !implicitTLS { if ok, _ := client.Extension("STARTTLS"); ok { if err = client.StartTLS(tlsConfig); err != nil { + if isCertificateError(err) { + return certificateError(host, allowInsecureTLS, err) + } return fmt.Errorf("smtp starttls failed: %w", err) } } diff --git a/backend/internal/service/email_tls_test.go b/backend/internal/service/email_tls_test.go new file mode 100644 index 0000000..b888a72 --- /dev/null +++ b/backend/internal/service/email_tls_test.go @@ -0,0 +1,72 @@ +package service + +import ( + "errors" + "strings" + "testing" +) + +// Certificates are verified unless a project explicitly opted out. This is the property +// that stops an on-path attacker from reading SMTP credentials and mail. +func TestSMTPTLSConfigVerifiesCertificatesByDefault(t *testing.T) { + config := smtpTLSConfig("smtp.example.test", false) + + if config.InsecureSkipVerify { + t.Fatal("certificate verification is disabled without an opt-out") + } + if config.ServerName != "smtp.example.test" { + t.Fatalf("ServerName = %q, want the relay host so SNI and verification work", config.ServerName) + } +} + +// The opt-out exists for relays with a self-signed or expired certificate, and it has to +// stay scoped to the project that asked for it. +func TestSMTPTLSConfigHonoursThePerProjectOptOut(t *testing.T) { + if !smtpTLSConfig("smtp.example.test", true).InsecureSkipVerify { + t.Fatal("the per-project opt-out did not disable verification") + } +} + +func TestIsCertificateError(t *testing.T) { + cases := []struct { + err error + want bool + }{ + {errors.New("x509: certificate signed by unknown authority"), true}, + {errors.New("tls: failed to verify certificate: x509: certificate has expired"), true}, + {errors.New("dial tcp 10.0.0.1:587: connection refused"), false}, + {errors.New("smtp auth failed: 535 authentication failed"), false}, + } + for _, tc := range cases { + if got := isCertificateError(tc.err); got != tc.want { + t.Errorf("isCertificateError(%q) = %v, want %v", tc.err, got, tc.want) + } + } +} + +// A verification failure has to name the fix. Without the hint, turning verification on +// would turn every self-signed relay into a support thread. +func TestCertificateErrorPointsAtTheOptOut(t *testing.T) { + cause := errors.New("x509: certificate signed by unknown authority") + + blocked := certificateError("smtp.example.test", false, cause).Error() + if !strings.Contains(blocked, "smtp.example.test") { + t.Errorf("message does not name the relay: %q", blocked) + } + if !strings.Contains(blocked, "Allow insecure TLS") { + t.Errorf("message does not point at the project setting: %q", blocked) + } + if !strings.Contains(blocked, cause.Error()) { + t.Errorf("message dropped the underlying cause: %q", blocked) + } + + // With the opt-out already on there is nothing to suggest, and the underlying + // handshake failure is still reported. + allowed := certificateError("smtp.example.test", true, cause).Error() + if strings.Contains(allowed, "Allow insecure TLS") { + t.Errorf("message suggests a setting that is already enabled: %q", allowed) + } + if !strings.Contains(allowed, cause.Error()) { + t.Errorf("message dropped the underlying cause: %q", allowed) + } +} diff --git a/backend/internal/service/project.go b/backend/internal/service/project.go index 0d82c72..107e66f 100644 --- a/backend/internal/service/project.go +++ b/backend/internal/service/project.go @@ -125,7 +125,7 @@ func (s *ProjectService) GetByID(ctx context.Context, projectID, userID string) }) } -func (s *ProjectService) UpdateSMTP(ctx context.Context, projectID, userID, smtpHost string, smtpPort int32, smtpUser, smtpPassword, fromName, fromEmail string) (db.Project, error) { +func (s *ProjectService) UpdateSMTP(ctx context.Context, projectID, userID, smtpHost string, smtpPort int32, smtpUser, smtpPassword, fromName, fromEmail string, allowInsecureTLS bool) (db.Project, error) { pid, err := uuid.Parse(projectID) if err != nil { return db.Project{}, errors.New("invalid project id") @@ -154,6 +154,7 @@ func (s *ProjectService) UpdateSMTP(ctx context.Context, projectID, userID, smtp SmtpPasswordEncrypted: sql.NullString{String: encryptedPass, Valid: encryptedPass != ""}, FromName: sql.NullString{String: fromName, Valid: fromName != ""}, FromEmail: sql.NullString{String: fromEmail, Valid: fromEmail != ""}, + SmtpAllowInsecureTls: allowInsecureTLS, }) } @@ -253,7 +254,7 @@ func (s *ProjectService) UpdateBounceIMAP(ctx context.Context, projectID, userID UserID: uid, BounceImapHost: hostNS, BounceImapPort: portNS, - BounceImapUser: userNS, + BounceImapUser: userNS, BounceImapPasswordEncrypted: encrypted, BounceImapFolder: folder, BounceImapEnabled: enabled, diff --git a/backend/migrations/20260927140000_project_smtp_insecure_tls.sql b/backend/migrations/20260927140000_project_smtp_insecure_tls.sql new file mode 100644 index 0000000..9c490db --- /dev/null +++ b/backend/migrations/20260927140000_project_smtp_insecure_tls.sql @@ -0,0 +1,9 @@ +-- +goose Up +-- Outgoing SMTP used to accept any certificate. Verification is on by default now, so a +-- project pointing at a relay with a self-signed or expired certificate needs an explicit +-- opt-out. Keeping it per project keeps the exception scoped and visible instead of +-- weakening every project on the instance. +ALTER TABLE projects ADD COLUMN smtp_allow_insecure_tls BOOLEAN NOT NULL DEFAULT false; + +-- +goose Down +ALTER TABLE projects DROP COLUMN smtp_allow_insecure_tls; diff --git a/backend/sqlc/queries/projects.sql b/backend/sqlc/queries/projects.sql index ec45885..9c0188b 100644 --- a/backend/sqlc/queries/projects.sql +++ b/backend/sqlc/queries/projects.sql @@ -53,6 +53,7 @@ UPDATE projects SET smtp_password_encrypted = $6, from_name = $7, from_email = $8, + smtp_allow_insecure_tls = $9, updated_at = NOW() WHERE id = $1 AND workspace_id IN (SELECT wm.workspace_id FROM workspace_members wm WHERE wm.user_id = $2) diff --git a/docs/changelog.md b/docs/changelog.md index e7c6eb3..4227398 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -23,6 +23,8 @@ Pre-1.0 minor releases may contain breaking changes — check the version's note ### Security - **Headers could be injected through the subject.** Subjects are built from subscriber-supplied text — their name, email and custom fields are substituted into them — and that text was written into the message headers as-is. A value containing a line break could therefore add headers of the attacker's choosing, a `Bcc` for example, to the message SendDock sent. Carriage returns and line feeds are now stripped from every value that reaches a header: the subject, the sender name, the recipient and the unsubscribe URL. +- **Certificates are verified on outgoing SMTP.** Every connection accepted any certificate, so an attacker able to sit between the instance and the relay could read SMTP credentials and the mail itself while the connection still looked encrypted. Verification is now on for both implicit TLS and STARTTLS. A relay with a self-signed or expired certificate can be kept working per project under **Project → SMTP → Allow insecure TLS**, and the failure message names that setting so the fix is one click away. + ## [0.8.2] — 2026-09-01 diff --git a/docs/guide/smtp.md b/docs/guide/smtp.md index 990e098..69c1c65 100644 --- a/docs/guide/smtp.md +++ b/docs/guide/smtp.md @@ -17,6 +17,16 @@ Go to **SMTP Settings** in the project sidebar and fill in: | From Name | Display name for the sender (optional) | `My Newsletter` | | From Email | Email shown as sender (optional, defaults to username) | `noreply@mydomain.com` | +## TLS certificates + +SendDock verifies the certificate your SMTP relay presents, on both implicit TLS (465) and STARTTLS (25, 587, 2525). If verification fails, the send — and the **Test Connection** button — stop with a message naming the relay and the reason, for example `x509: certificate has expired`. + +That is the point of verification: without it, anyone able to sit between your instance and the relay can read the SMTP password and the mail itself, while the connection still looks encrypted. + +The fix is almost always on the mail server: renew the certificate. Self-hosted relays (Poste.io, Mailcow, Mail-in-a-Box) usually renew through Let's Encrypt automatically, but the renewal can silently stop working if DNS or the ACME challenge broke. + +If the relay is one you control and you cannot renew the certificate right now, **Project → SMTP → Allow insecure TLS** keeps sending without verifying it. The setting is per project, so the exception stays scoped to the relay that needs it instead of weakening every project on the instance. Use it as a stopgap, not as a configuration: while it is on, an on-path attacker can read your SMTP credentials and mail. SendDock flags it in the SMTP form so it does not go unnoticed. + ## Testing After saving, click **Test Connection**. SendDock will send a test email to the configured from address (or SMTP username) to verify the connection works. diff --git a/frontend/src/stores/projects.ts b/frontend/src/stores/projects.ts index 49cae33..327dbff 100644 --- a/frontend/src/stores/projects.ts +++ b/frontend/src/stores/projects.ts @@ -11,6 +11,7 @@ export interface Project { smtp_host: string | null smtp_port: number | null smtp_user: string | null + smtp_allow_insecure_tls: boolean created_at: string updated_at: string } diff --git a/frontend/src/views/project/SmtpSection.vue b/frontend/src/views/project/SmtpSection.vue index c583322..4d77411 100644 --- a/frontend/src/views/project/SmtpSection.vue +++ b/frontend/src/views/project/SmtpSection.vue @@ -5,6 +5,7 @@ import { useToastStore } from '@/stores/toast' import type { Project } from '@/stores/projects' import AppInput from '@/components/ui/AppInput.vue' import AppButton from '@/components/ui/AppButton.vue' +import AppCheckbox from '@/components/ui/AppCheckbox.vue' const props = defineProps<{ project: Project }>() const emit = defineEmits<{ updated: [] }>() @@ -19,6 +20,7 @@ const smtpUser = ref('') const smtpPassword = ref('') const fromName = ref('') const fromEmail = ref('') +const smtpAllowInsecureTls = ref(false) const hasSmtpConfig = computed(() => !!props.project.smtp_host) @@ -28,6 +30,7 @@ onMounted(() => { smtpUser.value = props.project.smtp_user ?? '' fromName.value = props.project.from_name ?? '' fromEmail.value = props.project.from_email ?? '' + smtpAllowInsecureTls.value = props.project.smtp_allow_insecure_tls ?? false }) async function handleSave() { @@ -47,6 +50,7 @@ async function handleSave() { smtp_password: smtpPassword.value, from_name: fromName.value, from_email: fromEmail.value, + smtp_allow_insecure_tls: smtpAllowInsecureTls.value, }, }) toast.success('SMTP settings saved') @@ -92,6 +96,18 @@ async function handleTest() { + +
From cd47f68fea12362a17ba042aff0d5a692dceb993 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Thu, 10 Sep 2026 22:31:24 -0500 Subject: [PATCH 06/27] fix(workspace): guard every demotion, and drop the role the database rejects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems in the same code path. The guard that refuses to remove the last workspace owner ran only when the target role was `member` — a value the database has never allowed. Demoting an owner to admin, developer or viewer therefore went through and left the workspace with no owner, which no screen can undo. The guard now runs for every target role except owner. `member` itself is gone. The constraint has permitted only owner, admin, developer and viewer since the table was created, so no row could ever carry it: assigning it validated in the API, failed at the constraint and surfaced as a 500, after the account had already been created. Its capabilities were identical to admin's and the dashboard never offered it. The API now rejects it as an invalid role, and a constraint violation maps to a 400 rather than a 500. (cherry picked from commit cfec47cea00af425c192498aa7897e37e133be3c) --- CHANGELOG.md | 6 +++ backend/internal/service/workspace.go | 40 ++++++++++---- backend/internal/service/workspace_caps.go | 27 ++++------ .../internal/service/workspace_role_test.go | 54 +++++++++++++++++-- docs/api/workspaces.md | 4 +- docs/changelog.md | 6 +++ frontend/src/stores/workspaces.ts | 6 +-- 7 files changed, 107 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf405c3..f491da9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,14 @@ Releases are also published on [GitHub](https://github.com/arkhe-systems/senddoc ## [Unreleased] +### Removed + +- **The `member` workspace role.** The API accepted it and this changelog advertised it, but the database constraint never allowed it, so assigning it failed with a 500 after leaving an account behind. Its capability set was also identical to `admin`. The roles are `owner`, `admin`, `developer` and `viewer`, which is what the database has permitted all along. + ### Fixed +- **Demoting the last workspace owner no longer leaves the workspace ownerless.** The guard that refuses to remove the last owner ran only when the target role was `member` — a value the database rejects — so demoting an owner to `admin`, `developer` or `viewer` went through and left the workspace with no owner, unrecoverable from the UI. The guard now runs for every target role except `owner`. + - **A restart no longer re-sends a broadcast another instance is delivering.** Broadcast jobs left in `sending` were reset to `retry` on every startup regardless of who owned them, so starting or rolling one instance re-queued jobs another instance was still sending, and those recipients received the same email twice. A claimed job now carries the worker's id and a lease, and startup recovery only reclaims jobs whose lease has expired — work in flight is left alone. ### Security diff --git a/backend/internal/service/workspace.go b/backend/internal/service/workspace.go index 312085b..5fea1df 100644 --- a/backend/internal/service/workspace.go +++ b/backend/internal/service/workspace.go @@ -10,12 +10,12 @@ import ( "github.com/arkhe-systems/senddock/internal/db" "github.com/arkhe-systems/senddock/pkg/license" "github.com/google/uuid" + "github.com/lib/pq" "golang.org/x/crypto/bcrypt" ) const ( - WorkspaceRoleOwner = "owner" - WorkspaceRoleMember = "member" + WorkspaceRoleOwner = "owner" ) var ( @@ -273,7 +273,7 @@ func (s *WorkspaceService) UpdateMemberRole(ctx context.Context, workspaceID, ac if err := s.requireOwner(ctx, workspaceID, actorID); err != nil { return err } - if role == WorkspaceRoleMember { + if demotionNeedsOwnerGuard(role) { if err := s.guardLastOwner(ctx, workspaceID, targetID); err != nil { return err } @@ -283,7 +283,32 @@ func (s *WorkspaceService) UpdateMemberRole(ctx context.Context, workspaceID, ac UserID: targetID, Role: role, }) - return err + if err != nil { + // The database rejects a role it does not know. That is a bad request, not a + // server failure, so do not let the constraint error surface as a 500. + if isCheckConstraintViolation(err) { + return ErrInvalidRole + } + return err + } + return nil +} + +// demotionNeedsOwnerGuard reports whether moving a member to role can strip the last owner +// from the workspace. Only a target of owner leaves ownership intact; every other role +// needs the guard, which is why it cannot be tied to one specific role. +func demotionNeedsOwnerGuard(role string) bool { + return role != WorkspaceRoleOwner +} + +// isCheckConstraintViolation reports whether the database rejected the value itself rather +// than failing to execute the statement. +func isCheckConstraintViolation(err error) bool { + var pqErr *pq.Error + if errors.As(err, &pqErr) { + return pqErr.Code == "23514" + } + return false } func (s *WorkspaceService) RemoveMember(ctx context.Context, workspaceID, actorID, targetID uuid.UUID) error { @@ -373,12 +398,9 @@ func normalizeRole(role string) string { return WorkspaceRoleDeveloper case WorkspaceRoleViewer: return WorkspaceRoleViewer - case WorkspaceRoleMember: - return WorkspaceRoleMember default: - // Empty or unrecognized roles fall through to "" so callers reject them - // with ErrInvalidRole, instead of silently defaulting to the - // near-admin "member" role. + // Empty or unrecognized roles fall through to "" so callers reject them with + // ErrInvalidRole instead of quietly picking a role for the caller. return "" } } diff --git a/backend/internal/service/workspace_caps.go b/backend/internal/service/workspace_caps.go index 4c89f8f..c51d9ed 100644 --- a/backend/internal/service/workspace_caps.go +++ b/backend/internal/service/workspace_caps.go @@ -11,17 +11,17 @@ import ( type Capability string const ( - CapMembersManage Capability = "members.manage" - CapWorkspaceDelete Capability = "workspace.delete" - CapProjectSettings Capability = "project.settings" - CapTemplatesWrite Capability = "templates.write" - CapSubscribersWrite Capability = "subscribers.write" - CapSendTransactional Capability = "send.transactional" - CapBroadcast Capability = "broadcast" - CapCampaignsWrite Capability = "campaigns.write" - CapAPIKeysManage Capability = "api_keys.manage" - CapSuppressionsWrite Capability = "suppressions.write" - CapWebhooksWrite Capability = "webhooks.write" + CapMembersManage Capability = "members.manage" + CapWorkspaceDelete Capability = "workspace.delete" + CapProjectSettings Capability = "project.settings" + CapTemplatesWrite Capability = "templates.write" + CapSubscribersWrite Capability = "subscribers.write" + CapSendTransactional Capability = "send.transactional" + CapBroadcast Capability = "broadcast" + CapCampaignsWrite Capability = "campaigns.write" + CapAPIKeysManage Capability = "api_keys.manage" + CapSuppressionsWrite Capability = "suppressions.write" + CapWebhooksWrite Capability = "webhooks.write" ) const ( @@ -47,11 +47,6 @@ var capabilities = map[string]map[Capability]bool{ WorkspaceRoleDeveloper: { CapSendTransactional: true, }, - WorkspaceRoleMember: { - CapProjectSettings: true, CapTemplatesWrite: true, CapSubscribersWrite: true, - CapSendTransactional: true, CapBroadcast: true, CapCampaignsWrite: true, - CapAPIKeysManage: true, CapSuppressionsWrite: true, CapWebhooksWrite: true, - }, WorkspaceRoleViewer: {}, } diff --git a/backend/internal/service/workspace_role_test.go b/backend/internal/service/workspace_role_test.go index 67d643d..61739e2 100644 --- a/backend/internal/service/workspace_role_test.go +++ b/backend/internal/service/workspace_role_test.go @@ -1,6 +1,12 @@ package service -import "testing" +import ( + "errors" + "fmt" + "testing" + + "github.com/lib/pq" +) func TestNormalizeRoleRejectsEmptyAndUnknown(t *testing.T) { cases := map[string]string{ @@ -8,16 +14,18 @@ func TestNormalizeRoleRejectsEmptyAndUnknown(t *testing.T) { "admin": WorkspaceRoleAdmin, "developer": WorkspaceRoleDeveloper, "viewer": WorkspaceRoleViewer, - "member": WorkspaceRoleMember, - "MEMBER": WorkspaceRoleMember, " Admin ": WorkspaceRoleAdmin, - // empty and unknown roles must normalize to "" so callers reject them - // instead of silently defaulting to the near-admin "member" role. + // Empty and unknown roles must normalize to "" so callers reject them with + // ErrInvalidRole instead of quietly picking a role for the caller. "": "", " ": "", "bogus": "", "root": "", "manager": "", + // "member" is not a role the database accepts, so treating it as valid could + // only ever end in a constraint violation. + "member": "", + "MEMBER": "", } for input, want := range cases { if got := normalizeRole(input); got != want { @@ -25,3 +33,39 @@ func TestNormalizeRoleRejectsEmptyAndUnknown(t *testing.T) { } } } + +// Every role except owner can strip ownership from a workspace, so each one has to run the +// last-owner guard. Tying the guard to a single role is what left demoted workspaces +// without an owner. +func TestDemotionNeedsOwnerGuard(t *testing.T) { + cases := map[string]bool{ + WorkspaceRoleOwner: false, + WorkspaceRoleAdmin: true, + WorkspaceRoleDeveloper: true, + WorkspaceRoleViewer: true, + } + for role, want := range cases { + if got := demotionNeedsOwnerGuard(role); got != want { + t.Errorf("demotionNeedsOwnerGuard(%q) = %v, want %v", role, got, want) + } + } +} + +// A role the database refuses is a bad request, not a server failure. +func TestIsCheckConstraintViolation(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + {"check violation", &pq.Error{Code: "23514"}, true}, + {"wrapped check violation", fmt.Errorf("update member role: %w", &pq.Error{Code: "23514"}), true}, + {"unique violation", &pq.Error{Code: "23505"}, false}, + {"plain error", errors.New("boom"), false}, + } + for _, tc := range cases { + if got := isCheckConstraintViolation(tc.err); got != tc.want { + t.Errorf("%s: isCheckConstraintViolation(%v) = %v, want %v", tc.name, tc.err, got, tc.want) + } + } +} diff --git a/docs/api/workspaces.md b/docs/api/workspaces.md index 0714b91..d378618 100644 --- a/docs/api/workspaces.md +++ b/docs/api/workspaces.md @@ -117,7 +117,7 @@ Owner only. The user must already have a SendDock account on this instance. Returns `404` (`{"error":"user not found"}`) if no account uses that email — use the [Create user](#create-user) endpoint below instead. -`role` defaults to `member`. Valid values: `owner`, `admin`, `developer`, `member`, `viewer`. See the [role matrix](../guide/workspaces#roles-capabilities). +`role` is required — omitting it or sending an unknown value returns `400`. Valid values: `owner`, `admin`, `developer`, `viewer`. See the [role matrix](../guide/workspaces#roles-capabilities). **Response — 201 Created** @@ -155,7 +155,7 @@ Owner only. Creates a new SendDock account and adds them to the workspace at the | `email` | yes | Must be unique. Returns `409 email already registered` otherwise. | | `name` | yes | Display name shown in member lists and audit log. | | `password` | yes | Minimum 8 characters. Pass it to the user out of band; they can change it after first login. | -| `role` | no | Defaults to `member`. Same enum as [Add member](#add-existing-member). | +| `role` | yes | Same enum as [Add member](#add-existing-member): `owner`, `admin`, `developer`, `viewer`. | **Response — 201 Created** diff --git a/docs/changelog.md b/docs/changelog.md index 4227398..f7230fa 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -16,8 +16,14 @@ Pre-1.0 minor releases may contain breaking changes — check the version's note ## [Unreleased] +### Removed + +- **The `member` workspace role.** The API accepted it and this changelog advertised it, but the database constraint never allowed it, so assigning it failed with a 500 after leaving an account behind. Its capability set was also identical to `admin`. The roles are `owner`, `admin`, `developer` and `viewer`, which is what the database has permitted all along. + ### Fixed +- **Demoting the last workspace owner no longer leaves the workspace ownerless.** The guard that refuses to remove the last owner ran only when the target role was `member` — a value the database rejects — so demoting an owner to `admin`, `developer` or `viewer` went through and left the workspace with no owner, unrecoverable from the UI. The guard now runs for every target role except `owner`. + - **A restart no longer re-sends a broadcast another instance is delivering.** Broadcast jobs left in `sending` were reset to `retry` on every startup regardless of who owned them, so starting or rolling one instance re-queued jobs another instance was still sending, and those recipients received the same email twice. A claimed job now carries the worker's id and a lease, and startup recovery only reclaims jobs whose lease has expired — work in flight is left alone. ### Security diff --git a/frontend/src/stores/workspaces.ts b/frontend/src/stores/workspaces.ts index fe3607e..081fcd3 100644 --- a/frontend/src/stores/workspaces.ts +++ b/frontend/src/stores/workspaces.ts @@ -11,7 +11,7 @@ export interface Workspace { role?: string } -export type WorkspaceRole = 'owner' | 'admin' | 'developer' | 'member' | 'viewer' +export type WorkspaceRole = 'owner' | 'admin' | 'developer' | 'viewer' export interface WorkspaceMember { user_id: string @@ -25,7 +25,6 @@ export const ROLE_LABEL: Record = { owner: 'Owner', admin: 'Admin', developer: 'Developer', - member: 'Member', viewer: 'Viewer', } @@ -33,7 +32,6 @@ export const ROLE_DESCRIPTION: Record = { owner: 'Full access — can manage members, delete the workspace, and do everything an admin can.', admin: 'Everything project-related: settings, templates, subscribers, sends, broadcasts, API keys.', developer: 'Send transactional email only (`/send`). Read-only on templates, subscribers, logs.', - member: 'Legacy role kept for backward compatibility. Same access as admin.', viewer: 'Read-only — view templates, subscribers, logs, analytics. Cannot send or modify anything.', } @@ -96,7 +94,7 @@ export const useWorkspaceStore = defineStore('workspaces', () => { return res.members || [] } - async function addMember(id: string, email: string, role: WorkspaceRole = 'member') { + async function addMember(id: string, email: string, role: WorkspaceRole = 'developer') { return api(`/workspaces/${id}/members`, { method: 'POST', body: { email, role }, From 43fa9ae4c0609594260e0624507338a5d125c8a6 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Fri, 11 Sep 2026 18:26:38 -0500 Subject: [PATCH 07/27] fix(segments): compare date custom fields as dates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greater/less-than rules compiled a custom field to a numeric cast, but date fields are stored as YYYY-MM-DD, so the cast failed the moment the segment ran: the preview returned an error and a broadcast targeting it never sent, while the segment itself saved without complaint. The comparison now casts by the rule's value — dates as dates, numbers as numbers — and a value that cannot be ordered is refused when the segment is saved instead of when it is used. Both paths are exercised against a real database, which is where the original cast failed rather than in the query builder. (cherry picked from commit 8656a529b4ebe8403462ca2396d9a88e4e789cf3) --- CHANGELOG.md | 2 + .../service/segment_comparison_test.go | 80 +++++++++++++++++++ backend/pkg/segments/segments.go | 56 ++++++++++++- backend/pkg/segments/segments_test.go | 46 +++++++++++ docs/changelog.md | 2 + 5 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 backend/internal/service/segment_comparison_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f491da9..98cdc5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Releases are also published on [GitHub](https://github.com/arkhe-systems/senddoc ### Fixed +- **Date rules no longer break a segment.** `greater than` and `less than` compiled a custom field to a numeric cast, but date fields are stored as `YYYY-MM-DD`, so a rule that saved without complaint failed when the segment actually ran: the preview errored and any broadcast targeting it never sent. The comparison now casts to match the value — dates compare as dates, numbers as numbers — and a value that cannot be ordered is refused while the segment is being saved rather than when it is used. + - **Demoting the last workspace owner no longer leaves the workspace ownerless.** The guard that refuses to remove the last owner ran only when the target role was `member` — a value the database rejects — so demoting an owner to `admin`, `developer` or `viewer` went through and left the workspace with no owner, unrecoverable from the UI. The guard now runs for every target role except `owner`. - **A restart no longer re-sends a broadcast another instance is delivering.** Broadcast jobs left in `sending` were reset to `retry` on every startup regardless of who owned them, so starting or rolling one instance re-queued jobs another instance was still sending, and those recipients received the same email twice. A claimed job now carries the worker's id and a lease, and startup recovery only reclaims jobs whose lease has expired — work in flight is left alone. diff --git a/backend/internal/service/segment_comparison_test.go b/backend/internal/service/segment_comparison_test.go new file mode 100644 index 0000000..f5216be --- /dev/null +++ b/backend/internal/service/segment_comparison_test.go @@ -0,0 +1,80 @@ +package service + +import ( + "context" + "strconv" + "testing" + + "github.com/arkhe-systems/senddock/pkg/segments" +) + +// The comparison a date rule compiles to has to run against a real database, because that +// is where the old cast failed: the segment saved fine and then took the preview and the +// broadcast down with an invalid-input-syntax error. +func TestSegmentDateComparisonRunsAgainstPostgres(t *testing.T) { + conn, _ := leaseTestDB(t) + ctx := context.Background() + + _, projectID, _ := leaseFixtures(t, conn) + + subscribers := []struct{ email, signup string }{ + {"before@example.test", "2025-12-31"}, + {"after@example.test", "2026-02-01"}, + } + for _, s := range subscribers { + if _, err := conn.ExecContext(ctx, + `INSERT INTO subscribers (project_id, email, metadata) VALUES ($1, $2, $3::jsonb)`, + projectID, s.email, `{"signup":"`+s.signup+`"}`, + ); err != nil { + t.Fatalf("insert subscriber %s: %v", s.email, err) + } + } + + where, args := segments.BuildWhere(segments.Predicate{ + Match: "all", + Rules: []segments.Rule{{Field: "custom.signup", Op: "gt", Value: "2026-01-01"}}, + }, 2) + + var matched int + query := `SELECT COUNT(*) FROM subscribers WHERE project_id = $1 AND ` + where + if err := conn.QueryRowContext(ctx, query, append([]any{projectID}, args...)...).Scan(&matched); err != nil { + t.Fatalf("date comparison failed to run: %v\nquery: %s", err, query) + } + if matched != 1 { + t.Fatalf("date comparison matched %d subscribers, want 1 (only the 2026-02-01 signup)", matched) + } +} + +// The numeric path has to keep working alongside it. +func TestSegmentNumberComparisonRunsAgainstPostgres(t *testing.T) { + conn, _ := leaseTestDB(t) + ctx := context.Background() + + _, projectID, _ := leaseFixtures(t, conn) + + for _, s := range []struct { + email string + seats int + }{{"small@example.test", 3}, {"big@example.test", 50}} { + if _, err := conn.ExecContext(ctx, + `INSERT INTO subscribers (project_id, email, metadata) VALUES ($1, $2, $3::jsonb)`, + projectID, s.email, `{"seats":`+strconv.Itoa(s.seats)+`}`, + ); err != nil { + t.Fatalf("insert subscriber %s: %v", s.email, err) + } + } + + where, args := segments.BuildWhere(segments.Predicate{ + Match: "all", + Rules: []segments.Rule{{Field: "custom.seats", Op: "gt", Value: float64(10)}}, + }, 2) + + var matched int + query := `SELECT COUNT(*) FROM subscribers WHERE project_id = $1 AND ` + where + if err := conn.QueryRowContext(ctx, query, append([]any{projectID}, args...)...).Scan(&matched); err != nil { + t.Fatalf("number comparison failed to run: %v\nquery: %s", err, query) + } + if matched != 1 { + t.Fatalf("number comparison matched %d subscribers, want 1", matched) + } +} diff --git a/backend/pkg/segments/segments.go b/backend/pkg/segments/segments.go index 5befcb0..1e27b87 100644 --- a/backend/pkg/segments/segments.go +++ b/backend/pkg/segments/segments.go @@ -5,7 +5,9 @@ import ( "errors" "fmt" "regexp" + "strconv" "strings" + "time" "github.com/lib/pq" ) @@ -13,8 +15,50 @@ import ( var ( ErrInvalidPredicate = errors.New("invalid segment predicate") keyPattern = regexp.MustCompile(`^[a-zA-Z0-9_]+$`) + datePattern = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}$`) ) +// comparisonCast picks the SQL cast for a greater/less-than rule. Custom field values live +// in the metadata JSON as text, so the cast has to match the field: a date stored as +// YYYY-MM-DD cast to numeric fails when the query runs, which turns a saved segment into a +// preview error and a broadcast that never sends. The rule value carries the format the +// picker produced, and that decides which cast applies. +func comparisonCast(value any) string { + if isDateValue(value) { + return "::date" + } + return "::numeric" +} + +func isDateValue(value any) bool { + s, ok := value.(string) + if !ok { + return false + } + s = strings.TrimSpace(s) + if !datePattern.MatchString(s) { + return false + } + _, err := time.Parse("2006-01-02", s) + return err == nil +} + +// comparableValue reports whether a greater/less-than rule carries something the database +// can order, so a rule that could only fail later is rejected while the segment is saved. +func comparableValue(value any) bool { + switch v := value.(type) { + case float64, int, int64: + return true + case string: + if isDateValue(v) { + return true + } + _, err := strconv.ParseFloat(strings.TrimSpace(v), 64) + return err == nil + } + return false +} + type Predicate struct { Match string `json:"match"` Rules []Rule `json:"rules"` @@ -63,7 +107,11 @@ func validateRule(rule Rule) error { return fmt.Errorf("%w: invalid custom field key", ErrInvalidPredicate) } switch rule.Op { - case "eq", "neq", "contains", "gt", "lt": + case "eq", "neq", "contains": + case "gt", "lt": + if !comparableValue(rule.Value) { + return fmt.Errorf("%w: greater/less than needs a number or a YYYY-MM-DD date", ErrInvalidPredicate) + } default: return fmt.Errorf("%w: custom fields support eq/neq/contains/gt/lt", ErrInvalidPredicate) } @@ -116,9 +164,11 @@ func buildRuleSQL(rule Rule, argIdx int) (string, []any) { case "contains": return column + " ILIKE '%' || " + placeholder + " || '%'", []any{fmt.Sprintf("%v", rule.Value)} case "gt": - return "(" + column + ")::numeric > " + placeholder, []any{rule.Value} + cast := comparisonCast(rule.Value) + return "(" + column + ")" + cast + " > " + placeholder + cast, []any{fmt.Sprintf("%v", rule.Value)} case "lt": - return "(" + column + ")::numeric < " + placeholder, []any{rule.Value} + cast := comparisonCast(rule.Value) + return "(" + column + ")" + cast + " < " + placeholder + cast, []any{fmt.Sprintf("%v", rule.Value)} default: return column + " = " + placeholder, []any{fmt.Sprintf("%v", rule.Value)} } diff --git a/backend/pkg/segments/segments_test.go b/backend/pkg/segments/segments_test.go index c2c197d..e397b53 100644 --- a/backend/pkg/segments/segments_test.go +++ b/backend/pkg/segments/segments_test.go @@ -22,6 +22,14 @@ func TestParsePredicateValidation(t *testing.T) { {"bad tags op", `{"rules":[{"field":"tags","op":"eq","value":["x"]}]}`, true}, {"unknown field", `{"rules":[{"field":"foo","op":"eq","value":"x"}]}`, true}, {"bad custom key", `{"rules":[{"field":"custom.a-b","op":"eq","value":"x"}]}`, true}, + {"custom gt with a date", `{"rules":[{"field":"custom.signup","op":"gt","value":"2026-01-01"}]}`, false}, + {"custom gt with a number", `{"rules":[{"field":"custom.seats","op":"gt","value":42}]}`, false}, + {"custom gt with a numeric string", `{"rules":[{"field":"custom.seats","op":"gt","value":"42"}]}`, false}, + {"custom lt with a date", `{"rules":[{"field":"custom.signup","op":"lt","value":"2026-06-30"}]}`, false}, + // A value the database cannot order would only fail when the segment is evaluated, + // so it has to be refused at save time. + {"custom gt with text", `{"rules":[{"field":"custom.plan","op":"gt","value":"pro"}]}`, true}, + {"custom lt with an impossible date", `{"rules":[{"field":"custom.signup","op":"lt","value":"2026-13-45"}]}`, true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -82,3 +90,41 @@ func TestBuildWhereEmpty(t *testing.T) { t.Fatalf("expected empty where for no rules, got %q / %v", where, args) } } + +// A date field stored as YYYY-MM-DD has to be compared as a date. Casting it to numeric is +// what made a saved segment fail at query time, taking the preview and the broadcast with it. +func TestBuildWhereCastsComparisonsByValue(t *testing.T) { + cases := []struct { + name string + value any + wantCast string + }{ + {"date", "2026-01-01", "::date"}, + {"padded date", " 2026-01-01 ", "::date"}, + {"number", float64(42), "::numeric"}, + {"numeric string", "42", "::numeric"}, + {"decimal string", "4.5", "::numeric"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + where, args := BuildWhere(Predicate{ + Match: "all", + Rules: []Rule{{Field: "custom.f", Op: "gt", Value: tc.value}}, + }, 1) + + if !strings.Contains(where, tc.wantCast) { + t.Fatalf("comparison %v compiled without %s: %q", tc.value, tc.wantCast, where) + } + other := "::numeric" + if tc.wantCast == "::numeric" { + other = "::date" + } + if strings.Contains(where, other) { + t.Fatalf("comparison %v compiled with the wrong cast %s: %q", tc.value, other, where) + } + if len(args) != 1 { + t.Fatalf("expected one argument, got %d", len(args)) + } + }) + } +} diff --git a/docs/changelog.md b/docs/changelog.md index f7230fa..4b9753a 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -22,6 +22,8 @@ Pre-1.0 minor releases may contain breaking changes — check the version's note ### Fixed +- **Date rules no longer break a segment.** `greater than` and `less than` compiled a custom field to a numeric cast, but date fields are stored as `YYYY-MM-DD`, so a rule that saved without complaint failed when the segment actually ran: the preview errored and any broadcast targeting it never sent. The comparison now casts to match the value — dates compare as dates, numbers as numbers — and a value that cannot be ordered is refused while the segment is being saved rather than when it is used. + - **Demoting the last workspace owner no longer leaves the workspace ownerless.** The guard that refuses to remove the last owner ran only when the target role was `member` — a value the database rejects — so demoting an owner to `admin`, `developer` or `viewer` went through and left the workspace with no owner, unrecoverable from the UI. The guard now runs for every target role except `owner`. - **A restart no longer re-sends a broadcast another instance is delivering.** Broadcast jobs left in `sending` were reset to `retry` on every startup regardless of who owned them, so starting or rolling one instance re-queued jobs another instance was still sending, and those recipients received the same email twice. A claimed job now carries the worker's id and a lease, and startup recovery only reclaims jobs whose lease has expired — work in flight is left alone. From 733c7f9bf63d4876516f2284242ba724f7f542d3 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Fri, 11 Sep 2026 21:03:12 -0500 Subject: [PATCH 08/27] fix(email): record an unknown outcome as pending, not sent The log row is written before the SMTP call so the tracking pixel and the click links have an id to point at, and it was written as `sent` at that moment. Every attempt therefore claimed a delivery before the relay had answered: an attempt that crashed or hung stayed counted as delivered forever, and the stats endpoint reported successes that never left the instance. Rows now start as `pending` and settle to `sent` once the relay accepts the message. The failed branch already overwrote the row, so the only rows that were never corrected are the ones nobody could correct. The status filter gained a Pending chip, and the docs and changelog describe the state. Existing rows are left alone: an old `sent` row is indistinguishable from a real one, and rewriting history to guess would be worse than the original bug. (cherry picked from commit 3d6b7af5002eb358f3860d49fa84e7fc4d7eff18) --- CHANGELOG.md | 2 + backend/internal/service/email.go | 7 +- .../internal/service/email_log_status_test.go | 84 +++++++++++++++++++ docs/changelog.md | 2 + docs/guide/logs.md | 2 +- docs/guide/sending.md | 1 + frontend/src/views/project/LogsSection.vue | 1 + 7 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 backend/internal/service/email_log_status_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 98cdc5f..4c9faab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Releases are also published on [GitHub](https://github.com/arkhe-systems/senddoc ### Fixed +- **A log row no longer claims the send succeeded before it did.** The row is written before the SMTP call so the tracking pixel and the click links have an id to point at, and it was recorded as `sent` at that moment. A crash or a hung attempt therefore left a row that claimed a delivery which never happened, and the stats endpoint counted it. Rows now start as `pending` and settle to `sent` once the relay accepts the message. + - **Date rules no longer break a segment.** `greater than` and `less than` compiled a custom field to a numeric cast, but date fields are stored as `YYYY-MM-DD`, so a rule that saved without complaint failed when the segment actually ran: the preview errored and any broadcast targeting it never sent. The comparison now casts to match the value — dates compare as dates, numbers as numbers — and a value that cannot be ordered is refused while the segment is being saved rather than when it is used. - **Demoting the last workspace owner no longer leaves the workspace ownerless.** The guard that refuses to remove the last owner ran only when the target role was `member` — a value the database rejects — so demoting an owner to `admin`, `developer` or `viewer` went through and left the workspace with no owner, unrecoverable from the UI. The guard now runs for every target role except `owner`. diff --git a/backend/internal/service/email.go b/backend/internal/service/email.go index c83c0e7..dfcd964 100644 --- a/backend/internal/service/email.go +++ b/backend/internal/service/email.go @@ -761,13 +761,17 @@ func (s *EmailService) GetStats(ctx context.Context, projectID string) (map[stri } func (s *EmailService) logPending(ctx context.Context, projectID uuid.UUID, subscriberID, templateID uuid.NullUUID, toEmail, subject string, broadcastID, newsletterID uuid.NullUUID) uuid.UUID { + // The row has to exist before the send so the tracking pixel and the click links have a + // log id to point at, but until the relay answers the outcome is unknown. Recording it as + // sent up front counted attempts that never left the instance, and a crash left them + // counted as delivered forever. logEntry, _ := s.queries.CreateEmailLog(ctx, db.CreateEmailLogParams{ ProjectID: projectID, SubscriberID: subscriberID, TemplateID: templateID, ToEmail: toEmail, Subject: subject, - Status: "sent", + Status: "pending", BroadcastID: broadcastID, NewsletterID: newsletterID, }) @@ -830,6 +834,7 @@ func (s *EmailService) trackAndSend(ctx context.Context, project db.Project, pro s.dispatchEmail(ctx, "email.failed", projectID, logID, to, subject, sendErr.Error()) } else { metrics.EmailSent() + s.markLogStatus(ctx, projectID, logID, "sent", nil) s.dispatchEmail(ctx, "email.sent", projectID, logID, to, subject, "") } return sendErr diff --git a/backend/internal/service/email_log_status_test.go b/backend/internal/service/email_log_status_test.go new file mode 100644 index 0000000..76ce9d0 --- /dev/null +++ b/backend/internal/service/email_log_status_test.go @@ -0,0 +1,84 @@ +package service + +import ( + "context" + "testing" + + "github.com/google/uuid" + + "github.com/arkhe-systems/senddock/internal/db" +) + +// A log row is written before the send so the tracking pixel and the click links have an id +// to point at. Recording it as sent at that moment counted attempts whose outcome was still +// unknown, and a crash in between left them counted as delivered forever. +func TestLogPendingDoesNotClaimTheSendSucceeded(t *testing.T) { + conn, queries := leaseTestDB(t) + ctx := context.Background() + + _, projectID, subscriberID := leaseFixtures(t, conn) + svc := NewEmailService(queries, nil, "", nil, nil, nil) + + logID := svc.logPending(ctx, projectID, + uuid.NullUUID{UUID: subscriberID, Valid: true}, uuid.NullUUID{}, + "recipient@example.test", "Subject", + uuid.NullUUID{}, uuid.NullUUID{}) + if logID == uuid.Nil { + t.Fatal("logPending returned no id") + } + + var status string + if err := conn.QueryRowContext(ctx, `SELECT status FROM email_logs WHERE id = $1`, logID).Scan(&status); err != nil { + t.Fatalf("read log row: %v", err) + } + if status != "pending" { + t.Fatalf("new log row status = %q, want %q", status, "pending") + } + + // The stats endpoint counts 'sent', so an unfinished attempt must not appear there. + sent, err := queries.CountEmailLogsByStatus(ctx, db.CountEmailLogsByStatusParams{ + ProjectID: projectID, + Status: "sent", + }) + if err != nil { + t.Fatalf("count sent logs: %v", err) + } + if sent != 0 { + t.Fatalf("stats count %d sends for an unfinished attempt, want 0", sent) + } +} + +// Once the relay accepts the message the row has to say so. +func TestMarkLogStatusSettlesTheOutcome(t *testing.T) { + conn, queries := leaseTestDB(t) + ctx := context.Background() + + _, projectID, subscriberID := leaseFixtures(t, conn) + svc := NewEmailService(queries, nil, "", nil, nil, nil) + + logID := svc.logPending(ctx, projectID, + uuid.NullUUID{UUID: subscriberID, Valid: true}, uuid.NullUUID{}, + "recipient@example.test", "Subject", + uuid.NullUUID{}, uuid.NullUUID{}) + + svc.markLogStatus(ctx, projectID, logID, "sent", nil) + + var status string + if err := conn.QueryRowContext(ctx, `SELECT status FROM email_logs WHERE id = $1`, logID).Scan(&status); err != nil { + t.Fatalf("read log row: %v", err) + } + if status != "sent" { + t.Fatalf("settled status = %q, want %q", status, "sent") + } + + sent, err := queries.CountEmailLogsByStatus(ctx, db.CountEmailLogsByStatusParams{ + ProjectID: projectID, + Status: "sent", + }) + if err != nil { + t.Fatalf("count sent logs: %v", err) + } + if sent != 1 { + t.Fatalf("stats count %d sends after a settled one, want 1", sent) + } +} diff --git a/docs/changelog.md b/docs/changelog.md index 4b9753a..5e0a7b7 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -22,6 +22,8 @@ Pre-1.0 minor releases may contain breaking changes — check the version's note ### Fixed +- **A log row no longer claims the send succeeded before it did.** The row is written before the SMTP call so the tracking pixel and the click links have an id to point at, and it was recorded as `sent` at that moment. A crash or a hung attempt therefore left a row that claimed a delivery which never happened, and the stats endpoint counted it. Rows now start as `pending` and settle to `sent` once the relay accepts the message. + - **Date rules no longer break a segment.** `greater than` and `less than` compiled a custom field to a numeric cast, but date fields are stored as `YYYY-MM-DD`, so a rule that saved without complaint failed when the segment actually ran: the preview errored and any broadcast targeting it never sent. The comparison now casts to match the value — dates compare as dates, numbers as numbers — and a value that cannot be ordered is refused while the segment is being saved rather than when it is used. - **Demoting the last workspace owner no longer leaves the workspace ownerless.** The guard that refuses to remove the last owner ran only when the target role was `member` — a value the database rejects — so demoting an owner to `admin`, `developer` or `viewer` went through and left the workspace with no owner, unrecoverable from the UI. The guard now runs for every target role except `owner`. diff --git a/docs/guide/logs.md b/docs/guide/logs.md index 54bb2e2..2c5fa08 100644 --- a/docs/guide/logs.md +++ b/docs/guide/logs.md @@ -12,7 +12,7 @@ Open it from **Logs** in the project sidebar. |---|---| | **To** | The recipient address. | | **Subject** | The rendered subject line. | -| **Status** | `sent`, `failed`, `bounced` or `suppressed` (see [Sending → statuses](./sending) and [Bounces](./bounces)). | +| **Status** | `pending`, `sent`, `failed`, `bounced` or `suppressed` (see [Sending → statuses](./sending) and [Bounces](./bounces)). | | **Engagement** | Whether the message was opened and/or clicked. | | **Date** | When it was processed. | diff --git a/docs/guide/sending.md b/docs/guide/sending.md index 9fcd949..9cb6cb0 100644 --- a/docs/guide/sending.md +++ b/docs/guide/sending.md @@ -291,6 +291,7 @@ Pagination defaults to **50 rows per page** and accepts up to **100** via the `l | Status | When it lands | |---|---| +| `pending` | The attempt was recorded and its outcome is not known yet — the process stopped before the relay answered. Not counted as sent. | | `sent` | SMTP relay accepted the message. | | `failed` | Soft failure (4xx) or unexpected error during the send. | | `bounced` | Hard failure (5xx) detected by [bounce ingestion](./bounces). The recipient is also added to the [suppression list](./suppressions). | diff --git a/frontend/src/views/project/LogsSection.vue b/frontend/src/views/project/LogsSection.vue index de05907..d3cf738 100644 --- a/frontend/src/views/project/LogsSection.vue +++ b/frontend/src/views/project/LogsSection.vue @@ -28,6 +28,7 @@ interface Template { id: string; name: string } const STATUS_CHIPS: { value: string; label: string }[] = [ { value: '', label: 'All' }, + { value: 'pending', label: 'Pending' }, { value: 'sent', label: 'Sent' }, { value: 'failed', label: 'Failed' }, { value: 'bounced', label: 'Bounced' }, From da1b883f5d01079808649a80a290802628b92517 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Fri, 11 Sep 2026 23:47:55 -0500 Subject: [PATCH 09/27] fix(webhooks): stop returning the signing secret on every read Any member of a workspace could read the secret of every webhook in the project. Reads require no capability, so a read-only viewer could take the signing key and sign deliveries of its own, and so could anyone holding a session. Listing a project's webhooks returned the key of each one, and so did fetching a single webhook and pausing it. That is not what the product promised: the dashboard says the secret is shown once, the API docs say the field comes back empty, and both describe the recovery path as creating a new webhook. Only the code disagreed, so the code changed. The secret now leaves the instance on the response that creates the webhook and nowhere else. FromWebhook redacts it for every read path, and FromCreatedWebhook is the single place allowed to put it back, so a new endpoint cannot reintroduce the leak by reusing the common mapper. Covered twice: the mapper's wire output, and the real handler over HTTP against a migrated database, including the viewer role. With the redaction reverted, the integration test prints the leaked response body. (cherry picked from commit b6e44bdeac736756daed69415863dd7742eb00f4) --- CHANGELOG.md | 2 + backend/internal/handler/webhook.go | 2 +- .../internal/handler/webhook_secret_test.go | 232 ++++++++++++++++++ backend/internal/response/response.go | 14 +- backend/internal/response/webhook_test.go | 92 +++++++ docs/api/webhooks.md | 4 +- docs/changelog.md | 2 + 7 files changed, 344 insertions(+), 4 deletions(-) create mode 100644 backend/internal/handler/webhook_secret_test.go create mode 100644 backend/internal/response/webhook_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c9faab..37740e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ Releases are also published on [GitHub](https://github.com/arkhe-systems/senddoc ### Security +- **A webhook's signing secret can no longer be read back.** Listing, fetching or pausing a webhook returned its secret, and reading webhooks needs no capability at all, so every member of the workspace — a read-only viewer included — could take the signing key and sign deliveries of its own. The dashboard meanwhile told the user the secret was shown once, and the API docs said the field came back empty. It is now returned by the creation response only. A secret that is lost is replaced by creating a new webhook, which is the recovery path both documents already described. + - **Headers could be injected through the subject.** Subjects are built from subscriber-supplied text — their name, email and custom fields are substituted into them — and that text was written into the message headers as-is. A value containing a line break could therefore add headers of the attacker's choosing, a `Bcc` for example, to the message SendDock sent. Carriage returns and line feeds are now stripped from every value that reaches a header: the subject, the sender name, the recipient and the unsubscribe URL. - **Certificates are verified on outgoing SMTP.** Every connection accepted any certificate, so an attacker able to sit between the instance and the relay could read SMTP credentials and the mail itself while the connection still looked encrypted. Verification is now on for both implicit TLS and STARTTLS. A relay with a self-signed or expired certificate can be kept working per project under **Project → SMTP → Allow insecure TLS**, and the failure message names that setting so the fix is one click away. diff --git a/backend/internal/handler/webhook.go b/backend/internal/handler/webhook.go index edc3721..77f3f08 100644 --- a/backend/internal/handler/webhook.go +++ b/backend/internal/handler/webhook.go @@ -128,7 +128,7 @@ func (h *WebhookHandler) Create(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(response.FromWebhook(hook)) + json.NewEncoder(w).Encode(response.FromCreatedWebhook(hook)) } func (h *WebhookHandler) List(w http.ResponseWriter, r *http.Request) { diff --git a/backend/internal/handler/webhook_secret_test.go b/backend/internal/handler/webhook_secret_test.go new file mode 100644 index 0000000..f6b15da --- /dev/null +++ b/backend/internal/handler/webhook_secret_test.go @@ -0,0 +1,232 @@ +package handler + +import ( + "context" + "database/sql" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/google/uuid" + _ "github.com/lib/pq" + + "github.com/arkhe-systems/senddock/internal/db" + "github.com/arkhe-systems/senddock/internal/service" + "github.com/arkhe-systems/senddock/pkg/auth" +) + +// The signing secret is handed out once, by the response that creates the webhook, +// and no read path may return it again. These tests drive the real handler over HTTP +// against a migrated database, because the leak this guards against lived in the +// response mapping shared by every read, not in a function that can be called alone. +// +// They skip unless DATABASE_URL points at a migrated instance, so the default test run +// stays free of external dependencies. + +type webhookSecretFixture struct { + mux http.Handler + conn *sql.DB + projectID string + userID string + base string +} + +func newWebhookSecretFixture(t *testing.T, role string) webhookSecretFixture { + t.Helper() + + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + t.Skip("DATABASE_URL is not set; skipping webhook secret integration test") + } + + conn, err := sql.Open("postgres", dsn) + if err != nil { + t.Fatalf("open database: %v", err) + } + if err := conn.Ping(); err != nil { + conn.Close() + t.Fatalf("ping database: %v", err) + } + t.Cleanup(func() { conn.Close() }) + + ctx := context.Background() + + var userID, workspaceID, projectID uuid.UUID + if err := conn.QueryRowContext(ctx, + `INSERT INTO users (email, name) VALUES ($1, $2) RETURNING id`, + "webhook-secret-"+uuid.NewString()+"@example.test", "Webhook Secret Test", + ).Scan(&userID); err != nil { + t.Fatalf("insert user: %v", err) + } + if err := conn.QueryRowContext(ctx, + `INSERT INTO workspaces (name, created_by) VALUES ($1, $2) RETURNING id`, + "webhook-secret-test", userID, + ).Scan(&workspaceID); err != nil { + t.Fatalf("insert workspace: %v", err) + } + t.Cleanup(func() { + if _, err := conn.ExecContext(ctx, `DELETE FROM workspaces WHERE id = $1`, workspaceID); err != nil { + t.Logf("cleanup workspace: %v", err) + } + }) + if _, err := conn.ExecContext(ctx, + `INSERT INTO workspace_members (workspace_id, user_id, role) VALUES ($1, $2, $3)`, + workspaceID, userID, role, + ); err != nil { + t.Fatalf("insert membership: %v", err) + } + if err := conn.QueryRowContext(ctx, + `INSERT INTO projects (workspace_id, user_id, name, from_email) VALUES ($1, $2, $3, $4) RETURNING id`, + workspaceID, userID, "webhook-secret-test", "sender@example.test", + ).Scan(&projectID); err != nil { + t.Fatalf("insert project: %v", err) + } + + queries := db.New(conn) + h := NewWebhookHandler(service.NewWebhookService(queries), service.NewProjectService(queries, "test-encryption-secret")) + + mux := http.NewServeMux() + mux.HandleFunc("POST /api/v1/projects/{id}/webhooks", h.Create) + mux.HandleFunc("GET /api/v1/projects/{id}/webhooks", h.List) + mux.HandleFunc("GET /api/v1/projects/{id}/webhooks/{webhookId}", h.Get) + mux.HandleFunc("PATCH /api/v1/projects/{id}/webhooks/{webhookId}", h.Patch) + + return webhookSecretFixture{ + mux: mux, + conn: conn, + projectID: projectID.String(), + userID: userID.String(), + base: "/api/v1/projects/" + projectID.String() + "/webhooks", + } +} + +// insertWebhook writes a webhook with a known secret, for tests that only exercise a +// read path and so do not need a role allowed to create one. +func (f webhookSecretFixture) insertWebhook(t *testing.T, secret string) string { + t.Helper() + + var id uuid.UUID + if err := f.conn.QueryRowContext(context.Background(), + `INSERT INTO webhooks (project_id, url, secret, events) VALUES ($1, $2, $3, $4) RETURNING id`, + f.projectID, "https://example.com/webhooks/senddock", secret, []string{"email.sent"}, + ).Scan(&id); err != nil { + t.Fatalf("insert webhook: %v", err) + } + return id.String() +} + +func (f webhookSecretFixture) request(t *testing.T, method, path, body string) *httptest.ResponseRecorder { + t.Helper() + + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req = req.WithContext(context.WithValue(req.Context(), auth.UserIDKey, f.userID)) + + rec := httptest.NewRecorder() + f.mux.ServeHTTP(rec, req) + return rec +} + +type webhookBody struct { + ID string `json:"id"` + Secret string `json:"secret"` + Active bool `json:"active"` +} + +func decodeWebhook(t *testing.T, body string) webhookBody { + t.Helper() + + var hook webhookBody + if err := json.Unmarshal([]byte(body), &hook); err != nil { + t.Fatalf("decode webhook: %v (body %s)", err, body) + } + return hook +} + +func TestWebhookSecretIsReturnedOnceAndNeverReadBack(t *testing.T) { + f := newWebhookSecretFixture(t, "owner") + + rec := f.request(t, http.MethodPost, f.base, + `{"url":"https://example.com/webhooks/senddock","events":["email.sent"]}`) + if rec.Code != http.StatusCreated { + t.Fatalf("create status = %d, want 201 (body %s)", rec.Code, rec.Body.String()) + } + + created := decodeWebhook(t, rec.Body.String()) + if len(created.Secret) != 64 { + t.Fatalf("created secret = %q, want a 64-character secret", created.Secret) + } + if created.ID == "" { + t.Fatal("created webhook has no id") + } + + // Every read below must be silent about a secret that is known to exist, so the + // assertions cannot pass merely because nothing was generated. + reads := []struct { + name string + method string + path string + body string + }{ + {"list", http.MethodGet, f.base, ""}, + {"get", http.MethodGet, f.base + "/" + created.ID, ""}, + {"patch", http.MethodPatch, f.base + "/" + created.ID, `{"active":false}`}, + } + + for _, read := range reads { + t.Run(read.name, func(t *testing.T) { + rec := f.request(t, read.method, read.path, read.body) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + + body := rec.Body.String() + if strings.Contains(body, created.Secret) { + t.Errorf("response leaked the signing secret: %s", body) + } + + if read.name == "list" { + var payload struct { + Webhooks []webhookBody `json:"webhooks"` + } + if err := json.Unmarshal([]byte(body), &payload); err != nil { + t.Fatalf("decode list: %v", err) + } + if len(payload.Webhooks) != 1 { + t.Fatalf("returned %d webhooks, want 1", len(payload.Webhooks)) + } + if payload.Webhooks[0].Secret != "" { + t.Errorf("listed secret = %q, want empty", payload.Webhooks[0].Secret) + } + return + } + + if hook := decodeWebhook(t, body); hook.Secret != "" { + t.Errorf("secret = %q, want empty", hook.Secret) + } + }) + } +} + +// A read-only member can list a project's webhooks, and that role has no business +// holding the signing key: with the secret readable it could sign deliveries of its +// own. Reading a webhook needs no capability, so this is the widest audience the +// field was exposed to. +func TestWebhookSecretIsHiddenFromAViewer(t *testing.T) { + f := newWebhookSecretFixture(t, "viewer") + + const secret = "9f0a3b1c2d3e4f5061728394a5b6c7d8e9f0a1b2c3d4e5f60718293a4b5c6d7e" + id := f.insertWebhook(t, secret) + + for _, path := range []string{f.base, f.base + "/" + id} { + rec := f.request(t, http.MethodGet, path, "") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + if strings.Contains(rec.Body.String(), secret) { + t.Errorf("viewer-visible response leaked the signing secret: %s", rec.Body.String()) + } + } +} diff --git a/backend/internal/response/response.go b/backend/internal/response/response.go index 06f7282..6580987 100644 --- a/backend/internal/response/response.go +++ b/backend/internal/response/response.go @@ -180,6 +180,9 @@ type Webhook struct { CreatedAt string `json:"created_at"` } +// FromWebhook renders a stored webhook without its signing secret. The secret is +// handed out only by FromCreatedWebhook, on the response that creates it, so every +// later read of a webhook leaves the field empty. func FromWebhook(w db.Webhook) Webhook { events := w.Events if events == nil { @@ -188,13 +191,22 @@ func FromWebhook(w db.Webhook) Webhook { return Webhook{ ID: w.ID.String(), URL: w.Url, - Secret: w.Secret, + Secret: "", Events: events, Active: w.Active, CreatedAt: w.CreatedAt.UTC().Format(time.RFC3339), } } +// FromCreatedWebhook renders a webhook together with its signing secret, for the +// one response that creates it. The secret cannot be read back afterwards, so a +// client that loses it creates a new webhook. +func FromCreatedWebhook(w db.Webhook) Webhook { + hook := FromWebhook(w) + hook.Secret = w.Secret + return hook +} + func FromWebhooks(hooks []db.Webhook) []Webhook { result := make([]Webhook, len(hooks)) for i, w := range hooks { diff --git a/backend/internal/response/webhook_test.go b/backend/internal/response/webhook_test.go new file mode 100644 index 0000000..68fde42 --- /dev/null +++ b/backend/internal/response/webhook_test.go @@ -0,0 +1,92 @@ +package response + +import ( + "encoding/json" + "strings" + "testing" + "time" + + "github.com/arkhe-systems/senddock/internal/db" + "github.com/google/uuid" +) + +const testWebhookSecret = "9f0a3b1c2d3e4f5061728394a5b6c7d8e9f0a1b2c3d4e5f60718293a4b5c6d7e" + +func testWebhook() db.Webhook { + return db.Webhook{ + ID: uuid.MustParse("11111111-1111-1111-1111-111111111111"), + ProjectID: uuid.MustParse("22222222-2222-2222-2222-222222222222"), + Url: "https://example.com/webhooks/senddock", + Secret: testWebhookSecret, + Events: []string{"email.sent"}, + Active: true, + CreatedAt: time.Date(2026, 4, 29, 5, 12, 0, 0, time.UTC), + } +} + +func TestFromWebhookHidesSecret(t *testing.T) { + hook := FromWebhook(testWebhook()) + + if hook.Secret != "" { + t.Errorf("secret = %q, want empty", hook.Secret) + } + if hook.ID != "11111111-1111-1111-1111-111111111111" { + t.Errorf("id = %q, want the webhook id", hook.ID) + } + if hook.URL != "https://example.com/webhooks/senddock" { + t.Errorf("url = %q, want the webhook url", hook.URL) + } + if !hook.Active { + t.Error("active = false, want true") + } + if len(hook.Events) != 1 || hook.Events[0] != "email.sent" { + t.Errorf("events = %v, want [email.sent]", hook.Events) + } +} + +func TestFromWebhookListHidesEverySecret(t *testing.T) { + hooks := []db.Webhook{testWebhook(), testWebhook()} + + for i, hook := range FromWebhooks(hooks) { + if hook.Secret != "" { + t.Errorf("webhooks[%d].secret = %q, want empty", i, hook.Secret) + } + } +} + +// The creation response is the one place the secret is allowed through, otherwise +// no client could ever verify a signature. +func TestFromCreatedWebhookKeepsSecret(t *testing.T) { + if got := FromCreatedWebhook(testWebhook()).Secret; got != testWebhookSecret { + t.Errorf("secret = %q, want the generated secret", got) + } +} + +// Guards the documented wire format: reads carry the field and it is empty, so a +// client holding a stored webhook has nothing it could sign with. +func TestFromWebhookJSONNeverCarriesSecret(t *testing.T) { + body, err := json.Marshal(struct { + Webhooks []Webhook `json:"webhooks"` + }{Webhooks: FromWebhooks([]db.Webhook{testWebhook()})}) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + if strings.Contains(string(body), testWebhookSecret) { + t.Errorf("list response leaked the secret: %s", body) + } + if !strings.Contains(string(body), `"secret":""`) { + t.Errorf("list response should carry an empty secret field: %s", body) + } +} + +func TestFromCreatedWebhookJSONCarriesSecret(t *testing.T) { + body, err := json.Marshal(FromCreatedWebhook(testWebhook())) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + if !strings.Contains(string(body), testWebhookSecret) { + t.Errorf("creation response dropped the secret: %s", body) + } +} diff --git a/docs/api/webhooks.md b/docs/api/webhooks.md index 932afaf..ac87c8d 100644 --- a/docs/api/webhooks.md +++ b/docs/api/webhooks.md @@ -38,7 +38,7 @@ Allowed events: `email.sent`, `email.failed`, `email.bounced`, `email.opened`, ` ``` ::: warning The secret is only returned here -The signing secret is shown **once**, on creation. SendDock never returns it again — subsequent `GET` calls return the secret field empty (or omit it). If you lose it, delete the webhook and create a new one. +The signing secret is shown **once**, on creation. SendDock never returns it again — every later `GET` or `PATCH` returns the secret field empty, and storing the response does not leave you with a readable secret. If you lose it, delete the webhook and create a new one. ::: ## List webhooks @@ -82,7 +82,7 @@ Only the `active` flag can be patched today — pause or resume delivery without {"active": false} ``` -Returns the updated webhook. +Returns the updated webhook. The `secret` field is empty here too — see the note on creation. A paused webhook (`active=false`) does **not** buffer events. Any deliveries that try to fire while it's paused are marked `failed` on the next dispatcher tick. diff --git a/docs/changelog.md b/docs/changelog.md index 5e0a7b7..75d7bf4 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -32,6 +32,8 @@ Pre-1.0 minor releases may contain breaking changes — check the version's note ### Security +- **A webhook's signing secret can no longer be read back.** Listing, fetching or pausing a webhook returned its secret, and reading webhooks needs no capability at all, so every member of the workspace — a read-only viewer included — could take the signing key and sign deliveries of its own. The dashboard meanwhile told the user the secret was shown once, and the API docs said the field came back empty. It is now returned by the creation response only. A secret that is lost is replaced by creating a new webhook, which is the recovery path both documents already described. + - **Headers could be injected through the subject.** Subjects are built from subscriber-supplied text — their name, email and custom fields are substituted into them — and that text was written into the message headers as-is. A value containing a line break could therefore add headers of the attacker's choosing, a `Bcc` for example, to the message SendDock sent. Carriage returns and line feeds are now stripped from every value that reaches a header: the subject, the sender name, the recipient and the unsubscribe URL. - **Certificates are verified on outgoing SMTP.** Every connection accepted any certificate, so an attacker able to sit between the instance and the relay could read SMTP credentials and the mail itself while the connection still looked encrypted. Verification is now on for both implicit TLS and STARTTLS. A relay with a self-signed or expired certificate can be kept working per project under **Project → SMTP → Allow insecure TLS**, and the failure message names that setting so the fix is one click away. From 292ac351f3b1f1b5985b69f93ca8be3ee938832b Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Sat, 12 Sep 2026 15:14:27 -0500 Subject: [PATCH 10/27] fix(compose): refuse to start without a database password docker-compose.prod.yml, the source-build path, fell back to `senddock` wherever POSTGRES_PASSWORD was used: in the app's connection string, in the database service and in the migration step. A deployment that never set the variable therefore came up with a password printed in this repository, while the docs already told operators the compose fails fast without it. It now fails fast, naming the variable, exactly as docker-compose.image.yml already did. The same trap sat one step earlier in .env.production.example, where the two required values were non-empty placeholders: `cp` plus a forgotten edit booted a stack on a published secret and the `:?` guard could not see it. Both required values now ship empty, so the compose stops and says which one is missing. Leaving the development compose alone: it is the `make dev` file, it is not a production path, and its Postgres is on a host port for local work. CI gains the guard this bug never had. One step asserts each production compose refuses to interpolate without the variable and does interpolate once it is set; another asserts the example env ships no values for the two required keys. Reintroducing either bug fails the build. (cherry picked from commit 9084f5bc809e5137dad23e4888250ffe66f82225) --- .env.production.example | 10 +++++-- .github/workflows/ci.yml | 42 ++++++++++++++++++++++++++-- CHANGELOG.md | 2 ++ docker-compose.prod.yml | 6 ++-- docs/changelog.md | 2 ++ docs/guide/environment.md | 2 +- docs/self-hosting/configuration.md | 2 +- docs/self-hosting/installation.md | 6 +++- docs/self-hosting/troubleshooting.md | 2 +- 9 files changed, 62 insertions(+), 12 deletions(-) diff --git a/.env.production.example b/.env.production.example index d56fe8d..644d4f5 100644 --- a/.env.production.example +++ b/.env.production.example @@ -3,16 +3,20 @@ # # Usage: # cp .env.production.example .env -# # replace every "change-me-..." below +# # generate the two required values below # docker compose -f docker-compose.prod.yml up -d --build +# +# The two required values ship empty on purpose and the compose refuses to start while +# either is missing, so a skipped step is a clear error rather than a stack running on a +# password published in this repository. # === REQUIRED === # JWT signing secret (min 32 chars) — generate with: openssl rand -hex 32 -JWT_SECRET=change-me-openssl-rand-hex-32 +JWT_SECRET= # Postgres password — generate with: openssl rand -base64 32 -POSTGRES_PASSWORD=change-me-openssl-rand-base64-32 +POSTGRES_PASSWORD= # === OPTIONAL === diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 932df41..e802832 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,10 +2,12 @@ name: CI # Nothing ran the test suite before this workflow existed, so a regression could # reach main with every check green simply because there were no checks. This runs -# the backend build, vet and tests plus the frontend type-check, build and tests. +# the backend build, vet and tests, the frontend type-check, build and tests, and the +# compose guards. # # CGO is disabled to match the released image: the Dockerfile builds with -# CGO_ENABLED=0, and the tests do not need a database (they are unit tests). +# CGO_ENABLED=0. The suite needs no database — the integration tests that use one skip +# themselves unless DATABASE_URL is set. on: push: branches: [main] @@ -60,3 +62,39 @@ jobs: - name: Test working-directory: frontend run: npm test + + compose: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + # A compose file that fills in a default for POSTGRES_PASSWORD starts a database + # with a password anyone can read, which is how docker-compose.prod.yml used to + # resolve its DSN to senddock:senddock. Each production compose has to refuse to + # interpolate without it and to interoperate once it is provided. + - name: The production composes require POSTGRES_PASSWORD + run: | + set -euo pipefail + export JWT_SECRET="$(head -c 48 /dev/urandom | base64)" + unset POSTGRES_PASSWORD + + for file in docker-compose.prod.yml docker-compose.image.yml; do + if docker compose -f "$file" config >/dev/null 2>&1; then + echo "::error file=$file::$file resolved without POSTGRES_PASSWORD, so it carries a default someone can guess" + exit 1 + fi + POSTGRES_PASSWORD=probe docker compose -f "$file" config >/dev/null + done + + # The same trap one step earlier: an example env whose required values are + # placeholders boots a stack on a password published in the repository. + - name: The example env ships no secret + run: | + set -euo pipefail + for key in JWT_SECRET POSTGRES_PASSWORD; do + line="$(grep -E "^${key}=" .env.production.example || true)" + if [ "$line" != "${key}=" ]; then + echo "::error file=.env.production.example::$key must ship empty, found: ${line:-nothing}" + exit 1 + fi + done diff --git a/CHANGELOG.md b/CHANGELOG.md index 37740e8..6ab4284 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ Releases are also published on [GitHub](https://github.com/arkhe-systems/senddoc ### Security +- **The source-build compose no longer starts on a published Postgres password.** `docker-compose.prod.yml` fell back to `senddock` when `POSTGRES_PASSWORD` was unset — in the app's connection string, in the database service and in the migration step — so a deployment that skipped the variable came up matching a password printed in this repository. It now refuses to start and names the variable, like the image compose already did. `.env.production.example` ships the two required values empty for the same reason: a placeholder that is copied and forgotten is a secret everyone can read, and the stack now stops instead of accepting it. + - **A webhook's signing secret can no longer be read back.** Listing, fetching or pausing a webhook returned its secret, and reading webhooks needs no capability at all, so every member of the workspace — a read-only viewer included — could take the signing key and sign deliveries of its own. The dashboard meanwhile told the user the secret was shown once, and the API docs said the field came back empty. It is now returned by the creation response only. A secret that is lost is replaced by creating a new webhook, which is the recovery path both documents already described. - **Headers could be injected through the subject.** Subjects are built from subscriber-supplied text — their name, email and custom fields are substituted into them — and that text was written into the message headers as-is. A value containing a line break could therefore add headers of the attacker's choosing, a `Bcc` for example, to the message SendDock sent. Carriage returns and line feeds are now stripped from every value that reaches a header: the subject, the sender name, the recipient and the unsubscribe URL. diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index a6c9934..55158c1 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -4,7 +4,7 @@ services: ports: - "8080:8080" environment: - DATABASE_URL: postgres://senddock:${POSTGRES_PASSWORD:-senddock}@postgres:5432/senddock?sslmode=disable + DATABASE_URL: postgres://senddock:${POSTGRES_PASSWORD:?POSTGRES_PASSWORD is required}@postgres:5432/senddock?sslmode=disable JWT_SECRET: ${JWT_SECRET:?Set JWT_SECRET in .env} REDIS_URL: redis://redis:6379 FRONTEND_URL: ${FRONTEND_URL:-http://localhost:8080} @@ -20,7 +20,7 @@ services: image: postgres:17-alpine environment: POSTGRES_USER: senddock - POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-senddock} + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?POSTGRES_PASSWORD is required} POSTGRES_DB: senddock volumes: - pgdata:/var/lib/postgresql/data @@ -41,7 +41,7 @@ services: sh -c "apk add --no-cache curl && curl -fsSL https://github.com/pressly/goose/releases/download/v3.24.1/goose_linux_x86_64 -o /usr/local/bin/goose && chmod +x /usr/local/bin/goose && - goose -dir /app/migrations postgres 'postgres://senddock:${POSTGRES_PASSWORD:-senddock}@postgres:5432/senddock?sslmode=disable' up" + goose -dir /app/migrations postgres 'postgres://senddock:${POSTGRES_PASSWORD:?POSTGRES_PASSWORD is required}@postgres:5432/senddock?sslmode=disable' up" depends_on: postgres: condition: service_healthy diff --git a/docs/changelog.md b/docs/changelog.md index 75d7bf4..303b837 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -32,6 +32,8 @@ Pre-1.0 minor releases may contain breaking changes — check the version's note ### Security +- **The source-build compose no longer starts on a published Postgres password.** `docker-compose.prod.yml` fell back to `senddock` when `POSTGRES_PASSWORD` was unset — in the app's connection string, in the database service and in the migration step — so a deployment that skipped the variable came up matching a password printed in this repository. It now refuses to start and names the variable, like the image compose already did. `.env.production.example` ships the two required values empty for the same reason: a placeholder that is copied and forgotten is a secret everyone can read, and the stack now stops instead of accepting it. + - **A webhook's signing secret can no longer be read back.** Listing, fetching or pausing a webhook returned its secret, and reading webhooks needs no capability at all, so every member of the workspace — a read-only viewer included — could take the signing key and sign deliveries of its own. The dashboard meanwhile told the user the secret was shown once, and the API docs said the field came back empty. It is now returned by the creation response only. A secret that is lost is replaced by creating a new webhook, which is the recovery path both documents already described. - **Headers could be injected through the subject.** Subjects are built from subscriber-supplied text — their name, email and custom fields are substituted into them — and that text was written into the message headers as-is. A value containing a line break could therefore add headers of the attacker's choosing, a `Bcc` for example, to the message SendDock sent. Carriage returns and line feeds are now stripped from every value that reaches a header: the subject, the sender name, the recipient and the unsubscribe URL. diff --git a/docs/guide/environment.md b/docs/guide/environment.md index fc5bfb9..cfb021f 100644 --- a/docs/guide/environment.md +++ b/docs/guide/environment.md @@ -30,7 +30,7 @@ These are consumed by the bundled `docker-compose.image.yml` / `docker-compose.p | Variable | Description | Default | |----------|-------------|---------| -| `POSTGRES_PASSWORD` | Password for the bundled Postgres service. Required — the compose fails fast if it's unset. Generate with `openssl rand -base64 32`. | — | +| `POSTGRES_PASSWORD` | Password for the bundled Postgres service. Required — both production composes refuse to start while it is unset or empty, which is why `.env.production.example` ships it empty. Generate with `openssl rand -base64 32`. | — | | `SENDDOCK_PORT` | Host port the SendDock container is exposed on. Mapped to `8080` inside the container (which is what `PORT` controls). Set to e.g. `9090` to reach SendDock at `http://host:9090`. | `8080` | ## Advanced overrides diff --git a/docs/self-hosting/configuration.md b/docs/self-hosting/configuration.md index 5ad9ff5..e9460a0 100644 --- a/docs/self-hosting/configuration.md +++ b/docs/self-hosting/configuration.md @@ -60,7 +60,7 @@ The bundled production composes already ship Redis. Only unset `REDIS_URL` for e Before exposing to the internet: - [ ] Generate `JWT_SECRET` from `openssl rand -hex 32` (or `base64 48`) — used for JWT signing **and** the HMAC on click-tracking URLs. Min 32 chars. -- [ ] Generate `POSTGRES_PASSWORD` from `openssl rand -base64 32` instead of using a guessable value +- [ ] Set `POSTGRES_PASSWORD` to the output of `openssl rand -base64 32` — the production composes refuse to start without it, so there is no default to forget - [ ] Set your public HTTPS domain under **Instance** in the dashboard (drives unsubscribe + tracking links inside outgoing emails, and the CORS origin) - [ ] Put SendDock behind HTTPS — `Secure: true` is set on auth cookies when the resolved URL starts with `https://` - [ ] Keep Redis enabled (it ships in the production composes); without it every rate limit is a no-op diff --git a/docs/self-hosting/installation.md b/docs/self-hosting/installation.md index d024dc8..abeb463 100644 --- a/docs/self-hosting/installation.md +++ b/docs/self-hosting/installation.md @@ -202,10 +202,14 @@ For self-hosters who want to compile Core from source — useful for audits, cus git clone https://github.com/arkhe-systems/senddock.git cd senddock cp .env.production.example .env -# edit .env: set JWT_SECRET and POSTGRES_PASSWORD (e.g. openssl rand -hex 32) +# edit .env and set the two required values: +# JWT_SECRET -> openssl rand -hex 32 +# POSTGRES_PASSWORD -> openssl rand -base64 32 docker compose -f docker-compose.prod.yml up -d --build ``` +Both values ship empty in the example on purpose: the compose refuses to start while either is missing, naming the variable, so skipping this step is a clear error instead of a stack running on a password anyone can read in the repository. + Windows users run the same `docker compose` command from PowerShell — no separate script. This path runs `docker-compose.prod.yml`, which builds the image locally instead of pulling the prebuilt one. To update later, `git pull && docker compose -f docker-compose.prod.yml up -d --build`. To reset, run `docker compose -f docker-compose.prod.yml down -v && rm .env` (**this deletes all data**, so use it only on test instances) and repeat the fresh-install steps above. diff --git a/docs/self-hosting/troubleshooting.md b/docs/self-hosting/troubleshooting.md index dd49338..43d46b9 100644 --- a/docs/self-hosting/troubleshooting.md +++ b/docs/self-hosting/troubleshooting.md @@ -177,7 +177,7 @@ docker compose -f docker-compose.prod.yml down -v rm .env ``` -This wipes the Postgres volume. Recreate `.env` from `.env.production.example` and bring the stack back up with `docker compose -f docker-compose.prod.yml up -d --build`, which gives you matching credentials. +This wipes the Postgres volume. Recreate `.env` from `.env.production.example`, set `JWT_SECRET` and `POSTGRES_PASSWORD` again — any values will do, since the volume that stored the old ones is gone — and bring the stack back up with `docker compose -f docker-compose.prod.yml up -d --build`. ### App container never becomes healthy after a source build From d943c7e1cbb617f026378f59351400f48630a199 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Sun, 13 Sep 2026 14:29:41 -0500 Subject: [PATCH 11/27] docs: write the roadmap status note in english ROADMAP.md is a public file and the status note added when the roadmap was reconciled was written in Spanish, so the repository mixed two languages in one document. A reader who does not know Spanish cannot tell which half of the file is current, and neither can review. The content is unchanged; it now reads in the language the rest of the file, and the rest of the public surface, is written in. (cherry picked from commit bf4ae8621eddf4fca8c92ef6a22b29174a06ec16) --- ROADMAP.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 35481df..29d5cf8 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,12 +1,12 @@ # SendDock Roadmap -> **Estado, reconciliado con el código.** Los ítems de las fases de abajo están verificados contra el -> código, no contra lo que se creía. Lo que sigue sin marcar sigue pendiente **de verdad** — entre otros: -> validación de email antes de enviar, onboarding de cloud, recuperación de contraseña, request logging y -> reset mensual de usage. +> **Status, reconciled with the code.** The items below are checked against the code, not against what +> was believed to be true. Anything still unticked is still genuinely pending — among others: email +> validation before sending, cloud onboarding, password reset, request logging and the monthly usage +> reset. > -> Este archivo describe **dirección de producto**. El trabajo activo y los bugs abiertos se trackean en -> [issues](https://github.com/Arkhe-Systems/senddock/issues), no acá. +> This file describes **product direction**. Active work and open bugs are tracked in +> [issues](https://github.com/Arkhe-Systems/senddock/issues), not here. ## Phase 1: Core Backend - [x] Project structure (Go modules, internal/, cmd/) From 24e8dfa0326ae0719cf38eadb7f1ef74c6ebc732 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Sun, 13 Sep 2026 17:52:09 -0500 Subject: [PATCH 12/27] docs: use english names in the sample data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two examples named a subscriber María and Sebastián while the rest of the documentation and the import fixture use names like Ada Lovelace, John Doe and Jane Smith. A reader cannot tell whether the accent is load-bearing — a hint that non-ASCII survives the round trip — or just how the example was written, and the answer changes what they expect from their own data. The webhook payload now matches the name the subscribers API already uses for the same example address. (cherry picked from commit 6104da39acf6745262a54a85dc303fd8c6a1b864) --- docs/guide/newsletters.md | 2 +- docs/guide/webhooks.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/guide/newsletters.md b/docs/guide/newsletters.md index f034feb..8c08d0f 100644 --- a/docs/guide/newsletters.md +++ b/docs/guide/newsletters.md @@ -16,7 +16,7 @@ So one subscriber can be `active` project-wide, a member of three newsletters, o - Unsubscribing from a newsletter only marks that membership; the subscriber stays `active` and keeps receiving your other newsletters and transactional email. - The [suppression list](/guide/suppressions) still gates every send, always. -> Example: María is in **Dev Tips** and **Product updates**. She unsubscribes from **Dev Tips**. Her project status stays `active`, she keeps receiving **Product updates**, and transactional email is untouched. Re-adding her to **Dev Tips** later clears that opt-out. +> Example: Mary is in **Dev Tips** and **Product updates**. She unsubscribes from **Dev Tips**. Her project status stays `active`, she keeps receiving **Product updates**, and transactional email is untouched. Re-adding her to **Dev Tips** later clears that opt-out. ## Creating and managing diff --git a/docs/guide/webhooks.md b/docs/guide/webhooks.md index 964b06d..fa9241d 100644 --- a/docs/guide/webhooks.md +++ b/docs/guide/webhooks.md @@ -105,7 +105,7 @@ Every delivery is an HTTP `POST` with a JSON body shaped like an envelope: "subscriber_id": "uuid", "project_id": "uuid", "email": "user@example.com", - "name": "Sebastián", + "name": "John Doe", "status": "active" // omitted on subscriber.unsubscribed } @@ -116,7 +116,7 @@ Every delivery is an HTTP `POST` with a JSON body shaped like an envelope: "newsletter_id": "uuid", "newsletter_name": "Dev Tips", "email": "user@example.com", - "name": "Sebastián" + "name": "John Doe" } ``` From 9a0baa63abd42bb12a288b4c8fcacfd4a7715983 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Tue, 15 Sep 2026 20:11:33 -0500 Subject: [PATCH 13/27] ci: check that everything tracked is in english MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The roadmap carried a status note written in Spanish for a week. Nothing caught it, because a mixed-language file reads fine to someone who knows both languages and a reviewer skimming a diff has no reason to look for one. scripts/check-language.sh fails on either signal: an accented word, which a stray identifier almost never produces, or a run of Spanish words that English does not use, which covers prose written without accents. Words English also uses (sin, solo, hay), language codes and colliding identifiers are left out on purpose, and a single hit is not enough to fail — a check that cries wolf gets switched off. It runs locally and from CI, and reports the file and the words rather than a count. An accented proper noun is not the exception to add: sample data uses English names like the rest of the repository. (cherry picked from commit 72595dbc00896c09b96a8036b87948a9f123a878) --- .github/workflows/ci.yml | 16 +++++++-- scripts/check-language.sh | 68 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) create mode 100755 scripts/check-language.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e802832..b2fbe54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,8 +2,8 @@ name: CI # Nothing ran the test suite before this workflow existed, so a regression could # reach main with every check green simply because there were no checks. This runs -# the backend build, vet and tests, the frontend type-check, build and tests, and the -# compose guards. +# the backend build, vet and tests, the frontend type-check, build and tests, the compose +# guards, and the language check. # # CGO is disabled to match the released image: the Dockerfile builds with # CGO_ENABLED=0. The suite needs no database — the integration tests that use one skip @@ -98,3 +98,15 @@ jobs: exit 1 fi done + + hygiene: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + # The repository ships in English. A file mixing two languages is unreadable in + # review and cannot be translated later without guessing which half is current, so + # the rule is checked rather than trusted. It also runs locally: + # scripts/check-language.sh + - name: Everything tracked is in English + run: scripts/check-language.sh diff --git a/scripts/check-language.sh b/scripts/check-language.sh new file mode 100755 index 0000000..9c6204c --- /dev/null +++ b/scripts/check-language.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# Verify that everything tracked by git is written in English. +# +# English is the language the repository ships in: code, comments, docs, changelog, +# roadmap, examples and sample data. Spanish is fine for working notes that stay out of +# git, but a tracked file that mixes the two is unreadable in review and cannot be +# translated later without guessing which half is current. This is the check that keeps +# that from creeping back in, because it is invisible to a reader who knows both +# languages and invisible to a reader who knows neither. +# +# Translations, if the project ever ships them, get their own files and their own +# review — they are not a reason to relax this. Neither is an accented proper noun in an +# example: sample data uses English names like the rest of the repository, so the fix is +# to use one, not to add an exception here. +# +# Usage: +# scripts/check-language.sh +set -euo pipefail + +root="$(cd "$(dirname "$0")/.." && pwd)" +cd "$root" + +# Spanish words that are not English words and are not common identifiers. Left out on +# purpose: tokens English also uses (sin, solo, hay), language codes (es) and code +# identifiers that collide (del). A check that cries wolf gets switched off. +spanish_words='que|para|los|las|una|desde|cuando|donde|porque|tambien|también|siempre|nunca|usuario|usuarios|correo|correos|contraseña|envio|envío|envios|envíos|plantilla|plantillas|pagina|página|paginas|páginas|segun|según|ademas|además|tiene|tienen|puede|pueden|debe|deben|hacer|falta|agregar|arreglar|cambiar|revisar|necesita|entonces|mientras|aunque|deberia|debería|seria|sería|está|están|debido|respecto' + +# A word, not a character: this way the report names what to fix, and a single accented +# character inside an otherwise English file is still caught. +accented_word='[A-Za-zÁÉÍÓÚÑáéíóúñ]*[áéíóúüñÁÉÍÓÚÜÑ¿¡][A-Za-zÁÉÍÓÚÑáéíóúñ]*' + +# One hit is not a language; three Spanish words in one file is prose. The threshold +# exists so a stray match cannot block a build. +word_threshold=3 + +failed=0 +checked=0 + +while IFS= read -r file; do + [ -f "$file" ] || continue + checked=$((checked + 1)) + + # -I keeps binaries out: a screenshot is not prose and cannot be translated. + words="$(grep -EIow "$spanish_words" -- "$file" 2>/dev/null | sort | uniq -c | sort -rn || true)" + word_count="$(printf '%s\n' "$words" | awk '{total += $1} END {print total + 0}')" + accents="$(grep -EIo "$accented_word" -- "$file" 2>/dev/null | sort -u | paste -sd' ' - || true)" + + if [ "$word_count" -lt "$word_threshold" ] && [ -z "$accents" ]; then + continue + fi + + failed=$((failed + 1)) + { + printf ' %s\n' "$file" + [ -n "$accents" ] && printf ' accented words: %s\n' "$accents" + if [ "$word_count" -ge "$word_threshold" ]; then + printf ' spanish words: %s\n' "$(printf '%s\n' "$words" | awk '{printf "%s(x%s) ", $2, $1}')" + fi + } >&2 +done < <(git ls-files) + +if [ "$failed" -gt 0 ]; then + printf '\ncheck-language: %s of %s tracked files are not in English (listed above).\n' "$failed" "$checked" >&2 + printf 'Rewrite them in English. Internal notes in Spanish do not belong in the repository.\n' >&2 + exit 1 +fi + +printf 'check-language: OK — %s tracked files, all in English\n' "$checked" From e6682482402b46269dcfa44b5880672f1b90df74 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Wed, 16 Sep 2026 00:34:18 -0500 Subject: [PATCH 14/27] fix(ci): exempt the language check from itself The script has to name the Spanish words it looks for, so once it became a tracked file the check flagged its own word list and failed. It only passed when I ran it before committing, which is exactly the window where the mistake is invisible. The exemption is by exact path, and the run reports how many files it skipped so the exception stays visible. A new file with Spanish still fails, and so does Spanish added to any other script. (cherry picked from commit 878f6f6c22826431c370314bc52fb82900c8cc71) --- scripts/check-language.sh | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/scripts/check-language.sh b/scripts/check-language.sh index 9c6204c..df9fe53 100755 --- a/scripts/check-language.sh +++ b/scripts/check-language.sh @@ -33,11 +33,21 @@ accented_word='[A-Za-zÁÉÍÓÚÑáéíóúñ]*[áéíóúüñÁÉÍÓÚÜÑ¿ # exists so a stray match cannot block a build. word_threshold=3 +# The check has to name the words it looks for, so it contains Spanish itself. It is the one +# file in the repository where they are allowed, and it is excluded by path rather than by +# loosening the pattern for everybody else. +self="$(cd "$(dirname "$0")" && pwd)/$(basename "$0")" + failed=0 checked=0 +exempt=0 while IFS= read -r file; do [ -f "$file" ] || continue + if [ "$root/$file" = "$self" ]; then + exempt=$((exempt + 1)) + continue + fi checked=$((checked + 1)) # -I keeps binaries out: a screenshot is not prose and cannot be translated. @@ -65,4 +75,8 @@ if [ "$failed" -gt 0 ]; then exit 1 fi -printf 'check-language: OK — %s tracked files, all in English\n' "$checked" +if [ "$exempt" -gt 0 ]; then + printf 'check-language: OK — %s tracked files in English (%s exempt: the check itself)\n' "$checked" "$exempt" +else + printf 'check-language: OK — %s tracked files, all in English\n' "$checked" +fi From 96c780eec5f553c41f0281b97957b507b45be4c4 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Wed, 16 Sep 2026 19:33:52 -0500 Subject: [PATCH 15/27] fix(workspace): create an account and its workspace in one transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registering wrote the account first and then its workspace and its membership, as separate statements. A failure on either of the later writes left an account that nobody could see or sign into, holding an email that could never be registered again — and the caller was told registration had failed, so nothing pointed at the row still sitting in the table. Two more flows had the same shape. Creating a workspace wrote the workspace and then its owner membership, so a failure left a workspace no member can list or delete, and one that keeps its creator from being deleted at all. Adding a member to an existing workspace wrote the account and then the membership, the case the audit named. Each of them now runs inside one transaction: the account and its workspace and its membership all land, or none of them do. A first use for the WithTx that sqlc generated and nothing called. Covered by integration tests that make the second write fail on purpose — a trigger, since no unique key can be made to collide without taking the decision away from the code under test. With the transaction removed, all three report the row that survived. The email constraint race now also reads as a taken email rather than an internal error, since the lookup that guards creation is a courtesy and the constraint is what actually holds. --- backend/internal/service/auth.go | 71 +++-- backend/internal/service/transaction_test.go | 277 ++++++++++++++++++ backend/internal/service/tx.go | 36 +++ backend/internal/service/workspace.go | 98 +++++-- .../internal/service/workspace_role_test.go | 22 ++ backend/pkg/app/app.go | 12 +- 6 files changed, 453 insertions(+), 63 deletions(-) create mode 100644 backend/internal/service/transaction_test.go create mode 100644 backend/internal/service/tx.go diff --git a/backend/internal/service/auth.go b/backend/internal/service/auth.go index d047d05..1145065 100644 --- a/backend/internal/service/auth.go +++ b/backend/internal/service/auth.go @@ -52,12 +52,14 @@ type AuthTokens struct { } type AuthService struct { queries *db.Queries + conn *sql.DB jwtSecret []byte } -func NewAuthService(queries *db.Queries, jwtSecret string) *AuthService { +func NewAuthService(queries *db.Queries, conn *sql.DB, jwtSecret string) *AuthService { return &AuthService{ queries: queries, + conn: conn, jwtSecret: []byte(jwtSecret), } } @@ -77,32 +79,43 @@ func (s *AuthService) Register(ctx context.Context, email, password, name string return AuthTokens{}, err } - user, err := s.queries.CreateUser(ctx, db.CreateUserParams{ - Email: email, - PasswordHash: sql.NullString{String: string(hash), Valid: true}, - Name: name, + // The account, its workspace and its owner membership are one registration. Written + // separately, a failure after the first leaves an account nobody can see or log in + // with, holding an email that can no longer be registered. + var userID uuid.UUID + err = withTransaction(ctx, s.conn, s.queries, func(q *db.Queries) error { + user, err := q.CreateUser(ctx, db.CreateUserParams{ + Email: email, + PasswordHash: sql.NullString{String: string(hash), Valid: true}, + Name: name, + }) + if err != nil { + return err + } + if err := s.bootstrapDefaultWorkspace(ctx, q, user.ID); err != nil { + return err + } + userID = user.ID + return nil }) - if err != nil { return AuthTokens{}, err } - if err := s.bootstrapDefaultWorkspace(ctx, user.ID); err != nil { - return AuthTokens{}, err - } - - return s.generateTokens(ctx, user.ID) + return s.generateTokens(ctx, userID) } -func (s *AuthService) bootstrapDefaultWorkspace(ctx context.Context, userID uuid.UUID) error { - ws, err := s.queries.CreateWorkspace(ctx, db.CreateWorkspaceParams{ +// bootstrapDefaultWorkspace runs on the queries it is handed so it joins the caller's +// transaction: a workspace created outside it would outlive a registration that failed. +func (s *AuthService) bootstrapDefaultWorkspace(ctx context.Context, q *db.Queries, userID uuid.UUID) error { + ws, err := q.CreateWorkspace(ctx, db.CreateWorkspaceParams{ Name: "My Workspace", CreatedBy: userID, }) if err != nil { return err } - _, err = s.queries.AddWorkspaceMember(ctx, db.AddWorkspaceMemberParams{ + _, err = q.AddWorkspaceMember(ctx, db.AddWorkspaceMemberParams{ WorkspaceID: ws.ID, UserID: userID, Role: WorkspaceRoleOwner, @@ -123,21 +136,29 @@ func (s *AuthService) RegisterUnverified(ctx context.Context, email, password, n if err != nil { return "", err } - user, err := s.queries.CreateUser(ctx, db.CreateUserParams{ - Email: email, - PasswordHash: sql.NullString{String: string(hash), Valid: true}, - Name: name, + var userID uuid.UUID + err = withTransaction(ctx, s.conn, s.queries, func(q *db.Queries) error { + user, err := q.CreateUser(ctx, db.CreateUserParams{ + Email: email, + PasswordHash: sql.NullString{String: string(hash), Valid: true}, + Name: name, + }) + if err != nil { + return err + } + if err := q.SetEmailVerified(ctx, db.SetEmailVerifiedParams{ID: user.ID, EmailVerified: false}); err != nil { + return err + } + if err := s.bootstrapDefaultWorkspace(ctx, q, user.ID); err != nil { + return err + } + userID = user.ID + return nil }) if err != nil { return "", err } - if err := s.queries.SetEmailVerified(ctx, db.SetEmailVerifiedParams{ID: user.ID, EmailVerified: false}); err != nil { - return "", err - } - if err := s.bootstrapDefaultWorkspace(ctx, user.ID); err != nil { - return "", err - } - return user.ID.String(), nil + return userID.String(), nil } func (s *AuthService) MarkEmailVerified(ctx context.Context, email string) (string, error) { diff --git a/backend/internal/service/transaction_test.go b/backend/internal/service/transaction_test.go new file mode 100644 index 0000000..9c20843 --- /dev/null +++ b/backend/internal/service/transaction_test.go @@ -0,0 +1,277 @@ +package service + +import ( + "context" + "database/sql" + "errors" + "os" + "strings" + "testing" + + "github.com/google/uuid" + _ "github.com/lib/pq" + + "github.com/arkhe-systems/senddock/internal/db" +) + +// These tests exercise the flows that write more than one row to mean one thing, against a +// real database. They skip unless DATABASE_URL points at a migrated instance. +// +// The failure they look for is the one nobody sees: a first row committed and a second one +// lost. The only way to reproduce it deterministically is to make the second write fail on +// purpose, which a trigger does — a unique key cannot be made to collide on demand without +// taking the decision away from the code under test. + +type allowAllGate struct{} + +func (allowAllGate) AllowsFeature(context.Context, string) bool { return true } + +func transactionTestDB(t *testing.T) (*sql.DB, *db.Queries) { + t.Helper() + + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + t.Skip("DATABASE_URL is not set; skipping transaction integration test") + } + + conn, err := sql.Open("postgres", dsn) + if err != nil { + t.Fatalf("open database: %v", err) + } + if err := conn.Ping(); err != nil { + conn.Close() + t.Fatalf("ping database: %v", err) + } + t.Cleanup(func() { conn.Close() }) + + return conn, db.New(conn) +} + +// failInsertsInto makes every insert into table fail, so the second write of a flow can be +// told to fail while the first one is already done. +func failInsertsInto(t *testing.T, conn *sql.DB, table string) { + t.Helper() + + const trigger = "senddock_test_fail_insert" + drop := func() { + _, _ = conn.ExecContext(context.Background(), `DROP TRIGGER IF EXISTS `+trigger+` ON `+table) + _, _ = conn.ExecContext(context.Background(), `DROP FUNCTION IF EXISTS `+trigger+`()`) + } + // A previous run may have died before its own cleanup ran. + drop() + + if _, err := conn.ExecContext(context.Background(), + `CREATE FUNCTION `+trigger+`() RETURNS trigger AS $$ BEGIN RAISE EXCEPTION 'forced failure for test'; END; $$ LANGUAGE plpgsql`, + ); err != nil { + t.Fatalf("create trigger function: %v", err) + } + if _, err := conn.ExecContext(context.Background(), + `CREATE TRIGGER `+trigger+` BEFORE INSERT ON `+table+` FOR EACH ROW EXECUTE FUNCTION `+trigger+`()`, + ); err != nil { + drop() + t.Fatalf("create trigger: %v", err) + } + t.Cleanup(drop) +} + +// workspaceFixture creates an owner and a workspace they own, which is what the flows that +// manage members require before they will write anything. +func workspaceFixture(t *testing.T, conn *sql.DB) (workspaceID, ownerID uuid.UUID, ownerEmail string) { + t.Helper() + + ctx := context.Background() + ownerEmail = "tx-owner-" + uuid.NewString() + "@example.test" + + if err := conn.QueryRowContext(ctx, + `INSERT INTO users (email, name) VALUES ($1, $2) RETURNING id`, + ownerEmail, "Transaction Owner", + ).Scan(&ownerID); err != nil { + t.Fatalf("insert owner: %v", err) + } + if err := conn.QueryRowContext(ctx, + `INSERT INTO workspaces (name, created_by) VALUES ($1, $2) RETURNING id`, + "tx-test", ownerID, + ).Scan(&workspaceID); err != nil { + t.Fatalf("insert workspace: %v", err) + } + t.Cleanup(func() { + if _, err := conn.ExecContext(ctx, `DELETE FROM workspaces WHERE id = $1`, workspaceID); err != nil { + t.Logf("cleanup workspace: %v", err) + } + if _, err := conn.ExecContext(ctx, `DELETE FROM users WHERE id = $1`, ownerID); err != nil { + t.Logf("cleanup owner: %v", err) + } + }) + if _, err := conn.ExecContext(ctx, + `INSERT INTO workspace_members (workspace_id, user_id, role) VALUES ($1, $2, 'owner')`, + workspaceID, ownerID, + ); err != nil { + t.Fatalf("insert owner membership: %v", err) + } + + return workspaceID, ownerID, ownerEmail +} + +// deleteUserByEmail removes a row the test may have left behind, so a failing run does not +// poison the next one. +func deleteUserByEmail(t *testing.T, conn *sql.DB, email string) { + t.Helper() + t.Cleanup(func() { + _, _ = conn.ExecContext(context.Background(), `DELETE FROM users WHERE email = $1`, email) + }) +} + +func TestRegisterCreatesAccountWorkspaceAndMembership(t *testing.T) { + conn, queries := transactionTestDB(t) + auth := NewAuthService(queries, conn, "test-secret-that-is-long-enough-for-tests") + + email := "register-" + uuid.NewString() + "@example.test" + deleteUserByEmail(t, conn, email) + + tokens, err := auth.Register(context.Background(), email, "Correct-Horse-Battery-1", "Grace Hopper") + if err != nil { + t.Fatalf("Register: %v", err) + } + if tokens.AccessToken == "" || tokens.RefreshToken == "" { + t.Fatalf("Register returned incomplete tokens: %+v", tokens) + } + + var userID uuid.UUID + if err := conn.QueryRowContext(context.Background(), + `SELECT id FROM users WHERE email = $1`, email).Scan(&userID); err != nil { + t.Fatalf("registered user is missing: %v", err) + } + + var memberships int + if err := conn.QueryRowContext(context.Background(), + `SELECT count(*) FROM workspace_members m + JOIN workspaces w ON w.id = m.workspace_id + WHERE m.user_id = $1 AND m.role = 'owner' AND w.created_by = $1`, userID).Scan(&memberships); err != nil { + t.Fatalf("count memberships: %v", err) + } + if memberships != 1 { + t.Errorf("registration produced %d owned workspaces, want 1", memberships) + } +} + +// The account is written first and the workspace second, so a failure on the workspace used +// to leave an account behind holding an email that can never be registered again. +func TestRegisterLeavesNoAccountWhenTheWorkspaceWriteFails(t *testing.T) { + conn, queries := transactionTestDB(t) + auth := NewAuthService(queries, conn, "test-secret-that-is-long-enough-for-tests") + + email := "register-orphan-" + uuid.NewString() + "@example.test" + deleteUserByEmail(t, conn, email) + + failInsertsInto(t, conn, "workspaces") + + if _, err := auth.Register(context.Background(), email, "Correct-Horse-Battery-1", "Ada Lovelace"); err == nil { + t.Fatal("Register reported success although the workspace could not be created") + } + + if _, err := queries.GetUserByEmail(context.Background(), email); !errors.Is(err, sql.ErrNoRows) { + t.Errorf("account survived a failed registration: err = %v", err) + } +} + +// The acceptance criterion of the audit item: a failing membership insert must not leave the +// account it was created for. +func TestCreateUserAndAddMemberLeavesNoOrphanWhenTheMembershipWriteFails(t *testing.T) { + conn, queries := transactionTestDB(t) + workspaces := NewWorkspaceService(queries, conn) + workspaces.SetLicenseGate(allowAllGate{}) + + workspaceID, ownerID, _ := workspaceFixture(t, conn) + + email := "orphan-" + uuid.NewString() + "@example.test" + deleteUserByEmail(t, conn, email) + + failInsertsInto(t, conn, "workspace_members") + + if _, err := workspaces.CreateUserAndAddMember( + context.Background(), workspaceID, ownerID, email, "Alan Turing", "Correct-Horse-Battery-1", "developer", + ); err == nil { + t.Fatal("CreateUserAndAddMember reported success although the membership could not be written") + } + + if _, err := queries.GetUserByEmail(context.Background(), email); !errors.Is(err, sql.ErrNoRows) { + t.Errorf("account survived a failed membership insert: err = %v", err) + } +} + +func TestCreateUserAndAddMemberWritesBothRows(t *testing.T) { + conn, queries := transactionTestDB(t) + workspaces := NewWorkspaceService(queries, conn) + workspaces.SetLicenseGate(allowAllGate{}) + + workspaceID, ownerID, _ := workspaceFixture(t, conn) + + email := "member-" + uuid.NewString() + "@example.test" + deleteUserByEmail(t, conn, email) + + created, err := workspaces.CreateUserAndAddMember( + context.Background(), workspaceID, ownerID, email, "Ada Lovelace", "Correct-Horse-Battery-1", "developer", + ) + if err != nil { + t.Fatalf("CreateUserAndAddMember: %v", err) + } + + var role string + if err := conn.QueryRowContext(context.Background(), + `SELECT role FROM workspace_members WHERE workspace_id = $1 AND user_id = $2`, + workspaceID, created.UserID).Scan(&role); err != nil { + t.Fatalf("membership row is missing: %v", err) + } + if role != "developer" { + t.Errorf("role = %q, want developer", role) + } +} + +// A workspace whose owner row never landed belongs to nobody: it is not in any list and no +// screen can delete it. +func TestCreateWorkspaceLeavesNothingBehindWhenTheMembershipWriteFails(t *testing.T) { + conn, queries := transactionTestDB(t) + workspaces := NewWorkspaceService(queries, conn) + + _, ownerID, _ := workspaceFixture(t, conn) + + name := "orphan-workspace-" + uuid.NewString()[:8] + failInsertsInto(t, conn, "workspace_members") + + if _, err := workspaces.Create(context.Background(), ownerID, name); err == nil { + t.Fatal("Create reported success although the owner membership could not be written") + } + + var count int + if err := conn.QueryRowContext(context.Background(), + `SELECT count(*) FROM workspaces WHERE name = $1`, name).Scan(&count); err != nil { + t.Fatalf("count workspaces: %v", err) + } + if count != 0 { + t.Errorf("workspace survived a failed owner insert (%d rows named %q)", count, name) + } +} + +// A trigger that raises is an error like any other, so the flow must not turn it into a +// silent success either. +func TestAFailedFlowReportsAnError(t *testing.T) { + conn, queries := transactionTestDB(t) + workspaces := NewWorkspaceService(queries, conn) + workspaces.SetLicenseGate(allowAllGate{}) + + workspaceID, ownerID, _ := workspaceFixture(t, conn) + email := "reported-" + uuid.NewString() + "@example.test" + deleteUserByEmail(t, conn, email) + + failInsertsInto(t, conn, "workspace_members") + + _, err := workspaces.CreateUserAndAddMember( + context.Background(), workspaceID, ownerID, email, "Grace Hopper", "Correct-Horse-Battery-1", "viewer", + ) + if err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(err.Error(), "forced failure") { + t.Errorf("error %q does not carry the reason the insert failed", err) + } +} diff --git a/backend/internal/service/tx.go b/backend/internal/service/tx.go new file mode 100644 index 0000000..654cf05 --- /dev/null +++ b/backend/internal/service/tx.go @@ -0,0 +1,36 @@ +package service + +import ( + "context" + "database/sql" + + "github.com/arkhe-systems/senddock/internal/db" +) + +// withTransaction runs fn inside a single database transaction and commits it only when fn +// returns nil. +// +// It exists for the flows that write more than one row to mean one thing: an account and +// its workspace and its membership, a workspace and its owner. Written as separate +// statements, a failure partway through leaves half of it behind — an account with no +// workspace, or a workspace no member can see or delete. The caller sees one error and +// assumes nothing happened; the database disagrees. +// +// The queries handed to fn are bound to that transaction. The ones the caller already +// holds keep pointing at the pool, so a statement issued through them would run outside +// it — every write in the flow has to go through the parameter. +func withTransaction(ctx context.Context, conn *sql.DB, queries *db.Queries, fn func(*db.Queries) error) error { + tx, err := conn.BeginTx(ctx, nil) + if err != nil { + return err + } + // A rollback after a successful commit reports sql.ErrTxDone, so the error is dropped: + // what matters is the result of the commit, returned below. + defer func() { _ = tx.Rollback() }() + + if err := fn(queries.WithTx(tx)); err != nil { + return err + } + + return tx.Commit() +} diff --git a/backend/internal/service/workspace.go b/backend/internal/service/workspace.go index 5fea1df..0a2eadf 100644 --- a/backend/internal/service/workspace.go +++ b/backend/internal/service/workspace.go @@ -32,12 +32,13 @@ var ( type WorkspaceService struct { queries *db.Queries + conn *sql.DB gate atomic.Value quota QuotaGate } -func NewWorkspaceService(queries *db.Queries) *WorkspaceService { - s := &WorkspaceService{queries: queries} +func NewWorkspaceService(queries *db.Queries, conn *sql.DB) *WorkspaceService { + s := &WorkspaceService{queries: queries, conn: conn} s.gate.Store(licenseGateHolder{license.DenyAll()}) return s } @@ -70,21 +71,31 @@ func (s *WorkspaceService) Create(ctx context.Context, userID uuid.UUID, name st if name == "" { return db.Workspace{}, errors.New("name is required") } - ws, err := s.queries.CreateWorkspace(ctx, db.CreateWorkspaceParams{ - Name: name, - CreatedBy: userID, + // A workspace whose owner row never landed is invisible to everyone: no screen lists + // it and no screen can delete it. Both writes go in together. + var ws db.Workspace + err := withTransaction(ctx, s.conn, s.queries, func(q *db.Queries) error { + created, err := q.CreateWorkspace(ctx, db.CreateWorkspaceParams{ + Name: name, + CreatedBy: userID, + }) + if err != nil { + return err + } + if _, err := q.AddWorkspaceMember(ctx, db.AddWorkspaceMemberParams{ + WorkspaceID: created.ID, + UserID: userID, + Role: WorkspaceRoleOwner, + InvitedBy: uuid.NullUUID{UUID: userID, Valid: true}, + }); err != nil { + return err + } + ws = created + return nil }) if err != nil { return db.Workspace{}, err } - if _, err := s.queries.AddWorkspaceMember(ctx, db.AddWorkspaceMemberParams{ - WorkspaceID: ws.ID, - UserID: userID, - Role: WorkspaceRoleOwner, - InvitedBy: uuid.NullUUID{UUID: userID, Valid: true}, - }); err != nil { - return db.Workspace{}, err - } return ws, nil } @@ -236,30 +247,42 @@ func (s *WorkspaceService) CreateUserAndAddMember(ctx context.Context, workspace return CreatedUser{}, err } - user, err := s.queries.CreateUser(ctx, db.CreateUserParams{ - Email: email, - PasswordHash: sql.NullString{String: string(hash), Valid: true}, - Name: name, + // An account that never joins the workspace is a half-created account: the owner who + // created it cannot see it, but its email is taken for good. + var created CreatedUser + err = withTransaction(ctx, s.conn, s.queries, func(q *db.Queries) error { + user, err := q.CreateUser(ctx, db.CreateUserParams{ + Email: email, + PasswordHash: sql.NullString{String: string(hash), Valid: true}, + Name: name, + }) + if err != nil { + if isUniqueViolation(err) { + return ErrEmailTaken + } + return err + } + if _, err := q.AddWorkspaceMember(ctx, db.AddWorkspaceMemberParams{ + WorkspaceID: workspaceID, + UserID: user.ID, + Role: role, + InvitedBy: uuid.NullUUID{UUID: actorID, Valid: true}, + }); err != nil { + return err + } + created = CreatedUser{ + UserID: user.ID, + Email: user.Email, + Name: user.Name, + Role: role, + } + return nil }) if err != nil { return CreatedUser{}, err } - if _, err := s.queries.AddWorkspaceMember(ctx, db.AddWorkspaceMemberParams{ - WorkspaceID: workspaceID, - UserID: user.ID, - Role: role, - InvitedBy: uuid.NullUUID{UUID: actorID, Valid: true}, - }); err != nil { - return CreatedUser{}, err - } - - return CreatedUser{ - UserID: user.ID, - Email: user.Email, - Name: user.Name, - Role: role, - }, nil + return created, nil } func (s *WorkspaceService) UpdateMemberRole(ctx context.Context, workspaceID, actorID, targetID uuid.UUID, role string) error { @@ -311,6 +334,17 @@ func isCheckConstraintViolation(err error) bool { return false } +// isUniqueViolation reports whether the error is a unique constraint rejection. The email +// lookups that guard account creation are a courtesy to the caller: the constraint is what +// actually holds, and losing that race is a taken email, not an internal failure. +func isUniqueViolation(err error) bool { + var pqErr *pq.Error + if errors.As(err, &pqErr) { + return pqErr.Code == "23505" + } + return false +} + func (s *WorkspaceService) RemoveMember(ctx context.Context, workspaceID, actorID, targetID uuid.UUID) error { if actorID != targetID { if !s.memberMgmtAllowed(ctx) { diff --git a/backend/internal/service/workspace_role_test.go b/backend/internal/service/workspace_role_test.go index 61739e2..60ef98c 100644 --- a/backend/internal/service/workspace_role_test.go +++ b/backend/internal/service/workspace_role_test.go @@ -69,3 +69,25 @@ func TestIsCheckConstraintViolation(t *testing.T) { } } } + +// A unique rejection is what the database says when two registrations race past the email +// lookup that guards account creation. It has to read as a taken email, not as an internal +// failure, or the loser of the race gets a 500 for something they can act on. +func TestIsUniqueViolation(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + {"unique violation", &pq.Error{Code: "23505"}, true}, + {"wrapped unique violation", fmt.Errorf("create user: %w", &pq.Error{Code: "23505"}), true}, + {"check violation", &pq.Error{Code: "23514"}, false}, + {"foreign key violation", &pq.Error{Code: "23503"}, false}, + {"plain error", errors.New("boom"), false}, + } + for _, tc := range cases { + if got := isUniqueViolation(tc.err); got != tc.want { + t.Errorf("%s: isUniqueViolation(%v) = %v, want %v", tc.name, tc.err, got, tc.want) + } + } +} diff --git a/backend/pkg/app/app.go b/backend/pkg/app/app.go index f9023de..286a7a1 100644 --- a/backend/pkg/app/app.go +++ b/backend/pkg/app/app.go @@ -203,15 +203,15 @@ func (a *App) Audit() *service.AuditService { return a.audit } func (a *App) Projects() *service.ProjectService { return a.projects } func (a *App) RegisterUnverified(ctx context.Context, email, password, name string) (string, error) { - return service.NewAuthService(a.queries, a.cfg.JWTSecret).RegisterUnverified(ctx, email, password, name) + return service.NewAuthService(a.queries, a.conn, a.cfg.JWTSecret).RegisterUnverified(ctx, email, password, name) } func (a *App) MarkEmailVerified(ctx context.Context, email string) (string, error) { - return service.NewAuthService(a.queries, a.cfg.JWTSecret).MarkEmailVerified(ctx, email) + return service.NewAuthService(a.queries, a.conn, a.cfg.JWTSecret).MarkEmailVerified(ctx, email) } func (a *App) IsUnverifiedUser(ctx context.Context, email string) (bool, error) { - return service.NewAuthService(a.queries, a.cfg.JWTSecret).IsUnverifiedUser(ctx, email) + return service.NewAuthService(a.queries, a.conn, a.cfg.JWTSecret).IsUnverifiedUser(ctx, email) } func (a *App) SetDeviceGate(g handler.DeviceGate) { @@ -248,7 +248,7 @@ func (a *App) IssueSession(ctx context.Context, w http.ResponseWriter, userID st if err != nil { return err } - tokens, err := service.NewAuthService(a.queries, a.cfg.JWTSecret).IssueTokens(ctx, uid) + tokens, err := service.NewAuthService(a.queries, a.conn, a.cfg.JWTSecret).IssueTokens(ctx, uid) if err != nil { return err } @@ -377,7 +377,7 @@ func (a *App) registerCoreRoutes(emailService *service.EmailService) { cfg := a.cfg queries := a.queries - authService := service.NewAuthService(queries, cfg.JWTSecret) + authService := service.NewAuthService(queries, a.conn, cfg.JWTSecret) authHandler := handler.NewAuthHandler(authService) if a.cache != nil { authHandler.SetLoginLimiter(a.cache) @@ -388,7 +388,7 @@ func (a *App) registerCoreRoutes(emailService *service.EmailService) { a.projects = projectService projectHandler := handler.NewProjectHandler(projectService) - workspaceService := service.NewWorkspaceService(queries) + workspaceService := service.NewWorkspaceService(queries, a.conn) a.workspaces = workspaceService workspaceHandler := handler.NewWorkspaceHandler(workspaceService, projectService) From 9e7ad729d998620046a9e0fa452b9cab0acf6b9f Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Wed, 16 Sep 2026 21:57:26 -0500 Subject: [PATCH 16/27] docs(api): correct the authentication reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page described a product that does not exist in several places, and two of them would cost a developer real time: it named a refresh token lifetime of seven days instead of two hours, and told you that changing your password leaves other sessions alive when it revokes every refresh token. The 2FA exchange used field names the API has never sent, so a client copied from the page could not complete a login, and the disable-2FA body was missing a required field. The error table invented a 403 body and claimed API keys carry full project access. Keys can exercise three capabilities, and the refusal for anything else is a different message than the one for a role. The first-boot endpoints were described with the wrong status codes and the wrong body — 201 with a message, and 403 rather than 409 once an account exists — and the CSV import that is offered nowhere in the backend is gone. Every claim was checked against the handler and service code before it was written, not against the audit that reported it. --- docs/api/authentication.md | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/docs/api/authentication.md b/docs/api/authentication.md index b0f0996..1282c15 100644 --- a/docs/api/authentication.md +++ b/docs/api/authentication.md @@ -27,17 +27,19 @@ Keys are project-scoped — the key identifies the project, so the URL `{id}` se | `POST /api/v1/projects/{id}/send` | Single transactional send (template or raw HTML). | | `POST /api/v1/projects/{id}/send/batch` | Same template, many recipients with per-recipient `data`. | | `POST /api/v1/projects/{id}/broadcast` | Send to every active subscriber. | -| `POST /api/v1/projects/{id}/subscribers/import` | CSV / JSON import with email validation. | +| `POST /api/v1/projects/{id}/subscribers/import` | JSON array import with email validation. | | `GET /api/v1/projects/{id}/stats` | Read-only summary counts (`total`, `sent`, `failed`, `bounced`, `suppressed`, `opened`). | -Every other endpoint requires cookie auth — see the next section. As a rule of thumb: **anything that mutates project, workspace, template, subscriber, webhook or audit-log state is cookie-only**, because role-based capabilities key off the user identity that an API key doesn't carry. +Every other endpoint requires cookie auth — see the next section. As a rule of thumb: **anything that mutates project, workspace, template, subscriber, webhook or audit-log state is cookie-only**, because role-based capabilities key off the user identity that an API key doesn't carry. A key can exercise only a fixed allowlist of capabilities — `send.transactional`, `broadcast` and `subscribers.write` — so a capability outside it is refused even on an endpoint mounted for both auth schemes. ### Errors | Status | Body | Meaning | |---|---|---| -| `401` | `{"error":"missing or invalid api key"}` | Header missing, malformed, or key revoked. | -| `403` | `{"error":"forbidden"}` | Key valid but capability denied. (API keys carry full project access today, so 403 only happens on cookie auth.) | +| `401` | `{"error":"missing or invalid api key"}` | Header missing or malformed, or a key that doesn't match one in the database. | +| `401` | `{"error":"invalid api key"}` | Key revoked. | +| `403` | `{"error":"this action is not permitted with an API key"}` | The endpoint requires a capability outside the API key allowlist. | +| `403` | `{"error":"your role does not allow this action"}` | Cookie auth, and your role lacks the capability for this endpoint. | ## Cookie session (dashboard SPA) @@ -56,7 +58,7 @@ POST /api/v1/auth/login } ``` -Sets an `access_token` (15 min lifetime) and `refresh_token` (7 days) cookie, both `HttpOnly`, `Secure` and `SameSite=Lax`. +Sets an `access_token` (15 min lifetime) and `refresh_token` (2 hours) cookie, both `HttpOnly` and `SameSite=Lax`. `Secure` is set when the deployment's origin is `https`. ### Refresh @@ -107,7 +109,7 @@ POST /api/v1/me/password Verifies `current_password` with bcrypt, then validates `new_password` against the registration rules: min 8 chars, at least one uppercase, one digit, one special character. Returns `200` on success, `400` on weak new password, `401` on bad current password. -Changing the password does **not** invalidate existing sessions on other devices — sign out from those manually if needed. +Changing the password revokes every outstanding refresh token, so all other sessions are logged out and their refresh attempts fail. The current access token keeps working until it expires (up to 15 minutes). ## Two-factor authentication @@ -129,7 +131,7 @@ No body. Response: } ``` -Recovery codes are returned **only here** and only this once. Persist them client-side immediately or you lose them. +Recovery codes are returned here and on [Regenerate recovery codes](#regenerate-recovery-codes) — the dashboard stores them client-side, so persist them yourself or regenerate a fresh set later. ### Confirm setup @@ -150,10 +152,10 @@ POST /api/v1/me/2fa/disable ``` ```json -{ "code": "123456" } +{ "password": "...", "code": "123456" } ``` -`code` is either a valid TOTP code or a single-use recovery code. No code, no disable — there is no admin override. +`code` is either a valid TOTP code or a single-use recovery code. Both fields are required — no password, no code, no disable, and there is no admin override. ### Regenerate recovery codes @@ -173,10 +175,10 @@ Requires a valid TOTP code. Invalidates the existing set and returns ten fresh o ### Login second step -When `/api/v1/auth/login` succeeds against an account with 2FA enabled, it returns `200` with `needs_2fa: true` and an intermediate token instead of setting session cookies: +When `/api/v1/auth/login` succeeds against an account with 2FA enabled, it returns `200` with `requires_2fa: true` and a two-factor token instead of setting session cookies: ```json -{ "needs_2fa": true, "intermediate_token": "..." } +{ "requires_2fa": true, "two_factor_token": "..." } ``` Complete the login by posting to: @@ -186,10 +188,10 @@ POST /api/v1/auth/2fa ``` ```json -{ "intermediate_token": "...", "code": "123456" } +{ "two_factor_token": "...", "code": "123456" } ``` -`code` is a TOTP code or a recovery code. On success, session cookies are set the same way as a regular login. The intermediate token is short-lived (a few minutes) and only valid for this exchange. +`code` is a TOTP code or a recovery code. On success, session cookies are set the same way as a regular login. The two-factor token is short-lived (5 minutes) and only valid for this exchange. ## Email-verified signup (Cloud) @@ -263,7 +265,7 @@ Completes the login **on the device that started it** — login cookies are set ## First-boot setup -These two endpoints exist only until the first user is created — after that they return errors. Self-hosted instances run them via the **Setup** screen the first time you open the dashboard; you almost never need to call them by hand. +These two endpoints exist only until the first user is created — after that `POST /api/v1/setup` fails while `GET /api/v1/setup/status` keeps answering. Self-hosted instances run them via the **Setup** screen the first time you open the dashboard; you almost never need to call them by hand. ### Status @@ -291,4 +293,4 @@ POST /api/v1/setup } ``` -Returns `200 OK` with the new user's `user_id` and sets login cookies in the same response. Calling this when users already exist returns `409 Conflict`. +Returns `201 Created` with `{"message":"setup complete"}` and sets login cookies in the same response. Calling this when users already exist returns `403 Forbidden` with `{"error":"setup already completed"}`. From ecd1c7307e10f8b156f43e7c56dd9e04699a3684 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Thu, 17 Sep 2026 20:26:44 -0500 Subject: [PATCH 17/27] docs: correct the subscribers and suppressions references Four facts the pages got wrong. The bulk action answers 200 with a message rather than 204, suppression timestamps are refreshed only when an address is added again and never by a send, the 401 on the suppression routes is a missing session cookie because those routes do not accept an API key at all, and access to the subscriber endpoints follows workspace membership rather than project ownership, with two documented exceptions. The email-validation wording was also stricter than the code: a domain with an A record and no MX passes, and only a domain that resolves to nothing is rejected. Both the API page and the guide carried the overstated version, so both changed. --- docs/api/subscribers.md | 8 ++++---- docs/api/suppressions.md | 6 +++--- docs/guide/subscribers.md | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/api/subscribers.md b/docs/api/subscribers.md index 04455ad..548c3c4 100644 --- a/docs/api/subscribers.md +++ b/docs/api/subscribers.md @@ -1,6 +1,6 @@ # Subscribers API -All endpoints require cookie authentication. The authenticated user must own the project. +All endpoints require cookie authentication, and access is granted by workspace membership rather than project ownership: reads need the caller to be a member of the project's workspace, writes need the `subscribers:write` capability (roles `owner` and `admin`). Two endpoints are exceptions — [Bulk Import](#bulk-import) also accepts an API key, and the [Waitlist](#waitlist-public) endpoint is public and takes no authentication. ## Add Subscriber @@ -101,7 +101,7 @@ Both default to `true`. Pass `?...=false` to relax validation — useful when yo | Parameter | Default | Effect when `false` | |---|---|---| -| `validate_mx` | `true` | Skip the DNS lookup. Domains with no MX still get accepted. Saves ~50 ms per unique domain. | +| `validate_mx` | `true` | Skip the DNS lookup. Every domain passes this check. Saves ~50 ms per unique domain. | | `validate_disposable` | `true` | Skip the built-in disposable-domain block-list. Mailinator / 10minutemail / etc. get imported. | Syntax validation is always on — addresses without a valid `local@domain.tld` form are rejected regardless. @@ -127,7 +127,7 @@ Syntax validation is always on — addresses without a valid `local@domain.tld` | `imported` | New subscribers actually inserted. | | `duplicates` | Rows whose `email` was already on the project (silent skip — not an error). | | `syntax_invalid` | Rows that failed RFC 5322 parsing. | -| `no_mx` | Rows whose domain has no MX record (only counted when `validate_mx=true`). | +| `no_mx` | Rows whose domain resolves to nothing — no MX record and no A/AAAA fallback (only counted when `validate_mx=true`). | | `disposable` | Rows whose domain is on the disposable block-list (only counted when `validate_disposable=true`). | | `suppressed` | Rows whose email is on the project's [suppression list](./suppressions); skipped without insert. | | `rejected` | Per-row breakdown of every row that didn't make it: `{email, name, reason}`. `reason` is one of `syntax_invalid`, `no_mx`, `disposable`, `suppressed`, `duplicate`. | @@ -162,7 +162,7 @@ Apply the same operation to many existing subscribers — the dashboard uses thi Cookie auth only (the role must have `subscribers:write`). For ingesting fresh rows, use [Bulk Import](#bulk-import) — that endpoint takes raw `email`/`name` rows and accepts API keys; this one operates on already-stored subscriber ids. -**Response** `204 No Content`. Subscriber ids that don't belong to the project are silently skipped. +**Response** `200 OK` with `{"message":"success"}`. Subscriber ids that don't belong to the project are silently skipped. ## Update Status diff --git a/docs/api/suppressions.md b/docs/api/suppressions.md index 5aef769..f895525 100644 --- a/docs/api/suppressions.md +++ b/docs/api/suppressions.md @@ -39,7 +39,7 @@ GET /api/v1/projects/{id}/suppressions } ``` -`source` is optional free-text recorded at insert time (e.g. the SMTP error code that caused the bounce). `last_seen_at` is updated whenever a new send attempt to the same address bumps into the list. +`source` is optional free-text recorded at insert time (e.g. the SMTP error code that caused the bounce). `last_seen_at` is only refreshed when the same address is added to the list again — a send that skips a suppressed address never touches it. ### Example @@ -84,7 +84,7 @@ Add one or more addresses to the list. Already-suppressed entries are silently d ### Capability -Requires the `suppressions:write` capability — owners and admins can call this endpoint; developers and viewers cannot. API keys cannot call it at all (see the cookie-auth note at the top). +Requires the `suppressions:write` capability. Of the workspace roles — `owner`, `admin`, `developer`, `viewer` — only `owner` and `admin` carry it; `developer` and `viewer` get `403`. API keys cannot call it at all (see the cookie-auth note at the top). ### Example @@ -123,7 +123,7 @@ curl -X DELETE "$YOUR_BASE_URL/api/v1/projects/$YOUR_PROJECT_ID/suppressions/01H | Status | Cause | |---|---| | `400` | `emails` array is empty / missing, invalid project id, invalid suppression id. | -| `401` | Missing / invalid API key. | +| `401` | Missing or invalid session cookie — these routes are cookie-only, so an API key never authenticates them. | | `403` | The role doesn't have `suppressions:write`. | | `404` | Project not found, or suppression id not part of this project. | diff --git a/docs/guide/subscribers.md b/docs/guide/subscribers.md index 9fe6a43..7608127 100644 --- a/docs/guide/subscribers.md +++ b/docs/guide/subscribers.md @@ -52,7 +52,7 @@ The dashboard importer takes CSV only. To import a JSON array (with `fields` and Every row goes through three checks before it lands in the database: 1. **Syntax** — the address must parse as a valid mailbox (RFC 5322). -2. **MX record** — SendDock resolves the domain's MX records. Domains with no MX (typo, dead domain) are rejected. +2. **MX record** — SendDock resolves the domain's MX records, falling back to A/AAAA. A domain that resolves to nothing at all (typo, dead domain) is rejected; one with an A record and no MX still passes. 3. **Disposable-domain block-list** — a built-in list of throwaway providers (Mailinator, 10minutemail, etc.) is rejected by default. Rows that fail any check are skipped. The **import results modal** breaks the input down into: From 6f2e9d4b63ac8ef7f17efdf2d83b911516344338 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Fri, 18 Sep 2026 19:47:15 -0500 Subject: [PATCH 18/27] docs(api): correct the sending reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five of these would have cost a developer a debugging session. The broadcast response was documented with counters the endpoint never returns — it answers the enqueue result, so `failed` is always 0 and the real progress lives in the broadcast history. The log row was said to omit `opened_at` and `clicked_at`, which it carries. The click detail listed an `ip_address` the query does not select. The batch response claims `suppressed` is omitted at zero, when it is always present. And the tracking pixel was documented without its `.gif`, which is the form SendDock actually emits. The suppression body was also narrower than the code: it is returned for a send addressed with `to`, whether by template or raw HTML, so both are described now. Broadcast status loses a `pending` value that no row can hold. Checked against the handlers, the response structs and the SQL, and the pixel path against a message captured on the wire during earlier work. --- docs/api/sending.md | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/docs/api/sending.md b/docs/api/sending.md index 10e5e15..4d4a4d2 100644 --- a/docs/api/sending.md +++ b/docs/api/sending.md @@ -61,13 +61,19 @@ For the subscriber-send variant (`subscriber_id` + `template_id`): {"sent": 1, "failed": 0} ``` -When the recipient is on the project's [suppression list](./suppressions), the endpoint **does not** error — it returns `200 OK` with: +When the recipient is on the project's [suppression list](./suppressions), the endpoint **does not** error — a send addressed with `to` (template or raw HTML) returns `200 OK` with: ```json {"message": "suppressed", "suppressed": 1} ``` -The send is recorded with status `suppressed` in the email log; no SMTP attempt is made. This is the same shape as the failure-counted version below — when present, `suppressed` always counts in addition to `sent` + `failed`. +The subscriber-send variant instead returns its usual count shape, with the recipient counted as suppressed rather than sent: + +```json +{"sent": 0, "failed": 0, "suppressed": 1} +``` + +The send is recorded with status `suppressed` in the email log; no SMTP attempt is made. When present, `suppressed` always counts in addition to `sent` + `failed`. ::: info Error contract A `template_id` or `subscriber_id` that doesn't exist in the project, a project without SMTP configured, or a subscriber that isn't `active` all return `400` with an `{error}` body. An SMTP-level rejection (the relay refused the message) also returns `400` with the SMTP error in `{error}` — the log row is marked `failed` or `bounced` accordingly. Only suppression is a "soft" `200`, shown above. @@ -99,7 +105,7 @@ Sends a template to multiple recipients in one request. Each recipient can have {"sent": 3, "failed": 0, "suppressed": 1} ``` -`suppressed` is omitted when zero. Recipients on the project's suppression list are skipped without an SMTP attempt and do **not** count towards `failed`. +`suppressed` is always present, and is `0` when no recipient was skipped. Recipients on the project's suppression list are skipped without an SMTP attempt and do **not** count towards `failed`. ## Broadcast @@ -130,10 +136,10 @@ Variables are replaced per subscriber. The `{{unsubscribe_url}}` is injected aut **Response** ```json -{"sent": 150, "failed": 2, "suppressed": 8} +{"sent": 150, "failed": 0, "broadcast_id": "uuid"} ``` -`suppressed` is the count of subscribers skipped because they were on the project's suppression list. They do **not** count towards `failed`. Field is omitted when zero. +The response reports the **enqueue** result only: `sent` is the number of recipients queued, `failed` is always `0`, and `broadcast_id` identifies the broadcast whose progress is tracked below. No message has been attempted yet, so `suppressed` is not part of this response; the real `sent_count`, `failed_count` and `suppressed_count` are read from [List broadcasts](#list-broadcasts) as the worker drains the queue. ::: info Public URL required The instance must have a [public URL](../guide/instance-settings#public-url) configured — without it the endpoint refuses with `400` because unsubscribe links can't be built. See [A public URL is required](../guide/sending#a-public-url-is-required-for-broadcasts-and-campaigns). @@ -178,10 +184,10 @@ Sends a test email to verify SMTP configuration. Cookie auth only. ## Open Tracking ``` -GET /t/{logId} +GET /t/{logId}.gif ``` -Public endpoint (no auth required, no file extension on the path). Returns a 1×1 transparent GIF in the response body with `Content-Type: image/gif`. This pixel is automatically injected into **every** outgoing email — subscriber sends, broadcasts, and raw transactional sends to a non-subscriber address (`/send`, `/send/batch`). When the recipient's email client loads the image, SendDock records the `opened_at` timestamp on the corresponding email log entry. Only the first open is recorded. +Public endpoint (no auth required). Returns a 1×1 transparent GIF in the response body with `Content-Type: image/gif`. SendDock emits the pixel as `/t/{logId}.gif` and the handler strips the `.gif` suffix. The pixel is automatically injected into **every** outgoing email — subscriber sends, broadcasts, and raw transactional sends to a non-subscriber address (`/send`, `/send/batch`). When the recipient's email client loads the image, SendDock records the `opened_at` timestamp on the corresponding email log entry. Only the first open is recorded. ## Click Tracking @@ -246,14 +252,16 @@ GET /api/v1/projects/{id}/logs?status=bounced&from=2026-01-01T00:00:00Z&to=2026- "subject": "Welcome!", "status": "sent", "error": null, - "sent_at": "2026-01-01T00:00:00Z" + "sent_at": "2026-01-01T00:00:00Z", + "opened_at": "2026-01-01T00:02:14Z", + "clicked_at": null } ], "total": 1520 } ``` -`opened_at` and `clicked_at` are **not** included in the log row (the row tracks delivery, not engagement). For per-email open/click data, use the detail endpoint below. +`opened_at` and `clicked_at` come with the row and are `null` until the recipient opens or clicks. For the per-click detail — URL, user agent and time of every click — use the detail endpoint below. ### Email log detail @@ -280,10 +288,10 @@ Cookie auth only. Backs the right-side drawer in the Logs page. Returns the full }, "clicks": [ { + "id": "uuid", "url": "https://example.com/pricing", - "user_agent": "Mozilla/5.0 ...", - "ip_address": "203.0.113.42", - "clicked_at": "2026-01-01T00:02:31Z" + "clicked_at": "2026-01-01T00:02:31Z", + "user_agent": "Mozilla/5.0 ..." } ] } @@ -309,17 +317,18 @@ Cookie auth only. Returns the broadcast history for the project, newest first, w "template_id": "uuid", "newsletter_id": null, "status": "sending", - "total": 12500, + "total_recipients": 12500, "sent_count": 8472, "failed_count": 31, - "created_at": "2026-01-15T10:00:00Z", - "completed_at": null + "started_at": "2026-01-15T10:00:00Z", + "finished_at": null } - ] + ], + "total": 1 } ``` -`status` is one of `pending`, `sending`, `completed`, `interrupted`. (`interrupted` is a legacy state from a mid-broadcast restart on older builds; current versions stay `sending` and resume from the job queue.) While a broadcast is `sending`, `sent_count` and `failed_count` reflect the live state of the per-recipient job queue. The dashboard polls this endpoint every five seconds to drive the live progress bars. +`status` is one of `sending`, `completed`, `interrupted`. (`interrupted` is a legacy state from a mid-broadcast restart on older builds; current versions stay `sending` and resume from the job queue.) While a broadcast is `sending`, `sent_count` and `failed_count` reflect the live state of the per-recipient job queue. The envelope also carries `total`, the number of broadcasts matching the filters, for pagination. The dashboard polls this endpoint every five seconds to drive the live progress bars. ## Stats From 29a85b3dc2bb0cd70b6746c8bd4aac38058d6429 Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Fri, 18 Sep 2026 22:13:38 -0500 Subject: [PATCH 19/27] docs: correct the SDK page and the self-hosting steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four things that would have sent someone down the wrong path. The SDK page promised a retry on every 5xx when it retries five statuses and treats the rest as final, and it claimed the newsletter event arrived in v0.8.2 — the server version — when it is SDK v0.2.0, so a reader would have looked for it in the wrong release notes. In the self-hosting guides, two commands named the wrong compose service: the source build calls it `app` and the image build calls it `senddock`, and the pages had them swapped, so the command a reader copies to debug a failed boot fails instead. Another page told you to restart the backend after setting the public URL, which the settings provider applies without a restart, and the rate limiter exemption listed only `/health` when `/metrics` is registered beside it. --- docs/api/sdk.md | 8 ++++---- docs/self-hosting/troubleshooting.md | 5 ++--- docs/self-hosting/updating.md | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/api/sdk.md b/docs/api/sdk.md index aff9b43..3c7b893 100644 --- a/docs/api/sdk.md +++ b/docs/api/sdk.md @@ -20,7 +20,7 @@ import { SendDock } from '@senddock/sdk' const senddock = new SendDock({ baseUrl: 'https://senddock.example.com', projectId: 'your-project-uuid', - apiKey: process.env.SENDDOCK_API_KEY, + apiKey: process.env.SENDDOCK_API_KEY!, }) ``` @@ -54,7 +54,7 @@ What you get on every call: - **Full typing.** Requests are validated by the compiler — an invalid body doesn't build. Responses are typed, including the discriminated union for suppressed recipients. - **One error type.** Anything that goes wrong throws `SendDockError` carrying the API's own message — a failed fetch never leaks a raw `TypeError` or `SyntaxError`. -- **Retries built in.** `429` waits for `Retry-After` (capped at 60s); `5xx` and network failures back off exponentially; `4xx` fails fast. +- **Retries built in.** `429` waits for `Retry-After` (capped at 60s); `500`, `502`, `503` and `504`, plus network failures, back off exponentially; anything else fails fast. - **Timeouts.** Every request carries an abort signal, so a hung connection can't block your process. ## Sending @@ -128,7 +128,7 @@ app.post('/webhooks/senddock', express.raw({ type: 'application/json' }), (req, const valid = verifyWebhookSignature({ payload: req.body, signature: req.get('X-SendDock-Signature') ?? '', - secret: process.env.SENDDOCK_WEBHOOK_SECRET, + secret: process.env.SENDDOCK_WEBHOOK_SECRET!, }) if (!valid) return res.status(401).end() const event = JSON.parse(req.body) @@ -136,7 +136,7 @@ app.post('/webhooks/senddock', express.raw({ type: 'application/json' }), (req, }) ``` -The check is timing-safe and rejects timestamps older than 5 minutes (configurable via `toleranceSeconds`). The secret is shown once, when the webhook is created. The SDK's event-type union includes `subscriber.newsletter_unsubscribed` (new in v0.8.2) alongside the core `email.*` and `subscriber.*` events. +The check is timing-safe and rejects timestamps older than 5 minutes (configurable via `toleranceSeconds`). The secret is shown once, when the webhook is created. The SDK's event-type union includes `subscriber.newsletter_unsubscribed` (new in SDK v0.2.0) alongside the core `email.*` and `subscriber.*` events. ## Error handling diff --git a/docs/self-hosting/troubleshooting.md b/docs/self-hosting/troubleshooting.md index 43d46b9..fb1f919 100644 --- a/docs/self-hosting/troubleshooting.md +++ b/docs/self-hosting/troubleshooting.md @@ -54,7 +54,6 @@ Single-recipient sends (`/send`) and `/send/batch` continue to work without a pu 1. Open the dashboard → any project → **Settings** → check the "Instance URL" panel. The URL shown there is what every email will use. 2. Set your public URL under **Instance** in the dashboard to the domain where SendDock is reachable, e.g. `https://email.mycompany.com`. 3. Make sure your reverse proxy forwards `/unsubscribe/*` and `/t/*` (open-tracking pixel) to the backend, not just `/api/*`. With the single-binary deploy, both routes are served by the Go process on the same port as the API. -4. Restart the backend so the new value takes effect. If you migrated from an older version, links generated **before** you set your public URL are signed against whatever URL/secret was active then — they will fail validation. New emails will work. @@ -147,7 +146,7 @@ The public waitlist endpoint (`POST /api/v1/projects/{id}/waitlist`) is a public **Fix in production (Docker):** migrations run automatically via `goose` on container startup — the entrypoint script blocks until they finish before exec'ing the binary. If you see this error after pulling a new image, the container almost certainly hit the error during boot. Check the logs: ```bash -docker compose logs app --tail 100 | grep -i 'goose\|migration' +docker compose logs senddock --tail 100 | grep -i 'goose\|migration' ``` If goose printed an error (e.g. couldn't connect to Postgres, or hit a conflicting schema), fix the underlying cause and restart with `docker compose up -d`. The next boot retries from where the last successful version stopped. @@ -261,7 +260,7 @@ The `--force` flag rotates the task even when the image reference appears unchan **The binary boots without Redis, but you almost certainly do need it.** `REDIS_URL` is technically optional, and when unset: -- The **global per-IP rate limiter** (default 600 req/min, every endpoint except `/health`) becomes a no-op. +- The **global per-IP rate limiter** (default 600 req/min, every endpoint except `/health` and `/metrics`) becomes a no-op. - The **per-project sending limits** on `/send`, `/send/batch`, `/broadcast` become no-ops. - The **GitHub releases cache** is skipped — every dashboard load hits the GitHub API directly (you'll burn through the anonymous rate limit fast on busy instances). - Email stats fall back to direct database queries — slightly slower, but functionally identical. diff --git a/docs/self-hosting/updating.md b/docs/self-hosting/updating.md index 4d953e9..c5b64f5 100644 --- a/docs/self-hosting/updating.md +++ b/docs/self-hosting/updating.md @@ -86,7 +86,7 @@ What happens: Same data preservation guarantees as the image flow. -If the build fails or the app never becomes healthy, check `docker compose -f docker-compose.prod.yml logs senddock`. +If the build fails or the app never becomes healthy, check `docker compose -f docker-compose.prod.yml logs app`. --- From 89dfe6231fba598a0f11ea912c0f74ce46e19e8f Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Sun, 20 Sep 2026 15:31:07 -0500 Subject: [PATCH 20/27] docs(api): correct the three Pro reference pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reports page documented response shapes the engine never returns — a flat `{dimension, rows}` and a `{row_dim, col_dim, cols}` pivot — when it answers `{dataset, measure, dimensions, rows}` and `{..., pivot{columns, rows}}`, and it listed a fraction of the measures that exist. It also described a `viz` field that is accepted and never read, so a reader would set it and wonder why the output did not change. The deliverability page named invented metric keys (`hard`, `soft`, `bounce_pct`, `spam_pct`) for a response that sends `hard_bounces`, `soft_bounces`, `bounce_rate_pct` and `complaint_rate_pct`, listed four mailbox providers when seven are recognised, and documented a check item as `record` when the code emits `name`. Its error table was missing the `400` a project without a from address gets. The audit-log page called the entry id a ULID when the column is a UUID, listed five target types when eleven are written, and promised a `404` for a project that does not exist where the handler answers `403` — a project id that cannot be parsed is the only `404`. All three pages also claimed the caller must own the project. These endpoints authorize workspace membership and check no role, which is what they say now. --- docs/api/audit-log.md | 14 ++++++------- docs/api/deliverability.md | 40 +++++++++++++++++++++++--------------- docs/api/reports.md | 29 ++++++++++++++++----------- 3 files changed, 49 insertions(+), 34 deletions(-) diff --git a/docs/api/audit-log.md b/docs/api/audit-log.md index a54c2b1..1646935 100644 --- a/docs/api/audit-log.md +++ b/docs/api/audit-log.md @@ -26,11 +26,11 @@ GET /api/v1/projects/{id}/audit-log { "entries": [ { - "id": "01H...", - "user_id": "01H...", + "id": "b7c9e2f4-1a3d-4e6b-8c02-5f7a9d1e3b40", + "user_id": "9d1f4a6c-2b8e-4c31-a5d7-6e8f0b2c4a15", "action": "smtp.update", "target_type": "project", - "target_id": "01H...", + "target_id": "4e6b8c02-5f7a-49d1-9e3b-40b7c9e2f4a3", "metadata": { "smtp_host": "smtp.mailgun.org", "smtp_user": "postmaster@acme.com", @@ -47,10 +47,10 @@ GET /api/v1/projects/{id}/audit-log | Field | Description | |---|---| -| `id` | Unique entry id (ULID). | +| `id` | Unique entry id (UUID). | | `user_id` | UUID of the user who took the action. Resolve to email via [`GET /workspaces/{id}/members`](./workspaces#list-members). | | `action` | Dotted action string. See the [full catalog](../guide/audit-log#what-gets-recorded). | -| `target_type` / `target_id` | The entity acted on — `project`, `api_key`, `webhook`, `suppression`, `workspace`. | +| `target_type` / `target_id` | The entity acted on — `project`, `api_key`, `webhook`, `suppression`, `workspace`, `field`, `subscriber`, `newsletter`, `segment`, `template`, `instance`. `suppression.add` records no `target_id`. | | `metadata` | Action-specific JSON. SMTP and bounce-IMAP entries record host / user / from address; never password fields. | | `ip_address` | IP of the request that triggered the action, parsed from `X-Forwarded-For` if present. | | `user_agent` | Browser / client UA. | @@ -62,8 +62,8 @@ GET /api/v1/projects/{id}/audit-log |---|---| | `401` | Missing cookie session. | | `402` | No valid Pro license key. | -| `403` | The authenticated user doesn't own this project. | -| `404` | Project not found. | +| `403` | The authenticated user isn't a member of the project's workspace (also returned for a project that doesn't exist). | +| `404` | The project id in the path isn't a valid UUID. | ### Examples diff --git a/docs/api/deliverability.md b/docs/api/deliverability.md index 8c24d72..4dc678e 100644 --- a/docs/api/deliverability.md +++ b/docs/api/deliverability.md @@ -2,7 +2,7 @@ Domain-health checks and per-provider send breakdowns that back the [Deliverability](../guide/deliverability) tab. Pro-gated: without a valid Pro license these endpoints return `402 Payment Required`. -Cookie auth only — like the rest of the analytics surface, these read project-scoped aggregates and require an authenticated owner, so a project API key can't call them. +Cookie auth only — like the rest of the analytics surface, these read project-scoped aggregates and require membership of the project's workspace, so a project API key can't call them. ## Domain health @@ -17,16 +17,15 @@ Resolves the SPF, DKIM and DMARC DNS records for the project's sending domain an ```json { "domain": "acme.com", - "from_email": "hello@acme.com", "checks": [ - { "record": "spf", "status": "pass", "detail": "v=spf1 include:...", "fix": "" }, - { "record": "dkim", "status": "warn", "detail": "No key on the common selectors", "fix": "Publish your DKIM key, or confirm the selector your relay uses." }, - { "record": "dmarc", "status": "fail", "detail": "No _dmarc record", "fix": "Add a TXT record at _dmarc.acme.com, e.g. v=DMARC1; p=none; rua=..." } + { "name": "SPF", "status": "pass", "detail": "SPF present with a strict -all policy", "value": "v=spf1 include:..." }, + { "name": "DKIM", "status": "warn", "detail": "No DKIM key found for the common selectors", "fix": "Publish a DKIM key with your sending provider (or use a custom selector we can't auto-detect)." }, + { "name": "DMARC", "status": "fail", "detail": "No DMARC record found", "fix": "Add a TXT record at _dmarc: v=DMARC1; p=none; rua=mailto:you@your-domain" } ] } ``` -`status` is one of `pass` / `warn` / `fail`. `fix` is empty on a pass. +`name` is `SPF`, `DKIM` or `DMARC`. `status` is one of `pass` / `warn` / `fail`. `value` carries the record that was read and `fix` the suggested action; both are omitted when empty, so a pass has no `fix`. ## Per-provider breakdown @@ -40,34 +39,43 @@ Groups the project's email logs by mailbox provider (inferred from the recipient ```json { + "from": "2026-05-31T00:00:00Z", + "to": "2026-06-30T00:00:00Z", + "total_bounced": 41, "providers": [ { "provider": "Gmail", - "sent": 4120, "bounced": 33, "opened": 1890, "clicked": 402, "complained": 3, - "hard": 20, "soft": 13, - "acceptance_pct": 99.2, "bounce_pct": 0.8, - "open_pct": 45.9, "click_pct": 9.8, "spam_pct": 0.07 + "sent": 4120, "failed": 4, "bounced": 33, "opened": 1890, "clicked": 402, "complained": 3, + "hard_bounces": 20, "soft_bounces": 13, + "acceptance_pct": 99.11, "bounce_rate_pct": 0.79, + "open_rate_pct": 45.87, "click_rate_pct": 9.76, "complaint_rate_pct": 0.07 } ] } ``` +The envelope echoes the resolved `from` / `to` window and `total_bounced`, the bounced rows in it. + | Field | Meaning | |---|---| -| `provider` | `Gmail`, `Outlook`, `Yahoo`, `Apple`, or `Other`. | -| `hard` / `soft` | Bounce split, classified from the bounce reason text. | +| `provider` | `Gmail`, `Outlook`, `Yahoo`, `Apple`, `AOL`, `Proton`, or `Other` (`Unknown` when the log row carries no recipient domain). | +| `failed` | Emails rejected at send time. | +| `hard_bounces` / `soft_bounces` | Bounce split, classified from the bounce reason text. | | `complained` | Spam complaints reported for this provider via the [complaint webhook](../guide/deliverability#wiring-the-complaint-webhook) (FBL). | -| `acceptance_pct` | Accepted ÷ attempted. | -| `spam_pct` | Complaints ÷ delivered — 0 unless a [complaint webhook](../guide/deliverability#wiring-the-complaint-webhook) is feeding you FBL data. | +| `acceptance_pct` | Sent ÷ attempted (`sent + failed + bounced`). | +| `bounce_rate_pct` | Bounced ÷ attempted. | +| `open_rate_pct` / `click_rate_pct` | Opened / clicked ÷ sent. | +| `complaint_rate_pct` | Complaints ÷ sent — 0 unless a [complaint webhook](../guide/deliverability#wiring-the-complaint-webhook) is feeding you FBL data. | ### Errors | Status | Cause | |---|---| +| `400` | Domain health only: the project has no from address, so there is no sender domain to resolve. | | `401` | Missing cookie session. | | `402` | No valid Pro license. | -| `403` | Authenticated user doesn't own this project. | -| `404` | Project not found. | +| `403` | The authenticated user isn't a member of the project's workspace (also returned for a project that doesn't exist). | +| `404` | The project id in the path isn't a valid UUID. | ## See also diff --git a/docs/api/reports.md b/docs/api/reports.md index c299eac..23b1e1c 100644 --- a/docs/api/reports.md +++ b/docs/api/reports.md @@ -2,7 +2,7 @@ The engine behind the [Reports](../guide/reports) builder: a catalog of what you can query, a runner that executes one report configuration, and CRUD for saved reports. Pro-gated: without a valid Pro license these endpoints return `402 Payment Required`. -Cookie auth only, project-owner scoped. +Cookie auth only, scoped to workspace membership: any member of the project's workspace can call these endpoints, and no workspace role is checked. ## Schema @@ -36,32 +36,39 @@ Executes a configuration without saving it (this is what the live preview calls) | Field | Notes | |---|---| | `dataset` | `subscribers` or `emails`. | -| `measure` | `count`, or for `emails` a rate: `open_rate`, `click_rate`, `bounce_rate`, `spam_rate`. | +| `measure` | `count` on either dataset. `emails` also takes the counts `sent`, `opened`, `clicked`, `bounced`, `complained` and the rates `open_rate`, `click_rate`, `bounce_rate`, `spam_rate`, `acceptance`. | | `dimensions` | One or two. Two produces a pivot. Allowed keys depend on the dataset — see the schema response. A `custom.` dimension is validated against `^[a-zA-Z0-9_]+$`. | | `filter` | Optional [segment predicate](./segments#predicate-shape). | | `window` | Optional date range + `day` / `week` / `month` granularity for time dimensions. | -| `viz` | `table`, `pivot`, `bar`, `line`, `area`, `donut`, `pie`. | +| `viz` | `table`, `pivot`, `bar`, `line`, `area`, `donut`, `pie`. Accepted but not read: the response shape depends only on how many dimensions you pass. | ### Response -One dimension returns a flat breakdown; two return a pivot: +One dimension returns a flat breakdown; two return a pivot. Both echo the configuration that ran: ```json { - "dimension": "provider", + "dataset": "emails", + "measure": "open_rate", + "dimensions": ["provider"], "rows": [ { "label": "Gmail", "value": 45.9 }, { "label": "Outlook", "value": 38.2 } ] } ``` ```json { - "row_dim": "provider", - "col_dim": "send_time", - "cols": ["2026-W22", "2026-W23"], - "rows": [ { "label": "Gmail", "cells": { "2026-W22": 44.1, "2026-W23": 47.0 } } ] + "dataset": "emails", + "measure": "open_rate", + "dimensions": ["provider", "send_time"], + "pivot": { + "columns": ["2026-W22", "2026-W23"], + "rows": [ { "label": "Gmail", "cells": { "2026-W22": 44.1, "2026-W23": 47.0 } } ] + } } ``` +`rows` is present on a flat report, `pivot` on a two-dimension one. A pivot cell is `0` for a combination with no rows. + ## Saved reports ``` @@ -80,8 +87,8 @@ A saved report is a `name` plus the same configuration object accepted by `/run` | `400` | Invalid configuration (unknown dimension/measure, wrong dimension count, bad custom-field key). | | `401` | Missing cookie session. | | `402` | No valid Pro license. | -| `403` | Authenticated user doesn't own this project. | -| `404` | Project or report not found. | +| `403` | The authenticated user isn't a member of the project's workspace (also returned for a project that doesn't exist). | +| `404` | Report not found, or the project id in the path isn't a valid UUID. | ## See also From 5914612b8f257408e90f9703185ecd63abe23fcf Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Sun, 20 Sep 2026 18:08:52 -0500 Subject: [PATCH 21/27] docs(api): correct the bounces and analytics references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bounce pages documented two response fields that no handler sends and a timestamp that does not exist, and they showed the bounce webhook answering with a full URL when it returns a relative path you are meant to join to your instance. One of the two 401 bodies was missing and the 400 for a malformed project id was not documented at all, so whoever hit it had nothing to look up. The analytics page called the window metrics `deliverability_pct` when the field is `acceptance_pct`, said the series fields can be `null` when they are always arrays, claimed the client cannot choose the bucket granularity when it can, and described neither `range_days` nor the fact that `from` and `to` are optional — they default to the last thirty days, and the page had been calling them required. --- docs/api/analytics.md | 14 ++++++++------ docs/api/bounces.md | 23 +++++++++++++---------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/docs/api/analytics.md b/docs/api/analytics.md index 34a0df2..df869b1 100644 --- a/docs/api/analytics.md +++ b/docs/api/analytics.md @@ -12,12 +12,14 @@ GET /api/v1/projects/{id}/analytics/overview?from=...&to=...&segment_id=...&news | Query | Required | Format | Description | |---|---|---|---| -| `from` | yes | RFC 3339 | Start of the window (UTC). | -| `to` | yes | RFC 3339 | End of the window (UTC). | +| `from` | no | RFC 3339 | Start of the window (UTC). Defaults to 30 days before the current time. | +| `to` | no | RFC 3339 | End of the window (UTC). Defaults to the current time. | +| `granularity` | no | string | Bucket size for `opens_series` and `clicks_series` — one of `hour`, `day`, `week`, `month`, `auto`. Defaults to `auto`, which picks one from the range length. Any other value returns `400`. | +| `range_days` | no | int | Convenience alternative to `from`: start the window this many days before the current time. Values from `1` to `3650` are accepted; anything else is ignored. `from` wins when both are set. | | `segment_id` | no | UUID | Restrict every metric to subscribers matching this [segment](/api/segments). Omit for all subscribers. Returns `404` if the segment doesn't exist in the project. | | `newsletter_id` | no | UUID | Restrict every metric to sends attributed to this [newsletter](/api/newsletters). Combinable with `segment_id`. A malformed UUID returns `400`. | -The bucket granularity (`hour` / `day` / `week` / `month`) is decided server-side from the range length — clients don't pick it. The `previous` block in the response covers the same-length window immediately before `from`, used for trend comparisons. When `segment_id` is set it is echoed back in the response and applied to every metric except *broadcasts in flight* (which is not per-subscriber). `newsletter_id` is echoed back the same way; only emails sent after the newsletter feature shipped carry the attribution, so older sends fall outside any newsletter scope. The campaign-stats and engagement endpoints backing the dashboard's other tabs accept the same `newsletter_id` parameter. +The bucket granularity is `granularity` when the client sends one, and otherwise derived from the range length. The `previous` block in the response covers the same-length window immediately before `from`, used for trend comparisons. When `segment_id` is set it is echoed back in the response and applied to every metric except *broadcasts in flight* (which is not per-subscriber). `newsletter_id` is echoed back the same way; only emails sent after the newsletter feature shipped carry the attribution, so older sends fall outside any newsletter scope. The campaign-stats and engagement endpoints backing the dashboard's other tabs accept the same `newsletter_id` parameter. **Response** @@ -33,7 +35,7 @@ The bucket granularity (`hour` / `day` / `week` / `month`) is decided server-sid "total_opened": 980, "total_clicked": 142, - "deliverability_pct": 98.7, + "acceptance_pct": 98.7, "open_rate_pct": 65.3, "click_rate_pct": 9.5, "click_to_open_pct": 14.5, @@ -62,7 +64,7 @@ The bucket granularity (`hour` / `day` / `week` / `month`) is decided server-sid "total_failed": 18, "total_opened": 905, "total_clicked": 130, - "deliverability_pct": 98.7, + "acceptance_pct": 98.7, "open_rate_pct": 64.2, "click_rate_pct": 9.2 } @@ -72,7 +74,7 @@ The bucket granularity (`hour` / `day` / `week` / `month`) is decided server-sid ### Field notes - All `total_*` and `*_pct` values cover the requested window. -- `opens_series`, `top_templates`, `top_clicked_links` and `sends_by_status` may be `null` instead of an empty array when there is no data — handle both. +- `opens_series`, `top_templates`, `top_clicked_links` and `sends_by_status` are always arrays — `[]` when the window has no data, never `null`. - `click_to_open_pct` is `clicks / opens × 100` (engagement among readers), not a portion of total sends. - `active_subscribers` is a project-wide snapshot (current count of `status=active`), not windowed. - `previous` mirrors the headline metrics for the equivalent window immediately before `from`. If there is no data in that previous window, the values are `0`. diff --git a/docs/api/bounces.md b/docs/api/bounces.md index adb2ff7..f27ec68 100644 --- a/docs/api/bounces.md +++ b/docs/api/bounces.md @@ -28,12 +28,11 @@ GET /api/v1/projects/{id}/bounce-imap "port": 993, "user": "bounces@acme.com", "folder": "INBOX", - "last_polled_at": "2026-05-06T15:30:00Z", - "last_error": "" + "password_set": true } ``` -The password is never returned. `last_polled_at` and `last_error` reflect the most recent poller run. +The password is never returned; `password_set` reports whether one is stored for this mailbox. ## Update bounce IMAP config @@ -83,12 +82,13 @@ GET /api/v1/projects/{id}/bounce-webhook ```json { - "url": "https://your-host.example.com/webhooks/bounces/01H...?token=01H...", - "rotated_at": "2026-04-30T14:22:11Z" + "project_id": "uuid", + "bounce_token": "uuid", + "path": "/webhooks/bounces/uuid?token=uuid" } ``` -`url` is the full ingest URL with the current token, ready to paste into your provider's webhook settings. The token is part of the URL — there is no separate field. +`path` is the ingest path with the current token, relative to your SendDock origin — prepend your instance URL before pasting it into your provider's webhook settings. `bounce_token` is the token on its own. ## Rotate the bounce webhook token @@ -96,14 +96,15 @@ GET /api/v1/projects/{id}/bounce-webhook POST /api/v1/projects/{id}/bounce-webhook/rotate ``` -Generates a new token and invalidates the old one. The `url` returned by `GET /bounce-webhook` after this call carries the new token. +Generates a new token and invalidates the old one. The `path` returned by `GET /bounce-webhook` after this call carries the new token. ### Response ```json { - "url": "https://your-host.example.com/webhooks/bounces/01H...?token=NEW...", - "rotated_at": "2026-05-06T16:00:00Z" + "project_id": "uuid", + "bounce_token": "new-uuid", + "path": "/webhooks/bounces/uuid?token=new-uuid" } ``` @@ -156,8 +157,10 @@ Send the verbatim payload Mailgun POSTs for `permanent_failure` (or `failed`) ev | Status | Body | Meaning | |---|---|---| | `200` | `{"status":"accepted","email":""}` | Recipient extracted and added to the suppression list. | +| `400` | `{"error":"invalid project id"}` | The `projectId` in the path is not a UUID. | | `400` | `{"error":"could not find email in payload"}` | The body matched neither the generic nor the Mailgun shape. | -| `401` | `{"error":"missing or invalid token"}` | URL token doesn't match this project. | +| `401` | `{"error":"missing or invalid token"}` | The `token` query parameter is missing or not a UUID. | +| `401` | `{"error":"invalid project or token"}` | Token doesn't match this project. | | `413` | `{"error":"payload too large"}` | Body > 64 KiB. | The endpoint is **idempotent at the suppression layer** — re-posting the same recipient is a no-op (already on the list). The rate limit is the global per-IP cap (600 req/min behind a proxy that sets `X-Forwarded-For`). From 2b2063314e22721d1210d373f3e3996fdcd1f1cb Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Mon, 21 Sep 2026 21:24:31 -0500 Subject: [PATCH 22/27] docs(guide): correct the webhooks and onboarding pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The webhook payload example told the reader `status` is omitted on `subscriber.unsubscribed`, which it is not: that event carries it, and the two events that genuinely do not are the unsubscribe performed from a link and the newsletter-specific one. A reader filtering on the field would have written the wrong branch. "What is SendDock" promised per-project API keys for everything the dashboard does, when five routes accept them — send, batch, broadcast, stats and import — and the other eighty-eight are cookie-only. That claim is what makes someone paste a key into a call that can never authenticate. The onboarding guide described a column-mapping step in the CSV importer that does not exist; the importer matches extra columns to custom fields by header. --- docs/guide/getting-started.md | 2 +- docs/guide/webhooks.md | 2 +- docs/guide/what-is-senddock.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/guide/getting-started.md b/docs/guide/getting-started.md index 6f3f7aa..57105d0 100644 --- a/docs/guide/getting-started.md +++ b/docs/guide/getting-started.md @@ -41,7 +41,7 @@ In the project sidebar, open **SMTP Settings**, fill in host, port, username and Open **Subscribers**. Add people the way that fits you: - **+ Add Subscriber** — enter an email (and name, tags, custom fields) by hand. -- **Import CSV** — bulk-import a list, mapping extra columns to [custom fields](./subscribers#custom-fields). +- **Import CSV** — bulk-import a list; extra columns are matched to [custom fields](./subscribers#custom-fields) by their header. ![The subscribers table with tags and custom fields](/screenshots/subscribers-fields-tags.png) diff --git a/docs/guide/webhooks.md b/docs/guide/webhooks.md index fa9241d..0702fea 100644 --- a/docs/guide/webhooks.md +++ b/docs/guide/webhooks.md @@ -106,7 +106,7 @@ Every delivery is an HTTP `POST` with a JSON body shaped like an envelope: "project_id": "uuid", "email": "user@example.com", "name": "John Doe", - "status": "active" // omitted on subscriber.unsubscribed + "status": "active" } // subscriber.newsletter_unsubscribed diff --git a/docs/guide/what-is-senddock.md b/docs/guide/what-is-senddock.md index 52d8c90..edf5ac6 100644 --- a/docs/guide/what-is-senddock.md +++ b/docs/guide/what-is-senddock.md @@ -7,7 +7,7 @@ SendDock is an open-source, self-hosted email marketing and transactional platfo ## The model - **Open core, AGPL-3.0.** The Community edition is fully usable without a license. -- **API-first.** Every dashboard action has a REST endpoint. Cookie auth for the UI, per-project API keys (`Authorization: Bearer sk_...`) for everything else. +- **API-first.** Every dashboard action has a REST endpoint. Cookie auth for the UI, per-project API keys (`Authorization: Bearer sk_...`) for the send, batch, broadcast, stats and import endpoints. - **Single-binary Go backend + Vue dashboard, deployed by Docker Compose.** One container for the app, Postgres for storage, Redis for rate limits. - **Pro and Team are tier flags on the same binary.** The binary contains a license validator that checks your activated key against [Lemon Squeezy](https://lemonsqueezy.com) — the vendor that issues Pro/Team keys — at startup and periodically, unlocking only the endpoints your key entitles you to. No separate build. From 672ec69dc5f695de070d7b8345b5c5deb964381f Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Wed, 23 Sep 2026 19:52:46 -0500 Subject: [PATCH 23/27] docs(guide): correct the account, audit log and environment pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The account page described the 2FA exchange with field names the API does not send, said changing your password leaves other sessions alive when it revokes every refresh token, and named a field the license status does not return. It also presented a client-side confirmation as something the API checks. The audit log page claimed entries are only recorded on a Pro plan, when the license gates reading and never writing, listed a fraction of the recorded actions, and promised login-attempt history that does not exist — the auth layer keeps a short-lived lockout counter and nothing else. It also promised a workspace trail visible in project logs and a filter by actor, neither of which the shipped build has. The environment page left `/metrics` out of the rate-limiter exemption, documented `SENDDOCK_LICENSE_ENDPOINT` as if it did something on official images where the build tag that reads it is absent, and said `DISPOSABLE_DOMAINS_FILE` replaces the built-in list when it extends it. --- docs/guide/account.md | 20 ++++++++++---------- docs/guide/audit-log.md | 14 ++++++++++---- docs/guide/environment.md | 6 +++--- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/docs/guide/account.md b/docs/guide/account.md index cfd2b55..c063cca 100644 --- a/docs/guide/account.md +++ b/docs/guide/account.md @@ -30,7 +30,7 @@ After enabling, every subsequent login becomes a two-step flow: password first, #### Disable 2FA -Click **Disable** on the Account page. You must enter a valid TOTP code **or** a recovery code to confirm — there is no admin override and no "I lost my phone" shortcut. This is intentional: if it were easier to disable, the protection would be worthless. +Click **Disable** on the Account page. You must enter your current password **and** a valid TOTP code (or a recovery code) to confirm — there is no admin override and no "I lost my phone" shortcut. This is intentional: if it were easier to disable, the protection would be worthless. #### Regenerate recovery codes @@ -65,7 +65,7 @@ Three-field form: current password, new password, confirm. The new password is c - at least one digit - at least one special character -The current password is verified server-side with bcrypt before the change applies. Changing the password does **not** sign out existing sessions on other devices — clear them by deleting the session cookies in those browsers, or wait for the JWT to expire. +The current password is verified server-side with bcrypt before the change applies. Changing the password revokes every refresh token issued to the account, so other sessions can't renew once their access token expires (15 minutes) — in practice you sign everyone out, including the browser you changed it from. ## Billing page (`/billing`) @@ -74,7 +74,7 @@ Open the avatar menu → **Billing**. What you see depends on whether you're sel ### Self-hosted - **Current plan** — Free, Pro or Team, from the license you've activated. -- When a license is active: `expires_at` (when the subscription renews or lapses) and `last_check` (when the validator last reached Lemon Squeezy). The validator caches the last-good response for 24 h, so a brief network outage won't lock you out. +- When a license is active: `expires_at` (when the subscription renews or lapses) and `checked_at` (when the validator last reached Lemon Squeezy). The validator caches the last-good response for 24 h, so a brief network outage won't lock you out. - When no key is set: paywall cards for Pro ($9/mo) and Team ($29/mo) linking directly to Lemon Squeezy checkout. - A note clarifying that SendDock charges **only** for features. BYO SMTP means you pay your SMTP provider for delivery — never SendDock. @@ -92,23 +92,23 @@ Both paths skip the per-send fees other tools charge — pricing is by self-host ``` POST /api/v1/auth/login { email, password } -→ 200 { needs_2fa: true, intermediate_token } -POST /api/v1/auth/2fa { intermediate_token, code } # TOTP or recovery code +→ 200 { requires_2fa: true, two_factor_token } +POST /api/v1/auth/2fa { two_factor_token, code } # TOTP or recovery code → 200 + session cookie ``` -The intermediate token is short-lived (a few minutes) and only valid for completing the 2FA step. If you're integrating against the API rather than the dashboard, expect this two-step response shape whenever the target account has 2FA on. +The two-factor token is short-lived (a few minutes) and only valid for completing the 2FA step. If you're integrating against the API rather than the dashboard, expect this two-step response shape whenever the target account has 2FA on. ## Related endpoints | Endpoint | Purpose | |---|---| -| `POST /api/v1/me/password` | Change password (current + new + confirm). | +| `POST /api/v1/me/password` | Change password (`current_password` + `new_password`; the dashboard's confirm field is checked in the form, not by the API). | | `POST /api/v1/me/2fa/setup` | Generate TOTP secret + recovery codes. Returns `otpauth_url`, base32 secret, ten recovery codes. | | `POST /api/v1/me/2fa/verify` | Confirm setup with a 6-digit code. Enables 2FA. | -| `POST /api/v1/me/2fa/disable` | Disable 2FA. Requires a TOTP code or a recovery code. | +| `POST /api/v1/me/2fa/disable` | Disable 2FA. Requires `password` plus a TOTP code or a recovery code. | | `POST /api/v1/me/2fa/recovery-codes` | Regenerate the recovery code set. Requires a TOTP code. | -| `POST /api/v1/auth/2fa` | Login second step. Trades `intermediate_token` + code for a session. | -| `GET /api/v1/me` | Current user (`user_id`, `email`, `name`, `plan`, `created_at`). | +| `POST /api/v1/auth/2fa` | Login second step. Trades `two_factor_token` + code for a session. | +| `GET /api/v1/me` | Current user (`user_id`, `email`, `name`, `plan`, `totp_enabled`, `created_at`). | See also [Security Checklist](/self-hosting/configuration#security-checklist) for the broader hardening pass before you expose SendDock to the internet. diff --git a/docs/guide/audit-log.md b/docs/guide/audit-log.md index 66b0d09..c883755 100644 --- a/docs/guide/audit-log.md +++ b/docs/guide/audit-log.md @@ -1,6 +1,6 @@ # Audit log -The audit log records every sensitive action taken on a project (and, on Team plans, on the workspace itself — workspace and member changes are recorded with a `workspace` target) — who did it, when, against what, and from which IP. Available on Pro and Team. Without a Pro license the endpoint and UI tab are hidden, and the underlying actions still happen, they just aren't recorded. +The audit log records every sensitive action taken on a project (and, on Team plans, on the workspace itself — workspace and member changes are recorded with a `workspace` target) — who did it, when, against what, and from which IP. Available on Pro and Team. Entries are written on every plan: the license gates **reading** the log, not recording it. Without a Pro license the UI tab is hidden and the endpoint answers `402 Payment Required`, and the actions are still recorded. ![The audit log with filterable actions](/screenshots/audit-log.png) @@ -9,10 +9,16 @@ The audit log records every sensitive action taken on a project (and, on Team pl | Category | Actions | |---|---| | Project lifecycle | `project.create`, `project.update`, `project.delete` | +| Unsubscribe page | `project.unsubscribe_template_updated` | +| Instance settings | `instance.settings.updated` | | SMTP & deliverability config | `smtp.update`, `bounce_imap.update`, `bounce_token.rotate` | | API keys | `api_key.create`, `api_key.revoke` | | Webhooks | `webhook.create`, `webhook.delete` | | Suppressions | `suppression.add`, `suppression.delete` | +| Subscribers | `subscriber.newsletters_updated`, `subscriber.bulk_delete`, `subscriber.bulk_update_status`, `subscriber.bulk_add_tags`, `subscriber.bulk_remove_tags`, `subscriber.bulk_add_newsletter`, `subscriber.bulk_remove_newsletter` | +| Custom fields | `field.created`, `field.updated`, `field.deleted` | +| Segments | `segment.created`, `segment.updated`, `segment.deleted` | +| Newsletters | `newsletter.created`, `newsletter.updated`, `newsletter.deleted` | | Workspace (Team) | `workspace.create`, `workspace.rename`, `workspace.delete`, `workspace.member_added`, `workspace.member_removed`, `workspace.member_role_changed`, `workspace.user_created` | | Sends | `broadcast.send` | @@ -21,13 +27,13 @@ Each entry stores: - **`created_at`** — UTC, ISO 8601. - **`user_id`** — UUID of the user who took the action. Resolve to email via [`GET /workspaces/{id}/members`](../api/workspaces#list-members) when displaying. - **`action`** — the string from the table above. -- **`target_type`** + **`target_id`** — the entity affected (`project`, `api_key`, `webhook`, `suppression`, `workspace`). +- **`target_type`** + **`target_id`** — the entity affected (`project`, `api_key`, `webhook`, `suppression`, `workspace`, `field`, `subscriber`, `newsletter`, `segment`, `template`, `instance`). - **`metadata`** — action-specific JSON (e.g. for `smtp.update` you get `smtp_host`, `smtp_user`, `from_email` — never the password). - **`ip_address`** + **`user_agent`** — request context, IP parsed from `X-Forwarded-For` if present. ## Where to see it -In the project sidebar, open **Audit Log**. Filter by action, by actor, or by date range. Each entry expands to show the metadata JSON. Workspace-level entries (Team) appear alongside project entries in the workspace's project logs. +In the project sidebar, open **Audit Log**. Filter by action or by date range. Each entry expands to show the metadata JSON. Workspace-level entries (Team) are recorded with a `workspace` target. ## API @@ -78,7 +84,7 @@ The audit log is for **administrative** actions, not data-plane traffic. It does - Individual `/send` calls (those are in the email log — millions of rows). - Open or click events (those are in the analytics tables). - Read operations (`GET /subscribers`, etc.). -- Login attempts (logged separately at the auth layer; surfaced in v0.7+). +- Login attempts — the auth layer keeps only a short-lived Redis failure counter (10 failures per identifier in a 15-minute window) to lock out brute force. There is no login history stored anywhere and nothing to query. If you need a full request log, use your reverse proxy's access log — Nginx, Caddy and Traefik all log per-request and are the right tool for that. The audit log is the curated subset of events worth showing in a "Who changed our SMTP password?" investigation. diff --git a/docs/guide/environment.md b/docs/guide/environment.md index cfb021f..b905fab 100644 --- a/docs/guide/environment.md +++ b/docs/guide/environment.md @@ -19,7 +19,7 @@ All configuration is done via environment variables. For self-hosting deployment | `PUBLIC_URL` | On self-hosted this is **deprecated** — set it from the dashboard under Instance instead; a value left here is imported on first boot and stops being read in v0.9. On the hosted product it stays the source of truth, since there is no Instance screen there. | _stored in the database_ | | `CLOUD` | Switches the binary to cloud mode. Truthy values `true` / `1` / `yes` (case-insensitive) select cloud; anything else — including unset — stays self-hosted. The legacy `DEPLOYMENT_MODE` is still read until v0.9 but only ever accepted one value (`cloud`); it's checked only when `CLOUD` isn't truthy, so a truthy `CLOUD` always wins. | _unset_ | | `SENDDOCK_LICENSE_KEY` | Pro / Team license key. On self-hosted this is **deprecated** — activate it from the dashboard under **Instance → License** instead ([Instance settings](/guide/instance-settings#pro-license)); a value left here is imported once on first boot and stops being read in v0.9. On the hosted product the environment stays the source of truth. Empty leaves the deployment on the free tier (Core only). | — | -| `RATE_LIMIT_PER_MINUTE` | Per-IP request cap for the **global** rate limiter (rolling 60s fixed window, applied to every HTTP endpoint except `/health`). Independent from the hard-coded per-project sending limits on `/send`, `/send/batch`, `/broadcast` (those are not configurable). Lower this on small deployments behind a single egress IP; raise it for high-traffic apps. Only enforced when `REDIS_URL` is set. | `600` | +| `RATE_LIMIT_PER_MINUTE` | Per-IP request cap for the **global** rate limiter (rolling 60s fixed window, applied to every HTTP endpoint except `/health` and `/metrics`). Independent from the hard-coded per-project sending limits on `/send`, `/send/batch`, `/broadcast` (those are not configurable). Lower this on small deployments behind a single egress IP; raise it for high-traffic apps. Only enforced when `REDIS_URL` is set. | `600` | | `SENDDOCK_WATCHTOWER_URL` | URL of the [Watchtower](https://containrrr.dev/watchtower/) HTTP API (typically `http://watchtower:8080` on the Docker network). When set and reachable, the dashboard's update modal shows an "Update now" button that triggers a Watchtower scan + image refresh. When unset, the modal falls back to the manual `docker compose pull && up -d` command. See [Updating → One-click updates](/self-hosting/updating#one-click-updates-from-the-dashboard-watchtower). | — | | `SENDDOCK_WATCHTOWER_TOKEN` | Bearer token expected by Watchtower's HTTP API (set as `WATCHTOWER_HTTP_API_TOKEN` on the Watchtower container). Required when `SENDDOCK_WATCHTOWER_URL` is set. | — | | `TEMPLATE_LIBRARY_URL` | Manifest URL for the starter library shown by the **★ Browse library** modal on the Templates page. Override to point at a private fork or a curated internal gallery — the schema is documented in the [public repo](https://github.com/Arkhe-Systems/senddock-templates). The backend caches the manifest in Redis for one hour, so changes take up to an hour to propagate. Empty disables the library button. | `https://raw.githubusercontent.com/Arkhe-Systems/senddock-templates/main/index.json` | @@ -39,9 +39,9 @@ You almost never need these — they're escape hatches for non-default deploymen | Variable | Description | Default | |----------|-------------|---------| -| `SENDDOCK_LICENSE_ENDPOINT` | Override the Lemon Squeezy API root the license validator calls — for a self-hosted Lemon Squeezy instance or a proxy in front of the licensing API. Test mode uses the same URL but a different key, so you do **not** need to set this for test mode. | `https://api.lemonsqueezy.com/v1` | +| `SENDDOCK_LICENSE_ENDPOINT` | Override the Lemon Squeezy API root the license validator calls — for a self-hosted Lemon Squeezy instance or a proxy in front of the licensing API. Only read by binaries built with the `licensedev` build tag, so it has **no effect on official images** or on any other build. Test mode uses the same URL but a different key, so you do **not** need to set this for test mode. | `https://api.lemonsqueezy.com/v1` | | `FRONTEND_DIST_PATH` | Filesystem path to the built frontend SPA (`index.html` and assets). The official Docker image sets this to `/app/frontend/dist`. Override only if you serve the SPA from a custom location. | `./frontend/dist` | -| `DISPOSABLE_DOMAINS_FILE` | Path to a newline-separated list of disposable email domains used by the import validator. Built-in list ships with the binary; setting this replaces it (does not extend). | _built-in_ | +| `DISPOSABLE_DOMAINS_FILE` | Path to a newline-separated list of disposable email domains used by the import validator. Built-in list ships with the binary; domains in the file are **added** to it (it does not replace the built-in list). | _built-in_ | ::: tip Where is the public URL now? Outgoing emails contain links like the unsubscribe URL and the open-tracking pixel. SendDock cannot guess what URL recipients will see — it has to be told. From 39c536bfdbf48fdec620a635b0493b58b6e26c4c Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Wed, 23 Sep 2026 23:16:19 -0500 Subject: [PATCH 24/27] docs(api): fix examples that could not work as written MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five of the seven language examples authenticated the subscriber listing with an API key, and that endpoint is cookie-only: the snippet returned 401 for anyone who copied it, in every language. They now send the session cookie, which in C# also means leaving the JSON helper behind, since it cannot attach one. Elsewhere the page crossed up raw-body APIs: `request.body()` is not how Express exposes the bytes, FastAPI's version is async, and ASP.NET Core has no `InputStream` — that was removed with the classic framework. It also omitted `/stats` from the endpoints that accept an API key and pointed at `GET /broadcasts` for tallies that an API key cannot read, and one error row sat outside the table where Markdown renders it as plain text. --- docs/api/code-examples.md | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/docs/api/code-examples.md b/docs/api/code-examples.md index 5471931..5a590cc 100644 --- a/docs/api/code-examples.md +++ b/docs/api/code-examples.md @@ -14,7 +14,7 @@ Every example uses three placeholders — replace them once and the rest works: | `YOUR_API_KEY` | A project-scoped API key (`sk_…`). Create one in **Settings → API Keys**, see the [API Keys API](./api-keys). | | `YOUR_PROJECT_ID` | The project UUID. It's the segment after `/projects/` in the dashboard URL. | -The API root is always `${YOUR_BASE_URL}/api/v1`. Authentication is `Authorization: Bearer ${YOUR_API_KEY}` for every endpoint that supports API keys (sending, broadcast, batch, subscribers/import). Cookie-only endpoints are noted on each reference page — full list at [Authentication → Endpoints that accept API keys](./authentication#endpoints-that-accept-api-keys). +The API root is always `${YOUR_BASE_URL}/api/v1`. Authentication is `Authorization: Bearer ${YOUR_API_KEY}` for every endpoint that supports API keys (sending, broadcast, batch, subscribers/import, stats). Cookie-only endpoints are noted on each reference page — full list at [Authentication → Endpoints that accept API keys](./authentication#endpoints-that-accept-api-keys). All request and response bodies are `application/json` unless stated otherwise. @@ -348,7 +348,7 @@ curl_setopt($ch, CURLOPT_POSTFIELDS, json_encode(["template_id" => "YOUR_TEMPLAT ::: -Returns `{"sent": N, "broadcast_id": "uuid"}` once the send has been enqueued for every active subscriber — `sent` is the number enqueued, not yet delivered; poll `GET /broadcasts` for the final sent/failed/suppressed tallies as the queue drains. Broadcast is rate-limited to **5 calls per hour per project** to prevent runaway sends. +Returns `{"sent": N, "broadcast_id": "uuid"}` once the send has been enqueued for every active subscriber — `sent` is the number enqueued, not yet delivered. The `failed` and `suppressed` fields of the same object are omitted here because the enqueue targets active subscribers only. `broadcast_id` is the handle on the broadcast after that: the dashboard's **Broadcasts** tab (`GET /projects/{id}/broadcasts`) reads its `sent_count`, `failed_count` and `suppressed_count` as the worker drains the queue — that endpoint is cookie-only, so it is not callable with an API key. Broadcast is rate-limited to **5 calls per hour per project** to prevent runaway sends. ## Add a subscriber @@ -432,7 +432,7 @@ Returns `200` with `{imported, duplicates, syntax_invalid, no_mx, disposable, su ## List subscribers (paginated) ::: warning Cookie auth only -`GET /subscribers` is cookie-only — a project API key cannot list subscribers (see [Authentication → Endpoints that accept API keys](./authentication#endpoints-that-accept-api-keys)). Log in first, then send the session cookie instead of `Authorization: Bearer`. The cURL example uses `-b cookies.txt`; in the other languages, drop the `Authorization` header and attach the session cookie you received at login. +`GET /subscribers` is cookie-only — a project API key cannot list subscribers (see [Authentication → Endpoints that accept API keys](./authentication#endpoints-that-accept-api-keys)). Log in first, then send the session cookie instead of `Authorization: Bearer`. The cURL example uses `-b cookies.txt`; the other examples send the `access_token` cookie you received at login. ::: ::: code-group @@ -445,13 +445,15 @@ curl -G "$YOUR_BASE_URL/api/v1/projects/$YOUR_PROJECT_ID/subscribers" \ ``` ```js [JavaScript] +const accessToken = 'YOUR_ACCESS_TOKEN' + async function* paginate() { const limit = 100 let offset = 0 while (true) { const res = await fetch( `${YOUR_BASE_URL}/api/v1/projects/${YOUR_PROJECT_ID}/subscribers?limit=${limit}&offset=${offset}`, - { headers: { Authorization: `Bearer ${YOUR_API_KEY}` } } + { headers: { Cookie: `access_token=${accessToken}` } } ) const { subscribers, total } = await res.json() yield* subscribers @@ -466,12 +468,14 @@ for await (const sub of paginate()) { ``` ```python [Python] +ACCESS_TOKEN = "YOUR_ACCESS_TOKEN" + def all_subscribers(): limit, offset = 100, 0 while True: res = requests.get( f"{YOUR_BASE_URL}/api/v1/projects/{YOUR_PROJECT_ID}/subscribers", - headers={"Authorization": f"Bearer {YOUR_API_KEY}"}, + cookies={"access_token": ACCESS_TOKEN}, params={"limit": limit, "offset": offset}, ) res.raise_for_status() @@ -493,13 +497,14 @@ import com.fasterxml.jackson.databind.ObjectMapper; var mapper = new ObjectMapper(); var http = HttpClient.newHttpClient(); +String accessToken = "YOUR_ACCESS_TOKEN"; int limit = 100, offset = 0; while (true) { var req = HttpRequest.newBuilder() .uri(URI.create(YOUR_BASE_URL + "/api/v1/projects/" + YOUR_PROJECT_ID + "/subscribers?limit=" + limit + "&offset=" + offset)) - .header("Authorization", "Bearer " + YOUR_API_KEY) + .header("Cookie", "access_token=" + accessToken) .build(); var res = http.send(req, HttpResponse.BodyHandlers.ofString()); JsonNode page = mapper.readTree(res.body()); @@ -513,26 +518,35 @@ while (true) { ``` ```csharp [.NET / C#] +var accessToken = "YOUR_ACCESS_TOKEN"; int limit = 100, offset = 0; while (true) { - var page = await http.GetFromJsonAsync( + using var req = new HttpRequestMessage( + HttpMethod.Get, $"projects/{YOUR_PROJECT_ID}/subscribers?limit={limit}&offset={offset}" ); - var subs = page.GetProperty("subscribers"); + req.Headers.Add("Cookie", $"access_token={accessToken}"); + var res = await http.SendAsync(req); + res.EnsureSuccessStatusCode(); + using var page = JsonDocument.Parse(await res.Content.ReadAsStringAsync()); + var root = page.RootElement; + var subs = root.GetProperty("subscribers"); foreach (var sub in subs.EnumerateArray()) { Console.WriteLine(sub.GetProperty("email").GetString()); } offset += subs.GetArrayLength(); - if (offset >= page.GetProperty("total").GetInt32() || subs.GetArrayLength() == 0) break; + if (offset >= root.GetProperty("total").GetInt32() || subs.GetArrayLength() == 0) break; } ``` ```php [PHP] +$accessToken = "YOUR_ACCESS_TOKEN"; $limit = 100; $offset = 0; while (true) { curl_setopt($ch, CURLOPT_URL, "$YOUR_BASE_URL/api/v1/projects/$YOUR_PROJECT_ID/subscribers?limit=$limit&offset=$offset"); curl_setopt($ch, CURLOPT_HTTPGET, true); + curl_setopt($ch, CURLOPT_COOKIE, "access_token=$accessToken"); $page = json_decode(curl_exec($ch), true); foreach ($page["subscribers"] as $sub) { echo $sub["email"] . "\n"; @@ -668,7 +682,7 @@ function verify(string $rawBody, string $header, string $secret, int $maxSkew = ::: ::: warning Read the raw body, not the parsed JSON -Most frameworks parse JSON automatically and lose insignificant whitespace, which breaks the HMAC. Capture the bytes **before** any parser touches them — `request.body()` in Express, `request.body` in FastAPI's raw form, `Request.InputStream` in ASP.NET, `php://input` in PHP. +Most frameworks parse JSON automatically and lose insignificant whitespace, which breaks the HMAC. Capture the bytes **before** any parser touches them — `express.raw({ type: 'application/json' })` in Express, `await request.body()` in FastAPI, `Request.Body` (with `Request.EnableBuffering()` for repeated reads) in ASP.NET Core, `php://input` in PHP. ::: ## Errors @@ -684,9 +698,9 @@ All errors return JSON of the shape `{"error": "human-readable message"}` with t | `404` | Project / template / subscriber not found. | | `409` | Duplicate (e.g. subscriber email already exists). | | `429` | Rate limited. Honor `Retry-After` (in seconds). | +| `5xx` | Server error. Retry with backoff. | A suppressed recipient is **not** an error: `/send` answers `200` with `{"message": "suppressed", "suppressed": 1}`, and batch/broadcast count those recipients in the `suppressed` field. The send is logged with status `suppressed` and no SMTP attempt happens. -| `5xx` | Server error. Retry with backoff. | ## What's next From cbd8fc5fbeaaf6c344043cd58b0e78b5089be9bc Mon Sep 17 00:00:00 2001 From: JuansesDev Date: Thu, 24 Sep 2026 20:26:53 -0500 Subject: [PATCH 25/27] docs: correct the remaining guide and reference sentences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last of the documentation pass, covering the guide pages and the four reference sentences that sat in the API pages. The tracking pixel was documented without its `.gif` in two more places, including the diagram on the same page that had just been corrected — the pixel SendDock writes carries the suffix and the handler strips it. Page templates were said to run through a CSS inliner on render, which only happens on an email send; suppression and campaign pages named their capabilities with a colon where the code defines dots; the workspace reference described a password rule that omits two of the three requirements, and dropped the Team-plan gate that removing another member carries while keeping it for the owner guard. The index and the projects reference claimed API keys work for everything and that projects are returned to their owner, when five endpoints accept a key and the query joins workspace membership. --- docs/api/campaigns.md | 4 ++-- docs/api/newsletters.md | 2 +- docs/api/projects.md | 4 ++-- docs/api/segments.md | 2 +- docs/api/subscribers.md | 4 ++-- docs/api/suppressions.md | 8 ++++---- docs/api/webhooks.md | 2 +- docs/api/workspaces.md | 5 +++-- docs/guide/bounces.md | 4 ++-- docs/guide/campaigns.md | 2 +- docs/guide/sending.md | 4 ++-- docs/guide/suppressions.md | 2 +- docs/guide/templates.md | 2 +- docs/index.md | 2 +- docs/self-hosting/troubleshooting.md | 2 +- 15 files changed, 25 insertions(+), 24 deletions(-) diff --git a/docs/api/campaigns.md b/docs/api/campaigns.md index 7dc2c7d..2c5be5b 100644 --- a/docs/api/campaigns.md +++ b/docs/api/campaigns.md @@ -1,6 +1,6 @@ # Campaigns API -Cookie auth only. Campaigns mutate workspace state and require role-based capabilities (`campaigns:write` for create / update / delete) that an API key does not carry — the role is bound to the user identity. +Cookie auth only. Campaigns mutate workspace state and require role-based capabilities (`campaigns.write` for create / update / delete) that an API key does not carry — the role is bound to the user identity. A campaign is a **scheduled broadcast** — delivered by the same worker as `POST /broadcast`; use `/broadcast` for immediate sends. Unlike `/broadcast`, campaigns accept `newsletter_id` but have no `segment_id` field. @@ -99,7 +99,7 @@ Reschedules or replaces a `scheduled` campaign. Same body shape as Create: `name`, `template_id` and `scheduled_at` are required (full replacement, not partial update). `scheduled_at` must be RFC 3339. `variables` and `newsletter_id` are optional and replace the previous values — omitting `newsletter_id` reverts the campaign to all active subscribers. -Only campaigns in `scheduled` status can be patched — once a campaign moves to `sending`, `sent` or `failed`, it's immutable. Cookie auth only; the role must have `campaigns:write` (owners, admins and developers, not viewers). +Only campaigns in `scheduled` status can be patched — once a campaign moves to `sending`, `sent` or `failed`, it's immutable. Cookie auth only; the role must have `campaigns.write` (owners, admins and developers, not viewers). **Response** the updated campaign object. diff --git a/docs/api/newsletters.md b/docs/api/newsletters.md index d307f4c..357d3c1 100644 --- a/docs/api/newsletters.md +++ b/docs/api/newsletters.md @@ -1,6 +1,6 @@ # Newsletters API -Newsletters are named publications inside a project. Subscribers keep a single project-level status, and on top of that hold per-newsletter memberships they can join and leave individually — unsubscribing from one newsletter never touches the others, the project status, or the suppression list. Cookie auth required; mutations need the `subscribers:write` role capability. Part of the free Core. +Newsletters are named publications inside a project. Subscribers keep a single project-level status, and on top of that hold per-newsletter memberships they can join and leave individually — unsubscribing from one newsletter never touches the others, the project status, or the suppression list. Cookie auth required; mutations need the `subscribers.write` role capability. Part of the free Core. See the [Newsletters guide](/guide/newsletters) for concepts and the unsubscribe behavior. diff --git a/docs/api/projects.md b/docs/api/projects.md index d4902da..6a14789 100644 --- a/docs/api/projects.md +++ b/docs/api/projects.md @@ -44,7 +44,7 @@ POST /api/v1/projects GET /api/v1/projects ``` -Returns an array of projects owned by the authenticated user. +Returns an array of the projects in every workspace the authenticated user is a member of. ## Get Project @@ -107,6 +107,6 @@ PUT /api/v1/projects/{id}/unsubscribe-template {"template_id": "uuid"} ``` -Sets the [page template](/api/templates#template-types) rendered on the public unsubscribe pages (both the confirmation and the done page). The template must belong to the project and have `type: "page"` — anything else returns `400`. Send `{"template_id": ""}` to clear the setting and fall back to the built-in page. Requires the `project:settings` capability. Deleting the template clears the setting automatically. +Sets the [page template](/api/templates#template-types) rendered on the public unsubscribe pages (both the confirmation and the done page). The template must belong to the project and have `type: "page"` — anything else returns `400`. Send `{"template_id": ""}` to clear the setting and fall back to the built-in page. Requires the `project.settings` capability. Deleting the template clears the setting automatically. Invalid or tampered unsubscribe links always render the built-in error page, never the branded template. diff --git a/docs/api/segments.md b/docs/api/segments.md index 4ed51b8..563d27c 100644 --- a/docs/api/segments.md +++ b/docs/api/segments.md @@ -1,6 +1,6 @@ # Segments API -Segments are saved filters over your subscribers, evaluated fresh on every use (no materialized membership). A broadcast can target a segment instead of "all active subscribers". Cookie auth required; mutations need the `subscribers:write` role capability. Part of the free Core. +Segments are saved filters over your subscribers, evaluated fresh on every use (no materialized membership). A broadcast can target a segment instead of "all active subscribers". Cookie auth required; mutations need the `subscribers.write` role capability. Part of the free Core. See the [Segments guide](/guide/segments) for concepts and examples. diff --git a/docs/api/subscribers.md b/docs/api/subscribers.md index 548c3c4..3a023ee 100644 --- a/docs/api/subscribers.md +++ b/docs/api/subscribers.md @@ -1,6 +1,6 @@ # Subscribers API -All endpoints require cookie authentication, and access is granted by workspace membership rather than project ownership: reads need the caller to be a member of the project's workspace, writes need the `subscribers:write` capability (roles `owner` and `admin`). Two endpoints are exceptions — [Bulk Import](#bulk-import) also accepts an API key, and the [Waitlist](#waitlist-public) endpoint is public and takes no authentication. +All endpoints require cookie authentication, and access is granted by workspace membership rather than project ownership: reads need the caller to be a member of the project's workspace, writes need the `subscribers.write` capability (roles `owner` and `admin`). Two endpoints are exceptions — [Bulk Import](#bulk-import) also accepts an API key, and the [Waitlist](#waitlist-public) endpoint is public and takes no authentication. ## Add Subscriber @@ -160,7 +160,7 @@ Apply the same operation to many existing subscribers — the dashboard uses thi | `tags` | string[] | required for `add_tags` / `remove_tags` | Non-empty list of tags to add to or remove from every selected subscriber. | | `newsletter_id` | string | required for `add_newsletter` / `remove_newsletter` | The [newsletter](/api/newsletters) to add every selected subscriber to (clearing any opt-out) or remove them from. | -Cookie auth only (the role must have `subscribers:write`). For ingesting fresh rows, use [Bulk Import](#bulk-import) — that endpoint takes raw `email`/`name` rows and accepts API keys; this one operates on already-stored subscriber ids. +Cookie auth only (the role must have `subscribers.write`). For ingesting fresh rows, use [Bulk Import](#bulk-import) — that endpoint takes raw `email`/`name` rows and accepts API keys; this one operates on already-stored subscriber ids. **Response** `200 OK` with `{"message":"success"}`. Subscriber ids that don't belong to the project are silently skipped. diff --git a/docs/api/suppressions.md b/docs/api/suppressions.md index f895525..b134da1 100644 --- a/docs/api/suppressions.md +++ b/docs/api/suppressions.md @@ -2,7 +2,7 @@ Manage the per-project suppression list — addresses that `/send`, `/send/batch` and `/broadcast` will skip. See the [Suppressions guide](../guide/suppressions) for the conceptual model and how the list interacts with bounces and unsubscribes. -**Cookie auth only.** Suppression management requires the `suppressions:write` capability that an API key (project-scoped, identity-less) does not carry. Use the dashboard, or call from your own UI built on the same login flow. +**Cookie auth only.** Suppression management requires the `suppressions.write` capability that an API key (project-scoped, identity-less) does not carry. Use the dashboard, or call from your own UI built on the same login flow. The cURL examples below use `-b cookies.txt` to indicate the cookie jar from a prior `POST /api/v1/auth/login`. @@ -84,7 +84,7 @@ Add one or more addresses to the list. Already-suppressed entries are silently d ### Capability -Requires the `suppressions:write` capability. Of the workspace roles — `owner`, `admin`, `developer`, `viewer` — only `owner` and `admin` carry it; `developer` and `viewer` get `403`. API keys cannot call it at all (see the cookie-auth note at the top). +Requires the `suppressions.write` capability. Of the workspace roles — `owner`, `admin`, `developer`, `viewer` — only `owner` and `admin` carry it; `developer` and `viewer` get `403`. API keys cannot call it at all (see the cookie-auth note at the top). ### Example @@ -109,7 +109,7 @@ Removes the entry. The next send to that address will go through normally — us ### Capability -Requires `suppressions:write`. +Requires `suppressions.write`. ### Example @@ -124,7 +124,7 @@ curl -X DELETE "$YOUR_BASE_URL/api/v1/projects/$YOUR_PROJECT_ID/suppressions/01H |---|---| | `400` | `emails` array is empty / missing, invalid project id, invalid suppression id. | | `401` | Missing or invalid session cookie — these routes are cookie-only, so an API key never authenticates them. | -| `403` | The role doesn't have `suppressions:write`. | +| `403` | The role doesn't have `suppressions.write`. | | `404` | Project not found, or suppression id not part of this project. | ## Audit diff --git a/docs/api/webhooks.md b/docs/api/webhooks.md index ac87c8d..51acc69 100644 --- a/docs/api/webhooks.md +++ b/docs/api/webhooks.md @@ -169,7 +169,7 @@ The `attempts` counter increments on every retry; combined with the [retry sched | Code | When | |---|---| | `400` | Body fails validation — invalid URL, unknown event type, missing required field. | -| `403` | Authenticated user does not own this project, or the role lacks `webhooks:write`. | +| `403` | Authenticated user does not own this project, or the role lacks `webhooks.write`. | | `404` | Project or webhook not found. | | `500` | Server error. | diff --git a/docs/api/workspaces.md b/docs/api/workspaces.md index d378618..51019a1 100644 --- a/docs/api/workspaces.md +++ b/docs/api/workspaces.md @@ -154,7 +154,7 @@ Owner only. Creates a new SendDock account and adds them to the workspace at the |---|---|---| | `email` | yes | Must be unique. Returns `409 email already registered` otherwise. | | `name` | yes | Display name shown in member lists and audit log. | -| `password` | yes | Minimum 8 characters. Pass it to the user out of band; they can change it after first login. | +| `password` | yes | Minimum 8 characters, with at least one uppercase letter, one digit and one special character. Pass it to the user out of band; they can change it after first login. | | `role` | yes | Same enum as [Add member](#add-existing-member): `owner`, `admin`, `developer`, `viewer`. | **Response — 201 Created** @@ -188,7 +188,7 @@ Returns `409 Conflict` (`{"error":"cannot remove the last owner"}`) if the chang DELETE /api/v1/workspaces/{id}/members/{userId} ``` -Owner can remove any member. A member can also remove themselves (leave the workspace). Returns `204 No Content`. Returns `409 Conflict` if removing the user would leave the workspace without an owner. +Owner can remove any other member (Team plan — `402 Payment Required` without it). A member can also remove themselves (leave the workspace), which needs no plan. Returns `204 No Content`. Returns `409 Conflict` if removing the user would leave the workspace without an owner. The removed user's access to every project in the workspace is revoked immediately. @@ -198,6 +198,7 @@ The removed user's access to every project in the workspace is revoked immediate |---|---| | `400` | Body fails validation, unknown role. | | `401` | Missing / wrong session cookie. | +| `402` | Removing another member requires a Team plan. | | `403` | Action requires owner role, or caller is not a member of the workspace. | | `404` | Workspace, member or invited user not found. | | `409` | Last-owner guard, or workspace still has projects on delete. | diff --git a/docs/guide/bounces.md b/docs/guide/bounces.md index 4618867..0cbddb8 100644 --- a/docs/guide/bounces.md +++ b/docs/guide/bounces.md @@ -39,7 +39,7 @@ POST https:///webhooks/bounces/{project_id}?token= The endpoint accepts: -- **Generic JSON** — `{"recipient": "user@example.com", "reason": "..."}` (or an array of those). +- **Generic JSON** — `{"email": "user@example.com", "reason": "..."}`. One address per request; when `reason` is omitted the `type` field is used instead. - **Mailgun event payloads** — sent verbatim from a Mailgun event-data webhook with `event=permanent_failure` or `event=failed`. Configure the destination in your provider's UI to point to that URL. The bounce token is shown in the project's **Settings** page (Bounce webhook section) and can be rotated from the same screen. @@ -68,7 +68,7 @@ The poller never deletes messages — it only flags them as read. |---|---| | **Email logs** | The send row's status changes from `sent` to `bounced`. | | **Suppression list** | Bounced recipients appear with reason `bounce`. | -| **Project stats** | `bounced` and `suppressed` are tracked separately from `sent` and `failed`. The dashboard's outcome cards show all four. | +| **Project stats** | `bounced` and `suppressed` are tracked separately from `sent` and `failed` in the stats endpoint. The dashboard's Overview cards cover Total Emails, Sent and Failed. | | **Webhooks** | `email.bounced` fires once with `{ "log_id", "to_email", "subject", "smtp_code", "reason" }` in `data`. | | **Audit log (Pro)** | Bounce-mailbox config changes and bounce-token rotations are recorded. | diff --git a/docs/guide/campaigns.md b/docs/guide/campaigns.md index 5bed620..f36a992 100644 --- a/docs/guide/campaigns.md +++ b/docs/guide/campaigns.md @@ -40,7 +40,7 @@ From the **Campaigns** tab in your project, click **+ New Campaign**, then: Click **Create** and it appears in the list. The programmatic equivalent, including the `variables` shape, is in the [Campaigns API reference](/api/campaigns#create-campaign). ::: tip Cookie auth only -Campaigns mutate workspace state and require role-based capabilities (`campaigns:write`). Each dashboard role maps to a set of capabilities — see [Members & roles](./members#roles--capabilities) for the matrix; only roles holding `campaigns:write` can create or edit campaigns. API keys, which are project-scoped and identity-less, can't call these endpoints — you'll get `401`. Schedule from the dashboard, or call the endpoints from your own UI built on the same cookie-session login. +Campaigns mutate workspace state and require role-based capabilities (`campaigns.write`). Each dashboard role maps to a set of capabilities — see [Members & roles](./members#roles--capabilities) for the matrix; only roles holding `campaigns.write` can create or edit campaigns. API keys, which are project-scoped and identity-less, can't call these endpoints — you'll get `401`. Schedule from the dashboard, or call the endpoints from your own UI built on the same cookie-session login. ::: ## List Campaigns diff --git a/docs/guide/sending.md b/docs/guide/sending.md index 9cb6cb0..dd14180 100644 --- a/docs/guide/sending.md +++ b/docs/guide/sending.md @@ -204,7 +204,7 @@ See the [Campaigns guide](/guide/campaigns) for details on creating and managing 1rewrite <a href>, inject pixel 2deliver 3email lands in inbox - 4GET /t/{logId}→ opened_at = NOW() + 4GET /t/{logId}.gif→ opened_at = NOW() 5GET /c/{logId}/{...}→ clicked_at + email_clicks 6302 → original URL @@ -216,7 +216,7 @@ Tracking is on by default for every send. The two touch points — the open pixe SendDock automatically injects a 1x1 transparent tracking pixel into **every** outgoing email — subscriber sends, broadcasts, and transactional sends to a raw address (`/send`, `/send/batch`). When the recipient opens the email and their email client loads the pixel, SendDock records the open. -- The tracking pixel URL is `GET /t/{logId}` (public, no auth — the response body is the 1×1 GIF, no file extension on the path) +- The tracking pixel URL is `GET /t/{logId}.gif` — the exact URL SendDock writes into the email (public, no auth; the response body is the 1×1 GIF) - Only the first open is recorded (`opened_at` timestamp on the email log) - The stats endpoint includes the `opened` count alongside `sent` and `failed` diff --git a/docs/guide/suppressions.md b/docs/guide/suppressions.md index 8ded144..40d2f20 100644 --- a/docs/guide/suppressions.md +++ b/docs/guide/suppressions.md @@ -37,7 +37,7 @@ In the project's **Suppressions** tab you can: ## API -The suppression list is managed from the dashboard (Project → **Suppressions** tab) and from the same endpoints under **cookie auth** — the ordinary logged-in session, which carries the user's identity and role. These endpoints require the `suppressions:write` capability (a per-action permission granted through a dashboard user's role — see [Members & roles](./members#roles--capabilities)) and are not callable with a project-scoped API key, which carries no identity. +The suppression list is managed from the dashboard (Project → **Suppressions** tab) and from the same endpoints under **cookie auth** — the ordinary logged-in session, which carries the user's identity and role. These endpoints require the `suppressions.write` capability (a per-action permission granted through a dashboard user's role — see [Members & roles](./members#roles--capabilities)) and are not callable with a project-scoped API key, which carries no identity. The full request and response shape for `GET / POST / DELETE /api/v1/projects/{id}/suppressions` lives in the [Suppressions API reference](/api/suppressions). The bounce sources documented in [Bounces](./bounces) write to the same list automatically — you usually only call these endpoints by hand for one-off corrections (a typo'd address that bounced once but should still receive sends, an explicit blocklist you imported from another tool). diff --git a/docs/guide/templates.md b/docs/guide/templates.md index 95b10ee..48d8d90 100644 --- a/docs/guide/templates.md +++ b/docs/guide/templates.md @@ -128,7 +128,7 @@ Page templates support these placeholders: A few rules keep the page safe and predictable: - The HTML is sanitized before rendering: scripts, iframes, forms, and event-handler attributes are stripped. Placeholder values are escaped, so they can't inject markup. -- Style with **inline `style` attributes** or a `