Harden env parsing and writes (#814)

This commit is contained in:
ekko
2026-05-17 15:39:31 +08:00
committed by GitHub
parent 6516d86dfc
commit ce5a9bb012
3 changed files with 39 additions and 21 deletions
@@ -88,7 +88,14 @@ export async function updateConfigYaml<T = void>(
// --- .env helpers --- // --- .env helpers ---
function assertValidEnvKey(key: string): void {
if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(key)) {
throw new Error(`Invalid .env key: ${JSON.stringify(key)}`)
}
}
export async function saveEnvValue(key: string, value: string): Promise<void> { export async function saveEnvValue(key: string, value: string): Promise<void> {
assertValidEnvKey(key)
const envPath = getActiveEnvPath() const envPath = getActiveEnvPath()
await safeFileStore.updateText(envPath, (raw) => { await safeFileStore.updateText(envPath, (raw) => {
const remove = !value const remove = !value
@@ -185,29 +185,27 @@ def _profile_home(profile: str | None) -> Path:
def _read_dotenv(path: Path) -> dict[str, str]: def _read_dotenv(path: Path) -> dict[str, str]:
if not path.exists(): if not path.exists():
return {} return {}
try:
from dotenv import dotenv_values
values = dotenv_values(path)
return {str(k): str(v) for k, v in values.items() if k and v is not None}
except Exception:
values: dict[str, str] = {} values: dict[str, str] = {}
try: try:
for line in path.read_text(encoding="utf-8").splitlines(): for line in path.read_text(encoding="utf-8").splitlines():
stripped = line.strip() stripped = line.strip()
if not stripped or stripped.startswith("#") or "=" not in stripped: if not stripped or stripped.startswith("#") or "=" not in stripped:
continue continue
if stripped.startswith("export "):
stripped = stripped[7:].strip()
key, value = stripped.split("=", 1) key, value = stripped.split("=", 1)
key = key.strip() key = key.strip()
value = value.strip() if not key or not (key[0].isalpha() or key[0] == "_"):
if not key:
continue continue
if not all(ch.isalnum() or ch == "_" for ch in key):
continue
value = value.strip()
if (value.startswith('"') and value.endswith('"')) or (value.startswith("'") and value.endswith("'")): if (value.startswith('"') and value.endswith('"')) or (value.startswith("'") and value.endswith("'")):
value = value[1:-1] value = value[1:-1]
values[key] = value values[key] = value
return values
except Exception: except Exception:
return {} return {}
return values
def _profile_dotenv_keys() -> set[str]: def _profile_dotenv_keys() -> set[str]:
@@ -67,6 +67,19 @@ describe('config-helpers locked file updates', () => {
expect(env).toContain('MOONSHOT_API_KEY=moonshot') expect(env).toContain('MOONSHOT_API_KEY=moonshot')
}) })
it('rejects invalid .env keys instead of writing keyless lines', async () => {
const envPath = join(hermesHome, '.env')
await writeFile(envPath, 'OPENROUTER_API_KEY=keep\n', 'utf-8')
const { saveEnvValue } = await loadHelpers()
await expect(saveEnvValue('', 'leaked-value')).rejects.toThrow('Invalid .env key')
await expect(saveEnvValue('=BROKEN', 'leaked-value')).rejects.toThrow('Invalid .env key')
const env = await readFile(envPath, 'utf-8')
expect(env).toBe('OPENROUTER_API_KEY=keep\n')
expect(env).not.toContain('=leaked-value')
})
it('skips writing config.yaml when an updater returns write false', async () => { it('skips writing config.yaml when an updater returns write false', async () => {
const configPath = join(hermesHome, 'config.yaml') const configPath = join(hermesHome, 'config.yaml')
await writeFile(configPath, 'model:\n default: old\n', 'utf-8') await writeFile(configPath, 'model:\n default: old\n', 'utf-8')