mirror of https://github.com/openclaw/openclaw.git
fix(mattermost): scope interaction secrets and DM routing (openclaw#19957) thanks @tonydehnke
This commit is contained in:
parent
87faf4ee58
commit
e1dc606c85
|
|
@ -120,6 +120,7 @@ Docs: https://docs.openclaw.ai
|
|||
- LINE/context and routing synthesis: fix group/room peer routing and command-authorization context propagation, and keep processing later events in mixed-success webhook batches. (from #21955, #24475, #27035, #28286) Thanks @lailoo, @mcaxtr, @jervyclaw, @Glucksberg, and @Takhoffman.
|
||||
- LINE/status/config/webhook synthesis: fix status false positives from snapshot/config state and accept LINE webhook HEAD probes for compatibility. (from #10487, #25726, #27537, #27908, #31387) Thanks @BlueBirdBack, @stakeswky, @loiie45e, @puritysb, and @mcaxtr.
|
||||
- LINE cleanup/test follow-ups: fold cleanup/test learnings into the synthesis review path while keeping runtime changes focused on regression fixes. (from #17630, #17289) Thanks @Clawborn and @davidahmann.
|
||||
- Mattermost/interactive buttons hardening: scope interaction HMAC secrets per account, route DM button-click sessions by sender identity, and restore Mattermost send test mocks for channel-name resolution coverage. (#19957) thanks @tonydehnke.
|
||||
|
||||
## 2026.3.2
|
||||
|
||||
|
|
|
|||
|
|
@ -164,7 +164,7 @@ const mattermostMessageActions: ChannelMessageActionAdapter = {
|
|||
let props: Record<string, unknown> | undefined;
|
||||
if (params.buttons && Array.isArray(params.buttons)) {
|
||||
const account = resolveMattermostAccount({ cfg, accountId: resolvedAccountId });
|
||||
if (account.botToken) setInteractionSecret(account.botToken);
|
||||
if (account.botToken) setInteractionSecret(account.accountId, account.botToken);
|
||||
const callbackUrl = resolveInteractionCallbackUrl(account.accountId, cfg);
|
||||
|
||||
// Flatten 2D array (rows of buttons) to 1D — core schema sends Array<Array<Button>>
|
||||
|
|
@ -191,6 +191,7 @@ const mattermostMessageActions: ChannelMessageActionAdapter = {
|
|||
props = {
|
||||
attachments: buildButtonAttachments({
|
||||
callbackUrl,
|
||||
accountId: account.accountId,
|
||||
buttons,
|
||||
text: attachmentText,
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -103,6 +103,16 @@ describe("generateInteractionToken / verifyInteractionToken", () => {
|
|||
const reorderedContext = { action: "do", action_id: "bm_do", tweet_id: "999" };
|
||||
expect(verifyInteractionToken(reorderedContext, token)).toBe(true);
|
||||
});
|
||||
|
||||
it("scopes tokens per account when account secrets differ", () => {
|
||||
setInteractionSecret("acct-a", "bot-token-a");
|
||||
setInteractionSecret("acct-b", "bot-token-b");
|
||||
const context = { action_id: "do_now", item_id: "123" };
|
||||
const tokenA = generateInteractionToken(context, "acct-a");
|
||||
|
||||
expect(verifyInteractionToken(context, tokenA, "acct-a")).toBe(true);
|
||||
expect(verifyInteractionToken(context, tokenA, "acct-b")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Callback URL registry ────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -63,32 +63,58 @@ export function resolveInteractionCallbackUrl(
|
|||
// ── HMAC token management ──────────────────────────────────────────────
|
||||
// Secret is derived from the bot token so it's stable across CLI and gateway processes.
|
||||
|
||||
let interactionSecret: string | undefined;
|
||||
const interactionSecrets = new Map<string, string>();
|
||||
let defaultInteractionSecret: string | undefined;
|
||||
|
||||
export function setInteractionSecret(botToken: string): void {
|
||||
interactionSecret = createHmac("sha256", "openclaw-mattermost-interactions")
|
||||
.update(botToken)
|
||||
.digest("hex");
|
||||
function deriveInteractionSecret(botToken: string): string {
|
||||
return createHmac("sha256", "openclaw-mattermost-interactions").update(botToken).digest("hex");
|
||||
}
|
||||
|
||||
export function getInteractionSecret(): string {
|
||||
if (!interactionSecret) {
|
||||
throw new Error(
|
||||
"Interaction secret not initialized — call setInteractionSecret(botToken) first",
|
||||
);
|
||||
export function setInteractionSecret(accountIdOrBotToken: string, botToken?: string): void {
|
||||
if (typeof botToken === "string") {
|
||||
interactionSecrets.set(accountIdOrBotToken, deriveInteractionSecret(botToken));
|
||||
return;
|
||||
}
|
||||
return interactionSecret;
|
||||
// Backward-compatible fallback for call sites/tests that only pass botToken.
|
||||
defaultInteractionSecret = deriveInteractionSecret(accountIdOrBotToken);
|
||||
}
|
||||
|
||||
export function generateInteractionToken(context: Record<string, unknown>): string {
|
||||
const secret = getInteractionSecret();
|
||||
export function getInteractionSecret(accountId?: string): string {
|
||||
const scoped = accountId ? interactionSecrets.get(accountId) : undefined;
|
||||
if (scoped) {
|
||||
return scoped;
|
||||
}
|
||||
if (defaultInteractionSecret) {
|
||||
return defaultInteractionSecret;
|
||||
}
|
||||
// Fallback for single-account runtimes that only registered scoped secrets.
|
||||
if (interactionSecrets.size === 1) {
|
||||
const first = interactionSecrets.values().next().value;
|
||||
if (typeof first === "string") {
|
||||
return first;
|
||||
}
|
||||
}
|
||||
throw new Error(
|
||||
"Interaction secret not initialized — call setInteractionSecret(accountId, botToken) first",
|
||||
);
|
||||
}
|
||||
|
||||
export function generateInteractionToken(
|
||||
context: Record<string, unknown>,
|
||||
accountId?: string,
|
||||
): string {
|
||||
const secret = getInteractionSecret(accountId);
|
||||
// Sort keys for stable serialization — Mattermost may reorder context keys
|
||||
const payload = JSON.stringify(context, Object.keys(context).sort());
|
||||
return createHmac("sha256", secret).update(payload).digest("hex");
|
||||
}
|
||||
|
||||
export function verifyInteractionToken(context: Record<string, unknown>, token: string): boolean {
|
||||
const expected = generateInteractionToken(context);
|
||||
export function verifyInteractionToken(
|
||||
context: Record<string, unknown>,
|
||||
token: string,
|
||||
accountId?: string,
|
||||
): boolean {
|
||||
const expected = generateInteractionToken(context, accountId);
|
||||
if (expected.length !== token.length) {
|
||||
return false;
|
||||
}
|
||||
|
|
@ -133,6 +159,7 @@ function sanitizeActionId(id: string): string {
|
|||
|
||||
export function buildButtonAttachments(params: {
|
||||
callbackUrl: string;
|
||||
accountId?: string;
|
||||
buttons: Array<{
|
||||
id: string;
|
||||
name: string;
|
||||
|
|
@ -147,7 +174,7 @@ export function buildButtonAttachments(params: {
|
|||
action_id: safeId,
|
||||
...btn.context,
|
||||
};
|
||||
const token = generateInteractionToken(context);
|
||||
const token = generateInteractionToken(context, params.accountId);
|
||||
return {
|
||||
id: safeId,
|
||||
type: "button" as const,
|
||||
|
|
@ -225,7 +252,7 @@ export function createMattermostInteractionHandler(params: {
|
|||
botUserId: string;
|
||||
accountId: string;
|
||||
callbackUrl: string;
|
||||
resolveSessionKey?: (channelId: string) => Promise<string>;
|
||||
resolveSessionKey?: (channelId: string, userId: string) => Promise<string>;
|
||||
dispatchButtonClick?: (opts: {
|
||||
channelId: string;
|
||||
userId: string;
|
||||
|
|
@ -236,7 +263,7 @@ export function createMattermostInteractionHandler(params: {
|
|||
}) => Promise<void>;
|
||||
log?: (message: string) => void;
|
||||
}): (req: IncomingMessage, res: ServerResponse) => Promise<void> {
|
||||
const { client, botUserId, accountId, log } = params;
|
||||
const { client, accountId, log } = params;
|
||||
const core = getMattermostRuntime();
|
||||
|
||||
return async (req: IncomingMessage, res: ServerResponse) => {
|
||||
|
|
@ -292,7 +319,7 @@ export function createMattermostInteractionHandler(params: {
|
|||
|
||||
// Strip _token before verification (it wasn't in the original context)
|
||||
const { _token, ...contextWithoutToken } = context;
|
||||
if (!verifyInteractionToken(contextWithoutToken, token)) {
|
||||
if (!verifyInteractionToken(contextWithoutToken, token, accountId)) {
|
||||
log?.("mattermost interaction: invalid _token");
|
||||
res.statusCode = 403;
|
||||
res.setHeader("Content-Type", "application/json");
|
||||
|
|
@ -323,7 +350,7 @@ export function createMattermostInteractionHandler(params: {
|
|||
`in channel ${payload.channel_id}`;
|
||||
|
||||
const sessionKey = params.resolveSessionKey
|
||||
? await params.resolveSessionKey(payload.channel_id)
|
||||
? await params.resolveSessionKey(payload.channel_id, payload.user_id)
|
||||
: `agent:main:mattermost:${accountId}:${payload.channel_id}`;
|
||||
|
||||
core.system.enqueueSystemEvent(eventLabel, {
|
||||
|
|
|
|||
|
|
@ -448,7 +448,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
|
|||
|
||||
// ─── Interactive buttons registration ──────────────────────────────────────
|
||||
// Derive a stable HMAC secret from the bot token so CLI and gateway share it.
|
||||
setInteractionSecret(botToken);
|
||||
setInteractionSecret(account.accountId, botToken);
|
||||
|
||||
// Register HTTP callback endpoint for interactive button clicks.
|
||||
// Mattermost POSTs to this URL when a user clicks a button action.
|
||||
|
|
@ -465,7 +465,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
|
|||
botUserId,
|
||||
accountId: account.accountId,
|
||||
callbackUrl,
|
||||
resolveSessionKey: async (channelId: string) => {
|
||||
resolveSessionKey: async (channelId: string, userId: string) => {
|
||||
const channelInfo = await resolveChannelInfo(channelId);
|
||||
const kind = mapMattermostChannelTypeToChatType(channelInfo?.type);
|
||||
const teamId = channelInfo?.team_id ?? undefined;
|
||||
|
|
@ -476,7 +476,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
|
|||
teamId,
|
||||
peer: {
|
||||
kind,
|
||||
id: kind === "direct" ? botUserId : channelId,
|
||||
id: kind === "direct" ? userId : channelId,
|
||||
},
|
||||
});
|
||||
return route.sessionKey;
|
||||
|
|
@ -495,7 +495,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
|
|||
teamId,
|
||||
peer: {
|
||||
kind,
|
||||
id: kind === "direct" ? botUserId : opts.channelId,
|
||||
id: kind === "direct" ? opts.userId : opts.channelId,
|
||||
},
|
||||
});
|
||||
const to = kind === "direct" ? `user:${opts.userId}` : `channel:${opts.channelId}`;
|
||||
|
|
|
|||
|
|
@ -12,7 +12,9 @@ const mockState = vi.hoisted(() => ({
|
|||
createMattermostClient: vi.fn(),
|
||||
createMattermostDirectChannel: vi.fn(),
|
||||
createMattermostPost: vi.fn(),
|
||||
fetchMattermostChannelByName: vi.fn(),
|
||||
fetchMattermostMe: vi.fn(),
|
||||
fetchMattermostUserTeams: vi.fn(),
|
||||
fetchMattermostUserByUsername: vi.fn(),
|
||||
normalizeMattermostBaseUrl: vi.fn((input: string | undefined) => input?.trim() ?? ""),
|
||||
uploadMattermostFile: vi.fn(),
|
||||
|
|
@ -30,7 +32,9 @@ vi.mock("./client.js", () => ({
|
|||
createMattermostClient: mockState.createMattermostClient,
|
||||
createMattermostDirectChannel: mockState.createMattermostDirectChannel,
|
||||
createMattermostPost: mockState.createMattermostPost,
|
||||
fetchMattermostChannelByName: mockState.fetchMattermostChannelByName,
|
||||
fetchMattermostMe: mockState.fetchMattermostMe,
|
||||
fetchMattermostUserTeams: mockState.fetchMattermostUserTeams,
|
||||
fetchMattermostUserByUsername: mockState.fetchMattermostUserByUsername,
|
||||
normalizeMattermostBaseUrl: mockState.normalizeMattermostBaseUrl,
|
||||
uploadMattermostFile: mockState.uploadMattermostFile,
|
||||
|
|
@ -71,11 +75,16 @@ describe("sendMessageMattermost", () => {
|
|||
mockState.createMattermostClient.mockReset();
|
||||
mockState.createMattermostDirectChannel.mockReset();
|
||||
mockState.createMattermostPost.mockReset();
|
||||
mockState.fetchMattermostChannelByName.mockReset();
|
||||
mockState.fetchMattermostMe.mockReset();
|
||||
mockState.fetchMattermostUserTeams.mockReset();
|
||||
mockState.fetchMattermostUserByUsername.mockReset();
|
||||
mockState.uploadMattermostFile.mockReset();
|
||||
mockState.createMattermostClient.mockReturnValue({});
|
||||
mockState.createMattermostPost.mockResolvedValue({ id: "post-1" });
|
||||
mockState.fetchMattermostMe.mockResolvedValue({ id: "bot-user" });
|
||||
mockState.fetchMattermostUserTeams.mockResolvedValue([{ id: "team-1" }]);
|
||||
mockState.fetchMattermostChannelByName.mockResolvedValue({ id: "town-square" });
|
||||
mockState.uploadMattermostFile.mockResolvedValue({ id: "file-1" });
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue