From ab63b5f7779ceed9e60ba8126a092daa5e5786b7 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 11:10:27 +0200 Subject: [PATCH 1/4] Restrict /s/* to GET and HEAD (closes #169) The static file server was attached with Mount, which registers every method, so POST, PUT and DELETE on an asset were answered 200 with the file. It is now registered for GET and HEAD only, inside a /s group whose method-not-allowed handler answers 405 with Allow: GET, HEAD. A method chi does not route at all, such as PROPFIND, still gets 405 from the top-level router, without Allow. The inverted test and the README route table say the same. Model: opus-5-5 --- README.md | 2 +- internal/server/routes.go | 20 +++++++++++-- internal/server/routes_test.go | 55 ++++++++++++++++++++++++---------- 3 files changed, 57 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 5729606..8a3ad14 100644 --- a/README.md +++ b/README.md @@ -2721,7 +2721,7 @@ abuse limit later; they are tracked as future work. | ------ | --------------------------- | ----------- | | `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) | | `GET` | `/.well-known/healthcheck` | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) | -| any | `/s/*` | Static file serving (embedded CSS, JS). Mounted for every method, not just `GET`/`HEAD`: chi's `Mount` registers all methods and `http.FileServer` special-cases only `HEAD` (by omitting the body), so a `POST` or `DELETE` to an asset is answered `200` with the file. Pinned by `TestStaticServesEveryMethod` | +| `GET`, `HEAD` | `/s/*` | Static file serving (embedded CSS, JS). `GET` and `HEAD` only — `POST`, `PUT`, `PATCH`, `DELETE`, `OPTIONS`, `TRACE` and `CONNECT` are answered `405 Method Not Allowed` with `Allow: GET, HEAD`. Any other method (such as `PROPFIND`) is refused by chi before it reaches this route, and gets `405` without an `Allow` header. Pinned by `TestStaticServesOnlyGetAndHead` | | `POST` | `/webhook/{uuid}` | Webhook receiver endpoint. `POST` only — every other method is answered `405 Method Not Allowed` with `Allow: POST`. Rate limited (see [Rate Limiting](#rate-limiting)) | #### Authentication Endpoints diff --git a/internal/server/routes.go b/internal/server/routes.go index 05872f8..7f3699a 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -92,11 +92,25 @@ func (s *Server) setupGlobalMiddleware() { func (s *Server) setupRoutes() { s.router.Get("/", s.h.HandleIndex()) - s.router.Mount( - "/s", - http.StripPrefix("/s", http.FileServer(http.FS(static.Static))), + // Static assets answer GET and HEAD only. chi's default 405 + // carries no Allow header, so this group supplies its own. + staticFiles := http.StripPrefix( + "/s", http.FileServer(http.FS(static.Static)), ) + s.router.Route("/s", func(r chi.Router) { + r.MethodNotAllowed(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Allow", "GET, HEAD") + http.Error( + w, + "Method Not Allowed", + http.StatusMethodNotAllowed, + ) + }) + r.Method(http.MethodGet, "/*", staticFiles) + r.Method(http.MethodHead, "/*", staticFiles) + }) + s.router.Route("/api/v1", func(_ chi.Router) { // API routes will be added here. }) diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index 97937ee..429a7f7 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -396,13 +396,15 @@ func (e *testEnv) storedHash(t *testing.T, username string) string { // --- /s static group --- -// TestStaticServesEveryMethod pins what the static mount actually -// answers. chi's Mount registers the handler for all methods and -// http.FileServer only special-cases HEAD (by suppressing the body), -// so a POST or a DELETE to an asset is served the file rather than -// refused. The README documents this; the test is what keeps the two -// from drifting. -func TestStaticServesEveryMethod(t *testing.T) { +// TestStaticServesOnlyGetAndHead pins the methods the static group +// answers: GET and HEAD are served the asset, and the other methods +// chi routes (POST, PUT, DELETE and the rest) are refused with 405 +// and an Allow header naming those two. A method chi does not route, +// such as PROPFIND, is refused with 405 by the top-level router +// before it reaches the static group, so it gets no Allow header. +// The README documents this; the test is what keeps the two from +// drifting. +func TestStaticServesOnlyGetAndHead(t *testing.T) { t.Parallel() env := newTestEnv(t) @@ -417,6 +419,7 @@ func TestStaticServesEveryMethod(t *testing.T) { http.MethodPost, http.MethodPut, http.MethodDelete, + "PROPFIND", } { t.Run(method, func(t *testing.T) { t.Parallel() @@ -428,18 +431,38 @@ func TestStaticServesEveryMethod(t *testing.T) { w := httptest.NewRecorder() env.router.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code, - "static mount answers every method") - - if method == http.MethodHead { + switch method { + case http.MethodGet: + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, body, w.Body.Bytes(), + "the asset itself is returned") + case http.MethodHead: + assert.Equal(t, http.StatusOK, w.Code) assert.Empty(t, w.Body.Bytes(), "HEAD must not carry a body") - - return + case "PROPFIND": + assert.Equal( + t, http.StatusMethodNotAllowed, w.Code, + ) + assert.Empty(t, w.Header().Get("Allow"), + "chi refuses a method it does not route "+ + "before the static group runs") + assert.NotContains( + t, w.Body.String(), string(body), + "a refused method must not get the asset", + ) + default: + assert.Equal( + t, http.StatusMethodNotAllowed, w.Code, + ) + assert.Equal( + t, "GET, HEAD", w.Header().Get("Allow"), + ) + assert.NotContains( + t, w.Body.String(), string(body), + "a refused method must not get the asset", + ) } - - assert.Equal(t, body, w.Body.Bytes(), - "the asset itself is returned") }) } } -- 2.54.0 From 8ad2a86e4b370f2f84729dd45539230544c9404f Mon Sep 17 00:00:00 2001 From: Jeffrey Paul <1+sneak@noreply.example.org> Date: Tue, 29 Sep 2026 12:01:33 +0200 Subject: [PATCH 2/4] Sneak/testdeploy (#356) Reviewed-on: https://git.eeqj.de/sneak/webhooker/pulls/356 --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 8a3ad14..0536a4e 100644 --- a/README.md +++ b/README.md @@ -3280,3 +3280,5 @@ MIT ## Author [@sneak](https://sneak.berlin) + + -- 2.54.0 From 1428154bbd95855c07685e843db3604c1eb91590 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 12:58:44 +0200 Subject: [PATCH 3/4] Let a browser with cookies from an earlier database log in (closes #359) A new database brings a new session key. A browser still holding the old session cookie got a 500 on a correct login: Session.Get returned the cookie's decode error and the login handler answered it with a 500. Get now treats a cookie that does not decode as absent, and logging in replaces it. gorilla/csrf already did the same for the CSRF cookie. A start that creates webhooker.db now logs "created a new, empty database" at WARN with its path, shortly before the first-boot banner, so an unexpectedly empty DATA_DIR is noticed. The codec tests now decode through the store, since Get no longer reports the codec's reason. Model: opus-5-5 --- README.md | 6 ++ internal/database/bootstrap_banner_test.go | 36 +++++++++++ internal/database/database.go | 14 ++++- internal/server/routes_test.go | 72 +++++++++++++++++++++- internal/session/codec_test.go | 15 ++--- internal/session/session.go | 14 ++++- 6 files changed, 145 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 0536a4e..25be1bf 100644 --- a/README.md +++ b/README.md @@ -538,6 +538,12 @@ its Argon2id hash. There is no second account and no forgot-password flow, so the banner and the reset command below are the only two ways in. +A start that finds no `webhooker.db` in `DATA_DIR` also logs +`created a new, empty database` at `WARN`, with the file's path, +shortly before the banner. On a deployment that has run before, that +line means `DATA_DIR` was empty, most often because its volume is not +mounted. + #### Recovering a lost admin password `webhooker resetpw` sets an existing account's password from the diff --git a/internal/database/bootstrap_banner_test.go b/internal/database/bootstrap_banner_test.go index f56b467..7f7860d 100644 --- a/internal/database/bootstrap_banner_test.go +++ b/internal/database/bootstrap_banner_test.go @@ -3,6 +3,8 @@ package database_test import ( "bytes" "context" + "log/slog" + "path/filepath" "strings" "testing" @@ -83,3 +85,37 @@ func TestFirstBoot_PrintsTheAdminPasswordAsABanner(t *testing.T) { t, ok, "the printed password must open the seeded account", ) } + +// TestNewDatabase_IsLoggedWithItsPath is the log half of +// https://git.eeqj.de/sneak/webhooker/issues/359. A DATA_DIR that is +// unexpectedly empty boots exactly like a first start, so the start +// that creates the database must say so, and where. Opening that +// database again must not. +func TestNewDatabase_IsLoggedWithItsPath(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + open := func() string { + var out bytes.Buffer + + db, err := database.Open(dir, slog.New(slog.NewTextHandler(&out, nil))) + require.NoError(t, err) + require.NoError(t, db.Close()) + + return out.String() + } + + const created = `level=WARN msg="created a new, empty database"` + + first := open() + second := open() + + assert.Contains( + t, first, + created+" path="+filepath.Join(dir, database.MainDBFileName), + ) + assert.NotContains( + t, second, created, "an existing database is not new", + ) +} diff --git a/internal/database/database.go b/internal/database/database.go index ba28bae..da8bd37 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "io/fs" "log/slog" "os" "path/filepath" @@ -199,6 +200,12 @@ func (d *Database) connectTo(dataDir string) error { // Construct the main application database path inside DATA_DIR. dbPath := filepath.Join(dataDir, MainDBFileName) + // Checked before opening, which creates the file. A DATA_DIR that + // is unexpectedly empty -- its volume not mounted, say -- looks + // exactly like a first start, so a new database is a warning. + _, statErr := os.Stat(dbPath) + created := errors.Is(statErr, fs.ErrNotExist) + // Opened through OpenSQLite so this handle carries the same WAL // journaling, busy timeout, immediate-transaction locking, and pool // bounds as every other database file. See sqlite_open.go. @@ -229,7 +236,12 @@ func (d *Database) connectTo(dataDir string) error { } d.db = db - d.log.Info("connected to database", "path", dbPath) + + if created { + d.log.Warn("created a new, empty database", "path", dbPath) + } else { + d.log.Info("connected to database", "path", dbPath) + } // Run migrations return d.migrate() diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index 429a7f7..a06272f 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "net/url" "regexp" + "slices" "strconv" "strings" "testing" @@ -220,9 +221,21 @@ func (e *testEnv) csrfFrom( // out of the markup has to be unescaped before it is submitted. token := html.UnescapeString(match[1]) - combined := make([]*http.Cookie, 0, len(cookies)) - combined = append(combined, cookies...) - combined = append(combined, w.Result().Cookies()...) + // A cookie the page sets replaces the one of the same name, as in + // a browser. Sent both, the server would read the first, older one. + set := w.Result().Cookies() + combined := make([]*http.Cookie, 0, len(cookies)+len(set)) + + for _, c := range cookies { + replaced := slices.ContainsFunc(set, func(n *http.Cookie) bool { + return n.Name == c.Name + }) + if !replaced { + combined = append(combined, c) + } + } + + combined = append(combined, set...) return token, combined } @@ -614,6 +627,59 @@ func TestPagesLogin_CorrectPasswordSurvivesASpentBudget( ) } +// TestPagesLogin_CookiesFromAnEarlierDatabase is +// https://git.eeqj.de/sneak/webhooker/issues/359. A new database +// brings a new session key, and the operator's browser still holds +// the session and CSRF cookies signed with the old one. Logging in +// must work as from a fresh browser and leave cookies the new key +// accepts. +func TestPagesLogin_CookiesFromAnEarlierDatabase(t *testing.T) { + t.Parallel() + + const ( + username = "operator" + password = "correct-horse-battery-staple" + ) + + earlier := newTestEnv(t) + earlierID, _ := earlier.seedUser(t, username, password) + _, stale := earlier.csrfFrom(t, "/pages/login", nil) + stale = append(stale, earlier.authCookies(t, earlierID, username)...) + + env := newTestEnv(t) + env.seedUser(t, username, password) + + token, cookies := env.csrfFrom(t, "/pages/login", stale) + + form := url.Values{} + form.Set("csrf_token", token) + form.Set("username", username) + form.Set("password", password) + + w := env.post("/pages/login", form, cookies) + require.Equal( + t, http.StatusSeeOther, w.Code, + "a session cookie from another key must not fail the login", + ) + + // The response deletes the old session cookie and then sets the + // new one; a browser keeps the last. + var fresh *http.Cookie + + for _, c := range w.Result().Cookies() { + if c.Name == session.SessionName { + fresh = c + } + } + + require.NotNil(t, fresh, "login must set a session cookie") + assert.Equal( + t, "/sources", + env.get("/", []*http.Cookie{fresh}).Header().Get("Location"), + "the new session cookie must authenticate", + ) +} + // --- /user/{username} group --- // TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged diff --git a/internal/session/codec_test.go b/internal/session/codec_test.go index f181b25..08e5a0a 100644 --- a/internal/session/codec_test.go +++ b/internal/session/codec_test.go @@ -19,8 +19,8 @@ import ( ) // The tests below exercise the securecookie codecs underneath the -// store and nothing else: Session.Get only decodes, so no server-side -// expiry check takes part in the result. They exist because +// store and nothing else: they decode through the store itself, so no +// server-side expiry check takes part in the result. They exist because // NewCookieStore gives its codecs a 30-day max age that assigning // store.Options does not override, which would let the codec accept a // cookie weeks past the cap the cookie attribute advertises. @@ -75,10 +75,11 @@ func restamp( return base64.URLEncoding.EncodeToString(payload) } -// decodeCookie feeds value back through the store's decode path. +// decodeCookie feeds value back through the store's decode path. It +// asks the store rather than Session.Get, which treats a cookie that +// does not decode as absent and so hides the codec's reason. func decodeCookie( t *testing.T, - s *session.Session, value string, ) (*sessions.Session, error) { t.Helper() @@ -94,7 +95,7 @@ func decodeCookie( SameSite: http.SameSiteLaxMode, }) - sess, err := s.Get(req) + sess, err := session.NewStore(testKey()).Get(req, session.SessionName) require.NotNil(t, sess) return sess, err @@ -105,7 +106,7 @@ func TestCodec_AcceptsCookieInsideAbsoluteCap(t *testing.T) { s := testSession(t) - sess, err := decodeCookie(t, s, restamp( + sess, err := decodeCookie(t, restamp( t, issuedCookie(t, s), time.Now().Add(-(testAbsoluteMaxAge-time.Hour)), @@ -126,7 +127,7 @@ func TestCodec_RejectsCookiePastAbsoluteCap(t *testing.T) { s := testSession(t) - sess, err := decodeCookie(t, s, restamp( + sess, err := decodeCookie(t, restamp( t, issuedCookie(t, s), time.Now().Add(-(testAbsoluteMaxAge+time.Hour)), diff --git a/internal/session/session.go b/internal/session/session.go index 97b6664..8e97734 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -224,10 +224,22 @@ func New( } // Get retrieves a session for the request. +// +// A session cookie that does not decode -- one signed with an earlier +// session key, say, because the database was made anew -- is treated +// as absent: the caller gets a new, empty session and no error, and +// the next save replaces the cookie. func (s *Session) Get( r *http.Request, ) (*sessions.Session, error) { - return s.store.Get(r, SessionName) + sess, err := s.store.Get(r, SessionName) + if sess == nil { + return nil, err + } + + // For a cookie that does not decode, gorilla/sessions returns a + // new, empty session alongside the error that is dropped here. + return sess, nil } // GetKey returns the raw 32-byte authentication key used for -- 2.54.0 From b79e4649a13e62432e4156095fbe7c4f325140a6 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 13:05:16 +0200 Subject: [PATCH 4/4] Container sets its data directory's owner and mode itself (#353) Closes https://git.eeqj.de/sneak/webhooker/issues/340. The image no longer sets `USER`. Its new `ENTRYPOINT`, `deploy/docker-entrypoint.sh`, starts as root, creates `DATA_DIR` if missing, gives the directory and anything in it owned by another user to `webhooker` (UID 1000), sets the directory to `0750`, and runs the command as `webhooker` through `su-exec`. An empty root-owned bind mount, or data left by another UID, now works as mounted; the app never runs as root and is still PID 1. `CMD` is still `/app/webhooker`, so the `resetpw` commands are unchanged. Started with `--user`, the script only runs the command. It is in `/usr/local/bin`, not `/app`, which belongs to `webhooker`. README: the UID 1000 ownership block, the upaas pre-deploy commands and the restore ownership step are gone; the upaas volume bullet names only the path. - Judgement call: `su-exec` over `setpriv`: Alpine's small tool for this, needing only musl; busybox's `setpriv` cannot change user, and util-linux's adds `libcap-ng`. - Deviation: `su-exec` is pinned by version (`0.2-r3`), not by hash; `ca-certificates` beside it is unpinned. - Judgement call: each start reads every entry's owner but changes only entries owned by someone else. - `docker exec` and the health check now run as root, since the image sets no `USER`. - No automated test covers the script: the suite runs inside `docker build`, which cannot start a container. - A missing host directory under upaas is https://git.eeqj.de/sneak/upaas/issues/235. Model: opus-5-5 Reviewed-on: https://git.eeqj.de/sneak/webhooker/pulls/353 Co-authored-by: clawbot <35+clawbot@noreply.example.org> --- Dockerfile | 11 +++- README.md | 122 +++++++++++++----------------------- deploy/docker-entrypoint.sh | 22 +++++++ 3 files changed, 76 insertions(+), 79 deletions(-) create mode 100755 deploy/docker-entrypoint.sh diff --git a/Dockerfile b/Dockerfile index bb47620..907ae40 100644 --- a/Dockerfile +++ b/Dockerfile @@ -88,7 +88,9 @@ RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static # alpine:3.21, 2026-03-17 FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709 -RUN apk --no-cache add ca-certificates +# su-exec 0.2-r3 (Alpine 3.21), 2026-09-29: the entrypoint runs the app +# as webhooker with it. +RUN apk --no-cache add ca-certificates su-exec=0.2-r3 # Create non-root user RUN addgroup -g 1000 -S webhooker && \ @@ -99,13 +101,17 @@ WORKDIR /app # Copy binary from builder COPY --from=builder /build/bin/webhooker /app/webhooker +# Not under /app, which belongs to webhooker: this script runs as root. +COPY deploy/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh + # Create data directory for all SQLite databases (main app DB + # per-webhook event DBs). DATA_DIR defaults to /var/lib/webhooker. RUN mkdir -p /var/lib/webhooker RUN chown -R webhooker:webhooker /app /var/lib/webhooker -USER webhooker +# No USER: the entrypoint starts as root to make the data directory +# webhooker's, then runs the app as webhooker. EXPOSE 8080 @@ -124,4 +130,5 @@ ENV BIND_ADDRESS=0.0.0.0 HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ CMD wget --no-verbose --tries=1 --spider http://localhost:8080/.well-known/healthcheck || exit 1 +ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"] CMD ["/app/webhooker"] diff --git a/README.md b/README.md index 25be1bf..c1bb19b 100644 --- a/README.md +++ b/README.md @@ -558,8 +558,9 @@ printf '%s' "$NEW_PASSWORD" | \ DATA_DIR=/var/lib/webhooker webhooker resetpw admin ``` -In a container it is the same binary, which the image sets as `CMD` -rather than `ENTRYPOINT`, so the whole command has to be given: +In a container it is the same binary. The image's `CMD` is +`/app/webhooker`, and a command given to `docker run` replaces all of +it, so the whole command has to be given: ```bash docker run --rm -v webhooker-data:/var/lib/webhooker \ @@ -696,38 +697,22 @@ those three values rather than trusting the figure. Measured at 65s on Docker 29.7.2.) A container `unhealthy` with `connection refused` in its health log, or a published port that resets connections, is this. -The container runs as a non-root user (`webhooker`, UID 1000), exposes -port 8080, and includes a health check against -`/.well-known/healthcheck`. The `/var/lib/webhooker` volume holds all -SQLite databases: the main application database (`webhooker.db`), the -per-webhook event databases (`events-{uuid}.db`), and any archive -databases written by `database` targets (`archive-{uuid}.db`). Mount -this as a persistent volume to preserve data across container -restarts. +The app runs as a non-root user (`webhooker`, UID 1000), exposes port +8080, and includes a health check against `/.well-known/healthcheck`. +The `/var/lib/webhooker` volume holds all SQLite databases: the main +application database (`webhooker.db`), the per-webhook event databases +(`events-{uuid}.db`), and any archive databases written by `database` +targets (`archive-{uuid}.db`). Mount this as a persistent volume to +preserve data across container 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 container sets its data directory's owner and mode itself +before the app starts**, so a host directory can be mounted as it is, +whoever owns it. The image's `ENTRYPOINT`, +`deploy/docker-entrypoint.sh`, starts as root, creates `DATA_DIR` if +it is missing, gives the directory and anything in it that belongs to +another user to `webhooker`, sets the directory to `0750`, and only +then runs the app as `webhooker`. Started with `--user`, it changes +nothing and runs the app as that user. **The file modes are not yours to set, and do not depend on the directory.** `webhooker.db` holds target configuration in plaintext — @@ -735,13 +720,10 @@ 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. +they are opened. The directory's `0750` 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. ### Running under upaas @@ -757,17 +739,6 @@ repository's `Dockerfile` and runs it. The app needs: app name, port `8080`. Leave `PORT` unset: the image's health check probes `8080`. - **Volume:** one host directory mounted at `/var/lib/webhooker`. - upaas bind-mounts the host path it is given and does not create it, - and the container does not start unless UID 1000 owns it (see - [Running with Docker](#running-with-docker)). Create it before the - first deploy: - - ```bash - mkdir -p /path/to/data - chown 1000:1000 /path/to/data - chmod 750 /path/to/data - ``` - - **Environment variables:** - `WEBHOOKER_ENVIRONMENT=prod` - `TRUSTED_PROXIES`: your reverse proxy's address on that Docker @@ -1026,12 +997,12 @@ done `.backup` reads through the WAL and writes a single consistent file with no sidecars of its own, so the destination is complete as it stands. Two caveats. First, the runtime image is `alpine:3.21` with only -`ca-certificates` added — the `sqlite3` CLI is **not** in it, so run -this on the host against the volume path, or from a throwaway container -that mounts the volume. Second, each file is captured at its own -instant, so a webhook created or an event delivered between two files -being copied lands in one and not the other. If you need the whole set -coherent as of a single moment, stop the service. +`ca-certificates` and `su-exec` added — the `sqlite3` CLI is **not** in +it, so run this on the host against the volume path, or from a +throwaway container that mounts the volume. Second, each file is +captured at its own instant, so a webhook created or an event delivered +between two files being copied lands in one and not the other. If you +need the whole set coherent as of a single moment, stop the service. Note that `sqlite3 .dump` is **not** one of these procedures: it is an export, it holds a read transaction open for as long as it runs, and @@ -1085,21 +1056,11 @@ with any `-wal`/`-shm` beside it, or wait until there are none. archive not opened since a crash. A copy salvaged from a crashed instance has them for everything, and needs all of them. -4. **Fix ownership.** The container runs as the non-root `webhooker` - user, UID 1000 / GID 1000. Restored files must be owned by (or - writable by) that UID, and so must the directory itself — SQLite - creates the `-wal` and `-shm` sidecars beside the database, so a - writable file inside a directory it cannot write is not enough: - - ```bash - chown -R 1000:1000 /path/to/data - ``` - - Restoring as `root` on the host and forgetting this step is the - usual way a restore fails. - -5. Start the service. `AutoMigrate` runs against each restored database - as it is opened. +4. Start the service. The container gives the directory and the + restored files to the `webhooker` user before the app starts, + whoever restored them (see + [Running with Docker](#running-with-docker)). `AutoMigrate` runs + against each restored database as it is opened. ### Upgrades @@ -3077,7 +3038,11 @@ check, see [The login endpoint](#the-login-endpoint). - Prometheus metrics behind basic auth - Static assets embedded in binary (no filesystem access needed at runtime) -- Container runs as non-root user (UID 1000) +- The app runs as the non-root `webhooker` user (UID 1000) in the + container. The image sets no `USER`, so these run as root: the + `ENTRYPOINT` script, which sets the data directory's owner and mode + before the app starts; the image's health check; and `docker exec`, + unless given `--user` - GORM soft deletes on every entity that carries `BaseModel`, which is all of them but `Setting` (data preserved for audit) @@ -3204,10 +3169,13 @@ version is fixed independently of the compiler's: `GO_LDFLAGS`, so neither can drop the `-X` that stamps the version. The version arrives as the `VERSION` build arg, since the context has no `.git` (see [Version stamping](#version-stamping)). -3. **Runtime stage** (`alpine:3.21`) — copies the static binary, - creates the `/var/lib/webhooker` directory for all SQLite databases, - runs as the non-root `webhooker` user (UID 1000), exposes port 8080, - and includes a health check against `/.well-known/healthcheck`. +3. **Runtime stage** (`alpine:3.21`) — copies the static binary and + `deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` + directory for all SQLite databases, exposes port 8080, and includes + a health check against `/.well-known/healthcheck`. It sets no + `USER`: the `ENTRYPOINT` script starts as root, sets the data + directory's owner and mode, and runs the app as the non-root + `webhooker` user (UID 1000) through `su-exec`. The lint stage invokes `golangci-lint` directly rather than `make lint`: it is already the pinned linter image, and `make lint` builds diff --git a/deploy/docker-entrypoint.sh b/deploy/docker-entrypoint.sh new file mode 100755 index 0000000..b081336 --- /dev/null +++ b/deploy/docker-entrypoint.sh @@ -0,0 +1,22 @@ +#!/bin/sh +# deploy/docker-entrypoint.sh: the image's ENTRYPOINT. A bind-mounted +# data directory keeps its owner from the host, often root, and the app +# could not write to it. Started as root, this creates DATA_DIR if +# needed, gives it and everything in it to webhooker, sets its mode, and +# runs the command as webhooker, so the app never runs as root. Started +# as another user, it only runs the command. +set -eu + +main() { + if [ "$(id -u)" != 0 ]; then + exec "$@" + fi + + dir="${DATA_DIR:-/var/lib/webhooker}" + mkdir -p "$dir" + find "$dir" ! -user webhooker -exec chown -h webhooker:webhooker {} + + chmod 750 "$dir" + exec su-exec webhooker "$@" +} + +main "$@" -- 2.54.0