nyrn
/
openclaw
mirror of https://github.com/openclaw/openclaw.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Integrations: tighten inbound callback and allowlist checks (#46787) 

* Integrations: harden inbound callback and allowlist handling

* Integrations: address review follow-ups

* Update CHANGELOG.md

* Mattermost: avoid command-gating open button callbacks
This commit is contained in:
Vincent Koc 2026-03-15 09:24:24 -07:00 committed by GitHub
parent 67b2d1b8e8
commit a47722de7e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 323 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGELOG.md
View File

@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai
- Zalo Personal/group gating: stop reapplying `dmPolicy.allowFrom` as a sender gate for already-allowlisted groups when `groupAllowFrom` is unset, so any member of an allowed group can trigger replies while DMs stay restricted. (#40146)
- Browser/remote CDP: honor strict browser SSRF policy during remote CDP reachability and `/json/version` discovery checks, redact sensitive `cdpUrl` tokens from status output, and warn when remote CDP targets private/internal hosts.
- Plugins/install precedence: keep bundled plugins ahead of auto-discovered globals by default, but let an explicitly installed plugin record win its own duplicate-id tie so installed channel plugins load from `~/.openclaw/extensions` after `openclaw plugins install`.
- Inbound policy hardening: tighten callback and webhook sender checks across Mattermost and Google Chat, match Nextcloud Talk rooms by stable room token, and treat explicit empty Twitch allowlists as deny-all. Thanks @vincentkoc.
- macOS/canvas actions: keep unattended local agent actions on trusted in-app canvas surfaces only, and stop exposing the deep-link fallback key to arbitrary page scripts. Thanks @vincentkoc.
- Agents/compaction: extend the enclosing run deadline once while compaction is actively in flight, and abort the underlying SDK compaction on timeout/cancel so large-session compactions stop freezing mid-run. (#46889) Thanks @asyncjason.
- Models/openai-completions: default non-native OpenAI-compatible providers to omit tool-definition `strict` fields unless users explicitly opt back in, so tool calling keeps working on providers that reject that option. (#45497) Thanks @sahancava.
@ -36,9 +37,6 @@ Docs: https://docs.openclaw.ai
- WhatsApp/login: wait for pending creds writes before reopening after Baileys `515` pairing restarts in both QR login and `channels login` flows, and keep the restart coverage pinned to the real wrapped error shape plus per-account creds queues. (#27910) Thanks @asyncjason.
- Agents/openai-compatible tool calls: deduplicate repeated tool call ids across live assistant messages and replayed history so OpenAI-compatible backends no longer reject duplicate `tool_call_id` values with HTTP 400. (#40996) Thanks @xaeon2026.
- Security/device pairing: harden `device.token.rotate` deny handling by keeping public failures generic while logging internal deny reasons and preserving approved-baseline enforcement. (`GHSA-7jrw-x62h-64p8`)
### Fixes
- Slack/interactive replies: preserve `channelData.slack.blocks` through live DM delivery and preview-finalized edits so Block Kit button and select directives render instead of falling back to raw text. Thanks @vincentkoc.
- Zalo/plugin runtime: export `resolveClientIp` from `openclaw/plugin-sdk/zalo` so installed builds no longer crash on startup when the webhook monitor loads from the packaged extension instead of the monorepo source tree. (#46549) Thanks @No898.
- CI/channel test routing: move the built-in channel suites into `test:channels` and keep them out of `test:extensions`, so extension CI no longer fails after the channel migration while targeted test routing still sends Slack, Signal, and iMessage suites to the right lane. (#46066) Thanks @scoootscooob.

97
extensions/googlechat/src/auth.test.ts Normal file
View File

@ -0,0 +1,97 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
const mocks = vi.hoisted(() => ({
verifyIdToken: vi.fn(),
}));
vi.mock("google-auth-library", () => ({
GoogleAuth: class {},
OAuth2Client: class {
verifyIdToken = mocks.verifyIdToken;
},
}));
const { verifyGoogleChatRequest } = await import("./auth.js");
function mockTicket(payload: Record<string, unknown>) {
mocks.verifyIdToken.mockResolvedValue({
getPayload: () => payload,
});
}
describe("verifyGoogleChatRequest", () => {
beforeEach(() => {
mocks.verifyIdToken.mockReset();
});
it("accepts Google Chat app-url tokens from the Chat issuer", async () => {
mockTicket({
email: "chat@system.gserviceaccount.com",
email_verified: true,
});
await expect(
verifyGoogleChatRequest({
bearer: "token",
audienceType: "app-url",
audience: "https://example.com/googlechat",
}),
).resolves.toEqual({ ok: true });
});
it("rejects add-on tokens when no principal binding is configured", async () => {
mockTicket({
email: "service-123@gcp-sa-gsuiteaddons.iam.gserviceaccount.com",
email_verified: true,
sub: "principal-1",
});
await expect(
verifyGoogleChatRequest({
bearer: "token",
audienceType: "app-url",
audience: "https://example.com/googlechat",
}),
).resolves.toEqual({
ok: false,
reason: "missing add-on principal binding",
});
});
it("accepts add-on tokens only when the bound principal matches", async () => {
mockTicket({
email: "service-123@gcp-sa-gsuiteaddons.iam.gserviceaccount.com",
email_verified: true,
sub: "principal-1",
});
await expect(
verifyGoogleChatRequest({
bearer: "token",
audienceType: "app-url",
audience: "https://example.com/googlechat",
expectedAddOnPrincipal: "principal-1",
}),
).resolves.toEqual({ ok: true });
});
it("rejects add-on tokens when the bound principal does not match", async () => {
mockTicket({
email: "service-123@gcp-sa-gsuiteaddons.iam.gserviceaccount.com",
email_verified: true,
sub: "principal-2",
});
await expect(
verifyGoogleChatRequest({
bearer: "token",
audienceType: "app-url",
audience: "https://example.com/googlechat",
expectedAddOnPrincipal: "principal-1",
}),
).resolves.toEqual({
ok: false,
reason: "unexpected add-on principal: principal-2",
});
});
});

31
extensions/googlechat/src/auth.ts
View File

@ -94,6 +94,7 @@ export async function verifyGoogleChatRequest(params: {
bearer?: string | null;
audienceType?: GoogleChatAudienceType | null;
audience?: string | null;
expectedAddOnPrincipal?: string | null;
}): Promise<{ ok: boolean; reason?: string }> {
const bearer = params.bearer?.trim();
if (!bearer) {
@ -112,10 +113,32 @@ export async function verifyGoogleChatRequest(params: {
audience,
});
const payload = ticket.getPayload();
const email = payload?.email ?? "";
const ok =
payload?.email_verified && (email === CHAT_ISSUER || ADDON_ISSUER_PATTERN.test(email));
return ok ? { ok: true } : { ok: false, reason: `invalid issuer: ${email}` };
const email = String(payload?.email ?? "")
.trim()
.toLowerCase();
if (!payload?.email_verified) {
return { ok: false, reason: "email not verified" };
}
if (email === CHAT_ISSUER) {
return { ok: true };
}
if (!ADDON_ISSUER_PATTERN.test(email)) {
return { ok: false, reason: `invalid issuer: ${email}` };
}
const expectedAddOnPrincipal = params.expectedAddOnPrincipal?.trim().toLowerCase();
if (!expectedAddOnPrincipal) {
return { ok: false, reason: "missing add-on principal binding" };
}
const tokenPrincipal = String(payload?.sub ?? "")
.trim()
.toLowerCase();
if (!tokenPrincipal || tokenPrincipal !== expectedAddOnPrincipal) {
return {
ok: false,
reason: `unexpected add-on principal: ${tokenPrincipal || "<missing>"}`,
};
}
return { ok: true };
} catch (err) {
return { ok: false, reason: err instanceof Error ? err.message : "invalid token" };
}

2
extensions/googlechat/src/monitor-webhook.ts
View File

@ -132,6 +132,7 @@ export function createGoogleChatWebhookRequestHandler(params: {
bearer: headerBearer,
audienceType: target.audienceType,
audience: target.audience,
expectedAddOnPrincipal: target.account.config.appPrincipal,
});
return verification.ok;
},
@ -166,6 +167,7 @@ export function createGoogleChatWebhookRequestHandler(params: {
bearer: parsed.addOnBearerToken,
audienceType: target.audienceType,
audience: target.audience,
expectedAddOnPrincipal: target.account.config.appPrincipal,
});
return verification.ok;
},

31
extensions/mattermost/src/mattermost/interactions.test.ts
View File

@ -738,6 +738,37 @@ describe("createMattermostInteractionHandler", () => {
expectSuccessfulApprovalUpdate(res, requestLog);
});
it("blocks button dispatch when the sender is not allowed for the action", async () => {
const { context, token } = createActionContext();
const dispatchButtonClick = vi.fn();
const handleInteraction = vi.fn();
const handler = createMattermostInteractionHandler({
client: {
request: async (_path: string, init?: { method?: string }) =>
init?.method === "PUT" ? { id: "post-1" } : createActionPost(),
} as unknown as MattermostClient,
botUserId: "bot",
accountId: "acct",
authorizeButtonClick: async () => ({
ok: false,
response: {
ephemeral_text: "blocked",
},
}),
handleInteraction,
dispatchButtonClick,
});
const res = await runHandler(handler, {
body: createInteractionBody({ context, token }),
});
expect(res.statusCode).toBe(200);
expect(res.body).toContain("blocked");
expect(handleInteraction).not.toHaveBeenCalled();
expect(dispatchButtonClick).not.toHaveBeenCalled();
});
it("forwards fetched post threading metadata to session and button callbacks", async () => {
const enqueueSystemEvent = vi.fn();
setMattermostRuntime({

35
extensions/mattermost/src/mattermost/interactions.ts
View File

@ -37,6 +37,10 @@ export type MattermostInteractionResponse = {
ephemeral_text?: string;
};
export type MattermostInteractionAuthorizationResult =
| { ok: true }
| { ok: false; statusCode?: number; response?: MattermostInteractionResponse };
export type MattermostInteractiveButtonInput = {
id?: string;
callback_data?: string;
@ -404,6 +408,10 @@ export function createMattermostInteractionHandler(params: {
context: Record<string, unknown>;
post: MattermostPost;
}) => Promise<MattermostInteractionResponse | null>;
authorizeButtonClick?: (opts: {
payload: MattermostInteractionPayload;
post: MattermostPost;
}) => Promise<MattermostInteractionAuthorizationResult>;
dispatchButtonClick?: (opts: {
channelId: string;
userId: string;
@ -566,6 +574,33 @@ export function createMattermostInteractionHandler(params: {
`post=${payload.post_id} channel=${payload.channel_id}`,
);
if (params.authorizeButtonClick) {
try {
const authorization = await params.authorizeButtonClick({
payload,
post: originalPost,
});
if (!authorization.ok) {
res.statusCode = authorization.statusCode ?? 200;
res.setHeader("Content-Type", "application/json");
res.end(
JSON.stringify(
authorization.response ?? {
ephemeral_text: "You are not allowed to use this action here.",
},
),
);
return;
}
} catch (err) {
log?.(`mattermost interaction: authorization failed: ${String(err)}`);
res.statusCode = 500;
res.setHeader("Content-Type", "application/json");
res.end(JSON.stringify({ error: "Interaction authorization failed" }));
return;
}
}
if (params.handleInteraction) {
try {
const response = await params.handleInteraction({

39
extensions/mattermost/src/mattermost/monitor.ts
View File

@ -567,6 +567,45 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
trustedProxies: cfg.gateway?.trustedProxies,
allowRealIpFallback: cfg.gateway?.allowRealIpFallback === true,
handleInteraction: handleModelPickerInteraction,
authorizeButtonClick: async ({ payload, post }) => {
const channelInfo = await resolveChannelInfo(payload.channel_id);
const isDirect = channelInfo?.type?.trim().toUpperCase() === "D";
const allowTextCommands = core.channel.commands.shouldHandleTextCommands({
cfg,
surface: "mattermost",
});
const decision = authorizeMattermostCommandInvocation({
account,
cfg,
senderId: payload.user_id,
senderName: payload.user_name ?? "",
channelId: payload.channel_id,
channelInfo,
storeAllowFrom: isDirect
? await readStoreAllowFromForDmPolicy({
provider: "mattermost",
accountId: account.accountId,
dmPolicy: account.config.dmPolicy ?? "pairing",
readStore: pairing.readStoreForDmPolicy,
})
: undefined,
allowTextCommands,
hasControlCommand: false,
});
if (decision.ok) {
return { ok: true };
}
return {
ok: false,
response: {
update: {
message: post.message ?? "",
props: post.props as Record<string, unknown> | undefined,
},
ephemeral_text: `OpenClaw ignored this action for ${decision.roomLabel}.`,
},
};
},
resolveSessionKey: async ({ channelId, userId, post }) => {
const channelInfo = await resolveChannelInfo(channelId);
const kind = mapMattermostChannelTypeToChatType(channelInfo?.type);

73
extensions/nextcloud-talk/src/inbound.authz.test.ts
View File

@ -81,4 +81,77 @@ describe("nextcloud-talk inbound authz", () => {
});
expect(buildMentionRegexes).not.toHaveBeenCalled();
});
it("matches group rooms by token instead of colliding room names", async () => {
const readAllowFromStore = vi.fn(async () => []);
const buildMentionRegexes = vi.fn(() => [/@openclaw/i]);
setNextcloudTalkRuntime({
channel: {
pairing: {
readAllowFromStore,
},
commands: {
shouldHandleTextCommands: () => false,
},
text: {
hasControlCommand: () => false,
},
mentions: {
buildMentionRegexes,
matchesMentionPatterns: () => false,
},
},
} as unknown as PluginRuntime);
const message: NextcloudTalkInboundMessage = {
messageId: "m-2",
roomToken: "room-attacker",
roomName: "Room Trusted",
senderId: "trusted-user",
senderName: "Trusted User",
text: "hello",
mediaType: "text/plain",
timestamp: Date.now(),
isGroupChat: true,
};
const account: ResolvedNextcloudTalkAccount = {
accountId: "default",
enabled: true,
baseUrl: "",
secret: "",
secretSource: "none",
config: {
dmPolicy: "pairing",
allowFrom: [],
groupPolicy: "allowlist",
groupAllowFrom: ["trusted-user"],
rooms: {
"room-trusted": {
enabled: true,
},
},
},
};
await handleNextcloudTalkInbound({
message,
account,
config: {
channels: {
"nextcloud-talk": {
groupPolicy: "allowlist",
groupAllowFrom: ["trusted-user"],
},
},
},
runtime: {
log: vi.fn(),
error: vi.fn(),
} as unknown as RuntimeEnv,
});
expect(buildMentionRegexes).not.toHaveBeenCalled();
});
});

1
extensions/nextcloud-talk/src/inbound.ts
View File

@ -114,7 +114,6 @@ export async function handleNextcloudTalkInbound(params: {
const roomMatch = resolveNextcloudTalkRoomMatch({
rooms: account.config.rooms,
roomToken,
roomName,
});
const roomConfig = roomMatch.roomConfig;
if (isGroup && !roomMatch.allowed) {

10
extensions/nextcloud-talk/src/policy.ts
View File

@ -57,16 +57,10 @@ export type NextcloudTalkRoomMatch = {
export function resolveNextcloudTalkRoomMatch(params: {
rooms?: Record<string, NextcloudTalkRoomConfig>;
roomToken: string;
roomName?: string | null;
}): NextcloudTalkRoomMatch {
const rooms = params.rooms ?? {};
const allowlistConfigured = Object.keys(rooms).length > 0;
const roomName = params.roomName?.trim() || undefined;
const roomCandidates = buildChannelKeyCandidates(
params.roomToken,
roomName,
roomName ? normalizeChannelSlug(roomName) : undefined,
);
const roomCandidates = buildChannelKeyCandidates(params.roomToken);
const match = resolveChannelEntryMatchWithFallback({
entries: rooms,
keys: roomCandidates,
@ -101,11 +95,9 @@ export function resolveNextcloudTalkGroupToolPolicy(
if (!roomToken) {
return undefined;
}
const roomName = params.groupChannel?.trim() || undefined;
const match = resolveNextcloudTalkRoomMatch({
rooms: cfg.channels?.["nextcloud-talk"]?.rooms,
roomToken,
roomName,
});
return match.roomConfig?.tools ?? match.wildcardConfig?.tools;
}

7
extensions/twitch/src/access-control.test.ts
View File

@ -160,6 +160,13 @@ describe("checkTwitchAccessControl", () => {
});
});
it("blocks everyone when allowFrom is explicitly empty", () => {
expectAllowFromBlocked({
allowFrom: [],
reason: "allowFrom",
});
});
it("blocks messages without userId", () => {
expectAllowFromBlocked({
allowFrom: ["123456"],

8
extensions/twitch/src/access-control.ts
View File

@ -48,8 +48,14 @@ export function checkTwitchAccessControl(params: {
}
}
if (account.allowFrom && account.allowFrom.length > 0) {
if (account.allowFrom !== undefined) {
const allowFrom = account.allowFrom;
if (allowFrom.length === 0) {
return {
allowed: false,
reason: "sender is not in allowFrom allowlist",
};
}
const senderId = message.userId;
if (!senderId) {

2
src/config/types.googlechat.ts
View File

@ -75,6 +75,8 @@ export type GoogleChatAccountConfig = {
audienceType?: "app-url" | "project-number";
/** Audience value (app URL or project number). */
audience?: string;
/** Exact add-on principal to accept when app-url delivery uses add-on tokens. */
appPrincipal?: string;
/** Google Chat webhook path (default: /googlechat). */
webhookPath?: string;
/** Google Chat webhook URL (used to derive the path). */

1
src/config/zod-schema.providers-core.ts
View File

@ -767,6 +767,7 @@ export const GoogleChatAccountSchema = z
serviceAccountFile: z.string().optional(),
audienceType: z.enum(["app-url", "project-number"]).optional(),
audience: z.string().optional(),
appPrincipal: z.string().optional(),
webhookPath: z.string().optional(),
webhookUrl: z.string().optional(),
botUser: z.string().optional(),