Author SHA1 Message Date
sneak 86224d4982 Default WEBHOOKER_ENVIRONMENT to prod
check / check (push) Successful in 4m2s
An unset WEBHOOKER_ENVIRONMENT now resolves to prod rather than dev, so
an operator who forgets the variable is not silently permissive. The
only behaviour dev still changes is the CORS middleware, which answers
every origin with Access-Control-Allow-Origin: *; that is now off
unless dev is set explicitly. Cookie Secure and CSRF strictness are
decided per request from the transport and are unaffected.

Updated resolveEnvironment and its comment, the README configuration
table and prose, and tests covering the default and the explicit dev.
Comments that justified behaviour by the old dev default (the
TRUSTED_PROXIES warning, a CSRF test) were corrected; that warning
still fires in every environment when TRUSTED_PROXIES is empty.

Model: opus-4-8
2026-09-21 13:04:56 +00:00
clawbot 39afa69bfc Consolidate the data directory mode into one owner (closes #288)
check / check (push) Successful in 6m34s
Two packages each declared the 0o750 mode for DATA_DIR and both created the directory. internal/datadir now exports DirPerm as the single definition, and internal/database uses it in both places it creates the directory. The value is unchanged, so existing deployments see no permission change. datadir owns it because guarding and creating DATA_DIR is that package's whole purpose and it imports nothing that would form a cycle.

Model: opus-4-8 (implementation and review); fable-5-1 (merge)
2026-09-21 10:01:51 +02:00
clawbotandsneak 888eaf526b State the UUID-is-the-credential rule as a rule (closes #301) (#302)
check / check (push) Successful in 4m28s
Closes #301. Docs-only apart from one test comment; no behaviour change.

The receiver has authenticated on the entrypoint UUID alone since inbound signature verification was removed in #279. The README described that as the current state. It did not say it is the decision, which leaves a future contributor free to propose HMAC as an improvement rather than as a reversal.

What changed:

- `## The entrypoint URL is the authentication secret` now states the rule: the v4 UUID at `/webhook/{uuid}` is the credential and the only one; no shared secret, HMAC signature, bearer token or second factor will be added, including as defence in depth. It names the removal that settled it, and it says explicitly that signature headers a sender sends anyway are stored and forwarded but never checked — the previous text left that ambiguous.
- The same section carries the two consequences an operator has to act on: the URL is a capability, so keep it out of logs, tickets and screenshots; and rotation means minting a new entrypoint, not changing a key.
- It also handles the case the rule will next be argued from: a sender that only supports signed payloads to a well-known URL is a constraint on that integration, to be raised on its own terms, not grounds to reintroduce shared secrets.
- The rule is reachable without scrolling 1,100 lines: a pointer in the intro, a new first bullet under Authentication (which previously listed the web UI, the API and `/metrics` and said nothing about the receiver at all), and a sharpened bullet under Security.

Stale language found and corrected: one, in `internal/delivery/redirect_test.go`. Its comment justified same-origin header retention partly by "the inbound signature the receiver verifies" — in this repo's vocabulary "the receiver" is `/webhook/{uuid}`, which verifies nothing. The endpoint that verifies it is the delivery target's, and the comment now says so.

Two places that read like stale signing language were checked and left alone as accurate: `internal/delivery/redirect.go` and `internal/server/sentry.go` describe signature headers senders put on the receiver route, which do arrive and are forwarded — neither claims webhooker checks them.

`REPO_POLICIES.md` was deliberately not touched. It is the cross-project policy document synced from `sneak/prompts` and carries `last_modified` front matter for that purpose, so a webhooker-specific carve-out does not belong in it. Worth knowing: its hardening section ends "if a standard security hardening measure exists for HTTP services and is not listed here, it is still expected. When in doubt, harden" — that is the sentence a future HMAC proposal will cite, and only the README now answers it.

`TODO.md` is untouched per its own Workflow section (issue branches do not touch it).

Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #302
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-08-30 04:05:38 +02:00
8 changed files with 96 additions and 52 deletions
+59 -22
View File
@@ -7,6 +7,13 @@ 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
@@ -85,7 +92,8 @@ them at once. A variable already present in the real environment wins
over the file's value for the same name. over the file's value for the same name.
The environment is selected by setting `WEBHOOKER_ENVIRONMENT` to `dev` The environment is selected by setting `WEBHOOKER_ENVIRONMENT` to `dev`
or `prod` (default: `dev`). The setting controls exactly one behavior: or `prod` (default: `prod`; `dev` must be set explicitly). The setting
controls exactly one behavior:
| Behavior | `dev` | `prod` | | Behavior | `dev` | `prod` |
| -------- | ----------------------- | ---------------- | | -------- | ----------------------- | ---------------- |
@@ -127,7 +135,7 @@ TTY detection, and security headers are always applied.
| Variable | Description | Default | | Variable | Description | Default |
| ----------------------- | ----------------------------------- | -------- | | ----------------------- | ----------------------------------- | -------- |
| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod` | `dev` | | `WEBHOOKER_ENVIRONMENT` | `dev` or `prod` | `prod` |
| `PORT` | HTTP listen port | `8080` | | `PORT` | HTTP listen port | `8080` |
| `BIND_ADDRESS` | IP address the HTTP listener binds. Loopback by default, so the cleartext listener is not published on every interface. The Docker image ships `0.0.0.0` instead. See [Bind address](#bind-address) | `127.0.0.1` (image: `0.0.0.0`) | | `BIND_ADDRESS` | IP address the HTTP listener binds. Loopback by default, so the cleartext listener is not published on every interface. The Docker image ships `0.0.0.0` instead. See [Bind address](#bind-address) | `127.0.0.1` (image: `0.0.0.0`) |
| `DATA_DIR` | Directory for all SQLite databases | `/var/lib/webhooker` | | `DATA_DIR` | Directory for all SQLite databases | `/var/lib/webhooker` |
@@ -393,9 +401,9 @@ the bucket is. See [Rate Limiting](#rate-limiting).
The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
address, which restores per-client buckets. webhooker logs a warning address, which restores per-client buckets. webhooker logs a warning
at startup whenever `TRUSTED_PROXIES` is empty, in every environment — at startup whenever `TRUSTED_PROXIES` is empty, in every environment —
not only when `WEBHOOKER_ENVIRONMENT=prod`, because that variable the warning does not depend on `WEBHOOKER_ENVIRONMENT`, because an
defaults to `dev` and an operator who never set it is precisely the operator who never configured the deployment is precisely the one at
one at risk. The warning is informational when nothing proxies to the risk. The warning is informational when nothing proxies to the
process: with no proxy in front, the peer address is the client's own process: with no proxy in front, the peer address is the client's own
and the buckets are already per-client. See and the buckets are already per-client. See
[Rate Limiting](#rate-limiting) for what each limit shares. [Rate Limiting](#rate-limiting) for what each limit shares.
@@ -747,15 +755,15 @@ reports.
that. that.
2. **Set `WEBHOOKER_ENVIRONMENT=prod`, and make sure the proxy sends 2. **Set `WEBHOOKER_ENVIRONMENT=prod`, and make sure the proxy sends
`X-Forwarded-Proto`.** These are two requirements, not one. The `X-Forwarded-Proto`.** These are two requirements, not one. The
environment setting decides CORS and nothing else: the default environment setting decides CORS and nothing else: `dev` answers
`dev` answers every origin with `Access-Control-Allow-Origin: *` every origin with `Access-Control-Allow-Origin: *` (without
(without credentials), which a server-rendered production credentials), which a server-rendered production deployment has no
deployment has no use for. Cookie `Secure` and the strict use for, and `prod` — the default — disables it. Cookie `Secure`
Origin/Referer mode are **not** tied to it — they are decided per and the strict Origin/Referer mode are **not** tied to it — they
request from the transport, which behind a proxy means the are decided per request from the transport, which behind a proxy
`X-Forwarded-Proto` header. The block below sets it; without it means the `X-Forwarded-Proto` header. The block below sets it;
every request is read as plaintext and cookies ship without without it every request is read as plaintext and cookies ship
`Secure`. See [Configuration](#configuration). without `Secure`. See [Configuration](#configuration).
3. **Set `TRUSTED_PROXIES` to the proxy's address.** Unset, every rate 3. **Set `TRUSTED_PROXIES` to the proxy's address.** Unset, every rate
limiter keys on the connecting peer, which behind a proxy is the limiter keys on the connecting peer, which behind a proxy is the
proxy on every request: all clients collapse into one global bucket proxy on every request: all clients collapse into one global bucket
@@ -1149,14 +1157,38 @@ backups at rest and restrict who can read them.
## The entrypoint URL is the authentication secret ## The entrypoint URL is the authentication secret
The receiver verifies nothing about an inbound request. The UUID in an **The entrypoint UUID is the credential, and it is the only one.**
entrypoint's URL is its credential: anyone who holds that URL can webhooker mints a version 4 UUID per entrypoint and serves it at
submit events to it, and the receiver checks nothing else about the `/webhook/{uuid}`. Possession of that URL is the authentication:
sender. Treat an entrypoint URL the way you would treat an API token. anyone who holds it can submit events to the entrypoint, and the
receiver verifies nothing else about the sender.
There is no way to rotate the UUID in place. To retire one, delete the There is no shared secret, no HMAC signature, no bearer token and no
entrypoint (or deactivate it, which answers `410`) and create a new second factor on the receiver, and none will be added. This was
one, then point the sender at the new URL. considered and rejected; the implementation that existed was removed
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
@@ -2867,6 +2899,10 @@ 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
@@ -2906,7 +2942,8 @@ 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 (see authorises submission, and no shared secret or signature check will
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
+9 -7
View File
@@ -585,12 +585,14 @@ func resolveMetricsAuth() (string, string, error) {
) )
} }
// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to // resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to prod
// dev, and rejects unrecognised values. // when it is unset so a deployment that forgets the variable is not
// silently permissive; dev must be set explicitly. It rejects
// unrecognised values.
func resolveEnvironment() (string, error) { func resolveEnvironment() (string, error) {
environment := os.Getenv("WEBHOOKER_ENVIRONMENT") environment := os.Getenv("WEBHOOKER_ENVIRONMENT")
if environment == "" { if environment == "" {
environment = EnvironmentDev environment = EnvironmentProd
} }
if environment != EnvironmentDev && if environment != EnvironmentDev &&
@@ -772,10 +774,10 @@ func (c *Config) warnEgressAllowlist(log *slog.Logger) {
// everyone else's wrong passwords, and the receiver's limits become // everyone else's wrong passwords, and the receiver's limits become
// service-wide ceilings. // service-wide ceilings.
// //
// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT. That // The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT: an
// variable defaults to dev, so gating on it would silence the warning // operator who never configured the deployment is exactly the case it
// for exactly the operator who forgot to configure the deployment — // exists to catch, so the exposure it announces is independent of the
// the case it exists to catch. // environment setting.
// //
// The default of trusting nobody is deliberate — trusting forwarded // The default of trusting nobody is deliberate — trusting forwarded
// headers from arbitrary peers lets any client choose its own bucket — // headers from arbitrary peers lets any client choose its own bucket —
+10 -11
View File
@@ -44,9 +44,9 @@ func TestEnvironmentConfig(t *testing.T) {
isProd bool isProd bool
}{ }{
{ {
name: "default is dev", name: "default is prod",
isDev: true, isDev: false,
isProd: false, isProd: true,
}, },
{ {
name: "explicit dev", name: "explicit dev",
@@ -848,10 +848,10 @@ func TestEgressAllowlistWarning(t *testing.T) {
// tells an operator a deployment behind a reverse proxy shares one // tells an operator a deployment behind a reverse proxy shares one
// rate-limit bucket between every client, which turns the receiver // rate-limit bucket between every client, which turns the receiver
// limits into service-wide ceilings and collapses login failure // limits into service-wide ceilings and collapses login failure
// counting. It must fire whenever TRUSTED_PROXIES is empty, // counting. It must fire whenever TRUSTED_PROXIES is empty, in any
// in any environment: WEBHOOKER_ENVIRONMENT defaults to dev, so gating // environment: the warning does not depend on WEBHOOKER_ENVIRONMENT,
// on it would silence the warning for exactly the operator who never // since an operator who never configured the deployment is exactly the
// configured the deployment. It stays quiet once proxies are named. // one it exists to catch. It stays quiet once proxies are named.
func TestSharedRateLimitBucketWarning(t *testing.T) { func TestSharedRateLimitBucketWarning(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
@@ -871,10 +871,9 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
expectWarning: false, expectWarning: false,
}, },
{ {
// The default environment. An internet-exposed // An internet-exposed deployment whose operator set
// deployment whose operator never set // WEBHOOKER_ENVIRONMENT=dev has exactly the exposure
// WEBHOOKER_ENVIRONMENT lands here and has exactly // the warning announces.
// the exposure the warning announces.
name: "dev without trusted proxies warns", name: "dev without trusted proxies warns",
environment: config.EnvironmentDev, environment: config.EnvironmentDev,
expectWarning: true, expectWarning: true,
+4 -2
View File
@@ -17,12 +17,12 @@ import (
"gorm.io/gorm" "gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/banner" "sneak.berlin/go/webhooker/internal/banner"
"sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/datadir"
"sneak.berlin/go/webhooker/internal/gormlog" "sneak.berlin/go/webhooker/internal/gormlog"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
) )
const ( const (
dataDirPerm = 0750
randomPasswordLen = 16 randomPasswordLen = 16
sessionKeyLen = 32 sessionKeyLen = 32
) )
@@ -185,7 +185,9 @@ func (d *Database) connect() error {
// caller's decision. // caller's decision.
func (d *Database) connectTo(dataDir string) error { func (d *Database) connectTo(dataDir string) error {
// Ensure the data directory exists before opening the database. // Ensure the data directory exists before opening the database.
err := os.MkdirAll(dataDir, dataDirPerm) // datadir.DirPerm is the single source of the directory mode; this
// package creates the directory too, since either may run first.
err := os.MkdirAll(dataDir, datadir.DirPerm)
if err != nil { if err != nil {
return fmt.Errorf( return fmt.Errorf(
"creating data directory %s: %w", "creating data directory %s: %w",
+4 -2
View File
@@ -13,6 +13,7 @@ import (
"gorm.io/driver/sqlite" "gorm.io/driver/sqlite"
"gorm.io/gorm" "gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/datadir"
"sneak.berlin/go/webhooker/internal/gormlog" "sneak.berlin/go/webhooker/internal/gormlog"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
) )
@@ -53,8 +54,9 @@ func NewWebhookDBManager(
log: params.Logger.Get(), log: params.Logger.Get(),
} }
// Create data directory if it doesn't exist // Create data directory if it doesn't exist. datadir.DirPerm is the
err := os.MkdirAll(m.dataDir, dataDirPerm) // single source of the directory mode; either package may run first.
err := os.MkdirAll(m.dataDir, datadir.DirPerm)
if err != nil { if err != nil {
return nil, fmt.Errorf( return nil, fmt.Errorf(
"creating data directory %s: %w", "creating data directory %s: %w",
+6 -4
View File
@@ -29,9 +29,11 @@ import (
// process that was killed with SIGKILL blocks nothing. // process that was killed with SIGKILL blocks nothing.
const LockFileName = "webhooker.lock" const LockFileName = "webhooker.lock"
// dirPerm is the mode Acquire creates DATA_DIR with. It matches what // DirPerm is the mode DATA_DIR is created with. It is the single
// internal/database uses, since whichever runs first creates it. // source of that mode: internal/database consumes it rather than
const dirPerm = 0o750 // keeping its own copy, so the two packages that both create the
// directory cannot drift into disagreeing about its permissions.
const DirPerm = 0o750
// ErrLocked reports that another live process holds the data // ErrLocked reports that another live process holds the data
// directory. Callers that need to know whether a deployment is running // directory. Callers that need to know whether a deployment is running
@@ -64,7 +66,7 @@ func Acquire(dir string) (*Lock, error) {
return nil, ErrNoDir return nil, ErrNoDir
} }
err := os.MkdirAll(dir, dirPerm) err := os.MkdirAll(dir, DirPerm)
if err != nil { if err != nil {
return nil, fmt.Errorf( return nil, fmt.Errorf(
"creating data directory %s: %w", dir, err, "creating data directory %s: %w", dir, err,
+2 -1
View File
@@ -170,7 +170,8 @@ 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 the receiver verifies. // signature header the target endpoint verifies. webhooker's own
// receiver verifies no signature; it only forwards the header.
func TestDelivery_SameOriginRedirectKeepsOriginScopedHeaders( func TestDelivery_SameOriginRedirectKeepsOriginScopedHeaders(
t *testing.T, t *testing.T,
) { ) {
+2 -3
View File
@@ -380,9 +380,8 @@ func csrfTookStrictPath(
// TestCSRF_ForwardedProtoSpellingsTakeStrictPath runs the header // TestCSRF_ForwardedProtoSpellingsTakeStrictPath runs the header
// spellings a real proxy emits through the middleware. The environment // spellings a real proxy emits through the middleware. The environment
// is dev -- the DEFAULT when WEBHOOKER_ENVIRONMENT is unset -- to pin // is set to dev -- the permissive setting -- to pin that the routing is
// that the routing is a per-request transport decision and owes // a per-request transport decision and owes nothing to configuration.
// nothing to configuration.
func TestCSRF_ForwardedProtoSpellingsTakeStrictPath(t *testing.T) { func TestCSRF_ForwardedProtoSpellingsTakeStrictPath(t *testing.T) {
t.Parallel() t.Parallel()