mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
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 <victormier@gmail.com>
This commit is contained in:
committed by
GitHub
parent
0cd47d830f
commit
392bbddf29
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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). */
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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;
|
||||
|
||||
35
src/agents/pi-tools.whatsapp-login-gating.test.ts
Normal file
35
src/agents/pi-tools.whatsapp-login-gating.test.ts
Normal file
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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<string, string[]> = {
|
||||
],
|
||||
};
|
||||
|
||||
const OWNER_ONLY_TOOL_NAMES = new Set<string>(["whatsapp_login"]);
|
||||
|
||||
const TOOL_PROFILES: Record<ToolProfileId, ToolProfilePolicy> = {
|
||||
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 [];
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -12,6 +12,7 @@ export type CommandContext = {
|
||||
channel: string;
|
||||
channelId?: ChannelId;
|
||||
ownerList: string[];
|
||||
senderIsOwner: boolean;
|
||||
isAuthorizedSender: boolean;
|
||||
senderId?: string;
|
||||
abortKey?: string;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -430,6 +430,7 @@ export async function agentCommand(
|
||||
currentThreadTs: runContext.currentThreadTs,
|
||||
replyToMode: runContext.replyToMode,
|
||||
hasRepliedRef: runContext.hasRepliedRef,
|
||||
senderIsOwner: true,
|
||||
sessionFile,
|
||||
workspaceDir,
|
||||
config: cfg,
|
||||
|
||||
@@ -301,6 +301,7 @@ const FIELD_LABELS: Record<string, string> = {
|
||||
"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<string, string> = {
|
||||
"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":
|
||||
|
||||
@@ -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<string | number>;
|
||||
};
|
||||
|
||||
export type ProviderCommandsConfig = {
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -728,6 +728,9 @@ async function handleSendAction(ctx: ResolvedActionContext): Promise<MessageActi
|
||||
required: !mediaHint && !hasCard,
|
||||
allowEmpty: true,
|
||||
}) ?? "";
|
||||
if (message.includes("\\n")) {
|
||||
message = message.replaceAll("\\n", "\n");
|
||||
}
|
||||
|
||||
const parsed = parseReplyDirectives(message);
|
||||
const mergedMediaUrls: string[] = [];
|
||||
|
||||
Reference in New Issue
Block a user