DIS-003: sanitizedEnv() -- Secrets aus Child-Env entfernen #22

Closed
dev wants to merge 2 commits from phase-0/sanitize-env into main
4 changed files with 232 additions and 13 deletions
Showing only changes of commit eb30a9c945 - Show all commits

View file

@ -4,6 +4,7 @@ import type { Conversation } from "../db/database";
import { loadAgentIdentity } from "./identity"; import { loadAgentIdentity } from "./identity";
import { loadDisclawConfig } from "../config/loader"; import { loadDisclawConfig } from "../config/loader";
import { resolveClaude } from "../runtime/resolve-claude"; import { resolveClaude } from "../runtime/resolve-claude";
import { sanitizedEnv } from "../runtime/env";
export interface RunAgentOptions { export interface RunAgentOptions {
workspacePath: string; workspacePath: string;
@ -144,13 +145,23 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
abortController, abortController,
} = options; } = options;
// Verify the workspace has an agent.yaml (validates it exists) // Verify the workspace has an agent.yaml (validates it exists) and get agent name
loadAgentIdentity(workspacePath); const identity = loadAgentIdentity(workspacePath);
const claudeCommand = await resolveClaude(); const claudeCommand = await resolveClaude();
const prompt = buildPrompt(userMessage, conversationHistory, channelName); const prompt = buildPrompt(userMessage, conversationHistory, channelName);
const timeoutMs = resolveTimeoutMs(); 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<string>((resolve, reject) => { return new Promise<string>((resolve, reject) => {
const args = [ const args = [
"-p", "-p",
@ -161,11 +172,10 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
const child = spawn(claudeCommand, args, { const child = spawn(claudeCommand, args, {
cwd: workspacePath, cwd: workspacePath,
env: { env: sanitizedEnv(
...process.env, { DISCLAW_AGENT_NAME: identity.name },
// Ensure Claude Code does not prompt for interactive input envBlocklist
CI: "true", ),
},
stdio: ["pipe", "pipe", "pipe"], stdio: ["pipe", "pipe", "pipe"],
windowsHide: true, windowsHide: true,
}); });

View file

@ -1,4 +1,5 @@
import * as fs from "fs"; import * as fs from "fs";
import * as os from "os";
import * as path from "path"; import * as path from "path";
import * as dotenv from "dotenv"; import * as dotenv from "dotenv";
import * as YAML from "yaml"; import * as YAML from "yaml";
@ -8,6 +9,7 @@ export interface DisclawConfig {
management_channel: string; management_channel: string;
claude_command: string; claude_command: string;
claude_timeout_seconds: number; claude_timeout_seconds: number;
env_blocklist: string[];
} }
export interface EnvConfig { export interface EnvConfig {
@ -16,6 +18,39 @@ export interface EnvConfig {
CLAUDE_PATH?: string; 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 { export function loadEnv(rootDir: string): EnvConfig {
dotenv.config({ path: path.join(rootDir, ".env") }); dotenv.config({ path: path.join(rootDir, ".env") });
@ -43,17 +78,15 @@ export function loadDisclawConfig(rootDir: string): DisclawConfig {
const parsed = YAML.parse(raw) as Partial<DisclawConfig>; const parsed = YAML.parse(raw) as Partial<DisclawConfig>;
// Apply defaults // Apply defaults
const rawWorkspacesRoot = parsed.workspaces_root ?? "~/.disclaw/workspaces";
const config: DisclawConfig = { const config: DisclawConfig = {
workspaces_root: parsed.workspaces_root ?? "./workspaces", workspaces_root: expandWorkspacesRoot(rawWorkspacesRoot, rootDir),
management_channel: parsed.management_channel ?? "disclaw", management_channel: parsed.management_channel ?? "disclaw",
claude_command: parsed.claude_command ?? "claude", claude_command: parsed.claude_command ?? "claude",
claude_timeout_seconds: parsed.claude_timeout_seconds ?? 120, 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; return config;
} }

70
src/runtime/env.ts Normal file
View file

@ -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<string> = 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<string, string>,
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;
}

View file

@ -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);
});
});