From 4cc4da8b1fbdd5831c977aa9638a1eaf706dc615 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 21 Aug 2026 20:38:20 +0800 Subject: [PATCH 1/7] fix(database): stop persistent PDO handles sharing one MySQL transaction The mysql and sandbox connections were opened with PDO::ATTR_PERSISTENT. PHP then keeps the MySQL session alive in its persistent pool after the PDO object is destroyed, and hands that same session to the next PDO built from the same DSN, username and password - including one built while another handle is still using it. Two handles then share one transaction, and a COMMIT through either ends it for both. The loser's commit() raises "There is no active transaction" for writes that have already been made durable, so the request reports failure for data that landed and anyone who retries applies it twice. Reproduced directly: two live handles reporting the same CONNECTION_ID, one commit, and the other raising the exact error with the row already visible from a third connection. Laravel cannot detect this. Connection::commit() decides whether to issue a COMMIT from its own $transactions counter, while PDO decides whether a COMMIT is legal from the server's SERVER_STATUS_IN_TRANS flag; nothing reconciles the two. Observed in production paths that have nothing to do with each other - onboarding account creation, ledger invoice creation, and inventory stock adjustments - because the fault is in the connection options, not in any caller. Persistent connections also silently defeat Octane's DisconnectFromDatabases listener: disconnect() drops the PHP object and leaves the server-side connection open. Measured on a dev stack, 40 concurrent requests left 17 MySQL connections open and still idle minutes later, with the listener enabled. Defaults to off. DB_PERSISTENT=true restores the previous behaviour for deployments that have measured the reconnect cost and where no request opens a transaction. --- config/database.connections.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/config/database.connections.php b/config/database.connections.php index e0851fff..581dc3a9 100644 --- a/config/database.connections.php +++ b/config/database.connections.php @@ -20,7 +20,19 @@ } $mysql_options = [ - PDO::ATTR_PERSISTENT => true, + // Persistent connections keep the MySQL session alive in PHP's persistent pool + // after the PDO object is gone, and hand that same session to the next PDO + // built from the same DSN/user/password - including one built while another + // handle is still using it. Two handles then share one transaction: a COMMIT + // through either ends it for both, so the other's commit() raises + // "There is no active transaction" for a write that already committed, and the + // caller reports failure for data that landed. Laravel cannot detect this, + // because commit() gates on its own $transactions counter rather than on + // PDO::inTransaction() (Illuminate\Database\Concerns\ManagesTransactions). + // + // Off by default. Set DB_PERSISTENT=true only where the reconnect cost is + // measured and no request opens a transaction. + PDO::ATTR_PERSISTENT => env('DB_PERSISTENT', false), PDO::ATTR_TIMEOUT => 5, ]; From 94d5131788afce9deec08364207b9c9d2202cb2e Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 21 Aug 2026 21:57:51 +0800 Subject: [PATCH 2/7] fix(webhooks): a queue worker signed lifecycle webhooks with a stale secret SendResourceLifecycleWebhook only populated the session context when a key was absent, then preferred that session value over the event's own. A long running queue worker keeps its session between jobs, so once it had handled an event from one API context every later event was signed and attributed with the first one's credentials, and its company/user context leaked across jobs too. The context serialized on the event is now authoritative: it is resolved once, written to the session unconditionally for downstream code to read, and the previous session state is restored in a finally block so the next job starts clean. Fixes #244 --- .../SendResourceLifecycleWebhook.php | 142 ++++++-- tests/Unit/EventsAndExceptionsTest.php | 13 +- .../ResourceLifecycleWebhookListenerTest.php | 330 ++++++++++++------ 3 files changed, 345 insertions(+), 140 deletions(-) diff --git a/src/Listeners/SendResourceLifecycleWebhook.php b/src/Listeners/SendResourceLifecycleWebhook.php index cfd6c2f2..6de632bb 100644 --- a/src/Listeners/SendResourceLifecycleWebhook.php +++ b/src/Listeners/SendResourceLifecycleWebhook.php @@ -18,6 +18,21 @@ class SendResourceLifecycleWebhook implements ShouldQueue { + /** + * Session keys which carry the request context a lifecycle event was created in. + * + * @var string[] + */ + protected static array $contextSessionKeys = [ + 'api_credential', + 'api_key', + 'api_secret', + 'api_environment', + 'is_sandbox', + 'company', + 'user', + ]; + /** * Handle the event. * @@ -27,15 +42,35 @@ class SendResourceLifecycleWebhook implements ShouldQueue */ public function handle($event) { - $this->setSessionFromEvent($event); + // The context serialized on the event is the only trustworthy source for this job. A long + // running queue worker keeps its session between jobs, so the context left behind by a + // previously handled event must never be preferred over, or leak into, this one. + $context = static::resolveEventContext($event); + $restoreSession = $this->applySessionContext($context); - // get session variables or fallback to event value - $companyId = session()->get('company', $event->companySession); - $apiCredentialId = session()->get('api_credential', $event->apiCredential); - $apiKey = session()->get('api_key', $event->apiKey ?? 'console'); - $apiSecret = session()->get('api_secret', $event->apiSecret ?? 'internal'); - $apiEnvironment = session()->get('api_environment', $event->apiEnvironment ?? 'live'); - $isSandbox = session()->get('is_sandbox', $event->isSandbox); + try { + $this->sendWebhooksForEvent($event, $context); + } finally { + $restoreSession(); + } + } + + /** + * Send the webhooks for a single lifecycle event using the context serialized on it. + * + * @param ResourceLifecycleEvent $event + * @param array $context + * + * @return void + */ + protected function sendWebhooksForEvent($event, array $context) + { + $companyId = $context['company']; + $apiCredentialId = $context['api_credential']; + $apiKey = $context['api_key']; + $apiSecret = $context['api_secret']; + $apiEnvironment = $context['api_environment']; + $isSandbox = $context['is_sandbox']; // Compute the event payload exactly once so the persisted ApiEvent record and the // outbound webhook body are guaranteed to be identical. $event->getEventData() resolves @@ -53,17 +88,14 @@ public function handle($event) 'description' => $this->getHumanReadableEventDescription($event), ]; - // Get api credential from session - $apiCredential = session('api_credential'); - // Validate api credential, if not uuid then it could be internal - if ($apiCredential && Str::isUuid($apiCredential) && ApiCredential::where('uuid', session('api_credential'))->exists()) { - $eventData['api_credential_uuid'] = $apiCredential; + if ($apiCredentialId && Str::isUuid($apiCredentialId) && ApiCredential::where('uuid', $apiCredentialId)->exists()) { + $eventData['api_credential_uuid'] = $apiCredentialId; } // Check if it was a personal access token which made the request - if ($apiCredential && is_numeric($apiCredential) && PersonalAccessToken::where('id', $apiCredential)->exists()) { - $eventData['access_token_id'] = (int) $apiCredential; + if ($apiCredentialId && is_numeric($apiCredentialId) && PersonalAccessToken::where('id', $apiCredentialId)->exists()) { + $eventData['access_token_id'] = (int) $apiCredentialId; } try { @@ -154,36 +186,72 @@ public function handle($event) } } - public function setSessionFromEvent($event) + /** + * Resolve the request context which was serialized onto the event when it was dispatched. + * + * @param ResourceLifecycleEvent $event + * + * @return array + */ + public static function resolveEventContext($event): array { - // set session variables if not set - if (!session()->has('api_credential')) { - session()->put('api_credential', $event->apiCredential); - } + return [ + 'api_credential' => $event->apiCredential, + 'api_key' => $event->apiKey ?? 'console', + 'api_secret' => $event->apiSecret ?? 'internal', + 'api_environment' => $event->apiEnvironment ?? 'live', + 'is_sandbox' => (bool) $event->isSandbox, + 'company' => $event->companySession, + 'user' => $event->userSession, + ]; + } - if (!session()->has('api_key')) { - session()->put('api_key', $event->apiKey); - } + /** + * Replace the session context with the context serialized on the event. + * + * The session is replaced unconditionally: a queue worker session may already hold the context + * of an event it handled earlier, and that context must not be applied to this event. Downstream + * code (model scopes, observers, resources) still reads this context from the session, so it is + * written there for the duration of the job only. + * + * @param ResourceLifecycleEvent $event + * + * @return callable a callback which restores the session to the state it was in before the event + */ + public function setSessionFromEvent($event): callable + { + return $this->applySessionContext(static::resolveEventContext($event)); + } - if (!session()->has('api_secret')) { - session()->put('api_secret', $event->apiSecret); - } + /** + * Write a resolved event context to the session, replacing whatever was there before. + * + * @param array $context + * + * @return callable a callback which restores the session to the state it was in before the event + */ + protected function applySessionContext(array $context): callable + { + $previous = []; - if (!session()->has('api_environment')) { - session()->put('api_environment', $event->apiEnvironment); - } + foreach (static::$contextSessionKeys as $key) { + if (session()->has($key)) { + $previous[$key] = session()->get($key); + } - if (!session()->has('is_sandbox')) { - session()->put('is_sandbox', $event->isSandbox); + session()->put($key, $context[$key]); } - if (!session()->has('company')) { - session()->put('company', $event->companySession); - } + return function () use ($previous) { + foreach (static::$contextSessionKeys as $key) { + if (array_key_exists($key, $previous)) { + session()->put($key, $previous[$key]); + continue; + } - if (!session()->has('user')) { - session()->put('user', $event->userSession); - } + session()->remove($key); + } + }; } /** diff --git a/tests/Unit/EventsAndExceptionsTest.php b/tests/Unit/EventsAndExceptionsTest.php index 22028ef3..c85d8c27 100644 --- a/tests/Unit/EventsAndExceptionsTest.php +++ b/tests/Unit/EventsAndExceptionsTest.php @@ -788,7 +788,11 @@ public function toArray($request): array 'companySession' => 'company-uuid', ]); - $listener->setSessionFromEvent($event); + // stale context left in a long running queue worker session must be replaced, not preserved + session()->put('api_credential', 'stale-credential-uuid'); + session()->put('api_secret', 'stale-secret'); + + $restoreSession = $listener->setSessionFromEvent($event); expect(session('api_credential'))->toBe('credential-uuid') ->and(session('api_key'))->toBe('key') @@ -798,4 +802,11 @@ public function toArray($request): array ->and(session('company'))->toBe('company-uuid') ->and(session('user'))->toBe('user-uuid') ->and($listener->getHumanReadableEventDescription($event))->toBe('A order (Order 1001) was assigned a driver via API'); + + $restoreSession(); + + expect(session('api_credential'))->toBe('stale-credential-uuid') + ->and(session('api_secret'))->toBe('stale-secret') + ->and(session()->has('company'))->toBeFalse() + ->and(session()->has('user'))->toBeFalse(); }); diff --git a/tests/Unit/Listeners/ResourceLifecycleWebhookListenerTest.php b/tests/Unit/Listeners/ResourceLifecycleWebhookListenerTest.php index b023c37c..a0a1804a 100644 --- a/tests/Unit/Listeners/ResourceLifecycleWebhookListenerTest.php +++ b/tests/Unit/Listeners/ResourceLifecycleWebhookListenerTest.php @@ -202,6 +202,35 @@ public function toWebhookPayload(): array } } + class ResourceLifecycleWebhookListenerSessionSpyResource extends JsonResource + { + public static array $observed = []; + + public static function reset(): void + { + static::$observed = []; + } + + public function toWebhookPayload(): array + { + static::$observed[] = [ + 'api_credential' => session('api_credential'), + 'api_key' => session('api_key'), + 'api_secret' => session('api_secret'), + 'api_environment' => session('api_environment'), + 'is_sandbox' => session('is_sandbox'), + 'company' => session('company'), + 'user' => session('user'), + ]; + + return [ + 'id' => $this->resource->public_id, + 'uuid' => $this->resource->uuid, + 'status' => $this->resource->status, + ]; + } + } + class ResourceLifecycleWebhookListenerEvent extends ResourceLifecycleEvent { public ?EloquentModel $record = null; @@ -234,6 +263,47 @@ public function getModelResource($model, ?string $namespace = null, ?int $versio } } + function resource_lifecycle_webhook_listener_record(array $attributes = []): FleetbaseModel + { + $record = new FleetbaseModel(); + $record->setRawAttributes(array_merge([ + 'uuid' => 'record-uuid', + 'public_id' => 'order_1234567', + 'company_uuid' => 'company-uuid', + 'status' => 'dispatched', + ], $attributes), true); + + return $record; + } + + function resource_lifecycle_webhook_listener_event(array $context, ?EloquentModel $record = null, ?JsonResource $resource = null): ResourceLifecycleWebhookListenerEvent + { + $record = $record ?? resource_lifecycle_webhook_listener_record(); + + return ResourceLifecycleWebhookListenerEvent::fake(array_merge([ + 'modelName' => 'order', + 'modelClassNamespace' => FleetbaseModel::class, + 'modelClassName' => 'Order', + 'modelHumanName' => 'order', + 'modelUuid' => 'record-uuid', + 'namespace' => '\\Fleetbase', + 'version' => 1, + 'eventName' => 'updated', + 'sentAt' => '2026-07-18 15:25:00', + 'eventId' => 'event_lifecycle', + 'apiVersion' => 'v1', + 'requestMethod' => 'PATCH', + 'apiCredential' => null, + 'apiSecret' => 'event-secret', + 'apiKey' => 'event-api-key', + 'apiEnvironment' => 'live', + 'isSandbox' => false, + 'data' => [], + 'userSession' => null, + 'companySession' => 'company-uuid', + ], $context), $record, $resource ?? new ResourceLifecycleWebhookListenerResource($record)); + } + function resource_lifecycle_webhook_listener_database(): array { EloquentModel::clearBootedModels(); @@ -403,6 +473,7 @@ function resource_lifecycle_webhook_listener_database(): array } afterEach(function () { + ResourceLifecycleWebhookListenerSessionSpyResource::reset(); session()->flush(); Carbon::setTestNow(); EloquentModel::clearBootedModels(); @@ -490,99 +561,182 @@ function resource_lifecycle_webhook_listener_database(): array ->and($bus->jobs[0]->meta['api_event_uuid'])->toBe($apiEvent->uuid) ->and($bus->jobs[0]->meta['webhook_uuid'])->toBe('webhook-enabled') ->and($bus->jobs[0]->headers)->toHaveKey('X-Fleetbase-Signature') - ->and(session('company'))->toBe('company-uuid') - ->and(session('api_environment'))->toBe('live'); + ->and(session('company'))->toBeNull() + ->and(session('api_environment'))->toBeNull(); }); - test('resource lifecycle webhook listener preserves session credential attribution', function () { + test('resource lifecycle webhook listener prefers event credential attribution over a stale worker session', function () { [$capsule, $bus] = resource_lifecycle_webhook_listener_database(); + // context left behind in the worker session by a previously handled event session()->put('api_credential', '11111111-1111-4111-8111-111111111111'); session()->put('api_key', 'session-api-key'); session()->put('api_secret', 'session-secret'); - $record = new FleetbaseModel(); - $record->setRawAttributes([ - 'uuid' => 'record-uuid', - 'public_id' => 'order_1234567', - 'company_uuid' => 'company-uuid', - 'status' => 'dispatched', - ], true); - - $event = ResourceLifecycleWebhookListenerEvent::fake([ - 'modelName' => 'order', - 'modelClassNamespace' => FleetbaseModel::class, - 'modelClassName' => 'Order', - 'modelHumanName' => 'order', - 'modelUuid' => 'record-uuid', - 'namespace' => '\\Fleetbase', - 'version' => 1, - 'eventName' => 'updated', - 'sentAt' => '2026-07-18 15:25:00', - 'eventId' => 'event_lifecycle', - 'apiVersion' => 'v1', - 'requestMethod' => 'PATCH', - 'apiCredential' => 'internal-console', - 'apiSecret' => 'event-secret', - 'apiKey' => 'event-api-key', - 'apiEnvironment' => 'live', - 'isSandbox' => false, - 'data' => [], - 'userSession' => null, - 'companySession' => 'company-uuid', - ], $record, new ResourceLifecycleWebhookListenerResource($record)); + $event = resource_lifecycle_webhook_listener_event([ + 'apiCredential' => '44', + 'apiSecret' => 'event-secret', + 'apiKey' => 'event-api-key', + ]); (new SendResourceLifecycleWebhook())->handle($event); $apiEvent = ApiEvent::first(); - expect($apiEvent->api_credential_uuid)->toBe('11111111-1111-4111-8111-111111111111') - ->and($apiEvent->access_token_id)->toBeNull() + expect($apiEvent->access_token_id)->toBe(44) + ->and($apiEvent->api_credential_uuid)->toBeNull() ->and($bus->jobs)->toHaveCount(1) - ->and($bus->jobs[0]->meta['api_key'])->toBe('session-api-key') - ->and($bus->jobs[0]->meta['api_credential_uuid'])->toBe('11111111-1111-4111-8111-111111111111') - ->and($bus->jobs[0]->meta['access_token_id'])->toBeNull() - ->and($bus->jobs[0]->headers)->toHaveKey('X-Fleetbase-Signature'); + ->and($bus->jobs[0]->meta['api_key'])->toBe('event-api-key') + ->and($bus->jobs[0]->meta['access_token_id'])->toBe(44) + ->and($bus->jobs[0]->meta['api_credential_uuid'])->toBeNull() + ->and($bus->jobs[0]->headers['X-Fleetbase-Signature'])->toBe(hash_hmac('sha256', json_encode($bus->jobs[0]->payload), 'event-secret')); + }); + + test('resource lifecycle webhook listener restores the previous session context after handling an event', function () { + resource_lifecycle_webhook_listener_database(); + + session()->put('api_credential', '11111111-1111-4111-8111-111111111111'); + session()->put('api_key', 'session-api-key'); + session()->put('company', 'other-company'); + + $record = resource_lifecycle_webhook_listener_record(); + $event = resource_lifecycle_webhook_listener_event([ + 'apiCredential' => '44', + 'apiKey' => 'event-api-key', + 'apiSecret' => 'event-secret', + 'companySession' => 'company-uuid', + 'userSession' => 'user-uuid', + ], $record, new ResourceLifecycleWebhookListenerSessionSpyResource($record)); + + (new SendResourceLifecycleWebhook())->handle($event); + + // the event context is visible to downstream code while the job runs + expect(ResourceLifecycleWebhookListenerSessionSpyResource::$observed[0])->toBe([ + 'api_credential' => '44', + 'api_key' => 'event-api-key', + 'api_secret' => 'event-secret', + 'api_environment' => 'live', + 'is_sandbox' => false, + 'company' => 'company-uuid', + 'user' => 'user-uuid', + ]) + // and the session it was running in is handed back untouched + ->and(session('api_credential'))->toBe('11111111-1111-4111-8111-111111111111') + ->and(session('api_key'))->toBe('session-api-key') + ->and(session('company'))->toBe('other-company') + ->and(session()->has('api_secret'))->toBeFalse() + ->and(session()->has('api_environment'))->toBeFalse() + ->and(session()->has('is_sandbox'))->toBeFalse() + ->and(session()->has('user'))->toBeFalse(); + }); + + test('resource lifecycle webhook listener signs each queued event with its own secret', function () { + [$capsule, $bus] = resource_lifecycle_webhook_listener_database(); + + $listener = new SendResourceLifecycleWebhook(); + $record = resource_lifecycle_webhook_listener_record(); + + // two events handled back to back by the same long running worker + $listener->handle(resource_lifecycle_webhook_listener_event([ + 'eventId' => 'event_first', + 'apiCredential' => '11111111-1111-4111-8111-111111111111', + 'apiKey' => 'flb_live_key', + 'apiSecret' => 'secret-a', + 'userSession' => 'user-uuid', + ], $record, new ResourceLifecycleWebhookListenerSessionSpyResource($record))); + + $listener->handle(resource_lifecycle_webhook_listener_event([ + 'eventId' => 'event_second', + 'apiCredential' => '44', + 'apiKey' => 'console', + 'apiSecret' => 'secret-b', + 'userSession' => null, + ], $record, new ResourceLifecycleWebhookListenerSessionSpyResource($record))); + + expect($bus->jobs)->toHaveCount(2); + + [$first, $second] = $bus->jobs; + + expect($first->headers['X-Fleetbase-Signature'])->toBe(hash_hmac('sha256', json_encode($first->payload), 'secret-a')) + ->and($first->headers['X-Fleetbase-Signature'])->not->toBe(hash_hmac('sha256', json_encode($first->payload), 'secret-b')) + ->and($second->headers['X-Fleetbase-Signature'])->toBe(hash_hmac('sha256', json_encode($second->payload), 'secret-b')) + ->and($second->headers['X-Fleetbase-Signature'])->not->toBe(hash_hmac('sha256', json_encode($second->payload), 'secret-a')) + ->and($first->meta['api_key'])->toBe('flb_live_key') + ->and($second->meta['api_key'])->toBe('console'); + + $apiEvents = ApiEvent::all(); + + expect($apiEvents)->toHaveCount(2) + ->and($apiEvents[0]->api_credential_uuid)->toBe('11111111-1111-4111-8111-111111111111') + ->and($apiEvents[0]->access_token_id)->toBeNull() + ->and($apiEvents[1]->api_credential_uuid)->toBeNull() + ->and($apiEvents[1]->access_token_id)->toBe(44); + + $observed = ResourceLifecycleWebhookListenerSessionSpyResource::$observed; + + expect($observed[0]['api_secret'])->toBe('secret-a') + ->and($observed[0]['api_credential'])->toBe('11111111-1111-4111-8111-111111111111') + ->and($observed[0]['api_key'])->toBe('flb_live_key') + ->and($observed[0]['user'])->toBe('user-uuid') + ->and($observed[1]['api_secret'])->toBe('secret-b') + ->and($observed[1]['api_credential'])->toBe('44') + ->and($observed[1]['api_key'])->toBe('console') + ->and($observed[1]['user'])->toBeNull() + ->and(session()->has('api_secret'))->toBeFalse(); + }); + + test('resource lifecycle webhook listener does not leak environment sandbox or company context between queued events', function () { + [$capsule, $bus] = resource_lifecycle_webhook_listener_database(); + + $listener = new SendResourceLifecycleWebhook(); + $record = resource_lifecycle_webhook_listener_record(); + + // a sandbox event is handled first and must not push the next live event into sandbox + $listener->handle(resource_lifecycle_webhook_listener_event([ + 'eventId' => 'event_sandbox', + 'apiEnvironment' => 'sandbox', + 'isSandbox' => true, + 'companySession' => 'company-uuid', + ], $record, new ResourceLifecycleWebhookListenerSessionSpyResource($record))); + + $listener->handle(resource_lifecycle_webhook_listener_event([ + 'eventId' => 'event_live', + 'apiEnvironment' => 'live', + 'isSandbox' => false, + 'companySession' => 'company-uuid', + ], $record, new ResourceLifecycleWebhookListenerSessionSpyResource($record))); + + expect($bus->jobs)->toHaveCount(2) + ->and($bus->jobs[0]->meta['webhook_uuid'])->toBe('webhook-sandbox') + ->and($bus->jobs[0]->meta['is_sandbox'])->toBeTrue() + ->and($bus->jobs[1]->meta['webhook_uuid'])->toBe('webhook-enabled') + ->and($bus->jobs[1]->meta['is_sandbox'])->toBeFalse(); + + $observed = ResourceLifecycleWebhookListenerSessionSpyResource::$observed; + + expect($observed[0]['api_environment'])->toBe('sandbox') + ->and($observed[0]['is_sandbox'])->toBeTrue() + ->and($observed[1]['api_environment'])->toBe('live') + ->and($observed[1]['is_sandbox'])->toBeFalse() + ->and(session()->has('is_sandbox'))->toBeFalse() + ->and(session()->has('company'))->toBeFalse(); }); test('resource lifecycle webhook listener logs failed sandbox dispatches with access token context', function () { [$capsule, $bus] = resource_lifecycle_webhook_listener_database(); config()->set('webhook-server.signer', ResourceLifecycleWebhookListenerFailingSigner::class); - session()->put('api_credential', '44'); - session()->put('api_environment', 'sandbox'); - session()->put('is_sandbox', true); - $record = new FleetbaseModel(); - $record->setRawAttributes([ - 'uuid' => 'record-uuid', - 'public_id' => 'order_1234567', - 'company_uuid' => 'company-uuid', - 'status' => 'dispatched', - ], true); + // stale live context from a previously handled event must not redirect this sandbox event + session()->put('api_credential', '11111111-1111-4111-8111-111111111111'); + session()->put('api_environment', 'live'); + session()->put('is_sandbox', false); - $event = ResourceLifecycleWebhookListenerEvent::fake([ - 'modelName' => 'order', - 'modelClassNamespace' => FleetbaseModel::class, - 'modelClassName' => 'Order', - 'modelHumanName' => 'order', - 'modelUuid' => 'record-uuid', - 'namespace' => '\\Fleetbase', - 'version' => 1, - 'eventName' => 'updated', - 'sentAt' => '2026-07-18 15:25:00', - 'eventId' => 'event_lifecycle', - 'apiVersion' => 'v1', - 'requestMethod' => 'PATCH', - 'apiCredential' => null, - 'apiSecret' => 'event-secret', - 'apiKey' => 'event-api-key', - 'apiEnvironment' => 'live', - 'isSandbox' => false, - 'data' => [], - 'userSession' => null, - 'companySession' => 'company-uuid', - ], $record, new ResourceLifecycleWebhookListenerResource($record)); + $event = resource_lifecycle_webhook_listener_event([ + 'apiCredential' => '44', + 'apiEnvironment' => 'sandbox', + 'isSandbox' => true, + ]); (new SendResourceLifecycleWebhook())->handle($event); @@ -608,38 +762,10 @@ function resource_lifecycle_webhook_listener_database(): array [$capsule, $bus] = resource_lifecycle_webhook_listener_database(); config()->set('webhook-server.signer', ResourceLifecycleWebhookListenerFailingSigner::class); - session()->put('api_credential', '11111111-1111-4111-8111-111111111111'); - $record = new FleetbaseModel(); - $record->setRawAttributes([ - 'uuid' => 'record-uuid', - 'public_id' => 'order_1234567', - 'company_uuid' => 'company-uuid', - 'status' => 'dispatched', - ], true); - - $event = ResourceLifecycleWebhookListenerEvent::fake([ - 'modelName' => 'order', - 'modelClassNamespace' => FleetbaseModel::class, - 'modelClassName' => 'Order', - 'modelHumanName' => 'order', - 'modelUuid' => 'record-uuid', - 'namespace' => '\\Fleetbase', - 'version' => 1, - 'eventName' => 'updated', - 'sentAt' => '2026-07-18 15:25:00', - 'eventId' => 'event_lifecycle', - 'apiVersion' => 'v1', - 'requestMethod' => 'PATCH', - 'apiCredential' => null, - 'apiSecret' => 'event-secret', - 'apiKey' => 'event-api-key', - 'apiEnvironment' => 'live', - 'isSandbox' => false, - 'data' => [], - 'userSession' => null, - 'companySession' => 'company-uuid', - ], $record, new ResourceLifecycleWebhookListenerResource($record)); + $event = resource_lifecycle_webhook_listener_event([ + 'apiCredential' => '11111111-1111-4111-8111-111111111111', + ]); (new SendResourceLifecycleWebhook())->handle($event); From f78da7b9060d840e38ff518b780675719647b682 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Wed, 26 Aug 2026 18:34:46 +0800 Subject: [PATCH 3/7] fix(auth): api credentials survived revocation and creator removal Three defects on the API credential authentication path, all of which left a credential live that an operator had every reason to believe was dead. 1. Soft-deleted credentials still authenticated. AuthenticateOnceWithBasicAuth looks the credential up with withoutGlobalScopes(), which strips SoftDeletingScope along with ExpiryScope. Expiry was re-applied in PHP; soft-deletion never was. A credential the console reports as "Deleted" therefore kept authenticating indefinitely -- and Delete is the only revocation most operators ever perform. Now rejected with the generic "not valid" 401, before the OPTIONS shortcut so a revoked key cannot seed api key session context on a preflight either. 2. Authentication was fail-open when the creating user was gone. A credential carries no identity of its own; it acts as the user that created it. When User::find() no longer resolved that user, the is_admin guard was skipped but setSession() still returned true. Authorization degraded safely -- a null user fails every group and admin check -- but authentication did not, so the key kept working on every read endpoint and every ungated write. Off-boarding a person did not revoke the keys they had created. Auth::setSession() now returns false in that case and the middleware answers 401. Note User is pinned to the mysql connection, so this resolves against the authoritative store for sandbox credentials too. 3. "Expire immediately" did not expire the credential. ApiCredential::setExpiresAtAttribute() maps 'immediately' to Carbon::now(), but Expirable::hasExpired() used a strict `<`, so now() < now() was false. ExpiryScope already treats an exactly-now expiry as expired (it keeps a row only while expires_at > now()), so the two disagreed on that boundary. hasExpired() is now inclusive, which makes the trait and the scope agree. Reported downstream against 1.6.35 and verified against main. Private tracker: FliitAU/fliit-extension#2212 --- .../AuthenticateOnceWithBasicAuth.php | 20 +++- src/Support/Auth.php | 23 +++- src/Traits/Expirable.php | 7 +- tests/Unit/Http/MiddlewareContractsTest.php | 103 ++++++++++++++++++ tests/Unit/Support/AuthSupportTest.php | 15 +++ tests/Unit/Traits/LifecycleTraitsTest.php | 9 +- 6 files changed, 168 insertions(+), 9 deletions(-) diff --git a/src/Http/Middleware/AuthenticateOnceWithBasicAuth.php b/src/Http/Middleware/AuthenticateOnceWithBasicAuth.php index 52a33363..43cc16be 100644 --- a/src/Http/Middleware/AuthenticateOnceWithBasicAuth.php +++ b/src/Http/Middleware/AuthenticateOnceWithBasicAuth.php @@ -82,6 +82,18 @@ public function authenticatedWithBasic(Request $request, $connection = null) return response()->error('Oops! The api credentials provided were not valid', 401); } + // Credentials have been revoked. + // + // withoutGlobalScopes() above strips SoftDeletingScope along with ExpiryScope, so + // the lookup deliberately sees deleted rows. Expiry is re-applied in PHP below, but + // soft-deletion never was — a credential the console reports as "Deleted" kept + // authenticating indefinitely, and Delete was the only revocation most operators + // ever performed. Treated as "not valid" rather than a distinct message so a caller + // cannot distinguish a revoked key from one that never existed. + if ($apiCredential->trashed()) { + return response()->error('Oops! The api credentials provided were not valid', 401); + } + // If OPTIONS set api key and continue if ($request->isMethod('OPTIONS')) { // Set api credential session @@ -96,7 +108,13 @@ public function authenticatedWithBasic(Request $request, $connection = null) } // Login user - Auth::setSession($apiCredential); + // + // Fails when the credential's creating user no longer resolves — the credential has + // no identity to act as, so the request is rejected rather than continuing with a + // half-populated session. + if (Auth::setSession($apiCredential) !== true) { + return response()->error('Oops! The api credentials provided were not valid', 401); + } // Bind the user resolver so $request->user() answers on the public API. // diff --git a/src/Support/Auth.php b/src/Support/Auth.php index bed5531a..e1dad804 100644 --- a/src/Support/Auth.php +++ b/src/Support/Auth.php @@ -62,15 +62,26 @@ public static function setSession($user = null, $login = false): bool if ($user instanceof ApiCredential) { $apiCredential = $user; - session(['company' => $apiCredential->company_uuid, 'user' => $apiCredential->user_uuid]); - // user couldn't be loaded, fallback with api credential if applicable - $user = User::find($apiCredential->user_uuid); - // Set is admin if user of api credential is admin - if ($user) { - session(['is_admin' => $user->isAdmin()]); + // An API credential carries no identity of its own — it acts as the user that + // created it. When that user no longer resolves (hard or soft deleted) there is + // no identity to run as, so authentication must fail closed. + // + // This previously fell through and returned true with `is_admin` simply never + // set. Authorization degraded safely, but authentication did not: the key kept + // working on every read endpoint and every ungated write, so off-boarding a + // person did not revoke the keys they had created. + $user = User::find($apiCredential->user_uuid); + if (!$user instanceof User) { + return false; } + session([ + 'company' => $apiCredential->company_uuid, + 'user' => $apiCredential->user_uuid, + 'is_admin' => $user->isAdmin(), + ]); + // track last usage of api credential $apiCredential->trackLastUsed(); diff --git a/src/Traits/Expirable.php b/src/Traits/Expirable.php index b32aae92..0177a63f 100644 --- a/src/Traits/Expirable.php +++ b/src/Traits/Expirable.php @@ -88,7 +88,12 @@ public function hasExpired() $column = $this->getExpiredAtColumn(); if (is_object($this->{$column})) { - return $this->{$column} < Carbon::now(); + // Inclusive, to agree with ExpiryScope, which keeps a row only while + // `expires_at > now()` and therefore already treats an exactly-now expiry as + // expired. A strict `<` here disagreed with the scope on that boundary, which + // is precisely the value ApiCredential writes for the console's "immediately" + // option (Carbon::now()) — so "expire this key right now" left it valid. + return $this->{$column} <= Carbon::now(); } return false; diff --git a/tests/Unit/Http/MiddlewareContractsTest.php b/tests/Unit/Http/MiddlewareContractsTest.php index e47eae99..c22e59a8 100644 --- a/tests/Unit/Http/MiddlewareContractsTest.php +++ b/tests/Unit/Http/MiddlewareContractsTest.php @@ -348,6 +348,16 @@ function middleware_contracts_basic_auth_database(): Capsule ['uuid' => 'sanctum-user-invalid-company', 'company_uuid' => 'company-1', 'type' => 'user'], ['uuid' => 'sanctum-user-valid-company', 'company_uuid' => '550e8400-e29b-41d4-a716-446655440000', 'type' => 'user'], ['uuid' => 'sanctum-user-token-fallback', 'company_uuid' => '550e8400-e29b-41d4-a716-446655440001', 'type' => 'driver'], + // The User model is pinned to the mysql connection (User::$connection), and + // production is authoritative for users — the sandbox schema only holds a + // mirror maintained by sandbox:sync. So a sandbox credential's creator is + // resolved here, not on the sandbox connection. + ['uuid' => 'sandbox-user-1', 'company_uuid' => 'sandbox-company-1', 'type' => 'admin'], + ]); + // An off-boarded creator: the row still exists, but soft-deleted, so User::find() + // no longer resolves it. + $db->table('users')->insert([ + ['uuid' => 'user-gone', 'company_uuid' => 'company-1', 'type' => 'admin', 'deleted_at' => '2026-07-19 00:00:00'], ]); $db->table('companies')->insert([ ['uuid' => 'company-1', 'owner_id' => 'user-1', 'owner_uuid' => 'user-1'], @@ -359,6 +369,12 @@ function middleware_contracts_basic_auth_database(): Capsule ['uuid' => 'credential-expired', 'user_uuid' => 'user-1', 'company_uuid' => 'company-1', 'name' => 'Expired', 'key' => 'flb_live_expired', 'secret' => '$expired_secret', 'test_mode' => 0, 'last_used_at' => null, 'expires_at' => '2020-01-01 00:00:00', 'created_at' => '2026-07-18 00:00:00', 'updated_at' => '2026-07-18 00:00:00'], ['uuid' => 'credential-sanctum', 'user_uuid' => 'sanctum-user-valid-company', 'company_uuid' => '550e8400-e29b-41d4-a716-446655440000', 'name' => 'Sanctum', 'key' => 'flb_live_sanctum', 'secret' => '$sanctum_secret', 'test_mode' => 0, 'last_used_at' => null, 'expires_at' => null, 'created_at' => '2026-07-18 00:00:00', 'updated_at' => '2026-07-18 00:00:00'], ]); + // Revoked (soft-deleted, and carrying no expiry) and orphaned (its creating user + // has been off-boarded) credentials. + $db->table('api_credentials')->insert([ + ['uuid' => 'credential-revoked', 'user_uuid' => 'user-1', 'company_uuid' => 'company-1', 'name' => 'Revoked', 'key' => 'flb_live_revoked', 'secret' => '$revoked_secret', 'test_mode' => 0, 'last_used_at' => null, 'expires_at' => null, 'created_at' => '2026-07-18 00:00:00', 'updated_at' => '2026-07-18 00:00:00', 'deleted_at' => '2026-07-19 00:00:00'], + ['uuid' => 'credential-orphaned', 'user_uuid' => 'user-gone', 'company_uuid' => 'company-1', 'name' => 'Orphaned', 'key' => 'flb_live_orphaned', 'secret' => '$orphaned_secret', 'test_mode' => 0, 'last_used_at' => null, 'expires_at' => null, 'created_at' => '2026-07-18 00:00:00', 'updated_at' => '2026-07-18 00:00:00', 'deleted_at' => null], + ]); $db->table('personal_access_tokens')->insert([ ['id' => 1, 'tokenable_type' => FleetbaseUser::class, 'tokenable_id' => 'sanctum-user-invalid-company', 'name' => 'invalid-company', 'token' => hash('sha256', 'plain-invalid-company-token'), 'abilities' => json_encode(['*']), 'created_at' => '2026-07-18 00:00:00', 'updated_at' => '2026-07-18 00:00:00'], ['id' => 2, 'tokenable_type' => FleetbaseUser::class, 'tokenable_id' => 'sanctum-user-valid-company', 'name' => 'valid-company', 'token' => hash('sha256', 'plain-valid-company-token'), 'abilities' => json_encode(['*']), 'created_at' => '2026-07-18 00:00:00', 'updated_at' => '2026-07-18 00:00:00'], @@ -1110,6 +1126,93 @@ function () use (&$expiredContinued) { ]); }); + test('basic auth middleware rejects revoked credentials', function () { + // The credential lookup uses withoutGlobalScopes(), which strips SoftDeletingScope + // along with ExpiryScope. Expiry is re-applied in PHP; soft-deletion was not, so a + // credential the console reports as "Deleted" authenticated indefinitely. It also + // carries no expiry here, so nothing else could catch it. + $capsule = middleware_contracts_basic_auth_database(); + session()->flush(); + + $request = Request::create('/v1/orders', 'GET', [], [], [], [ + 'HTTP_AUTHORIZATION' => 'Bearer flb_live_revoked', + ]); + $continued = false; + $response = (new AuthenticateOnceWithBasicAuth())->handle( + $request, + function () use (&$continued) { + $continued = true; + + return new JsonResponse(['ok' => true]); + } + ); + + expect($continued)->toBeFalse() + ->and($response->getStatusCode())->toBe(401) + ->and($response->getData(true))->toBe([ + 'errors' => ['Oops! The api credentials provided were not valid'], + ]) + ->and(session('api_credential'))->toBeNull() + ->and(session('user'))->toBeNull() + ->and($capsule->getConnection('mysql')->table('api_credentials')->where('uuid', 'credential-revoked')->value('last_used_at'))->toBeNull(); + }); + + test('basic auth middleware rejects revoked credentials on preflight requests', function () { + // Rejected before the OPTIONS shortcut, so a revoked key cannot seed api key + // session context on a preflight either. + middleware_contracts_basic_auth_database(); + session()->flush(); + + $request = Request::create('/v1/orders', 'OPTIONS', [], [], [], [ + 'HTTP_AUTHORIZATION' => 'Bearer flb_live_revoked', + ]); + $continued = false; + $response = (new AuthenticateOnceWithBasicAuth())->handle( + $request, + function () use (&$continued) { + $continued = true; + + return new JsonResponse(['preflight' => true]); + } + ); + + expect($continued)->toBeFalse() + ->and($response->getStatusCode())->toBe(401) + ->and(session('api_credential'))->toBeNull(); + }); + + test('basic auth middleware fails closed when the credential creator no longer resolves', function () { + // A credential acts as the user that created it. Once that user is soft-deleted + // there is no identity to run as, and the request must be rejected — previously + // `is_admin` was simply never set and authentication still succeeded, so + // off-boarding a person left every key they had created working. + $capsule = middleware_contracts_basic_auth_database(); + session()->flush(); + + $request = Request::create('/v1/orders', 'GET', [], [], [], [ + 'HTTP_AUTHORIZATION' => 'Bearer flb_live_orphaned', + ]); + $continued = false; + $response = (new AuthenticateOnceWithBasicAuth())->handle( + $request, + function () use (&$continued) { + $continued = true; + + return new JsonResponse(['ok' => true]); + } + ); + + expect($continued)->toBeFalse() + ->and($response->getStatusCode())->toBe(401) + ->and($response->getData(true))->toBe([ + 'errors' => ['Oops! The api credentials provided were not valid'], + ]) + ->and(session('user'))->toBeNull() + ->and(session('company'))->toBeNull() + ->and(session('api_credential'))->toBeNull() + ->and($capsule->getConnection('mysql')->table('api_credentials')->where('uuid', 'credential-orphaned')->value('last_used_at'))->toBeNull(); + }); + test('basic auth middleware falls back to sandbox for sdk secret keys', function () { middleware_contracts_basic_auth_database(); session()->flush(); diff --git a/tests/Unit/Support/AuthSupportTest.php b/tests/Unit/Support/AuthSupportTest.php index 8b10f0e4..4aa4c7cd 100644 --- a/tests/Unit/Support/AuthSupportTest.php +++ b/tests/Unit/Support/AuthSupportTest.php @@ -688,6 +688,21 @@ function auth_support_request(string $method = 'GET', ?string $controllerClass = ->and(Auth::getApiKey()->uuid)->toBe($credential->uuid); }); +test('auth support fails closed when an api credential creator no longer resolves', function () { + [$admin, , $credential] = auth_support_fixtures(); + + // Off-board the creating user. A credential has no identity of its own — it acts as + // its creator — so there is nothing left for it to run as. This used to return true + // with `is_admin` merely unset, leaving the key live on every read endpoint. + app('db')->table('users')->where('uuid', $admin->uuid)->update(['deleted_at' => '2026-07-17 11:00:00']); + + expect(Auth::setSession($credential))->toBeFalse() + ->and(session('user'))->toBeNull() + ->and(session('company'))->toBeNull() + ->and(session('is_admin'))->toBeNull() + ->and(app('db')->table('api_credentials')->where('uuid', $credential->uuid)->value('last_used_at'))->toBeNull(); +}); + test('auth support returns null when no api credential session exists', function () { auth_support_fixtures(); diff --git a/tests/Unit/Traits/LifecycleTraitsTest.php b/tests/Unit/Traits/LifecycleTraitsTest.php index 5722163b..04231dcb 100644 --- a/tests/Unit/Traits/LifecycleTraitsTest.php +++ b/tests/Unit/Traits/LifecycleTraitsTest.php @@ -282,8 +282,15 @@ function lifecycle_traits_uuid_database(): Capsule $expired = new LifecycleTraitsExpirableRecord([ 'expires_at' => Carbon::now()->subMinute(), ]); + // ExpiryScope keeps a row only while `expires_at > now()`, so an exactly-now expiry is + // already excluded from queries — hasExpired() has to agree. This is the value written + // for the console's "expire immediately" option. + $expiredNow = new LifecycleTraitsExpirableRecord([ + 'expires_at' => Carbon::now(), + ]); - expect($active->hasExpired())->toBeFalse() + expect($expiredNow->hasExpired())->toBeTrue() + ->and($active->hasExpired())->toBeFalse() ->and($active->timeToLive())->toBe(300) ->and($active->expiresAtTimestamp())->toBe(Carbon::now()->addMinutes(5)->timestamp) ->and($active->getExpiredAtColumn())->toBe('expires_at') From f62f0931645b95823b8d27442def55ad027de931 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 28 Aug 2026 14:59:16 +0800 Subject: [PATCH 4/7] test(auth): cover the user resolver guard the fail-closed change orphaned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage gate went red on AuthenticateOnceWithBasicAuth (57/58 statements): the early `return;` in bindUserResolver() was previously reached only because the sandbox fixture seeded sandbox-user-1 on the sandbox connection alone, so User::find() came back null on mysql. Correcting that fixture to mirror the real system — User is pinned to mysql and sandbox:sync copies mysql to sandbox — left the guard unexercised. bindUserResolver() is protected static, so a downstream middleware subclass can call it with arguments neither in-tree call site produces. Cover the guard directly through a harness subclass rather than reaching for @codeCoverageIgnore, asserting both halves refuse to bind and that the positive case still does. --- tests/Unit/Http/MiddlewareContractsTest.php | 36 +++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/Unit/Http/MiddlewareContractsTest.php b/tests/Unit/Http/MiddlewareContractsTest.php index c22e59a8..96027c7b 100644 --- a/tests/Unit/Http/MiddlewareContractsTest.php +++ b/tests/Unit/Http/MiddlewareContractsTest.php @@ -67,6 +67,14 @@ class MiddlewareContractsHeaders extends SetGlobalHeaders protected array $except = ['health']; } + class MiddlewareContractsBasicAuthHarness extends AuthenticateOnceWithBasicAuth + { + public static function bindUserResolverPublic(?Request $request, $user): void + { + static::bindUserResolver($request, $user); + } + } + class MiddlewareContractsCustomMiddlewareHarness { use Fleetbase\Traits\CustomMiddleware; @@ -1213,6 +1221,34 @@ function () use (&$continued) { ->and($capsule->getConnection('mysql')->table('api_credentials')->where('uuid', 'credential-orphaned')->value('last_used_at'))->toBeNull(); }); + test('basic auth middleware user resolver binding refuses incomplete arguments', function () { + // bindUserResolver() is protected static, so a downstream middleware subclass can + // call it with whatever it has. Neither in-tree call site can reach this guard -- + // the sanctum path checks `tokenable instanceof User` first, and the credential + // path now fails closed before it -- but the guard still has to hold for callers + // that are not this class. + middleware_contracts_basic_auth_database(); + + $request = Request::create('/v1/orders', 'GET'); + $user = new FleetbaseUser(); + $user->setRawAttributes(['uuid' => 'user-1'], true); + + // No request to bind onto. + MiddlewareContractsBasicAuthHarness::bindUserResolverPublic(null, $user); + + // No user to bind -- must not install a resolver that yields null, which would + // shadow a guard that resolves the user later in the stack. + MiddlewareContractsBasicAuthHarness::bindUserResolverPublic($request, null); + + expect($request->user())->toBeNull(); + + // The positive case still binds, so the guard is not simply rejecting everything. + MiddlewareContractsBasicAuthHarness::bindUserResolverPublic($request, $user); + + expect($request->user())->toBeInstanceOf(FleetbaseUser::class) + ->and($request->user()->uuid)->toBe('user-1'); + }); + test('basic auth middleware falls back to sandbox for sdk secret keys', function () { middleware_contracts_basic_auth_database(); session()->flush(); From 4b1aff4c0b25773e197c39c39be7411a1714017e Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 28 Aug 2026 15:01:10 +0800 Subject: [PATCH 5/7] chore(release): v1.6.60 --- RELEASE.md | 41 ++++++++++++++++++++++++++++++----------- composer.json | 2 +- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 3eeed5f8..4c1377cc 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,29 +1,48 @@ -> v1.6.59 ~ "Verification and credentials email can render again" +> v1.6.60 ~ "Revoked keys stay revoked, transactions stop colliding, webhooks stop signing with the wrong secret" --- ## Highlights -Both mail templates failed to compile, so **every verification and credentials email threw instead of sending**. Any flow that delivers a code — customer signup, SMS login with email fallback, password reset, account closure, driver login — returned a 400 carrying a Blade parse error. +Three independent defects, each of which let the platform keep doing something an operator had already told it to stop. -Shipped in v1.6.56 and present in v1.6.57 and v1.6.58. Upgrade if you are on any of those. +**Deleting an API credential did not revoke it.** The console hid the row and the key kept authenticating — indefinitely, on every endpoint. Combined with an "expire immediately" option that also did not take effect, a stock Fleetbase console had **no working way to revoke an API credential**. Anyone who has ever deleted a key in the console should read the upgrade steps. + +**Persistent PDO handles shared one MySQL transaction.** Writes landed and the API still answered `422 There is no active transaction`, so anyone who retried after the error applied the write twice. + +**A queue worker signed every lifecycle webhook with the first event's secret.** Every outbound webhook after the first carried the wrong HMAC key, along with the wrong credential, environment and company. + +--- +## Security Fixes +- **Soft-deleted API credentials still authenticated.** `AuthenticateOnceWithBasicAuth` looks the credential up with `withoutGlobalScopes()`, which strips `SoftDeletingScope` along with `ExpiryScope`. Expiry was re-applied in PHP; soft-deletion never was. A credential the console reports as **Deleted** kept authenticating indefinitely — and Delete is the only revocation most operators ever perform. Now rejected with a 401, before the `OPTIONS` shortcut so a revoked key cannot seed api key session context on a preflight either. + +- **Authentication was fail-open when the credential's creator was gone.** An API credential carries no identity of its own; it acts as the user that created it. When that user no longer resolved — deleted, or soft-deleted on off-boarding — the `is_admin` guard was skipped but `Auth::setSession()` still returned `true`. Authorization degraded safely, since a null user fails every group and admin check, but authentication did not: the key kept working on every read endpoint and every ungated write. Off-boarding a person did not revoke the keys they had created. `setSession()` now returns `false` in that case and the middleware answers a clean 401. --- ## Bug Fixes -- **`verification.blade.php` and `user-credentials.blade.php` did not parse.** The greeting read `Good Morning@if($user->name), ...@endif`, and Blade only treats `@` as a directive when the preceding character is **not** a word character — the rule that keeps `foo@bar.com` from compiling. So the `@if` was left as literal text while its `@endif` compiled anyway, leaving an unmatched `endif` that broke the enclosing `if/elseif/else`: +- **"Expire immediately" did not expire the credential.** `ApiCredential::setExpiresAtAttribute()` maps the console's `immediately` option to `Carbon::now()`, but `Expirable::hasExpired()` used a strict `<`, so `now() < now()` was false. `ExpiryScope` already disagreed with it — it keeps a row only while `expires_at > now()`, i.e. it treats an exactly-now expiry as expired. `hasExpired()` is now inclusive, so the trait and the scope agree. - ``` - syntax error, unexpected token "else", expecting end of file - (View: .../core-api/views/mail/verification.blade.php) - ``` +- **Persistent PDO handles shared one MySQL transaction.** `Connection::commit()` decides whether to issue a COMMIT from its own counter; PDO decides whether one is legal from the server's `SERVER_STATUS_IN_TRANS` flag, and nothing reconciled the two. With `PDO::ATTR_PERSISTENT => true`, PHP hands the same MySQL session to a second handle built from the same DSN while the first is still using it, so one handle could commit — and thereby invalidate — the other's transaction. Observed on onboarding account creation, ledger invoice creation and inventory stock adjustments, none of which share a code path. Persistence is now `env('DB_PERSISTENT', false)` on the `mysql` and `sandbox` connections. - The greeting is now built in one expression, so no directive sits against a word. +- **A queue worker signed lifecycle webhooks with a stale secret.** `SendResourceLifecycleWebhook::setSessionFromEvent()` only wrote a session key when it was absent, and `handle()` preferred the session value over the event's. A `queue:work` process is long-running and its session store is a container singleton, so once the worker handled one lifecycle event, every later event reused that first event's `api_secret` — plus its `api_credential`, `api_key`, `api_environment`, `is_sandbox`, `company` and `user`. The context serialized onto the event is now authoritative for the job that carries it, and a restorer running in a `finally` hands the session back as it was found. Reported in #244. --- ## Testing -- Added a test that compiles **every** Blade view in the package and runs `php -l` over the result. Nothing caught the original break because no test ever compiled a view — the templates were only exercised through mocked mailers, which never render them. +- New coverage for revoked credentials on both normal and `OPTIONS` requests, for a credential whose creator has been soft-deleted, for `Auth::setSession()` returning `false`, and for the exactly-now expiry boundary. +- Four tests for the webhook secret bleed, each verified to fail against the unpatched listener, covering per-event signing, sandbox/live isolation, session restoration, and event-over-session credential attribution. +- The middleware fixture previously seeded the sandbox user only on the sandbox connection. `User` is pinned to the `mysql` connection and `sandbox:sync` mirrors `mysql → sandbox`, so the fixture now matches the real system. --- ## Upgrade Steps -No migration and no configuration change. If verification emails were failing, they will work again once this is deployed; no codes need reissuing. +**Audit your API credentials.** Any credential deleted from the console before this release was never actually revoked and has been live the whole time. After upgrading, those keys stop working — which is the point, but it means an integration quietly running on a key someone believed they had deleted will break at upgrade rather than at deletion. List your credentials including soft-deleted rows before deploying if you want to know what will change. + +**Keys whose creator has been off-boarded will start returning 401.** That is the intent of the fix, but it is a behaviour change for anyone whose integrations run on a departed employee's credential. Reassign those to a dedicated service user before upgrading. + +**Deployments not running Octane may see per-request connect cost.** `PDO::ATTR_PERSISTENT` now defaults to off. Under Octane connection counts are unchanged (measured at 17 on a 24-thread FrankenPHP container). Short-lived PHP-FPM workers that relied on the pool will pay roughly 1–3 ms per request; `DB_PERSISTENT=true` restores the old behaviour, at the cost of re-arming the transaction bug for any request that opens a transaction. Note that persistent connections also silently defeated Octane's `DisconnectFromDatabases` listener, which now works as intended. + +No migration and no configuration change is required. + +--- +## Still Open +API credentials still carry no scope of their own — `Auth::setSession()` derives `is_admin` from whoever created the key, so every key an admin creates is a full-admin key regardless of what it is named. Least privilege is currently only reachable indirectly, by scoping the *creator* into a dedicated service user. Per-key roles, or an explicit key assignee decoupled from the creator, is a feature rather than a fix and is tracked separately. --- ## Need help? diff --git a/composer.json b/composer.json index 5743a14d..a6251875 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "fleetbase/core-api", - "version": "1.6.59", + "version": "1.6.60", "description": "Core Framework and Resources for Fleetbase API", "keywords": [ "fleetbase", From 60a4a0c911c278a92a5b2cf8af56ce449e504730 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 28 Aug 2026 16:48:50 +0800 Subject: [PATCH 6/7] fix: build query string explicitly in Utils::apiUrl Laravel's url() helper renders its second argument as rawurlencoded path segments with keys discarded, so apiUrl('/api/user', ['id' => 1]) produced https://host/api/user/1 instead of the documented ?id=1. Build the query string with http_build_query after the port insertion instead. Also correct the test bootstrap's url() shim to mirror the real UrlGenerator::to() path-segment semantics, which had been masking the divergence in UtilsTest. --- src/Support/Utils.php | 8 +++++++- tests/Pest.php | 6 +++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Support/Utils.php b/src/Support/Utils.php index 58ec2700..b9f1ab9b 100644 --- a/src/Support/Utils.php +++ b/src/Support/Utils.php @@ -38,7 +38,9 @@ class Utils public static function apiUrl(string $path, ?array $queryParams = [], int $port = 80): string { $isLocalDevelopment = app()->environment(['local', 'development']); - $baseURL = url($path, $queryParams, !$isLocalDevelopment); + // Laravel's url() renders extra parameters as path segments, not a query string, + // so the query string must be built here + $baseURL = url($path, [], !$isLocalDevelopment); // Check if default port is used to avoid appending it unnecessarily if (!in_array($port, [80, 443])) { @@ -52,6 +54,10 @@ public static function apiUrl(string $path, ?array $queryParams = [], int $port } } + if (!empty($queryParams)) { + $baseURL .= (str_contains($baseURL, '?') ? '&' : '?') . http_build_query($queryParams); + } + return $baseURL; } diff --git a/tests/Pest.php b/tests/Pest.php index 28ddbb13..77acb87e 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -311,11 +311,15 @@ function storage_path(string $path = ''): string if (!function_exists('url')) { function url(string $path = '', mixed $parameters = [], ?bool $secure = null): string { + // Mirrors Illuminate\Routing\UrlGenerator::to(): extra parameters become + // rawurlencoded path segments with keys discarded, NOT a query string $base = $secure ? 'https://fleetbase.test' : 'http://fleetbase.test'; $path = '/' . ltrim($path, '/'); if (is_array($parameters) && $parameters !== []) { - return $base . $path . '?' . http_build_query($parameters); + $tail = implode('/', array_map('rawurlencode', array_values($parameters))); + + return $base . rtrim($path, '/') . '/' . $tail; } return $base . $path; From cf70880f48f3c348ba761e52a9d2e71ef4be7916 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 31 Aug 2026 10:24:59 +0800 Subject: [PATCH 7/7] fix: add the report execution statistics columns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every saved report fails to execute, in core and in every extension. `Report::updateExecutionStats()` sets `execution_count`, `average_execution_time` and `last_result_count` and then saves. The reports table has none of the three, so the save throws: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'execution_count' in 'field list' The query itself has already succeeded by that point — the failure happens while recording that it ran — so a working report reports an error, and the report builder's preview renders an empty result with nothing in the console to explain it. `getPerformanceMetrics()` returns all three and `cloneWithConfig()` resets all three, so this is a feature that was written and never migrated rather than a naming slip. The columns are distinct from `execution_time` and `row_count`, which 2025_09_25_084135_report_enhancements added and the API resource exposes: those describe the most recent run, these accumulate across runs. `average_execution_time` is a float because it holds a mean of integer millisecond timings. They are deliberately not added to `$fillable`. The model maintains them itself, and making server-owned statistics mass-assignable would let a client set its own execution count. ReportModelTest covers the rolling-average arithmetic well and could not catch this: it drives the model through a spy whose `save()` is a counter, so nothing it does reaches a schema. The new test closes that specific gap by reading the migrations, so it needs no database and stays in the pure-unit suite. It fails without the migration and passes with it. Verified against a running instance: before, all six of a company's saved reports failed with the missing-column error; after, all six execute and return rows, `execution_count` increments, and the rolling average updates. --- ...dd_report_execution_statistics_columns.php | 60 ++++++++++++++++++ .../Models/ReportExecutionColumnsTest.php | 63 +++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 migrations/2026_08_31_000000_add_report_execution_statistics_columns.php create mode 100644 tests/Unit/Models/ReportExecutionColumnsTest.php diff --git a/migrations/2026_08_31_000000_add_report_execution_statistics_columns.php b/migrations/2026_08_31_000000_add_report_execution_statistics_columns.php new file mode 100644 index 00000000..ad43896b --- /dev/null +++ b/migrations/2026_08_31_000000_add_report_execution_statistics_columns.php @@ -0,0 +1,60 @@ +unsignedInteger('execution_count')->default(0)->after('row_count'); + + // A mean of integer millisecond timings, so it needs somewhere for the + // fraction to go — `execution_time` is an integer because it holds one + // measurement rather than an average of several. + $table->float('average_execution_time')->nullable()->comment('Mean execution time in milliseconds across all runs')->after('execution_count'); + + $table->integer('last_result_count')->nullable()->after('average_execution_time'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('reports', function (Blueprint $table) { + $table->dropColumn([ + 'execution_count', + 'average_execution_time', + 'last_result_count', + ]); + }); + } +}; diff --git a/tests/Unit/Models/ReportExecutionColumnsTest.php b/tests/Unit/Models/ReportExecutionColumnsTest.php new file mode 100644 index 00000000..cad064e5 --- /dev/null +++ b/tests/Unit/Models/ReportExecutionColumnsTest.php @@ -0,0 +1,63 @@ +[A-Za-z]+\(\s*'([a-z0-9_]+)'/", $block, $found); + $columns = array_merge($columns, $found[1]); + } + } + + return array_unique($columns); +} + +it('has a column for every execution statistic the report model writes', function () { + $columns = reportsTableColumns(); + + expect($columns)->not->toBeEmpty('no reports blueprints were found to read'); + + foreach (['execution_count', 'average_execution_time', 'last_result_count', 'last_executed_at'] as $column) { + expect($columns)->toContain($column); + } +}); + +it('still has the per-run columns the api resource exposes', function () { + // execution_time and row_count describe the last run and are returned by the Report + // resource; the statistics above accumulate across runs. Both sets must exist — + // adding one must not be mistaken for replacing the other. + $columns = reportsTableColumns(); + + expect($columns)->toContain('execution_time') + ->and($columns)->toContain('row_count'); +});