3 Commits
Author SHA1 Message Date
clawbot 573f7f986a Build the image's static binary with netgo, osusergo and sqlite_omit_load_extension (closes #525)
check / check (push) Waiting to run
The image's static relink linked glibc's getaddrinfo (from Go's cgo DNS
resolver) and dlopen (from SQLite extension loading). A static binary
still needs glibc's shared libraries at run time for both, and the
Alpine runtime has none. The relink now passes the standard build tags
netgo, osusergo and sqlite_omit_load_extension through a GO_TAGS
variable, which the Makefile's build target hands to go build as -tags
next to GO_LDFLAGS, so the version stamp is composed as before.

Model: opus-5-5
2026-10-13 10:43:47 +02:00
clawbot 373a2f828d Describe code in its own terms, not by an earlier build's data (closes #522)
check / check (push) In progress
Several comments justified code by what an earlier build would have
left in a database or on disk. Each now says what the code does:
reserveSQLiteFile sets the mode on every open whoever created the
file, and expired treats a session lacking a deadline's timestamp as
expired. The comments on retention, resubmitted events, archive
pruning, target headers, the redactor and the delivery result view
drop their mentions of older rows. parseMaxRetries loses its
paragraph on targets saved before the ceiling existed, and neither
BeforeSave nor retentionCutoff cites a migration. Comments only.

