From 132794fe74d233098e71e4cde19a5d3f25473552 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 23:28:46 +0000 Subject: [PATCH] feat(security): audit workspace skill symlink escapes --- CHANGELOG.md | 1 + docs/gateway/security/index.md | 67 ++++++++-------- src/security/audit-extra.async.ts | 126 ++++++++++++++++++++++++++++++ src/security/audit-extra.ts | 1 + src/security/audit.test.ts | 65 +++++++++++++++ src/security/audit.ts | 2 + 6 files changed, 229 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42864fbfc48..714384f9116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai - Feishu/DM pairing reply target: send pairing challenge replies to `chat:` instead of `user:` so Lark/Feishu private chats with user-id-only sender payloads receive pairing messages reliably. (#31403) Thanks @stakeswky. - Feishu/Lark private DM routing: treat inbound `chat_type: "private"` as direct-message context for pairing/mention-forward/reaction synthetic handling so Lark private chats behave like Feishu p2p DMs. (#31400) Thanks @stakeswky. - Sandbox/workspace mount permissions: make primary `/workspace` bind mounts read-only whenever `workspaceAccess` is not `rw` (including `none`) across both core sandbox container and sandbox browser create flows. (#32227) Thanks @guanyu-zhang. +- Security audit/skills workspace hardening: add `skills.workspace.symlink_escape` warning in `openclaw security audit` when workspace `skills/**/SKILL.md` resolves outside the workspace root (for example symlink-chain drift), plus docs coverage in the security glossary. - Signal/message actions: allow `react` to fall back to `toolContext.currentMessageId` when `messageId` is omitted, matching Telegram behavior and unblocking agent-initiated reactions on inbound turns. (#32217) Thanks @dunamismax. - Gateway/OpenAI chat completions: honor `x-openclaw-message-channel` when building `agentCommand` input for `/v1/chat/completions`, preserving caller channel identity instead of forcing `webchat`. (#30462) Thanks @bmendonca3. - Secrets/exec resolver timeout defaults: use provider `timeoutMs` as the default inactivity (`noOutputTimeoutMs`) watchdog for exec secret providers, preventing premature no-output kills for resolvers that start producing output after 2s. (#32235) Thanks @bmendonca3. diff --git a/docs/gateway/security/index.md b/docs/gateway/security/index.md index 470cb7df08f..d615b78669a 100644 --- a/docs/gateway/security/index.md +++ b/docs/gateway/security/index.md @@ -224,39 +224,40 @@ 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.allowed_origins_required` | critical | Non-loopback Control UI without explicit browser-origin allowlist | `gateway.controlUi.allowedOrigins` | no | -| `gateway.control_ui.host_header_origin_fallback` | warn/critical | Enables Host-header origin fallback (DNS rebinding hardening downgrade) | `gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback` | 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 | -| `sandbox.dangerous_network_mode` | critical | Sandbox Docker network uses `host` or `container:*` namespace-join mode | `agents.*.sandbox.docker.network` | 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_elevated` | critical | Open groups + elevated tools create high-impact prompt-injection paths | `channels.*.groupPolicy`, `tools.elevated.*` | 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 | -| `security.trust_model.multi_user_heuristic` | warn | Config looks multi-user while gateway trust model is personal-assistant | split trust boundaries, or shared-user hardening (`sandbox.mode`, tool deny/workspace scoping) | 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.allowed_origins_required` | critical | Non-loopback Control UI without explicit browser-origin allowlist | `gateway.controlUi.allowedOrigins` | no | +| `gateway.control_ui.host_header_origin_fallback` | warn/critical | Enables Host-header origin fallback (DNS rebinding hardening downgrade) | `gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback` | 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 | +| `sandbox.dangerous_network_mode` | critical | Sandbox Docker network uses `host` or `container:*` namespace-join mode | `agents.*.sandbox.docker.network` | 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 | +| `skills.workspace.symlink_escape` | warn | Workspace `skills/**/SKILL.md` resolves outside workspace root (symlink-chain drift) | workspace `skills/**` filesystem state | no | +| `security.exposure.open_groups_with_elevated` | critical | Open groups + elevated tools create high-impact prompt-injection paths | `channels.*.groupPolicy`, `tools.elevated.*` | 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 | +| `security.trust_model.multi_user_heuristic` | warn | Config looks multi-user while gateway trust model is personal-assistant | split trust boundaries, or shared-user hardening (`sandbox.mode`, tool deny/workspace scoping) | 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/src/security/audit-extra.async.ts b/src/security/audit-extra.async.ts index 11b0f98f7f1..6d0347261de 100644 --- a/src/security/audit-extra.async.ts +++ b/src/security/audit-extra.async.ts @@ -53,6 +53,8 @@ type ExecDockerRawFn = ( ) => Promise; type CodeSafetySummaryCache = Map>; +const MAX_WORKSPACE_SKILL_SCAN_FILES_PER_WORKSPACE = 2_000; +const MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS = 12; // -------------------------------------------------------------------------- // Helpers @@ -283,6 +285,58 @@ async function getCodeSafetySummary(params: { }); } +async function listWorkspaceSkillMarkdownFiles(workspaceDir: string): Promise { + const skillsRoot = path.join(workspaceDir, "skills"); + const rootStat = await safeStat(skillsRoot); + if (!rootStat.ok || !rootStat.isDir) { + return []; + } + + const skillFiles: string[] = []; + const queue: string[] = [skillsRoot]; + const visitedDirs = new Set(); + + while (queue.length > 0 && skillFiles.length < MAX_WORKSPACE_SKILL_SCAN_FILES_PER_WORKSPACE) { + const dir = queue.shift()!; + const dirRealPath = await fs.realpath(dir).catch(() => path.resolve(dir)); + if (visitedDirs.has(dirRealPath)) { + continue; + } + visitedDirs.add(dirRealPath); + + const entries = await fs.readdir(dir, { withFileTypes: true }).catch(() => []); + for (const entry of entries) { + if (entry.name.startsWith(".") || entry.name === "node_modules") { + continue; + } + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + queue.push(fullPath); + continue; + } + if (entry.isSymbolicLink()) { + const stat = await fs.stat(fullPath).catch(() => null); + if (!stat) { + continue; + } + if (stat.isDirectory()) { + queue.push(fullPath); + continue; + } + if (stat.isFile() && entry.name === "SKILL.md") { + skillFiles.push(fullPath); + } + continue; + } + if (entry.isFile() && entry.name === "SKILL.md") { + skillFiles.push(fullPath); + } + } + } + + return skillFiles; +} + // -------------------------------------------------------------------------- // Exported collectors // -------------------------------------------------------------------------- @@ -756,6 +810,78 @@ export async function collectPluginsTrustFindings(params: { return findings; } +export async function collectWorkspaceSkillSymlinkEscapeFindings(params: { + cfg: OpenClawConfig; +}): Promise { + const findings: SecurityAuditFinding[] = []; + const workspaceDirs = listAgentWorkspaceDirs(params.cfg); + if (workspaceDirs.length === 0) { + return findings; + } + + const escapedSkillFiles: Array<{ + workspaceDir: string; + skillFilePath: string; + skillRealPath: string; + }> = []; + const seenSkillPaths = new Set(); + + for (const workspaceDir of workspaceDirs) { + const workspacePath = path.resolve(workspaceDir); + const workspaceRealPath = await fs.realpath(workspacePath).catch(() => workspacePath); + const skillFilePaths = await listWorkspaceSkillMarkdownFiles(workspacePath); + + for (const skillFilePath of skillFilePaths) { + const canonicalSkillPath = path.resolve(skillFilePath); + if (seenSkillPaths.has(canonicalSkillPath)) { + continue; + } + seenSkillPaths.add(canonicalSkillPath); + + const skillRealPath = await fs.realpath(canonicalSkillPath).catch(() => null); + if (!skillRealPath) { + continue; + } + if (isPathInside(workspaceRealPath, skillRealPath)) { + continue; + } + escapedSkillFiles.push({ + workspaceDir: workspacePath, + skillFilePath: canonicalSkillPath, + skillRealPath, + }); + } + } + + if (escapedSkillFiles.length === 0) { + return findings; + } + + findings.push({ + checkId: "skills.workspace.symlink_escape", + severity: "warn", + title: "Workspace skill files resolve outside the workspace root", + detail: + "Detected workspace `skills/**/SKILL.md` paths whose realpath escapes their workspace root:\n" + + escapedSkillFiles + .slice(0, MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS) + .map( + (entry) => + `- workspace=${entry.workspaceDir}\n` + + ` skill=${entry.skillFilePath}\n` + + ` realpath=${entry.skillRealPath}`, + ) + .join("\n") + + (escapedSkillFiles.length > MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS + ? `\n- +${escapedSkillFiles.length - MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS} more` + : ""), + remediation: + "Keep workspace skills inside the workspace root (replace symlinked escapes with real in-workspace files), or move trusted shared skills to managed/bundled skill locations.", + }); + + return findings; +} + export async function collectIncludeFilePermFindings(params: { configSnapshot: ConfigFileSnapshot; env?: NodeJS.ProcessEnv; diff --git a/src/security/audit-extra.ts b/src/security/audit-extra.ts index 9345cb8732b..90fcc0c6bf3 100644 --- a/src/security/audit-extra.ts +++ b/src/security/audit-extra.ts @@ -35,5 +35,6 @@ export { collectPluginsCodeSafetyFindings, collectPluginsTrustFindings, collectStateDeepFilesystemFindings, + collectWorkspaceSkillSymlinkEscapeFindings, readConfigSnapshotForAudit, } from "./audit-extra.async.js"; diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index fd61d97b97f..f68c7b94664 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -844,6 +844,71 @@ description: test skill expect(res.findings.some((f) => f.checkId === "fs.config.perms_group_readable")).toBe(false); }); + it("warns when workspace skill files resolve outside workspace root", async () => { + if (isWindows) { + return; + } + + const tmp = await makeTmpDir("workspace-skill-symlink-escape"); + const stateDir = path.join(tmp, "state"); + const workspaceDir = path.join(tmp, "workspace"); + const outsideDir = path.join(tmp, "outside"); + await fs.mkdir(stateDir, { recursive: true, mode: 0o700 }); + await fs.mkdir(path.join(workspaceDir, "skills", "leak"), { recursive: true }); + await fs.mkdir(outsideDir, { recursive: true }); + + const outsideSkillPath = path.join(outsideDir, "SKILL.md"); + await fs.writeFile(outsideSkillPath, "# outside\n", "utf-8"); + await fs.symlink(outsideSkillPath, path.join(workspaceDir, "skills", "leak", "SKILL.md")); + + const configPath = path.join(stateDir, "openclaw.json"); + await fs.writeFile(configPath, "{}\n", "utf-8"); + await fs.chmod(configPath, 0o600); + + const res = await runSecurityAudit({ + config: { agents: { defaults: { workspace: workspaceDir } } }, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath, + execDockerRawFn: execDockerRawUnavailable, + }); + + const finding = res.findings.find((f) => f.checkId === "skills.workspace.symlink_escape"); + expect(finding?.severity).toBe("warn"); + expect(finding?.detail).toContain(outsideSkillPath); + }); + + it("does not warn for workspace skills that stay inside workspace root", async () => { + const tmp = await makeTmpDir("workspace-skill-in-root"); + const stateDir = path.join(tmp, "state"); + const workspaceDir = path.join(tmp, "workspace"); + await fs.mkdir(stateDir, { recursive: true, mode: 0o700 }); + await fs.mkdir(path.join(workspaceDir, "skills", "safe"), { recursive: true }); + await fs.writeFile( + path.join(workspaceDir, "skills", "safe", "SKILL.md"), + "# in workspace\n", + "utf-8", + ); + + const configPath = path.join(stateDir, "openclaw.json"); + await fs.writeFile(configPath, "{}\n", "utf-8"); + if (!isWindows) { + await fs.chmod(configPath, 0o600); + } + + const res = await runSecurityAudit({ + config: { agents: { defaults: { workspace: workspaceDir } } }, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath, + execDockerRawFn: execDockerRawUnavailable, + }); + + expect(res.findings.some((f) => f.checkId === "skills.workspace.symlink_escape")).toBe(false); + }); + it("scores small-model risk by tool/sandbox exposure", async () => { const cases: Array<{ name: string; diff --git a/src/security/audit.ts b/src/security/audit.ts index fbc1fcb322e..c21d349ccb7 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -40,6 +40,7 @@ import { collectPluginsCodeSafetyFindings, collectStateDeepFilesystemFindings, collectSyncedFolderFindings, + collectWorkspaceSkillSymlinkEscapeFindings, readConfigSnapshotForAudit, } from "./audit-extra.js"; import { @@ -1054,6 +1055,7 @@ export async function runSecurityAudit(opts: SecurityAuditOptions): Promise