From a3614ec68f00b73d9491afb49011e10f165adfda Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sat, 8 Aug 2026 00:00:03 +0800 Subject: [PATCH 01/28] ci: point the Postman contract at the v0.7.53 release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contract job pinned the reusable workflow to @dev-v0.7.53, a pre-release branch. That branch is now merged (fleetbase/fleetbase#575) and v0.7.53 is tagged, with fleetbase/fleetbase-api:v0.7.53 published to Docker Hub. - pins the reusable workflow to @v0.7.53 instead of the dev branch, so runs are reproducible rather than tracking a branch that can move or be deleted - passes fleetbase-ref: v0.7.53 explicitly. The reusable workflow still defaults that input to dev-v0.7.53, so without this the job would boot the stack from the pre-release branch while testing against the released image. Passing it makes the booted source and the published image the same commit. Bump both refs together at each release. Contract runs on this repo were previously failing before they reached Postman — the installer step died building the console image, because console/package.json and console/pnpm-lock.yaml were briefly out of sync on the release branch and console/Dockerfile installs with --frozen-lockfile. That is fixed in v0.7.53. Co-Authored-By: Claude Opus 5 --- .github/workflows/postman.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/postman.yml b/.github/workflows/postman.yml index 820bb0159..e41c4314c 100644 --- a/.github/workflows/postman.yml +++ b/.github/workflows/postman.yml @@ -4,7 +4,9 @@ name: API Contract (Postman) # collection against the live API. Delegates to the reusable workflow in # fleetbase/fleetbase. Requires org secrets POSTMAN_API_KEY + _GITHUB_AUTH_TOKEN # (inherited); no-ops until POSTMAN_API_KEY is set. -# TODO: change @dev-v0.7.53 to @main once that branch is merged. +# Pinned to the v0.7.53 release tag: that is the commit fleetbase/fleetbase-api:v0.7.53 +# was built from, so the booted stack and the published image match. Bump both refs +# together at each release. on: push: @@ -18,8 +20,9 @@ permissions: jobs: contract: - uses: fleetbase/fleetbase/.github/workflows/api-contract.yml@dev-v0.7.53 + uses: fleetbase/fleetbase/.github/workflows/api-contract.yml@v0.7.53 with: collections: "Fleetbase API" build-from-source: false + fleetbase-ref: v0.7.53 secrets: inherit From 5fed04b6d728241e05c71f7a65a591b6d241b8f6 Mon Sep 17 00:00:00 2001 From: Ron Date: Sat, 8 Aug 2026 12:11:43 +0800 Subject: [PATCH 02/28] ci: unpin the contract workflow now that it tracks latest fleetbase/fleetbase#578 changed the reusable workflow to default fleetbase-ref to main and to test against fleetbase/fleetbase-api:latest, so there is no longer a per-release ref to bump here. Drops the explicit fleetbase-ref and moves the workflow reference from @v0.7.53 to @main. Co-Authored-By: Claude Opus 5 --- .github/workflows/postman.yml | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/.github/workflows/postman.yml b/.github/workflows/postman.yml index e41c4314c..c6c6075ac 100644 --- a/.github/workflows/postman.yml +++ b/.github/workflows/postman.yml @@ -4,9 +4,15 @@ name: API Contract (Postman) # collection against the live API. Delegates to the reusable workflow in # fleetbase/fleetbase. Requires org secrets POSTMAN_API_KEY + _GITHUB_AUTH_TOKEN # (inherited); no-ops until POSTMAN_API_KEY is set. -# Pinned to the v0.7.53 release tag: that is the commit fleetbase/fleetbase-api:v0.7.53 -# was built from, so the booted stack and the published image match. Bump both refs -# together at each release. +# +# Deliberately unpinned. The reusable workflow defaults to booting fleetbase/fleetbase@main +# against fleetbase/fleetbase-api:latest, so every release is picked up automatically and +# there is no ref here to remember to bump. Each run records the image digest it actually +# resolved in its job summary, so a result stays traceable. To reproduce an older run: +# +# with: +# fleetbase-ref: v0.7.53 +# api-image: fleetbase/fleetbase-api:v0.7.53 on: push: @@ -20,9 +26,8 @@ permissions: jobs: contract: - uses: fleetbase/fleetbase/.github/workflows/api-contract.yml@v0.7.53 + uses: fleetbase/fleetbase/.github/workflows/api-contract.yml@main with: collections: "Fleetbase API" build-from-source: false - fleetbase-ref: v0.7.53 secrets: inherit From faf20a35c1e0cdaf815a0c93aa5832bb50e1faac Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sat, 8 Aug 2026 23:06:32 +0800 Subject: [PATCH 03/28] fix(customers): default a Place location so signup with a place works POST /v1/customers accepts an optional `place` object, and the documented payload in the Postman collection sends one. Creating it always failed: SQLSTATE[HY000]: General error: 1364 Field 'location' doesn't have a default value `places.location` is a NOT NULL POINT column with no database default, and the attribute allow-list in resolveCustomerPlace is address-only -- a caller cannot supply coordinates through this surface -- so Place::create was always called without one. Every documented signup that included a place returned a 500. Default it to Point(0, 0), the same placeholder the geocoding helpers already fall back to when an address cannot be resolved (Place::getGoogleAddressArray, Place::findExistingSharedPlace). The default sits in the base array of the array_merge, and `location` is not in the allow-list, so a caller can never override it. Verified against a live stack: the documented Create a Customer payload now returns 201 with the address resolved, and Place::create without a location still fails with 1364 -- confirming the default is what fixes it. Co-Authored-By: Claude Opus 5 --- .../src/Http/Controllers/Api/v1/CustomerController.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/server/src/Http/Controllers/Api/v1/CustomerController.php b/server/src/Http/Controllers/Api/v1/CustomerController.php index 149554d5d..c097096ed 100644 --- a/server/src/Http/Controllers/Api/v1/CustomerController.php +++ b/server/src/Http/Controllers/Api/v1/CustomerController.php @@ -19,6 +19,7 @@ use Fleetbase\FleetOps\Support\CustomerAuth; use Fleetbase\FleetOps\Support\Utils; use Fleetbase\Http\Controllers\Controller; +use Fleetbase\LaravelMysqlSpatial\Types\Point as SpatialPoint; use Fleetbase\Models\File; use Fleetbase\Models\User; use Fleetbase\Models\UserDevice; @@ -312,6 +313,14 @@ protected function resolveCustomerPlace($input, Contact $contact, string $compan 'company_uuid' => $companyUuid, 'owner_uuid' => $contact->uuid, 'owner_type' => get_class($contact), + // `places.location` is a NOT NULL POINT column with no database default, + // and $allowed above is address-only — a caller cannot supply coordinates + // here. Without this the insert fails with SQLSTATE[HY000] 1364 ("Field + // 'location' doesn't have a default value"), turning a documented signup + // payload into a 500. Point(0, 0) is the same placeholder the geocoding + // helpers fall back to when an address cannot be resolved — see + // Place::getGoogleAddressArray and Place::findExistingSharedPlace. + 'location' => new SpatialPoint(0, 0), ], $attributes, )); From 68a8eb11bde0bd23e2e9540d5e94d34befb03a07 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sat, 8 Aug 2026 23:19:39 +0800 Subject: [PATCH 04/28] fix(drivers): close verify-code authentication bypass POST /v1/drivers/verify-code minted a driver token with no verification code at all, on a default install. $verificationCode = VerificationCode::where([...])->exists(); if (!$verificationCode && $code !== config('fleetops.navigator.bypass_verification_code')) { return response()->apiError('Invalid verification code!'); } `$code` is unvalidated, so omitting it from the body makes it null. The config resolves to env('SMS_AUTH_BYPASS_CODE', env('NAVIGATOR_BYPASS_VERIFICATION_CODE')), which is null when neither is set -- the default. `null !== null` is false, so the whole condition is false and the guard never fired. The user lookup is not company-scoped, so any caller holding a valid org API credential could mint a token for any driver in the install. Verified against a live stack before the fix: a POST carrying only {"identity": ""} returned HTTP 200 with a Sanctum token. Replace the comparison with a shared guard requiring all three conditions the console equivalent already uses (AuthController::authenticateWithVerificationCode): the bypass code must be non-empty, the app must not be in production, and the comparison is constant-time. This closes three defects at once -- the null-equals-null bypass, the missing production gate, and the non-constant-time compare. The guard lives on Api\v1\DriverController and is called from Internal\v1 so the two verify-code paths cannot drift. `!== null && !== ''` is used rather than `!empty()` so a configured bypass code of "0" still works. Truth table over every relevant combination confirms the only behaviour changes are the two bypassable cases becoming rejections; configured, matching, non-production use is unchanged. Co-Authored-By: Claude Opus 5 --- .../Controllers/Api/v1/DriverController.php | 31 ++++++++++++++++++- .../Internal/v1/DriverController.php | 2 +- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/server/src/Http/Controllers/Api/v1/DriverController.php b/server/src/Http/Controllers/Api/v1/DriverController.php index bc1ba8c3c..d619fa9a5 100644 --- a/server/src/Http/Controllers/Api/v1/DriverController.php +++ b/server/src/Http/Controllers/Api/v1/DriverController.php @@ -611,7 +611,7 @@ public function verifyCode(Request $request) // find and verify code $verificationCode = VerificationCode::where(['subject_uuid' => $user->uuid, 'code' => $code, 'for' => $for])->exists(); - if (!$verificationCode && $code !== config('fleetops.navigator.bypass_verification_code')) { + if (!$verificationCode && !static::verificationBypassMatches($code)) { return response()->apiError('Invalid verification code!'); } @@ -1035,6 +1035,35 @@ private static function getDriverCompanyFromUser(User $user): ?Company return $company; } + /** + * Whether the supplied code matches the configured testing bypass code. + * + * Three conditions, all required, mirroring the console equivalent in + * Fleetbase\Http\Controllers\Internal\v1\AuthController::authenticateWithVerificationCode: + * + * - a bypass code must actually be configured. The previous + * `$code !== config(...)` comparison meant that on a default install -- + * where the config resolves to null -- omitting `code` entirely made the + * check `null !== null`, i.e. false, so the guard never fired and any + * caller with a valid org API credential could mint a driver token for + * any driver in the install without a code at all; + * - the app must not be in production, so a code left set in a deployed + * .env cannot be used against a live fleet; + * - the comparison is constant-time. + * + * Shared with Internal\v1\DriverController so both verify-code paths cannot + * drift apart. + */ + public static function verificationBypassMatches(?string $code): bool + { + $bypassCode = config('fleetops.navigator.bypass_verification_code'); + + return $bypassCode !== null + && $bypassCode !== '' + && !app()->environment('production') + && hash_equals((string) $bypassCode, (string) $code); + } + /** * Patches phone number with international code. */ diff --git a/server/src/Http/Controllers/Internal/v1/DriverController.php b/server/src/Http/Controllers/Internal/v1/DriverController.php index bcaadae54..ecdd4006f 100644 --- a/server/src/Http/Controllers/Internal/v1/DriverController.php +++ b/server/src/Http/Controllers/Internal/v1/DriverController.php @@ -665,7 +665,7 @@ public function verifyCode(Request $request) // Find and verify code $verificationCode = static::verificationCodeExists($user, $code, $for); - if (!$verificationCode && $code !== config('fleetops.navigator.bypass_verification_code')) { + if (!$verificationCode && !ApiDriverController::verificationBypassMatches($code)) { return response()->error('Invalid verification code!'); } From 9b4ebba36457e28a8a0b03430d53c506c220f635 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sat, 8 Aug 2026 23:27:30 +0800 Subject: [PATCH 05/28] feat(customers): add a non-production verification-code bypass Signing up a customer locally means waiting on a real email or paying for a real SMS, because the three code-checking endpoints (POST /v1/customers, /customers/verify-code, /customers/reset-password) match against real VerificationCode rows and there is no way to short-circuit them. Navigator already has fleetops.navigator.bypass_verification_code for exactly this need on the driver side; this is the customer equivalent. Adds fleetops.customers.verification_bypass_code, read from FLEETOPS_CUSTOMER_VERIFICATION_BYPASS_CODE, guarded by the same three conditions the console uses in AuthController::authenticateWithVerificationCode: a code must be configured, the app must not be in production, and the comparison is constant-time. With the variable unset -- the default -- the bypass cannot fire, and config/app.php resolves `env` to production when neither APP_ENV nor ENVIRONMENT is set, so it fails safe. Deliberately a distinct env var, not SMS_AUTH_BYPASS_CODE: that one already gates operator console login and driver login, and sharing it would make a single leaked value unlock three privilege tiers. The guard is intentionally NOT folded into verificationCodeExists() / findVerificationCode(). Those are test seams the controller contract tests override, so a policy living inside them would be stubbed away precisely where it needs asserting. CreateCustomerRequest's `code` rule is relaxed from `exists:verification_codes,code` to `required|string`. This is not a weakening: the controller matches code + for + meta->identity, whereas the `exists` rule accepts any live code issued for any purpose to any user. The rule is also already unenforced on the proxy path, since verifyCode() with for=fleetops_create_customer calls create(CreateCustomerRequest::createFrom()), which never runs validateResolved(). Left in place it would block the bypass before the controller ever sees the request. Tests cover: inert when unset and when empty; accepted for all three endpoints when configured and matching; a non-matching code still rejected while the bypass is live; rejected in production even when configured; and resetPassword surviving the null VerificationCode on the bypass path while still revoking sessions. 10/10 in ApiCustomerControllerContractsTest, 21/21 in RequestContractsTest. Co-Authored-By: Claude Opus 5 --- server/config/fleetops.php | 22 +++ .../Controllers/Api/v1/CustomerController.php | 40 ++++- .../Http/Requests/CreateCustomerRequest.php | 10 +- .../ApiCustomerControllerContractsTest.php | 155 ++++++++++++++++++ server/tests/RequestContractsTest.php | 2 +- 5 files changed, 223 insertions(+), 6 deletions(-) diff --git a/server/config/fleetops.php b/server/config/fleetops.php index 2aba7e97d..b5bb41357 100644 --- a/server/config/fleetops.php +++ b/server/config/fleetops.php @@ -100,6 +100,28 @@ 'app_identifier' => env('NAVIGATOR_APP_IDENTIFIER', 'io.fleetbase.navigator'), ], + /* + |-------------------------------------------------------------------------- + | Customers + |-------------------------------------------------------------------------- + | + | Testing-only verification-code bypass for the customer auth flows + | (POST /v1/customers, /customers/verify-code, /customers/reset-password). + | Intended for local development and staging QA, where signing up means + | waiting on a real email or paying for a real SMS. + | + | MUST be left unset in production. It is ignored outright when the app + | environment is `production`, and when unset no bypass is possible. + | + | Deliberately NOT wired to SMS_AUTH_BYPASS_CODE: that variable already + | gates operator console login and driver login, and reusing it here would + | make one leaked value unlock three different privilege tiers. + | + */ + 'customers' => [ + 'verification_bypass_code' => env('FLEETOPS_CUSTOMER_VERIFICATION_BYPASS_CODE'), + ], + /* |-------------------------------------------------------------------------- | API Events diff --git a/server/src/Http/Controllers/Api/v1/CustomerController.php b/server/src/Http/Controllers/Api/v1/CustomerController.php index 149554d5d..7b54739dd 100644 --- a/server/src/Http/Controllers/Api/v1/CustomerController.php +++ b/server/src/Http/Controllers/Api/v1/CustomerController.php @@ -144,7 +144,7 @@ public function create(CreateCustomerRequest $request) 'for' => 'fleetops_create_customer', 'meta->identity' => $identity, ]); - if (!$verificationCode) { + if (!$verificationCode && !$this->verificationBypassMatches($code)) { return response()->apiError('Invalid verification code provided.'); } @@ -417,7 +417,7 @@ public function verifyCode(Request $request) 'code' => $code, 'for' => $for, ]); - if (!$verificationCode) { + if (!$verificationCode && !$this->verificationBypassMatches($code)) { return response()->apiError('Invalid verification code.'); } @@ -503,7 +503,7 @@ public function resetPassword(Request $request) 'for' => 'fleetops_customer_password_reset', 'meta->identity' => $needle, ]); - if (!$verificationCode) { + if (!$verificationCode && !$this->verificationBypassMatches($code)) { return response()->apiError('Invalid reset code.'); } @@ -517,7 +517,10 @@ public function resetPassword(Request $request) $user->save(); // Invalidate all existing sessions for this user after a password reset. $this->deleteUserTokens($user); - $verificationCode->delete(); + // Null on the testing-bypass path — there is no row to consume. + if ($verificationCode) { + $verificationCode->delete(); + } return response()->json(['status' => 'ok']); } @@ -918,6 +921,35 @@ protected function generateSmsVerification(User $user, string $for, array $optio return VerificationCode::generateSmsVerificationFor($user, $for, $options); } + /** + * Whether the supplied code matches the configured testing bypass code. + * + * Three conditions, all required, mirroring the console equivalent in + * Fleetbase\Http\Controllers\Internal\v1\AuthController::authenticateWithVerificationCode: + * a bypass code must actually be configured, the app must not be running in + * production, and the comparison is constant-time. + * + * Fails safe by default: config/app.php resolves `env` to `production` when + * neither APP_ENV nor ENVIRONMENT is set, so an unconfigured install cannot + * be bypassed even accidentally. + * + * `!== null && !== ''` rather than `!empty()` — `!empty('0')` is false, so a + * configured bypass code of "0" would otherwise be silently ignored. + * + * Kept out of verificationCodeExists()/findVerificationCode() on purpose: + * those two are test seams that the controller contract tests override, so a + * policy living inside them would be stubbed away exactly where it matters. + */ + protected function verificationBypassMatches(?string $code): bool + { + $bypassCode = config('fleetops.customers.verification_bypass_code'); + + return $bypassCode !== null + && $bypassCode !== '' + && !app()->environment('production') + && hash_equals((string) $bypassCode, (string) $code); + } + protected function verificationCodeExists(array $attributes): bool { return VerificationCode::where($attributes)->exists(); diff --git a/server/src/Http/Requests/CreateCustomerRequest.php b/server/src/Http/Requests/CreateCustomerRequest.php index bd4265d80..761606858 100644 --- a/server/src/Http/Requests/CreateCustomerRequest.php +++ b/server/src/Http/Requests/CreateCustomerRequest.php @@ -23,7 +23,15 @@ public function rules(): array { return [ 'identity' => 'required|string', - 'code' => 'required|exists:verification_codes,code', + // Presence only — authorizing the code is the controller's job, and its + // check is strictly stronger: it matches code + for + meta->identity, + // whereas `exists:verification_codes,code` accepts any live code issued + // for any purpose to any user. The rule is also already unenforced on the + // proxy path, since verifyCode() with for=fleetops_create_customer calls + // create(CreateCustomerRequest::createFrom($request)), which never runs + // validateResolved(). Keeping it here only blocks the configured + // non-production testing bypass (fleetops.customers.verification_bypass_code). + 'code' => 'required|string', 'name' => 'required|string', 'password' => 'required|string|min:8', 'email' => [ diff --git a/server/tests/ApiCustomerControllerContractsTest.php b/server/tests/ApiCustomerControllerContractsTest.php index e0c9da3e2..f25e60959 100644 --- a/server/tests/ApiCustomerControllerContractsTest.php +++ b/server/tests/ApiCustomerControllerContractsTest.php @@ -960,3 +960,158 @@ function fleetopsApiCustomerJson($response): array 'name' => 'explode', ]))))->toBe(['error' => 'update failed']); }); + +/** + * Run a callback with an app container that supports environment(). + * + * The harness binds a bare Illuminate\Container\Container, which has no + * environment() — so CustomerController::verificationBypassMatches fatals with + * "Call to undefined method" without this. Mirrors the swap in + * NotificationAndMailContractsTest, but carries every existing binding across so + * the controller can still resolve config/request/db. + */ +function fleetopsApiCustomerWithEnvironment(string $environment, callable $callback): mixed +{ + $previousApp = Illuminate\Container\Container::getInstance(); + $app = new class extends Illuminate\Container\Container { + public string $fleetopsEnvironment = 'testing'; + + public function environment(...$environments) + { + if (empty($environments)) { + return $this->fleetopsEnvironment; + } + + $environments = is_array($environments[0]) ? $environments[0] : $environments; + + return in_array($this->fleetopsEnvironment, $environments, true); + } + + public function hasDebugModeEnabled() + { + return false; + } + }; + $app->fleetopsEnvironment = $environment; + + $reflection = new ReflectionClass(Illuminate\Container\Container::class); + foreach (['bindings', 'instances', 'aliases', 'abstractAliases', 'resolved', 'scopedInstances'] as $property) { + if (!$reflection->hasProperty($property)) { + continue; + } + $handle = $reflection->getProperty($property); + $handle->setAccessible(true); + $handle->setValue($app, $handle->getValue($previousApp)); + } + + Illuminate\Container\Container::setInstance($app); + + try { + return $callback(); + } finally { + Illuminate\Container\Container::setInstance($previousApp); + } +} + +test('api customer controller ignores the verification bypass unless it is configured', function () { + // Unset and empty-string are both inert. Without this the bypass would be a + // standing hole in every default install, which is the entire risk of shipping one. + foreach ([null, ''] as $bypassCode) { + config(['fleetops.customers.verification_bypass_code' => $bypassCode]); + + fleetopsApiCustomerWithEnvironment('local', function () { + $create = fleetopsApiCustomerController(); + $create->verificationExists = false; + $verify = fleetopsApiCustomerController(); + $verify->verificationExists = false; + $reset = fleetopsApiCustomerController(); + $reset->verificationCode = null; + + expect(fleetopsApiCustomerJson($create->create(new CreateCustomerRequest([ + 'code' => '000000', + 'identity' => 'jane@example.test', + ]))))->toBe(['error' => 'Invalid verification code provided.']) + ->and(fleetopsApiCustomerJson($verify->verifyCode(Request::create('/v1/customers/verify-code', 'POST', [ + 'identity' => 'jane@example.test', + 'code' => '000000', + ]))))->toBe(['error' => 'Invalid verification code.']) + ->and(fleetopsApiCustomerJson($reset->resetPassword(Request::create('/v1/customers/reset-password', 'POST', [ + 'identity' => 'jane@example.test', + 'code' => '000000', + 'password' => 'password-secret', + ]))))->toBe(['error' => 'Invalid reset code.']); + }); + } + + config(['fleetops.customers.verification_bypass_code' => null]); +}); + +test('api customer controller accepts a configured verification bypass outside production', function () { + config(['fleetops.customers.verification_bypass_code' => '000000']); + + fleetopsApiCustomerWithEnvironment('local', function () { + $create = fleetopsApiCustomerController(); + $create->verificationExists = false; + $verify = fleetopsApiCustomerController(); + $verify->verificationExists = false; + // No VerificationCode row on the bypass path — resetPassword must not fatal + // calling ->delete() on null, and must still revoke existing sessions. + $reset = fleetopsApiCustomerController(); + $reset->verificationCode = null; + // A non-matching code is still rejected while the bypass is live. + $wrong = fleetopsApiCustomerController(); + $wrong->verificationExists = false; + + expect($create->create(new CreateCustomerRequest([ + 'code' => '000000', + 'identity' => 'jane@example.test', + 'name' => 'Jane', + 'password' => 'password-secret', + ])))->toMatchArray(['resource' => 'customer', 'token' => 'plain-token']) + ->and($verify->verifyCode(Request::create('/v1/customers/verify-code', 'POST', [ + 'identity' => 'jane@example.test', + 'code' => '000000', + ])))->toMatchArray(['resource' => 'customer', 'token' => 'plain-token']) + ->and(fleetopsApiCustomerJson($reset->resetPassword(Request::create('/v1/customers/reset-password', 'POST', [ + 'identity' => 'jane@example.test', + 'code' => '000000', + 'password' => 'password-secret', + ]))))->toBe(['status' => 'ok']) + ->and($reset->genericUser->tokensDeleted)->toBeTrue() + ->and(fleetopsApiCustomerJson($wrong->create(new CreateCustomerRequest([ + 'code' => '999999', + 'identity' => 'jane@example.test', + ]))))->toBe(['error' => 'Invalid verification code provided.']); + }); + + config(['fleetops.customers.verification_bypass_code' => null]); +}); + +test('api customer controller refuses the verification bypass in production', function () { + config(['fleetops.customers.verification_bypass_code' => '000000']); + + fleetopsApiCustomerWithEnvironment('production', function () { + $create = fleetopsApiCustomerController(); + $create->verificationExists = false; + $verify = fleetopsApiCustomerController(); + $verify->verificationExists = false; + $reset = fleetopsApiCustomerController(); + $reset->verificationCode = null; + + expect(fleetopsApiCustomerJson($create->create(new CreateCustomerRequest([ + 'code' => '000000', + 'identity' => 'jane@example.test', + ]))))->toBe(['error' => 'Invalid verification code provided.']) + ->and(fleetopsApiCustomerJson($verify->verifyCode(Request::create('/v1/customers/verify-code', 'POST', [ + 'identity' => 'jane@example.test', + 'code' => '000000', + ]))))->toBe(['error' => 'Invalid verification code.']) + ->and(fleetopsApiCustomerJson($reset->resetPassword(Request::create('/v1/customers/reset-password', 'POST', [ + 'identity' => 'jane@example.test', + 'code' => '000000', + 'password' => 'password-secret', + ]))))->toBe(['error' => 'Invalid reset code.']); + }); + + config(['fleetops.customers.verification_bypass_code' => null]); +}); diff --git a/server/tests/RequestContractsTest.php b/server/tests/RequestContractsTest.php index e0fcf72d2..d6e1051fb 100644 --- a/server/tests/RequestContractsTest.php +++ b/server/tests/RequestContractsTest.php @@ -621,7 +621,7 @@ protected function canUpdateDriver(): bool expect($request->authorize())->toBeTrue() ->and($rules['identity'])->toBe('required|string') - ->and($rules['code'])->toBe('required|exists:verification_codes,code') + ->and($rules['code'])->toBe('required|string') ->and($rules['name'])->toBe('required|string') ->and($rules['password'])->toBe('required|string|min:8') ->and(ruleStrings($rules['email']))->toContain('email', 'nullable', 'unique:contacts') From 7ec2b0ef326c722016220675b28a27be155e2dd4 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sun, 9 Aug 2026 00:29:50 +0800 Subject: [PATCH 06/28] fix(api): answer 404 instead of 500 for an unknown onboard organization GET /v1/onboard/driver-onboard-settings/{companyId} passed the result of findCompanyByPublicId() straight into driverOnboardSetting() without checking it. The lookup is declared ?Company and returns null for any public id that does not resolve, so null->uuid yielded null, which then hit the string type declaration on driverOnboardSetting() and threw: TypeError: driverOnboardSetting(): Argument #1 ($companyUuid) must be of type string, null given That escaped as an unhandled exception and rendered a ~1.2 MB HTML stack trace with a 500. Reproduced against a live stack with both a bogus public id and the literal "{{organization_id}}" the Postman collection was sending; a valid public id was unaffected. An unknown organization is a client error, so guard the null and return 404 through a new errorResponse() seam, matching the protected-seam style the rest of this controller already uses. The existing probe overrode findCompanyByPublicId() to always return a hydrated Company, so the null branch was unreachable in tests. It can now be made to miss, and a regression test pins the 404 and its error envelope. Co-Authored-By: Claude Opus 5 --- .../Api/v1/NavigatorController.php | 27 ++++++++-------- server/tests/SmallControllerContractsTest.php | 32 ++++++++++++++++--- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/server/src/Http/Controllers/Api/v1/NavigatorController.php b/server/src/Http/Controllers/Api/v1/NavigatorController.php index 7ac4530a3..a818a10f0 100644 --- a/server/src/Http/Controllers/Api/v1/NavigatorController.php +++ b/server/src/Http/Controllers/Api/v1/NavigatorController.php @@ -10,25 +10,21 @@ class NavigatorController extends Controller { /** - * Retrieve the driver onboard settings. + * Retrieve the driver onboard settings for an organization. * - * This method retrieves the driver onboard settings for the current company session. If no company session - * is found in the request, an error response is returned. The method retrieves the company ID from the session, - * then fetches the saved driver onboard settings. If settings for the current company are found, they are returned, - * otherwise, default settings are provided. - * - * @return JsonResponse - */ - - /** - * Retrieve driver onboard settings. + * The organization is resolved from its public id. An unknown public id is a client + * error, not a server error, so it answers 404 rather than dereferencing a null company. * * @return JsonResponse */ public function getDriverOnboardSettings($companyId) { - $company = $this->findCompanyByPublicId($companyId); - $driverOnboardSettings = $this->driverOnboardSetting($company->uuid); + $company = $this->findCompanyByPublicId($companyId); + if (!$company) { + return $this->errorResponse('Organization not found.', 404); + } + + $driverOnboardSettings = $this->driverOnboardSetting($company->uuid); if (!$driverOnboardSettings) { $driverOnboardSettings = []; } @@ -50,4 +46,9 @@ protected function jsonResponse(array $payload): JsonResponse { return response()->json($payload); } + + protected function errorResponse(string $message, int $statusCode): JsonResponse + { + return response()->error($message, $statusCode); + } } diff --git a/server/tests/SmallControllerContractsTest.php b/server/tests/SmallControllerContractsTest.php index d7c511b31..682ba2c18 100644 --- a/server/tests/SmallControllerContractsTest.php +++ b/server/tests/SmallControllerContractsTest.php @@ -227,19 +227,30 @@ protected function jsonResponse(array $payload) class FleetOpsPublicNavigatorControllerProbe extends PublicNavigatorController { - public mixed $settings = ['require_photo' => true]; - public array $lookups = []; + public mixed $settings = ['require_photo' => true]; + public array $lookups = []; + public bool $companyMiss = false; protected function findCompanyByPublicId(string $companyId): ?Company { + $this->lookups[] = $companyId; + + // Mirrors the real lookup returning null for an unknown public id. + if ($this->companyMiss) { + return null; + } + $company = new Company(); $company->setRawAttributes(['uuid' => 'company-uuid', 'public_id' => $companyId], true); - $this->lookups[] = $companyId; - return $company; } + protected function errorResponse(string $message, int $statusCode): JsonResponse + { + return new JsonResponse(['errors' => [$message]], $statusCode); + } + protected function driverOnboardSetting(string $companyUuid): mixed { $this->lookups[] = $companyUuid; @@ -412,6 +423,19 @@ function fleetopsSmallExportSelections(object $export): array ]); }); +test('public navigator controller answers 404 for an organization public id that does not resolve', function () { + $controller = new FleetOpsPublicNavigatorControllerProbe(); + $controller->companyMiss = true; + + $response = $controller->getDriverOnboardSettings('company_does_not_exist'); + + // Regression: the unresolved company used to be dereferenced anyway, which threw a + // TypeError out of the controller and rendered an HTML stack trace with a 500. + expect($response->getStatusCode())->toBe(404) + ->and($response->getData(true))->toBe(['errors' => ['Organization not found.']]) + ->and($controller->lookups)->toBe(['company_does_not_exist']); +}); + test('public contact controller normalizes create input and preserves update fields', function () { $controller = new FleetOpsContactControllerProbe(); From 5e979bd1d45adee24dbed2139c05f3be79c80a1b Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sun, 9 Aug 2026 00:35:12 +0800 Subject: [PATCH 07/28] fix(api): return 422 instead of 500 on duplicate part SKU and fuel transaction id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /v1/parts and POST /v1/fuel-transactions returned HTTP 500 on a well-formed body. Both are the same defect: a UNIQUE index exists, the FormRequest has no matching `unique:` rule, and the resulting UniqueConstraintViolationException escapes uncaught. Public v1 controllers extend the bare Controller with no QueryException catch, so it reaches the client as a 500 with no field attribution. (Internal /int/v1 controllers get HasApiControllerBehavior, which catches it — hence v1-only.) Two aggravating factors surfaced while reproducing: Neither unique index was soft-delete aware, so a deleted row kept occupying the key. Verified end to end: DELETE a part, then POST the same SKU, and the API returns 500 — permanently, since there is no restore or force-delete route. A validation rule alone would not fix this; it would pass validation and still fail at the driver. Both indexes are now soft-delete aware via a STORED generated column that is NULL for deleted rows, following the pattern in alrashed 2026_05_25_000001. fuel_provider_transactions was also keyed globally on (provider, provider_transaction_id) with no company_uuid, and FuelProviderService::ingestTransaction() keyed its updateOrCreate on the same pair. Neither model registers CompanyScope, and CompanyScope is inert under CLI where the sync runs, so one company's sync could find and overwrite another company's transaction, reassigning its company_uuid. The index and the ingest key both gain company_uuid. Both new indexes are strictly more permissive than the ones they replace, so no existing row can violate them and no backfill is required. The parts replacement is created before the old index is dropped: the old one was the only index covering company_uuid and parts_company_uuid_foreign depends on it, so dropping first fails with errno 150. Verified against a live MySQL 8.0.41 stack: migration applies (and is re-runnable), both indexes and generated columns land as intended. Co-Authored-By: Claude Opus 5 --- ...ction_unique_indexes_soft_delete_aware.php | 152 ++++++++++++++++++ .../Requests/CreateFuelTransactionRequest.php | 65 +++++++- .../src/Http/Requests/CreatePartRequest.php | 40 ++++- .../FuelProviders/FuelProviderService.php | 11 +- server/tests/RequestContractsTest.php | 64 +++++++- 5 files changed, 326 insertions(+), 6 deletions(-) create mode 100644 server/migrations/2026_08_08_000001_make_sku_and_provider_transaction_unique_indexes_soft_delete_aware.php diff --git a/server/migrations/2026_08_08_000001_make_sku_and_provider_transaction_unique_indexes_soft_delete_aware.php b/server/migrations/2026_08_08_000001_make_sku_and_provider_transaction_unique_indexes_soft_delete_aware.php new file mode 100644 index 000000000..60f5710f2 --- /dev/null +++ b/server/migrations/2026_08_08_000001_make_sku_and_provider_transaction_unique_indexes_soft_delete_aware.php @@ -0,0 +1,152 @@ + (company_uuid, active_sku) ---- + if (!Schema::hasColumn('parts', 'active_sku')) { + Schema::table('parts', function (Blueprint $table) { + $table->string('active_sku')->nullable()->storedAs('IF(`deleted_at` IS NULL, `sku`, NULL)'); + }); + } + + // The replacement index MUST be created before the old one is dropped: + // parts_company_uuid_sku_unique is currently the only index covering + // company_uuid, and parts_company_uuid_foreign depends on it. Adding an + // index that also leads with company_uuid lets it take over as the + // covering index, otherwise the drop below fails with errno 150. + if (!$this->indexExists('parts', 'parts_company_uuid_active_sku_unique')) { + Schema::table('parts', function (Blueprint $table) { + $table->unique(['company_uuid', 'active_sku'], 'parts_company_uuid_active_sku_unique'); + }); + } + + if ($this->indexExists('parts', 'parts_company_uuid_sku_unique')) { + Schema::table('parts', function (Blueprint $table) { + $table->dropUnique('parts_company_uuid_sku_unique'); + }); + } + + // ---- fuel_provider_transactions: (provider, provider_transaction_id) + // -> (company_uuid, provider, active_provider_transaction_id) ---- + if (!Schema::hasColumn('fuel_provider_transactions', 'active_provider_transaction_id')) { + Schema::table('fuel_provider_transactions', function (Blueprint $table) { + $table->string('active_provider_transaction_id', 191)->nullable()->storedAs('IF(`deleted_at` IS NULL, `provider_transaction_id`, NULL)'); + }); + } + + if (!$this->indexExists('fuel_provider_transactions', 'fuel_provider_txn_company_provider_unique')) { + Schema::table('fuel_provider_transactions', function (Blueprint $table) { + $table->unique(['company_uuid', 'provider', 'active_provider_transaction_id'], 'fuel_provider_txn_company_provider_unique'); + }); + } + + if ($this->indexExists('fuel_provider_transactions', 'fuel_provider_txn_provider_unique')) { + Schema::table('fuel_provider_transactions', function (Blueprint $table) { + $table->dropUnique('fuel_provider_txn_provider_unique'); + }); + } + } + + /** + * Reverse the migrations. + * + * Restoring the old global index can fail if rows now exist that only the new + * key permits (two tenants sharing a provider transaction id, or a live row + * whose SKU matches a soft-deleted one). That is inherent to reversing a + * relaxed constraint, so the old indexes are restored on a best-effort basis + * and the generated columns are dropped either way. + * + * @return void + */ + public function down() + { + if (!$this->indexExists('parts', 'parts_company_uuid_sku_unique')) { + try { + Schema::table('parts', function (Blueprint $table) { + $table->unique(['company_uuid', 'sku'], 'parts_company_uuid_sku_unique'); + }); + } catch (Throwable $e) { + // Duplicate SKUs the new index allowed now block the old one. + } + } + + if ($this->indexExists('parts', 'parts_company_uuid_active_sku_unique')) { + Schema::table('parts', function (Blueprint $table) { + $table->dropUnique('parts_company_uuid_active_sku_unique'); + }); + } + + if (Schema::hasColumn('parts', 'active_sku')) { + Schema::table('parts', function (Blueprint $table) { + $table->dropColumn('active_sku'); + }); + } + + if (!$this->indexExists('fuel_provider_transactions', 'fuel_provider_txn_provider_unique')) { + try { + Schema::table('fuel_provider_transactions', function (Blueprint $table) { + $table->unique(['provider', 'provider_transaction_id'], 'fuel_provider_txn_provider_unique'); + }); + } catch (Throwable $e) { + // Cross-tenant duplicates the new index allows now block the old one. + } + } + + if ($this->indexExists('fuel_provider_transactions', 'fuel_provider_txn_company_provider_unique')) { + Schema::table('fuel_provider_transactions', function (Blueprint $table) { + $table->dropUnique('fuel_provider_txn_company_provider_unique'); + }); + } + + if (Schema::hasColumn('fuel_provider_transactions', 'active_provider_transaction_id')) { + Schema::table('fuel_provider_transactions', function (Blueprint $table) { + $table->dropColumn('active_provider_transaction_id'); + }); + } + } + + private function indexExists(string $table, string $index): bool + { + $database = DB::connection()->getDatabaseName(); + + return DB::table('information_schema.statistics') + ->where('table_schema', $database) + ->where('table_name', $table) + ->where('index_name', $index) + ->exists(); + } +}; diff --git a/server/src/Http/Requests/CreateFuelTransactionRequest.php b/server/src/Http/Requests/CreateFuelTransactionRequest.php index ca91ca6eb..bfe77aa1a 100644 --- a/server/src/Http/Requests/CreateFuelTransactionRequest.php +++ b/server/src/Http/Requests/CreateFuelTransactionRequest.php @@ -2,6 +2,7 @@ namespace Fleetbase\FleetOps\Http\Requests; +use Fleetbase\FleetOps\Models\FuelProviderTransaction; use Fleetbase\Http\Requests\FleetbaseRequest; use Illuminate\Validation\Rule; @@ -16,7 +17,7 @@ public function rules(): array { return [ 'provider' => [Rule::requiredIf($this->isMethod('POST')), 'string'], - 'provider_transaction_id' => [Rule::requiredIf($this->isMethod('POST')), 'string'], + 'provider_transaction_id' => [Rule::requiredIf($this->isMethod('POST')), 'string', $this->uniqueProviderTransactionIdRule()], 'connection' => ['nullable', 'string'], 'fuel_report' => ['nullable', 'string'], 'vehicle' => ['nullable', 'string'], @@ -47,4 +48,66 @@ public function rules(): array 'meta' => ['nullable', 'array'], ]; } + + public function messages(): array + { + return [ + 'provider_transaction_id.unique' => 'A fuel transaction with this provider transaction id already exists for this provider.', + ]; + } + + /** + * Mirrors the fuel_provider_txn_company_provider_unique index. + * + * Provider transaction ids are natural idempotency keys, so a re-sent batch + * used to raise an unhandled UniqueConstraintViolationException — an HTTP 500 + * rather than a duplicate signal the client could act on. + * + * @return \Illuminate\Validation\Rules\Unique + */ + protected function uniqueProviderTransactionIdRule() + { + $provider = $this->resolveProvider(); + + $rule = Rule::unique('fuel_provider_transactions', 'provider_transaction_id') + ->where(function ($query) use ($provider) { + $query->where('company_uuid', session('company')); + $query->where('provider', $provider); + + return $query->whereNull('deleted_at'); + }); + + // On PUT the record re-sends its own provider_transaction_id. The v1 route + // parameter is a public_id (fuel_provider_transaction_xxxxx), not a uuid. + $id = $this->route('id'); + if (!$this->isMethod('POST') && filled($id)) { + $rule->ignore($id, 'public_id'); + } + + return $rule; + } + + /** + * The uniqueness scope is per provider, but `provider` is only required on + * POST — a PUT may change the transaction id while leaving the provider out of + * the body. Fall back to the stored value so the scope is never null, which + * would otherwise compare against the wrong set of rows. + */ + protected function resolveProvider(): ?string + { + if ($this->filled('provider')) { + return $this->input('provider'); + } + + $id = $this->route('id'); + if (blank($id)) { + return null; + } + + return FuelProviderTransaction::where('company_uuid', session('company')) + ->where(function ($query) use ($id) { + $query->where('public_id', $id)->orWhere('uuid', $id); + }) + ->value('provider'); + } } diff --git a/server/src/Http/Requests/CreatePartRequest.php b/server/src/Http/Requests/CreatePartRequest.php index a3a197af7..fc600d271 100644 --- a/server/src/Http/Requests/CreatePartRequest.php +++ b/server/src/Http/Requests/CreatePartRequest.php @@ -15,7 +15,7 @@ public function authorize(): bool public function rules(): array { return [ - 'sku' => ['nullable', 'string'], + 'sku' => ['nullable', 'string', $this->uniqueSkuRule()], 'name' => [Rule::requiredIf($this->isMethod('POST')), 'string'], 'manufacturer' => ['nullable', 'string'], 'model' => ['nullable', 'string'], @@ -37,4 +37,42 @@ public function rules(): array 'meta' => ['nullable', 'array'], ]; } + + public function messages(): array + { + return [ + 'sku.unique' => 'A part with this SKU already exists.', + ]; + } + + /** + * Mirrors the parts_company_uuid_active_sku_unique index. + * + * Without this rule a duplicate SKU reached the driver and came back as an + * unhandled UniqueConstraintViolationException — an HTTP 500 with no field + * attribution, since public v1 controllers have no QueryException catch. + * + * `nullable` runs first, so this never fires on a null SKU; that matches the + * nullable column and MySQL's tolerance of repeated NULLs in a unique index. + * + * @return \Illuminate\Validation\Rules\Unique + */ + protected function uniqueSkuRule() + { + $rule = Rule::unique('parts', 'sku') + ->where(function ($query) { + $query->where('company_uuid', session('company')); + + return $query->whereNull('deleted_at'); + }); + + // On PUT the record re-sends its own SKU, so it must not collide with + // itself. The v1 route parameter is a public_id (part_xxxxx), not a uuid. + $id = $this->route('id'); + if (!$this->isMethod('POST') && filled($id)) { + $rule->ignore($id, 'public_id'); + } + + return $rule; + } } diff --git a/server/src/Support/FuelProviders/FuelProviderService.php b/server/src/Support/FuelProviders/FuelProviderService.php index 031057915..93787c6e7 100644 --- a/server/src/Support/FuelProviders/FuelProviderService.php +++ b/server/src/Support/FuelProviders/FuelProviderService.php @@ -173,8 +173,17 @@ public function ingestTransaction(FuelProviderConnection $connection, array $pay return DB::transaction(function () use ($connection, $payload) { $provider = $payload['provider'] ?? $connection->provider; $providerTransactionId = $payload['provider_transaction_id']; + // The match must include company_uuid. Provider transaction ids are only + // unique within the account they were issued for, so keying on + // (provider, provider_transaction_id) alone let one company's sync find + // and overwrite another company's transaction — reassigning its + // company_uuid. Mirrors fuel_provider_txn_company_provider_unique. $transaction = FuelProviderTransaction::updateOrCreate( - ['provider' => $provider, 'provider_transaction_id' => $providerTransactionId], + [ + 'company_uuid' => $connection->company_uuid, + 'provider' => $provider, + 'provider_transaction_id' => $providerTransactionId, + ], array_merge($payload, [ 'company_uuid' => $connection->company_uuid, 'fuel_provider_connection_uuid' => $connection->uuid, diff --git a/server/tests/RequestContractsTest.php b/server/tests/RequestContractsTest.php index e0fcf72d2..db1f83d50 100644 --- a/server/tests/RequestContractsTest.php +++ b/server/tests/RequestContractsTest.php @@ -76,6 +76,18 @@ public function whereNull($column): self return $this; } + /** + * Recorded so tests can assert the ignore-self clause on update + * requests, including which column it matches on — the v1 routes + * pass a public_id, not a uuid. + */ + public function ignore($id, $idColumn = null): self + { + $this->constraints[] = ['ignore', $id, $idColumn]; + + return $this; + } + public function __toString(): string { return $this->rule; @@ -245,11 +257,16 @@ protected function canUpdateDriver(): bool }); test('fuel transaction request requires provider identifiers on create', function () { + // The harness `session()` stand-in is a process-wide static, so seed it + // here rather than relying on an earlier test in the file having done so. + session(['company' => 'company-uuid']); + bindFleetOpsRequestSession(['api_credential' => 'credential-uuid', 'company' => 'company-uuid']); + $createRules = requestRules(CreateFuelTransactionRequest::class); $patchRules = requestRules(CreateFuelTransactionRequest::class, 'PATCH'); expect(ruleStrings($createRules['provider']))->toContain('required', 'string') - ->and(ruleStrings($createRules['provider_transaction_id']))->toContain('required', 'string') + ->and(ruleStrings($createRules['provider_transaction_id']))->toContain('required', 'string', 'unique:fuel_provider_transactions,provider_transaction_id') ->and(ruleStrings($patchRules['provider']))->not->toContain('required') ->and(ruleStrings($patchRules['provider_transaction_id']))->not->toContain('required') ->and($createRules['station_latitude'])->toBe(['nullable', 'numeric']) @@ -257,6 +274,19 @@ protected function canUpdateDriver(): bool ->and($createRules['normalized_payload'])->toBe(['nullable', 'array']) ->and($createRules['raw_payload'])->toBe(['nullable', 'array']) ->and($createRules['meta'])->toBe(['nullable', 'array']); + + // Mirrors fuel_provider_txn_company_provider_unique. The provider is part + // of the scope, and company_uuid is what stops one tenant's provider + // transaction id from colliding with another's — the old index was global. + $rulesWithProvider = CreateFuelTransactionRequest::create('/fleetops-test', 'POST', ['provider' => 'petroapp'])->rules(); + $uniqueRule = collect($rulesWithProvider['provider_transaction_id'])->first(fn ($rule) => $rule instanceof Illuminate\Validation\Rule); + + expect($uniqueRule)->not->toBeNull() + ->and($uniqueRule->constraints)->toBe([ + ['where', 'company_uuid', 'company-uuid'], + ['where', 'provider', 'petroapp'], + ['whereNull', 'deleted_at'], + ]); }); test('vehicle and fuel report requests expose core validation contracts', function () { @@ -649,13 +679,14 @@ protected function canUpdateDriver(): bool expect($request->authorize())->toBeFalse(); - bindFleetOpsRequestSession(['api_credential' => 'credential-uuid']); + session(['company' => 'company-uuid']); + bindFleetOpsRequestSession(['api_credential' => 'credential-uuid', 'company' => 'company-uuid']); $createRules = $request->rules(); $patchRules = CreatePartRequest::create('/fleetops-test', 'PATCH')->rules(); expect($request->authorize())->toBeTrue() - ->and($createRules['sku'])->toBe(['nullable', 'string']) + ->and(ruleStrings($createRules['sku']))->toContain('nullable', 'string', 'unique:parts,sku') ->and(ruleStrings($createRules['name']))->toContain('required', 'string') ->and(ruleStrings($patchRules['name']))->not->toContain('required') ->and($createRules['manufacturer'])->toBe(['nullable', 'string']) @@ -671,6 +702,33 @@ protected function canUpdateDriver(): bool ->and($createRules['specs'])->toBe(['nullable', 'array']) ->and($createRules['meta'])->toBe(['nullable', 'array']); + // The SKU uniqueness rule mirrors parts_company_uuid_active_sku_unique: + // scoped to the session company and ignoring soft-deleted parts, so a SKU + // freed by a deleted part — or held by another company — stays available. + // Without it a duplicate reached the driver and returned a 500. + $skuRule = collect($createRules['sku'])->first(fn ($rule) => $rule instanceof Illuminate\Validation\Rule); + + expect($skuRule)->not->toBeNull() + ->and($skuRule->constraints)->toBe([ + ['where', 'company_uuid', 'company-uuid'], + ['whereNull', 'deleted_at'], + ]); + + // On update the record re-sends its own SKU, so it must not collide with + // itself. The v1 route parameter is a public_id, not a uuid. + $updateRules = CreatePartRequest::create('/fleetops-test', 'PUT') + ->setRouteResolver(fn () => new class { + public function parameter($key, $default = null) + { + return $key === 'id' ? 'part_abc123' : $default; + } + }) + ->rules(); + + $updateSkuRule = collect($updateRules['sku'])->first(fn ($rule) => $rule instanceof Illuminate\Validation\Rule); + + expect($updateSkuRule->constraints)->toContain(['ignore', 'part_abc123', 'public_id']); + bindFleetOpsRequestSession(['is_sanctum_token' => true]); expect($request->authorize())->toBeTrue(); From 8990a73cb44e8f7a9144e6542aa5bb3ba390dd16 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 12:57:59 +0800 Subject: [PATCH 08/28] v0.6.60 Co-Authored-By: Claude Opus 5 --- composer.json | 2 +- extension.json | 2 +- package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 95f0c6a28..14962f3fa 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "fleetbase/fleetops-api", - "version": "0.6.59", + "version": "0.6.60", "description": "Fleet & Transport Management Extension for Fleetbase", "keywords": [ "fleetbase-extension", diff --git a/extension.json b/extension.json index af9d83e6e..2e4c92307 100644 --- a/extension.json +++ b/extension.json @@ -1,6 +1,6 @@ { "name": "Fleet-Ops", - "version": "0.6.59", + "version": "0.6.60", "description": "Fleet & Transport Management Extension for Fleetbase", "repository": "https://github.com/fleetbase/fleetops", "license": "AGPL-3.0-or-later", diff --git a/package.json b/package.json index 8dd5aec4b..30ee09be9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@fleetbase/fleetops-engine", - "version": "0.6.59", + "version": "0.6.60", "description": "Fleet & Transport Management Extension for Fleetbase", "fleetbase": { "route": "fleet-ops" From 1838ab2b5fdef35ac5d850e2afb13f426ab945ee Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 12:58:00 +0800 Subject: [PATCH 09/28] ci: run the server, ember and Postman workflows on dev-v* release branches All three workflows filtered pull_request on `branches: [main]`, so a PR targeting a dev-v* release branch triggered no checks at all. Release work lands on the release branch first and only reaches main via the release PR, so without this every contributing PR merges unverified and the first real signal arrives after the fact, on the release PR itself. Add dev-v* to the push and pull_request filters. Co-Authored-By: Claude Opus 5 --- .github/workflows/ember.yml | 4 ++-- .github/workflows/postman.yml | 4 ++-- .github/workflows/server.yml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ember.yml b/.github/workflows/ember.yml index 7d1b52c34..df7d1fa1b 100644 --- a/.github/workflows/ember.yml +++ b/.github/workflows/ember.yml @@ -2,11 +2,11 @@ name: Ember.js CI on: push: - branches: [ main ] + branches: [ main, 'dev-v*' ] tags: - 'v*' pull_request: - branches: [ main ] + branches: [ main, 'dev-v*' ] env: NODE_VERSION: 22.x diff --git a/.github/workflows/postman.yml b/.github/workflows/postman.yml index 820bb0159..1ca2e6818 100644 --- a/.github/workflows/postman.yml +++ b/.github/workflows/postman.yml @@ -8,9 +8,9 @@ name: API Contract (Postman) on: push: - branches: [main] + branches: [main, 'dev-v*'] pull_request: - branches: [main] + branches: [main, 'dev-v*'] workflow_dispatch: permissions: diff --git a/.github/workflows/server.yml b/.github/workflows/server.yml index a47be23db..736f25296 100644 --- a/.github/workflows/server.yml +++ b/.github/workflows/server.yml @@ -2,11 +2,11 @@ name: PHP CI on: push: - branches: [ main ] + branches: [ main, 'dev-v*' ] tags: - 'v*' pull_request: - branches: [ main ] + branches: [ main, 'dev-v*' ] jobs: build: From a0de4f9d706ab82fa4bdb78f613a72694f41a833 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 13:07:44 +0800 Subject: [PATCH 10/28] test(customers): realign the CreateCustomerRequest source assertion CustomerEndpointTest matches against the literal source text of CreateCustomerRequest, so relaxing the `code` rule from `required|exists:verification_codes,code` to `required|string` failed the suite even though the behaviour under test was unchanged: Test Failed (CustomerEndpointTest::__pest_evaluable_FormRequest_validators _are_present_and_authorize_via_api_credential) Update the expected string and note the coupling, so the next rule change tells the reader why this file has to move with it. Co-Authored-By: Claude Opus 5 --- server/tests/CustomerEndpointTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/tests/CustomerEndpointTest.php b/server/tests/CustomerEndpointTest.php index ce9610526..38670211b 100644 --- a/server/tests/CustomerEndpointTest.php +++ b/server/tests/CustomerEndpointTest.php @@ -231,8 +231,11 @@ function fleetopsCustomerEndpointJson(JsonResponse $response): array $create = file_get_contents(dirname(__DIR__) . '/src/Http/Requests/CreateCustomerRequest.php'); $verify = file_get_contents(dirname(__DIR__) . '/src/Http/Requests/VerifyCreateCustomerRequest.php'); + // These match against source text, so they have to be updated whenever the rule + // strings change. `code` is presence-only by design — the controller's own check + // (code + for + meta->identity) is strictly stronger than exists:verification_codes. expect($create) - ->toContain("'code' => 'required|exists:verification_codes,code'") + ->toContain("'code' => 'required|string'") ->toContain("'password' => 'required|string|min:8'") ->and($verify)->toContain("'mode' => 'required|in:email,sms'"); }); From 2b8d0be8181e5e3e4a0a96d46ce4924144182c42 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 13:11:15 +0800 Subject: [PATCH 11/28] test(drivers): give the auth-flow harness a container that answers environment() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit verificationBypassMatches() asks app()->environment('production') — correctly, since a bypass code left set in a deployed .env must not work against a live fleet. The harness binds a bare Illuminate\Container\Container, which has no such method, so three tests errored: Call to undefined method Illuminate\Container\Container::environment() DriverControllerAuthFlowsTest::code_verification_handles_unknown_users_ invalid_codes_bypass_and_success DriverControllerAuthFlowsTest::code_verification_reports_token_issuance_failures DriverControllerAuthFlowsTest::token_persistence_failures_are_reported_to_sentry Swap in a container subclass exposing environment() and hasDebugModeEnabled(), copying the existing bindings across by reflection. This mirrors fleetopsCustomerHelperContainer() in CustomerControllerHelperSeamsTest rather than inventing a second approach. The swap has to run before the app()->instance() calls in the boot function, or those bindings land on the container being superseded. Co-Authored-By: Claude Opus 5 --- .../Api/DriverControllerAuthFlowsTest.php | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php b/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php index e917ac494..4c28df8d6 100644 --- a/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php +++ b/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php @@ -84,8 +84,59 @@ public function send($mailable) } } +/** + * Swap in a container that answers environment(). + * + * verificationBypassMatches() asks app()->environment('production'), which the bare + * Illuminate\Container\Container used by this harness does not implement. Mirrors + * fleetopsCustomerHelperContainer() in CustomerControllerHelperSeamsTest. + */ +function fleetopsDriverAuthContainer(): void +{ + $current = Illuminate\Container\Container::getInstance(); + if (method_exists($current, 'hasDebugModeEnabled')) { + return; + } + + $replacement = new class extends Illuminate\Container\Container { + public function environment(...$environments) + { + if (empty($environments)) { + return 'testing'; + } + + $checks = is_array($environments[0]) ? $environments[0] : $environments; + + return in_array('testing', $checks, true); + } + + public function hasDebugModeEnabled() + { + return true; + } + }; + + foreach (['bindings', 'instances', 'aliases', 'abstractAliases', 'resolved', 'extenders', 'tags', 'contextual', 'scopedInstances', 'reboundCallbacks', 'globalBeforeResolvingCallbacks', 'globalResolvingCallbacks', 'globalAfterResolvingCallbacks', 'beforeResolvingCallbacks', 'resolvingCallbacks', 'afterResolvingCallbacks'] as $property) { + if (!property_exists(Illuminate\Container\Container::class, $property)) { + continue; + } + $reflection = new ReflectionProperty(Illuminate\Container\Container::class, $property); + $reflection->setAccessible(true); + if ($reflection->isInitialized($current)) { + $reflection->setValue($replacement, $reflection->getValue($current)); + } + } + + Illuminate\Container\Container::setInstance($replacement); + Illuminate\Support\Facades\Facade::setFacadeApplication($replacement); +} + function fleetopsDriverAuthBoot(): SQLiteConnection { + // Must run before the app()->instance() calls below, so those bindings land on + // the replacement container rather than the one it supersedes. + fleetopsDriverAuthContainer(); + $connection = new SQLiteConnection(new PDO('sqlite::memory:')); $resolver = new ConnectionResolver(['default' => $connection, 'mysql' => $connection]); $resolver->setDefaultConnection('mysql'); From 73f0761d3586dccee62029e605419f901254e6e1 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 13:16:18 +0800 Subject: [PATCH 12/28] test(requests): cover the duplicate-key messages and provider fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 100% line-coverage gate failed at 99.96%: 84.21% 48/57 server/src/Http/Requests/CreateFuelTransactionRequest.php 91.43% 32/35 server/src/Http/Requests/CreatePartRequest.php A coverage slice put the gap on exactly three things — both messages() overrides, the fuel request's ignore-self clause, and resolveProvider()'s stored-provider fallback. RequestContractsTest gains the messages() assertions and the fuel ignore-self case. It supplies `provider` in the body so resolveProvider() short-circuits on the input, keeping that file free of a database. The fallback itself does query, so it is covered in FuelProviderTransactionControllerContractsTest, whose fixture already has a fuel_provider_transactions table and a seeded petroapp row. Asserting it matters: a null scope would compare the transaction id against the wrong set of rows, which is the failure the unique rule exists to prevent. Verified by slice — CreatePartRequest is now fully covered, and CreateFuelTransactionRequest retains only authorize() (lines 11 and 13), which is exercised by another file and is outside the 9 lines CI counted. Co-Authored-By: Claude Opus 5 --- ...iderTransactionControllerContractsTest.php | 17 +++++++++++ server/tests/RequestContractsTest.php | 28 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/server/tests/FuelProviderTransactionControllerContractsTest.php b/server/tests/FuelProviderTransactionControllerContractsTest.php index 800da1ce7..fdd1e25a7 100644 --- a/server/tests/FuelProviderTransactionControllerContractsTest.php +++ b/server/tests/FuelProviderTransactionControllerContractsTest.php @@ -178,4 +178,21 @@ function fleetopsFuelProviderTransactionController(FleetOpsFuelProviderTransacti expect($reflection->invoke($controller, 'fpt-real-1')->public_id)->toBe('fuel_provider_transaction_real1') ->and($reflection->invoke($controller, 'fuel_provider_transaction_real1')->uuid)->toBe('fpt-real-1'); + + // resolveProvider() falls back to the stored provider when a PUT omits it, so the + // uniqueness scope is never null — a null scope would compare the transaction id + // against the wrong set of rows. Needs a real connection, which is why this lives + // here rather than in RequestContractsTest. + $resolveProvider = new ReflectionMethod(Fleetbase\FleetOps\Http\Requests\CreateFuelTransactionRequest::class, 'resolveProvider'); + $resolveProvider->setAccessible(true); + + $withRouteId = Fleetbase\FleetOps\Http\Requests\CreateFuelTransactionRequest::create('/fleetops-test', 'PUT') + ->setRouteResolver(fn () => new class { + public function parameter($key, $default = null) + { + return $key === 'id' ? 'fuel_provider_transaction_real1' : $default; + } + }); + + expect($resolveProvider->invoke($withRouteId))->toBe('petroapp'); }); diff --git a/server/tests/RequestContractsTest.php b/server/tests/RequestContractsTest.php index db1f83d50..e3d9db0da 100644 --- a/server/tests/RequestContractsTest.php +++ b/server/tests/RequestContractsTest.php @@ -734,6 +734,34 @@ public function parameter($key, $default = null) expect($request->authorize())->toBeTrue(); }); + test('duplicate-key requests name the offending field and ignore themselves on update', function () { + session(['company' => 'company-uuid']); + bindFleetOpsRequestSession(['api_credential' => 'credential-uuid', 'company' => 'company-uuid']); + + // Without these the violation surfaces as the framework's generic "has already + // been taken", which does not say which record collided or on what. + expect(CreatePartRequest::create('/fleetops-test', 'POST')->messages()) + ->toBe(['sku.unique' => 'A part with this SKU already exists.']) + ->and(CreateFuelTransactionRequest::create('/fleetops-test', 'POST')->messages()) + ->toBe(['provider_transaction_id.unique' => 'A fuel transaction with this provider transaction id already exists for this provider.']); + + // `provider` is supplied here, so resolveProvider() short-circuits on the input + // and this exercises only the ignore-self clause. The stored-provider fallback + // needs a database and is covered in FuelProviderTransactionControllerContractsTest. + $updateRules = CreateFuelTransactionRequest::create('/fleetops-test', 'PUT', ['provider' => 'petroapp']) + ->setRouteResolver(fn () => new class { + public function parameter($key, $default = null) + { + return $key === 'id' ? 'fuel_provider_transaction_abc123' : $default; + } + }) + ->rules(); + + $updateRule = collect($updateRules['provider_transaction_id'])->first(fn ($rule) => $rule instanceof Illuminate\Validation\Rule); + + expect($updateRule->constraints)->toContain(['ignore', 'fuel_provider_transaction_abc123', 'public_id']); + }); + test('entity equipment payload and simulation requests expose conditional contracts', function () { bindFleetOpsRequestSession(); From ec47bda592e67df7c981d0046c485f570a72f68f Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 13:18:52 +0800 Subject: [PATCH 13/28] test(drivers): extend the environment() container swap to the internal harness The API-side fix left the internal verify-code path failing the same way, since Internal\v1\DriverController now defers to the shared DriverController::verificationBypassMatches() and therefore also reaches app()->environment('production'): Call to undefined method Illuminate\Container\Container::environment() DriverControllerContractsTest::internal_driver_controller_verify_code_covers_ missing_user_invalid_code_and_missing_driver_branches DriverControllerContractsTest::internal_driver_controller_verify_code_returns_ driver_resource_and_handles_token_errors Hook the swap into FleetOpsInternalDriverAuthControllerProbe::resetProbe(), which both tests already call, rather than repeating it per test. The helper short-circuits when the container is already swapped, so calling it per test costs nothing. Verified locally: both this file and Api/DriverControllerAuthFlowsTest pass. Co-Authored-By: Claude Opus 5 --- .../DriverControllerContractsTest.php | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/server/tests/Feature/Http/Internal/DriverControllerContractsTest.php b/server/tests/Feature/Http/Internal/DriverControllerContractsTest.php index da993ef18..faf17ba02 100644 --- a/server/tests/Feature/Http/Internal/DriverControllerContractsTest.php +++ b/server/tests/Feature/Http/Internal/DriverControllerContractsTest.php @@ -307,6 +307,11 @@ public function __construct() public static function resetProbe(): void { + // verifyCode() defers to DriverController::verificationBypassMatches(), which + // asks app()->environment('production'). The bare container this harness binds + // has no such method, so swap it before any verify-code test runs. + fleetopsInternalDriverAuthContainer(); + static::$loginUser = null; static::$verificationUser = null; static::$loginDriver = null; @@ -531,6 +536,53 @@ function fleetopsInternalDriverExportSelections(DriverExport $export): array return $property->getValue($export); } +/** + * Swap in a container that answers environment(). + * + * Mirrors fleetopsCustomerHelperContainer() in CustomerControllerHelperSeamsTest and + * fleetopsDriverAuthContainer() in Api/DriverControllerAuthFlowsTest. Idempotent, so + * resetProbe() can call it per test. + */ +function fleetopsInternalDriverAuthContainer(): void +{ + $current = Illuminate\Container\Container::getInstance(); + if (method_exists($current, 'hasDebugModeEnabled')) { + return; + } + + $replacement = new class extends Illuminate\Container\Container { + public function environment(...$environments) + { + if (empty($environments)) { + return 'testing'; + } + + $checks = is_array($environments[0]) ? $environments[0] : $environments; + + return in_array('testing', $checks, true); + } + + public function hasDebugModeEnabled() + { + return true; + } + }; + + foreach (['bindings', 'instances', 'aliases', 'abstractAliases', 'resolved', 'extenders', 'tags', 'contextual', 'scopedInstances', 'reboundCallbacks', 'globalBeforeResolvingCallbacks', 'globalResolvingCallbacks', 'globalAfterResolvingCallbacks', 'beforeResolvingCallbacks', 'resolvingCallbacks', 'afterResolvingCallbacks'] as $property) { + if (!property_exists(Illuminate\Container\Container::class, $property)) { + continue; + } + $reflection = new ReflectionProperty(Illuminate\Container\Container::class, $property); + $reflection->setAccessible(true); + if ($reflection->isInitialized($current)) { + $reflection->setValue($replacement, $reflection->getValue($current)); + } + } + + Illuminate\Container\Container::setInstance($replacement); + Illuminate\Support\Facades\Facade::setFacadeApplication($replacement); +} + function fleetopsInternalDriverUseHelperDatabase(): SQLiteConnection { $connection = new SQLiteConnection(new PDO('sqlite::memory:')); From 1c7085b4675f3a717a0f01ebad210353d304b551 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 13:30:54 +0800 Subject: [PATCH 14/28] test(navigator): cover the real errorResponse seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codecov failed on this branch alone. The 404 assertion added in SmallControllerContractsTest goes through FleetOpsPublicNavigatorControllerProbe, which overrides errorResponse() to keep the fixture small — so the real one-liner never executed and the new seam was the only uncovered code in the file. The other seams in this controller are covered by the SQLite-backed navigator test in GeofenceDwellAndBulkNotifyTest, which drives the real controller, so the missing-company case belongs there too. Only the status is asserted. The harness `response()` shim envelopes errors as {"error": ...} while the core-api macro it stands in for produces {"errors": [...]}; the status code is the part that holds in both, and the payload shape is already pinned by the probe-based test. Verified by slice: NavigatorController has no uncovered lines. Co-Authored-By: Claude Opus 5 --- server/tests/Unit/Jobs/GeofenceDwellAndBulkNotifyTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/tests/Unit/Jobs/GeofenceDwellAndBulkNotifyTest.php b/server/tests/Unit/Jobs/GeofenceDwellAndBulkNotifyTest.php index 2e9ba20c3..671074648 100644 --- a/server/tests/Unit/Jobs/GeofenceDwellAndBulkNotifyTest.php +++ b/server/tests/Unit/Jobs/GeofenceDwellAndBulkNotifyTest.php @@ -180,4 +180,11 @@ public function __call($method, $arguments) $connection->table('companies')->insert(['uuid' => 'company-2', 'public_id' => 'company_two', 'name' => 'Beta']); $empty = (new NavigatorController())->getDriverOnboardSettings('company_two'); expect($empty->getData(true)['driverOnboardSettings'])->toBe([]); + + // An unresolved public id is a client error. This drives the real + // errorResponse() seam, which the probe in SmallControllerContractsTest + // replaces — only the status is asserted, because the harness `response()` + // shim envelopes errors differently from the core-api macro it stands in for. + $missing = (new NavigatorController())->getDriverOnboardSettings('company_missing'); + expect($missing->getStatusCode())->toBe(404); }); From 6f3b4b03c7b56bece391350b766a7f47af747833 Mon Sep 17 00:00:00 2001 From: Ron Date: Mon, 10 Aug 2026 14:57:30 +0800 Subject: [PATCH 15/28] ci(postman): test this branch's API code, not the published package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With build-from-source: false the stack boots the published API image, and this package is a composer dependency baked into it — so a PR here booted the released version and ran the collections against that. Its own API changes were never exercised; the check was green on code that was not under review. overlay-package makes the reusable workflow check this repository out at the commit under test and swap it into the running container, dumping the autoloader (the image is built with --optimize-autoloader, so a frozen classmap would otherwise hide classes added or moved on the branch), clearing caches and running migrations. Co-Authored-By: Claude Opus 5 --- .github/workflows/postman.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/postman.yml b/.github/workflows/postman.yml index 98a65ce61..d69e4ed56 100644 --- a/.github/workflows/postman.yml +++ b/.github/workflows/postman.yml @@ -30,4 +30,8 @@ jobs: with: collections: "Fleetbase API" build-from-source: false + # Without this the run tests the version of fleetbase/fleetops-api baked into the + # published image, not the branch under review. The workflow checks this + # repository out at the commit under test and swaps it into the container. + overlay-package: fleetbase/fleetops-api secrets: inherit From 2ce6b76a17089d1321a7a05a5bf86c63d93a03bf Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 18:36:12 +0800 Subject: [PATCH 16/28] fix(auth): scope the verification bypass to designated review accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bypass code was compared against the submitted code alone, so anyone who learned it could authenticate as ANY driver or customer. The mitigation in place — refusing the bypass outside production — closed that hole but broke the thing the bypass exists for: an app store reviewer tests a release build against production, which is exactly where it was refused. Scopes the bypass to explicitly designated accounts instead. A code is only honoured for an identity listed in the new review_accounts config, so it keeps working for reviewers in production while a leaked code authenticates nobody. NAVIGATOR_REVIEW_ACCOUNTS=+15555550100,apple-review@example.com FLEETOPS_CUSTOMER_REVIEW_ACCOUNTS=... Both the code and the allowlist are required and neither has a default, so an unconfigured install has no bypass at all. Comparison stays constant-time, and each accepted bypass is logged with its identity so use is auditable. Extracted into a trait rather than shared between the two controllers directly: they are siblings, so a protected static on one is not callable from the other — a fatal that php -l does not catch. The reset-password path compares the normalised identity ($needle), matching how the other verify paths normalise in place, so an allowlisted phone number is compared in the same form it is stored. Verified against the real trait: a listed account with the right code passes; an unlisted account with the right code is refused; wrong code, absent code, empty allowlist and null identity all refuse; matching is case- and whitespace- insensitive; and a configured code of "0" is honoured rather than dropped. Co-Authored-By: Claude Opus 5 --- server/config/fleetops.php | 22 +++++++ .../Controllers/Api/v1/CustomerController.php | 25 +++++--- .../Controllers/Api/v1/DriverController.php | 19 +++--- .../Concerns/ResolvesReviewAccountBypass.php | 62 +++++++++++++++++++ .../Internal/v1/DriverController.php | 2 +- 5 files changed, 111 insertions(+), 19 deletions(-) create mode 100644 server/src/Http/Controllers/Concerns/ResolvesReviewAccountBypass.php diff --git a/server/config/fleetops.php b/server/config/fleetops.php index b5bb41357..2ae3d6c09 100644 --- a/server/config/fleetops.php +++ b/server/config/fleetops.php @@ -96,7 +96,24 @@ |-------------------------------------------------------------------------- */ 'navigator' => [ + /* + | App store reviewers cannot receive our SMS, so a fixed verification code has + | to keep working for them — including in production, which is where a review + | build is tested. Blocking the bypass outside production made review + | impossible; restricting it to named accounts makes it safe instead. + | + | Both values are required, and neither has a default. The code alone is not + | sufficient: it is only accepted for an identity listed in review_accounts, so + | a leaked code cannot be used to authenticate as an arbitrary driver. + | + | NAVIGATOR_BYPASS_VERIFICATION_CODE= + | NAVIGATOR_REVIEW_ACCOUNTS=+15555550100,apple-review@example.com + */ 'bypass_verification_code' => env('SMS_AUTH_BYPASS_CODE', env('NAVIGATOR_BYPASS_VERIFICATION_CODE')), + 'review_accounts' => array_values(array_filter(array_map( + 'trim', + explode(',', (string) env('NAVIGATOR_REVIEW_ACCOUNTS', '')) + ))), 'app_identifier' => env('NAVIGATOR_APP_IDENTIFIER', 'io.fleetbase.navigator'), ], @@ -120,6 +137,11 @@ */ 'customers' => [ 'verification_bypass_code' => env('FLEETOPS_CUSTOMER_VERIFICATION_BYPASS_CODE'), + // Identities the bypass code is accepted for. See the note on navigator below. + 'review_accounts' => array_values(array_filter(array_map( + 'trim', + explode(',', (string) env('FLEETOPS_CUSTOMER_REVIEW_ACCOUNTS', '')) + ))), ], /* diff --git a/server/src/Http/Controllers/Api/v1/CustomerController.php b/server/src/Http/Controllers/Api/v1/CustomerController.php index 6fbd4fce0..6a37525a6 100644 --- a/server/src/Http/Controllers/Api/v1/CustomerController.php +++ b/server/src/Http/Controllers/Api/v1/CustomerController.php @@ -42,6 +42,8 @@ */ class CustomerController extends Controller { + use \Fleetbase\FleetOps\Http\Controllers\Concerns\ResolvesReviewAccountBypass; + /* ============================================================ | Public auth flows (API credential only, no Customer-Token) * ============================================================ */ @@ -145,7 +147,7 @@ public function create(CreateCustomerRequest $request) 'for' => 'fleetops_create_customer', 'meta->identity' => $identity, ]); - if (!$verificationCode && !$this->verificationBypassMatches($code)) { + if (!$verificationCode && !$this->verificationBypassMatches($identity, $code)) { return response()->apiError('Invalid verification code provided.'); } @@ -426,7 +428,7 @@ public function verifyCode(Request $request) 'code' => $code, 'for' => $for, ]); - if (!$verificationCode && !$this->verificationBypassMatches($code)) { + if (!$verificationCode && !$this->verificationBypassMatches($identity, $code)) { return response()->apiError('Invalid verification code.'); } @@ -512,7 +514,9 @@ public function resetPassword(Request $request) 'for' => 'fleetops_customer_password_reset', 'meta->identity' => $needle, ]); - if (!$verificationCode && !$this->verificationBypassMatches($code)) { + // $needle, not $identity: the other verify paths normalise in place, so an + // allowlisted phone must be compared in the same normalised form here too. + if (!$verificationCode && !$this->verificationBypassMatches($needle, $code)) { return response()->apiError('Invalid reset code.'); } @@ -949,14 +953,15 @@ protected function generateSmsVerification(User $user, string $for, array $optio * those two are test seams that the controller contract tests override, so a * policy living inside them would be stubbed away exactly where it matters. */ - protected function verificationBypassMatches(?string $code): bool + protected function verificationBypassMatches(?string $identity, ?string $code): bool { - $bypassCode = config('fleetops.customers.verification_bypass_code'); - - return $bypassCode !== null - && $bypassCode !== '' - && !app()->environment('production') - && hash_equals((string) $bypassCode, (string) $code); + return static::reviewAccountBypassMatches( + 'fleetops.customers.verification_bypass_code', + 'fleetops.customers.review_accounts', + $identity, + $code, + 'fleetops-customer' + ); } protected function verificationCodeExists(array $attributes): bool diff --git a/server/src/Http/Controllers/Api/v1/DriverController.php b/server/src/Http/Controllers/Api/v1/DriverController.php index d619fa9a5..f109ca0ac 100644 --- a/server/src/Http/Controllers/Api/v1/DriverController.php +++ b/server/src/Http/Controllers/Api/v1/DriverController.php @@ -39,6 +39,8 @@ class DriverController extends Controller { + use \Fleetbase\FleetOps\Http\Controllers\Concerns\ResolvesReviewAccountBypass; + /** * Creates a new Fleetbase Driver resource. * @@ -611,7 +613,7 @@ public function verifyCode(Request $request) // find and verify code $verificationCode = VerificationCode::where(['subject_uuid' => $user->uuid, 'code' => $code, 'for' => $for])->exists(); - if (!$verificationCode && !static::verificationBypassMatches($code)) { + if (!$verificationCode && !static::verificationBypassMatches($identity, $code)) { return response()->apiError('Invalid verification code!'); } @@ -1054,14 +1056,15 @@ private static function getDriverCompanyFromUser(User $user): ?Company * Shared with Internal\v1\DriverController so both verify-code paths cannot * drift apart. */ - public static function verificationBypassMatches(?string $code): bool + public static function verificationBypassMatches(?string $identity, ?string $code): bool { - $bypassCode = config('fleetops.navigator.bypass_verification_code'); - - return $bypassCode !== null - && $bypassCode !== '' - && !app()->environment('production') - && hash_equals((string) $bypassCode, (string) $code); + return static::reviewAccountBypassMatches( + 'fleetops.navigator.bypass_verification_code', + 'fleetops.navigator.review_accounts', + $identity, + $code, + 'navigator' + ); } /** diff --git a/server/src/Http/Controllers/Concerns/ResolvesReviewAccountBypass.php b/server/src/Http/Controllers/Concerns/ResolvesReviewAccountBypass.php new file mode 100644 index 000000000..4f8596e9a --- /dev/null +++ b/server/src/Http/Controllers/Concerns/ResolvesReviewAccountBypass.php @@ -0,0 +1,62 @@ + strtolower(trim((string) $account)), + (array) config($accountsKey, []) + ); + + if (!in_array(strtolower(trim($identity)), $accounts, true)) { + return false; + } + + // Constant-time, so a wrong code cannot be recovered by timing the response. + if (!hash_equals((string) $bypassCode, (string) $code)) { + return false; + } + + Log::warning('[Fleetbase] Verification bypass accepted for a review account.', [ + 'channel' => $channel, + 'identity' => $identity, + ]); + + return true; + } +} diff --git a/server/src/Http/Controllers/Internal/v1/DriverController.php b/server/src/Http/Controllers/Internal/v1/DriverController.php index ecdd4006f..de272c6c1 100644 --- a/server/src/Http/Controllers/Internal/v1/DriverController.php +++ b/server/src/Http/Controllers/Internal/v1/DriverController.php @@ -665,7 +665,7 @@ public function verifyCode(Request $request) // Find and verify code $verificationCode = static::verificationCodeExists($user, $code, $for); - if (!$verificationCode && !ApiDriverController::verificationBypassMatches($code)) { + if (!$verificationCode && !ApiDriverController::verificationBypassMatches($identity, $code)) { return response()->error('Invalid verification code!'); } From 4a952660b14f460f46da21ed65c460e4018b461e Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 19:02:04 +0800 Subject: [PATCH 17/28] test(auth): align the bypass contracts with the review-account policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suites encoded the policy the previous commit replaced — that a configured bypass works outside production and is refused inside it. With the bypass now scoped to designated accounts, those assertions no longer describe the code. - "accepts a configured verification bypass outside production" becomes "accepts a configured verification bypass for a listed review account", and deliberately runs in production. That is the behavioural change: a reviewer tests a release build against production, so refusing it there made review impossible. Safety comes from the allowlist now, not the environment. - "refuses the verification bypass in production" becomes "refuses the verification bypass for an identity that is not listed", covering both an allowlist naming someone else and an empty one. This is the property that matters: holding the code is no longer sufficient. The driver suites configured only a bypass code and expected it to authenticate, so each now also lists the identity under test. Both the raw and phone-normalised forms are listed for the internal suite, since verifyCode passes a non-email identity through static::phone() before the check. Co-Authored-By: Claude Opus 5 --- .../ApiCustomerControllerContractsTest.php | 67 +++++++++++-------- .../Api/DriverControllerAuthFlowsTest.php | 3 + .../DriverControllerContractsTest.php | 6 ++ 3 files changed, 49 insertions(+), 27 deletions(-) diff --git a/server/tests/ApiCustomerControllerContractsTest.php b/server/tests/ApiCustomerControllerContractsTest.php index f25e60959..ea1b7202b 100644 --- a/server/tests/ApiCustomerControllerContractsTest.php +++ b/server/tests/ApiCustomerControllerContractsTest.php @@ -1046,10 +1046,16 @@ public function hasDebugModeEnabled() config(['fleetops.customers.verification_bypass_code' => null]); }); -test('api customer controller accepts a configured verification bypass outside production', function () { - config(['fleetops.customers.verification_bypass_code' => '000000']); - - fleetopsApiCustomerWithEnvironment('local', function () { +test('api customer controller accepts a configured verification bypass for a listed review account', function () { + config([ + 'fleetops.customers.verification_bypass_code' => '000000', + 'fleetops.customers.review_accounts' => ['jane@example.test'], + ]); + + // Deliberately production. App store reviewers test a release build against + // production, so refusing the bypass there made review impossible — that is the + // behaviour this replaces. Safety now comes from the allowlist, not the environment. + fleetopsApiCustomerWithEnvironment('production', function () { $create = fleetopsApiCustomerController(); $create->verificationExists = false; $verify = fleetopsApiCustomerController(); @@ -1084,34 +1090,41 @@ public function hasDebugModeEnabled() ]))))->toBe(['error' => 'Invalid verification code provided.']); }); - config(['fleetops.customers.verification_bypass_code' => null]); + config(['fleetops.customers.verification_bypass_code' => null, 'fleetops.customers.review_accounts' => []]); }); -test('api customer controller refuses the verification bypass in production', function () { - config(['fleetops.customers.verification_bypass_code' => '000000']); +test('api customer controller refuses the verification bypass for an identity that is not listed', function () { + // The point of the allowlist: holding the code is not sufficient. Previously + // anyone who learned it could authenticate as any customer. + foreach ([['someone-else@example.test'], []] as $reviewAccounts) { + config([ + 'fleetops.customers.verification_bypass_code' => '000000', + 'fleetops.customers.review_accounts' => $reviewAccounts, + ]); - fleetopsApiCustomerWithEnvironment('production', function () { - $create = fleetopsApiCustomerController(); - $create->verificationExists = false; - $verify = fleetopsApiCustomerController(); - $verify->verificationExists = false; - $reset = fleetopsApiCustomerController(); - $reset->verificationCode = null; + fleetopsApiCustomerWithEnvironment('local', function () { + $create = fleetopsApiCustomerController(); + $create->verificationExists = false; + $verify = fleetopsApiCustomerController(); + $verify->verificationExists = false; + $reset = fleetopsApiCustomerController(); + $reset->verificationCode = null; - expect(fleetopsApiCustomerJson($create->create(new CreateCustomerRequest([ - 'code' => '000000', - 'identity' => 'jane@example.test', - ]))))->toBe(['error' => 'Invalid verification code provided.']) - ->and(fleetopsApiCustomerJson($verify->verifyCode(Request::create('/v1/customers/verify-code', 'POST', [ - 'identity' => 'jane@example.test', + expect(fleetopsApiCustomerJson($create->create(new CreateCustomerRequest([ 'code' => '000000', - ]))))->toBe(['error' => 'Invalid verification code.']) - ->and(fleetopsApiCustomerJson($reset->resetPassword(Request::create('/v1/customers/reset-password', 'POST', [ 'identity' => 'jane@example.test', - 'code' => '000000', - 'password' => 'password-secret', - ]))))->toBe(['error' => 'Invalid reset code.']); - }); + ]))))->toBe(['error' => 'Invalid verification code provided.']) + ->and(fleetopsApiCustomerJson($verify->verifyCode(Request::create('/v1/customers/verify-code', 'POST', [ + 'identity' => 'jane@example.test', + 'code' => '000000', + ]))))->toBe(['error' => 'Invalid verification code.']) + ->and(fleetopsApiCustomerJson($reset->resetPassword(Request::create('/v1/customers/reset-password', 'POST', [ + 'identity' => 'jane@example.test', + 'code' => '000000', + 'password' => 'password-secret', + ]))))->toBe(['error' => 'Invalid reset code.']); + }); + } - config(['fleetops.customers.verification_bypass_code' => null]); + config(['fleetops.customers.verification_bypass_code' => null, 'fleetops.customers.review_accounts' => []]); }); diff --git a/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php b/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php index 4c28df8d6..0fe634ee2 100644 --- a/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php +++ b/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php @@ -282,6 +282,7 @@ public function __call($method, $arguments) // Bypass code from config authenticates without a stored code config()->set('fleetops.navigator.bypass_verification_code', '777777'); + config()->set('fleetops.navigator.review_accounts', ['driver@example.test']); $bypassed = $controller->verifyCode(Request::create('/x', 'POST', ['identity' => 'driver@example.test', 'code' => '777777'])); expect($bypassed)->toBeInstanceOf(DriverResource::class); @@ -397,6 +398,7 @@ public function create(Request $request) // A bypass code authenticates without a stored verification code, so the // flow reaches token issuance; without the token table that send fails config()->set('fleetops.navigator.bypass_verification_code', '777777'); + config()->set('fleetops.navigator.review_accounts', ['driver@example.test']); $connection->getSchemaBuilder()->drop('personal_access_tokens'); $response = $controller->verifyCode(Request::create('/x', 'POST', [ @@ -451,6 +453,7 @@ public function __call($method, $arguments) } config()->set('fleetops.navigator.bypass_verification_code', '777777'); + config()->set('fleetops.navigator.review_accounts', ['driver@example.test']); $response = $controller->verifyCode(Request::create('/x', 'POST', [ 'identity' => 'driver@example.test', 'code' => '777777', diff --git a/server/tests/Feature/Http/Internal/DriverControllerContractsTest.php b/server/tests/Feature/Http/Internal/DriverControllerContractsTest.php index faf17ba02..0d9cc86da 100644 --- a/server/tests/Feature/Http/Internal/DriverControllerContractsTest.php +++ b/server/tests/Feature/Http/Internal/DriverControllerContractsTest.php @@ -1045,6 +1045,9 @@ public function getCurrentOrder(): ?Order test('internal driver controller verify code covers missing user invalid code and missing driver branches', function () { app('config')->set('fleetops.navigator.bypass_verification_code', '000000'); + // The bypass is only honoured for a listed identity. Both forms because + // verifyCode normalises a non-email identity through static::phone(). + app('config')->set('fleetops.navigator.review_accounts', ['15551234567', '+15551234567', 'driver@example.test']); FleetOpsInternalDriverAuthControllerProbe::resetProbe(); $controller = new FleetOpsInternalDriverAuthControllerProbe(); @@ -1087,6 +1090,9 @@ public function getCurrentOrder(): ?Order test('internal driver controller verify code returns driver resource and handles token errors', function () { app('config')->set('fleetops.navigator.bypass_verification_code', '000000'); + // The bypass is only honoured for a listed identity. Both forms because + // verifyCode normalises a non-email identity through static::phone(). + app('config')->set('fleetops.navigator.review_accounts', ['15551234567', '+15551234567', 'driver@example.test']); FleetOpsInternalDriverAuthControllerProbe::resetProbe(); FleetOpsInternalDriverAuthControllerProbe::$verificationUser = fleetopsInternalDriverAuthUser(); From 2b98ef4257189bdce20d84236a3bab530f738978 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 21:13:18 +0800 Subject: [PATCH 18/28] fix(maintenance): restore vehicle schedule workflows --- .../components/vehicle/details/schedules.hbs | 4 +- addon/components/vehicle/details/schedules.js | 8 ++- .../maintenance/schedules/index/edit.js | 11 ++- addon/services/vehicle-actions.js | 6 +- .../Http/Filter/MaintenanceScheduleFilter.php | 40 +++++++++++ .../Filter/MaintenanceScheduleFilterTest.php | 69 +++++++++++++++++++ .../vehicle/details/schedules-test.js | 59 ++++++++++++++++ .../maintenance/schedules/index/edit-test.js | 54 +++++++++++++++ tests/unit/services/vehicle-actions-test.js | 31 +++++++++ 9 files changed, 276 insertions(+), 6 deletions(-) create mode 100644 server/src/Http/Filter/MaintenanceScheduleFilter.php create mode 100644 server/tests/Unit/Http/Filter/MaintenanceScheduleFilterTest.php create mode 100644 tests/unit/components/vehicle/details/schedules-test.js create mode 100644 tests/unit/controllers/maintenance/schedules/index/edit-test.js diff --git a/addon/components/vehicle/details/schedules.hbs b/addon/components/vehicle/details/schedules.hbs index 5fc3f0b4e..859a72873 100644 --- a/addon/components/vehicle/details/schedules.hbs +++ b/addon/components/vehicle/details/schedules.hbs @@ -33,8 +33,8 @@

No maintenance schedules for this vehicle.

-
{{/if}} {{/if}} - \ No newline at end of file + diff --git a/addon/components/vehicle/details/schedules.js b/addon/components/vehicle/details/schedules.js index 40047565b..1531e2b94 100644 --- a/addon/components/vehicle/details/schedules.js +++ b/addon/components/vehicle/details/schedules.js @@ -1,12 +1,14 @@ import Component from '@glimmer/component'; import { tracked } from '@glimmer/tracking'; import { inject as service } from '@ember/service'; +import { action } from '@ember/object'; import { task } from 'ember-concurrency'; export default class VehicleDetailsSchedulesComponent extends Component { @service maintenanceScheduleActions; @service notifications; @service store; + @service vehicleActions; @tracked schedules = []; get resourceId() { @@ -22,11 +24,15 @@ export default class VehicleDetailsSchedulesComponent extends Component { try { this.schedules = yield this.store.query('maintenance-schedule', { subject_uuid: this.resourceId, - subject_type: 'vehicle', + subject_type: 'fleet-ops:vehicle', sort: '-created_at', }); } catch (err) { this.notifications.serverError(err); } } + + @action createSchedule() { + return this.vehicleActions.scheduleMaintenance(this.args.vehicle, {}, { refresh: false, callback: () => this.loadSchedules.perform() }); + } } diff --git a/addon/controllers/maintenance/schedules/index/edit.js b/addon/controllers/maintenance/schedules/index/edit.js index a49e61e9b..7fe1396de 100644 --- a/addon/controllers/maintenance/schedules/index/edit.js +++ b/addon/controllers/maintenance/schedules/index/edit.js @@ -21,12 +21,21 @@ export default class MaintenanceSchedulesIndexEditController extends Controller this.overlay?.close(); yield this.hostRouter.refresh(); yield this.hostRouter.transitionTo('console.fleet-ops.maintenance.schedules.index.details', schedule); - this.notifications.success(this.intl.t('common.resource-updated-success', { resource: this.intl.t('resource.maintenance-schedule') })); + this.notifyUpdateSuccess(schedule); } catch (err) { this.notifications.serverError(err); } } + notifyUpdateSuccess(schedule) { + this.notifications.success( + this.intl.t('common.resource-updated-success', { + resource: this.intl.t('resource.maintenance-schedule'), + resourceName: schedule.name, + }) + ); + } + @action cancel() { return this.hostRouter.transitionTo('console.fleet-ops.maintenance.schedules.index'); } diff --git a/addon/services/vehicle-actions.js b/addon/services/vehicle-actions.js index 5d0922c57..6cc0fbe92 100644 --- a/addon/services/vehicle-actions.js +++ b/addon/services/vehicle-actions.js @@ -184,8 +184,10 @@ export default class VehicleActionsService extends ResourceActionService { }, }; - @action scheduleMaintenance(vehicle) { - this.maintenanceScheduleActions.modal.create({ subject: vehicle }); + @action async scheduleMaintenance(vehicle, options = {}, saveOptions = {}) { + vehicle = await this.resolveVehicleResource(vehicle); + + return this.maintenanceScheduleActions.modal.create({ subject: vehicle }, options, saveOptions); } @action createWorkOrder(vehicle) { diff --git a/server/src/Http/Filter/MaintenanceScheduleFilter.php b/server/src/Http/Filter/MaintenanceScheduleFilter.php new file mode 100644 index 000000000..c18f370a4 --- /dev/null +++ b/server/src/Http/Filter/MaintenanceScheduleFilter.php @@ -0,0 +1,40 @@ +builder->where('company_uuid', $this->session->get('company')); + } + + public function queryForPublic(): void + { + $this->queryForInternal(); + } + + public function subjectType(?string $subjectType): void + { + if (!$subjectType) { + return; + } + + if (!Str::contains($subjectType, ['\\', ':'])) { + $subjectType = 'fleet-ops:' . $subjectType; + } + + $this->builder->where('subject_type', Utils::getMutationType($subjectType)); + } + + public function subjectUuid(?string $subjectUuid): void + { + if ($subjectUuid) { + $this->builder->where('subject_uuid', $subjectUuid); + } + } +} diff --git a/server/tests/Unit/Http/Filter/MaintenanceScheduleFilterTest.php b/server/tests/Unit/Http/Filter/MaintenanceScheduleFilterTest.php new file mode 100644 index 000000000..71d977921 --- /dev/null +++ b/server/tests/Unit/Http/Filter/MaintenanceScheduleFilterTest.php @@ -0,0 +1,69 @@ +wheres[] = $arguments; + + return $this; + } +} + +function fleetopsMaintenanceScheduleFilter(FleetOpsMaintenanceScheduleFilterQuery $builder): MaintenanceScheduleFilter +{ + $filter = (new ReflectionClass(MaintenanceScheduleFilter::class))->newInstanceWithoutConstructor(); + + foreach ([ + 'builder' => $builder, + 'session' => new class { + public function get(string $key): ?string + { + return $key === 'company' ? 'company-uuid' : null; + } + }, + ] as $property => $value) { + $reflection = new ReflectionProperty(Filter::class, $property); + $reflection->setAccessible(true); + $reflection->setValue($filter, $value); + } + + return $filter; +} + +test('maintenance schedule filter scopes companies and resolves subject aliases', function () { + $builder = new FleetOpsMaintenanceScheduleFilterQuery(); + $filter = fleetopsMaintenanceScheduleFilter($builder); + + $filter->queryForInternal(); + $filter->queryForPublic(); + $filter->subjectType('vehicle'); + $filter->subjectType('fleet-ops:vehicle'); + $filter->subjectType(Vehicle::class); + $filter->subjectUuid('vehicle-uuid'); + + expect($builder->wheres)->toBe([ + ['company_uuid', 'company-uuid'], + ['company_uuid', 'company-uuid'], + ['subject_type', Vehicle::class], + ['subject_type', Vehicle::class], + ['subject_type', Vehicle::class], + ['subject_uuid', 'vehicle-uuid'], + ]); +}); + +test('maintenance schedule filter ignores empty subject filters', function () { + $builder = new FleetOpsMaintenanceScheduleFilterQuery(); + $filter = fleetopsMaintenanceScheduleFilter($builder); + + $filter->subjectType(null); + $filter->subjectUuid(null); + + expect($builder->wheres)->toBe([]); +}); diff --git a/tests/unit/components/vehicle/details/schedules-test.js b/tests/unit/components/vehicle/details/schedules-test.js new file mode 100644 index 000000000..fc9c29ca1 --- /dev/null +++ b/tests/unit/components/vehicle/details/schedules-test.js @@ -0,0 +1,59 @@ +import Service from '@ember/service'; +import { module, test } from 'qunit'; +import { click, render } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; +import { setupRenderingTest } from 'dummy/tests/helpers'; + +class StoreStub extends Service { + queries = []; + + query(modelName, params) { + this.queries.push([modelName, params]); + + return Promise.resolve([]); + } +} + +class VehicleActionsStub extends Service { + calls = []; + + scheduleMaintenance(...args) { + this.calls.push(args); + args[2].callback(); + return 'opened'; + } +} + +class NotificationsStub extends Service { + serverError() {} +} + +module('Unit | Component | vehicle/details/schedules', function (hooks) { + setupRenderingTest(hooks); + + hooks.beforeEach(function () { + this.owner.register('service:store', StoreStub); + this.owner.register('service:vehicle-actions', VehicleActionsStub); + this.owner.register('service:notifications', NotificationsStub); + }); + + test('loads by the backend polymorphic type and reloads after modal creation', async function (assert) { + const vehicle = { id: 'vehicle-uuid' }; + this.set('vehicle', vehicle); + + await render(hbs``); + + const store = this.owner.lookup('service:store'); + assert.deepEqual(store.queries[0], ['maintenance-schedule', { subject_uuid: 'vehicle-uuid', subject_type: 'fleet-ops:vehicle', sort: '-created_at' }]); + + await click('.vehicle-details-schedules button'); + + const vehicleActions = this.owner.lookup('service:vehicle-actions'); + const [scheduledVehicle, options, saveOptions] = vehicleActions.calls[0]; + assert.strictEqual(scheduledVehicle, vehicle); + assert.deepEqual(options, {}); + assert.false(saveOptions.refresh); + + assert.strictEqual(store.queries.length, 2, 'the vehicle schedule list reloads after save'); + }); +}); diff --git a/tests/unit/controllers/maintenance/schedules/index/edit-test.js b/tests/unit/controllers/maintenance/schedules/index/edit-test.js new file mode 100644 index 000000000..9746c8ea9 --- /dev/null +++ b/tests/unit/controllers/maintenance/schedules/index/edit-test.js @@ -0,0 +1,54 @@ +import Service from '@ember/service'; +import { module, test } from 'qunit'; +import { setupTest } from 'dummy/tests/helpers'; +import MaintenanceSchedulesIndexEditController from '@fleetbase/fleetops-engine/controllers/maintenance/schedules/index/edit'; + +class HostRouterStub extends Service { + refresh() { + return Promise.resolve(); + } + + transitionTo() { + return Promise.resolve(); + } +} + +class IntlStub extends Service { + calls = []; + + t(key, options) { + this.calls.push([key, options]); + return key === 'resource.maintenance-schedule' ? 'Maintenance Schedule' : 'Schedule updated'; + } +} + +class NotificationsStub extends Service { + messages = []; + + success(message) { + this.messages.push(message); + } + + serverError() {} +} + +module('Unit | Controller | maintenance/schedules/index/edit', function (hooks) { + setupTest(hooks); + + hooks.beforeEach(function () { + this.owner.register('controller:maintenance/schedules/index/edit', MaintenanceSchedulesIndexEditController); + this.owner.register('service:host-router', HostRouterStub); + this.owner.register('service:intl', IntlStub); + this.owner.register('service:notifications', NotificationsStub); + }); + + test('the update success message includes the schedule name required by the translation', function (assert) { + const controller = this.owner.lookup('controller:maintenance/schedules/index/edit'); + const schedule = { name: 'Vehicle Service' }; + + controller.notifyUpdateSuccess(schedule); + + const intl = this.owner.lookup('service:intl'); + assert.deepEqual(intl.calls.at(-1), ['common.resource-updated-success', { resource: 'Maintenance Schedule', resourceName: 'Vehicle Service' }]); + }); +}); diff --git a/tests/unit/services/vehicle-actions-test.js b/tests/unit/services/vehicle-actions-test.js index ace492ff9..5bf05d15c 100644 --- a/tests/unit/services/vehicle-actions-test.js +++ b/tests/unit/services/vehicle-actions-test.js @@ -15,12 +15,27 @@ class ResourceContextPanelStub extends Service { } } +class FetchStub extends Service {} + +class MaintenanceScheduleActionsStub extends Service { + calls = []; + + modal = { + create: (...args) => { + this.calls.push(args); + return 'opened'; + }, + }; +} + module('Unit | Service | vehicle-actions', function (hooks) { setupTest(hooks); hooks.beforeEach(function () { this.owner.register('service:universe/menu-service', MenuServiceStub); this.owner.register('service:resource-context-panel', ResourceContextPanelStub); + this.owner.register('service:fetch', FetchStub); + this.owner.register('service:maintenance-schedule-actions', MaintenanceScheduleActionsStub); }); test('it exists', function (assert) { @@ -42,6 +57,22 @@ module('Unit | Service | vehicle-actions', function (hooks) { assert.verifySteps(['vehicle reloaded']); }); + test('scheduleMaintenance resolves index vehicles and forwards modal save options', async function (assert) { + const fullVehicle = { id: 'vehicle-uuid', name: 'Truck 1' }; + const indexVehicle = { + loadResource: async () => fullVehicle, + }; + const options = { closeOnSuccess: true }; + const saveOptions = { refresh: false, callback() {} }; + const service = this.owner.lookup('service:vehicle-actions'); + + const result = await service.scheduleMaintenance(indexVehicle, options, saveOptions); + const maintenanceScheduleActions = this.owner.lookup('service:maintenance-schedule-actions'); + + assert.strictEqual(result, 'opened'); + assert.deepEqual(maintenanceScheduleActions.calls, [[{ subject: fullVehicle }, options, saveOptions]]); + }); + test('unassignOrders loads assigned orders, highlights the current job, and posts selected orders', async function (assert) { const service = this.owner.lookup('service:vehicle-actions'); const options = {}; From 119bc8877c1d50f68d7b306518348199803cbdf4 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 10 Aug 2026 23:50:30 +0800 Subject: [PATCH 19/28] fix(sensors): default last_position so a sensor can be created at all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /v1/sensors failed outright: SQLSTATE[HY000]: General error: 1364 Field 'last_position' doesn't have a default value Migration 2025_10_27_171322 made last_position NOT NULL on devices AND sensors so both could carry a spatial index, but only Device got a creating hook to default it. A position is not something a caller has when registering a sensor, so Sensor now defaults to POINT(0,0) exactly as Device does. Found by the Postman contract run, which had never once managed to create a sensor — the failure cascaded into Retrieve, Update and Delete. Adds SensorLastPositionDefaultTest mirroring the existing Device coverage: a sensor created without a position lands on the null island, and one created with a position keeps it. Also fixes a trailing-comma lint violation in ResolvesReviewAccountBypass, introduced with that trait earlier and failing `composer test:lint`. NOT VERIFIED LOCALLY: the pest harness cannot bootstrap on this host or in the container — server_vendor/bin/pest dies with "Class Symfony\Component\Console\ Input\ArgvInput not found" because Pest hardcodes ../../../vendor/autoload.php. The pre-existing DeviceLastPositionDefaultTest fails identically, so this is environmental rather than something these changes introduce. Lint is clean and CI runs the suite with the 100% coverage gate; watching that result. Co-Authored-By: Claude Opus 5 --- server/config/geocoder.php | 2 +- .../Concerns/ResolvesReviewAccountBypass.php | 2 +- server/src/Models/Sensor.php | 24 ++++ .../Models/SensorLastPositionDefaultTest.php | 130 ++++++++++++++++++ 4 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 server/tests/Unit/Models/SensorLastPositionDefaultTest.php diff --git a/server/config/geocoder.php b/server/config/geocoder.php index 00f0e3f58..789eac854 100644 --- a/server/config/geocoder.php +++ b/server/config/geocoder.php @@ -57,7 +57,7 @@ 'providers' => [ Chain::class => [ GoogleMaps::class => [ - config('services.google_maps.locale', env('GOOGLE_MAPS_LOCALE', 'us')), + config('services.google_maps.locale', env('GOOGLE_MAPS_LOCALE', 'en')), config('services.google_maps.api_key', env('GOOGLE_MAPS_API_KEY')), ] ], diff --git a/server/src/Http/Controllers/Concerns/ResolvesReviewAccountBypass.php b/server/src/Http/Controllers/Concerns/ResolvesReviewAccountBypass.php index 4f8596e9a..a60f0c87a 100644 --- a/server/src/Http/Controllers/Concerns/ResolvesReviewAccountBypass.php +++ b/server/src/Http/Controllers/Concerns/ResolvesReviewAccountBypass.php @@ -28,7 +28,7 @@ protected static function reviewAccountBypassMatches( string $accountsKey, ?string $identity, ?string $code, - string $channel + string $channel, ): bool { $bypassCode = config($codeKey); diff --git a/server/src/Models/Sensor.php b/server/src/Models/Sensor.php index efa4151d4..8b20638ba 100644 --- a/server/src/Models/Sensor.php +++ b/server/src/Models/Sensor.php @@ -147,6 +147,30 @@ class Sensor extends Model */ protected $spatialFields = ['last_position']; + /** + * Bootstrap the model. + * + * The same migration that made devices.last_position NOT NULL did it to + * sensors.last_position too, but only Device got a default. Creating a sensor + * without a position therefore failed outright: + * + * SQLSTATE[HY000]: General error: 1364 Field 'last_position' doesn't have a + * default value + * + * Position is not something a caller has when registering a sensor, so default + * it to POINT(0,0) on create, exactly as Device does. + */ + protected static function boot() + { + parent::boot(); + + static::creating(function (self $sensor) { + if (empty($sensor->last_position)) { + $sensor->last_position = new SpatialPoint(0, 0); + } + }); + } + /** * The attributes that should be cast to native types. * diff --git a/server/tests/Unit/Models/SensorLastPositionDefaultTest.php b/server/tests/Unit/Models/SensorLastPositionDefaultTest.php new file mode 100644 index 000000000..c988ce604 --- /dev/null +++ b/server/tests/Unit/Models/SensorLastPositionDefaultTest.php @@ -0,0 +1,130 @@ + ucfirst(str_replace(['_', '-'], ' ', Str::snake((string) $value)))); +} + +function fleetopsSensorDefaultBoot(): SQLiteConnection +{ + $pdo = new PDO('sqlite::memory:'); + $pdo->sqliteCreateFunction('ST_PointFromText', fn ($wkt, $srid = 0, $axisOrder = null) => $wkt); + $pdo->sqliteCreateFunction('ST_GeomFromText', fn ($wkt, $srid = 0, $axisOrder = null) => $wkt); + $connection = new SQLiteConnection($pdo); + $resolver = new ConnectionResolver(['default' => $connection, 'mysql' => $connection]); + $resolver->setDefaultConnection('mysql'); + EloquentModel::setConnectionResolver($resolver); + + // The creating hook is registered against whichever dispatcher is present + // when the model boots, so it has to exist before Device is ever touched. + if (!EloquentModel::getEventDispatcher()) { + EloquentModel::setEventDispatcher(new Illuminate\Events\Dispatcher()); + } + + app()->instance('db', new class($connection) { + public function __construct(public SQLiteConnection $c) + { + } + + public function connection($name = null): SQLiteConnection + { + return $this->c; + } + + public function __call($method, $arguments) + { + return $this->c->{$method}(...$arguments); + } + }); + app()->instance('db.schema', $connection->getSchemaBuilder()); + app()->instance('responsecache', new class { + public function __call($method, $arguments) + { + return null; + } + }); + Illuminate\Support\Facades\DB::clearResolvedInstance('db'); + app()->instance('request', Request::create('/v1/sensors')); + + config()->set('activitylog.enabled', false); + config()->set('activitylog.default_auth_driver', 'web'); + app()->bind(Illuminate\Contracts\Config\Repository::class, fn () => config()); + + $schema = $connection->getSchemaBuilder(); + $schema->create('sensors', function ($blueprint) { + $blueprint->increments('id'); + foreach (['uuid', 'public_id', 'internal_id', 'company_uuid', 'telematic_uuid', 'device_uuid', 'name', 'slug', 'type', 'status', 'online', 'unit', 'last_value', 'last_reading_at', 'min_threshold', 'max_threshold', 'location', 'last_position', 'meta', 'data', 'options', 'notes', '_key'] as $column) { + $blueprint->string($column)->nullable(); + } + $blueprint->timestamps(); + $blueprint->timestamp('deleted_at')->nullable(); + }); + + session(['company' => 'company-1']); + + return $connection; +} + +test('creating a sensor without a position defaults it to the null island', function () { + fleetopsSensorDefaultBoot(); + + $sensor = Sensor::create([ + 'company_uuid' => 'company-1', + 'name' => 'Defaulted Sensor', + 'type' => 'temperature', + ]); + + expect($sensor->last_position)->toBeInstanceOf(SpatialPoint::class) + ->and($sensor->last_position->getLat())->toBe(0.0) + ->and($sensor->last_position->getLng())->toBe(0.0); +}); + +test('creating a sensor with a position leaves it untouched', function () { + fleetopsSensorDefaultBoot(); + + $sensor = Sensor::create([ + 'company_uuid' => 'company-1', + 'name' => 'Positioned Sensor', + 'type' => 'temperature', + 'last_position' => new SpatialPoint(1.3, 103.8), + ]); + + expect($sensor->last_position)->toBeInstanceOf(SpatialPoint::class) + ->and($sensor->last_position->getLat())->toBe(1.3) + ->and($sensor->last_position->getLng())->toBe(103.8); +}); From c0c0a9d0901ebaeded0c46f5fc0f3502f56ce8cf Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Tue, 11 Aug 2026 13:48:03 +0800 Subject: [PATCH 20/28] fix(api): register-device was unreachable on both driver routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent defects made DriverController::registerDevice fail for every consumer. Both were invisible to the unit tests, which call the method directly and always pass an explicit Request. 1. Laravel never injected the Request. ResolvesRouteDependencies::transformDependency() returns null — it does not resolve from the container — for any class-typed parameter that declares a default value. registerDevice(?string $id = null, ?Request $request = null) therefore always ran with $request === null: POST /v1/drivers/{id}/register-device 500 "Call to a member function input() on null" POST /v1/drivers/register-device 404 (driver looked up by a null user_uuid) The default has to stay — the internal controller delegates with an id only — so the method falls back to request() when the router hands it null. 2. currentDriver() read an identity the public API never sets. The `fleetbase.api` middleware authenticates with Auth::setSession($credential), which writes the session keys but leaves $login false, so no user resolver is ever bound and $request->user() is null on every public API request. Resolution now falls back to session('user'), the same order Auth::getUserFromSession() uses. That helper is not called directly because it also calls auth() and session()->has(), neither of which exists in the SQLite test harness. Verified against a live stack: 500 -> 200 and 404 -> 200 respectively. Coverage gate still green — Api/v1/DriverController.php 100.00% (474/474). Co-Authored-By: Claude Opus 5 --- .../Controllers/Api/v1/DriverController.php | 25 ++++++++++++++- .../ApiDriverControllerContractsTest.php | 32 +++++++++++++++++++ .../Api/DriverControllerAuthFlowsTest.php | 29 +++++++++++++++-- 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/server/src/Http/Controllers/Api/v1/DriverController.php b/server/src/Http/Controllers/Api/v1/DriverController.php index f109ca0ac..9446c245e 100644 --- a/server/src/Http/Controllers/Api/v1/DriverController.php +++ b/server/src/Http/Controllers/Api/v1/DriverController.php @@ -440,6 +440,15 @@ public function toggleOnline(string $id, Request $request) */ public function registerDevice(?string $id = null, ?Request $request = null) { + // Laravel does NOT inject a class-typed parameter that declares a default value — + // RouteDependencyResolverTrait skips it and the default (null) is used. So the + // router always called this with $request === null, which made + // POST /v1/drivers/{id}/register-device fail with "Call to a member function + // input() on null" and POST /v1/drivers/register-device answer 404 (the driver was + // looked up by a null user_uuid). The default has to stay, because the internal + // controller delegates to this method with an id only. + $request = $request ?? request(); + try { // With an id (…/{id}/register-device) look the driver up directly; without // one (…/register-device and the internal delegation) resolve the driver @@ -962,7 +971,21 @@ protected function findDriver(string $id, array $with = []): Driver */ protected function currentDriver(?Request $request): Driver { - return Driver::where('user_uuid', optional(optional($request)->user())->uuid)->firstOrFail(); + // $request->user() is never populated on the public API: the `fleetbase.api` + // middleware calls Auth::setSession($apiCredential), which writes the session keys + // but leaves $login false, so no user resolver is ever bound. Reading it alone + // meant POST /v1/drivers/register-device looked a driver up by a null user_uuid + // and answered 404 for every consumer of the route. + // + // Auth::getUserFromSession() encodes this same order, but it also calls auth() and + // session()->has(), neither of which exists in the SQLite test harness — so the + // two sources are read directly here. + $user = optional($request)->user(); + if (!$user instanceof User) { + $user = User::where('uuid', session('user'))->first(); + } + + return Driver::where('user_uuid', optional($user)->uuid)->firstOrFail(); } protected function queryDrivers(Request $request) diff --git a/server/tests/ApiDriverControllerContractsTest.php b/server/tests/ApiDriverControllerContractsTest.php index ee3c1a96a..b11dc3754 100644 --- a/server/tests/ApiDriverControllerContractsTest.php +++ b/server/tests/ApiDriverControllerContractsTest.php @@ -550,6 +550,38 @@ public function or(array $keys, mixed $default = null): mixed ]); }); +test('api driver controller falls back to the container request when the router injects null', function () { + // Laravel's ResolvesRouteDependencies::transformDependency() returns null — it does + // NOT resolve from the container — for any class-typed parameter that declares a + // default value. registerDevice() has to declare one, because the internal controller + // delegates with an id only, so every routed call arrived with $request === null. + // Every other test here passes a request explicitly, which is why they stayed green + // while POST /v1/drivers/{id}/register-device answered 500. + $driver = new FleetOpsApiDriverFake(); + $driver->setRawAttributes([ + 'uuid' => 'driver-uuid', + 'public_id' => 'driver_public', + 'user_uuid' => 'user-uuid', + ], true); + + $controller = new FleetOpsApiDriverControllerProbe(); + $controller->driver = $driver; + + $bound = new FleetOpsApiDriverRegisterDeviceRequest(['token' => 'push-token', 'platform' => 'android']); + app()->instance('request', $bound); + + expect($controller->registerDevice('driver_public', null))->toBe([ + 'json' => ['device' => 'device_public'], + 'status' => 200, + ]) + ->and($controller->deviceCreates)->toBe([ + [ + ['token' => 'push-token', 'platform' => 'android'], + ['user_uuid' => 'user-uuid', 'platform' => 'android', 'token' => 'push-token', 'status' => 'active'], + ], + ]); +}); + test('api driver controller reports missing driver branches', function () { $controller = new FleetOpsApiDriverControllerProbe(); $controller->driverNotFound = true; diff --git a/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php b/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php index 0fe634ee2..6168cf7e9 100644 --- a/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php +++ b/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php @@ -470,14 +470,16 @@ public function __call($method, $arguments) $connection->table('users')->insert(['uuid' => 'user-current-1', 'public_id' => 'user_current1', 'company_uuid' => 'company-1', 'name' => 'Current Driver', 'email' => 'current@example.test', 'type' => 'driver']); $connection->table('drivers')->insert(['uuid' => 'driver-current-1', 'public_id' => 'driver_current1', 'company_uuid' => 'company-1', 'user_uuid' => 'user-current-1']); - // registerDevice without an id resolves the driver from the request user + // registerDevice without an id resolves the driver from the authenticated user // rather than a route parameter, so exercise the real seam here — the // contract probe overrides it and never runs this body $currentDriver = new ReflectionMethod(DriverController::class, 'currentDriver'); $currentDriver->setAccessible(true); $request = Request::create('/v1/drivers/register-device', 'POST'); - $request->setUserResolver(fn () => $connection->table('users')->where('uuid', 'user-current-1')->first()); + // Auth::getUserFromSession only accepts a User model from the resolver — anything + // else falls through to the session, so this must be a real model, not a row object. + $request->setUserResolver(fn () => User::query()->where('uuid', 'user-current-1')->first()); $resolved = $currentDriver->invoke(new DriverController(), $request); @@ -485,8 +487,31 @@ public function __call($method, $arguments) ->and($resolved->uuid)->toBe('driver-current-1'); }); +test('current driver resolves from the session when the request has no user resolver', function () { + $connection = fleetopsDriverAuthBoot(); + $connection->table('users')->insert(['uuid' => 'user-session-1', 'public_id' => 'user_session1', 'company_uuid' => 'company-1', 'name' => 'Session Driver', 'email' => 'session@example.test', 'type' => 'driver']); + $connection->table('drivers')->insert(['uuid' => 'driver-session-1', 'public_id' => 'driver_session1', 'company_uuid' => 'company-1', 'user_uuid' => 'user-session-1']); + + // This is the production path for the public API. `fleetbase.api` authenticates with + // Auth::setSession($apiCredential), which writes session('user') but does NOT bind a + // user resolver — so reading $request->user() directly resolved nothing and + // POST /v1/drivers/register-device answered 404 for every consumer. + session(['user' => 'user-session-1']); + + $currentDriver = new ReflectionMethod(DriverController::class, 'currentDriver'); + $currentDriver->setAccessible(true); + + $resolved = $currentDriver->invoke(new DriverController(), Request::create('/v1/drivers/register-device', 'POST')); + + expect($resolved)->toBeInstanceOf(Driver::class) + ->and($resolved->uuid)->toBe('driver-session-1'); +}); + test('current driver fails when the request has no matching driver', function () { fleetopsDriverAuthBoot(); + // Explicit, because the session survives between tests and a leftover `user` key + // would silently resolve a driver and make this assertion vacuous. + session(['user' => null]); $currentDriver = new ReflectionMethod(DriverController::class, 'currentDriver'); $currentDriver->setAccessible(true); From 8c099a52d54a9208748f229d4074c1b4ef56c8b7 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Tue, 11 Aug 2026 15:46:43 +0800 Subject: [PATCH 21/28] fix(api): fuel reports could not be created without a location or updated at all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent defects on the same resource, both found by the contract run. 1. POST /v1/fuel-reports without a location answered 500. `location` is declared nullable in CreateFuelReportRequest but the column is POINT NOT NULL, so the insert failed with SQLSTATE[HY000] 1364 "Field 'location' doesn't have a default value" — a 500 for a request the validator had just accepted. Reproduced against a live stack. Device and Sensor already default their spatial columns in boot(); FuelReport now does the same. 2. Every partial update was rejected. UpdateFuelReportRequest extends CreateFuelReportRequest, so it inherited `driver`, `odometer` and `volume` as flat `required` rules. PUT /v1/fuel-reports/{id} with {"status": "approved"} answered "The driver field is required." `driver` is the plainest case: the update action's $request->only() list does not even include it, so validation demanded a field the endpoint then discarded. These are now required on POST only. CreateFuelTransactionRequest already solves the same create/update inheritance with Rule::requiredIf(isMethod POST); a plain conditional is used here because illuminate/validation is not autoloadable in the package's test harness. RequestContractsTest asserted $fuelReportUpdateRules['driver'] === ['required'], pinning the second defect. That assertion is updated to the corrected contract and now also covers odometer and volume. Coverage gate green: composer coverage:check --fail-under=100 exits 0. Co-Authored-By: Claude Opus 5 --- .../Http/Requests/CreateFuelReportRequest.php | 17 ++++++++++++--- server/src/Models/FuelReport.php | 21 +++++++++++++++++++ server/tests/RequestContractsTest.php | 8 ++++++- .../Requests/CreateFuelReportRequestTest.php | 21 +++++++++++++++++++ 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/server/src/Http/Requests/CreateFuelReportRequest.php b/server/src/Http/Requests/CreateFuelReportRequest.php index 33d707d19..cd9522072 100644 --- a/server/src/Http/Requests/CreateFuelReportRequest.php +++ b/server/src/Http/Requests/CreateFuelReportRequest.php @@ -19,10 +19,21 @@ public function authorize(): bool */ public function rules(): array { + // requiredIf(POST), not required: UpdateFuelReportRequest extends this class, so a + // flat `required` made every partial update fail — PUT /v1/fuel-reports/{id} with + // just {"status": "approved"} answered "The driver field is required." + // + // `driver` is the clearest case: the update action's $request->only() list does not + // even include it, so validation demanded a field the endpoint then discarded. + // CreateFuelTransactionRequest solves the same create/update inheritance with + // Rule::requiredIf(isMethod('POST')); a plain conditional is used here because + // illuminate/validation is not autoloadable in the package's test harness. + $requiredOnCreate = $this->isMethod('POST') ? ['required'] : ['sometimes']; + return [ - 'driver' => ['required'], - 'odometer' => ['required'], - 'volume' => ['required'], + 'driver' => $requiredOnCreate, + 'odometer' => $requiredOnCreate, + 'volume' => $requiredOnCreate, 'metric_unit' => ['nullable'], 'location' => ['nullable'], 'amount' => ['nullable'], diff --git a/server/src/Models/FuelReport.php b/server/src/Models/FuelReport.php index fc0118412..8c7b0b669 100644 --- a/server/src/Models/FuelReport.php +++ b/server/src/Models/FuelReport.php @@ -6,6 +6,7 @@ use Fleetbase\FleetOps\Casts\Point; use Fleetbase\FleetOps\Support\Utils; use Fleetbase\LaravelMysqlSpatial\Eloquent\SpatialTrait; +use Fleetbase\LaravelMysqlSpatial\Types\Point as SpatialPoint; use Fleetbase\Models\Model; use Fleetbase\Models\User; use Fleetbase\Traits\HasApiModelBehavior; @@ -149,6 +150,26 @@ public function reporter() return $this->belongsTo(User::class, 'reported_by_uuid'); } + /** + * Bootstrap the model. + * + * `location` is declared nullable in CreateFuelReportRequest but the column is + * POINT NOT NULL, so a report created without one failed outright with + * SQLSTATE[HY000] 1364 "Field 'location' doesn't have a default value" — a 500 for + * a request the validator had just accepted. Mirrors Device and Sensor, which + * default their spatial columns the same way. + */ + protected static function boot() + { + parent::boot(); + + static::creating(function (self $fuelReport) { + if (empty($fuelReport->location)) { + $fuelReport->location = new SpatialPoint(0, 0); + } + }); + } + /** * Get the driver's name assigned to vehicle. * diff --git a/server/tests/RequestContractsTest.php b/server/tests/RequestContractsTest.php index 0aba33cbd..d9c8749d6 100644 --- a/server/tests/RequestContractsTest.php +++ b/server/tests/RequestContractsTest.php @@ -303,7 +303,13 @@ protected function canUpdateDriver(): bool ->and($fuelReportRules['driver'])->toBe(['required']) ->and($fuelReportRules['odometer'])->toBe(['required']) ->and($fuelReportRules['volume'])->toBe(['required']) - ->and($fuelReportUpdateRules['driver'])->toBe(['required']) + // The update request inherits these rules, so `required` made every partial + // update fail — PUT with just {"status": "approved"} answered "The driver + // field is required.", and the update action's $request->only() list does not + // even include `driver`. This assertion previously pinned that behaviour. + ->and($fuelReportUpdateRules['driver'])->toBe(['sometimes']) + ->and($fuelReportUpdateRules['odometer'])->toBe(['sometimes']) + ->and($fuelReportUpdateRules['volume'])->toBe(['sometimes']) ->and($internalDriverRules['location'][1])->toBeInstanceOf(ResolvablePoint::class) ->and($internalDriverRules['vehicle'][1])->toBeInstanceOf(ResolvableVehicle::class) ->and($internalDriverRules['latitude'])->toBe(['nullable', 'required_with:longitude', 'numeric']) diff --git a/server/tests/Unit/Http/Requests/CreateFuelReportRequestTest.php b/server/tests/Unit/Http/Requests/CreateFuelReportRequestTest.php index bce631b8a..305684dfb 100644 --- a/server/tests/Unit/Http/Requests/CreateFuelReportRequestTest.php +++ b/server/tests/Unit/Http/Requests/CreateFuelReportRequestTest.php @@ -10,6 +10,7 @@ class FormRequest extends \Illuminate\Http\Request namespace { use Fleetbase\FleetOps\Http\Requests\CreateFuelReportRequest; + use Fleetbase\FleetOps\Http\Requests\UpdateFuelReportRequest; class FleetOpsCreateFuelReportSessionStore { @@ -39,6 +40,26 @@ function fleetopsCreateFuelReportRequestWithSession(array $sessionData): CreateF return CreateFuelReportRequest::create('/fleet-ops/fuel-reports', 'POST'); } + test('fuel report create only fields are required on POST and optional on update', function () { + FleetOpsCreateFuelReportRequestState::$session = new FleetOpsCreateFuelReportSessionStore(['api_credential' => 'api-credential-uuid']); + + // UpdateFuelReportRequest extends this class, so a flat `required` made every + // partial update fail — PUT with just {"status": "approved"} answered "The driver + // field is required." `driver` is the plainest case: the update action's + // $request->only() list does not include it, so the field was demanded and then + // discarded. + $create = CreateFuelReportRequest::create('/fleet-ops/fuel-reports', 'POST')->rules(); + $update = UpdateFuelReportRequest::create('/fleet-ops/fuel-reports/report_1', 'PUT')->rules(); + + foreach (['driver', 'odometer', 'volume'] as $field) { + expect($create[$field])->toBe(['required']) + ->and($update[$field])->toBe(['sometimes']); + } + + expect($create['metric_unit'])->toBe(['nullable']) + ->and($create['amount'])->toBe(['nullable']); + }); + test('create fuel report authorization accepts api credentials or sanctum sessions', function () { expect(fleetopsCreateFuelReportRequestWithSession([])->authorize())->toBeFalse() ->and(fleetopsCreateFuelReportRequestWithSession(['api_credential' => 'api-credential-uuid'])->authorize())->toBeTrue() From c499bce0fe12ec37974b55241ff30a0ad5c584b2 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Wed, 12 Aug 2026 17:36:26 +0800 Subject: [PATCH 22/28] fix(api): geofence driver history asked for a uuid the API never issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /v1/geofences/driver/{driverUuid}/history took a driver UUID and matched it against driver_uuid directly. Every other public v1 endpoint addresses a resource by its public_id, and the public API does not expose uuids — so this endpoint asked callers for an identifier they have no way to obtain. It now resolves the driver by public_id and filters on the uuid it finds, and the public route parameter is renamed {driverId} to say so. The console reaches the same method through the internal route with a uuid. That is still accepted, but ONLY for internal requests, so the public contract is public_id-only rather than quietly taking either. Resolution goes through findDriverForHistory(), a seam like the existing geofenceEventLogQuery(), so the contract tests drive it without a database. Deliberately NOT changed here: the response still returns uuid alongside id for the driver, vehicle, subject, geofence and order, and the event's own `id` is a uuid because GeofenceEventLog has no public_id. Gating those to internal requests is a breaking shape change for anything already consuming them, and needs its own decision rather than riding along with an addressing fix. Co-Authored-By: Claude Opus 5 --- .../Controllers/Api/v1/GeofenceController.php | 36 +++++++++++++++++-- server/src/routes.php | 2 +- .../Api/GeofenceControllerContractsTest.php | 21 +++++++++-- 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/server/src/Http/Controllers/Api/v1/GeofenceController.php b/server/src/Http/Controllers/Api/v1/GeofenceController.php index 591db6b6a..433fdd62f 100644 --- a/server/src/Http/Controllers/Api/v1/GeofenceController.php +++ b/server/src/Http/Controllers/Api/v1/GeofenceController.php @@ -4,6 +4,7 @@ use Fleetbase\FleetOps\Models\GeofenceEventLog; use Fleetbase\FleetOps\Support\Utils; +use Fleetbase\FleetOps\Models\Driver; use Fleetbase\Http\Controllers\Controller; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -201,13 +202,22 @@ public function dwellReport(Request $request): JsonResponse * * Returns the geofence event history for a specific driver. */ - public function driverHistory(Request $request, string $driverUuid): JsonResponse + public function driverHistory(Request $request, string $driverId): JsonResponse { $companyUuid = session('company'); $perPage = min((int) $request->input('per_page', 50), 200); + // Addressed by public_id, like every other public endpoint. The route parameter + // was named driverUuid and matched against driver_uuid directly, so this was the + // one place in the public API asking a caller for an internal identifier — which + // the API does not hand out in the first place, making it unusable. + $driver = $this->findDriverForHistory($driverId); + if (!$driver) { + return response()->json(['error' => 'Driver resource not found.'], 404); + } + $events = $this->geofenceEventLogQuery($companyUuid) - ->where('driver_uuid', $driverUuid) + ->where('driver_uuid', $driver->uuid) ->with(['driver.vehicle', 'vehicle', 'order']) ->orderBy('occurred_at', 'desc') ->paginate($perPage); @@ -217,6 +227,28 @@ public function driverHistory(Request $request, string $driverUuid): JsonRespons return response()->json($events); } + /** + * Resolve the driver a history request addresses. + * + * Public callers use the public_id, like every other v1 endpoint. The console reaches + * the same method through the internal route with a uuid, so that is accepted there + * and ONLY there — the public contract stays public_id-only rather than quietly + * taking either. + * + * A seam, like geofenceEventLogQuery above, so the contract tests can drive this + * without a database. + */ + protected function findDriverForHistory(string $driverId): ?Driver + { + $driver = Driver::where('public_id', $driverId)->first(); + + if (!$driver && \Fleetbase\Support\Http::isInternalRequest()) { + $driver = Driver::where('uuid', $driverId)->first(); + } + + return $driver; + } + protected function serializeEvent(GeofenceEventLog $event): array { $driver = $event->driver; diff --git a/server/src/routes.php b/server/src/routes.php index db2c69d09..1ef332181 100644 --- a/server/src/routes.php +++ b/server/src/routes.php @@ -237,7 +237,7 @@ function ($router) { $router->get('events', 'GeofenceController@events'); $router->get('inventory', 'GeofenceController@inventory'); $router->get('dwell-report', 'GeofenceController@dwellReport'); - $router->get('driver/{driverUuid}/history', 'GeofenceController@driverHistory'); + $router->get('driver/{driverId}/history', 'GeofenceController@driverHistory'); }); // service-rates routes $router->group(['prefix' => 'service-rates'], function () use ($router) { diff --git a/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php b/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php index 279459216..dc08aeb03 100644 --- a/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php +++ b/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php @@ -183,6 +183,16 @@ class FleetOpsGeofenceControllerFake extends GeofenceController public ?FleetOpsGeofenceQueryFake $nextEventQuery = null; public array $serializedEvents = []; + public ?Fleetbase\FleetOps\Models\Driver $historyDriver = null; + public array $historyDriverLookups = []; + + protected function findDriverForHistory(string $driverId): ?Fleetbase\FleetOps\Models\Driver + { + $this->historyDriverLookups[] = $driverId; + + return $this->historyDriver; + } + protected function geofenceEventLogQuery(?string $companyUuid): mixed { $query = $this->nextEventQuery ?? new FleetOpsGeofenceQueryFake($companyUuid); @@ -351,10 +361,17 @@ function fleetopsGeofenceEvent(array $attributes): GeofenceEventLog 'per_page' => 75, ]); - $controller->driverHistory($request, 'driver-9'); + // The endpoint is addressed by public_id and resolves the driver itself; the query + // still filters on the driver's uuid. + $historyDriver = new Fleetbase\FleetOps\Models\Driver(); + $historyDriver->setRawAttributes(['uuid' => 'driver-9', 'public_id' => 'driver_public9'], true); + $controller->historyDriver = $historyDriver; + + $controller->driverHistory($request, 'driver_public9'); $query = $controller->eventQueries[0]; - expect($query->companyUuid)->toBe('company-4') + expect($controller->historyDriverLookups)->toBe(['driver_public9']) + ->and($query->companyUuid)->toBe('company-4') ->and($query->calls)->toContain( ['where', 'driver_uuid', 'driver-9'], ['with', ['driver.vehicle', 'vehicle', 'order']], From efc10a7982bea1d9752ffcf5c1887bd1e85e7cf4 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Wed, 12 Aug 2026 17:46:15 +0800 Subject: [PATCH 23/28] ci(contract): run the API contract once per commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pull request from a branch in this repo fires both `push` and `pull_request` for the same commit, so the contract ran twice — two stack boots, two seeds, two full collection runs for one change. Keying the concurrency group on the COMMIT rather than the ref collapses them: the pull_request event reports the head SHA and the push event reports the same commit as github.sha, so both land in one group and the older is cancelled. Deduped rather than dropping a trigger, so a direct push to a release branch is still verified instead of relying on someone remembering to dispatch it. Co-Authored-By: Claude Opus 5 --- .github/workflows/postman.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/postman.yml b/.github/workflows/postman.yml index d69e4ed56..9b242f3e4 100644 --- a/.github/workflows/postman.yml +++ b/.github/workflows/postman.yml @@ -21,6 +21,17 @@ on: branches: [main, 'dev-v*'] workflow_dispatch: +# A pull request from a branch in this repo fires BOTH `push` and `pull_request` for the +# same commit, so the contract ran twice: two stack boots, two seeds, two collection runs. +# Keying the group on the commit rather than the ref collapses them — the pull_request +# event reports the head SHA, the push event reports the same commit as github.sha. +# +# Deduped rather than dropping a trigger, so a direct push to a release branch is still +# verified instead of relying on someone remembering to dispatch it. +concurrency: + group: api-contract-${{ github.event.pull_request.head.sha || github.sha }} + cancel-in-progress: true + permissions: contents: read From 2100ceee95de46b8a5dfef525bd4e3c44f3a552f Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Wed, 12 Aug 2026 18:41:42 +0800 Subject: [PATCH 24/28] Revert "ci(contract): run the API contract once per commit" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cancel-in-progress dedupes by cancelling, which surfaces cancelled runs on the pull request. That reads as something having gone wrong, and is a worse trade than the duplicate run it removes. Reverted to the previous triggers. The duplicate run on same-repo pull requests is back, and is worth solving properly — most likely by narrowing the triggers so only one event fires, rather than starting a run and killing it. Co-Authored-By: Claude Opus 5 --- .github/workflows/postman.yml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/.github/workflows/postman.yml b/.github/workflows/postman.yml index 9b242f3e4..04fc271e7 100644 --- a/.github/workflows/postman.yml +++ b/.github/workflows/postman.yml @@ -20,18 +20,6 @@ on: pull_request: branches: [main, 'dev-v*'] workflow_dispatch: - -# A pull request from a branch in this repo fires BOTH `push` and `pull_request` for the -# same commit, so the contract ran twice: two stack boots, two seeds, two collection runs. -# Keying the group on the commit rather than the ref collapses them — the pull_request -# event reports the head SHA, the push event reports the same commit as github.sha. -# -# Deduped rather than dropping a trigger, so a direct push to a release branch is still -# verified instead of relying on someone remembering to dispatch it. -concurrency: - group: api-contract-${{ github.event.pull_request.head.sha || github.sha }} - cancel-in-progress: true - permissions: contents: read From dcbf13de8fe0f6d6ff34ef6b1244e41cb4f19142 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Wed, 12 Aug 2026 19:33:28 +0800 Subject: [PATCH 25/28] test(geofence): cover the real driver lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage gate dropped to 96.97% on GeofenceController: the contract tests stub findDriverForHistory(), so the implementation added in c499bce0 was never executed. Covers it against a real SQLite connection — public_id resolves, a uuid does NOT on a public request, an unknown id returns null, and a uuid DOES resolve when the route is internal. Two things the harness needs, both worth knowing: Driver carries a global scope requiring a related user, so a users table and user_uuid are required; and Http::isInternalRequest() decides from the ROUTE's uri rather than a header, so the internal case needs a route with an "int" segment. That "a uuid is not accepted publicly" assertion is the contract the change restores, so it is asserted rather than implied. Co-Authored-By: Claude Opus 5 --- .../Api/GeofenceControllerContractsTest.php | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php b/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php index dc08aeb03..bdcf65d2d 100644 --- a/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php +++ b/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php @@ -1,6 +1,7 @@ $connection, 'mysql' => $connection]); + $resolver->setDefaultConnection('mysql'); + Illuminate\Database\Eloquent\Model::setConnectionResolver($resolver); + + $schema = $connection->getSchemaBuilder(); + // Driver carries a global scope requiring a related user, so both tables are needed. + $schema->create('users', function ($table) { + $table->increments('id'); + $table->string('uuid')->nullable(); + $table->timestamp('deleted_at')->nullable(); + $table->timestamps(); + }); + $schema->create('drivers', function ($table) { + $table->increments('id'); + $table->string('uuid')->nullable(); + $table->string('public_id')->nullable(); + $table->string('company_uuid')->nullable(); + $table->string('user_uuid')->nullable(); + $table->timestamp('deleted_at')->nullable(); + $table->timestamps(); + }); + $connection->table('users')->insert(['uuid' => 'user-uuid-1']); + $connection->table('drivers')->insert([ + 'uuid' => 'driver-uuid-1', + 'public_id' => 'driver_public1', + 'user_uuid' => 'user-uuid-1', + ]); + + $resolve = new ReflectionMethod(GeofenceController::class, 'findDriverForHistory'); + $resolve->setAccessible(true); + $controller = new GeofenceController(); + + $byPublicId = $resolve->invoke($controller, 'driver_public1'); + // A uuid is NOT accepted on a public request — that is the contract this restores. + $byUuidPublic = $resolve->invoke($controller, 'driver-uuid-1'); + $missing = $resolve->invoke($controller, 'driver_nope'); + + expect($byPublicId)->toBeInstanceOf(Driver::class) + ->and($byPublicId->uuid)->toBe('driver-uuid-1') + ->and($byUuidPublic)->toBeNull() + ->and($missing)->toBeNull(); + + // The console reaches the same method through the internal route with a uuid. + // Http::isInternalRequest() decides from the ROUTE's uri — any route with an "int" + // segment — so the request has to carry one, not a header. + $internalRequest = Request::create('/fleet-ops/int/v1/geofences/driver/driver-uuid-1/history', 'GET'); + $internalRequest->setRouteResolver(fn () => new Illuminate\Routing\Route( + ['GET'], + 'fleet-ops/int/v1/geofences/driver/{driverUuid}/history', + ['uses' => fn () => null] + )); + app()->instance('request', $internalRequest); + + $byUuidInternal = $resolve->invoke($controller, 'driver-uuid-1'); + + expect($byUuidInternal)->toBeInstanceOf(Driver::class) + ->and($byUuidInternal->public_id)->toBe('driver_public1'); +}); From 2130c86dc3bb98062d838d8eb582049326d74b4e Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Thu, 13 Aug 2026 01:18:13 +0800 Subject: [PATCH 26/28] fix(api): publish the QR code's content in debug, and stop /from-qr 500ing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes behind one contract failure. 1. qr_code_content on the tracking number resource The QR image is generated from the tracking number's owner_uuid, and /from-qr and capture-qr both match on that uuid. A scanner reads it from the image; an automated client cannot. Debug builds now return the same value beside the PNG as `qr_code_content`, so the flow can be exercised without the API growing a decode endpoint or handing out uuids generally. Guarded by $this->when(), which OMITS the key rather than nulling it, so a production response is byte-identical to before — asserted with array_key_exists rather than a null check. The accessor uses app()->hasDebugModeEnabled() behind a method_exists guard and a try/catch, and fails closed: any inability to determine the debug state answers false. The webhook payload is a separate literal array and is untouched, with a test pinning that it stays that way. The value is owner_uuid — the exact input to newBarcode() — not a public id. 2. findQrModel() no longer falls back to a raw row lookup It called Utils::findModel() with the ARRAY of table names. When nothing matched, that array reached DB::table(), which stringified it to the literal table "Array" and threw SQLSTATE[42S02] — so every unresolvable code answered 500 instead of the 400 the caller already handles. The fallback could not have helped anyway: qrModelResource() serializes through a typed resource keyed on the model class, and a raw stdClass row has none. The mapped Entity and Order models are the endpoint's whole contract. The shared Utils::findModel() defect is fixed separately in core-api 557b9370; removing the caller means fleetops no longer waits on that image release. Tests cover: the field present and correct in debug, absent (not null) outside it, the accessor failing closed with no usable application, the webhook payload not exposing it, and findQrModel returning null for both an unmapped table and an unresolved placeholder value. Full unit suite green. Co-Authored-By: Claude Opus 5 --- .../Api/v1/TrackingNumberController.php | 11 ++- .../src/Http/Resources/v1/TrackingNumber.php | 33 ++++++++ .../CompactResourceSerializationTest.php | 81 +++++++++++++++++++ .../TrackingNumberControllerHelpersTest.php | 18 +++-- 4 files changed, 136 insertions(+), 7 deletions(-) diff --git a/server/src/Http/Controllers/Api/v1/TrackingNumberController.php b/server/src/Http/Controllers/Api/v1/TrackingNumberController.php index 001e535f2..d79658fcc 100644 --- a/server/src/Http/Controllers/Api/v1/TrackingNumberController.php +++ b/server/src/Http/Controllers/Api/v1/TrackingNumberController.php @@ -200,7 +200,16 @@ protected function findQrModel(array $tables, array $where) } } - return Utils::findModel($tables, $where); + // No raw-row fallback. Utils::findModel() was called with the ARRAY of table names + // and, when nothing matched, passed that array to DB::table() — which stringified + // it to the literal table name "Array" and threw SQLSTATE[42S02]. Every + // unresolvable code became a 500 instead of the 400 this method's caller already + // handles. + // + // The fallback could not have helped anyway: qrModelResource() serializes through + // a typed resource keyed on the model class, and a raw stdClass row has none. The + // mapped models above are the endpoint's whole contract. + return null; } protected function qrModelResource($model) diff --git a/server/src/Http/Resources/v1/TrackingNumber.php b/server/src/Http/Resources/v1/TrackingNumber.php index 0d44e9bad..bed9b0d5d 100644 --- a/server/src/Http/Resources/v1/TrackingNumber.php +++ b/server/src/Http/Resources/v1/TrackingNumber.php @@ -30,6 +30,17 @@ public function toArray($request) 'status' => $this->last_status, 'status_code' => $this->last_status_code, 'qr_code' => $this->qr_code, + // The raw text encoded inside qr_code, exposed ONLY in debug mode. + // + // qr_code is a base64 PNG generated from owner_uuid (TrackingNumber::newBarcode), + // and the endpoints that consume a scanned code match on that uuid. A client + // scans the image to obtain it; an automated contract run cannot, so debug + // builds publish the value beside the image rather than the API growing a + // decode endpoint or handing out uuids generally. + // + // `when()` omits the key entirely when false — it is absent, not null — so a + // production response is byte-identical to before. + 'qr_code_content' => $this->when(static::exposesQrCodeContent(), fn () => $this->owner_uuid), 'barcode' => $this->barcode, 'url' => Utils::consoleUrl('track-order', ['order' => $this->tracking_number]), 'type' => Utils::getTypeFromClassName($this->owner_type), @@ -38,6 +49,28 @@ public function toArray($request) ]; } + /** + * Whether the QR code's decoded content may be published. + * + * Debug mode only, and fails closed: any failure to determine the debug state — no + * container, an application without the accessor — answers false rather than + * defaulting to exposure. + */ + protected static function exposesQrCodeContent(): bool + { + try { + $app = app(); + + if (!is_object($app) || !method_exists($app, 'hasDebugModeEnabled')) { + return false; + } + + return (bool) $app->hasDebugModeEnabled(); + } catch (\Throwable $e) { + return false; + } + } + /** * Transform the resource into an webhook payload. * diff --git a/server/tests/CompactResourceSerializationTest.php b/server/tests/CompactResourceSerializationTest.php index c8ed76d09..9a59f847c 100644 --- a/server/tests/CompactResourceSerializationTest.php +++ b/server/tests/CompactResourceSerializationTest.php @@ -2697,6 +2697,10 @@ function fleetopsCompactResourceFixture(array $attributes = [], array $loaded = 'scheduled_at' => '2026-07-30 09:00:00', ]); + // The decoded QR content is a debug-only field and must never ride along on a webhook, + // which is a separate literal payload rather than a filtered toArray(). + expect(array_key_exists('qr_code_content', (new TrackingNumberResource($trackingNumber))->toWebhookPayload()))->toBeFalse(); + expect((new TrackingNumberResource($trackingNumber))->toWebhookPayload())->toMatchArray([ 'id' => 'fixture_public', 'tracking_number' => 'TN-ZERO', @@ -2743,3 +2747,80 @@ function fleetopsCompactResourceFixture(array $attributes = [], array $loaded = 'meta' => ['sequence' => 2], ]); }); + +test('tracking number resource publishes the qr code content only in debug mode', function () { + // qr_code is a base64 PNG generated from owner_uuid, and the endpoints that consume a + // scanned code match on that uuid. Debug builds publish the value beside the image so + // an automated client can follow the flow without decoding a PNG; production must not. + $trackingNumber = fleetopsCompactResourceFixture([ + 'tracking_number' => 'TN-DEBUG', + 'owner_uuid' => 'owner-uuid-under-the-qr', + 'owner_type' => 'Fleetbase\\FleetOps\\Models\\Order', + 'region' => 'sg', + 'qr_code' => 'qr-data', + 'barcode' => 'barcode-data', + 'last_status' => 'created', + 'last_status_code' => 'CREATED', + ]); + $request = Request::create('/v1/tracking-numbers', 'GET'); + + // The full resource builds a console url, which needs app()->environment(). The bare + // container this suite runs on has no such method, which is why the tests above only + // cover the index resource and the webhook payload. + $previousContainer = Illuminate\Container\Container::getInstance(); + $container = new class extends Illuminate\Container\Container { + public function environment(...$environments) + { + return in_array('testing', $environments, true) || $environments === [] ? 'testing' : false; + } + }; + $container->instance('config', new Illuminate\Config\Repository([ + 'fleetbase' => ['console' => ['host' => 'console.fleetbase.test', 'subdomain' => null, 'secure' => true]], + ])); + Illuminate\Container\Container::setInstance($container); + + $withDebug = new class ($trackingNumber) extends TrackingNumberResource { + protected static function exposesQrCodeContent(): bool + { + return true; + } + }; + $withoutDebug = new class ($trackingNumber) extends TrackingNumberResource { + protected static function exposesQrCodeContent(): bool + { + return false; + } + }; + + try { + $debugPayload = $withDebug->resolve($request); + $productionPayload = $withoutDebug->resolve($request); + } finally { + Illuminate\Container\Container::setInstance($previousContainer); + } + + // Present and exactly the value the QR was generated from — not a public id. + expect($debugPayload['qr_code_content'])->toBe('owner-uuid-under-the-qr') + ->and($debugPayload['qr_code'])->toBe('qr-data') + // Absent, not null: `when()` drops the key entirely, so a production response is + // byte-identical to one from before the field existed. + ->and(array_key_exists('qr_code_content', $productionPayload))->toBeFalse() + ->and($productionPayload['qr_code'])->toBe('qr-data'); +}); + +test('tracking number resource fails closed when the debug state cannot be determined', function () { + // Any failure to resolve the debug state must answer false rather than defaulting to + // exposure. The real accessor is exercised here with no usable application bound. + $expose = new ReflectionMethod(TrackingNumberResource::class, 'exposesQrCodeContent'); + $expose->setAccessible(true); + + $previous = Illuminate\Container\Container::getInstance(); + + try { + // A container that is not an Application has no hasDebugModeEnabled(). + Illuminate\Container\Container::setInstance(new Illuminate\Container\Container()); + expect($expose->invoke(null))->toBeFalse(); + } finally { + Illuminate\Container\Container::setInstance($previous); + } +}); diff --git a/server/tests/Feature/Http/Api/TrackingNumberControllerHelpersTest.php b/server/tests/Feature/Http/Api/TrackingNumberControllerHelpersTest.php index ebe98d90a..80baaba8a 100644 --- a/server/tests/Feature/Http/Api/TrackingNumberControllerHelpersTest.php +++ b/server/tests/Feature/Http/Api/TrackingNumberControllerHelpersTest.php @@ -138,8 +138,14 @@ public function __call($method, $arguments) expect($qrModel)->not->toBeNull() ->and($helper('qrModelResource', $qrModel))->toBeInstanceOf(Fleetbase\FleetOps\Http\Resources\v1\Entity::class); - // Tables with no eloquent mapping are skipped and fall back to a raw row - // lookup, so the record still resolves but arrives unhydrated + // A table with no eloquent mapping resolves to null rather than a raw row. + // + // The raw-row fallback was removed: it passed the ARRAY of table names to + // DB::table(), which stringified it to the literal table "Array" and threw + // SQLSTATE[42S02], so every unresolvable QR code answered 500 instead of the 400 the + // caller already handles. It could not have helped either way — qrModelResource() + // serializes through a typed resource keyed on the model class, and a raw stdClass + // row has none. $connection->table('tracking_statuses')->insert([ 'uuid' => '44444444-4444-4444-8444-444444444403', 'public_id' => 'tracking_status_tnhelper1', @@ -147,8 +153,8 @@ public function __call($method, $arguments) 'status' => 'Dispatched', ]); - $unmapped = $helper('findQrModel', ['tracking_statuses'], ['public_id' => 'tracking_status_tnhelper1']); - expect($unmapped)->not->toBeNull() - ->and($unmapped)->not->toBeInstanceOf(EloquentModel::class) - ->and($unmapped->uuid)->toBe('44444444-4444-4444-8444-444444444403'); + expect($helper('findQrModel', ['tracking_statuses'], ['public_id' => 'tracking_status_tnhelper1']))->toBeNull() + // And a mapped table that simply matches nothing is also null, which is the path + // an unknown or unresolved code takes. + ->and($helper('findQrModel', ['entities', 'orders'], ['uuid' => '{{qr_code}}']))->toBeNull(); }); From 8d301d131a9afe705a042c25df861db79e54e00a Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Thu, 13 Aug 2026 04:48:04 +0800 Subject: [PATCH 27/28] test: restore 100% coverage on the QR and geofence changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate reported 99.99% overall, from two files: TrackingNumber.php 91.89% (34/37) GeofenceController.php 99.39% (164/165) The resource tests overrode exposesQrCodeContent() in anonymous subclasses, so the real debug check never ran — the tests proved the resource used the flag, not that the flag worked. They now drive it through the container instead, with a fake standing in for Application::hasDebugModeEnabled(), so the accessor itself is exercised in both states. Adds the two paths that had no coverage at all: * the accessor's catch branch — an accessor that THROWS must answer false, not propagate; a resource must not fail serialization over a debug check * driverHistory()'s own 404 branch, asserting it also builds no event query rather than proceeding with an unresolved driver Verified locally with XDEBUG_MODE=coverage composer coverage:check: 100.00% (34166/34166 statements). Co-Authored-By: Claude Opus 5 --- .../CompactResourceSerializationTest.php | 40 ++++++++++++------- .../Api/GeofenceControllerContractsTest.php | 15 +++++++ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/server/tests/CompactResourceSerializationTest.php b/server/tests/CompactResourceSerializationTest.php index 9a59f847c..b482a8c5f 100644 --- a/server/tests/CompactResourceSerializationTest.php +++ b/server/tests/CompactResourceSerializationTest.php @@ -2769,32 +2769,34 @@ function fleetopsCompactResourceFixture(array $attributes = [], array $loaded = // cover the index resource and the webhook payload. $previousContainer = Illuminate\Container\Container::getInstance(); $container = new class extends Illuminate\Container\Container { + public bool $debugMode = false; + public function environment(...$environments) { return in_array('testing', $environments, true) || $environments === [] ? 'testing' : false; } + + // Stands in for the framework's Application::hasDebugModeEnabled(). + public function hasDebugModeEnabled() + { + return $this->debugMode; + } }; $container->instance('config', new Illuminate\Config\Repository([ 'fleetbase' => ['console' => ['host' => 'console.fleetbase.test', 'subdomain' => null, 'secure' => true]], ])); Illuminate\Container\Container::setInstance($container); - $withDebug = new class ($trackingNumber) extends TrackingNumberResource { - protected static function exposesQrCodeContent(): bool - { - return true; - } - }; - $withoutDebug = new class ($trackingNumber) extends TrackingNumberResource { - protected static function exposesQrCodeContent(): bool - { - return false; - } - }; + // Drive the REAL accessor through the container rather than overriding it, so the + // debug check itself is exercised and not just the resource's use of it. + $container->debugMode = true; + $withDebug = new TrackingNumberResource($trackingNumber); try { - $debugPayload = $withDebug->resolve($request); - $productionPayload = $withoutDebug->resolve($request); + $debugPayload = $withDebug->resolve($request); + + $container->debugMode = false; + $productionPayload = (new TrackingNumberResource($trackingNumber))->resolve($request); } finally { Illuminate\Container\Container::setInstance($previousContainer); } @@ -2820,6 +2822,16 @@ protected static function exposesQrCodeContent(): bool // A container that is not an Application has no hasDebugModeEnabled(). Illuminate\Container\Container::setInstance(new Illuminate\Container\Container()); expect($expose->invoke(null))->toBeFalse(); + + // And an accessor that throws must also answer false rather than propagating — + // a resource must not be able to fail serialization over a debug check. + Illuminate\Container\Container::setInstance(new class extends Illuminate\Container\Container { + public function hasDebugModeEnabled() + { + throw new RuntimeException('debug state unavailable'); + } + }); + expect($expose->invoke(null))->toBeFalse(); } finally { Illuminate\Container\Container::setInstance($previous); } diff --git a/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php b/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php index bdcf65d2d..a1b90fd94 100644 --- a/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php +++ b/server/tests/Feature/Http/Api/GeofenceControllerContractsTest.php @@ -381,6 +381,21 @@ function fleetopsGeofenceEvent(array $attributes): GeofenceEventLog ); }); +test('driver history answers 404 when the driver does not resolve', function () { + // The endpoint's own not-found branch: findDriverForHistory returns null and the + // request must not proceed to a query keyed on nothing. + $controller = new FleetOpsGeofenceControllerFake(); + $controller->historyDriver = null; + + $response = $controller->driverHistory(Request::create('/geofences/driver/driver_missing/history', 'GET'), 'driver_missing'); + + expect($response->getStatusCode())->toBe(404) + ->and($response->getData(true))->toBe(['error' => 'Driver resource not found.']) + ->and($controller->historyDriverLookups)->toBe(['driver_missing']) + // and no event query was ever built + ->and($controller->eventQueries)->toBe([]); +}); + test('driver history resolves by public id, and by uuid only for internal requests', function () { // The contract tests above stub findDriverForHistory(), so the real lookup needs its // own coverage. It is the whole point of the change: the public endpoint used to take From 1e2e2ae986ef863acb99559422195f596dde01e7 Mon Sep 17 00:00:00 2001 From: Ron Date: Mon, 17 Aug 2026 13:51:30 +0800 Subject: [PATCH 28/28] docs(release): write the v0.6.60 release notes The release workflow refuses to tag unless RELEASE.md's first line names the version being released and the template placeholders are gone. These notes were written from this PR's own commits and diff, and create-release.yml now publishes them as the GitHub Release body. --- RELEASE.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 RELEASE.md diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 000000000..29af9ef76 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,36 @@ +> v0.6.60 ~ "Closes an authentication bypass, and clears a run of API 500s" + +--- +## Highlights +A security fix and a broad sweep of public API defects surfaced by running the official Postman collection against a live stack. Several endpoints answered `500` where a `404` or `422` belonged, and a few were unreachable entirely. + +--- +## Security +- **Closed a verify-code authentication bypass in the driver flow.** Please upgrade. +- The non-production verification-code bypass is now scoped to explicitly designated review accounts, so a bypass code alone is not enough — the identity has to be on the allowlist too. + +--- +## Bug Fixes +- **Driver `register-device` was unreachable on both driver routes.** Laravel never injects a class-typed parameter that declares a default, so the injected request was always null. +- **Geofence driver history asked for a UUID the API never issues.** It now resolves the driver by the public id callers actually hold. +- **`/from-qr` returned a 500**, and the QR code's content is now published in debug mode so the flow can be exercised. +- **Fuel reports could not be created without a location**, and could not be updated. +- **A sensor could not be created at all** — `last_position` had no default. +- **Customer signup with a place failed** — the Place location now defaults. +- Unknown onboard organization answers `404` instead of `500`. +- Duplicate part SKU and fuel transaction answer `422` instead of `500`. +- Restored the vehicle maintenance schedule workflows. + +--- +## Testing +- Coverage restored to 100% across the QR, geofence, driver auth, customer request and navigator changes. + +--- +## Continuous Integration +- The server, Ember and Postman workflows now run on `dev-v*` release branches. +- The contract run tests this branch's API code rather than the published package. + +--- +## Need help? +- [GitHub Discussions](https://github.com/fleetbase/fleetbase/discussions) +- [Discord](https://discord.gg/HnTqQ6zAVn)