From a0361b8ba959e8506dc79d638b6e6a00d12887e4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 14 Feb 2026 13:45:58 +0100 Subject: [PATCH] fix(security): restrict hook transform module loading --- CHANGELOG.md | 6 + docs/automation/gmail-pubsub.md | 2 +- docs/automation/webhook.md | 2 +- docs/gateway/configuration-examples.md | 4 +- docs/gateway/configuration-reference.md | 2 +- src/gateway/hooks-mapping.test.ts | 173 ++++++++++++++++++++---- src/gateway/hooks-mapping.ts | 49 +++++-- 7 files changed, 199 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf42377c9..9c6736dce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ Docs: https://docs.openclaw.ai +## Unreleased + +### Fixes + +- Security/Hooks: restrict hook transform modules to `~/.openclaw/hooks/transforms` (prevents path traversal/escape module loads via config). + ## 2026.2.13 ### Changes diff --git a/docs/automation/gmail-pubsub.md b/docs/automation/gmail-pubsub.md index 734ae6f77..b853b9955 100644 --- a/docs/automation/gmail-pubsub.md +++ b/docs/automation/gmail-pubsub.md @@ -88,7 +88,7 @@ Notes: To disable (dangerous), set `hooks.gmail.allowUnsafeExternalContent: true`. To customize payload handling further, add `hooks.mappings` or a JS/TS transform module -under `hooks.transformsDir` (see [Webhooks](/automation/webhook)). +under `~/.openclaw/hooks/transforms` (see [Webhooks](/automation/webhook)). ## Wizard (recommended) diff --git a/docs/automation/webhook.md b/docs/automation/webhook.md index 30556ee0c..56aae528f 100644 --- a/docs/automation/webhook.md +++ b/docs/automation/webhook.md @@ -139,7 +139,7 @@ Mapping options (summary): - `hooks.presets: ["gmail"]` enables the built-in Gmail mapping. - `hooks.mappings` lets you define `match`, `action`, and templates in config. -- `hooks.transformsDir` + `transform.module` loads a JS/TS module for custom logic. +- `hooks.transformsDir` + `transform.module` loads a JS/TS module for custom logic (restricted to `~/.openclaw/hooks/transforms`). - Use `match.source` to keep a generic ingest endpoint (payload-driven routing). - TS transforms require a TS loader (e.g. `bun` or `tsx`) or precompiled `.js` at runtime. - Set `deliver: true` + `channel`/`to` on mappings to route replies to a chat surface diff --git a/docs/gateway/configuration-examples.md b/docs/gateway/configuration-examples.md index ca77eef13..960f37c00 100644 --- a/docs/gateway/configuration-examples.md +++ b/docs/gateway/configuration-examples.md @@ -363,7 +363,7 @@ Save to `~/.openclaw/openclaw.json` and you can DM the bot from that number. path: "/hooks", token: "shared-secret", presets: ["gmail"], - transformsDir: "~/.openclaw/hooks", + transformsDir: "~/.openclaw/hooks/transforms", mappings: [ { id: "gmail-hook", @@ -380,7 +380,7 @@ Save to `~/.openclaw/openclaw.json` and you can DM the bot from that number. thinking: "low", timeoutSeconds: 300, transform: { - module: "./transforms/gmail.js", + module: "gmail.js", export: "transformGmail", }, }, diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 0655cb386..4f3a33980 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -1987,7 +1987,7 @@ See [Multiple Gateways](/gateway/multiple-gateways). allowedSessionKeyPrefixes: ["hook:"], allowedAgentIds: ["hooks", "main"], presets: ["gmail"], - transformsDir: "~/.openclaw/hooks", + transformsDir: "~/.openclaw/hooks/transforms", mappings: [ { match: { path: "gmail" }, diff --git a/src/gateway/hooks-mapping.test.ts b/src/gateway/hooks-mapping.test.ts index 3666b850f..3eeb0adc7 100644 --- a/src/gateway/hooks-mapping.test.ts +++ b/src/gateway/hooks-mapping.test.ts @@ -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: {}, diff --git a/src/gateway/hooks-mapping.ts b/src/gateway/hooks-mapping.ts index f3e3ccb62..e7236e9f9 100644 --- a/src/gateway/hooks-mapping.ts +++ b/src/gateway/hooks-mapping.ts @@ -102,7 +102,10 @@ type HookTransformFn = ( ctx: HookMappingContext, ) => HookTransformResult | Promise; -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, 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 {