Author SHA1 Message Date
clawbot 41a1ecae8d Bound username length at creation (closes #184)
check / check (push) Successful in 5m15s
A login stores the username in the session cookie, which cannot carry
a value past about 4096 bytes, so a username of about 2 KB or more
could never log in and the login answered 500.

Usernames are now limited to 1024 bytes, about half of what the cookie
can carry. The User model rejects a longer one with
ErrUsernameTooLong, and a check constraint on the users table rejects
it for any path that bypasses the model. The comment on
MaxUsernameBytes gives the arithmetic.

Model: opus-5-5
2026-09-29 08:35:29 +00:00
9 changed files with 252 additions and 151 deletions
+2 -9
View File
@@ -88,9 +88,7 @@ RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static
# alpine:3.21, 2026-03-17
FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
# 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
RUN apk --no-cache add ca-certificates
# Create non-root user
RUN addgroup -g 1000 -S webhooker && \
@@ -101,17 +99,13 @@ 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
# No USER: the entrypoint starts as root to make the data directory
# webhooker's, then runs the app as webhooker.
USER webhooker
EXPOSE 8080
@@ -130,5 +124,4 @@ 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"]
+87 -57
View File
@@ -552,9 +552,8 @@ printf '%s' "$NEW_PASSWORD" | \
DATA_DIR=/var/lib/webhooker webhooker resetpw admin
```
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:
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:
```bash
docker run --rm -v webhooker-data:/var/lib/webhooker \
@@ -691,22 +690,38 @@ 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 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 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 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 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 —
@@ -714,10 +729,13 @@ 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. 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.
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.
### Running under upaas
@@ -733,6 +751,17 @@ 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
@@ -991,12 +1020,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` 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.
`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.
Note that `sqlite3 <db> .dump` is **not** one of these procedures: it is
an export, it holds a read transaction open for as long as it runs, and
@@ -1050,11 +1079,21 @@ 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. 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.
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.
### Upgrades
@@ -1433,7 +1472,7 @@ A registered user of the webhooker service.
| Field | Type | Description |
| ---------- | -------- | ----------- |
| `id` | UUID | Primary key |
| `username` | string | Unique login name |
| `username` | string | Unique login name, at most 1024 bytes so that it fits in the session cookie |
| `password` | string | Argon2id hash (never exposed via API) |
**Relations:** Has many Webhooks. Has many APIKeys.
@@ -2373,14 +2412,14 @@ Removing either cap fails 14 subtests.
`internal/middleware/logbound_test.go` and
`internal/handlers/logbound_test.go` drive 8 KB of client-chosen text
at each of these — 1 KB at `invalid password`, whose accounts are
shared with the successful-login line, where a username past 4 KB
overflows the session cookie and answers 500 before that line is
written — through both handlers, and through seven fills: plain text
as the baseline, and then the quotation mark, backslash, tab, newline,
C0 control and astral non-printable, six characters the wider of the
two handlers spends more on than the client spent sending them. Every
case holds each line to the 2,560-byte ceiling. That per-line ceiling
at each of these — just under 1 KB at `invalid password`, whose
accounts are shared with the successful-login line and so must stay
within the 1024-byte username limit — through both handlers, and
through seven fills: plain text as the baseline, and then the
quotation mark, backslash, tab, newline, C0 control and astral
non-printable, six characters the wider of the two handlers spends
more on than the client spent sending them. Every case holds each
line to the 2,560-byte ceiling. That per-line ceiling
is what the figure above states, and every row establishes it.
Three of the sites go further and bound the whole flood's output — the
@@ -2682,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`) |
| `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` |
| 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` |
| `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
@@ -3032,11 +3071,7 @@ check, see [The login endpoint](#the-login-endpoint).
- Prometheus metrics behind basic auth
- Static assets embedded in binary (no filesystem access needed at
runtime)
- 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`
- Container runs as non-root user (UID 1000)
- GORM soft deletes on every entity that carries `BaseModel`, which is
all of them but `Setting` (data preserved for audit)
@@ -3163,13 +3198,10 @@ 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 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`.
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`.
The lint stage invokes `golangci-lint` directly rather than `make lint`:
it is already the pinned linter image, and `make lint` builds
@@ -3248,5 +3280,3 @@ MIT
## Author
[@sneak](https://sneak.berlin)
-22
View File
@@ -1,22 +0,0 @@
#!/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 "$@"
+46 -2
View File
@@ -1,13 +1,57 @@
package database
import (
"errors"
"fmt"
"gorm.io/gorm"
)
// MaxUsernameBytes is the longest username, in bytes, that a user may
// have. The same number appears in the check constraint on
// User.Username, because a struct tag cannot reference a constant.
//
// A login stores the username in the session cookie, and both
// securecookie and browsers refuse a cookie value past about 4096
// bytes. That value is the session base64-encoded twice, so it holds
// 4096 × 3/4 × 3/4 = 2304 bytes of session, and the signature,
// timestamp and the session's other values take about 270 of those: a
// username longer than about 2030 bytes can never log in. The limit is
// about half that, so the session can carry more values later without
// locking out an account whose username is already at the limit.
const MaxUsernameBytes = 1024
// ErrUsernameTooLong is returned when a user is saved with a username
// longer than MaxUsernameBytes.
var ErrUsernameTooLong = errors.New("username is too long")
// User represents a user of the webhooker service
//
//nolint:lll // a struct tag cannot wrap
type User struct {
BaseModel
Username string `gorm:"uniqueIndex;not null" json:"username"`
Password string `gorm:"not null" json:"-"` // Argon2 hashed
Username string `gorm:"uniqueIndex;not null;check:length(CAST(username AS BLOB)) <= 1024" json:"username"`
Password string `gorm:"not null" json:"-"` // Argon2 hashed
// Relations
Webhooks []Webhook `json:"webhooks,omitempty"`
APIKeys []APIKey `json:"apiKeys,omitempty"`
}
// BeforeSave rejects a username longer than MaxUsernameBytes, so every
// path that saves a user through GORM gets ErrUsernameTooLong rather
// than the database's constraint error. The check constraint behind it
// holds for any path that writes the table without this model.
func (u *User) BeforeSave(_ *gorm.DB) error {
if len(u.Username) > MaxUsernameBytes {
return fmt.Errorf(
"%w: %d bytes, limit is %d",
ErrUsernameTooLong,
len(u.Username),
MaxUsernameBytes,
)
}
return nil
}
+65
View File
@@ -0,0 +1,65 @@
package database_test
import (
"strings"
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
)
// usernameAtLimit is exactly MaxUsernameBytes long, built from a
// two-byte character. A check that counted characters rather than bytes
// would see half the length and let the one-byte-longer name through.
func usernameAtLimit() string {
return strings.Repeat("é", database.MaxUsernameBytes/2)
}
func TestUserCreate_RejectsOverlongUsername(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
err := db.Create(&database.User{
Username: usernameAtLimit() + "x",
Password: "hash",
}).Error
require.ErrorIs(t, err, database.ErrUsernameTooLong)
}
func TestUserCreate_AcceptsUsernameAtLimit(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
require.NoError(t, db.Create(&database.User{
Username: usernameAtLimit(),
Password: "hash",
}).Error)
}
// TestUsersTable_EnforcesUsernameLimitWithoutTheModel inserts with raw
// SQL, as a path that bypassed User.BeforeSave would, so only the
// table's check constraint stands between it and an over-long
// username. Accepting the name at the limit and refusing the next byte
// also pins the constraint's number to MaxUsernameBytes.
func TestUsersTable_EnforcesUsernameLimitWithoutTheModel(t *testing.T) {
t.Parallel()
db := startedTestDB(t)
insert := "INSERT INTO users (id, username, password) VALUES (?, ?, ?)"
require.NoError(t, db.Exec(
insert, uuid.New().String(), usernameAtLimit(), "hash",
).Error)
err := db.Exec(
insert, uuid.New().String(), usernameAtLimit()+"x", "hash",
).Error
require.Error(t, err)
assert.Contains(t, err.Error(), "CHECK constraint failed")
}
+30
View File
@@ -453,3 +453,33 @@ func TestLogin_SuccessCreatesSession(t *testing.T) {
"the issued cookie must carry an authenticated session",
)
}
// TestLogin_UsernameAtLimitCanLogIn shows that a username of exactly
// database.MaxUsernameBytes still fits in the session cookie. Past
// what the cookie can carry, a correct login answers 500.
func TestLogin_UsernameAtLimitCanLogIn(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
)
app := newTestApp(t, &h, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
username := strings.Repeat("a", database.MaxUsernameBytes)
hash, err := database.HashPassword(operatorPassword)
require.NoError(t, err)
require.NoError(t, db.DB().Create(&database.User{
Username: username,
Password: hash,
}).Error)
w := submitLogin(h, sharedProxyPeer, username, operatorPassword)
assert.Equal(t, http.StatusSeeOther, w.Code)
}
+3 -5
View File
@@ -339,11 +339,9 @@ const storedUserPassword = "correct-horse-battery-staple"
// storedFillBytes is the raw length of the client-chosen value in
// those accounts' usernames. It is well past the 512-byte field
// budget, so the line is still truncated, but short enough that the
// session cookie a successful login writes stays inside
// securecookie's 4 KB limit: the cookie is written BEFORE the
// "user logged in" line, so an 8 KB username answers 500 and never
// reaches it.
const storedFillBytes = 1024
// whole username, markers and fill name included, stays within
// database.MaxUsernameBytes.
const storedFillBytes = 960
// storedFill builds a username fill of storedFillBytes raw bytes out
// of repetitions of ch, with both markers at its far end.
+3 -17
View File
@@ -92,25 +92,11 @@ func (s *Server) setupGlobalMiddleware() {
func (s *Server) setupRoutes() {
s.router.Get("/", s.h.HandleIndex())
// 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.Mount(
"/s",
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.
})
+16 -39
View File
@@ -396,15 +396,13 @@ func (e *testEnv) storedHash(t *testing.T, username string) string {
// --- /s static group ---
// 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) {
// 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) {
t.Parallel()
env := newTestEnv(t)
@@ -419,7 +417,6 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) {
http.MethodPost,
http.MethodPut,
http.MethodDelete,
"PROPFIND",
} {
t.Run(method, func(t *testing.T) {
t.Parallel()
@@ -431,38 +428,18 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) {
w := httptest.NewRecorder()
env.router.ServeHTTP(w, req)
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.Equal(t, http.StatusOK, w.Code,
"static mount answers every method")
if method == http.MethodHead {
assert.Empty(t, w.Body.Bytes(),
"HEAD must not carry a body")
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",
)
return
}
assert.Equal(t, body, w.Body.Bytes(),
"the asset itself is returned")
})
}
}