Repository navigation
sec(db): pgx error logs carry bound query arguments including session tokens and hashes #231
Description
Summary
The pgx tracer logs the SQL, the bound arguments and the error whenever a query fails, and the sanitizer strips the args key only at debug level. A transient failure of a session insert, a password-hash update or an approval-token update therefore writes the raw session token, bcrypt hash or approval token to CloudWatch at warn or error level. pgx truncates only past 64 bytes, so a 64-hex token and a 60-character bcrypt hash are logged whole. Two existing tests pin the current behaviour as intended.
Location
internal/database/connection.go:399-405 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
Failure scenario
Pgx v5 tracelog.TraceQueryEnd logs {"sql", "args", "err"} at LogLevelError whenever a query fails. sanitizeLogData strips the args key only when level == LogLevelDebug. A transient failure of INSERT INTO sessions (token, ...), UPDATE users SET password_hash = $1, or UPDATE purchase_executions SET approval_token = $7 therefore writes the raw session token, bcrypt hash or approval token to CloudWatch (pgx truncates strings only past 64 bytes; a 64-hex token and a 60-char bcrypt hash are logged whole). security_test.go:184-198 pins this as intended ("args kept at warn/error level").
Evidence
for k, v := range data {
if isSensitiveKey(k) {
continue
}
if k == "args" && level == tracelog.LogLevelDebug && !bindParams {
continue
}
safe[k] = v
}Suggested fix
Drop the level == LogLevelDebug condition so args is stripped at every level unless DB_LOG_BIND_PARAMETERS=true, and flip the two security_test cases to expect stripping.
Found by the 2026-09-02 codebase audit, finding A04-004, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.
Activity
cristim commentedon Sep 29, 2026
Merged in PR #453 at
45079333d2380145a04b09a03bf19c08bcb13588.Verified the actual merged tree matches independently reviewed commit
279baa3e57a244aaf2aa793c8757de8913151d63exactly (git diff --exit-codepassed). On the merged commit, fresh local PostgreSQL 14.24 and Go 1.26.6 race tests passed for the real failed-query regression, sanitizer semantics and debug/info/warn/error output (2.467s).The bound INSERT fails with SQLSTATE 23514. Default logs retain the diagnostic and omit all three synthetic token/hash values plus
args; explicit opt-in includes those values as the positive control. The original implementation failed these assertions before the fix. Local execution used a testhelper setup overlay to connect to isolated native PostgreSQL; the production connection, pgx tracer and logger were unchanged.The code fix is verified and this issue can remain closed. Deployment completion and production behavior are not verified by this comment; automatic deployment monitoring is separate. No new follow-up was identified. The existing deployment authentication work remains tracked in #2.
cristim commentedon Sep 29, 2026
Deployment update for merged commit
45079333d2380145a04b09a03bf19c08bcb13588: the automatic dev deployment workflows now report success for AWS Lambda, GCP Cloud Run, and Azure Container Apps. Each pipeline reports successful Terraform Apply, health-endpoint checks and smoke tests at this commit.This updates the deployment status in the preceding comment. The specific secret-redaction regression was verified against local PostgreSQL, as recorded above; it was not exercised against these deployed environments. Successful deployment pipelines do not establish that the separate scope of #2 is resolved.