fix(notifications): resolve only registered notifiables from saved notification settings

CHANGELOG.md

@@ -19,3 +19,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Removed
 - Legacy duck-typed transformers (`$target` property + static `output($model, $data)`), `ResourceTransformerRegistry::transform(Model, array)`, `resolveByTarget()`, `fixClassName()` and the static `$transformers` array. The `User` resource no longer calls the registry directly.
 - Adds first version
+
+### Security
+- `NotificationRegistry` only resolves notifiables registered with `registerNotifiable()`. A company's saved notification settings could name any `dynamic:<property>`, which reached any public no-argument method on the notification subject (for example fleetops `Order::dispatch()`), or any class name, which the container built. Unregistered definitions are now skipped; extensions must register every `dynamic:` name and notifiable class they expect to receive.

src/Support/NotificationRegistry.php

@@ -391,9 +391,35 @@ protected static function resolveNotificationSettings(...$params): mixed
         return Setting::lookupForCompany($companyUuid ?? session('company'), 'notification_settings', []);
     }
 
+    /**
+     * Determine whether a notifiable definition was registered with `registerNotifiable()`.
+     *
+     * @param mixed $definition a notifiable class name or a `dynamic:<property>` name
+     */
+    public static function isRegisteredNotifiable($definition): bool
+    {
+        if (!is_string($definition) || $definition === '') {
+            return false;
+        }
+
+        $definition = ltrim($definition, '\\');
+        foreach (static::$notifiables as $notifiable) {
+            if (is_string($notifiable) && ltrim($notifiable, '\\') === $definition) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Resolve a notifiable object to an Eloquent model.
      *
+     * The notifiable object comes from the company's saved notification settings, so only registered
+     * notifiables are resolved. An unregistered `dynamic:<property>` would reach any public method on the
+     * subject that takes no arguments, through `resolveDynamicNotifiable()` or Eloquent's relation getter,
+     * and an unregistered class would be built by the container. Anything else is skipped.
+     *
      * @param array $notifiableObject an associative array containing the definition and primary key to resolve the notifiable object
      *
      * @return Model|null the Eloquent model or null if it cannot be resolved
@@ -404,6 +430,10 @@ protected static function resolveNotifiable(array $notifiableObject, $subject =
         $primaryKey = data_get($notifiableObject, 'primaryKey');
         $key        = data_get($notifiableObject, 'key');
 
+        if (!static::isRegisteredNotifiable($definition)) {
+            return null;
+        }
+
         // Handle dynamic subject resolution
         if (Str::startsWith($definition, 'dynamic:')) {
             $property = str_replace('dynamic:', '', $definition);

tests/Unit/Support/NotificationRegistryTest.php

@@ -116,6 +116,47 @@ class NotificationRegistryDispatchPropertySubject extends EloquentModel
     public $timestamps = false;
 }
 
+class NotificationRegistrySideEffectSubject extends EloquentModel
+{
+    public static array $calls = [];
+
+    protected $primaryKey = 'uuid';
+
+    public $incrementing = false;
+
+    protected $keyType = 'string';
+
+    public $timestamps = false;
+
+    public function dispatch(): void
+    {
+        static::$calls[] = $this->uuid ?? 'new-instance';
+    }
+}
+
+class NotificationRegistryLoadingSubject extends NotificationRegistrySideEffectSubject
+{
+    // Mirrors fleetops Order::resolveDynamicNotifiable(), which loads any property it is given
+    public function resolveDynamicNotifiable(string $property): mixed
+    {
+        if (!$this->relationLoaded($property)) {
+            $this->load($property);
+        }
+
+        return $this->{$property};
+    }
+}
+
+class NotificationRegistryConstructorProbe
+{
+    public static int $constructed = 0;
+
+    public function __construct()
+    {
+        static::$constructed++;
+    }
+}
+
 class NotificationRegistryDispatchCacheFake
 {
     private array $values = [];
@@ -218,8 +259,12 @@ function notification_registry_dispatch_database(): Capsule
         Fleetbase\Models\Group::class,
         Fleetbase\Models\Role::class,
         Fleetbase\Models\Company::class,
+        NotificationRegistryDispatchTarget::class,
+        NotificationRegistryDispatchGroup::class,
     ];
-    NotificationRegistryDispatchTarget::$sent = [];
+    NotificationRegistryDispatchTarget::$sent           = [];
+    NotificationRegistrySideEffectSubject::$calls       = [];
+    NotificationRegistryConstructorProbe::$constructed  = 0;
 });
 
 afterEach(function () {
@@ -321,6 +366,7 @@ function notification_registry_dispatch_database(): Capsule
     session(['company' => 'company-1']);
 
     NotificationRegistry::register(NotificationRegistryDispatchNotification::class);
+    NotificationRegistry::registerNotifiable('dynamic:assignee');
 
     Fleetbase\Models\Setting::query()->create([
         'key'   => 'company.company-1.notification_settings',
@@ -379,6 +425,7 @@ function notification_registry_dispatch_database(): Capsule
 
 test('notification registry dispatches by definition name with dynamic subject context', function () {
     notification_registry_dispatch_database();
+    NotificationRegistry::registerNotifiable('dynamic:dispatcher');
 
     Fleetbase\Models\Setting::query()->create([
         'key'   => 'company.company-1.notification_settings',
@@ -505,6 +552,7 @@ function notification_registry_dispatch_database(): Capsule
 
 test('notification registry resolves dynamic notifiables from subject properties', function () {
     notification_registry_dispatch_database();
+    NotificationRegistry::registerNotifiable('dynamic:assignee');
 
     Fleetbase\Models\Setting::query()->create([
         'key'   => 'company.company-1.notification_settings',
@@ -565,6 +613,106 @@ function notification_registry_dispatch_database(): Capsule
     expect(NotificationRegistryDispatchTarget::$sent)->toBe([]);
 });
 
+function notification_registry_settings_with(array $notifiables): void
+{
+    Fleetbase\Models\Setting::query()->create([
+        'key'   => 'company.company-1.notification_settings',
+        'value' => [
+            'notificationRegistryDispatchNotification__manualNotice' => [
+                'notifiables' => [
+                    ...$notifiables,
+                    [
+                        'definition' => NotificationRegistryDispatchTarget::class,
+                        'primaryKey' => 'uuid',
+                        'key'        => 'target-2',
+                    ],
+                ],
+            ],
+        ],
+    ]);
+}
+
+test('notification registry recognises only registered notifiable definitions', function () {
+    NotificationRegistry::registerNotifiable('dynamic:driver');
+
+    expect(NotificationRegistry::isRegisteredNotifiable('dynamic:driver'))->toBeTrue()
+        ->and(NotificationRegistry::isRegisteredNotifiable(Fleetbase\Models\User::class))->toBeTrue()
+        ->and(NotificationRegistry::isRegisteredNotifiable('\\' . Fleetbase\Models\User::class))->toBeTrue()
+        ->and(NotificationRegistry::isRegisteredNotifiable('dynamic:dispatch'))->toBeFalse()
+        ->and(NotificationRegistry::isRegisteredNotifiable('dynamic:Driver'))->toBeFalse()
+        ->and(NotificationRegistry::isRegisteredNotifiable(NotificationRegistryConstructorProbe::class))->toBeFalse()
+        ->and(NotificationRegistry::isRegisteredNotifiable(''))->toBeFalse()
+        ->and(NotificationRegistry::isRegisteredNotifiable(null))->toBeFalse()
+        ->and(NotificationRegistry::isRegisteredNotifiable(['dynamic:driver']))->toBeFalse();
+});
+
+test('notification registry does not read an unregistered dynamic property from the subject', function () {
+    notification_registry_dispatch_database();
+    notification_registry_settings_with([
+        ['definition' => 'dynamic:dispatch', 'primaryKey' => 'uuid', 'key' => 'ignored-for-dynamic'],
+    ]);
+
+    $subject               = new NotificationRegistrySideEffectSubject();
+    $subject->uuid         = 'subject-side-effect';
+    $subject->company_uuid = 'company-1';
+
+    NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Manual Notice', $subject, 'skipped');
+
+    expect(NotificationRegistrySideEffectSubject::$calls)->toBe([])
+        ->and(array_column(NotificationRegistryDispatchTarget::$sent, 'target'))->toBe(['target-2']);
+});
+
+test('notification registry does not pass an unregistered dynamic property to the subject resolver', function () {
+    notification_registry_dispatch_database();
+    notification_registry_settings_with([
+        ['definition' => 'dynamic:dispatch', 'primaryKey' => 'uuid', 'key' => 'ignored-for-dynamic'],
+    ]);
+
+    $subject               = new NotificationRegistryLoadingSubject();
+    $subject->uuid         = 'subject-loading';
+    $subject->company_uuid = 'company-1';
+
+    NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Manual Notice', $subject, 'skipped');
+
+    expect(NotificationRegistrySideEffectSubject::$calls)->toBe([])
+        ->and(array_column(NotificationRegistryDispatchTarget::$sent, 'target'))->toBe(['target-2']);
+});
+
+test('notification registry still resolves a registered dynamic property through the subject resolver', function () {
+    notification_registry_dispatch_database();
+    NotificationRegistry::registerNotifiable('dynamic:dispatcher');
+    notification_registry_settings_with([
+        ['definition' => 'dynamic:dispatcher', 'primaryKey' => 'uuid', 'key' => 'ignored-for-dynamic'],
+    ]);
+
+    $subject               = new NotificationRegistryDispatchSubject();
+    $subject->uuid         = 'subject-registered';
+    $subject->company_uuid = 'company-1';
+
+    NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Manual Notice', $subject, 'registered');
+
+    expect(array_column(NotificationRegistryDispatchTarget::$sent, 'target'))->toBe(['target-1', 'target-2']);
+});
+
+test('notification registry does not build an unregistered notifiable class', function () {
+    notification_registry_dispatch_database();
+    NotificationRegistry::$notifiables = array_values(array_diff(NotificationRegistry::$notifiables, [NotificationRegistryDispatchGroup::class]));
+    notification_registry_settings_with([
+        ['definition' => NotificationRegistryConstructorProbe::class, 'primaryKey' => 'uuid', 'key' => 'probe'],
+        ['definition' => NotificationRegistryDispatchGroup::class, 'primaryKey' => 'uuid', 'key' => 'group-1'],
+        ['definition' => ['not', 'a', 'class'], 'primaryKey' => 'uuid', 'key' => 'array'],
+    ]);
+
+    $subject               = new NotificationRegistryDispatchSubject();
+    $subject->uuid         = 'subject-class';
+    $subject->company_uuid = 'company-1';
+
+    NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Manual Notice', $subject, 'class');
+
+    expect(NotificationRegistryConstructorProbe::$constructed)->toBe(0)
+        ->and(array_column(NotificationRegistryDispatchTarget::$sent, 'target'))->toBe(['target-2']);
+});
+
 function notification_registry_company_settings(string $companyUuid, string $settingsKey, string $targetUuid = 'target-1'): void
 {
     Fleetbase\Models\Setting::query()->create([