diff --git a/cloud/apps/connector/src/connector-database.ts b/cloud/apps/connector/src/connector-database.ts index 1ee811ece8c..7b2f31c9a64 100644 --- a/cloud/apps/connector/src/connector-database.ts +++ b/cloud/apps/connector/src/connector-database.ts @@ -4,17 +4,20 @@ import { type CloudDatabase, type OpenCloudDatabaseInput } from '@orca-cloud/cloud-database' +import { connectorSchemaChanges } from './connector-schema-changes.js' import { connectorSchemaStatements } from './connector-schema.js' import { discordSchemaStatements } from './discord-schema.js' import { slackSchemaStatements } from './slack-schema.js' // Production has run these CREATE TABLEs, so editing one never reaches it. Change the schema by -// appending idempotent statements (ADD COLUMN IF NOT EXISTS, nullable or defaulted) after them; -// connector-schema-upgrade.test.ts boots the shipped baseline, then this list. +// appending idempotent statements (ADD COLUMN IF NOT EXISTS, nullable or defaulted) to +// connector-schema-changes.ts; connector-schema-upgrade.test.ts boots the shipped baseline, then +// this list. export const connectorBootStatements = () => [ ...connectorSchemaStatements(), ...slackSchemaStatements(), - ...discordSchemaStatements() + ...discordSchemaStatements(), + ...connectorSchemaChanges() ] export async function openConnectorDatabase( diff --git a/cloud/apps/connector/src/connector-posts.ts b/cloud/apps/connector/src/connector-posts.ts index b1e2a09276d..a29b2b816a5 100644 --- a/cloud/apps/connector/src/connector-posts.ts +++ b/cloud/apps/connector/src/connector-posts.ts @@ -155,7 +155,8 @@ export class ConnectorPosts { `SELECT * FROM connector_posts p WHERE p.source = ? AND p.state IN ('queued', 'sending') AND p.next_at <= ? AND NOT EXISTS (SELECT 1 FROM connector_posts ahead - WHERE ahead.source = p.source AND ahead.container_id = p.container_id + WHERE ahead.source = p.source AND ahead.workspace_id = p.workspace_id + AND ahead.container_id = p.container_id AND ahead.state IN ('queued', 'sending') AND (ahead.queued_at < p.queued_at OR (ahead.queued_at = p.queued_at AND ahead.post_id < p.post_id))) diff --git a/cloud/apps/connector/src/connector-schema-changes.ts b/cloud/apps/connector/src/connector-schema-changes.ts new file mode 100644 index 00000000000..54853622a1b --- /dev/null +++ b/cloud/apps/connector/src/connector-schema-changes.ts @@ -0,0 +1,27 @@ +// Changes since the shipped schema, applied after it on every boot, in order. Each must be +// idempotent and work for the revision still serving during a roll; never edit one that shipped. +// Split on semicolons, so none in comments. +const CONNECTOR_SCHEMA_CHANGES = ` +CREATE TABLE IF NOT EXISTS connector_slack_apps ( + app_id TEXT PRIMARY KEY, + client_id TEXT NOT NULL, + client_secret TEXT NOT NULL, + signing_secret TEXT NOT NULL, + created_at BIGINT NOT NULL +); +-- NULL for a workspace installed before apps were rows: only the legacy env-secret route reaches +-- it, until the operator binds it to its app. +ALTER TABLE connector_slack_installs ADD COLUMN IF NOT EXISTS app_id TEXT; +CREATE INDEX IF NOT EXISTS connector_slack_installs_app ON connector_slack_installs(app_id); + +-- Channel order is per workspace: the same channel id in another workspace is another channel. +-- The shipped connector_posts_container stays: the shipped list recreates it on every boot. +CREATE INDEX IF NOT EXISTS connector_posts_channel_head + ON connector_posts(source, workspace_id, container_id, state, queued_at); +` + +export function connectorSchemaChanges(): string[] { + return CONNECTOR_SCHEMA_CHANGES.split(';') + .map((statement) => statement.trim()) + .filter((statement) => statement.length > 0) +} diff --git a/cloud/apps/connector/src/connector-server.ts b/cloud/apps/connector/src/connector-server.ts index e78d33fe1ab..01cd631e81c 100644 --- a/cloud/apps/connector/src/connector-server.ts +++ b/cloud/apps/connector/src/connector-server.ts @@ -18,6 +18,8 @@ import type { DiscordRestApi } from './discord-rest.js' const UNAUTHENTICATED_PER_MINUTE_PER_IP = 30 const AUTHENTICATED_PER_MINUTE_PER_IP = 6_000 const AUTHENTICATED_PER_MINUTE_PER_HOST = 600 +// Before the signature is checked; Slack sends every workspace's traffic from shared addresses. +const SLACK_PER_MINUTE_PER_IP = 6_000 export type ConnectorServerOptions = { now?: () => number @@ -113,7 +115,11 @@ export function createConnectorServer( limitBody, core }) - slack?.registerRoutes(app, core) + slack?.registerRoutes( + app, + core, + limitByClientIp(new RateLimiter(SLACK_PER_MINUTE_PER_IP, now), config.trustedProxyHops) + ) discord?.registerRoutes(app, core) // Local development only: a stand-in for Slack ingress until it exists. if (isLoopbackOrigin(config.publicUrl)) { diff --git a/cloud/apps/connector/src/slack-apps.ts b/cloud/apps/connector/src/slack-apps.ts new file mode 100644 index 00000000000..43127198433 --- /dev/null +++ b/cloud/apps/connector/src/slack-apps.ts @@ -0,0 +1,56 @@ +import type { CloudDatabase } from '@orca-cloud/cloud-database' +import { openSecret, sealSecret } from './slack-sealing.js' + +export type SlackAppCredentials = { + appId: string + clientId: string + clientSecret: string + signingSecret: string +} + +// Bot tokens are sealed under their bare team id (the shipped format), so a purpose prefix keeps +// these from ever matching one. +export const appSecretAad = (secret: 'client' | 'signing', appId: string) => + `slack-${secret}-secret\0${appId}` + +// One row per Slack app Orca serves: a company's own copy, or one registered by an operator. Its +// signing secret verifies the requests Slack sends to that app's own URLs. +export class SlackApps { + constructor(private readonly tokenKey: Buffer) {} + + // Null for an unknown app. A row that won't open (another key, corrupted) throws: that is an + // operator fault, not a bad request. + async signingSecret(database: CloudDatabase, appId: string): Promise { + const [row] = await database.query( + 'SELECT signing_secret FROM connector_slack_apps WHERE app_id = ?', + [appId] + ) + return row + ? openSecret(this.tokenKey, appSecretAad('signing', appId), String(row.signing_secret)) + : null + } + + async exists(database: CloudDatabase, appId: string): Promise { + const [row] = await database.query('SELECT 1 AS found FROM connector_slack_apps WHERE app_id = ?', [ + appId + ]) + return row !== undefined + } + + // False when the app is already registered: replacing an app's secrets is its own operation, + // never a side effect of registering again. + async register(database: CloudDatabase, app: SlackAppCredentials, now: number): Promise { + const [result] = await database.query( + `INSERT INTO connector_slack_apps (app_id, client_id, client_secret, signing_secret, created_at) + VALUES (?, ?, ?, ?, ?) ON CONFLICT (app_id) DO NOTHING`, + [ + app.appId, + app.clientId, + sealSecret(this.tokenKey, appSecretAad('client', app.appId), app.clientSecret), + sealSecret(this.tokenKey, appSecretAad('signing', app.appId), app.signingSecret), + now + ] + ) + return Number(result?.changes ?? 0) > 0 + } +} diff --git a/cloud/apps/connector/src/slack-clicks.test.ts b/cloud/apps/connector/src/slack-clicks.test.ts index f2c2560c222..17f781fd063 100644 --- a/cloud/apps/connector/src/slack-clicks.test.ts +++ b/cloud/apps/connector/src/slack-clicks.test.ts @@ -1,13 +1,18 @@ +import { createHmac } from 'node:crypto' import { ConnectorPollResponseSchema } from '@orca-cloud/connector-contract' import { afterEach, describe, expect, it, vi } from 'vitest' import { cleanupConnectorHarnesses } from './connector-server-harness.test-fixture.js' import { answerDeliveryId } from './owner-controls.js' +import { SlackApps } from './slack-apps.js' +import { SlackInstalls } from './slack-installs.js' import { + APP, CHANNEL, conversation, EXTERNAL_ID, OWNER, startedThread, + TEAM, TEAMMATE, turnEnded, turnStarted @@ -170,12 +175,59 @@ describe('Slack prompt cards', () => { } }) + it('takes a press only from the app installed in the card’s workspace', async () => { + const harness = await asked() + const now = harness.now() + await new SlackApps(harness.tokenKey).register( + harness.database, + { appId: 'A2', clientId: '3.4', clientSecret: 'cs-2', signingSecret: 'secret-2' }, + now + ) + await new SlackInstalls(harness.tokenKey).save( + harness.database, + { teamId: 'T2', appId: 'A2', botUserId: 'UBOT2', botToken: 'xoxb-2' }, + now + ) + const { ts } = await harness.card() + // App 2's owners press the live card in app 1's workspace, naming either app. + for (const appId of ['A2', APP]) { + const body = new URLSearchParams({ + payload: JSON.stringify({ + type: 'block_actions', + api_app_id: appId, + team: { id: TEAM }, + user: { id: OWNER }, + channel: { id: CHANNEL }, + container: { message_ts: ts }, + actions: [{ action_id: 'answer:0', value: 'allow_once' }] + }) + }).toString() + const timestamp = String(Math.floor(now / 1000)) + const response = await harness.request('/slack/apps/A2/interactivity', { + method: 'POST', + headers: { + 'content-type': 'application/x-www-form-urlencoded', + 'x-slack-request-timestamp': timestamp, + 'x-slack-signature': `v0=${createHmac('sha256', 'secret-2').update(`v0:${timestamp}:${body}`).digest('hex')}` + }, + body + }) + expect(response.status).toBe(200) + } + expect(await harness.deliveries()).toEqual([]) + expect((await harness.card()).blocks).toHaveLength(2) + + // App 1's own press still answers it. + await harness.click({ messageTs: ts, value: 'allow_once' }) + expect(await harness.deliveries()).toEqual([expect.objectContaining({ kind: 'answer' })]) + }) + it('ignores a click on a message that isn’t a card, and refuses an unsigned one', async () => { const harness = await asked() expect( (await harness.click({ messageTs: '1800000999.000001', value: 'allow_once' })).status ).toBe(200) - const response = await harness.request('/slack/interactivity', { + const response = await harness.request(`/slack/apps/${APP}/interactivity`, { method: 'POST', headers: { 'content-type': 'application/x-www-form-urlencoded' }, body: 'payload=%7B%7D' diff --git a/cloud/apps/connector/src/slack-clicks.ts b/cloud/apps/connector/src/slack-clicks.ts index 7bc0222d4a3..4146bca6995 100644 --- a/cloud/apps/connector/src/slack-clicks.ts +++ b/cloud/apps/connector/src/slack-clicks.ts @@ -2,6 +2,7 @@ import { z } from 'zod' const BlockActionsSchema = z.looseObject({ type: z.literal('block_actions'), + api_app_id: z.string().min(1), team: z.looseObject({ id: z.string().min(1) }), user: z.looseObject({ id: z.string().min(1) }), channel: z.looseObject({ id: z.string().min(1) }), @@ -14,6 +15,8 @@ const BlockActionsSchema = z.looseObject({ // A press of an answer button on a prompt card. What it answers is found from the message it is // on; the button only says which option. export type SlackClick = { + // The app Slack delivered it for; must be the app whose URL received it. + appId: string teamId: string channelId: string userId: string @@ -38,6 +41,7 @@ export function readSlackClick(rawBody: string): SlackClick | 'ignored' | 'inval return 'ignored' } return { + appId: parsed.data.api_app_id, teamId: parsed.data.team.id, channelId: parsed.data.channel.id, userId: parsed.data.user.id, diff --git a/cloud/apps/connector/src/slack-config.test.ts b/cloud/apps/connector/src/slack-config.test.ts index 5ab9333ff15..f8c324812ed 100644 --- a/cloud/apps/connector/src/slack-config.test.ts +++ b/cloud/apps/connector/src/slack-config.test.ts @@ -5,33 +5,35 @@ import { parseSlackConfig } from './slack-config.js' describe('parseSlackConfig', () => { const key = randomBytes(32).toString('base64') - it('is off without a signing secret', () => { + it('is off without a token key', () => { expect(parseSlackConfig({})).toBeUndefined() }) - it('reads the allowlist, kill switch and key', () => { + it('reads the kill switch and key', () => { + expect( + parseSlackConfig({ ORCA_CONNECTOR_SLACK_PAUSED: 'true', ORCA_CONNECTOR_SLACK_TOKEN_KEY: key }) + ).toEqual({ paused: true, tokenKey: Buffer.from(key, 'base64') }) + }) + + it('reads the legacy route’s secret and allowlist', () => { expect( parseSlackConfig({ ORCA_CONNECTOR_SLACK_SIGNING_SECRET: 'secret', ORCA_CONNECTOR_SLACK_TEAM_IDS: 'T1, T2', - ORCA_CONNECTOR_SLACK_PAUSED: 'true', ORCA_CONNECTOR_SLACK_TOKEN_KEY: key }) ).toEqual({ - signingSecret: 'secret', - teamIds: new Set(['T1', 'T2']), - paused: true, + legacy: { signingSecret: 'secret', teamIds: new Set(['T1', 'T2']) }, + paused: false, tokenKey: Buffer.from(key, 'base64') }) }) it.each([ - ['no allowlist', { ORCA_CONNECTOR_SLACK_TOKEN_KEY: key }], - ['no key', { ORCA_CONNECTOR_SLACK_TEAM_IDS: 'T1' }], - ['a short key', { ORCA_CONNECTOR_SLACK_TEAM_IDS: 'T1', ORCA_CONNECTOR_SLACK_TOKEN_KEY: 'AAAA' }] + ['a legacy secret without its allowlist', { ORCA_CONNECTOR_SLACK_SIGNING_SECRET: 's', ORCA_CONNECTOR_SLACK_TOKEN_KEY: key }], + ['a legacy secret without a key', { ORCA_CONNECTOR_SLACK_SIGNING_SECRET: 's', ORCA_CONNECTOR_SLACK_TEAM_IDS: 'T1' }], + ['a short key', { ORCA_CONNECTOR_SLACK_TOKEN_KEY: 'AAAA' }] ])('refuses %s', (_name, env) => { - expect(() => - parseSlackConfig({ ORCA_CONNECTOR_SLACK_SIGNING_SECRET: 'secret', ...env }) - ).toThrow() + expect(() => parseSlackConfig(env)).toThrow() }) }) diff --git a/cloud/apps/connector/src/slack-config.ts b/cloud/apps/connector/src/slack-config.ts index 756fa60bece..c08fa857c2d 100644 --- a/cloud/apps/connector/src/slack-config.ts +++ b/cloud/apps/connector/src/slack-config.ts @@ -1,12 +1,13 @@ import { z } from 'zod' export type SlackConfig = { - signingSecret: string - // Q20 allowlist: events from any other workspace are acknowledged and ignored. - teamIds: ReadonlySet + // The route Slack used before apps were rows: one env secret, served only for allowlisted + // workspaces. Removed once staging's app posts to its own URL. + legacy?: { signingSecret: string; teamIds: ReadonlySet } // Cloud-wide kill switch: ingress still answers 200 but queues nothing. paused: boolean - // AES-256 key (from Secret Manager) sealing the stored bot tokens. + // AES-256 key (from Secret Manager) sealing every stored Slack secret: bot tokens and each app's + // client and signing secrets. tokenKey: Buffer } @@ -25,23 +26,26 @@ export function decodeSlackTokenKey(value: string): Buffer { return key } -// Slack is off without a signing secret; with one, the allowlist and token key are required too. +// Slack is off without a token key; which apps and workspaces it serves are rows. The legacy +// signing secret, if set, still needs its allowlist. export function parseSlackConfig(env: Record): SlackConfig | undefined { const parsed = SlackEnvSchema.parse(env) const secret = parsed.ORCA_CONNECTOR_SLACK_SIGNING_SECRET - if (secret === undefined) { + if (parsed.ORCA_CONNECTOR_SLACK_TOKEN_KEY === undefined) { + if (secret !== undefined) { + throw new Error('Slack needs ORCA_CONNECTOR_SLACK_TOKEN_KEY') + } return undefined } const teamIds = (parsed.ORCA_CONNECTOR_SLACK_TEAM_IDS ?? '') .split(',') .map((team) => team.trim()) .filter((team) => team.length > 0) - if (teamIds.length === 0 || parsed.ORCA_CONNECTOR_SLACK_TOKEN_KEY === undefined) { - throw new Error('Slack needs ORCA_CONNECTOR_SLACK_TEAM_IDS and ORCA_CONNECTOR_SLACK_TOKEN_KEY') + if (secret !== undefined && teamIds.length === 0) { + throw new Error('The legacy Slack signing secret needs ORCA_CONNECTOR_SLACK_TEAM_IDS') } return { - signingSecret: secret, - teamIds: new Set(teamIds), + ...(secret === undefined ? {} : { legacy: { signingSecret: secret, teamIds: new Set(teamIds) } }), paused: parsed.ORCA_CONNECTOR_SLACK_PAUSED === 'true', tokenKey: decodeSlackTokenKey(parsed.ORCA_CONNECTOR_SLACK_TOKEN_KEY) } diff --git a/cloud/apps/connector/src/slack-edge.ts b/cloud/apps/connector/src/slack-edge.ts index ef429dfc036..7965c9377d0 100644 --- a/cloud/apps/connector/src/slack-edge.ts +++ b/cloud/apps/connector/src/slack-edge.ts @@ -1,10 +1,12 @@ import type { CloudDatabase } from '@orca-cloud/cloud-database' -import type { Context, Hono } from 'hono' +import type { Context, Hono, MiddlewareHandler } from 'hono' import { bodyLimit } from 'hono/body-limit' import { every } from './connector-background.js' import type { ConnectorAdapter } from './connector-adapter.js' import type { ConnectorCore } from './connector-core.js' import type { MachineEnv } from './machine-auth.js' +import { RateLimiter } from './rate-limiter.js' +import { SlackApps } from './slack-apps.js' import { readSlackClick } from './slack-clicks.js' import type { SlackConfig } from './slack-config.js' import { inboundSlackMessage, readSlackEnvelope } from './slack-events.js' @@ -22,6 +24,8 @@ import { createSlackWebApi, type SlackWebApi } from './slack-web-api.js' // Slack's payloads can carry blocks and files well past the machine API's limit. const MAX_SLACK_BODY_BYTES = 1024 * 1024 const MEMBER_CHECK_INTERVAL_MS = 5_000 +// Per instance, after the signature: what one app's own owners can make us do. +const VERIFIED_PER_MINUTE_PER_APP = 600 export type SlackEdge = ReturnType @@ -34,7 +38,9 @@ export function createSlackEdge(deps: { api?: SlackWebApi }) { const { config, database, now } = deps + const apps = new SlackApps(config.tokenKey) const installs = new SlackInstalls(config.tokenKey) + const appRequests = new RateLimiter(VERIFIED_PER_MINUTE_PER_APP, now) const api = deps.api ?? createSlackWebApi() const members = new SlackMembers({ database, installs, api, now }) const history = new SlackThreadHistory({ database, installs, members, api }) @@ -56,38 +62,70 @@ export function createSlackEdge(deps: { members.trustedNames(transaction, teamId, userIds) } - // Slack's own requests: signed over the raw body, size-capped. + // Which workspaces a verified request may act in: those its app is installed in, or for the legacy + // env-secret route, the allowlisted ones whatever their app. + type Scope = { appId: string } | { legacyTeams: ReadonlySet } + type Verified = (context: Context, rawBody: string, scope: Scope) => Promise + + // The request's own install, if its scope reaches it; anything else is acknowledged and ignored. + const installIn = async (scope: Scope, appId: string, teamId: string) => { + const install = await installs.find(database, teamId) + if (!install) { + return null + } + return 'appId' in scope + ? appId === scope.appId && install.appId === scope.appId + ? install + : null + : scope.legacyTeams.has(teamId) + ? install + : null + } + + // Slack's requests, signed over the raw body, size-capped. Nothing in the body is read before the + // signature checks out; on an app's own URL the path only picks which secret it must match. const fromSlack = ( app: Hono, path: string, - handle: (context: Context, rawBody: string) => Promise + limitClient: MiddlewareHandler, + verifier: (context: Context) => Promise<{ secret: string; scope: Scope } | null>, + handle: Verified ) => app.post( path, + limitClient, bodyLimit({ maxSize: MAX_SLACK_BODY_BYTES, onError: (context) => context.json({ error: 'request_too_large' }, 413) }), async (context) => { const rawBody = await context.req.text() - const verified = verifySlackSignature({ - signingSecret: config.signingSecret, - timestamp: context.req.header('x-slack-request-timestamp'), - signature: context.req.header('x-slack-signature'), - rawBody, - nowMs: now() - }) - return verified - ? await handle(context, rawBody) - : context.json({ error: 'invalid_signature' }, 401) + const key = await verifier(context) + const verified = + key !== null && + verifySlackSignature({ + signingSecret: key.secret, + timestamp: context.req.header('x-slack-request-timestamp'), + signature: context.req.header('x-slack-signature'), + rawBody, + nowMs: now() + }) + if (!key || !verified) { + return context.json({ error: 'invalid_signature' }, 401) + } + if ('appId' in key.scope && !appRequests.allow(key.scope.appId)) { + return context.json({ error: 'rate_limited' }, 429) + } + return await handle(context, rawBody, key.scope) } ) function registerRoutes( app: Hono, - core: Pick + core: Pick, + limitClient: MiddlewareHandler ) { - fromSlack(app, '/slack/events', async (context, rawBody) => { + const events: Verified = async (context, rawBody, scope) => { let payload: unknown try { payload = JSON.parse(rawBody) @@ -102,39 +140,56 @@ export function createSlackEdge(deps: { return context.json({ challenge: envelope.challenge }) } // Answered before any Slack call; a database failure is a 500, so Slack retries. - if (envelope.kind === 'message' && config.teamIds.has(envelope.message.teamId)) { - const install = await installs.find(database, envelope.message.teamId) - if (install && envelope.message.userId !== install.botUserId) { - await core.route(inboundSlackMessage(envelope.message, install, config.paused)) + if (envelope.kind === 'message') { + const { message } = envelope + const install = await installIn(scope, message.appId, message.teamId) + if (install && message.userId !== install.botUserId) { + await core.route(inboundSlackMessage(message, install, config.paused)) } } return context.body(null, 200) - }) + } // Answer presses on prompt cards; answered before any Slack call, like events. - fromSlack(app, '/slack/interactivity', async (context, rawBody) => { + const interactivity: Verified = async (context, rawBody, scope) => { const click = readSlackClick(rawBody) if (click === 'invalid') { return context.json({ error: 'invalid_request' }, 400) } - if (click !== 'ignored' && config.teamIds.has(click.teamId)) { - const install = await installs.find(database, click.teamId) - if (install) { - const reply = await core.answerCard('slack', { - workspaceId: click.teamId, - channelId: click.channelId, - messageRef: click.messageTs, - userId: click.userId, - optionId: click.optionId, - paused: config.paused || install.paused - }) - if (reply) { - await core.notify('slack', reply) - } + const install = click === 'ignored' ? null : await installIn(scope, click.appId, click.teamId) + if (click !== 'ignored' && install) { + const reply = await core.answerCard('slack', { + workspaceId: click.teamId, + channelId: click.channelId, + messageRef: click.messageTs, + userId: click.userId, + optionId: click.optionId, + paused: config.paused || install.paused + }) + if (reply) { + await core.notify('slack', reply) } } return context.body(null, 200) - }) + } + + const ownApp = async (context: Context) => { + const appId = context.req.param('appId') ?? '' + const secret = await apps.signingSecret(database, appId) + return secret === null ? null : { secret, scope: { appId } } + } + fromSlack(app, '/slack/apps/:appId/events', limitClient, ownApp, events) + fromSlack(app, '/slack/apps/:appId/interactivity', limitClient, ownApp, interactivity) + + const { legacy } = config + if (legacy) { + const legacyKey = async () => ({ + secret: legacy.signingSecret, + scope: { legacyTeams: legacy.teamIds } + }) + fromSlack(app, '/slack/events', limitClient, legacyKey, events) + fromSlack(app, '/slack/interactivity', limitClient, legacyKey, interactivity) + } } return { diff --git a/cloud/apps/connector/src/slack-events.ts b/cloud/apps/connector/src/slack-events.ts index 6b822e85654..616eadc28a2 100644 --- a/cloud/apps/connector/src/slack-events.ts +++ b/cloud/apps/connector/src/slack-events.ts @@ -9,6 +9,7 @@ const UrlVerificationSchema = z.object({ const EventCallbackSchema = z.object({ type: z.literal('event_callback'), + api_app_id: z.string().min(1), team_id: z.string().min(1), // Slack Connect: a channel shared with another company. is_ext_shared_channel: z.boolean().optional(), @@ -32,6 +33,8 @@ export const PERSON_SUBTYPES = new Set(['thread_broadcast', 'file_share']) // One person's post, the same whether Slack sent it as message or app_mention. export type SlackMessage = { + // The app Slack delivered it for; must be the app whose URL received it. + appId: string teamId: string channelId: string userId: string @@ -69,6 +72,7 @@ export function readSlackEnvelope(payload: unknown): SlackEnvelope { return { kind: 'message', message: { + appId: callback.data.api_app_id, teamId: callback.data.team_id, channelId: event.data.channel, userId: user, diff --git a/cloud/apps/connector/src/slack-harness.test-fixture.ts b/cloud/apps/connector/src/slack-harness.test-fixture.ts index 1c8f9f43c07..4771b23f0db 100644 --- a/cloud/apps/connector/src/slack-harness.test-fixture.ts +++ b/cloud/apps/connector/src/slack-harness.test-fixture.ts @@ -10,6 +10,7 @@ import type { SlackConfig } from './slack-config.js' import { runSlackOperator } from './slack-operator.js' export const SIGNING_SECRET = 'test-signing-secret' +export const APP = 'A1' export const TEAM = 'T1' export const CHANNEL = 'C1' export const BOT = 'UBOT' @@ -61,8 +62,6 @@ export async function createSlackHarness(slack: Partial = {}) { const tokenKey = randomBytes(32) const harness = await createConnectorHarness({ slack: { - signingSecret: SIGNING_SECRET, - teamIds: new Set([TEAM]), paused: false, tokenKey, ...slack @@ -76,15 +75,19 @@ export async function createSlackHarness(slack: Partial = {}) { runSlackOperator(argv, { database: harness.database, tokenKey, - readSecret: async () => 'xoxb-test-token', + readSecret: async () => + argv[0] === 'register-app' + ? JSON.stringify({ clientSecret: 'test-client-secret', signingSecret: SIGNING_SECRET }) + : 'xoxb-test-token', now: harness.now }) - await operator('install', '--team', TEAM, '--bot-user', BOT) + await operator('register-app', '--app-id', APP, '--client-id', '1.2') + await operator('install', '--team', TEAM, '--bot-user', BOT, '--app-id', APP) const signedPost = ( body: string, timestamp = Math.floor(harness.now() / 1000), - path = '/slack/events', + path = `/slack/apps/${APP}/events`, contentType = 'application/json' ) => harness.request(path, { @@ -111,6 +114,7 @@ export async function createSlackHarness(slack: Partial = {}) { signedPost( JSON.stringify({ type: 'event_callback', + api_app_id: APP, team_id: TEAM, event: { type: 'message', @@ -129,6 +133,7 @@ export async function createSlackHarness(slack: Partial = {}) { new URLSearchParams({ payload: JSON.stringify({ type: 'block_actions', + api_app_id: APP, team: { id: TEAM }, user: { id: fields.user ?? OWNER }, channel: { id: CHANNEL }, @@ -137,7 +142,7 @@ export async function createSlackHarness(slack: Partial = {}) { }) }).toString(), undefined, - '/slack/interactivity', + `/slack/apps/${APP}/interactivity`, 'application/x-www-form-urlencoded' ) diff --git a/cloud/apps/connector/src/slack-ingress.test.ts b/cloud/apps/connector/src/slack-ingress.test.ts index 201d644b81e..b28a5ae546b 100644 --- a/cloud/apps/connector/src/slack-ingress.test.ts +++ b/cloud/apps/connector/src/slack-ingress.test.ts @@ -1,7 +1,11 @@ +import { createHmac } from 'node:crypto' import { ConnectorPollResponseSchema } from '@orca-cloud/connector-contract' import { afterEach, describe, expect, it, vi } from 'vitest' import { cleanupConnectorHarnesses } from './connector-server-harness.test-fixture.js' +import { SlackApps } from './slack-apps.js' +import { SlackInstalls } from './slack-installs.js' import { + APP, BOT, CHANNEL, createSlackHarness, @@ -50,7 +54,7 @@ describe('Slack event ingress', () => { const stale = await harness.signedPost('{}', Math.floor(harness.now() / 1000) - 301) expect(stale.status).toBe(401) - const forged = await harness.request('/slack/events', { + const forged = await harness.request(`/slack/apps/${APP}/events`, { method: 'POST', headers: { 'x-slack-request-timestamp': String(Math.floor(harness.now() / 1000)), @@ -62,6 +66,94 @@ describe('Slack event ingress', () => { expect(SIGNING_SECRET).toBeTruthy() }) + it('verifies each app with its own secret, and acts only where that app is installed', async () => { + const harness = await linkedOwner() + const now = harness.now() + await new SlackApps(harness.tokenKey).register( + harness.database, + { appId: 'A2', clientId: '3.4', clientSecret: 'cs-2', signingSecret: 'secret-2' }, + now + ) + const installs = new SlackInstalls(harness.tokenKey) + const install = { appId: 'A2', botUserId: 'UBOT2', botToken: 'xoxb-2' } + // One Orca app per workspace: app 2 can't take over app 1's workspace. + expect(await installs.save(harness.database, { ...install, teamId: TEAM }, now)).toBe(false) + expect(await installs.save(harness.database, { ...install, teamId: 'T2' }, now)).toBe(true) + const signedBy = (secret: string, path: string, body: string) => { + const timestamp = String(Math.floor(now / 1000)) + return harness.request(path, { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'x-slack-request-timestamp': timestamp, + 'x-slack-signature': `v0=${createHmac('sha256', secret).update(`v0:${timestamp}:${body}`).digest('hex')}` + }, + body + }) + } + const event = (appId: string, teamId: string) => + JSON.stringify({ + type: 'event_callback', + api_app_id: appId, + team_id: teamId, + event: { type: 'message', user: OWNER, channel: CHANNEL, channel_type: 'channel', ...mention() } + }) + + // The path only picks the key: app 2's secret doesn't open app 1's URL, nor does an unknown app. + expect((await signedBy('secret-2', `/slack/apps/${APP}/events`, event(APP, TEAM))).status).toBe(401) + expect((await signedBy('secret-2', '/slack/apps/A9/events', event('A9', TEAM))).status).toBe(401) + // Signed by app 2 but naming app 1, or app 2 speaking for app 1's workspace: acknowledged, ignored. + expect((await signedBy('secret-2', '/slack/apps/A2/events', event(APP, TEAM))).status).toBe(200) + expect((await signedBy('secret-2', '/slack/apps/A2/events', event('A2', TEAM))).status).toBe(200) + expect(await harness.poll()).toEqual([]) + expect(await harness.posts()).toEqual([]) + + // A row that won't open is an operator fault, not a bad request: Slack retries a 500. + await harness.database.query( + "UPDATE connector_slack_apps SET signing_secret = 'v1.AAAA.AAAA.AAAA' WHERE app_id = 'A2'" + ) + expect((await signedBy('secret-2', '/slack/apps/A2/events', event('A2', 'T2'))).status).toBe(500) + + // App 1's own signed event in its own workspace starts the conversation. + expect((await harness.message(mention())).status).toBe(200) + expect(await harness.poll()).toEqual([expect.objectContaining({ kind: 'start' })]) + }) + + it('keeps the legacy env-secret route for allowlisted workspaces until the app’s URL takes over', async () => { + const harness = await linkedOwner({ + legacy: { signingSecret: 'legacy-secret', teamIds: new Set([TEAM]) } + }) + // As before apps were rows: the install has no app yet. + await harness.database.query('UPDATE connector_slack_installs SET app_id = NULL') + const timestamp = String(Math.floor(harness.now() / 1000)) + const legacyPost = (body: string, secret = 'legacy-secret') => + harness.request('/slack/events', { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'x-slack-request-timestamp': timestamp, + 'x-slack-signature': `v0=${createHmac('sha256', secret).update(`v0:${timestamp}:${body}`).digest('hex')}` + }, + body + }) + const event = (teamId: string) => + JSON.stringify({ + type: 'event_callback', + api_app_id: APP, + team_id: teamId, + event: { type: 'message', user: OWNER, channel: CHANNEL, channel_type: 'channel', ...mention() } + }) + // The app's own URL can't reach an install that isn't bound to it. + expect((await harness.message(mention())).status).toBe(200) + expect(await harness.poll()).toEqual([]) + // The legacy route still needs its own secret and its allowlist. + expect((await legacyPost(event(TEAM), SIGNING_SECRET)).status).toBe(401) + expect((await legacyPost(event('T9'))).status).toBe(200) + expect(await harness.poll()).toEqual([]) + expect((await legacyPost(event(TEAM))).status).toBe(200) + expect(await harness.poll()).toEqual([expect.objectContaining({ kind: 'start' })]) + }) + it('starts the owner’s conversation once for the message and app_mention twins', async () => { const harness = await linkedOwner() expect((await harness.message(mention())).status).toBe(200) diff --git a/cloud/apps/connector/src/slack-installs.test.ts b/cloud/apps/connector/src/slack-installs.test.ts deleted file mode 100644 index 14f700528f1..00000000000 --- a/cloud/apps/connector/src/slack-installs.test.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { randomBytes } from 'node:crypto' -import { describe, expect, it } from 'vitest' -import { openBotToken, sealBotToken } from './slack-installs.js' - -describe('bot token sealing', () => { - it('opens only with the same key and team', () => { - const key = randomBytes(32) - const sealed = sealBotToken(key, 'T1', 'xoxb-secret') - expect(sealed).not.toContain('xoxb') - expect(openBotToken(key, 'T1', sealed)).toBe('xoxb-secret') - expect(() => openBotToken(key, 'T2', sealed)).toThrow() - expect(() => openBotToken(randomBytes(32), 'T1', sealed)).toThrow() - }) -}) diff --git a/cloud/apps/connector/src/slack-installs.ts b/cloud/apps/connector/src/slack-installs.ts index c2acffd74dc..a82e586f5e5 100644 --- a/cloud/apps/connector/src/slack-installs.ts +++ b/cloud/apps/connector/src/slack-installs.ts @@ -1,42 +1,32 @@ -import { createCipheriv, createDecipheriv, randomBytes } from 'node:crypto' import type { CloudDatabase } from '@orca-cloud/cloud-database' +import { openSecret, sealSecret } from './slack-sealing.js' -export type SlackInstall = { teamId: string; botUserId: string; paused: boolean } - -// AES-256-GCM bound to the team id, so a sealed token copied to another row won't open. -export function sealBotToken(key: Buffer, teamId: string, token: string): string { - const iv = randomBytes(12) - const cipher = createCipheriv('aes-256-gcm', key, iv).setAAD(Buffer.from(teamId)) - const sealed = Buffer.concat([cipher.update(token, 'utf8'), cipher.final()]) - return ['v1', ...[iv, cipher.getAuthTag(), sealed].map((part) => part.toString('base64'))].join( - '.' - ) +// appId is null for a workspace installed before apps were rows: only the legacy env-secret route +// reaches it, until the operator binds it to its app. +export type SlackInstall = { + teamId: string + appId: string | null + botUserId: string + paused: boolean } -export function openBotToken(key: Buffer, teamId: string, sealed: string): string { - const [version, iv, tag, data] = sealed.split('.') - if (version !== 'v1' || !iv || !tag || !data) { - throw new Error('unreadable sealed bot token') - } - const decipher = createDecipheriv('aes-256-gcm', key, Buffer.from(iv, 'base64')) - .setAAD(Buffer.from(teamId)) - .setAuthTag(Buffer.from(tag, 'base64')) - return Buffer.concat([decipher.update(Buffer.from(data, 'base64')), decipher.final()]).toString( - 'utf8' - ) -} - -// One row per Slack workspace, written by the operator command (no OAuth install in v1). +// One row per Slack workspace: its bot, and which Orca app it is installed through. A workspace has +// at most one Orca app. Bot tokens are sealed under the team id. export class SlackInstalls { constructor(private readonly tokenKey: Buffer) {} async find(database: CloudDatabase, teamId: string): Promise { const [row] = await database.query( - 'SELECT bot_user_id, paused FROM connector_slack_installs WHERE team_id = ?', + 'SELECT app_id, bot_user_id, paused FROM connector_slack_installs WHERE team_id = ?', [teamId] ) return row - ? { teamId, botUserId: String(row.bot_user_id), paused: Number(row.paused) === 1 } + ? { + teamId, + appId: row.app_id === null ? null : String(row.app_id), + botUserId: String(row.bot_user_id), + paused: Number(row.paused) === 1 + } : null } @@ -50,29 +40,37 @@ export class SlackInstalls { } // Sealed under another key or corrupted: as good as no install. try { - return openBotToken(this.tokenKey, teamId, String(row.bot_token)) + return openSecret(this.tokenKey, teamId, String(row.bot_token)) } catch { return null } } + // False when the workspace already belongs to a different Orca app. The same app reinstalling, or + // a pre-apps install being bound to its app, updates the row in place. async save( database: CloudDatabase, - install: { teamId: string; botUserId: string; botToken: string }, + install: { teamId: string; appId: string; botUserId: string; botToken: string }, now: number - ): Promise { - await database.query( - `INSERT INTO connector_slack_installs (team_id, bot_user_id, bot_token, paused, installed_at) - VALUES (?, ?, ?, 0, ?) - ON CONFLICT (team_id) DO UPDATE SET bot_user_id = excluded.bot_user_id, - bot_token = excluded.bot_token, installed_at = excluded.installed_at`, + ): Promise { + const [result] = await database.query( + `INSERT INTO connector_slack_installs + (team_id, app_id, bot_user_id, bot_token, paused, installed_at) + VALUES (?, ?, ?, ?, 0, ?) + ON CONFLICT (team_id) DO UPDATE SET app_id = excluded.app_id, + bot_user_id = excluded.bot_user_id, bot_token = excluded.bot_token, + installed_at = excluded.installed_at + WHERE connector_slack_installs.app_id IS NULL + OR connector_slack_installs.app_id = excluded.app_id`, [ install.teamId, + install.appId, install.botUserId, - sealBotToken(this.tokenKey, install.teamId, install.botToken), + sealSecret(this.tokenKey, install.teamId, install.botToken), now ] ) + return Number(result?.changes ?? 0) > 0 } async setPaused(database: CloudDatabase, teamId: string, paused: boolean): Promise { diff --git a/cloud/apps/connector/src/slack-operator.test.ts b/cloud/apps/connector/src/slack-operator.test.ts index c176a58e991..eeee1a8dd6d 100644 --- a/cloud/apps/connector/src/slack-operator.test.ts +++ b/cloud/apps/connector/src/slack-operator.test.ts @@ -2,7 +2,9 @@ import { randomBytes } from 'node:crypto' import { afterEach, describe, expect, it } from 'vitest' import { openInMemoryConnectorDatabase } from './connector-database.js' import { findIdentity } from './connector-identities.js' +import { SlackApps } from './slack-apps.js' import { SlackInstalls } from './slack-installs.js' +import { sealSecret } from './slack-sealing.js' import { runSlackOperator } from './slack-operator.js' const closers: (() => Promise)[] = [] @@ -10,7 +12,12 @@ afterEach(async () => { await Promise.all(closers.splice(0).map((close) => close())) }) -async function setup(secret = 'xoxb-1') { +const APP_SECRETS = JSON.stringify({ clientSecret: 'cs-1', signingSecret: 'ss-1' }) +const REGISTER = ['register-app', '--app-id', 'A1', '--client-id', '1.2'] +const INSTALL = ['install', '--team', 'T1', '--bot-user', 'UBOT', '--app-id', 'A1'] + +// register-app reads the app's secrets from stdin, install reads the bot token. +async function setup(stdin: { app?: string; bot?: string } = {}) { const database = await openInMemoryConnectorDatabase() closers.push(() => database.close()) const tokenKey = randomBytes(32) @@ -18,37 +25,86 @@ async function setup(secret = 'xoxb-1') { runSlackOperator(argv, { database, tokenKey, - readSecret: async () => `${secret}\n`, + readSecret: async () => + argv[0] === 'register-app' ? (stdin.app ?? APP_SECRETS) : (stdin.bot ?? 'xoxb-1\n'), now: () => 1 }) return { database, tokenKey, run } } describe('Slack operator command', () => { - it('stores the bot token sealed, and pauses or resumes the workspace', async () => { + it('registers an app and installs it sealed, and pauses or resumes the workspace', async () => { const { database, tokenKey, run } = await setup() - expect(await run('install', '--team', 'T1', '--bot-user', 'UBOT')).toBe( - 'Saved the bot token for T1.' + expect(await run(...REGISTER)).toBe('Registered A1.') + expect(await run(...INSTALL)).toBe('Saved the bot token for T1, installed through A1.') + const rows = await database.query( + `SELECT a.client_secret, a.signing_secret, i.bot_token FROM connector_slack_apps a + JOIN connector_slack_installs i ON i.app_id = a.app_id` ) - const [row] = await database.query('SELECT bot_token FROM connector_slack_installs') - expect(String(row?.bot_token)).not.toContain('xoxb') + expect(rows).toHaveLength(1) + expect(JSON.stringify(rows)).not.toMatch(/cs-1|ss-1|xoxb/) + expect(await new SlackApps(tokenKey).signingSecret(database, 'A1')).toBe('ss-1') const installs = new SlackInstalls(tokenKey) expect(await installs.botToken(database, 'T1')).toBe('xoxb-1') expect(await run('pause', '--team', 'T1')).toBe('Paused T1.') expect(await installs.find(database, 'T1')).toEqual({ teamId: 'T1', + appId: 'A1', botUserId: 'UBOT', paused: true }) await expect(run('resume', '--team', 'T2')).rejects.toThrow('T2 is not installed') }) - it('refuses something that is not a bot token or a Slack id', async () => { + it('binds a workspace installed before apps were rows, keeping its sealed token readable', async () => { + const { database, tokenKey, run } = await setup() + // As the shipped install command left it: no app, the token sealed under the team id. + await database.query( + `INSERT INTO connector_slack_installs (team_id, bot_user_id, bot_token, paused, installed_at) + VALUES ('T1', 'UBOT', ?, 0, 1)`, + [sealSecret(tokenKey, 'T1', 'xoxb-old')] + ) + const installs = new SlackInstalls(tokenKey) + expect(await installs.find(database, 'T1')).toMatchObject({ appId: null }) + expect(await installs.botToken(database, 'T1')).toBe('xoxb-old') + await run(...REGISTER) + await run(...INSTALL) + expect(await installs.find(database, 'T1')).toMatchObject({ appId: 'A1' }) + expect(await installs.botToken(database, 'T1')).toBe('xoxb-1') + }) + + it('keeps one Orca app per workspace, and never replaces an app’s secrets', async () => { + const { database, tokenKey, run } = await setup() + await run(...REGISTER) + await run(...INSTALL) + await run('register-app', '--app-id', 'A2', '--client-id', '3.4') await expect( - (await setup('xoxp-user')).run('install', '--team', 'T1', '--bot-user', 'UBOT') - ).rejects.toThrow('bot token') + run('install', '--team', 'T1', '--bot-user', 'U2', '--app-id', 'A2') + ).rejects.toThrow('T1 already has another Orca app installed') + expect(await new SlackInstalls(tokenKey).find(database, 'T1')).toMatchObject({ + appId: 'A1', + botUserId: 'UBOT' + }) + await expect(run(...REGISTER)).rejects.toThrow('A1 is already registered') await expect( - (await setup()).run('install', '--team', 't1', '--bot-user', 'UBOT') + run('install', '--team', 'T2', '--bot-user', 'U2', '--app-id', 'A9') + ).rejects.toThrow('A9 is not registered') + }) + + it('refuses bad stdin without echoing it, and bad ids', async () => { + const leaked = JSON.stringify({ clientSecret: 'cs-1', signingSecret: 'ss-1', extra: 'x-secret' }) + const refused = (await setup({ app: leaked })).run(...REGISTER) + await expect(refused).rejects.toThrow('stdin must be') + await expect(refused).rejects.not.toThrow('x-secret') + await expect((await setup({ app: 'not json' })).run(...REGISTER)).rejects.toThrow('stdin must be') + const notBot = await setup({ bot: 'xoxp-user' }) + await notBot.run(...REGISTER) + await expect(notBot.run(...INSTALL)).rejects.toThrow('bot token') + await expect( + (await setup()).run('register-app', '--app-id', 'A1', '--client-id', 'x') + ).rejects.toThrow('--client-id') + await expect( + (await setup()).run('install', '--team', 't1', '--bot-user', 'U', '--app-id', 'A1') ).rejects.toThrow('--team') }) diff --git a/cloud/apps/connector/src/slack-operator.ts b/cloud/apps/connector/src/slack-operator.ts index f7ffac94d0a..e0fc9f06dde 100644 --- a/cloud/apps/connector/src/slack-operator.ts +++ b/cloud/apps/connector/src/slack-operator.ts @@ -1,15 +1,25 @@ import { parseArgs } from 'node:util' import type { CloudDatabase } from '@orca-cloud/cloud-database' +import { z } from 'zod' import { describeAccountHosts } from './connector-hosts.js' import { LINK_OPTIONS, linkIdentity, required } from './operator-link.js' +import { SlackApps } from './slack-apps.js' import { SlackInstalls } from './slack-installs.js' // Slack ids are upper-case letters and digits; anything else is a typo, and a bot user id ends up // in a pattern. const SLACK_ID = /^[A-Z0-9]+$/ +const CLIENT_ID = /^\d+\.\d+$/ + +// Read from stdin as one JSON object, so no secret is ever a command-line argument. +const AppSecretsSchema = z + .object({ clientSecret: z.string().min(1), signingSecret: z.string().min(1) }) + .strict() export const SLACK_OPERATOR_USAGE = `Usage: - install --team --bot-user bot token is read from stdin + register-app --app-id --client-id + stdin: {"clientSecret": "…", "signingSecret": "…"} + install --team --bot-user --app-id bot token is read from stdin pause --team | resume --team link --team --user --account [--default-host ] [--default-project ] @@ -23,8 +33,8 @@ function slackId(value: string | undefined, flag: string): string { return value } -// Dogfood seeding (no OAuth install or Slack sign-in in v1): one workspace's bot token, and which -// Orca account each Slack user is, with their machine-choice defaults. +// Dogfood seeding (no OAuth install or Slack sign-in yet): an app Orca serves and the workspace it +// is installed in, and which Orca account each Slack user is, with their machine-choice defaults. export async function runSlackOperator( argv: string[], deps: { @@ -40,25 +50,55 @@ export async function runSlackOperator( options: { ...LINK_OPTIONS, team: { type: 'string' }, - 'bot-user': { type: 'string' } + 'bot-user': { type: 'string' }, + 'app-id': { type: 'string' }, + 'client-id': { type: 'string' } } }) - const installs = () => { + const tokenKey = () => { if (!deps.tokenKey) { throw new Error('ORCA_CONNECTOR_SLACK_TOKEN_KEY is required') } - return new SlackInstalls(deps.tokenKey) + return deps.tokenKey } + const installs = () => new SlackInstalls(tokenKey()) switch (command) { + case 'register-app': { + const appId = slackId(values['app-id'], 'app-id') + const clientId = values['client-id'] + if (!clientId || !CLIENT_ID.test(clientId)) { + throw new Error('--client-id needs a Slack client id') + } + let secrets: z.infer + try { + secrets = AppSecretsSchema.parse(JSON.parse(await deps.readSecret())) + } catch { + // Never echo the parse error: it can quote the secrets. + throw new Error('stdin must be {"clientSecret": "…", "signingSecret": "…"}') + } + const app = { appId, clientId, ...secrets } + if (!(await new SlackApps(tokenKey()).register(deps.database, app, deps.now()))) { + throw new Error(`${appId} is already registered`) + } + return `Registered ${appId}.` + } + // Also binds a workspace installed before apps were rows to its app. case 'install': { const teamId = slackId(values.team, 'team') const botUserId = slackId(values['bot-user'], 'bot-user') + const appId = slackId(values['app-id'], 'app-id') const botToken = (await deps.readSecret()).trim() if (!botToken.startsWith('xoxb-')) { throw new Error('stdin must be a bot token (xoxb-…)') } - await installs().save(deps.database, { teamId, botUserId, botToken }, deps.now()) - return `Saved the bot token for ${teamId}.` + if (!(await new SlackApps(tokenKey()).exists(deps.database, appId))) { + throw new Error(`${appId} is not registered`) + } + const install = { teamId, appId, botUserId, botToken } + if (!(await installs().save(deps.database, install, deps.now()))) { + throw new Error(`${teamId} already has another Orca app installed`) + } + return `Saved the bot token for ${teamId}, installed through ${appId}.` } case 'pause': case 'resume': { diff --git a/cloud/apps/connector/src/slack-posting.test.ts b/cloud/apps/connector/src/slack-posting.test.ts index 8f63df15593..4d61af5d2b9 100644 --- a/cloud/apps/connector/src/slack-posting.test.ts +++ b/cloud/apps/connector/src/slack-posting.test.ts @@ -279,6 +279,31 @@ describe('Slack thread status and posts', () => { expect((await states())[`reply:${harness.machine.hostId}:r2`]).toBe('uncertain') }) + it('orders sends per channel within one workspace only', async () => { + const harness = await startedThread() + const posts = new ConnectorPosts(harness.database, harness.now, 'slack') + const place = { containerId: 'C9', threadId: '1800000000.000900' } + await harness.database.transaction(async (transaction) => { + await posts.queue(transaction, { + ...place, + workspaceId: 'T8', + postId: 'other-workspace', + kind: 'message', + body: { text: 'a' } + }) + await posts.queue(transaction, { + ...place, + workspaceId: 'T9', + postId: 'this-workspace', + kind: 'message', + body: { text: 'b' } + }) + }) + // The same channel id in another workspace is another channel: neither waits on the other. + const claimed = (await posts.claim()).map((post) => post.postId) + expect(claimed).toEqual(expect.arrayContaining(['other-workspace', 'this-workspace'])) + }) + it('a second worker, as another role runs, claims nothing while the first holds the lease', async () => { const harness = await startedThread() const api = new ConnectorPosts(harness.database, harness.now, 'slack') diff --git a/cloud/apps/connector/src/slack-sealing.test.ts b/cloud/apps/connector/src/slack-sealing.test.ts new file mode 100644 index 00000000000..1df4cba3bfa --- /dev/null +++ b/cloud/apps/connector/src/slack-sealing.test.ts @@ -0,0 +1,24 @@ +import { randomBytes } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { appSecretAad } from './slack-apps.js' +import { openSecret, sealSecret } from './slack-sealing.js' + +describe('secret sealing', () => { + it('opens only with the same key and AAD', () => { + const key = randomBytes(32) + const sealed = sealSecret(key, 'T1', 'xoxb-secret') + expect(sealed).not.toContain('xoxb') + expect(openSecret(key, 'T1', sealed)).toBe('xoxb-secret') + expect(() => openSecret(key, 'T2', sealed)).toThrow() + expect(() => openSecret(randomBytes(32), 'T1', sealed)).toThrow() + }) + + it('keeps an app’s two secrets, other apps and bot tokens apart', () => { + const key = randomBytes(32) + const sealed = sealSecret(key, appSecretAad('signing', 'A1'), 's') + expect(openSecret(key, appSecretAad('signing', 'A1'), sealed)).toBe('s') + for (const aad of [appSecretAad('client', 'A1'), appSecretAad('signing', 'A2'), 'A1']) { + expect(() => openSecret(key, aad, sealed)).toThrow() + } + }) +}) diff --git a/cloud/apps/connector/src/slack-sealing.ts b/cloud/apps/connector/src/slack-sealing.ts new file mode 100644 index 00000000000..e003035a9a8 --- /dev/null +++ b/cloud/apps/connector/src/slack-sealing.ts @@ -0,0 +1,25 @@ +import { createCipheriv, createDecipheriv, randomBytes } from 'node:crypto' + +// AES-256-GCM under the Slack token key. The AAD names what the value is and which row holds it, so +// a sealed value copied to another column or row won't open. +export function sealSecret(key: Buffer, aad: string, value: string): string { + const iv = randomBytes(12) + const cipher = createCipheriv('aes-256-gcm', key, iv).setAAD(Buffer.from(aad)) + const sealed = Buffer.concat([cipher.update(value, 'utf8'), cipher.final()]) + return ['v1', ...[iv, cipher.getAuthTag(), sealed].map((part) => part.toString('base64'))].join( + '.' + ) +} + +export function openSecret(key: Buffer, aad: string, sealed: string): string { + const [version, iv, tag, data] = sealed.split('.') + if (version !== 'v1' || !iv || !tag || !data) { + throw new Error('unreadable sealed secret') + } + const decipher = createDecipheriv('aes-256-gcm', key, Buffer.from(iv, 'base64')) + .setAAD(Buffer.from(aad)) + .setAuthTag(Buffer.from(tag, 'base64')) + return Buffer.concat([decipher.update(Buffer.from(data, 'base64')), decipher.final()]).toString( + 'utf8' + ) +} diff --git a/cloud/packages/cloud-database/src/cloud-database.test.ts b/cloud/packages/cloud-database/src/cloud-database.test.ts index 72047e09865..22100609d42 100644 --- a/cloud/packages/cloud-database/src/cloud-database.test.ts +++ b/cloud/packages/cloud-database/src/cloud-database.test.ts @@ -34,6 +34,21 @@ describe('cloud database', () => { await reader.close() }) + it('adds a column on SQLite only while it is missing, so every boot can repeat it', async () => { + const dataDir = mkdtempSync(join(tmpdir(), 'orca-cloud-database-')) + dirs.push(dataDir) + const schema = [ + ...SCHEMA, + 'ALTER TABLE items ADD COLUMN IF NOT EXISTS note TEXT', + 'ALTER TABLE "items" ADD COLUMN IF NOT EXISTS "Extra" TEXT' + ] + for (let boot = 0; boot < 2; boot++) { + const database = await openCloudDatabase({ service: 'sample', schemaStatements: schema, dataDir }) + await database.query('INSERT INTO items (id, value, note) VALUES (?, ?, ?)', [`${boot}`, 1, 'n']) + await database.close() + } + }) + it('rolls back a failed transaction in memory', async () => { const database = await openInMemoryCloudDatabase(SCHEMA) await expect( diff --git a/cloud/packages/cloud-database/src/cloud-database.ts b/cloud/packages/cloud-database/src/cloud-database.ts index ec7602cfbb8..bd578a014f3 100644 --- a/cloud/packages/cloud-database/src/cloud-database.ts +++ b/cloud/packages/cloud-database/src/cloud-database.ts @@ -3,7 +3,7 @@ import { join } from 'node:path' import { DatabaseSync } from 'node:sqlite' import pg from 'pg' import { parseIntoClientConfig } from 'pg-connection-string' -import { applyPostgresSchema } from '@orca-cloud/postgres-schema' +import { applyPostgresSchema, schemaLockTarget } from '@orca-cloud/postgres-schema' const POSTGRES_LOCK_TIMEOUT_MS = 1_000 const POSTGRES_CONNECTION_TIMEOUT_MS = 2_000 @@ -236,9 +236,22 @@ class PostgresDatabase implements CloudDatabase { } } +// SQLite has no ADD COLUMN IF NOT EXISTS: run it as a plain ADD COLUMN only while the column is missing. async function applySchema(database: CloudDatabase, statements: string[]): Promise { for (const statement of statements) { - await database.query(statement) + const target = schemaLockTarget(statement) + if (target?.kind !== 'column') { + await database.query(statement) + continue + } + // SQLite names: quotes stripped, compared without case. + const [present] = await database.query( + 'SELECT 1 FROM pragma_table_info(?) WHERE lower(name) = lower(?)', + [target.table.replace(/^"|"$/g, ''), target.name.replace(/^"|"$/g, '')] + ) + if (!present) { + await database.query(statement.replace(/\bADD\s+COLUMN\s+IF\s+NOT\s+EXISTS\b/i, 'ADD COLUMN')) + } } }