From 90531864b3d5e48653e2d4dccccfbf4da02c7cb5 Mon Sep 17 00:00:00 2001 From: mwiegand Date: Thu, 14 May 2026 23:53:32 +0200 Subject: [PATCH] harden(left4me-overlay): fix idmap collision risk, gate test stubs on PRINT_ONLY, wrap os.stat Issue #1: idmap target now uses parent+name (overlays_workshop instead of workshop) to prevent basename collisions across allowlist roots; explicit die() on collision detected in the loop. Issue #2: env-var uid stubs (renamed to LEFT4ME_TEST_SANDBOX_UID etc.) are only honoured when LEFT4ME_OVERLAY_PRINT_ONLY=1, so a misconfigured systemd unit override cannot influence real uid mapping. Issue #3: os.stat(lowerdir) is wrapped in try/except OSError with a die() that shell-quotes the path and includes the exception, matching the helper's existing error style. Co-Authored-By: Claude Opus 4.7 --- .../usr/local/libexec/left4me/left4me-overlay | 59 +++++++++++++------ l4d2host/tests/test_overlay_helper.py | 10 ++-- 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/deploy/files/usr/local/libexec/left4me/left4me-overlay b/deploy/files/usr/local/libexec/left4me/left4me-overlay index e52c798..c01a5af 100644 --- a/deploy/files/usr/local/libexec/left4me/left4me-overlay +++ b/deploy/files/usr/local/libexec/left4me/left4me-overlay @@ -70,24 +70,28 @@ def _lookup_uid(username: str) -> tuple[int, int]: def _get_user_ids() -> tuple[int, int, int, int]: """Return (sandbox_uid, sandbox_gid, left4me_uid, left4me_gid). - In normal operation, looks up the real system users. When the test-only - env vars _L4D2_SANDBOX_UID/_L4D2_SANDBOX_GID/_LEFT4ME_UID/_LEFT4ME_GID - are set, those values are used directly so tests can run without root - and without real system users being present. + In normal operation, looks up the real system users. In PRINT_ONLY + (test) mode the env vars LEFT4ME_TEST_SANDBOX_UID/LEFT4ME_TEST_SANDBOX_GID/ + LEFT4ME_TEST_LEFT4ME_UID/LEFT4ME_TEST_LEFT4ME_GID may be used to inject + synthetic uids so tests can run without root and without real system + users present. The stubs are intentionally ignored outside PRINT_ONLY + mode so that a misconfigured systemd unit override cannot influence the + real uid mapping. """ - sandbox_uid_env = os.environ.get("_L4D2_SANDBOX_UID") - sandbox_gid_env = os.environ.get("_L4D2_SANDBOX_GID") - left4me_uid_env = os.environ.get("_LEFT4ME_UID") - left4me_gid_env = os.environ.get("_LEFT4ME_GID") + if os.environ.get("LEFT4ME_OVERLAY_PRINT_ONLY") == "1": + sandbox_uid_env = os.environ.get("LEFT4ME_TEST_SANDBOX_UID") + sandbox_gid_env = os.environ.get("LEFT4ME_TEST_SANDBOX_GID") + left4me_uid_env = os.environ.get("LEFT4ME_TEST_LEFT4ME_UID") + left4me_gid_env = os.environ.get("LEFT4ME_TEST_LEFT4ME_GID") - if all(v is not None for v in (sandbox_uid_env, sandbox_gid_env, - left4me_uid_env, left4me_gid_env)): - return ( - int(sandbox_uid_env), # type: ignore[arg-type] - int(sandbox_gid_env), # type: ignore[arg-type] - int(left4me_uid_env), # type: ignore[arg-type] - int(left4me_gid_env), # type: ignore[arg-type] - ) + if all(v is not None for v in (sandbox_uid_env, sandbox_gid_env, + left4me_uid_env, left4me_gid_env)): + return ( + int(sandbox_uid_env), # type: ignore[arg-type] + int(sandbox_gid_env), # type: ignore[arg-type] + int(left4me_uid_env), # type: ignore[arg-type] + int(left4me_gid_env), # type: ignore[arg-type] + ) sandbox_uid, sandbox_gid = _lookup_uid("l4d2-sandbox") left4me_uid, left4me_gid = _lookup_uid("left4me") @@ -223,13 +227,30 @@ def cmd_mount(name: str) -> None: idmap_dir = runtime_name_dir / "idmap" final_lowerdirs: list[str] = [] bind_argvs: list[list[str]] = [] + seen_idmap_targets: dict[Path, str] = {} for lowerdir in canonical_lowerdirs: - st = os.stat(lowerdir) + try: + st = os.stat(lowerdir) + except OSError as exc: + die(f"failed to stat lowerdir {shlex.quote(lowerdir)}: {exc}") if st.st_uid == sandbox_uid: # This lowerdir needs idmap remapping. - overlay_id = Path(lowerdir).name - idmap_target = idmap_dir / overlay_id + # Include the parent dirname to avoid basename collisions between + # lowerdirs from different allowlist roots (e.g. overlays/foo and + # workshop_cache/foo would otherwise map to the same idmap target). + p = Path(lowerdir) + lowerdir_basename = f"{p.parent.name}_{p.name}" + idmap_target = idmap_dir / lowerdir_basename + + # Belt-and-braces: detect if two different lowerdirs would collide + # on the same idmap target after the parent+name derivation. + if idmap_target in seen_idmap_targets: + die( + f"idmap target collision: lowerdirs {shlex.quote(seen_idmap_targets[idmap_target])}" + f" and {shlex.quote(lowerdir)} both map to {idmap_target}" + ) + seen_idmap_targets[idmap_target] = lowerdir if os.environ.get("LEFT4ME_OVERLAY_PRINT_ONLY") != "1": idmap_dir.mkdir(mode=0o700, exist_ok=True) diff --git a/l4d2host/tests/test_overlay_helper.py b/l4d2host/tests/test_overlay_helper.py index b97381d..6beda12 100644 --- a/l4d2host/tests/test_overlay_helper.py +++ b/l4d2host/tests/test_overlay_helper.py @@ -42,10 +42,10 @@ def _run(args: list[str], root: Path, extra_env: dict[str, str] | None = None) - "LEFT4ME_ROOT": str(root), "LEFT4ME_OVERLAY_PRINT_ONLY": "1", # Inject synthetic user ids so tests work without real system users. - "_L4D2_SANDBOX_UID": str(FAKE_SANDBOX_UID), - "_L4D2_SANDBOX_GID": str(FAKE_SANDBOX_GID), - "_LEFT4ME_UID": str(FAKE_LEFT4ME_UID), - "_LEFT4ME_GID": str(FAKE_LEFT4ME_GID), + "LEFT4ME_TEST_SANDBOX_UID": str(FAKE_SANDBOX_UID), + "LEFT4ME_TEST_SANDBOX_GID": str(FAKE_SANDBOX_GID), + "LEFT4ME_TEST_LEFT4ME_UID": str(FAKE_LEFT4ME_UID), + "LEFT4ME_TEST_LEFT4ME_GID": str(FAKE_LEFT4ME_GID), } if extra_env: env.update(extra_env) @@ -222,7 +222,7 @@ def test_mount_idmaps_sandbox_owned_lowerdir(tmp_path: Path) -> None: assert f"--map-users={FAKE_SANDBOX_UID}:{FAKE_LEFT4ME_UID}:1" in bind_parts assert f"--map-groups={FAKE_SANDBOX_GID}:{FAKE_LEFT4ME_GID}:1" in bind_parts assert bind_parts[-2] == str(overlay_dir) - idmap_target = str(tmp_path / "runtime" / "alpha" / "idmap" / "workshop") + idmap_target = str(tmp_path / "runtime" / "alpha" / "idmap" / "overlays_workshop") assert bind_parts[-1] == idmap_target overlay_parts = shlex.split(lines[1])