fix(security): enforce plugin and hook path containment

This commit is contained in:
Peter Steinberger
2026-02-19 15:34:58 +01:00
parent 10379e7dcd
commit 81b19aaa1a
14 changed files with 387 additions and 8 deletions

View File

@@ -21,6 +21,7 @@ Docs: https://docs.openclaw.ai
- Security/Net: enforce strict dotted-decimal IPv4 literals in SSRF checks and fail closed on unsupported legacy forms (octal/hex/short/packed, for example `0177.0.0.1`, `127.1`, `2130706433`) before DNS lookup.
- Security/Discord: enforce trusted-sender guild permission checks for moderation actions (`timeout`, `kick`, `ban`) and ignore untrusted `senderUserId` params to prevent privilege escalation in tool-driven flows. Thanks @aether-ai-agent for reporting.
- Security/ACP+Exec: add `openclaw acp --token-file/--password-file` secret-file support (with inline secret flag warnings), redact ACP working-directory prefixes to `~` home-relative paths, constrain exec script preflight file inspection to the effective `workdir` boundary, and add security-audit warnings when `tools.exec.host="sandbox"` is configured while sandbox mode is off.
- Security/Plugins/Hooks: enforce runtime/package path containment with realpath checks so `openclaw.extensions`, `openclaw.hooks`, and hook handler modules cannot escape their trusted roots via traversal or symlinks.
- Security/ACP: harden ACP bridge session management with duplicate-session refresh, idle-session reaping, oldest-idle soft-cap eviction, and burst rate limiting on session creation to reduce local DoS risk without disrupting normal IDE usage.
- Security/Plugins/Hooks: add optional `--pin` for npm plugin/hook installs, persist resolved npm metadata (`name`, `version`, `spec`, integrity, shasum, timestamp), warn/confirm on integrity drift during updates, and extend `openclaw security audit` to flag unpinned specs, missing integrity metadata, and install-record version drift.
- Security/Plugins: harden plugin discovery by blocking unsafe candidates (root escapes, world-writable paths, suspicious ownership), add startup warnings when `plugins.allow` is empty with discoverable non-bundled plugins, and warn on loaded plugins without install/load-path provenance.

View File

@@ -119,6 +119,8 @@ Example `package.json`:
Each entry points to a hook directory containing `HOOK.md` and `handler.ts` (or `index.ts`).
Hook packs can ship dependencies; they will be installed under `~/.openclaw/hooks/<id>`.
Each `openclaw.hooks` entry must stay inside the package directory after symlink
resolution; entries that escape are rejected.
Security note: `openclaw hooks install` installs dependencies with `npm install --ignore-scripts`
(no lifecycle scripts). Keep hook pack dependency trees "pure JS/TS" and avoid packages that rely

View File

@@ -151,6 +151,10 @@ becomes `name/<fileBase>`.
If your plugin imports npm deps, install them in that directory so
`node_modules` is available (`npm install` / `pnpm install`).
Security guardrail: every `openclaw.extensions` entry must stay inside the plugin
directory after symlink resolution. Entries that escape the package directory are
rejected.
Security note: `openclaw plugins install` installs plugin dependencies with
`npm install --ignore-scripts` (no lifecycle scripts). Keep plugin dependency
trees "pure JS/TS" and avoid packages that require `postinstall` builds.

View File

