fix(secrets-store): validate key/value to reject newlines and bad keys

Author: 0xmariowu

autosearch/core/secrets_store.py

@@ -19,6 +19,7 @@
 
 import logging
 import os
+import re
 import shlex
 import tempfile
 from pathlib import Path
@@ -29,6 +30,7 @@
     _fcntl = None
 
 _FILE_INJECTED_VALUES: dict[str, str] = {}
+_ENV_KEY_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
 _log = logging.getLogger(__name__)
 
 
@@ -73,6 +75,7 @@ def load_secrets(path: Path | None = None) -> dict[str, str]:
 
 def write_secret(key: str, value: str, *, path: Path | None = None) -> None:
     """Atomically write or replace one KEY=value entry in the secrets file."""
+    _validate_secret_entry(key, value)
     target = path or secrets_path()
     target.parent.mkdir(parents=True, exist_ok=True)
     lock_path = target.with_name(f"{target.name}.lock")
@@ -120,7 +123,7 @@ def write_secret(key: str, value: str, *, path: Path | None = None) -> None:
 
 
 def _replace_or_append_secret(text: str, key: str, value: str) -> str:
-    replacement = f"{key}={shlex.quote(value)}"
+    replacement = _format_secret_line(key, value)
     lines = text.splitlines()
     replaced = False
     next_lines: list[str] = []
@@ -137,6 +140,18 @@ def _replace_or_append_secret(text: str, key: str, value: str) -> str:
     return "\n".join(next_lines) + "\n"
 
 
+def _format_secret_line(key: str, value: str) -> str:
+    _validate_secret_entry(key, value)
+    return f"{key}={shlex.quote(value)}"
+
+
+def _validate_secret_entry(key: str, value: str) -> None:
+    if not _ENV_KEY_RE.fullmatch(key):
+        raise ValueError("secret key must match [A-Za-z_][A-Za-z0-9_]*")
+    if any(char in value for char in ("\n", "\r", "\0")):
+        raise ValueError("secret value must not contain newline, carriage return, or NUL")
+
+
 def _secret_line_key(raw: str) -> str | None:
     line = raw.strip()
     if not line or line.startswith("#") or "=" not in line:

tests/unit/test_secrets_store.py

@@ -101,6 +101,32 @@ def test_write_secret_basic(tmp_path):
     assert load_secrets(f) == {"OPENAI_API_KEY": "sk-basic"}
 
 
+@pytest.mark.parametrize(
+    "bad_key",
+    ["", "1OPENAI_API_KEY", "OPENAI-API-KEY", "OPENAI.API.KEY", "OPENAI API KEY"],
+)
+def test_write_secret_rejects_invalid_key(tmp_path, bad_key):
+    f = tmp_path / "ai-secrets.env"
+
+    with pytest.raises(ValueError, match=r"secret key must match"):
+        write_secret(bad_key, "sk-basic", path=f)
+
+    assert not f.exists()
+
+
+@pytest.mark.parametrize(
+    "bad_value",
+    ["sk-line-1\nINJECTED_KEY=bad", "sk-line-1\rsk-line-2", "sk-prefix\0sk-suffix"],
+)
+def test_write_secret_rejects_newline_and_nul_values(tmp_path, bad_value):
+    f = tmp_path / "ai-secrets.env"
+
+    with pytest.raises(ValueError, match=r"secret value must not contain"):
+        write_secret("OPENAI_API_KEY", bad_value, path=f)
+
+    assert not f.exists()
+
+
 def test_write_secret_preserves_comments_and_unknown_lines(tmp_path):
     f = tmp_path / "ai-secrets.env"
     _write(