From ef1d8a18b861e5710834a0dc279d1213553dee2a Mon Sep 17 00:00:00 2001 From: Pavel Sokolov Date: Mon, 24 Aug 2026 08:39:04 +0300 Subject: [PATCH 1/2] Mark /opt/esp-matter as a safe git directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #27. The checkout belongs to root, because the build created it, while the documented invocation runs the 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 ESP-IDF base already marks $IDF_PATH the same way, which is why the two answered differently to one `git -C … rev-parse HEAD`. Two entries, both naming paths rather than `*`: - `/opt/esp-matter`, the superproject; - `/opt/esp-matter/*`, its submodules — git checks ownership per repository, and connectedhomeip brings dozens of nested ones, far too many to list. That suffix is version-dependent: measured as ignored by git 2.43, which this base ships, and honoured by 2.47, where it covers subdirectories but not the parent — hence both lines. Today the second is inert and harmless; it starts working when the base's git catches up, and until then a submodule wants a per-invocation `-c safe.directory=`, which the README shows. Be precise about what the narrow form buys, because the first draft of this comment overstated it: `safe.directory` matches a *path*, not an owner, so a repository bind-mounted over /opt/esp-matter would be trusted too. What it keeps is the ownership check everywhere else in the container, which `*` switches off wholesale. The entries are asserted rather than assumed — the build runs as root, so nothing else here would notice their absence, which is exactly how this gap survived until a consumer running under `-u` hit it. Three assertions, each measured: - both entries are present (`grep -qFx`, fail-closed on empty output); - no blanket `*` is — the base is overridable and pinned by digest, so a fork's base, or a future espressif/idf, could trust everything while this file claims path scoping. Upstream adds `$IDF_PATH` alone today, checked; - the repository actually reads from a uid that does not own it, **through the entrypoint** as well as directly. The entrypoint runs `set -e` and sources both export.sh scripts before the command, so a working command is not the same as a working README recipe. setpriv's own failures are separated from git's first (`setpriv … true`), since without CAP_SETGID `--clear-groups` fails and blaming safe.directory for that sends the next reader to the wrong place. All six paths of that layer were exercised in a plain container: both entries present, each one missing, a blanket `*` added, a broken entrypoint, and setpriv absent. The README section is scoped to what the image actually provisions — git under `-u`. A full firmware build as a non-root uid also wants a writable HOME and cache directories, which this image does not set up and the host image does; saying so beats implying otherwise. Not verified by building the image: `espressif/idf` will not pull on this machine (traffic interception drops the connection well short of its 3.4 GB layer). The behaviour behind every claim above was established in plain containers on the two git versions. The pull request builds all three esp-matter variants on both architectures. --- CLAUDE.md | 14 ++++++++ images/esp-matter/Dockerfile | 62 ++++++++++++++++++++++++++++++++++++ images/esp-matter/README.md | 37 +++++++++++++++++++++ 3 files changed, 113 insertions(+) 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..c4b4eb0 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,42 @@ 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. + { git config --system --get-all safe.directory | grep -qFx '*' \ + && { echo "a blanket safe.directory '*' is set - this image claims path scoping" >&2; \ + exit 1; } || true; } && \ +# 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: From 04b48ec2e90f1213e4f768b6cdcd58926dc90ba3 Mon Sep 17 00:00:00 2001 From: Pavel Sokolov Date: Mon, 24 Aug 2026 08:57:57 +0300 Subject: [PATCH 2/2] Spell the blanket-entry assertion as an if MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot read `grep -qFx '*' && { …; exit 1; } || true` as self-defeating. It is not: a brace group is not a subshell, so `exit` leaves the whole RUN and `||` never sees it — checked directly, the layer exits 1 with the message and the following command does not run. But the form invites that reading, and the next reader gets to make the same call, so it is now an `if`. Same behaviour, verified both ways: passes with no blanket entry, fails the layer with one. --- images/esp-matter/Dockerfile | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/images/esp-matter/Dockerfile b/images/esp-matter/Dockerfile index c4b4eb0..29e61b4 100644 --- a/images/esp-matter/Dockerfile +++ b/images/esp-matter/Dockerfile @@ -161,9 +161,13 @@ RUN source ${IDF_PATH}/export.sh && \ # 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. - { git config --system --get-all safe.directory | grep -qFx '*' \ - && { echo "a blanket safe.directory '*' is set - this image claims path scoping" >&2; \ - exit 1; } || true; } && \ +# `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.