v1.6.56 - #237
Open
roncodes wants to merge 17 commits into
Open
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Release branch collecting: * ci: fix the Postman contract and track the latest release automatically (#235), which also carries the Sentry config probe fix. sentry/sentry 4.30.0 stopped throwing from ClientBuilder::create() on an invalid DSN, and composer.lock is gitignored so CI resolves it fresh — main's PHP CI has been red since that release landed, independent of any change in this repo. * fix(exceptions): stop rendering HTML stack traces to API clients (#236). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ci: fix the Postman contract and track the latest release automatically
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #237 +/- ##
============================================
Coverage 100.00% 100.00%
- Complexity 6656 6727 +71
============================================
Files 394 397 +3
Lines 22173 22442 +269
============================================
+ Hits 22173 22442 +269
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Both workflows filtered on `branches: [main]` for pull_request, so a PR targeting a dev-v* release branch triggered no checks at all. Since release work lands on the release branch first and only reaches main via the release PR, every contributing PR merged unverified and the first real signal arrived after the fact, on the release PR itself. Seen on #236: retargeting it from main to dev-v1.6.56 silently dropped its checks, leaving only a stale run from before the rebase. Add dev-v* to the push and pull_request filters in both workflows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Handler::render() never inspected the request. It routed on the exception's
short class name against a hardcoded six-item allowlist and passed everything
else to parent::render(), which falls through to shouldReturnJson() ->
$request->expectsJson(). Nothing on /v1/* forces JSON — the fleetbase.api
middleware group has no equivalent of a ForceJsonResponse — so any API client
that omitted an Accept: application/json header received Laravel's HTML error
page for every unlisted exception.
Confirmed against a live stack on unrelated endpoints, so this was not
specific to any one route:
PUT /v1/orders -> 405 text/html 1,105,667 bytes
POST /v1/onboard/driver-onboard-settings/x -> 405 text/html 1,105,923 bytes
MethodNotAllowedHttpException is thrown during routing, before route
middleware runs, which is why a middleware-based fix would not have covered
it. QueryException, TypeError, AccessDeniedHttpException, ValidationException
and most of Fleetbase's own exceptions fall through the same way.
Three changes:
* shouldReturnJson() returns true whenever APP_DEBUG is off, so a deployed API
never answers HTML. With debugging on it defers to the framework, keeping
the HTML debug page as a local development affordance.
* convertExceptionToArray() emits the {"errors": [...]} envelope used by
response()->error() everywhere else, and withholds file, line, class and
frames when debugging is off. HTTP exception messages are preserved because
they only describe the request the caller already made; anything else
collapses to "Server Error".
* The allowlist relied on response()->error()'s 400 default, so it answered
400 for cases that have a correct status. NotFoundHttpException now returns
404, ThrottleRequestsException 429, AuthenticationException 401 and
TokenMismatchException 419. FleetbaseRequestValidationException is
deliberately left at 400; 422 is the conventional answer but has the widest
blast radius on the console, so it is left for a separate change.
Note this makes leak protection depend on APP_DEBUG being false in deployed
environments. That is already the Laravel default.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(exceptions): stop rendering HTML stack traces to API clients
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 <noreply@anthropic.com>
ci(postman): test this branch's API code, and unpin the contract workflow
GET /v1/files/{id}/download validated with
Http\Requests\Internal\DownloadFileRequest, whose rules require a uuid:
'id' => ['required_without:file', 'uuid', 'exists:files,uuid']
Every other public endpoint addresses a resource by public_id and explicitly
rejects uuids, and an upload returns file_xxxxxxxx — so a consumer could not
download the file it had just uploaded. It got 422 "The file identifier must be
a valid UUID." Found by the Postman contract run.
The controller was never the problem: File::findRecordOrFail() already resolves
a public_id and answers 404 for an unknown file. Only the validation refused.
Adds a public request class and points the public controller at it. The
internal controller keeps the internal one, because the console genuinely works
in uuids and its contract should not change.
Two differences from the internal rules, both deliberate:
* the identifier is a string, not a uuid;
* existence is left to the controller, so a missing file is 404 rather than
the 422 an `exists` rule would produce.
authorize() also returns true rather than checking for a session user. The
route is behind the fleetbase.api middleware group, which authenticates the API
credential; a session user is the wrong notion of identity for a
key-authenticated request.
Tests: 1415 passing. Three new cases cover the public rules, that authorize
succeeds without a session, and that the route id is merged and the messages
are stable. Coverage measured at 100% overall, and 17/17 statements on the new
class.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `fleetbase.api` middleware authenticates a credential and then calls
Auth::setSession(), which writes session('user') but takes $login = false. The
default guard is session-based with no login, so nothing ever bound a user
resolver — $request->user() returned null on every public API request, even
though the request was fully authenticated.
Extensions that read the standard accessor got nothing, and the failures looked
unrelated to each other:
* ledger — all four /ledger/v1/wallet* routes answered 401 to every
credential, including a driver's own Sanctum token, which authenticates
fine against every other public endpoint
* fleetops — POST /v1/drivers/register-device answered 404 because the driver
was looked up by a null user_uuid
Both branches now bind the resolver: the API-credential path to the credential's
owner, the Sanctum path to the token's tokenable. A user already resolved by a
real guard is never displaced, so the internal session routes are unaffected.
Full suite: 1417 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Utils::findModel() loops when given an array of tables, but when no table matched it fell out of the loop and ran the scalar lookup below with the ARRAY still in hand. DB::table() stringified it, so the query became select * from `Array` where `uuid` = ... and threw SQLSTATE[42S02] "Base table or view not found". Every "not found" became a 500 for callers already written to treat null as not-found. Reproduced against a live stack: POST /v1/tracking-numbers/from-qr with a code that resolves nothing answered 500 with an HTML stack trace, where TrackingNumberController::fromQR has an explicit 400 "Unable to find QR code value" branch waiting for null. Any client scanning a stale or damaged QR hit this. getUuid() directly above returns null in exactly this situation; findModel was the odd one out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adding `use Fleetbase\Models\User` for the resolver binding made the existing inline \Fleetbase\Models\User redundant; php-cs-fixer's fully_qualified_strict_types rule flagged it and failed Lint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Utils::delinkify() typed its argument as a non-nullable string, and the verification and credentials mail views call it on `$user->name`. A user created from an identity alone — exactly what the storefront customer-creation flow does before Create a Customer supplies a name — has none, so rendering the email threw Utils::delinkify(): Argument #1 ($text) must be of type string, null given from inside the compiled Blade view. That surfaced as a 500 on POST /storefront/v1/customers/request-creation-code, and would hit any caller sending mail to a user whose name is not set. delinkify() now accepts null and returns an empty string, which is the right contract for a text-formatting helper handed a model attribute. The greeting is fixed too. Left alone it would have rendered "Good Morning, !" — the comma and name are now conditional, so a nameless recipient gets a clean "Good Morning!". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release branch for core-api 1.6.56, cut from
mainwithflb version-bump(patch). The bump commit touchescomposer.jsononly, matching the convention of previous releases.What this release collects
Two PRs retarget onto this branch:
ci/postman-contract-v0.7.53feature/api-json-error-responsesWhy the Sentry fix matters for this release
mainis currently red on PHP CI, and has been since before either PR was opened.sentry/sentry4.30.0 stopped throwing fromClientBuilder::create()on an invalid DSN — validation became lazy.SettingController::testSentryConfig()relies on that throw to reportstatus: error, so it now returnsstatus: successandSettingControllerExternalProbesTestfails.Because
composer.lockis gitignored, CI resolves dependencies fresh on every run, so the drift landed without any commit in this repo.main's last PHP CI run was 2026-08-04, before that release; nothing had run PHP CI onmainsince, which is why the breakage only surfaced when #236 opened.Verified by running the failing test against unmodified
origin/main(f173de9) — it fails there with zero changes applied. #235 fixes it by validating explicitly through\Sentry\Dsn::createFromString().Expected CI
With both PRs merged into this branch, the full unit suite is green. Measured locally on PHP 8.4 with the #235 fix applied on top of #236:
Zero failures.
Merge order
main🤖 Generated with Claude Code