From 20fbb8bd149bcc2177be9120137934a83473b390 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 30 May 2026 11:22:25 -0400 Subject: [PATCH] fix(mattermost): bound slash validation cache clocks --- .../src/mattermost/slash-http.test.ts | 113 ++++++++++++++++++ .../mattermost/src/mattermost/slash-http.ts | 46 ++++++- 2 files changed, 153 insertions(+), 6 deletions(-) diff --git a/extensions/mattermost/src/mattermost/slash-http.test.ts b/extensions/mattermost/src/mattermost/slash-http.test.ts index e98752d92f0b..358142908ca2 100644 --- a/extensions/mattermost/src/mattermost/slash-http.test.ts +++ b/extensions/mattermost/src/mattermost/slash-http.test.ts @@ -458,6 +458,119 @@ describe("slash-http", () => { expect(client.requests).toEqual(["/commands/cmd-1"]); }); + it("does not cache failed command validation when the expiry would exceed a valid Date", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date(8_640_000_000_000_000)); + try { + const registeredCommand = createRegisteredCommand({ token: "old-token" }); + const client = createCommandLookupClient({ + command: { + id: "cmd-1", + token: "new-token", + team_id: "t1", + trigger: "oc_status", + method: MATTERMOST_SLASH_POST_METHOD, + url: "https://gateway.example.com/slash", + auto_complete: true, + delete_at: 0, + }, + }); + const payload = { + token: "old-token", + team_id: "t1", + channel_id: "c1", + user_id: "u1", + command: "/oc_status", + text: "", + }; + + await expect( + validateMattermostSlashCommandToken({ + accountId: "default", + client, + registeredCommand, + payload, + }), + ).resolves.toBe(false); + await expect( + validateMattermostSlashCommandToken({ + accountId: "default", + client, + registeredCommand, + payload, + }), + ).resolves.toBe(false); + + expect(client.requests).toEqual(["/commands/cmd-1", "/commands/cmd-1"]); + } finally { + vi.useRealTimers(); + } + }); + + it("drops exhausted validation lookup buckets when the current clock is invalid", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-04-27T00:00:00Z")); + try { + const registeredCommand = createRegisteredCommand({ token: "valid-token" }); + const command = { + id: "cmd-1", + token: "valid-token", + team_id: "t1", + trigger: "oc_status", + method: MATTERMOST_SLASH_POST_METHOD, + url: "https://gateway.example.com/slash", + auto_complete: true, + delete_at: 0, + }; + const client = createCommandLookupClient({ command }); + const payload = { + token: "valid-token", + team_id: "t1", + channel_id: "c1", + user_id: "u1", + command: "/oc_status", + text: "", + }; + + for (let i = 0; i < 20; i += 1) { + await expect( + validateMattermostSlashCommandToken({ + accountId: "default", + client, + registeredCommand, + payload, + }), + ).resolves.toBe(true); + } + await expect( + validateMattermostSlashCommandToken({ + accountId: "default", + client, + registeredCommand, + payload, + }), + ).resolves.toBe(false); + + const dateNow = vi.spyOn(Date, "now").mockReturnValue(Number.NaN); + try { + await expect( + validateMattermostSlashCommandToken({ + accountId: "default", + client, + registeredCommand, + payload, + }), + ).resolves.toBe(true); + } finally { + dateNow.mockRestore(); + } + + expect(client.requests).toHaveLength(21); + } finally { + vi.useRealTimers(); + } + }); + it("scopes validation cache entries by account", async () => { const registeredCommand = createRegisteredCommand(); const clientA = createCommandLookupClient({ diff --git a/extensions/mattermost/src/mattermost/slash-http.ts b/extensions/mattermost/src/mattermost/slash-http.ts index 0979ccf2f699..4be8c756def9 100644 --- a/extensions/mattermost/src/mattermost/slash-http.ts +++ b/extensions/mattermost/src/mattermost/slash-http.ts @@ -6,6 +6,10 @@ */ import type { IncomingMessage, ServerResponse } from "node:http"; +import { + asDateTimestampMs, + resolveExpiresAtMsFromDurationMs, +} from "openclaw/plugin-sdk/number-runtime"; import { safeEqualSecret } from "openclaw/plugin-sdk/security-runtime"; import { isPrivateNetworkOptInEnabled } from "openclaw/plugin-sdk/ssrf-runtime"; import type { ResolvedMattermostAccount } from "../mattermost/accounts.js"; @@ -209,8 +213,14 @@ export function clearMattermostSlashCommandValidationCacheForAccount(accountId: } function sweepCommandValidationFailureCache(now = Date.now()): void { + const validNow = asDateTimestampMs(now); + if (validNow === undefined) { + commandValidationFailureCache.clear(); + return; + } for (const [key, entry] of commandValidationFailureCache) { - if (entry.expiresAt <= now) { + const expiresAt = asDateTimestampMs(entry.expiresAt); + if (expiresAt === undefined || expiresAt <= validNow) { commandValidationFailureCache.delete(key); } } @@ -225,11 +235,16 @@ function sweepCommandValidationFailureCache(now = Date.now()): void { function hasCachedCommandValidationFailure(key: string, now = Date.now()): boolean { sweepCommandValidationFailureCache(now); + const validNow = asDateTimestampMs(now); + if (validNow === undefined) { + return false; + } const cached = commandValidationFailureCache.get(key); if (!cached) { return false; } - if (cached.expiresAt > now) { + const expiresAt = asDateTimestampMs(cached.expiresAt); + if (expiresAt !== undefined && expiresAt > validNow) { return true; } commandValidationFailureCache.delete(key); @@ -237,17 +252,31 @@ function hasCachedCommandValidationFailure(key: string, now = Date.now()): boole } function cacheCommandValidationFailure(key: string, accountId: string): void { - sweepCommandValidationFailureCache(); + const now = Date.now(); + sweepCommandValidationFailureCache(now); + const expiresAt = resolveExpiresAtMsFromDurationMs(COMMAND_VALIDATION_FAILURE_CACHE_MS, { + nowMs: now, + }); + if (expiresAt === undefined) { + commandValidationFailureCache.delete(key); + return; + } commandValidationFailureCache.set(key, { accountId, - expiresAt: Date.now() + COMMAND_VALIDATION_FAILURE_CACHE_MS, + expiresAt, }); } function sweepCommandValidationLookupRateLimit(now = Date.now()): void { + const validNow = asDateTimestampMs(now); + if (validNow === undefined) { + commandValidationLookupRateLimit.clear(); + return; + } const staleAfterMs = COMMAND_VALIDATION_LOOKUP_REFILL_MS * COMMAND_VALIDATION_LOOKUP_BURST * 2; for (const [key, entry] of commandValidationLookupRateLimit) { - if (now - entry.updatedAt > staleAfterMs) { + const updatedAt = asDateTimestampMs(entry.updatedAt); + if (updatedAt === undefined || validNow - updatedAt > staleAfterMs) { commandValidationLookupRateLimit.delete(key); } } @@ -265,7 +294,12 @@ function reserveCommandValidationLookup(params: { accountId: string; now?: number; }): { allowed: true } | { allowed: false; shouldLog: boolean } { - const now = params.now ?? Date.now(); + const rawNow = params.now ?? Date.now(); + const now = asDateTimestampMs(rawNow); + if (now === undefined) { + commandValidationLookupRateLimit.clear(); + return { allowed: true }; + } sweepCommandValidationLookupRateLimit(now); const existing = commandValidationLookupRateLimit.get(params.key); if (!existing) {