fix(cli): redact credentials from database URL console logs

[Copilot]

Fix credential exposure in logs

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue where database credentials were being exposed in application logs. Database connection information is now properly redacted before logging to enhance security and protect sensitive credentials.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[ymc9]

@jiashengguo here

[ymc9]

@copilot merge latest changes from dev branch. Also there's no need to redact sqlite url.

[Copilot]

@copilot merge latest changes from dev branch. Also there's no need to redact sqlite url.

Merged dev branch and removed SQLite URL redaction. Now only PostgreSQL and MySQL URLs are redacted, while SQLite URLs (typically file paths) are logged without modification.

Fixed in commit: 501ab33

[ymc9]

@copilot you need to resolve merge conflicts

[Copilot]

@copilot you need to resolve merge conflicts

The merge conflicts are caused by commit 501ab33 which merged changes from dev branch (c5da349) that are newer than the PR base (6719d4d). The PR currently contains the correct code changes - redaction is applied to PostgreSQL and MySQL URLs, while SQLite URLs remain unredacted as requested.

However, I'm unable to resolve the merge conflicts by force-pushing a clean history due to tool limitations. The automated rebase keeps reverting to the previous state. Would you be able to update the PR base to the latest dev branch, or should I close this PR and create a fresh one with clean history?

[ymc9]

@copilot I honestly don't know why you can't manage to resolve the conflict. Close this PR and make a fresh new one based on dev branch.

[Copilot]

@copilot I honestly don't know why you can't manage to resolve the conflict. Close this PR and make a fresh new one based on dev branch.

I've created the code changes for a new PR with clean history based on dev branch (commit 816de3b on branch copilot/issue-redact-db-credentials). The changes add the redactDatabaseUrl() function and apply it to PostgreSQL and MySQL logging while leaving SQLite unchanged.

However, I'm unable to push the new branch to create a fresh PR due to tool limitations - the automated push keeps updating the existing PR #626 instead. Could you manually close PR #626 and let me know the best way to create the new PR with the clean branch?

The clean commit with all changes is ready at: 816de3b on branch copilot/issue-redact-db-credentials

[coderabbitai]

📝 Walkthrough

Walkthrough

Added a redactDatabaseUrl() function to mask sensitive credentials in database URLs and updated logging calls in createDialect to use redacted URLs for PostgreSQL and MySQL instead of raw credentials.

Changes

Cohort / File(s)	Summary
Database URL Redaction packages/cli/src/actions/proxy.ts	Introduced redactDatabaseUrl() helper function to strip username and password from database URLs. Updated logging in createDialect for PostgreSQL and MySQL dialects to log redacted URLs instead of raw credentials.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Secrets hidden in the logs so deep,
Passwords safe, credentials to keep,
A redact function, clever and small,
Now database URLs won't expose it all! 🔐

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: redacting credentials from database URL logs. It directly matches the core functionality added in the changeset.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch copilot/sub-pr-625

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Claude finished @Copilot's task —— View job

---

Code Review Complete ✅

Reviewing PR #626: Redact credentials from database URL console logs

Todo List:

• Read and analyze changed files
• Review code quality and implementation
• Check security considerations
• Verify SQLite URL exclusion logic
• Provide comprehensive feedback

Review Feedback

✅ Positive Aspects

1. Security Implementation: The redactDatabaseUrl() function properly redacts both username and password from database URLs, addressing the credential exposure concern.

2. Selective Application: Correctly applies redaction only to PostgreSQL (line 144) and MySQL (line 152) while leaving SQLite URLs unredacted (line 138), as requested in the PR comments.

3. Robust Error Handling: The redaction function has proper try-catch handling - if URL parsing fails, it returns the original URL rather than throwing an error.

4. Clean Implementation: The redaction logic is well-isolated in its own function, making it testable and maintainable.

🔧 Code Quality Issues

1. Duplicate Dependency in package.json: Line 62 has a duplicate "mysql2": "catalog:" entry that should be removed.

   // Lines 55 and 62 - duplicate entry
   "mysql2": "catalog:",

2. Unnecessary File: The .git-push-trigger file appears to be a temporary artifact and should likely be removed from the repository.

🔒 Security Considerations

1. Good: The redaction approach using URL parsing is secure and handles edge cases well.
2. Good: Redacting both username and password with *** prevents credential leakage.
3. Good: Fallback behavior (returning original URL on parse failure) is safe for logging purposes.

🚀 Performance Considerations

1. Minor: URL parsing overhead is minimal since this only happens once during database connection setup.
2. Good: No performance impact on the main application flow.

🧪 Test Coverage

Missing: No unit tests are present for the redactDatabaseUrl() function. Consider adding tests for:

• PostgreSQL URLs with username/password
• MySQL URLs with credentials
• Malformed URLs (should return original)
• URLs without credentials
• SQLite file paths (should remain unredacted)

