From 6817c0ec7b4fa830123d4f5c340f075a4bd04ee2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 21:58:40 +0100 Subject: [PATCH] fix(security): tighten elevated allowFrom sender matching --- CHANGELOG.md | 2 +- docs/tools/elevated.md | 6 + ...evated-directive-unapproved-sender.test.ts | 2 +- src/auto-reply/reply/reply-elevated.test.ts | 36 +++ src/auto-reply/reply/reply-elevated.ts | 274 +++++++++++++++--- 5 files changed, 282 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77f1561f7f4..87d906b5908 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,7 @@ Docs: https://docs.openclaw.ai - 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. -- Security/Elevated: match `tools.elevated.allowFrom` against sender identities only (not recipient `ctx.To`), closing a recipient-token bypass for `/elevated` authorization. (#11022) Thanks @coygeek. +- Security/Elevated: match `tools.elevated.allowFrom` against sender identities only (not recipient `ctx.To`), closing a recipient-token bypass for `/elevated` authorization. This ships in the next npm release. Thanks @jiseoung for reporting. - Webchat/Sessions: preserve external session routing metadata when internal `chat.send` turns run under `webchat`, so explicit channel-keyed sessions (for example Telegram) no longer get rewritten to `webchat` and misroute follow-up delivery. (#23258) Thanks @binary64. - Webchat/Sessions: preserve existing session `label` across `/new` and `/reset` rollovers so reset sessions remain discoverable in session history lists. (#23755) Thanks @ThunderStormer. - Control UI/WebSocket: stop and clear the browser gateway client on UI teardown so remounts cannot leave orphan websocket clients that create duplicate active connections. (#23422) Thanks @floatinggball-design. diff --git a/docs/tools/elevated.md b/docs/tools/elevated.md index c9b8d87a949..eed788eda8c 100644 --- a/docs/tools/elevated.md +++ b/docs/tools/elevated.md @@ -46,6 +46,12 @@ title: "Elevated Mode" - Feature gate: `tools.elevated.enabled` (default can be off via config even if the code supports it). - Sender allowlist: `tools.elevated.allowFrom` with per-provider allowlists (e.g. `discord`, `whatsapp`). +- Unprefixed allowlist entries match sender-scoped identity values only (`SenderId`, `SenderE164`, `From`); recipient routing fields are never used for elevated authorization. +- Mutable sender metadata requires explicit prefixes: + - `name:` matches `SenderName` + - `username:` matches `SenderUsername` + - `tag:` matches `SenderTag` + - `id:`, `from:`, `e164:` are available for explicit identity targeting - Per-agent gate: `agents.list[].tools.elevated.enabled` (optional; can only further restrict). - Per-agent allowlist: `agents.list[].tools.elevated.allowFrom` (optional; when set, the sender must match **both** global + per-agent allowlists). - Discord fallback: if `tools.elevated.allowFrom.discord` is omitted, the `channels.discord.allowFrom` list is used as a fallback (legacy: `channels.discord.dm.allowFrom`). Set `tools.elevated.allowFrom.discord` (even `[]`) to override. Per-agent allowlists do **not** use the fallback. diff --git a/src/auto-reply/reply.triggers.trigger-handling.ignores-inline-elevated-directive-unapproved-sender.test.ts b/src/auto-reply/reply.triggers.trigger-handling.ignores-inline-elevated-directive-unapproved-sender.test.ts index 519e6e9edfc..c8532b38bad 100644 --- a/src/auto-reply/reply.triggers.trigger-handling.ignores-inline-elevated-directive-unapproved-sender.test.ts +++ b/src/auto-reply/reply.triggers.trigger-handling.ignores-inline-elevated-directive-unapproved-sender.test.ts @@ -48,7 +48,7 @@ describe("trigger handling", () => { it("uses tools.elevated.allowFrom.discord for elevated approval", async () => { await withTempHome(async (home) => { const cfg = makeCfg(home); - cfg.tools = { elevated: { allowFrom: { discord: ["steipete"] } } }; + cfg.tools = { elevated: { allowFrom: { discord: ["123"] } } }; const res = await getReplyFromConfig( { diff --git a/src/auto-reply/reply/reply-elevated.test.ts b/src/auto-reply/reply/reply-elevated.test.ts index 51371cb9059..74fba60acf7 100644 --- a/src/auto-reply/reply/reply-elevated.test.ts +++ b/src/auto-reply/reply/reply-elevated.test.ts @@ -19,6 +19,7 @@ function buildContext(overrides?: Partial): MsgContext { return { Provider: "whatsapp", Surface: "whatsapp", + SenderId: "+15550001111", From: "whatsapp:+15550001111", SenderE164: "+15550001111", To: "+15559990000", @@ -55,4 +56,39 @@ describe("resolveElevatedPermissions", () => { key: "tools.elevated.allowFrom.whatsapp", }); }); + + it("does not authorize untyped mutable sender fields", () => { + const result = resolveElevatedPermissions({ + cfg: buildConfig(["owner-display-name"]), + agentId: "main", + provider: "whatsapp", + ctx: buildContext({ + SenderName: "owner-display-name", + SenderUsername: "owner-display-name", + SenderTag: "owner-display-name", + }), + }); + + expect(result.enabled).toBe(true); + expect(result.allowed).toBe(false); + expect(result.failures).toContainEqual({ + gate: "allowFrom", + key: "tools.elevated.allowFrom.whatsapp", + }); + }); + + it("authorizes mutable sender fields only with explicit prefix", () => { + const result = resolveElevatedPermissions({ + cfg: buildConfig(["username:owner_username"]), + agentId: "main", + provider: "whatsapp", + ctx: buildContext({ + SenderUsername: "owner_username", + }), + }); + + expect(result.enabled).toBe(true); + expect(result.allowed).toBe(true); + expect(result.failures).toHaveLength(0); + }); }); diff --git a/src/auto-reply/reply/reply-elevated.ts b/src/auto-reply/reply/reply-elevated.ts index 744274fdae5..7e755fa0d70 100644 --- a/src/auto-reply/reply/reply-elevated.ts +++ b/src/auto-reply/reply/reply-elevated.ts @@ -8,6 +8,17 @@ import { INTERNAL_MESSAGE_CHANNEL } from "../../utils/message-channel.js"; import type { MsgContext } from "../templating.js"; export { formatElevatedUnavailableMessage } from "./elevated-unavailable.js"; +type ExplicitElevatedAllowField = "id" | "from" | "e164" | "name" | "username" | "tag"; + +const EXPLICIT_ELEVATED_ALLOW_FIELDS = new Set([ + "id", + "from", + "e164", + "name", + "username", + "tag", +]); + function normalizeAllowToken(value?: string) { if (!value) { return ""; @@ -48,7 +59,120 @@ function resolveElevatedAllowList( return Array.isArray(value) ? value : fallbackAllowFrom; } +function parseExplicitElevatedAllowEntry( + entry: string, +): { field: ExplicitElevatedAllowField; value: string } | null { + const separatorIndex = entry.indexOf(":"); + if (separatorIndex <= 0) { + return null; + } + const fieldRaw = entry.slice(0, separatorIndex).trim().toLowerCase(); + if (!EXPLICIT_ELEVATED_ALLOW_FIELDS.has(fieldRaw as ExplicitElevatedAllowField)) { + return null; + } + const value = entry.slice(separatorIndex + 1).trim(); + if (!value) { + return null; + } + return { + field: fieldRaw as ExplicitElevatedAllowField, + value, + }; +} + +function addTokenVariants(tokens: Set, value: string) { + if (!value) { + return; + } + tokens.add(value); + const normalized = normalizeAllowToken(value); + if (normalized) { + tokens.add(normalized); + } +} + +function addProviderFormattedTokens(params: { + cfg: OpenClawConfig; + provider: string; + accountId?: string; + values: string[]; + tokens: Set; +}) { + const normalizedProvider = normalizeChannelId(params.provider); + const dock = normalizedProvider ? getChannelDock(normalizedProvider) : undefined; + const formatted = dock?.config?.formatAllowFrom + ? dock.config.formatAllowFrom({ + cfg: params.cfg, + accountId: params.accountId, + allowFrom: params.values, + }) + : params.values.map((entry) => String(entry).trim()).filter(Boolean); + for (const entry of formatted) { + addTokenVariants(params.tokens, entry); + } +} + +function matchesProviderFormattedTokens(params: { + cfg: OpenClawConfig; + provider: string; + accountId?: string; + value: string; + includeStripped?: boolean; + tokens: Set; +}): boolean { + const probeTokens = new Set(); + const values = params.includeStripped + ? [params.value, stripSenderPrefix(params.value)].filter(Boolean) + : [params.value]; + addProviderFormattedTokens({ + cfg: params.cfg, + provider: params.provider, + accountId: params.accountId, + values, + tokens: probeTokens, + }); + for (const token of probeTokens) { + if (params.tokens.has(token)) { + return true; + } + } + return false; +} + +function buildMutableTokens(value?: string): Set { + const tokens = new Set(); + const trimmed = value?.trim(); + if (!trimmed) { + return tokens; + } + addTokenVariants(tokens, trimmed); + const slugged = slugAllowToken(trimmed); + if (slugged) { + addTokenVariants(tokens, slugged); + } + return tokens; +} + +function matchesMutableTokens(value: string, tokens: Set): boolean { + if (!value || tokens.size === 0) { + return false; + } + const probes = new Set(); + addTokenVariants(probes, value); + const slugged = slugAllowToken(value); + if (slugged) { + addTokenVariants(probes, slugged); + } + for (const probe of probes) { + if (tokens.has(probe)) { + return true; + } + } + return false; +} + function isApprovedElevatedSender(params: { + cfg: OpenClawConfig; provider: string; ctx: MsgContext; allowFrom?: AgentElevatedAllowFromConfig; @@ -71,48 +195,124 @@ function isApprovedElevatedSender(params: { return true; } - const tokens = new Set(); - const addToken = (value?: string) => { - if (!value) { - return; - } - const trimmed = value.trim(); - if (!trimmed) { - return; - } - tokens.add(trimmed); - const normalized = normalizeAllowToken(trimmed); - if (normalized) { - tokens.add(normalized); - } - const slugged = slugAllowToken(trimmed); - if (slugged) { - tokens.add(slugged); - } - }; + const senderIdTokens = new Set(); + const senderFromTokens = new Set(); + const senderE164Tokens = new Set(); - addToken(params.ctx.SenderName); - addToken(params.ctx.SenderUsername); - addToken(params.ctx.SenderTag); - addToken(params.ctx.SenderE164); - addToken(params.ctx.From); - addToken(stripSenderPrefix(params.ctx.From)); + if (params.ctx.SenderId?.trim()) { + addProviderFormattedTokens({ + cfg: params.cfg, + provider: params.provider, + accountId: params.ctx.AccountId, + values: [params.ctx.SenderId, stripSenderPrefix(params.ctx.SenderId)].filter(Boolean), + tokens: senderIdTokens, + }); + } + if (params.ctx.From?.trim()) { + addProviderFormattedTokens({ + cfg: params.cfg, + provider: params.provider, + accountId: params.ctx.AccountId, + values: [params.ctx.From, stripSenderPrefix(params.ctx.From)].filter(Boolean), + tokens: senderFromTokens, + }); + } + if (params.ctx.SenderE164?.trim()) { + addProviderFormattedTokens({ + cfg: params.cfg, + provider: params.provider, + accountId: params.ctx.AccountId, + values: [params.ctx.SenderE164], + tokens: senderE164Tokens, + }); + } + const senderIdentityTokens = new Set([ + ...senderIdTokens, + ...senderFromTokens, + ...senderE164Tokens, + ]); - for (const rawEntry of allowTokens) { - const entry = rawEntry.trim(); - if (!entry) { + const senderNameTokens = buildMutableTokens(params.ctx.SenderName); + const senderUsernameTokens = buildMutableTokens(params.ctx.SenderUsername); + const senderTagTokens = buildMutableTokens(params.ctx.SenderTag); + + for (const entry of allowTokens) { + const explicitEntry = parseExplicitElevatedAllowEntry(entry); + if (!explicitEntry) { + if ( + matchesProviderFormattedTokens({ + cfg: params.cfg, + provider: params.provider, + accountId: params.ctx.AccountId, + value: entry, + includeStripped: true, + tokens: senderIdentityTokens, + }) + ) { + return true; + } continue; } - const stripped = stripSenderPrefix(entry); - if (tokens.has(entry) || tokens.has(stripped)) { - return true; + if (explicitEntry.field === "id") { + if ( + matchesProviderFormattedTokens({ + cfg: params.cfg, + provider: params.provider, + accountId: params.ctx.AccountId, + value: explicitEntry.value, + includeStripped: true, + tokens: senderIdTokens, + }) + ) { + return true; + } + continue; } - const normalized = normalizeAllowToken(stripped); - if (normalized && tokens.has(normalized)) { - return true; + if (explicitEntry.field === "from") { + if ( + matchesProviderFormattedTokens({ + cfg: params.cfg, + provider: params.provider, + accountId: params.ctx.AccountId, + value: explicitEntry.value, + includeStripped: true, + tokens: senderFromTokens, + }) + ) { + return true; + } + continue; } - const slugged = slugAllowToken(stripped); - if (slugged && tokens.has(slugged)) { + if (explicitEntry.field === "e164") { + if ( + matchesProviderFormattedTokens({ + cfg: params.cfg, + provider: params.provider, + accountId: params.ctx.AccountId, + value: explicitEntry.value, + tokens: senderE164Tokens, + }) + ) { + return true; + } + continue; + } + if (explicitEntry.field === "name") { + if (matchesMutableTokens(explicitEntry.value, senderNameTokens)) { + return true; + } + continue; + } + if (explicitEntry.field === "username") { + if (matchesMutableTokens(explicitEntry.value, senderUsernameTokens)) { + return true; + } + continue; + } + if ( + explicitEntry.field === "tag" && + matchesMutableTokens(explicitEntry.value, senderTagTokens) + ) { return true; } } @@ -162,6 +362,7 @@ export function resolveElevatedPermissions(params: { : undefined; const fallbackAllowFrom = dockFallbackAllowFrom; const globalAllowed = isApprovedElevatedSender({ + cfg: params.cfg, provider: params.provider, ctx: params.ctx, allowFrom: globalConfig?.allowFrom, @@ -177,6 +378,7 @@ export function resolveElevatedPermissions(params: { const agentAllowed = agentConfig?.allowFrom ? isApprovedElevatedSender({ + cfg: params.cfg, provider: params.provider, ctx: params.ctx, allowFrom: agentConfig.allowFrom,