mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
fix(cli): quote .env values with internal whitespace in save_env_value
_quote_env_value previously left internal spaces unquoted (only #/"/' and leading/trailing whitespace triggered). Spaced macOS paths written via hermes setup SSH / Google Chat SA path / hermes config set produced lines that python-dotenv still parsed but shell `set -a; . file` word-split. Extend needs_quoting with any(c.isspace()); escaping dialect unchanged.
This commit is contained in:
Paulo Nascimento
Brooklyn Nicholson
parent
bf411238d8
commit
4441e11c77
2 changed files with 210 additions and 1 deletions
|
|
@ -7829,11 +7829,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
|
||||
|
|
|
|||
|
|
@ -607,6 +607,210 @@ class TestSaveEnvValueSecure:
|
|||
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 @@ class TestSaveConfigAtomicity:
|
|||
|
||||
# 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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue