Log SQL with placeholders, never bound values (closes #207)
All checks were successful
check / check (push) Successful in 3m39s

With DEBUG=true the GORM adapter logged fully interpolated statements.
On a first boot that put two secrets in the log: the INSERT into
settings carrying the base64 session encryption key -- which is the
whole of the session security model, since anyone holding it can forge
an authenticated session cookie -- and the INSERT into users carrying
the admin account's Argon2id password hash. Debug logs get pasted into
issues and chats.

internal/gormlog.Logger now implements gorm.ParamsFilter and discards
the bound values, so GORM renders the statement with its placeholders
intact instead of substituting them in. This is unconditional rather
than a denylist of tables known to hold a secret: a table added later
is covered without anyone remembering to add it, and the cost of
missing one is a credential in a log. It applies at every level,
including the routine arm an operator reaches at DEBUG, which is the
only level at which a successful INSERT is written at all.

Truncation was never a fix for this. The session key is 44 base64
characters and an Argon2id hash under 100, so both fit inside every
budget the adapter applies; a truncated secret is still a secret.

internal/gormlog/firstboot_test.go boots the real graph -- config.New
reading DEBUG from the environment, internal/logger building its
production handler, database.New migrating and creating the admin
user, session.New taking the session key -- against an empty DATA_DIR,
captures stdout, and asserts that neither the session key nor the
password hash appears in it. It reads both secrets back out of the
SQLite file afterwards, so the assertions are made against the values
that boot actually generated. Three requires guard against vacuity:
the capture has to contain a DEBUG line and both INSERTs, or the
absence of the secrets proves nothing. values_test.go pins the same
property per arm of Trace, and that an INSERT keeps one placeholder
per value it bound.

Removing the filter fails all three new tests.

README documents what DEBUG=true does and does not expose, including
the one secret still logged in the clear on purpose: the initial admin
password, at INFO, once, because that line is the only place an
operator ever sees it.
This commit is contained in:
2026-08-20 04:15:49 +00:00
parent a13e5b7ded
commit 31228608d1
6 changed files with 807 additions and 17 deletions

View File

@@ -17,6 +17,10 @@
// level the operator controls, they are shaped by whichever handler
// internal/logger selected, and every value a client can influence is
// spent through logfield.Truncate.
//
// It also logs no bound value at all. See ParamsFilter: the statement
// is written with its placeholders intact, at every level, so the
// values a statement carries never reach the log in the first place.
package gormlog
import (
@@ -26,6 +30,7 @@ import (
"log/slog"
"time"
"gorm.io/gorm"
gormlogger "gorm.io/gorm/logger"
"sneak.berlin/go/webhooker/internal/logfield"
)
@@ -47,8 +52,14 @@ type Logger struct {
}
// Interface compliance is asserted here rather than discovered at the
// gorm.Open call sites.
var _ gormlogger.Interface = (*Logger)(nil)
// gorm.Open call sites. gorm.ParamsFilter is the optional half: GORM
// type-asserts for it and silently keeps interpolating if it is
// missing, so losing it would cost no build error and no test that
// does not look at the emitted SQL.
var (
_ gormlogger.Interface = (*Logger)(nil)
_ gorm.ParamsFilter = (*Logger)(nil)
)
// New returns a GORM logger that writes through log.
func New(log *slog.Logger) *Logger {
@@ -71,6 +82,46 @@ func (l *Logger) LogMode(gormlogger.LogLevel) gormlogger.Interface {
return l
}
// ParamsFilter drops every bound value before GORM renders a statement
// for the log, so what is logged is the statement's shape — its
// placeholders — and never the values in it.
//
// GORM builds the string it hands to Trace by calling
// Dialector.Explain(sql, vars...), which substitutes each value into
// the statement. Discarding vars here leaves the '?' placeholders in
// place, because ExplainSQL only substitutes while it still has a
// value for the next one. That happens before Trace is reached, so it
// holds on all three of its arms: the failed statement, the slow one,
// and the routine one an operator sees at DEBUG.
//
// This is the whole of the fix, and it is deliberately unconditional
// rather than a list of tables to redact. At first boot the two
// statements that carry a secret are the INSERT into settings holding
// the base64 session key — which is the entire session security model,
// since anyone with it can mint a valid cookie — and the INSERT into
// users holding the Argon2id hash. A denylist would have had to be
// extended by hand for every table added afterwards, and the cost of
// missing one is a credential in a log that gets pasted into issues.
//
// What is given up is the ability to read a value out of the log. The
// statement, the table, the error and the row count are all still
// there, which is what identifies a failing statement; reproducing it
// needs the values, and those an operator now gets from the database
// rather than from the log.
//
// One GORM path does not consult this: (*gorm.DB).Scan records the
// statement through gorm's own traceRecorder, which does not implement
// this interface. No production code path calls it; its one caller is
// internal/database/database_test.go:91, whose SELECT 1 binds nothing.
// scan_guard_test.go fails if a non-test file calls it.
// (*gorm.DB).Pluck, Row and Raw all run through the normal callback
// processor and are filtered.
func (l *Logger) ParamsFilter(
_ context.Context, sql string, _ ...any,
) (string, []any) {
return sql, nil
}
// Info logs one of GORM's own informational messages.
func (l *Logger) Info(
ctx context.Context, msg string, data ...any,
@@ -93,9 +144,9 @@ func (l *Logger) Error(
}
// Trace reports the outcome of a single statement. GORM calls it for
// every statement it runs, so the cheap paths stay cheap: fc()
// renders the interpolated SQL and is called only on a branch that
// will actually emit.
// every statement it runs, so the cheap paths stay cheap: fc() renders
// the statement — with placeholders, per ParamsFilter — and is called
// only on a branch that will actually emit.
//
// The arms are ordered exactly as GORM's own Trace orders them —
// non-record-not-found error, then slow, then the routine case — so