From 7e3acd02d925b25fcf5fb5afd0076954bb6fc769 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:07:02 +0530 Subject: [PATCH] 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. --- hermes_cli/memory_setup.py | 19 +++++++++++++-- tests/hermes_cli/test_memory_setup.py | 34 +++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/hermes_cli/memory_setup.py b/hermes_cli/memory_setup.py index e08f61c5d34..c1858fc832f 100644 --- a/hermes_cli/memory_setup.py +++ b/hermes_cli/memory_setup.py @@ -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. diff --git a/tests/hermes_cli/test_memory_setup.py b/tests/hermes_cli/test_memory_setup.py index b5b574230c7..487f0949d7f 100644 --- a/tests/hermes_cli/test_memory_setup.py +++ b/tests/hermes_cli/test_memory_setup.py @@ -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"