Fix release readiness issues for 5.x (#1001)

* Fix release readiness issues for 5.x

- 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

* Fix additional release readiness issues from deep dive

- Restrict unserialize() to safe CakePHP schema classes only
- Fix strpos() logic bug by using str_contains()
- 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

* Add missing CakePHP schema classes to unserialize allowlist

TableSchema contains nested Column, Index, Constraint and other
schema objects that also need to be allowed for deserialization.

* Fix docblock annotation spacing in SqlserverAdapter

Author: dereuromark

src/BaseSeed.php

@@ -238,7 +238,9 @@ public function isIdempotent(): bool
     public function call(string $seeder, array $options = []): void
     {
         $io = $this->getIo();
-        assert($io !== null, 'Requires ConsoleIo');
+        if ($io === null) {
+            throw new RuntimeException('ConsoleIo is required for calling other seeders.');
+        }
         $io->out('');
         $io->out(
             ' ====' .
@@ -285,7 +287,9 @@ protected function runCall(string $seeder, array $options = []): void
             'source' => $options['source'],
         ]);
         $io = $this->getIo();
-        assert($io !== null, 'Missing ConsoleIo instance');
+        if ($io === null) {
+            throw new RuntimeException('ConsoleIo is required for calling other seeders.');
+        }
         $manager = $factory->createManager($io);
         $manager->seed($seeder);
     }

src/Command/BakeMigrationDiffCommand.php

@@ -19,8 +19,15 @@
 use Cake\Console\ConsoleIo;
 use Cake\Console\ConsoleOptionParser;
 use Cake\Database\Connection;
+use Cake\Database\Schema\CachedCollection;
+use Cake\Database\Schema\CheckConstraint;
 use Cake\Database\Schema\CollectionInterface;
+use Cake\Database\Schema\Column;
+use Cake\Database\Schema\Constraint;
+use Cake\Database\Schema\ForeignKey;
+use Cake\Database\Schema\Index;
 use Cake\Database\Schema\TableSchema;
+use Cake\Database\Schema\UniqueKey;
 use Cake\Datasource\ConnectionManager;
 use Cake\Event\Event;
 use Cake\Event\EventManager;
@@ -485,7 +492,7 @@ protected function checkSync(): bool
             $lastVersion = $this->migratedItems[0]['version'];
             $lastFile = end($this->migrationsFiles);
 
-            return $lastFile && (bool)strpos($lastFile, (string)$lastVersion);
+            return $lastFile && str_contains($lastFile, (string)$lastVersion);
         }
 
         return false;
@@ -546,7 +553,21 @@ protected function getDumpSchema(Arguments $args): array
             $this->io->abort($msg);
         }
 
-        return unserialize((string)file_get_contents($path));
+        $contents = (string)file_get_contents($path);
+
+        // Use allowed_classes to restrict deserialization to safe CakePHP schema classes
+        return unserialize($contents, [
+            'allowed_classes' => [
+                TableSchema::class,
+                CachedCollection::class,
+                Column::class,
+                Index::class,
+                Constraint::class,
+                UniqueKey::class,
+                ForeignKey::class,
+                CheckConstraint::class,
+            ],
+        ]);
     }
 
     /**

src/Command/DumpCommand.php

@@ -141,7 +141,7 @@ public function execute(Arguments $args, ConsoleIo $io): ?int
 
             return self::CODE_SUCCESS;
         }
-        $io->error("An error occurred while writing dump file `{$filePath}`");
+        $io->err("<error>An error occurred while writing dump file `{$filePath}`</error>");
 
         return self::CODE_ERROR;
     }

src/Db/Adapter/MysqlAdapter.php

@@ -725,7 +725,9 @@ protected function getRenameColumnInstructions(string $tableName, string $column
         foreach ($rows as $row) {
             if (strcasecmp($row['Field'], $columnName) === 0) {
                 $null = $row['Null'] === 'NO' ? 'NOT NULL' : 'NULL';
-                $comment = isset($row['Comment']) ? ' COMMENT ' . '\'' . addslashes($row['Comment']) . '\'' : '';
+                $comment = isset($row['Comment']) && $row['Comment'] !== ''
+                    ? ' COMMENT ' . $this->getConnection()->getDriver()->schemaValue($row['Comment'])
+                    : '';
 
                 // create the extra string by also filtering out the DEFAULT_GENERATED option (MySQL 8 fix)
                 $extras = array_filter(

src/Db/Adapter/SqlserverAdapter.php

@@ -1114,27 +1114,38 @@ private function updateSQLForIdentityInsert(string $tableName, string $sql): str
 
     /**
      * @inheritDoc
+     *
+     * Note: Check constraints are not supported for SQL Server adapter.
+     * This method returns an empty array. Use raw SQL via execute() if you need
+     * check constraints on SQL Server.
      */
     protected function getCheckConstraints(string $tableName): array
     {
-        // TODO: Implement check constraints for SQL Server
         return [];
     }
 
     /**
      * @inheritDoc
+     * @throws \BadMethodCallException Check constraints are not supported for SQL Server.
      */
     protected function getAddCheckConstraintInstructions(TableMetadata $table, CheckConstraint $checkConstraint): AlterInstructions
     {
-        throw new BadMethodCallException('Check constraints are not yet implemented for SQL Server adapter');
+        throw new BadMethodCallException(
+            'Check constraints are not supported for the SQL Server adapter. ' .
+            'Use $this->execute() with raw SQL to add check constraints.',
+        );
     }
 
     /**
      * @inheritDoc
+     * @throws \BadMethodCallException Check constraints are not supported for SQL Server.
      */
     protected function getDropCheckConstraintInstructions(string $tableName, string $constraintName): AlterInstructions
     {
-        throw new BadMethodCallException('Check constraints are not yet implemented for SQL Server adapter');
+        throw new BadMethodCallException(
+            'Check constraints are not supported for the SQL Server adapter. ' .
+            'Use $this->execute() with raw SQL to drop check constraints.',
+        );
     }
 
     /**

src/Db/Table.php

@@ -955,7 +955,7 @@ public function saveData(): void
         $c = array_keys($row);
         foreach ($this->getData() as $row) {
             $k = array_keys($row);
-            if ($k != $c) {
+            if ($k !== $c) {
                 $bulk = false;
                 break;
             }

src/Db/Table/Column.php

@@ -789,6 +789,7 @@ protected function getValidOptions(): array
             'update',
             'comment',
             'signed',
+            'unsigned',
             'timezone',
             'properties',
             'values',

src/Migration/Manager.php

@@ -1101,18 +1101,46 @@ protected function getSeedDependenciesInstances(SeedInterface $seed): array
      * Order seeds by dependencies
      *
      * @param \Migrations\SeedInterface[] $seeds Seeds
+     * @param array<string, true> $visiting Seeds currently being visited (for cycle detection)
+     * @param array<string, true> $visited Seeds that have been fully processed
      * @return \Migrations\SeedInterface[]
+     * @throws \RuntimeException When a circular dependency is detected
      */
-    protected function orderSeedsByDependencies(array $seeds): array
+    protected function orderSeedsByDependencies(array $seeds, array $visiting = [], array &$visited = []): array
     {
         $orderedSeeds = [];
         foreach ($seeds as $seed) {
             $name = $seed->getName();
-            $orderedSeeds[$name] = $seed;
+
+            // Skip if already fully processed
+            if (isset($visited[$name])) {
+                continue;
+            }
+
+            // Check for circular dependency
+            if (isset($visiting[$name])) {
+                $cycle = array_keys($visiting);
+                $cycle[] = $name;
+                throw new RuntimeException(
+                    'Circular dependency detected in seeds: ' . implode(' -> ', $cycle),
+                );
+            }
+
+            // Mark as currently visiting
+            $visiting[$name] = true;
+
             $dependencies = $this->getSeedDependenciesInstances($seed);
             if ($dependencies) {
-                $orderedSeeds = array_merge($this->orderSeedsByDependencies($dependencies), $orderedSeeds);
+                $orderedSeeds = array_merge(
+                    $this->orderSeedsByDependencies($dependencies, $visiting, $visited),
+                    $orderedSeeds,
+                );
             }
+
+            // Mark as fully visited and add to result
+            $visited[$name] = true;
+            unset($visiting[$name]);
+            $orderedSeeds[$name] = $seed;
         }
 
         return $orderedSeeds;

src/Migrations.php

@@ -36,7 +36,7 @@ class Migrations
      *
      * @var string
      */
-    protected string $command;
+    protected string $command = '';
 
     /**
      * Constructor

src/TestSuite/Migrator.php

@@ -199,7 +199,7 @@ protected function shouldDropTables(Migrations $migrations, array $options): boo
         if (!empty($messages['missing'])) {
             $hasProblems = true;
             $output[] = 'Applied but missing migrations:';
-            $output = array_merge($output, array_map($itemize, $messages['down']));
+            $output = array_merge($output, array_map($itemize, $messages['missing']));
             $output[] = '';
         }
         if ($output) {