@@ -238,6 +238,74 @@ describe("installHooksFromPath", () => {
expect(result.targetDir).toBe(path.join(stateDir, "hooks", "my-hook"));
expect(fs.existsSync(path.join(result.targetDir, "HOOK.md"))).toBe(true);
});
it("rejects hook pack entries that traverse outside package directory", async () => {
const stateDir = makeTempDir();
const workDir = makeTempDir();
const pkgDir = path.join(workDir, "package");
const outsideHookDir = path.join(workDir, "outside");
fs.mkdirSync(pkgDir, { recursive: true });
fs.mkdirSync(outsideHookDir, { recursive: true });
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify({
name: "@openclaw/test-hooks",
version: "0.0.1",
openclaw: { hooks: ["../outside"] },
}),
"utf-8",
);
fs.writeFileSync(path.join(outsideHookDir, "HOOK.md"), "---\nname: outside\n---\n", "utf-8");
fs.writeFileSync(path.join(outsideHookDir, "handler.ts"), "export default async () => {};\n");
const result = await installHooksFromPath({
path: pkgDir,
hooksDir: path.join(stateDir, "hooks"),
});
expect(result.ok).toBe(false);
if (result.ok) {
return;
}
expect(result.error).toContain("openclaw.hooks entry escapes package directory");
});
it("rejects hook pack entries that escape via symlink", async () => {
const stateDir = makeTempDir();
const workDir = makeTempDir();
const pkgDir = path.join(workDir, "package");
const outsideHookDir = path.join(workDir, "outside");
const linkedDir = path.join(pkgDir, "linked");
fs.mkdirSync(pkgDir, { recursive: true });
fs.mkdirSync(outsideHookDir, { recursive: true });
fs.writeFileSync(path.join(outsideHookDir, "HOOK.md"), "---\nname: outside\n---\n", "utf-8");
fs.writeFileSync(path.join(outsideHookDir, "handler.ts"), "export default async () => {};\n");
try {
fs.symlinkSync(outsideHookDir, linkedDir, process.platform === "win32" ? "junction" : "dir");
} catch {
return;
}
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify({
name: "@openclaw/test-hooks",
version: "0.0.1",
openclaw: { hooks: ["./linked"] },
}),
"utf-8",
);
const result = await installHooksFromPath({
path: pkgDir,
hooksDir: path.join(stateDir, "hooks"),
});
expect(result.ok).toBe(false);
if (result.ok) {
return;
}
expect(result.error).toContain("openclaw.hooks entry resolves outside package directory");
});
});
describe("installHooksFromNpmSpec", () => {

View File

@@ -18,6 +18,7 @@ import {
withTempDir,
} from "../infra/install-source-utils.js";
import { validateRegistryNpmSpec } from "../infra/npm-registry-spec.js";
import { isPathInside, isPathInsideWithRealpath } from "../security/scan-paths.js";
import { CONFIG_DIR, resolveUserPath } from "../utils.js";
import { parseFrontmatter } from "./frontmatter.js";
@@ -218,7 +219,23 @@ async function installHookPackageFromDir(params: {
const resolvedHooks = [] as string[];
for (const entry of hookEntries) {
const hookDir = path.resolve(params.packageDir, entry);
if (!isPathInside(params.packageDir, hookDir)) {
return {
ok: false,
error: `openclaw.hooks entry escapes package directory: ${entry}`,
};
}
await validateHookDir(hookDir);
if (
!isPathInsideWithRealpath(params.packageDir, hookDir, {
requireRealpath: true,
})
) {
return {
ok: false,
error: `openclaw.hooks entry resolves outside package directory: ${entry}`,
};
}
const hookName = await resolveHookNameFromDir(hookDir);
resolvedHooks.push(hookName);
}

View File

@@ -266,5 +266,73 @@ describe("loader", () => {
// This test mainly verifies that loading and triggering doesn't crash
expect(getRegisteredEventKeys()).toContain("command:new");
});
it("rejects directory hook handlers that escape hook dir via symlink", async () => {
const outsideHandlerPath = path.join(fixtureRoot, `outside-handler-${caseId}.js`);
await fs.writeFile(outsideHandlerPath, "export default async function() {}", "utf-8");
const hookDir = path.join(tmpDir, "hooks", "symlink-hook");
await fs.mkdir(hookDir, { recursive: true });
await fs.writeFile(
path.join(hookDir, "HOOK.md"),
[
"---",
"name: symlink-hook",
"description: symlink test",
'metadata: {"openclaw":{"events":["command:new"]}}',
"---",
"",
"# Symlink Hook",
].join("\n"),
"utf-8",
);
try {
await fs.symlink(outsideHandlerPath, path.join(hookDir, "handler.js"));
} catch {
return;
}
const cfg: OpenClawConfig = {
hooks: {
internal: {
enabled: true,
},
},
};
const count = await loadInternalHooks(cfg, tmpDir);
expect(count).toBe(0);
expect(getRegisteredEventKeys()).not.toContain("command:new");
});
it("rejects legacy handler modules that escape workspace via symlink", async () => {
const outsideHandlerPath = path.join(fixtureRoot, `outside-legacy-${caseId}.js`);
await fs.writeFile(outsideHandlerPath, "export default async function() {}", "utf-8");
const linkedHandlerPath = path.join(tmpDir, "legacy-handler.js");
try {
await fs.symlink(outsideHandlerPath, linkedHandlerPath);
} catch {
return;
}
const cfg: OpenClawConfig = {
hooks: {
internal: {
enabled: true,
handlers: [
{
event: "command:new",
module: "legacy-handler.js",
},
],
},
},
};
const count = await loadInternalHooks(cfg, tmpDir);
expect(count).toBe(0);
expect(getRegisteredEventKeys()).not.toContain("command:new");
});
});
});

