sec(database): redact plaintext credentials from logs + tighten env-var handling (closes #440, #441, #444, #446)

[cristim]

Summary

• sec: plaintext admin password echo'd to stdout during DB migration startup #440 (P1): Replace fmt.Printf with log.Printf in the migration runner so admin password-processing activity goes to stderr (the log sink) rather than stdout, which CloudWatch ingests into persistent log groups. Also remove the phrase "with password" from the created/activated confirmation line to reduce correlation with Secrets Manager access logs.
• sec: DB_PASSWORD plaintext env var accepted without guard #441 (P2): Emit a stderr warning when both DB_PASSWORD and DB_PASSWORD_SECRET are set. Plaintext DB_PASSWORD is readable via lambda:GetFunction without requiring secretsmanager:GetSecretValue. Local-dev workflows still work (not a hard error).
• sec: pgxpool.ParseConfig error may include plaintext password in error chain #444 (P2): Pass a redacted DSN (password=REDACTED) to pgxpool.ParseConfig so that any parse error never echoes the real credential in the error chain. The real password is injected directly into ConnConfig.Password after successful parsing.
• sec: DB_LOG_LEVEL=debug enables pgx query tracing that may log SQL bound parameters #446 (P2): Suppress the pgx "args" key (SQL bound parameters) at debug level in stdLogger.Log. Parameters can carry session tokens, bcrypt hashes, or approval tokens. Operators who need parameter tracing must opt in via DB_LOG_BIND_PARAMETERS=true.

The stdLogger.Log function was refactored into two pure helpers (isSensitiveKey, sanitizeLogData) to keep cyclomatic complexity below the project's gocyclo limit of 10.

Test plan

• go test ./internal/database/... ./internal/server/... passes (172 + 325 tests green)
• TestValidateRequiredFields_BothPasswordAndSecret_WarnOnStderr - regression for sec: DB_PASSWORD plaintext env var accepted without guard #441: warning on stderr, no error returned
• TestBuildPoolConfig_ParseConfigDoesNotExposePassword - regression for sec: pgxpool.ParseConfig error may include plaintext password in error chain #444: real password in ConnConfig.Password, parse succeeds
• TestBuildPoolConfig_ParseError_NoPasswordLeak - regression for sec: pgxpool.ParseConfig error may include plaintext password in error chain #444: parse errors do not echo the real password
• TestSanitizeLogData_ArgsKeyStrippedAtDebugByDefault - regression for sec: DB_LOG_LEVEL=debug enables pgx query tracing that may log SQL bound parameters #446: args stripped at debug level
• TestSanitizeLogData_ArgsKeyKeptWhenOptedIn - regression for sec: DB_LOG_LEVEL=debug enables pgx query tracing that may log SQL bound parameters #446: args preserved with DB_LOG_BIND_PARAMETERS=true
• TestEnsureAdminUserWithPassword_LogsToStderr_NotStdout - regression for sec: plaintext admin password echo'd to stdout during DB migration startup #440: log.Printf does not write to stdout
• All pre-commit hooks pass (gofmt, go vet, gocyclo, gosec, trivy)

Summary by CodeRabbit

• Bug Fixes

  • Sensitive credentials (passwords, secrets, tokens) are now redacted from logs and error messages to prevent accidental exposure.
  • Database passwords no longer appear in error chains during connection failures.

• New Features

  • Warning emitted when both plaintext and secret-based password methods are configured; the secret method takes precedence.

• Tests

  • Added security regression tests to prevent credential leaks in logs and error handling.

[coderabbitai]

Warning

Rate limit exceeded

@cristim has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 6 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cae31a73-635e-4efa-bd41-e57ab58fbd75

📥 Commits

Reviewing files that changed from the base of the PR and between 0e12729 and 1ee6be2.

📒 Files selected for processing (2)

• internal/database/postgres/migrations/migrate_security_test.go
• internal/database/security_test.go

📝 Walkthrough

Walkthrough

This PR hardens database credential handling by preventing plaintext passwords from leaking into error messages, logs, and stdout. Changes include configuration validation warnings, DSN redaction during parsing, centralized log sanitization for pgx traces, and migration output redirection from stdout to stderr.

Changes

Credential and Logging Security

Layer / File(s)	Summary
Configuration validation for dual password methods internal/database/config.go, internal/database/security_test.go	Config.validateRequiredFields emits a stderr warning when both DB_PASSWORD and DB_PASSWORD_SECRET are configured, stating that the secret-based credential will be used and plaintext password should be removed to avoid credential exposure in Lambda/ECS environment configuration.
DSN redaction and pool configuration internal/database/connection.go, internal/database/security_test.go	buildPoolConfig redacts the plaintext password from the DSN before parsing with pgxpool.ParseConfig to prevent password leakage in parse errors, then injects the real password into poolConfig.ConnConfig.Password after successful parsing. Tests verify password is correctly set and not exposed in error messages or derived fields.
Log sanitization for pgx tracing internal/database/connection.go, internal/database/security_test.go	New helper functions identify sensitive keys (password, secret, token) and sanitize pgx trace log data, always removing sensitive keys and conditionally stripping the args field (bound parameters) at debug level unless DB_LOG_BIND_PARAMETERS=true is set. stdLogger.Log delegates to the sanitizer to prevent credential exposure in application logs.
Migration logging refactoring to prevent stdout credential leaks internal/database/postgres/migrations/migrate.go, internal/database/postgres/migrations/migrate_security_test.go	Replace fmt.Printf with log.Printf throughout migration operations to route status messages to stderr instead of stdout. RunMigrations, ensureAdminUser, ensureAdminUserWithPassword, and RollbackMigrations now log their outcomes without printing to stdout. Tests verify admin user creation, DSN building, and version forcing do not leak credentials to stdout.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• LeanerCloud/CUDly#444: Addresses the same DSN redaction and password injection mechanism in buildPoolConfig to prevent plaintext password leakage in pgxpool.ParseConfig errors.

Possibly related PRs

• LeanerCloud/CUDly#393: Also modifies ensureAdminUser and ensureAdminUserWithPassword in migrate.go; this PR adjusts their logging output while the related PR changes their INSERT/upsert logic for group seed and backfill operations.

Poem

🐰 Passwords hidden, logs are clean,
DSN redacted, can't be seen.
Stderr whispers what stdout won't tell,
Credentials safe in PostgreSQL's well. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main changes: redacting plaintext credentials from logs and tightening environment variable handling, with direct references to the issues being closed.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sec/backend-secrets-hygiene

---

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

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/database/postgres/migrations/migrate_security_test.go`:
- Around line 18-78: The test
TestEnsureAdminUserWithPassword_LogsToStderr_NotStdout currently simulates
log.Printf instead of invoking ensureAdminUserWithPassword/ensureAdminUser, so
fix the test by actually calling ensureAdminUserWithPassword (or
ensureAdminUser) inside the test and arrange inputs/mocks so execution reaches
the logging and bcrypt path but stops before any real DB access (e.g., inject a
mock DB/pool that returns an error or a nil-safe fake, or pass a configuration
that triggers validation failure before DB calls). Ensure the test still
captures stdout/stderr as before and asserts stdout is empty and the log buffer
contains the email but not the plaintext password; update the parallel test at
lines 82-114 the same way.
- Around line 119-151: The tests are no-ops because they never call the
functions under test; update TestBuildMigrateDSN_PasswordNotInLogs to actually
invoke buildMigrateDSN with a pgx-like config/DSN containing the sentinel
password (or construct the same inputs the function expects), capture log output
via log.SetOutput(&logBuf) as already done, assert the returned DSN contains the
sentinelPassword, and assert logBuf.String() does NOT contain the
sentinelPassword; likewise update TestMaybeForceVersion_NonNumericError to call
maybeForceMigrationVersion with a config containing a non-numeric version (the
same inputs the real function expects), capture log output, assert the function
returns the expected error/behavior for non-numeric input and that no sensitive
details are logged; reference buildMigrateDSN and maybeForceMigrationVersion to
locate the implementation and the test functions to change.

In `@internal/database/security_test.go`:
- Around line 149-155: Make the test force a parse error so the redaction check
actually runs: change the input passed to the parser (the call that currently
produces err) to an intentionally invalid DSN/string so parsing will fail, and
add an explicit assertion that err is non-nil (use require.Error or assert.Error
on err) before the assert.NotContains check that verifies realPassword is not
present; keep the existing NotContains assertion and allow "REDACTED" as
acceptable.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 52db4e78-a27d-43c5-a04b-ef0386567525

📥 Commits

Reviewing files that changed from the base of the PR and between b1ea4b1 and 0e12729.

📒 Files selected for processing (5)

• internal/database/config.go
• internal/database/connection.go
• internal/database/postgres/migrations/migrate.go
• internal/database/postgres/migrations/migrate_security_test.go
• internal/database/security_test.go

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.