From f4dd0577b055f77af783105bd65eae32f3d5e6a1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 10:17:30 +0100 Subject: [PATCH] fix(security): block hook transform symlink escapes --- CHANGELOG.md | 1 + src/gateway/hooks-mapping.test.ts | 86 +++++++++++++++++++++++++++++++ src/gateway/hooks-mapping.ts | 45 +++++++++++++++- 3 files changed, 130 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91fdfe6e7..69cac1565 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai - Security/Hooks auth: normalize hook auth rate-limit client IP keys so IPv4 and IPv4-mapped IPv6 addresses share one throttle bucket, preventing dual-form auth-attempt budget bypasses. This ships in the next npm release. Thanks @aether-ai-agent for reporting. - Security/Exec approvals: treat `env` and shell-dispatch wrappers as transparent during allowlist analysis on node-host and macOS companion paths so policy checks match the effective executable/inline shell payload instead of the wrapper binary, blocking wrapper-smuggled allowlist bypasses. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Channels: harden Slack external menu token handling by switching to CSPRNG tokens, validating token shape, requiring user identity for external option lookups, and avoiding fabricated timestamp `trigger_id` fallbacks; also switch Tlon Urbit channel IDs to CSPRNG UUIDs, centralize secure ID/token generation via shared infra helpers, and add a guardrail test to block new runtime `Date.now()+Math.random()` token/id patterns. +- Security/Hooks transforms: enforce symlink-safe containment for webhook transform module paths (including `hooks.transformsDir` and `hooks.mappings[].transform.module`) by resolving existing-path ancestors via realpath before import, while preserving in-root symlink support; add regression coverage for both escape and allow cases. This ships in the next npm release. Thanks @aether-ai-agent for reporting. - Telegram/WSL2: disable `autoSelectFamily` by default on WSL2 and memoize WSL2 detection in Telegram network decision logic to avoid repeated sync `/proc/version` probes on fetch/send paths. (#21916) Thanks @MizukiMachine. - Telegram/Streaming: preserve archived draft preview mapping after flush and clean superseded reasoning preview bubbles so multi-message preview finals no longer cross-edit or orphan stale messages under send/rotation races. (#23202) Thanks @obviyus. - Slack/Slash commands: preserve the Bolt app receiver when registering external select options handlers so monitor startup does not crash on runtimes that require bound `app.options` calls. (#23209) Thanks @0xgaia. diff --git a/src/gateway/hooks-mapping.test.ts b/src/gateway/hooks-mapping.test.ts index 05554d7ca..74bb2301d 100644 --- a/src/gateway/hooks-mapping.test.ts +++ b/src/gateway/hooks-mapping.test.ts @@ -240,6 +240,92 @@ describe("hooks mapping", () => { const result = await applyNullTransformFromTempConfig({ configDir, transformsDir: "subdir" }); expectSkippedTransformResult(result); }); + + it.runIf(process.platform !== "win32")( + "rejects transform module symlink escape outside transformsDir", + () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-symlink-module-")); + const transformsRoot = path.join(configDir, "hooks", "transforms"); + fs.mkdirSync(transformsRoot, { recursive: true }); + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-outside-module-")); + const outsideModule = path.join(outsideDir, "evil.mjs"); + fs.writeFileSync(outsideModule, 'export default () => ({ kind: "wake", text: "owned" });'); + fs.symlinkSync(outsideModule, path.join(transformsRoot, "linked.mjs")); + expect(() => + resolveHookMappings( + { + mappings: [ + { + match: { path: "custom" }, + action: "agent", + transform: { module: "linked.mjs" }, + }, + ], + }, + { configDir }, + ), + ).toThrow(/must be within/); + }, + ); + + it.runIf(process.platform !== "win32")( + "rejects transformsDir symlink escape outside transforms root", + () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-symlink-dir-")); + const transformsRoot = path.join(configDir, "hooks", "transforms"); + fs.mkdirSync(transformsRoot, { recursive: true }); + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-outside-dir-")); + fs.writeFileSync(path.join(outsideDir, "transform.mjs"), "export default () => null;"); + fs.symlinkSync(outsideDir, path.join(transformsRoot, "escape"), "dir"); + expect(() => + resolveHookMappings( + { + transformsDir: "escape", + mappings: [ + { + match: { path: "custom" }, + action: "agent", + transform: { module: "transform.mjs" }, + }, + ], + }, + { configDir }, + ), + ).toThrow(/Hook transformsDir/); + }, + ); + + it.runIf(process.platform !== "win32")("accepts in-root transform module symlink", async () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-symlink-ok-")); + const transformsRoot = path.join(configDir, "hooks", "transforms"); + const nestedDir = path.join(transformsRoot, "nested"); + fs.mkdirSync(nestedDir, { recursive: true }); + fs.writeFileSync(path.join(nestedDir, "transform.mjs"), "export default () => null;"); + fs.symlinkSync(path.join(nestedDir, "transform.mjs"), path.join(transformsRoot, "linked.mjs")); + + const mappings = resolveHookMappings( + { + mappings: [ + { + match: { path: "skip" }, + action: "agent", + transform: { module: "linked.mjs" }, + }, + ], + }, + { configDir }, + ); + + const result = await applyHookMappings(mappings, { + payload: {}, + headers: {}, + url: new URL("http://127.0.0.1:18789/hooks/skip"), + path: "skip", + }); + + expectSkippedTransformResult(result); + }); + it("treats null transform as a handled skip", async () => { const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-skip-")); const result = await applyNullTransformFromTempConfig({ configDir }); diff --git a/src/gateway/hooks-mapping.ts b/src/gateway/hooks-mapping.ts index 20c3a76cc..ec8557d37 100644 --- a/src/gateway/hooks-mapping.ts +++ b/src/gateway/hooks-mapping.ts @@ -1,3 +1,4 @@ +import fs from "node:fs"; import path from "node:path"; import { CONFIG_PATH, type HookMappingConfig, type HooksConfig } from "../config/config.js"; import { importFileModule, resolveFunctionModuleExport } from "../hooks/module-loader.js"; @@ -355,6 +356,34 @@ function resolvePath(baseDir: string, target: string): string { return path.isAbsolute(target) ? path.resolve(target) : path.resolve(baseDir, target); } +function escapesBase(baseDir: string, candidate: string): boolean { + const relative = path.relative(baseDir, candidate); + return relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative); +} + +function safeRealpathSync(candidate: string): string | null { + try { + const nativeRealpath = fs.realpathSync.native as ((path: string) => string) | undefined; + return nativeRealpath ? nativeRealpath(candidate) : fs.realpathSync(candidate); + } catch { + return null; + } +} + +function resolveExistingAncestor(candidate: string): string | null { + let current = path.resolve(candidate); + while (true) { + if (fs.existsSync(current)) { + return current; + } + const parent = path.dirname(current); + if (parent === current) { + return null; + } + current = parent; + } +} + function resolveContainedPath(baseDir: string, target: string, label: string): string { const base = path.resolve(baseDir); const trimmed = target?.trim(); @@ -362,8 +391,20 @@ function resolveContainedPath(baseDir: string, target: string, label: string): s throw new Error(`${label} module path is required`); } const resolved = resolvePath(base, trimmed); - const relative = path.relative(base, resolved); - if (relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) { + if (escapesBase(base, resolved)) { + throw new Error(`${label} module path must be within ${base}: ${target}`); + } + + // Block symlink escapes for existing path segments while preserving current + // behavior for not-yet-created files. + const baseRealpath = safeRealpathSync(base); + const existingAncestor = resolveExistingAncestor(resolved); + const existingAncestorRealpath = existingAncestor ? safeRealpathSync(existingAncestor) : null; + if ( + baseRealpath && + existingAncestorRealpath && + escapesBase(baseRealpath, existingAncestorRealpath) + ) { throw new Error(`${label} module path must be within ${base}: ${target}`); } return resolved;