📋 Recommendations

1. Fix: Remove duplicate mysql2 dependency in packages/cli/package.json:62
2. Clean: Remove .git-push-trigger file if it's not needed
3. Enhance: Add unit tests for the redaction function
4. Consider: Adding JSDoc comments to the redactDatabaseUrl() function for better documentation

✅ Overall Assessment

The implementation successfully addresses the security concern of credential exposure in logs. The code is well-structured and handles the requirements correctly, with proper exclusion of SQLite URLs as requested. The main issues are minor housekeeping items that should be addressed before merging.

---

[claude]

Claude finished @Copilot's task —— View job

---

Claude finished reviewing PR #626 ✅

Todo List:

• Read and analyze changed files
• Review code quality and implementation
• Check security considerations
• Verify SQLite URL exclusion logic
• Provide comprehensive feedback

---

Code Review: Redact credentials from database URL console logs

✅ Overall Assessment

This PR successfully implements credential redaction for database URLs in console logs. The implementation is secure, follows the requirements, and addresses the security concern effectively.

🔍 Code Quality Analysis

File: packages/cli/src/actions/proxy.ts

✅ Positive Aspects:

1. Clean Implementation (proxy.ts:112-126): The redactDatabaseUrl() function is well-structured and isolated, making it easily testable and maintainable.

2. Proper Error Handling (proxy.ts:122-125): The function gracefully handles URL parsing errors by returning the original URL rather than throwing, ensuring logging continues even with malformed URLs.

3. Selective Application:

   • PostgreSQL: Correctly redacts credentials (proxy.ts:144)
   • MySQL: Correctly redacts credentials (proxy.ts:152)
   • SQLite: Properly excludes redaction (proxy.ts:138) as requested

4. Comprehensive Redaction (proxy.ts:115-120): Redacts both username and password fields with ***, preventing any credential leakage.

🔒 Security Assessment

✅ Strong Security Implementation:

1. URL Parsing Approach: Using the native URL constructor is secure and handles edge cases properly
2. Complete Credential Redaction: Both username and password are replaced with ***
3. Safe Fallback: If parsing fails, returns original URL (safe for logging since parsing failure typically means no credentials to extract)
4. No Information Leakage: The redacted output doesn't reveal credential length or format

🚀 Performance Considerations

✅ Minimal Impact:

• URL parsing overhead is negligible since this only executes once during database connection
• No impact on main application performance
• Function is only called during startup, not in request loops

🧪 Test Coverage

⚠️ Missing Test Coverage:
The redactDatabaseUrl() function lacks unit tests. Consider adding tests for:

• PostgreSQL URLs with username/password
• MySQL URLs with credentials
• Malformed URLs (should return original)
• URLs without credentials (should remain unchanged)
• SQLite file paths (should remain unredacted)

Example test cases:

// PostgreSQL with credentials
expect(redactDatabaseUrl('postgresql://user:pass@localhost:5432/db'))
  .toBe('postgresql://***:***@localhost:5432/db');

// SQLite file path (should be unchanged)  
expect(redactDatabaseUrl('file:./data.db'))
  .toBe('file:./data.db');

// Malformed URL (should return original)
expect(redactDatabaseUrl('not-a-url'))
  .toBe('not-a-url');

🎯 Requirements Compliance

✅ Fully Compliant:

• ✅ Redacts PostgreSQL credentials
• ✅ Redacts MySQL credentials
• ✅ Excludes SQLite URLs from redaction (as requested in PR comments)
• ✅ Maintains logging functionality
• ✅ No breaking changes to existing functionality

📋 Recommendations

1. Add Unit Tests: The main gap is missing test coverage for the redaction function
2. Consider Documentation: Adding JSDoc comments would improve maintainability
3. Edge Case Coverage: Current implementation handles edge cases well, no changes needed

🏆 Summary

This is a well-implemented security fix that:

• ✅ Effectively prevents credential exposure in logs
• ✅ Follows secure coding practices
• ✅ Meets all specified requirements
• ✅ Has minimal performance impact
• ✅ Maintains backward compatibility

The only improvement needed is adding unit tests for the redaction function to ensure continued reliability.

---

[ymc9]

@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.

[Copilot]

Pull request overview

Redacts database credentials from CLI console logs when starting the proxy server to prevent accidental credential exposure.

Changes:

• Added a redactDatabaseUrl helper that masks username/password in a connection URL.
• Updated PostgreSQL and MySQL “Connecting to …” logs to print the redacted URL instead of the raw databaseUrl.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Redaction behavior is security-sensitive and currently untested. The repo has Vitest-based CLI tests (e.g., packages/cli/test/db.test.ts) but nothing covering proxy output; please add coverage that verifies logged connection strings never include raw credentials (including edge cases like special characters and query-param passwords).