diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 256c55b..8c3eba6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,19 +1,19 @@ name: release -# Publishes an immutable, version-tagged image on ghcr.io (spec 10.1). +# Publishes an immutable, version-tagged image on ghcr.io. # Ordinary commits and bare git tag pushes do not publish anything. A published # GitHub Release runs this workflow directly (same pattern as gosentry / imap-scrub). # You can also run it manually via workflow_dispatch with an explicit SemVer X.Y.Z. # -# Native per-architecture builds (see docs/development.md), not qemu: -# running the full Postfix/OpenDKIM stack under emulation for the e2e gate -# below is impractically slow. Each arch builds, e2e-gates and pushes its own -# tag on its own native runner; a merge job then combines them into the one -# manifest tag documented in deploy/docker-compose.yml. "test, then push" (not -# push-by-digest then test) is deliberate: it means the bytes that get tagged -# are exactly the bytes that passed e2e. Per-arch tags are pushed only so -# imagetools can assemble the multi-arch manifest; merge removes them from GHCR -# so operators see a single version tag (spec 10.1). +# Native per-architecture builds (see docs/development.md), not qemu: running +# the full Postfix/OpenDKIM stack under emulation for the e2e gate below is +# impractically slow. Each arch builds, e2e-gates and pushes its own tag on its +# own native runner; a merge job then combines them into the one manifest tag +# documented in deploy/docker-compose.yml. "test, then push" (not push-by-digest +# then test) is deliberate: it means the bytes that get tagged are exactly the +# bytes that passed e2e. Per-arch tags are pushed only so imagetools can +# assemble the multi-arch manifest; merge removes them from GHCR so operators +# see a single version tag. on: release: types: [published] diff --git a/CHANGELOG.md b/CHANGELOG.md index c4c1edf..e88a646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,16 +15,8 @@ rate limit is correct for a non-default level-1 window. Schema migration - Code review plan (`docs/plans/code-review-2026-08.md`) — full codebase audit covering architecture, code quality, security, GUI, tests, documentation, and AGPL compliance. 38 findings across 11 categories with a prioritised - implementation plan in 6 phases. - -### Removed - -- Plan documents for already-shipped features: `domain-stats-auto-ratelimit`, - `dmarc-reports`, `inbound-relay`, `panel-docs`, `queue-retries`, - `send-log-retention`. Only unimplemented plans remain in `docs/plans/`. - -### Added - + implementation plan in 6 phases, and a status table recording what was + implemented, what was already in place, and what was declined. - Roadmap item and plan for `/preflight` installation check page ([docs/plans/preflight.md](docs/plans/preflight.md)) — instance-level infrastructure verification (rDNS, TLS, ports, HELO banner, proxy headers, @@ -46,6 +38,21 @@ rate limit is correct for a non-default level-1 window. Schema migration ### Fixed +- The setup account's password is hashed at bcrypt cost `12` like every other + panel password; it was still being written at `bcrypt.DefaultCost` (10). The + work factor now has one definition, `auth.BcryptCost`. bcrypt records the + cost in the hash, so existing passwords keep verifying unchanged. +- Login/setup rate limiter: protecting blocked buckets from eviction is now + bounded. Blocked buckets are still never evicted to make room, but past + `maxBuckets × 4` they are evicted too (nearest expiry first) — otherwise an + attacker with many source addresses could grow the bucket map without limit + by making each address fail its way to blocked. The overflow is logged at + most once per window. +- `postqueue -p` output is bounded to 4 MiB (the timeout alone left the size of + the listing, and therefore the panel's allocation, up to the queue). +- [schema-migrations.md](docs/schema-migrations.md): the schema section was + still headed `v9` while the chain head is `v10`, and three links pointed at + plan files deleted when those features shipped. - postqueue reader is now time-bounded: `postqueue -p` runs under a 5-second context timeout so a stuck queue command cannot hang the queue panel path indefinitely. @@ -96,6 +103,18 @@ rate limit is correct for a non-default level-1 window. Schema migration ### Changed +- Extracted `internal/configsafe` — the empty-value and forbidden-character + check every generated config file shares. Each writer keeps its own forbidden + set next to the file it protects (an OpenDKIM table and a Postfix map break on + different characters); only the check itself is shared. +- Build, deploy and migration comments no longer cite the archived + specification. The `spec N` references CHANGELOG `[0.5.0]` said were gone + survived in `build/`, `deploy/`, the first two migrations and the release + workflow; they are removed and the affected comment paragraphs re-wrapped. + Comments only — no build or configuration behaviour changes. +- Dropped the `parseDomainRateLimitForm` / `parseAppRateLimitForm` pass-through + wrappers left behind when the two parsers were merged: nothing but their own + tests still called them. - Extracted shared helpers: `internal/atomicfile` (atomic temp-write+rename) and `internal/supervisor` (supervisorctl start/signal wrappers), and reused them in domain/postfix code paths. @@ -130,6 +149,12 @@ rate limit is correct for a non-default level-1 window. Schema migration version-cut table through `1.9.1`; [schema-migrations.md](docs/schema-migrations.md) last-updated note. No behaviour change. +### Removed + +- Plan documents for already-shipped features: `domain-stats-auto-ratelimit`, + `dmarc-reports`, `inbound-relay`, `panel-docs`, `queue-retries`, + `send-log-retention`. Only unimplemented plans remain in `docs/plans/`. + ## [1.9.1] - 2026-08-19 Port-25 Postfix startup when inbound relay, DMARC ingest, or the inbound antispam diff --git a/build/Dockerfile b/build/Dockerfile index 177b6d6..25ba6f1 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -1,7 +1,7 @@ # syntax=docker/dockerfile:1 # # SelfPost — single Debian-slim image running postfix + opendkim + panel under -# supervisord (spec 4). Build from the repository root: +# supervisord. Build from the repository root: # # docker build -f build/Dockerfile -t selfpost:dev --build-arg VERSION=dev . @@ -10,7 +10,7 @@ FROM golang:1.26-bookworm AS build WORKDIR /src -# Version stamped into both binaries; MUST match the image tag (spec 7.5.A). +# Version stamped into both binaries; MUST match the image tag. ARG VERSION=dev # Module metadata first for layer caching. @@ -52,13 +52,13 @@ RUN echo "postfix postfix/mailname string localhost" | debconf-set-selections \ curl \ && rm -rf /var/lib/apt/lists/* -# Unprivileged user for the panel process (spec 7.6.8). +# Unprivileged user for the panel process. RUN useradd --system --no-create-home --shell /usr/sbin/nologin panel -# Shared group bridging the unprivileged services (spec 5.1, 6): the panel -# generates per-domain DKIM keys, application SASL accounts (sasldb2) and the -# Postfix sender map, while OpenDKIM and Postfix (different users) must read -# them. Membership in this group — plus setgid dirs under /data (set up in +# Shared group bridging the unprivileged services: the panel generates +# per-domain DKIM keys, application SASL accounts (sasldb2) and the Postfix +# sender map, while OpenDKIM and Postfix (different users) must read them. +# Membership in this group — plus setgid dirs under /data (set up in # entrypoint.sh) — lets OpenDKIM read the panel-owned keys and lets Postfix read # the sasldb2/sender map, and lets the panel reach the supervisor control socket # to signal OpenDKIM/Postfix reloads without any process running as root. @@ -67,15 +67,15 @@ RUN groupadd --system selfpost \ && usermod -aG selfpost opendkim \ && usermod -aG selfpost postfix -# The panel reads the mail queue with `postqueue -p` (spec 7.2.11, and the -# status page's queue card). postqueue is setgid postdrop, which normally gives -# it the group needed to reach Postfix's showq socket — but the documented -# deployment runs with `no-new-privileges`, which disables setgid transitions, -# so the panel would always see "Permission denied". Making `panel` a real -# member of postdrop grants the same access without relying on a setgid -# escalation the hardening deliberately forbids. postdrop membership is -# read-side only: it does not let the panel bypass any Postfix restriction that -# a local user does not already have through the world-executable sendmail. +# The panel reads the mail queue with `postqueue -p` (and the status page's +# queue card). postqueue is setgid postdrop, which normally gives it the group +# needed to reach Postfix's showq socket — but the documented deployment runs +# with `no-new-privileges`, which disables setgid transitions, so the panel +# would always see "Permission denied". Making `panel` a real member of postdrop +# grants the same access without relying on a setgid escalation the hardening +# deliberately forbids. postdrop membership is read-side only: it does not let +# the panel bypass any Postfix restriction that a local user does not already +# have through the world-executable sendmail. RUN usermod -aG postdrop panel # Runtime directories: milter sockets and the consolidated persistent root. @@ -118,5 +118,5 @@ HEALTHCHECK --interval=30s --timeout=5s --start-period=90s --retries=3 \ CMD curl -fsS http://127.0.0.1:8080/healthz || exit 1 # The entrypoint fixes /data ownership (bind mount) as root, then execs -# supervisord, which becomes PID 1 and owns process supervision (spec 4). +# supervisord, which becomes PID 1 and owns process supervision. ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] diff --git a/build/crashexit.py b/build/crashexit.py index 368d2d3..d7f2c5e 100644 --- a/build/crashexit.py +++ b/build/crashexit.py @@ -4,7 +4,7 @@ If any managed program exhausts its restart retries and enters FATAL, this listener signals supervisord (its parent) to terminate, so the whole container exits and Docker's `restart` policy can recreate it cleanly — rather than -lingering "alive" with a dead Postfix/OpenDKIM/panel (spec 4). +lingering "alive" with a dead Postfix/OpenDKIM/panel. Communication with supervisord uses the event listener protocol over stdin and stdout, so stdout must carry only protocol tokens. diff --git a/build/entrypoint.sh b/build/entrypoint.sh index 1155e4f..ca5b0a0 100644 --- a/build/entrypoint.sh +++ b/build/entrypoint.sh @@ -9,14 +9,14 @@ set -e # SELFPOST_HOSTNAME is an identity, not a setting with a safe default: it must # simultaneously match the PTR/rDNS record, the certificate CN/SAN, and the -# Cyrus SASL realm (spec 5.2 p.3, 8). The panel (main.go saslRealm()) and -# postfix-config.sh each fall back independently when it's unset — to -# `localhost` and to the container hostname respectively — so accounts get -# written under one realm and looked up under another and authentication -# silently fails for every application, while HELO also stops matching the -# PTR record and mail that does go out lands in spam. No fallback can be -# correct, so fail loudly here, before either side of that split has a chance -# to run, rather than leave a green panel with broken mail. +# Cyrus SASL realm. The panel (main.go saslRealm()) and postfix-config.sh each +# fall back independently when it's unset — to `localhost` and to the container +# hostname respectively — so accounts get written under one realm and looked up +# under another and authentication silently fails for every application, while +# HELO also stops matching the PTR record and mail that does go out lands in +# spam. No fallback can be correct, so fail loudly here, before either side of +# that split has a chance to run, rather than leave a green panel with broken +# mail. if [ -z "$SELFPOST_HOSTNAME" ]; then cat >&2 <<'EOF' FATAL: SELFPOST_HOSTNAME is not set. @@ -52,11 +52,11 @@ case "$SELFPOST_HOSTNAME" in ;; esac -# The persistent root /data is a host bind mount (spec 9), so it arrives owned -# by the host user (typically root), not by the unprivileged panel user that -# actually writes the SQLite database, setup token and DKIM keys (spec 7.6.8). -# Fix its ownership here — the one place still running as root — before handing -# off to supervisord, which starts the panel as the panel user. +# The persistent root /data is a host bind mount, so it arrives owned by the +# host user (typically root), not by the unprivileged panel user that actually +# writes the SQLite database, setup token and DKIM keys. Fix its ownership here +# — the one place still running as root — before handing off to supervisord, +# which starts the panel as the panel user. # # Mode must stay world-traversable (0755): OpenDKIM and Postfix reach their # trees under /data as other users. Go's testing.TempDir is 0700, and a bare @@ -71,7 +71,7 @@ chmod 755 /data # normalised on its own below. find /data -mindepth 1 -maxdepth 1 ! -user panel ! -name log ! -name postfix -exec chown -R panel:panel {} + -# DKIM key tree (spec 6, 9). The panel (user `panel`) generates keys and writes +# DKIM key tree. The panel (user `panel`) generates keys and writes # the OpenDKIM tables; OpenDKIM (user `opendkim`) must read them. Normalise the # tree on every start so it is correct whether /data is fresh, restarted, or # just restored from a backup: @@ -89,7 +89,7 @@ find /data/opendkim -type d -exec chmod 2750 {} + chmod 0640 /data/opendkim/KeyTable /data/opendkim/SigningTable find /data/opendkim/keys -type f -name '*.private' -exec chmod 0640 {} + -# Application SASL accounts (spec 5.1, 9). The panel (user `panel`) writes the +# Application SASL accounts. The panel (user `panel`) writes the # sasldb2 via saslpasswd2; Postfix (user `postfix`) reads it to authenticate SMTP # clients. Share it through the `selfpost` group the same way as the DKIM tree: # setgid directory so new files inherit the group, and the database itself @@ -99,8 +99,8 @@ chown -R panel:selfpost /data/sasl chmod 2750 /data/sasl [ -e /data/sasl/sasldb2 ] && chmod 0640 /data/sasl/sasldb2 -# Postfix state under /data (spec 5.1, architecture.md § Persistence). The panel -# writes sender_login_maps; Postfix owns the on-disk queue tree under queue/. +# Postfix state under /data (architecture.md § Persistence). The panel writes +# sender_login_maps; Postfix owns the on-disk queue tree under queue/. mkdir -p /data/postfix/queue [ -e /data/postfix/sender_login_maps ] || : > /data/postfix/sender_login_maps # Inbound relay maps (written by the panel when INBOUND_RELAY_ENABLE=true). @@ -132,14 +132,14 @@ chown -R postfix:selfpost /data/log chmod 2750 /data/log find /data/log -type f -exec chmod 0640 {} + -# Milter socket directories (spec 5 p.3, 7.3). Postfix (user `postfix`) must -# actually CONNECT to both milter sockets — OpenDKIM's and the panel's -# journal-milter — not just probe them at start-up. The sockets are -# created by the opendkim and panel users respectively, so bridge them to -# `postfix` through the shared `selfpost` group: group-owned + setgid dirs mean -# each socket created inside inherits group `selfpost`, and group-traversable -# (2750) lets postfix reach it. Without this, smtpd cannot talk to OpenDKIM and, -# because signing is strict (default_action=tempfail), rejects all mail. +# Milter socket directories. Postfix (user `postfix`) must actually CONNECT to +# both milter sockets — OpenDKIM's and the panel's journal-milter — not just +# probe them at start-up. The sockets are created by the opendkim and panel +# users respectively, so bridge them to `postfix` through the shared `selfpost` +# group: group-owned + setgid dirs mean each socket created inside inherits +# group `selfpost`, and group-traversable (2750) lets postfix reach it. Without +# this, smtpd cannot talk to OpenDKIM and, because signing is strict +# (default_action=tempfail), rejects all mail. mkdir -p /run/opendkim /run/selfpost chown opendkim:selfpost /run/opendkim chown panel:selfpost /run/selfpost diff --git a/build/logrotate-loop.sh b/build/logrotate-loop.sh index 4a16970..ed59572 100644 --- a/build/logrotate-loop.sh +++ b/build/logrotate-loop.sh @@ -1,13 +1,13 @@ #!/bin/sh -# Periodic logrotate for /data/log/mail.log (spec 9, 10). Rotation renames the -# file, recreates it (`create 0640 postfix selfpost`, matching a cold container +# Periodic logrotate for /data/log/mail.log. Rotation renames the file, +# recreates it (`create 0640 postfix selfpost`, matching a cold container # start), then runs `postfix reload` (the same mechanism `postfix logrotate` # uses): postlogd keeps writing to the renamed inode until reload, and the -# panel's log-tailer holds its own descriptor on that inode, so nothing -# written before the reload is lost. `create` (rather than `nocreate`) matters -# here beyond timing: a postlogd-triggered recreate lands the file at 0600 -# owned by postfix, which the unprivileged panel process cannot read — -# confirmed on a live container — so logrotate must be the one to create it. +# panel's log-tailer holds its own descriptor on that inode, so nothing written +# before the reload is lost. `create` (rather than `nocreate`) matters here +# beyond timing: a postlogd-triggered recreate lands the file at 0600 owned by +# postfix, which the unprivileged panel process cannot read — confirmed on a +# live container — so logrotate must be the one to create it. # # logrotate itself only rotates once the configured "daily" period has elapsed # (tracked in /var/lib/logrotate/status), so it is safe to invoke this more diff --git a/build/opendkim.conf b/build/opendkim.conf index 4a6898a..82ac531 100644 --- a/build/opendkim.conf +++ b/build/opendkim.conf @@ -1,9 +1,9 @@ -# OpenDKIM — per-domain signing (spec 6). +# OpenDKIM — per-domain signing. # # Signing is strictly per-domain: each sending domain has its own key pair and # selector. The panel generates keys under /data/opendkim/keys// and # keeps KeyTable/SigningTable in sync as domains are added or removed, then asks -# supervisord to send this process SIGUSR1 to reload the tables (spec 6.2, 6.5). +# supervisord to send this process SIGUSR1 to reload the tables. # # Both table files are created (empty) by entrypoint.sh before OpenDKIM starts, # so the daemon comes up cleanly on a fresh /data with no domains yet and simply diff --git a/build/postfix-config.sh b/build/postfix-config.sh index c83d767..f0aaa01 100644 --- a/build/postfix-config.sh +++ b/build/postfix-config.sh @@ -1,16 +1,16 @@ #!/bin/sh -# Generate the outbound-relay Postfix configuration (spec 5, 5.1, 5.2). +# Generate the outbound-relay Postfix configuration. # # Run as root from entrypoint.sh on every container start, BEFORE supervisord # starts the Postfix wrapper. Like the rest of SelfPost's runtime state it is # re-derived from the environment on each start rather than persisted, so a # fresh, restarted or restored container always ends up with the same config # (the only panel-edited Postfix file, sender_login_maps, lives under /data and -# is generated separately by the panel — spec 5.1). +# is generated separately by the panel). # # main.cf is written with `postconf -e`, the master.cf submission services with # `postconf -M`/`-P`. No user input is interpolated: every value here comes from -# a fixed literal or a container environment variable (spec 7.6.3). +# a fixed literal or a container environment variable. set -eu # postconf -P refuses master.cf -o values that contain whitespace; inbound port @@ -44,16 +44,16 @@ master_cf_set_smtp_inet_o() { mv /etc/postfix/master.cf.new /etc/postfix/master.cf } -# --- environment knobs (spec 8) ---------------------------------------------- +# --- environment knobs ------------------------------------------------------- # Server hostname: used as HELO name AND, crucially, as the Cyrus SASL realm the # sasldb2 accounts are looked up under. The panel creates accounts under realm # $SELFPOST_HOSTNAME (SASL_REALM), so myhostname MUST match or authentication # fails. Fall back to the container hostname only outside a real deployment. HOSTNAME_VALUE="${SELFPOST_HOSTNAME:-$(hostname -f 2>/dev/null || hostname)}" -# TLS material supplied by the reverse-proxy through a read-only bind mount -# (spec 5.2). The relay requires TLS on 465; if these files are absent the -# master still starts but TLS handshakes on 465 fail until they appear. +# TLS material supplied by the reverse-proxy through a read-only bind mount. The +# relay requires TLS on 465; if these files are absent the master still starts +# but TLS handshakes on 465 fail until they appear. # # Postfix insists the private key is root-owned and mode 0600. The bind mount # is often :ro and owned by the host user (e2e TempDir / CI runner UID), so @@ -75,8 +75,8 @@ if [ -f "$TLS_CERT_SRC" ] && [ -f "$TLS_KEY_SRC" ]; then TLS_KEY="$TLS_INTERNAL_DIR/privkey.pem" fi -# Level-1 rate limit (native Postfix anvil, spec 5 p.5 / 7.4). Conservative -# defaults, sensible during IP warm-up (spec 10). +# Level-1 rate limit (native Postfix anvil). Conservative defaults, sensible +# during IP warm-up. RATE_MSGS="${RATE_LIMIT_MESSAGES_PER_IP:-100}" RATE_WINDOW="${RATE_LIMIT_WINDOW_SECONDS:-3600}" @@ -85,16 +85,16 @@ RATE_WINDOW="${RATE_LIMIT_WINDOW_SECONDS:-3600}" OPENDKIM_SOCK="${OPENDKIM_SOCKET:-/run/opendkim/opendkim.sock}" JOURNAL_SOCK="${JOURNAL_MILTER_SOCKET:-/run/selfpost/journal.sock}" -# Persistent panel-managed sender map (spec 5.1); texthash needs no postmap, so -# the unprivileged panel can rewrite it and just ask for a reload. +# Persistent panel-managed sender map; texthash needs no postmap, so the +# unprivileged panel can rewrite it and just ask for a reload. SENDER_LOGIN_MAPS="${POSTFIX_SENDER_LOGIN_MAPS:-/data/postfix/sender_login_maps}" # Transit mail queue under /data so deferred/active messages survive container # recreate (architecture.md § Persistence). Distinct from sender_login_maps. QUEUE_DIR="${POSTFIX_QUEUE_DIR:-/data/postfix/queue}" SASLDB_PATH="${SASL_DB_PATH:-/data/sasl/sasldb2}" -# Optional submission service on 587 (spec 5 p.1: off by default, enabled only -# when a client library needs STARTTLS on 587 instead of implicit TLS on 465). +# Optional submission service on 587 (off by default, enabled only when a client +# library needs STARTTLS on 587 instead of implicit TLS on 465). SUBMISSION_ENABLE="${SUBMISSION_ENABLE:-false}" # Optional inbound relay (backup-MX / forwarder). Off by default: port 25 does @@ -153,13 +153,13 @@ postconf -e \ "alias_maps=" \ "alias_database=" -# Outbound delivery: straight to the recipient MX, opportunistic TLS (spec 5 p.2). +# Outbound delivery: straight to the recipient MX, opportunistic TLS. postconf -e \ "smtp_tls_security_level=may" \ "smtp_tls_note_starttls_offer=yes" -# TLS server material shared by every inbound service (spec 5.2). auth_only -# guarantees credentials are never accepted before TLS is up on any port. +# TLS server material shared by every inbound service. auth_only guarantees +# credentials are never accepted before TLS is up on any port. postconf -e \ "smtpd_tls_cert_file=${TLS_CERT}" \ "smtpd_tls_key_file=${TLS_KEY}" \ @@ -167,11 +167,11 @@ postconf -e \ "smtpd_tls_auth_only=yes" \ "smtpd_tls_loglevel=1" -# SASL: Cyrus with the local sasldb2 the panel maintains (spec 5.1). The realm -# is left implicit (smtpd_sasl_local_domain empty) so the authenticated name -# Postfix uses for sender_login_maps is the BARE login the panel writes into the -# map; the sasldb2 lookup still resolves because Postfix hands Cyrus $myhostname -# as the server realm, which equals the realm the accounts were created under. +# SASL: Cyrus with the local sasldb2 the panel maintains. The realm is left +# implicit (smtpd_sasl_local_domain empty) so the authenticated name Postfix +# uses for sender_login_maps is the BARE login the panel writes into the map; +# the sasldb2 lookup still resolves because Postfix hands Cyrus $myhostname as +# the server realm, which equals the realm the accounts were created under. postconf -e \ "smtpd_sasl_auth_enable=yes" \ "smtpd_sasl_type=cyrus" \ @@ -181,43 +181,42 @@ postconf -e \ "smtpd_sasl_tls_security_options=noanonymous" \ "broken_sasl_auth_clients=yes" -# Sender binding (spec 5.1 p.3, the critical anti-spoofing control). texthash -# resolves the full address first, then the "@domain" wildcard, so both address -# modes work from the same map. +# Sender binding (the critical anti-spoofing control). texthash resolves the +# full address first, then the "@domain" wildcard, so both address modes work +# from the same map. postconf -e \ "smtpd_sender_login_maps=texthash:${SENDER_LOGIN_MAPS}" \ "queue_directory=${QUEUE_DIR}" # Restrictions: authenticated clients only, no relay to foreign destinations, # and every authenticated sender address must be owned by its login. NO -# permit_mynetworks anywhere — authorisation is by credentials, never by network -# (spec 5 p.1/p.4, 5.1). This is what makes an open relay impossible. +# permit_mynetworks anywhere — authorisation is by credentials, never by +# network. This is what makes an open relay impossible. postconf -e \ "smtpd_helo_required=yes" \ "smtpd_relay_restrictions=permit_sasl_authenticated, reject_unauth_destination" \ "smtpd_recipient_restrictions=permit_sasl_authenticated, reject_unauth_destination" \ "smtpd_sender_restrictions=reject_sender_login_mismatch, permit" -# Level-1 rate limit by client IP (spec 5 p.5). Backstop that keeps working even -# if the journal-milter (level 2) is down. +# Level-1 rate limit by client IP. Backstop that keeps working even if the +# journal-milter (level 2) is down. postconf -e \ "smtpd_client_message_rate_limit=${RATE_MSGS}" \ "anvil_rate_time_unit=${RATE_WINDOW}s" -# Milter chain (spec 5 p.3, 7.3). OpenDKIM signs and is treated strictly -# (default_action=tempfail: if it is unreachable, defer rather than send -# unsigned). The journal-milter is monitoring only and is fail-open -# (default_action=accept): its failure must never block the relay. Per-milter -# settings use Postfix 3.0+ brace syntax. +# Milter chain. OpenDKIM signs and is treated strictly (default_action=tempfail: +# if it is unreachable, defer rather than send unsigned). The journal-milter is +# monitoring only and is fail-open (default_action=accept): its failure must +# never block the relay. Per-milter settings use Postfix 3.0+ brace syntax. postconf -e \ "milter_protocol=6" \ "milter_default_action=tempfail" \ "smtpd_milters={ unix:${OPENDKIM_SOCK}, default_action=tempfail }, { unix:${JOURNAL_SOCK}, default_action=accept }" \ "non_smtpd_milters=" -# Bounded milter timeouts (spec 7.3): a *hung* milter (socket accepts but never -# replies) must fail open just like a crash, not stall mail acceptance until the -# Postfix defaults (300s content) elapse. With default_action per milter, a +# Bounded milter timeouts: a *hung* milter (socket accepts but never replies) +# must fail open just like a crash, not stall mail acceptance until the Postfix +# defaults (300s content) elapse. With default_action per milter, a # journal-milter hang then resolves to accept and an OpenDKIM hang to tempfail, # but within seconds rather than minutes. Values are well above any healthy # response time (signing/DB insert are sub-second), so they never fire in normal @@ -228,8 +227,8 @@ postconf -e \ "milter_content_timeout=${MILTER_CONTENT_TIMEOUT:-30s}" # --- master.cf: inbound submission services ---------------------------------- -# smtps (465, implicit/wrapper TLS) — the primary, always-on submission service -# (spec 5 p.1). chroot=n so smtpd can read the sasldb2 and sender map under /data +# smtps (465, implicit/wrapper TLS) — the primary, always-on submission +# service. chroot=n so smtpd can read the sasldb2 and sender map under /data # and the Cyrus config outside any chroot. postconf -M "smtps/inet=smtps inet n - n - - smtpd" postconf -P \ @@ -237,10 +236,10 @@ postconf -P \ "smtps/inet/smtpd_sasl_auth_enable=yes" \ "smtps/inet/smtpd_client_restrictions=permit_sasl_authenticated,reject" -# submission (587, STARTTLS) — optional (spec 5 p.1). Same SASL/milter/limits; -# the only difference is TLS is negotiated via STARTTLS, so require encryption -# before auth. Added only when SUBMISSION_ENABLE=true, otherwise removed so a -# restart after disabling it does not leave the port listening. +# submission (587, STARTTLS) — optional. Same SASL/milter/limits; the only +# difference is TLS is negotiated via STARTTLS, so require encryption before +# auth. Added only when SUBMISSION_ENABLE=true, otherwise removed so a restart +# after disabling it does not leave the port listening. if [ "${SUBMISSION_ENABLE}" = "true" ]; then postconf -M "submission/inet=submission inet n - n - - smtpd" postconf -P \ @@ -364,18 +363,18 @@ else "smtp_tls_policy_maps=" fi -# Disable chroot for every service (spec 5 p.2). Debian ships the smtp delivery -# agent and others chrooted to /var/spool/postfix, where they cannot read -# /etc/resolv.conf — so outbound MX lookups fail with "Host not found" and mail -# never leaves. Inside a container the chroot buys little (the container is the -# isolation boundary) and breaks DNS/TLS trust-store access, so turn it off -# uniformly. Our own smtps/submission services are already n; this covers the -# delivery agents and the rest. +# Disable chroot for every service. Debian ships the smtp delivery agent and +# others chrooted to /var/spool/postfix, where they cannot read /etc/resolv.conf +# — so outbound MX lookups fail with "Host not found" and mail never leaves. +# Inside a container the chroot buys little (the container is the isolation +# boundary) and breaks DNS/TLS trust-store access, so turn it off uniformly. Our +# own smtps/submission services are already n; this covers the delivery agents +# and the rest. postconf -F "*/*/chroot=n" # --- Cyrus SASL app config for smtpd ----------------------------------------- # Tells the Cyrus library (invoked by smtpd via smtpd_sasl_path=smtpd) to verify -# passwords straight from the panel-maintained sasldb2 (spec 5.1). PLAIN/LOGIN +# passwords straight from the panel-maintained sasldb2. PLAIN/LOGIN # only — both are safe because TLS is mandatory before auth on every port. mkdir -p /etc/postfix/sasl cat > /etc/postfix/sasl/smtpd.conf < :443 redirect vhost, and renews in the background via its own timer. -# The resulting PEM files land at -# /etc/letsencrypt/live/mail.example.com/{fullchain,privkey}.pem on the host -# — bind-mount that directory read-only into the SelfPost container (see +# (`certbot --apache -d mail.example.com`). Certbot edits this file in place to +# add the SSLCertificateFile/SSLCertificateKeyFile directives and a :80 -> :443 +# redirect vhost, and renews in the background via its own timer. The resulting +# PEM files land at +# /etc/letsencrypt/live/mail.example.com/{fullchain,privkey}.pem on the host — +# bind-mount that directory read-only into the SelfPost container (see # ../docker-compose.yml, the `./certs:/etc/postfix/tls:ro` volume) so Postfix # can present the very same certificate on 465/587. One certificate, two -# consumers (spec 10 p.4) — no separate cert just for the panel. +# consumers — no separate cert just for the panel. ServerName mail.example.com @@ -26,9 +25,9 @@ ProxyPass "/" "http://127.0.0.1:8080/" ProxyPassReverse "/" "http://127.0.0.1:8080/" - # The panel is plain HTTP behind the proxy; it never terminates TLS - # itself (spec 10 p.2). Cookies are still marked Secure by the panel - # (spec 7.6.6) because the browser only ever sees this HTTPS vhost. + # The panel is plain HTTP behind the proxy; it never terminates TLS itself. + # Cookies are still marked Secure by the panel because the browser only ever + # sees this HTTPS vhost. diff --git a/deploy/caddy/docker-compose.caddy.yml b/deploy/caddy/docker-compose.caddy.yml index 4ea8377..7cef432 100644 --- a/deploy/caddy/docker-compose.caddy.yml +++ b/deploy/caddy/docker-compose.caddy.yml @@ -1,10 +1,10 @@ -# Alternative reverse-proxy: Caddy (spec 10.3) — the simplest option, fully -# automatic ACME with no separate certbot container. Caddy writes certificates -# as plain PEM under its own data directory, which this fragment bind-mounts -# to the host so SelfPost can read the same files. +# Alternative reverse-proxy: Caddy — the simplest option, fully automatic ACME +# with no separate certbot container. Caddy writes certificates as plain PEM +# under its own data directory, which this fragment bind-mounts to the host so +# SelfPost can read the same files. # -# NOTE (spec 10.3 explicitly flags this): Caddy's on-disk cert path includes +# NOTE: Caddy's on-disk cert path includes # the ACME CA's name as a path segment, e.g. # /caddy/certificates/acme-v02.api.letsencrypt.org-directory//.crt # This has been stable across recent Caddy releases but is an internal diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 474d1e7..6dbb4f2 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -1,12 +1,12 @@ -# SelfPost — default deployment, reverse-proxy = Apache (spec 10, 10.5). +# SelfPost — default deployment, reverse-proxy = Apache. # -# This file only runs SelfPost itself. Apache is assumed to already be -# installed on the HOST (the target audience for this project typically runs -# Apache there already — spec 10.5) and reverse-proxies HTTPS for the panel; -# it is not containerised here. See ../apache/selfpost-vhost.conf for a ready -# vhost fragment, and docs/guide.md "Reverse proxy" for the -# certbot steps that produce the PEM files this compose file mounts. +# This file only runs SelfPost itself. Apache is assumed to already be installed +# on the HOST (the target audience for this project typically runs Apache there +# already) and reverse-proxies HTTPS for the panel; it is not containerised +# here. See ../apache/selfpost-vhost.conf for a ready vhost fragment, and +# docs/guide.md "Reverse proxy" for the certbot steps that produce the PEM files +# this compose file mounts. # # Usage: # 1. Copy this file (and .env.example as .env) next to your own ./data and @@ -15,10 +15,9 @@ # TLS paths are fixed in this file to match that bind mount, not .env. # 3. docker compose up -d # -# The image tag below is FIXED on purpose (spec 10 p.10, 7.5.A): backup -# restore compares the manifest version against the running binary's version, -# so ":latest" would make that check meaningless. Bump the tag deliberately -# when you want to upgrade. +# The image tag below is FIXED on purpose: backup restore compares the manifest +# version against the running binary's version, so ":latest" would make that +# check meaningless. Bump the tag deliberately when you want to upgrade. services: selfpost: @@ -27,11 +26,11 @@ services: environment: SELFPOST_HOSTNAME: "${SELFPOST_HOSTNAME:?set the mail/panel hostname, e.g. mail.example.com}" # Path Postfix reads inside the container — matches the certs bind mount - # below. Point these at your reverse-proxy's PEM output (spec 10 p.2). + # below. Point these at your reverse-proxy's PEM output. TLS_CERT_FILE: /etc/postfix/tls/fullchain.pem TLS_KEY_FILE: /etc/postfix/tls/privkey.pem - # Set to true to also publish RFC 6409 submission (587/STARTTLS) - # alongside the primary 465/smtps listener (spec 5). + # Set to true to also publish RFC 6409 submission (587/STARTTLS) alongside + # the primary 465/smtps listener. SUBMISSION_ENABLE: "${SUBMISSION_ENABLE:-false}" INBOUND_RELAY_ENABLE: "${INBOUND_RELAY_ENABLE:-false}" DMARC_REPORTS_ENABLE: "${DMARC_REPORTS_ENABLE:-false}" @@ -41,14 +40,14 @@ services: INBOUND_ANTISPAM_MILTER_ACTION: "${INBOUND_ANTISPAM_MILTER_ACTION:-accept}" INBOUND_RATE_LIMIT_MESSAGES_PER_IP: "${INBOUND_RATE_LIMIT_MESSAGES_PER_IP:-20}" INBOUND_MESSAGE_SIZE_LIMIT: "${INBOUND_MESSAGE_SIZE_LIMIT:-26214400}" - # Level-1 backstop rate limit (anvil, spec 5.5, 7.4); per-domain/app - # limits (level 2) are configured later from the panel itself. + # Level-1 backstop rate limit (anvil); per-domain/app limits (level 2) are + # configured later from the panel itself. RATE_LIMIT_MESSAGES_PER_IP: "${RATE_LIMIT_MESSAGES_PER_IP:-100}" RATE_LIMIT_WINDOW_SECONDS: "${RATE_LIMIT_WINDOW_SECONDS:-3600}" # How long the send log keeps rows before the background sweep deletes - # them (spec 7.3, 9) — the main driver of /data growth over time. + # them — the main driver of /data growth over time. SEND_LOG_RETENTION_DAYS: "${SEND_LOG_RETENTION_DAYS:-90}" - # Sliding idle timeout for the panel login session, in days (spec 7.6.6). + # Sliding idle timeout for the panel login session, in days. PANEL_SESSION_IDLE_DAYS: "${PANEL_SESSION_IDLE_DAYS:-7}" # Resolvers the panel's PTR/SPF/DKIM/DMARC checks query directly, so they # report what the internet sees rather than what this host's own stub @@ -59,27 +58,27 @@ services: # client spoof its rate-limit key. TRUSTED_PROXY_CIDR: "${TRUSTED_PROXY_CIDR:-}" volumes: - # All persistent state lives under /data (spec 9): SQLite DB, DKIM keys, - # sasldb2, sender map, setup token. Back this up (panel button or the - # selfpost-backup CLI) before you touch it directly. - # Postfix's mail.log is here too (./data/log, rotated daily, 14 files - # kept) so the delivery lines that resolve a queued send-log row survive - # a container recreate; it is diagnostics and is left out of backups. + # All persistent state lives under /data: SQLite DB, DKIM keys, sasldb2, + # sender map, setup token. Back this up (panel button or the + # selfpost-backup CLI) before you touch it directly. Postfix's mail.log is + # here too (./data/log, rotated daily, 14 files kept) so the delivery + # lines that resolve a queued send-log row survive a container recreate; + # it is diagnostics and is left out of backups. - ./data:/data # Read-only project root for full backups: docker-compose.yml, .env, and # certs/ are archived alongside /data (architecture.md § Persistence). - .:/selfpost-deploy:ro - # Read-only: SelfPost only ever reads certificates, never manages them - # (spec 10 p.2). Point this at wherever your reverse-proxy/certbot - # writes PEM files, e.g. /etc/letsencrypt on the host. + # Read-only: SelfPost only ever reads certificates, never manages them. + # Point this at wherever your reverse-proxy/certbot writes PEM files, e.g. + # /etc/letsencrypt on the host. - ./certs:/etc/postfix/tls:ro ports: # 465 (smtps, primary) and optionally 587 (submission/STARTTLS) are - # published directly — mail traffic bypasses Apache entirely, it only - # ever proxies the panel's HTTP(S) (spec 10 p.2-3). The panel itself - # (8080) is intentionally NOT published here: Apache reaches it over - # the host network at 127.0.0.1:8080 (see the vhost fragment), so the - # panel is never directly reachable from the internet without TLS. + # published directly — mail traffic bypasses Apache entirely, it only ever + # proxies the panel's HTTP(S). The panel itself (8080) is intentionally + # NOT published here: Apache reaches it over the host network at + # 127.0.0.1:8080 (see the vhost fragment), so the panel is never directly + # reachable from the internet without TLS. - "465:465" # 587 is mapped even when SUBMISSION_ENABLE=false; Postfix listens only # when the variable is true — see docs/guide.md "Published ports". @@ -88,7 +87,7 @@ services: # when the variable is true. - "25:25" - "127.0.0.1:8080:8080" - # Hardening (spec 10 p.6). SelfPost's entrypoint still needs to run as + # Hardening. SelfPost's entrypoint still needs to run as # root very briefly to fix /data ownership and normalise permissions # under the shared `selfpost` group (see build/entrypoint.sh) before # supervisord drops the panel to an unprivileged user — so this cannot be diff --git a/deploy/nginx/docker-compose.nginx.yml b/deploy/nginx/docker-compose.nginx.yml index 07bc567..1bc15a3 100644 --- a/deploy/nginx/docker-compose.nginx.yml +++ b/deploy/nginx/docker-compose.nginx.yml @@ -1,7 +1,7 @@ -# Alternative reverse-proxy: nginx (spec 10.3). PEM files land on the host -# exactly like the Apache+certbot scenario — nginx and certbot are close -# cousins here, both writing/reading plain PEM under /etc/letsencrypt. +# Alternative reverse-proxy: nginx. PEM files land on the host exactly like the +# Apache+certbot scenario — nginx and certbot are close cousins here, both +# writing/reading plain PEM under /etc/letsencrypt. # # This is a FRAGMENT, not a full replacement for ../docker-compose.yml: it # adds an nginx + certbot pair and removes the panel's host port publish (nginx @@ -29,9 +29,9 @@ services: volumes: !override - ./data:/data - .:/selfpost-deploy:ro - # Same host directory certbot below writes into — plain bind mount, - # no named volume, so the PEM files are as directly inspectable as in - # the Apache scenario (spec 10.3). + # Same host directory certbot below writes into — plain bind mount, no + # named volume, so the PEM files are as directly inspectable as in the + # Apache scenario. - ./nginx/certbot-etc/live/mail.example.com:/etc/postfix/tls:ro nginx: diff --git a/deploy/nginx/nginx.conf.example b/deploy/nginx/nginx.conf.example index a5e9ee3..4ca40a7 100644 --- a/deploy/nginx/nginx.conf.example +++ b/deploy/nginx/nginx.conf.example @@ -1,6 +1,6 @@ -# nginx vhost for the SelfPost panel (spec 10.3). Proxies HTTPS to the panel -# over the compose network; mail (465/587) is published directly by the -# selfpost container and never touches nginx. +# nginx vhost for the SelfPost panel. Proxies HTTPS to the panel over the +# compose network; mail (465/587) is published directly by the selfpost +# container and never touches nginx. server { listen 80; diff --git a/deploy/traefik/docker-compose.traefik.yml b/deploy/traefik/docker-compose.traefik.yml index 7674ebb..b2b6455 100644 --- a/deploy/traefik/docker-compose.traefik.yml +++ b/deploy/traefik/docker-compose.traefik.yml @@ -1,7 +1,7 @@ -# Alternative reverse-proxy: Traefik (spec 10.3). Traefik does NOT write plain -# PEM files — everything (cert + key) lives bundled inside acme.json. SelfPost -# needs separate PEM files for Postfix, so an extraction step is required; see +# Alternative reverse-proxy: Traefik. Traefik does NOT write plain PEM files — +# everything (cert + key) lives bundled inside acme.json. SelfPost needs +# separate PEM files for Postfix, so an extraction step is required; see # extract-cert.sh in this directory and the cron/systemd-timer note below. # # Merge with the base file — run this from the deploy/ directory (Compose diff --git a/deploy/traefik/extract-cert.sh b/deploy/traefik/extract-cert.sh index 4eb1bf4..e942512 100644 --- a/deploy/traefik/extract-cert.sh +++ b/deploy/traefik/extract-cert.sh @@ -1,9 +1,9 @@ #!/bin/sh -# Extracts a PEM cert/key pair for one domain out of Traefik's acme.json -# (spec 10.3: "Traefik keeps certificates in acme.json, so a PEM extraction -# step is required"). Run this on the host, after Traefik has issued or -# renewed the certificate, and again on a schedule (cron/systemd timer) since -# acme.json is not itself watched by SelfPost/Postfix. +# Extracts a PEM cert/key pair for one domain out of Traefik's acme.json — +# Traefik keeps its certificates there, so a PEM extraction step is required. +# Run this on the host, after Traefik has issued or renewed the certificate, and +# again on a schedule (cron/systemd timer) since acme.json is not itself watched +# by SelfPost/Postfix. # # Requires jq. Usage: ./extract-cert.sh set -eu diff --git a/docs/plans/code-review-2026-08.md b/docs/plans/code-review-2026-08.md index d31a115..30c36d8 100644 --- a/docs/plans/code-review-2026-08.md +++ b/docs/plans/code-review-2026-08.md @@ -311,3 +311,59 @@ SelfPost — зрелый, хорошо структурированный пр | 29 | Accessibility help drawer (§10.1) | Sonnet | 15 мин | | 30 | `io.LimitReader` для postqueue (§6.3) | Sonnet | 10 мин | | 31 | Extractить `internal/configsafe` (§5.6) | Sonnet | 30 мин | + +--- + +## Статус реализации + +Обновлено 2026-08-19 по факту проверки дерева (`main`). + +| # | Задача | Статус | +|---|--------|--------| +| 1 | CSS для DMARC-страниц | сделано | +| 2 | SPF CIDR | не требовалось — `coversAny()` уже разбирает CIDR | +| 3 | `io.LimitReader` в DMARC ingest/parse | сделано | +| 4 | Вытеснение bucket'ов | сделано; плюс потолок роста карты заблокированных | +| 5 | Логирование fail-open auth IP | не требовалось — лог уже был | +| 6 | Мёртвый код в demotion guard | сделано | +| 7 | Flash-сообщения в настройках | сделано | +| 8 | `.check-row` | сделано иначе: класс определён в `panel.css` | +| 9 | `internal/atomicfile` | сделано | +| 10 | `internal/supervisor` | сделано | +| 11 | Объединение rate-limit парсеров | сделано; обёртки-заглушки удалены | +| 12 | Plan-файлы / CHANGELOG | сделано | +| 13 | `schema-migrations.md` | сделано: заголовок `v10`, битые ссылки убраны | +| 14 | Ссылки на spec в build/deploy | сделано: `build/`, `deploy/`, миграции, workflow | +| 15 | `dmarc-ingest` symlink | не требовалось — уже описан в `architecture.md` | +| 16 | Ссылки в roadmap | сделано | +| 17 | Типизированные template data structs | **не сделано** | +| 18 | Миграция на `log/slog` | **не сделано** | +| 19 | Build tags для `machine.go` | отклонено, см. ниже | +| 20 | Guide URL → `{{.SourceURL}}` | сделано | +| 21 | Тесты DMARC service layer | **не сделано** | +| 22 | Тесты хендлеров | **не сделано** | +| 23 | postfix/dmarc injection cases | сделано | +| 24 | `validate_email_test` | сделано | +| 25 | Тесты error pages / битого бэкапа | **не сделано** | +| 26 | CSP `unsafe-inline` | не требовалось — директивы в CSP нет | +| 27 | bcrypt cost 12 | сделано, включая setup-аккаунт (`auth.BcryptCost`) | +| 28 | Re-auth перед бэкапом | **не сделано** | +| 29 | Accessibility help drawer | сделано | +| 30 | Ограничение `postqueue -p` | сделано: таймаут 5 с + лимит вывода 4 МиБ | +| 31 | `internal/configsafe` | сделано | + +**Отклонено с обоснованием.** §7.3 (задача 19): чтение `/proc` параметризовано +корнем каталога и не содержит платформенного кода, его тесты разбирают +подставные деревья и проходят на любой ОС, а отсутствие `/proc` уже +обрабатывается штатно (`TestMachineSamplerWithoutProc`). `//go:build linux` +не даёт ни выигрыша при сборке, ни в рантайме, но убирает эти тесты на машине +разработчика. + +**Открытый вопрос.** §4.4: решение «добавить per-session CSRF-токен» записано, +но задачи под него в таблицах фаз нет, и токенов в коде нет. Нужно либо +завести задачу, либо вернуть §4.4 в статус осознанного компромисса. + +**Вне фазовых таблиц** (в план не переносились и не делались): §5.5, §6.1, +§6.2, §6.4, §7.4, §8.6, §10.3, §10.4, §10.5, а также §9.3–9.4 сверх задач +23–25. + diff --git a/docs/schema-migrations.md b/docs/schema-migrations.md index 75e6e59..7b4fe05 100644 --- a/docs/schema-migrations.md +++ b/docs/schema-migrations.md @@ -70,13 +70,14 @@ replace the embedded set (see [Planned 2.x squash](#planned-2x-squash)). **Kind:** *DDL* — schema only; *data* — `INSERT`/`UPDATE` that must stay correct for operators upgrading from older 1.x images. -Feature plans that introduced schema work: [inbound-relay](plans/inbound-relay.md), -[domain-stats-auto-ratelimit](plans/domain-stats-auto-ratelimit.md), -[dmarc-reports](plans/dmarc-reports.md). +The feature plans that introduced schema work (`inbound-relay`, +`domain-stats-auto-ratelimit`, `dmarc-reports`) were deleted once they +shipped; what each migration was for is the table above, and the release it +came with is in [CHANGELOG.md](../CHANGELOG.md). --- -## Schema after head (v9) +## Schema after head (v10) Tables present in a fully migrated database: @@ -98,7 +99,7 @@ Tables present in a fully migrated database: | `dmarc_reports` | 0008 | Parsed aggregate report summaries | | `dmarc_report_records` | 0008 | Per-source rows inside a report | -**Not present after v9:** `admin` (dropped in 0005). +**Not present after v10:** `admin` (dropped in 0005). --- @@ -174,19 +175,19 @@ git; fresh 2.x installs only run the baseline plus post-2.x files. Tracked as [schema-squash](roadmap.md#schema-squash). **Not** a reason to cut 2.x on its own — only bundled with another breaking change or an explicit major. -**Goal:** embed one baseline SQL file equal to the v9 schema (plus any later 1.x -migrations if 2.x is cut later), instead of the full 1.x chain. Fresh 2.x -`/data` directories skip create-then-drop `admin`. +**Goal:** embed one baseline SQL file equal to the schema at whatever the 1.x +chain head is when the squash ships (`v10` today), instead of the full 1.x +chain. Fresh 2.x `/data` directories skip create-then-drop `admin`. **Upgrade gate (required when squash ships):** | `user_version` | 2.x behaviour | |---|---| | `0` (empty DB) | Apply baseline; set `user_version` to new chain head | -| `>= 9` (fully migrated 1.x) | Skip; schema already matches baseline | -| `1`…`8` (mid-chain 1.x) | **Refuse to start** — run the last 1.x image once, then 2.x | +| `>= N` (fully migrated 1.x) | Skip; schema already matches baseline | +| `1`…`N-1` (mid-chain 1.x) | **Refuse to start** — run the last 1.x image once, then 2.x | -Adjust the `>= N` and `1`…`N-1` thresholds to the chain head at cut time. +`N` is the chain head at cut time (`10` today). **Done when:** baseline embedded; gate tested; [guide.md](guide.md) states that 2.x will not open an unfinished 1.x database. diff --git a/internal/configsafe/configsafe.go b/internal/configsafe/configsafe.go new file mode 100644 index 0000000..6503425 --- /dev/null +++ b/internal/configsafe/configsafe.go @@ -0,0 +1,28 @@ +// Package configsafe holds the defence-in-depth check that every generated +// configuration file shares: a value about to be written into a table line must +// be non-empty and must not carry a character that could end the field, end the +// line, or otherwise change what the file means (security.md). +// +// The forbidden set stays with the caller rather than living here, because what +// breaks a line differs per format — a comma separates values in a Postfix map, +// a colon separates fields in an OpenDKIM table — and the set belongs next to +// the writer it protects, where a reviewer of that writer can see it. +package configsafe + +import ( + "fmt" + "strings" +) + +// Token reports whether value is safe to interpolate into a config line. what +// names the field for the error message ("domain", "login", …) and forbidden +// lists the characters the format cannot carry. +func Token(what, value, forbidden string) error { + if value == "" { + return fmt.Errorf("empty %s", what) + } + if strings.ContainsAny(value, forbidden) { + return fmt.Errorf("unsafe character in %s %q", what, value) + } + return nil +} diff --git a/internal/configsafe/configsafe_test.go b/internal/configsafe/configsafe_test.go new file mode 100644 index 0000000..8aef4b8 --- /dev/null +++ b/internal/configsafe/configsafe_test.go @@ -0,0 +1,25 @@ +package configsafe + +import "testing" + +// The check is the last line of defence before a value reaches a config file, +// so it must reject both halves of the problem: an empty value that would write +// a malformed line, and any character from the caller's forbidden set. +func TestToken(t *testing.T) { + if err := Token("domain", "example.com", " \t\r\n,\\"); err != nil { + t.Fatalf("clean value rejected: %v", err) + } + if err := Token("domain", "", " "); err == nil { + t.Fatal("empty value accepted") + } + for _, bad := range []string{"a b", "a\tb", "a\rb", "a\nb", "a,b", "a\\b"} { + if err := Token("domain", bad, " \t\r\n,\\"); err == nil { + t.Errorf("Token(%q) = nil, want error", bad) + } + } + // A character outside the caller's set is that format's business, not this + // function's: a colon is fatal in an OpenDKIM table and fine in a texthash. + if err := Token("domain", "a:b", " \t\r\n,\\"); err != nil { + t.Errorf("character outside the forbidden set rejected: %v", err) + } +} diff --git a/internal/domain/opendkim.go b/internal/domain/opendkim.go index 5983026..fcb9177 100644 --- a/internal/domain/opendkim.go +++ b/internal/domain/opendkim.go @@ -10,6 +10,7 @@ import ( "sort" "strings" + "github.com/mixeme/selfpost/internal/configsafe" "github.com/mixeme/selfpost/internal/supervisor" ) @@ -202,18 +203,23 @@ func renderTables(keysDir string, domains []SigningDomain) (keyTable, signingTab return []byte(kt.String()), []byte(st.String()), nil } +// opendkimForbidden is what may never appear in a KeyTable or SigningTable +// value: whitespace and the newline that would end the line, and the colon and +// path separators that carry meaning inside a table entry. +const opendkimForbidden = " \t\r\n:/\\" + // assertConfigSafe rejects any domain/selector value that could break out of a // single table line. Domains are already whitelisted to [a-z0-9.-] and selectors // to a similar set before they reach here (security.md); this is defence in depth // against a validation gap ever letting whitespace, a newline or a field // separator through into a config file (security.md). func assertConfigSafe(domainName, selector string) error { - for _, v := range []string{domainName, selector} { - if v == "" { - return fmt.Errorf("opendkim: empty domain or selector") - } - if strings.ContainsAny(v, " \t\r\n:/\\") { - return fmt.Errorf("opendkim: unsafe character in %q", v) + for _, f := range []struct{ what, value string }{ + {"domain", domainName}, + {"selector", selector}, + } { + if err := configsafe.Token(f.what, f.value, opendkimForbidden); err != nil { + return fmt.Errorf("opendkim: %w", err) } } return nil diff --git a/internal/postfix/inbound.go b/internal/postfix/inbound.go index 5b833d3..9d53618 100644 --- a/internal/postfix/inbound.go +++ b/internal/postfix/inbound.go @@ -6,6 +6,8 @@ import ( "sort" "strconv" "strings" + + "github.com/mixeme/selfpost/internal/configsafe" ) // InboundRoute is one inbound domain's Postfix map material: the relay @@ -115,13 +117,15 @@ func assertInboundRouteSafe(r InboundRoute) error { return nil } +// texthashForbidden is what may never appear in a texthash line: whitespace and +// the newline that would end it, the comma that separates values, and the +// backslash. +const texthashForbidden = " \t\r\n,\\" + // assertMapToken rejects values that could break out of a texthash line. func assertMapToken(v, what string) error { - if v == "" { - return fmt.Errorf("postfix: empty %s", what) - } - if strings.ContainsAny(v, " \t\r\n,\\") { - return fmt.Errorf("postfix: unsafe character in %s %q", what, v) + if err := configsafe.Token(what, v, texthashForbidden); err != nil { + return fmt.Errorf("postfix: %w", err) } return nil } diff --git a/internal/postfix/postfix.go b/internal/postfix/postfix.go index 7c20d7b..dfcf052 100644 --- a/internal/postfix/postfix.go +++ b/internal/postfix/postfix.go @@ -11,6 +11,7 @@ import ( "sort" "strings" + "github.com/mixeme/selfpost/internal/configsafe" "github.com/mixeme/selfpost/internal/supervisor" ) @@ -123,6 +124,15 @@ func appendUnique(list []string, v string) []string { return append(list, v) } +// What may never reach a sender-map line: whitespace and the newline that would +// end it, the comma that separates values, the colon a lookup key cannot carry, +// and the backslash. A login additionally may not carry "@" — that would make it +// look like an address. +const ( + senderMapAddressForbidden = " \t\r\n,:\\" + senderMapLoginForbidden = " \t\r\n,:@\\" +) + // assertMapSafe rejects any address/login value that could break out of a single // map line or inject a directive. Addresses are validated to a strict whitelist // (letters, digits, '@', '.', '-', '_', '+') and logins to an even stricter one @@ -130,14 +140,11 @@ func appendUnique(list []string, v string) []string { // letting whitespace, a newline or a comma (the value separator) through into // the file (security.md). func assertMapSafe(address, login string) error { - if address == "" || login == "" { - return fmt.Errorf("postfix: empty address or login") + if err := configsafe.Token("address", address, senderMapAddressForbidden); err != nil { + return fmt.Errorf("postfix: %w", err) } - if strings.ContainsAny(address, " \t\r\n,:\\") { - return fmt.Errorf("postfix: unsafe character in address %q", address) - } - if strings.ContainsAny(login, " \t\r\n,:@\\") { - return fmt.Errorf("postfix: unsafe character in login %q", login) + if err := configsafe.Token("login", login, senderMapLoginForbidden); err != nil { + return fmt.Errorf("postfix: %w", err) } return nil } diff --git a/internal/postfix/queue.go b/internal/postfix/queue.go index d629cdf..cd2ba08 100644 --- a/internal/postfix/queue.go +++ b/internal/postfix/queue.go @@ -1,6 +1,7 @@ package postfix import ( + "bytes" "context" "fmt" "os/exec" @@ -11,6 +12,35 @@ import ( const postqueueTimeout = 5 * time.Second +// maxQueueOutput bounds how much of `postqueue -p` the panel keeps. A deferred +// queue with a very large backlog prints one paragraph per message, and the +// whole listing is held in memory and then rendered into a page — so the read +// is capped and the remainder reported as truncated rather than being allowed +// to size the panel's memory by the size of the queue. +const maxQueueOutput = 4 << 20 + +// limitWriter keeps the first remaining bytes written to it and drops the rest, +// recording that it did. Writes always report full consumption so postqueue is +// never killed by a short write; the extra output is simply discarded. +type limitWriter struct { + buf bytes.Buffer + remaining int + truncated bool +} + +func (l *limitWriter) Write(p []byte) (int, error) { + keep := len(p) + if keep > l.remaining { + keep = l.remaining + l.truncated = true + } + if keep > 0 { + l.buf.Write(p[:keep]) + l.remaining -= keep + } + return len(p), nil +} + // Queue returns Postfix's own human-readable mail-queue listing // (architecture.md § Panel HTTP surface): active, deferred and held messages, // exactly as an administrator would see via the CLI. The command takes a @@ -21,14 +51,21 @@ func Queue() (string, error) { ctx, cancel := context.WithTimeout(context.Background(), postqueueTimeout) defer cancel() cmd := exec.CommandContext(ctx, "postqueue", "-p") - out, err := cmd.CombinedOutput() + out := &limitWriter{remaining: maxQueueOutput} + cmd.Stdout = out + cmd.Stderr = out + err := cmd.Run() if ctx.Err() == context.DeadlineExceeded { return "", fmt.Errorf("postqueue -p timed out after %s", postqueueTimeout) } if err != nil { - return "", fmt.Errorf("postqueue -p: %w: %s", err, strings.TrimSpace(string(out))) + return "", fmt.Errorf("postqueue -p: %w: %s", err, strings.TrimSpace(out.buf.String())) } - return string(out), nil + text := out.buf.String() + if out.truncated { + text += fmt.Sprintf("\n[listing truncated at %d bytes]\n", maxQueueOutput) + } + return text, nil } // QueueIDs returns the set of queue ids Postfix is still holding — everything diff --git a/internal/postfix/queue_test.go b/internal/postfix/queue_test.go index fbeb988..f9fbe85 100644 --- a/internal/postfix/queue_test.go +++ b/internal/postfix/queue_test.go @@ -41,3 +41,34 @@ func TestParseQueueIDsOnAnEmptyQueue(t *testing.T) { t.Errorf("got %v, want no ids", ids) } } + +// The queue listing is held in memory and rendered into a page, so a very large +// queue must not size the panel's allocation: the writer keeps the cap and +// reports the rest as dropped, while still accepting every write so postqueue +// itself is never cut short. +func TestLimitWriterCapsOutput(t *testing.T) { + w := &limitWriter{remaining: 8} + n, err := w.Write([]byte("12345")) + if n != 5 || err != nil { + t.Fatalf("first write = %d, %v", n, err) + } + if w.truncated { + t.Fatal("truncated before the cap was reached") + } + n, err = w.Write([]byte("67890")) + if n != 5 || err != nil { + t.Fatalf("second write = %d, %v", n, err) + } + if !w.truncated { + t.Fatal("write past the cap was not recorded as truncated") + } + if got := w.buf.String(); got != "12345678" { + t.Fatalf("kept %q, want the first 8 bytes", got) + } + if n, err := w.Write([]byte("more")); n != 4 || err != nil { + t.Fatalf("write after the cap = %d, %v (must still accept)", n, err) + } + if got := w.buf.String(); got != "12345678" { + t.Fatalf("buffer grew past the cap: %q", got) + } +} diff --git a/internal/store/migrations/0001_init.sql b/internal/store/migrations/0001_init.sql index d8ae924..c6f31f6 100644 --- a/internal/store/migrations/0001_init.sql +++ b/internal/store/migrations/0001_init.sql @@ -1,9 +1,9 @@ --- Initial SelfPost schema (spec 9). One SQLite file under /data holds the whole --- panel state so a single directory backup/restore is sufficient (spec 7.5.A). +-- Initial SelfPost schema. One SQLite file under /data holds the whole panel +-- state so a single directory backup/restore is sufficient. --- Single administrator account (spec 7.6.1). Exactly one row is allowed; the --- presence of that row is what marks primary setup as complete, which is why --- the /setup route disappears once it exists. +-- Single administrator account. Exactly one row is allowed; the presence of +-- that row is what marks primary setup as complete, which is why the /setup +-- route disappears once it exists. CREATE TABLE admin ( id INTEGER PRIMARY KEY CHECK (id = 1), username TEXT NOT NULL, @@ -17,8 +17,8 @@ CREATE TABLE settings ( value TEXT NOT NULL ); --- Sending domains managed through the panel (spec 4.1). DKIM keys themselves --- live on disk under /data; this row records the selector and metadata. +-- Sending domains managed through the panel. DKIM keys themselves live on disk +-- under /data; this row records the selector and metadata. CREATE TABLE domains ( id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL UNIQUE, @@ -26,8 +26,8 @@ CREATE TABLE domains ( created_at TEXT NOT NULL ); --- Applications bound to a domain (spec 4.1). address_mode is either the domain --- wildcard or an explicit address list; the SASL login is globally unique. +-- Applications bound to a domain. address_mode is either the domain wildcard or +-- an explicit address list; the SASL login is globally unique. CREATE TABLE applications ( id INTEGER PRIMARY KEY AUTOINCREMENT, domain_id INTEGER NOT NULL REFERENCES domains(id) ON DELETE CASCADE, @@ -37,7 +37,7 @@ CREATE TABLE applications ( ); -- Explicit sender addresses for applications in 'list' mode. Each address must --- belong to the application's domain (validated in the panel, spec 7.6.2). +-- belong to the application's domain (validated in the panel). CREATE TABLE application_addresses ( id INTEGER PRIMARY KEY AUTOINCREMENT, application_id INTEGER NOT NULL REFERENCES applications(id) ON DELETE CASCADE, @@ -45,8 +45,8 @@ CREATE TABLE application_addresses ( UNIQUE (application_id, address) ); --- Structured send log (spec 7.3). One row per (queue-id, recipient); the --- log-tailer advances status from queued to a final state. +-- Structured send log. One row per (queue-id, recipient); the log-tailer +-- advances status from queued to a final state. CREATE TABLE send_log ( id INTEGER PRIMARY KEY AUTOINCREMENT, queue_id TEXT, @@ -63,8 +63,8 @@ CREATE INDEX idx_send_log_queue_id ON send_log (queue_id); CREATE INDEX idx_send_log_domain ON send_log (domain); CREATE INDEX idx_send_log_created_at ON send_log (created_at); --- Differentiated rate limits per domain/application (spec 7.4). Both the IP --- binding and the message limit are optional. +-- Differentiated rate limits per domain/application. Both the IP binding and +-- the message limit are optional. CREATE TABLE rate_limits ( id INTEGER PRIMARY KEY AUTOINCREMENT, scope TEXT NOT NULL CHECK (scope IN ('domain', 'application')), diff --git a/internal/store/migrations/0002_sessions.sql b/internal/store/migrations/0002_sessions.sql index 7e14a16..22387cf 100644 --- a/internal/store/migrations/0002_sessions.sql +++ b/internal/store/migrations/0002_sessions.sql @@ -1,8 +1,8 @@ --- Panel login sessions (spec 7.6.6, plan B.1). Persisted so a login survives a --- container restart or redeploy; only the SHA-256 of the session token is --- stored, never the token itself, so a stolen database file cannot be used to --- sign in. expires_at implements the sliding idle timeout: it is pushed --- forward on activity rather than being fixed at creation time. +-- Panel login sessions. Persisted so a login survives a container restart or +-- redeploy; only the SHA-256 of the session token is stored, never the token +-- itself, so a stolen database file cannot be used to sign in. expires_at +-- implements the sliding idle timeout: it is pushed forward on activity rather +-- than being fixed at creation time. CREATE TABLE sessions ( token_hash TEXT PRIMARY KEY, username TEXT NOT NULL, diff --git a/internal/web/auth/handlers.go b/internal/web/auth/handlers.go index 331d039..bfce960 100644 --- a/internal/web/auth/handlers.go +++ b/internal/web/auth/handlers.go @@ -15,6 +15,13 @@ const ( sessionCookiePrefixed = "__Host-" + sessionCookieBase ) +// BcryptCost is the work factor for every panel password hash — the setup +// account created here and the ones the settings and user pages write +// (security.md). bcrypt records the cost inside the hash, so raising it only +// affects passwords set from now on; existing hashes keep verifying at the +// cost they were written with. +const BcryptCost = 12 + func (m *Module) sessionCookie() string { if m.cfg.CookieSecure { return sessionCookiePrefixed @@ -219,7 +226,7 @@ func (m *Module) submitSetup(w http.ResponseWriter, r *http.Request, token strin return } - hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + hash, err := bcrypt.GenerateFromPassword([]byte(password), BcryptCost) if err != nil { logf("panel: setup: hashing password failed: %v", err) m.renderSetupForm(w, http.StatusInternalServerError, token, "Internal error. Please try again.") diff --git a/internal/web/auth/ratelimit.go b/internal/web/auth/ratelimit.go index 6277935..db3f794 100644 --- a/internal/web/auth/ratelimit.go +++ b/internal/web/auth/ratelimit.go @@ -7,6 +7,16 @@ import ( const defaultMaxBuckets = 4096 +// blockedBucketCeiling is how far past maxBuckets the map may grow while every +// bucket in it is blocked. Blocked buckets are normally protected from +// eviction (see evictNearest), but protecting them unconditionally would let +// an attacker with many source addresses grow the map without bound — each +// address only has to fail max times. Past the ceiling blocked buckets are +// evicted too, nearest expiry first: bounded memory wins over holding every +// block, and the addresses that lose their bucket have to spend max attempts +// again to get back to blocked. +const blockedBucketCeiling = 4 + // rateLimiter is a simple fixed-window per-key counter used to throttle the // setup and login routes (security.md). Keys are client IPs. type rateLimiter struct { @@ -14,8 +24,9 @@ type rateLimiter struct { window time.Duration maxBuckets int - mu sync.Mutex - buckets map[string]*rlBucket + mu sync.Mutex + buckets map[string]*rlBucket + lastOverflow time.Time } type rlBucket struct { @@ -68,22 +79,38 @@ func (r *rateLimiter) makeRoom(now time.Time) { return } r.sweep(now) + hardCap := r.maxBuckets * blockedBucketCeiling for len(r.buckets) >= r.maxBuckets { - if !r.evictNearest() { + if r.evictNearest(false) { + continue + } + // Nothing evictable is left: every remaining bucket is blocked. Hold + // them until the map reaches the hard ceiling, then start evicting + // blocked buckets as well so memory stays bounded. + if len(r.buckets) < hardCap { break } + if !r.evictNearest(true) { + break + } + if now.Sub(r.lastOverflow) >= r.window { + r.lastOverflow = now + logf("panel: login rate limiter holds %d blocked buckets (cap %d) — evicting blocked entries; "+ + "this means many distinct client addresses are failing authentication", len(r.buckets), r.maxBuckets) + } } } -// evictNearest removes the bucket whose window ends soonest, skipping buckets -// that are currently blocked (count >= max) so an attacker cannot flush active -// rate-limit state by filling the map with new keys. -func (r *rateLimiter) evictNearest() bool { +// evictNearest removes the bucket whose window ends soonest and reports whether +// it removed one. Blocked buckets (count >= max) are skipped unless evictBlocked +// is set, so an attacker cannot flush active rate-limit state by filling the map +// with new keys. +func (r *rateLimiter) evictNearest(evictBlocked bool) bool { var bestKey string var bestEnds time.Time first := true for k, b := range r.buckets { - if b.count >= r.max { + if !evictBlocked && b.count >= r.max { continue } if first || b.windowEnds.Before(bestEnds) { diff --git a/internal/web/auth/ratelimit_test.go b/internal/web/auth/ratelimit_test.go index 696bff9..2893e0a 100644 --- a/internal/web/auth/ratelimit_test.go +++ b/internal/web/auth/ratelimit_test.go @@ -1,6 +1,7 @@ package auth import ( + "fmt" "testing" "time" ) @@ -123,3 +124,53 @@ func TestRateLimiterCapsBucketCount(t *testing.T) { t.Fatalf("bucket count = %d, want at most 3", len(r.buckets)) } } + +// A flood of new addresses must not be able to reset the limiter for an address +// that is already blocked: below the hard ceiling, blocked buckets survive +// eviction even though the map is over its soft cap. +func TestRateLimiterKeepsBlockedBucketsUnderPressure(t *testing.T) { + r := newRateLimiter(2, time.Minute) + r.maxBuckets = 3 + + blocked := []string{"203.0.113.7", "198.51.100.9", "192.0.2.5"} + for _, key := range blocked { + for i := 0; i < 2; i++ { + if !r.Allow(key) { + t.Fatalf("%s refused before reaching the limit", key) + } + } + if r.Allow(key) { + t.Fatalf("%s was not blocked after 2 attempts", key) + } + } + + // Fresh addresses arrive and find nothing evictable. + for i := 0; i < 10; i++ { + r.Allow(fmt.Sprintf("198.51.100.%d", 100+i)) + } + + for _, key := range blocked { + if r.Allow(key) { + t.Fatalf("%s lost its block after new addresses arrived", key) + } + } +} + +// Protecting blocked buckets is bounded: past maxBuckets * blockedBucketCeiling +// they are evicted too, so an attacker cannot grow the map without limit by +// burning through source addresses. +func TestRateLimiterBoundsBlockedBuckets(t *testing.T) { + r := newRateLimiter(1, time.Minute) + r.maxBuckets = 2 + hardCap := r.maxBuckets * blockedBucketCeiling + + for i := 0; i < 100; i++ { + r.Allow(fmt.Sprintf("192.0.2.%d", i)) + } + + r.mu.Lock() + defer r.mu.Unlock() + if len(r.buckets) > hardCap { + t.Fatalf("bucket count = %d, want at most the hard cap %d", len(r.buckets), hardCap) + } +} diff --git a/internal/web/handlers/handlers_ratelimit.go b/internal/web/handlers/handlers_ratelimit.go index 7cd9e97..651804e 100644 --- a/internal/web/handlers/handlers_ratelimit.go +++ b/internal/web/handlers/handlers_ratelimit.go @@ -98,14 +98,6 @@ func parseRateLimitForm(r *http.Request, l1Max int) (rateLimitInput, error) { return rateLimitInput{mode: store.RateLimitModeManual, maxMessages: maxMessages, windowSeconds: windowSeconds}, nil } -func parseDomainRateLimitForm(r *http.Request, l1Max int) (rateLimitInput, error) { - return parseRateLimitForm(r, l1Max) -} - -func parseAppRateLimitForm(r *http.Request, l1Max int) (rateLimitInput, error) { - return parseRateLimitForm(r, l1Max) -} - func parseAppAuthIPsForm(r *http.Request) (bool, []string, error) { if err := r.ParseForm(); err != nil { return false, nil, fmt.Errorf("invalid form submission") diff --git a/internal/web/handlers/handlers_ratelimit_test.go b/internal/web/handlers/handlers_ratelimit_test.go index 4caaa51..f4ef2a3 100644 --- a/internal/web/handlers/handlers_ratelimit_test.go +++ b/internal/web/handlers/handlers_ratelimit_test.go @@ -8,7 +8,9 @@ import ( "testing" ) -func TestParseDomainRateLimitForm(t *testing.T) { +// Domain and application limits are parsed by the same function; the form +// fields and the level-1 backstop are identical for both scopes. +func TestParseRateLimitForm(t *testing.T) { t.Parallel() form := func(vals url.Values) *http.Request { r := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(vals.Encode())) @@ -16,7 +18,7 @@ func TestParseDomainRateLimitForm(t *testing.T) { return r } - in, err := parseDomainRateLimitForm(form(url.Values{ + in, err := parseRateLimitForm(form(url.Values{ "mode": {"manual"}, "max_messages": {"50"}, "window_seconds": {"3600"}, @@ -25,44 +27,12 @@ func TestParseDomainRateLimitForm(t *testing.T) { t.Fatalf("valid domain = %+v err=%v", in, err) } - in, err = parseDomainRateLimitForm(form(url.Values{"max_messages": {""}}), 100) + in, err = parseRateLimitForm(form(url.Values{"max_messages": {""}}), 100) if err != nil || !in.clear { t.Fatalf("empty max should clear: %+v err=%v", in, err) } - _, err = parseDomainRateLimitForm(form(url.Values{ - "mode": {"manual"}, - "max_messages": {"150"}, - "window_seconds": {"3600"}, - }), 100) - if err == nil || !strings.Contains(err.Error(), "level-1") { - t.Fatalf("over L1 want error, got %v", err) - } -} - -func TestParseAppRateLimitForm(t *testing.T) { - t.Parallel() - form := func(vals url.Values) *http.Request { - r := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(vals.Encode())) - r.Header.Set("Content-Type", "application/x-www-form-urlencoded") - return r - } - - in, err := parseAppRateLimitForm(form(url.Values{ - "mode": {"manual"}, - "max_messages": {"80"}, - "window_seconds": {"3600"}, - }), 100) - if err != nil || in.maxMessages != 80 { - t.Fatalf("valid app limit = %+v err=%v", in, err) - } - - in, err = parseAppRateLimitForm(form(url.Values{"max_messages": {""}}), 100) - if err != nil || !in.clear { - t.Fatalf("empty max should clear: %+v err=%v", in, err) - } - - _, err = parseAppRateLimitForm(form(url.Values{ + _, err = parseRateLimitForm(form(url.Values{ "mode": {"manual"}, "max_messages": {"150"}, "window_seconds": {"3600"}, @@ -99,21 +69,21 @@ func TestParseAppAuthIPsForm(t *testing.T) { } } -func TestParseDomainRateLimitFormAuto(t *testing.T) { +func TestParseRateLimitFormAuto(t *testing.T) { t.Parallel() form := func(vals url.Values) *http.Request { r := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(vals.Encode())) r.Header.Set("Content-Type", "application/x-www-form-urlencoded") return r } - in, err := parseDomainRateLimitForm(form(url.Values{ + in, err := parseRateLimitForm(form(url.Values{ "mode": {"auto"}, "auto_multiplier": {"2.5"}, }), 100) if err != nil || in.mode != "auto" || in.autoMultiplier != 2.5 { t.Fatalf("auto domain = %+v err=%v", in, err) } - _, err = parseDomainRateLimitForm(form(url.Values{ + _, err = parseRateLimitForm(form(url.Values{ "mode": {"auto"}, "auto_multiplier": {"10"}, }), 100) diff --git a/internal/web/handlers/password.go b/internal/web/handlers/password.go index 0a0aa11..1ff36ef 100644 --- a/internal/web/handlers/password.go +++ b/internal/web/handlers/password.go @@ -1,3 +1,7 @@ package handlers -const bcryptCost = 12 +import "github.com/mixeme/selfpost/internal/web/auth" + +// bcryptCost is the panel-wide password work factor; see auth.BcryptCost for +// why it lives in one place. +const bcryptCost = auth.BcryptCost