docs(deploy): a Coolify runbook, checked against a real container - #63
Merged
Conversation
Point Coolify at the repo, Build Pack: Dockerfile, port 8091. The image
already did the work — multi-stage, pinned digests, non-root under tini,
/healthz HEALTHCHECK — so this is config and documentation, not a rebuild.
THE FOOTGUN THIS EXISTS FOR. AYS_TRUSTED_PROXIES is unset by default, and
that default is correct: honouring X-Forwarded-For from any caller walks
straight past the login throttle. But _get_client_ip then falls back to the
direct peer, and behind a reverse proxy the direct peer is the PROXY, for
every request. The limiter allows 5 attempts per 5 minutes per client, so
five failed logins by one stranger lock out every user of the instance.
Nothing looks wrong while that is misconfigured. The app starts, serves and
throttles; the symptom arrives minutes later as "nobody can log in". So the
app says so at startup — a WARNING on a named logger, not a print, so it
lands in the platform's log view and the suite can build a thousand apps
without it becoming noise. The default is NOT changed: defaulting to trust
the private ranges would reverse the F-2 fix and hand anything on the same
network the ability to spoof past the throttle.
RUNNING IT FOUND A REAL PROBLEM, which is why it was run rather than
written. Built the image and started it with production env:
AYS_ENV=prod -> curl /openapi.json -> 200
That reads like the docs gate failing. It is not: the response is the SPA
shell, identical to what any nonsense URL returns, because the static
catch-all GET /{full_path:path} answers everything unmatched. The schema is
not served.
But every test in test_docs_gating.py builds an app with NO static dir and
asserts 404 — a status code production never produces. The gate holds; the
assertion mechanism stops being true the moment the app is deployed. Two
tests now cover the deployed shape: with a static dir and env=prod the body
must not contain the schema, and with env=dev the real JSON must still get
past the catch-all, since route order decides that and route order is easy
to change by accident. Mutation-checked: forcing is_dev True fails all
three parametrized cases.
Every claim in DEPLOY.md's verification section was run against a real
container before being written down, both branches:
prod : <!doctype html>
dev : {"openapi":"3.1.0","info":{"title":"AreY
The first draft of that section also told operators to check the Secure
cookie flag with `curl -I /api/auth/status`. There is no cookie there —
cookies are set only by a SUCCESSFUL login — so it was an instruction that
would always look broken. Replaced with the browser check and the symptom
to expect.
docker-compose.yml is marked LOCAL DEVELOPMENT ONLY: it publishes to
127.0.0.1, which a platform's proxy cannot reach.
.dockerignore does not exclude .beads/ (23 MB, holds a credential key). It
cannot reach an image — Coolify builds from a git clone, the key is
gitignored, and the Dockerfile has no COPY . . — so it is build-context
hygiene, and areyousievious-b1r already owns that file. Left alone.
1125 tests, 4 xfailed, ruff clean.
|
Claude finished @derekslenk's task in 3m 3s —— View job Code Review in progress
|
|
Claude finished @derekslenk's task in 1m 49s —— View job Claude finished the code reviewTodo list:
ReviewWent through every hunk in
No bugs, inaccuracies, or unverified claims found — the diff matches what it documents, and the two new docs-gating tests close a real gap (the old suite only asserted a 404 that production never returns, since the SPA catch-all answers everything unmatched with 200). No findings to report. This PR is well-scoped, and the "deliberately not done" section correctly identifies out-of-scope items (rate-limiter keying change, |
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.
Point Coolify at the repo, Build Pack Dockerfile, port 8091. The image already did the work — multi-stage, pinned digests, non-root under
tini,/healthzhealthcheck — so this is config and documentation, not a rebuild.The footgun this exists for
AYS_TRUSTED_PROXIESis unset by default, and that default is correct: honouringX-Forwarded-Forfrom any caller walks straight past the login throttle (CWE-348).But
_get_client_ipthen falls back to the direct peer — and behind a reverse proxy the direct peer is the proxy, for every request. The limiter allows 5 attempts per 5 minutes per client, so:Nothing looks wrong while that's misconfigured. The app starts, serves and throttles; the symptom arrives minutes later as "nobody can log in". So the app now says so at startup — a
WARNINGon a named logger, not aprint, so it lands in the platform's log view and the suite can build a thousand apps without it becoming noise.The default is not changed. Defaulting to "trust the private ranges" would reverse the F-2 fix and hand anything on the same network the ability to spoof past the throttle.
Running it found a real problem
Which is why it was run rather than written. Built the image, started it with production env:
That reads like the docs gate failing. It isn't — the response is the SPA shell, byte-identical to what any nonsense URL returns, because the static catch-all
GET /{full_path:path}answers everything unmatched. The schema is not served.But every test in
test_docs_gating.pybuilds an app with no static dir and asserts404— a status code production never produces. The gate holds; the assertion mechanism stops being true the moment the app is deployed.Two tests now cover the deployed shape:
env=prod→ the body must not contain the schemaenv=dev→ the real JSON must still get past the catch-all, since route order decides that and route order is easy to change by accidentMutation-checked: forcing
is_devTrue fails all three parametrized cases.Every claim in the runbook was executed
Both branches, against real containers, and the output is pasted into the doc verbatim:
The first draft also told operators to check the
Securecookie flag withcurl -I /api/auth/status. There is no cookie there — cookies are set only by a successful login — so it was an instruction that would always look broken. Replaced with the browser check and the symptom to expect (logged straight back out).Also in here
docker-compose.ymlmarked LOCAL DEVELOPMENT ONLY — it publishes to127.0.0.1, which a platform's proxy cannot reach.README.mdrow forAYS_TRUSTED_PROXIESnow names the behind-a-proxy consequence, not just the mechanism.Deliberately not done
.dockerignorechange. It doesn't exclude.beads/(23 MB, holds a credential key), but that can't reach an image — Coolify builds from a git clone, the key is gitignored, and the Dockerfile has noCOPY . .. It's build-context hygiene, andareyousievious-b1ralready owns that file.Verification
1125 passed, 4 xfailed·ruffclean · frontend untouched and greenhealthy, container confirmed running asuid=1000(ays)AYS_TRUSTED_PROXIESand absent in one with it🤖 Generated with Claude Code