From 392bbddf29c7809dec3ebce5d254047cf73ccfa3 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Wed, 4 Feb 2026 19:49:36 -0500 Subject: [PATCH] Security: owner-only tools + command auth hardening (#9202) * Security: gate whatsapp_login by sender auth * Security: treat undefined senderAuthorized as unauthorized (opt-in) * fix: gate whatsapp_login to owner senders (#8768) (thanks @victormier) * fix: add explicit owner allowlist for tools (#8768) (thanks @victormier) * fix: normalize escaped newlines in send actions (#8768) (thanks @victormier) --------- Co-authored-by: Victor Mier --- CHANGELOG.md | 2 + src/agents/pi-embedded-runner/compact.ts | 3 + src/agents/pi-embedded-runner/run.ts | 2 + src/agents/pi-embedded-runner/run/attempt.ts | 1 + src/agents/pi-embedded-runner/run/params.ts | 2 + src/agents/pi-embedded-runner/run/types.ts | 2 + src/agents/pi-tools.ts | 14 ++-- .../pi-tools.whatsapp-login-gating.test.ts | 35 +++++++++ src/agents/tool-policy.ts | 29 ++++++++ src/auto-reply/command-auth.ts | 72 +++++++++++++++++-- src/auto-reply/command-control.test.ts | 35 +++++++++ src/auto-reply/reply/commands-compact.ts | 1 + .../reply/commands-context-report.ts | 1 + src/auto-reply/reply/commands-context.ts | 1 + src/auto-reply/reply/commands-types.ts | 1 + src/auto-reply/reply/get-reply-run.ts | 1 + src/commands/agent.ts | 1 + src/config/schema.ts | 3 + src/config/types.messages.ts | 2 + src/config/zod-schema.session.ts | 1 + src/infra/outbound/message-action-runner.ts | 3 + 21 files changed, 202 insertions(+), 10 deletions(-) create mode 100644 src/agents/pi-tools.whatsapp-login-gating.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fdaf0db00..9cceb525f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ Docs: https://docs.openclaw.ai - Web UI: apply button styling to the new-messages indicator. - Security: keep untrusted channel metadata out of system prompts (Slack/Discord). Thanks @KonstantinMirin. - Security: enforce sandboxed media paths for message tool attachments. (#9182) Thanks @victormier. +- Security: require explicit credentials for gateway URL overrides to prevent credential leakage. (#8113) Thanks @victormier. +- Security: gate `whatsapp_login` tool to owner senders and default-deny non-owner contexts. (#8768) Thanks @victormier. - Voice call: harden webhook verification with host allowlists/proxy trust and keep ngrok loopback bypass. - Voice call: add regression coverage for anonymous inbound caller IDs with allowlist policy. (#8104) Thanks @victormier. - Cron: accept epoch timestamps and 0ms durations in CLI `--at` parsing. diff --git a/src/agents/pi-embedded-runner/compact.ts b/src/agents/pi-embedded-runner/compact.ts index 10aa9f89d3..8c5a63d32d 100644 --- a/src/agents/pi-embedded-runner/compact.ts +++ b/src/agents/pi-embedded-runner/compact.ts @@ -88,6 +88,8 @@ export type CompactEmbeddedPiSessionParams = { groupSpace?: string | null; /** Parent session key for subagent policy inheritance. */ spawnedBy?: string | null; + /** Whether the sender is an owner (required for owner-only tools). */ + senderIsOwner?: boolean; sessionFile: string; workspaceDir: string; agentDir?: string; @@ -227,6 +229,7 @@ export async function compactEmbeddedPiSessionDirect( groupChannel: params.groupChannel, groupSpace: params.groupSpace, spawnedBy: params.spawnedBy, + senderIsOwner: params.senderIsOwner, agentDir, workspaceDir: effectiveWorkspace, config: params.config, diff --git a/src/agents/pi-embedded-runner/run.ts b/src/agents/pi-embedded-runner/run.ts index e7c9ad44a6..4356a8d98a 100644 --- a/src/agents/pi-embedded-runner/run.ts +++ b/src/agents/pi-embedded-runner/run.ts @@ -324,6 +324,7 @@ export async function runEmbeddedPiAgent( groupChannel: params.groupChannel, groupSpace: params.groupSpace, spawnedBy: params.spawnedBy, + senderIsOwner: params.senderIsOwner, currentChannelId: params.currentChannelId, currentThreadTs: params.currentThreadTs, replyToMode: params.replyToMode, @@ -391,6 +392,7 @@ export async function runEmbeddedPiAgent( agentDir, config: params.config, skillsSnapshot: params.skillsSnapshot, + senderIsOwner: params.senderIsOwner, provider, model: modelId, thinkLevel, diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index abb624fbee..83fe17cfb1 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -225,6 +225,7 @@ export async function runEmbeddedAttempt( senderName: params.senderName, senderUsername: params.senderUsername, senderE164: params.senderE164, + senderIsOwner: params.senderIsOwner, sessionKey: params.sessionKey ?? params.sessionId, agentDir, workspaceDir: effectiveWorkspace, diff --git a/src/agents/pi-embedded-runner/run/params.ts b/src/agents/pi-embedded-runner/run/params.ts index e6927a28ff..93f5c5c927 100644 --- a/src/agents/pi-embedded-runner/run/params.ts +++ b/src/agents/pi-embedded-runner/run/params.ts @@ -39,6 +39,8 @@ export type RunEmbeddedPiAgentParams = { senderName?: string | null; senderUsername?: string | null; senderE164?: string | null; + /** Whether the sender is an owner (required for owner-only tools). */ + senderIsOwner?: boolean; /** Current channel ID for auto-threading (Slack). */ currentChannelId?: string; /** Current thread timestamp for auto-threading (Slack). */ diff --git a/src/agents/pi-embedded-runner/run/types.ts b/src/agents/pi-embedded-runner/run/types.ts index 8d8542b8c0..931afcd24c 100644 --- a/src/agents/pi-embedded-runner/run/types.ts +++ b/src/agents/pi-embedded-runner/run/types.ts @@ -31,6 +31,8 @@ export type EmbeddedRunAttemptParams = { senderName?: string | null; senderUsername?: string | null; senderE164?: string | null; + /** Whether the sender is an owner (required for owner-only tools). */ + senderIsOwner?: boolean; currentChannelId?: string; currentThreadTs?: string; replyToMode?: "off" | "first" | "all"; diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index cec1c8cbd6..0508cda22e 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -44,6 +44,7 @@ import { } from "./pi-tools.read.js"; import { cleanToolSchemaForGemini, normalizeToolParameters } from "./pi-tools.schema.js"; import { + applyOwnerOnlyToolPolicy, buildPluginToolGroups, collectExplicitAllowlist, expandPolicyWithPluginGroups, @@ -161,6 +162,8 @@ export function createOpenClawCodingTools(options?: { requireExplicitMessageTarget?: boolean; /** If true, omit the message tool from the tool list. */ disableMessageTool?: boolean; + /** Whether the sender is an owner (required for owner-only tools). */ + senderIsOwner?: boolean; }): AnyAgentTool[] { const execToolName = "exec"; const sandbox = options?.sandbox?.enabled ? options.sandbox : undefined; @@ -357,14 +360,17 @@ export function createOpenClawCodingTools(options?: { requesterAgentIdOverride: agentId, }), ]; + // Security: treat unknown/undefined as unauthorized (opt-in, not opt-out) + const senderIsOwner = options?.senderIsOwner === true; + const toolsByAuthorization = applyOwnerOnlyToolPolicy(tools, senderIsOwner); const coreToolNames = new Set( - tools + toolsByAuthorization .filter((tool) => !getPluginToolMeta(tool)) .map((tool) => normalizeToolName(tool.name)) .filter(Boolean), ); const pluginGroups = buildPluginToolGroups({ - tools, + tools: toolsByAuthorization, toolMeta: (tool) => getPluginToolMeta(tool), }); const resolvePolicy = (policy: typeof profilePolicy, label: string) => { @@ -401,8 +407,8 @@ export function createOpenClawCodingTools(options?: { const subagentPolicyExpanded = expandPolicyWithPluginGroups(subagentPolicy, pluginGroups); const toolsFiltered = profilePolicyExpanded - ? filterToolsByPolicy(tools, profilePolicyExpanded) - : tools; + ? filterToolsByPolicy(toolsByAuthorization, profilePolicyExpanded) + : toolsByAuthorization; const providerProfileFiltered = providerProfileExpanded ? filterToolsByPolicy(toolsFiltered, providerProfileExpanded) : toolsFiltered; diff --git a/src/agents/pi-tools.whatsapp-login-gating.test.ts b/src/agents/pi-tools.whatsapp-login-gating.test.ts new file mode 100644 index 0000000000..a0b6aa8a6e --- /dev/null +++ b/src/agents/pi-tools.whatsapp-login-gating.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it, vi } from "vitest"; +import "./test-helpers/fast-coding-tools.js"; +import { createOpenClawCodingTools } from "./pi-tools.js"; + +vi.mock("./channel-tools.js", () => { + const stubTool = (name: string) => ({ + name, + description: `${name} stub`, + parameters: { type: "object", properties: {} }, + execute: vi.fn(), + }); + return { + listChannelAgentTools: () => [stubTool("whatsapp_login")], + }; +}); + +describe("whatsapp_login tool gating", () => { + it("removes whatsapp_login for unauthorized senders", () => { + const tools = createOpenClawCodingTools({ senderIsOwner: false }); + const toolNames = tools.map((tool) => tool.name); + expect(toolNames).not.toContain("whatsapp_login"); + }); + + it("keeps whatsapp_login for authorized senders", () => { + const tools = createOpenClawCodingTools({ senderIsOwner: true }); + const toolNames = tools.map((tool) => tool.name); + expect(toolNames).toContain("whatsapp_login"); + }); + + it("defaults to removing whatsapp_login when owner status is unknown", () => { + const tools = createOpenClawCodingTools(); + const toolNames = tools.map((tool) => tool.name); + expect(toolNames).not.toContain("whatsapp_login"); + }); +}); diff --git a/src/agents/tool-policy.ts b/src/agents/tool-policy.ts index a14cb73ac0..e318f9ee19 100644 --- a/src/agents/tool-policy.ts +++ b/src/agents/tool-policy.ts @@ -1,3 +1,5 @@ +import type { AnyAgentTool } from "./tools/common.js"; + export type ToolProfileId = "minimal" | "coding" | "messaging" | "full"; type ToolProfilePolicy = { @@ -56,6 +58,8 @@ export const TOOL_GROUPS: Record = { ], }; +const OWNER_ONLY_TOOL_NAMES = new Set(["whatsapp_login"]); + const TOOL_PROFILES: Record = { minimal: { allow: ["session_status"], @@ -80,6 +84,31 @@ export function normalizeToolName(name: string) { return TOOL_NAME_ALIASES[normalized] ?? normalized; } +export function isOwnerOnlyToolName(name: string) { + return OWNER_ONLY_TOOL_NAMES.has(normalizeToolName(name)); +} + +export function applyOwnerOnlyToolPolicy(tools: AnyAgentTool[], senderIsOwner: boolean) { + const withGuard = tools.map((tool) => { + if (!isOwnerOnlyToolName(tool.name)) { + return tool; + } + if (senderIsOwner || !tool.execute) { + return tool; + } + return { + ...tool, + execute: async () => { + throw new Error("Tool restricted to owner senders."); + }, + }; + }); + if (senderIsOwner) { + return withGuard; + } + return withGuard.filter((tool) => !isOwnerOnlyToolName(tool.name)); +} + export function normalizeToolList(list?: string[]) { if (!list) { return []; diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts index f08c9a6be6..187fa6261c 100644 --- a/src/auto-reply/command-auth.ts +++ b/src/auto-reply/command-auth.ts @@ -9,6 +9,7 @@ export type CommandAuthorization = { providerId?: ChannelId; ownerList: string[]; senderId?: string; + senderIsOwner: boolean; isAuthorizedSender: boolean; from?: string; to?: string; @@ -83,6 +84,47 @@ function normalizeAllowFromEntry(params: { return normalized.filter((entry) => entry.trim().length > 0); } +function resolveOwnerAllowFromList(params: { + dock?: ChannelDock; + cfg: OpenClawConfig; + accountId?: string | null; + providerId?: ChannelId; +}): string[] { + const raw = params.cfg.commands?.ownerAllowFrom; + if (!Array.isArray(raw) || raw.length === 0) { + return []; + } + const filtered: string[] = []; + for (const entry of raw) { + const trimmed = String(entry ?? "").trim(); + if (!trimmed) { + continue; + } + const separatorIndex = trimmed.indexOf(":"); + if (separatorIndex > 0) { + const prefix = trimmed.slice(0, separatorIndex); + const channel = normalizeAnyChannelId(prefix); + if (channel) { + if (params.providerId && channel !== params.providerId) { + continue; + } + const remainder = trimmed.slice(separatorIndex + 1).trim(); + if (remainder) { + filtered.push(remainder); + } + continue; + } + } + filtered.push(trimmed); + } + return formatAllowFromList({ + dock: params.dock, + cfg: params.cfg, + accountId: params.accountId, + allowFrom: filtered, + }); +} + function resolveSenderCandidates(params: { dock?: ChannelDock; providerId?: ChannelId; @@ -141,11 +183,17 @@ export function resolveCommandAuthorization(params: { accountId: ctx.AccountId, allowFrom: Array.isArray(allowFromRaw) ? allowFromRaw : [], }); + const ownerAllowFromList = resolveOwnerAllowFromList({ + dock, + cfg, + accountId: ctx.AccountId, + providerId, + }); const allowAll = allowFromList.length === 0 || allowFromList.some((entry) => entry.trim() === "*"); - const ownerCandidates = allowAll ? [] : allowFromList.filter((entry) => entry !== "*"); - if (!allowAll && ownerCandidates.length === 0 && to) { + const ownerCandidatesForCommands = allowAll ? [] : allowFromList.filter((entry) => entry !== "*"); + if (!allowAll && ownerCandidatesForCommands.length === 0 && to) { const normalizedTo = normalizeAllowFromEntry({ dock, cfg, @@ -153,10 +201,13 @@ export function resolveCommandAuthorization(params: { value: to, }); if (normalizedTo.length > 0) { - ownerCandidates.push(...normalizedTo); + ownerCandidatesForCommands.push(...normalizedTo); } } - const ownerList = Array.from(new Set(ownerCandidates)); + const explicitOwners = ownerAllowFromList.filter((entry) => entry !== "*"); + const ownerList = Array.from( + new Set(explicitOwners.length > 0 ? explicitOwners : ownerCandidatesForCommands), + ); const senderCandidates = resolveSenderCandidates({ dock, @@ -170,16 +221,25 @@ export function resolveCommandAuthorization(params: { const matchedSender = ownerList.length ? senderCandidates.find((candidate) => ownerList.includes(candidate)) : undefined; + const matchedCommandOwner = ownerCandidatesForCommands.length + ? senderCandidates.find((candidate) => ownerCandidatesForCommands.includes(candidate)) + : undefined; const senderId = matchedSender ?? senderCandidates[0]; const enforceOwner = Boolean(dock?.commands?.enforceOwnerForCommands); - const isOwner = !enforceOwner || allowAll || ownerList.length === 0 || Boolean(matchedSender); - const isAuthorizedSender = commandAuthorized && isOwner; + const senderIsOwner = Boolean(matchedSender); + const isOwnerForCommands = + !enforceOwner || + allowAll || + ownerCandidatesForCommands.length === 0 || + Boolean(matchedCommandOwner); + const isAuthorizedSender = commandAuthorized && isOwnerForCommands; return { providerId, ownerList, senderId: senderId || undefined, + senderIsOwner, isAuthorizedSender, from: from || undefined, to: to || undefined, diff --git a/src/auto-reply/command-control.test.ts b/src/auto-reply/command-control.test.ts index c06d81aa73..860e7550bf 100644 --- a/src/auto-reply/command-control.test.ts +++ b/src/auto-reply/command-control.test.ts @@ -132,6 +132,41 @@ describe("resolveCommandAuthorization", () => { expect(auth.senderId).toBe("+41796666864"); expect(auth.isAuthorizedSender).toBe(true); }); + + it("uses explicit owner allowlist when allowFrom is wildcard", () => { + const cfg = { + commands: { ownerAllowFrom: ["whatsapp:+15551234567"] }, + channels: { whatsapp: { allowFrom: ["*"] } }, + } as OpenClawConfig; + + const ownerCtx = { + Provider: "whatsapp", + Surface: "whatsapp", + From: "whatsapp:+15551234567", + SenderE164: "+15551234567", + } as MsgContext; + const ownerAuth = resolveCommandAuthorization({ + ctx: ownerCtx, + cfg, + commandAuthorized: true, + }); + expect(ownerAuth.senderIsOwner).toBe(true); + expect(ownerAuth.isAuthorizedSender).toBe(true); + + const otherCtx = { + Provider: "whatsapp", + Surface: "whatsapp", + From: "whatsapp:+19995551234", + SenderE164: "+19995551234", + } as MsgContext; + const otherAuth = resolveCommandAuthorization({ + ctx: otherCtx, + cfg, + commandAuthorized: true, + }); + expect(otherAuth.senderIsOwner).toBe(false); + expect(otherAuth.isAuthorizedSender).toBe(true); + }); }); describe("control command parsing", () => { diff --git a/src/auto-reply/reply/commands-compact.ts b/src/auto-reply/reply/commands-compact.ts index 59dcbcd8b6..3df9a9bf01 100644 --- a/src/auto-reply/reply/commands-compact.ts +++ b/src/auto-reply/reply/commands-compact.ts @@ -92,6 +92,7 @@ export const handleCompactCommand: CommandHandler = async (params) => { defaultLevel: "off", }, customInstructions, + senderIsOwner: params.command.senderIsOwner, ownerNumbers: params.command.ownerList.length > 0 ? params.command.ownerList : undefined, }); diff --git a/src/auto-reply/reply/commands-context-report.ts b/src/auto-reply/reply/commands-context-report.ts index eb6d36d217..833964523d 100644 --- a/src/auto-reply/reply/commands-context-report.ts +++ b/src/auto-reply/reply/commands-context-report.ts @@ -92,6 +92,7 @@ async function resolveContextReport( groupChannel: params.sessionEntry?.groupChannel ?? undefined, groupSpace: params.sessionEntry?.space ?? undefined, spawnedBy: params.sessionEntry?.spawnedBy ?? undefined, + senderIsOwner: params.command.senderIsOwner, modelProvider: params.provider, modelId: params.model, }); diff --git a/src/auto-reply/reply/commands-context.ts b/src/auto-reply/reply/commands-context.ts index bacff2edd7..af78c2b38f 100644 --- a/src/auto-reply/reply/commands-context.ts +++ b/src/auto-reply/reply/commands-context.ts @@ -33,6 +33,7 @@ export function buildCommandContext(params: { channel, channelId: auth.providerId, ownerList: auth.ownerList, + senderIsOwner: auth.senderIsOwner, isAuthorizedSender: auth.isAuthorizedSender, senderId: auth.senderId, abortKey, diff --git a/src/auto-reply/reply/commands-types.ts b/src/auto-reply/reply/commands-types.ts index 69689f6e98..96b615b21f 100644 --- a/src/auto-reply/reply/commands-types.ts +++ b/src/auto-reply/reply/commands-types.ts @@ -12,6 +12,7 @@ export type CommandContext = { channel: string; channelId?: ChannelId; ownerList: string[]; + senderIsOwner: boolean; isAuthorizedSender: boolean; senderId?: string; abortKey?: string; diff --git a/src/auto-reply/reply/get-reply-run.ts b/src/auto-reply/reply/get-reply-run.ts index aae01c1268..7531622adb 100644 --- a/src/auto-reply/reply/get-reply-run.ts +++ b/src/auto-reply/reply/get-reply-run.ts @@ -378,6 +378,7 @@ export async function runPreparedReply( senderName: sessionCtx.SenderName?.trim() || undefined, senderUsername: sessionCtx.SenderUsername?.trim() || undefined, senderE164: sessionCtx.SenderE164?.trim() || undefined, + senderIsOwner: command.senderIsOwner, sessionFile, workspaceDir, config: cfg, diff --git a/src/commands/agent.ts b/src/commands/agent.ts index 20d3b6c89c..a426775513 100644 --- a/src/commands/agent.ts +++ b/src/commands/agent.ts @@ -430,6 +430,7 @@ export async function agentCommand( currentThreadTs: runContext.currentThreadTs, replyToMode: runContext.replyToMode, hasRepliedRef: runContext.hasRepliedRef, + senderIsOwner: true, sessionFile, workspaceDir, config: cfg, diff --git a/src/config/schema.ts b/src/config/schema.ts index c918d38f0b..175265ac16 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -301,6 +301,7 @@ const FIELD_LABELS: Record = { "commands.debug": "Allow /debug", "commands.restart": "Allow Restart", "commands.useAccessGroups": "Use Access Groups", + "commands.ownerAllowFrom": "Command Owners", "ui.seamColor": "Accent Color", "ui.assistant.name": "Assistant Name", "ui.assistant.avatar": "Assistant Avatar", @@ -661,6 +662,8 @@ const FIELD_HELP: Record = { "commands.debug": "Allow /debug chat command for runtime-only overrides (default: false).", "commands.restart": "Allow /restart and gateway restart tool actions (default: false).", "commands.useAccessGroups": "Enforce access-group allowlists/policies for commands.", + "commands.ownerAllowFrom": + "Explicit owner allowlist for owner-only tools/commands. Use channel-native IDs (optionally prefixed like \"whatsapp:+15551234567\"). '*' is ignored.", "session.dmScope": 'DM session scoping: "main" keeps continuity; "per-peer", "per-channel-peer", or "per-account-channel-peer" isolates DM history (recommended for shared inboxes/multi-account).', "session.identityLinks": diff --git a/src/config/types.messages.ts b/src/config/types.messages.ts index 37ef4e9424..97de53417f 100644 --- a/src/config/types.messages.ts +++ b/src/config/types.messages.ts @@ -107,6 +107,8 @@ export type CommandsConfig = { restart?: boolean; /** Enforce access-group allowlists/policies for commands (default: true). */ useAccessGroups?: boolean; + /** Explicit owner allowlist for owner-only tools/commands (channel-native IDs). */ + ownerAllowFrom?: Array; }; export type ProviderCommandsConfig = { diff --git a/src/config/zod-schema.session.ts b/src/config/zod-schema.session.ts index e7a7245822..136c82d04c 100644 --- a/src/config/zod-schema.session.ts +++ b/src/config/zod-schema.session.ts @@ -112,6 +112,7 @@ export const CommandsSchema = z debug: z.boolean().optional(), restart: z.boolean().optional(), useAccessGroups: z.boolean().optional(), + ownerAllowFrom: z.array(z.union([z.string(), z.number()])).optional(), }) .strict() .optional() diff --git a/src/infra/outbound/message-action-runner.ts b/src/infra/outbound/message-action-runner.ts index bc4ae734ff..e60a01e87b 100644 --- a/src/infra/outbound/message-action-runner.ts +++ b/src/infra/outbound/message-action-runner.ts @@ -728,6 +728,9 @@ async function handleSendAction(ctx: ResolvedActionContext): Promise