fix(constants): empty/unreadable managed marker still counts as managed
Address the review findings on #117359: - _is_managed_home(): drop "" from the not-managed tuple — the legacy NixOS empty/unreadable marker maps to managed in hermes_cli.config.get_managed_system, and the delegation was silently re-enabling chmod on managed installs (#77579) - _container_or_chmod_skipped(): widen to the canonical _detect_container signals so Podman/containerd/K8s runtimes without HERMES_CONTAINER keep operator modes (the sharing case #117347 sets out to honor) - _chown_dir_to_hermes_uid(): docstring no longer claims a literal contract with hermes_cli.config._chown_to_hermes_uid (no win32 early return here; os.chown AttributeError makes it a no-op) Red/green: the three new parity tests fail with the fix stashed.
This commit is contained in:
@@ -1020,30 +1020,31 @@ def _is_managed_home() -> bool:
|
||||
marker = marker_file.read_text(encoding="utf-8", errors="replace").strip().lower()
|
||||
except OSError:
|
||||
marker = ""
|
||||
if marker is None or marker in ("", "brew", "homebrew", "false", "0", "no", "off"):
|
||||
if marker is None or marker in ("brew", "homebrew", "false", "0", "no", "off"):
|
||||
return False
|
||||
# An empty or unreadable marker ("" — including the OSError fallback above) is the legacy
|
||||
# NixOS shape and still counts as managed, matching hermes_cli.config.get_managed_system.
|
||||
return True
|
||||
|
||||
|
||||
def _container_or_chmod_skipped() -> bool:
|
||||
"""Docker/Podman/LXC detection honoring the ``HERMES_CONTAINER``/``HERMES_SKIP_CHMOD``
|
||||
overrides — the same signals as ``hermes_cli.config._is_container``, deliberately not the
|
||||
cached :func:`is_container` (that one ignores these env overrides)."""
|
||||
if (os.environ.get("HERMES_CONTAINER") or os.environ.get("HERMES_SKIP_CHMOD")
|
||||
or os.path.exists("/.dockerenv")):
|
||||
"""Container/chmod-skip detection: the ``HERMES_CONTAINER``/``HERMES_SKIP_CHMOD`` operator
|
||||
overrides on top of the canonical :func:`_detect_container` signals (same breadth as
|
||||
:func:`is_container` — Docker/Podman/LXC/Kubernetes, cgroup and root-mountinfo). The cached
|
||||
:func:`is_container` itself is deliberately avoided: it ignores these env overrides and is
|
||||
computed only once per process, so tests could not flip it."""
|
||||
if os.environ.get("HERMES_CONTAINER") or os.environ.get("HERMES_SKIP_CHMOD"):
|
||||
return True
|
||||
try:
|
||||
with open("/proc/1/cgroup", "r", encoding="utf-8") as f:
|
||||
return any(m in f.read() for m in ("docker", "lxc", "kubepods"))
|
||||
except OSError:
|
||||
return False
|
||||
return _detect_container()
|
||||
|
||||
|
||||
def _chown_dir_to_hermes_uid(path) -> None:
|
||||
"""Chown *path* to ``HERMES_UID:HERMES_GID`` when set; EPERM/ENOENT are non-fatal.
|
||||
|
||||
Same contract as ``hermes_cli.config._chown_to_hermes_uid`` — used by
|
||||
:func:`apply_secure_dir_policy` so Docker deployments keep directory ownership consistent.
|
||||
Used by :func:`apply_secure_dir_policy` so Docker deployments keep directory ownership
|
||||
consistent. Unlike ``hermes_cli.config._chown_to_hermes_uid`` there is no ``win32``
|
||||
early return here — on Windows the ``AttributeError`` from the missing ``os.chown``
|
||||
below is what makes this a no-op.
|
||||
"""
|
||||
def env_int(name: str):
|
||||
try:
|
||||
|
||||
@@ -102,6 +102,42 @@ class TestScratchDirPermissionPolicy:
|
||||
scratch = get_scratch_dir(tmp_path, prune=False)
|
||||
assert stat.S_IMODE(os.stat(scratch).st_mode) == 0o2770
|
||||
|
||||
def test_empty_managed_marker_counts_as_managed(self, tmp_path, monkeypatch):
|
||||
# Legacy NixOS module wrote an empty marker; config.get_managed_system treats it as
|
||||
# managed, so the parity twin must too (not re-enable chmod on managed installs).
|
||||
home = tmp_path / "home"
|
||||
home.mkdir()
|
||||
self._isolate_env(monkeypatch, tmp_path)
|
||||
(home / ".managed").write_text("", encoding="utf-8")
|
||||
pre = tmp_path / "cache" / "scratch"
|
||||
pre.mkdir(parents=True)
|
||||
os.chmod(pre, 0o2770)
|
||||
scratch = get_scratch_dir(tmp_path, prune=False)
|
||||
assert stat.S_IMODE(os.stat(scratch).st_mode) == 0o2770
|
||||
|
||||
def test_unreadable_managed_marker_counts_as_managed(self, tmp_path, monkeypatch):
|
||||
# A marker that exists but cannot be read (OSError -> "") still counts as managed.
|
||||
home = tmp_path / "home"
|
||||
home.mkdir()
|
||||
self._isolate_env(monkeypatch, tmp_path)
|
||||
(home / ".managed").mkdir()
|
||||
pre = tmp_path / "cache" / "scratch"
|
||||
pre.mkdir(parents=True)
|
||||
os.chmod(pre, 0o2770)
|
||||
scratch = get_scratch_dir(tmp_path, prune=False)
|
||||
assert stat.S_IMODE(os.stat(scratch).st_mode) == 0o2770
|
||||
|
||||
def test_canonical_container_signal_without_env_override_keeps_operator_mode(self, tmp_path, monkeypatch):
|
||||
# Podman/containerd/K8s runtimes often don't export HERMES_CONTAINER; the canonical
|
||||
# _detect_container breadth (not the narrower legacy signal set) must skip the chmod.
|
||||
self._isolate_env(monkeypatch, tmp_path)
|
||||
monkeypatch.setattr("hermes_constants._detect_container", lambda: True)
|
||||
pre = tmp_path / "cache" / "scratch"
|
||||
pre.mkdir(parents=True)
|
||||
os.chmod(pre, 0o750)
|
||||
scratch = get_scratch_dir(tmp_path, prune=False)
|
||||
assert stat.S_IMODE(os.stat(scratch).st_mode) == 0o750
|
||||
|
||||
def test_repeated_calls_do_not_strip_setgid(self, tmp_path, monkeypatch):
|
||||
self._isolate_env(monkeypatch, tmp_path)
|
||||
monkeypatch.setenv("HERMES_HOME_MODE", "2770")
|
||||
|
||||
Reference in New Issue
Block a user