diff --git a/backend/__tests__/service/uninstallRevokesRuntimeCredential.test.js b/backend/__tests__/service/uninstallRevokesRuntimeCredential.test.js new file mode 100644 index 000000000..16dc485ea --- /dev/null +++ b/backend/__tests__/service/uninstallRevokesRuntimeCredential.test.js @@ -0,0 +1,147 @@ +// Every way an agent's last installation goes away retires its runtime +// token (TASK-625), the way the uninstall routes do (#2221): deleting the pod +// and pruning a stale install now do it too, and a reinstall of the same +// identity starts from a fresh token. +// +// A real mongod, because the claim is about which stored records survive; the +// probe is the real runtime auth middleware, so "dead" means what an agent +// would see: its next call is a 401. +const express = require('express'); +const rateLimit = require('express-rate-limit'); +const request = require('supertest'); +const mongoose = require('mongoose'); +const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../utils/testUtils'); + +process.env.PG_HOST = ''; +process.env.NODE_ENV = 'test'; + +jest.mock('../../services/starterTaskService', () => ({ completeConnectAgentStarterTask: jest.fn() })); + +const Pod = require('../../models/Pod'); +const Message = require('../../models/Message'); +const User = require('../../models/User'); +const AgentCredential = require('../../models/AgentCredential'); +const { AgentInstallation } = require('../../models/AgentRegistry'); +const AgentIdentityService = require('../../services/agentIdentityService'); +const tokens = require('../../routes/registry/tokens'); + +const { issueRuntimeTokenForAgent } = tokens; +const { deletePodCascade } = require('../../services/podDeletionService'); +const { pruneStaleInstallations } = require('../../services/agentInstallationCleanupService'); +const agentRuntimeAuth = require('../../middleware/agentRuntimeAuth'); + +const app = express(); +const probeLimit = rateLimit({ windowMs: 60 * 1000, max: 10000 }); +app.get('/probe', probeLimit, agentRuntimeAuth, (req, res) => res.json({ agent: req.agentUser?.username || null })); +const probe = (token) => request(app).get('/probe').set('Authorization', `Bearer ${token}`); + +const AGENT = 'maya-claude'; +const ownerId = new mongoose.Types.ObjectId(); + +const makePod = (name) => Pod.create({ name, type: 'chat', createdBy: ownerId, members: [ownerId] }); +const install = (pod, extra = {}) => AgentInstallation.create({ + agentName: AGENT, + instanceId: 'default', + podId: pod._id, + installedBy: ownerId, + status: 'active', + displayName: 'Maya Claude', + version: '1.0.0', + ...extra, +}); + +const issue = async (installation) => { + const agentUser = await AgentIdentityService.getOrCreateAgentUser(AGENT, { instanceId: 'default' }); + const issued = await issueRuntimeTokenForAgent(agentUser, 'CLI key', installation, { ownerUserId: ownerId }); + return issued.token; +}; + +const credentialStatus = async () => { + const rows = await AgentCredential.find({ kind: 'runtime' }).select('status').lean(); + return rows.map((row) => row.status); +}; + +describe('the last installation going away revokes the runtime token', () => { + beforeAll(async () => { await setupMongoDb(); }); + afterAll(async () => { await closeMongoDb(); }); + beforeEach(async () => { + await clearMongoDb(); + jest.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => jest.restoreAllMocks()); + + test('deleting the only pod revokes it, and a reinstall does not revive it', async () => { + const pod = await makePod('Only pod'); + const token = await issue(await install(pod)); + expect((await probe(token)).status).toBe(200); + + await deletePodCascade(String(pod._id)); + + expect(await credentialStatus()).toEqual(['revoked']); + expect(await User.countDocuments({ 'agentRuntimeTokens.0': { $exists: true } })).toBe(0); + await install(await makePod('New pod')); + expect((await probe(token)).status).toBe(401); + }); + + test('deleting one of two pods leaves the token working', async () => { + const pod = await makePod('Doomed'); + const keep = await makePod('Kept'); + const token = await issue(await install(pod)); + await install(keep); + + await deletePodCascade(String(pod._id)); + + expect(await credentialStatus()).toEqual(['active']); + expect((await probe(token)).status).toBe(200); + }); + + test('pruning the last stale install revokes it, and a reinstall does not revive it', async () => { + const pod = await makePod('Stale pod'); + const installation = await install(pod); + const token = await issue(installation); + await AgentInstallation.updateOne( + { _id: installation._id }, + { $set: { status: 'stale', staleSince: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000) } }, + ); + + const result = await pruneStaleInstallations(14); + + expect(result).toEqual({ deleted: 1, failures: 0 }); + expect(await credentialStatus()).toEqual(['revoked']); + await install(pod); + expect((await probe(token)).status).toBe(401); + }); + test('a prune whose revoke fails keeps the row, and the next run finishes the job', async () => { + const pod = await makePod('Retry pod'); + const installation = await install(pod); + await issue(installation); + await AgentInstallation.updateOne( + { _id: installation._id }, + { $set: { status: 'stale', staleSince: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000) } }, + ); + jest.spyOn(console, 'error').mockImplementation(() => {}); + jest.spyOn(tokens, 'revokeRuntimeTokensAfterLastUninstall').mockRejectedValueOnce(new Error('db blip')); + + expect(await pruneStaleInstallations(14)).toEqual({ deleted: 0, failures: 1 }); + expect(await AgentInstallation.countDocuments({ _id: installation._id })).toBe(1); + expect(await credentialStatus()).toEqual(['active']); + + expect(await pruneStaleInstallations(14)).toEqual({ deleted: 1, failures: 0 }); + expect(await credentialStatus()).toEqual(['revoked']); + }); + test('a pod deletion whose revoke fails deletes nothing, and the retry revokes', async () => { + const pod = await makePod('Retry delete'); + await issue(await install(pod)); + await Message.create({ podId: pod._id, userId: ownerId, content: 'keep me' }); + jest.spyOn(tokens, 'revokeRuntimeTokensAfterLastUninstall').mockRejectedValueOnce(new Error('db blip')); + + await expect(deletePodCascade(String(pod._id))).rejects.toThrow('db blip'); + expect(await Pod.countDocuments({ _id: pod._id })).toBe(1); + expect(await Message.countDocuments({ podId: pod._id })).toBe(1); + expect(await AgentInstallation.countDocuments({ podId: pod._id })).toBe(1); + + await deletePodCascade(String(pod._id)); + expect(await credentialStatus()).toEqual(['revoked']); + expect(await Pod.countDocuments({ _id: pod._id })).toBe(0); + }); +}); diff --git a/backend/__tests__/unit/controllers/podController.test.js b/backend/__tests__/unit/controllers/podController.test.js index 2bd4a235c..2c522fc59 100644 --- a/backend/__tests__/unit/controllers/podController.test.js +++ b/backend/__tests__/unit/controllers/podController.test.js @@ -30,6 +30,9 @@ jest.mock('../../../models/AgentRegistry', () => ({ isInstalled: jest.fn(), install: jest.fn(), deleteMany: jest.fn(), + // deletePodCascade reads the pod's agents before deleting them. + find: jest.fn(() => ({ select: () => ({ lean: async () => [] }) })), + updateMany: jest.fn(), }, })); jest.mock('../../../models/AgentProfile', () => ({ diff --git a/backend/__tests__/unit/routes/hosted.test.js b/backend/__tests__/unit/routes/hosted.test.js index f3051f544..a1bb25897 100644 --- a/backend/__tests__/unit/routes/hosted.test.js +++ b/backend/__tests__/unit/routes/hosted.test.js @@ -405,6 +405,10 @@ describe('/api/hosted', () => { expect(res.status).toBe(200); expect(res.body).toMatchObject({ deprovisioned: true }); expect(installation.config.get('hosted').provisionedAt).toBeNull(); + // The stopped worker held the only copy; nothing else may keep using it. + expect(mockRevokeRuntimeTokens).toHaveBeenCalledWith(expect.objectContaining({ + agentName: 'scout', instanceId: 'default', installation, + })); }); // TASK-265: a migration that marks the row native while the diff --git a/backend/routes/hosted.ts b/backend/routes/hosted.ts index f3e3957e9..a93178871 100644 --- a/backend/routes/hosted.ts +++ b/backend/routes/hosted.ts @@ -354,8 +354,7 @@ router.post('/provision', hostedRateLimit, auth, async (req: any, res: any) => { * POST /api/hosted/deprovision { agentName, instanceId? } * Stops the runtime. The installation and identity stay — ADR-001 §3 identity * continuity; re-provisioning finds the same memory. The token does NOT - * survive: provision rotates on every call, so a - * re-provision mints fresh and orphans anything still holding the old one. + * survive: it is revoked here, and provision mints a fresh one. */ router.post('/deprovision', hostedRateLimit, auth, async (req: any, res: any) => { try { @@ -375,6 +374,17 @@ router.post('/deprovision', hostedRateLimit, auth, async (req: any, res: any) => } if (!requireConfigured(res, hostedEngineOf(installation))) return undefined; await hostedRuntime.deprovisionAgent(agentName, instanceId); + // The stopped runtime was the token's only holder. Revoke it now rather + // than at the next provision, which may never come. + // Without a bot user the installation copies are still revoked. + const agentUser = await User.findOne({ + username: AgentIdentityService.buildAgentUsername(agentName, instanceId), + isBot: true, + }); + await revokeRuntimeTokensForAgent({ agentUser, agentName, instanceId, installation }); + // A socket authenticated with the old token stays open until told to go. + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + require('../services/agentWebSocketService').disconnectIdentity(agentName, instanceId); // A row migrated meanwhile keeps the migration's record; the worker is // stopped either way, which is what both wanted. await writeDurableObjectRecord(installation, { provisionedAt: null, deprovisionedAt: new Date() }); diff --git a/backend/services/agentInstallationCleanupService.ts b/backend/services/agentInstallationCleanupService.ts index cee9dbee8..455507b03 100644 --- a/backend/services/agentInstallationCleanupService.ts +++ b/backend/services/agentInstallationCleanupService.ts @@ -312,8 +312,15 @@ export async function pruneStaleInstallations( await Pod.updateOne({ _id: inst.podId }, { $pull: { members: agentUser._id } }); } - await AgentInstallation.deleteOne({ _id: inst._id }); - deleted += 1; + // A pruned install is an uninstall: once none is active, the agent's + // runtime token stops authenticating too. A stale row is not active, so + // revoke first; if it throws, the row survives for the next run. + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { revokeRuntimeTokensAfterLastUninstall } = require('../routes/registry/tokens'); + await revokeRuntimeTokensAfterLastUninstall(inst.agentName, inst.instanceId || 'default'); + // Only if still stale: a reinstall may have reactivated the row meanwhile. + const removed = await AgentInstallation.deleteOne({ _id: inst._id, status: 'stale' }); + if (removed?.deletedCount) deleted += 1; } catch (err) { console.error( '[installation-cleanup] failed to prune install %s (%s/%s): %s', diff --git a/backend/services/podDeletionService.ts b/backend/services/podDeletionService.ts index af68e53a3..9d08dd307 100644 --- a/backend/services/podDeletionService.ts +++ b/backend/services/podDeletionService.ts @@ -22,6 +22,27 @@ if (process.env.PG_HOST) { } export const deletePodCascade = async (podId: string): Promise => { + // An agent whose last install was in this pod loses its runtime token, as + // on uninstall. Uninstall the rows and revoke before deleting anything: + // if a revoke throws, the rows are still here and a retried delete finds + // them again. The helper counts active installs, so an agent installed in + // another pod keeps its token. + const installedAgents = await AgentInstallation.find({ podId }) + .select('agentName instanceId').lean(); + await AgentInstallation.updateMany({ podId }, { $set: { status: 'uninstalled' } }); + const identities = new Map(); + for (const row of installedAgents || []) { + const instanceId = row.instanceId || 'default'; + identities.set(`${row.agentName}:${instanceId}`, { agentName: row.agentName, instanceId }); + } + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { revokeRuntimeTokensAfterLastUninstall } = require('../routes/registry/tokens'); + for (const { agentName, instanceId } of identities.values()) { + await revokeRuntimeTokensAfterLastUninstall(agentName, instanceId); + } + + // Nothing irreversible runs before the revoke above, so a failure there + // leaves the pod and its content for the retry. // A hierarchy link is not ownership. Keep children and their contents. await Pod.updateMany({ parentPod: podId }, { $unset: { parentPod: 1 } }); await Message.deleteMany({ podId });