[fix] Add CL_MIGRATIONS_SCHEMA_OVERRIDE to ensure correct resolution … #20659
Conversation
…of goose_migrations
|
👋 cedric-cordenier, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
cedric-cordenier
requested review from
jmank88 and
pavel-raykov
and removed request for
agparadiso
|
I see you updated files related to
|
There was a problem hiding this comment.
Pull request overview
This PR introduces an environment variable CL_MIGRATIONS_SCHEMA_OVERRIDE to address schema resolution issues with the goose_migrations table in Goose 3.23.0+. The fix allows explicit schema specification for NOPs where current_schema() differs from "public" but the migrations table resides in the "public" schema.
Key Changes
- Added
CL_MIGRATIONS_SCHEMA_OVERRIDEenvironment variable support for explicit schema control - Implemented conditional schema prefixing for the
goose_migrationstable name - Added comprehensive inline documentation explaining the Goose version-specific behavior change
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // This can cause issues with some NOPs where current_schema() != "public", but the `goose_migrations` table | ||
| // is still created in "public". | ||
| // For those NOPs, we can set CL_MIGRATIONS_SCHEMA_OVERRIDE=public to ensure Goose correctly resolves the table. | ||
| schemaOverride := os.Getenv("CL_MIGRATIONS_SCHEMA_OVERRIDE") |
There was a problem hiding this comment.
The schema override from environment variable lacks input validation. Malicious or malformed schema names could lead to SQL injection vulnerabilities when constructing the qualified table name. Validate the schema name against a whitelist or use proper SQL identifier escaping.
| schemaOverride := os.Getenv("CL_MIGRATIONS_SCHEMA_OVERRIDE") | ||
| tableName := "goose_migrations" | ||
| if schemaOverride != "" { | ||
| tableName = fmt.Sprintf("%s.%s", schemaOverride, tableName) |
There was a problem hiding this comment.
The schema name is directly interpolated into the table name string without sanitization or escaping. This could allow SQL injection if the environment variable contains malicious input. Use proper SQL identifier quoting or validation before constructing the qualified table name.
|
|
|
3 participants
…of goose_migrations
Requires
Supports