[cristim]

Failed pgx queries logged bound arguments at error level, exposing session tokens, password hashes and approval tokens. Strip arguments at every log level unless DB_LOG_BIND_PARAMETERS=true; preserve SQL/error diagnostics and info's message-only output.

Closes #231

Validation:

• Real failed bound INSERT against isolated local PostgreSQL 14.24: SQLSTATE 23514 reproduced all three synthetic secrets in pre-fix logs; the same test passes after the fix. Explicit opt-in still exposes the synthetic arguments as a positive control.
• The committed integration test uses the existing PostgreSQL container helper. Local verification substituted only that helper's connection setup through a Go overlay; production connection, pgx tracer and logger ran unchanged. No Docker, cloud resources or production data used.
• Full database package race suite, package build/vet, and CI-pinned golangci-lint 2.10.1 passed.
• Tests cover actual debug/info/warn/error dispatch, literal opt-in, sensitive keys and input-map preservation. Added comments: 3 of 152 nonblank lines (2.0%).

Independent gpt-6-astra review: two clean implementation passes and final commit review at 279baa3e57a244aaf2aa793c8757de8913151d63, using the session's approved reviewer substitution. Reviewer independently passed the real PostgreSQL and logger race tests at this commit and confirmed a redaction-removal mutation fails on the exposed synthetic values. No actionable findings. CodeRabbit is optional under the authorized local-review path; CI and normal merge checks still apply.

Summary by CodeRabbit

• Bug Fixes
  • Database query logs now omit bound parameter values at every log level unless DB_LOG_BIND_PARAMETERS is set to exactly true. Sensitive values remain redacted either way.
  • Query text and database errors continue to appear in logs, including when a query fails because of a constraint violation.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: f3601cbf-76d9-45c8-b3dd-b3405055f98d

📥 Commits

Reviewing files that changed from the base of the PR and between 9cbd634 and 279baa3.

📒 Files selected for processing (3)

• internal/database/connection.go
• internal/database/query_logging_integration_test.go
• internal/database/security_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

📝 Walkthrough

Walkthrough

The database log sanitizer now removes bound query arguments at every log level unless DB_LOG_BIND_PARAMETERS is exactly "true". Unit and PostgreSQL integration tests verify sensitive-value filtering and query-failure logging.

Changes

Query Log Redaction

Layer / File(s)	Summary
Sanitizer behavior and logger tests internal/database/connection.go, internal/database/security_test.go	The sanitizer filters bound arguments unless parameter logging is enabled. Tests cover exact environment-variable values, sensitive-key removal, and logger output across levels.
PostgreSQL query-failure logging test internal/database/query_logging_integration_test.go	The integration test checks the constraint error code and verifies that logs contain the query and error. It checks bound-secret visibility with parameter logging disabled and enabled.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: High

Merge Risk: ⚪ Minimal · up to 279ba

Bound query arguments are omitted unless explicitly enabled, while existing diagnostic behavior is preserved. The change is mergeable subject to normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: redacting bound query arguments at every log level.
Linked Issues check	✅ Passed	The implementation for [#231] removes the args field at every log level unless DB_LOG_BIND_PARAMETERS equals exactly "true". It continues to remove password, secret, and token fields. The …
Out of Scope Changes check	✅ Passed	The changed production code is limited to query-log sanitization. The unit and integration test changes verify the linked security behavior and existing logging semantics. No unrelated production beha…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}