fix(db): bridge go-sql-driver/mysql logger to slog (bookshelf-xrfax) #1238
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "bd-bookshelf-xrfax"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Plain-text
[mysql] ...driver lines bypassed the app's structured JSON logging. This PR installs aDriverLoggeradapter so driver self-healing messages (e.g. "closing bad idle connection: EOF") become structured Warn-level slog entries with\"component\":\"mysql_driver\".internal/db/driver_logger.go: exportedDriverLogger+NewDriverLoggeradapting*slog.Loggerto thego-sql-driver/mysqlLoggerinterface ({ Print(v ...any) }); unexportedinstallDriverLoggercallsmysqldriver.SetLoggeronce. The driver import is aliased asmysqldriverto avoid collision with the existinggolang-migrate/database/mysqlimport indb.go.internal/db/db.go:OpencallsinstallDriverLogger(logger)on entry.internal/db/driver_logger_test.go: black-box test (package db_test) asserting message,component="mysql_driver"attr, andWARNlevel in JSON output.installDriverLoggeris covered via the existingdb.Openintegration tests.Test plan
make buildpassesmake linton./internal/db/— 0 issuesmake test(unit) — all passinternal/dbtests includingDriverLoggerspecsCloses bead bookshelf-xrfax on merge.
Plain-text [mysql] lines (e.g. "closing bad idle connection: EOF") bypassed the app's structured JSON logging. Install a DriverLogger adapter via mysqldriver.SetLogger so driver self-healing messages become structured Warn-level slog entries with component="mysql_driver". - New internal/db/driver_logger.go: exported DriverLogger + NewDriverLogger adapting *slog.Logger to the driver's { Print(v ...any) } interface; unexported installDriverLogger calls mysqldriver.SetLogger once. - Aliases the driver import as mysqldriver to avoid collision with the existing golang-migrate/database/mysql import in db.go. - db.Open calls installDriverLogger(logger) on entry (last-write-wins global side-effect is fine across the 4 Open callers in a single process). - Black-box test (package db_test) asserts message text, component attr, and Warn level in the JSON output. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>Security Review — PR #1238 (driver log → slog)
Adversarial focus: credential leakage via the mysql driver logger, log injection/forging, global
SetLoggerabuse, new boundaries.(1) Secret leakage — CLEAR. Audited every call site that reaches the driver's
Logger.Printin go-sql-driver/mysql v1.9.3 (connection.go:64mc.log,connector.go:119/148,packets.go:58/68/86/100/155,auth.go:341,connection.go:189/214/706). All log error strings, auth-plugin names, or sequence numbers.cfg.Passwdis referenced only inauth.go(scramble) anddsn.go:262(FormatDSN) — neither reaches the logger. The DSN itself is never passed toPrint. So this path cannot surface credentials and does not bypassmaskDSN(the DSN never enters it).(2) Log injection / forging — CLEAR. The driver message is passed to
logger.Warn(msg). slog handlers escape themsgfield (JSONHandler JSON-escapes newlines/quotes; TextHandler quotes on special chars), so no newline-forging of fake log records is possible. Driver messages are driver-internal, not app-user input; the only externally-influenced content (a server error string) requires a trusted-DB compromise and is escaped regardless.(3) Global
SetLoggerside-effect — CLEAR.installDriverLoggersets it to the app's own*slog.Logger(last-caller-wins is documented). No external input controls the installed logger; not abusable.(4) New inputs/boundaries — none. No new request surface, no user-supplied data, no multi-user scoping surface.
No findings.
REVIEW VERDICT: 0 blocker, 0 major, 0 minor
Code Review: bookshelf-xrfax
Review of the mysql driver logger bridge to slog (new
internal/db/driver_logger.go, integration intodb.Open).Phase 1: Spec Compliance
Phase 2: Code Quality
No findings. All checks pass:
Import aliasing:
mysqldriver "github.com/go-sql-driver/mysql"(line 7 of driver_logger.go) correctly aliases the import to avoid confusion with golang-migrate's mysql package. Zero collision risk.Print() formatting: Line 28 uses
strings.TrimSpace(fmt.Sprintln(v...))— correct. Sprintln adds a newline; TrimSpace removes it, preventing double-newlines in slog JSON output. No allocation concern (driver logs are rare, not hot-path).Warn level: Appropriate for self-healing driver events (e.g., "closing bad idle connection: EOF"). Not Error (alert noise), not Info (log spam). Good judgment in docstring (line 26).
Global side-effect in db.Open: Intentional and idempotent. SetLogger last-write-wins; all Open calls in a process share the same driver logger (by design). Error from SetLogger correctly ignored (only fails on nil logger, which never happens). Behavior well-documented in installDriverLogger docstring (lines 33–35).
Black-box test:
package db_test(notdb), exercises only public interface (NewDriverLogger, Print). Three Its, one Expect each (project convention). Asserts message content, component attribute ("mysql_driver"), and Warn level — all three behavioral aspects of the integration covered.Exports justified: DriverLogger and NewDriverLogger exported per project-conventions rule of wrapping third-party interfaces at the boundary. Comment (lines 12–14) explicitly cites the justification. Correct pattern.
Coverage: installDriverLogger called from db.Open (line 76), which has extensive integration test coverage (books_integration_test, comic_integration_test, db_test, slow_query_test, etc.). Should hit 100%.
REVIEW VERDICT: 0 blocker, 0 major, 0 minor
Security Review — PR #1238 (mysql driver log → slog)
Adversarial focus: credential leakage via the driver logger, log injection/forging, global
SetLoggerabuse, new boundaries.(1) Secret leakage — CLEAR. Audited every call site that reaches the driver's
Logger.Printin go-sql-driver/mysql v1.9.3:connection.go:64(mc.log),connector.go:119/148,packets.go:58/68/86/100/155,auth.go:341,connection.go:189/214/706. All emit error strings, auth-plugin names, or sequence numbers.cfg.Passwdis referenced only inauth.go(scramble) anddsn.go:262(FormatDSN) — neither reaches the logger, and the DSN itself is never passed toPrint. So this path cannot surface credentials and does not bypassmaskDSN(the DSN never enters it).(2) Log injection / forging — CLEAR. The driver message is passed to
logger.Warn(msg). slog handlers escape themsgfield (JSONHandler JSON-escapes newlines/quotes; TextHandler quotes on special chars), so an embedded newline cannot forge a fake log record. Driver messages are driver-internal, not app-user input; the only externally-influenced content (a server-returned error string) presupposes a trusted-DB compromise and is escaped regardless.(3) Global
SetLoggerside-effect — CLEAR.installDriverLoggersets the driver's package logger to the app's own*slog.Logger(last-caller-wins, documented). No external input controls the installed logger; not abusable.(4) New inputs/boundaries — none. No new request surface, no user-supplied data, no multi-user-scoping surface.
No findings.
REVIEW VERDICT: 0 blocker, 0 major, 0 minor
fcf1583d607f557b39c8