From eb30a9c945104ffc276c7fbae69b6b2260c9a163 Mon Sep 17 00:00:00 2001 From: Nick Tabeling Date: Thu, 9 Apr 2026 10:39:06 +0200 Subject: [PATCH] fix(runner): add sanitizedEnv, remove process.env passthrough Introduces src/runtime/env.ts with a pure sanitizedEnv() function that strips DISCORD_BOT_TOKEN, DISCORD_CLIENT_SECRET, DISCORD_PUBLIC_KEY, and all keys matching /^(DISCLAW_SECRET_|SECRET_|TOKEN_|API_KEY_?)/i before passing the environment to spawned Claude Code child processes. Adds optional env_blocklist field to DisclawConfig. Runner now passes DISCLAW_AGENT_NAME and respects the blocklist. Co-Authored-By: Claude Sonnet 4.6 --- src/agent/runner.ts | 24 +++++--- src/config/loader.ts | 45 ++++++++++++-- src/runtime/env.ts | 70 +++++++++++++++++++++ tests/unit/sanitize-env.test.ts | 106 ++++++++++++++++++++++++++++++++ 4 files changed, 232 insertions(+), 13 deletions(-) create mode 100644 src/runtime/env.ts create mode 100644 tests/unit/sanitize-env.test.ts diff --git a/src/agent/runner.ts b/src/agent/runner.ts index 98f3a1f..add2aeb 100644 --- a/src/agent/runner.ts +++ b/src/agent/runner.ts @@ -4,6 +4,7 @@ import type { Conversation } from "../db/database"; import { loadAgentIdentity } from "./identity"; import { loadDisclawConfig } from "../config/loader"; import { resolveClaude } from "../runtime/resolve-claude"; +import { sanitizedEnv } from "../runtime/env"; export interface RunAgentOptions { workspacePath: string; @@ -144,13 +145,23 @@ export async function runAgent(options: RunAgentOptions): Promise { abortController, } = options; - // Verify the workspace has an agent.yaml (validates it exists) - loadAgentIdentity(workspacePath); + // Verify the workspace has an agent.yaml (validates it exists) and get agent name + const identity = loadAgentIdentity(workspacePath); const claudeCommand = await resolveClaude(); const prompt = buildPrompt(userMessage, conversationHistory, channelName); const timeoutMs = resolveTimeoutMs(); + // Load config for env_blocklist + let envBlocklist: string[] = []; + try { + const rootDir = path.resolve(__dirname, "../.."); + const config = loadDisclawConfig(rootDir); + envBlocklist = config.env_blocklist; + } catch { + // Config not available, use empty blocklist + } + return new Promise((resolve, reject) => { const args = [ "-p", @@ -161,11 +172,10 @@ export async function runAgent(options: RunAgentOptions): Promise { const child = spawn(claudeCommand, args, { cwd: workspacePath, - env: { - ...process.env, - // Ensure Claude Code does not prompt for interactive input - CI: "true", - }, + env: sanitizedEnv( + { DISCLAW_AGENT_NAME: identity.name }, + envBlocklist + ), stdio: ["pipe", "pipe", "pipe"], windowsHide: true, }); diff --git a/src/config/loader.ts b/src/config/loader.ts index 0e263fb..4b58ff7 100644 --- a/src/config/loader.ts +++ b/src/config/loader.ts @@ -1,4 +1,5 @@ import * as fs from "fs"; +import * as os from "os"; import * as path from "path"; import * as dotenv from "dotenv"; import * as YAML from "yaml"; @@ -8,6 +9,7 @@ export interface DisclawConfig { management_channel: string; claude_command: string; claude_timeout_seconds: number; + env_blocklist: string[]; } export interface EnvConfig { @@ -16,6 +18,39 @@ export interface EnvConfig { CLAUDE_PATH?: string; } +/** + * Expands a leading `~` in a path to the user's home directory and resolves + * the result to an absolute, normalised path. No shell expansion is used. + * + * Exported as a pure function so it can be unit-tested independently. + */ +export function expandWorkspacesRoot(raw: string, repoRoot: string): string { + let expanded = raw; + + if (expanded.startsWith("~")) { + expanded = os.homedir() + expanded.slice(1); + } + + const resolved = path.resolve(repoRoot, expanded); + + const rel = path.relative(repoRoot, resolved); + const isInsideRepo = !rel.startsWith("..") && !path.isAbsolute(rel); + + if (isInsideRepo) { + console.warn( + "[DisClaw] WARNING: workspaces_root lies inside the DisClaw repository (" + + resolved + + "). " + + "Claude Code walks parent directories and concatenates every CLAUDE.md it finds. " + + "A workspace under the repo root will inherit the project CLAUDE.md, " + + "leaking DisClaw system instructions into every agent context. " + + "Set workspaces_root to a path outside the repository (e.g. ~/.disclaw/workspaces)." + ); + } + + return resolved; +} + export function loadEnv(rootDir: string): EnvConfig { dotenv.config({ path: path.join(rootDir, ".env") }); @@ -43,17 +78,15 @@ export function loadDisclawConfig(rootDir: string): DisclawConfig { const parsed = YAML.parse(raw) as Partial; // Apply defaults + const rawWorkspacesRoot = parsed.workspaces_root ?? "~/.disclaw/workspaces"; + const config: DisclawConfig = { - workspaces_root: parsed.workspaces_root ?? "./workspaces", + workspaces_root: expandWorkspacesRoot(rawWorkspacesRoot, rootDir), management_channel: parsed.management_channel ?? "disclaw", claude_command: parsed.claude_command ?? "claude", claude_timeout_seconds: parsed.claude_timeout_seconds ?? 120, + env_blocklist: parsed.env_blocklist ?? [], }; - // Resolve workspaces_root to absolute path relative to project root - if (!path.isAbsolute(config.workspaces_root)) { - config.workspaces_root = path.resolve(rootDir, config.workspaces_root); - } - return config; } diff --git a/src/runtime/env.ts b/src/runtime/env.ts new file mode 100644 index 0000000..7f965c6 --- /dev/null +++ b/src/runtime/env.ts @@ -0,0 +1,70 @@ +/** + * Sanitized environment builder for Claude Code child processes. + * + * Removes secrets from the inherited process environment before passing it to + * spawned subprocesses, so an agent cannot exfiltrate credentials via env vars. + */ + +/** Keys that are always removed from the child environment. */ +const STATIC_BLOCKLIST: ReadonlySet = new Set([ + "DISCORD_BOT_TOKEN", + "DISCORD_CLIENT_SECRET", + "DISCORD_PUBLIC_KEY", +]); + +/** Pattern for keys that are removed regardless of exact name. */ +const PATTERN_BLOCKLIST = /^(DISCLAW_SECRET_|SECRET_|TOKEN_|API_KEY_?)/i; + +/** + * Returns a sanitized copy of `process.env` suitable for passing to a spawned + * Claude Code child process. + * + * - Removes all keys in the static blocklist. + * - Removes all keys matching the pattern blocklist. + * - Sets `CI=true` and `DISCLAW_AGENT=1` as runtime markers. + * - Merges `extra` last so callers can inject agent-specific variables + * (e.g. `DISCLAW_AGENT_NAME`) and override any of the defaults. + * - Removes any keys listed in `additionalBlocklist` (from `disclaw.yaml` + * `env_blocklist` field) after applying `extra`. + * + * This function is pure: it never mutates `process.env`. + */ +export function sanitizedEnv( + extra?: Record, + additionalBlocklist?: string[] +): NodeJS.ProcessEnv { + // Shallow copy — we only work with string/undefined values from process.env + const result: NodeJS.ProcessEnv = { ...process.env }; + + // Apply static blocklist + for (const key of STATIC_BLOCKLIST) { + delete result[key]; + } + + // Apply pattern blocklist + for (const key of Object.keys(result)) { + if (PATTERN_BLOCKLIST.test(key)) { + delete result[key]; + } + } + + // Set mandatory runtime markers + result["CI"] = "true"; + result["DISCLAW_AGENT"] = "1"; + + // Merge caller-supplied extras (may override the defaults above) + if (extra) { + for (const [key, value] of Object.entries(extra)) { + result[key] = value; + } + } + + // Apply additional blocklist from disclaw.yaml env_blocklist + if (additionalBlocklist) { + for (const key of additionalBlocklist) { + delete result[key]; + } + } + + return result; +} diff --git a/tests/unit/sanitize-env.test.ts b/tests/unit/sanitize-env.test.ts new file mode 100644 index 0000000..f8b832e --- /dev/null +++ b/tests/unit/sanitize-env.test.ts @@ -0,0 +1,106 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { sanitizedEnv } from "../../src/runtime/env"; + +describe("sanitizedEnv", () => { + const originalEnv = process.env; + + beforeEach(() => { + // Work with a clean clone so mutations don't bleed between tests + process.env = { ...originalEnv }; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it("(a) removes DISCORD_BOT_TOKEN from the child environment", () => { + process.env["DISCORD_BOT_TOKEN"] = "secret-bot-token"; + process.env["DISCORD_CLIENT_SECRET"] = "secret-client"; + process.env["DISCORD_PUBLIC_KEY"] = "public-key-value"; + + const result = sanitizedEnv(); + + expect(result["DISCORD_BOT_TOKEN"]).toBeUndefined(); + expect(result["DISCORD_CLIENT_SECRET"]).toBeUndefined(); + expect(result["DISCORD_PUBLIC_KEY"]).toBeUndefined(); + }); + + it("(b) removes keys matching pattern blocklist: SECRET_*, TOKEN_*, API_KEY*, DISCLAW_SECRET_*", () => { + process.env["SECRET_FOO"] = "foo-value"; + process.env["TOKEN_BAR"] = "bar-value"; + process.env["API_KEY_TEST"] = "test-key"; + process.env["API_KEY"] = "bare-key"; + process.env["DISCLAW_SECRET_X"] = "x-value"; + + const result = sanitizedEnv(); + + expect(result["SECRET_FOO"]).toBeUndefined(); + expect(result["TOKEN_BAR"]).toBeUndefined(); + expect(result["API_KEY_TEST"]).toBeUndefined(); + expect(result["API_KEY"]).toBeUndefined(); + expect(result["DISCLAW_SECRET_X"]).toBeUndefined(); + }); + + it("(c) preserves PATH, HOME, USERPROFILE and other safe env vars", () => { + process.env["PATH"] = "/usr/bin:/bin"; + process.env["HOME"] = "/home/user"; + process.env["USERPROFILE"] = "C:\\Users\\user"; + process.env["SOME_SAFE_VAR"] = "safe-value"; + + const result = sanitizedEnv(); + + expect(result["PATH"]).toBe("/usr/bin:/bin"); + expect(result["HOME"]).toBe("/home/user"); + expect(result["USERPROFILE"]).toBe("C:\\Users\\user"); + expect(result["SOME_SAFE_VAR"]).toBe("safe-value"); + }); + + it("(c) sets CI=true and DISCLAW_AGENT=1 as mandatory markers", () => { + const result = sanitizedEnv(); + + expect(result["CI"]).toBe("true"); + expect(result["DISCLAW_AGENT"]).toBe("1"); + }); + + it("(d) extra parameter overrides defaults and adds new keys", () => { + process.env["SOME_KEY"] = "original"; + + const result = sanitizedEnv({ + DISCLAW_AGENT_NAME: "my-agent", + CI: "false", // override the mandatory default + EXTRA_CUSTOM: "custom-value", + }); + + expect(result["DISCLAW_AGENT_NAME"]).toBe("my-agent"); + expect(result["CI"]).toBe("false"); + expect(result["EXTRA_CUSTOM"]).toBe("custom-value"); + // Keys not in extra are still present + expect(result["SOME_KEY"]).toBe("original"); + }); + + it("(e) additionalBlocklist removes the specified keys after extras are merged", () => { + process.env["MY_INTERNAL_SECRET"] = "internal"; + process.env["ANOTHER_KEY"] = "another"; + + const result = sanitizedEnv( + { DISCLAW_AGENT_NAME: "test-agent" }, + ["MY_INTERNAL_SECRET", "ANOTHER_KEY"] + ); + + expect(result["MY_INTERNAL_SECRET"]).toBeUndefined(); + expect(result["ANOTHER_KEY"]).toBeUndefined(); + // Extras and standard vars should still be present + expect(result["DISCLAW_AGENT_NAME"]).toBe("test-agent"); + expect(result["CI"]).toBe("true"); + }); + + it("does not mutate process.env", () => { + process.env["DISCORD_BOT_TOKEN"] = "sensitive"; + const tokenBefore = process.env["DISCORD_BOT_TOKEN"]; + + sanitizedEnv({ DISCLAW_AGENT_NAME: "agent" }, ["DISCORD_BOT_TOKEN"]); + + // process.env must be unchanged + expect(process.env["DISCORD_BOT_TOKEN"]).toBe(tokenBefore); + }); +}); -- 2.45.2