diff --git a/tests/tools/test_base_environment.py b/tests/tools/test_base_environment.py index 8d6e6ab7328..d629d95ea29 100644 --- a/tests/tools/test_base_environment.py +++ b/tests/tools/test_base_environment.py @@ -174,6 +174,32 @@ class TestAtomicSnapshotWrite: assert "$BASHPID" in boot assert ".tmp.$$" not in boot + def test_snapshot_writes_use_private_umask_after_user_command(self): + env = _TestableEnv() + env._snapshot_ready = True + wrapped = env._wrap_command("echo hi", "/tmp") + + assert "umask 077" in wrapped + assert wrapped.index("eval 'echo hi'") < wrapped.index("umask 077") + assert wrapped.index("umask 077") < wrapped.index("export -p >") + + def test_init_session_bootstrap_uses_private_umask(self): + env = _TestableEnv() + captured = {} + + def fake_run_bash(cmd_string, *, login=False, timeout=120, stdin_data=None): + captured["cmd"] = cmd_string + raise RuntimeError("stop after capture") + + env._run_bash = fake_run_bash # type: ignore[assignment] + try: + env.init_session() + except Exception: + pass + boot = captured.get("cmd", "") + assert "umask 077" in boot + assert boot.index("umask 077") < boot.index("export -p >") + class TestAtomicSnapshotConcurrencyBehavioral: """Behavioral regression for #38249 — actually EXECUTES the generated @@ -250,6 +276,57 @@ class TestAtomicSnapshotConcurrencyBehavioral: assert "export GOOD=1" in out.stdout, "good snapshot was destroyed by a failed export" +class TestSnapshotFileModes: + """Snapshot metadata files are private without changing user command umask.""" + + def test_snapshot_and_cwd_files_are_0600(self, tmp_path): + import os + from pathlib import Path + import shutil + import stat + import subprocess + if not shutil.which("bash"): + import pytest + pytest.skip("bash required") + + class ExecutableEnv(BaseEnvironment): + def __init__(self, temp_dir): + self._temp_dir = str(temp_dir) + super().__init__(cwd=str(temp_dir), timeout=10) + + def get_temp_dir(self): + return self._temp_dir + + def _run_bash(self, cmd_string, *, login=False, timeout=120, stdin_data=None): + proc = subprocess.Popen( + ["/bin/bash", "-lc", cmd_string], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + stdin=subprocess.DEVNULL, + text=True, + cwd=self.cwd, + ) + proc.communicate(timeout=timeout) + return proc + + def cleanup(self): + pass + + old_umask = os.umask(0o022) + try: + env = ExecutableEnv(tmp_path) + env.init_session() + + user_file = tmp_path / "user-created.txt" + env.execute(f"touch {user_file}") + + assert stat.S_IMODE(user_file.stat().st_mode) == 0o644 + assert stat.S_IMODE(Path(env._snapshot_path).stat().st_mode) == 0o600 + assert stat.S_IMODE(Path(env._cwd_file).stat().st_mode) == 0o600 + finally: + os.umask(old_umask) + + class TestExtractCwdFromOutput: def test_happy_path(self): env = _TestableEnv() diff --git a/tools/environments/base.py b/tools/environments/base.py index ef5b683aad2..93d56c0c5ce 100644 --- a/tools/environments/base.py +++ b/tools/environments/base.py @@ -395,6 +395,7 @@ class BaseEnvironment(ABC): # with ``$BASHPID`` left outside the quotes so it still expands. _snap_tmp = shlex.quote(self._snapshot_path + ".tmp.") + "$BASHPID" bootstrap = ( + f"umask 077\n" f"export -p > {_snap_tmp}\n" # Dump function definitions, filtering out private (``_``-prefixed) # helpers — mainly bash-completion internals (``_git``, ``_make``…) @@ -502,6 +503,9 @@ class BaseEnvironment(ABC): # Run the actual command parts.append(f"eval '{escaped}'") parts.append("__hermes_ec=$?") + # Restrict Hermes metadata files without changing the user's command + # umask. Snapshot files may contain env-carried secrets. + parts.append("umask 077") # Re-dump env vars to snapshot (atomic replacement to avoid races). # Chain mv on the export succeeding so a failed/partial dump never