diff --git a/CLAUDE.md b/CLAUDE.md index 4bbe9e7..3fb5ba6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -311,6 +311,20 @@ before changing anything here. Ubuntu 24.04 ships PEP 668, so there is no system interpreter to install into. Either way call tools by name, never by absolute path: `/opt/esp/python_env/idf5.4_py3.12_env/bin/python3` was tried and reverted — it pins a version-stamped directory that a version bump invalidates. +- **A repository the image built is owned by root, and the documented invocation + runs as the caller's uid**, so git refuses it as "dubious ownership" — in the + container only. Every image whose git matters marks its own trees: + `espressif/idf` does `$IDF_PATH`, esp-matter does `/opt/esp-matter`, and host + does `'*'`. The scope differs on purpose: the ESP images mark directories they + created, while host exists to run against a checkout mounted at a path it cannot + know (a job `container:` gets `/__w//`). A path entry is not a + provenance check either — git matches the path and not the owner — so what the + narrow form buys is that the ownership check still applies everywhere else. + Submodules are separate repositories, so esp-matter adds `/opt/esp-matter/*` + beside the plain path: that suffix covers subdirectories on git 2.47 and is + ignored by the 2.43 its base ships, so until the base moves a submodule wants a + per-invocation `-c safe.directory=`. Measure before restating either + behaviour; both were established by running the two versions. - esp-matter activates both environments at runtime through `ENTRYPOINT ["/opt/esp/esp_matter_entrypoint.sh"]`; overriding that entrypoint in a derived image silently breaks the "no sourcing needed" promise in its README. diff --git a/images/esp-matter/Dockerfile b/images/esp-matter/Dockerfile index cde9bc4..29e61b4 100644 --- a/images/esp-matter/Dockerfile +++ b/images/esp-matter/Dockerfile @@ -72,6 +72,32 @@ RUN git init esp-matter && \ cd connectedhomeip/connectedhomeip && \ ./scripts/checkout_submodules.py --platform esp32 linux --shallow +# The checkout above belongs to root, because the build created it, while the +# documented invocation runs this image as the caller's uid (`-u $(id -u):$(id -g)`, +# which is how build output in a bind-mounted workspace stays owned by the caller). +# Git then refuses the repository as "dubious ownership" - inside the container +# only, on a tree the image built itself. The base image already marks $IDF_PATH +# the same way, which is why ESP-IDF and ESP-Matter answered differently to the +# same `git -C … rev-parse HEAD`; this closes that asymmetry. +# +# Named paths rather than `*`, as upstream does. Be precise about what that buys: +# `safe.directory` matches a *path*, with no notion of who owns it, so this does +# not verify provenance - a caller who bind-mounts their own repository over +# /opt/esp-matter gets it trusted too. What the choice does buy is that everything +# *else* in the container stays subject to the ownership check, which `*` would +# switch off wholesale. The caller is the trust boundary either way. +# +# Two entries, because git checks ownership per repository and a submodule is its +# own: connectedhomeip brings dozens of nested ones, far too many to list. The +# `/*` suffix covers exactly those, and it is version-dependent - verified as +# ignored by git 2.43 (which this base ships) and honoured by 2.47, where it +# matches subdirectories but not the parent, hence both lines. On today's base the +# second line is inert and harmless; it starts working when the base's git catches +# up, and until then a reader that needs a submodule passes +# `-c safe.directory=` for it. +RUN git config --system --add safe.directory /opt/esp-matter && \ + git config --system --add safe.directory '/opt/esp-matter/*' + # Answers "what is actually in here" without starting the container. LABEL org.opencontainers.image.revision="${ESP_MATTER_REF}" \ org.opencontainers.image.version="${ESP_MATTER_VERSION}" \ @@ -124,6 +150,46 @@ RUN source ${IDF_PATH}/export.sh && \ { [ "${idf_reported}" = "ESP-IDF ${IDF_VERSION}" ] \ || { echo "expected 'ESP-IDF ${IDF_VERSION}', got '${idf_reported}'" >&2; \ exit 1; }; } && \ +# The safe.directory entries, asserted rather than assumed: the build runs as root, +# so nothing else here would notice their absence - which is exactly how the gap in +# this image survived until a consumer running under `-u` hit it. Each grep is +# fail-closed (an empty `git config` output fails it). + git config --system --get-all safe.directory | grep -qx /opt/esp-matter && \ + git config --system --get-all safe.directory | grep -qFx '/opt/esp-matter/*' && \ +# And the *absence* of a blanket entry, which is the half a positive check cannot +# see: the base image is overridable and pinned by digest, so a fork's base - or a +# future espressif/idf - adding `safe.directory = *` would leave every repository +# trusted while this file and the README claim otherwise. Today's upstream adds +# `$IDF_PATH` alone (checked), so this asserts a property rather than guessing one. +# `if`, not `grep && { … } || true`: that form is correct (a brace group is not a +# subshell, so `exit` leaves the layer rather than being caught by `||`) but reads +# as though the `|| true` neutralised it. Same behaviour, one obvious reading. + { if git config --system --get-all safe.directory | grep -qFx '*'; then \ + echo "a blanket safe.directory '*' is set - this image claims path scoping" >&2; \ + exit 1; \ + fi; } && \ +# Then the effect, from a uid that does not own the tree. setpriv's own failures +# are separated from git's first: without CAP_SETGID `--clear-groups` fails, and +# blaming safe.directory for that would send the next reader to the wrong place. + { if ! command -v setpriv >/dev/null 2>&1; then \ + echo "setpriv unavailable - safe.directory asserted by configuration only"; \ + elif ! setpriv --reuid=65534 --regid=65534 --clear-groups true 2>/dev/null; then \ + echo "setpriv cannot drop privileges here - asserted by configuration only"; \ + else \ + setpriv --reuid=65534 --regid=65534 --clear-groups \ + git -C /opt/esp-matter rev-parse HEAD >/dev/null \ + || { echo "git still refuses /opt/esp-matter from an unprivileged uid" >&2; \ + exit 1; }; \ +# The documented invocation goes through the entrypoint, which runs `set -e` and +# sources both export.sh scripts before the command - so the command working is +# not the same as the recipe in the README working. This runs that path, as the +# same unprivileged uid. + setpriv --reuid=65534 --regid=65534 --clear-groups \ + /opt/esp/esp_matter_entrypoint.sh git -C /opt/esp-matter rev-parse HEAD >/dev/null \ + || { echo "the documented entrypoint path fails for an unprivileged uid" >&2; \ + exit 1; }; \ + echo "safe.directory verified from an unprivileged uid, entrypoint included"; \ + fi; } && \ echo "ESP-Matter ${ESP_MATTER_VERSION} installation verified successfully" # Use custom entrypoint to activate environments (like entrypoint.sh in esp-idf image) diff --git a/images/esp-matter/README.md b/images/esp-matter/README.md index 0854228..2335665 100644 --- a/images/esp-matter/README.md +++ b/images/esp-matter/README.md @@ -250,6 +250,43 @@ services: command: idf.py build ``` +## Reading the Image's Git Trees as Another User + +Passing `-u $(id -u):$(id -g)` keeps build output in a mounted workspace owned by +you rather than by root. This section covers **git** under that flag, which is what +the image provisions for; a full firmware build as a non-root uid additionally +wants a writable `HOME` and cache directories, which this image does not set up +(the [host image](../host/README.md#running-as-the-invoking-user) does). + +- **`$ESP_MATTER_PATH` is marked safe, so git works there.** The checkout belongs + to root — the build created it — while the process is your uid, and git refuses + that as *dubious ownership* unless told otherwise. The image adds + `safe.directory /opt/esp-matter` to its system config, matching what the ESP-IDF + base already does for `$IDF_PATH`: + + ```bash + docker run --rm -u $(id -u):$(id -g) git -C /opt/esp-matter rev-parse HEAD + ``` + +- **Its submodules are covered only on a new enough git.** Git checks ownership per + repository, so each submodule is a separate decision, and connectedhomeip brings + dozens of nested ones — far too many to name. The image therefore also adds + `safe.directory /opt/esp-matter/*`, which covers subdirectories on git 2.47 and + is ignored by the 2.43 the current base ships. Until the base moves, read a + submodule with a per-invocation exception, which also keeps the trust decision + with whoever makes it: + + ```bash + docker run --rm -u $(id -u):$(id -g) \ + git -c safe.directory=/opt/esp-matter/connectedhomeip/connectedhomeip \ + -C /opt/esp-matter/connectedhomeip/connectedhomeip rev-parse HEAD + ``` + +Both entries name paths rather than `*`. That is not a provenance check — git +matches the path, not the owner, so a repository you bind-mount over +`/opt/esp-matter` would be trusted too. What it does keep is the ownership check +everywhere else in the container, which `*` would switch off wholesale. + ## Environment Variables The image sets the following Matter-specific variables: