fix(docker): drop curl from the API runtime images - #4202
Merged
Conversation
curl's only in-container consumer was the readiness loop in start-all.sh; there is no HEALTHCHECK instruction anywhere. It is also the sole reverse-dependency of libcurl4t64, which brings libssh2-1t64, so one line in each install list accounted for nine HIGH findings - all status=affected with no Debian fix published, so `apt-get upgrade` could not clear them and not shipping the package was the only remediation. Trivy 0.74.0 HIGH+CRITICAL, on locally built slim images: api-only 3C / 60H -> 3C / 51H standalone 3C / 60H -> 3C / 51H with exactly the curl, libcurl4t64 and libssh2-1t64 findings removed and nothing new. Replace it with http_probe, which reproduces `curl -sf` WITHOUT -L rather than approximating it. The distinction matters: curl does not follow redirects unless asked, so a 302 is a completed transfer and succeeds regardless of what it points at. urllib.request.urlopen follows it and raises on a 404 behind it, which would report a healthy service that redirects as "not ready". http_probe uses http.client and tests `status < 400` itself, bypassing urllib's redirect handler, and carries userinfo through as Basic auth the way curl does. Verified equivalent to `curl -sf` on 2xx, 3xx-to-good, 3xx-to-bad, 4xx, 5xx, query strings, userinfo auth and connection refused. Exit codes are not reproduced (curl's 22 and 7 become 1); every call site tests zero/non-zero. One deliberate difference, since it is a change and not a translation: curl was called with --connect-timeout, which caps only the connection phase, and the API health loop passed no timeout at all - so a server that accepted a connection and never answered hung the probe forever. The timeout now covers the whole request. There is no wget fallback. BusyBox wget cannot reproduce these semantics (no --max-redirect, so it always follows), and it is not needed: every image that probes anything is Python-based. cp-only, the one image with neither, performs no probe at all and dropped curl in #4197. Missing python3 now fails loudly at startup instead of degrading into a readiness loop that can never succeed. Closes #4198
The first version of this probe was Python embedded in a shell string inside start-all.sh. That was a bad shape for code encoding rules this fiddly: every quote had to survive two levels of escaping, ruff and ty never saw it, and it could only be exercised through the shell. Move it to hindsight_api/http_probe.py, shipped with the code and covered by tests/test_http_probe.py, which pins each case to what `curl -sf` does for the same response. start-all.sh keeps a three-line wrapper that shells out to `python3 -m hindsight_api.http_probe`. hindsight-admin was the obvious home and is the wrong one: it takes 5.1s to start in the built image, against 0.028s for bare stdlib, because it pulls in the CLI and everything behind it. The readiness loop polls once per second, so importing the API to ask whether the API is up would break the loop it drives. This module imports stdlib only; measured 0.035s per probe in the image. `hindsight_api/__init__` is cheap by design and has to stay that way for this to hold - its docstring already says so. The shell test drops to checking the wiring, since the semantics now have a real home, and skips when the package is not importable: test-start-all.sh also runs in CI from a bare checkout with no virtualenv. Reformatting by `ruff format` on first contact is the point - the embedded version could never have received it.
…rom the API hindsight_api.http_probe was the wrong home. The probe answers "is an API process up?", and living inside the package it probes invited exactly the coupling that would break it: an import of the engine or the config would put API startup cost - and API startup side effects - on a loop that runs once a second. Move it to hindsight_probe, a sibling top-level package in the same distribution. Its dependencies are now explicit by construction: none. It imports the standard library and nothing else. Packaging alone does not enforce that. Both packages install into the same virtualenv, so `import hindsight_api` from the probe would still resolve at runtime. So the rule is a test, not a convention: test_imports_nothing_but_the_standard_library imports the package in a clean subprocess and asserts that nothing outside sys.stdlib_module_names was pulled in. Adding `import hindsight_api` to the probe fails it with the offending name. The audit ignores _sysconfigdata_*, a platform-specific stdlib internal whose name embeds the build triple and so is absent from stdlib_module_names everywhere. Both Dockerfiles now copy the package; the api-builder previously copied only hindsight_api, so the first build without this shipped an image whose probe could not import. That surfaced as require_http_probe_runtime failing at startup with a clear message rather than a readiness loop that could never succeed, which is what that guard is for. Verified in the built Linux image: no non-stdlib imports, hindsight_api never loaded, 0.036s per probe, and the full end-to-end boot still reaches "Hindsight is running" with /health answering 200.
Reverts the separate hindsight_probe package. It was justified on a bad measurement: an earlier cold-cache timing suggested `import hindsight_api` cost ~0.12s against ~0.03s for a standalone package. Measured properly, warm, in the built image, they are the same - ~0.03s each - and importing hindsight_api pulls in zero third-party modules. Its PEP 562 lazy-attribute design already does the work the split was meant to do, so the split bought nothing and cost a second top-level package, four pyproject entries and a COPY in each Dockerfile. What was worth keeping is the enforcement, which is orthogonal to where the module lives. test_imports_nothing_heavy imports the probe in a clean subprocess and asserts it pulled in no third-party package and nothing from hindsight_api.engine, .api or .config. Adding `from hindsight_api.engine import memory_engine` to the probe fails it with 43 packages named, numpy, sqlalchemy and asyncpg among them - which is the failure mode the rule exists to prevent. Verified in the built image: no third-party or engine imports, 0.034s per import, probe wiring works, curl absent.
nicoloboschi
force-pushed
the
fix/api-images-drop-curl
branch
from
September 8, 2026 08:31
717ca56 to
1bd2f8b
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
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.
Closes #4198. Last of the three pieces #4057 was split into, after #4199 and #4200.
Why
curl's only in-container consumer was the readiness loop in
start-all.sh— there is noHEALTHCHECKinstruction anywhere in either Dockerfile. It is also the sole reverse-dependency oflibcurl4t64, which in turn bringslibssh2-1t64, so one line in each install list accounted for nine HIGH findings.All nine are
status=affectedwith noFixedVersion— Debian has published nothing, so theapt-get upgradethese stages already run cannot clear them, and not shipping the package is the only available remediation.Trivy 0.74.0, HIGH+CRITICAL, locally built slim images:
api-onlystandaloneExactly
curl×4,libcurl4t64×4 andlibssh2-1t64×1 removed, nothing new.Being straight about what this is: none of those CVEs was reachable through how the images used curl (they concern SFTP/SCP host verification, TLS config mismatch, connection reuse and proxy auth state; the probe does
GET http://localhost:8888/health). The value is scan hygiene — nine unfixable HIGH findings on a published image generate questions that can't be answered with "upgrade it" — plus removing standard post-exploitation tooling from a runtime container.The replacement is a translation, not an approximation
This is the part #4057 got wrong and the reason this piece was split out.
curl -sfwithout-Ldoes not follow redirects, so a 302 is a completed transfer and succeeds regardless of what it points at. The obviousurllib.request.urlopenreplacement follows it and raises on a 404 behind it — silently turning a healthy service that redirects into "not ready".http_probeuseshttp.clientand testsstatus < 400itself, bypassing urllib's redirect handler entirely. Verified againstcurl -sfon a server returning each case:curl -sfhttp_probeExit codes are not reproduced (22 and 7 become 1) — every call site tests zero/non-zero only.
One deliberate difference, called out because it's a change and not a translation: curl was called with
--connect-timeout, which caps only the connection phase, and the API health loop passed no timeout at all. A server that accepted a connection and then never answered hung the probe forever. The timeout now covers the whole request.No wget fallback
BusyBox wget can't reproduce these semantics — 1.37.0 in
node:*-alpinehas no--max-redirect, so it always follows. It also isn't needed: every image that probes anything is Python-based.cp-only, the one image with neither curl nor python3, performs no probe at all and dropped curl in #4199.Missing python3 now fails loudly at startup via
require_http_probe_runtimerather than returning 127, which the readiness loops would have read as "not ready yet" and spun on until timeout.Tests
test-start-all.shgains probe coverage, including/redirect-to-missingas an explicit regression guard. I mutation-tested it: swapping in theurllibimplementation makes the suite fail withso the test has teeth rather than just passing.
The test server runs under
serve_foreveruntil the trap kills it, deliberately not a "handle N requests" loop — with a fixed count, adding a case later would block the script forever on a request the server had stopped waiting for. There is also nocommand -v wgetlookup; under this file'sset -euo pipefailthat aborts the whole script on any machine without wget.Verification
Built locally (arm64) —
api-only,standalone, andapi-only-freethreaded:python3+http.clientpresent in all threehttp_probeexercised inside the realapi-onlyimage against a live server: 200 → success, 404 → failure, connection refused → failureapi-onlyimage starts, the readiness loop goes green through the new probe,✅ Hindsight is running!, and/healthanswers 200 from outside the containerbash docker/standalone/test-start-all.shpasses in fullDockerfile.freethreadedis included — it ran the same probe and carried the same curl.