fix(memory-setup): sanitize .env values in the core writer too

Widens the salvaged .env injection fix (#50315) to the sibling site it
missed: hermes_cli/memory_setup.py::_write_env_vars is the near-identical
core writer the openviking plugin's copy was forked from, is fed directly
by interactive _prompt() (pasted API keys), and is reused by other memory
plugins (e.g. supermemory imports it). A pasted secret with an embedded
CR/LF injected an arbitrary extra KEY=VALUE line on the next read.

Same _env_line_safe() treatment as the plugin writer (strip every
str.splitlines() separator + NUL), matching config.save_env_value's
existing newline strip. Mutation-checked: reverting the sanitizer makes
the new regression tests fail.
This commit is contained in:
kshitijk4poor 2026-07-22 15:07:02 +05:30 committed by kshitij
parent 8f0da78f84
commit 7e3acd02d9
2 changed files with 51 additions and 2 deletions

View file

@ -379,6 +379,21 @@ def cmd_setup(args) -> None:
print("\n Start a new session to activate.\n")
def _env_line_safe(value) -> str:
"""Neutralize characters that would break ``.env`` line structure.
``.env`` is strictly line-oriented (one ``KEY=VALUE`` per line) and
values are interpolated straight into that line. A pasted secret with an
embedded CR/LF would spill onto a new line and be re-parsed as a
*separate* ``KEY=VALUE`` entry on the next read injecting an arbitrary
variable into the credentials file. Strip every separator recognized by
``str.splitlines()`` plus NUL so a value can only occupy its own line.
Mirrors the openviking plugin's writer and ``config.save_env_value``.
"""
text = value if isinstance(value, str) else str(value)
return "".join(text.replace("\x00", "").splitlines())
def _write_env_vars(env_path: Path, env_writes: dict) -> None:
"""Append or update env vars in .env file."""
env_path.parent.mkdir(parents=True, exist_ok=True)
@ -392,14 +407,14 @@ def _write_env_vars(env_path: Path, env_writes: dict) -> None:
for line in existing_lines:
key_match = line.split("=", 1)[0].strip() if "=" in line else ""
if key_match in env_writes:
new_lines.append(f"{key_match}={env_writes[key_match]}")
new_lines.append(f"{key_match}={_env_line_safe(env_writes[key_match])}")
updated_keys.add(key_match)
else:
new_lines.append(line)
for key, val in env_writes.items():
if key not in updated_keys:
new_lines.append(f"{key}={val}")
new_lines.append(f"{key}={_env_line_safe(val)}")
env_path.write_text("\n".join(new_lines) + "\n", encoding="utf-8")
# Restrict permissions — .env holds API keys and tokens.

View file

@ -196,3 +196,37 @@ def test_cmd_setup_generic_choice_cancel_writes_nothing(tmp_path, monkeypatch):
save_config.assert_not_called()
provider.save_config.assert_not_called()
assert not (tmp_path / ".env").exists()
def test_write_env_vars_strips_line_separators_and_nul(tmp_path):
"""A pasted secret with embedded CR/LF/NUL must not inject an extra
KEY=VALUE line into .env (mirrors the openviking plugin's writer)."""
env_path = tmp_path / ".env"
memory_setup._write_env_vars(
env_path,
{"PROVIDER_API_KEY": "good\nINJECTED_KEY=attacker\r\u2028\x00tail"},
)
lines = env_path.read_text(encoding="utf-8").splitlines()
assert lines == ["PROVIDER_API_KEY=goodINJECTED_KEY=attackertail"]
parsed = dict(line.split("=", 1) for line in lines if "=" in line)
assert set(parsed) == {"PROVIDER_API_KEY"}
def test_write_env_vars_strips_newlines_when_updating_existing_key(tmp_path):
env_path = tmp_path / ".env"
env_path.write_text("PROVIDER_API_KEY=old\nKEEP=1\n", encoding="utf-8")
memory_setup._write_env_vars(env_path, {"PROVIDER_API_KEY": "new\r\nROGUE=1"})
lines = env_path.read_text(encoding="utf-8").splitlines()
assert "PROVIDER_API_KEY=newROGUE=1" in lines
assert "KEEP=1" in lines
assert all(not line.startswith("ROGUE=") for line in lines)
def test_write_env_vars_plain_value_roundtrips(tmp_path):
env_path = tmp_path / ".env"
memory_setup._write_env_vars(env_path, {"PROVIDER_API_KEY": "sk-plain-1234"})
assert env_path.read_text(encoding="utf-8") == "PROVIDER_API_KEY=sk-plain-1234\n"