security: guard remaining unserialize() calls with allowed_classes => false

[TheWitness]

Updates controllers/settings.php's unserialize() call to:

$settings = @unserialize($obj->settings, ["allowed_classes" => false]);

and audits the rest of the codebase for any other unserialize() calls missing the allowed_classes => false guard. Found and fixed 4 more:

• lib/Doctrine/Collection.php and lib/Doctrine/Connection.php — internal unserialize() calls inside their Serializable::unserialize() implementations.
• lib/Doctrine/Record.php — same, plus the per-column unserialize() used when hydrating array/object typed columns.
• lib/Doctrine/Parser/YamlSf/Inline.php — unserialize() of embedded !!php/object: YAML scalars.

Also relaxed tests/Security/UnserializeTest.php's settings.php guard check to accept either quote style, since this line now uses double quotes.

[Copilot]

🟡 Changes recommended

Doctrine and YAML deserialization compatibility issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Hardens remaining PHP unserialize() calls with allowed_classes => false and updates the related security test.

Changes:

• Guards settings, Doctrine, and YAML deserialization paths.
• Relaxes the test to accept either quote style.

File summaries

File	Summary
tests/Security/UnserializeTest.php	Accepts either quote style for the guard check.
lib/Doctrine/Record.php	Guards record restoration but breaks serialized related-record references.
lib/Doctrine/Parser/YamlSf/Inline.php	Guards YAML object loading but breaks documented object round-tripping.
lib/Doctrine/Connection.php	Guards connection restoration but invalidates serialized table objects.
lib/Doctrine/Collection.php	Guards collection restoration but invalidates serialized record objects.
controllers/settings.php	Adds the deserialization class guard.

Review details

Suppressed comments (1)

lib/Doctrine/Record.php:756

• The object column path intentionally serializes its value in Doctrine_Record::serialize() (see lines 703-706), but this change now restores that payload with every class disabled. Any legitimate object-valued column therefore comes back as __PHP_Incomplete_Class instead of the stored object. If object columns remain supported, use a validated class allowlist for this path; otherwise reject/disable the column type explicitly rather than silently changing its value type.

                    $this->_data[$k] = unserialize($this->_data[$k], array('allowed_classes' => false));

• Files reviewed: 6/6 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[TheWitness]

Since we are modifying Doctrine in the near term, we have to take these things on. I don't like it, but it's one of the reasons that I wanted this one public archived. Not yet though. Let's attempt to give it more legs.