diff --git a/CHANGELOG.md b/CHANGELOG.md index 95dd2800b..0cc8df3e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Docs: https://docs.openclaw.ai - Channels/Group policy: fail closed when `groupPolicy: "allowlist"` is set without explicit `groups`, honor account-level `groupPolicy` overrides, and enforce `groupPolicy: "disabled"` as a hard group block. (#22215) Thanks @etereo. - Sandbox/Media: map container workspace paths (`/workspace/...` and `file:///workspace/...`) back to the host sandbox root for outbound media validation, preventing false deny errors for sandbox-generated local media. (#23083) Thanks @echo931. - Sandbox/Docker: apply custom bind mounts after workspace mounts and prioritize bind-source resolution on overlapping paths, so explicit workspace binds are no longer ignored. (#22669) Thanks @tasaankaeris. +- Exec approvals/Forwarding: restore Discord text forwarding when component approvals are not configured, and carry request snapshots through resolve events so resolved notices still forward after cache misses/restarts. (#22988) Thanks @bubmiller. - Config/Memory: allow `"mistral"` in `agents.defaults.memorySearch.provider` and `agents.defaults.memorySearch.fallback` schema validation. (#14934) Thanks @ThomsenDrake. - Security/Feishu: enforce ID-only allowlist matching for DM/group sender authorization, normalize Feishu ID prefixes during checks, and ignore mutable display names so display-name collisions cannot satisfy allowlist entries. This ships in the next npm release. Thanks @jiseoung for reporting. - Feishu/Commands: in group chats, command authorization now falls back to top-level `channels.feishu.allowFrom` when per-group `allowFrom` is not set, so `/command` no longer gets blocked by an unintended empty allowlist. (#23756) diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 798102d20..18fb34591 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -186,6 +186,7 @@ export function createExecApprovalHandlers( respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "invalid decision")); return; } + const snapshot = manager.getSnapshot(p.id); const resolvedBy = client?.connect?.client?.displayName ?? client?.connect?.client?.id; const ok = manager.resolve(p.id, decision, resolvedBy ?? null); if (!ok) { @@ -194,11 +195,17 @@ export function createExecApprovalHandlers( } context.broadcast( "exec.approval.resolved", - { id: p.id, decision, resolvedBy, ts: Date.now() }, + { id: p.id, decision, resolvedBy, ts: Date.now(), request: snapshot?.request }, { dropIfSlow: true }, ); void opts?.forwarder - ?.handleResolved({ id: p.id, decision, resolvedBy, ts: Date.now() }) + ?.handleResolved({ + id: p.id, + decision, + resolvedBy, + ts: Date.now(), + request: snapshot?.request, + }) .catch((err) => { context.logGateway?.error?.(`exec approvals: forward resolve failed: ${String(err)}`); }); diff --git a/src/infra/exec-approval-forwarder.test.ts b/src/infra/exec-approval-forwarder.test.ts index bc1eba50a..31648250b 100644 --- a/src/infra/exec-approval-forwarder.test.ts +++ b/src/infra/exec-approval-forwarder.test.ts @@ -113,7 +113,7 @@ describe("exec approval forwarder", () => { expect(getFirstDeliveryText(deliver)).toContain("Command:\n```\necho `uname`\necho done\n```"); }); - it("skips discord forwarding targets", async () => { + it("forwards to discord when discord exec approvals handler is disabled", async () => { vi.useFakeTimers(); const cfg = { approvals: { exec: { enabled: true, mode: "session" } }, @@ -126,9 +126,61 @@ describe("exec approval forwarder", () => { await forwarder.handleRequested(baseRequest); + expect(deliver).toHaveBeenCalledTimes(1); + }); + + it("skips discord forwarding when discord exec approvals handler is enabled", async () => { + vi.useFakeTimers(); + const cfg = { + channels: { + discord: { + execApprovals: { + enabled: true, + approvers: ["123"], + }, + }, + }, + approvals: { exec: { enabled: true, mode: "session" } }, + } as OpenClawConfig; + + const { deliver, forwarder } = createForwarder({ + cfg, + resolveSessionTarget: () => ({ channel: "discord", to: "channel:123" }), + }); + + await forwarder.handleRequested(baseRequest); + expect(deliver).not.toHaveBeenCalled(); }); + it("can forward resolved notices without pending cache when request payload is present", async () => { + vi.useFakeTimers(); + const cfg = { + approvals: { + exec: { + enabled: true, + mode: "targets", + targets: [{ channel: "telegram", to: "123" }], + }, + }, + } as OpenClawConfig; + const { deliver, forwarder } = createForwarder({ cfg }); + + await forwarder.handleResolved({ + id: "req-missing", + decision: "allow-once", + resolvedBy: "telegram:123", + ts: 2000, + request: { + command: "echo ok", + agentId: "main", + sessionKey: "agent:main:main", + }, + }); + + expect(deliver).toHaveBeenCalledTimes(1); + }); + it("uses a longer fence when command already contains triple backticks", async () => { vi.useFakeTimers(); const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG }); diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index b703f5959..ce06adda3 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -6,7 +6,7 @@ import type { ExecApprovalForwardTarget, } from "../config/types.approvals.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; -import { parseAgentSessionKey } from "../routing/session-key.js"; +import { normalizeAccountId, parseAgentSessionKey } from "../routing/session-key.js"; import { isDeliverableMessageChannel, normalizeMessageChannel } from "../utils/message-channel.js"; import type { ExecApprovalDecision, @@ -98,10 +98,49 @@ function buildTargetKey(target: ExecApprovalForwardTarget): string { return [channel, target.to, accountId, threadId].join(":"); } -// Discord has component-based exec approvals; skip the text fallback there. -function shouldSkipDiscordForwarding(target: ExecApprovalForwardTarget): boolean { +function resolveChannelAccountConfig( + accounts: Record | undefined, + accountId?: string, +): T | undefined { + if (!accounts || !accountId?.trim()) { + return undefined; + } + const normalized = normalizeAccountId(accountId); + const direct = accounts[normalized]; + if (direct) { + return direct; + } + const fallbackKey = Object.keys(accounts).find( + (key) => key.toLowerCase() === normalized.toLowerCase(), + ); + return fallbackKey ? accounts[fallbackKey] : undefined; +} + +// Discord has component-based exec approvals; skip text fallback only when the +// Discord-specific handler is enabled for the same target account. +function shouldSkipDiscordForwarding( + target: ExecApprovalForwardTarget, + cfg: OpenClawConfig, +): boolean { const channel = normalizeMessageChannel(target.channel) ?? target.channel; - return channel === "discord"; + if (channel !== "discord") { + return false; + } + const discord = cfg.channels?.discord as + | { + execApprovals?: { enabled?: boolean; approvers?: Array }; + accounts?: Record< + string, + { execApprovals?: { enabled?: boolean; approvers?: Array } } + >; + } + | undefined; + if (!discord) { + return false; + } + const account = resolveChannelAccountConfig(discord.accounts, target.accountId); + const execApprovals = account?.execApprovals ?? discord.execApprovals; + return Boolean(execApprovals?.enabled && (execApprovals.approvers?.length ?? 0) > 0); } function formatApprovalCommand(command: string): { inline: boolean; text: string } { @@ -228,6 +267,48 @@ async function deliverToTargets(params: { await Promise.allSettled(deliveries); } +function resolveForwardTargets(params: { + cfg: OpenClawConfig; + config?: ExecApprovalForwardingConfig; + request: ExecApprovalRequest; + resolveSessionTarget: (params: { + cfg: OpenClawConfig; + request: ExecApprovalRequest; + }) => ExecApprovalForwardTarget | null; +}): ForwardTarget[] { + const mode = normalizeMode(params.config?.mode); + const targets: ForwardTarget[] = []; + const seen = new Set(); + + if (mode === "session" || mode === "both") { + const sessionTarget = params.resolveSessionTarget({ + cfg: params.cfg, + request: params.request, + }); + if (sessionTarget) { + const key = buildTargetKey(sessionTarget); + if (!seen.has(key)) { + seen.add(key); + targets.push({ ...sessionTarget, source: "session" }); + } + } + } + + if (mode === "targets" || mode === "both") { + const explicitTargets = params.config?.targets ?? []; + for (const target of explicitTargets) { + const key = buildTargetKey(target); + if (seen.has(key)) { + continue; + } + seen.add(key); + targets.push({ ...target, source: "target" }); + } + } + + return targets; +} + export function createExecApprovalForwarder( deps: ExecApprovalForwarderDeps = {}, ): ExecApprovalForwarder { @@ -243,35 +324,12 @@ export function createExecApprovalForwarder( if (!shouldForward({ config, request })) { return; } - - const mode = normalizeMode(config?.mode); - const targets: ForwardTarget[] = []; - const seen = new Set(); - - if (mode === "session" || mode === "both") { - const sessionTarget = resolveSessionTarget({ cfg, request }); - if (sessionTarget) { - const key = buildTargetKey(sessionTarget); - if (!seen.has(key)) { - seen.add(key); - targets.push({ ...sessionTarget, source: "session" }); - } - } - } - - if (mode === "targets" || mode === "both") { - const explicitTargets = config?.targets ?? []; - for (const target of explicitTargets) { - const key = buildTargetKey(target); - if (seen.has(key)) { - continue; - } - seen.add(key); - targets.push({ ...target, source: "target" }); - } - } - - const filteredTargets = targets.filter((target) => !shouldSkipDiscordForwarding(target)); + const filteredTargets = resolveForwardTargets({ + cfg, + config, + request, + resolveSessionTarget, + }).filter((target) => !shouldSkipDiscordForwarding(target, cfg)); if (filteredTargets.length === 0) { return; @@ -310,17 +368,37 @@ export function createExecApprovalForwarder( const handleResolved = async (resolved: ExecApprovalResolved) => { const entry = pending.get(resolved.id); - if (!entry) { + if (entry) { + if (entry.timeoutId) { + clearTimeout(entry.timeoutId); + } + pending.delete(resolved.id); + } + const cfg = getConfig(); + let targets = entry?.targets; + + if (!targets && resolved.request) { + const request: ExecApprovalRequest = { + id: resolved.id, + request: resolved.request, + createdAtMs: resolved.ts, + expiresAtMs: resolved.ts, + }; + const config = cfg.approvals?.exec; + if (shouldForward({ config, request })) { + targets = resolveForwardTargets({ + cfg, + config, + request, + resolveSessionTarget, + }).filter((target) => !shouldSkipDiscordForwarding(target, cfg)); + } + } + if (!targets || targets.length === 0) { return; } - if (entry.timeoutId) { - clearTimeout(entry.timeoutId); - } - pending.delete(resolved.id); - - const cfg = getConfig(); const text = buildResolvedMessage(resolved); - await deliverToTargets({ cfg, targets: entry.targets, text, deliver }); + await deliverToTargets({ cfg, targets, text, deliver }); }; const stop = () => { diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index b2d425e43..4fd3f6347 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -32,6 +32,7 @@ export type ExecApprovalResolved = { decision: ExecApprovalDecision; resolvedBy?: string | null; ts: number; + request?: ExecApprovalRequest["request"]; }; export type ExecApprovalsDefaults = {