View File

@@ -9,6 +9,7 @@ import path from "node:path";
import { pathToFileURL } from "node:url";
import type { OpenClawConfig } from "../config/config.js";
import { createSubsystemLogger } from "../logging/subsystem.js";
import { isPathInsideWithRealpath } from "../security/scan-paths.js";
import { resolveHookConfig } from "./config.js";
import { shouldIncludeHook } from "./config.js";
import type { InternalHookHandler } from "./internal-hooks.js";
@@ -71,6 +72,16 @@ export async function loadInternalHooks(
}
try {
if (
!isPathInsideWithRealpath(entry.hook.baseDir, entry.hook.handlerPath, {
requireRealpath: true,
})
) {
log.error(
`Hook '${entry.hook.name}' handler path resolves outside hook directory: ${entry.hook.handlerPath}`,
);
continue;
}
// Import handler module with cache-busting
const url = pathToFileURL(entry.hook.handlerPath).href;
const cacheBustedUrl = `${url}?t=${Date.now()}`;
@@ -135,6 +146,16 @@ export async function loadInternalHooks(
log.error(`Handler module path must stay within workspaceDir: ${rawModule}`);
continue;
}
if (
!isPathInsideWithRealpath(baseDir, modulePath, {
requireRealpath: true,
})
) {
log.error(
`Handler module path resolves outside workspaceDir after symlink resolution: ${rawModule}`,
);
continue;
}
// Import the module with cache-busting to ensure fresh reload
const url = pathToFileURL(modulePath).href;

View File

@@ -66,4 +66,40 @@ describe("hooks workspace", () => {
const entries = loadHookEntriesFromDir({ dir: hooksRoot, source: "openclaw-workspace" });
expect(entries.some((e) => e.hook.name === "nested")).toBe(true);
});
it("ignores package.json hook paths that escape via symlink", () => {
const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-workspace-link-"));
const hooksRoot = path.join(root, "hooks");
fs.mkdirSync(hooksRoot, { recursive: true });
const pkgDir = path.join(hooksRoot, "pkg");
const outsideDir = path.join(root, "outside");
const linkedDir = path.join(pkgDir, "linked");
fs.mkdirSync(pkgDir, { recursive: true });
fs.mkdirSync(outsideDir, { recursive: true });
fs.writeFileSync(path.join(outsideDir, "HOOK.md"), "---\nname: outside\n---\n");
fs.writeFileSync(path.join(outsideDir, "handler.js"), "export default async () => {};\n");
try {
fs.symlinkSync(outsideDir, linkedDir, process.platform === "win32" ? "junction" : "dir");
} catch {
return;
}
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify(
{
name: "pkg",
[MANIFEST_KEY]: {
hooks: ["./linked"],
},
},
null,
2,
),
);
const entries = loadHookEntriesFromDir({ dir: hooksRoot, source: "openclaw-workspace" });
expect(entries.some((e) => e.hook.name === "outside")).toBe(false);
});
});

View File

