mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-05 13:00:22 +00:00
feat(security): audit workspace skill symlink escapes
This commit is contained in:
@@ -53,6 +53,8 @@ type ExecDockerRawFn = (
|
||||
) => Promise<ExecDockerRawResult>;
|
||||
|
||||
type CodeSafetySummaryCache = Map<string, Promise<unknown>>;
|
||||
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<string[]> {
|
||||
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<string>();
|
||||
|
||||
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<SecurityAuditFinding[]> {
|
||||
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<string>();
|
||||
|
||||
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;
|
||||
|
||||
@@ -35,5 +35,6 @@ export {
|
||||
collectPluginsCodeSafetyFindings,
|
||||
collectPluginsTrustFindings,
|
||||
collectStateDeepFilesystemFindings,
|
||||
collectWorkspaceSkillSymlinkEscapeFindings,
|
||||
readConfigSnapshotForAudit,
|
||||
} from "./audit-extra.async.js";
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<Secu
|
||||
findings.push(
|
||||
...(await collectStateDeepFilesystemFindings({ cfg, env, stateDir, platform, execIcacls })),
|
||||
);
|
||||
findings.push(...(await collectWorkspaceSkillSymlinkEscapeFindings({ cfg })));
|
||||
findings.push(
|
||||
...(await collectSandboxBrowserHashLabelFindings({
|
||||
execDockerRawFn: opts.execDockerRawFn,
|
||||
|
||||
Reference in New Issue
Block a user