Add the Docker client to the host image - #25
Merged
Merged
Conversation
Closes #24. Every POSIX job in the consuming project now runs in this image except the one that stands a broker up for its integration suite: the runner starts that broker as a container, and there was no `docker` in here to start it with. So that job alone stayed pinned to the runner, and the apt-plus-build-paho action survived for it. The client only. The containers it starts are siblings, spoken to through a socket the caller mounts, and a daemon in here would be actively wrong. That rules out apt: Ubuntu 24.04 has no client-only package (`docker-cli` does not exist, and `docker.io` pulls in the daemon, containerd and runc), while Docker's own apt repository would put a third-party source and a signing key in the image without buying automatic updates - the label has to name an exact version, so the package would be pinned there too, and a pinned package is the same manual bump with more moving parts. It therefore comes from Docker's static release the same way lychee does: a literal checksum per architecture, verified before anything is unpacked. Docker publishes no .sha256 beside those tarballs, so both sums were computed here and confirmed by a second download. Both downloads now also refuse a redirect off https, which the digest already covers and costs nothing to state. The tarball carries the whole engine and exactly one member is extracted - an unqualified `tar -xz` would install dockerd, containerd and runc alongside it. That is the whole of the "no daemon" promise, so the verification layer asserts it rather than trusting it: dockerd, containerd, runc and docker-proxy must all be absent, and a later bump reaching for a plugin cannot widen the extraction unnoticed. The client's version is asserted too, by equality rather than by substring - it is a label the consumer tests its own tree against, and `*29.7.2*` would also accept 29.7.21. `docker version --format` prints the client's version before it goes on to ask for a daemon that is not here, so an empty answer from a broken binary fails the comparison, which is the direction this has to fail in. The README gains the section the client needs and loses the sentence it contradicts, and says the three things a reader can be hurt by: - mounting the socket is granting root on the host, so it never belongs in a job running untrusted code - a `pull_request` from a fork being the case worth naming, since the CI example on the same page uses that trigger; - a sibling's `-v` paths are resolved by the daemon, on the host, and a path that does not exist there is silently created empty rather than refused; - no CLI plugins at all - no compose and no buildx. `docker build` still works, falling back to the daemon's classic builder, so only BuildKit-only features are out. Both socket recipes are run against the built image: as root the mounted socket works, and an unprivileged uid needs the owning group - read from *inside* a container, because Docker Desktop proxies the socket in as `0:0` while on Linux it carries the host's docker gid, and a gid read on the Mac would be the wrong number. What the issue asked to leave out stays out: no Node.js (the runner mounts its own), no mosquitto-clients (the readiness probe is stdlib sockets), no compose plugin.
There was a problem hiding this comment.
Pull request overview
This PR updates the repository’s host developer image to include the Docker client only (no daemon), enabling consuming CI jobs to start sibling containers via a mounted Docker socket.
Changes:
- Install the Docker CLI from Docker’s static release tarball with per-architecture SHA-256 verification.
- Add a build-time verification step that asserts the Docker CLI version and checks for absence of daemon-related binaries.
- Update documentation (root README + host README) and internal pinning notes to reflect the new Docker CLI pin/labels.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates the image roster description to mention the docker CLI in the host image. |
| images/versions.json | Updates the documentation note about host Dockerfile ARG-pinned tools to include DOCKER_VERSION. |
| images/host/README.md | Documents Docker CLI availability, sibling-container workflow, and security implications of mounting docker.sock. |
| images/host/Dockerfile | Adds Docker CLI install (static tarball + checksum) and verification/labeling for dev.jethome.docker-cli.version. |
| CLAUDE.md | Updates repo guidance to include DOCKER_VERSION alongside other host tool pins. |
| .github/dependabot.yml | Updates commentary explaining why certain host pins (incl. Docker tarball checksums) are excluded from Dependabot automation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot's point on #25: `command -v` answers for the search path only, while the extraction that assertion guards runs with / as its working directory. A widened `tar` would drop /dockerd - not on PATH, and so invisible to the very check written to catch it. `find / -xdev` sees it, every name the tarball carries is listed rather than just the daemon, and the walk costs under a second. Verified from both sides: the layer stays green on the real image, and a stub binary planted at /dockerd fails it.
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.
Closes #24.
Every POSIX job in the consuming project now runs in this image except the one
that stands a broker up for its integration suite: the runner starts that broker
as a container, and there was no
dockerin here to start it with. So that jobalone stayed pinned to the runner, and the apt-plus-build-paho action survived
for it.
The client, and only the client
The containers it starts are siblings, spoken to through a socket the caller
mounts. A daemon in this image would be actively wrong, and that shapes where the
binary comes from:
docker.io(Ubuntu)docker-clihas no candidateSo it comes from Docker's static release, the same shape
lycheealready useshere: a literal SHA-256 per architecture, verified before anything is
unpacked. Docker publishes no
.sha256beside those tarballs, so both sums werecomputed for this change and confirmed by a second download. Both downloads now
also refuse a redirect off https — the digest already covers that, and stating it
costs nothing.
The tarball carries the whole engine, and exactly one member is extracted. That
single argument is the entire "no daemon" promise, so the verification layer
asserts it rather than trusting it:
dockerd,containerd,runcanddocker-proxymust all be absent, and a later bump reaching for a plugin cannotwiden the extraction unnoticed. (Checked from the other side too: with a stub
dockerdplanted onPATH, the assertion fails the layer.)The version is asserted by equality, not by substring — it is a label the
consumer tests its own tree against, and
*29.7.2*would also accept29.7.21.docker version --formatprints the client's version before it goes on to askfor a daemon that is not here, so an empty answer from a broken binary fails the
comparison, which is the direction this has to fail in.
What the README now says, because a reader can be hurt by it
daemon can run
--privilegedwith/mounted. So it never belongs in a jobrunning untrusted code — a
pull_requestfrom a fork being the case worthnaming, since the CI example on that same page uses that trigger.
-vpaths are resolved by the daemon, on the host. A path thatdoes not exist there is silently created empty rather than refused, and the
service then starts with defaults while the test fails somewhere else entirely.
compose, and nobuildx.docker buildstillworks, falling back to the daemon's classic builder, so only BuildKit-only
features are out.
Both socket recipes were run against the built image: as root the mounted socket
works, and an unprivileged uid needs the owning group — read from inside a
container, because Docker Desktop proxies the socket in as
0:0while on Linuxit carries the host's docker gid, and a gid read on the Mac would be the wrong
number.
Left out, as the issue asked
No Node.js (the runner mounts its own), no
mosquitto-clients(the readinessprobe is stdlib sockets), no compose plugin.
Note on the checks
images/versions.jsonis in every image workflow'spaths:filter, so this PRrebuilds esp-idf, esp-matter and platformio as well as host. That is the
documented trade of keeping the version data in one file, not a defect.