From ce5a9bb0123256bcfa80853ba31f0fd9dc7f0861 Mon Sep 17 00:00:00 2001 From: ekko <152005280+EKKOLearnAI@users.noreply.github.com> Date: Sun, 17 May 2026 15:39:31 +0800 Subject: [PATCH] Harden env parsing and writes (#814) --- .../server/src/services/config-helpers.ts | 7 ++++ .../hermes/agent-bridge/hermes_bridge.py | 40 +++++++++---------- tests/server/config-helpers-file-lock.test.ts | 13 ++++++ 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/packages/server/src/services/config-helpers.ts b/packages/server/src/services/config-helpers.ts index 4181809..8c5ea85 100644 --- a/packages/server/src/services/config-helpers.ts +++ b/packages/server/src/services/config-helpers.ts @@ -88,7 +88,14 @@ export async function updateConfigYaml( // --- .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 { + assertValidEnvKey(key) const envPath = getActiveEnvPath() await safeFileStore.updateText(envPath, (raw) => { const remove = !value diff --git a/packages/server/src/services/hermes/agent-bridge/hermes_bridge.py b/packages/server/src/services/hermes/agent-bridge/hermes_bridge.py index a403f37..00eee14 100644 --- a/packages/server/src/services/hermes/agent-bridge/hermes_bridge.py +++ b/packages/server/src/services/hermes/agent-bridge/hermes_bridge.py @@ -185,29 +185,27 @@ def _profile_home(profile: str | None) -> Path: def _read_dotenv(path: Path) -> dict[str, str]: if not path.exists(): return {} + values: dict[str, str] = {} 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] = {} - try: - for line in path.read_text(encoding="utf-8").splitlines(): - stripped = line.strip() - if not stripped or stripped.startswith("#") or "=" not in stripped: - continue - key, value = stripped.split("=", 1) - key = key.strip() - value = value.strip() - if not key: - continue - if (value.startswith('"') and value.endswith('"')) or (value.startswith("'") and value.endswith("'")): - value = value[1:-1] - values[key] = value - except Exception: - return {} + for line in path.read_text(encoding="utf-8").splitlines(): + stripped = line.strip() + if not stripped or stripped.startswith("#") or "=" not in stripped: + continue + if stripped.startswith("export "): + stripped = stripped[7:].strip() + key, value = stripped.split("=", 1) + key = key.strip() + if not key or not (key[0].isalpha() or key[0] == "_"): + 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("'")): + value = value[1:-1] + values[key] = value return values + except Exception: + return {} def _profile_dotenv_keys() -> set[str]: diff --git a/tests/server/config-helpers-file-lock.test.ts b/tests/server/config-helpers-file-lock.test.ts index cda2f1c..0cc9f3f 100644 --- a/tests/server/config-helpers-file-lock.test.ts +++ b/tests/server/config-helpers-file-lock.test.ts @@ -67,6 +67,19 @@ describe('config-helpers locked file updates', () => { 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 () => { const configPath = join(hermesHome, 'config.yaml') await writeFile(configPath, 'model:\n default: old\n', 'utf-8')