fix(security): restrict hook transform module loading

This commit is contained in:
Peter Steinberger
2026-02-14 13:45:58 +01:00
parent 6543ce717c
commit a0361b8ba9
7 changed files with 199 additions and 39 deletions

View File

@@ -62,24 +62,28 @@ describe("hooks mapping", () => {
});
it("runs transform module", async () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-"));
const modPath = path.join(dir, "transform.mjs");
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-"));
const transformsRoot = path.join(configDir, "hooks", "transforms");
fs.mkdirSync(transformsRoot, { recursive: true });
const modPath = path.join(transformsRoot, "transform.mjs");
const placeholder = "${payload.name}";
fs.writeFileSync(
modPath,
`export default ({ payload }) => ({ kind: "wake", text: \`Ping ${placeholder}\` });`,
);
const mappings = resolveHookMappings({
transformsDir: dir,
mappings: [
{
match: { path: "custom" },
action: "agent",
transform: { module: "transform.mjs" },
},
],
});
const mappings = resolveHookMappings(
{
mappings: [
{
match: { path: "custom" },
action: "agent",
transform: { module: "transform.mjs" },
},
],
},
{ configDir },
);
const result = await applyHookMappings(mappings, {
payload: { name: "Ada" },
@@ -97,21 +101,142 @@ describe("hooks mapping", () => {
}
});
it("rejects transform module traversal outside transformsDir", () => {
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-traversal-"));
const transformsRoot = path.join(configDir, "hooks", "transforms");
fs.mkdirSync(transformsRoot, { recursive: true });
expect(() =>
resolveHookMappings(
{
mappings: [
{
match: { path: "custom" },
action: "agent",
transform: { module: "../evil.mjs" },
},
],
},
{ configDir },
),
).toThrow(/must be within/);
});
it("rejects absolute transform module path outside transformsDir", () => {
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-abs-"));
const transformsRoot = path.join(configDir, "hooks", "transforms");
fs.mkdirSync(transformsRoot, { recursive: true });
const outside = path.join(os.tmpdir(), "evil.mjs");
expect(() =>
resolveHookMappings(
{
mappings: [
{
match: { path: "custom" },
action: "agent",
transform: { module: outside },
},
],
},
{ configDir },
),
).toThrow(/must be within/);
});
it("rejects transformsDir traversal outside the transforms root", () => {
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-xformdir-trav-"));
const transformsRoot = path.join(configDir, "hooks", "transforms");
fs.mkdirSync(transformsRoot, { recursive: true });
expect(() =>
resolveHookMappings(
{
transformsDir: "..",
mappings: [
{
match: { path: "custom" },
action: "agent",
transform: { module: "transform.mjs" },
},
],
},
{ configDir },
),
).toThrow(/Hook transformsDir/);
});
it("rejects transformsDir absolute path outside the transforms root", () => {
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-xformdir-abs-"));
const transformsRoot = path.join(configDir, "hooks", "transforms");
fs.mkdirSync(transformsRoot, { recursive: true });
expect(() =>
resolveHookMappings(
{
transformsDir: os.tmpdir(),
mappings: [
{
match: { path: "custom" },
action: "agent",
transform: { module: "transform.mjs" },
},
],
},
{ configDir },
),
).toThrow(/Hook transformsDir/);
});
it("accepts transformsDir subdirectory within the transforms root", async () => {
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-xformdir-ok-"));
const transformsSubdir = path.join(configDir, "hooks", "transforms", "subdir");
fs.mkdirSync(transformsSubdir, { recursive: true });
fs.writeFileSync(path.join(transformsSubdir, "transform.mjs"), "export default () => null;");
const mappings = resolveHookMappings(
{
transformsDir: "subdir",
mappings: [
{
match: { path: "skip" },
action: "agent",
transform: { module: "transform.mjs" },
},
],
},
{ configDir },
);
const result = await applyHookMappings(mappings, {
payload: {},
headers: {},
url: new URL("http://127.0.0.1:18789/hooks/skip"),
path: "skip",
});
expect(result?.ok).toBe(true);
if (result?.ok) {
expect(result.action).toBeNull();
expect("skipped" in result).toBe(true);
}
});
it("treats null transform as a handled skip", async () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-skip-"));
const modPath = path.join(dir, "transform.mjs");
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-skip-"));
const transformsRoot = path.join(configDir, "hooks", "transforms");
fs.mkdirSync(transformsRoot, { recursive: true });
const modPath = path.join(transformsRoot, "transform.mjs");
fs.writeFileSync(modPath, "export default () => null;");
const mappings = resolveHookMappings({
transformsDir: dir,
mappings: [
{
match: { path: "skip" },
action: "agent",
transform: { module: "transform.mjs" },
},
],
});
const mappings = resolveHookMappings(
{
mappings: [
{
match: { path: "skip" },
action: "agent",
transform: { module: "transform.mjs" },
},
],
},
{ configDir },
);
const result = await applyHookMappings(mappings, {
payload: {},

View File

@@ -102,7 +102,10 @@ type HookTransformFn = (
ctx: HookMappingContext,
) => HookTransformResult | Promise<HookTransformResult>;
export function resolveHookMappings(hooks?: HooksConfig): HookMappingResolved[] {
export function resolveHookMappings(
hooks?: HooksConfig,
opts?: { configDir?: string },
): HookMappingResolved[] {
const presets = hooks?.presets ?? [];
const gmailAllowUnsafe = hooks?.gmail?.allowUnsafeExternalContent;
const mappings: HookMappingConfig[] = [];
@@ -129,10 +132,13 @@ export function resolveHookMappings(hooks?: HooksConfig): HookMappingResolved[]
return [];
}
const configDir = path.dirname(CONFIG_PATH);
const transformsDir = hooks?.transformsDir
? resolvePath(configDir, hooks.transformsDir)
: configDir;
const configDir = path.resolve(opts?.configDir ?? path.dirname(CONFIG_PATH));
const transformsRootDir = path.join(configDir, "hooks", "transforms");
const transformsDir = resolveOptionalContainedPath(
transformsRootDir,
hooks?.transformsDir,
"Hook transformsDir",
);
return mappings.map((mapping, index) => normalizeHookMapping(mapping, index, transformsDir));
}
@@ -187,7 +193,7 @@ function normalizeHookMapping(
const wakeMode = mapping.wakeMode ?? "now";
const transform = mapping.transform
? {
modulePath: resolvePath(transformsDir, mapping.transform.module),
modulePath: resolveContainedPath(transformsDir, mapping.transform.module, "Hook transform"),
exportName: mapping.transform.export?.trim() || undefined,
}
: undefined;
@@ -340,12 +346,35 @@ function resolveTransformFn(mod: Record<string, unknown>, exportName?: string):
function resolvePath(baseDir: string, target: string): string {
if (!target) {
return baseDir;
return path.resolve(baseDir);
}
if (path.isAbsolute(target)) {
return target;
return path.isAbsolute(target) ? path.resolve(target) : path.resolve(baseDir, target);
}
function resolveContainedPath(baseDir: string, target: string, label: string): string {
const base = path.resolve(baseDir);
const trimmed = target?.trim();
if (!trimmed) {
throw new Error(`${label} module path is required`);
}
return path.join(baseDir, target);
const resolved = resolvePath(base, trimmed);
const relative = path.relative(base, resolved);
if (relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) {
throw new Error(`${label} module path must be within ${base}: ${target}`);
}
return resolved;
}
function resolveOptionalContainedPath(
baseDir: string,
target: string | undefined,
label: string,
): string {
const trimmed = target?.trim();
if (!trimmed) {
return path.resolve(baseDir);
}
return resolveContainedPath(baseDir, trimmed, label);
}
function normalizeMatchPath(raw?: string): string | undefined {