fix(discord): canonicalize resolved allowlists to ids

This commit is contained in:
Peter Steinberger
2026-02-21 19:53:00 +01:00
parent 3ed71d6f76
commit 747bb581b3
5 changed files with 168 additions and 30 deletions

View File

@@ -34,7 +34,7 @@ Docs: https://docs.openclaw.ai
- Chat/Usage/TUI: strip synthetic inbound metadata blocks (including `Conversation info` and trailing `Untrusted context` channel metadata wrappers) from displayed conversation history so internal prompt context no longer leaks into user-visible logs.
- Security/Exec: in non-default setups that manually add `sort` to `tools.exec.safeBins`, block `sort --compress-program` so allowlist-mode safe-bin checks cannot bypass approval. Thanks @tdjackey for reporting.
- Security/Discord: add `openclaw security audit` warnings for name/tag-based Discord allowlist entries (DM allowlists, guild/channel `users`, and pairing-store entries), highlighting slug-collision risk while keeping name-based matching supported. Thanks @tdjackey for reporting.
- Security/Discord: add `openclaw security audit` warnings for name/tag-based Discord allowlist entries (DM allowlists, guild/channel `users`, and pairing-store entries), highlighting slug-collision risk while keeping name-based matching supported, and canonicalize resolved Discord allowlist names to IDs at runtime without rewriting config files. Thanks @tdjackey for reporting.
- Security/BlueBubbles: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected `pairing`/`allowlist` DM gating for BlueBubbles and blocking unauthorized DM/reaction processing when no allowlist entries are configured. This ships in the next npm release. Thanks @tdjackey for reporting.
- Doctor/State integrity: only require/create the OAuth credentials directory when WhatsApp or pairing-backed channels are configured, and downgrade fresh-install missing-dir noise to an informational warning.
- Agents/Sanitization: stop rewriting billing-shaped assistant text outside explicit error context so normal replies about billing/credits/payment are preserved across messaging channels. (#17834, fixes #11359)

View File

@@ -2,6 +2,8 @@ import { describe, expect, it } from "vitest";
import {
addAllowlistUserEntriesFromConfigEntry,
buildAllowlistResolutionSummary,
canonicalizeAllowlistWithResolvedIds,
patchAllowlistUsersInConfigEntries,
} from "./resolve-utils.js";
describe("buildAllowlistResolutionSummary", () => {
@@ -40,3 +42,46 @@ describe("addAllowlistUserEntriesFromConfigEntry", () => {
expect(Array.from(target)).toEqual(["a"]);
});
});
describe("canonicalizeAllowlistWithResolvedIds", () => {
it("replaces resolved names with ids and keeps unresolved entries", () => {
const resolvedMap = new Map([
["Alice#1234", { input: "Alice#1234", resolved: true, id: "111" }],
["bob", { input: "bob", resolved: false }],
]);
const result = canonicalizeAllowlistWithResolvedIds({
existing: ["Alice#1234", "bob", "222", "*"],
resolvedMap,
});
expect(result).toEqual(["111", "bob", "222", "*"]);
});
it("deduplicates ids after canonicalization", () => {
const resolvedMap = new Map([["alice", { input: "alice", resolved: true, id: "111" }]]);
const result = canonicalizeAllowlistWithResolvedIds({
existing: ["alice", "111", "alice"],
resolvedMap,
});
expect(result).toEqual(["111"]);
});
});
describe("patchAllowlistUsersInConfigEntries", () => {
it("supports canonicalization strategy for nested users", () => {
const entries = {
alpha: { users: ["Alice", "111", "Bob"] },
beta: { users: ["*"] },
};
const resolvedMap = new Map([
["Alice", { input: "Alice", resolved: true, id: "111" }],
["Bob", { input: "Bob", resolved: false }],
]);
const patched = patchAllowlistUsersInConfigEntries({
entries,
resolvedMap,
strategy: "canonicalize",
});
expect((patched.alpha as { users: string[] }).users).toEqual(["111", "Bob"]);
expect((patched.beta as { users: string[] }).users).toEqual(["*"]);
});
});

View File

@@ -6,31 +6,32 @@ export type AllowlistUserResolutionLike = {
id?: string;
};
function dedupeAllowlistEntries(entries: string[]): string[] {
const seen = new Set<string>();
const deduped: string[] = [];
for (const entry of entries) {
const normalized = entry.trim();
if (!normalized) {
continue;
}
const key = normalized.toLowerCase();
if (seen.has(key)) {
continue;
}
seen.add(key);
deduped.push(normalized);
}
return deduped;
}
export function mergeAllowlist(params: {
existing?: Array<string | number>;
additions: string[];
}): string[] {
const seen = new Set<string>();
const merged: string[] = [];
const push = (value: string) => {
const normalized = value.trim();
if (!normalized) {
return;
}
const key = normalized.toLowerCase();
if (seen.has(key)) {
return;
}
seen.add(key);
merged.push(normalized);
};
for (const entry of params.existing ?? []) {
push(String(entry));
}
for (const entry of params.additions) {
push(entry);
}
return merged;
return dedupeAllowlistEntries([
...(params.existing ?? []).map((entry) => String(entry)),
...params.additions,
]);
}
export function buildAllowlistResolutionSummary<T extends AllowlistUserResolutionLike>(
@@ -71,10 +72,33 @@ export function resolveAllowlistIdAdditions<T extends AllowlistUserResolutionLik
return additions;
}
export function canonicalizeAllowlistWithResolvedIds<
T extends AllowlistUserResolutionLike,
>(params: { existing?: Array<string | number>; resolvedMap: Map<string, T> }): string[] {
const canonicalized: string[] = [];
for (const entry of params.existing ?? []) {
const trimmed = String(entry).trim();
if (!trimmed) {
continue;
}
if (trimmed === "*") {
canonicalized.push(trimmed);
continue;
}
const resolved = params.resolvedMap.get(trimmed);
canonicalized.push(resolved?.resolved && resolved.id ? resolved.id : trimmed);
}
return dedupeAllowlistEntries(canonicalized);
}
export function patchAllowlistUsersInConfigEntries<
T extends AllowlistUserResolutionLike,
TEntries extends Record<string, unknown>,
>(params: { entries: TEntries; resolvedMap: Map<string, T> }): TEntries {
>(params: {
entries: TEntries;
resolvedMap: Map<string, T>;
strategy?: "merge" | "canonicalize";
}): TEntries {
const nextEntries: Record<string, unknown> = { ...params.entries };
for (const [entryKey, entryConfig] of Object.entries(params.entries)) {
if (!entryConfig || typeof entryConfig !== "object") {
@@ -88,9 +112,16 @@ export function patchAllowlistUsersInConfigEntries<
existing: users,
resolvedMap: params.resolvedMap,
});
const resolvedUsers =
params.strategy === "canonicalize"
? canonicalizeAllowlistWithResolvedIds({
existing: users,
resolvedMap: params.resolvedMap,
})
: mergeAllowlist({ existing: users, additions });
nextEntries[entryKey] = {
...entryConfig,
users: mergeAllowlist({ existing: users, additions }),
users: resolvedUsers,
};
}
return nextEntries as TEntries;

View File

@@ -0,0 +1,57 @@
import { describe, expect, it, vi } from "vitest";
import type { RuntimeEnv } from "../../runtime.js";
const { resolveDiscordChannelAllowlistMock, resolveDiscordUserAllowlistMock } = vi.hoisted(() => ({
resolveDiscordChannelAllowlistMock: vi.fn(async () => []),
resolveDiscordUserAllowlistMock: vi.fn(async (params: { entries: string[] }) =>
params.entries.map((entry) => {
switch (entry) {
case "Alice":
return { input: entry, resolved: true, id: "111" };
case "Bob":
return { input: entry, resolved: true, id: "222" };
case "Carol":
return { input: entry, resolved: false };
default:
return { input: entry, resolved: true, id: entry };
}
}),
),
}));
vi.mock("../resolve-channels.js", () => ({
resolveDiscordChannelAllowlist: resolveDiscordChannelAllowlistMock,
}));
vi.mock("../resolve-users.js", () => ({
resolveDiscordUserAllowlist: resolveDiscordUserAllowlistMock,
}));
import { resolveDiscordAllowlistConfig } from "./provider.allowlist.js";
describe("resolveDiscordAllowlistConfig", () => {
it("canonicalizes resolved user names to ids in runtime config", async () => {
const runtime = { log: vi.fn(), error: vi.fn(), exit: vi.fn() } as unknown as RuntimeEnv;
const result = await resolveDiscordAllowlistConfig({
token: "token",
allowFrom: ["Alice", "111", "*"],
guildEntries: {
"*": {
users: ["Bob", "999"],
channels: {
"*": {
users: ["Carol", "888"],
},
},
},
},
fetcher: vi.fn() as unknown as typeof fetch,
runtime,
});
expect(result.allowFrom).toEqual(["111", "*"]);
expect(result.guildEntries?.["*"]?.users).toEqual(["222", "999"]);
expect(result.guildEntries?.["*"]?.channels?.["*"]?.users).toEqual(["Carol", "888"]);
expect(resolveDiscordUserAllowlistMock).toHaveBeenCalledTimes(2);
});
});

View File

@@ -1,9 +1,8 @@
import {
addAllowlistUserEntriesFromConfigEntry,
buildAllowlistResolutionSummary,
mergeAllowlist,
canonicalizeAllowlistWithResolvedIds,
patchAllowlistUsersInConfigEntries,
resolveAllowlistIdAdditions,
summarizeMapping,
} from "../../channels/allowlists/resolve-utils.js";
import type { DiscordGuildEntry } from "../../config/types.discord.js";
@@ -138,8 +137,11 @@ export async function resolveDiscordAllowlistConfig(params: {
entries: allowEntries.map((entry) => String(entry)),
fetcher: params.fetcher,
});
const { mapping, unresolved, additions } = buildAllowlistResolutionSummary(resolvedUsers);
allowFrom = mergeAllowlist({ existing: allowFrom, additions });
const { resolvedMap, mapping, unresolved } = buildAllowlistResolutionSummary(resolvedUsers);
allowFrom = canonicalizeAllowlistWithResolvedIds({
existing: allowFrom,
resolvedMap,
});
summarizeMapping("discord users", mapping, unresolved, params.runtime);
} catch (err) {
params.runtime.log?.(
@@ -178,14 +180,17 @@ export async function resolveDiscordAllowlistConfig(params: {
const nextGuild = { ...guildConfig } as Record<string, unknown>;
const users = (guildConfig as { users?: string[] }).users;
if (Array.isArray(users) && users.length > 0) {
const additions = resolveAllowlistIdAdditions({ existing: users, resolvedMap });
nextGuild.users = mergeAllowlist({ existing: users, additions });
nextGuild.users = canonicalizeAllowlistWithResolvedIds({
existing: users,
resolvedMap,
});
}
const channels = (guildConfig as { channels?: Record<string, unknown> }).channels ?? {};
if (channels && typeof channels === "object") {
nextGuild.channels = patchAllowlistUsersInConfigEntries({
entries: channels,
resolvedMap,
strategy: "canonicalize",
});
}
nextGuilds[guildKey] = nextGuild as DiscordGuildEntry;