fix(cli): quote .env values with internal whitespace in save_env_value

hermes_cli/config.py

@@ -7773,11 +7773,16 @@ def _quote_env_value(value: str) -> str:
     """Quote .env values containing characters with special dotenv meaning."""
     if value == "":
         return value
+    # Internal whitespace (space/tab/etc.) must be quoted so shell `set -a; . file`
+    # word-splits don't break paths like macOS "Application Support". Leading/
+    # trailing whitespace is already covered by the strip check; any() covers
+    # internal runs that strip() would leave alone.
     needs_quoting = (
         "#" in value
         or '"' in value
         or "'" in value
         or value != value.strip()
+        or any(c.isspace() for c in value)
     )
     if not needs_quoting:
         return value

tests/hermes_cli/test_config.py

@@ -607,6 +607,210 @@ def test_save_env_value_updates_hash_value_with_quotes(self, tmp_path):
             assert parsed["ANTHROPIC_TOKEN"] == token
             assert load_env()["ANTHROPIC_TOKEN"] == token
 
+    def test_save_env_value_quotes_values_with_internal_spaces(self, tmp_path):
+        """Internal spaces must be quoted so shell-sourcing does not word-split.
+
+        Sibling of installer #57247: core writer left
+        TERMINAL_SSH_KEY=/Users/.../Application Support/... unquoted.
+        python-dotenv still parsed it; ``set -a; . file`` failed.
+        """
+        import subprocess
+        from dotenv import dotenv_values
+
+        path = "/Users/paulo/Library/Application Support/hermes/keys/id_ed25519"
+        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
+            os.environ.pop("TERMINAL_SSH_KEY", None)
+            save_env_value("TERMINAL_SSH_KEY", path)
+
+            env_path = tmp_path / ".env"
+            content = env_path.read_text(encoding="utf-8")
+            assert f'TERMINAL_SSH_KEY="{path}"' in content
+
+            parsed = dotenv_values(str(env_path))
+            assert parsed["TERMINAL_SSH_KEY"] == path
+            assert load_env()["TERMINAL_SSH_KEY"] == path
+
+            # Shell source must round-trip (this is what the bug broke).
+            r = subprocess.run(
+                [
+                    "env",
+                    "-i",
+                    "sh",
+                    "-c",
+                    f"set -a; . '{env_path}'; set +a; "
+                    f'printf "%s" "$TERMINAL_SSH_KEY"',
+                ],
+                capture_output=True,
+                text=True,
+            )
+            assert r.returncode == 0, r.stderr
+            assert r.stderr == ""
+            assert r.stdout == path
+
+    def test_save_env_value_quotes_values_with_tabs(self, tmp_path):
+        """Tabs trigger quoting; round-trip via dotenv and shell source."""
+        import subprocess
+        from dotenv import dotenv_values
+
+        value = "left\tright"
+        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
+            os.environ.pop("TABBY_KEY", None)
+            save_env_value("TABBY_KEY", value)
+
+            env_path = tmp_path / ".env"
+            content = env_path.read_text(encoding="utf-8")
+            assert f'TABBY_KEY="{value}"' in content
+
+            parsed = dotenv_values(str(env_path))
+            assert parsed["TABBY_KEY"] == value
+            assert load_env()["TABBY_KEY"] == value
+
+            r = subprocess.run(
+                [
+                    "env",
+                    "-i",
+                    "sh",
+                    "-c",
+                    f"set -a; . '{env_path}'; set +a; "
+                    f'printf "%s" "$TABBY_KEY"',
+                ],
+                capture_output=True,
+                text=True,
+            )
+            assert r.returncode == 0, r.stderr
+            assert r.stderr == ""
+            assert r.stdout == value
+
+    def test_save_env_value_spaced_path_is_idempotent(self, tmp_path):
+        """Saving the same spaced value twice must not grow quotes."""
+        path = "/Users/me/Application Support/key"
+        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
+            os.environ.pop("TERMINAL_SSH_KEY", None)
+            save_env_value("TERMINAL_SSH_KEY", path)
+            first = (tmp_path / ".env").read_text(encoding="utf-8")
+            save_env_value("TERMINAL_SSH_KEY", path)
+            second = (tmp_path / ".env").read_text(encoding="utf-8")
+
+            assert first == second
+            assert first.count('TERMINAL_SSH_KEY="') == 1
+            assert '""' not in first
+            assert f'TERMINAL_SSH_KEY="{path}"' in first
+
+    def test_save_env_value_readback_resave_is_idempotent(self, tmp_path):
+        """hermes setup path: dotenv unquotes, then re-save must not grow quotes."""
+        from dotenv import dotenv_values
+
+        path = "/Users/me/Application Support/key"
+        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
+            os.environ.pop("TERMINAL_SSH_KEY", None)
+            save_env_value("TERMINAL_SSH_KEY", path)
+            first = (tmp_path / ".env").read_text(encoding="utf-8")
+
+            # Real read-back boundary (what setup uses via get_env_value/dotenv).
+            read_back = dotenv_values(str(tmp_path / ".env"))["TERMINAL_SSH_KEY"]
+            assert read_back == path
+            save_env_value("TERMINAL_SSH_KEY", read_back)
+            second = (tmp_path / ".env").read_text(encoding="utf-8")
+
+            assert first == second
+            assert f'TERMINAL_SSH_KEY="{path}"' in second
+
+    def test_save_env_value_strips_newlines_before_quoting(self, tmp_path):
+        """save_env_value strips \\n/\\r before _quote_env_value; result is one line.
+
+        Pins the boundary so any(c.isspace()) never quotes multi-line dotenv
+        values through this writer (newlines never reach the quoter).
+        """
+        from dotenv import dotenv_values
+
+        raw = "line1\nline2\rline3"
+        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
+            os.environ.pop("MULTI_KEY", None)
+            save_env_value("MULTI_KEY", raw)
+
+            content = (tmp_path / ".env").read_text(encoding="utf-8")
+            # Single KEY= line, no embedded raw newlines in the value payload.
+            lines = [ln for ln in content.splitlines() if ln.startswith("MULTI_KEY=")]
+            assert len(lines) == 1
+            assert "\n" not in lines[0]
+            assert "\r" not in lines[0]
+            # Newlines stripped -> "line1line2line3" has no whitespace -> unquoted.
+            assert lines[0] == "MULTI_KEY=line1line2line3"
+            parsed = dotenv_values(str(tmp_path / ".env"))
+            assert parsed["MULTI_KEY"] == "line1line2line3"
+
+    def test_save_env_value_simple_values_stay_unquoted(self, tmp_path):
+        """No quoting churn: plain values remain bare; untouched lines unchanged."""
+        env_path = tmp_path / ".env"
+        # Pre-existing lines: one simple, one already correctly bare.
+        env_path.write_text(
+            "KEEP_SIMPLE=plainvalue\n"
+            "OTHER_KEY=foo123\n",
+            encoding="utf-8",
+        )
+        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
+            os.environ.pop("NEW_KEY", None)
+            os.environ.pop("KEEP_SIMPLE", None)
+            save_env_value("NEW_KEY", "bar-simple")
+
+            content = env_path.read_text(encoding="utf-8")
+            # Newly written simple value is unquoted.
+            assert "NEW_KEY=bar-simple\n" in content
+            assert 'NEW_KEY="' not in content
+            # Untouched pre-existing simple lines are not re-quoted.
+            assert "KEEP_SIMPLE=plainvalue\n" in content
+            assert "OTHER_KEY=foo123\n" in content
+            assert 'KEEP_SIMPLE="' not in content
+            assert 'OTHER_KEY="' not in content
+
+    def test_save_env_value_does_not_requote_untouched_spaced_lines(self, tmp_path):
+        """Mass-requote guard: rewriting another key leaves legacy spaced
+        lines as-is (fix only applies when that key is saved again).
+        """
+        env_path = tmp_path / ".env"
+        legacy = (
+            "TERMINAL_SSH_KEY=/Users/me/Application Support/key\n"
+            "PLAIN=ok\n"
+        )
+        env_path.write_text(legacy, encoding="utf-8")
+        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
+            os.environ.pop("PLAIN", None)
+            save_env_value("PLAIN", "ok2")
+
+            content = env_path.read_text(encoding="utf-8")
+            # Legacy spaced line not re-serialized by this write.
+            assert (
+                "TERMINAL_SSH_KEY=/Users/me/Application Support/key\n" in content
+            )
+            assert 'TERMINAL_SSH_KEY="' not in content
+            assert "PLAIN=ok2\n" in content
+
+    def test_save_env_value_already_quoted_input_is_not_double_wrapped_idempotently(
+        self, tmp_path
+    ):
+        """Callers pass raw values; if a value literally contains quote
+        characters, escaping+wrap is the dialect (#57249). Re-saving the
+        same raw value is stable (no quote growth). load_env round-trips.
+        """
+        # User-typed value that already includes surrounding quotes as data.
+        raw = '"/Users/me/Application Support/key"'
+        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
+            os.environ.pop("TERMINAL_SSH_KEY", None)
+            save_env_value("TERMINAL_SSH_KEY", raw)
+            first = (tmp_path / ".env").read_text(encoding="utf-8")
+            save_env_value("TERMINAL_SSH_KEY", raw)
+            second = (tmp_path / ".env").read_text(encoding="utf-8")
+            assert first == second
+            # One outer wrap layer only (escaped inner quotes, not nested wraps).
+            line = [
+                ln for ln in first.splitlines() if ln.startswith("TERMINAL_SSH_KEY=")
+            ][0]
+            assert line.startswith('TERMINAL_SSH_KEY="')
+            assert line.endswith('"')
+            assert line.count('TERMINAL_SSH_KEY="') == 1
+            # Escaping dialect end-to-end: load sees the raw input, not stripped quotes.
+            assert load_env()["TERMINAL_SSH_KEY"] == raw
+
 
 class TestRemoveEnvValue:
     def test_removes_key_from_env_file(self, tmp_path):
@@ -726,7 +930,7 @@ def test_atomic_write_creates_valid_yaml(self, tmp_path):
 
             # Read raw YAML to verify it's valid and correct
             config_path = tmp_path / "config.yaml"
-            with open(config_path) as f:
+            with open(config_path, encoding="utf-8") as f:
                 raw = yaml.safe_load(f)
             assert raw["model"] == "test/atomic-model"
             assert raw["agent"]["max_turns"] == 77