From ae8d4a8eec112934a7cdc47a004ccf8bbe3bf2b6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 10:09:55 +0100 Subject: [PATCH] fix(security): harden channel token and id generation --- CHANGELOG.md | 1 + extensions/tlon/src/urbit/channel-client.ts | 3 ++- extensions/tlon/src/urbit/sse-client.ts | 5 ++-- src/auto-reply/reply/agent-runner.ts | 4 +-- src/infra/outbound/delivery-queue.ts | 4 +-- src/infra/secure-random.test.ts | 20 ++++++++++++++ src/infra/secure-random.ts | 9 +++++++ src/slack/monitor/slash.test.ts | 29 ++++++++++++++++++--- src/slack/monitor/slash.ts | 20 +++++++++++--- 9 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 src/infra/secure-random.test.ts create mode 100644 src/infra/secure-random.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b3d4965e3..f3c25c63d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Docs: https://docs.openclaw.ai - Security/Gateway: emit a startup security warning when insecure/dangerous config flags are enabled (including `gateway.controlUi.dangerouslyDisableDeviceAuth=true`) and point operators to `openclaw security audit`. - Security/Hooks auth: normalize hook auth rate-limit client IP keys so IPv4 and IPv4-mapped IPv6 addresses share one throttle bucket, preventing dual-form auth-attempt budget bypasses. This ships in the next npm release. Thanks @aether-ai-agent for reporting. - Security/Exec approvals: treat `env` and shell-dispatch wrappers as transparent during allowlist analysis on node-host and macOS companion paths so policy checks match the effective executable/inline shell payload instead of the wrapper binary, blocking wrapper-smuggled allowlist bypasses. This ships in the next npm release. Thanks @tdjackey for reporting. +- Security/Channels: harden Slack external menu token handling by switching to CSPRNG tokens, validating token shape, requiring user identity for external option lookups, and avoiding fabricated timestamp `trigger_id` fallbacks; also switch Tlon Urbit channel IDs to CSPRNG UUIDs and centralize secure ID/token generation via shared infra helpers. - Telegram/WSL2: disable `autoSelectFamily` by default on WSL2 and memoize WSL2 detection in Telegram network decision logic to avoid repeated sync `/proc/version` probes on fetch/send paths. (#21916) Thanks @MizukiMachine. - Telegram/Streaming: preserve archived draft preview mapping after flush and clean superseded reasoning preview bubbles so multi-message preview finals no longer cross-edit or orphan stale messages under send/rotation races. (#23202) Thanks @obviyus. - Slack/Slash commands: preserve the Bolt app receiver when registering external select options handlers so monitor startup does not crash on runtimes that require bound `app.options` calls. (#23209) Thanks @0xgaia. diff --git a/extensions/tlon/src/urbit/channel-client.ts b/extensions/tlon/src/urbit/channel-client.ts index fb8af656a..499860075 100644 --- a/extensions/tlon/src/urbit/channel-client.ts +++ b/extensions/tlon/src/urbit/channel-client.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import type { LookupFn, SsrFPolicy } from "openclaw/plugin-sdk"; import { ensureUrbitChannelOpen, pokeUrbitChannel, scryUrbitPath } from "./channel-ops.js"; import { getUrbitContext, normalizeUrbitCookie } from "./context.js"; @@ -43,7 +44,7 @@ export class UrbitChannelClient { return; } - const channelId = `${Math.floor(Date.now() / 1000)}-${Math.random().toString(36).substring(2, 8)}`; + const channelId = `${Math.floor(Date.now() / 1000)}-${randomUUID()}`; this.channelId = channelId; try { diff --git a/extensions/tlon/src/urbit/sse-client.ts b/extensions/tlon/src/urbit/sse-client.ts index b75d43f77..df128e51b 100644 --- a/extensions/tlon/src/urbit/sse-client.ts +++ b/extensions/tlon/src/urbit/sse-client.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import { Readable } from "node:stream"; import type { LookupFn, SsrFPolicy } from "openclaw/plugin-sdk"; import { ensureUrbitChannelOpen, pokeUrbitChannel, scryUrbitPath } from "./channel-ops.js"; @@ -59,7 +60,7 @@ export class UrbitSSEClient { this.url = ctx.baseUrl; this.cookie = normalizeUrbitCookie(cookie); this.ship = ctx.ship; - this.channelId = `${Math.floor(Date.now() / 1000)}-${Math.random().toString(36).substring(2, 8)}`; + this.channelId = `${Math.floor(Date.now() / 1000)}-${randomUUID()}`; this.channelUrl = new URL(`/~/channel/${this.channelId}`, this.url).toString(); this.onReconnect = options.onReconnect ?? null; this.autoReconnect = options.autoReconnect !== false; @@ -343,7 +344,7 @@ export class UrbitSSEClient { await new Promise((resolve) => setTimeout(resolve, delay)); try { - this.channelId = `${Math.floor(Date.now() / 1000)}-${Math.random().toString(36).substring(2, 8)}`; + this.channelId = `${Math.floor(Date.now() / 1000)}-${randomUUID()}`; this.channelUrl = new URL(`/~/channel/${this.channelId}`, this.url).toString(); if (this.onReconnect) { diff --git a/src/auto-reply/reply/agent-runner.ts b/src/auto-reply/reply/agent-runner.ts index e11017092..4fe94914f 100644 --- a/src/auto-reply/reply/agent-runner.ts +++ b/src/auto-reply/reply/agent-runner.ts @@ -1,4 +1,3 @@ -import crypto from "node:crypto"; import fs from "node:fs"; import { lookupContextTokens } from "../../agents/context.js"; import { DEFAULT_CONTEXT_TOKENS } from "../../agents/defaults.js"; @@ -17,6 +16,7 @@ import { import type { TypingMode } from "../../config/types.js"; import { emitAgentEvent } from "../../infra/agent-events.js"; import { emitDiagnosticEvent, isDiagnosticsEnabled } from "../../infra/diagnostic-events.js"; +import { generateSecureUuid } from "../../infra/secure-random.js"; import { enqueueSystemEvent } from "../../infra/system-events.js"; import { defaultRuntime } from "../../runtime.js"; import { estimateUsageCost, resolveModelCostConfig } from "../../utils/usage-format.js"; @@ -289,7 +289,7 @@ export async function runReplyAgent(params: { return false; } const prevSessionId = cleanupTranscripts ? prevEntry.sessionId : undefined; - const nextSessionId = crypto.randomUUID(); + const nextSessionId = generateSecureUuid(); const nextEntry: SessionEntry = { ...prevEntry, sessionId: nextSessionId, diff --git a/src/infra/outbound/delivery-queue.ts b/src/infra/outbound/delivery-queue.ts index 331875da4..d5bba175e 100644 --- a/src/infra/outbound/delivery-queue.ts +++ b/src/infra/outbound/delivery-queue.ts @@ -1,9 +1,9 @@ -import crypto from "node:crypto"; import fs from "node:fs"; import path from "node:path"; import type { ReplyPayload } from "../../auto-reply/types.js"; import type { OpenClawConfig } from "../../config/config.js"; import { resolveStateDir } from "../../config/paths.js"; +import { generateSecureUuid } from "../secure-random.js"; import type { OutboundChannel } from "./targets.js"; const QUEUE_DIRNAME = "delivery-queue"; @@ -83,7 +83,7 @@ export async function enqueueDelivery( stateDir?: string, ): Promise { const queueDir = await ensureQueueDir(stateDir); - const id = crypto.randomUUID(); + const id = generateSecureUuid(); const entry: QueuedDelivery = { id, enqueuedAt: Date.now(), diff --git a/src/infra/secure-random.test.ts b/src/infra/secure-random.test.ts new file mode 100644 index 000000000..96f08252d --- /dev/null +++ b/src/infra/secure-random.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from "vitest"; +import { generateSecureToken, generateSecureUuid } from "./secure-random.js"; + +describe("secure-random", () => { + it("generates UUIDs", () => { + const first = generateSecureUuid(); + const second = generateSecureUuid(); + expect(first).not.toBe(second); + expect(first).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i, + ); + }); + + it("generates url-safe tokens", () => { + const defaultToken = generateSecureToken(); + const token18 = generateSecureToken(18); + expect(defaultToken).toMatch(/^[A-Za-z0-9_-]+$/); + expect(token18).toMatch(/^[A-Za-z0-9_-]{24}$/); + }); +}); diff --git a/src/infra/secure-random.ts b/src/infra/secure-random.ts new file mode 100644 index 000000000..05c961e50 --- /dev/null +++ b/src/infra/secure-random.ts @@ -0,0 +1,9 @@ +import { randomBytes, randomUUID } from "node:crypto"; + +export function generateSecureUuid(): string { + return randomUUID(); +} + +export function generateSecureToken(bytes = 16): string { + return randomBytes(bytes).toString("base64url"); +} diff --git a/src/slack/monitor/slash.test.ts b/src/slack/monitor/slash.test.ts index bbfe59e66..c1e396028 100644 --- a/src/slack/monitor/slash.test.ts +++ b/src/slack/monitor/slash.test.ts @@ -504,9 +504,10 @@ describe("Slack native command argument menus", () => { const element = actions?.elements?.[0]; expect(element?.type).toBe("external_select"); expect(element?.action_id).toBe("openclaw_cmdarg"); - expect(payload.blocks?.find((block) => block.type === "actions")?.block_id).toContain( - "openclaw_cmdarg_ext:", - ); + const blockId = payload.blocks?.find((block) => block.type === "actions")?.block_id; + expect(blockId).toContain("openclaw_cmdarg_ext:"); + const token = (blockId ?? "").slice("openclaw_cmdarg_ext:".length); + expect(token).toMatch(/^[A-Za-z0-9_-]{24}$/); }); it("serves filtered options for external_select menus", async () => { @@ -536,6 +537,28 @@ describe("Slack native command argument menus", () => { expect(optionTexts.some((text) => text.includes("Period 12"))).toBe(true); }); + it("rejects external_select option requests without user identity", async () => { + const { respond } = await runCommandHandler(reportExternalHandler); + + const payload = respond.mock.calls[0]?.[0] as { + blocks?: Array<{ type: string; block_id?: string }>; + }; + const blockId = payload.blocks?.find((block) => block.type === "actions")?.block_id; + expect(blockId).toContain("openclaw_cmdarg_ext:"); + + const ackOptions = vi.fn().mockResolvedValue(undefined); + await argMenuOptionsHandler({ + ack: ackOptions, + body: { + value: "period 1", + actions: [{ block_id: blockId }], + }, + }); + + expect(ackOptions).toHaveBeenCalledTimes(1); + expect(ackOptions).toHaveBeenCalledWith({ options: [] }); + }); + it("rejects menu clicks from other users", async () => { const respond = await runArgMenuAction(argMenuHandler, { action: { diff --git a/src/slack/monitor/slash.ts b/src/slack/monitor/slash.ts index 4b98b0bbc..e188f3bd8 100644 --- a/src/slack/monitor/slash.ts +++ b/src/slack/monitor/slash.ts @@ -5,6 +5,7 @@ import { formatAllowlistMatchMeta } from "../../channels/allowlist-match.js"; import { resolveCommandAuthorizedFromAuthorizers } from "../../channels/command-gating.js"; import { resolveNativeCommandsEnabled, resolveNativeSkillsEnabled } from "../../config/commands.js"; import { danger, logVerbose } from "../../globals.js"; +import { generateSecureToken } from "../../infra/secure-random.js"; import { buildPairingReply } from "../../pairing/pairing-messages.js"; import { readChannelAllowFromStore, @@ -37,6 +38,7 @@ const SLACK_COMMAND_ARG_SELECT_OPTIONS_MAX = 100; const SLACK_COMMAND_ARG_SELECT_OPTION_VALUE_MAX = 75; const SLACK_COMMAND_ARG_EXTERNAL_PREFIX = "openclaw_cmdarg_ext:"; const SLACK_COMMAND_ARG_EXTERNAL_TTL_MS = 10 * 60 * 1000; +const SLACK_COMMAND_ARG_EXTERNAL_TOKEN_PATTERN = /^[A-Za-z0-9_-]{24}$/; const SLACK_HEADER_TEXT_MAX = 150; type EncodedMenuChoice = { label: string; value: string }; @@ -78,12 +80,21 @@ function pruneSlackExternalArgMenuStore(now = Date.now()) { } } +function createSlackExternalArgMenuToken(): string { + // 18 bytes -> 24 base64url chars; loop avoids replacing an existing live token. + let token = ""; + do { + token = generateSecureToken(18); + } while (slackExternalArgMenuStore.has(token)); + return token; +} + function storeSlackExternalArgMenu(params: { choices: EncodedMenuChoice[]; userId: string; }): string { pruneSlackExternalArgMenuStore(); - const token = `${Date.now().toString(36)}${Math.random().toString(36).slice(2, 10)}`; + const token = createSlackExternalArgMenuToken(); slackExternalArgMenuStore.set(token, { choices: params.choices, userId: params.userId, @@ -97,7 +108,7 @@ function readSlackExternalArgMenuToken(raw: unknown): string | undefined { return undefined; } const token = raw.slice(SLACK_COMMAND_ARG_EXTERNAL_PREFIX.length).trim(); - return token.length > 0 ? token : undefined; + return SLACK_COMMAND_ARG_EXTERNAL_TOKEN_PATTERN.test(token) ? token : undefined; } type CommandsRegistry = typeof import("../../auto-reply/commands-registry.js"); @@ -783,7 +794,8 @@ export async function registerSlackMonitorSlashCommands(params: { await ack({ options: [] }); return; } - if (typedBody.user?.id && typedBody.user.id !== entry.userId) { + const requesterUserId = typedBody.user?.id?.trim(); + if (!requesterUserId || requesterUserId !== entry.userId) { await ack({ options: [] }); return; } @@ -860,7 +872,7 @@ export async function registerSlackMonitorSlashCommands(params: { user_name: userName, channel_id: body.channel?.id ?? "", channel_name: body.channel?.name ?? body.channel?.id ?? "", - trigger_id: triggerId ?? String(Date.now()), + trigger_id: triggerId, } as SlackCommandMiddlewareArgs["command"]; await handleSlashCommand({ command: commandPayload,