fix(command): restrict allowed_classes in CommandJob unserialize() to prevent RCE

[DeepDiver1975]

Summary

• CommandJob::run() called \unserialize($data) without allowed_classes, allowing PHP Object Injection if an attacker writes a crafted payload to oc_jobs.argument (e.g. via SQL injection or shared-hosting DB access)
• __wakeup()/__destruct() gadget chains from Guzzle, Symfony, and Doctrine could achieve RCE as the web-server user
• Fix: two-pass deserialization — Pass 1 extracts class name without instantiation, Pass 2 restricts to only the verified ICommand class

Security Impact

High (defense-in-depth) — exploitation requires prior DB write access; escalates that to full RCE

Test plan

• Tests verify legitimate ICommand objects still execute correctly
• Tests verify gadget payloads targeting non-ICommand classes are blocked
• Run make test TEST_PHP_SUITE=lib/Command

🤖 Generated with Claude Code

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[DeepDiver1975]

Code Review — fix(command): restrict allowed_classes in CommandJob unserialize()

Overview: Implements a two-pass deserialization strategy in CommandJob::run() to prevent PHP Object Injection. First pass uses allowed_classes: false to safely extract the stored class name without triggering any magic methods, then validates it implements ICommand before a second pass with only that class permitted.

Correctness

The two-pass approach is the correct pattern for this scenario (unlike ClosureJob where a fixed allowlist works, here the class name is dynamic). Key points:

• First pass with allowed_classes: false produces __PHP_Incomplete_Class — no __wakeup() or __destruct() fires. ✅
• Class name extraction via ((array) $incomplete)['__PHP_Incomplete_Class_Name'] is the standard PHP idiom for this. ✅
• is_a($className, ICommand::class, true) — the third argument true allows checking without instantiating, safe for not-yet-loaded classes. ✅
• Second pass restricts to exactly [$className] — a concrete ICommand subclass — so no other classes in the gadget chain can be deserialized. ✅

One subtle concern: class_exists() with autoloading

\class_exists($className) will trigger the autoloader for the named class. An attacker-supplied class name that doesn't exist will invoke autoloading for that name. On a standard Composer autoloader this is harmless (file not found → false). However, if a class with a malicious name existed on the filesystem and matched the autoload map, it would be loaded. This is a theoretical concern only in unusual configurations — in practice, the is_a(..., ICommand::class, true) guard immediately follows and would reject anything that doesn't implement ICommand. Not blocking.

Tests

CommandJobTest is thorough:

• testRunExecutesLegitimateICommand — happy path works ✅
• testMagicMethodsNotTriggeredOnUntrustedPayload — key security assertion: __wakeup() not called, InvalidArgumentException thrown ✅
• testRunRejectsNonObjectPayload — scalar input rejected ✅

The TestableCommandJob subclass exposing run() as runPublic() is the right pattern for testing protected methods without reflection.

Summary

Aspect	Assessment
Security fix	✅ PHP Object Injection prevented via two-pass strategy
Class validation	✅ Interface check before any instantiation
Magic method protection	✅ First-pass uses allowed_classes=false
Tests	✅ All three key cases covered

Verdict: Ready to merge.