filtering secrets from terminal snapshot files

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #60199 fix(security): create terminal env snapshots owner-only (0600)

  merged to main

  files: scripts/release.py, tests/tools/test_base_environment.py, tools/environments/base.py

  Show the diff

  • scripts/release.py

    @@ -1811,6 +1811,7 @@
         "[redacted:email]": "vanthinh6886",  # PR #28018 salvage (yaml/flock/atomic write guards)
         "[redacted:email]": "erikengervall",  # PR #28774 (firecrawl integration tag)
         "[redacted:email]": "egilewski",  # PR #30432 (MEDIA path traversal fix, GHSA-jmf9-9729-7pp8)
    +    "[redacted:email]": "andrewhomeyer",  # PR #20056 (snapshot owner-only perms, co-author on #57386 salvage)
         "[redacted:email]": "McClean-Edison",  # PR #29817 (register_auxiliary_task plugin API)
         "[email protected]": "OYLFLMH",  # PR #48312 salvage (cli_refresh_interval config, #48309)
         "[redacted:email]": "SamuelZ12",  # PR #7480 (show recap after in-session resume)

  • tests/tools/test_base_environment.py

    @@ -174,6 +174,32 @@ def fake_run_bash(cmd_string, *, login=False, timeout=120, stdin_data=None):
             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 @@ def test_failed_export_does_not_destroy_good_snapshot(self, tmp_path):
             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()

  • tools/environments/base.py

    @@ -395,6 +395,7 @@ def init_session(self):
             # 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 @@ def _wrap_command(self, command: str, cwd: str) -> str:
             # 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