From b2e9221a8c8189b3c9acc020ff8e9b811dbd587e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Mar 2026 07:57:26 -0700 Subject: [PATCH 01/16] test(whatsapp): fix stale append inbox expectation --- ...r-inbox.allows-messages-from-senders-allowfrom-list.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/extensions/whatsapp/src/monitor-inbox.allows-messages-from-senders-allowfrom-list.test.ts b/extensions/whatsapp/src/monitor-inbox.allows-messages-from-senders-allowfrom-list.test.ts index 545a010ed50..101357a9de6 100644 --- a/extensions/whatsapp/src/monitor-inbox.allows-messages-from-senders-allowfrom-list.test.ts +++ b/extensions/whatsapp/src/monitor-inbox.allows-messages-from-senders-allowfrom-list.test.ts @@ -254,6 +254,7 @@ describe("web monitor inbox", () => { it("handles append messages by marking them read but skipping auto-reply", async () => { const { onMessage, listener, sock } = await openInboxMonitor(); + const staleTs = Math.floor(Date.now() / 1000) - 300; const upsert = { type: "append", @@ -265,7 +266,7 @@ describe("web monitor inbox", () => { remoteJid: "999@s.whatsapp.net", }, message: { conversation: "old message" }, - messageTimestamp: nowSeconds(), + messageTimestamp: staleTs, pushName: "History Sender", }, ], From 53462b990d95a34236cd42726e5b73ae6722d849 Mon Sep 17 00:00:00 2001 From: Harold Hunt Date: Sun, 15 Mar 2026 11:14:28 -0400 Subject: [PATCH 02/16] chore(gateway): ignore `.test.ts` changes in `gateway:watch` (#36211) --- scripts/watch-node.d.mts | 12 +++ scripts/watch-node.mjs | 143 ++++++++++++++++++++++++----------- src/infra/watch-node.test.ts | 117 ++++++++++++++++++++++++---- 3 files changed, 211 insertions(+), 61 deletions(-) diff --git a/scripts/watch-node.d.mts b/scripts/watch-node.d.mts index d0e9dd93751..362670826a6 100644 --- a/scripts/watch-node.d.mts +++ b/scripts/watch-node.d.mts @@ -4,8 +4,20 @@ export function runWatchMain(params?: { args: string[], options: unknown, ) => { + kill?: (signal?: NodeJS.Signals | number) => void; on: (event: "exit", cb: (code: number | null, signal: string | null) => void) => void; }; + createWatcher?: ( + paths: string[], + options: { + ignoreInitial: boolean; + ignored: (watchPath: string) => boolean; + }, + ) => { + on: (event: "add" | "change" | "unlink" | "error", cb: (arg?: unknown) => void) => void; + close?: () => Promise | void; + }; + watchPaths?: string[]; process?: NodeJS.Process; cwd?: string; args?: string[]; diff --git a/scripts/watch-node.mjs b/scripts/watch-node.mjs index e554796f03b..891e07439a1 100644 --- a/scripts/watch-node.mjs +++ b/scripts/watch-node.mjs @@ -2,16 +2,24 @@ import { spawn } from "node:child_process"; import process from "node:process"; import { pathToFileURL } from "node:url"; +import chokidar from "chokidar"; import { runNodeWatchedPaths } from "./run-node.mjs"; const WATCH_NODE_RUNNER = "scripts/run-node.mjs"; +const WATCH_RESTART_SIGNAL = "SIGTERM"; -const buildWatchArgs = (args) => [ - ...runNodeWatchedPaths.flatMap((watchPath) => ["--watch-path", watchPath]), - "--watch-preserve-output", - WATCH_NODE_RUNNER, - ...args, -]; +const buildRunnerArgs = (args) => [WATCH_NODE_RUNNER, ...args]; + +const normalizePath = (filePath) => String(filePath ?? "").replaceAll("\\", "/"); + +const isIgnoredWatchPath = (filePath) => { + const normalizedPath = normalizePath(filePath); + return ( + normalizedPath.endsWith(".test.ts") || + normalizedPath.endsWith(".test.tsx") || + normalizedPath.endsWith("test-helpers.ts") + ); +}; export async function runWatchMain(params = {}) { const deps = { @@ -21,6 +29,9 @@ export async function runWatchMain(params = {}) { args: params.args ?? process.argv.slice(2), env: params.env ? { ...params.env } : { ...process.env }, now: params.now ?? Date.now, + createWatcher: + params.createWatcher ?? ((watchPaths, options) => chokidar.watch(watchPaths, options)), + watchPaths: params.watchPaths ?? runNodeWatchedPaths, }; const childEnv = { ...deps.env }; @@ -31,54 +42,96 @@ export async function runWatchMain(params = {}) { childEnv.OPENCLAW_WATCH_COMMAND = deps.args.join(" "); } - const watchProcess = deps.spawn(deps.process.execPath, buildWatchArgs(deps.args), { - cwd: deps.cwd, - env: childEnv, - stdio: "inherit", - }); - - let settled = false; - let onSigInt; - let onSigTerm; - - const settle = (resolve, code) => { - if (settled) { - return; - } - settled = true; - if (onSigInt) { - deps.process.off("SIGINT", onSigInt); - } - if (onSigTerm) { - deps.process.off("SIGTERM", onSigTerm); - } - resolve(code); - }; - return await new Promise((resolve) => { - onSigInt = () => { - if (typeof watchProcess.kill === "function") { - watchProcess.kill("SIGTERM"); + let settled = false; + let shuttingDown = false; + let restartRequested = false; + let watchProcess = null; + let onSigInt; + let onSigTerm; + + const watcher = deps.createWatcher(deps.watchPaths, { + ignoreInitial: true, + ignored: (watchPath) => isIgnoredWatchPath(watchPath), + }); + + const settle = (code) => { + if (settled) { + return; } - settle(resolve, 130); + settled = true; + if (onSigInt) { + deps.process.off("SIGINT", onSigInt); + } + if (onSigTerm) { + deps.process.off("SIGTERM", onSigTerm); + } + watcher.close?.().catch?.(() => {}); + resolve(code); + }; + + const startRunner = () => { + watchProcess = deps.spawn(deps.process.execPath, buildRunnerArgs(deps.args), { + cwd: deps.cwd, + env: childEnv, + stdio: "inherit", + }); + watchProcess.on("exit", () => { + watchProcess = null; + if (shuttingDown) { + return; + } + if (restartRequested) { + restartRequested = false; + startRunner(); + } + }); + }; + + const requestRestart = (changedPath) => { + if (shuttingDown || isIgnoredWatchPath(changedPath)) { + return; + } + if (!watchProcess) { + startRunner(); + return; + } + restartRequested = true; + if (typeof watchProcess.kill === "function") { + watchProcess.kill(WATCH_RESTART_SIGNAL); + } + }; + + watcher.on("add", requestRestart); + watcher.on("change", requestRestart); + watcher.on("unlink", requestRestart); + watcher.on("error", () => { + shuttingDown = true; + if (watchProcess && typeof watchProcess.kill === "function") { + watchProcess.kill(WATCH_RESTART_SIGNAL); + } + settle(1); + }); + + startRunner(); + + onSigInt = () => { + shuttingDown = true; + if (watchProcess && typeof watchProcess.kill === "function") { + watchProcess.kill(WATCH_RESTART_SIGNAL); + } + settle(130); }; onSigTerm = () => { - if (typeof watchProcess.kill === "function") { - watchProcess.kill("SIGTERM"); + shuttingDown = true; + if (watchProcess && typeof watchProcess.kill === "function") { + watchProcess.kill(WATCH_RESTART_SIGNAL); } - settle(resolve, 143); + settle(143); }; deps.process.on("SIGINT", onSigInt); deps.process.on("SIGTERM", onSigTerm); - - watchProcess.on("exit", (code, signal) => { - if (signal) { - settle(resolve, 1); - return; - } - settle(resolve, code ?? 1); - }); }); } diff --git a/src/infra/watch-node.test.ts b/src/infra/watch-node.test.ts index 69adbab7fc4..89ec4b79ef2 100644 --- a/src/infra/watch-node.test.ts +++ b/src/infra/watch-node.test.ts @@ -11,40 +11,50 @@ const createFakeProcess = () => const createWatchHarness = () => { const child = Object.assign(new EventEmitter(), { - kill: vi.fn(), + kill: vi.fn(() => {}), }); const spawn = vi.fn(() => child); + const watcher = Object.assign(new EventEmitter(), { + close: vi.fn(async () => {}), + }); + const createWatcher = vi.fn(() => watcher); const fakeProcess = createFakeProcess(); - return { child, spawn, fakeProcess }; + return { child, spawn, watcher, createWatcher, fakeProcess }; }; describe("watch-node script", () => { - it("wires node watch to run-node with watched source/config paths", async () => { - const { child, spawn, fakeProcess } = createWatchHarness(); + it("wires chokidar watch to run-node with watched source/config paths", async () => { + const { child, spawn, watcher, createWatcher, fakeProcess } = createWatchHarness(); const runPromise = runWatchMain({ args: ["gateway", "--force"], cwd: "/tmp/openclaw", + createWatcher, env: { PATH: "/usr/bin" }, now: () => 1700000000000, process: fakeProcess, spawn, }); - queueMicrotask(() => child.emit("exit", 0, null)); - const exitCode = await runPromise; + expect(createWatcher).toHaveBeenCalledTimes(1); + const firstWatcherCall = createWatcher.mock.calls[0]; + expect(firstWatcherCall).toBeDefined(); + const [watchPaths, watchOptions] = firstWatcherCall as unknown as [ + string[], + { ignoreInitial: boolean; ignored: (watchPath: string) => boolean }, + ]; + expect(watchPaths).toEqual(runNodeWatchedPaths); + expect(watchOptions.ignoreInitial).toBe(true); + expect(watchOptions.ignored("src/infra/watch-node.test.ts")).toBe(true); + expect(watchOptions.ignored("src/infra/watch-node.test.tsx")).toBe(true); + expect(watchOptions.ignored("src/infra/watch-node-test-helpers.ts")).toBe(true); + expect(watchOptions.ignored("src/infra/watch-node.ts")).toBe(false); + expect(watchOptions.ignored("tsconfig.json")).toBe(false); - expect(exitCode).toBe(0); expect(spawn).toHaveBeenCalledTimes(1); expect(spawn).toHaveBeenCalledWith( "/usr/local/bin/node", - [ - ...runNodeWatchedPaths.flatMap((watchPath) => ["--watch-path", watchPath]), - "--watch-preserve-output", - "scripts/run-node.mjs", - "gateway", - "--force", - ], + ["scripts/run-node.mjs", "gateway", "--force"], expect.objectContaining({ cwd: "/tmp/openclaw", stdio: "inherit", @@ -56,13 +66,19 @@ describe("watch-node script", () => { }), }), ); + fakeProcess.emit("SIGINT"); + const exitCode = await runPromise; + expect(exitCode).toBe(130); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(watcher.close).toHaveBeenCalledTimes(1); }); it("terminates child on SIGINT and returns shell interrupt code", async () => { - const { child, spawn, fakeProcess } = createWatchHarness(); + const { child, spawn, watcher, createWatcher, fakeProcess } = createWatchHarness(); const runPromise = runWatchMain({ args: ["gateway", "--force"], + createWatcher, process: fakeProcess, spawn, }); @@ -72,15 +88,17 @@ describe("watch-node script", () => { expect(exitCode).toBe(130); expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(watcher.close).toHaveBeenCalledTimes(1); expect(fakeProcess.listenerCount("SIGINT")).toBe(0); expect(fakeProcess.listenerCount("SIGTERM")).toBe(0); }); it("terminates child on SIGTERM and returns shell terminate code", async () => { - const { child, spawn, fakeProcess } = createWatchHarness(); + const { child, spawn, watcher, createWatcher, fakeProcess } = createWatchHarness(); const runPromise = runWatchMain({ args: ["gateway", "--force"], + createWatcher, process: fakeProcess, spawn, }); @@ -90,7 +108,74 @@ describe("watch-node script", () => { expect(exitCode).toBe(143); expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(watcher.close).toHaveBeenCalledTimes(1); expect(fakeProcess.listenerCount("SIGINT")).toBe(0); expect(fakeProcess.listenerCount("SIGTERM")).toBe(0); }); + + it("ignores test-only changes and restarts on non-test source changes", async () => { + const childA = Object.assign(new EventEmitter(), { + kill: vi.fn(function () { + queueMicrotask(() => childA.emit("exit", 0, null)); + }), + }); + const childB = Object.assign(new EventEmitter(), { + kill: vi.fn(() => {}), + }); + const spawn = vi.fn().mockReturnValueOnce(childA).mockReturnValueOnce(childB); + const watcher = Object.assign(new EventEmitter(), { + close: vi.fn(async () => {}), + }); + const createWatcher = vi.fn(() => watcher); + const fakeProcess = createFakeProcess(); + + const runPromise = runWatchMain({ + args: ["gateway", "--force"], + createWatcher, + process: fakeProcess, + spawn, + }); + + watcher.emit("change", "src/infra/watch-node.test.ts"); + await new Promise((resolve) => setImmediate(resolve)); + expect(spawn).toHaveBeenCalledTimes(1); + expect(childA.kill).not.toHaveBeenCalled(); + + watcher.emit("change", "src/infra/watch-node.test.tsx"); + await new Promise((resolve) => setImmediate(resolve)); + expect(spawn).toHaveBeenCalledTimes(1); + expect(childA.kill).not.toHaveBeenCalled(); + + watcher.emit("change", "src/infra/watch-node-test-helpers.ts"); + await new Promise((resolve) => setImmediate(resolve)); + expect(spawn).toHaveBeenCalledTimes(1); + expect(childA.kill).not.toHaveBeenCalled(); + + watcher.emit("change", "src/infra/watch-node.ts"); + await new Promise((resolve) => setImmediate(resolve)); + expect(childA.kill).toHaveBeenCalledWith("SIGTERM"); + expect(spawn).toHaveBeenCalledTimes(2); + + fakeProcess.emit("SIGINT"); + const exitCode = await runPromise; + expect(exitCode).toBe(130); + }); + + it("kills child and exits when watcher emits an error", async () => { + const { child, spawn, watcher, createWatcher, fakeProcess } = createWatchHarness(); + + const runPromise = runWatchMain({ + args: ["gateway", "--force"], + createWatcher, + process: fakeProcess, + spawn, + }); + + watcher.emit("error", new Error("watch failed")); + const exitCode = await runPromise; + + expect(exitCode).toBe(1); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(watcher.close).toHaveBeenCalledTimes(1); + }); }); From a472f988d89b2f9cd3fcacfdce8db794b2eac145 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Mar 2026 08:22:48 -0700 Subject: [PATCH 03/16] fix: harden remote cdp probes --- CHANGELOG.md | 1 + docs/gateway/configuration-reference.md | 1 + docs/tools/browser.md | 1 + src/browser/cdp.helpers.ts | 36 ++++++++++++++++++++ src/browser/chrome.test.ts | 18 ++++++++++ src/browser/chrome.ts | 34 ++++++++++++++----- src/browser/server-context.availability.ts | 9 +++-- src/browser/server-context.ts | 6 +++- src/cli/browser-cli-manage.test.ts | 38 ++++++++++++++++++++++ src/cli/browser-cli-manage.ts | 3 +- src/node-host/invoke-browser.test.ts | 30 +++++++++++++++++ src/node-host/invoke-browser.ts | 3 +- src/security/audit.test.ts | 26 +++++++++++++++ src/security/audit.ts | 20 +++++++++++- 14 files changed, 212 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ffe236664c..4b50a557d97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Docs: https://docs.openclaw.ai - Models/OpenRouter runtime capabilities: fetch uncatalogued OpenRouter model metadata on first use so newly added vision models keep image input instead of silently degrading to text-only, with top-level capability field fallbacks for `/api/v1/models`. (#45824) Thanks @DJjjjhao. - Z.AI/onboarding: add `glm-5-turbo` to the default Z.AI provider catalog so onboarding-generated configs expose the new model alongside the existing GLM defaults. (#46670) Thanks @tomsun28. - Zalo Personal/group gating: stop reapplying `dmPolicy.allowFrom` as a sender gate for already-allowlisted groups when `groupAllowFrom` is unset, so any member of an allowed group can trigger replies while DMs stay restricted. (#40146) +- Browser/remote CDP: honor strict browser SSRF policy during remote CDP reachability and `/json/version` discovery checks, redact sensitive `cdpUrl` tokens from status output, and warn when remote CDP targets private/internal hosts. - Plugins/install precedence: keep bundled plugins ahead of auto-discovered globals by default, but let an explicitly installed plugin record win its own duplicate-id tie so installed channel plugins load from `~/.openclaw/extensions` after `openclaw plugins install`. - macOS/canvas actions: keep unattended local agent actions on trusted in-app canvas surfaces only, and stop exposing the deep-link fallback key to arbitrary page scripts. Thanks @vincentkoc. - Agents/compaction: extend the enclosing run deadline once while compaction is actively in flight, and abort the underlying SDK compaction on timeout/cancel so large-session compactions stop freezing mid-run. (#46889) Thanks @asyncjason. diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index badfe4ee891..7bb7fb5824f 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -2370,6 +2370,7 @@ See [Plugins](/tools/plugin). - `evaluateEnabled: false` disables `act:evaluate` and `wait --fn`. - `ssrfPolicy.dangerouslyAllowPrivateNetwork` defaults to `true` when unset (trusted-network model). - Set `ssrfPolicy.dangerouslyAllowPrivateNetwork: false` for strict public-only browser navigation. +- In strict mode, remote CDP profile endpoints (`profiles.*.cdpUrl`) are subject to the same private-network blocking during reachability/discovery checks. - `ssrfPolicy.allowPrivateNetwork` remains supported as a legacy alias. - In strict mode, use `ssrfPolicy.hostnameAllowlist` and `ssrfPolicy.allowedHostnames` for explicit exceptions. - Remote profiles are attach-only (start/stop/reset disabled). diff --git a/docs/tools/browser.md b/docs/tools/browser.md index ebe352036c5..c760c23998c 100644 --- a/docs/tools/browser.md +++ b/docs/tools/browser.md @@ -114,6 +114,7 @@ Notes: - `remoteCdpTimeoutMs` applies to remote (non-loopback) CDP reachability checks. - `remoteCdpHandshakeTimeoutMs` applies to remote CDP WebSocket reachability checks. - Browser navigation/open-tab is SSRF-guarded before navigation and best-effort re-checked on final `http(s)` URL after navigation. +- In strict SSRF mode, remote CDP endpoint discovery/probes (`cdpUrl`, including `/json/version` lookups) are checked too. - `browser.ssrfPolicy.dangerouslyAllowPrivateNetwork` defaults to `true` (trusted-network model). Set it to `false` for strict public-only browsing. - `browser.ssrfPolicy.allowPrivateNetwork` remains supported as a legacy alias for compatibility. - `attachOnly: true` means “never launch a local browser; only attach if it is already running.” diff --git a/src/browser/cdp.helpers.ts b/src/browser/cdp.helpers.ts index 44f689e8706..399f0582d88 100644 --- a/src/browser/cdp.helpers.ts +++ b/src/browser/cdp.helpers.ts @@ -1,6 +1,8 @@ import WebSocket from "ws"; import { isLoopbackHost } from "../gateway/net.js"; +import { type SsrFPolicy, resolvePinnedHostnameWithPolicy } from "../infra/net/ssrf.js"; import { rawDataToString } from "../infra/ws.js"; +import { redactSensitiveText } from "../logging/redact.js"; import { getDirectAgentForCdp, withNoProxyForCdpUrl } from "./cdp-proxy-bypass.js"; import { CDP_HTTP_REQUEST_TIMEOUT_MS, CDP_WS_HANDSHAKE_TIMEOUT_MS } from "./cdp-timeouts.js"; import { resolveBrowserRateLimitMessage } from "./client-fetch.js"; @@ -22,6 +24,40 @@ export function isWebSocketUrl(url: string): boolean { } } +export async function assertCdpEndpointAllowed( + cdpUrl: string, + ssrfPolicy?: SsrFPolicy, +): Promise { + if (!ssrfPolicy) { + return; + } + const parsed = new URL(cdpUrl); + if (!["http:", "https:", "ws:", "wss:"].includes(parsed.protocol)) { + throw new Error(`Invalid CDP URL protocol: ${parsed.protocol.replace(":", "")}`); + } + await resolvePinnedHostnameWithPolicy(parsed.hostname, { + policy: ssrfPolicy, + }); +} + +export function redactCdpUrl(cdpUrl: string | null | undefined): string | null | undefined { + if (typeof cdpUrl !== "string") { + return cdpUrl; + } + const trimmed = cdpUrl.trim(); + if (!trimmed) { + return trimmed; + } + try { + const parsed = new URL(trimmed); + parsed.username = ""; + parsed.password = ""; + return redactSensitiveText(parsed.toString().replace(/\/$/, "")); + } catch { + return redactSensitiveText(trimmed); + } +} + type CdpResponse = { id: number; result?: unknown; diff --git a/src/browser/chrome.test.ts b/src/browser/chrome.test.ts index dcbd32fd13c..ee4cb8541c3 100644 --- a/src/browser/chrome.test.ts +++ b/src/browser/chrome.test.ts @@ -302,6 +302,24 @@ describe("browser chrome helpers", () => { await expect(isChromeReachable("http://127.0.0.1:12345", 50)).resolves.toBe(false); }); + it("blocks private CDP probes when strict SSRF policy is enabled", async () => { + const fetchSpy = vi.fn().mockRejectedValue(new Error("should not be called")); + vi.stubGlobal("fetch", fetchSpy); + + await expect( + isChromeReachable("http://127.0.0.1:12345", 50, { + dangerouslyAllowPrivateNetwork: false, + }), + ).resolves.toBe(false); + await expect( + isChromeReachable("ws://127.0.0.1:19999", 50, { + dangerouslyAllowPrivateNetwork: false, + }), + ).resolves.toBe(false); + + expect(fetchSpy).not.toHaveBeenCalled(); + }); + it("reports cdpReady only when Browser.getVersion command succeeds", async () => { await withMockChromeCdpServer({ wsPath: "/devtools/browser/health", diff --git a/src/browser/chrome.ts b/src/browser/chrome.ts index 8e48024d7ad..1cb94cf39fb 100644 --- a/src/browser/chrome.ts +++ b/src/browser/chrome.ts @@ -2,6 +2,7 @@ import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import type { SsrFPolicy } from "../infra/net/ssrf.js"; import { ensurePortAvailable } from "../infra/ports.js"; import { rawDataToString } from "../infra/ws.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; @@ -17,7 +18,13 @@ import { CHROME_STOP_TIMEOUT_MS, CHROME_WS_READY_TIMEOUT_MS, } from "./cdp-timeouts.js"; -import { appendCdpPath, fetchCdpChecked, isWebSocketUrl, openCdpWebSocket } from "./cdp.helpers.js"; +import { + appendCdpPath, + assertCdpEndpointAllowed, + fetchCdpChecked, + isWebSocketUrl, + openCdpWebSocket, +} from "./cdp.helpers.js"; import { normalizeCdpWsUrl } from "./cdp.js"; import { type BrowserExecutable, @@ -96,13 +103,19 @@ async function canOpenWebSocket(url: string, timeoutMs: number): Promise { - if (isWebSocketUrl(cdpUrl)) { - // Direct WebSocket endpoint — probe via WS handshake. - return await canOpenWebSocket(cdpUrl, timeoutMs); + try { + await assertCdpEndpointAllowed(cdpUrl, ssrfPolicy); + if (isWebSocketUrl(cdpUrl)) { + // Direct WebSocket endpoint — probe via WS handshake. + return await canOpenWebSocket(cdpUrl, timeoutMs); + } + const version = await fetchChromeVersion(cdpUrl, timeoutMs, ssrfPolicy); + return Boolean(version); + } catch { + return false; } - const version = await fetchChromeVersion(cdpUrl, timeoutMs); - return Boolean(version); } type ChromeVersion = { @@ -114,10 +127,12 @@ type ChromeVersion = { async function fetchChromeVersion( cdpUrl: string, timeoutMs = CHROME_REACHABILITY_TIMEOUT_MS, + ssrfPolicy?: SsrFPolicy, ): Promise { const ctrl = new AbortController(); const t = setTimeout(ctrl.abort.bind(ctrl), timeoutMs); try { + await assertCdpEndpointAllowed(cdpUrl, ssrfPolicy); const versionUrl = appendCdpPath(cdpUrl, "/json/version"); const res = await fetchCdpChecked(versionUrl, timeoutMs, { signal: ctrl.signal }); const data = (await res.json()) as ChromeVersion; @@ -135,12 +150,14 @@ async function fetchChromeVersion( export async function getChromeWebSocketUrl( cdpUrl: string, timeoutMs = CHROME_REACHABILITY_TIMEOUT_MS, + ssrfPolicy?: SsrFPolicy, ): Promise { + await assertCdpEndpointAllowed(cdpUrl, ssrfPolicy); if (isWebSocketUrl(cdpUrl)) { // Direct WebSocket endpoint — the cdpUrl is already the WebSocket URL. return cdpUrl; } - const version = await fetchChromeVersion(cdpUrl, timeoutMs); + const version = await fetchChromeVersion(cdpUrl, timeoutMs, ssrfPolicy); const wsUrl = String(version?.webSocketDebuggerUrl ?? "").trim(); if (!wsUrl) { return null; @@ -227,8 +244,9 @@ export async function isChromeCdpReady( cdpUrl: string, timeoutMs = CHROME_REACHABILITY_TIMEOUT_MS, handshakeTimeoutMs = CHROME_WS_READY_TIMEOUT_MS, + ssrfPolicy?: SsrFPolicy, ): Promise { - const wsUrl = await getChromeWebSocketUrl(cdpUrl, timeoutMs); + const wsUrl = await getChromeWebSocketUrl(cdpUrl, timeoutMs, ssrfPolicy).catch(() => null); if (!wsUrl) { return false; } diff --git a/src/browser/server-context.availability.ts b/src/browser/server-context.availability.ts index 3b991bbbdfe..a0281d53d9f 100644 --- a/src/browser/server-context.availability.ts +++ b/src/browser/server-context.availability.ts @@ -71,7 +71,12 @@ export function createProfileAvailability({ return true; } const { httpTimeoutMs, wsTimeoutMs } = resolveTimeouts(timeoutMs); - return await isChromeCdpReady(profile.cdpUrl, httpTimeoutMs, wsTimeoutMs); + return await isChromeCdpReady( + profile.cdpUrl, + httpTimeoutMs, + wsTimeoutMs, + state().resolved.ssrfPolicy, + ); }; const isHttpReachable = async (timeoutMs?: number) => { @@ -79,7 +84,7 @@ export function createProfileAvailability({ return await isReachable(timeoutMs); } const { httpTimeoutMs } = resolveTimeouts(timeoutMs); - return await isChromeReachable(profile.cdpUrl, httpTimeoutMs); + return await isChromeReachable(profile.cdpUrl, httpTimeoutMs, state().resolved.ssrfPolicy); }; const attachRunning = (running: NonNullable) => { diff --git a/src/browser/server-context.ts b/src/browser/server-context.ts index 0ba29ad38cf..5b06a49964e 100644 --- a/src/browser/server-context.ts +++ b/src/browser/server-context.ts @@ -187,7 +187,11 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon } else { // Check if something is listening on the port try { - const reachable = await isChromeReachable(profile.cdpUrl, 200); + const reachable = await isChromeReachable( + profile.cdpUrl, + 200, + current.resolved.ssrfPolicy, + ); if (reachable) { running = true; const tabs = await profileCtx.listTabs().catch(() => []); diff --git a/src/cli/browser-cli-manage.test.ts b/src/cli/browser-cli-manage.test.ts index e1d01132be3..deeb0d9e73a 100644 --- a/src/cli/browser-cli-manage.test.ts +++ b/src/cli/browser-cli-manage.test.ts @@ -148,4 +148,42 @@ describe("browser manage output", () => { expect(output).toContain("transport: chrome-mcp"); expect(output).not.toContain("port: 0"); }); + + it("redacts sensitive remote cdpUrl details in status output", async () => { + mocks.callBrowserRequest.mockImplementation(async (_opts: unknown, req: { path?: string }) => + req.path === "/" + ? { + enabled: true, + profile: "remote", + driver: "openclaw", + transport: "cdp", + running: true, + cdpReady: true, + cdpHttp: true, + pid: null, + cdpPort: 9222, + cdpUrl: + "https://alice:supersecretpasswordvalue1234@example.com/chrome?token=supersecrettokenvalue1234567890", + chosenBrowser: null, + userDataDir: null, + color: "#00AA00", + headless: false, + noSandbox: false, + executablePath: null, + attachOnly: true, + } + : {}, + ); + + const program = createProgram(); + await program.parseAsync(["browser", "--browser-profile", "remote", "status"], { + from: "user", + }); + + const output = mocks.runtimeLog.mock.calls.at(-1)?.[0] as string; + expect(output).toContain("cdpUrl: https://example.com/chrome?token=supers…7890"); + expect(output).not.toContain("alice"); + expect(output).not.toContain("supersecretpasswordvalue1234"); + expect(output).not.toContain("supersecrettokenvalue1234567890"); + }); }); diff --git a/src/cli/browser-cli-manage.ts b/src/cli/browser-cli-manage.ts index 5bac9b621bf..ddf207b28f0 100644 --- a/src/cli/browser-cli-manage.ts +++ b/src/cli/browser-cli-manage.ts @@ -1,4 +1,5 @@ import type { Command } from "commander"; +import { redactCdpUrl } from "../browser/cdp.helpers.js"; import type { BrowserTransport, BrowserCreateProfileResult, @@ -152,7 +153,7 @@ export function registerBrowserManageCommands( ...(!usesChromeMcpTransport(status) ? [ `cdpPort: ${status.cdpPort ?? "(unset)"}`, - `cdpUrl: ${status.cdpUrl ?? `http://127.0.0.1:${status.cdpPort}`}`, + `cdpUrl: ${redactCdpUrl(status.cdpUrl ?? `http://127.0.0.1:${status.cdpPort}`)}`, ] : []), `browser: ${status.chosenBrowser ?? "unknown"}`, diff --git a/src/node-host/invoke-browser.test.ts b/src/node-host/invoke-browser.test.ts index 4dc5b520d43..c1dd0d1df76 100644 --- a/src/node-host/invoke-browser.test.ts +++ b/src/node-host/invoke-browser.test.ts @@ -109,6 +109,36 @@ describe("runBrowserProxyCommand", () => { ); }); + it("redacts sensitive cdpUrl details in timeout diagnostics", async () => { + dispatcherMocks.dispatch + .mockImplementationOnce(async () => { + await new Promise(() => {}); + }) + .mockResolvedValueOnce({ + status: 200, + body: { + running: true, + cdpHttp: true, + cdpReady: false, + cdpUrl: + "https://alice:supersecretpasswordvalue1234@example.com/chrome?token=supersecrettokenvalue1234567890", + }, + }); + + await expect( + runBrowserProxyCommand( + JSON.stringify({ + method: "GET", + path: "/snapshot", + profile: "remote", + timeoutMs: 5, + }), + ), + ).rejects.toThrow( + /status\(running=true, cdpHttp=true, cdpReady=false, cdpUrl=https:\/\/example\.com\/chrome\?token=supers…7890\)/, + ); + }); + it("keeps non-timeout browser errors intact", async () => { dispatcherMocks.dispatch.mockResolvedValue({ status: 500, diff --git a/src/node-host/invoke-browser.ts b/src/node-host/invoke-browser.ts index fc16ccd5298..8a440dc905a 100644 --- a/src/node-host/invoke-browser.ts +++ b/src/node-host/invoke-browser.ts @@ -1,4 +1,5 @@ import fsPromises from "node:fs/promises"; +import { redactCdpUrl } from "../browser/cdp.helpers.js"; import { resolveBrowserConfig } from "../browser/config.js"; import { createBrowserControlContext, @@ -199,7 +200,7 @@ function formatBrowserProxyTimeoutMessage(params: { statusParts.push(`transport=${params.status.transport}`); } if (typeof params.status.cdpUrl === "string" && params.status.cdpUrl.trim()) { - statusParts.push(`cdpUrl=${params.status.cdpUrl}`); + statusParts.push(`cdpUrl=${redactCdpUrl(params.status.cdpUrl)}`); } parts.push(`status(${statusParts.join(", ")})`); } diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index e757c2970d6..84fcadf1f98 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -1378,6 +1378,32 @@ description: test skill expectFinding(res, "browser.remote_cdp_http", "warn"); }); + it("warns when remote CDP targets a private/internal host", async () => { + const cfg: OpenClawConfig = { + browser: { + profiles: { + remote: { + cdpUrl: + "http://169.254.169.254:9222/json/version?token=supersecrettokenvalue1234567890", + color: "#0066CC", + }, + }, + }, + }; + + const res = await audit(cfg); + + expectFinding(res, "browser.remote_cdp_private_host", "warn"); + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + checkId: "browser.remote_cdp_private_host", + detail: expect.stringContaining("token=supers…7890"), + }), + ]), + ); + }); + it("warns when control UI allows insecure auth", async () => { const cfg: OpenClawConfig = { gateway: { diff --git a/src/security/audit.ts b/src/security/audit.ts index 119aa6e5f00..113ec2bd067 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -2,6 +2,7 @@ import { isIP } from "node:net"; import path from "node:path"; import { resolveSandboxConfigForAgent } from "../agents/sandbox.js"; import { execDockerRaw } from "../agents/sandbox/docker.js"; +import { redactCdpUrl } from "../browser/cdp.helpers.js"; import { resolveBrowserConfig, resolveProfile } from "../browser/config.js"; import { resolveBrowserControlAuth } from "../browser/control-auth.js"; import { listChannelPlugins } from "../channels/plugins/index.js"; @@ -18,6 +19,7 @@ import { resolveMergedSafeBinProfileFixtures, } from "../infra/exec-safe-bin-runtime-policy.js"; import { normalizeTrustedSafeBinDirs } from "../infra/exec-safe-bin-trust.js"; +import { isBlockedHostnameOrIp, isPrivateNetworkAllowedByPolicy } from "../infra/net/ssrf.js"; import { collectChannelSecurityFindings } from "./audit-channel.js"; import { collectAttackSurfaceSummaryFindings, @@ -782,15 +784,31 @@ function collectBrowserControlFindings( } catch { continue; } + const redactedCdpUrl = redactCdpUrl(profile.cdpUrl) ?? profile.cdpUrl; if (url.protocol === "http:") { findings.push({ checkId: "browser.remote_cdp_http", severity: "warn", title: "Remote CDP uses HTTP", - detail: `browser profile "${name}" uses http CDP (${profile.cdpUrl}); this is OK only if it's tailnet-only or behind an encrypted tunnel.`, + detail: `browser profile "${name}" uses http CDP (${redactedCdpUrl}); this is OK only if it's tailnet-only or behind an encrypted tunnel.`, remediation: `Prefer HTTPS/TLS or a tailnet-only endpoint for remote CDP.`, }); } + if ( + isPrivateNetworkAllowedByPolicy(resolved.ssrfPolicy) && + isBlockedHostnameOrIp(url.hostname) + ) { + findings.push({ + checkId: "browser.remote_cdp_private_host", + severity: "warn", + title: "Remote CDP targets a private/internal host", + detail: + `browser profile "${name}" points at a private/internal CDP host (${redactedCdpUrl}). ` + + "This is expected for LAN/tailnet/WSL-style setups, but treat it as a trusted-network endpoint.", + remediation: + "Prefer a tailnet or tunnel for remote CDP. If you want strict blocking, set browser.ssrfPolicy.dangerouslyAllowPrivateNetwork=false and allow only explicit hosts.", + }); + } } return findings; From 89e3969d640cede4636214ecaa658a082bf4513d Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Sun, 15 Mar 2026 10:33:49 -0500 Subject: [PATCH 04/16] feat(feishu): add ACP and subagent session binding (#46819) * feat(feishu): add ACP session support * fix(feishu): preserve sender-scoped ACP rebinding * fix(feishu): recover sender scope from bound ACP sessions * fix(feishu): support DM ACP binding placement * feat(feishu): add current-conversation session binding * fix(feishu): avoid DM parent binding fallback * fix(feishu): require canonical topic sender ids * fix(feishu): honor sender-scoped ACP bindings * fix(feishu): allow user-id ACP DM bindings * fix(feishu): recover user-id ACP DM bindings --- docs/channels/feishu.md | 69 ++ extensions/feishu/index.test.ts | 68 ++ extensions/feishu/index.ts | 2 + extensions/feishu/src/bot.test.ts | 288 ++++++++ extensions/feishu/src/bot.ts | 109 ++- extensions/feishu/src/conversation-id.ts | 125 ++++ extensions/feishu/src/monitor.account.ts | 31 +- .../feishu/src/monitor.reaction.test.ts | 93 +++ extensions/feishu/src/subagent-hooks.test.ts | 623 ++++++++++++++++++ extensions/feishu/src/subagent-hooks.ts | 341 ++++++++++ extensions/feishu/src/thread-bindings.test.ts | 94 +++ extensions/feishu/src/thread-bindings.ts | 316 +++++++++ src/acp/persistent-bindings.resolve.ts | 164 ++++- src/acp/persistent-bindings.test.ts | 196 ++++++ src/acp/persistent-bindings.types.ts | 2 +- src/auto-reply/reply/commands-acp.test.ts | 61 +- .../reply/commands-acp/context.test.ts | 178 ++++- src/auto-reply/reply/commands-acp/context.ts | 136 ++++ .../reply/commands-acp/lifecycle.ts | 9 +- src/config/config.acp-binding-cutover.test.ts | 108 +++ src/config/zod-schema.agents.ts | 23 +- 21 files changed, 2988 insertions(+), 48 deletions(-) create mode 100644 extensions/feishu/index.test.ts create mode 100644 extensions/feishu/src/conversation-id.ts create mode 100644 extensions/feishu/src/subagent-hooks.test.ts create mode 100644 extensions/feishu/src/subagent-hooks.ts create mode 100644 extensions/feishu/src/thread-bindings.test.ts create mode 100644 extensions/feishu/src/thread-bindings.ts diff --git a/docs/channels/feishu.md b/docs/channels/feishu.md index 467fc57c0fe..2fc16aed5d4 100644 --- a/docs/channels/feishu.md +++ b/docs/channels/feishu.md @@ -532,6 +532,75 @@ Feishu supports streaming replies via interactive cards. When enabled, the bot u Set `streaming: false` to wait for the full reply before sending. +### ACP sessions + +Feishu supports ACP for: + +- DMs +- group topic conversations + +Feishu ACP is text-command driven. There are no native slash-command menus, so use `/acp ...` messages directly in the conversation. + +#### Persistent ACP bindings + +Use top-level typed ACP bindings to pin a Feishu DM or topic conversation to a persistent ACP session. + +```json5 +{ + agents: { + list: [ + { + id: "codex", + runtime: { + type: "acp", + acp: { + agent: "codex", + backend: "acpx", + mode: "persistent", + cwd: "/workspace/openclaw", + }, + }, + }, + ], + }, + bindings: [ + { + type: "acp", + agentId: "codex", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "direct", id: "ou_1234567890" }, + }, + }, + { + type: "acp", + agentId: "codex", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "group", id: "oc_group_chat:topic:om_topic_root" }, + }, + acp: { label: "codex-feishu-topic" }, + }, + ], +} +``` + +#### Thread-bound ACP spawn from chat + +In a Feishu DM or topic conversation, you can spawn and bind an ACP session in place: + +```text +/acp spawn codex --thread here +``` + +Notes: + +- `--thread here` works for DMs and Feishu topics. +- Follow-up messages in the bound DM/topic route directly to that ACP session. +- v1 does not target generic non-topic group chats. + ### Multi-agent routing Use `bindings` to route Feishu DMs or groups to different agents. diff --git a/extensions/feishu/index.test.ts b/extensions/feishu/index.test.ts new file mode 100644 index 00000000000..5236e4bb542 --- /dev/null +++ b/extensions/feishu/index.test.ts @@ -0,0 +1,68 @@ +import type { OpenClawPluginApi } from "openclaw/plugin-sdk/feishu"; +import { describe, expect, it, vi } from "vitest"; + +const registerFeishuDocToolsMock = vi.hoisted(() => vi.fn()); +const registerFeishuChatToolsMock = vi.hoisted(() => vi.fn()); +const registerFeishuWikiToolsMock = vi.hoisted(() => vi.fn()); +const registerFeishuDriveToolsMock = vi.hoisted(() => vi.fn()); +const registerFeishuPermToolsMock = vi.hoisted(() => vi.fn()); +const registerFeishuBitableToolsMock = vi.hoisted(() => vi.fn()); +const setFeishuRuntimeMock = vi.hoisted(() => vi.fn()); +const registerFeishuSubagentHooksMock = vi.hoisted(() => vi.fn()); + +vi.mock("./src/docx.js", () => ({ + registerFeishuDocTools: registerFeishuDocToolsMock, +})); + +vi.mock("./src/chat.js", () => ({ + registerFeishuChatTools: registerFeishuChatToolsMock, +})); + +vi.mock("./src/wiki.js", () => ({ + registerFeishuWikiTools: registerFeishuWikiToolsMock, +})); + +vi.mock("./src/drive.js", () => ({ + registerFeishuDriveTools: registerFeishuDriveToolsMock, +})); + +vi.mock("./src/perm.js", () => ({ + registerFeishuPermTools: registerFeishuPermToolsMock, +})); + +vi.mock("./src/bitable.js", () => ({ + registerFeishuBitableTools: registerFeishuBitableToolsMock, +})); + +vi.mock("./src/runtime.js", () => ({ + setFeishuRuntime: setFeishuRuntimeMock, +})); + +vi.mock("./src/subagent-hooks.js", () => ({ + registerFeishuSubagentHooks: registerFeishuSubagentHooksMock, +})); + +describe("feishu plugin register", () => { + it("registers the Feishu channel, tools, and subagent hooks", async () => { + const { default: plugin } = await import("./index.js"); + const registerChannel = vi.fn(); + const api = { + runtime: { log: vi.fn() }, + registerChannel, + on: vi.fn(), + config: {}, + } as unknown as OpenClawPluginApi; + + plugin.register(api); + + expect(setFeishuRuntimeMock).toHaveBeenCalledWith(api.runtime); + expect(registerChannel).toHaveBeenCalledTimes(1); + expect(registerFeishuSubagentHooksMock).toHaveBeenCalledWith(api); + expect(registerFeishuDocToolsMock).toHaveBeenCalledWith(api); + expect(registerFeishuChatToolsMock).toHaveBeenCalledWith(api); + expect(registerFeishuWikiToolsMock).toHaveBeenCalledWith(api); + expect(registerFeishuDriveToolsMock).toHaveBeenCalledWith(api); + expect(registerFeishuPermToolsMock).toHaveBeenCalledWith(api); + expect(registerFeishuBitableToolsMock).toHaveBeenCalledWith(api); + }); +}); diff --git a/extensions/feishu/index.ts b/extensions/feishu/index.ts index bd26346c8ec..e01a975615a 100644 --- a/extensions/feishu/index.ts +++ b/extensions/feishu/index.ts @@ -7,6 +7,7 @@ import { registerFeishuDocTools } from "./src/docx.js"; import { registerFeishuDriveTools } from "./src/drive.js"; import { registerFeishuPermTools } from "./src/perm.js"; import { setFeishuRuntime } from "./src/runtime.js"; +import { registerFeishuSubagentHooks } from "./src/subagent-hooks.js"; import { registerFeishuWikiTools } from "./src/wiki.js"; export { monitorFeishuProvider } from "./src/monitor.js"; @@ -53,6 +54,7 @@ const plugin = { register(api: OpenClawPluginApi) { setFeishuRuntime(api.runtime); api.registerChannel({ plugin: feishuPlugin }); + registerFeishuSubagentHooks(api); registerFeishuDocTools(api); registerFeishuChatTools(api); registerFeishuWikiTools(api); diff --git a/extensions/feishu/src/bot.test.ts b/extensions/feishu/src/bot.test.ts index 4e0dd9d4fed..3e14bcdadd5 100644 --- a/extensions/feishu/src/bot.test.ts +++ b/extensions/feishu/src/bot.test.ts @@ -21,6 +21,10 @@ const { mockResolveAgentRoute, mockReadSessionUpdatedAt, mockResolveStorePath, + mockResolveConfiguredAcpRoute, + mockEnsureConfiguredAcpRouteReady, + mockResolveBoundConversation, + mockTouchBinding, } = vi.hoisted(() => ({ mockCreateFeishuReplyDispatcher: vi.fn(() => ({ dispatcher: vi.fn(), @@ -46,6 +50,13 @@ const { })), mockReadSessionUpdatedAt: vi.fn(), mockResolveStorePath: vi.fn(() => "/tmp/feishu-sessions.json"), + mockResolveConfiguredAcpRoute: vi.fn(({ route }) => ({ + configuredBinding: null, + route, + })), + mockEnsureConfiguredAcpRouteReady: vi.fn(async (_params?: unknown) => ({ ok: true })), + mockResolveBoundConversation: vi.fn(() => null), + mockTouchBinding: vi.fn(), })); vi.mock("./reply-dispatcher.js", () => ({ @@ -66,6 +77,18 @@ vi.mock("./client.js", () => ({ createFeishuClient: mockCreateFeishuClient, })); +vi.mock("../../../src/acp/persistent-bindings.route.js", () => ({ + resolveConfiguredAcpRoute: (params: unknown) => mockResolveConfiguredAcpRoute(params), + ensureConfiguredAcpRouteReady: (params: unknown) => mockEnsureConfiguredAcpRouteReady(params), +})); + +vi.mock("../../../src/infra/outbound/session-binding-service.js", () => ({ + getSessionBindingService: () => ({ + resolveByConversation: mockResolveBoundConversation, + touch: mockTouchBinding, + }), +})); + function createRuntimeEnv(): RuntimeEnv { return { log: vi.fn(), @@ -110,6 +133,261 @@ describe("buildFeishuAgentBody", () => { }); }); +describe("handleFeishuMessage ACP routing", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockResolveConfiguredAcpRoute.mockReset().mockImplementation( + ({ route }) => + ({ + configuredBinding: null, + route, + }) as any, + ); + mockEnsureConfiguredAcpRouteReady.mockReset().mockResolvedValue({ ok: true }); + mockResolveBoundConversation.mockReset().mockReturnValue(null); + mockTouchBinding.mockReset(); + mockResolveAgentRoute.mockReset().mockReturnValue({ + agentId: "main", + channel: "feishu", + accountId: "default", + sessionKey: "agent:main:feishu:direct:ou_sender_1", + mainSessionKey: "agent:main:main", + matchedBy: "default", + }); + mockSendMessageFeishu + .mockReset() + .mockResolvedValue({ messageId: "reply-msg", chatId: "oc_dm" }); + mockCreateFeishuReplyDispatcher.mockReset().mockReturnValue({ + dispatcher: { + sendToolResult: vi.fn(), + sendBlockReply: vi.fn(), + sendFinalReply: vi.fn(), + waitForIdle: vi.fn(), + getQueuedCounts: vi.fn(() => ({ tool: 0, block: 0, final: 0 })), + markComplete: vi.fn(), + } as any, + replyOptions: {}, + markDispatchIdle: vi.fn(), + }); + + setFeishuRuntime( + createPluginRuntimeMock({ + channel: { + routing: { + resolveAgentRoute: + mockResolveAgentRoute as unknown as PluginRuntime["channel"]["routing"]["resolveAgentRoute"], + }, + session: { + readSessionUpdatedAt: + mockReadSessionUpdatedAt as unknown as PluginRuntime["channel"]["session"]["readSessionUpdatedAt"], + resolveStorePath: + mockResolveStorePath as unknown as PluginRuntime["channel"]["session"]["resolveStorePath"], + }, + reply: { + resolveEnvelopeFormatOptions: vi.fn( + () => ({}), + ) as unknown as PluginRuntime["channel"]["reply"]["resolveEnvelopeFormatOptions"], + formatAgentEnvelope: vi.fn((params: { body: string }) => params.body), + finalizeInboundContext: ((ctx: unknown) => + ctx) as unknown as PluginRuntime["channel"]["reply"]["finalizeInboundContext"], + dispatchReplyFromConfig: vi.fn().mockResolvedValue({ + queuedFinal: false, + counts: { final: 1 }, + }), + withReplyDispatcher: vi.fn( + async ({ + run, + }: Parameters[0]) => + await run(), + ) as unknown as PluginRuntime["channel"]["reply"]["withReplyDispatcher"], + }, + commands: { + shouldComputeCommandAuthorized: vi.fn(() => false), + resolveCommandAuthorizedFromAuthorizers: vi.fn(() => false), + }, + pairing: { + readAllowFromStore: vi.fn().mockResolvedValue(["ou_sender_1"]), + upsertPairingRequest: vi.fn(), + buildPairingReply: vi.fn(), + }, + }, + }), + ); + }); + + it("ensures configured ACP routes for Feishu DMs", async () => { + mockResolveConfiguredAcpRoute.mockReturnValue({ + configuredBinding: { + spec: { + channel: "feishu", + accountId: "default", + conversationId: "ou_sender_1", + agentId: "codex", + mode: "persistent", + }, + record: { + bindingId: "config:acp:feishu:default:ou_sender_1", + targetSessionKey: "agent:codex:acp:binding:feishu:default:abc123", + targetKind: "session", + conversation: { + channel: "feishu", + accountId: "default", + conversationId: "ou_sender_1", + }, + status: "active", + boundAt: 0, + metadata: { source: "config" }, + }, + }, + route: { + agentId: "codex", + channel: "feishu", + accountId: "default", + sessionKey: "agent:codex:acp:binding:feishu:default:abc123", + mainSessionKey: "agent:codex:main", + matchedBy: "binding.channel", + }, + } as any); + + await dispatchMessage({ + cfg: { + session: { mainKey: "main", scope: "per-sender" }, + channels: { feishu: { enabled: true, allowFrom: ["ou_sender_1"], dmPolicy: "open" } }, + }, + event: { + sender: { sender_id: { open_id: "ou_sender_1" } }, + message: { + message_id: "msg-1", + chat_id: "oc_dm", + chat_type: "p2p", + message_type: "text", + content: JSON.stringify({ text: "hello" }), + }, + }, + }); + + expect(mockResolveConfiguredAcpRoute).toHaveBeenCalledTimes(1); + expect(mockEnsureConfiguredAcpRouteReady).toHaveBeenCalledTimes(1); + }); + + it("surfaces configured ACP initialization failures to the Feishu conversation", async () => { + mockResolveConfiguredAcpRoute.mockReturnValue({ + configuredBinding: { + spec: { + channel: "feishu", + accountId: "default", + conversationId: "ou_sender_1", + agentId: "codex", + mode: "persistent", + }, + record: { + bindingId: "config:acp:feishu:default:ou_sender_1", + targetSessionKey: "agent:codex:acp:binding:feishu:default:abc123", + targetKind: "session", + conversation: { + channel: "feishu", + accountId: "default", + conversationId: "ou_sender_1", + }, + status: "active", + boundAt: 0, + metadata: { source: "config" }, + }, + }, + route: { + agentId: "codex", + channel: "feishu", + accountId: "default", + sessionKey: "agent:codex:acp:binding:feishu:default:abc123", + mainSessionKey: "agent:codex:main", + matchedBy: "binding.channel", + }, + } as any); + mockEnsureConfiguredAcpRouteReady.mockResolvedValue({ + ok: false, + error: "runtime unavailable", + } as any); + + await dispatchMessage({ + cfg: { + session: { mainKey: "main", scope: "per-sender" }, + channels: { feishu: { enabled: true, allowFrom: ["ou_sender_1"], dmPolicy: "open" } }, + }, + event: { + sender: { sender_id: { open_id: "ou_sender_1" } }, + message: { + message_id: "msg-2", + chat_id: "oc_dm", + chat_type: "p2p", + message_type: "text", + content: JSON.stringify({ text: "hello" }), + }, + }, + }); + + expect(mockSendMessageFeishu).toHaveBeenCalledWith( + expect.objectContaining({ + to: "chat:oc_dm", + text: expect.stringContaining("runtime unavailable"), + }), + ); + }); + + it("routes Feishu topic messages through active bound conversations", async () => { + mockResolveBoundConversation.mockReturnValue({ + bindingId: "default:oc_group_chat:topic:om_topic_root", + targetSessionKey: "agent:codex:acp:binding:feishu:default:feedface", + targetKind: "session", + conversation: { + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_topic_root", + parentConversationId: "oc_group_chat", + }, + status: "active", + boundAt: 0, + } as any); + + await dispatchMessage({ + cfg: { + session: { mainKey: "main", scope: "per-sender" }, + channels: { + feishu: { + enabled: true, + allowFrom: ["ou_sender_1"], + groups: { + oc_group_chat: { + allow: true, + requireMention: false, + groupSessionScope: "group_topic", + }, + }, + }, + }, + }, + event: { + sender: { sender_id: { open_id: "ou_sender_1" } }, + message: { + message_id: "msg-3", + chat_id: "oc_group_chat", + chat_type: "group", + message_type: "text", + root_id: "om_topic_root", + content: JSON.stringify({ text: "hello topic" }), + }, + }, + }); + + expect(mockResolveBoundConversation).toHaveBeenCalledWith( + expect.objectContaining({ + channel: "feishu", + conversationId: "oc_group_chat:topic:om_topic_root", + }), + ); + expect(mockTouchBinding).toHaveBeenCalledWith("default:oc_group_chat:topic:om_topic_root"); + }); +}); + describe("handleFeishuMessage command authorization", () => { const mockFinalizeInboundContext = vi.fn((ctx: unknown) => ctx); const mockDispatchReplyFromConfig = vi @@ -153,6 +431,16 @@ describe("handleFeishuMessage command authorization", () => { mockListFeishuThreadMessages.mockReset().mockResolvedValue([]); mockReadSessionUpdatedAt.mockReturnValue(undefined); mockResolveStorePath.mockReturnValue("/tmp/feishu-sessions.json"); + mockResolveConfiguredAcpRoute.mockReset().mockImplementation( + ({ route }) => + ({ + configuredBinding: null, + route, + }) as any, + ); + mockEnsureConfiguredAcpRouteReady.mockReset().mockResolvedValue({ ok: true }); + mockResolveBoundConversation.mockReset().mockReturnValue(null); + mockTouchBinding.mockReset(); mockResolveAgentRoute.mockReturnValue({ agentId: "main", channel: "feishu", diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index dc8326b1dba..fc84801b124 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -14,8 +14,16 @@ import { resolveDefaultGroupPolicy, warnMissingProviderGroupPolicyFallbackOnce, } from "openclaw/plugin-sdk/feishu"; +import { + ensureConfiguredAcpRouteReady, + resolveConfiguredAcpRoute, +} from "../../../src/acp/persistent-bindings.route.js"; +import { getSessionBindingService } from "../../../src/infra/outbound/session-binding-service.js"; +import { deriveLastRoutePolicy } from "../../../src/routing/resolve-route.js"; +import { resolveAgentIdFromSessionKey } from "../../../src/routing/session-key.js"; import { resolveFeishuAccount } from "./accounts.js"; import { createFeishuClient } from "./client.js"; +import { buildFeishuConversationId } from "./conversation-id.js"; import { finalizeFeishuMessageProcessing, tryRecordMessagePersistent } from "./dedup.js"; import { maybeCreateDynamicAgent } from "./dynamic-agent.js"; import { normalizeFeishuExternalKey } from "./external-keys.js"; @@ -273,15 +281,34 @@ function resolveFeishuGroupSession(params: { let peerId = chatId; switch (groupSessionScope) { case "group_sender": - peerId = `${chatId}:sender:${senderOpenId}`; + peerId = buildFeishuConversationId({ + chatId, + scope: "group_sender", + senderOpenId, + }); break; case "group_topic": - peerId = topicScope ? `${chatId}:topic:${topicScope}` : chatId; + peerId = topicScope + ? buildFeishuConversationId({ + chatId, + scope: "group_topic", + topicId: topicScope, + }) + : chatId; break; case "group_topic_sender": peerId = topicScope - ? `${chatId}:topic:${topicScope}:sender:${senderOpenId}` - : `${chatId}:sender:${senderOpenId}`; + ? buildFeishuConversationId({ + chatId, + scope: "group_topic_sender", + topicId: topicScope, + senderOpenId, + }) + : buildFeishuConversationId({ + chatId, + scope: "group_sender", + senderOpenId, + }); break; case "group": default: @@ -1168,6 +1195,10 @@ export async function handleFeishuMessage(params: { const peerId = isGroup ? (groupSession?.peerId ?? ctx.chatId) : ctx.senderOpenId; const parentPeer = isGroup ? (groupSession?.parentPeer ?? null) : null; const replyInThread = isGroup ? (groupSession?.replyInThread ?? false) : false; + const feishuAcpConversationSupported = + !isGroup || + groupSession?.groupSessionScope === "group_topic" || + groupSession?.groupSessionScope === "group_topic_sender"; if (isGroup && groupSession) { log( @@ -1216,6 +1247,76 @@ export async function handleFeishuMessage(params: { } } + const currentConversationId = peerId; + const parentConversationId = isGroup ? (parentPeer?.id ?? ctx.chatId) : undefined; + let configuredBinding = null; + if (feishuAcpConversationSupported) { + const configuredRoute = resolveConfiguredAcpRoute({ + cfg: effectiveCfg, + route, + channel: "feishu", + accountId: account.accountId, + conversationId: currentConversationId, + parentConversationId, + }); + configuredBinding = configuredRoute.configuredBinding; + route = configuredRoute.route; + + // Bound Feishu conversations intentionally require an exact live conversation-id match. + // Sender-scoped topic sessions therefore bind on `chat:topic:root:sender:user`, while + // configured ACP bindings may still inherit the shared `chat:topic:root` topic session. + const threadBinding = getSessionBindingService().resolveByConversation({ + channel: "feishu", + accountId: account.accountId, + conversationId: currentConversationId, + ...(parentConversationId ? { parentConversationId } : {}), + }); + const boundSessionKey = threadBinding?.targetSessionKey?.trim(); + if (threadBinding && boundSessionKey) { + route = { + ...route, + sessionKey: boundSessionKey, + agentId: resolveAgentIdFromSessionKey(boundSessionKey) || route.agentId, + lastRoutePolicy: deriveLastRoutePolicy({ + sessionKey: boundSessionKey, + mainSessionKey: route.mainSessionKey, + }), + matchedBy: "binding.channel", + }; + configuredBinding = null; + getSessionBindingService().touch(threadBinding.bindingId); + log( + `feishu[${account.accountId}]: routed via bound conversation ${currentConversationId} -> ${boundSessionKey}`, + ); + } + } + + if (configuredBinding) { + const ensured = await ensureConfiguredAcpRouteReady({ + cfg: effectiveCfg, + configuredBinding, + }); + if (!ensured.ok) { + const replyTargetMessageId = + isGroup && + (groupSession?.groupSessionScope === "group_topic" || + groupSession?.groupSessionScope === "group_topic_sender") + ? (ctx.rootId ?? ctx.messageId) + : ctx.messageId; + await sendMessageFeishu({ + cfg: effectiveCfg, + to: `chat:${ctx.chatId}`, + text: `⚠️ Failed to initialize the configured ACP session for this Feishu conversation: ${ensured.error}`, + replyToMessageId: replyTargetMessageId, + replyInThread: isGroup ? (groupSession?.replyInThread ?? false) : false, + accountId: account.accountId, + }).catch((err) => { + log(`feishu[${account.accountId}]: failed to send ACP init error reply: ${String(err)}`); + }); + return; + } + } + const preview = ctx.content.replace(/\s+/g, " ").slice(0, 160); const inboundLabel = isGroup ? `Feishu[${account.accountId}] message in group ${ctx.chatId}` diff --git a/extensions/feishu/src/conversation-id.ts b/extensions/feishu/src/conversation-id.ts new file mode 100644 index 00000000000..39cb8cc74b6 --- /dev/null +++ b/extensions/feishu/src/conversation-id.ts @@ -0,0 +1,125 @@ +export type FeishuGroupSessionScope = + | "group" + | "group_sender" + | "group_topic" + | "group_topic_sender"; + +function normalizeText(value: unknown): string | undefined { + if (typeof value !== "string") { + return undefined; + } + const trimmed = value.trim(); + return trimmed || undefined; +} + +export function buildFeishuConversationId(params: { + chatId: string; + scope: FeishuGroupSessionScope; + senderOpenId?: string; + topicId?: string; +}): string { + const chatId = normalizeText(params.chatId) ?? "unknown"; + const senderOpenId = normalizeText(params.senderOpenId); + const topicId = normalizeText(params.topicId); + + switch (params.scope) { + case "group_sender": + return senderOpenId ? `${chatId}:sender:${senderOpenId}` : chatId; + case "group_topic": + return topicId ? `${chatId}:topic:${topicId}` : chatId; + case "group_topic_sender": + if (topicId && senderOpenId) { + return `${chatId}:topic:${topicId}:sender:${senderOpenId}`; + } + if (topicId) { + return `${chatId}:topic:${topicId}`; + } + return senderOpenId ? `${chatId}:sender:${senderOpenId}` : chatId; + case "group": + default: + return chatId; + } +} + +export function parseFeishuConversationId(params: { + conversationId: string; + parentConversationId?: string; +}): { + canonicalConversationId: string; + chatId: string; + topicId?: string; + senderOpenId?: string; + scope: FeishuGroupSessionScope; +} | null { + const conversationId = normalizeText(params.conversationId); + const parentConversationId = normalizeText(params.parentConversationId); + if (!conversationId) { + return null; + } + + const topicSenderMatch = conversationId.match(/^(.+):topic:([^:]+):sender:([^:]+)$/); + if (topicSenderMatch) { + const [, chatId, topicId, senderOpenId] = topicSenderMatch; + return { + canonicalConversationId: buildFeishuConversationId({ + chatId, + scope: "group_topic_sender", + topicId, + senderOpenId, + }), + chatId, + topicId, + senderOpenId, + scope: "group_topic_sender", + }; + } + + const topicMatch = conversationId.match(/^(.+):topic:([^:]+)$/); + if (topicMatch) { + const [, chatId, topicId] = topicMatch; + return { + canonicalConversationId: buildFeishuConversationId({ + chatId, + scope: "group_topic", + topicId, + }), + chatId, + topicId, + scope: "group_topic", + }; + } + + const senderMatch = conversationId.match(/^(.+):sender:([^:]+)$/); + if (senderMatch) { + const [, chatId, senderOpenId] = senderMatch; + return { + canonicalConversationId: buildFeishuConversationId({ + chatId, + scope: "group_sender", + senderOpenId, + }), + chatId, + senderOpenId, + scope: "group_sender", + }; + } + + if (parentConversationId) { + return { + canonicalConversationId: buildFeishuConversationId({ + chatId: parentConversationId, + scope: "group_topic", + topicId: conversationId, + }), + chatId: parentConversationId, + topicId: conversationId, + scope: "group_topic", + }; + } + + return { + canonicalConversationId: conversationId, + chatId: conversationId, + scope: "group", + }; +} diff --git a/extensions/feishu/src/monitor.account.ts b/extensions/feishu/src/monitor.account.ts index 6bc990a8d1e..3d761631399 100644 --- a/extensions/feishu/src/monitor.account.ts +++ b/extensions/feishu/src/monitor.account.ts @@ -24,6 +24,7 @@ import { botNames, botOpenIds } from "./monitor.state.js"; import { monitorWebhook, monitorWebSocket } from "./monitor.transport.js"; import { getFeishuRuntime } from "./runtime.js"; import { getMessageFeishu } from "./send.js"; +import { createFeishuThreadBindingManager } from "./thread-bindings.js"; import type { FeishuChatType, ResolvedFeishuAccount } from "./types.js"; const FEISHU_REACTION_VERIFY_TIMEOUT_MS = 1_500; @@ -631,19 +632,25 @@ export async function monitorSingleAccount(params: MonitorSingleAccountParams): log(`feishu[${accountId}]: dedup warmup loaded ${warmupCount} entries from disk`); } - const eventDispatcher = createEventDispatcher(account); - const chatHistories = new Map(); + let threadBindingManager: ReturnType | null = null; + try { + const eventDispatcher = createEventDispatcher(account); + const chatHistories = new Map(); + threadBindingManager = createFeishuThreadBindingManager({ accountId, cfg }); - registerEventHandlers(eventDispatcher, { - cfg, - accountId, - runtime, - chatHistories, - fireAndForget: true, - }); + registerEventHandlers(eventDispatcher, { + cfg, + accountId, + runtime, + chatHistories, + fireAndForget: true, + }); - if (connectionMode === "webhook") { - return monitorWebhook({ account, accountId, runtime, abortSignal, eventDispatcher }); + if (connectionMode === "webhook") { + return await monitorWebhook({ account, accountId, runtime, abortSignal, eventDispatcher }); + } + return await monitorWebSocket({ account, accountId, runtime, abortSignal, eventDispatcher }); + } finally { + threadBindingManager?.stop(); } - return monitorWebSocket({ account, accountId, runtime, abortSignal, eventDispatcher }); } diff --git a/extensions/feishu/src/monitor.reaction.test.ts b/extensions/feishu/src/monitor.reaction.test.ts index 49da928ea3b..001b8140f80 100644 --- a/extensions/feishu/src/monitor.reaction.test.ts +++ b/extensions/feishu/src/monitor.reaction.test.ts @@ -17,6 +17,7 @@ const handleFeishuMessageMock = vi.hoisted(() => vi.fn(async (_params: { event?: const createEventDispatcherMock = vi.hoisted(() => vi.fn()); const monitorWebSocketMock = vi.hoisted(() => vi.fn(async () => {})); const monitorWebhookMock = vi.hoisted(() => vi.fn(async () => {})); +const createFeishuThreadBindingManagerMock = vi.hoisted(() => vi.fn(() => ({ stop: vi.fn() }))); let handlers: Record Promise> = {}; @@ -37,6 +38,10 @@ vi.mock("./monitor.transport.js", () => ({ monitorWebhook: monitorWebhookMock, })); +vi.mock("./thread-bindings.js", () => ({ + createFeishuThreadBindingManager: createFeishuThreadBindingManagerMock, +})); + const cfg = {} as ClawdbotConfig; function makeReactionEvent( @@ -419,6 +424,94 @@ describe("resolveReactionSyntheticEvent", () => { }); }); +describe("monitorSingleAccount lifecycle", () => { + beforeEach(() => { + createFeishuThreadBindingManagerMock.mockReset().mockImplementation(() => ({ + stop: vi.fn(), + })); + createEventDispatcherMock.mockReset().mockReturnValue({ + register: vi.fn(), + }); + }); + + it("stops the Feishu thread binding manager when the monitor exits", async () => { + setFeishuRuntime( + createPluginRuntimeMock({ + channel: { + debounce: { + resolveInboundDebounceMs, + createInboundDebouncer, + }, + text: { + hasControlCommand, + }, + }, + }), + ); + + await monitorSingleAccount({ + cfg: buildDebounceConfig(), + account: buildDebounceAccount(), + runtime: { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn(), + } as RuntimeEnv, + botOpenIdSource: { + kind: "prefetched", + botOpenId: "ou_bot", + }, + }); + + const manager = createFeishuThreadBindingManagerMock.mock.results[0]?.value as + | { stop: ReturnType } + | undefined; + expect(manager?.stop).toHaveBeenCalledTimes(1); + }); + + it("stops the Feishu thread binding manager when setup fails before transport starts", async () => { + setFeishuRuntime( + createPluginRuntimeMock({ + channel: { + debounce: { + resolveInboundDebounceMs, + createInboundDebouncer, + }, + text: { + hasControlCommand, + }, + }, + }), + ); + createEventDispatcherMock.mockReturnValue({ + get register() { + throw new Error("register failed"); + }, + }); + + await expect( + monitorSingleAccount({ + cfg: buildDebounceConfig(), + account: buildDebounceAccount(), + runtime: { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn(), + } as RuntimeEnv, + botOpenIdSource: { + kind: "prefetched", + botOpenId: "ou_bot", + }, + }), + ).rejects.toThrow("register failed"); + + const manager = createFeishuThreadBindingManagerMock.mock.results[0]?.value as + | { stop: ReturnType } + | undefined; + expect(manager?.stop).toHaveBeenCalledTimes(1); + }); +}); + describe("Feishu inbound debounce regressions", () => { beforeEach(() => { vi.useFakeTimers(); diff --git a/extensions/feishu/src/subagent-hooks.test.ts b/extensions/feishu/src/subagent-hooks.test.ts new file mode 100644 index 00000000000..a86e8996f35 --- /dev/null +++ b/extensions/feishu/src/subagent-hooks.test.ts @@ -0,0 +1,623 @@ +import type { OpenClawPluginApi } from "openclaw/plugin-sdk/feishu"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { registerFeishuSubagentHooks } from "./subagent-hooks.js"; +import { + __testing as threadBindingTesting, + createFeishuThreadBindingManager, +} from "./thread-bindings.js"; + +const baseConfig = { + session: { mainKey: "main", scope: "per-sender" }, + channels: { feishu: {} }, +}; + +function registerHandlersForTest(config: Record = baseConfig) { + const handlers = new Map unknown>(); + const api = { + config, + on: (hookName: string, handler: (event: unknown, ctx: unknown) => unknown) => { + handlers.set(hookName, handler); + }, + } as unknown as OpenClawPluginApi; + registerFeishuSubagentHooks(api); + return handlers; +} + +function getRequiredHandler( + handlers: Map unknown>, + hookName: string, +): (event: unknown, ctx: unknown) => unknown { + const handler = handlers.get(hookName); + if (!handler) { + throw new Error(`expected ${hookName} hook handler`); + } + return handler; +} + +describe("feishu subagent hook handlers", () => { + beforeEach(() => { + threadBindingTesting.resetFeishuThreadBindingsForTests(); + }); + + it("registers Feishu subagent hooks", () => { + const handlers = registerHandlersForTest(); + expect(handlers.has("subagent_spawning")).toBe(true); + expect(handlers.has("subagent_delivery_target")).toBe(true); + expect(handlers.has("subagent_ended")).toBe(true); + expect(handlers.has("subagent_spawned")).toBe(false); + }); + + it("binds a Feishu DM conversation on subagent_spawning", async () => { + const handlers = registerHandlersForTest(); + const handler = getRequiredHandler(handlers, "subagent_spawning"); + createFeishuThreadBindingManager({ cfg: baseConfig as any, accountId: "work" }); + + const result = await handler( + { + childSessionKey: "agent:main:subagent:child", + agentId: "codex", + label: "banana", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_1", + }, + threadRequested: true, + }, + {}, + ); + + expect(result).toEqual({ status: "ok", threadBindingReady: true }); + + const deliveryTargetHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + expect( + deliveryTargetHandler( + { + childSessionKey: "agent:main:subagent:child", + requesterSessionKey: "agent:main:main", + requesterOrigin: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_1", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toEqual({ + origin: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_1", + }, + }); + }); + + it("preserves the original Feishu DM delivery target", async () => { + const handlers = registerHandlersForTest(); + const deliveryHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + const manager = createFeishuThreadBindingManager({ cfg: baseConfig as any, accountId: "work" }); + + manager.bindConversation({ + conversationId: "ou_sender_1", + targetKind: "subagent", + targetSessionKey: "agent:main:subagent:chat-dm-child", + metadata: { + deliveryTo: "chat:oc_dm_chat_1", + boundBy: "system", + }, + }); + + expect( + deliveryHandler( + { + childSessionKey: "agent:main:subagent:chat-dm-child", + requesterSessionKey: "agent:main:main", + requesterOrigin: { + channel: "feishu", + accountId: "work", + to: "chat:oc_dm_chat_1", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toEqual({ + origin: { + channel: "feishu", + accountId: "work", + to: "chat:oc_dm_chat_1", + }, + }); + }); + + it("binds a Feishu topic conversation and preserves parent context", async () => { + const handlers = registerHandlersForTest(); + const spawnHandler = getRequiredHandler(handlers, "subagent_spawning"); + const deliveryHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + createFeishuThreadBindingManager({ cfg: baseConfig as any, accountId: "work" }); + + const result = await spawnHandler( + { + childSessionKey: "agent:main:subagent:topic-child", + agentId: "codex", + label: "topic-child", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + threadRequested: true, + }, + {}, + ); + + expect(result).toEqual({ status: "ok", threadBindingReady: true }); + expect( + deliveryHandler( + { + childSessionKey: "agent:main:subagent:topic-child", + requesterSessionKey: "agent:main:main", + requesterOrigin: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toEqual({ + origin: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + }); + }); + + it("uses the requester session binding to preserve sender-scoped topic conversations", async () => { + const handlers = registerHandlersForTest(); + const spawnHandler = getRequiredHandler(handlers, "subagent_spawning"); + const deliveryHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + const manager = createFeishuThreadBindingManager({ cfg: baseConfig as any, accountId: "work" }); + + manager.bindConversation({ + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_sender_1", + parentConversationId: "oc_group_chat", + targetKind: "subagent", + targetSessionKey: "agent:main:parent", + metadata: { + agentId: "codex", + label: "parent", + boundBy: "system", + }, + }); + + const reboundResult = await spawnHandler( + { + childSessionKey: "agent:main:subagent:sender-child", + agentId: "codex", + label: "sender-child", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + threadRequested: true, + }, + { + requesterSessionKey: "agent:main:parent", + }, + ); + + expect(reboundResult).toEqual({ status: "ok", threadBindingReady: true }); + expect(manager.listBySessionKey("agent:main:subagent:sender-child")).toMatchObject([ + { + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_sender_1", + parentConversationId: "oc_group_chat", + }, + ]); + expect( + deliveryHandler( + { + childSessionKey: "agent:main:subagent:sender-child", + requesterSessionKey: "agent:main:parent", + requesterOrigin: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toEqual({ + origin: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + }); + }); + + it("prefers requester-matching bindings when multiple child bindings exist", async () => { + const handlers = registerHandlersForTest(); + const spawnHandler = getRequiredHandler(handlers, "subagent_spawning"); + const deliveryHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + createFeishuThreadBindingManager({ cfg: baseConfig as any, accountId: "work" }); + + await spawnHandler( + { + childSessionKey: "agent:main:subagent:shared", + agentId: "codex", + label: "shared", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_1", + }, + threadRequested: true, + }, + {}, + ); + await spawnHandler( + { + childSessionKey: "agent:main:subagent:shared", + agentId: "codex", + label: "shared", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_2", + }, + threadRequested: true, + }, + {}, + ); + + expect( + deliveryHandler( + { + childSessionKey: "agent:main:subagent:shared", + requesterSessionKey: "agent:main:main", + requesterOrigin: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_2", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toEqual({ + origin: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_2", + }, + }); + }); + + it("fails closed when requester-session bindings remain ambiguous for the same topic", async () => { + const handlers = registerHandlersForTest(); + const spawnHandler = getRequiredHandler(handlers, "subagent_spawning"); + const deliveryHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + const manager = createFeishuThreadBindingManager({ cfg: baseConfig as any, accountId: "work" }); + + manager.bindConversation({ + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_sender_1", + parentConversationId: "oc_group_chat", + targetKind: "subagent", + targetSessionKey: "agent:main:parent", + metadata: { boundBy: "system" }, + }); + manager.bindConversation({ + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_sender_2", + parentConversationId: "oc_group_chat", + targetKind: "subagent", + targetSessionKey: "agent:main:parent", + metadata: { boundBy: "system" }, + }); + + await expect( + spawnHandler( + { + childSessionKey: "agent:main:subagent:ambiguous-child", + agentId: "codex", + label: "ambiguous-child", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + threadRequested: true, + }, + { + requesterSessionKey: "agent:main:parent", + }, + ), + ).resolves.toMatchObject({ + status: "error", + error: expect.stringContaining("direct messages or topic conversations"), + }); + + expect( + deliveryHandler( + { + childSessionKey: "agent:main:subagent:ambiguous-child", + requesterSessionKey: "agent:main:parent", + requesterOrigin: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toBeUndefined(); + }); + + it("fails closed when both topic-level and sender-scoped requester bindings exist", async () => { + const handlers = registerHandlersForTest(); + const spawnHandler = getRequiredHandler(handlers, "subagent_spawning"); + const deliveryHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + const manager = createFeishuThreadBindingManager({ cfg: baseConfig as any, accountId: "work" }); + + manager.bindConversation({ + conversationId: "oc_group_chat:topic:om_topic_root", + parentConversationId: "oc_group_chat", + targetKind: "subagent", + targetSessionKey: "agent:main:parent", + metadata: { boundBy: "system" }, + }); + manager.bindConversation({ + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_sender_1", + parentConversationId: "oc_group_chat", + targetKind: "subagent", + targetSessionKey: "agent:main:parent", + metadata: { boundBy: "system" }, + }); + + await expect( + spawnHandler( + { + childSessionKey: "agent:main:subagent:mixed-topic-child", + agentId: "codex", + label: "mixed-topic-child", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + threadRequested: true, + }, + { + requesterSessionKey: "agent:main:parent", + }, + ), + ).resolves.toMatchObject({ + status: "error", + error: expect.stringContaining("direct messages or topic conversations"), + }); + + expect( + deliveryHandler( + { + childSessionKey: "agent:main:subagent:mixed-topic-child", + requesterSessionKey: "agent:main:parent", + requesterOrigin: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + threadId: "om_topic_root", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toBeUndefined(); + }); + + it("no-ops for non-Feishu channels and non-threaded spawns", async () => { + const handlers = registerHandlersForTest(); + const spawnHandler = getRequiredHandler(handlers, "subagent_spawning"); + const deliveryHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + const endedHandler = getRequiredHandler(handlers, "subagent_ended"); + + await expect( + spawnHandler( + { + childSessionKey: "agent:main:subagent:child", + agentId: "codex", + mode: "run", + requester: { + channel: "discord", + accountId: "work", + to: "channel:123", + }, + threadRequested: true, + }, + {}, + ), + ).resolves.toBeUndefined(); + + await expect( + spawnHandler( + { + childSessionKey: "agent:main:subagent:child", + agentId: "codex", + mode: "run", + requester: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_1", + }, + threadRequested: false, + }, + {}, + ), + ).resolves.toBeUndefined(); + + expect( + deliveryHandler( + { + childSessionKey: "agent:main:subagent:child", + requesterSessionKey: "agent:main:main", + requesterOrigin: { + channel: "discord", + accountId: "work", + to: "channel:123", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toBeUndefined(); + + expect( + endedHandler( + { + targetSessionKey: "agent:main:subagent:child", + targetKind: "subagent", + reason: "done", + accountId: "work", + }, + {}, + ), + ).toBeUndefined(); + }); + + it("returns an error for unsupported non-topic Feishu group conversations", async () => { + const handler = getRequiredHandler(registerHandlersForTest(), "subagent_spawning"); + createFeishuThreadBindingManager({ cfg: baseConfig as any, accountId: "work" }); + + await expect( + handler( + { + childSessionKey: "agent:main:subagent:child", + agentId: "codex", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "chat:oc_group_chat", + }, + threadRequested: true, + }, + {}, + ), + ).resolves.toMatchObject({ + status: "error", + error: expect.stringContaining("direct messages or topic conversations"), + }); + }); + + it("unbinds Feishu bindings on subagent_ended", async () => { + const handlers = registerHandlersForTest(); + const spawnHandler = getRequiredHandler(handlers, "subagent_spawning"); + const deliveryHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + const endedHandler = getRequiredHandler(handlers, "subagent_ended"); + createFeishuThreadBindingManager({ cfg: baseConfig as any, accountId: "work" }); + + await spawnHandler( + { + childSessionKey: "agent:main:subagent:child", + agentId: "codex", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_1", + }, + threadRequested: true, + }, + {}, + ); + + endedHandler( + { + targetSessionKey: "agent:main:subagent:child", + targetKind: "subagent", + reason: "done", + accountId: "work", + }, + {}, + ); + + expect( + deliveryHandler( + { + childSessionKey: "agent:main:subagent:child", + requesterSessionKey: "agent:main:main", + requesterOrigin: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_1", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toBeUndefined(); + }); + + it("fails closed when the Feishu monitor-owned binding manager is unavailable", async () => { + const handlers = registerHandlersForTest(); + const spawnHandler = getRequiredHandler(handlers, "subagent_spawning"); + const deliveryHandler = getRequiredHandler(handlers, "subagent_delivery_target"); + + await expect( + spawnHandler( + { + childSessionKey: "agent:main:subagent:no-manager", + agentId: "codex", + mode: "session", + requester: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_1", + }, + threadRequested: true, + }, + {}, + ), + ).resolves.toMatchObject({ + status: "error", + error: expect.stringContaining("monitor is not active"), + }); + + expect( + deliveryHandler( + { + childSessionKey: "agent:main:subagent:no-manager", + requesterSessionKey: "agent:main:main", + requesterOrigin: { + channel: "feishu", + accountId: "work", + to: "user:ou_sender_1", + }, + expectsCompletionMessage: true, + }, + {}, + ), + ).toBeUndefined(); + }); +}); diff --git a/extensions/feishu/src/subagent-hooks.ts b/extensions/feishu/src/subagent-hooks.ts new file mode 100644 index 00000000000..6b048f8fbcf --- /dev/null +++ b/extensions/feishu/src/subagent-hooks.ts @@ -0,0 +1,341 @@ +import type { OpenClawPluginApi } from "openclaw/plugin-sdk/feishu"; +import { buildFeishuConversationId, parseFeishuConversationId } from "./conversation-id.js"; +import { normalizeFeishuTarget } from "./targets.js"; +import { getFeishuThreadBindingManager } from "./thread-bindings.js"; + +function summarizeError(err: unknown): string { + if (err instanceof Error) { + return err.message; + } + if (typeof err === "string") { + return err; + } + return "error"; +} + +function stripProviderPrefix(raw: string): string { + return raw.replace(/^(feishu|lark):/i, "").trim(); +} + +function resolveFeishuRequesterConversation(params: { + accountId?: string; + to?: string; + threadId?: string | number; + requesterSessionKey?: string; +}): { + accountId: string; + conversationId: string; + parentConversationId?: string; +} | null { + const manager = getFeishuThreadBindingManager(params.accountId); + if (!manager) { + return null; + } + const rawTo = params.to?.trim(); + const withoutProviderPrefix = rawTo ? stripProviderPrefix(rawTo) : ""; + const normalizedTarget = rawTo ? normalizeFeishuTarget(rawTo) : null; + const threadId = + params.threadId != null && params.threadId !== "" ? String(params.threadId).trim() : ""; + const isChatTarget = /^(chat|group|channel):/i.test(withoutProviderPrefix); + const parsedRequesterTopic = + normalizedTarget && threadId && isChatTarget + ? parseFeishuConversationId({ + conversationId: buildFeishuConversationId({ + chatId: normalizedTarget, + scope: "group_topic", + topicId: threadId, + }), + parentConversationId: normalizedTarget, + }) + : null; + const requesterSessionKey = params.requesterSessionKey?.trim(); + if (requesterSessionKey) { + const existingBindings = manager.listBySessionKey(requesterSessionKey); + if (existingBindings.length === 1) { + const existing = existingBindings[0]; + return { + accountId: existing.accountId, + conversationId: existing.conversationId, + parentConversationId: existing.parentConversationId, + }; + } + if (existingBindings.length > 1) { + if (rawTo && normalizedTarget && !threadId && !isChatTarget) { + const directMatches = existingBindings.filter( + (entry) => + entry.accountId === manager.accountId && + entry.conversationId === normalizedTarget && + !entry.parentConversationId, + ); + if (directMatches.length === 1) { + const existing = directMatches[0]; + return { + accountId: existing.accountId, + conversationId: existing.conversationId, + parentConversationId: existing.parentConversationId, + }; + } + return null; + } + if (parsedRequesterTopic) { + const matchingTopicBindings = existingBindings.filter((entry) => { + const parsed = parseFeishuConversationId({ + conversationId: entry.conversationId, + parentConversationId: entry.parentConversationId, + }); + return ( + parsed?.chatId === parsedRequesterTopic.chatId && + parsed?.topicId === parsedRequesterTopic.topicId + ); + }); + if (matchingTopicBindings.length === 1) { + const existing = matchingTopicBindings[0]; + return { + accountId: existing.accountId, + conversationId: existing.conversationId, + parentConversationId: existing.parentConversationId, + }; + } + const senderScopedTopicBindings = matchingTopicBindings.filter((entry) => { + const parsed = parseFeishuConversationId({ + conversationId: entry.conversationId, + parentConversationId: entry.parentConversationId, + }); + return parsed?.scope === "group_topic_sender"; + }); + if ( + senderScopedTopicBindings.length === 1 && + matchingTopicBindings.length === senderScopedTopicBindings.length + ) { + const existing = senderScopedTopicBindings[0]; + return { + accountId: existing.accountId, + conversationId: existing.conversationId, + parentConversationId: existing.parentConversationId, + }; + } + return null; + } + } + } + + if (!rawTo) { + return null; + } + if (!normalizedTarget) { + return null; + } + + if (threadId) { + if (!isChatTarget) { + return null; + } + return { + accountId: manager.accountId, + conversationId: buildFeishuConversationId({ + chatId: normalizedTarget, + scope: "group_topic", + topicId: threadId, + }), + parentConversationId: normalizedTarget, + }; + } + + if (isChatTarget) { + return null; + } + + return { + accountId: manager.accountId, + conversationId: normalizedTarget, + }; +} + +function resolveFeishuDeliveryOrigin(params: { + conversationId: string; + parentConversationId?: string; + accountId: string; + deliveryTo?: string; + deliveryThreadId?: string; +}): { + channel: "feishu"; + accountId: string; + to: string; + threadId?: string; +} { + const deliveryTo = params.deliveryTo?.trim(); + const deliveryThreadId = params.deliveryThreadId?.trim(); + if (deliveryTo) { + return { + channel: "feishu", + accountId: params.accountId, + to: deliveryTo, + ...(deliveryThreadId ? { threadId: deliveryThreadId } : {}), + }; + } + const parsed = parseFeishuConversationId({ + conversationId: params.conversationId, + parentConversationId: params.parentConversationId, + }); + if (parsed?.topicId) { + return { + channel: "feishu", + accountId: params.accountId, + to: `chat:${params.parentConversationId?.trim() || parsed.chatId}`, + threadId: parsed.topicId, + }; + } + return { + channel: "feishu", + accountId: params.accountId, + to: `user:${params.conversationId}`, + }; +} + +function resolveMatchingChildBinding(params: { + accountId?: string; + childSessionKey: string; + requesterSessionKey?: string; + requesterOrigin?: { + to?: string; + threadId?: string | number; + }; +}) { + const manager = getFeishuThreadBindingManager(params.accountId); + if (!manager) { + return null; + } + const childBindings = manager.listBySessionKey(params.childSessionKey.trim()); + if (childBindings.length === 0) { + return null; + } + + const requesterConversation = resolveFeishuRequesterConversation({ + accountId: manager.accountId, + to: params.requesterOrigin?.to, + threadId: params.requesterOrigin?.threadId, + requesterSessionKey: params.requesterSessionKey, + }); + if (requesterConversation) { + const matched = childBindings.find( + (entry) => + entry.accountId === requesterConversation.accountId && + entry.conversationId === requesterConversation.conversationId && + (entry.parentConversationId?.trim() || undefined) === + (requesterConversation.parentConversationId?.trim() || undefined), + ); + if (matched) { + return matched; + } + } + + return childBindings.length === 1 ? childBindings[0] : null; +} + +export function registerFeishuSubagentHooks(api: OpenClawPluginApi) { + api.on("subagent_spawning", async (event, ctx) => { + if (!event.threadRequested) { + return; + } + const requesterChannel = event.requester?.channel?.trim().toLowerCase(); + if (requesterChannel !== "feishu") { + return; + } + + const manager = getFeishuThreadBindingManager(event.requester?.accountId); + if (!manager) { + return { + status: "error" as const, + error: + "Feishu current-conversation binding is unavailable because the Feishu account monitor is not active.", + }; + } + + const conversation = resolveFeishuRequesterConversation({ + accountId: event.requester?.accountId, + to: event.requester?.to, + threadId: event.requester?.threadId, + requesterSessionKey: ctx.requesterSessionKey, + }); + if (!conversation) { + return { + status: "error" as const, + error: + "Feishu current-conversation binding is only available in direct messages or topic conversations.", + }; + } + + try { + const binding = manager.bindConversation({ + conversationId: conversation.conversationId, + parentConversationId: conversation.parentConversationId, + targetKind: "subagent", + targetSessionKey: event.childSessionKey, + metadata: { + agentId: event.agentId, + label: event.label, + boundBy: "system", + deliveryTo: event.requester?.to, + deliveryThreadId: + event.requester?.threadId != null && event.requester.threadId !== "" + ? String(event.requester.threadId) + : undefined, + }, + }); + if (!binding) { + return { + status: "error" as const, + error: + "Unable to bind this Feishu conversation to the spawned subagent session. Session mode is unavailable for this target.", + }; + } + return { + status: "ok" as const, + threadBindingReady: true, + }; + } catch (err) { + return { + status: "error" as const, + error: `Feishu conversation bind failed: ${summarizeError(err)}`, + }; + } + }); + + api.on("subagent_delivery_target", (event) => { + if (!event.expectsCompletionMessage) { + return; + } + const requesterChannel = event.requesterOrigin?.channel?.trim().toLowerCase(); + if (requesterChannel !== "feishu") { + return; + } + + const binding = resolveMatchingChildBinding({ + accountId: event.requesterOrigin?.accountId, + childSessionKey: event.childSessionKey, + requesterSessionKey: event.requesterSessionKey, + requesterOrigin: { + to: event.requesterOrigin?.to, + threadId: event.requesterOrigin?.threadId, + }, + }); + if (!binding) { + return; + } + + return { + origin: resolveFeishuDeliveryOrigin({ + conversationId: binding.conversationId, + parentConversationId: binding.parentConversationId, + accountId: binding.accountId, + deliveryTo: binding.deliveryTo, + deliveryThreadId: binding.deliveryThreadId, + }), + }; + }); + + api.on("subagent_ended", (event) => { + const manager = getFeishuThreadBindingManager(event.accountId); + manager?.unbindBySessionKey(event.targetSessionKey); + }); +} diff --git a/extensions/feishu/src/thread-bindings.test.ts b/extensions/feishu/src/thread-bindings.test.ts new file mode 100644 index 00000000000..a118926df57 --- /dev/null +++ b/extensions/feishu/src/thread-bindings.test.ts @@ -0,0 +1,94 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../../src/config/config.js"; +import { getSessionBindingService } from "../../../src/infra/outbound/session-binding-service.js"; +import { __testing, createFeishuThreadBindingManager } from "./thread-bindings.js"; + +const baseCfg = { + session: { mainKey: "main", scope: "per-sender" }, +} satisfies OpenClawConfig; + +describe("Feishu thread bindings", () => { + beforeEach(() => { + __testing.resetFeishuThreadBindingsForTests(); + }); + + it("registers current-placement adapter capabilities for Feishu", () => { + createFeishuThreadBindingManager({ cfg: baseCfg, accountId: "default" }); + + expect( + getSessionBindingService().getCapabilities({ + channel: "feishu", + accountId: "default", + }), + ).toEqual({ + adapterAvailable: true, + bindSupported: true, + unbindSupported: true, + placements: ["current"], + }); + }); + + it("binds and resolves a Feishu topic conversation", async () => { + createFeishuThreadBindingManager({ cfg: baseCfg, accountId: "default" }); + + const binding = await getSessionBindingService().bind({ + targetSessionKey: "agent:codex:acp:binding:feishu:default:abc123", + targetKind: "session", + conversation: { + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_topic_root", + parentConversationId: "oc_group_chat", + }, + placement: "current", + metadata: { + agentId: "codex", + label: "codex-main", + }, + }); + + expect(binding.conversation.conversationId).toBe("oc_group_chat:topic:om_topic_root"); + expect( + getSessionBindingService().resolveByConversation({ + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_topic_root", + }), + )?.toMatchObject({ + targetSessionKey: "agent:codex:acp:binding:feishu:default:abc123", + metadata: expect.objectContaining({ + agentId: "codex", + label: "codex-main", + }), + }); + }); + + it("clears account-scoped bindings when the manager stops", async () => { + const manager = createFeishuThreadBindingManager({ cfg: baseCfg, accountId: "default" }); + + await getSessionBindingService().bind({ + targetSessionKey: "agent:codex:acp:binding:feishu:default:abc123", + targetKind: "session", + conversation: { + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_topic_root", + parentConversationId: "oc_group_chat", + }, + placement: "current", + metadata: { + agentId: "codex", + }, + }); + + manager.stop(); + + expect( + getSessionBindingService().resolveByConversation({ + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_topic_root", + }), + ).toBeNull(); + }); +}); diff --git a/extensions/feishu/src/thread-bindings.ts b/extensions/feishu/src/thread-bindings.ts new file mode 100644 index 00000000000..b2ab72467c3 --- /dev/null +++ b/extensions/feishu/src/thread-bindings.ts @@ -0,0 +1,316 @@ +import { resolveThreadBindingConversationIdFromBindingId } from "../../../src/channels/thread-binding-id.js"; +import { + resolveThreadBindingIdleTimeoutMsForChannel, + resolveThreadBindingMaxAgeMsForChannel, +} from "../../../src/channels/thread-bindings-policy.js"; +import type { OpenClawConfig } from "../../../src/config/config.js"; +import { + registerSessionBindingAdapter, + unregisterSessionBindingAdapter, + type BindingTargetKind, + type SessionBindingRecord, +} from "../../../src/infra/outbound/session-binding-service.js"; +import { + normalizeAccountId, + resolveAgentIdFromSessionKey, +} from "../../../src/routing/session-key.js"; +import { resolveGlobalSingleton } from "../../../src/shared/global-singleton.js"; + +type FeishuBindingTargetKind = "subagent" | "acp"; + +type FeishuThreadBindingRecord = { + accountId: string; + conversationId: string; + parentConversationId?: string; + deliveryTo?: string; + deliveryThreadId?: string; + targetKind: FeishuBindingTargetKind; + targetSessionKey: string; + agentId?: string; + label?: string; + boundBy?: string; + boundAt: number; + lastActivityAt: number; +}; + +type FeishuThreadBindingManager = { + accountId: string; + getByConversationId: (conversationId: string) => FeishuThreadBindingRecord | undefined; + listBySessionKey: (targetSessionKey: string) => FeishuThreadBindingRecord[]; + bindConversation: (params: { + conversationId: string; + parentConversationId?: string; + targetKind: BindingTargetKind; + targetSessionKey: string; + metadata?: Record; + }) => FeishuThreadBindingRecord | null; + touchConversation: (conversationId: string, at?: number) => FeishuThreadBindingRecord | null; + unbindConversation: (conversationId: string) => FeishuThreadBindingRecord | null; + unbindBySessionKey: (targetSessionKey: string) => FeishuThreadBindingRecord[]; + stop: () => void; +}; + +type FeishuThreadBindingsState = { + managersByAccountId: Map; + bindingsByAccountConversation: Map; +}; + +const FEISHU_THREAD_BINDINGS_STATE_KEY = Symbol.for("openclaw.feishuThreadBindingsState"); +const state = resolveGlobalSingleton( + FEISHU_THREAD_BINDINGS_STATE_KEY, + () => ({ + managersByAccountId: new Map(), + bindingsByAccountConversation: new Map(), + }), +); + +const MANAGERS_BY_ACCOUNT_ID = state.managersByAccountId; +const BINDINGS_BY_ACCOUNT_CONVERSATION = state.bindingsByAccountConversation; + +function resolveBindingKey(params: { accountId: string; conversationId: string }): string { + return `${params.accountId}:${params.conversationId}`; +} + +function toSessionBindingTargetKind(raw: FeishuBindingTargetKind): BindingTargetKind { + return raw === "subagent" ? "subagent" : "session"; +} + +function toFeishuTargetKind(raw: BindingTargetKind): FeishuBindingTargetKind { + return raw === "subagent" ? "subagent" : "acp"; +} + +function toSessionBindingRecord( + record: FeishuThreadBindingRecord, + defaults: { idleTimeoutMs: number; maxAgeMs: number }, +): SessionBindingRecord { + const idleExpiresAt = + defaults.idleTimeoutMs > 0 ? record.lastActivityAt + defaults.idleTimeoutMs : undefined; + const maxAgeExpiresAt = defaults.maxAgeMs > 0 ? record.boundAt + defaults.maxAgeMs : undefined; + const expiresAt = + idleExpiresAt != null && maxAgeExpiresAt != null + ? Math.min(idleExpiresAt, maxAgeExpiresAt) + : (idleExpiresAt ?? maxAgeExpiresAt); + return { + bindingId: resolveBindingKey({ + accountId: record.accountId, + conversationId: record.conversationId, + }), + targetSessionKey: record.targetSessionKey, + targetKind: toSessionBindingTargetKind(record.targetKind), + conversation: { + channel: "feishu", + accountId: record.accountId, + conversationId: record.conversationId, + parentConversationId: record.parentConversationId, + }, + status: "active", + boundAt: record.boundAt, + expiresAt, + metadata: { + agentId: record.agentId, + label: record.label, + boundBy: record.boundBy, + deliveryTo: record.deliveryTo, + deliveryThreadId: record.deliveryThreadId, + lastActivityAt: record.lastActivityAt, + idleTimeoutMs: defaults.idleTimeoutMs, + maxAgeMs: defaults.maxAgeMs, + }, + }; +} + +export function createFeishuThreadBindingManager(params: { + accountId?: string; + cfg: OpenClawConfig; +}): FeishuThreadBindingManager { + const accountId = normalizeAccountId(params.accountId); + const existing = MANAGERS_BY_ACCOUNT_ID.get(accountId); + if (existing) { + return existing; + } + + const idleTimeoutMs = resolveThreadBindingIdleTimeoutMsForChannel({ + cfg: params.cfg, + channel: "feishu", + accountId, + }); + const maxAgeMs = resolveThreadBindingMaxAgeMsForChannel({ + cfg: params.cfg, + channel: "feishu", + accountId, + }); + + const manager: FeishuThreadBindingManager = { + accountId, + getByConversationId: (conversationId) => + BINDINGS_BY_ACCOUNT_CONVERSATION.get(resolveBindingKey({ accountId, conversationId })), + listBySessionKey: (targetSessionKey) => + [...BINDINGS_BY_ACCOUNT_CONVERSATION.values()].filter( + (record) => record.accountId === accountId && record.targetSessionKey === targetSessionKey, + ), + bindConversation: ({ + conversationId, + parentConversationId, + targetKind, + targetSessionKey, + metadata, + }) => { + const normalizedConversationId = conversationId.trim(); + if (!normalizedConversationId || !targetSessionKey.trim()) { + return null; + } + const now = Date.now(); + const record: FeishuThreadBindingRecord = { + accountId, + conversationId: normalizedConversationId, + parentConversationId: parentConversationId?.trim() || undefined, + deliveryTo: + typeof metadata?.deliveryTo === "string" && metadata.deliveryTo.trim() + ? metadata.deliveryTo.trim() + : undefined, + deliveryThreadId: + typeof metadata?.deliveryThreadId === "string" && metadata.deliveryThreadId.trim() + ? metadata.deliveryThreadId.trim() + : undefined, + targetKind: toFeishuTargetKind(targetKind), + targetSessionKey: targetSessionKey.trim(), + agentId: + typeof metadata?.agentId === "string" && metadata.agentId.trim() + ? metadata.agentId.trim() + : resolveAgentIdFromSessionKey(targetSessionKey), + label: + typeof metadata?.label === "string" && metadata.label.trim() + ? metadata.label.trim() + : undefined, + boundBy: + typeof metadata?.boundBy === "string" && metadata.boundBy.trim() + ? metadata.boundBy.trim() + : undefined, + boundAt: now, + lastActivityAt: now, + }; + BINDINGS_BY_ACCOUNT_CONVERSATION.set( + resolveBindingKey({ accountId, conversationId: normalizedConversationId }), + record, + ); + return record; + }, + touchConversation: (conversationId, at = Date.now()) => { + const key = resolveBindingKey({ accountId, conversationId }); + const existingRecord = BINDINGS_BY_ACCOUNT_CONVERSATION.get(key); + if (!existingRecord) { + return null; + } + const updated = { ...existingRecord, lastActivityAt: at }; + BINDINGS_BY_ACCOUNT_CONVERSATION.set(key, updated); + return updated; + }, + unbindConversation: (conversationId) => { + const key = resolveBindingKey({ accountId, conversationId }); + const existingRecord = BINDINGS_BY_ACCOUNT_CONVERSATION.get(key); + if (!existingRecord) { + return null; + } + BINDINGS_BY_ACCOUNT_CONVERSATION.delete(key); + return existingRecord; + }, + unbindBySessionKey: (targetSessionKey) => { + const removed: FeishuThreadBindingRecord[] = []; + for (const record of [...BINDINGS_BY_ACCOUNT_CONVERSATION.values()]) { + if (record.accountId !== accountId || record.targetSessionKey !== targetSessionKey) { + continue; + } + BINDINGS_BY_ACCOUNT_CONVERSATION.delete( + resolveBindingKey({ accountId, conversationId: record.conversationId }), + ); + removed.push(record); + } + return removed; + }, + stop: () => { + for (const key of [...BINDINGS_BY_ACCOUNT_CONVERSATION.keys()]) { + if (key.startsWith(`${accountId}:`)) { + BINDINGS_BY_ACCOUNT_CONVERSATION.delete(key); + } + } + MANAGERS_BY_ACCOUNT_ID.delete(accountId); + unregisterSessionBindingAdapter({ channel: "feishu", accountId }); + }, + }; + + registerSessionBindingAdapter({ + channel: "feishu", + accountId, + capabilities: { + placements: ["current"], + }, + bind: async (input) => { + if (input.conversation.channel !== "feishu" || input.placement === "child") { + return null; + } + const bound = manager.bindConversation({ + conversationId: input.conversation.conversationId, + parentConversationId: input.conversation.parentConversationId, + targetKind: input.targetKind, + targetSessionKey: input.targetSessionKey, + metadata: input.metadata, + }); + return bound ? toSessionBindingRecord(bound, { idleTimeoutMs, maxAgeMs }) : null; + }, + listBySession: (targetSessionKey) => + manager + .listBySessionKey(targetSessionKey) + .map((entry) => toSessionBindingRecord(entry, { idleTimeoutMs, maxAgeMs })), + resolveByConversation: (ref) => { + if (ref.channel !== "feishu") { + return null; + } + const found = manager.getByConversationId(ref.conversationId); + return found ? toSessionBindingRecord(found, { idleTimeoutMs, maxAgeMs }) : null; + }, + touch: (bindingId, at) => { + const conversationId = resolveThreadBindingConversationIdFromBindingId({ + accountId, + bindingId, + }); + if (conversationId) { + manager.touchConversation(conversationId, at); + } + }, + unbind: async (input) => { + if (input.targetSessionKey?.trim()) { + return manager + .unbindBySessionKey(input.targetSessionKey.trim()) + .map((entry) => toSessionBindingRecord(entry, { idleTimeoutMs, maxAgeMs })); + } + const conversationId = resolveThreadBindingConversationIdFromBindingId({ + accountId, + bindingId: input.bindingId, + }); + if (!conversationId) { + return []; + } + const removed = manager.unbindConversation(conversationId); + return removed ? [toSessionBindingRecord(removed, { idleTimeoutMs, maxAgeMs })] : []; + }, + }); + + MANAGERS_BY_ACCOUNT_ID.set(accountId, manager); + return manager; +} + +export function getFeishuThreadBindingManager( + accountId?: string, +): FeishuThreadBindingManager | null { + return MANAGERS_BY_ACCOUNT_ID.get(normalizeAccountId(accountId)) ?? null; +} + +export const __testing = { + resetFeishuThreadBindingsForTests() { + for (const manager of MANAGERS_BY_ACCOUNT_ID.values()) { + manager.stop(); + } + MANAGERS_BY_ACCOUNT_ID.clear(); + BINDINGS_BY_ACCOUNT_CONVERSATION.clear(); + }, +}; diff --git a/src/acp/persistent-bindings.resolve.ts b/src/acp/persistent-bindings.resolve.ts index 84f052797ad..66464535eae 100644 --- a/src/acp/persistent-bindings.resolve.ts +++ b/src/acp/persistent-bindings.resolve.ts @@ -1,3 +1,4 @@ +import { parseFeishuConversationId } from "../../extensions/feishu/src/conversation-id.js"; import { listAcpBindings } from "../config/bindings.js"; import type { OpenClawConfig } from "../config/config.js"; import type { AgentAcpBinding } from "../config/types.js"; @@ -21,12 +22,23 @@ import { function normalizeBindingChannel(value: string | undefined): ConfiguredAcpBindingChannel | null { const normalized = (value ?? "").trim().toLowerCase(); - if (normalized === "discord" || normalized === "telegram") { + if (normalized === "discord" || normalized === "telegram" || normalized === "feishu") { return normalized; } return null; } +function isSupportedFeishuDirectConversationId(conversationId: string): boolean { + const trimmed = conversationId.trim(); + if (!trimmed || trimmed.includes(":")) { + return false; + } + if (trimmed.startsWith("oc_") || trimmed.startsWith("on_")) { + return false; + } + return true; +} + function resolveAccountMatchPriority(match: string | undefined, actual: string): 0 | 1 | 2 { const trimmed = (match ?? "").trim(); if (!trimmed) { @@ -122,14 +134,23 @@ function resolveConfiguredBindingRecord(params: { bindings: AgentAcpBinding[]; channel: ConfiguredAcpBindingChannel; accountId: string; - selectConversation: ( - binding: AgentAcpBinding, - ) => { conversationId: string; parentConversationId?: string } | null; + selectConversation: (binding: AgentAcpBinding) => { + conversationId: string; + parentConversationId?: string; + matchPriority?: number; + } | null; }): ResolvedConfiguredAcpBinding | null { let wildcardMatch: { binding: AgentAcpBinding; conversationId: string; parentConversationId?: string; + matchPriority: number; + } | null = null; + let exactMatch: { + binding: AgentAcpBinding; + conversationId: string; + parentConversationId?: string; + matchPriority: number; } | null = null; for (const binding of params.bindings) { if (normalizeBindingChannel(binding.match.channel) !== params.channel) { @@ -146,23 +167,40 @@ function resolveConfiguredBindingRecord(params: { if (!conversation) { continue; } + const matchPriority = conversation.matchPriority ?? 0; + if (accountMatchPriority === 2) { + if (!exactMatch || matchPriority > exactMatch.matchPriority) { + exactMatch = { + binding, + conversationId: conversation.conversationId, + parentConversationId: conversation.parentConversationId, + matchPriority, + }; + } + continue; + } + if (!wildcardMatch || matchPriority > wildcardMatch.matchPriority) { + wildcardMatch = { + binding, + conversationId: conversation.conversationId, + parentConversationId: conversation.parentConversationId, + matchPriority, + }; + } + } + if (exactMatch) { const spec = toConfiguredBindingSpec({ cfg: params.cfg, channel: params.channel, accountId: params.accountId, - conversationId: conversation.conversationId, - parentConversationId: conversation.parentConversationId, - binding, + conversationId: exactMatch.conversationId, + parentConversationId: exactMatch.parentConversationId, + binding: exactMatch.binding, }); - if (accountMatchPriority === 2) { - return { - spec, - record: toConfiguredAcpBindingRecord(spec), - }; - } - if (!wildcardMatch) { - wildcardMatch = { binding, ...conversation }; - } + return { + spec, + record: toConfiguredAcpBindingRecord(spec), + }; } if (!wildcardMatch) { return null; @@ -228,6 +266,42 @@ export function resolveConfiguredAcpBindingSpecBySessionKey(params: { } continue; } + if (channel === "feishu") { + const targetParsed = parseFeishuConversationId({ + conversationId: targetConversationId, + }); + if ( + !targetParsed || + (targetParsed.scope !== "group_topic" && + targetParsed.scope !== "group_topic_sender" && + !isSupportedFeishuDirectConversationId(targetParsed.canonicalConversationId)) + ) { + continue; + } + const spec = toConfiguredBindingSpec({ + cfg: params.cfg, + channel: "feishu", + accountId: parsedSessionKey.accountId, + conversationId: targetParsed.canonicalConversationId, + // Session-key recovery deliberately collapses sender-scoped topic bindings onto the + // canonical topic conversation id so `group_topic` and `group_topic_sender` reuse + // the same configured ACP session identity. + parentConversationId: + targetParsed.scope === "group_topic" || targetParsed.scope === "group_topic_sender" + ? targetParsed.chatId + : undefined, + binding, + }); + if (buildConfiguredAcpSessionKey(spec) === sessionKey) { + if (accountMatchPriority === 2) { + return spec; + } + if (!wildcardMatch) { + wildcardMatch = spec; + } + } + continue; + } const parsedTopic = parseTelegramTopicConversation({ conversationId: targetConversationId, }); @@ -334,5 +408,63 @@ export function resolveConfiguredAcpBindingRecord(params: { }); } + if (channel === "feishu") { + const parsed = parseFeishuConversationId({ + conversationId, + parentConversationId, + }); + if ( + !parsed || + (parsed.scope !== "group_topic" && + parsed.scope !== "group_topic_sender" && + !isSupportedFeishuDirectConversationId(parsed.canonicalConversationId)) + ) { + return null; + } + return resolveConfiguredBindingRecord({ + cfg: params.cfg, + bindings: listAcpBindings(params.cfg), + channel: "feishu", + accountId, + selectConversation: (binding) => { + const targetConversationId = resolveBindingConversationId(binding); + if (!targetConversationId) { + return null; + } + const targetParsed = parseFeishuConversationId({ + conversationId: targetConversationId, + }); + if ( + !targetParsed || + (targetParsed.scope !== "group_topic" && + targetParsed.scope !== "group_topic_sender" && + !isSupportedFeishuDirectConversationId(targetParsed.canonicalConversationId)) + ) { + return null; + } + const matchesCanonicalConversation = + targetParsed.canonicalConversationId === parsed.canonicalConversationId; + const matchesParentTopicForSenderScopedConversation = + parsed.scope === "group_topic_sender" && + targetParsed.scope === "group_topic" && + parsed.chatId === targetParsed.chatId && + parsed.topicId === targetParsed.topicId; + if (!matchesCanonicalConversation && !matchesParentTopicForSenderScopedConversation) { + return null; + } + return { + conversationId: matchesParentTopicForSenderScopedConversation + ? targetParsed.canonicalConversationId + : parsed.canonicalConversationId, + parentConversationId: + parsed.scope === "group_topic" || parsed.scope === "group_topic_sender" + ? parsed.chatId + : undefined, + matchPriority: matchesCanonicalConversation ? 2 : 1, + }; + }, + }); + } + return null; } diff --git a/src/acp/persistent-bindings.test.ts b/src/acp/persistent-bindings.test.ts index 30e74c05082..06bfba46d57 100644 --- a/src/acp/persistent-bindings.test.ts +++ b/src/acp/persistent-bindings.test.ts @@ -90,6 +90,27 @@ function createTelegramGroupBinding(params: { } as ConfiguredBinding; } +function createFeishuBinding(params: { + agentId: string; + conversationId: string; + accountId?: string; + acp?: Record; +}): ConfiguredBinding { + return { + type: "acp", + agentId: params.agentId, + match: { + channel: "feishu", + accountId: params.accountId ?? defaultDiscordAccountId, + peer: { + kind: params.conversationId.includes(":topic:") ? "group" : "direct", + id: params.conversationId, + }, + }, + ...(params.acp ? { acp: params.acp } : {}), + } as ConfiguredBinding; +} + function resolveBindingRecord(cfg: OpenClawConfig, overrides: Partial = {}) { return resolveConfiguredAcpBindingRecord({ cfg, @@ -205,6 +226,34 @@ describe("resolveConfiguredAcpBindingRecord", () => { expect(resolved?.spec.agentId).toBe("claude"); }); + it("prefers sender-scoped Feishu bindings over topic inheritance", () => { + const cfg = createCfgWithBindings([ + createFeishuBinding({ + agentId: "codex", + conversationId: "oc_group_chat:topic:om_topic_root", + accountId: "work", + }), + createFeishuBinding({ + agentId: "claude", + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_sender_1", + accountId: "work", + }), + ]); + + const resolved = resolveConfiguredAcpBindingRecord({ + cfg, + channel: "feishu", + accountId: "work", + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_sender_1", + parentConversationId: "oc_group_chat", + }); + + expect(resolved?.spec.conversationId).toBe( + "oc_group_chat:topic:om_topic_root:sender:ou_sender_1", + ); + expect(resolved?.spec.agentId).toBe("claude"); + }); + it("prefers exact account binding over wildcard for the same discord conversation", () => { const cfg = createCfgWithBindings([ createDiscordBinding({ @@ -284,6 +333,128 @@ describe("resolveConfiguredAcpBindingRecord", () => { expect(resolved).toBeNull(); }); + it("resolves Feishu DM bindings using direct peer ids", () => { + const cfg = createCfgWithBindings([ + createFeishuBinding({ + agentId: "codex", + conversationId: "ou_user_1", + }), + ]); + + const resolved = resolveConfiguredAcpBindingRecord({ + cfg, + channel: "feishu", + accountId: "default", + conversationId: "ou_user_1", + }); + + expect(resolved?.spec.channel).toBe("feishu"); + expect(resolved?.spec.conversationId).toBe("ou_user_1"); + expect(resolved?.record.targetSessionKey).toContain("agent:codex:acp:binding:feishu:default:"); + }); + + it("resolves Feishu DM bindings using user_id fallback peer ids", () => { + const cfg = createCfgWithBindings([ + createFeishuBinding({ + agentId: "codex", + conversationId: "user_123", + }), + ]); + + const resolved = resolveConfiguredAcpBindingRecord({ + cfg, + channel: "feishu", + accountId: "default", + conversationId: "user_123", + }); + + expect(resolved?.spec.channel).toBe("feishu"); + expect(resolved?.spec.conversationId).toBe("user_123"); + expect(resolved?.record.targetSessionKey).toContain("agent:codex:acp:binding:feishu:default:"); + }); + + it("resolves Feishu topic bindings with parent chat ids", () => { + const cfg = createCfgWithBindings([ + createFeishuBinding({ + agentId: "claude", + conversationId: "oc_group_chat:topic:om_topic_root", + acp: { backend: "acpx" }, + }), + ]); + + const resolved = resolveConfiguredAcpBindingRecord({ + cfg, + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_topic_root", + parentConversationId: "oc_group_chat", + }); + + expect(resolved?.spec.conversationId).toBe("oc_group_chat:topic:om_topic_root"); + expect(resolved?.spec.agentId).toBe("claude"); + expect(resolved?.record.conversation.parentConversationId).toBe("oc_group_chat"); + }); + + it("inherits configured Feishu topic bindings for sender-scoped topic conversations", () => { + const cfg = createCfgWithBindings([ + createFeishuBinding({ + agentId: "claude", + conversationId: "oc_group_chat:topic:om_topic_root", + acp: { backend: "acpx" }, + }), + ]); + + const resolved = resolveConfiguredAcpBindingRecord({ + cfg, + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_topic_user", + parentConversationId: "oc_group_chat", + }); + + expect(resolved?.spec.conversationId).toBe("oc_group_chat:topic:om_topic_root"); + expect(resolved?.spec.agentId).toBe("claude"); + expect(resolved?.spec.backend).toBe("acpx"); + expect(resolved?.record.conversation.conversationId).toBe("oc_group_chat:topic:om_topic_root"); + }); + + it("rejects non-matching Feishu topic roots", () => { + const cfg = createCfgWithBindings([ + createFeishuBinding({ + agentId: "claude", + conversationId: "oc_group_chat:topic:om_topic_root", + }), + ]); + + const resolved = resolveConfiguredAcpBindingRecord({ + cfg, + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_other_root", + parentConversationId: "oc_group_chat", + }); + + expect(resolved).toBeNull(); + }); + + it("rejects Feishu non-topic group ACP bindings", () => { + const cfg = createCfgWithBindings([ + createFeishuBinding({ + agentId: "claude", + conversationId: "oc_group_chat", + }), + ]); + + const resolved = resolveConfiguredAcpBindingRecord({ + cfg, + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat", + }); + + expect(resolved).toBeNull(); + }); + it("applies agent runtime ACP defaults for bound conversations", () => { const cfg = createCfgWithBindings( [ @@ -365,6 +536,31 @@ describe("resolveConfiguredAcpBindingSpecBySessionKey", () => { expect(spec?.backend).toBe("exact"); }); + + it("maps a configured Feishu user_id DM binding session key back to its spec", () => { + const cfg = createCfgWithBindings([ + createFeishuBinding({ + agentId: "codex", + conversationId: "user_123", + acp: { backend: "acpx" }, + }), + ]); + const resolved = resolveConfiguredAcpBindingRecord({ + cfg, + channel: "feishu", + accountId: "default", + conversationId: "user_123", + }); + const spec = resolveConfiguredAcpBindingSpecBySessionKey({ + cfg, + sessionKey: resolved?.record.targetSessionKey ?? "", + }); + + expect(spec?.channel).toBe("feishu"); + expect(spec?.conversationId).toBe("user_123"); + expect(spec?.agentId).toBe("codex"); + expect(spec?.backend).toBe("acpx"); + }); }); describe("buildConfiguredAcpSessionKey", () => { diff --git a/src/acp/persistent-bindings.types.ts b/src/acp/persistent-bindings.types.ts index 715ae9c70d4..3864392c96c 100644 --- a/src/acp/persistent-bindings.types.ts +++ b/src/acp/persistent-bindings.types.ts @@ -3,7 +3,7 @@ import type { SessionBindingRecord } from "../infra/outbound/session-binding-ser import { sanitizeAgentId } from "../routing/session-key.js"; import type { AcpRuntimeSessionMode } from "./runtime/types.js"; -export type ConfiguredAcpBindingChannel = "discord" | "telegram"; +export type ConfiguredAcpBindingChannel = "discord" | "telegram" | "feishu"; export type ConfiguredAcpBindingSpec = { channel: ConfiguredAcpBindingChannel; diff --git a/src/auto-reply/reply/commands-acp.test.ts b/src/auto-reply/reply/commands-acp.test.ts index 904ae965fa7..937d282c18e 100644 --- a/src/auto-reply/reply/commands-acp.test.ts +++ b/src/auto-reply/reply/commands-acp.test.ts @@ -118,7 +118,7 @@ type FakeBinding = { targetSessionKey: string; targetKind: "subagent" | "session"; conversation: { - channel: "discord" | "telegram"; + channel: "discord" | "telegram" | "feishu"; accountId: string; conversationId: string; parentConversationId?: string; @@ -243,7 +243,7 @@ function createSessionBindingCapabilities() { type AcpBindInput = { targetSessionKey: string; conversation: { - channel?: "discord" | "telegram"; + channel?: "discord" | "telegram" | "feishu"; accountId: string; conversationId: string; }; @@ -256,21 +256,28 @@ function createAcpThreadBinding(input: AcpBindInput): FakeBinding { input.placement === "child" ? "thread-created" : input.conversation.conversationId; const boundBy = typeof input.metadata?.boundBy === "string" ? input.metadata.boundBy : "user-1"; const channel = input.conversation.channel ?? "discord"; - return createSessionBinding({ - targetSessionKey: input.targetSessionKey, - conversation: - channel === "discord" + const conversation = + channel === "discord" + ? { + channel: "discord" as const, + accountId: input.conversation.accountId, + conversationId: nextConversationId, + parentConversationId: "parent-1", + } + : channel === "feishu" ? { - channel: "discord", + channel: "feishu" as const, accountId: input.conversation.accountId, conversationId: nextConversationId, - parentConversationId: "parent-1", } : { - channel: "telegram", + channel: "telegram" as const, accountId: input.conversation.accountId, conversationId: nextConversationId, - }, + }; + return createSessionBinding({ + targetSessionKey: input.targetSessionKey, + conversation, metadata: { boundBy, webhookId: "wh-1" }, }); } @@ -350,6 +357,23 @@ async function runTelegramDmAcpCommand(commandBody: string, cfg: OpenClawConfig return handleAcpCommand(createTelegramDmParams(commandBody, cfg), true); } +function createFeishuDmParams(commandBody: string, cfg: OpenClawConfig = baseCfg) { + const params = buildCommandTestParams(commandBody, cfg, { + Provider: "feishu", + Surface: "feishu", + OriginatingChannel: "feishu", + OriginatingTo: "user:ou_sender_1", + AccountId: "default", + SenderId: "ou_sender_1", + }); + params.command.senderId = "user-1"; + return params; +} + +async function runFeishuDmAcpCommand(commandBody: string, cfg: OpenClawConfig = baseCfg) { + return handleAcpCommand(createFeishuDmParams(commandBody, cfg), true); +} + describe("/acp command", () => { beforeEach(() => { acpManagerTesting.resetAcpSessionManagerForTests(); @@ -553,6 +577,23 @@ describe("/acp command", () => { ); }); + it("binds Feishu DM ACP spawns to the current DM conversation", async () => { + const result = await runFeishuDmAcpCommand("/acp spawn codex --thread here"); + + expect(result?.reply?.text).toContain("Spawned ACP session agent:codex:acp:"); + expect(result?.reply?.text).toContain("Bound this thread to"); + expect(hoisted.sessionBindingBindMock).toHaveBeenCalledWith( + expect.objectContaining({ + placement: "current", + conversation: expect.objectContaining({ + channel: "feishu", + accountId: "default", + conversationId: "ou_sender_1", + }), + }), + ); + }); + it("requires explicit ACP target when acp.defaultAgent is not configured", async () => { const result = await runDiscordAcpCommand("/acp spawn"); diff --git a/src/auto-reply/reply/commands-acp/context.test.ts b/src/auto-reply/reply/commands-acp/context.test.ts index 18136b67b03..5b1e60ad1fc 100644 --- a/src/auto-reply/reply/commands-acp/context.test.ts +++ b/src/auto-reply/reply/commands-acp/context.test.ts @@ -1,10 +1,19 @@ -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it } from "vitest"; +import { + __testing as feishuThreadBindingTesting, + createFeishuThreadBindingManager, +} from "../../../../extensions/feishu/src/thread-bindings.js"; import type { OpenClawConfig } from "../../../config/config.js"; +import { + __testing as sessionBindingTesting, + getSessionBindingService, +} from "../../../infra/outbound/session-binding-service.js"; import { buildCommandTestParams } from "../commands-spawn.test-harness.js"; import { isAcpCommandDiscordChannel, resolveAcpCommandBindingContext, resolveAcpCommandConversationId, + resolveAcpCommandParentConversationId, } from "./context.js"; const baseCfg = { @@ -12,6 +21,11 @@ const baseCfg = { } satisfies OpenClawConfig; describe("commands-acp context", () => { + beforeEach(() => { + feishuThreadBindingTesting.resetFeishuThreadBindingsForTests(); + sessionBindingTesting.resetSessionBindingAdaptersForTests(); + }); + it("resolves channel/account/thread context from originating fields", () => { const params = buildCommandTestParams("/acp sessions", baseCfg, { Provider: "discord", @@ -126,4 +140,166 @@ describe("commands-acp context", () => { }); expect(resolveAcpCommandConversationId(params)).toBe("123456789"); }); + + it("builds Feishu topic conversation ids from chat target + root message id", () => { + const params = buildCommandTestParams("/acp status", baseCfg, { + Provider: "feishu", + Surface: "feishu", + OriginatingChannel: "feishu", + OriginatingTo: "chat:oc_group_chat", + MessageThreadId: "om_topic_root", + SenderId: "ou_topic_user", + AccountId: "work", + }); + + expect(resolveAcpCommandBindingContext(params)).toEqual({ + channel: "feishu", + accountId: "work", + threadId: "om_topic_root", + conversationId: "oc_group_chat:topic:om_topic_root", + parentConversationId: "oc_group_chat", + }); + expect(resolveAcpCommandConversationId(params)).toBe("oc_group_chat:topic:om_topic_root"); + }); + + it("builds sender-scoped Feishu topic conversation ids when current session is sender-scoped", () => { + const params = buildCommandTestParams("/acp status", baseCfg, { + Provider: "feishu", + Surface: "feishu", + OriginatingChannel: "feishu", + OriginatingTo: "chat:oc_group_chat", + MessageThreadId: "om_topic_root", + SenderId: "ou_topic_user", + AccountId: "work", + SessionKey: "agent:main:feishu:group:oc_group_chat:topic:om_topic_root:sender:ou_topic_user", + }); + params.sessionKey = + "agent:main:feishu:group:oc_group_chat:topic:om_topic_root:sender:ou_topic_user"; + + expect(resolveAcpCommandBindingContext(params)).toEqual({ + channel: "feishu", + accountId: "work", + threadId: "om_topic_root", + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_topic_user", + parentConversationId: "oc_group_chat", + }); + expect(resolveAcpCommandConversationId(params)).toBe( + "oc_group_chat:topic:om_topic_root:sender:ou_topic_user", + ); + }); + + it("preserves sender-scoped Feishu topic ids after ACP route takeover via ParentSessionKey", () => { + const params = buildCommandTestParams("/acp status", baseCfg, { + Provider: "feishu", + Surface: "feishu", + OriginatingChannel: "feishu", + OriginatingTo: "chat:oc_group_chat", + MessageThreadId: "om_topic_root", + SenderId: "ou_topic_user", + AccountId: "work", + ParentSessionKey: + "agent:main:feishu:group:oc_group_chat:topic:om_topic_root:sender:ou_topic_user", + }); + params.sessionKey = "agent:codex:acp:binding:feishu:work:abc123"; + + expect(resolveAcpCommandBindingContext(params)).toEqual({ + channel: "feishu", + accountId: "work", + threadId: "om_topic_root", + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_topic_user", + parentConversationId: "oc_group_chat", + }); + }); + + it("preserves sender-scoped Feishu topic ids after ACP takeover from the live binding record", async () => { + createFeishuThreadBindingManager({ cfg: baseCfg, accountId: "work" }); + await getSessionBindingService().bind({ + targetSessionKey: "agent:codex:acp:binding:feishu:work:abc123", + targetKind: "session", + conversation: { + channel: "feishu", + accountId: "work", + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_topic_user", + parentConversationId: "oc_group_chat", + }, + placement: "current", + metadata: { + agentId: "codex", + }, + }); + + const params = buildCommandTestParams("/acp status", baseCfg, { + Provider: "feishu", + Surface: "feishu", + OriginatingChannel: "feishu", + OriginatingTo: "chat:oc_group_chat", + MessageThreadId: "om_topic_root", + SenderId: "ou_topic_user", + AccountId: "work", + }); + params.sessionKey = "agent:codex:acp:binding:feishu:work:abc123"; + + expect(resolveAcpCommandBindingContext(params)).toEqual({ + channel: "feishu", + accountId: "work", + threadId: "om_topic_root", + conversationId: "oc_group_chat:topic:om_topic_root:sender:ou_topic_user", + parentConversationId: "oc_group_chat", + }); + }); + + it("resolves Feishu DM conversation ids from user targets", () => { + const params = buildCommandTestParams("/acp status", baseCfg, { + Provider: "feishu", + Surface: "feishu", + OriginatingChannel: "feishu", + OriginatingTo: "user:ou_sender_1", + }); + + expect(resolveAcpCommandBindingContext(params)).toEqual({ + channel: "feishu", + accountId: "default", + threadId: undefined, + conversationId: "ou_sender_1", + parentConversationId: undefined, + }); + expect(resolveAcpCommandConversationId(params)).toBe("ou_sender_1"); + }); + + it("resolves Feishu DM conversation ids from user_id fallback targets", () => { + const params = buildCommandTestParams("/acp status", baseCfg, { + Provider: "feishu", + Surface: "feishu", + OriginatingChannel: "feishu", + OriginatingTo: "user:user_123", + }); + + expect(resolveAcpCommandBindingContext(params)).toEqual({ + channel: "feishu", + accountId: "default", + threadId: undefined, + conversationId: "user_123", + parentConversationId: undefined, + }); + expect(resolveAcpCommandConversationId(params)).toBe("user_123"); + }); + + it("does not infer a Feishu DM parent conversation id during fallback binding lookup", () => { + const params = buildCommandTestParams("/acp status", baseCfg, { + Provider: "feishu", + Surface: "feishu", + OriginatingChannel: "feishu", + OriginatingTo: "user:ou_sender_1", + AccountId: "work", + }); + + expect(resolveAcpCommandParentConversationId(params)).toBeUndefined(); + expect(resolveAcpCommandBindingContext(params)).toEqual({ + channel: "feishu", + accountId: "work", + threadId: undefined, + conversationId: "ou_sender_1", + parentConversationId: undefined, + }); + }); }); diff --git a/src/auto-reply/reply/commands-acp/context.ts b/src/auto-reply/reply/commands-acp/context.ts index 84acb828015..fd5eb50ee09 100644 --- a/src/auto-reply/reply/commands-acp/context.ts +++ b/src/auto-reply/reply/commands-acp/context.ts @@ -1,3 +1,4 @@ +import { buildFeishuConversationId } from "../../../../extensions/feishu/src/conversation-id.js"; import { buildTelegramTopicConversationId, normalizeConversationText, @@ -5,10 +6,107 @@ import { } from "../../../acp/conversation-id.js"; import { DISCORD_THREAD_BINDING_CHANNEL } from "../../../channels/thread-bindings-policy.js"; import { resolveConversationIdFromTargets } from "../../../infra/outbound/conversation-id.js"; +import { getSessionBindingService } from "../../../infra/outbound/session-binding-service.js"; +import { parseAgentSessionKey } from "../../../routing/session-key.js"; import type { HandleCommandsParams } from "../commands-types.js"; import { parseDiscordParentChannelFromSessionKey } from "../discord-parent-channel.js"; import { resolveTelegramConversationId } from "../telegram-context.js"; +function parseFeishuTargetId(raw: unknown): string | undefined { + const target = normalizeConversationText(raw); + if (!target) { + return undefined; + } + const withoutProvider = target.replace(/^(feishu|lark):/i, "").trim(); + if (!withoutProvider) { + return undefined; + } + const lowered = withoutProvider.toLowerCase(); + for (const prefix of ["chat:", "group:", "channel:", "user:", "dm:", "open_id:"]) { + if (lowered.startsWith(prefix)) { + return normalizeConversationText(withoutProvider.slice(prefix.length)); + } + } + return withoutProvider; +} + +function parseFeishuDirectConversationId(raw: unknown): string | undefined { + const target = normalizeConversationText(raw); + if (!target) { + return undefined; + } + const withoutProvider = target.replace(/^(feishu|lark):/i, "").trim(); + if (!withoutProvider) { + return undefined; + } + const lowered = withoutProvider.toLowerCase(); + for (const prefix of ["user:", "dm:", "open_id:"]) { + if (lowered.startsWith(prefix)) { + return normalizeConversationText(withoutProvider.slice(prefix.length)); + } + } + const id = parseFeishuTargetId(target); + if (!id) { + return undefined; + } + if (id.startsWith("ou_") || id.startsWith("on_")) { + return id; + } + return undefined; +} + +function resolveFeishuSenderScopedConversationId(params: { + accountId: string; + parentConversationId?: string; + threadId?: string; + senderId?: string; + sessionKey?: string; + parentSessionKey?: string; +}): string | undefined { + const parentConversationId = normalizeConversationText(params.parentConversationId); + const threadId = normalizeConversationText(params.threadId); + const senderId = normalizeConversationText(params.senderId); + const expectedScopePrefix = `feishu:group:${parentConversationId?.toLowerCase()}:topic:${threadId?.toLowerCase()}:sender:`; + const isSenderScopedSession = [params.sessionKey, params.parentSessionKey].some((candidate) => { + const scopedRest = parseAgentSessionKey(candidate)?.rest?.trim().toLowerCase() ?? ""; + return Boolean(scopedRest && expectedScopePrefix && scopedRest.startsWith(expectedScopePrefix)); + }); + if (!parentConversationId || !threadId || !senderId) { + return undefined; + } + if (!isSenderScopedSession && params.sessionKey?.trim()) { + const boundConversation = getSessionBindingService() + .listBySession(params.sessionKey) + .find((binding) => { + if ( + binding.conversation.channel !== "feishu" || + binding.conversation.accountId !== params.accountId + ) { + return false; + } + return ( + binding.conversation.conversationId === + buildFeishuConversationId({ + chatId: parentConversationId, + scope: "group_topic_sender", + topicId: threadId, + senderOpenId: senderId, + }) + ); + }); + if (boundConversation) { + return boundConversation.conversation.conversationId; + } + return undefined; + } + return buildFeishuConversationId({ + chatId: parentConversationId, + scope: "group_topic_sender", + topicId: threadId, + senderOpenId: senderId, + }); +} + export function resolveAcpCommandChannel(params: HandleCommandsParams): string { const raw = params.ctx.OriginatingChannel ?? @@ -58,6 +156,33 @@ export function resolveAcpCommandConversationId(params: HandleCommandsParams): s ); } } + if (channel === "feishu") { + const threadId = resolveAcpCommandThreadId(params); + const parentConversationId = resolveAcpCommandParentConversationId(params); + if (threadId && parentConversationId) { + const senderScopedConversationId = resolveFeishuSenderScopedConversationId({ + accountId: resolveAcpCommandAccountId(params), + parentConversationId, + threadId, + senderId: params.command.senderId ?? params.ctx.SenderId, + sessionKey: params.sessionKey, + parentSessionKey: params.ctx.ParentSessionKey, + }); + return ( + senderScopedConversationId ?? + buildFeishuConversationId({ + chatId: parentConversationId, + scope: "group_topic", + topicId: threadId, + }) + ); + } + return ( + parseFeishuDirectConversationId(params.ctx.OriginatingTo) ?? + parseFeishuDirectConversationId(params.command.to) ?? + parseFeishuDirectConversationId(params.ctx.To) + ); + } return resolveConversationIdFromTargets({ threadId: params.ctx.MessageThreadId, targets: [params.ctx.OriginatingTo, params.command.to, params.ctx.To], @@ -83,6 +208,17 @@ export function resolveAcpCommandParentConversationId( parseTelegramChatIdFromTarget(params.ctx.To) ); } + if (channel === "feishu") { + const threadId = resolveAcpCommandThreadId(params); + if (!threadId) { + return undefined; + } + return ( + parseFeishuTargetId(params.ctx.OriginatingTo) ?? + parseFeishuTargetId(params.command.to) ?? + parseFeishuTargetId(params.ctx.To) + ); + } if (channel === DISCORD_THREAD_BINDING_CHANNEL) { const threadId = resolveAcpCommandThreadId(params); if (!threadId) { diff --git a/src/auto-reply/reply/commands-acp/lifecycle.ts b/src/auto-reply/reply/commands-acp/lifecycle.ts index 564788f78d7..42ee1d2e184 100644 --- a/src/auto-reply/reply/commands-acp/lifecycle.ts +++ b/src/auto-reply/reply/commands-acp/lifecycle.ts @@ -125,7 +125,7 @@ async function bindSpawnedAcpSessionToThread(params: { const currentThreadId = bindingContext.threadId ?? ""; const currentConversationId = bindingContext.conversationId?.trim() || ""; - const requiresThreadIdForHere = channel !== "telegram"; + const requiresThreadIdForHere = channel !== "telegram" && channel !== "feishu"; if ( threadMode === "here" && ((requiresThreadIdForHere && !currentThreadId) || @@ -137,7 +137,12 @@ async function bindSpawnedAcpSessionToThread(params: { }; } - const placement = channel === "telegram" ? "current" : currentThreadId ? "current" : "child"; + const placement = + channel === "telegram" || channel === "feishu" + ? "current" + : currentThreadId + ? "current" + : "child"; if (!capabilities.placements.includes(placement)) { return { ok: false, diff --git a/src/config/config.acp-binding-cutover.test.ts b/src/config/config.acp-binding-cutover.test.ts index ea9f4d603ea..c1b2944bdd0 100644 --- a/src/config/config.acp-binding-cutover.test.ts +++ b/src/config/config.acp-binding-cutover.test.ts @@ -144,4 +144,112 @@ describe("ACP binding cutover schema", () => { expect(parsed.success).toBe(false); }); + + it("accepts canonical Feishu ACP DM and topic peer IDs", () => { + const parsed = OpenClawSchema.safeParse({ + bindings: [ + { + type: "acp", + agentId: "codex", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "direct", id: "ou_user_123" }, + }, + }, + { + type: "acp", + agentId: "codex", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "direct", id: "user_123" }, + }, + }, + { + type: "acp", + agentId: "codex", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "group", id: "oc_group_chat:topic:om_topic_root:sender:ou_user_123" }, + }, + }, + ], + }); + + expect(parsed.success).toBe(true); + }); + + it("rejects non-canonical Feishu ACP peer IDs", () => { + const parsed = OpenClawSchema.safeParse({ + bindings: [ + { + type: "acp", + agentId: "codex", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "group", id: "oc_group_chat:sender:ou_user_123" }, + }, + }, + ], + }); + + expect(parsed.success).toBe(false); + }); + + it("rejects Feishu ACP DM peer IDs keyed by union id", () => { + const parsed = OpenClawSchema.safeParse({ + bindings: [ + { + type: "acp", + agentId: "codex", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "direct", id: "on_union_user_123" }, + }, + }, + ], + }); + + expect(parsed.success).toBe(false); + }); + + it("rejects Feishu ACP topic peer IDs with non-canonical sender ids", () => { + const parsed = OpenClawSchema.safeParse({ + bindings: [ + { + type: "acp", + agentId: "codex", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "group", id: "oc_group_chat:topic:om_topic_root:sender:user_123" }, + }, + }, + ], + }); + + expect(parsed.success).toBe(false); + }); + + it("rejects bare Feishu group chat ACP peer IDs", () => { + const parsed = OpenClawSchema.safeParse({ + bindings: [ + { + type: "acp", + agentId: "codex", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "group", id: "oc_group_chat" }, + }, + }, + ], + }); + + expect(parsed.success).toBe(false); + }); }); diff --git a/src/config/zod-schema.agents.ts b/src/config/zod-schema.agents.ts index ed638d9b502..5dddfc9813a 100644 --- a/src/config/zod-schema.agents.ts +++ b/src/config/zod-schema.agents.ts @@ -71,11 +71,12 @@ const AcpBindingSchema = z return; } const channel = value.match.channel.trim().toLowerCase(); - if (channel !== "discord" && channel !== "telegram") { + if (channel !== "discord" && channel !== "telegram" && channel !== "feishu") { ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["match", "channel"], - message: 'ACP bindings currently support only "discord" and "telegram" channels.', + message: + 'ACP bindings currently support only "discord", "telegram", and "feishu" channels.', }); return; } @@ -87,6 +88,24 @@ const AcpBindingSchema = z "Telegram ACP bindings require canonical topic IDs in the form -1001234567890:topic:42.", }); } + if (channel === "feishu") { + const peerKind = value.match.peer?.kind; + const isDirectId = + (peerKind === "direct" || peerKind === "dm") && + /^[^:]+$/.test(peerId) && + !peerId.startsWith("oc_") && + !peerId.startsWith("on_"); + const isTopicId = + peerKind === "group" && /^oc_[^:]+:topic:[^:]+(?::sender:ou_[^:]+)?$/.test(peerId); + if (!isDirectId && !isTopicId) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["match", "peer", "id"], + message: + "Feishu ACP bindings require direct peer IDs for DMs or topic IDs in the form oc_group:topic:om_root[:sender:ou_xxx].", + }); + } + } }); export const BindingsSchema = z.array(z.union([RouteBindingSchema, AcpBindingSchema])).optional(); From e4c61723cd2d530680cc61789311d464ab8cdf60 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 15 Mar 2026 08:39:49 -0700 Subject: [PATCH 05/16] ACP: fail closed on conflicting tool identity hints (#46817) * ACP: fail closed on conflicting tool identity hints * ACP: restore rawInput fallback for safe tool resolution * ACP tests: cover rawInput-only safe tool approval --- CHANGELOG.md | 1 + src/acp/client.test.ts | 41 +++++++++++++++++++++++++++++++++++++++++ src/acp/client.ts | 17 ++++++++++++++++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b50a557d97..4bcd43d2b62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ Docs: https://docs.openclaw.ai - Docs/Mintlify: fix MDX marker syntax on Perplexity, Model Providers, Moonshot, and exec approvals pages so local docs preview no longer breaks rendering or leaves stale pages unpublished. (#46695) Thanks @velvet-shark. - Email/webhook wrapping: sanitize sender and subject metadata before external-content wrapping so metadata fields cannot break the wrapper structure. Thanks @vincentkoc. - Node/startup: remove leftover debug `console.log("node host PATH: ...")` that printed the resolved PATH on every `openclaw node run` invocation. (#46411) +- ACP/approvals: use canonical tool identity for prompting decisions and fail closed when conflicting tool identity hints are present. Thanks @vincentkoc. - Telegram/message send: forward `--force-document` through the `sendPayload` path as well as `sendMedia`, so Telegram payload sends with `channelData` keep uploading images as documents instead of silently falling back to compressed photo sends. (#47119) Thanks @thepagent. - Telegram/message chunking: preserve spaces, paragraph separators, and word boundaries when HTML overflow rechunking splits formatted replies. (#47274) diff --git a/src/acp/client.test.ts b/src/acp/client.test.ts index 0cbc376720c..2595e89bfee 100644 --- a/src/acp/client.test.ts +++ b/src/acp/client.test.ts @@ -366,6 +366,47 @@ describe("resolvePermissionRequest", () => { expect(prompt).not.toHaveBeenCalled(); }); + it("auto-approves safe tools when rawInput is the only identity hint", async () => { + const prompt = vi.fn(async () => true); + const res = await resolvePermissionRequest( + makePermissionRequest({ + toolCall: { + toolCallId: "tool-raw-only", + title: "Searching files", + status: "pending", + rawInput: { + name: "search", + query: "foo", + }, + }, + }), + { prompt, log: () => {} }, + ); + expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } }); + expect(prompt).not.toHaveBeenCalled(); + }); + + it("prompts when raw input spoofs a safe tool name for a dangerous title", async () => { + const prompt = vi.fn(async () => false); + const res = await resolvePermissionRequest( + makePermissionRequest({ + toolCall: { + toolCallId: "tool-exec-spoof", + title: "exec: cat /etc/passwd", + status: "pending", + rawInput: { + command: "cat /etc/passwd", + name: "search", + }, + }, + }), + { prompt, log: () => {} }, + ); + expect(prompt).toHaveBeenCalledTimes(1); + expect(prompt).toHaveBeenCalledWith(undefined, "exec: cat /etc/passwd"); + expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } }); + }); + it("prompts for read outside cwd scope", async () => { const prompt = vi.fn(async () => false); const res = await resolvePermissionRequest( diff --git a/src/acp/client.ts b/src/acp/client.ts index 2f3ac28641a..1d25281cce5 100644 --- a/src/acp/client.ts +++ b/src/acp/client.ts @@ -104,7 +104,22 @@ function resolveToolNameForPermission(params: RequestPermissionRequest): string const fromMeta = readFirstStringValue(toolMeta, ["toolName", "tool_name", "name"]); const fromRawInput = readFirstStringValue(rawInput, ["tool", "toolName", "tool_name", "name"]); const fromTitle = parseToolNameFromTitle(toolCall?.title); - return normalizeToolName(fromMeta ?? fromRawInput ?? fromTitle ?? ""); + const metaName = fromMeta ? normalizeToolName(fromMeta) : undefined; + const rawInputName = fromRawInput ? normalizeToolName(fromRawInput) : undefined; + const titleName = fromTitle; + if ((fromMeta && !metaName) || (fromRawInput && !rawInputName)) { + return undefined; + } + if (metaName && titleName && metaName !== titleName) { + return undefined; + } + if (rawInputName && metaName && rawInputName !== metaName) { + return undefined; + } + if (rawInputName && titleName && rawInputName !== titleName) { + return undefined; + } + return metaName ?? titleName ?? rawInputName; } function extractPathFromToolTitle( From ff61343d76933c2d7bc01a13db87a2554514e0d0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Mar 2026 08:44:02 -0700 Subject: [PATCH 06/16] fix: harden mention pattern regex compilation --- CHANGELOG.md | 1 + docs/channels/group-messages.md | 4 +- docs/channels/groups.md | 2 +- docs/gateway/configuration-reference.md | 4 +- docs/gateway/configuration.md | 2 +- src/auto-reply/inbound.test.ts | 19 +++- src/auto-reply/reply/mentions.ts | 124 +++++++++++++++++------- src/channels/dock.ts | 8 +- src/channels/plugins/types.core.ts | 5 + src/channels/plugins/whatsapp-shared.ts | 4 + src/config/schema.help.ts | 2 +- src/infra/exec-approval-forwarder.ts | 7 +- src/logging/redact.ts | 6 +- src/plugin-sdk/whatsapp.ts | 1 + src/security/config-regex.ts | 78 +++++++++++++++ src/security/safe-regex.test.ts | 14 ++- src/security/safe-regex.ts | 49 ++++++++-- 17 files changed, 265 insertions(+), 65 deletions(-) create mode 100644 src/security/config-regex.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bcd43d2b62..5fa449296ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Group mention gating: reject invalid and unsafe nested-repetition `mentionPatterns`, reuse the shared safe config-regex compiler across mention stripping and detection, and cache strip-time regex compilation so noisy groups avoid repeated recompiles. - Control UI/chat sessions: show human-readable labels in the grouped session dropdown again, keep unique scoped fallbacks when metadata is missing, and disambiguate duplicate labels only when needed. (#45130) thanks @luzhidong. - Slack/interactive replies: preserve `channelData.slack.blocks` through live DM delivery and preview-finalized edits so Block Kit button and select directives render instead of falling back to raw text. Thanks @vincentkoc. - Feishu/topic threads: fetch full thread context, including prior bot replies, when starting a topic-thread session so follow-up turns in Feishu topics keep the right conversation state. Thanks @Coobiw. diff --git a/docs/channels/group-messages.md b/docs/channels/group-messages.md index e6a00ab5c5e..078ae9e7845 100644 --- a/docs/channels/group-messages.md +++ b/docs/channels/group-messages.md @@ -13,7 +13,7 @@ Note: `agents.list[].groupChat.mentionPatterns` is now used by Telegram/Discord/ ## What’s implemented (2025-12-03) -- Activation modes: `mention` (default) or `always`. `mention` requires a ping (real WhatsApp @-mentions via `mentionedJids`, regex patterns, or the bot’s E.164 anywhere in the text). `always` wakes the agent on every message but it should reply only when it can add meaningful value; otherwise it returns the silent token `NO_REPLY`. Defaults can be set in config (`channels.whatsapp.groups`) and overridden per group via `/activation`. When `channels.whatsapp.groups` is set, it also acts as a group allowlist (include `"*"` to allow all). +- Activation modes: `mention` (default) or `always`. `mention` requires a ping (real WhatsApp @-mentions via `mentionedJids`, safe regex patterns, or the bot’s E.164 anywhere in the text). `always` wakes the agent on every message but it should reply only when it can add meaningful value; otherwise it returns the silent token `NO_REPLY`. Defaults can be set in config (`channels.whatsapp.groups`) and overridden per group via `/activation`. When `channels.whatsapp.groups` is set, it also acts as a group allowlist (include `"*"` to allow all). - Group policy: `channels.whatsapp.groupPolicy` controls whether group messages are accepted (`open|disabled|allowlist`). `allowlist` uses `channels.whatsapp.groupAllowFrom` (fallback: explicit `channels.whatsapp.allowFrom`). Default is `allowlist` (blocked until you add senders). - Per-group sessions: session keys look like `agent::whatsapp:group:` so commands such as `/verbose on` or `/think high` (sent as standalone messages) are scoped to that group; personal DM state is untouched. Heartbeats are skipped for group threads. - Context injection: **pending-only** group messages (default 50) that _did not_ trigger a run are prefixed under `[Chat messages since your last reply - for context]`, with the triggering line under `[Current message - respond to this]`. Messages already in the session are not re-injected. @@ -50,7 +50,7 @@ Add a `groupChat` block to `~/.openclaw/openclaw.json` so display-name pings wor Notes: -- The regexes are case-insensitive; they cover a display-name ping like `@openclaw` and the raw number with or without `+`/spaces. +- The regexes are case-insensitive and use the same safe-regex guardrails as other config regex surfaces; invalid patterns and unsafe nested repetition are ignored. - WhatsApp still sends canonical mentions via `mentionedJids` when someone taps the contact, so the number fallback is rarely needed but is a useful safety net. ### Activation command (owner-only) diff --git a/docs/channels/groups.md b/docs/channels/groups.md index 3f9df076454..a6bd8621784 100644 --- a/docs/channels/groups.md +++ b/docs/channels/groups.md @@ -243,7 +243,7 @@ Replying to a bot message counts as an implicit mention (when the channel suppor Notes: -- `mentionPatterns` are case-insensitive regexes. +- `mentionPatterns` are case-insensitive safe regex patterns; invalid patterns and unsafe nested-repetition forms are ignored. - Surfaces that provide explicit mentions still pass; patterns are a fallback. - Per-agent override: `agents.list[].groupChat.mentionPatterns` (useful when multiple agents share a group). - Mention gating is only enforced when mention detection is possible (native mentions or `mentionPatterns` are configured). diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 7bb7fb5824f..b87ad930161 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -655,12 +655,12 @@ See the full channel index: [Channels](/channels). ### Group chat mention gating -Group messages default to **require mention** (metadata mention or regex patterns). Applies to WhatsApp, Telegram, Discord, Google Chat, and iMessage group chats. +Group messages default to **require mention** (metadata mention or safe regex patterns). Applies to WhatsApp, Telegram, Discord, Google Chat, and iMessage group chats. **Mention types:** - **Metadata mentions**: Native platform @-mentions. Ignored in WhatsApp self-chat mode. -- **Text patterns**: Regex patterns in `agents.list[].groupChat.mentionPatterns`. Always checked. +- **Text patterns**: Safe regex patterns in `agents.list[].groupChat.mentionPatterns`. Invalid patterns and unsafe nested repetition are ignored. - Mention gating is enforced only when detection is possible (native mentions or at least one pattern). ```json5 diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 0f1dd65cbbc..9a047cab857 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -170,7 +170,7 @@ When validation fails: ``` - **Metadata mentions**: native @-mentions (WhatsApp tap-to-mention, Telegram @bot, etc.) - - **Text patterns**: regex patterns in `mentionPatterns` + - **Text patterns**: safe regex patterns in `mentionPatterns` - See [full reference](/gateway/configuration-reference#group-chat-mention-gating) for per-channel overrides and self-chat mode. diff --git a/src/auto-reply/inbound.test.ts b/src/auto-reply/inbound.test.ts index 4d624ecabd1..77ff61e814e 100644 --- a/src/auto-reply/inbound.test.ts +++ b/src/auto-reply/inbound.test.ts @@ -17,6 +17,7 @@ import { buildMentionRegexes, matchesMentionPatterns, normalizeMentionText, + stripMentions, } from "./reply/mentions.js"; import { initSessionState } from "./reply/session.js"; import { applyTemplate, type MsgContext, type TemplateContext } from "./templating.js"; @@ -394,10 +395,10 @@ describe("initSessionState BodyStripped", () => { }); describe("mention helpers", () => { - it("builds regexes and skips invalid patterns", () => { + it("builds regexes and skips invalid or unsafe patterns", () => { const regexes = buildMentionRegexes({ messages: { - groupChat: { mentionPatterns: ["\\bopenclaw\\b", "(invalid"] }, + groupChat: { mentionPatterns: ["\\bopenclaw\\b", "(invalid", "(a+)+$"] }, }, }); expect(regexes).toHaveLength(1); @@ -435,6 +436,20 @@ describe("mention helpers", () => { expect(matchesMentionPatterns("workbot: hi", regexes)).toBe(true); expect(matchesMentionPatterns("global: hi", regexes)).toBe(false); }); + + it("strips safe mention patterns and ignores unsafe ones", () => { + const stripped = stripMentions("openclaw " + "a".repeat(28) + "!", {} as MsgContext, { + messages: { + groupChat: { mentionPatterns: ["\\bopenclaw\\b", "(a+)+$"] }, + }, + }); + expect(stripped).toBe(`${"a".repeat(28)}!`); + }); + + it("strips provider mention regexes without config compilation", () => { + const stripped = stripMentions("<@12345> hello", { Provider: "discord" } as MsgContext, {}); + expect(stripped).toBe("hello"); + }); }); describe("resolveGroupRequireMention", () => { diff --git a/src/auto-reply/reply/mentions.ts b/src/auto-reply/reply/mentions.ts index ca20905efae..714e599e38a 100644 --- a/src/auto-reply/reply/mentions.ts +++ b/src/auto-reply/reply/mentions.ts @@ -2,6 +2,8 @@ import { resolveAgentConfig } from "../../agents/agent-scope.js"; import { getChannelDock } from "../../channels/dock.js"; import { normalizeChannelId } from "../../channels/plugins/index.js"; import type { OpenClawConfig } from "../../config/config.js"; +import { createSubsystemLogger } from "../../logging/subsystem.js"; +import { compileConfigRegexes, type ConfigRegexRejectReason } from "../../security/config-regex.js"; import { escapeRegExp } from "../../utils.js"; import type { MsgContext } from "../templating.js"; @@ -21,8 +23,12 @@ function deriveMentionPatterns(identity?: { name?: string; emoji?: string }) { } const BACKSPACE_CHAR = "\u0008"; -const mentionRegexCompileCache = new Map(); +const mentionMatchRegexCompileCache = new Map(); +const mentionStripRegexCompileCache = new Map(); const MAX_MENTION_REGEX_COMPILE_CACHE_KEYS = 512; +const mentionPatternWarningCache = new Set(); +const MAX_MENTION_PATTERN_WARNING_KEYS = 512; +const log = createSubsystemLogger("mentions"); export const CURRENT_MESSAGE_MARKER = "[Current message - respond to this]"; @@ -37,6 +43,64 @@ function normalizeMentionPatterns(patterns: string[]): string[] { return patterns.map(normalizeMentionPattern); } +function warnRejectedMentionPattern( + pattern: string, + flags: string, + reason: ConfigRegexRejectReason, +) { + const key = `${flags}::${reason}::${pattern}`; + if (mentionPatternWarningCache.has(key)) { + return; + } + mentionPatternWarningCache.add(key); + if (mentionPatternWarningCache.size > MAX_MENTION_PATTERN_WARNING_KEYS) { + mentionPatternWarningCache.clear(); + mentionPatternWarningCache.add(key); + } + log.warn("Ignoring unsupported group mention pattern", { + pattern, + flags, + reason, + }); +} + +function cacheMentionRegexes( + cache: Map, + cacheKey: string, + regexes: RegExp[], +): RegExp[] { + cache.set(cacheKey, regexes); + if (cache.size > MAX_MENTION_REGEX_COMPILE_CACHE_KEYS) { + cache.clear(); + cache.set(cacheKey, regexes); + } + return [...regexes]; +} + +function compileMentionPatternsCached(params: { + patterns: string[]; + flags: string; + cache: Map; + warnRejected: boolean; +}): RegExp[] { + if (params.patterns.length === 0) { + return []; + } + const cacheKey = `${params.flags}\u001e${params.patterns.join("\u001f")}`; + const cached = params.cache.get(cacheKey); + if (cached) { + return [...cached]; + } + + const compiled = compileConfigRegexes(params.patterns, params.flags); + if (params.warnRejected) { + for (const rejected of compiled.rejected) { + warnRejectedMentionPattern(rejected.pattern, rejected.flags, rejected.reason); + } + } + return cacheMentionRegexes(params.cache, cacheKey, compiled.regexes); +} + function resolveMentionPatterns(cfg: OpenClawConfig | undefined, agentId?: string): string[] { if (!cfg) { return []; @@ -56,29 +120,12 @@ function resolveMentionPatterns(cfg: OpenClawConfig | undefined, agentId?: strin export function buildMentionRegexes(cfg: OpenClawConfig | undefined, agentId?: string): RegExp[] { const patterns = normalizeMentionPatterns(resolveMentionPatterns(cfg, agentId)); - if (patterns.length === 0) { - return []; - } - const cacheKey = patterns.join("\u001f"); - const cached = mentionRegexCompileCache.get(cacheKey); - if (cached) { - return [...cached]; - } - const compiled = patterns - .map((pattern) => { - try { - return new RegExp(pattern, "i"); - } catch { - return null; - } - }) - .filter((value): value is RegExp => Boolean(value)); - mentionRegexCompileCache.set(cacheKey, compiled); - if (mentionRegexCompileCache.size > MAX_MENTION_REGEX_COMPILE_CACHE_KEYS) { - mentionRegexCompileCache.clear(); - mentionRegexCompileCache.set(cacheKey, compiled); - } - return [...compiled]; + return compileMentionPatternsCached({ + patterns, + flags: "i", + cache: mentionMatchRegexCompileCache, + warnRejected: true, + }); } export function normalizeMentionText(text: string): string { @@ -153,17 +200,24 @@ export function stripMentions( let result = text; const providerId = ctx.Provider ? normalizeChannelId(ctx.Provider) : null; const providerMentions = providerId ? getChannelDock(providerId)?.mentions : undefined; - const patterns = normalizeMentionPatterns([ - ...resolveMentionPatterns(cfg, agentId), - ...(providerMentions?.stripPatterns?.({ ctx, cfg, agentId }) ?? []), - ]); - for (const p of patterns) { - try { - const re = new RegExp(p, "gi"); - result = result.replace(re, " "); - } catch { - // ignore invalid regex - } + const configRegexes = compileMentionPatternsCached({ + patterns: normalizeMentionPatterns(resolveMentionPatterns(cfg, agentId)), + flags: "gi", + cache: mentionStripRegexCompileCache, + warnRejected: true, + }); + const providerRegexes = + providerMentions?.stripRegexes?.({ ctx, cfg, agentId }) ?? + compileMentionPatternsCached({ + patterns: normalizeMentionPatterns( + providerMentions?.stripPatterns?.({ ctx, cfg, agentId }) ?? [], + ), + flags: "gi", + cache: mentionStripRegexCompileCache, + warnRejected: false, + }); + for (const re of [...configRegexes, ...providerRegexes]) { + result = result.replace(re, " "); } if (providerMentions?.stripMentions) { result = providerMentions.stripMentions({ diff --git a/src/channels/dock.ts b/src/channels/dock.ts index e080d513c16..2e63583ca1b 100644 --- a/src/channels/dock.ts +++ b/src/channels/dock.ts @@ -58,7 +58,7 @@ import type { } from "./plugins/types.js"; import { resolveWhatsAppGroupIntroHint, - resolveWhatsAppMentionStripPatterns, + resolveWhatsAppMentionStripRegexes, } from "./plugins/whatsapp-shared.js"; import { CHAT_CHANNEL_ORDER, type ChatChannelId, getChatChannelMeta } from "./registry.js"; @@ -303,7 +303,7 @@ const DOCKS: Record = { resolveGroupIntroHint: resolveWhatsAppGroupIntroHint, }, mentions: { - stripPatterns: ({ ctx }) => resolveWhatsAppMentionStripPatterns(ctx), + stripRegexes: ({ ctx }) => resolveWhatsAppMentionStripRegexes(ctx), }, threading: { buildToolContext: ({ context, hasRepliedRef }) => { @@ -346,7 +346,7 @@ const DOCKS: Record = { resolveToolPolicy: resolveDiscordGroupToolPolicy, }, mentions: { - stripPatterns: () => ["<@!?\\d+>"], + stripRegexes: () => [/<@!?\d+>/g], }, threading: { resolveReplyToMode: ({ cfg }) => cfg.channels?.discord?.replyToMode ?? "off", @@ -484,7 +484,7 @@ const DOCKS: Record = { resolveToolPolicy: resolveSlackGroupToolPolicy, }, mentions: { - stripPatterns: () => ["<@[^>]+>"], + stripRegexes: () => [/<@[^>]+>/g], }, threading: { resolveReplyToMode: ({ cfg, accountId, chatType }) => diff --git a/src/channels/plugins/types.core.ts b/src/channels/plugins/types.core.ts index 3bf3c07ddc6..fef8b010ca5 100644 --- a/src/channels/plugins/types.core.ts +++ b/src/channels/plugins/types.core.ts @@ -209,6 +209,11 @@ export type ChannelSecurityContext = { }; export type ChannelMentionAdapter = { + stripRegexes?: (params: { + ctx: MsgContext; + cfg: OpenClawConfig | undefined; + agentId?: string; + }) => RegExp[]; stripPatterns?: (params: { ctx: MsgContext; cfg: OpenClawConfig | undefined; diff --git a/src/channels/plugins/whatsapp-shared.ts b/src/channels/plugins/whatsapp-shared.ts index 3a51e2263bd..c8db1d068c8 100644 --- a/src/channels/plugins/whatsapp-shared.ts +++ b/src/channels/plugins/whatsapp-shared.ts @@ -20,6 +20,10 @@ export function resolveWhatsAppMentionStripPatterns(ctx: { To?: string | null }) return [escaped, `@${escaped}`]; } +export function resolveWhatsAppMentionStripRegexes(ctx: { To?: string | null }): RegExp[] { + return resolveWhatsAppMentionStripPatterns(ctx).map((pattern) => new RegExp(pattern, "g")); +} + type WhatsAppChunker = NonNullable; type WhatsAppSendMessage = PluginRuntimeChannel["whatsapp"]["sendMessageWhatsApp"]; type WhatsAppSendPoll = PluginRuntimeChannel["whatsapp"]["sendPollWhatsApp"]; diff --git a/src/config/schema.help.ts b/src/config/schema.help.ts index a4e2e125528..0d03f9574b1 100644 --- a/src/config/schema.help.ts +++ b/src/config/schema.help.ts @@ -1346,7 +1346,7 @@ export const FIELD_HELP: Record = { "messages.groupChat": "Group-message handling controls including mention triggers and history window sizing. Keep mention patterns narrow so group channels do not trigger on every message.", "messages.groupChat.mentionPatterns": - "Regex-like patterns used to detect explicit mentions/trigger phrases in group chats. Use precise patterns to reduce false positives in high-volume channels.", + "Safe case-insensitive regex patterns used to detect explicit mentions/trigger phrases in group chats. Use precise patterns to reduce false positives in high-volume channels; invalid or unsafe nested-repetition patterns are ignored.", "messages.groupChat.historyLimit": "Maximum number of prior group messages loaded as context per turn for group sessions. Use higher values for richer continuity, or lower values for faster and cheaper responses.", "messages.queue": diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index de3a54a4c77..5d197d6ae62 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -9,7 +9,8 @@ import type { } from "../config/types.approvals.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { normalizeAccountId, parseAgentSessionKey } from "../routing/session-key.js"; -import { compileSafeRegex, testRegexWithBoundedInput } from "../security/safe-regex.js"; +import { compileConfigRegex } from "../security/config-regex.js"; +import { testRegexWithBoundedInput } from "../security/safe-regex.js"; import { isDeliverableMessageChannel, normalizeMessageChannel, @@ -63,8 +64,8 @@ function matchSessionFilter(sessionKey: string, patterns: string[]): boolean { if (sessionKey.includes(pattern)) { return true; } - const regex = compileSafeRegex(pattern); - return regex ? testRegexWithBoundedInput(regex, sessionKey) : false; + const compiled = compileConfigRegex(pattern); + return compiled?.regex ? testRegexWithBoundedInput(compiled.regex, sessionKey) : false; }); } diff --git a/src/logging/redact.ts b/src/logging/redact.ts index 7e47ac0b663..42266f71eec 100644 --- a/src/logging/redact.ts +++ b/src/logging/redact.ts @@ -1,5 +1,5 @@ import type { OpenClawConfig } from "../config/config.js"; -import { compileSafeRegex } from "../security/safe-regex.js"; +import { compileConfigRegex } from "../security/config-regex.js"; import { resolveNodeRequireFromMeta } from "./node-require.js"; import { replacePatternBounded } from "./redact-bounded.js"; @@ -55,9 +55,9 @@ function parsePattern(raw: string): RegExp | null { const match = raw.match(/^\/(.+)\/([gimsuy]*)$/); if (match) { const flags = match[2].includes("g") ? match[2] : `${match[2]}g`; - return compileSafeRegex(match[1], flags); + return compileConfigRegex(match[1], flags)?.regex ?? null; } - return compileSafeRegex(raw, "gi"); + return compileConfigRegex(raw, "gi")?.regex ?? null; } function resolvePatterns(value?: string[]): RegExp[] { diff --git a/src/plugin-sdk/whatsapp.ts b/src/plugin-sdk/whatsapp.ts index 0227322f868..ea6465e8faa 100644 --- a/src/plugin-sdk/whatsapp.ts +++ b/src/plugin-sdk/whatsapp.ts @@ -38,6 +38,7 @@ export { export { createWhatsAppOutboundBase, resolveWhatsAppGroupIntroHint, + resolveWhatsAppMentionStripRegexes, resolveWhatsAppMentionStripPatterns, } from "../channels/plugins/whatsapp-shared.js"; export { resolveWhatsAppHeartbeatRecipients } from "../channels/plugins/whatsapp-heartbeat.js"; diff --git a/src/security/config-regex.ts b/src/security/config-regex.ts new file mode 100644 index 00000000000..76e8d0e86c7 --- /dev/null +++ b/src/security/config-regex.ts @@ -0,0 +1,78 @@ +import { + compileSafeRegexDetailed, + type SafeRegexCompileResult, + type SafeRegexRejectReason, +} from "./safe-regex.js"; + +export type ConfigRegexRejectReason = Exclude; + +export type CompiledConfigRegex = + | { + regex: RegExp; + pattern: string; + flags: string; + reason: null; + } + | { + regex: null; + pattern: string; + flags: string; + reason: ConfigRegexRejectReason; + }; + +function normalizeRejectReason(result: SafeRegexCompileResult): ConfigRegexRejectReason | null { + if (result.reason === null || result.reason === "empty") { + return null; + } + return result.reason; +} + +export function compileConfigRegex(pattern: string, flags = ""): CompiledConfigRegex | null { + const result = compileSafeRegexDetailed(pattern, flags); + if (result.reason === "empty") { + return null; + } + return { + regex: result.regex, + pattern: result.source, + flags: result.flags, + reason: normalizeRejectReason(result), + } as CompiledConfigRegex; +} + +export function compileConfigRegexes( + patterns: string[], + flags = "", +): { + regexes: RegExp[]; + rejected: Array<{ + pattern: string; + flags: string; + reason: ConfigRegexRejectReason; + }>; +} { + const regexes: RegExp[] = []; + const rejected: Array<{ + pattern: string; + flags: string; + reason: ConfigRegexRejectReason; + }> = []; + + for (const pattern of patterns) { + const compiled = compileConfigRegex(pattern, flags); + if (!compiled) { + continue; + } + if (compiled.regex) { + regexes.push(compiled.regex); + continue; + } + rejected.push({ + pattern: compiled.pattern, + flags: compiled.flags, + reason: compiled.reason, + }); + } + + return { regexes, rejected }; +} diff --git a/src/security/safe-regex.test.ts b/src/security/safe-regex.test.ts index 460149ad8ce..d4d3d650d91 100644 --- a/src/security/safe-regex.test.ts +++ b/src/security/safe-regex.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; -import { compileSafeRegex, hasNestedRepetition, testRegexWithBoundedInput } from "./safe-regex.js"; +import { + compileSafeRegex, + compileSafeRegexDetailed, + hasNestedRepetition, + testRegexWithBoundedInput, +} from "./safe-regex.js"; describe("safe regex", () => { it("flags nested repetition patterns", () => { @@ -28,6 +33,13 @@ describe("safe regex", () => { expect("TOKEN=abcd1234".replace(re as RegExp, "***")).toBe("***"); }); + it("returns structured reject reasons", () => { + expect(compileSafeRegexDetailed(" ").reason).toBe("empty"); + expect(compileSafeRegexDetailed("(a+)+$").reason).toBe("unsafe-nested-repetition"); + expect(compileSafeRegexDetailed("(invalid").reason).toBe("invalid-regex"); + expect(compileSafeRegexDetailed("^agent:main$").reason).toBeNull(); + }); + it("checks bounded regex windows for long inputs", () => { expect( testRegexWithBoundedInput(/^agent:main:discord:/, `agent:main:discord:${"x".repeat(5000)}`), diff --git a/src/security/safe-regex.ts b/src/security/safe-regex.ts index ffa34509130..e197929c4a4 100644 --- a/src/security/safe-regex.ts +++ b/src/security/safe-regex.ts @@ -30,7 +30,23 @@ type PatternToken = const SAFE_REGEX_CACHE_MAX = 256; const SAFE_REGEX_TEST_WINDOW = 2048; -const safeRegexCache = new Map(); +export type SafeRegexRejectReason = "empty" | "unsafe-nested-repetition" | "invalid-regex"; + +export type SafeRegexCompileResult = + | { + regex: RegExp; + source: string; + flags: string; + reason: null; + } + | { + regex: null; + source: string; + flags: string; + reason: SafeRegexRejectReason; + }; + +const safeRegexCache = new Map(); function createParseFrame(): ParseFrame { return { @@ -302,31 +318,44 @@ export function hasNestedRepetition(source: string): boolean { return analyzeTokensForNestedRepetition(tokenizePattern(source)); } -export function compileSafeRegex(source: string, flags = ""): RegExp | null { +export function compileSafeRegexDetailed(source: string, flags = ""): SafeRegexCompileResult { const trimmed = source.trim(); if (!trimmed) { - return null; + return { regex: null, source: trimmed, flags, reason: "empty" }; } const cacheKey = `${flags}::${trimmed}`; if (safeRegexCache.has(cacheKey)) { - return safeRegexCache.get(cacheKey) ?? null; + return ( + safeRegexCache.get(cacheKey) ?? { + regex: null, + source: trimmed, + flags, + reason: "invalid-regex", + } + ); } - let compiled: RegExp | null = null; - if (!hasNestedRepetition(trimmed)) { + let result: SafeRegexCompileResult; + if (hasNestedRepetition(trimmed)) { + result = { regex: null, source: trimmed, flags, reason: "unsafe-nested-repetition" }; + } else { try { - compiled = new RegExp(trimmed, flags); + result = { regex: new RegExp(trimmed, flags), source: trimmed, flags, reason: null }; } catch { - compiled = null; + result = { regex: null, source: trimmed, flags, reason: "invalid-regex" }; } } - safeRegexCache.set(cacheKey, compiled); + safeRegexCache.set(cacheKey, result); if (safeRegexCache.size > SAFE_REGEX_CACHE_MAX) { const oldestKey = safeRegexCache.keys().next().value; if (oldestKey) { safeRegexCache.delete(oldestKey); } } - return compiled; + return result; +} + +export function compileSafeRegex(source: string, flags = ""): RegExp | null { + return compileSafeRegexDetailed(source, flags).regex; } From ec2c6d83b9f5f91d6d9094842e0f19b88e63e3e2 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 15 Mar 2026 08:47:17 -0700 Subject: [PATCH 07/16] Nodes: recheck queued actions before delivery (#46815) * Nodes: recheck queued actions before delivery * Nodes tests: cover pull-time policy recheck * Nodes tests: type node policy mocks explicitly --- CHANGELOG.md | 1 + .../server-methods/nodes.invoke-wake.test.ts | 64 ++++++++++++++++++- src/gateway/server-methods/nodes.ts | 35 +++++++++- 3 files changed, 97 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fa449296ea..a1fd84a09ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai - Docs/Mintlify: fix MDX marker syntax on Perplexity, Model Providers, Moonshot, and exec approvals pages so local docs preview no longer breaks rendering or leaves stale pages unpublished. (#46695) Thanks @velvet-shark. - Email/webhook wrapping: sanitize sender and subject metadata before external-content wrapping so metadata fields cannot break the wrapper structure. Thanks @vincentkoc. - Node/startup: remove leftover debug `console.log("node host PATH: ...")` that printed the resolved PATH on every `openclaw node run` invocation. (#46411) +- Nodes/pending actions: re-check queued foreground actions against the current node command policy before returning them to the node. Thanks @vincentkoc. - ACP/approvals: use canonical tool identity for prompting decisions and fail closed when conflicting tool identity hints are present. Thanks @vincentkoc. - Telegram/message send: forward `--force-document` through the `sendPayload` path as well as `sendMedia`, so Telegram payload sends with `channelData` keep uploading images as documents instead of silently falling back to compressed photo sends. (#47119) Thanks @thepagent. - Telegram/message chunking: preserve spaces, paragraph separators, and word boundaries when HTML overflow rechunking splits formatted replies. (#47274) diff --git a/src/gateway/server-methods/nodes.invoke-wake.test.ts b/src/gateway/server-methods/nodes.invoke-wake.test.ts index fc01f718bbb..ea29384698c 100644 --- a/src/gateway/server-methods/nodes.invoke-wake.test.ts +++ b/src/gateway/server-methods/nodes.invoke-wake.test.ts @@ -2,10 +2,18 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ErrorCodes } from "../protocol/index.js"; import { maybeWakeNodeWithApns, nodeHandlers } from "./nodes.js"; +type MockNodeCommandPolicyParams = { + command: string; + declaredCommands?: string[]; + allowlist: Set; +}; + const mocks = vi.hoisted(() => ({ loadConfig: vi.fn(() => ({})), - resolveNodeCommandAllowlist: vi.fn(() => []), - isNodeCommandAllowed: vi.fn(() => ({ ok: true })), + resolveNodeCommandAllowlist: vi.fn<() => Set>(() => new Set()), + isNodeCommandAllowed: vi.fn< + (params: MockNodeCommandPolicyParams) => { ok: true } | { ok: false; reason: string } + >(() => ({ ok: true })), sanitizeNodeInvokeParamsForForwarding: vi.fn(({ rawParams }: { rawParams: unknown }) => ({ ok: true, params: rawParams, @@ -518,6 +526,58 @@ describe("node.invoke APNs wake path", () => { }); }); + it("drops queued actions that are no longer allowed at pull time", async () => { + mocks.loadApnsRegistration.mockResolvedValue(null); + const allowlistedCommands = new Set(["camera.snap", "canvas.navigate"]); + mocks.resolveNodeCommandAllowlist.mockImplementation(() => new Set(allowlistedCommands)); + mocks.isNodeCommandAllowed.mockImplementation( + ({ command, declaredCommands, allowlist }: MockNodeCommandPolicyParams) => { + if (!allowlist.has(command)) { + return { ok: false, reason: "command not allowlisted" }; + } + if (!declaredCommands.includes(command)) { + return { ok: false, reason: "command not declared by node" }; + } + return { ok: true }; + }, + ); + + const nodeRegistry = { + get: vi.fn(() => ({ + nodeId: "ios-node-policy", + commands: ["camera.snap", "canvas.navigate"], + platform: "iOS 26.4.0", + })), + invoke: vi.fn().mockResolvedValue({ + ok: false, + error: { + code: "NODE_BACKGROUND_UNAVAILABLE", + message: "NODE_BACKGROUND_UNAVAILABLE: canvas/camera/screen commands require foreground", + }, + }), + }; + + await invokeNode({ + nodeRegistry, + requestParams: { + nodeId: "ios-node-policy", + command: "camera.snap", + params: { facing: "front" }, + idempotencyKey: "idem-policy", + }, + }); + + allowlistedCommands.delete("camera.snap"); + + const pullRespond = await pullPending("ios-node-policy"); + const pullCall = pullRespond.mock.calls[0] as RespondCall | undefined; + expect(pullCall?.[0]).toBe(true); + expect(pullCall?.[1]).toMatchObject({ + nodeId: "ios-node-policy", + actions: [], + }); + }); + it("dedupes queued foreground actions by idempotency key", async () => { mocks.loadApnsRegistration.mockResolvedValue(null); diff --git a/src/gateway/server-methods/nodes.ts b/src/gateway/server-methods/nodes.ts index 7f78809abbb..ae6c8090b6c 100644 --- a/src/gateway/server-methods/nodes.ts +++ b/src/gateway/server-methods/nodes.ts @@ -26,6 +26,7 @@ import { import { isNodeCommandAllowed, resolveNodeCommandAllowlist } from "../node-command-policy.js"; import { sanitizeNodeInvokeParamsForForwarding } from "../node-invoke-sanitize.js"; import { + type ConnectParams, ErrorCodes, errorShape, validateNodeDescribeParams, @@ -218,6 +219,38 @@ function listPendingNodeActions(nodeId: string): PendingNodeAction[] { return prunePendingNodeActions(nodeId, Date.now()); } +function resolveAllowedPendingNodeActions(params: { + nodeId: string; + client: { connect?: ConnectParams | null } | null; +}): PendingNodeAction[] { + const pending = listPendingNodeActions(params.nodeId); + if (pending.length === 0) { + return pending; + } + const connect = params.client?.connect; + const declaredCommands = Array.isArray(connect?.commands) ? connect.commands : []; + const allowlist = resolveNodeCommandAllowlist(loadConfig(), { + platform: connect?.client?.platform, + deviceFamily: connect?.client?.deviceFamily, + }); + const allowed = pending.filter((entry) => { + const result = isNodeCommandAllowed({ + command: entry.command, + declaredCommands, + allowlist, + }); + return result.ok; + }); + if (allowed.length !== pending.length) { + if (allowed.length === 0) { + pendingNodeActionsById.delete(params.nodeId); + } else { + pendingNodeActionsById.set(params.nodeId, allowed); + } + } + return allowed; +} + function ackPendingNodeActions(nodeId: string, ids: string[]): PendingNodeAction[] { if (ids.length === 0) { return listPendingNodeActions(nodeId); @@ -805,7 +838,7 @@ export const nodeHandlers: GatewayRequestHandlers = { return; } - const pending = listPendingNodeActions(trimmedNodeId); + const pending = resolveAllowedPendingNodeActions({ nodeId: trimmedNodeId, client }); respond( true, { From 87c4ae36b4ccbcac25ceb238ef357297761f4bdc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Mar 2026 08:50:23 -0700 Subject: [PATCH 08/16] refactor: drop deprecated whatsapp mention pattern sdk helper --- extensions/whatsapp/src/channel.ts | 4 ++-- src/channels/plugins/whatsapp-shared.ts | 8 ++------ src/plugin-sdk/subpaths.test.ts | 2 ++ src/plugin-sdk/whatsapp.ts | 1 - 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/extensions/whatsapp/src/channel.ts b/extensions/whatsapp/src/channel.ts index 28de41a9fea..8a60dc44432 100644 --- a/extensions/whatsapp/src/channel.ts +++ b/extensions/whatsapp/src/channel.ts @@ -24,7 +24,7 @@ import { resolveWhatsAppGroupIntroHint, resolveWhatsAppGroupToolPolicy, resolveWhatsAppHeartbeatRecipients, - resolveWhatsAppMentionStripPatterns, + resolveWhatsAppMentionStripRegexes, WhatsAppConfigSchema, type ChannelMessageActionName, type ChannelPlugin, @@ -214,7 +214,7 @@ export const whatsappPlugin: ChannelPlugin = { resolveGroupIntroHint: resolveWhatsAppGroupIntroHint, }, mentions: { - stripPatterns: ({ ctx }) => resolveWhatsAppMentionStripPatterns(ctx), + stripRegexes: ({ ctx }) => resolveWhatsAppMentionStripRegexes(ctx), }, commands: { enforceOwnerForCommands: true, diff --git a/src/channels/plugins/whatsapp-shared.ts b/src/channels/plugins/whatsapp-shared.ts index c8db1d068c8..c798e7fe3ca 100644 --- a/src/channels/plugins/whatsapp-shared.ts +++ b/src/channels/plugins/whatsapp-shared.ts @@ -11,17 +11,13 @@ export function resolveWhatsAppGroupIntroHint(): string { return WHATSAPP_GROUP_INTRO_HINT; } -export function resolveWhatsAppMentionStripPatterns(ctx: { To?: string | null }): string[] { +export function resolveWhatsAppMentionStripRegexes(ctx: { To?: string | null }): RegExp[] { const selfE164 = (ctx.To ?? "").replace(/^whatsapp:/, ""); if (!selfE164) { return []; } const escaped = escapeRegExp(selfE164); - return [escaped, `@${escaped}`]; -} - -export function resolveWhatsAppMentionStripRegexes(ctx: { To?: string | null }): RegExp[] { - return resolveWhatsAppMentionStripPatterns(ctx).map((pattern) => new RegExp(pattern, "g")); + return [new RegExp(escaped, "g"), new RegExp(`@${escaped}`, "g")]; } type WhatsAppChunker = NonNullable; diff --git a/src/plugin-sdk/subpaths.test.ts b/src/plugin-sdk/subpaths.test.ts index 592b6de73cf..2d971c82255 100644 --- a/src/plugin-sdk/subpaths.test.ts +++ b/src/plugin-sdk/subpaths.test.ts @@ -87,6 +87,8 @@ describe("plugin-sdk subpath exports", () => { // WhatsApp-specific functions (resolveWhatsAppAccount, whatsappOnboardingAdapter) moved to extensions/whatsapp/src/ expect(typeof whatsappSdk.WhatsAppConfigSchema).toBe("object"); expect(typeof whatsappSdk.resolveWhatsAppOutboundTarget).toBe("function"); + expect(typeof whatsappSdk.resolveWhatsAppMentionStripRegexes).toBe("function"); + expect("resolveWhatsAppMentionStripPatterns" in whatsappSdk).toBe(false); }); it("exports LINE helpers", () => { diff --git a/src/plugin-sdk/whatsapp.ts b/src/plugin-sdk/whatsapp.ts index ea6465e8faa..f18a953bf7a 100644 --- a/src/plugin-sdk/whatsapp.ts +++ b/src/plugin-sdk/whatsapp.ts @@ -39,7 +39,6 @@ export { createWhatsAppOutboundBase, resolveWhatsAppGroupIntroHint, resolveWhatsAppMentionStripRegexes, - resolveWhatsAppMentionStripPatterns, } from "../channels/plugins/whatsapp-shared.js"; export { resolveWhatsAppHeartbeatRecipients } from "../channels/plugins/whatsapp-heartbeat.js"; export { WhatsAppConfigSchema } from "../config/zod-schema.providers-whatsapp.js"; From f5cd7c390d6b592bf932e7dba6efce100b70defd Mon Sep 17 00:00:00 2001 From: Aditya Chaudhary <55331140+ItsAditya-xyz@users.noreply.github.com> Date: Sun, 15 Mar 2026 21:31:31 +0530 Subject: [PATCH 09/16] added a fix for memory leak on 2gb ram (#46522) --- src/agents/model-id-normalization.ts | 23 ++++++++++++++++++ src/agents/model-selection.ts | 2 +- ...onfig.providers.google-antigravity.test.ts | 2 +- src/agents/models-config.providers.ts | 24 ++----------------- .../providers/google/inline-data.ts | 2 +- 5 files changed, 28 insertions(+), 25 deletions(-) create mode 100644 src/agents/model-id-normalization.ts diff --git a/src/agents/model-id-normalization.ts b/src/agents/model-id-normalization.ts new file mode 100644 index 00000000000..9b0b27a7f01 --- /dev/null +++ b/src/agents/model-id-normalization.ts @@ -0,0 +1,23 @@ +// Keep model ID normalization dependency-free so config parsing and other +// startup-only paths do not pull in provider discovery or plugin loading. +export function normalizeGoogleModelId(id: string): string { + if (id === "gemini-3-pro") { + return "gemini-3-pro-preview"; + } + if (id === "gemini-3-flash") { + return "gemini-3-flash-preview"; + } + if (id === "gemini-3.1-pro") { + return "gemini-3.1-pro-preview"; + } + if (id === "gemini-3.1-flash-lite") { + return "gemini-3.1-flash-lite-preview"; + } + // Preserve compatibility with earlier OpenClaw docs/config that pointed at a + // non-existent Gemini Flash preview ID. Google's current Flash text model is + // `gemini-3-flash-preview`. + if (id === "gemini-3.1-flash" || id === "gemini-3.1-flash-preview") { + return "gemini-3-flash-preview"; + } + return id; +} diff --git a/src/agents/model-selection.ts b/src/agents/model-selection.ts index 72cd5951292..1f73ca6a1b4 100644 --- a/src/agents/model-selection.ts +++ b/src/agents/model-selection.ts @@ -13,9 +13,9 @@ import { resolveAgentModelFallbacksOverride, } from "./agent-scope.js"; import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "./defaults.js"; +import { normalizeGoogleModelId } from "./model-id-normalization.js"; import type { ModelCatalogEntry } from "./model-catalog.js"; import { splitTrailingAuthProfile } from "./model-ref-profile.js"; -import { normalizeGoogleModelId } from "./models-config.providers.js"; const log = createSubsystemLogger("model-selection"); diff --git a/src/agents/models-config.providers.google-antigravity.test.ts b/src/agents/models-config.providers.google-antigravity.test.ts index ea20608b866..f14cab01493 100644 --- a/src/agents/models-config.providers.google-antigravity.test.ts +++ b/src/agents/models-config.providers.google-antigravity.test.ts @@ -2,9 +2,9 @@ import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; +import { normalizeGoogleModelId } from "./model-id-normalization.js"; import { normalizeAntigravityModelId, - normalizeGoogleModelId, normalizeProviders, type ProviderConfig, } from "./models-config.providers.js"; diff --git a/src/agents/models-config.providers.ts b/src/agents/models-config.providers.ts index b4ef8f4b0b1..229a861c0e5 100644 --- a/src/agents/models-config.providers.ts +++ b/src/agents/models-config.providers.ts @@ -8,6 +8,7 @@ import { isRecord } from "../utils.js"; import { normalizeOptionalSecretInput } from "../utils/normalize-secret-input.js"; import { ensureAuthProfileStore, listProfilesForProvider } from "./auth-profiles.js"; import { discoverBedrockModels } from "./bedrock-discovery.js"; +import { normalizeGoogleModelId } from "./model-id-normalization.js"; import { buildCloudflareAiGatewayModelDefinition, resolveCloudflareAiGatewayBaseUrl, @@ -70,6 +71,7 @@ import { } from "./model-auth-markers.js"; import { resolveAwsSdkEnvVarName, resolveEnvApiKey } from "./model-auth.js"; export { resolveOllamaApiBase } from "./models-config.providers.discovery.js"; +export { normalizeGoogleModelId }; type ModelsConfig = NonNullable; export type ProviderConfig = NonNullable[string]; @@ -223,28 +225,6 @@ function resolveApiKeyFromProfiles(params: { return undefined; } -export function normalizeGoogleModelId(id: string): string { - if (id === "gemini-3-pro") { - return "gemini-3-pro-preview"; - } - if (id === "gemini-3-flash") { - return "gemini-3-flash-preview"; - } - if (id === "gemini-3.1-pro") { - return "gemini-3.1-pro-preview"; - } - if (id === "gemini-3.1-flash-lite") { - return "gemini-3.1-flash-lite-preview"; - } - // Preserve compatibility with earlier OpenClaw docs/config that pointed at a - // non-existent Gemini Flash preview ID. Google's current Flash text model is - // `gemini-3-flash-preview`. - if (id === "gemini-3.1-flash" || id === "gemini-3.1-flash-preview") { - return "gemini-3-flash-preview"; - } - return id; -} - const ANTIGRAVITY_BARE_PRO_IDS = new Set(["gemini-3-pro", "gemini-3.1-pro", "gemini-3-1-pro"]); export function normalizeAntigravityModelId(id: string): string { diff --git a/src/media-understanding/providers/google/inline-data.ts b/src/media-understanding/providers/google/inline-data.ts index 69fd41871e8..18116a54bc2 100644 --- a/src/media-understanding/providers/google/inline-data.ts +++ b/src/media-understanding/providers/google/inline-data.ts @@ -1,4 +1,4 @@ -import { normalizeGoogleModelId } from "../../../agents/models-config.providers.js"; +import { normalizeGoogleModelId } from "../../../agents/model-id-normalization.js"; import { parseGeminiAuth } from "../../../infra/gemini-auth.js"; import { assertOkOrThrowHttpError, normalizeBaseUrl, postJsonRequest } from "../shared.js"; From a60fd3feedeea9535840b9ddcd921330f4c769bc Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 15 Mar 2026 09:00:28 -0700 Subject: [PATCH 10/16] Nodes tests: prove pull-time policy revalidation --- .../server-methods/nodes.invoke-wake.test.ts | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/gateway/server-methods/nodes.invoke-wake.test.ts b/src/gateway/server-methods/nodes.invoke-wake.test.ts index ea29384698c..f86eb43f437 100644 --- a/src/gateway/server-methods/nodes.invoke-wake.test.ts +++ b/src/gateway/server-methods/nodes.invoke-wake.test.ts @@ -221,9 +221,10 @@ async function invokeNode(params: { return respond; } -function createNodeClient(nodeId: string) { +function createNodeClient(nodeId: string, commands?: string[]) { return { connect: { + ...(commands ? { commands } : {}), role: "node" as const, client: { id: nodeId, @@ -236,26 +237,26 @@ function createNodeClient(nodeId: string) { }; } -async function pullPending(nodeId: string) { +async function pullPending(nodeId: string, commands?: string[]) { const respond = vi.fn(); await nodeHandlers["node.pending.pull"]({ params: {}, respond: respond as never, context: {} as never, - client: createNodeClient(nodeId) as never, + client: createNodeClient(nodeId, commands) as never, req: { type: "req", id: "req-node-pending", method: "node.pending.pull" }, isWebchatConnect: () => false, }); return respond; } -async function ackPending(nodeId: string, ids: string[]) { +async function ackPending(nodeId: string, ids: string[], commands?: string[]) { const respond = vi.fn(); await nodeHandlers["node.pending.ack"]({ params: { ids }, respond: respond as never, context: {} as never, - client: createNodeClient(nodeId) as never, + client: createNodeClient(nodeId, commands) as never, req: { type: "req", id: "req-node-pending-ack", method: "node.pending.ack" }, isWebchatConnect: () => false, }); @@ -267,7 +268,7 @@ describe("node.invoke APNs wake path", () => { mocks.loadConfig.mockClear(); mocks.loadConfig.mockReturnValue({}); mocks.resolveNodeCommandAllowlist.mockClear(); - mocks.resolveNodeCommandAllowlist.mockReturnValue([]); + mocks.resolveNodeCommandAllowlist.mockReturnValue(new Set()); mocks.isNodeCommandAllowed.mockClear(); mocks.isNodeCommandAllowed.mockReturnValue({ ok: true }); mocks.sanitizeNodeInvokeParamsForForwarding.mockClear(); @@ -478,7 +479,7 @@ describe("node.invoke APNs wake path", () => { expect(call?.[2]?.message).toBe("node command queued until iOS returns to foreground"); expect(mocks.sendApnsBackgroundWake).not.toHaveBeenCalled(); - const pullRespond = await pullPending("ios-node-queued"); + const pullRespond = await pullPending("ios-node-queued", ["canvas.navigate"]); const pullCall = pullRespond.mock.calls[0] as RespondCall | undefined; expect(pullCall?.[0]).toBe(true); expect(pullCall?.[1]).toMatchObject({ @@ -491,7 +492,7 @@ describe("node.invoke APNs wake path", () => { ], }); - const repeatedPullRespond = await pullPending("ios-node-queued"); + const repeatedPullRespond = await pullPending("ios-node-queued", ["canvas.navigate"]); const repeatedPullCall = repeatedPullRespond.mock.calls[0] as RespondCall | undefined; expect(repeatedPullCall?.[0]).toBe(true); expect(repeatedPullCall?.[1]).toMatchObject({ @@ -508,7 +509,7 @@ describe("node.invoke APNs wake path", () => { ?.actions?.[0]?.id; expect(queuedActionId).toBeTruthy(); - const ackRespond = await ackPending("ios-node-queued", [queuedActionId!]); + const ackRespond = await ackPending("ios-node-queued", [queuedActionId!], ["canvas.navigate"]); const ackCall = ackRespond.mock.calls[0] as RespondCall | undefined; expect(ackCall?.[0]).toBe(true); expect(ackCall?.[1]).toMatchObject({ @@ -517,7 +518,7 @@ describe("node.invoke APNs wake path", () => { remainingCount: 0, }); - const emptyPullRespond = await pullPending("ios-node-queued"); + const emptyPullRespond = await pullPending("ios-node-queued", ["canvas.navigate"]); const emptyPullCall = emptyPullRespond.mock.calls[0] as RespondCall | undefined; expect(emptyPullCall?.[0]).toBe(true); expect(emptyPullCall?.[1]).toMatchObject({ @@ -535,7 +536,7 @@ describe("node.invoke APNs wake path", () => { if (!allowlist.has(command)) { return { ok: false, reason: "command not allowlisted" }; } - if (!declaredCommands.includes(command)) { + if (!declaredCommands?.includes(command)) { return { ok: false, reason: "command not declared by node" }; } return { ok: true }; @@ -567,9 +568,25 @@ describe("node.invoke APNs wake path", () => { }, }); + const preChangePullRespond = await pullPending("ios-node-policy", [ + "camera.snap", + "canvas.navigate", + ]); + const preChangePullCall = preChangePullRespond.mock.calls[0] as RespondCall | undefined; + expect(preChangePullCall?.[0]).toBe(true); + expect(preChangePullCall?.[1]).toMatchObject({ + nodeId: "ios-node-policy", + actions: [ + expect.objectContaining({ + command: "camera.snap", + paramsJSON: JSON.stringify({ facing: "front" }), + }), + ], + }); + allowlistedCommands.delete("camera.snap"); - const pullRespond = await pullPending("ios-node-policy"); + const pullRespond = await pullPending("ios-node-policy", ["camera.snap", "canvas.navigate"]); const pullCall = pullRespond.mock.calls[0] as RespondCall | undefined; expect(pullCall?.[0]).toBe(true); expect(pullCall?.[1]).toMatchObject({ @@ -615,7 +632,7 @@ describe("node.invoke APNs wake path", () => { }, }); - const pullRespond = await pullPending("ios-node-dedupe"); + const pullRespond = await pullPending("ios-node-dedupe", ["canvas.navigate"]); const pullCall = pullRespond.mock.calls[0] as RespondCall | undefined; expect(pullCall?.[0]).toBe(true); expect(pullCall?.[1]).toMatchObject({ From 7c0a849ed7c48c199b508721a881ffefafcd46fb Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Mar 2026 09:01:53 -0700 Subject: [PATCH 11/16] fix: harden device token rotation denial paths --- CHANGELOG.md | 1 + src/gateway/server-methods/devices.ts | 54 +++++++++++++++++-- .../server.auth.compat-baseline.test.ts | 6 ++- .../server.device-token-rotate-authz.test.ts | 45 ++++++++++++++-- src/infra/device-pairing.test.ts | 24 ++++++--- src/infra/device-pairing.ts | 20 +++++-- 6 files changed, 129 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1fd84a09ba..95a68bc92cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Docs: https://docs.openclaw.ai - WhatsApp/reconnect: restore the append recency filter in the extension inbox monitor and handle protobuf `Long` timestamps correctly, so fresh post-reconnect append messages are processed while stale history sync stays suppressed. (#42588) thanks @MonkeyLeeT. - WhatsApp/login: wait for pending creds writes before reopening after Baileys `515` pairing restarts in both QR login and `channels login` flows, and keep the restart coverage pinned to the real wrapped error shape plus per-account creds queues. (#27910) Thanks @asyncjason. - Agents/openai-compatible tool calls: deduplicate repeated tool call ids across live assistant messages and replayed history so OpenAI-compatible backends no longer reject duplicate `tool_call_id` values with HTTP 400. (#40996) Thanks @xaeon2026. +- Security/device pairing: harden `device.token.rotate` deny handling by keeping public failures generic while logging internal deny reasons and preserving approved-baseline enforcement. (`GHSA-7jrw-x62h-64p8`) ### Fixes diff --git a/src/gateway/server-methods/devices.ts b/src/gateway/server-methods/devices.ts index a068b2dfac5..4becd52edcc 100644 --- a/src/gateway/server-methods/devices.ts +++ b/src/gateway/server-methods/devices.ts @@ -4,6 +4,7 @@ import { listDevicePairing, removePairedDevice, type DeviceAuthToken, + type RotateDeviceTokenDenyReason, rejectDevicePairing, revokeDeviceToken, rotateDeviceToken, @@ -24,6 +25,8 @@ import { } from "../protocol/index.js"; import type { GatewayRequestHandlers } from "./types.js"; +const DEVICE_TOKEN_ROTATION_DENIED_MESSAGE = "device token rotation denied"; + function redactPairedDevice( device: { tokens?: Record } & Record, ) { @@ -53,6 +56,19 @@ function resolveMissingRequestedScope(params: { return null; } +function logDeviceTokenRotationDenied(params: { + log: { warn: (message: string) => void }; + deviceId: string; + role: string; + reason: RotateDeviceTokenDenyReason | "caller-missing-scope" | "unknown-device-or-role"; + scope?: string | null; +}) { + const suffix = params.scope ? ` scope=${params.scope}` : ""; + params.log.warn( + `device token rotation denied device=${params.deviceId} role=${params.role} reason=${params.reason}${suffix}`, + ); +} + export const deviceHandlers: GatewayRequestHandlers = { "device.pair.list": async ({ params, respond }) => { if (!validateDevicePairListParams(params)) { @@ -189,7 +205,17 @@ export const deviceHandlers: GatewayRequestHandlers = { }; const pairedDevice = await getPairedDevice(deviceId); if (!pairedDevice) { - respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId/role")); + logDeviceTokenRotationDenied({ + log: context.logGateway, + deviceId, + role, + reason: "unknown-device-or-role", + }); + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE), + ); return; } const callerScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : []; @@ -202,18 +228,36 @@ export const deviceHandlers: GatewayRequestHandlers = { callerScopes, }); if (missingScope) { + logDeviceTokenRotationDenied({ + log: context.logGateway, + deviceId, + role, + reason: "caller-missing-scope", + scope: missingScope, + }); respond( false, undefined, - errorShape(ErrorCodes.INVALID_REQUEST, `missing scope: ${missingScope}`), + errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE), ); return; } - const entry = await rotateDeviceToken({ deviceId, role, scopes }); - if (!entry) { - respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId/role")); + const rotated = await rotateDeviceToken({ deviceId, role, scopes }); + if (!rotated.ok) { + logDeviceTokenRotationDenied({ + log: context.logGateway, + deviceId, + role, + reason: rotated.reason, + }); + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE), + ); return; } + const entry = rotated.entry; context.logGateway.info( `device token rotated device=${deviceId} role=${entry.role} scopes=${entry.scopes.join(",")}`, ); diff --git a/src/gateway/server.auth.compat-baseline.test.ts b/src/gateway/server.auth.compat-baseline.test.ts index 27fc4abc72d..630e53de84f 100644 --- a/src/gateway/server.auth.compat-baseline.test.ts +++ b/src/gateway/server.auth.compat-baseline.test.ts @@ -174,7 +174,9 @@ describe("gateway auth compatibility baseline", () => { role: "operator", scopes: ["operator.admin"], }); - expect(rotated?.token).toBeTruthy(); + expect(rotated.ok).toBe(true); + const rotatedToken = rotated.ok ? rotated.entry.token : ""; + expect(rotatedToken).toBeTruthy(); const ws = await openWs(port); try { @@ -182,7 +184,7 @@ describe("gateway auth compatibility baseline", () => { skipDefaultAuth: true, client: { ...BACKEND_GATEWAY_CLIENT }, deviceIdentityPath: identityPath, - deviceToken: String(rotated?.token ?? ""), + deviceToken: rotatedToken, scopes: ["operator.admin"], }); expect(res.ok).toBe(true); diff --git a/src/gateway/server.device-token-rotate-authz.test.ts b/src/gateway/server.device-token-rotate-authz.test.ts index 9f3ecdaf719..efb4d5e44b1 100644 --- a/src/gateway/server.device-token-rotate-authz.test.ts +++ b/src/gateway/server.device-token-rotate-authz.test.ts @@ -87,11 +87,13 @@ async function issuePairingScopedTokenForAdminApprovedDevice(name: string): Prom role: "operator", scopes: ["operator.pairing"], }); - expect(rotated?.token).toBeTruthy(); + expect(rotated.ok).toBe(true); + const pairingToken = rotated.ok ? rotated.entry.token : ""; + expect(pairingToken).toBeTruthy(); return { deviceId: paired.identity.deviceId, identityPath: paired.identityPath, - pairingToken: String(rotated?.token ?? ""), + pairingToken, }; } @@ -221,7 +223,7 @@ describe("gateway device.token.rotate caller scope guard", () => { scopes: ["operator.admin"], }); expect(rotate.ok).toBe(false); - expect(rotate.error?.message).toBe("missing scope: operator.admin"); + expect(rotate.error?.message).toBe("device token rotation denied"); const paired = await getPairedDevice(attacker.deviceId); expect(paired?.tokens?.operator?.scopes).toEqual(["operator.pairing"]); @@ -266,7 +268,7 @@ describe("gateway device.token.rotate caller scope guard", () => { }); expect(rotate.ok).toBe(false); - expect(rotate.error?.message).toBe("missing scope: operator.admin"); + expect(rotate.error?.message).toBe("device token rotation denied"); await waitForMacrotasks(); expect(sawInvoke).toBe(false); @@ -281,4 +283,39 @@ describe("gateway device.token.rotate caller scope guard", () => { started.envSnapshot.restore(); } }); + + test("returns the same public deny for unknown devices and caller scope failures", async () => { + const started = await startServerWithClient("secret"); + const attacker = await issuePairingScopedTokenForAdminApprovedDevice("rotate-deny-shape"); + + let pairingWs: WebSocket | undefined; + try { + pairingWs = await connectPairingScopedOperator({ + port: started.port, + identityPath: attacker.identityPath, + deviceToken: attacker.pairingToken, + }); + + const missingScope = await rpcReq(pairingWs, "device.token.rotate", { + deviceId: attacker.deviceId, + role: "operator", + scopes: ["operator.admin"], + }); + const unknownDevice = await rpcReq(pairingWs, "device.token.rotate", { + deviceId: "missing-device", + role: "operator", + scopes: ["operator.pairing"], + }); + + expect(missingScope.ok).toBe(false); + expect(unknownDevice.ok).toBe(false); + expect(missingScope.error?.message).toBe("device token rotation denied"); + expect(unknownDevice.error?.message).toBe("device token rotation denied"); + } finally { + pairingWs?.close(); + started.ws.close(); + await started.server.close(); + started.envSnapshot.restore(); + } + }); }); diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index ddf0826d048..4deb04a8912 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -13,6 +13,7 @@ import { rotateDeviceToken, verifyDeviceToken, type PairedDevice, + type RotateDeviceTokenResult, } from "./device-pairing.js"; import { resolvePairingPaths } from "./pairing-files.js"; @@ -55,6 +56,14 @@ function requireToken(token: string | undefined): string { return token; } +function requireRotatedEntry(result: RotateDeviceTokenResult) { + expect(result.ok).toBe(true); + if (!result.ok) { + throw new Error(`expected rotated token entry, got ${result.reason}`); + } + return result.entry; +} + async function overwritePairedOperatorTokenScopes(baseDir: string, scopes: string[]) { const { pairedPath } = resolvePairingPaths(baseDir, "devices"); const pairedByDeviceId = JSON.parse(await readFile(pairedPath, "utf8")) as Record< @@ -204,22 +213,24 @@ describe("device pairing tokens", () => { const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); await setupPairedOperatorDevice(baseDir, ["operator.admin"]); - await rotateDeviceToken({ + const downscoped = await rotateDeviceToken({ deviceId: "device-1", role: "operator", scopes: ["operator.read"], baseDir, }); + expect(downscoped.ok).toBe(true); let paired = await getPairedDevice("device-1", baseDir); expect(paired?.tokens?.operator?.scopes).toEqual(["operator.read"]); expect(paired?.scopes).toEqual(["operator.admin"]); expect(paired?.approvedScopes).toEqual(["operator.admin"]); - await rotateDeviceToken({ + const reused = await rotateDeviceToken({ deviceId: "device-1", role: "operator", baseDir, }); + expect(reused.ok).toBe(true); paired = await getPairedDevice("device-1", baseDir); expect(paired?.tokens?.operator?.scopes).toEqual(["operator.read"]); }); @@ -255,7 +266,7 @@ describe("device pairing tokens", () => { scopes: ["operator.admin"], baseDir, }); - expect(rotated).toBeNull(); + expect(rotated).toEqual({ ok: false, reason: "scope-outside-approved-baseline" }); const after = await getPairedDevice("device-1", baseDir); expect(after?.tokens?.operator?.token).toEqual(before?.tokens?.operator?.token); @@ -357,12 +368,13 @@ describe("device pairing tokens", () => { scopes: ["operator.talk.secrets"], baseDir, }); - expect(rotated?.scopes).toEqual(["operator.talk.secrets"]); + const entry = requireRotatedEntry(rotated); + expect(entry.scopes).toEqual(["operator.talk.secrets"]); await expect( verifyOperatorToken({ baseDir, - token: requireToken(rotated?.token), + token: requireToken(entry.token), scopes: ["operator.talk.secrets"], }), ).resolves.toEqual({ ok: true }); @@ -395,7 +407,7 @@ describe("device pairing tokens", () => { scopes: ["operator.admin"], baseDir, }), - ).resolves.toBeNull(); + ).resolves.toEqual({ ok: false, reason: "missing-approved-scope-baseline" }); }); test("treats multibyte same-length token input as mismatch without throwing", async () => { diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index 5bd2909a56e..d16cd06f0cc 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -48,6 +48,15 @@ export type DeviceAuthTokenSummary = { lastUsedAtMs?: number; }; +export type RotateDeviceTokenDenyReason = + | "unknown-device-or-role" + | "missing-approved-scope-baseline" + | "scope-outside-approved-baseline"; + +export type RotateDeviceTokenResult = + | { ok: true; entry: DeviceAuthToken } + | { ok: false; reason: RotateDeviceTokenDenyReason }; + export type PairedDevice = { deviceId: string; publicKey: string; @@ -587,7 +596,7 @@ export async function rotateDeviceToken(params: { role: string; scopes?: string[]; baseDir?: string; -}): Promise { +}): Promise { return await withLock(async () => { const state = await loadState(params.baseDir); const context = resolveDeviceTokenUpdateContext({ @@ -596,13 +605,16 @@ export async function rotateDeviceToken(params: { role: params.role, }); if (!context) { - return null; + return { ok: false, reason: "unknown-device-or-role" }; } const { device, role, tokens, existing } = context; const requestedScopes = normalizeDeviceAuthScopes( params.scopes ?? existing?.scopes ?? device.scopes, ); const approvedScopes = resolveApprovedDeviceScopeBaseline(device); + if (!approvedScopes) { + return { ok: false, reason: "missing-approved-scope-baseline" }; + } if ( !scopesWithinApprovedDeviceBaseline({ role, @@ -610,7 +622,7 @@ export async function rotateDeviceToken(params: { approvedScopes, }) ) { - return null; + return { ok: false, reason: "scope-outside-approved-baseline" }; } const now = Date.now(); const next = buildDeviceAuthToken({ @@ -624,7 +636,7 @@ export async function rotateDeviceToken(params: { device.tokens = tokens; state.pairedByDeviceId[device.deviceId] = device; await persistState(state, params.baseDir); - return next; + return { ok: true, entry: next }; }); } From 0c7ae04262ea61b43edc8276b8cc1d62a01842f8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Mar 2026 09:02:55 -0700 Subject: [PATCH 12/16] style: format imported model helpers --- src/agents/model-selection.ts | 2 +- src/agents/models-config.providers.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/agents/model-selection.ts b/src/agents/model-selection.ts index 1f73ca6a1b4..0f8f5568618 100644 --- a/src/agents/model-selection.ts +++ b/src/agents/model-selection.ts @@ -13,8 +13,8 @@ import { resolveAgentModelFallbacksOverride, } from "./agent-scope.js"; import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "./defaults.js"; -import { normalizeGoogleModelId } from "./model-id-normalization.js"; import type { ModelCatalogEntry } from "./model-catalog.js"; +import { normalizeGoogleModelId } from "./model-id-normalization.js"; import { splitTrailingAuthProfile } from "./model-ref-profile.js"; const log = createSubsystemLogger("model-selection"); diff --git a/src/agents/models-config.providers.ts b/src/agents/models-config.providers.ts index 229a861c0e5..03110d3fba5 100644 --- a/src/agents/models-config.providers.ts +++ b/src/agents/models-config.providers.ts @@ -8,11 +8,11 @@ import { isRecord } from "../utils.js"; import { normalizeOptionalSecretInput } from "../utils/normalize-secret-input.js"; import { ensureAuthProfileStore, listProfilesForProvider } from "./auth-profiles.js"; import { discoverBedrockModels } from "./bedrock-discovery.js"; -import { normalizeGoogleModelId } from "./model-id-normalization.js"; import { buildCloudflareAiGatewayModelDefinition, resolveCloudflareAiGatewayBaseUrl, } from "./cloudflare-ai-gateway.js"; +import { normalizeGoogleModelId } from "./model-id-normalization.js"; import { buildHuggingfaceProvider, buildKilocodeProviderWithDiscovery, From 8d44b16b7cc7705dc878ef7cc9fbcba6dcb9e179 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 15 Mar 2026 09:07:10 -0700 Subject: [PATCH 13/16] Plugins: preserve scoped ids and reserve bundled duplicates (#47413) * Plugins: preserve scoped ids and reserve bundled duplicates * Changelog: add plugin scoped id note * Plugins: harden scoped install ids * Plugins: reserve scoped install dirs * Plugins: migrate legacy scoped update ids --- CHANGELOG.md | 1 + src/infra/install-safe-path.ts | 4 +- src/infra/install-target.ts | 2 + src/plugins/install.test.ts | 81 +++++++++++++++++++++++---- src/plugins/install.ts | 77 ++++++++++++++++++++++--- src/plugins/loader.test.ts | 40 ++++++++++++- src/plugins/loader.ts | 22 +++++--- src/plugins/manifest-registry.test.ts | 30 ++++++++++ src/plugins/manifest-registry.ts | 24 ++++++-- src/plugins/update.test.ts | 57 +++++++++++++++++++ src/plugins/update.ts | 80 +++++++++++++++++++++++++- 11 files changed, 377 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95a68bc92cb..6b05fec4ff7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ Docs: https://docs.openclaw.ai - ACP/approvals: use canonical tool identity for prompting decisions and fail closed when conflicting tool identity hints are present. Thanks @vincentkoc. - Telegram/message send: forward `--force-document` through the `sendPayload` path as well as `sendMedia`, so Telegram payload sends with `channelData` keep uploading images as documents instead of silently falling back to compressed photo sends. (#47119) Thanks @thepagent. - Telegram/message chunking: preserve spaces, paragraph separators, and word boundaries when HTML overflow rechunking splits formatted replies. (#47274) +- Plugins/scoped ids: preserve scoped plugin ids during install and config keying, and keep bundled plugins ahead of discovered duplicate ids by default so `@scope/name` plugins no longer collide with unscoped installs. Thanks @vincentkoc. ## 2026.3.13 diff --git a/src/infra/install-safe-path.ts b/src/infra/install-safe-path.ts index 13cc88562ed..a2f012e70fb 100644 --- a/src/infra/install-safe-path.ts +++ b/src/infra/install-safe-path.ts @@ -47,8 +47,10 @@ export function resolveSafeInstallDir(params: { baseDir: string; id: string; invalidNameMessage: string; + nameEncoder?: (id: string) => string; }): { ok: true; path: string } | { ok: false; error: string } { - const targetDir = path.join(params.baseDir, safeDirName(params.id)); + const encodedName = (params.nameEncoder ?? safeDirName)(params.id); + const targetDir = path.join(params.baseDir, encodedName); const resolvedBase = path.resolve(params.baseDir); const resolvedTarget = path.resolve(targetDir); const relative = path.relative(resolvedBase, resolvedTarget); diff --git a/src/infra/install-target.ts b/src/infra/install-target.ts index 38dd103c01c..dd954a92112 100644 --- a/src/infra/install-target.ts +++ b/src/infra/install-target.ts @@ -7,12 +7,14 @@ export async function resolveCanonicalInstallTarget(params: { id: string; invalidNameMessage: string; boundaryLabel: string; + nameEncoder?: (id: string) => string; }): Promise<{ ok: true; targetDir: string } | { ok: false; error: string }> { await fs.mkdir(params.baseDir, { recursive: true }); const targetDirResult = resolveSafeInstallDir({ baseDir: params.baseDir, id: params.id, invalidNameMessage: params.invalidNameMessage, + nameEncoder: params.nameEncoder, }); if (!targetDirResult.ok) { return { ok: false, error: targetDirResult.error }; diff --git a/src/plugins/install.test.ts b/src/plugins/install.test.ts index 5f698a8e64b..db2fcfaf8f9 100644 --- a/src/plugins/install.test.ts +++ b/src/plugins/install.test.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import * as tar from "tar"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { safePathSegmentHashed } from "../infra/install-safe-path.js"; import * as skillScanner from "../security/skill-scanner.js"; import { expectSingleNpmPackIgnoreScriptsCall } from "../test-utils/exec-assertions.js"; import { @@ -20,6 +21,7 @@ let installPluginFromDir: typeof import("./install.js").installPluginFromDir; let installPluginFromNpmSpec: typeof import("./install.js").installPluginFromNpmSpec; let installPluginFromPath: typeof import("./install.js").installPluginFromPath; let PLUGIN_INSTALL_ERROR_CODE: typeof import("./install.js").PLUGIN_INSTALL_ERROR_CODE; +let resolvePluginInstallDir: typeof import("./install.js").resolvePluginInstallDir; let runCommandWithTimeout: typeof import("../process/exec.js").runCommandWithTimeout; let suiteTempRoot = ""; let suiteFixtureRoot = ""; @@ -157,7 +159,9 @@ async function setupVoiceCallArchiveInstall(params: { outName: string; version: } function expectPluginFiles(result: { targetDir: string }, stateDir: string, pluginId: string) { - expect(result.targetDir).toBe(path.join(stateDir, "extensions", pluginId)); + expect(result.targetDir).toBe( + resolvePluginInstallDir(pluginId, path.join(stateDir, "extensions")), + ); expect(fs.existsSync(path.join(result.targetDir, "package.json"))).toBe(true); expect(fs.existsSync(path.join(result.targetDir, "dist", "index.js"))).toBe(true); } @@ -331,6 +335,7 @@ beforeAll(async () => { installPluginFromNpmSpec, installPluginFromPath, PLUGIN_INSTALL_ERROR_CODE, + resolvePluginInstallDir, } = await import("./install.js")); ({ runCommandWithTimeout } = await import("../process/exec.js")); @@ -394,7 +399,7 @@ beforeEach(() => { }); describe("installPluginFromArchive", () => { - it("installs into ~/.openclaw/extensions and uses unscoped id", async () => { + it("installs into ~/.openclaw/extensions and preserves scoped package ids", async () => { const { stateDir, archivePath, extensionsDir } = await setupVoiceCallArchiveInstall({ outName: "plugin.tgz", version: "0.0.1", @@ -404,7 +409,7 @@ describe("installPluginFromArchive", () => { archivePath, extensionsDir, }); - expectSuccessfulArchiveInstall({ result, stateDir, pluginId: "voice-call" }); + expectSuccessfulArchiveInstall({ result, stateDir, pluginId: "@openclaw/voice-call" }); }); it("rejects installing when plugin already exists", async () => { @@ -443,7 +448,7 @@ describe("installPluginFromArchive", () => { archivePath, extensionsDir, }); - expectSuccessfulArchiveInstall({ result, stateDir, pluginId: "zipper" }); + expectSuccessfulArchiveInstall({ result, stateDir, pluginId: "@openclaw/zipper" }); }); it("allows updates when mode is update", async () => { @@ -615,16 +620,17 @@ describe("installPluginFromArchive", () => { }); describe("installPluginFromDir", () => { - function expectInstalledAsMemoryCognee( + function expectInstalledWithPluginId( result: Awaited>, extensionsDir: string, + pluginId: string, ) { expect(result.ok).toBe(true); if (!result.ok) { return; } - expect(result.pluginId).toBe("memory-cognee"); - expect(result.targetDir).toBe(path.join(extensionsDir, "memory-cognee")); + expect(result.pluginId).toBe(pluginId); + expect(result.targetDir).toBe(resolvePluginInstallDir(pluginId, extensionsDir)); } it("uses --ignore-scripts for dependency install", async () => { @@ -689,17 +695,17 @@ describe("installPluginFromDir", () => { logger: { info: (msg: string) => infoMessages.push(msg), warn: () => {} }, }); - expectInstalledAsMemoryCognee(res, extensionsDir); + expectInstalledWithPluginId(res, extensionsDir, "memory-cognee"); expect( infoMessages.some((msg) => msg.includes( - 'Plugin manifest id "memory-cognee" differs from npm package name "cognee-openclaw"', + 'Plugin manifest id "memory-cognee" differs from npm package name "@openclaw/cognee-openclaw"', ), ), ).toBe(true); }); - it("normalizes scoped manifest ids to unscoped install keys", async () => { + it("preserves scoped manifest ids as install keys", async () => { const { pluginDir, extensionsDir } = setupManifestInstallFixture({ manifestId: "@team/memory-cognee", }); @@ -707,11 +713,62 @@ describe("installPluginFromDir", () => { const res = await installPluginFromDir({ dirPath: pluginDir, extensionsDir, - expectedPluginId: "memory-cognee", + expectedPluginId: "@team/memory-cognee", logger: { info: () => {}, warn: () => {} }, }); - expectInstalledAsMemoryCognee(res, extensionsDir); + expectInstalledWithPluginId(res, extensionsDir, "@team/memory-cognee"); + }); + + it("preserves scoped package names when no plugin manifest id is present", async () => { + const { pluginDir, extensionsDir } = setupInstallPluginFromDirFixture(); + + const res = await installPluginFromDir({ + dirPath: pluginDir, + extensionsDir, + }); + + expectInstalledWithPluginId(res, extensionsDir, "@openclaw/test-plugin"); + }); + + it("accepts legacy unscoped expected ids for scoped package names without manifest ids", async () => { + const { pluginDir, extensionsDir } = setupInstallPluginFromDirFixture(); + + const res = await installPluginFromDir({ + dirPath: pluginDir, + extensionsDir, + expectedPluginId: "test-plugin", + }); + + expectInstalledWithPluginId(res, extensionsDir, "@openclaw/test-plugin"); + }); + + it("rejects bare @ as an invalid scoped id", () => { + expect(() => resolvePluginInstallDir("@")).toThrow( + "invalid plugin name: scoped ids must use @scope/name format", + ); + }); + + it("rejects empty scoped segments like @/name", () => { + expect(() => resolvePluginInstallDir("@/name")).toThrow( + "invalid plugin name: scoped ids must use @scope/name format", + ); + }); + + it("rejects two-segment ids without a scope prefix", () => { + expect(() => resolvePluginInstallDir("team/name")).toThrow( + "invalid plugin name: scoped ids must use @scope/name format", + ); + }); + + it("uses a unique hashed install dir for scoped ids", () => { + const extensionsDir = path.join(makeTempDir(), "extensions"); + const scopedTarget = resolvePluginInstallDir("@scope/name", extensionsDir); + const hashedFlatId = safePathSegmentHashed("@scope/name"); + const flatTarget = resolvePluginInstallDir(hashedFlatId, extensionsDir); + + expect(path.basename(scopedTarget)).toBe(`@${hashedFlatId}`); + expect(scopedTarget).not.toBe(flatTarget); }); }); diff --git a/src/plugins/install.ts b/src/plugins/install.ts index e6e107877cf..ab87377d32e 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -11,6 +11,7 @@ import { installPackageDir } from "../infra/install-package-dir.js"; import { resolveSafeInstallDir, safeDirName, + safePathSegmentHashed, unscopedPackageName, } from "../infra/install-safe-path.js"; import { @@ -84,19 +85,68 @@ function safeFileName(input: string): string { return safeDirName(input); } +function encodePluginInstallDirName(pluginId: string): string { + const trimmed = pluginId.trim(); + if (!trimmed.includes("/")) { + return safeDirName(trimmed); + } + // Scoped plugin ids need a reserved on-disk namespace so they cannot collide + // with valid unscoped ids that happen to match the hashed slug. + return `@${safePathSegmentHashed(trimmed)}`; +} + function validatePluginId(pluginId: string): string | null { - if (!pluginId) { + const trimmed = pluginId.trim(); + if (!trimmed) { return "invalid plugin name: missing"; } - if (pluginId === "." || pluginId === "..") { - return "invalid plugin name: reserved path segment"; - } - if (pluginId.includes("/") || pluginId.includes("\\")) { + if (trimmed.includes("\\")) { return "invalid plugin name: path separators not allowed"; } + const segments = trimmed.split("/"); + if (segments.some((segment) => !segment)) { + return "invalid plugin name: malformed scope"; + } + if (segments.some((segment) => segment === "." || segment === "..")) { + return "invalid plugin name: reserved path segment"; + } + if (segments.length === 1) { + if (trimmed.startsWith("@")) { + return "invalid plugin name: scoped ids must use @scope/name format"; + } + return null; + } + if (segments.length !== 2) { + return "invalid plugin name: path separators not allowed"; + } + if (!segments[0]?.startsWith("@") || segments[0].length < 2) { + return "invalid plugin name: scoped ids must use @scope/name format"; + } return null; } +function matchesExpectedPluginId(params: { + expectedPluginId?: string; + pluginId: string; + manifestPluginId?: string; + npmPluginId: string; +}): boolean { + if (!params.expectedPluginId) { + return true; + } + if (params.expectedPluginId === params.pluginId) { + return true; + } + // Backward compatibility: older install records keyed scoped npm packages by + // their unscoped package name. Preserve update-in-place for those records + // unless the package declares an explicit manifest id override. + return ( + !params.manifestPluginId && + params.pluginId === params.npmPluginId && + params.expectedPluginId === unscopedPackageName(params.npmPluginId) + ); +} + function ensureOpenClawExtensions(params: { manifest: PackageManifest }): | { ok: true; @@ -195,6 +245,7 @@ export function resolvePluginInstallDir(pluginId: string, extensionsDir?: string baseDir: extensionsBase, id: pluginId, invalidNameMessage: "invalid plugin name: path traversal detected", + nameEncoder: encodePluginInstallDirName, }); if (!targetDirResult.ok) { throw new Error(targetDirResult.error); @@ -233,8 +284,8 @@ async function installPluginFromPackageDir( } const extensions = extensionsResult.entries; - const pkgName = typeof manifest.name === "string" ? manifest.name : ""; - const npmPluginId = pkgName ? unscopedPackageName(pkgName) : "plugin"; + const pkgName = typeof manifest.name === "string" ? manifest.name.trim() : ""; + const npmPluginId = pkgName || "plugin"; // Prefer the canonical `id` from openclaw.plugin.json over the npm package name. // This avoids a latent key-mismatch bug: if the manifest id (e.g. "memory-cognee") @@ -243,7 +294,7 @@ async function installPluginFromPackageDir( const ocManifestResult = loadPluginManifest(params.packageDir); const manifestPluginId = ocManifestResult.ok && ocManifestResult.manifest.id - ? unscopedPackageName(ocManifestResult.manifest.id) + ? ocManifestResult.manifest.id.trim() : undefined; const pluginId = manifestPluginId ?? npmPluginId; @@ -251,7 +302,14 @@ async function installPluginFromPackageDir( if (pluginIdError) { return { ok: false, error: pluginIdError }; } - if (params.expectedPluginId && params.expectedPluginId !== pluginId) { + if ( + !matchesExpectedPluginId({ + expectedPluginId: params.expectedPluginId, + pluginId, + manifestPluginId, + npmPluginId, + }) + ) { return { ok: false, error: `plugin id mismatch: expected ${params.expectedPluginId}, got ${pluginId}`, @@ -313,6 +371,7 @@ async function installPluginFromPackageDir( id: pluginId, invalidNameMessage: "invalid plugin name: path traversal detected", boundaryLabel: "extensions directory", + nameEncoder: encodePluginInstallDirName, }); if (!targetDirResult.ok) { return { ok: false, error: targetDirResult.error }; diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index 4771d98aa31..c37cfbfd46c 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -1692,7 +1692,37 @@ describe("loadOpenClawPlugins", () => { expect(workspacePlugin?.status).toBe("loaded"); }); - it("lets an explicitly trusted workspace plugin shadow a bundled plugin with the same id", () => { + it("keeps scoped and unscoped plugin ids distinct", () => { + useNoBundledPlugins(); + const scoped = writePlugin({ + id: "@team/shadowed", + body: `module.exports = { id: "@team/shadowed", register() {} };`, + filename: "scoped.cjs", + }); + const unscoped = writePlugin({ + id: "shadowed", + body: `module.exports = { id: "shadowed", register() {} };`, + filename: "unscoped.cjs", + }); + + const registry = loadOpenClawPlugins({ + cache: false, + config: { + plugins: { + load: { paths: [scoped.file, unscoped.file] }, + allow: ["@team/shadowed", "shadowed"], + }, + }, + }); + + expect(registry.plugins.find((entry) => entry.id === "@team/shadowed")?.status).toBe("loaded"); + expect(registry.plugins.find((entry) => entry.id === "shadowed")?.status).toBe("loaded"); + expect( + registry.diagnostics.some((diag) => String(diag.message).includes("duplicate plugin id")), + ).toBe(false); + }); + + it("keeps bundled plugins ahead of trusted workspace duplicates with the same id", () => { const bundledDir = makeTempDir(); writePlugin({ id: "shadowed", @@ -1719,6 +1749,9 @@ describe("loadOpenClawPlugins", () => { plugins: { enabled: true, allow: ["shadowed"], + entries: { + shadowed: { enabled: true }, + }, }, }, }); @@ -1726,8 +1759,9 @@ describe("loadOpenClawPlugins", () => { const entries = registry.plugins.filter((entry) => entry.id === "shadowed"); const loaded = entries.find((entry) => entry.status === "loaded"); const overridden = entries.find((entry) => entry.status === "disabled"); - expect(loaded?.origin).toBe("workspace"); - expect(overridden?.origin).toBe("bundled"); + expect(loaded?.origin).toBe("bundled"); + expect(overridden?.origin).toBe("workspace"); + expect(overridden?.error).toContain("overridden by bundled plugin"); }); it("warns when loaded non-bundled plugin has no install/load-path provenance", () => { diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index 698918964f9..253ad63afc4 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -485,16 +485,20 @@ function resolveCandidateDuplicateRank(params: { env: params.env, }); - switch (params.candidate.origin) { - case "config": - return 0; - case "workspace": - return 1; - case "global": - return isExplicitInstall ? 2 : 4; - case "bundled": - return 3; + if (params.candidate.origin === "config") { + return 0; } + if (params.candidate.origin === "global" && isExplicitInstall) { + return 1; + } + if (params.candidate.origin === "bundled") { + // Bundled plugin ids stay reserved unless the operator configured an override. + return 2; + } + if (params.candidate.origin === "workspace") { + return 3; + } + return 4; } function compareDuplicateCandidateOrder(params: { diff --git a/src/plugins/manifest-registry.test.ts b/src/plugins/manifest-registry.test.ts index bbdc8020d6e..214c9b3b23f 100644 --- a/src/plugins/manifest-registry.test.ts +++ b/src/plugins/manifest-registry.test.ts @@ -225,6 +225,36 @@ describe("loadPluginManifestRegistry", () => { ).toBe(true); }); + it("reports bundled plugins as the duplicate winner for workspace duplicates", () => { + const bundledDir = makeTempDir(); + const workspaceDir = makeTempDir(); + const manifest = { id: "shadowed", configSchema: { type: "object" } }; + writeManifest(bundledDir, manifest); + writeManifest(workspaceDir, manifest); + + const registry = loadPluginManifestRegistry({ + cache: false, + candidates: [ + createPluginCandidate({ + idHint: "shadowed", + rootDir: bundledDir, + origin: "bundled", + }), + createPluginCandidate({ + idHint: "shadowed", + rootDir: workspaceDir, + origin: "workspace", + }), + ], + }); + + expect( + registry.diagnostics.some((diag) => + diag.message.includes("workspace plugin will be overridden by bundled plugin"), + ), + ).toBe(true); + }); + it("suppresses duplicate warning when candidates share the same physical directory via symlink", () => { const realDir = makeTempDir(); const manifest = { id: "feishu", configSchema: { type: "object" } }; diff --git a/src/plugins/manifest-registry.ts b/src/plugins/manifest-registry.ts index 79fb3facf8e..285b3042004 100644 --- a/src/plugins/manifest-registry.ts +++ b/src/plugins/manifest-registry.ts @@ -13,7 +13,8 @@ type SeenIdEntry = { recordIndex: number; }; -// Precedence: config > workspace > explicit-install global > bundled > auto-discovered global +// Canonicalize identical physical plugin roots with the most explicit source. +// This only applies when multiple candidates resolve to the same on-disk plugin. const PLUGIN_ORIGIN_RANK: Readonly> = { config: 0, workspace: 1, @@ -167,17 +168,28 @@ function resolveDuplicatePrecedenceRank(params: { config?: OpenClawConfig; env: NodeJS.ProcessEnv; }): number { - if (params.candidate.origin === "global") { - return matchesInstalledPluginRecord({ + if (params.candidate.origin === "config") { + return 0; + } + if ( + params.candidate.origin === "global" && + matchesInstalledPluginRecord({ pluginId: params.pluginId, candidate: params.candidate, config: params.config, env: params.env, }) - ? 2 - : 4; + ) { + return 1; } - return PLUGIN_ORIGIN_RANK[params.candidate.origin]; + if (params.candidate.origin === "bundled") { + // Bundled plugin ids are reserved unless the operator explicitly overrides them. + return 2; + } + if (params.candidate.origin === "workspace") { + return 3; + } + return 4; } export function loadPluginManifestRegistry(params: { diff --git a/src/plugins/update.test.ts b/src/plugins/update.test.ts index 65ef9966a83..4d3b72ed65d 100644 --- a/src/plugins/update.test.ts +++ b/src/plugins/update.test.ts @@ -156,6 +156,63 @@ describe("updateNpmInstalledPlugins", () => { }, ]); }); + + it("migrates legacy unscoped install keys when a scoped npm package updates", async () => { + installPluginFromNpmSpecMock.mockResolvedValue({ + ok: true, + pluginId: "@openclaw/voice-call", + targetDir: "/tmp/openclaw-voice-call", + version: "0.0.2", + extensions: ["index.ts"], + }); + + const { updateNpmInstalledPlugins } = await import("./update.js"); + const result = await updateNpmInstalledPlugins({ + config: { + plugins: { + allow: ["voice-call"], + deny: ["voice-call"], + slots: { memory: "voice-call" }, + entries: { + "voice-call": { + enabled: false, + hooks: { allowPromptInjection: false }, + }, + }, + installs: { + "voice-call": { + source: "npm", + spec: "@openclaw/voice-call", + installPath: "/tmp/voice-call", + }, + }, + }, + }, + pluginIds: ["voice-call"], + }); + + expect(installPluginFromNpmSpecMock).toHaveBeenCalledWith( + expect.objectContaining({ + spec: "@openclaw/voice-call", + expectedPluginId: "voice-call", + }), + ); + expect(result.config.plugins?.allow).toEqual(["@openclaw/voice-call"]); + expect(result.config.plugins?.deny).toEqual(["@openclaw/voice-call"]); + expect(result.config.plugins?.slots?.memory).toBe("@openclaw/voice-call"); + expect(result.config.plugins?.entries?.["@openclaw/voice-call"]).toEqual({ + enabled: false, + hooks: { allowPromptInjection: false }, + }); + expect(result.config.plugins?.entries?.["voice-call"]).toBeUndefined(); + expect(result.config.plugins?.installs?.["@openclaw/voice-call"]).toMatchObject({ + source: "npm", + spec: "@openclaw/voice-call", + installPath: "/tmp/openclaw-voice-call", + version: "0.0.2", + }); + expect(result.config.plugins?.installs?.["voice-call"]).toBeUndefined(); + }); }); describe("syncPluginsForUpdateChannel", () => { diff --git a/src/plugins/update.ts b/src/plugins/update.ts index b214558bc57..af6434e84cc 100644 --- a/src/plugins/update.ts +++ b/src/plugins/update.ts @@ -172,6 +172,79 @@ function buildLoadPathHelpers(existing: string[], env: NodeJS.ProcessEnv = proce }; } +function replacePluginIdInList( + entries: string[] | undefined, + fromId: string, + toId: string, +): string[] | undefined { + if (!entries || entries.length === 0 || fromId === toId) { + return entries; + } + const next: string[] = []; + for (const entry of entries) { + const value = entry === fromId ? toId : entry; + if (!next.includes(value)) { + next.push(value); + } + } + return next; +} + +function migratePluginConfigId(cfg: OpenClawConfig, fromId: string, toId: string): OpenClawConfig { + if (fromId === toId) { + return cfg; + } + + const installs = cfg.plugins?.installs; + const entries = cfg.plugins?.entries; + const slots = cfg.plugins?.slots; + const allow = replacePluginIdInList(cfg.plugins?.allow, fromId, toId); + const deny = replacePluginIdInList(cfg.plugins?.deny, fromId, toId); + + const nextInstalls = installs ? { ...installs } : undefined; + if (nextInstalls && fromId in nextInstalls) { + const record = nextInstalls[fromId]; + if (record && !(toId in nextInstalls)) { + nextInstalls[toId] = record; + } + delete nextInstalls[fromId]; + } + + const nextEntries = entries ? { ...entries } : undefined; + if (nextEntries && fromId in nextEntries) { + const entry = nextEntries[fromId]; + if (entry) { + nextEntries[toId] = nextEntries[toId] + ? { + ...entry, + ...nextEntries[toId], + } + : entry; + } + delete nextEntries[fromId]; + } + + const nextSlots = + slots?.memory === fromId + ? { + ...slots, + memory: toId, + } + : slots; + + return { + ...cfg, + plugins: { + ...cfg.plugins, + allow, + deny, + entries: nextEntries, + installs: nextInstalls, + slots: nextSlots, + }, + }; +} + function createPluginUpdateIntegrityDriftHandler(params: { pluginId: string; dryRun: boolean; @@ -362,9 +435,14 @@ export async function updateNpmInstalledPlugins(params: { continue; } + const resolvedPluginId = result.pluginId; + if (resolvedPluginId !== pluginId) { + next = migratePluginConfigId(next, pluginId, resolvedPluginId); + } + const nextVersion = result.version ?? (await readInstalledPackageVersion(result.targetDir)); next = recordPluginInstall(next, { - pluginId, + pluginId: resolvedPluginId, source: "npm", spec: record.spec, installPath: result.targetDir, From 67b2d1b8e82e7002c8820a3542cf8e569b1d3dcf Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 15 Mar 2026 09:10:40 -0700 Subject: [PATCH 14/16] CLI: reduce channels add startup memory (#46784) * CLI: lazy-load channel subcommand handlers * Channels: defer add command dependencies * CLI: skip status JSON plugin preload * CLI: cover status JSON route preload * Status: trim JSON security audit path * Status: update JSON fast-path tests * CLI: cover root help fast path * CLI: fast-path root help * Status: keep JSON security parity * Status: restore JSON security tests * CLI: document status plugin preload * Channels: reuse Telegram account import --- src/cli/channels-cli.ts | 16 ++++++------- src/cli/program/command-registry.ts | 4 ++++ src/cli/program/root-help.ts | 29 +++++++++++++++++++++++ src/cli/program/routes.test.ts | 2 +- src/cli/run-main.exit.test.ts | 25 ++++++++++++++++++++ src/cli/run-main.test.ts | 10 ++++++++ src/cli/run-main.ts | 17 +++++++++++++- src/commands/channels/add.ts | 36 ++++++++++++++++++----------- src/commands/status.test.ts | 10 ++++++-- 9 files changed, 123 insertions(+), 26 deletions(-) create mode 100644 src/cli/program/root-help.ts diff --git a/src/cli/channels-cli.ts b/src/cli/channels-cli.ts index 8a1b8eb3f53..3015ed1d42a 100644 --- a/src/cli/channels-cli.ts +++ b/src/cli/channels-cli.ts @@ -1,13 +1,4 @@ import type { Command } from "commander"; -import { - channelsAddCommand, - channelsCapabilitiesCommand, - channelsListCommand, - channelsLogsCommand, - channelsRemoveCommand, - channelsResolveCommand, - channelsStatusCommand, -} from "../commands/channels.js"; import { danger } from "../globals.js"; import { defaultRuntime } from "../runtime.js"; import { formatDocsLink } from "../terminal/links.js"; @@ -96,6 +87,7 @@ export function registerChannelsCli(program: Command) { .option("--json", "Output JSON", false) .action(async (opts) => { await runChannelsCommand(async () => { + const { channelsListCommand } = await import("../commands/channels.js"); await channelsListCommand(opts, defaultRuntime); }); }); @@ -108,6 +100,7 @@ export function registerChannelsCli(program: Command) { .option("--json", "Output JSON", false) .action(async (opts) => { await runChannelsCommand(async () => { + const { channelsStatusCommand } = await import("../commands/channels.js"); await channelsStatusCommand(opts, defaultRuntime); }); }); @@ -122,6 +115,7 @@ export function registerChannelsCli(program: Command) { .option("--json", "Output JSON", false) .action(async (opts) => { await runChannelsCommand(async () => { + const { channelsCapabilitiesCommand } = await import("../commands/channels.js"); await channelsCapabilitiesCommand(opts, defaultRuntime); }); }); @@ -136,6 +130,7 @@ export function registerChannelsCli(program: Command) { .option("--json", "Output JSON", false) .action(async (entries, opts) => { await runChannelsCommand(async () => { + const { channelsResolveCommand } = await import("../commands/channels.js"); await channelsResolveCommand( { channel: opts.channel as string | undefined, @@ -157,6 +152,7 @@ export function registerChannelsCli(program: Command) { .option("--json", "Output JSON", false) .action(async (opts) => { await runChannelsCommand(async () => { + const { channelsLogsCommand } = await import("../commands/channels.js"); await channelsLogsCommand(opts, defaultRuntime); }); }); @@ -200,6 +196,7 @@ export function registerChannelsCli(program: Command) { .option("--use-env", "Use env token (default account only)", false) .action(async (opts, command) => { await runChannelsCommand(async () => { + const { channelsAddCommand } = await import("../commands/channels.js"); const hasFlags = hasExplicitOptions(command, optionNamesAdd); await channelsAddCommand(opts, defaultRuntime, { hasFlags }); }); @@ -213,6 +210,7 @@ export function registerChannelsCli(program: Command) { .option("--delete", "Delete config entries (no prompt)", false) .action(async (opts, command) => { await runChannelsCommand(async () => { + const { channelsRemoveCommand } = await import("../commands/channels.js"); const hasFlags = hasExplicitOptions(command, optionNamesRemove); await channelsRemoveCommand(opts, defaultRuntime, { hasFlags }); }); diff --git a/src/cli/program/command-registry.ts b/src/cli/program/command-registry.ts index 3e2338f3475..ad468878aeb 100644 --- a/src/cli/program/command-registry.ts +++ b/src/cli/program/command-registry.ts @@ -235,6 +235,10 @@ function collectCoreCliCommandNames(predicate?: (command: CoreCliCommandDescript return names; } +export function getCoreCliCommandDescriptors(): ReadonlyArray { + return coreEntries.flatMap((entry) => entry.commands); +} + export function getCoreCliCommandNames(): string[] { return collectCoreCliCommandNames(); } diff --git a/src/cli/program/root-help.ts b/src/cli/program/root-help.ts new file mode 100644 index 00000000000..b80302e9818 --- /dev/null +++ b/src/cli/program/root-help.ts @@ -0,0 +1,29 @@ +import { Command } from "commander"; +import { VERSION } from "../../version.js"; +import { getCoreCliCommandDescriptors } from "./command-registry.js"; +import { configureProgramHelp } from "./help.js"; +import { getSubCliEntries } from "./register.subclis.js"; + +function buildRootHelpProgram(): Command { + const program = new Command(); + configureProgramHelp(program, { + programVersion: VERSION, + channelOptions: [], + messageChannelOptions: "", + agentChannelOptions: "", + }); + + for (const command of getCoreCliCommandDescriptors()) { + program.command(command.name).description(command.description); + } + for (const command of getSubCliEntries()) { + program.command(command.name).description(command.description); + } + + return program; +} + +export function outputRootHelp(): void { + const program = buildRootHelpProgram(); + program.outputHelp(); +} diff --git a/src/cli/program/routes.test.ts b/src/cli/program/routes.test.ts index 61be251097e..e7958a684a5 100644 --- a/src/cli/program/routes.test.ts +++ b/src/cli/program/routes.test.ts @@ -32,7 +32,7 @@ describe("program routes", () => { await expect(route?.run(argv)).resolves.toBe(false); } - it("matches status route and always loads plugins for security parity", () => { + it("matches status route and always preloads plugins", () => { const route = expectRoute(["status"]); expect(route?.loadPlugins).toBe(true); }); diff --git a/src/cli/run-main.exit.test.ts b/src/cli/run-main.exit.test.ts index 3e56c1ce794..6af996ed820 100644 --- a/src/cli/run-main.exit.test.ts +++ b/src/cli/run-main.exit.test.ts @@ -7,6 +7,8 @@ const normalizeEnvMock = vi.hoisted(() => vi.fn()); const ensurePathMock = vi.hoisted(() => vi.fn()); const assertRuntimeMock = vi.hoisted(() => vi.fn()); const closeAllMemorySearchManagersMock = vi.hoisted(() => vi.fn(async () => {})); +const outputRootHelpMock = vi.hoisted(() => vi.fn()); +const buildProgramMock = vi.hoisted(() => vi.fn()); vi.mock("./route.js", () => ({ tryRouteCli: tryRouteCliMock, @@ -32,6 +34,14 @@ vi.mock("../memory/search-manager.js", () => ({ closeAllMemorySearchManagers: closeAllMemorySearchManagersMock, })); +vi.mock("./program/root-help.js", () => ({ + outputRootHelp: outputRootHelpMock, +})); + +vi.mock("./program.js", () => ({ + buildProgram: buildProgramMock, +})); + const { runCli } = await import("./run-main.js"); describe("runCli exit behavior", () => { @@ -52,4 +62,19 @@ describe("runCli exit behavior", () => { expect(exitSpy).not.toHaveBeenCalled(); exitSpy.mockRestore(); }); + + it("renders root help without building the full program", async () => { + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`unexpected process.exit(${String(code)})`); + }) as typeof process.exit); + + await runCli(["node", "openclaw", "--help"]); + + expect(tryRouteCliMock).not.toHaveBeenCalled(); + expect(outputRootHelpMock).toHaveBeenCalledTimes(1); + expect(buildProgramMock).not.toHaveBeenCalled(); + expect(closeAllMemorySearchManagersMock).toHaveBeenCalledTimes(1); + expect(exitSpy).not.toHaveBeenCalled(); + exitSpy.mockRestore(); + }); }); diff --git a/src/cli/run-main.test.ts b/src/cli/run-main.test.ts index 495a23684d1..63259259134 100644 --- a/src/cli/run-main.test.ts +++ b/src/cli/run-main.test.ts @@ -4,6 +4,7 @@ import { shouldEnsureCliPath, shouldRegisterPrimarySubcommand, shouldSkipPluginCommandRegistration, + shouldUseRootHelpFastPath, } from "./run-main.js"; describe("rewriteUpdateFlagArgv", () => { @@ -126,3 +127,12 @@ describe("shouldEnsureCliPath", () => { expect(shouldEnsureCliPath(["node", "openclaw", "acp", "-v"])).toBe(true); }); }); + +describe("shouldUseRootHelpFastPath", () => { + it("uses the fast path for root help only", () => { + expect(shouldUseRootHelpFastPath(["node", "openclaw", "--help"])).toBe(true); + expect(shouldUseRootHelpFastPath(["node", "openclaw", "--profile", "work", "-h"])).toBe(true); + expect(shouldUseRootHelpFastPath(["node", "openclaw", "status", "--help"])).toBe(false); + expect(shouldUseRootHelpFastPath(["node", "openclaw", "--help", "status"])).toBe(false); + }); +}); diff --git a/src/cli/run-main.ts b/src/cli/run-main.ts index c0673ddf2af..188448a64e4 100644 --- a/src/cli/run-main.ts +++ b/src/cli/run-main.ts @@ -8,7 +8,12 @@ import { ensureOpenClawCliOnPath } from "../infra/path-env.js"; import { assertSupportedRuntime } from "../infra/runtime-guard.js"; import { installUnhandledRejectionHandler } from "../infra/unhandled-rejections.js"; import { enableConsoleCapture } from "../logging.js"; -import { getCommandPathWithRootOptions, getPrimaryCommand, hasHelpOrVersion } from "./argv.js"; +import { + getCommandPathWithRootOptions, + getPrimaryCommand, + hasHelpOrVersion, + isRootHelpInvocation, +} from "./argv.js"; import { applyCliProfileEnv, parseCliProfileArgs } from "./profile.js"; import { tryRouteCli } from "./route.js"; import { normalizeWindowsArgv } from "./windows-argv.js"; @@ -71,6 +76,10 @@ export function shouldEnsureCliPath(argv: string[]): boolean { return true; } +export function shouldUseRootHelpFastPath(argv: string[]): boolean { + return isRootHelpInvocation(argv); +} + export async function runCli(argv: string[] = process.argv) { let normalizedArgv = normalizeWindowsArgv(argv); const parsedProfile = parseCliProfileArgs(normalizedArgv); @@ -92,6 +101,12 @@ export async function runCli(argv: string[] = process.argv) { assertSupportedRuntime(); try { + if (shouldUseRootHelpFastPath(normalizedArgv)) { + const { outputRootHelp } = await import("./program/root-help.js"); + outputRootHelp(); + return; + } + if (await tryRouteCli(normalizedArgv)) { return; } diff --git a/src/commands/channels/add.ts b/src/commands/channels/add.ts index 3cc2f305870..52a358f4946 100644 --- a/src/commands/channels/add.ts +++ b/src/commands/channels/add.ts @@ -1,5 +1,3 @@ -import { resolveTelegramAccount } from "../../../extensions/telegram/src/accounts.js"; -import { deleteTelegramUpdateOffset } from "../../../extensions/telegram/src/update-offset-store.js"; import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../../agents/agent-scope.js"; import { listChannelPluginCatalogEntries } from "../../channels/plugins/catalog.js"; import { parseOptionalDelimitedEntries } from "../../channels/plugins/helpers.js"; @@ -11,13 +9,7 @@ import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../../routing/session-ke import { defaultRuntime, type RuntimeEnv } from "../../runtime.js"; import { createClackPrompter } from "../../wizard/clack-prompter.js"; import { applyAgentBindings, describeBinding } from "../agents.bindings.js"; -import { buildAgentSummaries } from "../agents.config.js"; -import { setupChannels } from "../onboard-channels.js"; import type { ChannelChoice } from "../onboard-types.js"; -import { - ensureOnboardingPluginInstalled, - reloadOnboardingPluginRegistry, -} from "../onboarding/plugin-install.js"; import { applyAccountName, applyChannelAccountConfig } from "./add-mutators.js"; import { channelLabel, requireValidConfig, shouldUseWizard } from "./shared.js"; @@ -56,6 +48,10 @@ export async function channelsAddCommand( const useWizard = shouldUseWizard(params); if (useWizard) { + const [{ buildAgentSummaries }, { setupChannels }] = await Promise.all([ + import("../agents.config.js"), + import("../onboard-channels.js"), + ]); const prompter = createClackPrompter(); let selection: ChannelChoice[] = []; const accountIds: Partial> = {}; @@ -176,6 +172,8 @@ export async function channelsAddCommand( let catalogEntry = channel ? undefined : resolveCatalogChannelEntry(rawChannel, nextConfig); if (!channel && catalogEntry) { + const { ensureOnboardingPluginInstalled, reloadOnboardingPluginRegistry } = + await import("../onboarding/plugin-install.js"); const prompter = createClackPrompter(); const workspaceDir = resolveAgentWorkspaceDir(nextConfig, resolveDefaultAgentId(nextConfig)); const result = await ensureOnboardingPluginInstalled({ @@ -269,10 +267,20 @@ export async function channelsAddCommand( return; } - const previousTelegramToken = - channel === "telegram" - ? resolveTelegramAccount({ cfg: nextConfig, accountId }).token.trim() - : ""; + let previousTelegramToken = ""; + let resolveTelegramAccount: + | (( + params: Parameters< + typeof import("../../../extensions/telegram/src/accounts.js").resolveTelegramAccount + >[0], + ) => ReturnType< + typeof import("../../../extensions/telegram/src/accounts.js").resolveTelegramAccount + >) + | undefined; + if (channel === "telegram") { + ({ resolveTelegramAccount } = await import("../../../extensions/telegram/src/accounts.js")); + previousTelegramToken = resolveTelegramAccount({ cfg: nextConfig, accountId }).token.trim(); + } if (accountId !== DEFAULT_ACCOUNT_ID) { nextConfig = moveSingleAccountChannelSectionToDefaultAccount({ @@ -288,7 +296,9 @@ export async function channelsAddCommand( input, }); - if (channel === "telegram") { + if (channel === "telegram" && resolveTelegramAccount) { + const { deleteTelegramUpdateOffset } = + await import("../../../extensions/telegram/src/update-offset-store.js"); const nextTelegramToken = resolveTelegramAccount({ cfg: nextConfig, accountId }).token.trim(); if (previousTelegramToken !== nextTelegramToken) { // Clear stale polling offsets after Telegram token rotation. diff --git a/src/commands/status.test.ts b/src/commands/status.test.ts index e307ffa3694..c40693302ac 100644 --- a/src/commands/status.test.ts +++ b/src/commands/status.test.ts @@ -417,6 +417,12 @@ describe("statusCommand", () => { expect(payload.securityAudit.summary.warn).toBe(1); expect(payload.gatewayService.label).toBe("LaunchAgent"); expect(payload.nodeService.label).toBe("LaunchAgent"); + expect(mocks.runSecurityAudit).toHaveBeenCalledWith( + expect.objectContaining({ + includeFilesystem: true, + includeChannelSecurity: true, + }), + ); }); it("surfaces unknown usage when totalTokens is missing", async () => { @@ -505,8 +511,8 @@ describe("statusCommand", () => { await statusCommand({ json: true }, runtime as never); const payload = JSON.parse(String(runtimeLogMock.mock.calls.at(-1)?.[0])); - expect(payload.gateway.error).toContain("gateway.auth.token"); - expect(payload.gateway.error).toContain("SecretRef"); + expect(payload.gateway.error ?? payload.gateway.authWarning ?? null).not.toBeNull(); + expect(runtime.error).not.toHaveBeenCalled(); }); it("surfaces channel runtime errors from the gateway", async () => { From a47722de7e3c9cbda8d5512747ca7e3bb8f6ee66 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 15 Mar 2026 09:24:24 -0700 Subject: [PATCH 15/16] Integrations: tighten inbound callback and allowlist checks (#46787) * Integrations: harden inbound callback and allowlist handling * Integrations: address review follow-ups * Update CHANGELOG.md * Mattermost: avoid command-gating open button callbacks --- CHANGELOG.md | 4 +- extensions/googlechat/src/auth.test.ts | 97 +++++++++++++++++++ extensions/googlechat/src/auth.ts | 31 +++++- extensions/googlechat/src/monitor-webhook.ts | 2 + .../src/mattermost/interactions.test.ts | 31 ++++++ .../mattermost/src/mattermost/interactions.ts | 35 +++++++ .../mattermost/src/mattermost/monitor.ts | 39 ++++++++ .../nextcloud-talk/src/inbound.authz.test.ts | 73 ++++++++++++++ extensions/nextcloud-talk/src/inbound.ts | 1 - extensions/nextcloud-talk/src/policy.ts | 10 +- extensions/twitch/src/access-control.test.ts | 7 ++ extensions/twitch/src/access-control.ts | 8 +- src/config/types.googlechat.ts | 2 + src/config/zod-schema.providers-core.ts | 1 + 14 files changed, 323 insertions(+), 18 deletions(-) create mode 100644 extensions/googlechat/src/auth.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b05fec4ff7..98b77975d4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai - Zalo Personal/group gating: stop reapplying `dmPolicy.allowFrom` as a sender gate for already-allowlisted groups when `groupAllowFrom` is unset, so any member of an allowed group can trigger replies while DMs stay restricted. (#40146) - Browser/remote CDP: honor strict browser SSRF policy during remote CDP reachability and `/json/version` discovery checks, redact sensitive `cdpUrl` tokens from status output, and warn when remote CDP targets private/internal hosts. - Plugins/install precedence: keep bundled plugins ahead of auto-discovered globals by default, but let an explicitly installed plugin record win its own duplicate-id tie so installed channel plugins load from `~/.openclaw/extensions` after `openclaw plugins install`. +- Inbound policy hardening: tighten callback and webhook sender checks across Mattermost and Google Chat, match Nextcloud Talk rooms by stable room token, and treat explicit empty Twitch allowlists as deny-all. Thanks @vincentkoc. - macOS/canvas actions: keep unattended local agent actions on trusted in-app canvas surfaces only, and stop exposing the deep-link fallback key to arbitrary page scripts. Thanks @vincentkoc. - Agents/compaction: extend the enclosing run deadline once while compaction is actively in flight, and abort the underlying SDK compaction on timeout/cancel so large-session compactions stop freezing mid-run. (#46889) Thanks @asyncjason. - Models/openai-completions: default non-native OpenAI-compatible providers to omit tool-definition `strict` fields unless users explicitly opt back in, so tool calling keeps working on providers that reject that option. (#45497) Thanks @sahancava. @@ -36,9 +37,6 @@ Docs: https://docs.openclaw.ai - WhatsApp/login: wait for pending creds writes before reopening after Baileys `515` pairing restarts in both QR login and `channels login` flows, and keep the restart coverage pinned to the real wrapped error shape plus per-account creds queues. (#27910) Thanks @asyncjason. - Agents/openai-compatible tool calls: deduplicate repeated tool call ids across live assistant messages and replayed history so OpenAI-compatible backends no longer reject duplicate `tool_call_id` values with HTTP 400. (#40996) Thanks @xaeon2026. - Security/device pairing: harden `device.token.rotate` deny handling by keeping public failures generic while logging internal deny reasons and preserving approved-baseline enforcement. (`GHSA-7jrw-x62h-64p8`) - -### Fixes - - Slack/interactive replies: preserve `channelData.slack.blocks` through live DM delivery and preview-finalized edits so Block Kit button and select directives render instead of falling back to raw text. Thanks @vincentkoc. - Zalo/plugin runtime: export `resolveClientIp` from `openclaw/plugin-sdk/zalo` so installed builds no longer crash on startup when the webhook monitor loads from the packaged extension instead of the monorepo source tree. (#46549) Thanks @No898. - CI/channel test routing: move the built-in channel suites into `test:channels` and keep them out of `test:extensions`, so extension CI no longer fails after the channel migration while targeted test routing still sends Slack, Signal, and iMessage suites to the right lane. (#46066) Thanks @scoootscooob. diff --git a/extensions/googlechat/src/auth.test.ts b/extensions/googlechat/src/auth.test.ts new file mode 100644 index 00000000000..9fa39e51c65 --- /dev/null +++ b/extensions/googlechat/src/auth.test.ts @@ -0,0 +1,97 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + verifyIdToken: vi.fn(), +})); + +vi.mock("google-auth-library", () => ({ + GoogleAuth: class {}, + OAuth2Client: class { + verifyIdToken = mocks.verifyIdToken; + }, +})); + +const { verifyGoogleChatRequest } = await import("./auth.js"); + +function mockTicket(payload: Record) { + mocks.verifyIdToken.mockResolvedValue({ + getPayload: () => payload, + }); +} + +describe("verifyGoogleChatRequest", () => { + beforeEach(() => { + mocks.verifyIdToken.mockReset(); + }); + + it("accepts Google Chat app-url tokens from the Chat issuer", async () => { + mockTicket({ + email: "chat@system.gserviceaccount.com", + email_verified: true, + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + }), + ).resolves.toEqual({ ok: true }); + }); + + it("rejects add-on tokens when no principal binding is configured", async () => { + mockTicket({ + email: "service-123@gcp-sa-gsuiteaddons.iam.gserviceaccount.com", + email_verified: true, + sub: "principal-1", + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + }), + ).resolves.toEqual({ + ok: false, + reason: "missing add-on principal binding", + }); + }); + + it("accepts add-on tokens only when the bound principal matches", async () => { + mockTicket({ + email: "service-123@gcp-sa-gsuiteaddons.iam.gserviceaccount.com", + email_verified: true, + sub: "principal-1", + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + expectedAddOnPrincipal: "principal-1", + }), + ).resolves.toEqual({ ok: true }); + }); + + it("rejects add-on tokens when the bound principal does not match", async () => { + mockTicket({ + email: "service-123@gcp-sa-gsuiteaddons.iam.gserviceaccount.com", + email_verified: true, + sub: "principal-2", + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + expectedAddOnPrincipal: "principal-1", + }), + ).resolves.toEqual({ + ok: false, + reason: "unexpected add-on principal: principal-2", + }); + }); +}); diff --git a/extensions/googlechat/src/auth.ts b/extensions/googlechat/src/auth.ts index 6870ea8ec0f..dd20d1267f7 100644 --- a/extensions/googlechat/src/auth.ts +++ b/extensions/googlechat/src/auth.ts @@ -94,6 +94,7 @@ export async function verifyGoogleChatRequest(params: { bearer?: string | null; audienceType?: GoogleChatAudienceType | null; audience?: string | null; + expectedAddOnPrincipal?: string | null; }): Promise<{ ok: boolean; reason?: string }> { const bearer = params.bearer?.trim(); if (!bearer) { @@ -112,10 +113,32 @@ export async function verifyGoogleChatRequest(params: { audience, }); const payload = ticket.getPayload(); - const email = payload?.email ?? ""; - const ok = - payload?.email_verified && (email === CHAT_ISSUER || ADDON_ISSUER_PATTERN.test(email)); - return ok ? { ok: true } : { ok: false, reason: `invalid issuer: ${email}` }; + const email = String(payload?.email ?? "") + .trim() + .toLowerCase(); + if (!payload?.email_verified) { + return { ok: false, reason: "email not verified" }; + } + if (email === CHAT_ISSUER) { + return { ok: true }; + } + if (!ADDON_ISSUER_PATTERN.test(email)) { + return { ok: false, reason: `invalid issuer: ${email}` }; + } + const expectedAddOnPrincipal = params.expectedAddOnPrincipal?.trim().toLowerCase(); + if (!expectedAddOnPrincipal) { + return { ok: false, reason: "missing add-on principal binding" }; + } + const tokenPrincipal = String(payload?.sub ?? "") + .trim() + .toLowerCase(); + if (!tokenPrincipal || tokenPrincipal !== expectedAddOnPrincipal) { + return { + ok: false, + reason: `unexpected add-on principal: ${tokenPrincipal || ""}`, + }; + } + return { ok: true }; } catch (err) { return { ok: false, reason: err instanceof Error ? err.message : "invalid token" }; } diff --git a/extensions/googlechat/src/monitor-webhook.ts b/extensions/googlechat/src/monitor-webhook.ts index cde54214575..56f355cce83 100644 --- a/extensions/googlechat/src/monitor-webhook.ts +++ b/extensions/googlechat/src/monitor-webhook.ts @@ -132,6 +132,7 @@ export function createGoogleChatWebhookRequestHandler(params: { bearer: headerBearer, audienceType: target.audienceType, audience: target.audience, + expectedAddOnPrincipal: target.account.config.appPrincipal, }); return verification.ok; }, @@ -166,6 +167,7 @@ export function createGoogleChatWebhookRequestHandler(params: { bearer: parsed.addOnBearerToken, audienceType: target.audienceType, audience: target.audience, + expectedAddOnPrincipal: target.account.config.appPrincipal, }); return verification.ok; }, diff --git a/extensions/mattermost/src/mattermost/interactions.test.ts b/extensions/mattermost/src/mattermost/interactions.test.ts index 62c7bdb757f..dea16d51e57 100644 --- a/extensions/mattermost/src/mattermost/interactions.test.ts +++ b/extensions/mattermost/src/mattermost/interactions.test.ts @@ -738,6 +738,37 @@ describe("createMattermostInteractionHandler", () => { expectSuccessfulApprovalUpdate(res, requestLog); }); + it("blocks button dispatch when the sender is not allowed for the action", async () => { + const { context, token } = createActionContext(); + const dispatchButtonClick = vi.fn(); + const handleInteraction = vi.fn(); + const handler = createMattermostInteractionHandler({ + client: { + request: async (_path: string, init?: { method?: string }) => + init?.method === "PUT" ? { id: "post-1" } : createActionPost(), + } as unknown as MattermostClient, + botUserId: "bot", + accountId: "acct", + authorizeButtonClick: async () => ({ + ok: false, + response: { + ephemeral_text: "blocked", + }, + }), + handleInteraction, + dispatchButtonClick, + }); + + const res = await runHandler(handler, { + body: createInteractionBody({ context, token }), + }); + + expect(res.statusCode).toBe(200); + expect(res.body).toContain("blocked"); + expect(handleInteraction).not.toHaveBeenCalled(); + expect(dispatchButtonClick).not.toHaveBeenCalled(); + }); + it("forwards fetched post threading metadata to session and button callbacks", async () => { const enqueueSystemEvent = vi.fn(); setMattermostRuntime({ diff --git a/extensions/mattermost/src/mattermost/interactions.ts b/extensions/mattermost/src/mattermost/interactions.ts index f99d0b5d3ac..f4ef06cf1ed 100644 --- a/extensions/mattermost/src/mattermost/interactions.ts +++ b/extensions/mattermost/src/mattermost/interactions.ts @@ -37,6 +37,10 @@ export type MattermostInteractionResponse = { ephemeral_text?: string; }; +export type MattermostInteractionAuthorizationResult = + | { ok: true } + | { ok: false; statusCode?: number; response?: MattermostInteractionResponse }; + export type MattermostInteractiveButtonInput = { id?: string; callback_data?: string; @@ -404,6 +408,10 @@ export function createMattermostInteractionHandler(params: { context: Record; post: MattermostPost; }) => Promise; + authorizeButtonClick?: (opts: { + payload: MattermostInteractionPayload; + post: MattermostPost; + }) => Promise; dispatchButtonClick?: (opts: { channelId: string; userId: string; @@ -566,6 +574,33 @@ export function createMattermostInteractionHandler(params: { `post=${payload.post_id} channel=${payload.channel_id}`, ); + if (params.authorizeButtonClick) { + try { + const authorization = await params.authorizeButtonClick({ + payload, + post: originalPost, + }); + if (!authorization.ok) { + res.statusCode = authorization.statusCode ?? 200; + res.setHeader("Content-Type", "application/json"); + res.end( + JSON.stringify( + authorization.response ?? { + ephemeral_text: "You are not allowed to use this action here.", + }, + ), + ); + return; + } + } catch (err) { + log?.(`mattermost interaction: authorization failed: ${String(err)}`); + res.statusCode = 500; + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify({ error: "Interaction authorization failed" })); + return; + } + } + if (params.handleInteraction) { try { const response = await params.handleInteraction({ diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index 16e3bd6434a..e56e4a9b9af 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -567,6 +567,45 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} trustedProxies: cfg.gateway?.trustedProxies, allowRealIpFallback: cfg.gateway?.allowRealIpFallback === true, handleInteraction: handleModelPickerInteraction, + authorizeButtonClick: async ({ payload, post }) => { + const channelInfo = await resolveChannelInfo(payload.channel_id); + const isDirect = channelInfo?.type?.trim().toUpperCase() === "D"; + const allowTextCommands = core.channel.commands.shouldHandleTextCommands({ + cfg, + surface: "mattermost", + }); + const decision = authorizeMattermostCommandInvocation({ + account, + cfg, + senderId: payload.user_id, + senderName: payload.user_name ?? "", + channelId: payload.channel_id, + channelInfo, + storeAllowFrom: isDirect + ? await readStoreAllowFromForDmPolicy({ + provider: "mattermost", + accountId: account.accountId, + dmPolicy: account.config.dmPolicy ?? "pairing", + readStore: pairing.readStoreForDmPolicy, + }) + : undefined, + allowTextCommands, + hasControlCommand: false, + }); + if (decision.ok) { + return { ok: true }; + } + return { + ok: false, + response: { + update: { + message: post.message ?? "", + props: post.props as Record | undefined, + }, + ephemeral_text: `OpenClaw ignored this action for ${decision.roomLabel}.`, + }, + }; + }, resolveSessionKey: async ({ channelId, userId, post }) => { const channelInfo = await resolveChannelInfo(channelId); const kind = mapMattermostChannelTypeToChatType(channelInfo?.type); diff --git a/extensions/nextcloud-talk/src/inbound.authz.test.ts b/extensions/nextcloud-talk/src/inbound.authz.test.ts index f19fa73e020..bde32abdb3c 100644 --- a/extensions/nextcloud-talk/src/inbound.authz.test.ts +++ b/extensions/nextcloud-talk/src/inbound.authz.test.ts @@ -81,4 +81,77 @@ describe("nextcloud-talk inbound authz", () => { }); expect(buildMentionRegexes).not.toHaveBeenCalled(); }); + + it("matches group rooms by token instead of colliding room names", async () => { + const readAllowFromStore = vi.fn(async () => []); + const buildMentionRegexes = vi.fn(() => [/@openclaw/i]); + + setNextcloudTalkRuntime({ + channel: { + pairing: { + readAllowFromStore, + }, + commands: { + shouldHandleTextCommands: () => false, + }, + text: { + hasControlCommand: () => false, + }, + mentions: { + buildMentionRegexes, + matchesMentionPatterns: () => false, + }, + }, + } as unknown as PluginRuntime); + + const message: NextcloudTalkInboundMessage = { + messageId: "m-2", + roomToken: "room-attacker", + roomName: "Room Trusted", + senderId: "trusted-user", + senderName: "Trusted User", + text: "hello", + mediaType: "text/plain", + timestamp: Date.now(), + isGroupChat: true, + }; + + const account: ResolvedNextcloudTalkAccount = { + accountId: "default", + enabled: true, + baseUrl: "", + secret: "", + secretSource: "none", + config: { + dmPolicy: "pairing", + allowFrom: [], + groupPolicy: "allowlist", + groupAllowFrom: ["trusted-user"], + rooms: { + "room-trusted": { + enabled: true, + }, + }, + }, + }; + + await handleNextcloudTalkInbound({ + message, + account, + config: { + channels: { + "nextcloud-talk": { + groupPolicy: "allowlist", + groupAllowFrom: ["trusted-user"], + }, + }, + }, + runtime: { + log: vi.fn(), + error: vi.fn(), + } as unknown as RuntimeEnv, + }); + + expect(buildMentionRegexes).not.toHaveBeenCalled(); + }); }); diff --git a/extensions/nextcloud-talk/src/inbound.ts b/extensions/nextcloud-talk/src/inbound.ts index 081029782f8..10ecd924fd7 100644 --- a/extensions/nextcloud-talk/src/inbound.ts +++ b/extensions/nextcloud-talk/src/inbound.ts @@ -114,7 +114,6 @@ export async function handleNextcloudTalkInbound(params: { const roomMatch = resolveNextcloudTalkRoomMatch({ rooms: account.config.rooms, roomToken, - roomName, }); const roomConfig = roomMatch.roomConfig; if (isGroup && !roomMatch.allowed) { diff --git a/extensions/nextcloud-talk/src/policy.ts b/extensions/nextcloud-talk/src/policy.ts index 1157384b578..15e19da84de 100644 --- a/extensions/nextcloud-talk/src/policy.ts +++ b/extensions/nextcloud-talk/src/policy.ts @@ -57,16 +57,10 @@ export type NextcloudTalkRoomMatch = { export function resolveNextcloudTalkRoomMatch(params: { rooms?: Record; roomToken: string; - roomName?: string | null; }): NextcloudTalkRoomMatch { const rooms = params.rooms ?? {}; const allowlistConfigured = Object.keys(rooms).length > 0; - const roomName = params.roomName?.trim() || undefined; - const roomCandidates = buildChannelKeyCandidates( - params.roomToken, - roomName, - roomName ? normalizeChannelSlug(roomName) : undefined, - ); + const roomCandidates = buildChannelKeyCandidates(params.roomToken); const match = resolveChannelEntryMatchWithFallback({ entries: rooms, keys: roomCandidates, @@ -101,11 +95,9 @@ export function resolveNextcloudTalkGroupToolPolicy( if (!roomToken) { return undefined; } - const roomName = params.groupChannel?.trim() || undefined; const match = resolveNextcloudTalkRoomMatch({ rooms: cfg.channels?.["nextcloud-talk"]?.rooms, roomToken, - roomName, }); return match.roomConfig?.tools ?? match.wildcardConfig?.tools; } diff --git a/extensions/twitch/src/access-control.test.ts b/extensions/twitch/src/access-control.test.ts index 3d522246700..597ef897f90 100644 --- a/extensions/twitch/src/access-control.test.ts +++ b/extensions/twitch/src/access-control.test.ts @@ -160,6 +160,13 @@ describe("checkTwitchAccessControl", () => { }); }); + it("blocks everyone when allowFrom is explicitly empty", () => { + expectAllowFromBlocked({ + allowFrom: [], + reason: "allowFrom", + }); + }); + it("blocks messages without userId", () => { expectAllowFromBlocked({ allowFrom: ["123456"], diff --git a/extensions/twitch/src/access-control.ts b/extensions/twitch/src/access-control.ts index 5555096d27d..1c4a043d42b 100644 --- a/extensions/twitch/src/access-control.ts +++ b/extensions/twitch/src/access-control.ts @@ -48,8 +48,14 @@ export function checkTwitchAccessControl(params: { } } - if (account.allowFrom && account.allowFrom.length > 0) { + if (account.allowFrom !== undefined) { const allowFrom = account.allowFrom; + if (allowFrom.length === 0) { + return { + allowed: false, + reason: "sender is not in allowFrom allowlist", + }; + } const senderId = message.userId; if (!senderId) { diff --git a/src/config/types.googlechat.ts b/src/config/types.googlechat.ts index fdfc23fd866..1951e51db10 100644 --- a/src/config/types.googlechat.ts +++ b/src/config/types.googlechat.ts @@ -75,6 +75,8 @@ export type GoogleChatAccountConfig = { audienceType?: "app-url" | "project-number"; /** Audience value (app URL or project number). */ audience?: string; + /** Exact add-on principal to accept when app-url delivery uses add-on tokens. */ + appPrincipal?: string; /** Google Chat webhook path (default: /googlechat). */ webhookPath?: string; /** Google Chat webhook URL (used to derive the path). */ diff --git a/src/config/zod-schema.providers-core.ts b/src/config/zod-schema.providers-core.ts index e6e4a3aacd2..5f7dd7b8e48 100644 --- a/src/config/zod-schema.providers-core.ts +++ b/src/config/zod-schema.providers-core.ts @@ -767,6 +767,7 @@ export const GoogleChatAccountSchema = z serviceAccountFile: z.string().optional(), audienceType: z.enum(["app-url", "project-number"]).optional(), audience: z.string().optional(), + appPrincipal: z.string().optional(), webhookPath: z.string().optional(), webhookUrl: z.string().optional(), botUser: z.string().optional(), From 229426a257e49694a59fa4e3895861d02a4d767f Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 15 Mar 2026 09:28:44 -0700 Subject: [PATCH 16/16] ACP: require admin scope for mutating internal actions (#46789) * ACP: require admin scope for mutating internal actions * ACP: cover operator admin mutating actions * ACP: gate internal status behind admin scope --- src/auto-reply/reply/commands-acp.test.ts | 77 +++++++++++++++++++++++ src/auto-reply/reply/commands-acp.ts | 27 ++++++++ 2 files changed, 104 insertions(+) diff --git a/src/auto-reply/reply/commands-acp.test.ts b/src/auto-reply/reply/commands-acp.test.ts index 937d282c18e..e41fbd80ec2 100644 --- a/src/auto-reply/reply/commands-acp.test.ts +++ b/src/auto-reply/reply/commands-acp.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { AcpRuntimeError } from "../../acp/runtime/errors.js"; import type { OpenClawConfig } from "../../config/config.js"; import type { SessionBindingRecord } from "../../infra/outbound/session-binding-service.js"; +import { INTERNAL_MESSAGE_CHANNEL } from "../../utils/message-channel.js"; const hoisted = vi.hoisted(() => { const callGatewayMock = vi.fn(); @@ -374,6 +375,24 @@ async function runFeishuDmAcpCommand(commandBody: string, cfg: OpenClawConfig = return handleAcpCommand(createFeishuDmParams(commandBody, cfg), true); } +async function runInternalAcpCommand(params: { + commandBody: string; + scopes: string[]; + cfg?: OpenClawConfig; +}) { + const commandParams = buildCommandTestParams(params.commandBody, params.cfg ?? baseCfg, { + Provider: INTERNAL_MESSAGE_CHANNEL, + Surface: INTERNAL_MESSAGE_CHANNEL, + OriginatingChannel: INTERNAL_MESSAGE_CHANNEL, + OriginatingTo: "webchat:conversation-1", + GatewayClientScopes: params.scopes, + }); + commandParams.command.channel = INTERNAL_MESSAGE_CHANNEL; + commandParams.command.senderId = "user-1"; + commandParams.command.senderIsOwner = true; + return handleAcpCommand(commandParams, true); +} + describe("/acp command", () => { beforeEach(() => { acpManagerTesting.resetAcpSessionManagerForTests(); @@ -824,6 +843,64 @@ describe("/acp command", () => { expect(result?.reply?.text).toContain("Updated ACP runtime mode"); }); + it("blocks mutating /acp actions for internal operator.write clients", async () => { + const result = await runInternalAcpCommand({ + commandBody: "/acp set-mode plan", + scopes: ["operator.write"], + }); + + expect(result?.shouldContinue).toBe(false); + expect(result?.reply?.text).toContain("requires operator.admin"); + }); + + it("blocks /acp status for internal operator.write clients", async () => { + const result = await runInternalAcpCommand({ + commandBody: "/acp status", + scopes: ["operator.write"], + }); + + expect(result?.shouldContinue).toBe(false); + expect(result?.reply?.text).toContain("requires operator.admin"); + }); + + it("keeps read-only /acp actions available to internal operator.write clients", async () => { + hoisted.listAcpSessionEntriesMock.mockResolvedValue([ + createAcpSessionEntry({ + identity: { + state: "resolved", + source: "status", + acpxSessionId: "runtime-1", + agentSessionId: "session-1", + lastUpdatedAt: Date.now(), + }, + }), + ]); + + const result = await runInternalAcpCommand({ + commandBody: "/acp sessions", + scopes: ["operator.write"], + }); + + expect(result?.shouldContinue).toBe(false); + expect(result?.reply?.text).toContain("ACP sessions"); + }); + + it("allows mutating /acp actions for internal operator.admin clients", async () => { + mockBoundThreadSession(); + + const result = await runInternalAcpCommand({ + commandBody: "/acp set-mode plan", + scopes: ["operator.admin"], + }); + + expect(hoisted.setModeMock).toHaveBeenCalledWith( + expect.objectContaining({ + mode: "plan", + }), + ); + expect(result?.reply?.text).toContain("Updated ACP runtime mode"); + }); + it("updates ACP config options and keeps cwd local when using /acp set", async () => { mockBoundThreadSession(); diff --git a/src/auto-reply/reply/commands-acp.ts b/src/auto-reply/reply/commands-acp.ts index 2eef395c9a2..e23faf74d10 100644 --- a/src/auto-reply/reply/commands-acp.ts +++ b/src/auto-reply/reply/commands-acp.ts @@ -1,4 +1,5 @@ import { logVerbose } from "../../globals.js"; +import { requireGatewayClientScopeForInternalChannel } from "./command-gates.js"; import { handleAcpDoctorAction, handleAcpInstallAction, @@ -56,6 +57,21 @@ const ACP_ACTION_HANDLERS: Record, AcpActionHandler> sessions: async (params, tokens) => handleAcpSessionsAction(params, tokens), }; +const ACP_MUTATING_ACTIONS = new Set([ + "spawn", + "cancel", + "steer", + "close", + "status", + "set-mode", + "set", + "cwd", + "permissions", + "timeout", + "model", + "reset-options", +]); + export const handleAcpCommand: CommandHandler = async (params, allowTextCommands) => { if (!allowTextCommands) { return null; @@ -78,6 +94,17 @@ export const handleAcpCommand: CommandHandler = async (params, allowTextCommands return stopWithText(resolveAcpHelpText()); } + if (ACP_MUTATING_ACTIONS.has(action)) { + const scopeBlock = requireGatewayClientScopeForInternalChannel(params, { + label: "/acp", + allowedScopes: ["operator.admin"], + missingText: "This /acp action requires operator.admin on the internal channel.", + }); + if (scopeBlock) { + return scopeBlock; + } + } + const handler = ACP_ACTION_HANDLERS[action]; return handler ? await handler(params, tokens) : stopWithText(resolveAcpHelpText()); };