@@ -2,6 +2,7 @@ import fs from "node:fs";
import path from "node:path";
import { MANIFEST_KEY } from "../compat/legacy-names.js";
import type { OpenClawConfig } from "../config/config.js";
import { isPathInsideWithRealpath } from "../security/scan-paths.js";
import { CONFIG_DIR, resolveUserPath } from "../utils.js";
import { resolveBundledHooksDir } from "./bundled-dir.js";
import { shouldIncludeHook } from "./config.js";
@@ -55,8 +56,11 @@ function resolvePackageHooks(manifest: HookPackageManifest): string[] {
function resolveContainedDir(baseDir: string, targetDir: string): string | null {
const base = path.resolve(baseDir);
const resolved = path.resolve(baseDir, targetDir);
const relative = path.relative(base, resolved);
if (relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) {
if (
!isPathInsideWithRealpath(base, resolved, {
requireRealpath: true,
})
) {
return null;
}
return resolved;

View File

@@ -149,8 +149,7 @@ describe("discoverOpenClawPlugins", () => {
const ids = candidates.map((c) => c.idHint);
expect(ids).toContain("demo-plugin-dir");
});
it("blocks extension entries that escape plugin root", async () => {
it("blocks extension entries that escape package directory", async () => {
const stateDir = makeTempDir();
const globalExt = path.join(stateDir, "extensions", "escape-pack");
const outside = path.join(stateDir, "outside.js");
@@ -172,10 +171,43 @@ describe("discoverOpenClawPlugins", () => {
expect(result.candidates).toHaveLength(0);
expect(
result.diagnostics.some((diag) => diag.message.includes("source escapes plugin root")),
result.diagnostics.some((diag) => diag.message.includes("escapes package directory")),
).toBe(true);
});
it("rejects package extension entries that escape via symlink", async () => {
const stateDir = makeTempDir();
const globalExt = path.join(stateDir, "extensions", "pack");
const outsideDir = path.join(stateDir, "outside");
const linkedDir = path.join(globalExt, "linked");
fs.mkdirSync(globalExt, { recursive: true });
fs.mkdirSync(outsideDir, { recursive: true });
fs.writeFileSync(path.join(outsideDir, "escape.ts"), "export default {}", "utf-8");
try {
fs.symlinkSync(outsideDir, linkedDir, process.platform === "win32" ? "junction" : "dir");
} catch {
return;
}
fs.writeFileSync(
path.join(globalExt, "package.json"),
JSON.stringify({
name: "@openclaw/pack",
openclaw: { extensions: ["./linked/escape.ts"] },
}),
"utf-8",
);
const { candidates, diagnostics } = await withStateDir(stateDir, async () => {
return discoverOpenClawPlugins({});
});
expect(candidates.some((candidate) => candidate.idHint === "pack")).toBe(false);
expect(diagnostics.some((entry) => entry.message.includes("escapes package directory"))).toBe(
true,
);
});
it.runIf(process.platform !== "win32")("blocks world-writable plugin paths", async () => {
const stateDir = makeTempDir();
const globalExt = path.join(stateDir, "extensions");

View File

@@ -1,5 +1,6 @@
import fs from "node:fs";
import path from "node:path";
import { isPathInsideWithRealpath } from "../security/scan-paths.js";
import { resolveConfigDir, resolveUserPath } from "../utils.js";
import { resolveBundledPluginsDir } from "./bundled-dir.js";
import {
@@ -294,6 +295,28 @@ function addCandidate(params: {
});
}
function resolvePackageEntrySource(params: {
packageDir: string;
entryPath: string;
sourceLabel: string;
diagnostics: PluginDiagnostic[];
}): string | null {
const source = path.resolve(params.packageDir, params.entryPath);
if (
!isPathInsideWithRealpath(params.packageDir, source, {
requireRealpath: true,
})
) {
params.diagnostics.push({
level: "error",
message: `extension entry escapes package directory: ${params.entryPath}`,
source: params.sourceLabel,
});
return null;
}
return source;
}
function discoverInDirectory(params: {
dir: string;
origin: PluginOrigin;
@@ -345,7 +368,15 @@ function discoverInDirectory(params: {
if (extensions.length > 0) {
for (const extPath of extensions) {
const resolved = path.resolve(fullPath, extPath);
const resolved = resolvePackageEntrySource({
packageDir: fullPath,
entryPath: extPath,
sourceLabel: fullPath,
diagnostics: params.diagnostics,
});
if (!resolved) {
continue;
}
addCandidate({
candidates: params.candidates,
diagnostics: params.diagnostics,
@@ -438,7 +469,15 @@ function discoverFromPath(params: {
if (extensions.length > 0) {
for (const extPath of extensions) {
const source = path.resolve(resolved, extPath);
const source = resolvePackageEntrySource({
packageDir: resolved,
entryPath: extPath,
sourceLabel: resolved,
diagnostics: params.diagnostics,
});
if (!source) {
continue;
}
addCandidate({
candidates: params.candidates,
diagnostics: params.diagnostics,

View File

@@ -489,7 +489,6 @@ describe("loadOpenClawPlugins", () => {
expect(loaded?.origin).toBe("config");
expect(overridden?.origin).toBe("bundled");
});
it("warns when plugins.allow is empty and non-bundled plugins are discoverable", () => {
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins";
const plugin = writePlugin({
@@ -561,4 +560,48 @@ describe("loadOpenClawPlugins", () => {
}
}
});
it("rejects plugin entry files that escape plugin root via symlink", () => {
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins";
const pluginDir = makeTempDir();
const outsideDir = makeTempDir();
const outsideEntry = path.join(outsideDir, "outside.js");
const linkedEntry = path.join(pluginDir, "entry.js");
fs.writeFileSync(
outsideEntry,
'export default { id: "symlinked", register() { throw new Error("should not run"); } };',
"utf-8",
);
fs.writeFileSync(
path.join(pluginDir, "openclaw.plugin.json"),
JSON.stringify(
{
id: "symlinked",
configSchema: EMPTY_PLUGIN_SCHEMA,
},
null,
2,
),
"utf-8",
);
try {
fs.symlinkSync(outsideEntry, linkedEntry);
} catch {
return;
}
const registry = loadOpenClawPlugins({
cache: false,
config: {
plugins: {
load: { paths: [linkedEntry] },
allow: ["symlinked"],
},
},
});
const record = registry.plugins.find((entry) => entry.id === "symlinked");
expect(record?.status).not.toBe("loaded");
expect(registry.diagnostics.some((entry) => entry.message.includes("escapes"))).toBe(true);
});
});

View File

@@ -5,6 +5,7 @@ import { createJiti } from "jiti";
import type { OpenClawConfig } from "../config/config.js";
import type { GatewayRequestHandler } from "../gateway/server-methods/types.js";
import { createSubsystemLogger } from "../logging/subsystem.js";
import { isPathInsideWithRealpath } from "../security/scan-paths.js";
import { resolveUserPath } from "../utils.js";
import { clearPluginCommands } from "./commands.js";
import {
@@ -485,6 +486,24 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
continue;
}
if (
!isPathInsideWithRealpath(candidate.rootDir, candidate.source, {
requireRealpath: true,
})
) {
record.status = "error";
record.error = "plugin entry path escapes plugin root";
registry.plugins.push(record);
seenIds.set(pluginId, candidate.origin);
registry.diagnostics.push({
level: "error",
pluginId: record.id,
source: record.source,
message: record.error,
});
continue;
}
let mod: OpenClawPluginModule | null = null;
try {
mod = getJiti()(candidate.source) as OpenClawPluginModule;

View File

@@ -1,3 +1,4 @@
import fs from "node:fs";
import path from "node:path";
export function isPathInside(basePath: string, candidatePath: string): boolean {
@@ -7,6 +8,30 @@ export function isPathInside(basePath: string, candidatePath: string): boolean {
return rel === "" || (!rel.startsWith(`..${path.sep}`) && rel !== ".." && !path.isAbsolute(rel));
}
function safeRealpathSync(filePath: string): string | null {
try {
return fs.realpathSync(filePath);
} catch {
return null;
}
}
export function isPathInsideWithRealpath(
basePath: string,
candidatePath: string,
opts?: { requireRealpath?: boolean },
): boolean {
if (!isPathInside(basePath, candidatePath)) {
return false;
}
const baseReal = safeRealpathSync(basePath);
const candidateReal = safeRealpathSync(candidatePath);
if (!baseReal || !candidateReal) {
return opts?.requireRealpath !== true;
}
return isPathInside(baseReal, candidateReal);
}
export function extensionUsesSkippedScannerPath(entry: string): boolean {
const segments = entry.split(/[\\/]+/).filter(Boolean);
return segments.some(