fix: stop /health/ready disclosing the database it cannot reach - #22
Merged
Merged
Conversation
ttncode
force-pushed
the
fix/readiness-reason-leak
branch
from
September 13, 2026 21:48
7f897e4 to
87227ca
Compare
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
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.
What this changes
/health/readyreturned the driver's own exception message to the caller, inevery adapter that has the route:
That names the host, its address on the compose network, the port, the username,
the database and the dialect, on an endpoint nothing authenticates and compose
publishes on the application's own port. It does not leak the password β
SQLAlchemy masks it when rendering a URL, and neither PDO's nor Prisma's
connection errors echo it β so this is topology disclosure, not credential
disclosure.
It also bought nothing. Every consumer of a readiness endpoint β Kubernetes,
compose's own
depends_on: service_healthy, a load balancer β reads the statuscode and discards the body. The one reader who wants the reason is a developer,
and a log line serves them better than an HTTP response they have to provoke.
So the reason moves to the log and the body keeps only the status:
nestjsreason: (error as Error).messagethis.logger.warn(...)laravel-api,laravel-inertia'reason' => $e->getMessage()logger()->warning(...)flaskreason=str(error)current_app.logger.warning(...)All three families change together. Leaving one behind would mean three
adapters disagreeing about what a readiness failure looks like, which is worse
than the disclosure.
Laravel needed one more thing to make this an actual fix. Its default
stackchannel writes to
storage/logs/laravel.log, a path nothing reads in acontainer β so moving the reason there and stopping would have hidden the
failure rather than relocated it. Both laravel images now set
ENV LOG_CHANNEL=stderr, and both adapters'.env.exampleset it forhost-side
mise run dev.nestjs'sLoggerand Flask's app logger alreadywrite to stdout and stderr respectively.
Laravel uses the
logger()helper rather than theLogfacade on purpose: ause Illuminate\Support\Facades\Log;line would land between the twouselines
services/mongodb/drivers/laravel.shsplices, and pint'sordered_importswould then fail the generated project's ownformattask.The comment in the file says so.
No driver changes. Every splice in
services/shared/{nest,laravel,flask}.shandservices/mongodb/drivers/laravel.shmatches the probe anchor or the fallbackthrow; none of them touches the
catchblock.How it was verified
mise run lintand./scaffold lintβ cleanbats tests/new-flask.bats tests/new-nestjs.bats tests/new-laravel-api.batsβeach generates a real project and runs its own
ci-unit, which is whatcatches a pint or prettier disagreement about the edited files. That was the
live risk here: the laravel change adds a blank line before a
returnand astring concatenation, both of which pint has opinions about.
mise run test-runnerβ both lanessmokeanddeployfor every tier A adapter and,because
adapters/laravel-inertia/changed, the tier B pair as well.Checklist
mise run lintpassesmise run test-runnerpasses