diff --git a/backend/__tests__/unit/routes/slackOAuth.installables.test.js b/backend/__tests__/unit/routes/slackOAuth.installables.test.js index 5151c2d2d7..6a3873e6b4 100644 --- a/backend/__tests__/unit/routes/slackOAuth.installables.test.js +++ b/backend/__tests__/unit/routes/slackOAuth.installables.test.js @@ -14,7 +14,7 @@ jest.mock('../../../middleware/integrationRateLimit', () => ({ })); jest.mock('../../../models/Pod', () => ({ findById: jest.fn() })); jest.mock('../../../models/Integration', () => ({ - findOne: jest.fn(), findOneAndUpdate: jest.fn(), updateOne: jest.fn(), + findOne: jest.fn(), findOneAndUpdate: jest.fn(), findById: jest.fn(), updateOne: jest.fn(), })); jest.mock('../../../models/InstallableInstallation', () => ({ findOne: jest.fn() })); jest.mock('../../../utils/secret', () => ({ randomSecret: jest.fn(() => 'nonce-value') })); @@ -22,9 +22,16 @@ jest.mock('../../../services/telegramConnectCode', () => ({ mintConnectCode: jest.fn(() => ({ connectCode: 'r'.repeat(32), connectCodeExpiresAt: new Date(Date.now() + 60_000) })), })); jest.mock('../../../services/connectorSecrets', () => ({ get: jest.fn(), put: jest.fn(), revoke: jest.fn() })); -jest.mock('../../../services/slackApi', () => jest.fn().mockImplementation(() => ({ - openConversation: jest.fn(), -}))); +// Same reason as slackBridgeService.test.js: the constructor is stubbed, the +// escape is real — the connect marker's escaping is what this file asserts. +jest.mock('../../../services/slackApi', () => { + const actual = jest.requireActual('../../../services/slackApi'); + const mock = jest.fn().mockImplementation(() => ({ + openConversation: jest.fn(), + })); + mock.escapeSlackMrkdwn = actual.escapeSlackMrkdwn; + return mock; +}); jest.mock('../../../services/slackOAuthService', () => { class SlackOAuthConfigurationError extends Error { constructor() { super('not configured'); this.code = 'slack_oauth_not_configured'; } } class SlackOAuthExchangeError extends Error { constructor() { super('exchange failed'); this.code = 'slack_oauth_exchange_failed'; } } @@ -50,6 +57,7 @@ const InstallableInstallation = require('../../../models/InstallableInstallation const connectorSecrets = require('../../../services/connectorSecrets'); const { mintConnectCode } = require('../../../services/telegramConnectCode'); const SlackApi = require('../../../services/slackApi'); +const deliveryFailures = require('../../../services/connectorDeliveryFailureService'); const slackOAuth = require('../../../services/slackOAuthService'); const installableRoutes = require('../../../routes/installables'); @@ -230,7 +238,9 @@ describe('Slack installable OAuth routes', () => { Pod.findById .mockResolvedValueOnce({ createdBy: { toString: () => 'another' }, members: [ownerId] }) .mockReturnValueOnce({ - select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ name: 'Launch' }) }), + // Purposefully hostile: this name is interpolated into the marker a + // human reads in Slack, and mrkdwn would draw `<...>` as markup. + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ name: '' }) }), }); Integration.findOneAndUpdate.mockResolvedValue({ ...integration, @@ -252,7 +262,74 @@ describe('Slack installable OAuth routes', () => { ); expect(JSON.stringify(response.body)).not.toContain('secret-ref'); expect(JSON.stringify(response.body)).not.toContain('nonce-never-return'); - expect(SlackApi.mock.results[0].value.postMessage).toHaveBeenCalledWith('D1', '[Launch] connected'); + expect(SlackApi.mock.results[0].value.postMessage).toHaveBeenCalledWith('D1', '[<Evil|pod>] connected'); + }); + + test('a reconnect clears the reason an earlier flip left on the row (wren 73838)', async () => { + const pending = { + teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref', + expiresAt: new Date(Date.now() + 60_000), + }; + Integration.findOne.mockResolvedValue({ ...integration, config: { pendingBind: pending } }); + Pod.findById + .mockResolvedValueOnce({ createdBy: { toString: () => 'another' }, members: [ownerId] }) + .mockReturnValueOnce({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ name: 'Pod' }) }) }); + Integration.findOneAndUpdate.mockResolvedValue({ ...integration, status: 'connected', config: { ...pending, chatType: 'im' } }); + connectorSecrets.get.mockResolvedValue('xoxb-secret'); + SlackApi.mockImplementationOnce(() => ({ postMessage: jest.fn().mockResolvedValue({ ok: true, ts: '1.1' }) })); + + const response = await request(app).post('/api/installables/slack/confirm'); + + expect(response.status).toBe(200); + expect(response.body.status).toBe('connected'); + // A bind is the only thing that proves the connector works again, so it is + // the only thing that may clear what a flip wrote — otherwise a reconnect + // after any failure keeps naming a reason that is no longer true. + expect(Integration.findOneAndUpdate).toHaveBeenCalledWith( + expect.objectContaining({ _id: 'integration-1' }), + expect.objectContaining({ $set: expect.objectContaining({ errorMessage: null, errorMessageUserFacing: false }) }), + expect.anything(), + ); + }); + + test('a permanent refusal on the confirmation marker flips the connector and names the reason (wren 73837)', async () => { + const pending = { + teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref', + expiresAt: new Date(Date.now() + 60_000), + }; + // The marker goes to the chat this confirm just stored, so this is a + // bound-chat send — the twin of the Telegram bind confirmation. + Integration.findOne.mockResolvedValue({ ...integration, config: { pendingBind: pending } }); + Pod.findById + .mockResolvedValueOnce({ createdBy: { toString: () => 'another' }, members: [ownerId] }) + .mockReturnValueOnce({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ name: 'Pod' }) }) }); + Integration.findOneAndUpdate.mockResolvedValue({ ...integration, status: 'connected', config: { ...pending, chatType: 'im' } }); + connectorSecrets.get.mockResolvedValue('xoxb-secret'); + SlackApi.mockImplementationOnce(() => ({ postMessage: jest.fn().mockResolvedValue({ ok: false, error: 'channel_not_found' }) })); + // The route re-reads after a flip, so the response cannot keep saying + // "connected" while the row says the connector needs attention. + Integration.findById.mockResolvedValue({ + ...integration, status: 'error', errorMessage: deliveryFailures.SLACK_CHANNEL_GONE_REASON, config: {}, + }); + + const response = await request(app).post('/api/installables/slack/confirm'); + + expect(response.status).toBe(200); + expect(response.body.status).toBe('error'); + expect(Integration.findOneAndUpdate).toHaveBeenCalledWith( + { _id: 'integration-1', 'config.chatId': 'D1' }, + { + $set: { + status: 'error', + errorMessage: deliveryFailures.SLACK_CHANNEL_GONE_REASON, + // The page renders the message only when this flag is set, so the + // marker's own refusal has to write the pair, not just the string. + errorMessageUserFacing: true, + }, + $unset: { 'config.chatId': '' }, + }, + { new: true }, + ); }); test('rejects a pending Slack bind and revokes its secret reference', async () => { diff --git a/backend/__tests__/unit/routes/telegram.webhook.connectCode.test.js b/backend/__tests__/unit/routes/telegram.webhook.connectCode.test.js index cd890e66c1..f868e0cbb0 100644 --- a/backend/__tests__/unit/routes/telegram.webhook.connectCode.test.js +++ b/backend/__tests__/unit/routes/telegram.webhook.connectCode.test.js @@ -9,7 +9,12 @@ jest.mock('../../../models/Pod'); jest.mock('../../../models/Summary', () => ({ findOne: jest.fn() })); jest.mock('../../../services/integrationSummaryService', () => ({ createSummary: jest.fn() })); jest.mock('../../../services/agentEventService', () => ({ enqueue: jest.fn() })); -jest.mock('../../../services/telegramService', () => ({ sendMessage: jest.fn() })); +// Stub the network, keep the behaviour: the bridge escapes through this module's +// escapeHtml, so a bare stub leaves it undefined and the send is swallowed. +jest.mock('../../../services/telegramService', () => ({ + ...jest.requireActual('../../../services/telegramService'), + sendMessage: jest.fn(), +})); jest.mock('../../../integrations', () => ({ get: jest.fn() })); jest.mock('../../../services/telegramBridgeService', () => ({ relayTelegramMessageToPod: jest.fn() })); jest.mock('../../../models/WebhookDelivery', () => ({ diff --git a/backend/__tests__/unit/routes/telegram.webhook.test.js b/backend/__tests__/unit/routes/telegram.webhook.test.js index cd61a404b8..1079ba7d74 100644 --- a/backend/__tests__/unit/routes/telegram.webhook.test.js +++ b/backend/__tests__/unit/routes/telegram.webhook.test.js @@ -6,7 +6,13 @@ jest.mock('../../../models/Pod'); jest.mock('../../../models/Summary', () => ({ findOne: jest.fn() })); jest.mock('../../../services/integrationSummaryService', () => ({ createSummary: jest.fn() })); jest.mock('../../../services/agentEventService', () => ({ enqueue: jest.fn() })); -jest.mock('../../../services/telegramService', () => ({ sendMessage: jest.fn() })); +// `sendMessage` is stubbed because it is the network; `escapeHtml` must stay +// real, because escaping is behaviour this route is responsible for and a mock +// that dropped it turned every escaped send into a 500 (vera 73812). +jest.mock('../../../services/telegramService', () => ({ + ...jest.requireActual('../../../services/telegramService'), + sendMessage: jest.fn(), +})); jest.mock('../../../integrations', () => ({ get: jest.fn() })); jest.mock('../../../services/telegramBridgeService', () => ({ relayTelegramMessageToPod: jest.fn(), @@ -82,7 +88,16 @@ describe('Telegram webhook routes', () => { 'config.chatId': '42', 'config.chatType': 'group', }), + // The bind is the event that clears a named failure, and only a bind + // knows the connector works again (wren 73779). The flag is the pair's + // other half: it is what the page reads before rendering the message. + errorMessage: null, + errorMessageUserFacing: false, }), + // `new: true` is read back by the confirmation's classification: without + // it the doc in hand still carries the PRE-bind chat id, and a permanent + // failure of that confirmation could never undo the bind it just wrote. + { new: true }, ); expect(telegramService.sendMessage).toHaveBeenCalledWith( 'bot-token', @@ -91,6 +106,34 @@ describe('Telegram webhook routes', () => { ); }); + it('escapes a pod name in the bind confirmation, which Telegram parses as HTML', async () => { + const integration = { + _id: 'integration-1', + podId: 'pod-1', + config: { connectCode: 'abc123', connectCodeExpiresAt: new Date(Date.now() + 60000) }, + }; + Integration.findOne = jest.fn().mockResolvedValueOnce(integration).mockResolvedValueOnce(null); + Integration.findByIdAndUpdate = jest.fn().mockResolvedValue({ config: { chatId: '42' } }); + Pod.findById.mockReturnValue({ + lean: jest.fn().mockResolvedValue({ name: 'R&D ops' }), + }); + + const res = await request(app) + .post('/api/webhooks/telegram') + .send({ + message: { + text: '/commonly-enable abc123', + chat: { id: 42, title: 'Test Chat', type: 'group' }, + from: { id: 7, first_name: 'Sam' }, + }, + }); + + expect(res.status).toBe(200); + const text = telegramService.sendMessage.mock.calls[0][2]; + expect(text).toContain('Connected this chat to R&D <b>ops</b>'); + expect(text).not.toContain('ops'); + }); + it('posts integration summary on /summary', async () => { const integration = { _id: 'integration-1', @@ -285,7 +328,7 @@ describe('bridge command surface (/mode /status /mute /help)', () => { expect(res.status).toBe(200); const [, update] = Integration.findByIdAndUpdate.mock.calls[0]; const until = new Date(update.$set['config.relayMutedUntil']).getTime(); - expect(until).toBeLessThanOrEqual(Date.now() + 24 * 60 * 60_000 + 5000); + expect(until).toBeLessThanOrEqual(Date.now() + 24 * 60 * 60000 + 5000); expect(until).toBeGreaterThan(Date.now()); }); @@ -310,6 +353,52 @@ describe('bridge command surface (/mode /status /mute /help)', () => { ); }); + it('/status escapes the pod name and the lead agent handle it reports', async () => { + Integration.findOne = jest.fn().mockResolvedValue({ + ...linked(), + config: { + ...linked().config, + leadAgentUsername: 'lead', + }, + }); + Pod.findById.mockReturnValue({ + lean: jest.fn().mockResolvedValue({ name: 'R&D chat' }), + }); + + const res = await post('/status'); + + expect(res.status).toBe(200); + const text = telegramService.sendMessage.mock.calls[0][2]; + // Telegram HTML accepts , so an unescaped name renders a clickable + // link inside a message the user reads as the bot's own words. + expect(text).toContain('R&D <b>chat</b>'); + expect(text).not.toContain('chat'); + expect(text).toContain('<a href="https://evil.example">lead</a>'); + expect(text).not.toContain(''); + }); + + it('/tldr escapes the summary title and body, which are generated from pod text', async () => { + Integration.findOne = jest.fn().mockResolvedValue(linked()); + Summary.findOne.mockReturnValue({ + sort: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue({ + title: 'Sprint ', + content: 'Agent said ship it & left', + }), + }), + }); + + const res = await post('/tldr'); + + expect(res.status).toBe(200); + const text = telegramService.sendMessage.mock.calls[0][2]; + expect(text).toContain('<script>alert(1)</script>'); + expect(text).not.toContain('