The Sentry SDK builds Request.URL as scheme://host/path from the
concrete path, which on the receiver route is /webhook/<uuid> in full.
That UUID is a write capability, not an identifier: anyone holding it
can post events this service accepts and its targets then deliver. A
third-party tracker has its own retention, access control and deletion
policy, so the rule the local access log follows does not carry across
that boundary.
The BeforeSend hook now rebuilds the URL from the chi route pattern,
on every route rather than by route list, since a route-conditional
rule leaks on any route someone forgets to add. Scheme and host are
kept and everything else in the URL is discarded rather than edited:
the scheme is the CSRF TLS decision the header allowlist relies on,
and the host is operator configuration already carried by the
allowlisted Host header.
The pattern is reachable only on the error dispatch, where sentryhttp
puts the request on the context the client copies onto the hint. A
finished span captures with a nil hint, so BeforeSendTransaction sees
no context; there and wherever else the pattern is missing the path
falls back to the literal /(redacted), never to the concrete path, and
a URL that will not parse into a scheme is withheld whole. A
transaction event's SDK-built "METHOD /path" name carries the same
capability and is rewritten on the same terms.
r.FormValue falls back to the query string, so
POST /source/{id}/targets?url=<secret> created a working target from a
value carried on the request line — where proxy logs, browser history
and Referer all record it. Every form read is now r.PostFormValue,
including the login password and both password-change fields, which had
the same defect in a more acute form.
The Sentry leg needed more than the query string. sentryhttp attaches
the whole request to the scope, and ApplyToEvent copies the teed body
into Request.Data with no SendDefaultPII guard — so reading every field
from the body only pointed every credential this change protects at the
one field the first revision did not scrub. Body and query are now
redacted, Cookies and Env cleared, and Headers reduced to an allowlist,
because the SDK's own filter removes four names and would otherwise ship
X-Csrf-Token and the shared secrets senders put on the receiver route.
Also adds json:"-" to Target.Config, APIKey.Key and Setting.Value —
TargetView is the masking barrier for the HTML path only, and the first
handler to marshal a model would serialise a bearer token or the session
encryption key.
Independently reviewed three times. The second review found the Data
leak and proved it with a scratch module; the third disproved the
PR's own claim that BeforeSend gets no request, so the README now
records that redacting unconditionally is a deliberate choice rather
than a limitation — which is what makes #179 cheap to fix.
In the shipped default, any stranger denied the operator the only
administrative path at 5 requests per minute: TRUSTED_PROXIES is empty,
the README requires a reverse proxy, so every login POST shared one
bucket keyed on the proxy.
Credentials are now verified first and only a FAILED attempt spends
budget, so a correct password is never throttled. Failures are counted
per (client bucket, submitted username), bounded. Concurrent Argon2id
verifications are capped at two, and the queue for them at 16 — because
verifying first lets an attacker force a 64 MB hash per request, and
bounding the wait alone bounds nothing.
The issue's own recommendation was insufficient and is rejected here:
keying by username stops an attacker locking out a DIFFERENT account,
but this is a single-admin product with a predictable bootstrap
username, so flooding the operator's own name still locks them out.
This is speculative — it implements a corrected recommendation ahead of
the owner's ruling so the decision can be made by merging or reverting.
Three things are disclosed rather than glossed: online guessing rises
from 5/min to roughly 27/s, because the 429 is a label on the response
and not a gate in front of the hash; the residual exposure is a loss of
login AVAILABILITY, not latency, and a determined flood still denies
login while it runs, at ~400x the cost and clearing the moment it
stops; and the endpoint should be provisioned for ~400 MB resident, not
the 203 MB of live commitment it itemises.
Independently reviewed four times. Reviewers disproved the suspected
FIFO starvation by measurement, then caught two successive memory
bounds the code did not have — the second by parking waiters and
reading the heap rather than checking the arithmetic.
The 8 KB render cap from #135 left storage untouched but no route served
the rest, so a body over the cap was reachable only with filesystem
access to the SQLite files — in a product whose purpose is storing
webhooks so they can be inspected.
GET /source/{sourceID}/logs/{eventID}/body serves the whole body to the
webhook's owner, as application/octet-stream with an attachment
disposition and nosniff. Those are a security control, not formatting:
the bytes come from the public receiver and are handed back inside the
operator's authenticated origin, and the existing CSP would not stop a
stored HTML payload executing there. The truncation marker links to it
only when a body was actually cut.
Accepted deviation, documented rather than glossed: #157's definition of
done asks the route to stream from the row. It buffers whole instead,
because database/sql exposes no incremental handle on a SQLite BLOB and
substr range reads re-materialise the entire column per call — an
earlier revision chunked at 64 KiB and was 11-15x slower for a worse
bound. Three independent reviewers confirmed no streaming path exists.
Independently reviewed three times. Two earlier revisions each asserted
a memory bound the code did not have; the final reviewer measured
2.057x at the ingest cap and pinned the two overlapping allocations from
source — the driver's column buffer and database/sql's convertAssign
clone — confirming the stated "roughly two bodies, and 2x is a floor
not a ceiling" is now accurate, since SQLite's own materialisation sits
outside the Go heap.
fx defaults to a 15s stop timeout and the Dockerfile sets no grace
override, so Docker SIGKILLed at 10s and the bounded shutdown #130 built
— including the log line that tells an operator a component is wedged —
was unreachable in the image this repo produces.
Sets fx.StopTimeout to 5s, and lowers the HTTP drain to 3s so a
full-length drain no longer exhausts the whole sequence budget and skip
every later hook, database close included. The Sentry flush, which runs
in the same hook and honours no context, is clamped to the remaining
stop budget less a 2s tail reserve, so a stalled flush drops Sentry
events rather than the database close.
Also fixes a latent coin flip in the shared stop-hook waiter, which
reported "shutdown timed out" about half the time for a component that
drained cleanly against an already-expired context.
Independently reviewed three times. The final reviewer derived a
stronger invariant than the implementation claims — the server hook's
absolute end is bounded at stopTimeout minus the reserve regardless of
drain length or of time consumed by preceding hooks — and confirmed the
guard's 10ms sweep cannot step over the maximum, since both breakpoints
land on its grid. Both Sentry probe arms, the docker stop demo and every
mutation were reproduced independently.
Known residual, filed separately: the HTTP drain itself is not clamped
by the reserve, so slow preceding hooks can still jointly exhaust the
budget. Demonstrated with a 2.2s sweeper delay.
Publishing this README would have shipped false statements about the
product. Corrects the eight items on the issue plus everything a full
sweep turned up: the Slack circuit-breaker scope, a nonexistent WAL, the
wrong config key for slack targets, six undocumented routes, the
conditional /metrics registration, wrong retention bands, wrong shutdown
mechanism, and a Quick Start that led a new contributor into a red
build.
The lockout warning now fires whenever TRUSTED_PROXIES is empty rather
than only in production, since the variable it was gated on defaults to
dev. Rate-limit keying, the limits and the TRUSTED_PROXIES default are
untouched — those belong to #150.
What /s/* actually serves was settled empirically rather than by
reading: all five of GET/HEAD/POST/PUT/DELETE return 200, pinned by
TestStaticServesEveryMethod. Restricting it is filed separately.
Independently reviewed after three prior rounds. The reviewer
re-derived all fifteen claim-table rows against the code, including
every row a previous revision had marked "correct, left alone" and got
wrong, and found zero false; then verified every route method-by-method,
all twelve environment variables, all nine entity tables, and the
package tree against git ls-files. The Quick Start was confirmed by
running it in a fresh clone.
static/js/alpine.min.js was a committed minified bundle, which
REPO_POLICIES forbids, referenced by no content hash at all. A minified
blob is unreviewable, which is the shape a supply-chain compromise
takes.
script/fetch-assets now downloads Alpine 3.14.9 from the npm registry
and verifies sha256 on both the tarball and the extracted file, and
static/vendor_test.go re-hashes the bytes go:embed actually placed in
the binary. The shipped bytes are byte-identical to the blob that was
committed, so the served asset does not change.
Independently reviewed. Five negative controls reproduced by the
reviewer: flipped expected hash, repointed URL, post-fetch tampering,
asset absent, and manifest inconsistencies — each fails closed with
static/js/ left clean. Registry hashes confirmed against the pins, and
the runtime image was built, run and curled to confirm the asset is
still served and the login page still loads it.
Known gap, filed separately: static/static.go embeds the js directory
rather than named files, so a missing fetched asset is not a compile
error on ungated local build paths. Every gated path fails loudly, so
the release artifact is unaffected.
The receiver was the one unauthenticated, internet-facing endpoint with no
rate limit, so a misbehaving or hostile sender could flood a webhook
without bound. RECEIVER_RATE_LIMIT (default 120/min) now caps it, keyed on
client IP plus entrypoint path so one entrypoint cannot exhaust another's
budget. Over-limit requests get 429 with Retry-After.
The limiter deliberately does not reuse postRateLimit: that helper is
POST-only and keys on IP alone, whereas the receiver must count every
method. A test locks that property in.
Config parsing follows the fail-loudly idiom: a set-but-unparseable or
non-positive value aborts startup rather than falling back to the default.
Known limitation, tracked in #88: the key still trusts forwarded headers
unconditionally, so the limit is evadable by rotating X-Forwarded-For until
trusted-proxy gating lands.
check / check (push) Superseded by a newer commit; never tested
CSRF ran before MaxBodySize, so the CSRF middleware parsed the form body
before any cap applied and an oversized request was read in full before
being rejected. MaxBodySize is now the first middleware in all four route
groups that parse forms, ahead of CSRF and RequireAuth.
An oversize request therefore gets 413 without the handler running and
without state changing, including the password-change route.
Note the ordering trade: an unauthenticated client now receives 413 rather
than an auth redirect on /user/{username}/password.
check / check (push) Superseded by a newer commit; never tested
Adds an authenticated, CSRF-protected flow that lets a user change their own password from the profile page.
## Route
- New `POST /password` under the `/user/{username}` group in `setupUserRoutes` (`internal/server/routes.go`). That group already applies `CSRF`, `NoCache`, and `RequireAuth`, so the new endpoint inherits all three.
## Handler (`internal/handlers/profile.go`)
- `HandlePasswordChange` enforces own-user access: the `{username}` path parameter must equal the session username (same 403 rule `HandleProfile` uses). This check plus the session lookup is factored into a shared `profileOwnerOrDeny` helper now used by both handlers.
- Parses `current_password`, `new_password`, and `confirm_password` (body size limited via `http.MaxBytesReader`).
- Verifies the current password with `database.VerifyPassword` against the stored hash.
- Requires the new password to be non-empty and equal to the confirmation.
- Hashes the new password with `database.HashPassword` — the same Argon2id helper used to bootstrap the admin user — and persists it on the user row. No new crypto.
- Re-renders the profile page with a clear success or error message. Wrong current password, empty new password, and mismatched confirmation are each rejected with their own message and leave the stored hash unchanged.
## Template (`templates/profile.html`)
- Adds a "Change Password" card with current / new / confirm password fields plus the hidden `csrf_token` (matching the login form's CSRF embedding).
- Renders success/error alerts using the existing `alert-success` / `alert-error` styles. No new CSS classes, so no Tailwind rebuild is required.
## Tests (`internal/handlers/profile_test.go`)
- `TestHandlePasswordChange_Success`: seeds a user, posts a valid change, asserts success message and that the stored hash changed and verifies against the new password.
- `TestHandlePasswordChange_WrongCurrentPassword`: posts a wrong current password, asserts the rejection message and that the stored hash is unchanged.
Validated with `docker build .` (fmt-check, lint, test, build) — exit 0.
Closes#65
Co-authored-by: sneak <sneak@sneak.berlin>
Co-authored-by: Jeffrey Paul <sneak@noreply.example.org>
Reviewed-on: #83
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
Adds a `NoCache()` middleware that sets `Cache-Control: no-store` and `Pragma: no-cache`, and wires it onto the dynamic app route groups (`/pages`, `/user/{username}`, `/sources`, `/source/{sourceID}`) adjacent to their existing `CSRF()` call. The static `/s` mount, `/metrics`, `/webhook/{uuid}`, and `/.well-known/healthcheck` are intentionally left untouched (static assets are safe to cache; the others are not authenticated pages).
A middleware unit test asserts both headers are set.
Closes#61
Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #75
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
Enforces authentication for the `/user/{username}` route group at the middleware layer, matching every other authenticated route group.
## Changes
- **`internal/server/routes.go`** (`setupUserRoutes`): added `r.Use(s.mw.RequireAuth())` immediately after the existing `r.Use(s.mw.CSRF())` on the `/user/{username}` group, so auth is enforced by design (CSRF first, then RequireAuth) — consistent with `/sources` and `/source/{sourceID}`.
- **`internal/handlers/profile.go`** (`HandleProfile`): removed the now-dead unauthenticated-redirect branch (RequireAuth guarantees an authenticated session before the handler runs). The handler still reads the username and user id from the session for the own-profile-only check; a request for another user's profile still returns 403. The session-retrieval error is now handled as a 500.
## Tests (`internal/handlers/profile_test.go`)
- own profile returns 200
- another user's profile returns 403
- an unauthenticated request to `/user/{username}` is redirected to `/pages/login` at the middleware layer and never reaches the endpoint handler (routing-level test replicating the CSRF + RequireAuth chain)
## Validation
`docker build .` (fmt-check, lint, test, build) passes.
Closes#60
Co-authored-by: sneak <sneak@sneak.berlin>
Co-authored-by: Jeffrey Paul <sneak@noreply.example.org>
Reviewed-on: #71
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
Raise `httpWriteTimeout` in `internal/server/http.go` from 10s to `65 * time.Second` so it comfortably exceeds the router's 60s `requestTimeout`. This makes the `middleware.Timeout(60s)` the effective request limit — a slow response now returns a clean 503 from the middleware instead of being cut at the socket write deadline by the transport.
`httpReadTimeout` stays at 10s. A comment on `httpWriteTimeout` documents that it must remain above the 60s request timeout. Change is confined to `internal/server/http.go`; `routes.go` is untouched.
Closes#62
Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #72
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
Closes [issue #50](#50)
## Summary
Refactors the Dockerfile to use a separate lint stage with a pinned golangci-lint Docker image, following the pattern used by [sneak/pixa](https://git.eeqj.de/sneak/pixa). This replaces the previous approach of installing golangci-lint via curl in the builder stage.
## Changes
### Dockerfile
- **New `lint` stage** using `golangci/golangci-lint:v2.11.3` (Debian-based, pinned by sha256 digest) as a separate build stage
- **Builder stage** depends on lint via `COPY --from=lint /src/go.sum /dev/null` — build won't proceed unless linting passes
- **Go bumped** from 1.24 to 1.26.1 (`golang:1.26.1-bookworm`, pinned by sha256)
- **golangci-lint bumped** from v1.64.8 to v2.11.3
- All three Docker images (golangci-lint, golang, alpine) pinned by sha256 digest
- Debian-based golangci-lint image used (not Alpine) because mattn/go-sqlite3 CGO does not compile on musl (off64_t)
### Linter Config (.golangci.yml)
- Migrated from v1 to v2 format (`version: "2"` added)
- Removed linters no longer available in v2: `gofmt` (handled by `make fmt-check`), `gosimple` (merged into `staticcheck`), `typecheck` (always-on in v2)
- Same set of linters enabled — no rules weakened
### Code Fixes (all lint issues from v2 upgrade)
- Added package comments to all packages
- Added doc comments to all exported types, functions, and methods
- Fixed unchecked errors flagged by `errcheck` (sqlDB.Close, os.Setenv in tests, resp.Body.Close, fmt.Fprint)
- Fixed unused parameters flagged by `revive` (renamed to `_`)
- Fixed `gosec` G120 warnings: added `http.MaxBytesReader` before `r.ParseForm()` calls
- Fixed `staticcheck` QF1012: replaced `WriteString(fmt.Sprintf(...))` with `fmt.Fprintf`
- Fixed `staticcheck` QF1003: converted if/else chain to tagged switch
- Renamed `DeliveryTask` → `Task` to avoid package stutter (`delivery.Task` instead of `delivery.DeliveryTask`)
- Renamed shadowed builtin `max` parameter to `upperBound` in `cryptoRandInt`
- Used `t.Setenv` instead of `os.Setenv` in tests (auto-restores)
### README.md
- Updated version requirements: Go 1.26+, golangci-lint v2.11+
- Updated Dockerfile description in project structure
## Verification
`docker build .` passes cleanly — formatting check, linting, all tests, and build all succeed.
Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de>
Reviewed-on: #55
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
## Summary
This PR implements three security hardening measures:
### Security Headers Middleware (closes #34)
Adds a `SecurityHeaders()` middleware applied globally to all routes. Every response now includes:
- `Strict-Transport-Security: max-age=63072000; includeSubDomains; preload`
- `X-Content-Type-Options: nosniff`
- `X-Frame-Options: DENY`
- `Content-Security-Policy: default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'`
- `Referrer-Policy: strict-origin-when-cross-origin`
- `Permissions-Policy: camera=(), microphone=(), geolocation=()`
### Session Fixation Prevention (closes #38)
Adds a `Regenerate()` method to the session manager that destroys the old session and creates a new one with a fresh ID, copying all session values. Called after successful login to prevent session fixation attacks.
### Request Body Size Limits (closes #39)
Adds a `MaxBodySize()` middleware using `http.MaxBytesReader` to limit POST/PUT/PATCH request bodies to 1 MB. Applied to all form endpoints (`/pages`, `/sources`, `/source/*`).
## Files Changed
- `internal/middleware/middleware.go` — Added `SecurityHeaders()` and `MaxBodySize()` middleware
- `internal/session/session.go` — Added `Regenerate()` method for session fixation prevention
- `internal/handlers/auth.go` — Updated login handler to regenerate session after authentication
- `internal/server/routes.go` — Added SecurityHeaders globally, MaxBodySize to form route groups
- `README.md` — Documented new middleware in stack, updated Security section, moved items to completed TODO
closes #34, closes #38, closes #39
Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de>
Reviewed-on: #41
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
DBURL → DATA_DIR consolidation:
- Remove DBURL env var entirely; main DB now lives at {DATA_DIR}/webhooker.db
- database.go constructs DB path from config.DataDir, ensures dir exists
- Update DATA_DIR prod default from /data/events to /data
- Update all tests to use DataDir instead of DBURL
- Update Dockerfile: /data (not /data/events) for all SQLite databases
- Update README configuration table, Docker examples, architecture docs
Dead code removal:
- Remove unused IndexResponse struct (handlers/index.go)
- Remove unused TemplateData struct (handlers/handlers.go)
Stale comment cleanup:
- Remove TODO in server.go (DB cleanup handled by fx lifecycle)
- Fix nolint:golint → nolint:revive on ServerParams for consistency
- Clean up verbose middleware/routing comments in routes.go
- Fix TODO fan-out description (worker pool, not goroutine-per-target)
.gitignore fixes:
- Add data/ directory to gitignore
- Remove stale config.yaml entry (env-only config since rework)
The "database" target type now writes events to a separate
archived_events table instead of just marking the delivery as done.
This table persists independently of internal event retention/pruning,
allowing the data to be consumed by external systems or preserved
indefinitely.
New ArchivedEvent model copies the full event payload (method, headers,
body, content_type) along with webhook/entrypoint/event/target IDs.
Add toggle (activate/deactivate) and delete buttons for individual
entrypoints and targets on the webhook detail page. Each action is a
POST form submission with ownership verification.
New routes:
POST /source/{id}/entrypoints/{entrypointID}/delete
POST /source/{id}/entrypoints/{entrypointID}/toggle
POST /source/{id}/targets/{targetID}/delete
POST /source/{id}/targets/{targetID}/toggle
The serve() method called cleanShutdown() after ctx.Done(), and the fx
OnStop hook also called cleanShutdown(). Remove the call in serve() so
shutdown happens exactly once via the fx lifecycle.
Add method check at the top of HandleWebhook, returning 405 Method Not
Allowed with an Allow: POST header for any non-POST request. This
prevents GET, PUT, DELETE, etc. from being accepted at entrypoint URLs.
Add RequireAuth middleware that checks for a valid session and
redirects unauthenticated users to /pages/login. Applied to all
/sources and /source/{sourceID} routes. The middleware uses the
existing session package for authentication checks.
closes #9
The top-level entity that groups entrypoints and targets is now called
Webhook (was Processor). The inbound URL endpoint entity is now called
Entrypoint (was Webhook). This rename affects database models, handler
comments, routes, and README documentation.
closes #12
## Summary
This PR brings the webhooker repo into full REPO_POLICIES compliance, addressing both [issue #1](#1) and [issue #2](#2).
## Changes
### New files
- **`cmd/webhooker/main.go`** — The missing application entry point. Uses Uber fx to wire together all internal packages (config, database, logger, server, handlers, middleware, healthcheck, globals, session). Minimal glue code.
- **`REPO_POLICIES.md`** — Fetched from authoritative source (`sneak/prompts`)
- **`.editorconfig`** — Fetched from authoritative source
- **`.dockerignore`** — Sensible Go project exclusions
- **`.gitea/workflows/check.yml`** — CI workflow that runs `docker build .` on push to any branch (Gitea Actions format, actions/checkout pinned by sha256)
- **`configs/config.yaml.example`** — Moved from root `config.yaml`
### Modified files
- **`Makefile`** — Complete rewrite with all REPO_POLICIES required targets: `test`, `lint`, `fmt`, `fmt-check`, `check`, `build`, `hooks`, `docker`, `clean`, plus `dev`, `run`, `deps`
- **`Dockerfile`** — Complete rewrite:
- Builder: `golang:1.24` (Debian-based, pinned by `sha256:d2d2bc1c84f7...`). Debian needed because `gorm.io/driver/sqlite` pulls `mattn/go-sqlite3` (CGO) which fails on Alpine musl.
- golangci-lint v1.64.8 installed from GitHub release archive with sha256 verification (v1.x because `.golangci.yml` uses v1 config format)
- Runs `make check` (fmt-check + lint + test + build) as build step
- Final stage: `alpine:3.21` (pinned by `sha256:c3f8e73fdb79...`) with non-root user, healthcheck, port 8080
- **`README.md`** — Rewritten with all required REPO_POLICIES sections: description line with name/purpose/category/license/author, Getting Started, Rationale, Design, TODO (integrated from TODO.md), License, Author
- **`.gitignore`** — Fixed `webhooker` pattern to `/webhooker` (was blocking `cmd/webhooker/`), added `config.yaml` to prevent committing runtime config with secrets
- **`static/static.go`** — Removed `vendor` from embed directive (directory was empty/missing)
- **`internal/database/database_test.go`** — Fixed to use in-memory config via `afero.MemMapFs` instead of depending on `config.yaml` on disk. Test is now properly isolated.
- **`go.mod`/`go.sum`** — `go mod tidy`
### Removed files
- **`TODO.md`** — Content integrated into README.md TODO section
- **`config.yaml`** — Moved to `configs/config.yaml.example`
## Verification
- `docker build .` passes (lint ✅, test ✅, build ✅)
- All existing tests pass with no modifications to assertions or test logic
- `.golangci.yml` untouched
closes #1
closes #2
Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de>
Reviewed-on: #6
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
