diff --git a/CHANGELOG.md b/CHANGELOG.md index bd443141b40..14ec6e4ecb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,6 @@ Docs: https://docs.openclaw.ai - Security/Hooks: restrict hook transform modules to `~/.openclaw/hooks/transforms` (prevents path traversal/escape module loads via config). Config note: `hooks.transformsDir` must now be within that directory. Thanks @akhmittra. - Security/Hooks: ignore hook package manifest entries that point outside the package directory (prevents out-of-tree handler loads during hook discovery). - Ollama/Agents: avoid forcing `` tag enforcement for Ollama models, which could suppress all output as `(no output)`. (#16191) Thanks @Glucksberg. - ## 2026.2.13 ### Changes diff --git a/docs/automation/hooks.md b/docs/automation/hooks.md index f17865f6b72..ffdf32ab79b 100644 --- a/docs/automation/hooks.md +++ b/docs/automation/hooks.md @@ -400,6 +400,8 @@ The old config format still works for backwards compatibility: } ``` +Note: `module` must be a workspace-relative path. Absolute paths and traversal outside the workspace are rejected. + **Migration**: Use the new discovery-based system for new hooks. Legacy handlers are loaded after directory-based hooks. ## CLI Commands diff --git a/docs/automation/webhook.md b/docs/automation/webhook.md index 56aae528f5e..8072b4a1a3f 100644 --- a/docs/automation/webhook.md +++ b/docs/automation/webhook.md @@ -139,7 +139,9 @@ 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 (restricted to `~/.openclaw/hooks/transforms`). +- `hooks.transformsDir` + `transform.module` loads a JS/TS module for custom logic. + - `hooks.transformsDir` (if set) must stay within the transforms root under your OpenClaw config directory (typically `~/.openclaw/hooks/transforms`). + - `transform.module` must resolve within the effective transforms directory (traversal/escape paths are rejected). - 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/src/config/config.hooks-module-paths.test.ts b/src/config/config.hooks-module-paths.test.ts new file mode 100644 index 00000000000..57d949d7219 --- /dev/null +++ b/src/config/config.hooks-module-paths.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; +import { validateConfigObjectWithPlugins } from "./config.js"; + +describe("config hooks module paths", () => { + it("rejects absolute hooks.mappings[].transform.module", () => { + const res = validateConfigObjectWithPlugins({ + agents: { list: [{ id: "pi" }] }, + hooks: { + mappings: [ + { + match: { path: "custom" }, + action: "agent", + transform: { module: "/tmp/transform.mjs" }, + }, + ], + }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((iss) => iss.path === "hooks.mappings.0.transform.module")).toBe(true); + } + }); + + it("rejects escaping hooks.mappings[].transform.module", () => { + const res = validateConfigObjectWithPlugins({ + agents: { list: [{ id: "pi" }] }, + hooks: { + mappings: [ + { + match: { path: "custom" }, + action: "agent", + transform: { module: "../escape.mjs" }, + }, + ], + }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((iss) => iss.path === "hooks.mappings.0.transform.module")).toBe(true); + } + }); + + it("rejects absolute hooks.internal.handlers[].module", () => { + const res = validateConfigObjectWithPlugins({ + agents: { list: [{ id: "pi" }] }, + hooks: { + internal: { + enabled: true, + handlers: [{ event: "command:new", module: "/tmp/handler.mjs" }], + }, + }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((iss) => iss.path === "hooks.internal.handlers.0.module")).toBe(true); + } + }); +}); diff --git a/src/config/types.hooks.ts b/src/config/types.hooks.ts index 13e765e0f9d..3e13931cd60 100644 --- a/src/config/types.hooks.ts +++ b/src/config/types.hooks.ts @@ -75,7 +75,7 @@ export type HooksGmailConfig = { export type InternalHookHandlerConfig = { /** Event key to listen for (e.g., 'command:new', 'session:start') */ event: string; - /** Path to handler module (absolute or relative to cwd) */ + /** Path to handler module (workspace-relative) */ module: string; /** Export name from module (default: 'default') */ export?: string; diff --git a/src/config/zod-schema.hooks.ts b/src/config/zod-schema.hooks.ts index d542e2e46d2..f7583e334be 100644 --- a/src/config/zod-schema.hooks.ts +++ b/src/config/zod-schema.hooks.ts @@ -1,6 +1,35 @@ +import path from "node:path"; import { z } from "zod"; import { sensitive } from "./zod-schema.sensitive.js"; +function isSafeRelativeModulePath(raw: string): boolean { + const value = raw.trim(); + if (!value) { + return false; + } + // Hook modules are loaded via file-path resolution + dynamic import(). + // Keep this strictly relative to a configured base dir to avoid path traversal and surprises. + if (path.isAbsolute(value)) { + return false; + } + if (value.startsWith("~")) { + return false; + } + // Disallow URL-ish and drive-relative forms (e.g. "file:...", "C:foo"). + if (value.includes(":")) { + return false; + } + const parts = value.split(/[\\/]+/g); + if (parts.some((part) => part === "..")) { + return false; + } + return true; +} + +const SafeRelativeModulePathSchema = z + .string() + .refine(isSafeRelativeModulePath, "module must be a safe relative path (no absolute paths)"); + export const HookMappingSchema = z .object({ id: z.string().optional(), @@ -38,7 +67,7 @@ export const HookMappingSchema = z timeoutSeconds: z.number().int().positive().optional(), transform: z .object({ - module: z.string(), + module: SafeRelativeModulePathSchema, export: z.string().optional(), }) .strict() @@ -50,7 +79,7 @@ export const HookMappingSchema = z export const InternalHookHandlerSchema = z .object({ event: z.string(), - module: z.string(), + module: SafeRelativeModulePathSchema, export: z.string().optional(), }) .strict(); diff --git a/src/gateway/hooks-mapping.test.ts b/src/gateway/hooks-mapping.test.ts index 3eeb0adc765..9c7e85f6927 100644 --- a/src/gateway/hooks-mapping.test.ts +++ b/src/gateway/hooks-mapping.test.ts @@ -217,7 +217,6 @@ describe("hooks mapping", () => { expect("skipped" in result).toBe(true); } }); - it("treats null transform as a handled skip", async () => { const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-skip-")); const transformsRoot = path.join(configDir, "hooks", "transforms"); diff --git a/src/gateway/server.auth.e2e.test.ts b/src/gateway/server.auth.e2e.test.ts index 640e6885a38..49423a6552d 100644 --- a/src/gateway/server.auth.e2e.test.ts +++ b/src/gateway/server.auth.e2e.test.ts @@ -144,6 +144,18 @@ describe("gateway server auth/connect", () => { signedAtMs, token: token ?? null, }); + + test("ignores requested scopes when device identity is omitted", async () => { + const ws = await openWs(port); + const res = await connectReq(ws, { device: null }); + expect(res.ok).toBe(true); + + const health = await rpcReq(ws, "health"); + expect(health.ok).toBe(false); + expect(health.error?.message).toContain("missing scope"); + + ws.close(); + }); const device = { id: identity.deviceId, publicKey: publicKeyRawBase64UrlFromPem(identity.publicKeyPem), @@ -493,6 +505,9 @@ describe("gateway server auth/connect", () => { const ws = await openTailscaleWs(port); const res = await connectReq(ws, { token: "secret", device: null }); expect(res.ok).toBe(true); + const health = await rpcReq(ws, "health"); + expect(health.ok).toBe(false); + expect(health.error?.message).toContain("missing scope"); ws.close(); }); }); diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index ad67ad2acc6..b5d396f5cec 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -298,7 +298,9 @@ export function attachGatewayWsMessageHandler(params: { return; } // Default-deny: scopes must be explicit. Empty/missing scopes means no permissions. - const scopes = Array.isArray(connectParams.scopes) ? connectParams.scopes : []; + // Note: If the client does not present a device identity, we can't bind scopes to a paired + // device/token, so we will clear scopes after auth to avoid self-declared permissions. + let scopes = Array.isArray(connectParams.scopes) ? connectParams.scopes : []; connectParams.role = role; connectParams.scopes = scopes; @@ -428,6 +430,10 @@ export function attachGatewayWsMessageHandler(params: { close(1008, truncateCloseReason(authMessage)); }; if (!device) { + if (scopes.length > 0) { + scopes = []; + connectParams.scopes = scopes; + } const canSkipDevice = sharedAuthOk; if (isControlUi && !allowControlUiBypass) { diff --git a/src/hooks/loader.test.ts b/src/hooks/loader.test.ts index bd1113fdaf3..3f7dbe64339 100644 --- a/src/hooks/loader.test.ts +++ b/src/hooks/loader.test.ts @@ -79,7 +79,7 @@ describe("loader", () => { handlers: [ { event: "command:new", - module: handlerPath, + module: path.basename(handlerPath), }, ], }, @@ -106,8 +106,8 @@ describe("loader", () => { internal: { enabled: true, handlers: [ - { event: "command:new", module: handler1Path }, - { event: "command:stop", module: handler2Path }, + { event: "command:new", module: path.basename(handler1Path) }, + { event: "command:stop", module: path.basename(handler2Path) }, ], }, }, @@ -138,7 +138,7 @@ describe("loader", () => { handlers: [ { event: "command:new", - module: handlerPath, + module: path.basename(handlerPath), export: "myHandler", }, ], @@ -158,7 +158,7 @@ describe("loader", () => { handlers: [ { event: "command:new", - module: "/nonexistent/path/handler.js", + module: "missing-handler.js", }, ], }, @@ -182,7 +182,7 @@ describe("loader", () => { handlers: [ { event: "command:new", - module: handlerPath, + module: path.basename(handlerPath), }, ], }, @@ -199,8 +199,8 @@ describe("loader", () => { const handlerPath = path.join(tmpDir, "relative-handler.js"); await fs.writeFile(handlerPath, "export default async function() {}", "utf-8"); - // Get relative path from cwd - const relativePath = path.relative(process.cwd(), handlerPath); + // Relative to workspaceDir (tmpDir) + const relativePath = path.relative(tmpDir, handlerPath); const cfg: OpenClawConfig = { hooks: { @@ -241,7 +241,7 @@ describe("loader", () => { handlers: [ { event: "command:new", - module: handlerPath, + module: path.basename(handlerPath), }, ], }, diff --git a/src/hooks/loader.ts b/src/hooks/loader.ts index 9b6e854aa63..750904f76dd 100644 --- a/src/hooks/loader.ts +++ b/src/hooks/loader.ts @@ -116,10 +116,25 @@ export async function loadInternalHooks( const handlers = cfg.hooks.internal.handlers ?? []; for (const handlerConfig of handlers) { try { - // Resolve module path (absolute or relative to cwd) - const modulePath = path.isAbsolute(handlerConfig.module) - ? handlerConfig.module - : path.join(process.cwd(), handlerConfig.module); + // Legacy handler paths: keep them workspace-relative. + const rawModule = handlerConfig.module.trim(); + if (!rawModule) { + log.error("Handler module path is empty"); + continue; + } + if (path.isAbsolute(rawModule)) { + log.error( + `Handler module path must be workspace-relative (got absolute path): ${rawModule}`, + ); + continue; + } + const baseDir = path.resolve(workspaceDir); + const modulePath = path.resolve(baseDir, rawModule); + const rel = path.relative(baseDir, modulePath); + if (!rel || rel.startsWith("..") || path.isAbsolute(rel)) { + log.error(`Handler module path must stay within workspaceDir: ${rawModule}`); + continue; + } // Import the module with cache-busting to ensure fresh reload const url = pathToFileURL(modulePath).href;