From 874ad52e6c76ee58f2d5e120c0cd047a1fdad9ba Mon Sep 17 00:00:00 2001 From: DHDHLZ <731642078@qq.com> Date: Wed, 2 Sep 2026 19:56:52 +0800 Subject: [PATCH] fix(connector): stop first-come /link owner takeover across chat adapters Each messaging adapter (Telegram, Discord, Slack, Feishu) bound the first caller of /link as owner when none was configured. Because bots are publicly reachable, whoever messages /link during the unlinked setup window becomes the owner and inherits owner-gated surfaces (inbox, desk issues, UTA review actions). Now /link only binds a chat for the owner pre-configured in Connector settings; an unconfigured connector refuses to bind and explains the requirement. Regression specs cover refuse-when-unconfigured, bind-for-configured-owner, and reject-non-owner. --- services/connector/src/adapters/discord.ts | 9 +- .../connector/src/adapters/feishu.spec.ts | 2 +- services/connector/src/adapters/feishu.ts | 6 +- .../src/adapters/owner-link-guard.spec.ts | 100 ++++++++++++++++++ services/connector/src/adapters/slack.ts | 9 +- services/connector/src/adapters/telegram.ts | 6 +- 6 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 services/connector/src/adapters/owner-link-guard.spec.ts diff --git a/services/connector/src/adapters/discord.ts b/services/connector/src/adapters/discord.ts index b204b2a2e..13f10540c 100644 --- a/services/connector/src/adapters/discord.ts +++ b/services/connector/src/adapters/discord.ts @@ -149,12 +149,15 @@ export class DiscordConnectorAdapter implements ConnectorAdapter { private registerCommands(context: ConnectorAdapterContext): void { context.commands.register('link', async ({ userId, reply }) => { - if (this.ownerUserId && this.ownerUserId !== userId) { +if (this.ownerUserId && this.ownerUserId !== userId) { await reply('This connector is already linked to another account.') return } - this.ownerUserId = userId - await context.updateSettings({ ownerUserId: userId }) + if (!this.ownerUserId) { + await reply('This connector requires the owner account to be configured in Connector settings before linking. First-come /link binding is disabled to prevent a takeover by whoever messages the bot first.') + return + } + await context.updateSettings({ ownerUserId: this.ownerUserId }) this.tracker.healthy(userId) await reply('Discord is linked to this OpenAlice installation.') }) diff --git a/services/connector/src/adapters/feishu.spec.ts b/services/connector/src/adapters/feishu.spec.ts index 6549721dc..40e2fad35 100644 --- a/services/connector/src/adapters/feishu.spec.ts +++ b/services/connector/src/adapters/feishu.spec.ts @@ -218,7 +218,7 @@ describe('Feishu owner chat', () => { const adapter = new FeishuConnectorAdapter({ startupTimeoutMs: 200 }) await adapter.start({ enabled: true, - settings: { appId: APP_ID, appSecret: APP_SECRET }, + settings: { appId: APP_ID, appSecret: APP_SECRET, ownerUserId: 'ou_owner' }, }, context({ updateSettings })) await receiveHandler?.(p2pEvent('/link')) expect(updateSettings).toHaveBeenCalledWith({ ownerUserId: 'ou_owner', chatId: 'oc_chat' }) diff --git a/services/connector/src/adapters/feishu.ts b/services/connector/src/adapters/feishu.ts index 7bb30fc9e..d120227eb 100644 --- a/services/connector/src/adapters/feishu.ts +++ b/services/connector/src/adapters/feishu.ts @@ -272,10 +272,14 @@ export class FeishuConnectorAdapter implements ConnectorAdapter { private registerCommands(context: ConnectorAdapterContext): void { context.commands.register('link', async ({ userId, chatId, reply }) => { - if (this.ownerUserId && this.ownerUserId !== userId) { +if (this.ownerUserId && this.ownerUserId !== userId) { await reply('This connector is already linked to another account.') return } + if (!this.ownerUserId) { + await reply('This connector requires the owner account to be configured in Connector settings before linking. First-come /link binding is disabled to prevent a takeover by whoever messages the bot first.') + return + } if (!chatId) throw new Error('Feishu private chat ID is missing') this.ownerUserId = userId this.chatId = chatId diff --git a/services/connector/src/adapters/owner-link-guard.spec.ts b/services/connector/src/adapters/owner-link-guard.spec.ts new file mode 100644 index 000000000..5be8f86c2 --- /dev/null +++ b/services/connector/src/adapters/owner-link-guard.spec.ts @@ -0,0 +1,100 @@ +/** + * Regression tests for the /link owner-binding fix. + * + * Before: when the connector had no configured owner, the first person to + * message /link became owner (first-come takeover; whoever finds the bot + * first wins). + * After: /link only binds a chat for a caller that matches the owner + * configured in Connector settings. An unconfigured bot refuses to bind. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { CommandRegistry } from '../core/adapter.js' +import { TelegramConnectorAdapter } from './telegram.js' + +const startMock = vi.fn() +const getMe = vi.fn(async () => ({ id: 1, is_bot: true, first_name: 'OpenAlice', username: 'openalice_bot' })) +const setMyCommands = vi.fn(async () => undefined) + +vi.mock('grammy', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + Bot: class { + api = { config: { use() {} }, getMe, setMyCommands } + command() {} + on() {} + start(options: { onStart?: () => void }) { return startMock(options) } + stop() { return Promise.resolve() } + }, + InputFile: class {}, + } +}) +vi.mock('@grammyjs/auto-retry', () => ({ autoRetry: () => () => undefined })) + +function freshContext(updates: Array>) { + return { + commands: new CommandRegistry('telegram'), + updateSettings: async (patch: Record) => { updates.push(patch) }, + getServiceStatus: () => 'healthy', + sendTest: async () => 'probe', + forwardOwnerText: async () => undefined, + enqueueArtifactRequest: async () => 'art', + enqueueUtaRequest: async () => 'uta', + } as unknown as Parameters[1] +} + +async function startAdapter(settings: Record) { + startMock.mockImplementation((options: { onStart?: () => void }) => { + queueMicrotask(() => options.onStart?.()) + return new Promise(() => undefined) + }) + const updates: Array> = [] + const ctx = freshContext(updates) + const adapter = new TelegramConnectorAdapter({ attemptTimeoutMs: 200, reconnectDelayMs: 20 }) + await adapter.start({ enabled: true, settings }, ctx) + return { adapter, ctx, updates } +} + +async function runLink(ctx: ReturnType, userId: string, chatId: string): Promise { + let replyText = '' + const handled = await ctx.commands.execute({ + connectorId: 'telegram', command: 'link', userId, chatId, + reply: async (text: string) => { replyText = text }, + }) + expect(handled).toBe(true) + return replyText +} + +describe('/link owner binding guard', () => { + beforeEach(() => { + startMock.mockReset() + getMe.mockReset(); getMe.mockResolvedValue({ id: 1, is_bot: true, first_name: 'OpenAlice', username: 'openalice_bot' }) + setMyCommands.mockReset(); setMyCommands.mockResolvedValue(undefined) + }) + + it('refuses to bind an owner when none is configured (no first-come takeover)', async () => { + const { adapter, ctx, updates } = await startAdapter({ botToken: 'token' }) + const replyText = await runLink(ctx, 'stranger-1', '111') + expect(replyText).toMatch(/owner account to be configured/i) + expect(updates.length).toBe(0) // nothing persisted + expect(adapter.health().owner).toBeUndefined() + await adapter.stop() + }) + + it('binds the chat only for the configured owner', async () => { + const { adapter, ctx, updates } = await startAdapter({ botToken: 'token', ownerUserId: '42' }) + const replyText = await runLink(ctx, '42', '99') + expect(replyText).toMatch(/linked to this OpenAlice/i) + expect(updates).toContainEqual(expect.objectContaining({ ownerUserId: '42', chatId: '99' })) + expect(adapter.health().owner).toBe('42') + await adapter.stop() + }) + + it('still rejects a caller that is not the configured owner', async () => { + const { adapter, ctx, updates } = await startAdapter({ botToken: 'token', ownerUserId: '42' }) + const replyText = await runLink(ctx, 'stranger-1', '111') + expect(replyText).toMatch(/already linked/i) + expect(updates.length).toBe(0) + await adapter.stop() + }) +}) diff --git a/services/connector/src/adapters/slack.ts b/services/connector/src/adapters/slack.ts index 0dd9bd154..823bfaf04 100644 --- a/services/connector/src/adapters/slack.ts +++ b/services/connector/src/adapters/slack.ts @@ -156,12 +156,15 @@ export class SlackConnectorAdapter implements ConnectorAdapter { private registerCommands(context: ConnectorAdapterContext): void { context.commands.register('link', async ({ userId, reply }) => { - if (this.ownerUserId && this.ownerUserId !== userId) { +if (this.ownerUserId && this.ownerUserId !== userId) { await reply('This connector is already linked to another account.') return } - this.ownerUserId = userId - await context.updateSettings({ ownerUserId: userId }) + if (!this.ownerUserId) { + await reply('This connector requires the owner account to be configured in Connector settings before linking. First-come /link binding is disabled to prevent a takeover by whoever messages the bot first.') + return + } + await context.updateSettings({ ownerUserId: this.ownerUserId }) this.tracker.healthy(userId) await reply('Slack is linked to this OpenAlice installation.') }) diff --git a/services/connector/src/adapters/telegram.ts b/services/connector/src/adapters/telegram.ts index a9b18a557..63e62db0a 100644 --- a/services/connector/src/adapters/telegram.ts +++ b/services/connector/src/adapters/telegram.ts @@ -451,10 +451,14 @@ export class TelegramConnectorAdapter implements ConnectorAdapter { private registerCommands(context: ConnectorAdapterContext): void { context.commands.register('link', async ({ userId, chatId, reply }) => { - if (this.ownerUserId && this.ownerUserId !== userId) { +if (this.ownerUserId && this.ownerUserId !== userId) { await reply('This connector is already linked to another account.') return } + if (!this.ownerUserId) { + await reply('This connector requires the owner account to be configured in Connector settings before linking. First-come /link binding is disabled to prevent a takeover by whoever messages the bot first.') + return + } if (!chatId) throw new Error('Telegram private chat ID is missing') this.ownerUserId = userId this.chatId = chatId