v1.6.56 - #237
Open
roncodes wants to merge 13 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 6721 +65
============================================
Files 394 397 +3
Lines 22173 22431 +258
============================================
+ Hits 22173 22431 +258
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>
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