From ca1902fb4e9332cfbc2d7e00cea3b21421f940fd Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 15 Jan 2026 05:31:35 +0000 Subject: [PATCH] feat(security): expand audit and safe --fix --- CHANGELOG.md | 4 +- SECURITY.md | 15 + docs/gateway/security.md | 44 ++- src/security/audit-extra.ts | 574 ++++++++++++++++++++++++++++++++++++ src/security/audit-fs.ts | 64 ++++ src/security/audit.test.ts | 144 +++++++++ src/security/audit.ts | 107 +++---- src/security/fix.test.ts | 57 ++++ src/security/fix.ts | 139 ++++++++- 9 files changed, 1077 insertions(+), 71 deletions(-) create mode 100644 SECURITY.md create mode 100644 src/security/audit-extra.ts create mode 100644 src/security/audit-fs.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 856c0d772..1c7510ff9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,8 @@ - Daemon: support profile-aware service names for multi-gateway setups. (#671) — thanks @bjesuiter. - Docs: add FAQ entries for missing provider auth after adding agents and Gemini thinking signature errors. - Agents: add optional auth-profile copy prompt on `agents add` and improve auth error messaging. -- Security: add `clawdbot security audit` (`--deep`, `--fix`) and surface it in `status --all` and `doctor`. -- Security: add `clawdbot security audit` (`--deep`, `--fix`) and surface it in `status --all` and `doctor` (includes browser control exposure checks). +- Security: expand `clawdbot security audit` checks (model hygiene, config includes, plugin allowlists, exposure matrix) and extend `--fix` to tighten more sensitive state paths. +- Security: add `SECURITY.md` reporting policy. - Plugins: add Zalo channel plugin with gateway HTTP hooks and onboarding install prompt. (#854) — thanks @longmaba. - Onboarding: add a security checkpoint prompt (docs link + sandboxing hint); require `--accept-risk` for `--non-interactive`. - Docs: expand gateway security hardening guidance and incident response checklist. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 000000000..d2af462ba --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,15 @@ +# Security Policy + +If you believe you’ve found a security issue in Clawdbot, please report it privately. + +## Reporting + +- Email: `steipete@gmail.com` +- What to include: reproduction steps, impact assessment, and (if possible) a minimal PoC. + +## Operational Guidance + +For threat model + hardening guidance (including `clawdbot security audit --deep` and `--fix`), see: + +- `https://docs.clawd.bot/gateway/security` + diff --git a/docs/gateway/security.md b/docs/gateway/security.md index 935772882..b086f0cd1 100644 --- a/docs/gateway/security.md +++ b/docs/gateway/security.md @@ -27,7 +27,30 @@ It flags common footguns (Gateway auth exposure, browser control exposure, eleva `--fix` applies safe guardrails: - Tighten `groupPolicy="open"` to `groupPolicy="allowlist"` (and per-account variants) for common channels. - Turn `logging.redactSensitive="off"` back to `"tools"`. -- Tighten local perms (`~/.clawdbot` → `700`, config file → `600`). +- Tighten local perms (`~/.clawdbot` → `700`, config file → `600`, plus common state files like `credentials/*.json`, `agents/*/agent/auth-profiles.json`, and `agents/*/sessions/sessions.json`). + +### What the audit checks (high level) + +- **Inbound access** (DM policies, group policies, allowlists): can strangers trigger the bot? +- **Tool blast radius** (elevated tools + open rooms): could prompt injection turn into shell/file/network actions? +- **Network exposure** (Gateway bind/auth, Tailscale Serve/Funnel). +- **Browser control exposure** (remote controlUrl without token, HTTP, token reuse). +- **Local disk hygiene** (permissions, symlinks, config includes, “synced folder” paths). +- **Plugins** (extensions exist without an explicit allowlist). +- **Model hygiene** (warn when configured models look legacy; not a hard block). + +If you run `--deep`, Clawdbot also attempts a best-effort live Gateway probe. + +## Security Audit Checklist + +When the audit prints findings, treat this as a priority order: + +1. **Anything “open” + tools enabled**: lock down DMs/groups first (pairing/allowlists), then tighten tool policy/sandboxing. +2. **Public network exposure** (LAN bind, Funnel, missing auth): fix immediately. +3. **Browser control remote exposure**: treat it like a remote admin API (token required; HTTPS/tailnet-only). +4. **Permissions**: make sure state/config/credentials/auth are not group/world-readable. +5. **Plugins/extensions**: only load what you explicitly trust. +6. **Model choice**: prefer modern, instruction-hardened models for any bot with tools. ## The Threat Model @@ -108,7 +131,7 @@ Even with strong system prompts, **prompt injection is not solved**. What helps - Prefer mention gating in groups; avoid “always-on” bots in public rooms. - Treat links and pasted instructions as hostile by default. - Run sensitive tool execution in a sandbox; keep secrets out of the agent’s reachable filesystem. -- **Model choice matters:** we recommend Anthropic Opus 4.5 because it’s quite good at recognizing prompt injections (see [“A step forward on safety”](https://www.anthropic.com/news/claude-opus-4-5)). Using weaker models increases risk. +- **Model choice matters:** older/legacy models can be less robust against prompt injection and tool misuse. Prefer modern, instruction-hardened models for any bot with tools. We recommend Anthropic Opus 4.5 because it’s quite good at recognizing prompt injections (see [“A step forward on safety”](https://www.anthropic.com/news/claude-opus-4-5)). ## Reasoning & verbose output in groups @@ -117,6 +140,23 @@ was not meant for a public channel. In group settings, treat them as **debug only** and keep them off unless you explicitly need them. If you enable them, do so only in trusted DMs or tightly controlled rooms. +## Incident Response (if you suspect compromise) + +Assume “compromised” means: someone got into a room that can trigger the bot, or a token leaked, or a plugin/tool did something unexpected. + +1. **Stop the blast radius** + - Disable elevated tools (or stop the Gateway) until you understand what happened. + - Lock down inbound surfaces (DM policy, group allowlists, mention gating). +2. **Rotate secrets** + - Rotate `gateway.auth` token/password. + - Rotate `browser.controlToken` and `hooks.token` (if used). + - Revoke/rotate model provider credentials (API keys / OAuth). +3. **Review artifacts** + - Check Gateway logs and recent sessions/transcripts for unexpected tool calls. + - Review `extensions/` and remove anything you don’t fully trust. +4. **Re-run audit** + - `clawdbot security audit --deep` and confirm the report is clean. + ## Lessons Learned (The Hard Way) ### The `find ~` Incident 🦞 diff --git a/src/security/audit-extra.ts b/src/security/audit-extra.ts new file mode 100644 index 000000000..242363929 --- /dev/null +++ b/src/security/audit-extra.ts @@ -0,0 +1,574 @@ +import fs from "node:fs/promises"; +import path from "node:path"; + +import JSON5 from "json5"; + +import type { ClawdbotConfig, ConfigFileSnapshot } from "../config/config.js"; +import { createConfigIO } from "../config/config.js"; +import { resolveOAuthDir } from "../config/paths.js"; +import { resolveDefaultAgentId } from "../agents/agent-scope.js"; +import { INCLUDE_KEY, MAX_INCLUDE_DEPTH } from "../config/includes.js"; +import { normalizeAgentId } from "../routing/session-key.js"; +import { + formatOctal, + isGroupReadable, + isGroupWritable, + isWorldReadable, + isWorldWritable, + modeBits, + safeStat, +} from "./audit-fs.js"; + +export type SecurityAuditFinding = { + checkId: string; + severity: "info" | "warn" | "critical"; + title: string; + detail: string; + remediation?: string; +}; + +function expandTilde(p: string, env: NodeJS.ProcessEnv): string | null { + if (!p.startsWith("~")) return p; + const home = typeof env.HOME === "string" && env.HOME.trim() ? env.HOME.trim() : null; + if (!home) return null; + if (p === "~") return home; + if (p.startsWith("~/") || p.startsWith("~\\")) return path.join(home, p.slice(2)); + return null; +} + +function summarizeGroupPolicy(cfg: ClawdbotConfig): { open: number; allowlist: number; other: number } { + const channels = cfg.channels as Record | undefined; + if (!channels || typeof channels !== "object") return { open: 0, allowlist: 0, other: 0 }; + let open = 0; + let allowlist = 0; + let other = 0; + for (const value of Object.values(channels)) { + if (!value || typeof value !== "object") continue; + const section = value as Record; + const policy = section.groupPolicy; + if (policy === "open") open += 1; + else if (policy === "allowlist") allowlist += 1; + else other += 1; + } + return { open, allowlist, other }; +} + +export function collectAttackSurfaceSummaryFindings(cfg: ClawdbotConfig): SecurityAuditFinding[] { + const group = summarizeGroupPolicy(cfg); + const elevated = cfg.tools?.elevated?.enabled !== false; + const hooksEnabled = cfg.hooks?.enabled === true; + const browserEnabled = Boolean(cfg.browser?.enabled ?? cfg.browser?.controlUrl); + + const detail = + `groups: open=${group.open}, allowlist=${group.allowlist}` + + `\n` + + `tools.elevated: ${elevated ? "enabled" : "disabled"}` + + `\n` + + `hooks: ${hooksEnabled ? "enabled" : "disabled"}` + + `\n` + + `browser control: ${browserEnabled ? "enabled" : "disabled"}`; + + return [ + { + checkId: "summary.attack_surface", + severity: "info", + title: "Attack surface summary", + detail, + }, + ]; +} + +function isProbablySyncedPath(p: string): boolean { + const s = p.toLowerCase(); + return ( + s.includes("icloud") || + s.includes("dropbox") || + s.includes("google drive") || + s.includes("googledrive") || + s.includes("onedrive") + ); +} + +export function collectSyncedFolderFindings(params: { + stateDir: string; + configPath: string; +}): SecurityAuditFinding[] { + const findings: SecurityAuditFinding[] = []; + if (isProbablySyncedPath(params.stateDir) || isProbablySyncedPath(params.configPath)) { + findings.push({ + checkId: "fs.synced_dir", + severity: "warn", + title: "State/config path looks like a synced folder", + detail: `stateDir=${params.stateDir}, configPath=${params.configPath}. Synced folders (iCloud/Dropbox/OneDrive/Google Drive) can leak tokens and transcripts onto other devices.`, + remediation: `Keep CLAWDBOT_STATE_DIR on a local-only volume and re-run "clawdbot security audit --fix".`, + }); + } + return findings; +} + +function looksLikeEnvRef(value: string): boolean { + const v = value.trim(); + return v.startsWith("${") && v.endsWith("}"); +} + +export function collectSecretsInConfigFindings(cfg: ClawdbotConfig): SecurityAuditFinding[] { + const findings: SecurityAuditFinding[] = []; + const password = typeof cfg.gateway?.auth?.password === "string" ? cfg.gateway.auth.password.trim() : ""; + if (password && !looksLikeEnvRef(password)) { + findings.push({ + checkId: "config.secrets.gateway_password_in_config", + severity: "warn", + title: "Gateway password is stored in config", + detail: "gateway.auth.password is set in the config file; prefer environment variables for secrets when possible.", + remediation: "Prefer CLAWDBOT_GATEWAY_PASSWORD (env) and remove gateway.auth.password from disk.", + }); + } + + const browserToken = typeof cfg.browser?.controlToken === "string" ? cfg.browser.controlToken.trim() : ""; + if (browserToken && !looksLikeEnvRef(browserToken)) { + findings.push({ + checkId: "config.secrets.browser_control_token_in_config", + severity: "warn", + title: "Browser control token is stored in config", + detail: "browser.controlToken is set in the config file; prefer environment variables for secrets when possible.", + remediation: "Prefer CLAWDBOT_BROWSER_CONTROL_TOKEN (env) and remove browser.controlToken from disk.", + }); + } + + const hooksToken = typeof cfg.hooks?.token === "string" ? cfg.hooks.token.trim() : ""; + if (cfg.hooks?.enabled === true && hooksToken && !looksLikeEnvRef(hooksToken)) { + findings.push({ + checkId: "config.secrets.hooks_token_in_config", + severity: "info", + title: "Hooks token is stored in config", + detail: "hooks.token is set in the config file; keep config perms tight and treat it like an API secret.", + }); + } + + return findings; +} + +export function collectHooksHardeningFindings(cfg: ClawdbotConfig): SecurityAuditFinding[] { + const findings: SecurityAuditFinding[] = []; + if (cfg.hooks?.enabled !== true) return findings; + + const token = typeof cfg.hooks?.token === "string" ? cfg.hooks.token.trim() : ""; + if (token && token.length < 24) { + findings.push({ + checkId: "hooks.token_too_short", + severity: "warn", + title: "Hooks token looks short", + detail: `hooks.token is ${token.length} chars; prefer a long random token.`, + }); + } + + const gatewayToken = + typeof cfg.gateway?.auth?.token === "string" && cfg.gateway.auth.token.trim() + ? cfg.gateway.auth.token.trim() + : null; + if (token && gatewayToken && token === gatewayToken) { + findings.push({ + checkId: "hooks.token_reuse_gateway_token", + severity: "warn", + title: "Hooks token reuses the Gateway token", + detail: "hooks.token matches gateway.auth token; compromise of hooks expands blast radius to the Gateway API.", + remediation: "Use a separate hooks.token dedicated to hook ingress.", + }); + } + + const browserToken = + typeof cfg.browser?.controlToken === "string" && cfg.browser.controlToken.trim() + ? cfg.browser.controlToken.trim() + : process.env.CLAWDBOT_BROWSER_CONTROL_TOKEN?.trim() || null; + if (token && browserToken && token === browserToken) { + findings.push({ + checkId: "hooks.token_reuse_browser_token", + severity: "warn", + title: "Hooks token reuses the browser control token", + detail: "hooks.token matches browser control token; compromise of hooks may enable browser control endpoints.", + remediation: "Use a separate hooks.token dedicated to hook ingress.", + }); + } + + const rawPath = typeof cfg.hooks?.path === "string" ? cfg.hooks.path.trim() : ""; + if (rawPath === "/") { + findings.push({ + checkId: "hooks.path_root", + severity: "critical", + title: "Hooks base path is '/'", + detail: "hooks.path='/' would shadow other HTTP endpoints and is unsafe.", + remediation: "Use a dedicated path like '/hooks'.", + }); + } + + return findings; +} + +type ModelRef = { id: string; source: string }; + +function addModel(models: ModelRef[], raw: unknown, source: string) { + if (typeof raw !== "string") return; + const id = raw.trim(); + if (!id) return; + models.push({ id, source }); +} + +function collectModels(cfg: ClawdbotConfig): ModelRef[] { + const out: ModelRef[] = []; + addModel(out, cfg.agents?.defaults?.model?.primary, "agents.defaults.model.primary"); + for (const f of cfg.agents?.defaults?.model?.fallbacks ?? []) addModel(out, f, "agents.defaults.model.fallbacks"); + addModel(out, cfg.agents?.defaults?.imageModel?.primary, "agents.defaults.imageModel.primary"); + for (const f of cfg.agents?.defaults?.imageModel?.fallbacks ?? []) + addModel(out, f, "agents.defaults.imageModel.fallbacks"); + + const list = Array.isArray(cfg.agents?.list) ? cfg.agents?.list : []; + for (const agent of list ?? []) { + if (!agent || typeof agent !== "object") continue; + const id = typeof (agent as { id?: unknown }).id === "string" ? (agent as { id: string }).id : ""; + const model = (agent as { model?: unknown }).model; + if (typeof model === "string") { + addModel(out, model, `agents.list.${id}.model`); + } else if (model && typeof model === "object") { + addModel(out, (model as { primary?: unknown }).primary, `agents.list.${id}.model.primary`); + const fallbacks = (model as { fallbacks?: unknown }).fallbacks; + if (Array.isArray(fallbacks)) { + for (const f of fallbacks) addModel(out, f, `agents.list.${id}.model.fallbacks`); + } + } + } + return out; +} + +const LEGACY_MODEL_PATTERNS: Array<{ id: string; re: RegExp; label: string }> = [ + { id: "openai.gpt35", re: /\bgpt-3\.5\b/i, label: "GPT-3.5 family" }, + { id: "anthropic.claude2", re: /\bclaude-(instant|2)\b/i, label: "Claude 2/Instant family" }, + { id: "openai.gpt4_legacy", re: /\bgpt-4-(0314|0613)\b/i, label: "Legacy GPT-4 snapshots" }, +]; + +export function collectModelHygieneFindings(cfg: ClawdbotConfig): SecurityAuditFinding[] { + const findings: SecurityAuditFinding[] = []; + const models = collectModels(cfg); + if (models.length === 0) return findings; + + const matches: Array<{ model: string; source: string; reason: string }> = []; + for (const entry of models) { + for (const pat of LEGACY_MODEL_PATTERNS) { + if (pat.re.test(entry.id)) { + matches.push({ model: entry.id, source: entry.source, reason: pat.label }); + break; + } + } + } + + if (matches.length > 0) { + const lines = matches + .slice(0, 12) + .map((m) => `- ${m.model} (${m.reason}) @ ${m.source}`) + .join("\n"); + const more = matches.length > 12 ? `\n…${matches.length - 12} more` : ""; + findings.push({ + checkId: "models.legacy", + severity: "warn", + title: "Some configured models look legacy", + detail: + "Older/legacy models can be less robust against prompt injection and tool misuse.\n" + lines + more, + remediation: "Prefer modern, instruction-hardened models for any bot that can run tools.", + }); + } + + return findings; +} + +export async function collectPluginsTrustFindings(params: { + cfg: ClawdbotConfig; + stateDir: string; +}): Promise { + const findings: SecurityAuditFinding[] = []; + const extensionsDir = path.join(params.stateDir, "extensions"); + const st = await safeStat(extensionsDir); + if (!st.ok || !st.isDir) return findings; + + const entries = await fs.readdir(extensionsDir, { withFileTypes: true }).catch(() => []); + const pluginDirs = entries.filter((e) => e.isDirectory()).map((e) => e.name).filter(Boolean); + if (pluginDirs.length === 0) return findings; + + const allow = params.cfg.plugins?.allow; + const allowConfigured = Array.isArray(allow) && allow.length > 0; + if (!allowConfigured) { + findings.push({ + checkId: "plugins.extensions_no_allowlist", + severity: "warn", + title: "Extensions exist but plugins.allow is not set", + detail: `Found ${pluginDirs.length} extension(s) under ${extensionsDir}. Without plugins.allow, any discovered plugin id may load (depending on config and plugin behavior).`, + remediation: "Set plugins.allow to an explicit list of plugin ids you trust.", + }); + } + + return findings; +} + +function resolveIncludePath(baseConfigPath: string, includePath: string): string { + return path.normalize( + path.isAbsolute(includePath) + ? includePath + : path.resolve(path.dirname(baseConfigPath), includePath), + ); +} + +function listDirectIncludes(parsed: unknown): string[] { + const out: string[] = []; + const visit = (value: unknown) => { + if (!value) return; + if (Array.isArray(value)) { + for (const item of value) visit(item); + return; + } + if (typeof value !== "object") return; + const rec = value as Record; + const includeVal = rec[INCLUDE_KEY]; + if (typeof includeVal === "string") out.push(includeVal); + else if (Array.isArray(includeVal)) { + for (const item of includeVal) { + if (typeof item === "string") out.push(item); + } + } + for (const v of Object.values(rec)) visit(v); + }; + visit(parsed); + return out; +} + +async function collectIncludePathsRecursive(params: { + configPath: string; + parsed: unknown; +}): Promise { + const visited = new Set(); + const result: string[] = []; + + const walk = async (basePath: string, parsed: unknown, depth: number): Promise => { + if (depth > MAX_INCLUDE_DEPTH) return; + for (const raw of listDirectIncludes(parsed)) { + const resolved = resolveIncludePath(basePath, raw); + if (visited.has(resolved)) continue; + visited.add(resolved); + result.push(resolved); + const rawText = await fs.readFile(resolved, "utf-8").catch(() => null); + if (!rawText) continue; + const nestedParsed = (() => { + try { + return JSON5.parse(rawText) as unknown; + } catch { + return null; + } + })(); + if (nestedParsed) { + // eslint-disable-next-line no-await-in-loop + await walk(resolved, nestedParsed, depth + 1); + } + } + }; + + await walk(params.configPath, params.parsed, 0); + return result; +} + +export async function collectIncludeFilePermFindings(params: { + configSnapshot: ConfigFileSnapshot; +}): Promise { + const findings: SecurityAuditFinding[] = []; + if (!params.configSnapshot.exists) return findings; + + const configPath = params.configSnapshot.path; + const includePaths = await collectIncludePathsRecursive({ + configPath, + parsed: params.configSnapshot.parsed, + }); + if (includePaths.length === 0) return findings; + + for (const p of includePaths) { + // eslint-disable-next-line no-await-in-loop + const st = await safeStat(p); + if (!st.ok) continue; + const bits = modeBits(st.mode); + if (isWorldWritable(bits) || isGroupWritable(bits)) { + findings.push({ + checkId: "fs.config_include.perms_writable", + severity: "critical", + title: "Config include file is writable by others", + detail: `${p} mode=${formatOctal(bits)}; another user could influence your effective config.`, + remediation: `chmod 600 ${p}`, + }); + } else if (isWorldReadable(bits)) { + findings.push({ + checkId: "fs.config_include.perms_world_readable", + severity: "critical", + title: "Config include file is world-readable", + detail: `${p} mode=${formatOctal(bits)}; include files can contain tokens and private settings.`, + remediation: `chmod 600 ${p}`, + }); + } else if (isGroupReadable(bits)) { + findings.push({ + checkId: "fs.config_include.perms_group_readable", + severity: "warn", + title: "Config include file is group-readable", + detail: `${p} mode=${formatOctal(bits)}; include files can contain tokens and private settings.`, + remediation: `chmod 600 ${p}`, + }); + } + } + + return findings; +} + +export async function collectStateDeepFilesystemFindings(params: { + cfg: ClawdbotConfig; + env: NodeJS.ProcessEnv; + stateDir: string; +}): Promise { + const findings: SecurityAuditFinding[] = []; + const oauthDir = resolveOAuthDir(params.env, params.stateDir); + + const oauthStat = await safeStat(oauthDir); + if (oauthStat.ok && oauthStat.isDir) { + const bits = modeBits(oauthStat.mode); + if (isWorldWritable(bits) || isGroupWritable(bits)) { + findings.push({ + checkId: "fs.credentials_dir.perms_writable", + severity: "critical", + title: "Credentials dir is writable by others", + detail: `${oauthDir} mode=${formatOctal(bits)}; another user could drop/modify credential files.`, + remediation: `chmod 700 ${oauthDir}`, + }); + } else if (isGroupReadable(bits) || isWorldReadable(bits)) { + findings.push({ + checkId: "fs.credentials_dir.perms_readable", + severity: "warn", + title: "Credentials dir is readable by others", + detail: `${oauthDir} mode=${formatOctal(bits)}; credentials and allowlists can be sensitive.`, + remediation: `chmod 700 ${oauthDir}`, + }); + } + } + + const agentIds = Array.isArray(params.cfg.agents?.list) + ? params.cfg.agents?.list + .map((a) => (a && typeof a === "object" && typeof a.id === "string" ? a.id.trim() : "")) + .filter(Boolean) + : []; + const defaultAgentId = resolveDefaultAgentId(params.cfg); + const ids = Array.from(new Set([defaultAgentId, ...agentIds])).map((id) => normalizeAgentId(id)); + + for (const agentId of ids) { + const agentDir = path.join(params.stateDir, "agents", agentId, "agent"); + const authPath = path.join(agentDir, "auth-profiles.json"); + // eslint-disable-next-line no-await-in-loop + const authStat = await safeStat(authPath); + if (authStat.ok) { + const bits = modeBits(authStat.mode); + if (isWorldWritable(bits) || isGroupWritable(bits)) { + findings.push({ + checkId: "fs.auth_profiles.perms_writable", + severity: "critical", + title: "auth-profiles.json is writable by others", + detail: `${authPath} mode=${formatOctal(bits)}; another user could inject credentials.`, + remediation: `chmod 600 ${authPath}`, + }); + } else if (isWorldReadable(bits) || isGroupReadable(bits)) { + findings.push({ + checkId: "fs.auth_profiles.perms_readable", + severity: "warn", + title: "auth-profiles.json is readable by others", + detail: `${authPath} mode=${formatOctal(bits)}; auth-profiles.json contains API keys and OAuth tokens.`, + remediation: `chmod 600 ${authPath}`, + }); + } + } + + const storePath = path.join(params.stateDir, "agents", agentId, "sessions", "sessions.json"); + // eslint-disable-next-line no-await-in-loop + const storeStat = await safeStat(storePath); + if (storeStat.ok) { + const bits = modeBits(storeStat.mode); + if (isWorldReadable(bits) || isGroupReadable(bits)) { + findings.push({ + checkId: "fs.sessions_store.perms_readable", + severity: "warn", + title: "sessions.json is readable by others", + detail: `${storePath} mode=${formatOctal(bits)}; routing and transcript metadata can be sensitive.`, + remediation: `chmod 600 ${storePath}`, + }); + } + } + } + + const logFile = typeof params.cfg.logging?.file === "string" ? params.cfg.logging.file.trim() : ""; + if (logFile) { + const expanded = logFile.startsWith("~") ? expandTilde(logFile, params.env) : logFile; + if (expanded) { + const logPath = path.resolve(expanded); + const st = await safeStat(logPath); + if (st.ok) { + const bits = modeBits(st.mode); + if (isWorldReadable(bits) || isGroupReadable(bits)) { + findings.push({ + checkId: "fs.log_file.perms_readable", + severity: "warn", + title: "Log file is readable by others", + detail: `${logPath} mode=${formatOctal(bits)}; logs can contain private messages and tool output.`, + remediation: `chmod 600 ${logPath}`, + }); + } + } + } + } + + return findings; +} + +function listGroupPolicyOpen(cfg: ClawdbotConfig): string[] { + const out: string[] = []; + const channels = cfg.channels as Record | undefined; + if (!channels || typeof channels !== "object") return out; + for (const [channelId, value] of Object.entries(channels)) { + if (!value || typeof value !== "object") continue; + const section = value as Record; + if (section.groupPolicy === "open") out.push(`channels.${channelId}.groupPolicy`); + const accounts = section.accounts; + if (accounts && typeof accounts === "object") { + for (const [accountId, accountVal] of Object.entries(accounts)) { + if (!accountVal || typeof accountVal !== "object") continue; + const acc = accountVal as Record; + if (acc.groupPolicy === "open") out.push(`channels.${channelId}.accounts.${accountId}.groupPolicy`); + } + } + } + return out; +} + +export function collectExposureMatrixFindings(cfg: ClawdbotConfig): SecurityAuditFinding[] { + const findings: SecurityAuditFinding[] = []; + const openGroups = listGroupPolicyOpen(cfg); + if (openGroups.length === 0) return findings; + + const elevatedEnabled = cfg.tools?.elevated?.enabled !== false; + if (elevatedEnabled) { + findings.push({ + checkId: "security.exposure.open_groups_with_elevated", + severity: "critical", + title: "Open groupPolicy with elevated tools enabled", + detail: + `Found groupPolicy="open" at:\n${openGroups.map((p) => `- ${p}`).join("\n")}\n` + + "With tools.elevated enabled, a prompt injection in those rooms can become a high-impact incident.", + remediation: `Set groupPolicy="allowlist" and keep elevated allowlists extremely tight.`, + }); + } + + return findings; +} + +export async function readConfigSnapshotForAudit(params: { + env: NodeJS.ProcessEnv; + configPath: string; +}): Promise { + return await createConfigIO({ env: params.env, configPath: params.configPath }).readConfigFileSnapshot(); +} diff --git a/src/security/audit-fs.ts b/src/security/audit-fs.ts new file mode 100644 index 000000000..47e19b476 --- /dev/null +++ b/src/security/audit-fs.ts @@ -0,0 +1,64 @@ +import fs from "node:fs/promises"; + +export async function safeStat(targetPath: string): Promise<{ + ok: boolean; + isSymlink: boolean; + isDir: boolean; + mode: number | null; + uid: number | null; + gid: number | null; + error?: string; +}> { + try { + const lst = await fs.lstat(targetPath); + return { + ok: true, + isSymlink: lst.isSymbolicLink(), + isDir: lst.isDirectory(), + mode: typeof lst.mode === "number" ? lst.mode : null, + uid: typeof lst.uid === "number" ? lst.uid : null, + gid: typeof lst.gid === "number" ? lst.gid : null, + }; + } catch (err) { + return { + ok: false, + isSymlink: false, + isDir: false, + mode: null, + uid: null, + gid: null, + error: String(err), + }; + } +} + +export function modeBits(mode: number | null): number | null { + if (mode == null) return null; + return mode & 0o777; +} + +export function formatOctal(bits: number | null): string { + if (bits == null) return "unknown"; + return bits.toString(8).padStart(3, "0"); +} + +export function isWorldWritable(bits: number | null): boolean { + if (bits == null) return false; + return (bits & 0o002) !== 0; +} + +export function isGroupWritable(bits: number | null): boolean { + if (bits == null) return false; + return (bits & 0o020) !== 0; +} + +export function isWorldReadable(bits: number | null): boolean { + if (bits == null) return false; + return (bits & 0o004) !== 0; +} + +export function isGroupReadable(bits: number | null): boolean { + if (bits == null) return false; + return (bits & 0o040) !== 0; +} + diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 1a4d7a67b..c46c41648 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -2,8 +2,32 @@ import { describe, expect, it } from "vitest"; import type { ClawdbotConfig } from "../config/config.js"; import { runSecurityAudit } from "./audit.js"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; describe("security audit", () => { + it("includes an attack surface summary (info)", async () => { + const cfg: ClawdbotConfig = { + channels: { whatsapp: { groupPolicy: "open" }, telegram: { groupPolicy: "allowlist" } }, + tools: { elevated: { enabled: true, allowFrom: { whatsapp: ["+1"] } } }, + hooks: { enabled: true }, + browser: { enabled: true }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: false, + includeChannelSecurity: false, + }); + + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ checkId: "summary.attack_surface", severity: "info" }), + ]), + ); + }); + it("flags non-loopback bind without auth as critical", async () => { const cfg: ClawdbotConfig = { gateway: { @@ -175,4 +199,124 @@ describe("security audit", () => { ]), ); }); + + it("warns on legacy model configuration", async () => { + const cfg: ClawdbotConfig = { + agents: { defaults: { model: { primary: "openai/gpt-3.5-turbo" } } }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: false, + includeChannelSecurity: false, + }); + + expect(res.findings).toEqual( + expect.arrayContaining([expect.objectContaining({ checkId: "models.legacy", severity: "warn" })]), + ); + }); + + it("warns when hooks token looks short", async () => { + const cfg: ClawdbotConfig = { + hooks: { enabled: true, token: "short" }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: false, + includeChannelSecurity: false, + }); + + expect(res.findings).toEqual( + expect.arrayContaining([expect.objectContaining({ checkId: "hooks.token_too_short", severity: "warn" })]), + ); + }); + + it("warns when state/config look like a synced folder", async () => { + const cfg: ClawdbotConfig = {}; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: false, + includeChannelSecurity: false, + stateDir: "/Users/test/Dropbox/.clawdbot", + configPath: "/Users/test/Dropbox/.clawdbot/clawdbot.json", + }); + + expect(res.findings).toEqual( + expect.arrayContaining([expect.objectContaining({ checkId: "fs.synced_dir", severity: "warn" })]), + ); + }); + + it("flags group/world-readable config include files", async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-")); + const stateDir = path.join(tmp, "state"); + await fs.mkdir(stateDir, { recursive: true, mode: 0o700 }); + + const includePath = path.join(stateDir, "extra.json5"); + await fs.writeFile(includePath, "{ logging: { redactSensitive: 'off' } }\n", "utf-8"); + await fs.chmod(includePath, 0o644); + + const configPath = path.join(stateDir, "clawdbot.json"); + await fs.writeFile(configPath, `{ "$include": "./extra.json5" }\n`, "utf-8"); + await fs.chmod(configPath, 0o600); + + const cfg: ClawdbotConfig = { logging: { redactSensitive: "off" } }; + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath, + }); + + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ checkId: "fs.config_include.perms_world_readable", severity: "critical" }), + ]), + ); + }); + + it("flags extensions without plugins.allow", async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-")); + const stateDir = path.join(tmp, "state"); + await fs.mkdir(path.join(stateDir, "extensions", "some-plugin"), { recursive: true, mode: 0o700 }); + + const cfg: ClawdbotConfig = {}; + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath: path.join(stateDir, "clawdbot.json"), + }); + + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ checkId: "plugins.extensions_no_allowlist", severity: "warn" }), + ]), + ); + }); + + it("flags open groupPolicy when tools.elevated is enabled", async () => { + const cfg: ClawdbotConfig = { + tools: { elevated: { enabled: true, allowFrom: { whatsapp: ["+1"] } } }, + channels: { whatsapp: { groupPolicy: "open" } }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: false, + includeChannelSecurity: false, + }); + + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + checkId: "security.exposure.open_groups_with_elevated", + severity: "critical", + }), + ]), + ); + }); }); diff --git a/src/security/audit.ts b/src/security/audit.ts index 7df7e5603..9c5397fa8 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -1,5 +1,3 @@ -import fs from "node:fs/promises"; - import { listChannelPlugins } from "../channels/plugins/index.js"; import { resolveChannelDefaultAccountId } from "../channels/plugins/helpers.js"; import type { ChannelId } from "../channels/plugins/types.js"; @@ -9,6 +7,27 @@ import { resolveConfigPath, resolveStateDir } from "../config/paths.js"; import { resolveGatewayAuth } from "../gateway/auth.js"; import { buildGatewayConnectionDetails } from "../gateway/call.js"; import { probeGateway } from "../gateway/probe.js"; +import { + collectAttackSurfaceSummaryFindings, + collectExposureMatrixFindings, + collectHooksHardeningFindings, + collectIncludeFilePermFindings, + collectModelHygieneFindings, + collectPluginsTrustFindings, + collectSecretsInConfigFindings, + collectStateDeepFilesystemFindings, + collectSyncedFolderFindings, + readConfigSnapshotForAudit, +} from "./audit-extra.js"; +import { + formatOctal, + isGroupReadable, + isGroupWritable, + isWorldReadable, + isWorldWritable, + modeBits, + safeStat, +} from "./audit-fs.js"; export type SecurityAuditSeverity = "info" | "warn" | "critical"; @@ -93,68 +112,6 @@ function classifyChannelWarningSeverity(message: string): SecurityAuditSeverity return "warn"; } -async function safeStat(targetPath: string): Promise<{ - ok: boolean; - isSymlink: boolean; - isDir: boolean; - mode: number | null; - uid: number | null; - gid: number | null; - error?: string; -}> { - try { - const lst = await fs.lstat(targetPath); - return { - ok: true, - isSymlink: lst.isSymbolicLink(), - isDir: lst.isDirectory(), - mode: typeof lst.mode === "number" ? lst.mode : null, - uid: typeof lst.uid === "number" ? lst.uid : null, - gid: typeof lst.gid === "number" ? lst.gid : null, - }; - } catch (err) { - return { - ok: false, - isSymlink: false, - isDir: false, - mode: null, - uid: null, - gid: null, - error: String(err), - }; - } -} - -function modeBits(mode: number | null): number | null { - if (mode == null) return null; - return mode & 0o777; -} - -function formatOctal(bits: number | null): string { - if (bits == null) return "unknown"; - return bits.toString(8).padStart(3, "0"); -} - -function isWorldWritable(bits: number | null): boolean { - if (bits == null) return false; - return (bits & 0o002) !== 0; -} - -function isGroupWritable(bits: number | null): boolean { - if (bits == null) return false; - return (bits & 0o020) !== 0; -} - -function isWorldReadable(bits: number | null): boolean { - if (bits == null) return false; - return (bits & 0o004) !== 0; -} - -function isGroupReadable(bits: number | null): boolean { - if (bits == null) return false; - return (bits & 0o040) !== 0; -} - async function collectFilesystemFindings(params: { stateDir: string; configPath: string; @@ -580,16 +537,34 @@ async function maybeProbeGateway(params: { export async function runSecurityAudit(opts: SecurityAuditOptions): Promise { const findings: SecurityAuditFinding[] = []; const cfg = opts.config; - const stateDir = opts.stateDir ?? resolveStateDir(); - const configPath = opts.configPath ?? resolveConfigPath(); + const env = process.env; + const stateDir = opts.stateDir ?? resolveStateDir(env); + const configPath = opts.configPath ?? resolveConfigPath(env, stateDir); + + findings.push(...collectAttackSurfaceSummaryFindings(cfg)); + findings.push(...collectSyncedFolderFindings({ stateDir, configPath })); findings.push(...collectGatewayConfigFindings(cfg)); findings.push(...collectBrowserControlFindings(cfg)); findings.push(...collectLoggingFindings(cfg)); findings.push(...collectElevatedFindings(cfg)); + findings.push(...collectHooksHardeningFindings(cfg)); + findings.push(...collectSecretsInConfigFindings(cfg)); + findings.push(...collectModelHygieneFindings(cfg)); + findings.push(...collectExposureMatrixFindings(cfg)); + + const configSnapshot = + opts.includeFilesystem !== false + ? await readConfigSnapshotForAudit({ env, configPath }).catch(() => null) + : null; if (opts.includeFilesystem !== false) { findings.push(...(await collectFilesystemFindings({ stateDir, configPath }))); + if (configSnapshot) { + findings.push(...(await collectIncludeFilePermFindings({ configSnapshot }))); + } + findings.push(...(await collectStateDeepFilesystemFindings({ cfg, env, stateDir }))); + findings.push(...(await collectPluginsTrustFindings({ cfg, stateDir }))); } if (opts.includeChannelSecurity !== false) { diff --git a/src/security/fix.test.ts b/src/security/fix.test.ts index 06ea0fc34..f423079be 100644 --- a/src/security/fix.test.ts +++ b/src/security/fix.test.ts @@ -207,4 +207,61 @@ describe("security fix", () => { const configMode = (await fs.stat(configPath)).mode & 0o777; expectPerms(configMode, 0o600); }); + + it("tightens perms for credentials + agent auth/sessions + include files", async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-fix-")); + const stateDir = path.join(tmp, "state"); + await fs.mkdir(stateDir, { recursive: true }); + + const includesDir = path.join(stateDir, "includes"); + await fs.mkdir(includesDir, { recursive: true }); + const includePath = path.join(includesDir, "extra.json5"); + await fs.writeFile(includePath, "{ logging: { redactSensitive: 'off' } }\n", "utf-8"); + await fs.chmod(includePath, 0o644); + + const configPath = path.join(stateDir, "clawdbot.json"); + await fs.writeFile( + configPath, + `{ "$include": "./includes/extra.json5", channels: { whatsapp: { groupPolicy: "open" } } }\n`, + "utf-8", + ); + await fs.chmod(configPath, 0o644); + + const credsDir = path.join(stateDir, "credentials"); + await fs.mkdir(credsDir, { recursive: true }); + const allowFromPath = path.join(credsDir, "whatsapp-allowFrom.json"); + await fs.writeFile( + allowFromPath, + `${JSON.stringify({ version: 1, allowFrom: ["+15550002222"] }, null, 2)}\n`, + "utf-8", + ); + await fs.chmod(allowFromPath, 0o644); + + const agentDir = path.join(stateDir, "agents", "main", "agent"); + await fs.mkdir(agentDir, { recursive: true }); + const authProfilesPath = path.join(agentDir, "auth-profiles.json"); + await fs.writeFile(authProfilesPath, "{}\n", "utf-8"); + await fs.chmod(authProfilesPath, 0o644); + + const sessionsDir = path.join(stateDir, "agents", "main", "sessions"); + await fs.mkdir(sessionsDir, { recursive: true }); + const sessionsStorePath = path.join(sessionsDir, "sessions.json"); + await fs.writeFile(sessionsStorePath, "{}\n", "utf-8"); + await fs.chmod(sessionsStorePath, 0o644); + + const env = { + ...process.env, + CLAWDBOT_STATE_DIR: stateDir, + CLAWDBOT_CONFIG_PATH: "", + }; + + const res = await fixSecurityFootguns({ env }); + expect(res.ok).toBe(true); + + expect((await fs.stat(credsDir)).mode & 0o777).toBe(0o700); + expect((await fs.stat(allowFromPath)).mode & 0o777).toBe(0o600); + expect((await fs.stat(authProfilesPath)).mode & 0o777).toBe(0o600); + expect((await fs.stat(sessionsStorePath)).mode & 0o777).toBe(0o600); + expect((await fs.stat(includePath)).mode & 0o777).toBe(0o600); + }); }); diff --git a/src/security/fix.ts b/src/security/fix.ts index 347fed396..1d3e4bf17 100644 --- a/src/security/fix.ts +++ b/src/security/fix.ts @@ -1,8 +1,14 @@ import fs from "node:fs/promises"; +import path from "node:path"; + +import JSON5 from "json5"; import type { ClawdbotConfig } from "../config/config.js"; import { createConfigIO } from "../config/config.js"; -import { resolveConfigPath, resolveStateDir } from "../config/paths.js"; +import { resolveConfigPath, resolveOAuthDir, resolveStateDir } from "../config/paths.js"; +import { resolveDefaultAgentId } from "../agents/agent-scope.js"; +import { INCLUDE_KEY, MAX_INCLUDE_DEPTH } from "../config/includes.js"; +import { normalizeAgentId } from "../routing/session-key.js"; import { readChannelAllowFromStore } from "../pairing/pairing-store.js"; export type SecurityFixChmodAction = { @@ -186,6 +192,122 @@ function applyConfigFixes(params: { cfg: ClawdbotConfig; env: NodeJS.ProcessEnv return { cfg: next, changes, policyFlips }; } +function listDirectIncludes(parsed: unknown): string[] { + const out: string[] = []; + const visit = (value: unknown) => { + if (!value) return; + if (Array.isArray(value)) { + for (const item of value) visit(item); + return; + } + if (typeof value !== "object") return; + const rec = value as Record; + const includeVal = rec[INCLUDE_KEY]; + if (typeof includeVal === "string") out.push(includeVal); + else if (Array.isArray(includeVal)) { + for (const item of includeVal) { + if (typeof item === "string") out.push(item); + } + } + for (const v of Object.values(rec)) visit(v); + }; + visit(parsed); + return out; +} + +function resolveIncludePath(baseConfigPath: string, includePath: string): string { + return path.normalize( + path.isAbsolute(includePath) + ? includePath + : path.resolve(path.dirname(baseConfigPath), includePath), + ); +} + +async function collectIncludePathsRecursive(params: { + configPath: string; + parsed: unknown; +}): Promise { + const visited = new Set(); + const result: string[] = []; + + const walk = async (basePath: string, parsed: unknown, depth: number): Promise => { + if (depth > MAX_INCLUDE_DEPTH) return; + for (const raw of listDirectIncludes(parsed)) { + const resolved = resolveIncludePath(basePath, raw); + if (visited.has(resolved)) continue; + visited.add(resolved); + result.push(resolved); + const rawText = await fs.readFile(resolved, "utf-8").catch(() => null); + if (!rawText) continue; + const nestedParsed = (() => { + try { + return JSON5.parse(rawText) as unknown; + } catch { + return null; + } + })(); + if (nestedParsed) { + // eslint-disable-next-line no-await-in-loop + await walk(resolved, nestedParsed, depth + 1); + } + } + }; + + await walk(params.configPath, params.parsed, 0); + return result; +} + +async function chmodCredentialsAndAgentState(params: { + env: NodeJS.ProcessEnv; + stateDir: string; + cfg: ClawdbotConfig; + actions: SecurityFixChmodAction[]; +}): Promise { + const credsDir = resolveOAuthDir(params.env, params.stateDir); + params.actions.push(await safeChmod({ path: credsDir, mode: 0o700, require: "dir" })); + + const credsEntries = await fs.readdir(credsDir, { withFileTypes: true }).catch(() => []); + for (const entry of credsEntries) { + if (!entry.isFile()) continue; + if (!entry.name.endsWith(".json")) continue; + const p = path.join(credsDir, entry.name); + // eslint-disable-next-line no-await-in-loop + params.actions.push(await safeChmod({ path: p, mode: 0o600, require: "file" })); + } + + const ids = new Set(); + ids.add(resolveDefaultAgentId(params.cfg)); + const list = Array.isArray(params.cfg.agents?.list) ? params.cfg.agents?.list : []; + for (const agent of list ?? []) { + if (!agent || typeof agent !== "object") continue; + const id = typeof (agent as { id?: unknown }).id === "string" ? (agent as { id: string }).id.trim() : ""; + if (id) ids.add(id); + } + + for (const agentId of ids) { + const normalizedAgentId = normalizeAgentId(agentId); + const agentRoot = path.join(params.stateDir, "agents", normalizedAgentId); + const agentDir = path.join(agentRoot, "agent"); + const sessionsDir = path.join(agentRoot, "sessions"); + + // eslint-disable-next-line no-await-in-loop + params.actions.push(await safeChmod({ path: agentRoot, mode: 0o700, require: "dir" })); + // eslint-disable-next-line no-await-in-loop + params.actions.push(await safeChmod({ path: agentDir, mode: 0o700, require: "dir" })); + + const authPath = path.join(agentDir, "auth-profiles.json"); + // eslint-disable-next-line no-await-in-loop + params.actions.push(await safeChmod({ path: authPath, mode: 0o600, require: "file" })); + + // eslint-disable-next-line no-await-in-loop + params.actions.push(await safeChmod({ path: sessionsDir, mode: 0o700, require: "dir" })); + + const storePath = path.join(sessionsDir, "sessions.json"); + // eslint-disable-next-line no-await-in-loop + params.actions.push(await safeChmod({ path: storePath, mode: 0o600, require: "file" })); + } +} + export async function fixSecurityFootguns(opts?: { env?: NodeJS.ProcessEnv; stateDir?: string; @@ -232,6 +354,21 @@ export async function fixSecurityFootguns(opts?: { actions.push(await safeChmod({ path: stateDir, mode: 0o700, require: "dir" })); actions.push(await safeChmod({ path: configPath, mode: 0o600, require: "file" })); + if (snap.exists) { + const includePaths = await collectIncludePathsRecursive({ + configPath: snap.path, + parsed: snap.parsed, + }).catch(() => []); + for (const p of includePaths) { + // eslint-disable-next-line no-await-in-loop + actions.push(await safeChmod({ path: p, mode: 0o600, require: "file" })); + } + } + + await chmodCredentialsAndAgentState({ env, stateDir, cfg: snap.config ?? {}, actions }).catch((err) => { + errors.push(`chmodCredentialsAndAgentState failed: ${String(err)}`); + }); + return { ok: errors.length === 0, stateDir,