fix(sanity): verify the ControlPath ssh RESOLVES, not the dir the image creates
Lint / hadolint (push) Successful in 9s
Lint / skill-floor (push) Successful in 13s
Lint / doc-drift (push) Successful in 12s
Lint / actionlint (push) Successful in 24s

recreate-sanity-check.sh asserted `/tmp/sshcm exists with mode 700` and printed
a green tick while every ssh in the container died rc=255. It was right about
what it checked: the breakage was in the directory a CONFIG named, not the one
the image creates, and the old check could not see the disagreement between them.

Found on the v1.9.2 first boot on emb-7kj4vr4g. A durable ~/.pi/ssh/config,
hand-written into the ~/.pi named volume by the previous session so it would
survive the recreate, declared `ControlPath /tmp/ssh-cm/%C` — with a hyphen.
Nothing here creates that path; the canonical spelling is /tmp/sshcm, identical
in Dockerfile.base, entrypoint-user.sh, recreate-sanity-check.sh and
smoke-test.sh. ControlMaster auto with an unusable ControlPath does not degrade
to an unmultiplexed connection — it fails hard:

    unix_listener: cannot bind to path /tmp/ssh-cm/<hash>: No such file or directory

rc=255, remote command never runs.

Now: resolve the ControlPath ssh itself would use via `ssh -G` and require its
parent to exist and be writable. -G applies real config precedence (first-value-
wins, the system drop-in, Include, an -F override), so it answers "which rule
captured this host" instead of re-implementing the guess, and it never opens a
connection — 0.116s for 48 hosts.

This also puts a check under a caveat documented in prose in Dockerfile.base
("SSH client defaults") and verified nowhere: a per-host ControlPath under a
read-only bind-mounted ~/.ssh gives the identical failure, "Read-only file
system". On the machine this was built on that is 16 of 50 hosts — freeipa-1..6,
gitea.egl.lan, runner-1..3, tor-ms22 — never reported by anything before.

Severity split is deliberate. Default ssh precedence legitimately lands in the
read-only ~/.ssh for any host whose own config pins it there, and the supported
workaround (ssh -F ~/.ssh-local/config, generated every start by
setup-lan-access.sh) already exists, so that is a warn. Failing it would paint
the script red on every run of every device, and a check that fires benignly
every time is one you learn to ignore — the same reasoning that keeps
lint-shell.sh at -S error. The sidecar route is prescribed, so there it is a
hard fail. Host lists cap at six names plus a count: unreadable output is
ignored output.

Teeth proven both directions, each sabotage confirmed by diff BEFORE the result
was believed: ControlPath -> nonexistent dir => rc=1; -> existing-but-read-only
dir => rc=1; restored => rc=0 with the sidecar byte-identical.

No config was added to fix the original problem — the fix was to DELETE
~/.pi/ssh/config, a third hand-maintained copy of what setup-lan-access.sh
already generates from version control on every container start, with fewer
features and one typo. The host-owned ~/.ssh/config is left alone on purpose:
those ~/.ssh/cm paths are correct on the host, where ~/.ssh is writable.

Also lint-shell.sh: the SC2088 count in the severity-choice rationale said 19
and is now 20, with the reproduce command recorded. That command needs a `$ `
prefix — a comment whose first word is "shellcheck" is parsed as a directive,
and the malformed one tripped SC1072/SC1073 at severity error. The gate caught
it on the very commit that introduced it.
This commit is contained in:
Joakim Persson
2026-09-15 22:59:43 +02:00
parent c7d369f28d
commit f25efa074d
3 changed files with 179 additions and 3 deletions
+67
View File
@@ -13,6 +13,73 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`).
## Unreleased
**`scripts/recreate-sanity-check.sh` asserted that `/tmp/sshcm` exists while every
`ssh` in the container was dying `rc=255`, and it was right to — it was checking
the directory the *image* creates, and the breakage was in the directory a
*config* named.** Found on the v1.9.2 first boot on `emb-7kj4vr4g`. A durable
`~/.pi/ssh/config` (hand-written into the `~/.pi` named volume by the previous
session, so it would survive the recreate) declared `ControlPath
/tmp/ssh-cm/%C` — with a hyphen. Nothing in this repo creates that path; the
canonical directory is `/tmp/sshcm`, spelled the same way in four places
(`Dockerfile.base`, `entrypoint-user.sh`, `recreate-sanity-check.sh`,
`smoke-test.sh`). Result:
```
unix_listener: cannot bind to path /tmp/ssh-cm/<hash>: No such file or directory
```
`rc=255`, and the remote command never ran at all — `ControlMaster auto` with an
unusable `ControlPath` fails hard rather than falling back to an unmultiplexed
connection. The old check passed truthfully, about the wrong object. **Two
independent facts about the same subsystem can both be true while the subsystem
is dead; a check that asserts only one of them cannot see their disagreement.**
**New in `recreate-sanity-check.sh`: resolve the ControlPath `ssh` itself would
use, via `ssh -G`, and require its parent to exist and be writable.** `-G`
applies real config precedence — first-obtained-value-wins, the system drop-in,
`Include`, an `-F` override — so it answers "which rule captured this host"
instead of re-implementing the guess. It never opens a connection: measured
0.116 s for 48 hosts.
This also puts a check under a caveat that had been documented in prose in
`Dockerfile.base` ("SSH client defaults") and verified nowhere: a per-host
`ControlPath ~/.ssh/cm/%r@%h:%p` inherited from a **read-only** bind-mounted
`~/.ssh` produces the identical failure, `cannot bind … Read-only file system`.
On the machine where this was built that is **16 of 50 hosts** — `freeipa-1..6`,
`gitea.egl.lan`, `runner-1..3`, `tor-ms22` and more — none of which had ever been
reported by anything.
**The two routes get different severities, deliberately.** Default `ssh`
precedence legitimately lands in the read-only `~/.ssh` on any host whose own
config pins it there, and the supported workaround (`ssh -F
~/.ssh-local/config`, generated every container start by `setup-lan-access.sh`)
already exists — so that is a `warn`. Making it a failure would paint the script
red on every run of every device, and **a check that fires benignly every time is
one you learn to ignore**, which is the same reasoning that keeps `lint-shell.sh`
at `-S error`. The sidecar route is the prescribed one, so there an unusable
directory is a hard `fail`. Host lists are capped at six names plus a count for
the same reason: unreadable output is ignored output.
Teeth proven in both directions, with each sabotage confirmed by `diff` *before*
the result was believed — a vacuous sabotage that silently fails to apply
reports "gate passed" and is worse than no test:
| Sabotage | Class | Result |
|---|---|---|
| `ControlPath` → nonexistent dir | the original hyphen bug | `✗` `rc=1` |
| `ControlPath` → existing but read-only dir | the `Dockerfile.base` caveat | `✗` `rc=1` |
| restored | — | `✓` `rc=0`, sidecar byte-identical |
No config was added to this repo to fix the original problem, because the fix was
to **delete** the offending file: `~/.pi/ssh/config` was a third hand-maintained
copy of what `setup-lan-access.sh` already generates from version control on
every start, with fewer features (no `known_hosts` sidecar, no
`StrictHostKeyChecking accept-new`) and one typo. The host-owned `~/.ssh/config`
is also left alone on purpose — those `~/.ssh/cm` paths are correct *on the host*,
where `~/.ssh` is writable, and the container-side override is the right layer.
---
**The size numbers on the Docker Hub page were the only claim in these docs with
nothing in the repo to check them against, and they had gone 20% wrong across
eight releases.** Every other claim `scripts/check-doc-drift.sh` guards is