Re-derive three teardown justifications, and sanitize a cmdline on screen (refs #345, refs #348, refs #31, refs #344)

[vyskocilm]

refs #345, refs #348, refs #31, refs #344

Four comments describing something other than what the code does, plus the half of #344 that needs no decision.

#344's matcher is NOT fixed here, and the reason is a design question rather than a missing engine — see the last section.

#345 — the pidfd claim, measured rather than restated

orphansweep.go said "while the fd is held the kernel cannot recycle that pid number… there is no residual pid-reuse window at all". Measured false on this host, in a fresh user+pid namespace writing ns_last_pid:

pinned pid           = 7 (reaped, pidfd still open)
fdinfo says          = Pid: -1  NSpid: -1
next child got pid   = 7  (collision=true)
signal through pin   = no such process
new occupant alive   = true

The number recycles. What the pin guarantees is the other half, and it is the half this sweep needs: the pidfd refers to the task it opened, dies with it, and returns ESRCH rather than reaching the new occupant — #303's central claim, confirmed by the same run. That is what makes the kill fail closed.

The comment now says that, and says what follows from it: anything reading /proc/<pid> by number after the open may be describing a different task, which is exactly what the starttime and namespace-inode checks are for. reap.go and teardown.go already word it this way.

Why it was worth correcting with no exploitable consequence today: the false version is the licence a later edit needs to act on the number inside that window — a kill(2) fallback, a cgroup lookup, a /proc/<pid> write.

#348 — the wrapper justification, re-derived

reap.go justified identity-by-path with the wrapper case: a podman forwarding to the host, engine parented to the host's systemd, process tree a lie. Closed since Tier C, two independent ways:

• The argv names guest paths only. Spec resolves --root, --runroot and the socket through Engine.guestPath, which refuses a path no graft exposes, so the argv is entirely under /snug. Those paths resolve only inside the engine's derived mount namespace — an engine that exec'd on the host could not open its own store or bind its own socket, so it cannot start.
• Preflight P1 refuses the shim by name, before that. Measured on this host: "podman resolves to /usr/bin/podman, a host-escape helper (distrobox-host-exec) … snug will not run the container engine through it."

Path identity stays, for the reason that holds now: it verifies the cascade (the engine is pid 1 of its own namespace, so the collapse fells it and every container), and a recorded pid is not an option because libpod's are numbered in the engine's own namespace (#167).

A wrapper that re-execs inside the derived view is still possible and is not that case: it is a descendant in the engine's pid namespace, so the cascade already covers it. Named, per the issue's step 3.

TestEngineArgvNamesOnlyGuestPaths makes the first half checkable — every path-shaped argv element under /snug, and spec.Sock still the host path because P1's waitForSocket and the container proxy dial it. Positive control asserts the argv carries at least three paths, so it cannot pass on a Spec that stopped resolving anything.

#31 — step 4 contradicted its own code 210 lines below

inengine.go described step 4 as "a plain private COPY of the host tree, deliberately NOT derived from the resolved Policy — that is Tier C's later pieces". The body: :257 setns into the sandbox's mount namespace, :268 unshare, :278 MS_REC|MS_PRIVATE, :292-312 move_mount every graft and MOUNT_ATTR_RDONLY where it is AccessRO.

Step 4 is now those four calls in order (4a–4d), states that p.Grafts is their author (invariant 6), and keeps the distinction the stale text got right: the view is not the container bind filter, and neither may be read as the other. Sub-lettered rather than renumbered, so nothing downstream of it shifts.

Same statement at enginefork.go's #146 inventory — "no derived mount VIEW, no grafts — those are issue #125's later pieces" — now says C0 is the precondition for the view rather than a piece that shipped without it.

#344, partial — the sink that needs no decision

describe() rendered a matched process's command line with only NUL replaced. A command line is not snug's text: any process can put anything in its own argv, so an ESC there erases the line snug printed above it and a bidi override reverses the order the rest reads in. It goes through policy.VisibleText now.

TestNoSnugScreenEmitsARawControlCharacter drives the --dry-run screen and does not reach internal/engine, so TestDescribeSanitisesACommandLine does, with a live decoy. Mutation observed: without the sanitiser, describe emitted '\x1b' raw, from a command line snug does not own.

That test's two false starts are worth carrying, because both are harness traps #344 names:

attempt	what happened
/bin/sleep "30<ESC>[1A…"	sleep rejects the duration, the decoy becomes a zombie, and a zombie's cmdline reads back empty — the positive control caught it
single-command sh -c	exec-optimised, crafted argv gone from the cmdline

Working decoy: sh -c 'sleep 30; :' <crafted argv>. The empty-cmdline window (#318) is polled past rather than raced — the control caught that too, on the first run.

#344's matcher: handed back, and it is not the engine

Feasibility checked before starting, as asked. The core defect is measurable here without podman and the ratchet for it is pure. What blocks it is a design decision:

paths() returns the host socket; the engine's cmdline names the guest one, so stopLocked step 3's waitQuiet returns nil on its first poll every run and the SIGKILL fallback and the WARNING screen are unreachable. But swapping in the guest path makes the matcher match a process that is supposed to be alive: stopLocked runs from live Go code with the engine still up by its own insistence, exclude holds only snug's pid and the reaper's, and the engine then exits on its own idleTimeout (10s) inside a quietBudget of 15s. Every clean @podman-socket run would sit there — the 15.3s the issue measured with a decoy.

So step 3 has to learn either that the engine is expected alive (which needs an engine pid exclude does not have) or that its verification belongs where the cascade actually happens (the reaper path, after snug's process dies). That is a decision about what teardown verifies and when, and #348's re-derivation above is the argument that it is the second — but it is not mine to make. Nothing in this PR presumes an answer, and reap.go's accident sentence now records the measured mismatch and points at #344 rather than reading as though the matcher were correct.

Gate

make gate green — 12 packages, no failures. make integration not run: no engine can start in this container (no /etc/subuid range, no pasta, /usr/bin/podman is the distrobox shim), and the same failures reproduce on main. Nothing here touches an integration surface that could run.