mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-08 06:54:24 +00:00
fix(security): block hook manifest path escapes
This commit is contained in:
@@ -6,7 +6,8 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Security/Hooks: restrict hook transform modules to `~/.openclaw/hooks/transforms` (prevents path traversal/escape module loads via config).
|
||||
- Security/Hooks: restrict hook transform modules to `~/.openclaw/hooks/transforms` (prevents path traversal/escape module loads via config). Thanks @akhmittra.
|
||||
- Security/Hooks: ignore hook package manifest entries that point outside the package directory (prevents out-of-tree handler loads during hook discovery).
|
||||
|
||||
## 2026.2.13
|
||||
|
||||
|
||||
69
src/hooks/workspace.test.ts
Normal file
69
src/hooks/workspace.test.ts
Normal file
@@ -0,0 +1,69 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { MANIFEST_KEY } from "../compat/legacy-names.js";
|
||||
import { loadHookEntriesFromDir } from "./workspace.js";
|
||||
|
||||
describe("hooks workspace", () => {
|
||||
it("ignores package.json hook paths that traverse outside package directory", () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-workspace-"));
|
||||
const hooksRoot = path.join(root, "hooks");
|
||||
fs.mkdirSync(hooksRoot, { recursive: true });
|
||||
|
||||
const pkgDir = path.join(hooksRoot, "pkg");
|
||||
fs.mkdirSync(pkgDir, { recursive: true });
|
||||
|
||||
const outsideHookDir = path.join(root, "outside");
|
||||
fs.mkdirSync(outsideHookDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(outsideHookDir, "HOOK.md"), "---\nname: outside\n---\n");
|
||||
fs.writeFileSync(path.join(outsideHookDir, "handler.js"), "export default async () => {};\n");
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(pkgDir, "package.json"),
|
||||
JSON.stringify(
|
||||
{
|
||||
name: "pkg",
|
||||
[MANIFEST_KEY]: {
|
||||
hooks: ["../outside"],
|
||||
},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
|
||||
const entries = loadHookEntriesFromDir({ dir: hooksRoot, source: "openclaw-workspace" });
|
||||
expect(entries.some((e) => e.hook.name === "outside")).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts package.json hook paths within package directory", () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-workspace-ok-"));
|
||||
const hooksRoot = path.join(root, "hooks");
|
||||
fs.mkdirSync(hooksRoot, { recursive: true });
|
||||
|
||||
const pkgDir = path.join(hooksRoot, "pkg");
|
||||
const nested = path.join(pkgDir, "nested");
|
||||
fs.mkdirSync(nested, { recursive: true });
|
||||
|
||||
fs.writeFileSync(path.join(nested, "HOOK.md"), "---\nname: nested\n---\n");
|
||||
fs.writeFileSync(path.join(nested, "handler.js"), "export default async () => {};\n");
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(pkgDir, "package.json"),
|
||||
JSON.stringify(
|
||||
{
|
||||
name: "pkg",
|
||||
[MANIFEST_KEY]: {
|
||||
hooks: ["./nested"],
|
||||
},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
|
||||
const entries = loadHookEntriesFromDir({ dir: hooksRoot, source: "openclaw-workspace" });
|
||||
expect(entries.some((e) => e.hook.name === "nested")).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -52,6 +52,16 @@ function resolvePackageHooks(manifest: HookPackageManifest): string[] {
|
||||
return raw.map((entry) => (typeof entry === "string" ? entry.trim() : "")).filter(Boolean);
|
||||
}
|
||||
|
||||
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)) {
|
||||
return null;
|
||||
}
|
||||
return resolved;
|
||||
}
|
||||
|
||||
function loadHookFromDir(params: {
|
||||
hookDir: string;
|
||||
source: HookSource;
|
||||
@@ -129,7 +139,13 @@ function loadHooksFromDir(params: { dir: string; source: HookSource; pluginId?:
|
||||
|
||||
if (packageHooks.length > 0) {
|
||||
for (const hookPath of packageHooks) {
|
||||
const resolvedHookDir = path.resolve(hookDir, hookPath);
|
||||
const resolvedHookDir = resolveContainedDir(hookDir, hookPath);
|
||||
if (!resolvedHookDir) {
|
||||
console.warn(
|
||||
`[hooks] Ignoring out-of-package hook path "${hookPath}" in ${hookDir} (must be within package directory)`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
const hook = loadHookFromDir({
|
||||
hookDir: resolvedHookDir,
|
||||
source,
|
||||
|
||||
Reference in New Issue
Block a user