Fix release readiness issues for 5.x #1001
Merged
+82
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add cycle detection to seed dependency ordering to prevent infinite recursion when seeds have circular dependencies - Add 'unsigned' to valid column options for API consistency with 'signed' - Fix SQL injection vulnerability in MysqlAdapter by replacing addslashes() with proper driver escaping for column comments - Fix non-existent $io->error() method call in DumpCommand (should be err()) - Document SQL Server check constraints as unsupported with improved error messages guiding users to use raw SQL
- Restrict unserialize() to safe CakePHP schema classes only - Fix strpos() logic bug by using str_contains() - Validate --source option to prevent path traversal - Initialize $command property to prevent uninitialized access - Fix weak equality (!=) to strict (!==) in Table::saveData() - Fix copy-paste bug in Migrator (was using 'down' instead of 'missing') - Replace assert() with explicit RuntimeException in BaseSeed
TableSchema contains nested Column, Index, Constraint and other schema objects that also need to be allowed for deserialization.
markstory
reviewed
Jan 20, 2026
dereuromark
added a commit
that referenced
this pull request
Jan 20, 2026
This backports several important bug fixes from recent 5.x PRs: **From PR #1001 (Fix release readiness issues for 5.x):** - Fix copy-paste bug in Migrator::shouldDropTables() using $messages['down'] instead of $messages['missing'] - Fix uninitialized $command property in Migrations.php - Fix weak equality in Table::saveData() (use !== instead of !=) - Replace assert() with explicit RuntimeException in BaseSeed for production safety - Fix DumpCommand using non-existent $io->error() method (should be $io->err()) - Replace unsafe addslashes() with proper driver escaping (schemaValue()) for column comments in MysqlAdapter::getRenameColumnInstructions() **From PR #1002 (Quote database names in PostgreSQL and SQL Server adapters):** - PostgresAdapter: Quote database name and charset in createDatabase() - PostgresAdapter: Quote database name in dropDatabase() - SqlserverAdapter: Use quoteSchemaName() instead of manual brackets in createDatabase() and dropDatabase() - SqlserverAdapter: Fix SQL injection vulnerability in dropDatabase() **From PR #1003 (Improve SQL quoting and fix docblock issues):** - SqlserverAdapter: Use quoteString() for sp_rename parameters in getRenameTableInstructions() and getRenameColumnInstructions() - PostgresAdapter/SqlserverAdapter: Use quoteColumnName() for foreign key column definitions instead of hard-coded double quotes
markstory
pushed a commit
that referenced
this pull request
Jan 21, 2026
This backports several important bug fixes from recent 5.x PRs: **From PR #1001 (Fix release readiness issues for 5.x):** - Fix copy-paste bug in Migrator::shouldDropTables() using $messages['down'] instead of $messages['missing'] - Fix uninitialized $command property in Migrations.php - Fix weak equality in Table::saveData() (use !== instead of !=) - Replace assert() with explicit RuntimeException in BaseSeed for production safety - Fix DumpCommand using non-existent $io->error() method (should be $io->err()) - Replace unsafe addslashes() with proper driver escaping (schemaValue()) for column comments in MysqlAdapter::getRenameColumnInstructions() **From PR #1002 (Quote database names in PostgreSQL and SQL Server adapters):** - PostgresAdapter: Quote database name and charset in createDatabase() - PostgresAdapter: Quote database name in dropDatabase() - SqlserverAdapter: Use quoteSchemaName() instead of manual brackets in createDatabase() and dropDatabase() - SqlserverAdapter: Fix SQL injection vulnerability in dropDatabase() **From PR #1003 (Improve SQL quoting and fix docblock issues):** - SqlserverAdapter: Use quoteString() for sp_rename parameters in getRenameTableInstructions() and getRenameColumnInstructions() - PostgresAdapter/SqlserverAdapter: Use quoteColumnName() for foreign key column definitions instead of hard-coded double quotes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses several release readiness issues identified in the 5.x branch:
First commit fixes:
Seed dependency cycle detection: Added proper cycle detection to
orderSeedsByDependencies()to prevent infinite recursion when seeds have circular dependencies. Now throws aRuntimeExceptionwith a clear message showing the cycle path.Column
unsignedoption: Added'unsigned'to valid column options for API consistency. Previously only'signed'was valid, requiring users to callsetUnsigned()directly.SQL injection fix: Replaced unsafe
addslashes()with proper driver escaping (schemaValue()) for column comments inMysqlAdapter::getRenameColumnInstructions().DumpCommand fix: Fixed non-existent
$io->error()method call (should be$io->err()).SQL Server documentation: Documented check constraints as unsupported for SQL Server adapter with improved error messages guiding users to use raw SQL via
$this->execute().Second commit fixes (from deep dive analysis):
Safer deserialization: Restricted
unserialize()in BakeMigrationDiffCommand to only allow safe CakePHP schema classes, mitigating potential RCE.strpos() logic bug: Fixed
strpos()returning false when match is at position 0 by usingstr_contains()instead.Path traversal prevention: Added validation for
--sourceoption to prevent directory traversal attacks.Uninitialized property: Initialized
$commandproperty inMigrations.phpto prevent access errors.Strict comparison: Fixed weak equality (
!=) to strict equality (!==) inTable::saveData()for array key comparison.Copy-paste bug: Fixed
Migrator::shouldDropTables()which was using$messages['down']instead of$messages['missing'].Production-safe error handling: Replaced
assert()calls with explicitRuntimeExceptioninBaseSeedsince assertions can be disabled in production.Test plan