diff --git a/backend/__tests__/service/integrations-e2e.test.js b/backend/__tests__/service/integrations-e2e.test.js index 3f95c9c843..de4f9636a8 100644 --- a/backend/__tests__/service/integrations-e2e.test.js +++ b/backend/__tests__/service/integrations-e2e.test.js @@ -31,6 +31,7 @@ const DiscordIntegration = require('../../models/DiscordIntegration'); // Services const AgentEventService = require('../../services/agentEventService'); const IntegrationSummaryService = require('../../services/integrationSummaryService'); +const { storeGroupmeAccessToken } = require('../../services/chatConnectorTokens'); const schedulerService = require('../../services/schedulerService'); // SchedulerService is exported as an instance, static methods are on constructor @@ -1671,13 +1672,21 @@ describe('Integrations E2E Tests', () => { .set('Authorization', `Bearer ${authToken}`) .send({ label: 'GroupMe Messages Token' }); + // TASK-539: the token lives in connectorSecrets; the row keeps the ref. + const savedKeys = process.env.CONNECTOR_SECRET_KEYS; + const savedActiveKey = process.env.CONNECTOR_SECRET_ACTIVE_KEY; + process.env.CONNECTOR_SECRET_KEYS = `k1:${Buffer.alloc(32, 5).toString('base64')}`; + process.env.CONNECTOR_SECRET_ACTIVE_KEY = 'k1'; + const integrationId = new mongoose.Types.ObjectId(); + const refs = await storeGroupmeAccessToken(String(integrationId), 'groupme-token-xyz'); const integration = await Integration.create({ + _id: integrationId, podId: testPod._id, type: 'groupme', status: 'connected', config: { groupId: 'group-xyz', - accessToken: 'groupme-token-xyz', + ...refs, agentAccessEnabled: true, }, createdBy: testUser._id, @@ -1706,9 +1715,13 @@ describe('Integrations E2E Tests', () => { .get(`/api/agents/runtime/pods/${testPod._id}/integrations/${integration._id}/messages?limit=20`) .set('Authorization', `Bearer ${tokenRes.body.token}`); + process.env.CONNECTOR_SECRET_KEYS = savedKeys; + process.env.CONNECTOR_SECRET_ACTIVE_KEY = savedActiveKey; + expect(res.status).toBe(200); expect(res.body.messages).toHaveLength(1); expect(res.body.messages[0].authorName).toBe('bob'); + expect(JSON.stringify(axios.get.mock.calls)).toContain('groupme-token-xyz'); }); }); }); diff --git a/backend/__tests__/unit/integrations/slackProvider.bindingShape.test.js b/backend/__tests__/unit/integrations/slackProvider.bindingShape.test.js index 12087f43c6..9703e7c19c 100644 --- a/backend/__tests__/unit/integrations/slackProvider.bindingShape.test.js +++ b/backend/__tests__/unit/integrations/slackProvider.bindingShape.test.js @@ -70,14 +70,13 @@ describe('slackProvider — the row shape the bind writes', () => { }); }); - it('still answers for a legacy row from its own channel and token', async () => { + it('does not read a legacy row\'s stored token copy; only the ref is a source (TASK-539)', async () => { mockHistory.mockResolvedValue({ messages: [] }); - await legacyRow().syncRecent({ hours: 1 }); - + await expect(legacyRow().syncRecent({ hours: 1 })).rejects.toThrow(/missing its bot token/); expect(mockGet).not.toHaveBeenCalled(); - expect(mockSlackApi).toHaveBeenCalledWith('xoxb-legacy'); - expect(mockHistory.mock.calls[0][0]).toBe('C0456'); + expect(mockSlackApi).not.toHaveBeenCalled(); + await expect(legacyRow().health()).resolves.toEqual({ ok: false, error: 'Slack connector has no bot token' }); }); it('does not fall back to the instance token, which is retired (TASK-151)', async () => { diff --git a/backend/__tests__/unit/routes/agentsRuntime.integrations.messages.test.js b/backend/__tests__/unit/routes/agentsRuntime.integrations.messages.test.js index 90fa44094e..e8fdf326f8 100644 --- a/backend/__tests__/unit/routes/agentsRuntime.integrations.messages.test.js +++ b/backend/__tests__/unit/routes/agentsRuntime.integrations.messages.test.js @@ -28,9 +28,17 @@ jest.mock('../../../services/dmService', () => ({ getOrCreateAgentDM: jest.fn() jest.mock('../../../models/AgentRegistry', () => ({ AgentInstallation: { findOne: jest.fn(), find: jest.fn() } })); // TASK-124: the discord branch's fetch is asserted to receive the env token. jest.mock('../../../services/discordService', () => ({ fetchMessages: jest.fn() })); +// TASK-539: the groupme branch reads its token through connectorSecrets. +jest.mock('../../../services/groupmeService', () => ({ fetchMessages: jest.fn() })); +jest.mock('../../../services/connectorSecrets', () => { + class ConnectorSecretNotFoundError extends Error {} + return { get: jest.fn(), ConnectorSecretNotFoundError }; +}); const Integration = require('../../../models/Integration'); const DiscordService = require('../../../services/discordService'); +const GroupMeService = require('../../../services/groupmeService'); +const connectorSecrets = require('../../../services/connectorSecrets'); const router = require('../../../routes/agentsRuntime'); const getRouteHandler = (path, method) => { @@ -197,4 +205,97 @@ describe('agentsRuntime integration messages route', () => { expect(DiscordService.fetchMessages).not.toHaveBeenCalled(); }); }); + // TASK-539: the token lives in connectorSecrets, and the row's own copy is + // never read, so a stale plaintext token cannot be served. + describe('groupme: the token comes from the connector secret', () => { + const groupmeRow = (config = {}) => ({ + lean: jest.fn().mockResolvedValue({ + _id: 'groupme-1', + type: 'groupme', + config: { + agentAccessEnabled: true, globalAgentAccess: true, groupId: 'group-1', ...config, + }, + }), + }); + + const call = async () => { + const handler = getRouteHandler('/pods/:podId/integrations/:integrationId/messages', 'get'); + const req = { + params: { podId: 'pod-1', integrationId: 'groupme-1' }, + query: {}, + agentInstallation: { podId: 'pod-1', scopes: ['integration:messages:read'] }, + agentInstallations: [{ podId: 'pod-1', scopes: ['integration:messages:read'] }], + }; + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + await handler(req, res); + return res; + }; + + it('fetches with the secret behind the ref, not the row copy', async () => { + connectorSecrets.get.mockResolvedValue('groupme-secret-token'); + GroupMeService.fetchMessages.mockResolvedValue([]); + Integration.findOne.mockReturnValue(groupmeRow({ accessTokenRef: 'ref-1', accessToken: 'stale-copy' })); + + const res = await call(); + + expect(connectorSecrets.get).toHaveBeenCalledWith('ref-1'); + expect(GroupMeService.fetchMessages).toHaveBeenCalledWith( + expect.objectContaining({ accessToken: 'groupme-secret-token', groupId: 'group-1' }), + ); + expect(res.status).not.toHaveBeenCalledWith(400); + }); + + it('refuses a row that carries only a plaintext copy', async () => { + Integration.findOne.mockReturnValue(groupmeRow({ accessToken: 'plaintext-only' })); + + const res = await call(); + + expect(res.status).toHaveBeenCalledWith(400); + expect(GroupMeService.fetchMessages).not.toHaveBeenCalled(); + }); + + it('refuses, rather than 500s, when the secret behind the ref is gone', async () => { + connectorSecrets.get.mockRejectedValue(new connectorSecrets.ConnectorSecretNotFoundError('gone')); + Integration.findOne.mockReturnValue(groupmeRow({ accessTokenRef: 'ref-gone' })); + + const res = await call(); + + expect(res.status).toHaveBeenCalledWith(400); + expect(GroupMeService.fetchMessages).not.toHaveBeenCalled(); + }); + + it('answers 500, not "missing token", when the key ring or decryption fails', async () => { + connectorSecrets.get.mockRejectedValue(new Error('key ring unavailable')); + Integration.findOne.mockReturnValue(groupmeRow({ accessTokenRef: 'ref-1' })); + + const res = await call(); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.status).not.toHaveBeenCalledWith(400); + expect(GroupMeService.fetchMessages).not.toHaveBeenCalled(); + }); + + it('logs a failed fetch without the request config that carries the token', async () => { + const util = require('util'); // eslint-disable-line global-require + connectorSecrets.get.mockResolvedValue('groupme-secret-token'); + GroupMeService.fetchMessages.mockRejectedValue(Object.assign( + new Error('Request failed with status code 401'), + { config: { params: { token: 'groupme-secret-token' } }, response: { status: 401 } }, + )); + Integration.findOne.mockReturnValue(groupmeRow({ accessTokenRef: 'ref-1' })); + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + try { + const res = await call(); + + expect(res.status).toHaveBeenCalledWith(500); + expect(spy).toHaveBeenCalled(); + spy.mock.calls.forEach((args) => { + expect(util.inspect(args, { depth: 10 })).not.toContain('groupme-secret-token'); + }); + expect(JSON.stringify(res.json.mock.calls)).not.toContain('groupme-secret-token'); + } finally { + spy.mockRestore(); + } + }); + }); }); diff --git a/backend/__tests__/unit/scripts/migrateChatTokenCopies.test.js b/backend/__tests__/unit/scripts/migrateChatTokenCopies.test.js new file mode 100644 index 0000000000..2c08f5da17 --- /dev/null +++ b/backend/__tests__/unit/scripts/migrateChatTokenCopies.test.js @@ -0,0 +1,194 @@ +jest.mock('jsonwebtoken', () => ({})); + +const mongoose = require('mongoose'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const ConnectorSecret = require('../../../models/ConnectorSecret'); +const connectorSecrets = require('../../../services/connectorSecrets'); +const { GROUPME_ACCESS_TOKEN } = require('../../../services/connectorSecretKinds'); +const { readGroupmeAccessToken } = require('../../../services/chatConnectorTokens'); +const { migrateChatTokenCopies, formatReport } = require('../../../scripts/migrate-chat-token-copies'); +const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../../utils/testUtils'); + +const rows = () => mongoose.connection.collection('integrations'); +const key = (fill) => Buffer.alloc(32, fill).toString('base64'); +const GM = 'groupme-plaintext-sentinel'; +const GM_OLD = 'groupme-stale-copy-sentinel'; +const SLACK = 'slack-plaintext-sentinel'; +const REFRESH = 'refresh-plaintext-sentinel'; + +const seed = async () => { + const ids = { + move: new mongoose.Types.ObjectId(), + hasRef: new mongoose.Types.ObjectId(), + blank: new mongoose.Types.ObjectId(), + clean: new mongoose.Types.ObjectId(), + slack: new mongoose.Types.ObjectId(), + slackBound: new mongoose.Types.ObjectId(), + x: new mongoose.Types.ObjectId(), + }; + const existingRef = await connectorSecrets.put(String(ids.hasRef), GROUPME_ACCESS_TOKEN, 'groupme-current-token'); + await rows().insertMany([ + { + _id: ids.move, + type: 'groupme', + config: { + botId: 'b1', groupId: 'g1', accessToken: GM, refreshToken: REFRESH, + }, + }, + { _id: ids.hasRef, type: 'groupme', config: { botId: 'b2', accessToken: GM_OLD, accessTokenRef: existingRef } }, + { _id: ids.blank, type: 'groupme', config: { botId: 'b3', accessToken: '', refreshToken: null } }, + { _id: ids.clean, type: 'groupme', config: { botId: 'b4' } }, + { _id: ids.slack, type: 'slack', config: { accessToken: SLACK, refreshToken: REFRESH, signingSecret: 'kept-signing' } }, + { _id: ids.slackBound, type: 'slack', config: { botTokenRef: 'slack-ref', botToken: 'kept-bot-token' } }, + { _id: ids.x, type: 'x', config: { accessToken: 'x-not-ours', feedAccessTokenRef: 'x-ref' } }, + ]); + return { ids, existingRef }; +}; + +const byId = async (id) => rows().findOne({ _id: id }); + +describe('migrate GroupMe and Slack token copies', () => { + const saved = { + keys: process.env.CONNECTOR_SECRET_KEYS, + activeKey: process.env.CONNECTOR_SECRET_ACTIVE_KEY, + }; + + beforeAll(async () => { + await setupMongoDb(); + await ConnectorSecret.syncIndexes(); + }); + afterAll(async () => { + await closeMongoDb(); + process.env.CONNECTOR_SECRET_KEYS = saved.keys; + process.env.CONNECTOR_SECRET_ACTIVE_KEY = saved.activeKey; + }); + beforeEach(async () => { + await clearMongoDb(); + process.env.CONNECTOR_SECRET_KEYS = `k1:${key(9)}`; + process.env.CONNECTOR_SECRET_ACTIVE_KEY = 'k1'; + }); + + it('defaults to dry run: counts every field by type, writes nothing, prints no value', async () => { + const { existingRef } = await seed(); + const before = await rows().find().toArray(); + const secretsBefore = await ConnectorSecret.countDocuments(); + const report = await migrateChatTokenCopies(); + expect(report).toEqual({ + groupme: { + candidates: 3, + accessFields: 3, + refreshFields: 2, + toMove: 1, + moved: 0, + cleared: 0, + remaining: 3, + inventory: { botToken: 0, signingSecret: 0, appToken: 0 }, + }, + slack: { + candidates: 1, + accessFields: 1, + refreshFields: 1, + toMove: 0, + moved: 0, + cleared: 0, + remaining: 1, + inventory: { botToken: 1, signingSecret: 1, appToken: 0 }, + }, + }); + expect(await rows().find().toArray()).toEqual(before); + expect(await ConnectorSecret.countDocuments()).toBe(secretsBefore); + const output = formatReport(report, false).join('\n'); + expect(output).toContain('DRY-RUN'); + [GM, GM_OLD, SLACK, REFRESH, existingRef, 'kept-bot-token'].forEach((value) => ( + expect(output).not.toContain(value) + )); + }); + + it('moves a GroupMe token into connectorSecrets, keeps an existing ref, and scrubs both types', async () => { + const { ids, existingRef } = await seed(); + const before = await rows().find().toArray(); + const report = await migrateChatTokenCopies({ apply: true }); + expect(report.groupme).toMatchObject({ + toMove: 1, moved: 1, cleared: 3, remaining: 0, + }); + expect(report.slack).toMatchObject({ cleared: 1, remaining: 0 }); + + const moved = await byId(ids.move); + expect(moved.config).toEqual({ botId: 'b1', groupId: 'g1', accessTokenRef: expect.any(String) }); + expect(await readGroupmeAccessToken(moved.config)).toBe(GM); + + // The ref is authoritative: an older plaintext copy never replaces it. + const kept = await byId(ids.hasRef); + expect(kept.config).toEqual({ botId: 'b2', accessTokenRef: existingRef }); + expect(await readGroupmeAccessToken(kept.config)).toBe('groupme-current-token'); + + expect((await byId(ids.blank)).config).toEqual({ botId: 'b3' }); + expect((await byId(ids.slack)).config).toEqual({ signingSecret: 'kept-signing' }); + + // Rows with nothing to do, and other types, are untouched. + const untouched = await Promise.all([ids.clean, ids.slackBound, ids.x].map(byId)); + untouched.forEach((row) => expect(row).toEqual(before.find((entry) => entry._id.equals(row._id)))); + const output = formatReport(report, true).join('\n'); + [GM, GM_OLD, SLACK, REFRESH, existingRef].forEach((value) => expect(output).not.toContain(value)); + }); + + it('is idempotent after apply', async () => { + await seed(); + await migrateChatTokenCopies({ apply: true }); + const once = await rows().find().toArray(); + const secrets = await ConnectorSecret.countDocuments(); + const report = await migrateChatTokenCopies({ apply: true }); + expect(report.groupme).toMatchObject({ + candidates: 0, toMove: 0, moved: 0, cleared: 0, remaining: 0, + }); + expect(report.slack).toMatchObject({ candidates: 0, cleared: 0, remaining: 0 }); + expect(await rows().find().toArray()).toEqual(once); + expect(await ConnectorSecret.countDocuments()).toBe(secrets); + }); + + it('keeps a token a live edit stored after the read, instead of writing the older copy over it', async () => { + const id = new mongoose.Types.ObjectId(); + await rows().insertOne({ _id: id, type: 'groupme', config: { botId: 'b5', accessToken: GM_OLD } }); + // A PATCH has stored its new token but not yet set the row's ref. + const liveRef = await connectorSecrets.put(String(id), GROUPME_ACCESS_TOKEN, 'groupme-live-edit'); + const report = await migrateChatTokenCopies({ apply: true }); + expect(report.groupme).toMatchObject({ + toMove: 1, moved: 0, cleared: 1, remaining: 0, + }); + const row = await byId(id); + expect(row.config).toEqual({ botId: 'b5', accessTokenRef: liveRef }); + expect(await readGroupmeAccessToken(row.config)).toBe('groupme-live-edit'); + }); + + it('stops before unsetting a GroupMe token it could not encrypt', async () => { + const { ids } = await seed(); + delete process.env.CONNECTOR_SECRET_KEYS; + delete process.env.CONNECTOR_SECRET_ACTIVE_KEY; + await expect(migrateChatTokenCopies({ apply: true })).rejects.toThrow(); + const row = await byId(ids.move); + expect(row.config.accessToken).toBe(GM); + expect(row.config.accessTokenRef).toBeUndefined(); + }); +}); + +describe('migrate CLI failure output', () => { + it.each([{ args: [] }, { args: ['--unexpected-sensitive-argument'] }])('fails closed without echoing sensitive input ($args)', ({ args }) => { + const secretUri = 'invalid-secret-uri-sentinel'; + const result = spawnSync(process.execPath, [ + '-r', require.resolve('ts-node/register'), + path.resolve(__dirname, '../../../scripts/migrate-chat-token-copies.ts'), + ...args, + ], { + cwd: path.resolve(__dirname, '../../..'), + env: { ...process.env, MONGO_URI: secretUri }, + encoding: 'utf8', + timeout: 30000, + }); + expect(result.status).toBe(1); + expect(result.stdout).toBe(''); + expect(result.stderr).toBe('[migrate-chat-token-copies] Failed. No values logged; rerun dry-run to inspect counts.\n'); + expect(result.stderr).not.toContain(secretUri); + expect(result.stderr).not.toContain('unexpected-sensitive-argument'); + }); +}); diff --git a/backend/__tests__/unit/services/chatConnectorTokens.test.js b/backend/__tests__/unit/services/chatConnectorTokens.test.js new file mode 100644 index 0000000000..56bf658649 --- /dev/null +++ b/backend/__tests__/unit/services/chatConnectorTokens.test.js @@ -0,0 +1,153 @@ +// TASK-539: the GroupMe access token is stored through connectorSecrets, and a +// Slack row's accessToken / refreshToken copy (which nothing reads) is dropped. +// Real Mongo, real models and the real generic connector routes. Each case reads +// the RAW stored row, so a token written to `config` under any key fails. +jest.mock('../../../middleware/auth', () => (req, res, next) => { + req.user = { id: 'bbbbbbbbbbbbbbbbbbbbbb01' }; + req.userId = req.user.id; + next(); +}); + +const express = require('express'); +const request = require('supertest'); +const Pod = require('../../../models/Pod'); +const User = require('../../../models/User'); +const Integration = require('../../../models/Integration'); +const ConnectorSecret = require('../../../models/ConnectorSecret'); +const integrationRoutes = require('../../../routes/integrations'); +const { toPublicIntegrationConfig } = require('../../../models/integrationPublicConfig'); +const { takeChatTokens, readGroupmeAccessToken } = require('../../../services/chatConnectorTokens'); +const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../../utils/testUtils'); + +const key = (fill) => Buffer.alloc(32, fill).toString('base64'); +const MEMBER = 'bbbbbbbbbbbbbbbbbbbbbb01'; +const rawRow = (type) => Integration.collection.findOne({ type }); + +const expectNoMaterialOnRow = (row, ...materials) => { + expect(row.config.accessToken).toBeUndefined(); + expect(row.config.refreshToken).toBeUndefined(); + const serialized = JSON.stringify(row); + materials.forEach((material) => expect(serialized).not.toContain(material)); +}; + +describe('GroupMe and Slack token copies', () => { + const saved = { + keys: process.env.CONNECTOR_SECRET_KEYS, + activeKey: process.env.CONNECTOR_SECRET_ACTIVE_KEY, + }; + let app; + let pod; + + beforeAll(async () => { + await setupMongoDb(); + await ConnectorSecret.syncIndexes(); + }); + + afterAll(async () => { + await closeMongoDb(); + process.env.CONNECTOR_SECRET_KEYS = saved.keys; + process.env.CONNECTOR_SECRET_ACTIVE_KEY = saved.activeKey; + }); + + beforeEach(async () => { + await clearMongoDb(); + process.env.CONNECTOR_SECRET_KEYS = `k1:${key(7)}`; + process.env.CONNECTOR_SECRET_ACTIVE_KEY = 'k1'; + // The edit gate reads the caller's User row. + await User.create({ + _id: MEMBER, username: 'chat-owner', email: 'chat-owner@example.test', password: 'unused-password', + }); + pod = await Pod.create({ name: 'Chats', createdBy: MEMBER, members: [MEMBER] }); + app = express(); + app.use(express.json()); + app.use('/api/integrations', integrationRoutes); + }); + + it('stores a GroupMe token a generic create or edit supplies, and a blank edit keeps it', async () => { + const created = await request(app).post('/api/integrations').send({ + podId: String(pod._id), + type: 'groupme', + config: { + botId: 'bot-1', groupId: 'group-1', accessToken: 'gm-access-1', refreshToken: 'gm-refresh-1', + }, + }); + expect(created.status).toBe(201); + const row = await rawRow('groupme'); + expectNoMaterialOnRow(row, 'gm-access-1', 'gm-refresh-1'); + expect(row.config.botId).toBe('bot-1'); + expect(await readGroupmeAccessToken(row.config)).toBe('gm-access-1'); + + const rotated = await request(app).patch(`/api/integrations/${row._id}`).send({ + config: { accessToken: 'gm-access-2', groupName: 'Group' }, + }); + expect(rotated.status).toBe(200); + const after = await rawRow('groupme'); + expectNoMaterialOnRow(after, 'gm-access-1', 'gm-access-2'); + // The store upserts on (integrationId, kind), so a rotation keeps the ref. + expect(after.config.accessTokenRef).toBe(row.config.accessTokenRef); + expect(after.config.groupName).toBe('Group'); + expect(await readGroupmeAccessToken(after.config)).toBe('gm-access-2'); + + // ChatRoom sends `accessToken: undefined` (dropped by JSON) or '' when the + // field is left blank; neither may clear the stored token. + const blank = await request(app).patch(`/api/integrations/${row._id}`).send({ + config: { accessToken: '', botId: 'bot-2' }, + }); + expect(blank.status).toBe(200); + const kept = await rawRow('groupme'); + expect(kept.config.accessTokenRef).toBe(row.config.accessTokenRef); + expect(kept.config.accessToken).toBeUndefined(); + expect(kept.config.botId).toBe('bot-2'); + expect(await readGroupmeAccessToken(kept.config)).toBe('gm-access-2'); + }); + + it('refuses a body-supplied accessTokenRef, so a row cannot point at another row\'s secret', async () => { + const created = await request(app).post('/api/integrations').send({ + podId: String(pod._id), + type: 'groupme', + config: { botId: 'bot-1', groupId: 'group-1', accessToken: 'gm-victim' }, + }); + expect(created.status).toBe(201); + const victim = await rawRow('groupme'); + + const planted = await request(app).post('/api/integrations').send({ + podId: String(pod._id), + type: 'groupme', + config: { botId: 'bot-9', groupId: 'group-9', accessTokenRef: victim.config.accessTokenRef }, + }); + expect(planted.status).toBe(201); + const row = await Integration.collection.findOne({ 'config.botId': 'bot-9' }); + expect(row.config.accessTokenRef).toBeUndefined(); + + const edited = await request(app).patch(`/api/integrations/${row._id}`).send({ + config: { accessTokenRef: victim.config.accessTokenRef }, + }); + expect(edited.status).toBe(200); + expect((await Integration.collection.findOne({ _id: row._id })).config.accessTokenRef).toBeUndefined(); + }); + + it('drops a Slack accessToken / refreshToken copy instead of storing it', async () => { + const row = await Integration.create({ + podId: pod._id, type: 'slack', createdBy: MEMBER, status: 'pending', config: { channelId: 'C1' }, + }); + const edited = await request(app).patch(`/api/integrations/${row._id}`).send({ + config: { accessToken: 'slack-access-1', refreshToken: 'slack-refresh-1', channelName: 'general' }, + }); + expect(edited.status).toBe(200); + const after = await rawRow('slack'); + expectNoMaterialOnRow(after, 'slack-access-1', 'slack-refresh-1'); + expect(after.config.channelName).toBe('general'); + expect(after.config.accessTokenRef).toBeUndefined(); + expect(await ConnectorSecret.countDocuments()).toBe(0); + }); + + it('withholds the GroupMe ref from every JSON projection', () => { + expect(toPublicIntegrationConfig({ accessTokenRef: 'ref-1', groupId: 'g' })).toEqual({ groupId: 'g' }); + }); + + it('splits the token fields out of a supplied config, keeping only GroupMe\'s access token', () => { + const config = { accessToken: 'a', refreshToken: 'r', botId: 'b' }; + expect(takeChatTokens('groupme', config)).toEqual({ rest: { botId: 'b' }, accessToken: 'a' }); + expect(takeChatTokens('slack', config)).toEqual({ rest: { botId: 'b' } }); + }); +}); diff --git a/backend/__tests__/unit/services/connectorSecrets.test.js b/backend/__tests__/unit/services/connectorSecrets.test.js index 85dbabbf99..690a5fec62 100644 --- a/backend/__tests__/unit/services/connectorSecrets.test.js +++ b/backend/__tests__/unit/services/connectorSecrets.test.js @@ -6,11 +6,13 @@ const { ConnectorSecretStaleWrite, get, put, + putIfAbsent, rewrap, revoke, } = require('../../../services/connectorSecrets'); const { DISCORD_WEBHOOK_URL, + GROUPME_ACCESS_TOKEN, SLACK_BOT_TOKEN, } = require('../../../services/connectorSecretKinds'); const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../../utils/testUtils'); @@ -119,4 +121,26 @@ describe('connectorSecrets', () => { await revoke(ref); expect(await ConnectorSecret.findById(ref)).toBeNull(); }); + + // TASK-539: the import path. Insert-only, so a secret a live edit stored + // first is never replaced with an older copy. + it('putIfAbsent inserts when the pair is free and reports it created', async () => { + const integrationId = new mongoose.Types.ObjectId().toString(); + const { ref, created } = await putIfAbsent(integrationId, GROUPME_ACCESS_TOKEN, 'imported'); + + expect(created).toBe(true); + expect(await get(ref)).toBe('imported'); + }); + + it('putIfAbsent returns the existing ref and keeps its material when the pair is taken', async () => { + const integrationId = new mongoose.Types.ObjectId().toString(); + const live = await put(integrationId, GROUPME_ACCESS_TOKEN, 'live-edit'); + + const { ref, created } = await putIfAbsent(integrationId, GROUPME_ACCESS_TOKEN, 'older-copy'); + + expect(created).toBe(false); + expect(ref).toBe(live); + expect(await get(live)).toBe('live-edit'); + expect(await ConnectorSecret.countDocuments({ integrationId })).toBe(1); + }); }); diff --git a/backend/__tests__/unit/services/groupmeService.fetchMessages.test.js b/backend/__tests__/unit/services/groupmeService.fetchMessages.test.js new file mode 100644 index 0000000000..51d8e8990e --- /dev/null +++ b/backend/__tests__/unit/services/groupmeService.fetchMessages.test.js @@ -0,0 +1,56 @@ +/** + * TASK-539: GroupMe takes the access token as a query parameter, so a failed + * request's axios error carries it in `config.params` and the request URL. + * fetchMessages rethrows the status only, so a caller that logs the error object + * never logs the token. + */ +const util = require('util'); + +jest.mock('axios', () => ({ get: jest.fn(), post: jest.fn() })); +const axios = require('axios'); +const { fetchMessages } = require('../../../services/groupmeService'); + +const TOKEN = 'groupme-secret-token'; + +const axiosError = (status) => Object.assign(new Error(status + ? `Request failed with status code ${status}` + : 'socket hang up'), { + config: { params: { token: TOKEN }, url: `https://api.groupme.com/v3/groups/g1/messages?token=${TOKEN}` }, + request: { path: `/v3/groups/g1/messages?token=${TOKEN}` }, + ...(status ? { response: { status, config: { params: { token: TOKEN } } } } : {}), +}); + +describe('groupmeService.fetchMessages error shape', () => { + beforeEach(() => axios.get.mockReset()); + + it('rethrows an HTTP failure with its status and without the token', async () => { + axios.get.mockRejectedValue(axiosError(401)); + + const err = await fetchMessages({ groupId: 'g1', accessToken: TOKEN }).catch((e) => e); + + expect(err).toBeInstanceOf(Error); + expect(err.status).toBe(401); + expect(err.message).toBe('GroupMe request failed with status 401'); + expect(util.inspect(err, { depth: 10 })).not.toContain(TOKEN); + expect(JSON.stringify(err)).not.toContain(TOKEN); + }); + + it('rethrows a network failure without the token', async () => { + axios.get.mockRejectedValue(axiosError(undefined)); + + const err = await fetchMessages({ groupId: 'g1', accessToken: TOKEN }).catch((e) => e); + + expect(err.message).toBe('GroupMe request failed'); + expect(err.status).toBeUndefined(); + expect(util.inspect(err, { depth: 10 })).not.toContain(TOKEN); + }); + + it('still sends the token to GroupMe and normalizes a success', async () => { + axios.get.mockResolvedValue({ data: { response: { messages: [{ id: 'm1', name: 'A', text: 'hi' }] } } }); + + const messages = await fetchMessages({ groupId: 'g1', accessToken: TOKEN, limit: 5 }); + + expect(axios.get.mock.calls[0][1]).toEqual({ params: { token: TOKEN, limit: 5 } }); + expect(messages).toEqual([expect.objectContaining({ id: 'm1', authorName: 'A', content: 'hi' })]); + }); +}); diff --git a/backend/integrations/providers/slackProvider.ts b/backend/integrations/providers/slackProvider.ts index 6bc14e94cd..0acfbbea41 100644 --- a/backend/integrations/providers/slackProvider.ts +++ b/backend/integrations/providers/slackProvider.ts @@ -61,11 +61,10 @@ function createSlackProvider(integration: { _id: unknown; config?: Record => { const ref = config.botTokenRef; - if (ref) return connectorSecrets.get(String(ref)); - // The row's own stored copy is still honoured (a pre-TASK-139 row may carry - // one); the process environment is not read (TASK-151). - const legacy = String(config.botToken || '').trim(); - return legacy || undefined; + // The ref is the only source (TASK-539). A pre-TASK-139 row's own + // `config.botToken` copy is not read, and neither is the process + // environment (TASK-151); the bind writes a ref for every live row. + return ref ? connectorSecrets.get(String(ref)) : undefined; }; return { diff --git a/backend/models/Integration.ts b/backend/models/Integration.ts index 798551e49e..adb47d3c91 100644 --- a/backend/models/Integration.ts +++ b/backend/models/Integration.ts @@ -95,6 +95,7 @@ export interface IIntegration extends Document { accessToken?: string; refreshToken?: string; feedAccessTokenRef?: string; + accessTokenRef?: string; feedRefreshTokenRef?: string; tokenType?: string; tokenExpiresAt?: Date; @@ -271,6 +272,9 @@ const IntegrationSchema = new Schema( // drops an undeclared `$set` in silence. feedAccessTokenRef: String, feedRefreshTokenRef: String, + // The GroupMe access token's ConnectorSecret ref (`services/chatConnectorTokens`), + // declared for the same reason. + accessTokenRef: String, tokenType: String, tokenExpiresAt: Date, oauthScopes: [String], diff --git a/backend/models/integrationPublicConfig.ts b/backend/models/integrationPublicConfig.ts index fa86313172..0bb1ed807d 100644 --- a/backend/models/integrationPublicConfig.ts +++ b/backend/models/integrationPublicConfig.ts @@ -45,6 +45,8 @@ export const INTEGRATION_SECRET_CONFIG_KEYS = [ // Slack and hosted-MCP refs beside them. 'feedAccessTokenRef', 'feedRefreshTokenRef', + // The GroupMe access token's ref (`services/chatConnectorTokens`). + 'accessTokenRef', ] as const; export const toPublicIntegrationConfig = ( diff --git a/backend/routes/agentsRuntime.ts b/backend/routes/agentsRuntime.ts index bbc59c2fa3..5f083bf3f1 100644 --- a/backend/routes/agentsRuntime.ts +++ b/backend/routes/agentsRuntime.ts @@ -3747,13 +3747,24 @@ router.get('/pods/:podId/integrations/:integrationId/messages', phase4RateLimit, after, }); } else if (integration.type === 'groupme') { - if (!integration.config?.accessToken || !integration.config?.groupId) { + // The token lives in connectorSecrets; the row's own `accessToken` is not + // read (TASK-539). A ref whose secret is gone answers like a missing token. + // Any other failure (key ring, decryption, database) is a 500. + const { readGroupmeAccessToken } = require('../services/chatConnectorTokens'); + const { ConnectorSecretNotFoundError } = require('../services/connectorSecrets'); + let accessToken: string | undefined; + try { + accessToken = await readGroupmeAccessToken(integration.config); + } catch (error) { + if (!(error instanceof ConnectorSecretNotFoundError)) throw error; + } + if (!accessToken || !integration.config?.groupId) { return res.status(400).json({ message: 'GroupMe integration missing accessToken or groupId' }); } const GroupMeService = require('../services/groupmeService'); messages = await GroupMeService.fetchMessages({ groupId: integration.config.groupId, - accessToken: integration.config.accessToken, + accessToken, limit, before, after, @@ -3766,7 +3777,9 @@ router.get('/pods/:podId/integrations/:integrationId/messages', phase4RateLimit, return res.json({ messages }); } catch (error: any) { - console.error('Error fetching messages for agent:', error); + // The message only: a provider's HTTP error object carries its request + // config, which holds the credential (a query param or an auth header). + console.error('Error fetching messages for agent:', (error as Error)?.message); return res.status(500).json({ message: (error as Error).message || 'Failed to fetch messages' }); } }); diff --git a/backend/routes/integrations.ts b/backend/routes/integrations.ts index 21bfe2516d..0ada2c9062 100644 --- a/backend/routes/integrations.ts +++ b/backend/routes/integrations.ts @@ -24,6 +24,7 @@ const connectorSecrets = require('../services/connectorSecrets'); const { DISCORD_WEBHOOK_URL } = require('../services/connectorSecretKinds'); // eslint-disable-next-line global-require const { isFeedType, takeFeedTokens, storeFeedTokens } = require('../services/feedCredentials'); +const { isChatTokenType, takeChatTokens, storeGroupmeAccessToken } = require('../services/chatConnectorTokens'); // eslint-disable-next-line global-require const Pod = require('../models/Pod'); // eslint-disable-next-line global-require @@ -517,6 +518,12 @@ router.post('/', writeIntegrationsRateLimit, auth, async (req: AuthReq, res: Res const { rest, tokens } = takeFeedTokens(nextConfig); rowConfig = { ...rest, ...await storeFeedTokens(String(integrationId), type, tokens) }; } + if (isChatTokenType(type)) { + // GroupMe's token is stored through connectorSecrets; Slack's copy has no + // reader and is dropped (TASK-539). + const { rest, accessToken } = takeChatTokens(type, nextConfig); + rowConfig = { ...rest, ...await storeGroupmeAccessToken(String(integrationId), accessToken) }; + } const integration = new Integration({ _id: integrationId, podId, type, config: rowConfig, createdBy: req.user?.id, status: 'pending', }); @@ -862,6 +869,10 @@ router.patch('/:id', writeIntegrationsRateLimit, auth, async (req: AuthReq, res: const { rest, tokens } = takeFeedTokens(incoming); written = { ...rest, ...await storeFeedTokens(String(integrationId), integration.type || '', tokens) }; } + if (incoming && isChatTokenType(integration.type)) { + const { rest, accessToken } = takeChatTokens(integration.type, incoming); + written = { ...rest, ...await storeGroupmeAccessToken(String(integrationId), accessToken) }; + } if (incoming && written) { Object.keys(written).forEach((key) => { update[`config.${key}`] = key in incoming ? nextConfig[key] : written[key]; diff --git a/backend/scripts/migrate-chat-token-copies.ts b/backend/scripts/migrate-chat-token-copies.ts new file mode 100644 index 0000000000..a945aa226d --- /dev/null +++ b/backend/scripts/migrate-chat-token-copies.ts @@ -0,0 +1,166 @@ +#!/usr/bin/env node +/** + * TASK-539: move GroupMe access tokens into connectorSecrets and scrub the + * GroupMe and Slack `config.accessToken` / `config.refreshToken` copies the + * generic connector routes used to store (from the #2719 review). + * + * Default dry run. SRE runs --apply only after reviewing the counts, and after + * the build carrying `services/chatConnectorTokens` is live, because that build + * is the one whose agent reader takes the ref instead of the row's copy. + * + * GroupMe: a row with a plaintext token and no `accessTokenRef` has the token + * encrypted under its id and the ref set in the same update that unsets the + * plaintext. A row that already has a ref keeps it: the ref is authoritative and + * is never replaced with an older plaintext copy. The import is insert-only, so + * a token a live edit stores between this script's read and its write is kept. `refreshToken` has no reader + * (GroupMe issues none) and is unset. + * Slack: the bot token is `config.botTokenRef`, written by the OAuth bind, and no + * code reads a row's own `accessToken` or `refreshToken`, so both are unset. + * + * The report counts the other credential-shaped fields on these rows too + * (`botToken`, `signingSecret`, `appToken`), so the inventory is whole. It never + * writes them. + * + * No token value, ref or row identifier is logged. + */ +/* eslint-disable no-console */ +import mongoose from 'mongoose'; + +// eslint-disable-next-line global-require +const connectorSecrets = require('../services/connectorSecrets'); +// eslint-disable-next-line global-require +const { GROUPME_ACCESS_TOKEN } = require('../services/connectorSecretKinds'); + +const CHAT_TYPES = ['groupme', 'slack'] as const; +type ChatType = typeof CHAT_TYPES[number]; +const ACCESS = 'config.accessToken'; +const REFRESH = 'config.refreshToken'; +const INVENTORY_ONLY = ['botToken', 'signingSecret', 'appToken'] as const; + +export interface Counts { + candidates: number; + accessFields: number; + refreshFields: number; + toMove: number; + moved: number; + cleared: number; + remaining: number; + inventory: Record; +} + +export type Report = Record; + +const material = (value: unknown): string => (typeof value === 'string' ? value.trim() : ''); + +const hasRef = (config: Record | undefined): boolean => Boolean( + material(config?.accessTokenRef), +); + +export const migrateChatTokenCopies = async ( + { apply = false }: { apply?: boolean } = {}, +): Promise => { + // The raw collection reaches historical keys whatever the current schema or + // toJSON redactor says. $exists includes blank and null residue. + const collection = mongoose.connection.collection('integrations'); + const report = {} as Report; + for (const type of CHAT_TYPES) { + const filter = { + type, + $or: [{ [ACCESS]: { $exists: true } }, { [REFRESH]: { $exists: true } }], + }; + // Sequential per type and per row: a failure stops the run, and a rerun is safe. + /* eslint-disable no-await-in-loop */ + const [candidates, accessFields, refreshFields, ...inventoryCounts] = await Promise.all([ + collection.countDocuments(filter), + collection.countDocuments({ type, [ACCESS]: { $exists: true } }), + collection.countDocuments({ type, [REFRESH]: { $exists: true } }), + ...INVENTORY_ONLY.map((field) => collection.countDocuments({ type, [`config.${field}`]: { $exists: true } })), + ]); + const inventory = Object.fromEntries( + INVENTORY_ONLY.map((field, i) => [field, inventoryCounts[i]]), + ) as Counts['inventory']; + + let toMove = 0; + let moved = 0; + let cleared = 0; + if (type === 'groupme') { + const rows = await collection + .find(filter, { projection: { [ACCESS]: 1, 'config.accessTokenRef': 1 } }) + .toArray(); + for (const row of rows) { + const config = row.config as Record | undefined; + const token = material(config?.accessToken); + const move = Boolean(token) && !hasRef(config); + if (move) toMove += 1; + if (apply) { + const $set: Record = {}; + let created = false; + if (move) { + // Encrypt first: if the key ring is missing this throws before the + // plaintext is unset, so no row loses its only copy. Insert-only: if + // a live edit stored a token after the read above, that newer secret + // wins and this older copy is dropped, never written over it. + const stored = await connectorSecrets.putIfAbsent( + String(row._id), GROUPME_ACCESS_TOKEN, token, + ); + $set['config.accessTokenRef'] = stored.ref; + ({ created } = stored); + } + const result = await collection.updateOne( + { _id: row._id }, + { ...(move ? { $set } : {}), $unset: { [ACCESS]: '', [REFRESH]: '' } }, + ); + if (result.modifiedCount) { + cleared += 1; + if (created) moved += 1; + } + } + } + } else if (apply) { + cleared = (await collection.updateMany(filter, { + $unset: { [ACCESS]: '', [REFRESH]: '' }, + })).modifiedCount; + } + const remaining = apply ? await collection.countDocuments(filter) : candidates; + /* eslint-enable no-await-in-loop */ + report[type] = { + candidates, accessFields, refreshFields, toMove, moved, cleared, remaining, inventory, + }; + } + return report; +}; + +export const formatReport = (report: Report, apply: boolean): string[] => [ + `[migrate-chat-token-copies] ${apply ? 'APPLY' : 'DRY-RUN'}`, + ...CHAT_TYPES.map((type) => { + const c = report[type]; + const inventory = INVENTORY_ONLY.map((field) => `${field}=${c.inventory[field]}`).join(' '); + return `${type}: candidates=${c.candidates} accessFields=${c.accessFields} ` + + `refreshFields=${c.refreshFields} toMove=${c.toMove} moved=${c.moved} ` + + `cleared=${c.cleared} remaining=${c.remaining} | not written: ${inventory}`; + }), + ...(apply ? [] : ['DRY RUN: nothing written. Review counts before --apply.']), +]; + +const main = async (): Promise => { + const args = process.argv.slice(2); + if (args.some((arg) => arg !== '--apply')) throw new Error('invalid arguments'); + if (!process.env.MONGO_URI) throw new Error('missing configuration'); + const apply = args.includes('--apply'); + try { + await mongoose.connect(process.env.MONGO_URI); + const report = await migrateChatTokenCopies({ apply }); + formatReport(report, apply).forEach((line) => console.log(line)); + } finally { + await mongoose.disconnect(); + } +}; + +if (require.main === module) { + main().catch(() => { + // Database and driver errors may carry connection strings or command + // values. Never echo the exception, the URI or command-line input. + console.error('[migrate-chat-token-copies] Failed. No values logged; rerun dry-run to inspect counts.'); + process.exitCode = 1; + }); +} diff --git a/backend/services/chatConnectorTokens.ts b/backend/services/chatConnectorTokens.ts new file mode 100644 index 0000000000..d7692733a8 --- /dev/null +++ b/backend/services/chatConnectorTokens.ts @@ -0,0 +1,73 @@ +/** + * The GroupMe and Slack token copies a generic connector write could leave on a + * row (TASK-539, from the #2719 review). + * + * GroupMe: an agent reading a group's history calls the GroupMe API with a + * member's access token (`routes/agentsRuntime` integration messages). It is + * stored the way the X / Instagram feed pair is: the material goes to + * `services/connectorSecrets` and the row keeps only `config.accessTokenRef`. + * GroupMe issues no refresh token, so a supplied one is dropped. + * + * Slack: the bot token is written by the OAuth bind as `config.botTokenRef` + * (`routes/installables`), and nothing reads a row's own `accessToken` or + * `refreshToken`. A supplied copy is dropped, not stored. + * + * There is no read of a row's own `config.accessToken`. A row still carrying + * one reads as having no credential until `scripts/migrate-chat-token-copies` + * moves it. + */ +// eslint-disable-next-line global-require +const connectorSecrets = require('./connectorSecrets'); +// eslint-disable-next-line global-require +const { GROUPME_ACCESS_TOKEN } = require('./connectorSecretKinds'); + +export const CHAT_TOKEN_CONFIG_KEYS = ['accessToken', 'refreshToken'] as const; + +const CHAT_TOKEN_TYPES = ['groupme', 'slack']; + +export const isChatTokenType = (type: unknown): boolean => CHAT_TOKEN_TYPES.includes(String(type || '')); + +/** + * Split the token keys out of a config a caller supplied. `rest` is safe to + * write to the row. `accessToken` is the GroupMe material to store, and is + * absent for Slack, whose copy has no reader. + */ +export const takeChatTokens = ( + type: unknown, + config: Record, +): { rest: Record; accessToken?: unknown } => { + const rest = { ...config }; + const { accessToken } = rest; + CHAT_TOKEN_CONFIG_KEYS.forEach((key) => { delete rest[key]; }); + return String(type) === 'groupme' ? { rest, accessToken } : { rest }; +}; + +/** + * Encrypt a GroupMe access token under the row's id and return the ref field to + * set on its config. A blank token writes nothing, so a save that leaves the + * field empty keeps the ref already on the row. + */ +export const storeGroupmeAccessToken = async ( + integrationId: string, + accessToken: unknown, +): Promise<{ accessTokenRef?: string }> => { + const material = typeof accessToken === 'string' ? accessToken.trim() : ''; + if (!material) return {}; + return { accessTokenRef: await connectorSecrets.put(integrationId, GROUPME_ACCESS_TOKEN, material) }; +}; + +export const readGroupmeAccessToken = async ( + config: { accessTokenRef?: unknown } | null | undefined, +): Promise => { + const ref = String(config?.accessTokenRef || '').trim(); + if (!ref) return undefined; + return connectorSecrets.get(ref); +}; + +module.exports = { + CHAT_TOKEN_CONFIG_KEYS, + isChatTokenType, + takeChatTokens, + storeGroupmeAccessToken, + readGroupmeAccessToken, +}; diff --git a/backend/services/connectorSecretKinds.ts b/backend/services/connectorSecretKinds.ts index 87df3defa4..4135b93078 100644 --- a/backend/services/connectorSecretKinds.ts +++ b/backend/services/connectorSecretKinds.ts @@ -141,6 +141,20 @@ export const INSTAGRAM_FEED_ACCESS_TOKEN: ConnectorSecretKindSpec = { unavailableReason: 'Instagram feed credential is unavailable', }; +/** + * The GroupMe access token (TASK-539). GroupMe's bot posts with `botId` alone, + * but an agent that reads a group's history (`agentsRuntime` integration + * messages) calls the GroupMe API with a member's access token, so the token is + * a bearer credential and is stored like the feed pair: the row keeps only the + * ref (`services/chatConnectorTokens`). GroupMe issues no refresh token. + */ +export const GROUPME_ACCESS_TOKEN: ConnectorSecretKindSpec = { + kind: 'groupme-access-token', + provider: 'groupme', + refPaths: ['config.accessTokenRef'], + unavailableReason: 'GroupMe access token is unavailable', +}; + export const CONNECTOR_SECRET_KINDS: readonly ConnectorSecretKindSpec[] = [ AGENT_PROVIDER_KEY, SLACK_BOT_TOKEN, @@ -151,6 +165,7 @@ export const CONNECTOR_SECRET_KINDS: readonly ConnectorSecretKindSpec[] = [ X_FEED_ACCESS_TOKEN, X_FEED_REFRESH_TOKEN, INSTAGRAM_FEED_ACCESS_TOKEN, + GROUPME_ACCESS_TOKEN, ]; /** Whether a kind's ref lives on an Integration row (the default owner). */ diff --git a/backend/services/connectorSecrets.ts b/backend/services/connectorSecrets.ts index f0cf2cb5d8..68322515c3 100644 --- a/backend/services/connectorSecrets.ts +++ b/backend/services/connectorSecrets.ts @@ -174,6 +174,34 @@ export const put = async ( return String(secret._id); }; +/** + * Store `material` only if `(integrationId, kind)` has no secret yet. Returns + * the ref and whether this call created it. A migration that imports an old + * plaintext copy uses this, so it never overwrites a secret a live writer stored + * after the migration read the row (TASK-539, CodeRabbit on #3004). On a + * conflict the existing secret wins and its ref is returned unchanged. + */ +export const putIfAbsent = async ( + integrationId: string, + spec: ConnectorSecretKindSpec, + material: string, +): Promise<{ ref: string; created: boolean }> => { + if (!material) throw new ConnectorSecretConfigurationError('Connector secret material must not be empty.'); + const id = asObjectId(integrationId); + const encrypted = encrypt(material, parseKeyRing()); + try { + const secret = await ConnectorSecret.create({ + integrationId: id, kind: spec.kind, provider: spec.provider, ...encrypted, + }) as IConnectorSecret; + return { ref: String(secret._id), created: true }; + } catch (error) { + if ((error as { code?: number }).code !== 11000) throw error; + const existing = await ConnectorSecret.findOne({ integrationId: id, kind: spec.kind }) as IConnectorSecret | null; + if (!existing) throw error; + return { ref: String(existing._id), created: false }; + } +}; + export const get = async (ref: string): Promise => { const secret = await ConnectorSecret.findById(ref) as IConnectorSecret | null; if (!secret) throw new ConnectorSecretNotFoundError(ref); @@ -206,6 +234,7 @@ module.exports = { ConnectorSecretNotFoundError, ConnectorSecretStaleWrite, put, + putIfAbsent, get, revoke, rewrap, diff --git a/backend/services/groupmeService.ts b/backend/services/groupmeService.ts index 948801f023..88358d83f1 100644 --- a/backend/services/groupmeService.ts +++ b/backend/services/groupmeService.ts @@ -87,10 +87,22 @@ async function fetchMessages(options: FetchOptions = {} as FetchOptions): Promis if (before) params.before_id = before; if (after) params.after_id = after; - const response = await axios.get( - `${GROUPME_API_BASE}/groups/${groupId}/messages`, - { params }, - ); + let response; + try { + response = await axios.get( + `${GROUPME_API_BASE}/groups/${groupId}/messages`, + { params }, + ); + } catch (error) { + // GroupMe takes the token as a query parameter, so the axios error's + // `config.params` (and its request URL) carry it. Rethrow only the status, + // so a caller that logs the error object never logs the token. + const status = (error as { response?: { status?: number } })?.response?.status; + throw Object.assign( + new Error(status ? `GroupMe request failed with status ${status}` : 'GroupMe request failed'), + status ? { status } : {}, + ); + } const messages = response.data?.response?.messages || []; return messages.map((msg) => ({ diff --git a/backend/utils/serverOwnedConfigKeys.ts b/backend/utils/serverOwnedConfigKeys.ts index 80e824fe59..976f9b89c0 100644 --- a/backend/utils/serverOwnedConfigKeys.ts +++ b/backend/utils/serverOwnedConfigKeys.ts @@ -107,6 +107,9 @@ export const SERVER_OWNED_CONFIG_KEYS = [ // The X / Instagram feed refs. Only `services/feedCredentials` writes them; a // body that supplied one would point its row at another row's secret. 'feedAccessTokenRef', 'feedRefreshTokenRef', + // The GroupMe access token's ref, for the same reason. Only + // `services/chatConnectorTokens` writes it. + 'accessTokenRef', ]; export const isServerOwnedConfigKey = (key: string): boolean => SERVER_OWNED_CONFIG_KEYS.includes(key); diff --git a/docs/runbooks/migrate-chat-token-copies.md b/docs/runbooks/migrate-chat-token-copies.md new file mode 100644 index 0000000000..88d474530f --- /dev/null +++ b/docs/runbooks/migrate-chat-token-copies.md @@ -0,0 +1,75 @@ +# Move GroupMe tokens and scrub GroupMe/Slack token copies (TASK-539) + +This is the GroupMe and Slack follow-up to +[scrub-feed-token-copies](scrub-feed-token-copies.md), which covers X and +Instagram only. SRE runs it through the ops-runner, on the intended instance, +after the build that carries `services/chatConnectorTokens` is live. Don't run +it while an older backend can still write the legacy fields. + +**What it changes, by type:** + +- **GroupMe.** `config.accessToken` is a live credential. An agent that reads a + group's history (`GET /api/agents/runtime/pods/:podId/integrations/:id/messages`) + calls the GroupMe API with it. That reader now takes `config.accessTokenRef` + only. So the migration **imports**: + - A row with a plaintext token and no ref has the token encrypted under its id + (kind `groupme-access-token`). In the same update, the ref is set and the + plaintext is unset. + - A row that already has a ref keeps it. The older plaintext copy is dropped + and is never written over the ref. + - The import is insert-only. If someone edits the connector while the script + runs, the edit stores its token between the script's read and its write. + That newer token wins, and the older copy is dropped. So `--apply` needs no + write freeze, and `moved` counts only the secrets the script created. + - `refreshToken` has no reader, because GroupMe issues none. It is unset. +- **Slack.** The bot token is `config.botTokenRef`, written by the OAuth bind. + Nothing reads a row's own `accessToken` or `refreshToken`, so both are + **redacted**, not imported. + +Between the deploy and `--apply`, a GroupMe row that carries only plaintext +answers the agent read with 400 ("missing accessToken"). Its bot posts and +`!summary` commands are unaffected, because they use `botId`. To keep that +window short, apply soon after the deploy. + +From the deployed backend directory, with `MONGO_URI` and the connector secret +key ring (`CONNECTOR_SECRET_KEYS`, `CONNECTOR_SECRET_ACTIVE_KEY`) supplied by the +existing ops-runner secret environment (never paste them into a command): + +```sh +npx ts-node scripts/migrate-chat-token-copies.ts +# Review the DRY-RUN counts by type, then explicitly apply: +npx ts-node scripts/migrate-chat-token-copies.ts --apply +npx ts-node scripts/migrate-chat-token-copies.ts +``` + +**What the counts mean.** The final dry run should report zero candidates and +`toMove=0` for both types. Each line is counts only: + +- the candidate rows; +- the access and refresh fields; +- `toMove`, the GroupMe tokens that will be encrypted; +- `moved` and `cleared`; +- `remaining`; +- after `| not written:`, how many rows still carry `botToken`, `signingSecret` + or `appToken`. Those fields are inventory only and this script never writes + them. No code reads a Slack row's `botToken` any more: `providers/slackProvider`'s + `tokenFor` takes `botTokenRef` only. A nonzero Slack `botToken` count is + copies left at rest, for a later scrub. + +No credential value, ref or row id is printed. + +**Failure and reruns.** If the key ring is missing, the GroupMe encrypt throws +before that row is updated, so no row loses its only copy. The command exits +non-zero with a fixed message and never echoes the database error. A partial +apply is safe to rerun: applied rows no longer match, and a moved row's secret is +an upsert on `(integrationId, kind)`. + +**Recording and scope.** Record these in the ops record, without secret +material: + +- the instance and the deployed SHA; +- the dry-run, apply and final counts; +- the exit status. + +The PR that adds this script supplies the tool. It doesn't certify that any +instance has been migrated or that older backups are clean. diff --git a/docs/runbooks/scrub-feed-token-copies.md b/docs/runbooks/scrub-feed-token-copies.md index 950e2fd1df..e06381710b 100644 --- a/docs/runbooks/scrub-feed-token-copies.md +++ b/docs/runbooks/scrub-feed-token-copies.md @@ -7,7 +7,8 @@ run it while an older backend can still write the legacy fields. This migration **redacts**, rather than imports, `config.accessToken` and `config.refreshToken` on X and Instagram Integration rows, including disabled rows and blank/null residue. Existing encrypted feed refs remain authoritative -and unchanged. Other connector types (including GroupMe) are untouched. +and unchanged. Other connector types are untouched; GroupMe and Slack have +their own migration, [migrate-chat-token-copies](migrate-chat-token-copies.md). A feed with only legacy plaintext already has no usable credential under #2692; reconnect it to supply a fresh token. Do not reuse an older plaintext copy over a newer encrypted credential.