Agents: sanitize skill env overrides

CHANGELOG.md

@@ -52,6 +52,7 @@ Docs: https://docs.openclaw.ai
 
 - Discord/Gateway: handle close code 4014 (missing privileged gateway intents) without crashing the gateway. Thanks @thewilloftheshadow.
 - Security/Net: strip sensitive headers (`Authorization`, `Proxy-Authorization`, `Cookie`, `Cookie2`) on cross-origin redirects in `fetchWithSsrFGuard` to prevent credential forwarding across origin boundaries. (#20313) Thanks @afurm.
+- Skills/Security: sanitize skill env overrides to block unsafe runtime injection variables and only allow sensitive keys when declared in skill metadata, with warnings for suspicious values. Thanks @thewilloftheshadow.
 - Auto-reply/Runner: emit `onAgentRunStart` only after agent lifecycle or tool activity begins (and only once per run), so fallback preflight errors no longer mark runs as started. (#21165) Thanks @shakkernerd.
 - Agents/Failover: treat non-default override runs as direct fallback-to-configured-primary (skip configured fallback chain), normalize default-model detection for provider casing/whitespace, and add regression coverage for override/auth error paths. (#18820) Thanks @Glucksberg.
 - Auto-reply/Tool results: serialize tool-result delivery and keep the delivery chain progressing after individual failures so concurrent tool outputs preserve user-visible ordering. (#21231) thanks @ahdernasr.

src/agents/sandbox/sanitize-env-vars.ts

@@ -42,7 +42,7 @@ export type EnvSanitizationOptions = {
   customAllowedPatterns?: ReadonlyArray<RegExp>;
 };
 
-function validateEnvVarValue(value: string): string | undefined {
+export function validateEnvVarValue(value: string): string | undefined {
   if (value.includes("\0")) {
     return "Contains null bytes";
   }

src/agents/skills.e2e.test.ts

@@ -296,4 +296,102 @@ describe("applySkillEnvOverrides", () => {
       }
     }
   });
+
+  it("blocks unsafe env overrides but allows declared secrets", async () => {
+    const workspaceDir = await makeWorkspace();
+    const skillDir = path.join(workspaceDir, "skills", "unsafe-env-skill");
+    await writeSkill({
+      dir: skillDir,
+      name: "unsafe-env-skill",
+      description: "Needs env",
+      metadata:
+        '{"openclaw":{"requires":{"env":["OPENAI_API_KEY","NODE_OPTIONS"]},"primaryEnv":"OPENAI_API_KEY"}}',
+    });
+
+    const entries = loadWorkspaceSkillEntries(workspaceDir, {
+      managedSkillsDir: path.join(workspaceDir, ".managed"),
+    });
+
+    const originalApiKey = process.env.OPENAI_API_KEY;
+    const originalNodeOptions = process.env.NODE_OPTIONS;
+    delete process.env.OPENAI_API_KEY;
+    delete process.env.NODE_OPTIONS;
+
+    const restore = applySkillEnvOverrides({
+      skills: entries,
+      config: {
+        skills: {
+          entries: {
+            "unsafe-env-skill": {
+              env: {
+                OPENAI_API_KEY: "sk-test",
+                NODE_OPTIONS: "--require /tmp/evil.js",
+              },
+            },
+          },
+        },
+      },
+    });
+
+    try {
+      expect(process.env.OPENAI_API_KEY).toBe("sk-test");
+      expect(process.env.NODE_OPTIONS).toBeUndefined();
+    } finally {
+      restore();
+      if (originalApiKey === undefined) {
+        expect(process.env.OPENAI_API_KEY).toBeUndefined();
+      } else {
+        expect(process.env.OPENAI_API_KEY).toBe(originalApiKey);
+      }
+      if (originalNodeOptions === undefined) {
+        expect(process.env.NODE_OPTIONS).toBeUndefined();
+      } else {
+        expect(process.env.NODE_OPTIONS).toBe(originalNodeOptions);
+      }
+    }
+  });
+
+  it("allows required env overrides from snapshots", async () => {
+    const workspaceDir = await makeWorkspace();
+    const skillDir = path.join(workspaceDir, "skills", "snapshot-env-skill");
+    await writeSkill({
+      dir: skillDir,
+      name: "snapshot-env-skill",
+      description: "Needs env",
+      metadata: '{"openclaw":{"requires":{"env":["OPENAI_API_KEY"]}}}',
+    });
+
+    const snapshot = buildWorkspaceSkillSnapshot(workspaceDir, {
+      managedSkillsDir: path.join(workspaceDir, ".managed"),
+    });
+
+    const originalApiKey = process.env.OPENAI_API_KEY;
+    delete process.env.OPENAI_API_KEY;
+
+    const restore = applySkillEnvOverridesFromSnapshot({
+      snapshot,
+      config: {
+        skills: {
+          entries: {
+            "snapshot-env-skill": {
+              env: {
+                OPENAI_API_KEY: "snap-secret",
+              },
+            },
+          },
+        },
+      },
+    });
+
+    try {
+      expect(process.env.OPENAI_API_KEY).toBe("snap-secret");
+    } finally {
+      restore();
+      if (originalApiKey === undefined) {
+        expect(process.env.OPENAI_API_KEY).toBeUndefined();
+      } else {
+        expect(process.env.OPENAI_API_KEY).toBe(originalApiKey);
+      }
+    }
+  });
 });

src/agents/skills/env-overrides.ts

@@ -1,4 +1,5 @@
 import type { OpenClawConfig } from "../../config/config.js";
+import { sanitizeEnvVars, validateEnvVarValue } from "../sandbox/sanitize-env-vars.js";
 import { resolveSkillConfig } from "./config.js";
 import { resolveSkillKey } from "./frontmatter.js";
 import type { SkillEntry, SkillSnapshot } from "./types.js";
@@ -6,25 +7,123 @@ import type { SkillEntry, SkillSnapshot } from "./types.js";
 type EnvUpdate = { key: string; prev: string | undefined };
 type SkillConfig = NonNullable<ReturnType<typeof resolveSkillConfig>>;
 
+type SanitizedSkillEnvOverrides = {
+  allowed: Record<string, string>;
+  blocked: string[];
+  warnings: string[];
+};
+
+// Never allow skill env overrides that can alter runtime loader flags.
+const HARD_BLOCKED_SKILL_ENV_PATTERNS: ReadonlyArray<RegExp> = [
+  /^NODE_OPTIONS$/i,
+  /^OPENSSL_CONF$/i,
+  /^LD_PRELOAD$/i,
+  /^DYLD_INSERT_LIBRARIES$/i,
+];
+
+function matchesAnyPattern(value: string, patterns: readonly RegExp[]): boolean {
+  return patterns.some((pattern) => pattern.test(value));
+}
+
+function sanitizeSkillEnvOverrides(params: {
+  overrides: Record<string, string>;
+  allowedSensitiveKeys: Set<string>;
+}): SanitizedSkillEnvOverrides {
+  if (Object.keys(params.overrides).length === 0) {
+    return { allowed: {}, blocked: [], warnings: [] };
+  }
+
+  const result = sanitizeEnvVars(params.overrides, {
+    customBlockedPatterns: HARD_BLOCKED_SKILL_ENV_PATTERNS,
+  });
+  const allowed = { ...result.allowed };
+  const blocked: string[] = [];
+  const warnings = [...result.warnings];
+
+  for (const key of result.blocked) {
+    if (
+      matchesAnyPattern(key, HARD_BLOCKED_SKILL_ENV_PATTERNS) ||
+      !params.allowedSensitiveKeys.has(key)
+    ) {
+      blocked.push(key);
+      continue;
+    }
+    const value = params.overrides[key];
+    if (!value) {
+      continue;
+    }
+    const warning = validateEnvVarValue(value);
+    if (warning) {
+      if (warning === "Contains null bytes") {
+        blocked.push(key);
+        continue;
+      }
+      warnings.push(`${key}: ${warning}`);
+    }
+    allowed[key] = value;
+  }
+
+  return { allowed, blocked, warnings };
+}
+
 function applySkillConfigEnvOverrides(params: {
   updates: EnvUpdate[];
   skillConfig: SkillConfig;
   primaryEnv?: string | null;
+  requiredEnv?: string[] | null;
+  skillKey: string;
 }) {
-  const { updates, skillConfig, primaryEnv } = params;
-  if (skillConfig.env) {
-    for (const [envKey, envValue] of Object.entries(skillConfig.env)) {
-      if (!envValue || process.env[envKey]) {
-        continue;
-      }
-      updates.push({ key: envKey, prev: process.env[envKey] });
-      process.env[envKey] = envValue;
+  const { updates, skillConfig, primaryEnv, requiredEnv, skillKey } = params;
+  const allowedSensitiveKeys = new Set<string>();
+  const normalizedPrimaryEnv = primaryEnv?.trim();
+  if (normalizedPrimaryEnv) {
+    allowedSensitiveKeys.add(normalizedPrimaryEnv);
+  }
+  for (const envName of requiredEnv ?? []) {
+    const trimmedEnv = envName.trim();
+    if (trimmedEnv) {
+      allowedSensitiveKeys.add(trimmedEnv);
     }
   }
 
-  if (primaryEnv && skillConfig.apiKey && !process.env[primaryEnv]) {
-    updates.push({ key: primaryEnv, prev: process.env[primaryEnv] });
-    process.env[primaryEnv] = skillConfig.apiKey;
+  const pendingOverrides: Record<string, string> = {};
+  if (skillConfig.env) {
+    for (const [rawKey, envValue] of Object.entries(skillConfig.env)) {
+      const envKey = rawKey.trim();
+      if (!envKey || !envValue || process.env[envKey]) {
+        continue;
+      }
+      pendingOverrides[envKey] = envValue;
+    }
+  }
+
+  if (normalizedPrimaryEnv && skillConfig.apiKey && !process.env[normalizedPrimaryEnv]) {
+    if (!pendingOverrides[normalizedPrimaryEnv]) {
+      pendingOverrides[normalizedPrimaryEnv] = skillConfig.apiKey;
+    }
+  }
+
+  const sanitized = sanitizeSkillEnvOverrides({
+    overrides: pendingOverrides,
+    allowedSensitiveKeys,
+  });
+
+  if (sanitized.blocked.length > 0) {
+    console.warn(
+      `[Security] Blocked skill env overrides for ${skillKey}:`,
+      sanitized.blocked.join(", "),
+    );
+  }
+  if (sanitized.warnings.length > 0) {
+    console.warn(`[Security] Suspicious skill env overrides for ${skillKey}:`, sanitized.warnings);
+  }
+
+  for (const [envKey, envValue] of Object.entries(sanitized.allowed)) {
+    if (process.env[envKey]) {
+      continue;
+    }
+    updates.push({ key: envKey, prev: process.env[envKey] });
+    process.env[envKey] = envValue;
   }
 }
 
@@ -55,6 +154,8 @@ export function applySkillEnvOverrides(params: { skills: SkillEntry[]; config?:
       updates,
       skillConfig,
       primaryEnv: entry.metadata?.primaryEnv,
+      requiredEnv: entry.metadata?.requires?.env,
+      skillKey,
     });
   }
 
@@ -81,6 +182,8 @@ export function applySkillEnvOverridesFromSnapshot(params: {
       updates,
       skillConfig,
       primaryEnv: skill.primaryEnv,
+      requiredEnv: skill.requiredEnv,
+      skillKey: skill.name,
     });
   }
 

src/agents/skills/types.ts

@@ -81,7 +81,7 @@ export type SkillEligibilityContext = {
 
 export type SkillSnapshot = {
   prompt: string;
-  skills: Array<{ name: string; primaryEnv?: string }>;
+  skills: Array<{ name: string; primaryEnv?: string; requiredEnv?: string[] }>;
   /** Normalized agent-level filter used to build this snapshot; undefined means unrestricted. */
   skillFilter?: string[];
   resolvedSkills?: Skill[];

src/agents/skills/workspace.ts

@@ -490,6 +490,7 @@ export function buildWorkspaceSkillSnapshot(
     skills: eligible.map((entry) => ({
       name: entry.skill.name,
       primaryEnv: entry.metadata?.primaryEnv,
+      requiredEnv: entry.metadata?.requires?.env?.slice(),
     })),
     ...(skillFilter === undefined ? {} : { skillFilter }),
     resolvedSkills,

src/config/sessions/types.ts

@@ -152,7 +152,7 @@ export type GroupKeyResolution = {
 
 export type SessionSkillSnapshot = {
   prompt: string;
-  skills: Array<{ name: string; primaryEnv?: string }>;
+  skills: Array<{ name: string; primaryEnv?: string; requiredEnv?: string[] }>;
   /** Normalized agent-level filter used to build this snapshot; undefined means unrestricted. */
   skillFilter?: string[];
   resolvedSkills?: Skill[];