Model: opus-5-5
2026-10-13 09:57:24 +02:00
clawbot 267d5b12f2 Escape the database path in the SQLite connection string (closes #518)
check / check (push) Successful in 7m13s
SQLite reads a file: URI's path only up to the first ? or #, and
decodes %XX escapes in it. With a data directory such as
/srv/hooks#1, every database opened the one file /srv/hooks. The
connection string is now built with url.URL, which escapes the path,
and OmitHost keeps a relative path relative.

Tests open databases in a directory whose name holds #, ? and %, and
at a relative path, and check through PRAGMA database_list that each
opened the file asked for. A handlers test comment that blamed # in
the temporary directory's path is dropped, as that is no longer true.

Model: opus-5-5
2026-10-13 09:30:24 +02:00
16 changed files with 145 additions and 60 deletions
+15 -9
View File
@@ -214,16 +214,22 @@ RUN version="$(make version VERSION="$VERSION")"; \
# (Alpine.js, extracted from its tarball in 3p/) first. # (Alpine.js, extracted from its tarball in 3p/) first.
RUN make build VERSION="$VERSION" RUN make build VERSION="$VERSION"
# Rebuild with static linking for Alpine runtime. # Rebuild with static linking for the Alpine runtime. The cgo binary the
# make build already verified compilation. # step above builds is dynamically linked against glibc, which Alpine
# The CGO binary from `make build` is dynamically linked against glibc, # (musl) does not have.
# which doesn't exist on Alpine (musl). Rebuild with static linking so
# the binary runs on Alpine without glibc.
# #
# The static flags go in through GO_LDFLAGS rather than a -ldflags of # A statically linked glibc still loads shared libraries at run time for
# their own: the build target composes them with the -X that stamps the # getaddrinfo and dlopen, and Alpine has none to load, so the build tags
# version, so this relink cannot silently drop the stamp. # keep both out of the binary: netgo and osusergo select Go's own DNS
RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"' # resolver and user lookup instead of glibc's, and
# sqlite_omit_load_extension leaves out SQLite extension loading, which
# webhooker never uses.
#
# The static flags and tags go in through GO_LDFLAGS and GO_TAGS rather
# than a -ldflags of their own: the build target composes them with the
# -X that stamps the version, so this relink cannot silently drop the
# stamp.
RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_TAGS=netgo,osusergo,sqlite_omit_load_extension GO_LDFLAGS='-extldflags "-static"'
# Runtime stage # Runtime stage
# alpine:3.21, 2026-03-17 # alpine:3.21, 2026-03-17
+6 -4
View File
@@ -16,14 +16,16 @@ VERSION ?= $(shell script/version)
# exists to correct. # exists to correct.
override VERSION := $(or $(strip $(VERSION)),$(shell script/version)) override VERSION := $(or $(strip $(VERSION)),$(shell script/version))
# Extra linker flags for the build target. The static relink in the # Extra linker flags and build tags for the build target. The static
# Dockerfile adds -extldflags here rather than passing its own -ldflags, # relink in the Dockerfile passes -extldflags and its build tags through
# so composing flags cannot drop the version stamp. # these rather than a -ldflags of its own, so composing flags cannot drop
# the version stamp.
# #
# The build target itself always passes -trimpath and -s -w, as the Go # The build target itself always passes -trimpath and -s -w, as the Go
# Dockerfile in REPO_POLICIES.md does: no build paths, symbol table or # Dockerfile in REPO_POLICIES.md does: no build paths, symbol table or
# debug information in the binary. # debug information in the binary.
GO_LDFLAGS ?= GO_LDFLAGS ?=
GO_TAGS ?=
bootstrap: bootstrap:
@script/bootstrap @script/bootstrap
@@ -53,7 +55,7 @@ check:
@script/check @script/check
build: assets build: assets
go build -trimpath -ldflags '$(strip -s -w -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker go build -trimpath -tags '$(GO_TAGS)' -ldflags '$(strip -s -w -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
run: build run: build
./bin/webhooker ./bin/webhooker
+8 -6
View File
@@ -3419,12 +3419,14 @@ fixed independently of the compiler's:
phases and the `css-check` and `markdown-check` stages passing (it copies a phases and the `css-check` and `markdown-check` stages passing (it copies a
file from each), runs `make build` (which extracts Alpine.js from `3p/` file from each), runs `make build` (which extracts Alpine.js from `3p/`
first), and then rebuilds the binary with `CGO_ENABLED=1` and static linking first), and then rebuilds the binary with `CGO_ENABLED=1` and static linking
so it runs on musl. Both builds go through `make build`, the relink adding so it runs on musl, with the build tags `netgo`, `osusergo` and
its `-extldflags` via `GO_LDFLAGS`, so neither can drop the `-X` that stamps `sqlite_omit_load_extension` so it needs none of glibc's shared libraries at
the version. The version is the `VERSION` build arg if one is given, run time. Both builds go through `make build`, the relink adding its
otherwise derived from the `.git` in the context, and the stage fails if a `-extldflags` via `GO_LDFLAGS` and its tags via `GO_TAGS`, so neither can
context with `.git` would stamp an empty version, `dev` or `unknown` (see drop the `-X` that stamps the version. The version is the `VERSION` build arg
[Version stamping](#version-stamping)). if one is given, otherwise derived from the `.git` in the context, and the
stage fails if a context with `.git` would stamp an empty version, `dev` or
`unknown` (see [Version stamping](#version-stamping)).
7. **Runtime stage** (`alpine:3.21`) — copies the static binary and 7. **Runtime stage** (`alpine:3.21`) — copies the static binary and
`deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` directory for `deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` directory for
all SQLite databases, exposes port 8080, and includes a health check against all SQLite databases, exposes port 8080, and includes a health check against
+1 -2
View File
@@ -45,8 +45,7 @@ type Event struct {
// ResubmittedFromID names the event this one was copied from by // ResubmittedFromID names the event this one was copied from by
// an operator resubmit. It is nil for an event that arrived on // an operator resubmit. It is nil for an event that arrived on
// the receiver, which is every event created before the column // the receiver. It is not a foreign key: the source event can be
// existed. It is not a foreign key: the source event can be
// reaped by retention while its copies remain, and the id is // reaped by retention while its copies remain, and the id is
// kept as the record of where the copy came from either way. // kept as the record of where the copy came from either way.
ResubmittedFromID *string `gorm:"type:uuid;index:idx_events_resubmitted_from_id,priority:1;index:idx_events_entrypoint_id,priority:3" json:"resubmittedFromId,omitempty"` ResubmittedFromID *string `gorm:"type:uuid;index:idx_events_resubmitted_from_id,priority:1;index:idx_events_entrypoint_id,priority:3" json:"resubmittedFromId,omitempty"`
+3 -3
View File
@@ -82,7 +82,7 @@ type Webhook struct {
// value while building the insert statement, which runs after // value while building the insert statement, which runs after
// BeforeSave; rewriting any later than this loses that race and the // BeforeSave; rewriting any later than this loses that race and the
// row lands at 30 days. Living on the model also means a future call // row lands at 30 days. Living on the model also means a future call
// site — a REST API, a fixture, a migration — cannot bypass it. // site, such as a REST API or a fixture, cannot bypass it.
func (w *Webhook) BeforeSave(_ *gorm.DB) error { func (w *Webhook) BeforeSave(_ *gorm.DB) error {
if w.RetentionDays <= 0 { if w.RetentionDays <= 0 {
w.RetentionDays = RetentionForeverDays w.RetentionDays = RetentionForeverDays
@@ -98,8 +98,8 @@ func (w *Webhook) BeforeSave(_ *gorm.DB) error {
// are exempt from reaping. // are exempt from reaping.
// //
// It accepts the RetentionForeverDays sentinel written by BeforeSave // It accepts the RetentionForeverDays sentinel written by BeforeSave
// and, defensively, the non-positive values that rows written before // and, defensively, any non-positive value, which BeforeSave treats
// the sentinel existed may still carry. // as a request to retain forever.
func retainsForever(retentionDays int) bool { func retainsForever(retentionDays int) bool {
return retentionDays <= 0 || return retentionDays <= 0 ||
retentionDays >= RetentionForeverDays retentionDays >= RetentionForeverDays
+1 -1
View File
@@ -253,7 +253,7 @@ func (r *RetentionReaper) reapWebhook(
// row in the database: the sweep then deletes every event, delivery, // row in the database: the sweep then deletes every event, delivery,
// and delivery result, including ones created seconds ago. Rejecting // and delivery result, including ones created seconds ago. Rejecting
// out-of-range input at the form is the primary guard; saturating here // out-of-range input at the form is the primary guard; saturating here
// means an old row, a migration, or a future call site cannot turn a // means a value that reaches the row by any other path cannot turn a
// too-large retention into total data loss. // too-large retention into total data loss.
func retentionCutoff( func retentionCutoff(
now time.Time, now time.Time,
+15 -4
View File
@@ -118,9 +118,8 @@ const SQLiteFilePerm fs.FileMode = 0o600
// SQLite still reports the absence rather than this function // SQLite still reports the absence rather than this function
// materializing an empty database the caller asked not to create. // materializing an empty database the caller asked not to create.
// //
// Chmod of a file that already exists is what tightens a data // The mode is set on every open, whoever created the file, so a
// directory an earlier build left at 0644 — including a developer's // database or sidecar found at a wider mode is tightened too.
// own scratch directory — without any migration machinery.
func reserveSQLiteFile(path string, create bool) error { func reserveSQLiteFile(path string, create bool) error {
if create { if create {
// gosec G304: the path is the database file the caller asked // gosec G304: the path is the database file the caller asked
@@ -242,7 +241,19 @@ func SQLiteDSN(path, mode string) string {
) )
q.Add("_pragma", "journal_mode(WAL)") q.Add("_pragma", "journal_mode(WAL)")
return "file:" + path + "?" + q.Encode() // SQLite reads a file: URI's path only up to the first ? or #, and
// decodes %XX escapes in it, so url.URL escapes the path. OmitHost
// keeps a relative path as "file:data/x.db"; written
// "file://data/x.db", SQLite would reject "data" as the URI's
// authority.
dsn := url.URL{
Scheme: "file",
Path: path,
OmitHost: true,
RawQuery: q.Encode(),
}
return dsn.String()
} }
// OpenSQLite opens the SQLite file at path with the service's // OpenSQLite opens the SQLite file at path with the service's
+75
View File
@@ -2,6 +2,8 @@ package database_test
import ( import (
"context" "context"
"database/sql"
"os"
"path/filepath" "path/filepath"
"strings" "strings"
"testing" "testing"
@@ -112,6 +114,79 @@ func TestMainDBAppliesPragmasOnALiveHandle(t *testing.T) {
assert.Equal(t, "10000", busy) assert.Equal(t, "10000", busy)
} }
// TestOpenSQLiteOpensThePathAskedFor uses a data directory whose name
// holds ?, # and %. SQLite reads a file: URI's path only up to a ? or
// #, and decodes %41 to A, so an unescaped path sent every database in
// that directory to one file outside it.
func TestOpenSQLiteOpensThePathAskedFor(t *testing.T) {
t.Parallel()
dir := filepath.Join(t.TempDir(), "hooks#1?x%41")
require.NoError(t, os.Mkdir(dir, 0o700))
for _, name := range []string{
database.MainDBFileName,
"events-" + uuid.New().String() + ".db",
} {
path := filepath.Join(dir, name)
sqlDB, err := database.OpenSQLite(
path, database.SQLiteModeCreate,
)
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, sqlDB.Close()) })
assertOpenedFile(t, sqlDB, path)
}
}
// TestOpenSQLiteOpensARelativePath opens a database at a path relative
// to the working directory, which is what a relative DATA_DIR such as
// the README's DATA_DIR=../webhooker-data gives OpenSQLite.
//
//nolint:paralleltest // t.Chdir moves the whole process.
func TestOpenSQLiteOpensARelativePath(t *testing.T) {
t.Chdir(t.TempDir())
require.NoError(t, os.Mkdir("data", 0o700))
path := filepath.Join("data", database.MainDBFileName)
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
require.NoError(t, err)
defer func() { require.NoError(t, sqlDB.Close()) }()
assertOpenedFile(t, sqlDB, path)
}
// assertOpenedFile checks that sqlDB is the SQLite file at path.
// database_list names the file SQLite actually opened, as an absolute
// path with symlinks resolved, so the files are compared, not the names.
func assertOpenedFile(t *testing.T, sqlDB *sql.DB, path string) {
t.Helper()
var (
seq int
schema, file string
)
require.NoError(t, sqlDB.
QueryRowContext(t.Context(), "pragma database_list").
Scan(&seq, &schema, &file))
asked, err := os.Stat(path)
require.NoError(t, err)
opened, err := os.Stat(file)
require.NoError(t, err)
assert.True(
t, os.SameFile(asked, opened),
"asked for %s, SQLite opened %s", path, file,
)
}
// TestConcurrentReaderDoesNotBlockWrites is the unit-scale form of the // TestConcurrentReaderDoesNotBlockWrites is the unit-scale form of the
// reproduction in // reproduction in
// https://git.eeqj.de/sneak/webhooker/issues/256: an operator's // https://git.eeqj.de/sneak/webhooker/issues/256: an operator's
+1 -2
View File
@@ -201,8 +201,7 @@ func (s *ArchiveSweeper) sweep(ctx context.Context) {
// sweepTarget prunes the archive of a single database target. // sweepTarget prunes the archive of a single database target.
// A missing, empty, or "never" expiry parses as a zero duration // A missing, empty, or "never" expiry parses as a zero duration
// and is skipped entirely, so those archives keep exactly the // and is skipped entirely, so those archives are never pruned.
// behaviour they had before the sweep existed.
func (s *ArchiveSweeper) sweepTarget(target *database.Target) { func (s *ArchiveSweeper) sweepTarget(target *database.Target) {
expiry, err := parseArchiveExpiry(target.Config) expiry, err := parseArchiveExpiry(target.Config)
if err != nil { if err != nil {
+2 -2
View File
@@ -82,8 +82,8 @@ func isReservedTargetHeader(name string) bool {
// one pair silently overwriting the other. // one pair silently overwriting the other.
// //
// An input with no pairs yields an empty map, which omitempty drops // An input with no pairs yields an empty map, which omitempty drops
// from the stored config: a target configured with no headers keeps // from the stored config, so a target configured with no headers
// the same config JSON it had before this field existed. // stores no headers key at all.
func ParseTargetHeaders(raw string) (map[string]string, error) { func ParseTargetHeaders(raw string) (map[string]string, error) {
headers := make(map[string]string) headers := make(map[string]string)
+3 -3
View File
@@ -13,9 +13,9 @@ import (
const RedactionMarker = "(redacted)" const RedactionMarker = "(redacted)"
// Redactor removes one target's own credential material from // Redactor removes one target's own credential material from
// text that target's remote peer chose: a delivery response // a delivery response body, whose text the target's remote
// body, or a delivery error stored before the delivery path // peer chose, and from a stored delivery error as a second
// learned to mask the URLs it embeds. // line behind the delivery path's own URL masking.
// //
// It removes byte-identical echoes of strings taken from the // It removes byte-identical echoes of strings taken from the
// target's stored configuration, and nothing else. Anything // target's stored configuration, and nothing else. Anything
@@ -42,9 +42,6 @@ func TestDeliveryAttempts_ReadInTheTargetTypesOwnTerms(t *testing.T) {
{database.TargetTypeLog, true, 0, "", "written to the log", ""}, {database.TargetTypeLog, true, 0, "", "written to the log", ""},
} }
// Named for type and outcome, so that no two cases share a name:
// Go adds "#01" to a repeated one, and a "#" in the temporary
// directory's path cuts the path SQLiteDSN gives SQLite short.
for _, tc := range cases { for _, tc := range cases {
t.Run(string(tc.targetType)+" "+tc.outcome, func(t *testing.T) { t.Run(string(tc.targetType)+" "+tc.outcome, func(t *testing.T) {
t.Parallel() t.Parallel()
+6 -8
View File
@@ -12,13 +12,12 @@ import (
// //
// The bound is enforced here and in SQL, because this page's // The bound is enforced here and in SQL, because this page's
// memory profile must not depend on a constant in another // memory profile must not depend on a constant in another
// package staying where it is, and because rows predating the // package staying where it is, or on every stored row having
// delivery engine's own cap or restored from an archive are // been written by the delivery engine under its own cap.
// not covered by it at all.
// //
// It happens to equal that engine cap, so a row written by the // It happens to equal that engine cap, so a row written by the
// current engine reaches this one exactly and is never cut // engine reaches this one exactly and is never cut twice.
// twice. Nothing here may assume the two differ: see view. // Nothing here may assume the two differ: see view.
const maxRenderedResponseBytes = 4096 const maxRenderedResponseBytes = 4096
// deliveryResultColumns is the delivery attempt projection. // deliveryResultColumns is the delivery attempt projection.
@@ -40,9 +39,8 @@ const deliveryResultColumns = "delivery_id, attempt_num, created_at, " +
// //
// Both Error and ResponseBody have been through the target's // Both Error and ResponseBody have been through the target's
// Redactor. The engine already masks the URL out of the // Redactor. The engine already masks the URL out of the
// errors it stores, so for errors this is a second line // errors it stores, so for errors this is a second line; for
// covering rows written before it did; for response bodies it // response bodies it is the only line, and its reach is what
// is the only line, and its reach is what
// delivery.Redactor documents. // delivery.Redactor documents.
type DeliveryResultView struct { type DeliveryResultView struct {
AttemptNum int AttemptNum int
-7
View File
@@ -49,13 +49,6 @@ var (
// working retry configuration over a typo. A default answers a // working retry configuration over a typo. A default answers a
// question that was not asked; it never answers one that was asked // question that was not asked; it never answers one that was asked
// badly. // badly.
//
// A target stored with a count above the ceiling before this
// validation existed keeps rendering and keeps delivering — nothing
// clamps the row. Re-saving it from the edit form does have to bring
// it into range, because the form submits the pre-filled value back
// and accepting it would be the ceiling not applying to the edit
// path. The 400 names the ceiling, so the fix is one field.
func parseMaxRetries(raw string, fallback int) (int, error) { func parseMaxRetries(raw string, fallback int) (int, error) {
raw = strings.TrimSpace(raw) raw = strings.TrimSpace(raw)
if raw == "" { if raw == "" {
+3 -4
View File
@@ -441,10 +441,9 @@ func (s *Session) Regenerate(
// //
// Whichever comes first ends the session. // Whichever comes first ends the session.
// //
// A session that claims to be authenticated but carries no // A session that claims to be authenticated but lacks the timestamp
// timestamps predates this check; it is treated as expired so the // a deadline is measured from is treated as expired, so the user
// user re-authenticates rather than being granted an unbounded // re-authenticates rather than being granted an unbounded session.
// session.
func (s *Session) expired(sess *sessions.Session) bool { func (s *Session) expired(sess *sessions.Session) bool {
now := s.now() now := s.now()
@@ -156,7 +156,8 @@ func TestVersion_IsStableAcrossInvocations(t *testing.T) {
// The build target is the only place composing linker flags. If a // The build target is the only place composing linker flags. If a
// future edit drops either half, the binary silently reports "dev" // future edit drops either half, the binary silently reports "dev"
// again (the defect this package exists for) or fails to link on // again (the defect this package exists for) or fails to link on
// Alpine. // Alpine. If it stops passing GO_TAGS, the image's static binary links
// glibc's getaddrinfo and dlopen again.
func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) { func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
t.Parallel() t.Parallel()
@@ -164,6 +165,7 @@ func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
require.Contains(t, makefile, "-X main.version=$(VERSION)") require.Contains(t, makefile, "-X main.version=$(VERSION)")
require.Contains(t, makefile, "$(GO_LDFLAGS)") require.Contains(t, makefile, "$(GO_LDFLAGS)")
require.Contains(t, makefile, "-tags '$(GO_TAGS)'")
require.Contains(t, makefile, "VERSION ?= $(shell script/version)") require.Contains(t, makefile, "VERSION ?= $(shell script/version)")
} }
@@ -233,7 +235,9 @@ func TestDockerfile_BuildsThroughTheMakeTarget(t *testing.T) {
"a raw go build bypasses the Makefile's -X flag") "a raw go build bypasses the Makefile's -X flag")
require.Contains(t, dockerfile, "ARG VERSION") require.Contains(t, dockerfile, "ARG VERSION")
require.Contains(t, dockerfile, require.Contains(t, dockerfile,
`make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'`) `make build VERSION="$VERSION"`+
` GO_TAGS=netgo,osusergo,sqlite_omit_load_extension`+
` GO_LDFLAGS='-extldflags "-static"'`)
} }
func read(t *testing.T, path string) string { func read(t *testing.T, path string) string {