Compare commits
1 Commits
clawbot/do
...
918533d897
| Author | SHA1 | Date | |
|---|---|---|---|
| 918533d897 |
120
README.md
120
README.md
@@ -7,13 +7,6 @@ services, durably stores them, and delivers them to configured targets
|
|||||||
with retry support, logging, and observability. Category: infrastructure
|
with retry support, logging, and observability. Category: infrastructure
|
||||||
/ web service. License: MIT.
|
/ web service. License: MIT.
|
||||||
|
|
||||||
Each entrypoint is a version 4 UUID served at `/webhook/{uuid}`, and
|
|
||||||
that UUID is the entrypoint's only credential. webhooker does not use
|
|
||||||
shared secrets, HMAC signatures or token headers on the receiver, and
|
|
||||||
will not add them — read
|
|
||||||
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret)
|
|
||||||
before deploying one.
|
|
||||||
|
|
||||||
## Getting Started
|
## Getting Started
|
||||||
|
|
||||||
### Prerequisites
|
### Prerequisites
|
||||||
@@ -693,44 +686,6 @@ databases written by `database` targets (`archive-{uuid}.db`). Mount
|
|||||||
this as a persistent volume to preserve data across container
|
this as a persistent volume to preserve data across container
|
||||||
restarts.
|
restarts.
|
||||||
|
|
||||||
**The bind-mounted directory must be owned by UID 1000, or the
|
|
||||||
container does not start.** Docker creates a `-v` source path that
|
|
||||||
does not exist yet as `root:root`, and the process runs as UID 1000,
|
|
||||||
so it cannot take its `DATA_DIR` lock:
|
|
||||||
|
|
||||||
```
|
|
||||||
webhooker: locking data directory /var/lib/webhooker: open
|
|
||||||
/var/lib/webhooker/webhooker.lock: permission denied
|
|
||||||
```
|
|
||||||
|
|
||||||
It exits non-zero at that point, before opening any database. Create
|
|
||||||
the directory ahead of the first `docker run`:
|
|
||||||
|
|
||||||
```bash
|
|
||||||
mkdir -p /path/to/data
|
|
||||||
chown 1000:1000 /path/to/data
|
|
||||||
chmod 750 /path/to/data
|
|
||||||
```
|
|
||||||
|
|
||||||
The same `chown` is what a restore needs — see step 4 of
|
|
||||||
[Restore](#restore). A **named volume** does not have this problem:
|
|
||||||
Docker copies the image's ownership onto a volume it initializes, and
|
|
||||||
the image creates `/var/lib/webhooker` owned by `webhooker`.
|
|
||||||
|
|
||||||
**The file modes are not yours to set, and do not depend on the
|
|
||||||
directory.** `webhooker.db` holds target configuration in plaintext —
|
|
||||||
bearer tokens, API keys, Slack webhook URLs — along with the session
|
|
||||||
encryption key, so webhooker creates every SQLite file it owns `0600`:
|
|
||||||
each database and both of its `-wal` and `-shm` sidecars, across all
|
|
||||||
three tiers. Files an earlier build left `0644` are tightened when
|
|
||||||
they are opened. A `DATA_DIR` webhooker creates itself is `0750`, but
|
|
||||||
a bind mount supplies its own directory and Docker's default for one
|
|
||||||
it creates is `0755`; the `0600` files hold there regardless. The
|
|
||||||
`chmod 750` above is defence in depth — it stops other local users
|
|
||||||
listing the directory and learning your webhook UUIDs from the
|
|
||||||
`events-{uuid}.db` filenames — not the barrier protecting the
|
|
||||||
credentials.
|
|
||||||
|
|
||||||
## Deployment behind a reverse proxy
|
## Deployment behind a reverse proxy
|
||||||
|
|
||||||
webhooker terminates no TLS of its own. It serves plaintext HTTP and
|
webhooker terminates no TLS of its own. It serves plaintext HTTP and
|
||||||
@@ -1156,38 +1111,14 @@ backups at rest and restrict who can read them.
|
|||||||
|
|
||||||
## The entrypoint URL is the authentication secret
|
## The entrypoint URL is the authentication secret
|
||||||
|
|
||||||
**The entrypoint UUID is the credential, and it is the only one.**
|
The receiver verifies nothing about an inbound request. The UUID in an
|
||||||
webhooker mints a version 4 UUID per entrypoint and serves it at
|
entrypoint's URL is its credential: anyone who holds that URL can
|
||||||
`/webhook/{uuid}`. Possession of that URL is the authentication:
|
submit events to it, and the receiver checks nothing else about the
|
||||||
anyone who holds it can submit events to the entrypoint, and the
|
sender. Treat an entrypoint URL the way you would treat an API token.
|
||||||
receiver verifies nothing else about the sender.
|
|
||||||
|
|
||||||
There is no shared secret, no HMAC signature, no bearer token and no
|
There is no way to rotate the UUID in place. To retire one, delete the
|
||||||
second factor on the receiver, and none will be added. This was
|
entrypoint (or deactivate it, which answers `410`) and create a new
|
||||||
considered and rejected; the implementation that existed was removed
|
one, then point the sender at the new URL.
|
||||||
in [PR #279](https://git.eeqj.de/sneak/webhooker/pulls/279), closing
|
|
||||||
[issue #67](https://git.eeqj.de/sneak/webhooker/issues/67) and
|
|
||||||
[issue #241](https://git.eeqj.de/sneak/webhooker/issues/241). A
|
|
||||||
proposal to reintroduce any of them — including as "defence in depth"
|
|
||||||
alongside the UUID — is answered by this section. Inbound signature
|
|
||||||
headers a sender sends anyway (`X-Hub-Signature` and its
|
|
||||||
per-provider equivalents) are stored and forwarded as ordinary
|
|
||||||
headers; nothing checks them.
|
|
||||||
|
|
||||||
What that means for an operator:
|
|
||||||
|
|
||||||
- **The URL is a capability, so treat it as a secret.** Keep it out of
|
|
||||||
logs, ticket bodies, chat messages and screenshots. Anyone who reads
|
|
||||||
it anywhere can post events as that sender.
|
|
||||||
- **Rotating means minting a new entrypoint, not changing a key.**
|
|
||||||
There is no way to rotate the UUID in place. To retire one, delete
|
|
||||||
the entrypoint (or deactivate it, which answers `410`) and create a
|
|
||||||
new one, then point the sender at the new URL.
|
|
||||||
- **A sender that cannot be given a secret URL is a constraint on that
|
|
||||||
integration, not a reason to change this.** If a service only
|
|
||||||
supports signed payloads to a well-known URL, raise it as its own
|
|
||||||
problem — pick a different integration path, or accept that it
|
|
||||||
cannot be used. It is not grounds to reintroduce shared secrets.
|
|
||||||
|
|
||||||
## Entrypoints
|
## Entrypoints
|
||||||
|
|
||||||
@@ -1265,16 +1196,6 @@ webhooker solves this by acting as a durable intermediary:
|
|||||||
backoff. Every delivery attempt is logged with status codes, response
|
backoff. Every delivery attempt is logged with status codes, response
|
||||||
bodies, and timing.
|
bodies, and timing.
|
||||||
|
|
||||||
**That guarantee is at-least-once, not exactly-once.** When a send
|
|
||||||
reaches its target but the write recording that outcome fails, the
|
|
||||||
delivery is deliberately left in a recoverable state rather than
|
|
||||||
marked done — losing a delivery is the worse failure — so the
|
|
||||||
pending sweep picks it up about fifteen minutes later, or the next
|
|
||||||
restart does, and the target receives a payload it already got.
|
|
||||||
webhooker adds no delivery identifier of its own to an outbound
|
|
||||||
request, so **make your receiver idempotent** against whatever the
|
|
||||||
payload itself carries.
|
|
||||||
|
|
||||||
3. **Observability** — Full request/response logging for every webhook
|
3. **Observability** — Full request/response logging for every webhook
|
||||||
received and every delivery attempted. Prometheus metrics expose
|
received and every delivery attempted. Prometheus metrics expose
|
||||||
volume, latency, and error rates. The web UI provides real-time
|
volume, latency, and error rates. The web UI provides real-time
|
||||||
@@ -1721,11 +1642,6 @@ webhooker uses **separate SQLite database files**: a main application
|
|||||||
database for configuration data and per-webhook databases for event
|
database for configuration data and per-webhook databases for event
|
||||||
storage. All database files live in the `DATA_DIR` directory.
|
storage. All database files live in the `DATA_DIR` directory.
|
||||||
|
|
||||||
Every one of them is created `0600`, and so is each `-wal` and `-shm`
|
|
||||||
sidecar. See
|
|
||||||
[Running with Docker](#running-with-docker) for what that does and
|
|
||||||
does not protect.
|
|
||||||
|
|
||||||
**Main Application Database** (`{DATA_DIR}/webhooker.db`) — stores
|
**Main Application Database** (`{DATA_DIR}/webhooker.db`) — stores
|
||||||
configuration and application state:
|
configuration and application state:
|
||||||
|
|
||||||
@@ -2657,15 +2573,12 @@ abuse limit later; they are tracked as future work.
|
|||||||
| `POST` | `/source/{id}/edit` | Edit webhook submission |
|
| `POST` | `/source/{id}/edit` | Edit webhook submission |
|
||||||
| `POST` | `/source/{id}/delete` | Delete webhook |
|
| `POST` | `/source/{id}/delete` | Delete webhook |
|
||||||
| `GET` | `/source/{id}/logs` | Webhook event logs |
|
| `GET` | `/source/{id}/logs` | Webhook event logs |
|
||||||
| `GET` | `/source/{id}/logs/{eventID}/body` | Download an event's full stored body. The log page renders each body only up to its cap, so this is the only route that serves a whole one; it is offered wherever a body is shown truncated |
|
|
||||||
| `POST` | `/source/{id}/deliveries/{deliveryID}/replay` | Replay a finished delivery: creates a new delivery for the same event against the target's current configuration (30 per minute per bucket, then `429`) |
|
| `POST` | `/source/{id}/deliveries/{deliveryID}/replay` | Replay a finished delivery: creates a new delivery for the same event against the target's current configuration (30 per minute per bucket, then `429`) |
|
||||||
| `POST` | `/source/{id}/events/{eventID}/resubmit` | Resubmit a stored event: creates a new event copying it and fans that out to every currently active target (30 per minute per bucket, then `429`) |
|
| `POST` | `/source/{id}/events/{eventID}/resubmit` | Resubmit a stored event: creates a new event copying it and fans that out to every currently active target (30 per minute per bucket, then `429`) |
|
||||||
| `POST` | `/source/{id}/entrypoints` | Add entrypoint to webhook |
|
| `POST` | `/source/{id}/entrypoints` | Add entrypoint to webhook |
|
||||||
| `POST` | `/source/{id}/entrypoints/{entrypointID}/delete` | Delete an entrypoint |
|
| `POST` | `/source/{id}/entrypoints/{entrypointID}/delete` | Delete an entrypoint |
|
||||||
| `POST` | `/source/{id}/entrypoints/{entrypointID}/toggle` | Enable or disable an entrypoint |
|
| `POST` | `/source/{id}/entrypoints/{entrypointID}/toggle` | Enable or disable an entrypoint |
|
||||||
| `POST` | `/source/{id}/targets` | Add target to webhook |
|
| `POST` | `/source/{id}/targets` | Add target to webhook |
|
||||||
| `GET` | `/source/{id}/targets/{targetID}/edit` | Edit target form. The one page that renders a target's destination URL and header values in full, rather than masked |
|
|
||||||
| `POST` | `/source/{id}/targets/{targetID}/edit` | Edit target submission |
|
|
||||||
| `POST` | `/source/{id}/targets/{targetID}/delete` | Delete a target |
|
| `POST` | `/source/{id}/targets/{targetID}/delete` | Delete a target |
|
||||||
| `POST` | `/source/{id}/targets/{targetID}/toggle` | Enable or disable a target |
|
| `POST` | `/source/{id}/targets/{targetID}/toggle` | Enable or disable a target |
|
||||||
|
|
||||||
@@ -2704,8 +2617,6 @@ webhooker/
|
|||||||
├── internal/
|
├── internal/
|
||||||
│ ├── banner/
|
│ ├── banner/
|
||||||
│ │ └── banner.go # Ruled block for the one credential shown in the clear
|
│ │ └── banner.go # Ruled block for the one credential shown in the clear
|
||||||
│ ├── ciscript/
|
|
||||||
│ │ └── doc.go # Tests for the CI shell scripts in script/; no runtime code
|
|
||||||
│ ├── resetpw/
|
│ ├── resetpw/
|
||||||
│ │ └── resetpw.go # `webhooker resetpw`: set an account's password, stopped deployments only
|
│ │ └── resetpw.go # `webhooker resetpw`: set an account's password, stopped deployments only
|
||||||
│ ├── config/
|
│ ├── config/
|
||||||
@@ -2775,17 +2686,13 @@ webhooker/
|
|||||||
│ │ ├── ratelimit.go # Per-IP rate limiting middleware (go-chi/httprate)
|
│ │ ├── ratelimit.go # Per-IP rate limiting middleware (go-chi/httprate)
|
||||||
│ │ ├── loginguard.go # Login failure counters and the Argon2id verification semaphore
|
│ │ ├── loginguard.go # Login failure counters and the Argon2id verification semaphore
|
||||||
│ │ └── testing.go # NewForTest: Middleware without the fx lifecycle
|
│ │ └── testing.go # NewForTest: Middleware without the fx lifecycle
|
||||||
│ ├── reqtls/
|
|
||||||
│ │ └── reqtls.go # IsTLS: the one TLS predicate, r.TLS or X-Forwarded-Proto
|
|
||||||
│ ├── server/
|
│ ├── server/
|
||||||
│ │ ├── server.go # Server struct, fx lifecycle, signal handling
|
│ │ ├── server.go # Server struct, fx lifecycle, signal handling
|
||||||
│ │ ├── http.go # HTTP server setup with timeouts
|
│ │ ├── http.go # HTTP server setup with timeouts
|
||||||
│ │ └── routes.go # All route definitions
|
│ │ └── routes.go # All route definitions
|
||||||
│ ├── session/
|
│ └── session/
|
||||||
│ │ ├── session.go # Cookie-based session management
|
│ ├── session.go # Cookie-based session management
|
||||||
│ │ └── testing.go # NewForTest: Session without the fx lifecycle
|
│ └── testing.go # NewForTest: Session without the fx lifecycle
|
||||||
│ └── versionscript/
|
|
||||||
│ └── doc.go # Tests for script/version and the build files that use it
|
|
||||||
├── static/
|
├── static/
|
||||||
│ ├── static.go # //go:embed directive
|
│ ├── static.go # //go:embed directive
|
||||||
│ ├── css/input.css # Tailwind input, source for tailwind.css (make css)
|
│ ├── css/input.css # Tailwind input, source for tailwind.css (make css)
|
||||||
@@ -2898,10 +2805,6 @@ check, see [The login endpoint](#the-login-endpoint).
|
|||||||
|
|
||||||
### Authentication
|
### Authentication
|
||||||
|
|
||||||
- **Webhook receiver:** the entrypoint UUID in the URL, and nothing
|
|
||||||
else. No shared secret, no HMAC signature, no token header, and none
|
|
||||||
will be added — see
|
|
||||||
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret).
|
|
||||||
- **Web UI:** Cookie-based sessions using gorilla/sessions with
|
- **Web UI:** Cookie-based sessions using gorilla/sessions with
|
||||||
encrypted cookies. Sessions are configured with HttpOnly, SameSite
|
encrypted cookies. Sessions are configured with HttpOnly, SameSite
|
||||||
Lax, and Secure whenever the request is on TLS — the flag follows the
|
Lax, and Secure whenever the request is on TLS — the flag follows the
|
||||||
@@ -2941,8 +2844,7 @@ check, see [The login endpoint](#the-login-endpoint).
|
|||||||
mode
|
mode
|
||||||
- **The entrypoint URL is the receiver's only credential.** Nothing
|
- **The entrypoint URL is the receiver's only credential.** Nothing
|
||||||
about an inbound request is verified; possession of the UUID
|
about an inbound request is verified; possession of the UUID
|
||||||
authorises submission, and no shared secret or signature check will
|
authorises submission (see
|
||||||
be added alongside it (see
|
|
||||||
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret))
|
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret))
|
||||||
- **SSRF prevention** for HTTP delivery targets: private/reserved IP
|
- **SSRF prevention** for HTTP delivery targets: private/reserved IP
|
||||||
ranges (RFC 1918, loopback, link-local, cloud metadata) are blocked
|
ranges (RFC 1918, loopback, link-local, cloud metadata) are blocked
|
||||||
|
|||||||
41
TODO.md
41
TODO.md
@@ -18,27 +18,18 @@ Issue branches do NOT touch this file — the manager maintains it on
|
|||||||
|
|
||||||
# Status
|
# Status
|
||||||
|
|
||||||
The milestone (https://git.eeqj.de/sneak/webhooker/milestone/9) is the
|
1.0.0 is open, with work remaining. The milestone
|
||||||
authoritative list, and the only place to read a count or a state of
|
(https://git.eeqj.de/sneak/webhooker/milestone/9) is the authoritative
|
||||||
play from. This file records where the project is, not what is in
|
list, and the only place to read a count or a state of play from. This
|
||||||
flight: a sentence whose truth depends on a branch being unmerged is
|
file records where the project is, not what is in flight: a sentence
|
||||||
wrong the moment it merges, and this file has been wrong that way
|
whose truth depends on a branch being unmerged is wrong the moment it
|
||||||
before.
|
merges, and this file has been wrong that way before.
|
||||||
|
|
||||||
The durability defect that held the tag has landed
|
The tag is held on a durability defect
|
||||||
(https://git.eeqj.de/sneak/webhooker/issues/256, commit `8d64259`).
|
(https://git.eeqj.de/sneak/webhooker/issues/256): a concurrent reader
|
||||||
Every SQLite handle opens with WAL journaling and a busy timeout, a
|
of a per-webhook event database strands delivered webhooks at
|
||||||
bookkeeping write that fails leaves its delivery in a recoverable
|
`pending`, and the next restart re-delivers them. That issue gates
|
||||||
state rather than a lying one, and recovery skips a delivery that
|
`v1.0.0`, and is where the fix's own state is tracked.
|
||||||
already has a successful result row. Final pre-tag verification
|
|
||||||
exercised it and confirmed it holds. Whatever the milestone still
|
|
||||||
shows open is what remains before `v1.0.0`.
|
|
||||||
|
|
||||||
Delivery is at-least-once by design, not by accident: a send whose
|
|
||||||
result row does not land is attempted again, so a receiver can see a
|
|
||||||
duplicate. That is deliberate — the alternative is a silent lost
|
|
||||||
delivery — and the README says so under Rationale. It is not a defect
|
|
||||||
to re-file.
|
|
||||||
|
|
||||||
One caveat on reading a green check: a docs-only commit deliberately
|
One caveat on reading a green check: a docs-only commit deliberately
|
||||||
replays from the layer cache
|
replays from the layer cache
|
||||||
@@ -48,11 +39,11 @@ commit invalidates the `COPY` layer and genuinely executes.
|
|||||||
|
|
||||||
# Next Step
|
# Next Step
|
||||||
|
|
||||||
Clear the rest of the open 1.0.0 milestone
|
Land https://git.eeqj.de/sneak/webhooker/issues/256, then clear the
|
||||||
(https://git.eeqj.de/sneak/webhooker/milestone/9) and tag `v1.0.0`.
|
rest of the open 1.0.0 milestone and tag `v1.0.0`. Merging `next` into
|
||||||
Merging `next` into `main` is a separate act from tagging and waits on
|
`main` is a separate act from tagging and waits on neither of those:
|
||||||
neither of those: `next` is kept mergeable at all times, which is the
|
`next` is kept mergeable at all times, which is the point of the
|
||||||
point of the branch.
|
branch.
|
||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
|||||||
@@ -1,240 +0,0 @@
|
|||||||
package database_test
|
|
||||||
|
|
||||||
import (
|
|
||||||
"context"
|
|
||||||
"io/fs"
|
|
||||||
"net/http"
|
|
||||||
"os"
|
|
||||||
"path/filepath"
|
|
||||||
"testing"
|
|
||||||
|
|
||||||
"github.com/google/uuid"
|
|
||||||
"github.com/stretchr/testify/assert"
|
|
||||||
"github.com/stretchr/testify/require"
|
|
||||||
"go.uber.org/fx/fxtest"
|
|
||||||
"sneak.berlin/go/webhooker/internal/config"
|
|
||||||
"sneak.berlin/go/webhooker/internal/database"
|
|
||||||
"sneak.berlin/go/webhooker/internal/globals"
|
|
||||||
"sneak.berlin/go/webhooker/internal/logger"
|
|
||||||
)
|
|
||||||
|
|
||||||
// ownerOnly is the mode every SQLite file the service owns must have.
|
|
||||||
// Spelled out rather than referencing database.SQLiteFilePerm so the
|
|
||||||
// test fails if the constant itself is loosened.
|
|
||||||
const ownerOnly fs.FileMode = 0o600
|
|
||||||
|
|
||||||
// requireOwnerOnly asserts that path exists and is readable and
|
|
||||||
// writable by its owner and by nobody else.
|
|
||||||
func requireOwnerOnly(t *testing.T, path string) {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
info, err := os.Stat(path)
|
|
||||||
require.NoError(t, err, "%s must exist", path)
|
|
||||||
assert.Equal(
|
|
||||||
t,
|
|
||||||
ownerOnly,
|
|
||||||
info.Mode().Perm(),
|
|
||||||
"%s holds credentials and must not be readable by "+
|
|
||||||
"anyone but its owner",
|
|
||||||
path,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// requireDatabaseSetOwnerOnly asserts the mode of a database file and
|
|
||||||
// of both WAL sidecars. The sidecars carry the same rows as the
|
|
||||||
// database, so tightening only the main file fixes nothing.
|
|
||||||
func requireDatabaseSetOwnerOnly(t *testing.T, dbPath string) {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
requireOwnerOnly(t, dbPath)
|
|
||||||
requireOwnerOnly(t, dbPath+"-wal")
|
|
||||||
requireOwnerOnly(t, dbPath+"-shm")
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestMainDatabaseFilesAreOwnerOnly covers the tier the defect was
|
|
||||||
// reported against: webhooker.db holds targets.config in plaintext —
|
|
||||||
// bearer tokens, API keys, Slack webhook URLs — and the session
|
|
||||||
// encryption key.
|
|
||||||
func TestMainDatabaseFilesAreOwnerOnly(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
lc := fxtest.NewLifecycle(t)
|
|
||||||
|
|
||||||
l, err := logger.New(lc, logger.LoggerParams{
|
|
||||||
Globals: &globals.Globals{
|
|
||||||
Appname: testAppname,
|
|
||||||
Version: testVersion,
|
|
||||||
},
|
|
||||||
})
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
// A directory the application creates itself, not one t.TempDir
|
|
||||||
// made at 0700, so the mode below is the application's.
|
|
||||||
dataDir := filepath.Join(t.TempDir(), "data")
|
|
||||||
|
|
||||||
db, err := database.New(lc, database.DatabaseParams{
|
|
||||||
Config: &config.Config{DataDir: dataDir},
|
|
||||||
Logger: l,
|
|
||||||
})
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
ctx := context.Background()
|
|
||||||
require.NoError(t, lc.Start(ctx))
|
|
||||||
|
|
||||||
defer func() { require.NoError(t, lc.Stop(ctx)) }()
|
|
||||||
|
|
||||||
// Write through the real model so the WAL is populated and both
|
|
||||||
// sidecars are on disk while the handle is open.
|
|
||||||
require.NoError(t, db.DB().Create(&database.Webhook{
|
|
||||||
Name: testWebhookName,
|
|
||||||
}).Error)
|
|
||||||
|
|
||||||
requireDatabaseSetOwnerOnly(
|
|
||||||
t, filepath.Join(dataDir, database.MainDBFileName),
|
|
||||||
)
|
|
||||||
|
|
||||||
// The data directory grants nothing to `other`. Asserted as a
|
|
||||||
// property rather than as an exact 0750, because MkdirAll applies
|
|
||||||
// the ambient umask: the exact mode is the developer's umask as
|
|
||||||
// much as the application's request, and pinning it would make
|
|
||||||
// `make check` pass or fail on where it is run. The group bits are
|
|
||||||
// deliberately left unasserted — deployments may rely on them.
|
|
||||||
info, err := os.Stat(dataDir)
|
|
||||||
require.NoError(t, err)
|
|
||||||
assert.Zero(
|
|
||||||
t,
|
|
||||||
info.Mode().Perm()&0o007,
|
|
||||||
"the data directory must not be world-accessible",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestPerWebhookEventDatabaseFilesAreOwnerOnly covers the events-*.db
|
|
||||||
// tier. These carry no credential canaries since
|
|
||||||
// https://git.eeqj.de/sneak/webhooker/issues/206, but they hold every
|
|
||||||
// received request body and header.
|
|
||||||
func TestPerWebhookEventDatabaseFilesAreOwnerOnly(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
mgr, lc := setupTestWebhookDBManager(t)
|
|
||||||
ctx := context.Background()
|
|
||||||
require.NoError(t, lc.Start(ctx))
|
|
||||||
|
|
||||||
defer func() { require.NoError(t, lc.Stop(ctx)) }()
|
|
||||||
|
|
||||||
webhookID := uuid.New().String()
|
|
||||||
|
|
||||||
db, err := mgr.GetDB(webhookID)
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
require.NoError(t, db.Create(&database.Event{
|
|
||||||
WebhookID: webhookID,
|
|
||||||
EntrypointID: uuid.New().String(),
|
|
||||||
Method: http.MethodPost,
|
|
||||||
Body: "{}",
|
|
||||||
}).Error)
|
|
||||||
|
|
||||||
requireDatabaseSetOwnerOnly(t, mgr.DBPath(webhookID))
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestArchiveDatabaseFilesAreOwnerOnly covers the archive-*.db tier.
|
|
||||||
// internal/delivery builds that path and opens it through OpenSQLite,
|
|
||||||
// the same single open path exercised here, so the mode is settled for
|
|
||||||
// all three tiers in one place.
|
|
||||||
func TestArchiveDatabaseFilesAreOwnerOnly(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
ctx := context.Background()
|
|
||||||
path := filepath.Join(
|
|
||||||
t.TempDir(), "archive-"+uuid.New().String()+".db",
|
|
||||||
)
|
|
||||||
|
|
||||||
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
defer func() { require.NoError(t, sqlDB.Close()) }()
|
|
||||||
|
|
||||||
_, err = sqlDB.ExecContext(ctx, "create table t (id integer)")
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
requireDatabaseSetOwnerOnly(t, path)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestOpenSQLiteTightensFilesLeftWorldReadable is the upgrade case: a
|
|
||||||
// data directory an earlier build left at 0644, including a
|
|
||||||
// developer's own scratch directory, is fixed when it is opened rather
|
|
||||||
// than staying exposed until it is recreated.
|
|
||||||
func TestOpenSQLiteTightensFilesLeftWorldReadable(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
dir := t.TempDir()
|
|
||||||
path := filepath.Join(dir, database.MainDBFileName)
|
|
||||||
|
|
||||||
// A database and both sidecars as the pre-fix build left them.
|
|
||||||
for _, p := range []string{path, path + "-wal", path + "-shm"} {
|
|
||||||
require.NoError(t, os.WriteFile(p, nil, 0o644)) //nolint:gosec // the mode under test
|
|
||||||
}
|
|
||||||
|
|
||||||
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
require.NoError(t, sqlDB.Close())
|
|
||||||
|
|
||||||
requireDatabaseSetOwnerOnly(t, path)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestOpenSQLiteExistingModeDoesNotCreateTheFile guards the mechanism
|
|
||||||
// the fix uses: OpenSQLite now creates the database file itself, and
|
|
||||||
// must not do so for a caller that asked for an existing database. An
|
|
||||||
// empty file materialized here would turn a missing-database error
|
|
||||||
// into a silently empty one.
|
|
||||||
func TestOpenSQLiteExistingModeDoesNotCreateTheFile(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
ctx := context.Background()
|
|
||||||
path := filepath.Join(t.TempDir(), "absent.db")
|
|
||||||
|
|
||||||
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeExisting)
|
|
||||||
if err == nil {
|
|
||||||
// sql.Open is lazy: force the connection that fails.
|
|
||||||
require.Error(t, sqlDB.PingContext(ctx))
|
|
||||||
require.NoError(t, sqlDB.Close())
|
|
||||||
}
|
|
||||||
|
|
||||||
_, statErr := os.Stat(path)
|
|
||||||
assert.ErrorIs(t, statErr, fs.ErrNotExist)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestReopenAfterRestartKeepsFilesOwnerOnly is the restart case: a
|
|
||||||
// process that closed its files must be able to open them again at
|
|
||||||
// 0600, including through a gorm handle, and the sidecars must come
|
|
||||||
// back at 0600 too rather than at SQLite's own default.
|
|
||||||
func TestReopenAfterRestartKeepsFilesOwnerOnly(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
ctx := context.Background()
|
|
||||||
dir := t.TempDir()
|
|
||||||
path := filepath.Join(dir, database.MainDBFileName)
|
|
||||||
|
|
||||||
first, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
_, err = first.ExecContext(ctx, "create table t (id integer)")
|
|
||||||
require.NoError(t, err)
|
|
||||||
require.NoError(t, first.Close())
|
|
||||||
|
|
||||||
second, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
defer func() { require.NoError(t, second.Close()) }()
|
|
||||||
|
|
||||||
_, err = second.ExecContext(ctx, "insert into t (id) values (1)")
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
requireDatabaseSetOwnerOnly(t, path)
|
|
||||||
|
|
||||||
var got int
|
|
||||||
|
|
||||||
require.NoError(t,
|
|
||||||
second.QueryRowContext(ctx, "select id from t").Scan(&got))
|
|
||||||
assert.Equal(t, 1, got)
|
|
||||||
}
|
|
||||||
@@ -2,11 +2,8 @@ package database
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"database/sql"
|
"database/sql"
|
||||||
"errors"
|
|
||||||
"fmt"
|
"fmt"
|
||||||
"io/fs"
|
|
||||||
"net/url"
|
"net/url"
|
||||||
"os"
|
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
_ "modernc.org/sqlite" // Pure Go SQLite driver
|
_ "modernc.org/sqlite" // Pure Go SQLite driver
|
||||||
@@ -75,90 +72,6 @@ const (
|
|||||||
sqliteConnMaxIdleTime = time.Minute
|
sqliteConnMaxIdleTime = time.Minute
|
||||||
)
|
)
|
||||||
|
|
||||||
// SQLiteFilePerm is the mode every SQLite file this service owns is
|
|
||||||
// created with and held at: owner read/write, nothing for group or
|
|
||||||
// other.
|
|
||||||
//
|
|
||||||
// These files hold credentials in plaintext. The main database stores
|
|
||||||
// `targets.config` — bearer tokens, API keys, Slack webhook URLs — and
|
|
||||||
// the session encryption key. SQLite left to itself creates them 0644
|
|
||||||
// (see reserveSQLiteFile), which made the 0750 data directory the only
|
|
||||||
// barrier; a bind-mounted directory supplied at 0755 removes it and
|
|
||||||
// every local user on the host can read every stored credential.
|
|
||||||
//
|
|
||||||
// This is a file-mode fix and not encryption at rest. An unattended
|
|
||||||
// process needs a key it can read without a human, so the key lands
|
|
||||||
// beside the data and an attacker who can read the database can read
|
|
||||||
// it too. See https://git.eeqj.de/sneak/webhooker/issues/212.
|
|
||||||
const SQLiteFilePerm fs.FileMode = 0o600
|
|
||||||
|
|
||||||
// reserveSQLiteFile puts path at SQLiteFilePerm before the driver ever
|
|
||||||
// touches it, and tightens any sidecar already on disk.
|
|
||||||
//
|
|
||||||
// The mode has to be settled here rather than by a chmod after opening,
|
|
||||||
// because SQLite picks it: robust_open substitutes
|
|
||||||
// SQLITE_DEFAULT_FILE_PERMISSIONS (0644) whenever it is handed mode 0,
|
|
||||||
// and findCreateFileMode yields 0 for a main database opened by URI
|
|
||||||
// with no `modeof` parameter. A chmod afterwards would leave a window
|
|
||||||
// in which the credentials are on disk world-readable.
|
|
||||||
//
|
|
||||||
// Creating the file ourselves also settles the sidecars, which is the
|
|
||||||
// half that could quietly not work. SQLite does not create those at a
|
|
||||||
// mode we choose — it derives both from the main database file:
|
|
||||||
// `-wal` through findCreateFileMode, which stats the path with the
|
|
||||||
// suffix stripped, and `-shm` in unixOpenSharedMemory from an fstat of
|
|
||||||
// the already-open database descriptor. A main file at 0600 therefore
|
|
||||||
// produces sidecars at 0600. A zero-length file is a valid empty
|
|
||||||
// database, so reserving it changes nothing else.
|
|
||||||
//
|
|
||||||
// create says whether the caller is opening in a mode that may create
|
|
||||||
// the database. When it is false a missing file is left missing, so
|
|
||||||
// SQLite still reports the absence rather than this function
|
|
||||||
// materializing an empty database the caller asked not to create.
|
|
||||||
//
|
|
||||||
// Chmod of a file that already exists is what tightens a data
|
|
||||||
// directory an earlier build left at 0644 — including a developer's
|
|
||||||
// own scratch directory — without any migration machinery.
|
|
||||||
func reserveSQLiteFile(path string, create bool) error {
|
|
||||||
if create {
|
|
||||||
// gosec G304: the path is the database file the caller asked
|
|
||||||
// to open, and the driver is about to open the same path
|
|
||||||
// anyway. Creating it here is what fixes its mode.
|
|
||||||
f, err := os.OpenFile( //nolint:gosec // see above
|
|
||||||
path, os.O_RDWR|os.O_CREATE, SQLiteFilePerm,
|
|
||||||
)
|
|
||||||
if err != nil {
|
|
||||||
return fmt.Errorf("creating %s: %w", path, err)
|
|
||||||
}
|
|
||||||
|
|
||||||
err = f.Close()
|
|
||||||
if err != nil {
|
|
||||||
return fmt.Errorf("closing %s: %w", path, err)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// O_CREATE leaves an existing file's mode alone, and umask can only
|
|
||||||
// have narrowed a new one. Chmod settles both cases at exactly
|
|
||||||
// SQLiteFilePerm.
|
|
||||||
for _, p := range append(
|
|
||||||
[]string{path}, sqliteSidecarPaths(path)...,
|
|
||||||
) {
|
|
||||||
err := os.Chmod(p, SQLiteFilePerm)
|
|
||||||
if err != nil && !errors.Is(err, fs.ErrNotExist) {
|
|
||||||
return fmt.Errorf("securing %s: %w", p, err)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// sqliteSidecarPaths returns the files SQLite maintains beside a
|
|
||||||
// database under WAL. They carry the same rows as the database itself,
|
|
||||||
// so a fix that tightens only the main file has fixed nothing.
|
|
||||||
func sqliteSidecarPaths(path string) []string {
|
|
||||||
return []string{path + "-wal", path + "-shm"}
|
|
||||||
}
|
|
||||||
|
|
||||||
// SQLiteDSN builds the connection string for one database file.
|
// SQLiteDSN builds the connection string for one database file.
|
||||||
//
|
//
|
||||||
// mode is the SQLite URI open mode: "rwc" to create the file when it
|
// mode is the SQLite URI open mode: "rwc" to create the file when it
|
||||||
@@ -225,17 +138,9 @@ func SQLiteDSN(path, mode string) string {
|
|||||||
// durability settings and pool bounds applied. mode is the SQLite URI
|
// durability settings and pool bounds applied. mode is the SQLite URI
|
||||||
// open mode ("rwc" or "rw").
|
// open mode ("rwc" or "rw").
|
||||||
//
|
//
|
||||||
// The file and its WAL sidecars are settled at SQLiteFilePerm before
|
|
||||||
// the driver sees the path; see reserveSQLiteFile.
|
|
||||||
//
|
|
||||||
// The handle is returned rather than a *gorm.DB because the callers
|
// The handle is returned rather than a *gorm.DB because the callers
|
||||||
// wrap it in gorm themselves with their own logger.
|
// wrap it in gorm themselves with their own logger.
|
||||||
func OpenSQLite(path, mode string) (*sql.DB, error) {
|
func OpenSQLite(path, mode string) (*sql.DB, error) {
|
||||||
err := reserveSQLiteFile(path, mode == SQLiteModeCreate)
|
|
||||||
if err != nil {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
|
|
||||||
sqlDB, err := sql.Open("sqlite", SQLiteDSN(path, mode))
|
sqlDB, err := sql.Open("sqlite", SQLiteDSN(path, mode))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf(
|
return nil, fmt.Errorf(
|
||||||
|
|||||||
@@ -440,7 +440,7 @@ func (e *Engine) processNewTask(
|
|||||||
|
|
||||||
event := buildEventFromTask(task)
|
event := buildEventFromTask(task)
|
||||||
|
|
||||||
event, err = e.hydrateEvent(
|
event, err = e.resolveEventBody(
|
||||||
webhookDB, event, task,
|
webhookDB, event, task,
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -512,7 +512,7 @@ func (e *Engine) processRetryTask(
|
|||||||
|
|
||||||
event := buildEventFromTask(task)
|
event := buildEventFromTask(task)
|
||||||
|
|
||||||
event, err = e.hydrateEvent(
|
event, err = e.resolveEventBody(
|
||||||
webhookDB, event, task,
|
webhookDB, event, task,
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -1547,11 +1547,6 @@ func truncate(s string, maxLen int) string {
|
|||||||
|
|
||||||
// --- Helper functions ---
|
// --- Helper functions ---
|
||||||
|
|
||||||
// buildEventFromTask reconstructs the event a Task describes, as far
|
|
||||||
// as the Task itself goes. The fields it cannot fill — the body when
|
|
||||||
// it was too large to inline, and the receipt time, which no Task
|
|
||||||
// carries — come from the stored row in hydrateEvent, which every
|
|
||||||
// caller of this function runs next.
|
|
||||||
func buildEventFromTask(task *Task) database.Event {
|
func buildEventFromTask(task *Task) database.Event {
|
||||||
event := database.Event{
|
event := database.Event{
|
||||||
EntrypointID: task.EntrypointID,
|
EntrypointID: task.EntrypointID,
|
||||||
@@ -1579,67 +1574,29 @@ func buildTargetFromTask(task *Task) database.Target {
|
|||||||
return target
|
return target
|
||||||
}
|
}
|
||||||
|
|
||||||
// hydrateEvent fills in the event fields a Task does not carry, by
|
func (e *Engine) resolveEventBody(
|
||||||
// reading the stored event row.
|
|
||||||
//
|
|
||||||
// CreatedAt is the event's receipt time and lives only in that row.
|
|
||||||
// The Slack target renders it into every message it sends, so an
|
|
||||||
// unhydrated event puts the zero time in front of a human on every
|
|
||||||
// notification the product delivers. See
|
|
||||||
// https://git.eeqj.de/sneak/webhooker/issues/257.
|
|
||||||
//
|
|
||||||
// The body comes from the same row when the Task did not inline it,
|
|
||||||
// which is the case for a body at or above MaxInlineBodySize.
|
|
||||||
//
|
|
||||||
// A read failure is fatal to the delivery only when the body depended
|
|
||||||
// on it. When the Task inlined the body, the delivery has everything
|
|
||||||
// it needs to be sent and goes ahead with the timestamp unset: the row
|
|
||||||
// can be gone under a retention reap while a queued delivery still
|
|
||||||
// holds its body, and dropping a deliverable event to protect one
|
|
||||||
// metadata field would be a worse failure than the one it prevents.
|
|
||||||
func (e *Engine) hydrateEvent(
|
|
||||||
webhookDB *gorm.DB,
|
webhookDB *gorm.DB,
|
||||||
event database.Event,
|
event database.Event,
|
||||||
task *Task,
|
task *Task,
|
||||||
) (database.Event, error) {
|
) (database.Event, error) {
|
||||||
columns := []string{"created_at"}
|
if task.Body != nil {
|
||||||
|
|
||||||
if task.Body == nil {
|
|
||||||
columns = append(columns, "body")
|
|
||||||
}
|
|
||||||
|
|
||||||
var dbEvent database.Event
|
|
||||||
|
|
||||||
err := webhookDB.Select(columns).
|
|
||||||
First(&dbEvent, "id = ?", task.EventID).Error
|
|
||||||
if err != nil {
|
|
||||||
if task.Body == nil {
|
|
||||||
return event, fmt.Errorf(
|
|
||||||
"fetching event body: %w", err,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
e.log.Warn(
|
|
||||||
"could not read the stored event; delivering "+
|
|
||||||
"the inlined body without its receipt time",
|
|
||||||
"event_id", task.EventID,
|
|
||||||
"delivery_id", task.DeliveryID,
|
|
||||||
"error", err,
|
|
||||||
)
|
|
||||||
|
|
||||||
event.Body = *task.Body
|
event.Body = *task.Body
|
||||||
|
|
||||||
return event, nil
|
return event, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
event.CreatedAt = dbEvent.CreatedAt
|
var dbEvent database.Event
|
||||||
|
|
||||||
if task.Body != nil {
|
err := webhookDB.Select("body").
|
||||||
event.Body = *task.Body
|
First(&dbEvent, "id = ?", task.EventID).Error
|
||||||
} else {
|
if err != nil {
|
||||||
event.Body = dbEvent.Body
|
return event, fmt.Errorf(
|
||||||
|
"fetching event body: %w", err,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
event.Body = dbEvent.Body
|
||||||
|
|
||||||
return event, nil
|
return event, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,442 +0,0 @@
|
|||||||
package delivery_test
|
|
||||||
|
|
||||||
import (
|
|
||||||
"context"
|
|
||||||
"encoding/json"
|
|
||||||
"io"
|
|
||||||
"net/http"
|
|
||||||
"net/http/httptest"
|
|
||||||
"testing"
|
|
||||||
"time"
|
|
||||||
|
|
||||||
"github.com/google/uuid"
|
|
||||||
"github.com/stretchr/testify/assert"
|
|
||||||
"github.com/stretchr/testify/require"
|
|
||||||
"gorm.io/gorm"
|
|
||||||
"sneak.berlin/go/webhooker/internal/database"
|
|
||||||
"sneak.berlin/go/webhooker/internal/delivery"
|
|
||||||
)
|
|
||||||
|
|
||||||
// tsEventCreatedAt is the receipt time seeded on the events these
|
|
||||||
// tests deliver. It is far enough from both the zero time and from
|
|
||||||
// now that neither can be mistaken for it.
|
|
||||||
func tsEventCreatedAt() time.Time {
|
|
||||||
return time.Date(
|
|
||||||
2026, time.March, 4, 5, 6, 7, 0, time.UTC,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// tsZeroStamp is what a Slack message renders when the event handed
|
|
||||||
// to FormatSlackMessage carries no CreatedAt.
|
|
||||||
const tsZeroStamp = "*Timestamp:* `0001-01-01T00:00:00Z`"
|
|
||||||
|
|
||||||
// tsEventBody is the body seeded on every event in this file. It is
|
|
||||||
// small enough that a Task can inline it.
|
|
||||||
const tsEventBody = `{"hello":"world"}`
|
|
||||||
|
|
||||||
// tsUndeliverableHook stands in for a Slack incoming webhook on the
|
|
||||||
// tests that never send: the config parser requires a URL, but no
|
|
||||||
// request is made.
|
|
||||||
const tsUndeliverableHook = "https://hooks.slack.com/services/T/B/x"
|
|
||||||
|
|
||||||
// tsSink is a stand-in Slack incoming webhook that records the raw
|
|
||||||
// body posted to it.
|
|
||||||
type tsSink struct {
|
|
||||||
*httptest.Server
|
|
||||||
|
|
||||||
bodies chan []byte
|
|
||||||
}
|
|
||||||
|
|
||||||
func newTSSink(t *testing.T) *tsSink {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
s := &tsSink{bodies: make(chan []byte, 8)}
|
|
||||||
|
|
||||||
s.Server = httptest.NewServer(http.HandlerFunc(
|
|
||||||
func(w http.ResponseWriter, r *http.Request) {
|
|
||||||
body, _ := io.ReadAll(r.Body)
|
|
||||||
|
|
||||||
select {
|
|
||||||
case s.bodies <- body:
|
|
||||||
default:
|
|
||||||
}
|
|
||||||
|
|
||||||
w.WriteHeader(http.StatusOK)
|
|
||||||
},
|
|
||||||
))
|
|
||||||
|
|
||||||
t.Cleanup(s.Close)
|
|
||||||
|
|
||||||
return s
|
|
||||||
}
|
|
||||||
|
|
||||||
// text returns the Slack message text from the single payload the
|
|
||||||
// sink received.
|
|
||||||
func (s *tsSink) text(t *testing.T) string {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
select {
|
|
||||||
case raw := <-s.bodies:
|
|
||||||
t.Logf("raw slack payload: %s", raw)
|
|
||||||
|
|
||||||
var payload struct {
|
|
||||||
Text string `json:"text"`
|
|
||||||
}
|
|
||||||
|
|
||||||
require.NoError(t, json.Unmarshal(raw, &payload))
|
|
||||||
|
|
||||||
return payload.Text
|
|
||||||
case <-time.After(5 * time.Second):
|
|
||||||
t.Fatal("slack sink received no payload")
|
|
||||||
|
|
||||||
return ""
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func tsSlackConfig(t *testing.T, url string) string {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
data, err := json.Marshal(
|
|
||||||
delivery.SlackTargetConfig{WebhookURL: url},
|
|
||||||
)
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
return string(data)
|
|
||||||
}
|
|
||||||
|
|
||||||
// tsSeedEvent writes an event whose CreatedAt is tsEventCreatedAt
|
|
||||||
// rather than the write time, so an assertion on the rendered
|
|
||||||
// timestamp cannot pass by accident against "roughly now".
|
|
||||||
func tsSeedEvent(
|
|
||||||
t *testing.T, db *gorm.DB, webhookID string,
|
|
||||||
) database.Event {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
event := database.Event{
|
|
||||||
WebhookID: webhookID,
|
|
||||||
EntrypointID: uuid.New().String(),
|
|
||||||
Method: http.MethodPost,
|
|
||||||
Headers: `{}`,
|
|
||||||
Body: tsEventBody,
|
|
||||||
ContentType: "application/json",
|
|
||||||
}
|
|
||||||
event.ID = uuid.New().String()
|
|
||||||
event.CreatedAt = tsEventCreatedAt()
|
|
||||||
event.UpdatedAt = tsEventCreatedAt()
|
|
||||||
|
|
||||||
require.NoError(t, db.Create(&event).Error)
|
|
||||||
|
|
||||||
var stored database.Event
|
|
||||||
|
|
||||||
require.NoError(t,
|
|
||||||
db.First(&stored, "id = ?", event.ID).Error,
|
|
||||||
)
|
|
||||||
require.Equal(t,
|
|
||||||
tsEventCreatedAt().UTC(), stored.CreatedAt.UTC(),
|
|
||||||
"seeded created_at did not round-trip",
|
|
||||||
)
|
|
||||||
|
|
||||||
return event
|
|
||||||
}
|
|
||||||
|
|
||||||
// tsSeedTarget writes the slack target row into the main database.
|
|
||||||
// The retry path confirms the target still exists before sending.
|
|
||||||
func tsSeedTarget(
|
|
||||||
t *testing.T, mainDB *gorm.DB, webhookID, config string,
|
|
||||||
) database.Target {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
target := database.Target{
|
|
||||||
WebhookID: webhookID,
|
|
||||||
Name: "slack-sink",
|
|
||||||
Type: database.TargetTypeSlack,
|
|
||||||
Config: config,
|
|
||||||
Active: true,
|
|
||||||
}
|
|
||||||
|
|
||||||
require.NoError(t, mainDB.Create(&target).Error)
|
|
||||||
|
|
||||||
return target
|
|
||||||
}
|
|
||||||
|
|
||||||
func tsTask(
|
|
||||||
d database.Delivery,
|
|
||||||
event database.Event,
|
|
||||||
webhookID string,
|
|
||||||
target database.Target,
|
|
||||||
attemptNum int,
|
|
||||||
body *string,
|
|
||||||
) delivery.Task {
|
|
||||||
return delivery.Task{
|
|
||||||
DeliveryID: d.ID,
|
|
||||||
EventID: event.ID,
|
|
||||||
WebhookID: webhookID,
|
|
||||||
EntrypointID: event.EntrypointID,
|
|
||||||
TargetID: target.ID,
|
|
||||||
TargetName: target.Name,
|
|
||||||
TargetType: database.TargetTypeSlack,
|
|
||||||
TargetConfig: target.Config,
|
|
||||||
MaxRetries: 0,
|
|
||||||
Method: event.Method,
|
|
||||||
Headers: event.Headers,
|
|
||||||
ContentType: event.ContentType,
|
|
||||||
Body: body,
|
|
||||||
AttemptNum: attemptNum,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func tsAssertRealTimestamp(t *testing.T, text string) {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
assert.NotContains(t, text, tsZeroStamp,
|
|
||||||
"slack message carries the zero timestamp",
|
|
||||||
)
|
|
||||||
assert.Contains(t, text,
|
|
||||||
"*Timestamp:* `"+
|
|
||||||
tsEventCreatedAt().UTC().Format(time.RFC3339)+"`",
|
|
||||||
"slack message does not carry the event's receipt time",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// tsCase is one end-to-end delivery of a seeded event to a slack
|
|
||||||
// sink, over whichever engine path `process` names.
|
|
||||||
type tsCase struct {
|
|
||||||
// status is the delivery row's status before the engine runs.
|
|
||||||
// The retry path refuses a delivery that is not retrying.
|
|
||||||
status database.DeliveryStatus
|
|
||||||
|
|
||||||
// inlineBody mirrors a Task built for a body under
|
|
||||||
// MaxInlineBodySize. When false the engine reads the body back
|
|
||||||
// from the stored row.
|
|
||||||
inlineBody bool
|
|
||||||
|
|
||||||
attemptNum int
|
|
||||||
|
|
||||||
process func(
|
|
||||||
ctx context.Context, e *delivery.Engine, task *delivery.Task,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// run delivers one event through the named path and returns the
|
|
||||||
// Slack message text the sink received.
|
|
||||||
func (c tsCase) run(t *testing.T) (iSetup, database.Delivery, string) {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
s := newISetup(t)
|
|
||||||
sink := newTSSink(t)
|
|
||||||
|
|
||||||
cfg := tsSlackConfig(t, sink.URL)
|
|
||||||
target := tsSeedTarget(t, s.MainDB, s.WebhookID, cfg)
|
|
||||||
event := tsSeedEvent(t, s.WebhookDB, s.WebhookID)
|
|
||||||
|
|
||||||
d := iSeedDelivery(
|
|
||||||
t, s.WebhookDB, event.ID, target.ID, c.status,
|
|
||||||
)
|
|
||||||
|
|
||||||
var body *string
|
|
||||||
|
|
||||||
if c.inlineBody {
|
|
||||||
bodyStr := event.Body
|
|
||||||
body = &bodyStr
|
|
||||||
}
|
|
||||||
|
|
||||||
task := tsTask(
|
|
||||||
d, event, s.WebhookID, target, c.attemptNum, body,
|
|
||||||
)
|
|
||||||
|
|
||||||
c.process(context.TODO(), s.Engine, &task)
|
|
||||||
|
|
||||||
return s, d, sink.text(t)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestSlackFirstAttemptCarriesEventTimestamp covers the path an
|
|
||||||
// event takes on its first delivery: the task comes from the
|
|
||||||
// receiver and the engine reconstructs the event from it.
|
|
||||||
func TestSlackFirstAttemptCarriesEventTimestamp(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
s, d, text := tsCase{
|
|
||||||
status: database.DeliveryStatusPending,
|
|
||||||
inlineBody: true,
|
|
||||||
attemptNum: 1,
|
|
||||||
process: func(
|
|
||||||
ctx context.Context,
|
|
||||||
e *delivery.Engine,
|
|
||||||
task *delivery.Task,
|
|
||||||
) {
|
|
||||||
e.ExportProcessNewTask(ctx, task)
|
|
||||||
},
|
|
||||||
}.run(t)
|
|
||||||
|
|
||||||
tsAssertRealTimestamp(t, text)
|
|
||||||
|
|
||||||
iAssertStatus(t, s.WebhookDB, d.ID,
|
|
||||||
database.DeliveryStatusDelivered,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestSlackFirstAttemptLargeBodyCarriesEventTimestamp covers the
|
|
||||||
// first-attempt path for an event whose body exceeded
|
|
||||||
// MaxInlineBodySize, so the task carries no body and the engine
|
|
||||||
// reads it back from the stored row.
|
|
||||||
func TestSlackFirstAttemptLargeBodyCarriesEventTimestamp(
|
|
||||||
t *testing.T,
|
|
||||||
) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
_, _, text := tsCase{
|
|
||||||
status: database.DeliveryStatusPending,
|
|
||||||
inlineBody: false,
|
|
||||||
attemptNum: 1,
|
|
||||||
process: func(
|
|
||||||
ctx context.Context,
|
|
||||||
e *delivery.Engine,
|
|
||||||
task *delivery.Task,
|
|
||||||
) {
|
|
||||||
e.ExportProcessNewTask(ctx, task)
|
|
||||||
},
|
|
||||||
}.run(t)
|
|
||||||
|
|
||||||
tsAssertRealTimestamp(t, text)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestSlackRetryCarriesEventTimestamp covers the retry path, which
|
|
||||||
// reconstructs the event from the same task the first attempt used.
|
|
||||||
func TestSlackRetryCarriesEventTimestamp(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
s, d, text := tsCase{
|
|
||||||
status: database.DeliveryStatusRetrying,
|
|
||||||
inlineBody: true,
|
|
||||||
attemptNum: 2,
|
|
||||||
process: func(
|
|
||||||
ctx context.Context,
|
|
||||||
e *delivery.Engine,
|
|
||||||
task *delivery.Task,
|
|
||||||
) {
|
|
||||||
e.ExportProcessRetryTask(ctx, task)
|
|
||||||
},
|
|
||||||
}.run(t)
|
|
||||||
|
|
||||||
tsAssertRealTimestamp(t, text)
|
|
||||||
|
|
||||||
iAssertStatus(t, s.WebhookDB, d.ID,
|
|
||||||
database.DeliveryStatusDelivered,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestFormatSlackMessageOverTaskReconstructedEvent asserts on the
|
|
||||||
// formatted message directly, over the event the delivery paths
|
|
||||||
// reconstruct from a Task. It is the unit-level guard under the
|
|
||||||
// end-to-end tests: revert the CreatedAt population in hydrateEvent
|
|
||||||
// and this fails on the zero timestamp.
|
|
||||||
func TestFormatSlackMessageOverTaskReconstructedEvent(
|
|
||||||
t *testing.T,
|
|
||||||
) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
s := newISetup(t)
|
|
||||||
|
|
||||||
cfg := tsSlackConfig(t, tsUndeliverableHook)
|
|
||||||
target := tsSeedTarget(t, s.MainDB, s.WebhookID, cfg)
|
|
||||||
event := tsSeedEvent(t, s.WebhookDB, s.WebhookID)
|
|
||||||
|
|
||||||
d := iSeedDelivery(
|
|
||||||
t, s.WebhookDB, event.ID, target.ID,
|
|
||||||
database.DeliveryStatusPending,
|
|
||||||
)
|
|
||||||
|
|
||||||
bodyStr := event.Body
|
|
||||||
task := tsTask(d, event, s.WebhookID, target, 1, &bodyStr)
|
|
||||||
|
|
||||||
rebuilt, err := s.Engine.ExportEventForTask(
|
|
||||||
s.WebhookDB, &task,
|
|
||||||
)
|
|
||||||
require.NoError(t, err)
|
|
||||||
assert.False(t, rebuilt.CreatedAt.IsZero(),
|
|
||||||
"reconstructed event carries the zero time",
|
|
||||||
)
|
|
||||||
assert.Equal(t,
|
|
||||||
tsEventCreatedAt().UTC(), rebuilt.CreatedAt.UTC(),
|
|
||||||
)
|
|
||||||
|
|
||||||
tsAssertRealTimestamp(
|
|
||||||
t, delivery.FormatSlackMessage(&rebuilt),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestFormatSlackMessageZeroTimestamp asserts the rendering choice
|
|
||||||
// directly, without going through the engine: a zero CreatedAt (the
|
|
||||||
// shape a reaped-row fallback produces) renders as "unknown" rather
|
|
||||||
// than the year-1 zero time, while a real CreatedAt still renders as
|
|
||||||
// RFC3339.
|
|
||||||
func TestFormatSlackMessageZeroTimestamp(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
zeroEvent := database.Event{
|
|
||||||
Method: http.MethodPost,
|
|
||||||
ContentType: testContentType,
|
|
||||||
Body: tsEventBody,
|
|
||||||
}
|
|
||||||
|
|
||||||
zeroText := delivery.FormatSlackMessage(&zeroEvent)
|
|
||||||
|
|
||||||
assert.NotContains(t, zeroText, "0001-01-01",
|
|
||||||
"slack message carries the zero-time year",
|
|
||||||
)
|
|
||||||
assert.Contains(t, zeroText, "*Timestamp:* `unknown`",
|
|
||||||
"slack message does not mark an unset receipt time as unknown",
|
|
||||||
)
|
|
||||||
|
|
||||||
nonZeroEvent := zeroEvent
|
|
||||||
nonZeroEvent.CreatedAt = tsEventCreatedAt()
|
|
||||||
|
|
||||||
nonZeroText := delivery.FormatSlackMessage(&nonZeroEvent)
|
|
||||||
|
|
||||||
assert.Contains(t, nonZeroText,
|
|
||||||
"*Timestamp:* `"+
|
|
||||||
tsEventCreatedAt().UTC().Format(time.RFC3339)+"`",
|
|
||||||
"slack message does not render a real receipt time as RFC3339",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestEventReconstructionSurvivesAReapedRow pins the fallback: an
|
|
||||||
// event row reaped by retention while its delivery still holds the
|
|
||||||
// body inline is still delivered, with the receipt time unset,
|
|
||||||
// rather than dropped.
|
|
||||||
func TestEventReconstructionSurvivesAReapedRow(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
s := newISetup(t)
|
|
||||||
|
|
||||||
cfg := tsSlackConfig(t, tsUndeliverableHook)
|
|
||||||
target := tsSeedTarget(t, s.MainDB, s.WebhookID, cfg)
|
|
||||||
event := tsSeedEvent(t, s.WebhookDB, s.WebhookID)
|
|
||||||
|
|
||||||
d := iSeedDelivery(
|
|
||||||
t, s.WebhookDB, event.ID, target.ID,
|
|
||||||
database.DeliveryStatusPending,
|
|
||||||
)
|
|
||||||
|
|
||||||
bodyStr := event.Body
|
|
||||||
task := tsTask(d, event, s.WebhookID, target, 1, &bodyStr)
|
|
||||||
|
|
||||||
require.NoError(t, s.WebhookDB.Unscoped().Delete(
|
|
||||||
&database.Event{}, "id = ?", event.ID,
|
|
||||||
).Error)
|
|
||||||
|
|
||||||
rebuilt, err := s.Engine.ExportEventForTask(
|
|
||||||
s.WebhookDB, &task,
|
|
||||||
)
|
|
||||||
require.NoError(t, err)
|
|
||||||
assert.Equal(t, bodyStr, rebuilt.Body)
|
|
||||||
assert.True(t, rebuilt.CreatedAt.IsZero())
|
|
||||||
|
|
||||||
// A task with no inlined body has nothing left to deliver, so
|
|
||||||
// the same reaped row is an error there.
|
|
||||||
noBody := task
|
|
||||||
noBody.Body = nil
|
|
||||||
|
|
||||||
_, err = s.Engine.ExportEventForTask(s.WebhookDB, &noBody)
|
|
||||||
require.Error(t, err)
|
|
||||||
}
|
|
||||||
@@ -151,16 +151,6 @@ func (e *Engine) ExportProcessRetryTask(
|
|||||||
e.processRetryTask(ctx, task)
|
e.processRetryTask(ctx, task)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ExportEventForTask exposes the event reconstruction the delivery
|
|
||||||
// paths run: buildEventFromTask followed by hydrateEvent.
|
|
||||||
func (e *Engine) ExportEventForTask(
|
|
||||||
webhookDB *gorm.DB, task *Task,
|
|
||||||
) (database.Event, error) {
|
|
||||||
return e.hydrateEvent(
|
|
||||||
webhookDB, buildEventFromTask(task), task,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// ExportProcessDelivery exposes processDelivery.
|
// ExportProcessDelivery exposes processDelivery.
|
||||||
func (e *Engine) ExportProcessDelivery(
|
func (e *Engine) ExportProcessDelivery(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
|
|||||||
@@ -170,8 +170,7 @@ func TestDelivery_CrossOriginRedirectDropsOriginScopedHeaders(
|
|||||||
// Stripping must not fire within the configured origin, or every
|
// Stripping must not fire within the configured origin, or every
|
||||||
// destination that redirects its own path would lose its
|
// destination that redirects its own path would lose its
|
||||||
// credential and start answering 401 — and would lose the inbound
|
// credential and start answering 401 — and would lose the inbound
|
||||||
// signature header the target endpoint verifies. webhooker's own
|
// signature the receiver verifies.
|
||||||
// receiver verifies no signature; it only forwards the header.
|
|
||||||
func TestDelivery_SameOriginRedirectKeepsOriginScopedHeaders(
|
func TestDelivery_SameOriginRedirectKeepsOriginScopedHeaders(
|
||||||
t *testing.T,
|
t *testing.T,
|
||||||
) {
|
) {
|
||||||
|
|||||||
@@ -231,15 +231,10 @@ func FormatSlackMessage(
|
|||||||
event.ContentType,
|
event.ContentType,
|
||||||
)
|
)
|
||||||
|
|
||||||
timestamp := "unknown"
|
|
||||||
if !event.CreatedAt.IsZero() {
|
|
||||||
timestamp = event.CreatedAt.UTC().Format(time.RFC3339)
|
|
||||||
}
|
|
||||||
|
|
||||||
fmt.Fprintf(
|
fmt.Fprintf(
|
||||||
&b,
|
&b,
|
||||||
"*Timestamp:* `%s`\n",
|
"*Timestamp:* `%s`\n",
|
||||||
timestamp,
|
event.CreatedAt.UTC().Format(time.RFC3339),
|
||||||
)
|
)
|
||||||
|
|
||||||
fmt.Fprintf(
|
fmt.Fprintf(
|
||||||
|
|||||||
Reference in New Issue
Block a user