Skip to content

fix: stop /health/ready disclosing the database it cannot reach - #22

Merged
ttncode merged 1 commit into
mainfrom
fix/readiness-reason-leak
Sep 13, 2026
Merged

ttncode merged 1 commit into
mainfrom
fix/readiness-reason-leak

Conversation

@ttncode

@ttncode ttncode commented Sep 13, 2026

Copy link
Copy Markdown
Owner

What this changes

/health/ready returned the driver's own exception message to the caller, in
every adapter that has the route:

{"status":"unavailable","reason":"connection to server at \"database\" (172.19.0.2), port 5432 failed: FATAL: password authentication failed for user \"app\""}

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 status
code 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:

Adapter Was Now
nestjs reason: (error as Error).message this.logger.warn(...)
laravel-api, laravel-inertia 'reason' => $e->getMessage() logger()->warning(...)
flask reason=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 stack
channel writes to storage/logs/laravel.log, a path nothing reads in a
container β€” 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.example set it for
host-side mise run dev. nestjs's Logger and Flask's app logger already
write to stdout and stderr respectively.

Laravel uses the logger() helper rather than the Log facade on purpose: a
use Illuminate\Support\Facades\Log; line would land between the two use
lines services/mongodb/drivers/laravel.sh splices, and pint's
ordered_imports would then fail the generated project's own format task.
The comment in the file says so.

No driver changes. Every splice in services/shared/{nest,laravel,flask}.sh and
services/mongodb/drivers/laravel.sh matches the probe anchor or the fallback
throw; none of them touches the catch block.

How it was verified

  • mise run lint and ./scaffold lint β€” clean
  • bats 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 what
    catches a pint or prettier disagreement about the edited files. That was the
    live risk here: the laravel change adds a blank line before a return and a
    string concatenation, both of which pint has opinions about.
  • mise run test-runner β€” both lanes
  • This PR's own CI, which runs smoke and deploy for every tier A adapter and,
    because adapters/laravel-inertia/ changed, the tier B pair as well.

Checklist

  • mise run lint passes
  • mise run test-runner passes
  • New behaviour has a test that fails without the change
  • Docs that describe changed behaviour were updated in the same commit
  • No unrelated changes

@ttncode
ttncode force-pushed the fix/readiness-reason-leak branch from 7f897e4 to 87227ca Compare September 13, 2026 21:48
@ttncode
ttncode merged commit a3878a6 into main Sep 13, 2026
21 checks passed
@ttncode
ttncode deleted the fix/readiness-reason-leak branch September 13, 2026 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant