fix: add explicit owner allowlist for tools (#8768) (thanks @victormier)

This commit is contained in:
Gustavo Madeira Santana
2026-02-04 18:58:31 -05:00
parent 8544373ce8
commit 253e0d0b4a
7 changed files with 134 additions and 22 deletions

View File

@@ -44,6 +44,7 @@ import {
} from "./pi-tools.read.js";
import { cleanToolSchemaForGemini, normalizeToolParameters } from "./pi-tools.schema.js";
import {
applyOwnerOnlyToolPolicy,
buildPluginToolGroups,
collectExplicitAllowlist,
expandPolicyWithPluginGroups,
@@ -361,23 +362,7 @@ export function createOpenClawCodingTools(options?: {
];
// Security: treat unknown/undefined as unauthorized (opt-in, not opt-out)
const senderIsOwner = options?.senderIsOwner === true;
const toolsWithOwnerGuard = tools.map((tool) => {
if (normalizeToolName(tool.name) !== "whatsapp_login") {
return tool;
}
if (senderIsOwner || !tool.execute) {
return tool;
}
return {
...tool,
execute: async () => {
throw new Error("whatsapp_login is restricted to owner senders.");
},
};
});
const toolsByAuthorization = senderIsOwner
? toolsWithOwnerGuard
: toolsWithOwnerGuard.filter((tool) => normalizeToolName(tool.name) !== "whatsapp_login");
const toolsByAuthorization = applyOwnerOnlyToolPolicy(tools, senderIsOwner);
const coreToolNames = new Set(
toolsByAuthorization
.filter((tool) => !getPluginToolMeta(tool))

View File

@@ -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 [];

View File

@@ -84,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;
@@ -142,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,
@@ -154,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,
@@ -171,11 +221,18 @@ 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 senderIsOwner = Boolean(matchedSender);
const isOwnerForCommands = !enforceOwner || allowAll || ownerList.length === 0 || senderIsOwner;
const isOwnerForCommands =
!enforceOwner ||
allowAll ||
ownerCandidatesForCommands.length === 0 ||
Boolean(matchedCommandOwner);
const isAuthorizedSender = commandAuthorized && isOwnerForCommands;
return {

View File

@@ -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", () => {

View File

@@ -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":

View File

@@ -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 = {

View File

@@ -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()