ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-27 12:56:01 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): audit agent skill MCP exec drift

Browse Source
This commit is contained in:
momothemage
2026-07-01 11:02:59 +08:00
parent 4ac5cf8636
commit ab3c29ef4c
5 changed files with 258 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/gateway/security/audit-checks.md
+1
View File
@@ -92,6 +92,7 @@ exhaustive):
| `tools.exec.host_sandbox_no_sandbox_defaults` | warn | `exec host=sandbox` fails closed 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` fails closed when sandbox is off | `agents.list[].tools.exec.host`, `agents.list[].sandbox.mode` | no |
| `tools.exec.security_full_configured` | warn/critical | Host exec is running with `security="full"` | `tools.exec.security`, `agents.list[].tools.exec.security` | no |
| `tools.exec.agent_skill_mcp_boundary_drift` | warn | Agent skill allowlists are present while host exec can reach MCP clients/registries | `agents.list[].tools.exec.*`, sandbox/OS isolation, MCP server credentials | no |
| `tools.exec.fs_tools_disabled_but_exec_enabled` | warn | Filesystem tool policy does not make shell execution read-only | `tools.deny`, `agents.list[].tools.deny`, `agents.*.sandbox.workspaceAccess` | no |
| `tools.exec.auto_allow_skills_enabled` | warn | Exec approvals trust skill bins implicitly | host approvals file | no |
| `tools.exec.allowlist_interpreter_without_strict_inline_eval` | warn | Interpreter allowlists permit inline eval without forced reapproval | `tools.exec.strictInlineEval`, `agents.list[].tools.exec.strictInlineEval`, exec approvals allowlist | no |
docs/tools/skills-config.md
+11
View File
@@ -321,6 +321,17 @@ different visible skill set per agent.
defaults — they do not merge. Set to `[]` to expose no skills for that agent.
</ParamField>
<Warning>
Agent skill allowlists are a visibility and loading filter for OpenClaw skill
discovery, prompts, slash-command discovery, sandbox sync, and skill
snapshots. They are not a shell-time authorization boundary. If an agent can
run host `exec`, that shell can still run external clients or read host files
that are visible to the execution user, including MCP client registries such
as `~/.openclaw/skills/config/mcporter.json`. For per-agent MCP isolation,
combine skill allowlists with sandbox/OS-user isolation, deny or tightly
allowlist host exec, and prefer per-agent credentials at the MCP server.
</Warning>
## Workshop (`skills.workshop`)
<ParamField path="skills.workshop.autonomous.enabled" type="boolean" default="false">
docs/tools/skills.md
+3
View File
@@ -104,6 +104,9 @@ regardless of where they are loaded from.
merge with defaults.
- The effective allowlist applies across prompt building, slash-command
discovery, sandbox sync, and skill snapshots.
- This is not a host shell authorization boundary. If the same agent can
use `exec`, constrain that shell separately with sandboxing, OS-user
isolation, exec deny/allowlists, and per-resource credentials.
</Accordion>
</AccordionGroup>
src/security/audit-config-basics.test.ts
+87
View File
@@ -1,4 +1,7 @@
// Covers baseline config security audit findings.
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { describe, expect, it } from "vitest";
import {
onInternalDiagnosticEvent,
@@ -63,6 +66,90 @@ describe("security audit config basics", () => {
).toBe(true);
});
it("flags per-agent skill allowlists combined with host exec and a global mcporter registry", async () => {
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-audit-mcporter-"));
try {
await fs.mkdir(path.join(stateDir, "skills", "config"), { recursive: true });
await fs.writeFile(
path.join(stateDir, "skills", "config", "mcporter.json"),
JSON.stringify({
mcpServers: {
"hugegraph-asset": { baseUrl: "http://asset.example.test/mcp" },
"whois-mcp": { baseUrl: "http://whois.example.test/mcp" },
},
}),
"utf8",
);
const report = await runSecurityAudit({
config: {
agents: {
list: [
{
id: "asset-agent",
skills: ["asset-lifecycle-tracking"],
tools: { exec: { host: "gateway", security: "full", ask: "off" } },
},
],
},
},
sourceConfig: {},
env: { OPENCLAW_STATE_DIR: stateDir },
stateDir,
includeFilesystem: false,
includeChannelSecurity: false,
});
expect(report.findings).toEqual(
expect.arrayContaining([
expect.objectContaining({
checkId: "tools.exec.agent_skill_mcp_boundary_drift",
severity: "warn",
detail: expect.stringContaining("asset-agent"),
}),
]),
);
const finding = report.findings.find(
(entry) => entry.checkId === "tools.exec.agent_skill_mcp_boundary_drift",
);
expect(finding?.detail).toContain("whois-mcp");
expect(finding?.detail).toContain("skills/config/mcporter.json");
} finally {
await fs.rm(stateDir, { recursive: true, force: true });
}
});
it("does not flag per-agent skill allowlists when matching agents deny exec", async () => {
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-audit-mcporter-deny-"));
try {
const report = await runSecurityAudit({
config: {
mcp: {
servers: {
docs: { command: "node", args: ["docs-mcp.js"] },
},
},
agents: {
defaults: { skills: ["docs-search"] },
list: [{ id: "docs-agent", tools: { exec: { security: "deny" } } }],
},
tools: { exec: { security: "deny" } },
},
sourceConfig: {},
env: { OPENCLAW_STATE_DIR: stateDir },
stateDir,
includeFilesystem: false,
includeChannelSecurity: false,
});
expect(report.findings.map((finding) => finding.checkId)).not.toContain(
"tools.exec.agent_skill_mcp_boundary_drift",
);
} finally {
await fs.rm(stateDir, { recursive: true, force: true });
}
});
it("suppresses configured accepted findings from the active audit report", async () => {
const report = await runSecurityAudit({
config: {
src/security/audit.ts
+156 -1
View File
@@ -1,4 +1,5 @@
// Orchestrates security audit collection and report formatting.
import fs from "node:fs/promises";
import path from "node:path";
import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id";
import { asNullableRecord } from "@openclaw/normalization-core/record-coerce";
@@ -21,6 +22,7 @@ import {
materializeGatewayAuthSecretRefs,
} from "../gateway/auth-config-utils.js";
import { isInterpreterLikeAllowlistPattern } from "../infra/command-analysis/inline-eval.js";
import { emitTrustedSecurityEvent } from "../infra/diagnostic-events.js";
import {
type ExecApprovalsFile,
loadExecApprovals,
@@ -34,7 +36,6 @@ import {
} from "../infra/exec-safe-bin-runtime-policy.js";
import { listRiskyConfiguredSafeBins } from "../infra/exec-safe-bin-semantics.js";
import { normalizeTrustedSafeBinDirs } from "../infra/exec-safe-bin-trust.js";
import { emitTrustedSecurityEvent } from "../infra/diagnostic-events.js";
import { DEFAULT_AGENT_ID } from "../routing/session-key.js";
import { collectDeepCodeSafetyFindings } from "./audit-deep-code-safety.js";
import { collectDeepProbeFindings } from "./audit-deep-probe-findings.js";
@@ -65,6 +66,17 @@ type ClaudePermissionModeHit = {
argSet: "args" | "resumeArgs";
mode: string;
};
type McpServerSourceSummary = {
label: string;
names: string[];
};
type AgentSkillMcpBoundaryScope = {
id: string;
skillSource: string;
execHost: string;
execSecurity: string;
execAsk: string;
};
export type {
SecurityAuditFinding,
@@ -1101,6 +1113,148 @@ export function collectExecRuntimeFindings(cfg: OpenClawConfig): SecurityAuditFi
return findings;
}
function formatNamesPreview(names: readonly string[]): string {
const visible = names.slice(0, 6);
const suffix = names.length > visible.length ? `, +${names.length - visible.length} more` : "";
return `${visible.join(", ")}${suffix}`;
}
function listConfiguredMcpServerNames(cfg: OpenClawConfig): string[] {
return Object.entries(cfg.mcp?.servers ?? {})
.filter(([, server]) => server?.enabled !== false)
.map(([name]) => name)
.toSorted();
}
async function readGlobalMcporterRegistrySummary(
stateDir: string,
): Promise<McpServerSourceSummary | null> {
const registryPath = path.join(stateDir, "skills", "config", "mcporter.json");
let parsed: unknown;
try {
parsed = JSON.parse(await fs.readFile(registryPath, "utf8")) as unknown;
} catch {
return null;
}
const mcpServers = asNullableRecord(asNullableRecord(parsed)?.mcpServers);
if (!mcpServers) {
return null;
}
const names = Object.entries(mcpServers)
.filter(([, value]) => asNullableRecord(value)?.enabled !== false)
.map(([name]) => name)
.toSorted();
return names.length > 0 ? { label: "skills/config/mcporter.json", names } : null;
}
function hasOwnSkillsAllowlist(entry: object | undefined): boolean {
return Boolean(entry && Object.hasOwn(entry, "skills"));
}
function collectAgentSkillMcpBoundaryScopes(cfg: OpenClawConfig): AgentSkillMcpBoundaryScope[] {
const agents = Array.isArray(cfg.agents?.list) ? cfg.agents.list : [];
const defaultsHaveSkillAllowlist = hasOwnSkillsAllowlist(cfg.agents?.defaults);
const candidates = [
...(defaultsHaveSkillAllowlist
? [
{
id: DEFAULT_AGENT_ID,
skillSource: "agents.defaults.skills",
agentId: undefined,
},
]
: []),
...agents
.filter(
(entry): entry is NonNullable<(typeof agents)[number]> =>
Boolean(entry) && typeof entry === "object" && typeof entry.id === "string",
)
.flatMap((entry) => {
if (hasOwnSkillsAllowlist(entry)) {
return [{ id: entry.id, skillSource: "agents.list[].skills", agentId: entry.id }];
}
if (defaultsHaveSkillAllowlist) {
return [
{
id: entry.id,
skillSource: "agents.defaults.skills (inherited)",
agentId: entry.id,
},
];
}
return [];
}),
];
return candidates.flatMap((candidate) => {
const sandboxMode = resolveSandboxConfigForAgent(cfg, candidate.agentId).mode;
const exec = resolveExecDefaults({
cfg,
agentId: candidate.agentId,
sandboxAvailable: sandboxMode !== "off",
});
if (exec.security === "deny" || exec.effectiveHost === "sandbox") {
return [];
}
return [
{
id: candidate.id,
skillSource: candidate.skillSource,
execHost: exec.effectiveHost,
execSecurity: exec.security,
execAsk: exec.ask,
},
];
});
}
export async function collectAgentSkillMcpBoundaryFindings(params: {
cfg: OpenClawConfig;
stateDir: string;
}): Promise<SecurityAuditFinding[]> {
const sources: McpServerSourceSummary[] = [];
const configServerNames = listConfiguredMcpServerNames(params.cfg);
if (configServerNames.length > 0) {
sources.push({ label: "mcp.servers", names: configServerNames });
}
const globalMcporterRegistry = await readGlobalMcporterRegistrySummary(params.stateDir);
if (globalMcporterRegistry) {
sources.push(globalMcporterRegistry);
}
if (sources.length === 0) {
return [];
}
const scopes = collectAgentSkillMcpBoundaryScopes(params.cfg);
if (scopes.length === 0) {
return [];
}
return [
{
checkId: "tools.exec.agent_skill_mcp_boundary_drift",
severity: "warn",
title: "Agent skill allowlists do not constrain host exec MCP clients",
detail:
`Detected agent skill allowlists on host-exec-capable scopes:\n${scopes
.slice(0, 8)
.map(
(scope) =>
`- ${scope.id}: ${scope.skillSource}, exec.host=${scope.execHost}, security=${scope.execSecurity}, ask=${scope.execAsk}`,
)
.join("\n")}` +
(scopes.length > 8 ? `\n- +${scopes.length - 8} more scopes.` : "") +
`\nMCP server registries visible to the gateway configuration/state:\n${sources
.map((source) => `- ${source.label}: ${formatNamesPreview(source.names)}`)
.join("\n")}\n` +
"agents.*.skills filters OpenClaw skill visibility and snapshots; it is not a shell-time authorization boundary. " +
"A host exec process can run external MCP clients or read a global mcporter registry unless sandbox, filesystem, network, or MCP credential boundaries block it.",
remediation:
'For agents that need per-agent MCP isolation, set their exec policy to security="deny" or a tight allowlist, run them in sandbox/container/OS-user isolation where the global MCP registry is not readable, split sensitive MCP servers into a separate gateway/trust boundary, or require per-agent MCP credentials at the server layer.',
},
];
}
function collectOpenExecSurfacePaths(cfg: OpenClawConfig): string[] {
const channels = asNullableRecord(cfg.channels);
if (!channels) {
@@ -1285,6 +1439,7 @@ export async function runSecurityAudit(opts: SecurityAuditOptions): Promise<Secu
findings.push(...collectLoggingFindings(cfg));
findings.push(...collectElevatedFindings(cfg));
findings.push(...collectExecRuntimeFindings(cfg));
findings.push(...(await collectAgentSkillMcpBoundaryFindings({ cfg, stateDir })));
const hooksGatewayAuthCfg = shouldMaterializeHooksGatewayAuthRefs(cfg)
? await materializeAuditGatewayAuthRefs({
cfg,