fix: sanitize go-migrate error containing credentials #487
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR addresses credential exposure in database migration error logs by implementing sanitization logic for the go-migrate library errors. The issue stems from the go-migrate library including full database URLs (with credentials) in error messages when connections fail.
- Implements comprehensive credential sanitization for database connection errors
- Adds robust test coverage for various error scenarios and URL formats
- Integrates sanitization into the migrator's New() function to prevent credential leaks
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/migrator/sanitize.go | Core sanitization logic for removing credentials from error messages |
| internal/migrator/sanitize_test.go | Unit tests for the sanitization functions |
| internal/migrator/migrator_test.go | Integration tests verifying credential protection in real connection scenarios |
| internal/migrator/migrator.go | Integration of sanitization into the New() function |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@alexluong - I would have okayed this but also asked copilot to give it a once over and it left a couple of comments. |
|
@leggetter thanks, I'm working on another task and will circle back to this later today. |
2 participants
resolves #470
The original issue is caused by an external package
go-migrateprinting credentials in the log. This PR sanitizes that error from go-migrate while attempting to maintain as much context as possible.Beyond this PR, I also created issue #480 for a more systematic solution regarding credentials & logging. Once implemented, we can consider revisting this and remove the custom sanitizer logic if applicable.