diff --git a/docs/gateway/security/index.md b/docs/gateway/security/index.md index 7bf0f84abc7..7abbea866d4 100644 --- a/docs/gateway/security/index.md +++ b/docs/gateway/security/index.md @@ -117,33 +117,34 @@ When the audit prints findings, treat this as a priority order: High-signal `checkId` values you will most likely see in real deployments (not exhaustive): -| `checkId` | Severity | Why it matters | Primary fix key/path | Auto-fix | -| -------------------------------------------------- | ------------- | ------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | -------- | -| `fs.state_dir.perms_world_writable` | critical | Other users/processes can modify full OpenClaw state | filesystem perms on `~/.openclaw` | yes | -| `fs.config.perms_writable` | critical | Others can change auth/tool policy/config | filesystem perms on `~/.openclaw/openclaw.json` | yes | -| `fs.config.perms_world_readable` | critical | Config can expose tokens/settings | filesystem perms on config file | yes | -| `gateway.bind_no_auth` | critical | Remote bind without shared secret | `gateway.bind`, `gateway.auth.*` | no | -| `gateway.loopback_no_auth` | critical | Reverse-proxied loopback may become unauthenticated | `gateway.auth.*`, proxy setup | no | -| `gateway.http.no_auth` | warn/critical | Gateway HTTP APIs reachable with `auth.mode="none"` | `gateway.auth.mode`, `gateway.http.endpoints.*` | no | -| `gateway.tools_invoke_http.dangerous_allow` | warn/critical | Re-enables dangerous tools over HTTP API | `gateway.tools.allow` | no | -| `gateway.nodes.allow_commands_dangerous` | warn/critical | Enables high-impact node commands (camera/screen/contacts/calendar/SMS) | `gateway.nodes.allowCommands` | no | -| `gateway.tailscale_funnel` | critical | Public internet exposure | `gateway.tailscale.mode` | no | -| `gateway.control_ui.insecure_auth` | warn | Insecure-auth compatibility toggle enabled | `gateway.controlUi.allowInsecureAuth` | no | -| `gateway.control_ui.device_auth_disabled` | critical | Disables device identity check | `gateway.controlUi.dangerouslyDisableDeviceAuth` | no | -| `gateway.real_ip_fallback_enabled` | warn/critical | Trusting `X-Real-IP` fallback can enable source-IP spoofing via proxy misconfig | `gateway.allowRealIpFallback`, `gateway.trustedProxies` | no | -| `discovery.mdns_full_mode` | warn/critical | mDNS full mode advertises `cliPath`/`sshPort` metadata on local network | `discovery.mdns.mode`, `gateway.bind` | no | -| `config.insecure_or_dangerous_flags` | warn | Any insecure/dangerous debug flags enabled | multiple keys (see finding detail) | no | -| `hooks.token_too_short` | warn | Easier brute force on hook ingress | `hooks.token` | no | -| `hooks.request_session_key_enabled` | warn/critical | External caller can choose sessionKey | `hooks.allowRequestSessionKey` | no | -| `hooks.request_session_key_prefixes_missing` | warn/critical | No bound on external session key shapes | `hooks.allowedSessionKeyPrefixes` | no | -| `logging.redact_off` | warn | Sensitive values leak to logs/status | `logging.redactSensitive` | yes | -| `sandbox.docker_config_mode_off` | warn | Sandbox Docker config present but inactive | `agents.*.sandbox.mode` | no | -| `tools.exec.host_sandbox_no_sandbox_defaults` | warn | `exec host=sandbox` resolves to host exec when sandbox is off | `tools.exec.host`, `agents.defaults.sandbox.mode` | no | -| `tools.exec.host_sandbox_no_sandbox_agents` | warn | Per-agent `exec host=sandbox` resolves to host exec when sandbox is off | `agents.list[].tools.exec.host`, `agents.list[].sandbox.mode` | no | -| `security.exposure.open_groups_with_runtime_or_fs` | critical/warn | Open groups can reach command/file tools without sandbox/workspace guards | `channels.*.groupPolicy`, `tools.profile/deny`, `tools.fs.workspaceOnly`, `agents.*.sandbox.mode` | no | -| `tools.profile_minimal_overridden` | warn | Agent overrides bypass global minimal profile | `agents.list[].tools.profile` | no | -| `plugins.tools_reachable_permissive_policy` | warn | Extension tools reachable in permissive contexts | `tools.profile` + tool allow/deny | no | -| `models.small_params` | critical/info | Small models + unsafe tool surfaces raise injection risk | model choice + sandbox/tool policy | no | +| `checkId` | Severity | Why it matters | Primary fix key/path | Auto-fix | +| -------------------------------------------------- | ------------- | ---------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | -------- | +| `fs.state_dir.perms_world_writable` | critical | Other users/processes can modify full OpenClaw state | filesystem perms on `~/.openclaw` | yes | +| `fs.config.perms_writable` | critical | Others can change auth/tool policy/config | filesystem perms on `~/.openclaw/openclaw.json` | yes | +| `fs.config.perms_world_readable` | critical | Config can expose tokens/settings | filesystem perms on config file | yes | +| `gateway.bind_no_auth` | critical | Remote bind without shared secret | `gateway.bind`, `gateway.auth.*` | no | +| `gateway.loopback_no_auth` | critical | Reverse-proxied loopback may become unauthenticated | `gateway.auth.*`, proxy setup | no | +| `gateway.http.no_auth` | warn/critical | Gateway HTTP APIs reachable with `auth.mode="none"` | `gateway.auth.mode`, `gateway.http.endpoints.*` | no | +| `gateway.tools_invoke_http.dangerous_allow` | warn/critical | Re-enables dangerous tools over HTTP API | `gateway.tools.allow` | no | +| `gateway.nodes.allow_commands_dangerous` | warn/critical | Enables high-impact node commands (camera/screen/contacts/calendar/SMS) | `gateway.nodes.allowCommands` | no | +| `gateway.tailscale_funnel` | critical | Public internet exposure | `gateway.tailscale.mode` | no | +| `gateway.control_ui.insecure_auth` | warn | Insecure-auth compatibility toggle enabled | `gateway.controlUi.allowInsecureAuth` | no | +| `gateway.control_ui.device_auth_disabled` | critical | Disables device identity check | `gateway.controlUi.dangerouslyDisableDeviceAuth` | no | +| `gateway.real_ip_fallback_enabled` | warn/critical | Trusting `X-Real-IP` fallback can enable source-IP spoofing via proxy misconfig | `gateway.allowRealIpFallback`, `gateway.trustedProxies` | no | +| `discovery.mdns_full_mode` | warn/critical | mDNS full mode advertises `cliPath`/`sshPort` metadata on local network | `discovery.mdns.mode`, `gateway.bind` | no | +| `config.insecure_or_dangerous_flags` | warn | Any insecure/dangerous debug flags enabled | multiple keys (see finding detail) | no | +| `hooks.token_too_short` | warn | Easier brute force on hook ingress | `hooks.token` | no | +| `hooks.request_session_key_enabled` | warn/critical | External caller can choose sessionKey | `hooks.allowRequestSessionKey` | no | +| `hooks.request_session_key_prefixes_missing` | warn/critical | No bound on external session key shapes | `hooks.allowedSessionKeyPrefixes` | no | +| `logging.redact_off` | warn | Sensitive values leak to logs/status | `logging.redactSensitive` | yes | +| `sandbox.docker_config_mode_off` | warn | Sandbox Docker config present but inactive | `agents.*.sandbox.mode` | no | +| `tools.exec.host_sandbox_no_sandbox_defaults` | warn | `exec host=sandbox` resolves to host exec when sandbox is off | `tools.exec.host`, `agents.defaults.sandbox.mode` | no | +| `tools.exec.host_sandbox_no_sandbox_agents` | warn | Per-agent `exec host=sandbox` resolves to host exec when sandbox is off | `agents.list[].tools.exec.host`, `agents.list[].sandbox.mode` | no | +| `tools.exec.safe_bins_interpreter_unprofiled` | warn | Interpreter/runtime bins in `safeBins` without explicit profiles broaden exec risk | `tools.exec.safeBins`, `tools.exec.safeBinProfiles`, `agents.list[].tools.exec.*` | no | +| `security.exposure.open_groups_with_runtime_or_fs` | critical/warn | Open groups can reach command/file tools without sandbox/workspace guards | `channels.*.groupPolicy`, `tools.profile/deny`, `tools.fs.workspaceOnly`, `agents.*.sandbox.mode` | no | +| `tools.profile_minimal_overridden` | warn | Agent overrides bypass global minimal profile | `agents.list[].tools.profile` | no | +| `plugins.tools_reachable_permissive_policy` | warn | Extension tools reachable in permissive contexts | `tools.profile` + tool allow/deny | no | +| `models.small_params` | critical/info | Small models + unsafe tool surfaces raise injection risk | model choice + sandbox/tool policy | no | ## Control UI over HTTP diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index eacc482bd30..8d34522770f 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -184,6 +184,8 @@ Configuration location: - `safeBins` comes from config (`tools.exec.safeBins` or per-agent `agents.list[].tools.exec.safeBins`). - `safeBinProfiles` comes from config (`tools.exec.safeBinProfiles` or per-agent `agents.list[].tools.exec.safeBinProfiles`). Per-agent profile keys override global keys. - allowlist entries live in host-local `~/.openclaw/exec-approvals.json` under `agents..allowlist` (or via Control UI / `openclaw approvals allowlist ...`). +- `openclaw security audit` warns with `tools.exec.safe_bins_interpreter_unprofiled` when interpreter/runtime bins appear in `safeBins` without explicit profiles. +- `openclaw doctor --fix` can scaffold missing custom `safeBinProfiles.` entries as `{}` (review and tighten afterward). Interpreter/runtime bins are not auto-scaffolded. Custom profile example: diff --git a/docs/tools/exec.md b/docs/tools/exec.md index ef24f3d7cd9..47842a7bb4c 100644 --- a/docs/tools/exec.md +++ b/docs/tools/exec.md @@ -134,6 +134,7 @@ Use the two controls for different jobs: - allowlist: explicit trust for executable paths. Do not treat `safeBins` as a generic allowlist, and do not add interpreter/runtime binaries (for example `python3`, `node`, `ruby`, `bash`). If you need those, use explicit allowlist entries and keep approval prompts enabled. +`openclaw security audit` warns when interpreter/runtime `safeBins` entries are missing explicit profiles, and `openclaw doctor --fix` can scaffold missing custom `safeBinProfiles` entries. For full policy details and examples, see [Exec approvals](/tools/exec-approvals#safe-bins-stdin-only) and [Safe bins versus allowlist](/tools/exec-approvals#safe-bins-versus-allowlist). diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index cb29e261841..3776cce960c 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -1,9 +1,8 @@ import fs from "node:fs/promises"; import path from "node:path"; import type { AgentTool, AgentToolResult } from "@mariozechner/pi-agent-core"; -import { type ExecHost, maxAsk, minSecurity, resolveSafeBins } from "../infra/exec-approvals.js"; -import { resolveSafeBinProfiles } from "../infra/exec-safe-bin-policy.js"; -import { getTrustedSafeBinDirs } from "../infra/exec-safe-bin-trust.js"; +import { type ExecHost, maxAsk, minSecurity } from "../infra/exec-approvals.js"; +import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; import { getShellPathFromLoginShell, resolveShellEnvFallbackTimeoutMs, @@ -164,15 +163,28 @@ export function createExecTool( ? defaults.timeoutSec : 1800; const defaultPathPrepend = normalizePathPrepend(defaults?.pathPrepend); - const safeBins = resolveSafeBins(defaults?.safeBins); - const safeBinProfiles = resolveSafeBinProfiles(defaults?.safeBinProfiles); - const unprofiledSafeBins = Array.from(safeBins).filter((entry) => !safeBinProfiles[entry]); + const { + safeBins, + safeBinProfiles, + trustedSafeBinDirs, + unprofiledSafeBins, + unprofiledInterpreterSafeBins, + } = resolveExecSafeBinRuntimePolicy({ + local: { + safeBins: defaults?.safeBins, + safeBinProfiles: defaults?.safeBinProfiles, + }, + }); if (unprofiledSafeBins.length > 0) { logInfo( `exec: ignoring unprofiled safeBins entries (${unprofiledSafeBins.toSorted().join(", ")}); use allowlist or define tools.exec.safeBinProfiles.`, ); } - const trustedSafeBinDirs = getTrustedSafeBinDirs(); + if (unprofiledInterpreterSafeBins.length > 0) { + logInfo( + `exec: interpreter/runtime binaries in safeBins (${unprofiledInterpreterSafeBins.join(", ")}) are unsafe without explicit hardened profiles; prefer allowlist entries`, + ); + } const notifyOnExit = defaults?.notifyOnExit !== false; const notifyOnExitEmptySuccess = defaults?.notifyOnExitEmptySuccess === true; const notifySessionKey = defaults?.sessionKey?.trim() || undefined; diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index ef1653365e4..9c53c3b0d00 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -7,6 +7,7 @@ import { } from "@mariozechner/pi-coding-agent"; import type { OpenClawConfig } from "../config/config.js"; import type { ToolLoopDetectionConfig } from "../config/types.tools.js"; +import { resolveMergedSafeBinProfileFixtures } from "../infra/exec-safe-bin-runtime-policy.js"; import { logWarn } from "../logger.js"; import { getPluginToolMeta } from "../plugins/tools.js"; import { isSubagentSessionKey } from "../routing/session-key.js"; @@ -97,13 +98,6 @@ function resolveExecConfig(params: { cfg?: OpenClawConfig; agentId?: string }) { const globalExec = cfg?.tools?.exec; const agentExec = cfg && params.agentId ? resolveAgentConfig(cfg, params.agentId)?.tools?.exec : undefined; - const mergedSafeBinProfiles = - globalExec?.safeBinProfiles || agentExec?.safeBinProfiles - ? { - ...globalExec?.safeBinProfiles, - ...agentExec?.safeBinProfiles, - } - : undefined; return { host: agentExec?.host ?? globalExec?.host, security: agentExec?.security ?? globalExec?.security, @@ -111,7 +105,10 @@ function resolveExecConfig(params: { cfg?: OpenClawConfig; agentId?: string }) { node: agentExec?.node ?? globalExec?.node, pathPrepend: agentExec?.pathPrepend ?? globalExec?.pathPrepend, safeBins: agentExec?.safeBins ?? globalExec?.safeBins, - safeBinProfiles: mergedSafeBinProfiles, + safeBinProfiles: resolveMergedSafeBinProfileFixtures({ + global: globalExec, + local: agentExec, + }), backgroundMs: agentExec?.backgroundMs ?? globalExec?.backgroundMs, timeoutSec: agentExec?.timeoutSec ?? globalExec?.timeoutSec, approvalRunningNoticeMs: diff --git a/src/commands/doctor-config-flow.safe-bins.test.ts b/src/commands/doctor-config-flow.safe-bins.test.ts new file mode 100644 index 00000000000..5d1651ce591 --- /dev/null +++ b/src/commands/doctor-config-flow.safe-bins.test.ts @@ -0,0 +1,108 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { withTempHome } from "../../test/helpers/temp-home.js"; + +const { noteSpy } = vi.hoisted(() => ({ + noteSpy: vi.fn(), +})); + +vi.mock("../terminal/note.js", () => ({ + note: noteSpy, +})); + +import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js"; + +async function runDoctorConfigWithInput(params: { + config: Record; + repair?: boolean; +}) { + return withTempHome(async (home) => { + const configDir = path.join(home, ".openclaw"); + await fs.mkdir(configDir, { recursive: true }); + await fs.writeFile( + path.join(configDir, "openclaw.json"), + JSON.stringify(params.config, null, 2), + "utf-8", + ); + return loadAndMaybeMigrateDoctorConfig({ + options: { nonInteractive: true, repair: params.repair }, + confirm: async () => false, + }); + }); +} + +describe("doctor config flow safe bins", () => { + beforeEach(() => { + noteSpy.mockClear(); + }); + + it("scaffolds missing custom safe-bin profiles on repair but skips interpreter bins", async () => { + const result = await runDoctorConfigWithInput({ + repair: true, + config: { + tools: { + exec: { + safeBins: ["myfilter", "python3"], + }, + }, + agents: { + list: [ + { + id: "ops", + tools: { + exec: { + safeBins: ["mytool", "node"], + }, + }, + }, + ], + }, + }, + }); + + const cfg = result.cfg as { + tools?: { + exec?: { + safeBinProfiles?: Record; + }; + }; + agents?: { + list?: Array<{ + id: string; + tools?: { + exec?: { + safeBinProfiles?: Record; + }; + }; + }>; + }; + }; + expect(cfg.tools?.exec?.safeBinProfiles?.myfilter).toEqual({}); + expect(cfg.tools?.exec?.safeBinProfiles?.python3).toBeUndefined(); + const ops = cfg.agents?.list?.find((entry) => entry.id === "ops"); + expect(ops?.tools?.exec?.safeBinProfiles?.mytool).toEqual({}); + expect(ops?.tools?.exec?.safeBinProfiles?.node).toBeUndefined(); + }); + + it("warns when interpreter/custom safeBins entries are missing profiles in non-repair mode", async () => { + await runDoctorConfigWithInput({ + config: { + tools: { + exec: { + safeBins: ["python3", "myfilter"], + }, + }, + }, + }); + + expect(noteSpy).toHaveBeenCalledWith( + expect.stringContaining("tools.exec.safeBins includes interpreter/runtime 'python3'"), + "Doctor warnings", + ); + expect(noteSpy).toHaveBeenCalledWith( + expect.stringContaining("openclaw doctor --fix"), + "Doctor warnings", + ); + }); +}); diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 0199f8bc506..6b4a5e13feb 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -15,6 +15,10 @@ import { readConfigFileSnapshot, } from "../config/config.js"; import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js"; +import { + listInterpreterLikeSafeBins, + resolveMergedSafeBinProfileFixtures, +} from "../infra/exec-safe-bin-runtime-policy.js"; import { listTelegramAccountIds, resolveTelegramAccount } from "../telegram/accounts.js"; import { note } from "../terminal/note.js"; import { isRecord, resolveHomeDir } from "../utils.js"; @@ -704,6 +708,134 @@ function maybeRepairOpenPolicyAllowFrom(cfg: OpenClawConfig): { return { config: next, changes }; } +type ExecSafeBinCoverageHit = { + scopePath: string; + bin: string; + isInterpreter: boolean; +}; + +type ExecSafeBinScopeRef = { + scopePath: string; + safeBins: string[]; + exec: Record; + mergedProfiles: Record; +}; + +function normalizeConfiguredSafeBins(entries: unknown): string[] { + if (!Array.isArray(entries)) { + return []; + } + return Array.from( + new Set( + entries + .map((entry) => (typeof entry === "string" ? entry.trim().toLowerCase() : "")) + .filter((entry) => entry.length > 0), + ), + ).toSorted(); +} + +function collectExecSafeBinScopes(cfg: OpenClawConfig): ExecSafeBinScopeRef[] { + const scopes: ExecSafeBinScopeRef[] = []; + const globalExec = asObjectRecord(cfg.tools?.exec); + if (globalExec) { + const safeBins = normalizeConfiguredSafeBins(globalExec.safeBins); + if (safeBins.length > 0) { + scopes.push({ + scopePath: "tools.exec", + safeBins, + exec: globalExec, + mergedProfiles: + resolveMergedSafeBinProfileFixtures({ + global: globalExec, + }) ?? {}, + }); + } + } + const agents = Array.isArray(cfg.agents?.list) ? cfg.agents.list : []; + for (const agent of agents) { + if (!agent || typeof agent !== "object" || typeof agent.id !== "string") { + continue; + } + const agentExec = asObjectRecord(agent.tools?.exec); + if (!agentExec) { + continue; + } + const safeBins = normalizeConfiguredSafeBins(agentExec.safeBins); + if (safeBins.length === 0) { + continue; + } + scopes.push({ + scopePath: `agents.list.${agent.id}.tools.exec`, + safeBins, + exec: agentExec, + mergedProfiles: + resolveMergedSafeBinProfileFixtures({ + global: globalExec, + local: agentExec, + }) ?? {}, + }); + } + return scopes; +} + +function scanExecSafeBinCoverage(cfg: OpenClawConfig): ExecSafeBinCoverageHit[] { + const hits: ExecSafeBinCoverageHit[] = []; + for (const scope of collectExecSafeBinScopes(cfg)) { + const interpreterBins = new Set(listInterpreterLikeSafeBins(scope.safeBins)); + for (const bin of scope.safeBins) { + if (scope.mergedProfiles[bin]) { + continue; + } + hits.push({ + scopePath: scope.scopePath, + bin, + isInterpreter: interpreterBins.has(bin), + }); + } + } + return hits; +} + +function maybeRepairExecSafeBinProfiles(cfg: OpenClawConfig): { + config: OpenClawConfig; + changes: string[]; + warnings: string[]; +} { + const next = structuredClone(cfg); + const changes: string[] = []; + const warnings: string[] = []; + + for (const scope of collectExecSafeBinScopes(next)) { + const interpreterBins = new Set(listInterpreterLikeSafeBins(scope.safeBins)); + const missingBins = scope.safeBins.filter((bin) => !scope.mergedProfiles[bin]); + if (missingBins.length === 0) { + continue; + } + const profileHolder = + asObjectRecord(scope.exec.safeBinProfiles) ?? (scope.exec.safeBinProfiles = {}); + for (const bin of missingBins) { + if (interpreterBins.has(bin)) { + warnings.push( + `- ${scope.scopePath}.safeBins includes interpreter/runtime '${bin}' without profile; remove it from safeBins or use explicit allowlist entries.`, + ); + continue; + } + if (profileHolder[bin] !== undefined) { + continue; + } + profileHolder[bin] = {}; + changes.push( + `- ${scope.scopePath}.safeBinProfiles.${bin}: added scaffold profile {} (review and tighten flags/positionals).`, + ); + } + } + + if (changes.length === 0 && warnings.length === 0) { + return { config: cfg, changes: [], warnings: [] }; + } + return { config: next, changes, warnings }; +} + async function maybeMigrateLegacyConfig(): Promise { const changes: string[] = []; const home = resolveHomeDir(); @@ -859,6 +991,16 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { pendingChanges = true; cfg = allowFromRepair.config; } + const safeBinProfileRepair = maybeRepairExecSafeBinProfiles(candidate); + if (safeBinProfileRepair.changes.length > 0) { + note(safeBinProfileRepair.changes.join("\n"), "Doctor changes"); + candidate = safeBinProfileRepair.config; + pendingChanges = true; + cfg = safeBinProfileRepair.config; + } + if (safeBinProfileRepair.warnings.length > 0) { + note(safeBinProfileRepair.warnings.join("\n"), "Doctor warnings"); + } } else { const hits = scanTelegramAllowFromUsernameEntries(candidate); if (hits.length > 0) { @@ -892,6 +1034,41 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { "Doctor warnings", ); } + + const safeBinCoverage = scanExecSafeBinCoverage(candidate); + if (safeBinCoverage.length > 0) { + const interpreterHits = safeBinCoverage.filter((hit) => hit.isInterpreter); + const customHits = safeBinCoverage.filter((hit) => !hit.isInterpreter); + const lines: string[] = []; + if (interpreterHits.length > 0) { + for (const hit of interpreterHits.slice(0, 5)) { + lines.push( + `- ${hit.scopePath}.safeBins includes interpreter/runtime '${hit.bin}' without profile.`, + ); + } + if (interpreterHits.length > 5) { + lines.push( + `- ${interpreterHits.length - 5} more interpreter/runtime safeBins entries are missing profiles.`, + ); + } + } + if (customHits.length > 0) { + for (const hit of customHits.slice(0, 5)) { + lines.push( + `- ${hit.scopePath}.safeBins entry '${hit.bin}' is missing safeBinProfiles.${hit.bin}.`, + ); + } + if (customHits.length > 5) { + lines.push( + `- ${customHits.length - 5} more custom safeBins entries are missing profiles.`, + ); + } + } + lines.push( + `- Run "${formatCliCommand("openclaw doctor --fix")}" to scaffold missing custom safeBinProfiles entries.`, + ); + note(lines.join("\n"), "Doctor warnings"); + } } const unknown = stripUnknownConfigKeys(candidate); diff --git a/src/config/io.compat.test.ts b/src/config/io.compat.test.ts index 7da811a76e1..6ac794b19b0 100644 --- a/src/config/io.compat.test.ts +++ b/src/config/io.compat.test.ts @@ -77,4 +77,60 @@ describe("config io paths", () => { expect(io.loadConfig().gateway?.port).toBe(20003); }); }); + + it("normalizes safeBinProfiles at config load time", async () => { + await withTempHome(async (home) => { + const configDir = path.join(home, ".openclaw"); + await fs.mkdir(configDir, { recursive: true }); + const configPath = path.join(configDir, "openclaw.json"); + await fs.writeFile( + configPath, + JSON.stringify( + { + tools: { + exec: { + safeBinProfiles: { + " MyFilter ": { + allowedValueFlags: ["--limit", " --limit ", ""], + }, + }, + }, + }, + agents: { + list: [ + { + id: "ops", + tools: { + exec: { + safeBinProfiles: { + " Custom ": { + deniedFlags: ["-f", " -f ", ""], + }, + }, + }, + }, + }, + ], + }, + }, + null, + 2, + ), + "utf-8", + ); + const io = createIoForHome(home); + expect(io.configPath).toBe(configPath); + const cfg = io.loadConfig(); + expect(cfg.tools?.exec?.safeBinProfiles).toEqual({ + myfilter: { + allowedValueFlags: ["--limit"], + }, + }); + expect(cfg.agents?.list?.[0]?.tools?.exec?.safeBinProfiles).toEqual({ + custom: { + deniedFlags: ["-f"], + }, + }); + }); + }); }); diff --git a/src/config/io.ts b/src/config/io.ts index c5df09e433a..2a41883f7ea 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -6,6 +6,7 @@ import { isDeepStrictEqual } from "node:util"; import JSON5 from "json5"; import { ensureOwnerDisplaySecret } from "../agents/owner-display.js"; import { loadDotEnv } from "../infra/dotenv.js"; +import { normalizeSafeBinProfileFixtures } from "../infra/exec-safe-bin-policy.js"; import { resolveRequiredHomeDir } from "../infra/home-dir.js"; import { loadShellEnvFallback, @@ -555,6 +556,33 @@ function maybeLoadDotEnvForConfig(env: NodeJS.ProcessEnv): void { loadDotEnv({ quiet: true }); } +function normalizeExecSafeBinProfilesInConfig(cfg: OpenClawConfig): void { + const normalizeExec = (exec: unknown) => { + if (!exec || typeof exec !== "object" || Array.isArray(exec)) { + return; + } + const typedExec = exec as { safeBinProfiles?: Record }; + const normalized = normalizeSafeBinProfileFixtures( + typedExec.safeBinProfiles as Record< + string, + { + minPositional?: number; + maxPositional?: number; + allowedValueFlags?: readonly string[]; + deniedFlags?: readonly string[]; + } + >, + ); + typedExec.safeBinProfiles = Object.keys(normalized).length > 0 ? normalized : undefined; + }; + + normalizeExec(cfg.tools?.exec); + const agents = Array.isArray(cfg.agents?.list) ? cfg.agents.list : []; + for (const agent of agents) { + normalizeExec(agent?.tools?.exec); + } +} + export function parseConfigJson5( raw: string, json5: { parse: (value: string) => unknown } = JSON5, @@ -675,6 +703,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { ), ); normalizeConfigPaths(cfg); + normalizeExecSafeBinProfilesInConfig(cfg); const duplicates = findDuplicateAgentDirs(cfg, { env: deps.env, @@ -875,6 +904,16 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { } warnIfConfigFromFuture(validated.config, deps.logger); + const snapshotConfig = normalizeConfigPaths( + applyTalkApiKey( + applyModelDefaults( + applyAgentDefaults( + applySessionDefaults(applyLoggingDefaults(applyMessageDefaults(validated.config))), + ), + ), + ), + ); + normalizeExecSafeBinProfilesInConfig(snapshotConfig); return { snapshot: { path: configPath, @@ -885,17 +924,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { // for config set/unset operations (issue #6070) resolved: coerceConfig(resolvedConfigRaw), valid: true, - config: normalizeConfigPaths( - applyTalkApiKey( - applyModelDefaults( - applyAgentDefaults( - applySessionDefaults( - applyLoggingDefaults(applyMessageDefaults(validated.config)), - ), - ), - ), - ), - ), + config: snapshotConfig, hash, issues: [], warnings: validated.warnings, diff --git a/src/infra/exec-safe-bin-policy.ts b/src/infra/exec-safe-bin-policy.ts index 79548738de9..878c0a55e5c 100644 --- a/src/infra/exec-safe-bin-policy.ts +++ b/src/infra/exec-safe-bin-policy.ts @@ -192,7 +192,44 @@ function normalizeSafeBinProfileName(raw: string): string | null { return name.length > 0 ? name : null; } -function normalizeSafeBinProfileFixtures( +function normalizeFixtureLimit(raw: number | undefined): number | undefined { + if (typeof raw !== "number" || !Number.isFinite(raw)) { + return undefined; + } + const next = Math.trunc(raw); + return next >= 0 ? next : undefined; +} + +function normalizeFixtureFlags( + flags: readonly string[] | undefined, +): readonly string[] | undefined { + if (!Array.isArray(flags) || flags.length === 0) { + return undefined; + } + const normalized = Array.from( + new Set(flags.map((flag) => flag.trim()).filter((flag) => flag.length > 0)), + ).toSorted((a, b) => a.localeCompare(b)); + return normalized.length > 0 ? normalized : undefined; +} + +function normalizeSafeBinProfileFixture(fixture: SafeBinProfileFixture): SafeBinProfileFixture { + const minPositional = normalizeFixtureLimit(fixture.minPositional); + const maxPositionalRaw = normalizeFixtureLimit(fixture.maxPositional); + const maxPositional = + minPositional !== undefined && + maxPositionalRaw !== undefined && + maxPositionalRaw < minPositional + ? minPositional + : maxPositionalRaw; + return { + minPositional, + maxPositional, + allowedValueFlags: normalizeFixtureFlags(fixture.allowedValueFlags), + deniedFlags: normalizeFixtureFlags(fixture.deniedFlags), + }; +} + +export function normalizeSafeBinProfileFixtures( fixtures?: SafeBinProfileFixtures | null, ): Record { const normalized: Record = {}; @@ -204,12 +241,7 @@ function normalizeSafeBinProfileFixtures( if (!name) { continue; } - normalized[name] = { - minPositional: fixture.minPositional, - maxPositional: fixture.maxPositional, - allowedValueFlags: fixture.allowedValueFlags, - deniedFlags: fixture.deniedFlags, - }; + normalized[name] = normalizeSafeBinProfileFixture(fixture); } return normalized; } diff --git a/src/infra/exec-safe-bin-runtime-policy.test.ts b/src/infra/exec-safe-bin-runtime-policy.test.ts new file mode 100644 index 00000000000..ef7f4504915 --- /dev/null +++ b/src/infra/exec-safe-bin-runtime-policy.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; +import { + isInterpreterLikeSafeBin, + listInterpreterLikeSafeBins, + resolveExecSafeBinRuntimePolicy, + resolveMergedSafeBinProfileFixtures, +} from "./exec-safe-bin-runtime-policy.js"; + +describe("exec safe-bin runtime policy", () => { + const interpreterCases: Array<{ bin: string; expected: boolean }> = [ + { bin: "python3", expected: true }, + { bin: "python3.12", expected: true }, + { bin: "node", expected: true }, + { bin: "node20", expected: true }, + { bin: "ruby3.2", expected: true }, + { bin: "bash", expected: true }, + { bin: "myfilter", expected: false }, + { bin: "jq", expected: false }, + ]; + + for (const testCase of interpreterCases) { + it(`classifies interpreter-like safe bin '${testCase.bin}'`, () => { + expect(isInterpreterLikeSafeBin(testCase.bin)).toBe(testCase.expected); + }); + } + + it("lists interpreter-like bins from a mixed set", () => { + expect(listInterpreterLikeSafeBins(["jq", "python3", "myfilter", "node"])).toEqual([ + "node", + "python3", + ]); + }); + + it("merges and normalizes safe-bin profile fixtures", () => { + const merged = resolveMergedSafeBinProfileFixtures({ + global: { + safeBinProfiles: { + " MyFilter ": { + deniedFlags: ["--file", " --file ", ""], + }, + }, + }, + local: { + safeBinProfiles: { + myfilter: { + maxPositional: 0, + }, + }, + }, + }); + expect(merged).toEqual({ + myfilter: { + maxPositional: 0, + }, + }); + }); + + it("computes unprofiled interpreter entries separately from custom profiled bins", () => { + const policy = resolveExecSafeBinRuntimePolicy({ + local: { + safeBins: ["python3", "myfilter"], + safeBinProfiles: { + myfilter: { maxPositional: 0 }, + }, + }, + }); + + expect(policy.safeBins.has("python3")).toBe(true); + expect(policy.safeBins.has("myfilter")).toBe(true); + expect(policy.unprofiledSafeBins).toEqual(["python3"]); + expect(policy.unprofiledInterpreterSafeBins).toEqual(["python3"]); + }); +}); diff --git a/src/infra/exec-safe-bin-runtime-policy.ts b/src/infra/exec-safe-bin-runtime-policy.ts new file mode 100644 index 00000000000..930206a70f3 --- /dev/null +++ b/src/infra/exec-safe-bin-runtime-policy.ts @@ -0,0 +1,127 @@ +import { resolveSafeBins } from "./exec-approvals-allowlist.js"; +import { + normalizeSafeBinProfileFixtures, + resolveSafeBinProfiles, + type SafeBinProfile, + type SafeBinProfileFixture, + type SafeBinProfileFixtures, +} from "./exec-safe-bin-policy.js"; +import { getTrustedSafeBinDirs } from "./exec-safe-bin-trust.js"; + +export type ExecSafeBinConfigScope = { + safeBins?: string[] | null; + safeBinProfiles?: SafeBinProfileFixtures | null; +}; + +const INTERPRETER_LIKE_SAFE_BINS = new Set([ + "ash", + "bash", + "bun", + "cmd", + "cmd.exe", + "cscript", + "dash", + "deno", + "fish", + "ksh", + "lua", + "node", + "nodejs", + "perl", + "php", + "powershell", + "powershell.exe", + "pypy", + "pwsh", + "pwsh.exe", + "python", + "python2", + "python3", + "ruby", + "sh", + "wscript", + "zsh", +]); + +const INTERPRETER_LIKE_PATTERNS = [ + /^python\d+(?:\.\d+)?$/, + /^ruby\d+(?:\.\d+)?$/, + /^perl\d+(?:\.\d+)?$/, + /^php\d+(?:\.\d+)?$/, + /^node\d+(?:\.\d+)?$/, +]; + +function normalizeSafeBinName(raw: string): string { + const trimmed = raw.trim().toLowerCase(); + if (!trimmed) { + return ""; + } + const tail = trimmed.split(/[\\/]/).at(-1); + return tail ?? trimmed; +} + +export function isInterpreterLikeSafeBin(raw: string): boolean { + const normalized = normalizeSafeBinName(raw); + if (!normalized) { + return false; + } + if (INTERPRETER_LIKE_SAFE_BINS.has(normalized)) { + return true; + } + return INTERPRETER_LIKE_PATTERNS.some((pattern) => pattern.test(normalized)); +} + +export function listInterpreterLikeSafeBins(entries: Iterable): string[] { + return Array.from(entries) + .map((entry) => normalizeSafeBinName(entry)) + .filter((entry) => entry.length > 0 && isInterpreterLikeSafeBin(entry)) + .toSorted(); +} + +export function resolveMergedSafeBinProfileFixtures(params: { + global?: ExecSafeBinConfigScope | null; + local?: ExecSafeBinConfigScope | null; +}): Record | undefined { + const global = normalizeSafeBinProfileFixtures(params.global?.safeBinProfiles); + const local = normalizeSafeBinProfileFixtures(params.local?.safeBinProfiles); + if (Object.keys(global).length === 0 && Object.keys(local).length === 0) { + return undefined; + } + return { + ...global, + ...local, + }; +} + +export function resolveExecSafeBinRuntimePolicy(params: { + global?: ExecSafeBinConfigScope | null; + local?: ExecSafeBinConfigScope | null; + pathEnv?: string | null; +}): { + safeBins: Set; + safeBinProfiles: Readonly>; + trustedSafeBinDirs: ReadonlySet; + unprofiledSafeBins: string[]; + unprofiledInterpreterSafeBins: string[]; +} { + const safeBins = resolveSafeBins(params.local?.safeBins ?? params.global?.safeBins); + const safeBinProfiles = resolveSafeBinProfiles( + resolveMergedSafeBinProfileFixtures({ + global: params.global, + local: params.local, + }), + ); + const unprofiledSafeBins = Array.from(safeBins) + .filter((entry) => !safeBinProfiles[entry]) + .toSorted(); + const trustedSafeBinDirs = params.pathEnv + ? getTrustedSafeBinDirs({ pathEnv: params.pathEnv }) + : getTrustedSafeBinDirs(); + return { + safeBins, + safeBinProfiles, + trustedSafeBinDirs, + unprofiledSafeBins, + unprofiledInterpreterSafeBins: listInterpreterLikeSafeBins(unprofiledSafeBins), + }; +} diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 7d6747e15f0..92f3b632b62 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -11,15 +11,13 @@ import { requiresExecApproval, resolveAllowAlwaysPatterns, resolveExecApprovals, - resolveSafeBins, type ExecAllowlistEntry, type ExecAsk, type ExecCommandSegment, type ExecSecurity, } from "../infra/exec-approvals.js"; import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js"; -import { resolveSafeBinProfiles } from "../infra/exec-safe-bin-policy.js"; -import { getTrustedSafeBinDirs } from "../infra/exec-safe-bin-trust.js"; +import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; import { sanitizeSystemRunEnvOverrides } from "../infra/host-env-security.js"; import { resolveSystemRunCommand } from "../infra/system-run-command.js"; import type { @@ -116,12 +114,10 @@ export async function handleSystemRunInvoke(opts: { shellWrapper: shellCommand !== null, }); const env = opts.sanitizeEnv(envOverrides); - const safeBins = resolveSafeBins(agentExec?.safeBins ?? cfg.tools?.exec?.safeBins); - const safeBinProfiles = resolveSafeBinProfiles({ - ...cfg.tools?.exec?.safeBinProfiles, - ...agentExec?.safeBinProfiles, + const { safeBins, safeBinProfiles, trustedSafeBinDirs } = resolveExecSafeBinRuntimePolicy({ + global: cfg.tools?.exec, + local: agentExec, }); - const trustedSafeBinDirs = getTrustedSafeBinDirs(); const bins = autoAllowSkills ? await opts.skillBins.current() : new Set(); let analysisOk = false; let allowlistMatches: ExecAllowlistEntry[] = []; diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 02060d49d7b..45198951a32 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -296,6 +296,70 @@ describe("security audit", () => { expect(hasFinding(res, "tools.exec.host_sandbox_no_sandbox_agents", "warn")).toBe(true); }); + it("warns for interpreter safeBins entries without explicit profiles", async () => { + const cfg: OpenClawConfig = { + tools: { + exec: { + safeBins: ["python3"], + }, + }, + agents: { + list: [ + { + id: "ops", + tools: { + exec: { + safeBins: ["node"], + }, + }, + }, + ], + }, + }; + + const res = await audit(cfg); + + expect(hasFinding(res, "tools.exec.safe_bins_interpreter_unprofiled", "warn")).toBe(true); + }); + + it("does not warn for interpreter safeBins when explicit profiles are present", async () => { + const cfg: OpenClawConfig = { + tools: { + exec: { + safeBins: ["python3"], + safeBinProfiles: { + python3: { + maxPositional: 0, + }, + }, + }, + }, + agents: { + list: [ + { + id: "ops", + tools: { + exec: { + safeBins: ["node"], + safeBinProfiles: { + node: { + maxPositional: 0, + }, + }, + }, + }, + }, + ], + }, + }; + + const res = await audit(cfg); + + expect( + res.findings.some((f) => f.checkId === "tools.exec.safe_bins_interpreter_unprofiled"), + ).toBe(false); + }); + it("warns when loopback control UI lacks trusted proxies", async () => { const cfg: OpenClawConfig = { gateway: { diff --git a/src/security/audit.ts b/src/security/audit.ts index 651fb619b25..fdb3be9ce07 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -11,6 +11,10 @@ import { resolveGatewayAuth } from "../gateway/auth.js"; import { buildGatewayConnectionDetails } from "../gateway/call.js"; import { resolveGatewayProbeAuth } from "../gateway/probe-auth.js"; import { probeGateway } from "../gateway/probe.js"; +import { + listInterpreterLikeSafeBins, + resolveMergedSafeBinProfileFixtures, +} from "../infra/exec-safe-bin-runtime-policy.js"; import { collectChannelSecurityFindings } from "./audit-channel.js"; import { collectAttackSurfaceSummaryFindings, @@ -695,6 +699,65 @@ function collectExecRuntimeFindings(cfg: OpenClawConfig): SecurityAuditFinding[] }); } + const normalizeConfiguredSafeBins = (entries: unknown): string[] => { + if (!Array.isArray(entries)) { + return []; + } + return Array.from( + new Set( + entries + .map((entry) => (typeof entry === "string" ? entry.trim().toLowerCase() : "")) + .filter((entry) => entry.length > 0), + ), + ).toSorted(); + }; + const interpreterHits: string[] = []; + const globalExec = cfg.tools?.exec; + const globalSafeBins = normalizeConfiguredSafeBins(globalExec?.safeBins); + if (globalSafeBins.length > 0) { + const merged = resolveMergedSafeBinProfileFixtures({ global: globalExec }) ?? {}; + const interpreters = listInterpreterLikeSafeBins(globalSafeBins).filter((bin) => !merged[bin]); + if (interpreters.length > 0) { + interpreterHits.push(`- tools.exec.safeBins: ${interpreters.join(", ")}`); + } + } + + for (const entry of agents) { + if (!entry || typeof entry !== "object" || typeof entry.id !== "string") { + continue; + } + const agentExec = entry.tools?.exec; + const agentSafeBins = normalizeConfiguredSafeBins(agentExec?.safeBins); + if (agentSafeBins.length === 0) { + continue; + } + const merged = + resolveMergedSafeBinProfileFixtures({ + global: globalExec, + local: agentExec, + }) ?? {}; + const interpreters = listInterpreterLikeSafeBins(agentSafeBins).filter((bin) => !merged[bin]); + if (interpreters.length === 0) { + continue; + } + interpreterHits.push( + `- agents.list.${entry.id}.tools.exec.safeBins: ${interpreters.join(", ")}`, + ); + } + + if (interpreterHits.length > 0) { + findings.push({ + checkId: "tools.exec.safe_bins_interpreter_unprofiled", + severity: "warn", + title: "safeBins includes interpreter/runtime binaries without explicit profiles", + detail: + `Detected interpreter-like safeBins entries missing explicit profiles:\n${interpreterHits.join("\n")}\n` + + "These entries can turn safeBins into a broad execution surface when used with permissive argv profiles.", + remediation: + "Remove interpreter/runtime bins from safeBins (prefer allowlist entries) or define hardened tools.exec.safeBinProfiles. rules.", + }); + } + return findings; }