From 3a2c379b719ecca010e935ae40b6ee430c843fcb Mon Sep 17 00:00:00 2001 From: mwiegand Date: Thu, 14 May 2026 23:42:36 +0200 Subject: [PATCH] plan(left4me-overlay): idmap lowerdir bind mounts for cross-uid copy-up Persist the implementation plan for adding idmapped bind mounts to left4me-overlay so that overlay copy-up from l4d2-sandbox-owned lower layers (script-built overlays) produces left4me-owned upperdir entries the gameserver can write. Mechanism verified end-to-end on ovh.left4me in a temp dir on 2026-05-14. --- .../plans/2026-05-14-overlay-idmap.md | 325 ++++++++++++++++++ 1 file changed, 325 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-14-overlay-idmap.md diff --git a/docs/superpowers/plans/2026-05-14-overlay-idmap.md b/docs/superpowers/plans/2026-05-14-overlay-idmap.md new file mode 100644 index 0000000..c3735e0 --- /dev/null +++ b/docs/superpowers/plans/2026-05-14-overlay-idmap.md @@ -0,0 +1,325 @@ +# Idmapped lowerdirs for left4me kernel-overlayfs + +## Context + +Kernel-overlayfs copy-up preserves the lower-layer file's owner and mode in the +upperdir. Script overlays today are built by `left4me-script-sandbox` running as +uid `l4d2-sandbox`, and the helper finalizes them as `l4d2-sandbox:l4d2-sandbox +0755`. When the L4D2 server (uid `left4me`) tries to write into a directory +that exists only in the lower layer — e.g. SourceMod's `addons/sourcemod/logs/` +for log rotation — copy-up succeeds but the result is `l4d2-sandbox`-owned, so +the write fails `EACCES`. Workshop overlays are unaffected because the web app +(uid `left4me`) builds them as `left4me`-owned with symlinks into a `left4me`- +owned cache. + +We considered four fixes (chown-flip on every rebuild, shared group, collapse +sandbox uid into `left4me`, idmapped lowerdir bind mounts). The user chose the +idmap path: disk state stays untouched, the mount stack remaps `l4d2-sandbox → +left4me` at mount time, kernel-overlayfs sees a `left4me`-owned lower layer, +and copy-up creates upperdir entries owned by `left4me` naturally. No +ownership flipping, no shared group, no security regression. + +Outcome: `sm_cvar` writes succeed, SM logs land in `runtime//upper/...`, +and any future "writes into a sandbox-built lower layer" works without +left4me-specific plumbing. + +## Environment confirmed + +Test server: Debian Trixie, kernel `6.12.86+deb13-amd64`, util-linux supports +`mount --map-users ::`. Idmapped lowerdirs for +overlayfs landed mainline in 6.6, so 6.12 is fine. Verified end-to-end on +`/var/lib/left4me/` (ext4) in a temp dir on 2026-05-14: + +1. Source dir owned `l4d2-sandbox:l4d2-sandbox` (uid 981). +2. `mount --bind --map-users=981:980:1 --map-groups=981:980:1 src dst` — `dst` + view shows uid 980 (left4me). +3. Overlay mount with the idmapped path as `lowerdir=` — merged view also + shows uid 980. +4. `sudo -u left4me touch merged/addons/sourcemod/logs/L_test.log` — succeeds. + `sudo -u left4me bash -c "echo x >> merged/file-from-sandbox.txt"` — + succeeds (copy-up of existing file). +5. `upper/` after writes is entirely `left4me`-owned (uid 980). + +Caveat surfaced during testing: `--map-users` direction is **on-disk uid +first**, not "inner-namespace uid". The util-linux man page calls it +`::` but `` means "the filesystem's native view" +(on disk) and `` means "what the mount exposes outward". Easy to get +wrong; do not trust the man page wording. + +## Approach + +The privileged mount helper grows one step before the overlay mount: for each +lowerdir whose owning uid is `l4d2-sandbox`, create an idmapped bind mount at +`runtime//idmap/` that remaps that uid to `left4me`. Use the +idmapped paths (instead of raw paths) in the `lowerdir=` string passed to +`mount -t overlay`. On umount, tear the idmap binds down after the overlay +itself is unmounted. + +Lowerdirs already owned by `left4me` (workshop builds, `installation/`, +caches) bypass the idmap step and are used as-is, so workshop overlays keep +working without behavior change. + +## Execution shape + +Implementation will be driven by `superpowers:subagent-driven-development`: +fresh implementer subagent per task, followed by a spec-compliance reviewer +then a code-quality reviewer. The project's `AGENTS.md` forbids git +worktrees, so all work happens in the live tree. Commits land directly on +`master` per the user-confirmed project pattern. + +Tasks ordered for review-friendly progression. Each task is independently +committable; the deploy/verify step at the end exercises the whole chain on +the real test server. + +### Task 1 — Idmap bind mounts in `left4me-overlay` + +Edit `deploy/files/usr/local/libexec/left4me/left4me-overlay` and +`l4d2host/tests/test_overlay_helper.py` together (TDD: write failing +PRINT_ONLY-mode tests first, then make them pass). + +Behavior to add: +- Resolve `l4d2_sandbox_uid` and `left4me_uid` (and gids) via `pwd.getpwnam` + / `grp.getgrnam`. Hard fail with a clear message if either is missing. +- On `mount `: before constructing the `lowerdir=` string, for each + resolved lowerdir, stat it; if the top-level dir's `st_uid` equals + `l4d2_sandbox_uid`, create `runtime//idmap/` (mode `0700`, + root-owned), and if it's not already a mountpoint, exec `mount --bind + --map-users=::1 + --map-groups=::1 `. Use + numeric uids/gids in the argv. Substitute the idmap path into the + `lowerdir=` colon string in place of the original path. +- On `umount `: after the existing `umount` of `merged`, iterate + `runtime//idmap/*`, `umount` each that is a mountpoint, then + `shutil.rmtree(runtime//idmap, ignore_errors=True)`. Idempotent. +- PRINT_ONLY mode emits the bind-mount argv (one line per bind) before the + overlay-mount argv, same shell-quoting style. + +Tests to add to `test_overlay_helper.py` (reuse PRINT_ONLY harness): +- `test_mount_idmaps_sandbox_owned_lowerdir` — tmp lower owned by + faked-sandbox uid, assert helper emits `mount --bind --map-users=...` + argv and the overlay `lowerdir=` references the idmap path. +- `test_mount_skips_idmap_for_left4me_owned_lowerdir` — assert no bind + argv, raw path in `lowerdir=`. +- `test_umount_unwinds_idmap_binds` — pre-seed an idmap subdir as a + mountpoint sentinel; assert the umount sequence in PRINT_ONLY includes + the bind teardown after the overlay umount. + +Uid lookup in tests: monkeypatch `pwd.getpwnam` to return synthetic uids +matching what the test's `chown` set up. (No root required.) + +### Task 2 — Deploy-artifact regression test + +Edit `deploy/tests/test_deploy_artifacts.py`. Add a single test that opens +`deploy/files/usr/local/libexec/left4me/left4me-overlay` and asserts the +strings `--map-users` and `runtime/` followed by `idmap` (or similar +identifying marker) are present. Cheap guard against silent regression of +the deploy artifact. + +### Task 3 — Deploy README mirror note + +Edit `deploy/README.md`. Add one line under the existing ckn-bw mirror +notes flagging that the helper file change must be picked up by +`bundles/left4me/` in ckn-bw (no new group, user, or unit needed). + +### Task 4 — Persist the plan in-repo + +Per `AGENTS.md`: "the persisted artifact must end up under +`docs/superpowers/` and be committed." Copy this scratch plan to +`docs/superpowers/plans/2026-05-14-overlay-idmap.md` and commit it. Do +this as a separate commit from Task 1 so the plan lands before the +implementation. + +### Task 5 — Deploy and verify on `left4.me` + +Out-of-band, after the code tasks land: +1. ckn-bw apply (or scp the helper into place on the test server) to get + the new helper deployed. +2. Stop server 2: `sudo systemctl stop left4me-server@2`. +3. Clear the stale `l4d2-sandbox`-owned upperdir SM dirs: + `sudo rm -rf /var/lib/left4me/runtime/2/upper/left4dead2/addons/sourcemod/{logs,data}`. +4. Start server 2: `sudo systemctl start left4me-server@2`. +5. Confirm `journalctl -u left4me-server@2 -o cat -n 50` shows the new + `mount --bind --map-users=...` line. +6. RCON `sm_cvar nb_update_frequency 0.0333`. Expect no `Platform returned + error: "Permission denied"` log line. +7. `sudo ls -ln /var/lib/left4me/runtime/2/upper/left4dead2/addons/sourcemod/logs/`. + Expect uid 980 (left4me). +8. Restart again to confirm idempotency: idmap binds set up fresh, no + leftover mounts from prior start. + +## Files to modify + +### 1. `deploy/files/usr/local/libexec/left4me/left4me-overlay` + +Single privileged code path; everything else flows from here. + +**Add a helper function** to decide whether a path needs idmapping. Stat the +directory; if its `st_uid` matches the resolved `l4d2-sandbox` uid, return the +idmapped path under `runtime//idmap/`; otherwise return the input path +unchanged. + +**In `cmd_mount`**, before constructing `lowerdir=`: +1. `os.makedirs(runtime_dir / "idmap", exist_ok=True)` (root-owned, mode + `0o700`; only the helper writes here). +2. Resolve `l4d2_sandbox_uid = pwd.getpwnam("l4d2-sandbox").pw_uid` and + `left4me_uid = pwd.getpwnam("left4me").pw_uid`. Cache. Fail fast with a + clear message if either user is missing. +3. For each `lowerdir` in the resolved list, compute + `idmapped_path(lowerdir)`. If remapping is required: + - Create the target directory under `runtime//idmap/` if + missing. + - Skip the bind if it's already mounted there (`os.path.ismount`). + - Otherwise exec `mount --bind + --map-users=::1 + --map-groups=::1 `. + **Direction note**: first arg is the on-disk uid, second arg is the + uid the mount exposes. We verified empirically that this is what the + kernel honors despite ambiguous man-page wording. +4. Pass the (possibly idmapped) paths into the `lowerdir=` colon string in + the same order. + +**In `cmd_umount`**, after the existing overlay umount: +- For each subdirectory under `runtime//idmap/`, if it's a mountpoint, + `umount` it. +- `shutil.rmtree(runtime_dir / "idmap", ignore_errors=True)` after all binds + are gone. +- Idempotent: re-running after the dir is gone is a no-op. + +**PRINT_ONLY mode**: emit the bind-mount argv before the overlay-mount argv, +on separate lines, so test assertions can match. Same shell-quoting. + +**Allowlist**: no change needed — the idmap binds land under `runtime/`, +which is already write-permitted for the helper. + +### 2. `l4d2host/tests/test_overlay_helper.py` + +Add tests using the existing PRINT_ONLY harness: + +- `test_mount_idmaps_sandbox_owned_lowerdir`: create a tmp lowerdir, `chown` + it to a fake-`l4d2-sandbox` (use `monkeypatch` on the uid lookup if running + unprivileged), run helper in PRINT_ONLY, assert a `mount --bind + --map-users=...` line appears and the `lowerdir=` string references the + idmap path. +- `test_mount_skips_idmap_for_left4me_owned_lowerdir`: tmp lowerdir owned by + the test user, assert no bind-mount argv emitted and `lowerdir=` + references the raw path. +- `test_umount_unwinds_idmap_binds`: pre-create `runtime//idmap/foo` as a + mountpoint sentinel, assert the PRINT_ONLY umount sequence includes the + bind-mount teardown before the overlay umount? Actually overlay-first, + then binds — match the helper order. + +Reuse the existing `LEFT4ME_OVERLAY_PRINT_ONLY=1` plumbing rather than +inventing a new mode. + +### 3. `deploy/tests/test_deploy_artifacts.py` + +Add a grep-style assertion that the helper file contains the strings +`--map-users` and `idmap` so the deploy artifact can't silently regress. + +### 4. `deploy/README.md` + +One-line mirror note: ckn-bw's `bundles/left4me/` ships the helper verbatim, +so no new bundle-side change is needed beyond updating the file. No new +group, no new user, no new systemd unit. Flag this explicitly so the next +deploy-to-prod step is "rebuild the helper file in ckn-bw, `bw apply +ovh.left4me`". + +## Migration + +No on-disk schema change. Existing overlays keep their current ownership +(`l4d2-sandbox`-owned for script builds, `left4me`-owned for workshop +builds). The mount helper picks the right path per lowerdir at next +`systemctl start `. + +Already-running instances on the test server pick up the change after a +service restart. Live SourceMod sessions whose `addons/sourcemod/logs/` +copy-up is already broken in upperdir need a fix too: the upperdir entries +are `l4d2-sandbox:l4d2-sandbox 0755` from the previous broken copy-up. The +helper doesn't touch upper/ on mount, so those stale entries persist. + +Two safe migration options: + +1. Manual: on the test server, stop server 2, `rm -rf + runtime/2/upper/left4dead2/addons/sourcemod/logs runtime/2/upper/left4dead2/addons/sourcemod/data`, + start it again. Copy-up will redo with idmapped lower → `left4me`-owned + upper. +2. Automatic: have `start_instance` proactively delete known-SM writable + dirs from `upper/` if their uid is `l4d2-sandbox`. Out of scope for this + change unless we hit it again. + +Recommend option 1 — one-shot, no code change. + +## Verification + +End-to-end test on `left4.me`: + +1. `bw apply ovh.left4me` (or scp the updated helper into place). +2. Stop server 2: `sudo systemctl stop left4me-server@2`. +3. Clean the stale broken SM upperdir: `sudo rm -rf + /var/lib/left4me/runtime/2/upper/left4dead2/addons/sourcemod/{logs,data}`. +4. Start server 2: `sudo systemctl start left4me-server@2`. +5. From inside `left4me-overlay mount` argv (check `journalctl -u + left4me-server@2 -o cat -n 50`), confirm `mount --bind --map-users=...` + was executed. +6. RCON to server 2, `sm_cvar nb_update_frequency 0.0333`. Expect no + `Platform returned error: "Permission denied"` log line. +7. `ls -ln + /var/lib/left4me/runtime/2/upper/left4dead2/addons/sourcemod/logs/`. + Expect files owned by `left4me`'s numeric uid. +8. `sudo umount /var/lib/left4me/runtime/2/merged` should still work; then + verify `runtime/2/idmap/` is cleaned up by `ExecStopPost`. Restart and + confirm idempotent (no leftover binds). + +Local tests: + +- `pytest l4d2host/tests/test_overlay_helper.py -q` +- `pytest deploy/tests/test_deploy_artifacts.py -q` + +## Risks and edge cases + +- **Workshop overlay misidentification**: a workshop overlay with a + `l4d2-sandbox`-owned subdir somehow (e.g. partial migration) would get + idmapped despite containing `left4me`-owned files. Files with other uids + through an idmap appear as the overflow uid (`nobody`/65534), which would + break reads. Mitigation: check ownership of the **top-level overlay + directory** as the trigger, not file-by-file. If the top is sandbox-owned, + trust the whole tree; if the top is left4me-owned, no idmap. This matches + what each builder actually produces. +- **`installation/` and caches**: always `left4me`-owned, never idmapped. +- **Symlinks inside script overlays**: idmap operates at the mount level, + not per-inode. Symlink ownership translates the same as files. Targets + inside the overlay resolve through the same mount. Targets outside (none + in script overlays today; workshop ones don't take this code path) would + not be affected. +- **Mount namespace**: the helper runs in PID 1's mount namespace via the + unit's `ExecStartPre=+nsenter ...`. Bind mounts created there persist + until the matching `ExecStopPost` umount, exactly like the overlay mount + itself. +- **Crash mid-build**: idmap binds are created only at `mount` time, not at + build time. A crashed build leaves no orphan mounts. +- **Crash mid-start (ExecStartPre fails between bind and overlay mount)**: + systemd's `Restart=on-failure` re-invokes ExecStartPre. The helper checks + `os.path.ismount` on each idmap target and skips already-mounted ones. + Idempotent. +- **`runtime//idmap/` cleanup on `_purge_instance`**: existing + `shutil.rmtree(runtime_dir)` after `disable_service` already triggers the + helper's umount sequence, which removes the idmap dir. No new code. +- **util-linux flag form**: prefer `--map-users ::` and + `--map-groups` (numeric uids/gids resolved by the helper) over the + `X-mount.idmap=` mount-option syntax — clearer and easier to log. + +## Out of scope + +- Web app uid split (`l4d2-web` separate from `left4me`) — orthogonal, + rejected for this change. +- Gameserver uid split (separating the gameserver-runtime uid from + `left4me`) — planned for a later session. **One forward-compat + coupling**: the helper looks up `pwd.getpwnam("left4me")` as the in-mount + target uid. When the gameserver moves to its own user (e.g. `l4d2-game`), + change that one string. Everything else (script-sandbox uid, workshop + builder uid, file-tree endpoint, idmap cleanup) is uid-agnostic. +- Replacing `l4d2-sandbox` with a different uid scheme — kept as defense in + depth. +- Spec doc updates (including the bubblewrap → systemd-run wording + correction in the existing script-overlay spec): dropped per user + decision; this change ships plan-only.