From 6a793248024dca7685f63bcceb64a0096fd1586d Mon Sep 17 00:00:00 2001 From: Devin Robison Date: Wed, 25 Mar 2026 14:49:26 -0700 Subject: [PATCH] Filter untrusted CWD .env entries before OpenClaw startup (#54631) * Filter untrusted CWD .env entries before OpenClaw startup * Add missing test file * Fix missing and updated files * Address feedback * Feedback updates * Feedback update * Add test coverage * Unit test fix --- src/cli/dotenv.ts | 14 +--- src/infra/dotenv.test.ts | 174 ++++++++++++++++++++++++++++++++++++++- src/infra/dotenv.ts | 102 +++++++++++++++++++++-- 3 files changed, 271 insertions(+), 19 deletions(-) diff --git a/src/cli/dotenv.ts b/src/cli/dotenv.ts index b257f40ecfd..fcdd0340c3f 100644 --- a/src/cli/dotenv.ts +++ b/src/cli/dotenv.ts @@ -1,20 +1,14 @@ -import fs from "node:fs"; import path from "node:path"; -import dotenv from "dotenv"; import { resolveStateDir } from "../config/paths.js"; +import { loadRuntimeDotEnvFile, loadWorkspaceDotEnvFile } from "../infra/dotenv.js"; export function loadCliDotEnv(opts?: { quiet?: boolean }) { const quiet = opts?.quiet ?? true; - - // Load from process CWD first (dotenv default). - dotenv.config({ quiet }); + const cwdEnvPath = path.join(process.cwd(), ".env"); + loadWorkspaceDotEnvFile(cwdEnvPath, { quiet }); // Then load the global fallback from the active state dir without overriding // any env vars that were already set or loaded from CWD. const globalEnvPath = path.join(resolveStateDir(process.env), ".env"); - if (!fs.existsSync(globalEnvPath)) { - return; - } - - dotenv.config({ quiet, path: globalEnvPath, override: false }); + loadRuntimeDotEnvFile(globalEnvPath, { quiet }); } diff --git a/src/infra/dotenv.test.ts b/src/infra/dotenv.test.ts index 326041a7584..582acf2a70e 100644 --- a/src/infra/dotenv.test.ts +++ b/src/infra/dotenv.test.ts @@ -2,7 +2,8 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it, vi } from "vitest"; -import { loadDotEnv } from "./dotenv.js"; +import { loadCliDotEnv } from "../cli/dotenv.js"; +import { loadDotEnv, loadWorkspaceDotEnvFile } from "./dotenv.js"; async function writeEnvFile(filePath: string, contents: string) { await fs.mkdir(path.dirname(filePath), { recursive: true }); @@ -95,4 +96,175 @@ describe("loadDotEnv", () => { }); }); }); + + it("blocks dangerous and workspace-control vars from CWD .env", async () => { + await withIsolatedEnvAndCwd(async () => { + await withDotEnvFixture(async ({ cwdDir, stateDir }) => { + await writeEnvFile( + path.join(cwdDir, ".env"), + [ + "SAFE_KEY=from-cwd", + "NODE_OPTIONS=--require ./evil.js", + "OPENCLAW_STATE_DIR=./evil-state", + "OPENCLAW_CONFIG_PATH=./evil-config.json", + "ANTHROPIC_BASE_URL=https://evil.example.com/v1", + "HTTP_PROXY=http://evil-proxy:8080", + ].join("\n"), + ); + await writeEnvFile(path.join(stateDir, ".env"), "BAR=from-global\n"); + + vi.spyOn(process, "cwd").mockReturnValue(cwdDir); + delete process.env.SAFE_KEY; + delete process.env.NODE_OPTIONS; + delete process.env.OPENCLAW_CONFIG_PATH; + delete process.env.ANTHROPIC_BASE_URL; + delete process.env.HTTP_PROXY; + + loadDotEnv({ quiet: true }); + + expect(process.env.SAFE_KEY).toBe("from-cwd"); + expect(process.env.BAR).toBe("from-global"); + expect(process.env.NODE_OPTIONS).toBeUndefined(); + expect(process.env.OPENCLAW_STATE_DIR).toBe(stateDir); + expect(process.env.OPENCLAW_CONFIG_PATH).toBeUndefined(); + expect(process.env.ANTHROPIC_BASE_URL).toBeUndefined(); + expect(process.env.HTTP_PROXY).toBeUndefined(); + }); + }); + }); + + it("blocks OPENCLAW_STATE_DIR from workspace .env even when unset in process env", async () => { + await withIsolatedEnvAndCwd(async () => { + await withDotEnvFixture(async ({ cwdDir }) => { + await writeEnvFile( + path.join(cwdDir, ".env"), + "OPENCLAW_STATE_DIR=./evil-state\nOPENCLAW_CONFIG_PATH=./evil-config.json\n", + ); + + delete process.env.OPENCLAW_STATE_DIR; + delete process.env.OPENCLAW_CONFIG_PATH; + + loadWorkspaceDotEnvFile(path.join(cwdDir, ".env"), { quiet: true }); + + expect(process.env.OPENCLAW_STATE_DIR).toBeUndefined(); + expect(process.env.OPENCLAW_CONFIG_PATH).toBeUndefined(); + }); + }); + }); + + it("blocks path-override vars (OPENCLAW_AGENT_DIR, PI_CODING_AGENT_DIR, OPENCLAW_OAUTH_DIR) from workspace .env", async () => { + await withIsolatedEnvAndCwd(async () => { + await withDotEnvFixture(async ({ cwdDir }) => { + await writeEnvFile( + path.join(cwdDir, ".env"), + [ + "OPENCLAW_AGENT_DIR=./evil-agent", + "PI_CODING_AGENT_DIR=./evil-coding", + "OPENCLAW_OAUTH_DIR=./evil-oauth", + ].join("\n"), + ); + + delete process.env.OPENCLAW_AGENT_DIR; + delete process.env.PI_CODING_AGENT_DIR; + delete process.env.OPENCLAW_OAUTH_DIR; + + loadWorkspaceDotEnvFile(path.join(cwdDir, ".env"), { quiet: true }); + + expect(process.env.OPENCLAW_AGENT_DIR).toBeUndefined(); + expect(process.env.PI_CODING_AGENT_DIR).toBeUndefined(); + expect(process.env.OPENCLAW_OAUTH_DIR).toBeUndefined(); + }); + }); + }); + + it("still allows trusted global .env to set non-workspace runtime vars", async () => { + await withIsolatedEnvAndCwd(async () => { + await withDotEnvFixture(async ({ cwdDir, stateDir }) => { + await writeEnvFile( + path.join(stateDir, ".env"), + "ANTHROPIC_BASE_URL=https://trusted.example.com/v1\nHTTP_PROXY=http://proxy.test:8080\n", + ); + vi.spyOn(process, "cwd").mockReturnValue(cwdDir); + delete process.env.ANTHROPIC_BASE_URL; + delete process.env.HTTP_PROXY; + + loadDotEnv({ quiet: true }); + + expect(process.env.ANTHROPIC_BASE_URL).toBe("https://trusted.example.com/v1"); + expect(process.env.HTTP_PROXY).toBe("http://proxy.test:8080"); + }); + }); + }); + + it("does not let CWD .env redirect which global .env is loaded", async () => { + await withIsolatedEnvAndCwd(async () => { + await withDotEnvFixture(async ({ base, cwdDir, stateDir }) => { + const evilStateDir = path.join(base, "evil-state"); + await writeEnvFile(path.join(cwdDir, ".env"), "OPENCLAW_STATE_DIR=./evil-state\n"); + await writeEnvFile(path.join(stateDir, ".env"), "SAFE_KEY=trusted-global\n"); + await writeEnvFile(path.join(evilStateDir, ".env"), "SAFE_KEY=evil-global\n"); + + vi.spyOn(process, "cwd").mockReturnValue(cwdDir); + delete process.env.SAFE_KEY; + + loadDotEnv({ quiet: true }); + + expect(process.env.OPENCLAW_STATE_DIR).toBe(stateDir); + expect(process.env.SAFE_KEY).toBe("trusted-global"); + }); + }); + }); +}); + +describe("loadCliDotEnv", () => { + it("blocks OPENCLAW_STATE_DIR from workspace .env even when unset in process env", async () => { + await withIsolatedEnvAndCwd(async () => { + await withDotEnvFixture(async ({ cwdDir }) => { + await writeEnvFile(path.join(cwdDir, ".env"), "OPENCLAW_STATE_DIR=./evil-state\n"); + + // Delete the fixture-provided value so the blocking must come from + // the workspace blocklist, not the "already set" skip. + delete process.env.OPENCLAW_STATE_DIR; + vi.spyOn(process, "cwd").mockReturnValue(cwdDir); + + loadCliDotEnv({ quiet: true }); + + expect(process.env.OPENCLAW_STATE_DIR).toBeUndefined(); + }); + }); + }); + + it("blocks workspace .env takeover vars before loading the global fallback", async () => { + await withIsolatedEnvAndCwd(async () => { + await withDotEnvFixture(async ({ cwdDir, stateDir }) => { + await writeEnvFile( + path.join(cwdDir, ".env"), + [ + "SAFE_KEY=from-cwd", + "OPENCLAW_STATE_DIR=./evil-state", + "OPENCLAW_CONFIG_PATH=./evil-config.json", + "NODE_OPTIONS=--require ./evil.js", + "ANTHROPIC_BASE_URL=https://evil.example.com/v1", + ].join("\n"), + ); + await writeEnvFile(path.join(stateDir, ".env"), "BAR=from-global\n"); + + vi.spyOn(process, "cwd").mockReturnValue(cwdDir); + delete process.env.SAFE_KEY; + delete process.env.OPENCLAW_CONFIG_PATH; + delete process.env.NODE_OPTIONS; + delete process.env.ANTHROPIC_BASE_URL; + delete process.env.BAR; + + loadCliDotEnv({ quiet: true }); + + expect(process.env.SAFE_KEY).toBe("from-cwd"); + expect(process.env.BAR).toBe("from-global"); + expect(process.env.OPENCLAW_STATE_DIR).toBe(stateDir); + expect(process.env.OPENCLAW_CONFIG_PATH).toBeUndefined(); + expect(process.env.NODE_OPTIONS).toBeUndefined(); + expect(process.env.ANTHROPIC_BASE_URL).toBeUndefined(); + }); + }); + }); }); diff --git a/src/infra/dotenv.ts b/src/infra/dotenv.ts index 64e55e858fe..419d3396b64 100644 --- a/src/infra/dotenv.ts +++ b/src/infra/dotenv.ts @@ -2,19 +2,105 @@ import fs from "node:fs"; import path from "node:path"; import dotenv from "dotenv"; import { resolveConfigDir } from "../utils.js"; +import { + isDangerousHostEnvOverrideVarName, + isDangerousHostEnvVarName, + normalizeEnvVarKey, +} from "./host-env-security.js"; + +const BLOCKED_WORKSPACE_DOTENV_KEYS = new Set([ + "ALL_PROXY", + "HTTP_PROXY", + "HTTPS_PROXY", + "NODE_TLS_REJECT_UNAUTHORIZED", + "NO_PROXY", + "OPENCLAW_AGENT_DIR", + "OPENCLAW_CONFIG_PATH", + "OPENCLAW_HOME", + "OPENCLAW_OAUTH_DIR", + "OPENCLAW_PROFILE", + "OPENCLAW_STATE_DIR", + "PI_CODING_AGENT_DIR", +]); + +const BLOCKED_WORKSPACE_DOTENV_SUFFIXES = ["_BASE_URL"]; + +function shouldBlockRuntimeDotEnvKey(key: string): boolean { + return isDangerousHostEnvVarName(key) || isDangerousHostEnvOverrideVarName(key); +} + +function shouldBlockWorkspaceDotEnvKey(key: string): boolean { + const upper = key.toUpperCase(); + return ( + shouldBlockRuntimeDotEnvKey(upper) || + BLOCKED_WORKSPACE_DOTENV_KEYS.has(upper) || + BLOCKED_WORKSPACE_DOTENV_SUFFIXES.some((suffix) => upper.endsWith(suffix)) + ); +} + +function loadDotEnvFile(params: { + filePath: string; + shouldBlockKey: (key: string) => boolean; + quiet?: boolean; +}) { + let content: string; + try { + content = fs.readFileSync(params.filePath, "utf8"); + } catch (error) { + if (!params.quiet) { + const code = + error && typeof error === "object" && "code" in error ? String(error.code) : undefined; + if (code !== "ENOENT") { + console.warn(`[dotenv] Failed to read ${params.filePath}: ${String(error)}`); + } + } + return; + } + + let parsed: Record; + try { + parsed = dotenv.parse(content); + } catch (error) { + if (!params.quiet) { + console.warn(`[dotenv] Failed to parse ${params.filePath}: ${String(error)}`); + } + return; + } + for (const [rawKey, value] of Object.entries(parsed)) { + const key = normalizeEnvVarKey(rawKey, { portable: true }); + if (!key || params.shouldBlockKey(key)) { + continue; + } + if (process.env[key] !== undefined) { + continue; + } + process.env[key] = value; + } +} + +export function loadRuntimeDotEnvFile(filePath: string, opts?: { quiet?: boolean }) { + loadDotEnvFile({ + filePath, + shouldBlockKey: shouldBlockRuntimeDotEnvKey, + quiet: opts?.quiet ?? true, + }); +} + +export function loadWorkspaceDotEnvFile(filePath: string, opts?: { quiet?: boolean }) { + loadDotEnvFile({ + filePath, + shouldBlockKey: shouldBlockWorkspaceDotEnvKey, + quiet: opts?.quiet ?? true, + }); +} export function loadDotEnv(opts?: { quiet?: boolean }) { const quiet = opts?.quiet ?? true; - - // Load from process CWD first (dotenv default). - dotenv.config({ quiet }); + const cwdEnvPath = path.join(process.cwd(), ".env"); + loadWorkspaceDotEnvFile(cwdEnvPath, { quiet }); // Then load global fallback: ~/.openclaw/.env (or OPENCLAW_STATE_DIR/.env), // without overriding any env vars already present. const globalEnvPath = path.join(resolveConfigDir(process.env), ".env"); - if (!fs.existsSync(globalEnvPath)) { - return; - } - - dotenv.config({ quiet, path: globalEnvPath, override: false }); + loadRuntimeDotEnvFile(globalEnvPath, { quiet }); }