refactor(security): centralize host env policy and harden env ingestion

This commit is contained in:
Peter Steinberger
2026-02-21 13:04:34 +01:00
parent 08e020881d
commit f202e73077
10 changed files with 201 additions and 31 deletions

View File

@@ -1,6 +1,8 @@
import Foundation
enum HostEnvSanitizer {
// Keep in sync with src/infra/host-env-security-policy.json.
// Parity is validated by src/infra/host-env-security.policy-parity.test.ts.
private static let blockedKeys: Set<String> = [
"NODE_OPTIONS",
"NODE_PATH",

View File

@@ -140,6 +140,31 @@ describe("buildGatewayInstallPlan", () => {
expect(plan.environment.HOME).toBe("/Users/me");
});
it("drops dangerous config env vars before service merge", async () => {
mockNodeGatewayPlanFixture({
serviceEnvironment: {
OPENCLAW_PORT: "3000",
},
});
const plan = await buildGatewayInstallPlan({
env: {},
port: 3000,
runtime: "node",
config: {
env: {
vars: {
NODE_OPTIONS: "--require /tmp/evil.js",
SAFE_KEY: "safe-value",
},
},
},
});
expect(plan.environment.NODE_OPTIONS).toBeUndefined();
expect(plan.environment.SAFE_KEY).toBe("safe-value");
});
it("does not include empty config env values", async () => {
mockNodeGatewayPlanFixture();

View File

@@ -1,5 +1,5 @@
import { formatCliCommand } from "../cli/command-format.js";
import { collectConfigEnvVars } from "../config/env-vars.js";
import { collectConfigServiceEnvVars } from "../config/env-vars.js";
import type { OpenClawConfig } from "../config/types.js";
import { resolveGatewayLaunchAgentLabel } from "../daemon/constants.js";
import { resolveGatewayProgramArguments } from "../daemon/program-args.js";
@@ -67,7 +67,7 @@ export async function buildGatewayInstallPlan(params: {
// Merge config env vars into the service environment (vars + inline env keys).
// Config env vars are added first so service-specific vars take precedence.
const environment: Record<string, string | undefined> = {
...collectConfigEnvVars(params.config),
...collectConfigServiceEnvVars(params.config),
};
Object.assign(environment, serviceEnvironment);

View File

@@ -3,7 +3,7 @@ import path from "node:path";
import { describe, expect, it } from "vitest";
import { loadDotEnv } from "../infra/dotenv.js";
import { resolveConfigEnvVars } from "./env-substitution.js";
import { applyConfigEnvVars, collectConfigEnvVars } from "./env-vars.js";
import { applyConfigEnvVars, collectConfigRuntimeEnvVars } from "./env-vars.js";
import { withEnvOverride, withTempHome } from "./test-helpers.js";
import type { OpenClawConfig } from "./types.js";
@@ -34,7 +34,7 @@ describe("config env vars", () => {
const config = {
env: { vars: { BASH_ENV: "/tmp/pwn.sh", OPENROUTER_API_KEY: "config-key" } },
};
const entries = collectConfigEnvVars(config as OpenClawConfig);
const entries = collectConfigRuntimeEnvVars(config as OpenClawConfig);
expect(entries.BASH_ENV).toBeUndefined();
expect(entries.OPENROUTER_API_KEY).toBe("config-key");
@@ -44,6 +44,24 @@ describe("config env vars", () => {
});
});
it("drops non-portable env keys from config env", async () => {
await withEnvOverride({ OPENROUTER_API_KEY: undefined }, async () => {
const config = {
env: {
vars: {
" BAD KEY": "oops",
OPENROUTER_API_KEY: "config-key",
},
"NOT-PORTABLE": "bad",
},
};
const entries = collectConfigRuntimeEnvVars(config as OpenClawConfig);
expect(entries.OPENROUTER_API_KEY).toBe("config-key");
expect(entries[" BAD KEY"]).toBeUndefined();
expect(entries["NOT-PORTABLE"]).toBeUndefined();
});
});
it("loads ${VAR} substitutions from ~/.openclaw/.env on repeated runtime loads", async () => {
await withTempHome(async (_home) => {
await withEnvOverride({ BRAVE_API_KEY: undefined }, async () => {

View File

@@ -1,7 +1,7 @@
import { isDangerousHostEnvVarName } from "../infra/host-env-security.js";
import { isDangerousHostEnvVarName, normalizeEnvVarKey } from "../infra/host-env-security.js";
import type { OpenClawConfig } from "./types.js";
export function collectConfigEnvVars(cfg?: OpenClawConfig): Record<string, string> {
function collectConfigEnvVarsByTarget(cfg?: OpenClawConfig): Record<string, string> {
const envConfig = cfg?.env;
if (!envConfig) {
return {};
@@ -10,10 +10,14 @@ export function collectConfigEnvVars(cfg?: OpenClawConfig): Record<string, strin
const entries: Record<string, string> = {};
if (envConfig.vars) {
for (const [key, value] of Object.entries(envConfig.vars)) {
for (const [rawKey, value] of Object.entries(envConfig.vars)) {
if (!value) {
continue;
}
const key = normalizeEnvVarKey(rawKey, { portable: true });
if (!key) {
continue;
}
if (isDangerousHostEnvVarName(key)) {
continue;
}
@@ -21,13 +25,17 @@ export function collectConfigEnvVars(cfg?: OpenClawConfig): Record<string, strin
}
}
for (const [key, value] of Object.entries(envConfig)) {
if (key === "shellEnv" || key === "vars") {
for (const [rawKey, value] of Object.entries(envConfig)) {
if (rawKey === "shellEnv" || rawKey === "vars") {
continue;
}
if (typeof value !== "string" || !value.trim()) {
continue;
}
const key = normalizeEnvVarKey(rawKey, { portable: true });
if (!key) {
continue;
}
if (isDangerousHostEnvVarName(key)) {
continue;
}
@@ -37,11 +45,24 @@ export function collectConfigEnvVars(cfg?: OpenClawConfig): Record<string, strin
return entries;
}
export function collectConfigRuntimeEnvVars(cfg?: OpenClawConfig): Record<string, string> {
return collectConfigEnvVarsByTarget(cfg);
}
export function collectConfigServiceEnvVars(cfg?: OpenClawConfig): Record<string, string> {
return collectConfigEnvVarsByTarget(cfg);
}
/** @deprecated Use `collectConfigRuntimeEnvVars` or `collectConfigServiceEnvVars`. */
export function collectConfigEnvVars(cfg?: OpenClawConfig): Record<string, string> {
return collectConfigRuntimeEnvVars(cfg);
}
export function applyConfigEnvVars(
cfg: OpenClawConfig,
env: NodeJS.ProcessEnv = process.env,
): void {
const entries = collectConfigEnvVars(cfg);
const entries = collectConfigRuntimeEnvVars(cfg);
for (const [key, value] of Object.entries(entries)) {
if (env[key]?.trim()) {
continue;

View File

@@ -37,7 +37,6 @@ import { parseTtsDirectives } from "../../tts/tts-core.js";
import { resolveTtsConfig, textToSpeech, type ResolvedTtsConfig } from "../../tts/tts.js";
const require = createRequire(import.meta.url);
const OpusScript = require("opusscript") as typeof import("opusscript");
const SAMPLE_RATE = 48_000;
const CHANNELS = 2;
@@ -149,6 +148,7 @@ type OpusDecoder = {
function createOpusDecoder(): { decoder: OpusDecoder; name: string } | null {
try {
const OpusScript = require("opusscript") as typeof import("opusscript");
const decoder = new OpusScript(SAMPLE_RATE, CHANNELS, OpusScript.Application.AUDIO);
return { decoder, name: "opusscript" };
} catch (err) {

View File

@@ -0,0 +1,18 @@
{
"blockedKeys": [
"NODE_OPTIONS",
"NODE_PATH",
"PYTHONHOME",
"PYTHONPATH",
"PERL5LIB",
"PERL5OPT",
"RUBYLIB",
"RUBYOPT",
"BASH_ENV",
"ENV",
"GCONV_PATH",
"IFS",
"SSLKEYLOGFILE"
],
"blockedPrefixes": ["DYLD_", "LD_", "BASH_FUNC_"]
}

View File

@@ -0,0 +1,38 @@
import fs from "node:fs";
import path from "node:path";
import { describe, expect, it } from "vitest";
type HostEnvSecurityPolicy = {
blockedKeys: string[];
blockedPrefixes: string[];
};
function parseSwiftStringArray(source: string, marker: string): string[] {
const escapedMarker = marker.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const re = new RegExp(`${escapedMarker}[\\s\\S]*?=\\s*\\[([\\s\\S]*?)\\]`, "m");
const match = source.match(re);
if (!match) {
throw new Error(`Failed to parse Swift array for marker: ${marker}`);
}
return Array.from(match[1].matchAll(/"([^"]+)"/g), (m) => m[1]);
}
describe("host env security policy parity", () => {
it("keeps macOS HostEnvSanitizer lists in sync with shared JSON policy", () => {
const repoRoot = process.cwd();
const policyPath = path.join(repoRoot, "src/infra/host-env-security-policy.json");
const swiftPath = path.join(repoRoot, "apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift");
const policy = JSON.parse(fs.readFileSync(policyPath, "utf8")) as HostEnvSecurityPolicy;
const swiftSource = fs.readFileSync(swiftPath, "utf8");
const swiftBlockedKeys = parseSwiftStringArray(swiftSource, "private static let blockedKeys");
const swiftBlockedPrefixes = parseSwiftStringArray(
swiftSource,
"private static let blockedPrefixes",
);
expect(swiftBlockedKeys).toEqual(policy.blockedKeys);
expect(swiftBlockedPrefixes).toEqual(policy.blockedPrefixes);
});
});

View File

@@ -1,5 +1,9 @@
import { describe, expect, it } from "vitest";
import { isDangerousHostEnvVarName, sanitizeHostExecEnv } from "./host-env-security.js";
import {
isDangerousHostEnvVarName,
normalizeEnvVarKey,
sanitizeHostExecEnv,
} from "./host-env-security.js";
describe("isDangerousHostEnvVarName", () => {
it("matches dangerous keys and prefixes case-insensitively", () => {
@@ -48,4 +52,30 @@ describe("sanitizeHostExecEnv", () => {
expect(env.SAFE).toBe("ok");
expect(env.HOME).toBe("/tmp/home");
});
it("drops non-portable env key names", () => {
const env = sanitizeHostExecEnv({
baseEnv: {
PATH: "/usr/bin:/bin",
},
overrides: {
" BAD KEY": "x",
"NOT-PORTABLE": "x",
GOOD_KEY: "ok",
},
});
expect(env.GOOD_KEY).toBe("ok");
expect(env[" BAD KEY"]).toBeUndefined();
expect(env["NOT-PORTABLE"]).toBeUndefined();
});
});
describe("normalizeEnvVarKey", () => {
it("normalizes and validates keys", () => {
expect(normalizeEnvVarKey(" OPENROUTER_API_KEY ")).toBe("OPENROUTER_API_KEY");
expect(normalizeEnvVarKey("NOT-PORTABLE", { portable: true })).toBeNull();
expect(normalizeEnvVarKey(" BASH_FUNC_echo%% ")).toBe("BASH_FUNC_echo%%");
expect(normalizeEnvVarKey(" ")).toBeNull();
});
});

View File

@@ -1,23 +1,41 @@
const HOST_DANGEROUS_ENV_KEY_VALUES = [
"NODE_OPTIONS",
"NODE_PATH",
"PYTHONHOME",
"PYTHONPATH",
"PERL5LIB",
"PERL5OPT",
"RUBYLIB",
"RUBYOPT",
"BASH_ENV",
"ENV",
"GCONV_PATH",
"IFS",
"SSLKEYLOGFILE",
] as const;
import HOST_ENV_SECURITY_POLICY_JSON from "./host-env-security-policy.json" with { type: "json" };
const PORTABLE_ENV_VAR_KEY = /^[A-Za-z_][A-Za-z0-9_]*$/;
type HostEnvSecurityPolicy = {
blockedKeys: string[];
blockedPrefixes: string[];
};
const HOST_ENV_SECURITY_POLICY = HOST_ENV_SECURITY_POLICY_JSON as HostEnvSecurityPolicy;
export const HOST_DANGEROUS_ENV_KEY_VALUES: readonly string[] = Object.freeze(
HOST_ENV_SECURITY_POLICY.blockedKeys.map((key) => key.toUpperCase()),
);
export const HOST_DANGEROUS_ENV_PREFIXES: readonly string[] = Object.freeze(
HOST_ENV_SECURITY_POLICY.blockedPrefixes.map((prefix) => prefix.toUpperCase()),
);
export const HOST_DANGEROUS_ENV_KEYS = new Set<string>(HOST_DANGEROUS_ENV_KEY_VALUES);
export const HOST_DANGEROUS_ENV_PREFIXES = ["DYLD_", "LD_", "BASH_FUNC_"] as const;
export function isDangerousHostEnvVarName(key: string): boolean {
export function normalizeEnvVarKey(
rawKey: string,
options?: { portable?: boolean },
): string | null {
const key = rawKey.trim();
if (!key) {
return null;
}
if (options?.portable && !PORTABLE_ENV_VAR_KEY.test(key)) {
return null;
}
return key;
}
export function isDangerousHostEnvVarName(rawKey: string): boolean {
const key = normalizeEnvVarKey(rawKey);
if (!key) {
return false;
}
const upper = key.toUpperCase();
if (HOST_DANGEROUS_ENV_KEYS.has(upper)) {
return true;
@@ -39,7 +57,7 @@ export function sanitizeHostExecEnv(params?: {
if (typeof value !== "string") {
continue;
}
const key = rawKey.trim();
const key = normalizeEnvVarKey(rawKey, { portable: true });
if (!key || isDangerousHostEnvVarName(key)) {
continue;
}
@@ -54,7 +72,7 @@ export function sanitizeHostExecEnv(params?: {
if (typeof value !== "string") {
continue;
}
const key = rawKey.trim();
const key = normalizeEnvVarKey(rawKey, { portable: true });
if (!key) {
continue;
}