separate write db and read db - #121
Open
sredxny wants to merge 13 commits into
Open
probelabs / Visor: security
failed
Sep 30, 2025 in 5m 5s
🚨 Check Failed
security check failed because fail_if condition was met.
Details
📊 Summary
- Total Issues: 4
- Critical Issues: 1
- Warning Issues: 2
🔍 Failure Condition Results
❌ Failed Conditions
- global_fail_if: Global failure condition met
⚠️ Severity: Error
🐛 Issues by Category
🔐 Security (3)
- 🚨 persistent/internal/driver/postgres/lifecycle.go:195 - The
Close()function will cause a panic due to attempting to close the same database connection twice when no separate read replica is configured. WhenReadConnectionStringis empty,l.readSQLDBandl.writeSQLDBpoint to the same instance. The function's logic leads to a second close call on the same underlying connection, causing apanic: close of closed channel. This can be exploited to cause a Denial of Service by crashing the application. ⚠️ persistent/internal/driver/postgres/schema.go:321 - TheDropTablefunction constructs a raw SQL query forSELECT COUNT(*)usingfmt.Sprintfwith an unsanitized table name. While table names are typically not user-controlled, this practice is unsafe and could lead to SQL injection if an attacker finds a way to influence thenameparameter.- ℹ️ persistent/internal/driver/postgres/lifecycle.go:104 - Error messages returned upon connection failure wrap the original database driver errors. This could potentially leak sensitive information about the database infrastructure (e.g., hostnames, usernames, IP addresses) into logs or to the end-user.
🧠 Logic (1)
⚠️ persistent/internal/driver/postgres/lifecycle_test.go:1 - The test suite lacks coverage for the new read/write connection functionality. Critical paths, such as connecting with aReadConnectionStringand correctly closing both single and dual connections, are untested. This omission allowed a critical Denial of Service vulnerability in theClose()function to go undetected.
Generated by Visor - AI-powered code review
Annotations
Check failure on line 217 in persistent/internal/driver/postgres/lifecycle.go
probelabs / Visor: security
security Issue
The `Close()` function will cause a panic due to attempting to close the same database connection twice when no separate read replica is configured. When `ReadConnectionString` is empty, `l.readSQLDB` and `l.writeSQLDB` point to the same instance. The function's logic leads to a second close call on the same underlying connection, causing a `panic: close of closed channel`. This can be exploited to cause a Denial of Service by crashing the application.
Raw output
To prevent a panic from a double-close operation, determine if a single connection is being used *before* any close operations are performed. Store this state in a boolean and use it to conditionally close the read connection only if it's a separate instance.
Check warning on line 321 in persistent/internal/driver/postgres/schema.go
probelabs / Visor: security
security Issue
The `DropTable` function constructs a raw SQL query for `SELECT COUNT(*)` using `fmt.Sprintf` with an unsanitized table name. While table names are typically not user-controlled, this practice is unsafe and could lead to SQL injection if an attacker finds a way to influence the `name` parameter.
Raw output
Sanitize the table name identifier before embedding it in the raw SQL string. The `sanitizeIdentifier` function introduced in `indexes.go` would be suitable for this purpose. The sanitized name should then be used in the `fmt.Sprintf` call.
Check warning on line 1 in persistent/internal/driver/postgres/lifecycle_test.go
probelabs / Visor: security
logic Issue
The test suite lacks coverage for the new read/write connection functionality. Critical paths, such as connecting with a `ReadConnectionString` and correctly closing both single and dual connections, are untested. This omission allowed a critical Denial of Service vulnerability in the `Close()` function to go undetected.
Raw output
Add new test cases to `TestLifeCycleConnect` to validate behavior when `ReadConnectionString` is provided. Create a dedicated `TestLifeCycleClose` function to ensure that `Close()` works correctly in both single-connection mode (does not panic) and dual-connection mode (closes both connections successfully).
Check notice on line 104 in persistent/internal/driver/postgres/lifecycle.go
probelabs / Visor: security
security Issue
Error messages returned upon connection failure wrap the original database driver errors. This could potentially leak sensitive information about the database infrastructure (e.g., hostnames, usernames, IP addresses) into logs or to the end-user.
Raw output
For production environments, consider logging the detailed error internally and returning a more generic, sanitized error message to the caller to avoid exposing internal system details. This prevents attackers from gaining intelligence about your infrastructure from error messages.
Loading