From cef29f2799a7e195311552355abc3112b08b3da3 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:42:36 -0700 Subject: [PATCH 1/2] Fix agent connection after credential revocation Co-authored-by: rivet Agent-Model: codex/gpt-6-sol/medium --- .../service/machineRevocation.test.js | 1 + .../service/runtimeTokenRevoke.test.js | 20 ++++++++++++++++--- .../registry.pod-agents-activity.test.js | 17 ++++++++++++++++ .../agentConnectionStateService.test.js | 7 +++++++ backend/routes/credentials.ts | 15 ++++++++++++++ backend/routes/registry/pod-agents.ts | 4 ++-- .../services/agentConnectionStateService.ts | 17 ++++++++++++---- 7 files changed, 72 insertions(+), 9 deletions(-) diff --git a/backend/__tests__/service/machineRevocation.test.js b/backend/__tests__/service/machineRevocation.test.js index 509a451612..f96bd2ad82 100644 --- a/backend/__tests__/service/machineRevocation.test.js +++ b/backend/__tests__/service/machineRevocation.test.js @@ -168,6 +168,7 @@ describe('machine credential revocation', () => { .set('Authorization', `Bearer ${generateTestToken(user._id)}`).expect(200); expect(await AgentCredential.findById(child.child._id)).toMatchObject({ status: 'revoked' }); + expect((await User.findById(child.child.agentUserId)).agentRuntimeTokens).toHaveLength(0); await daemonCall(first.daemonToken, first.machine.id).expect(401); expect(await agentCall(child.token)).toBe(401); }); diff --git a/backend/__tests__/service/runtimeTokenRevoke.test.js b/backend/__tests__/service/runtimeTokenRevoke.test.js index c0c7e7707d..ee423c6f5f 100644 --- a/backend/__tests__/service/runtimeTokenRevoke.test.js +++ b/backend/__tests__/service/runtimeTokenRevoke.test.js @@ -161,7 +161,7 @@ describe('revoking a runtime token kills it in every store', () => { }); test('the Settings credential revoke stops the agent token and keeps its history', async () => { - const { workspace } = await seed(); + const { workspace, workspaceInstall } = await seed(); const issued = await request(app) .post(`/api/registry/pods/${workspace._id}/agents/${AGENT}/runtime-tokens`) .send({ label: 'Settings key' }); @@ -169,9 +169,23 @@ describe('revoking a runtime token kills it in every store', () => { const listed = await request(app).get(`/api/credentials?agentName=${AGENT}&instanceId=default`); expect(listed.status).toBe(200); expect(listed.body).toEqual([expect.objectContaining({ label: 'Settings key', status: 'active' })]); + const tokenHash = (await User.findOne({ 'agentRuntimeTokens.0': { $exists: true } }).lean()) + .agentRuntimeTokens[0].tokenHash; + expect(await storesHolding(tokenHash)).toEqual({ users: 1, installations: 1 }); + workspaceInstall.config = new Map([['runtime', { runtimeType: 'webhook' }]]); + await workspaceInstall.save(); + await AgentCredential.updateOne({ tokenHash }, { $set: { lastUsedAt: new Date() } }); + const beforeCard = await request(app).get(`/api/registry/pods/${workspace._id}/agents`); + expect(beforeCard.status).toBe(200); + expect(beforeCard.body.agents[0].connection.state).toBe('connected'); const revoked = await request(app).delete(`/api/credentials/${listed.body[0]._id}?agentName=${AGENT}&instanceId=default`); expect(revoked.status).toBe(200); expect((await probe(issued.body.token)).status).toBe(401); + expect(await storesHolding(tokenHash)).toEqual({ users: 0, installations: 0 }); + expect((await AgentInstallation.findById(workspaceInstall._id).lean()).runtimeTokens).toHaveLength(0); + const afterCard = await request(app).get(`/api/registry/pods/${workspace._id}/agents`); + expect(afterCard.status).toBe(200); + expect(afterCard.body.agents[0].connection).toEqual({ state: 'revoked', lastSeenAt: null }); const after = await request(app).get(`/api/credentials?agentName=${AGENT}&instanceId=default`); expect(after.body).toEqual([expect.objectContaining({ label: 'Settings key', status: 'revoked' })]); }); @@ -189,8 +203,8 @@ describe('revoking a runtime token kills it in every store', () => { expect((await probe(issued.token)).status).toBe(401); expect((await request(app).get(`/api/credentials?agentName=${AGENT}&instanceId=default`)).body) .toEqual([expect.objectContaining({ label: 'Old CLI key', status: 'revoked' })]); - expect((await User.findById(agentUser._id).lean()).agentRuntimeTokens).toHaveLength(1); - expect((await AgentInstallation.findById(workspaceInstall._id).lean()).runtimeTokens).toHaveLength(1); + expect((await User.findById(agentUser._id).lean()).agentRuntimeTokens).toHaveLength(0); + expect((await AgentInstallation.findById(workspaceInstall._id).lean()).runtimeTokens).toHaveLength(0); }); test('an installation-only legacy key remains visible for owner revocation', async () => { diff --git a/backend/__tests__/unit/routes/registry.pod-agents-activity.test.js b/backend/__tests__/unit/routes/registry.pod-agents-activity.test.js index 75a49558ef..80594bc730 100644 --- a/backend/__tests__/unit/routes/registry.pod-agents-activity.test.js +++ b/backend/__tests__/unit/routes/registry.pod-agents-activity.test.js @@ -153,6 +153,23 @@ describe('pod agents lastActiveAt derivation (#915)', () => { expect(res.body.agents[0].connection).toEqual({ state: 'revoked', lastSeenAt: null }); }); + it('projects a revoked credential despite stale embedded copies on User and installation', async () => { + AgentInstallation.getInstalledAgents.mockResolvedValue([baseInstallation({ + agentName: 'local', config: new Map([['runtime', { runtimeType: 'webhook' }]]), + runtimeTokens: [{ tokenHash: 'dead-hash', lastUsedAt: new Date() }], + })]); + User.find.mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue([{ + _id: 'bot-1', username: 'local-default', botMetadata: {}, + agentRuntimeTokens: [{ tokenHash: 'dead-hash', lastUsedAt: new Date() }], + }]) }) }); + AgentCredential.find.mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue([{ + kind: 'runtime', agentUserId: 'bot-1', tokenHash: 'dead-hash', status: 'revoked', + }]) }) }); + const res = await request(app).get(`/api/registry/pods/${POD_ID}/agents`); + expect(res.status).toBe(200); + expect(res.body.agents[0].connection).toEqual({ state: 'revoked', lastSeenAt: null }); + }); + it('keeps a revoked wrapper key disconnected despite a live machine', async () => { AgentInstallation.getInstalledAgents.mockResolvedValue([baseInstallation({ agentName: 'local', config: new Map([['runtime', { runtimeType: 'wrapper' }]]), diff --git a/backend/__tests__/unit/services/agentConnectionStateService.test.js b/backend/__tests__/unit/services/agentConnectionStateService.test.js index 202f528105..fa1918b1d9 100644 --- a/backend/__tests__/unit/services/agentConnectionStateService.test.js +++ b/backend/__tests__/unit/services/agentConnectionStateService.test.js @@ -45,4 +45,11 @@ describe('agent connection verdict', () => { { lastUsedAt: seen(2) }, ], now }).state).toBe('connected'); }); + + it('does not trust an embedded copy after its ledger row is revoked', () => { + expect(deriveAgentConnection({ runtimeType: 'webhook', credentials: [ + { tokenHash: 'revoked-hash', status: 'revoked' }, + ], embeddedTokens: [{ tokenHash: 'revoked-hash', lastUsedAt: seen(0) }], now })) + .toEqual({ state: 'revoked', lastSeenAt: null }); + }); }); diff --git a/backend/routes/credentials.ts b/backend/routes/credentials.ts index 784a5d8283..ee8d721b47 100644 --- a/backend/routes/credentials.ts +++ b/backend/routes/credentials.ts @@ -11,6 +11,7 @@ import { createHash } from 'crypto'; import AgentCredential from '../models/AgentCredential'; import { AgentInstallation } from '../models/AgentRegistry'; import { ownsRuntimeIdentity } from '../services/runtimeIdentityOwnership'; +import { revokeRuntimeTokenEverywhere } from './registry/tokens'; // eslint-disable-next-line @typescript-eslint/no-require-imports const auth = require('../middleware/auth'); @@ -123,7 +124,21 @@ router.delete('/:id', credentialRateLimit, auth, async (req: AuthReq, res: expre if (!agentName && !isOwner && caller?.role !== 'admin') { return res.status(403).json({ message: 'Access denied' }); } + // Capture descendant hashes before the cascade. A daemon revoke also + // invalidates runtime children, whose embedded copies must be removed. + const runtimeHashes = new Set(); + let frontier: Types.ObjectId[] = [credential._id as Types.ObjectId]; + while (frontier.length) { + const rows = await AgentCredential.find({ _id: { $in: frontier } }) + .select('_id kind tokenHash').lean(); + for (const row of rows) { + if (row.kind === 'runtime' && row.tokenHash) runtimeHashes.add(row.tokenHash); + } + const children = await AgentCredential.find({ parentId: { $in: frontier } }).select('_id').lean(); + frontier = children.map((row) => row._id as Types.ObjectId); + } const revoked = await AgentCredential.revokeCascade(credential._id as Types.ObjectId); + await Promise.all([...runtimeHashes].map((tokenHash) => revokeRuntimeTokenEverywhere(tokenHash))); return res.json({ revoked }); } catch (err) { console.error('Error revoking credential:', err); diff --git a/backend/routes/registry/pod-agents.ts b/backend/routes/registry/pod-agents.ts index 521ffdd568..b90fcc9abf 100644 --- a/backend/routes/registry/pod-agents.ts +++ b/backend/routes/registry/pod-agents.ts @@ -260,7 +260,7 @@ podAgentsRouter.get('/pods/:podId/agents', auth, async (req: any, res: any) => { } const userRows = usernameSet.size > 0 ? await User.find({ username: { $in: Array.from(usernameSet) } }) - .select('username botMetadata agentRuntimeTokens.lastUsedAt') + .select('username botMetadata agentRuntimeTokens.tokenHash agentRuntimeTokens.lastUsedAt agentRuntimeTokens.expiresAt') .lean() : []; const userByUsername = new Map( @@ -282,7 +282,7 @@ podAgentsRouter.get('/pods/:podId/agents', auth, async (req: any, res: any) => { AgentCredential.find({ $or: [ { kind: 'runtime', agentUserId: { $in: agentIds } }, { kind: 'daemon', machineId: { $in: machineIds } }, - ] }).select('kind agentUserId machineId status lastUsedAt expiresAt').lean(), + ] }).select('kind agentUserId machineId tokenHash status lastUsedAt expiresAt').lean(), Machine.find({ machineId: { $in: machineIds } }).select('machineId lastSeenAt').lean(), ]); for (const credential of credentials as any[]) { diff --git a/backend/services/agentConnectionStateService.ts b/backend/services/agentConnectionStateService.ts index be043756f9..b391fe712b 100644 --- a/backend/services/agentConnectionStateService.ts +++ b/backend/services/agentConnectionStateService.ts @@ -3,7 +3,7 @@ export const BYO_STALE_MS = 5 * 60 * 1000; export const MACHINE_STALE_MS = 90 * 1000; -type Credential = { status?: string; lastUsedAt?: Date | string | null; expiresAt?: Date | string | null }; +type Credential = { tokenHash?: string; kind?: string; status?: string; lastUsedAt?: Date | string | null; expiresAt?: Date | string | null }; export const deriveAgentConnection = ({ runtimeType, credentials, embeddedTokens = [], machineLastSeenAt = null, now = Date.now(), @@ -21,8 +21,17 @@ export const deriveAgentConnection = ({ && (!credential.expiresAt || new Date(credential.expiresAt).getTime() > now)); const activeMachine = machineCredentials.filter((credential) => credential.status === 'active' && (!credential.expiresAt || new Date(credential.expiresAt).getTime() > now)); - // Embedded tokens predate the ledger. They remain valid on the auth path. - const hasEmbedded = embeddedTokens.length > 0; + // An embedded copy is usable only when no ledger row exists or its ledger + // row is active. Runtime auth applies the same ledger-first rule. + const ledgerByHash = new Map(credentials.filter((credential) => credential.tokenHash) + .map((credential) => [credential.tokenHash, credential])); + const usableEmbedded = embeddedTokens.filter((token) => { + const ledger = token.tokenHash ? ledgerByHash.get(token.tokenHash) : undefined; + return (!ledger || (ledger.kind !== 'daemon' && ledger.status === 'active' + && (!ledger.expiresAt || new Date(ledger.expiresAt).getTime() > now))) + && (!token.expiresAt || new Date(token.expiresAt).getTime() > now); + }); + const hasEmbedded = usableEmbedded.length > 0; if (!active.length && !hasEmbedded && credentials.some((credential) => credential.status === 'revoked')) { return { state: 'revoked', lastSeenAt: null }; } @@ -37,7 +46,7 @@ export const deriveAgentConnection = ({ } const dates = isMachine ? [machineLastSeenAt] - : [...active, ...embeddedTokens].map((credential) => credential.lastUsedAt); + : [...active, ...usableEmbedded].map((credential) => credential.lastUsedAt); const lastSeen = dates.reduce((latest, value) => { const time = value ? new Date(value).getTime() : 0; return Number.isFinite(time) ? Math.max(latest, time) : latest; From 9f3d6176b78fca704c7c7b090ebfea88cdb98b72 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 23 Oct 2026 19:15:18 -0700 Subject: [PATCH 2/2] Collect revoked daemon children after credential cascade Co-authored-by: rivet Agent-Model: codex/gpt-6-sol/medium --- .../service/machineRevocation.test.js | 33 +++++++++++++++++++ backend/routes/credentials.ts | 7 ++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/backend/__tests__/service/machineRevocation.test.js b/backend/__tests__/service/machineRevocation.test.js index f96bd2ad82..26a928715e 100644 --- a/backend/__tests__/service/machineRevocation.test.js +++ b/backend/__tests__/service/machineRevocation.test.js @@ -14,6 +14,7 @@ app.use(express.json()); app.use('/api/auth', require('../../routes/auth')); app.use('/api/machines', require('../../routes/machines')); app.use('/api/credentials', require('../../routes/credentials')); +app.use('/api/agent-binding', require('../../routes/agentBinding')); const agentRuntimeAuth = require('../../middleware/agentRuntimeAuth').default; const agentCall = async (token) => { @@ -173,6 +174,38 @@ describe('machine credential revocation', () => { expect(await agentCall(child.token)).toBe(401); }); + it('cleans a child minted between the old descendant snapshot and cascade', async () => { + const user = await owner(); + const device = await addDevice(user, 'z'); + const first = (await register(device.token, 'concurrent-revoke')).body; + const prior = await addRuntimeChild(user, first.machine, first.daemonToken, 'concurrent'); + const daemon = await AgentCredential.findOne({ tokenHash: hash(first.daemonToken) }); + const originalCascade = AgentCredential.revokeCascade.bind(AgentCredential); + let mintedToken; + const cascadeSpy = jest.spyOn(AgentCredential, 'revokeCascade').mockImplementationOnce(async (...args) => { + const minted = await request(app).post('/api/agent-binding/runtime-token') + .set('Authorization', `Bearer ${first.daemonToken}`) + .send({ agentName: 'bot-concurrent', instanceId: 'default', rotate: true }); + expect(minted.status).toBe(201); + mintedToken = minted.body.token; + expect(await agentCall(mintedToken)).toBe(204); + return originalCascade(...args); + }); + try { + await request(app).delete(`/api/credentials/${daemon._id}`) + .set('Authorization', `Bearer ${generateTestToken(user._id)}`).expect(200); + } finally { + cascadeSpy.mockRestore(); + } + + expect(mintedToken).toBeTruthy(); + expect((await AgentCredential.findOne({ tokenHash: hash(mintedToken) })).status).toBe('revoked'); + expect(await User.countDocuments({ 'agentRuntimeTokens.tokenHash': hash(mintedToken) })).toBe(0); + expect(await AgentInstallation.countDocuments({ 'runtimeTokens.tokenHash': hash(mintedToken) })).toBe(0); + expect(await agentCall(mintedToken)).toBe(401); + expect(await agentCall(prior.token)).toBe(401); + }); + it('retires pre-binding machines conservatively when their owner revokes a device', async () => { const user = await owner(); const device = await addDevice(user, 'd'); diff --git a/backend/routes/credentials.ts b/backend/routes/credentials.ts index ee8d721b47..192d54ac4c 100644 --- a/backend/routes/credentials.ts +++ b/backend/routes/credentials.ts @@ -124,8 +124,10 @@ router.delete('/:id', credentialRateLimit, auth, async (req: AuthReq, res: expre if (!agentName && !isOwner && caller?.role !== 'admin') { return res.status(403).json({ message: 'Access denied' }); } - // Capture descendant hashes before the cascade. A daemon revoke also - // invalidates runtime children, whose embedded copies must be removed. + // Revoke first, then collect descendant hashes. A daemon can mint a child + // after a pre-revoke snapshot; the cascade would revoke that child while + // cleanup missed its embedded User copy. + const revoked = await AgentCredential.revokeCascade(credential._id as Types.ObjectId); const runtimeHashes = new Set(); let frontier: Types.ObjectId[] = [credential._id as Types.ObjectId]; while (frontier.length) { @@ -137,7 +139,6 @@ router.delete('/:id', credentialRateLimit, auth, async (req: AuthReq, res: expre const children = await AgentCredential.find({ parentId: { $in: frontier } }).select('_id').lean(); frontier = children.map((row) => row._id as Types.ObjectId); } - const revoked = await AgentCredential.revokeCascade(credential._id as Types.ObjectId); await Promise.all([...runtimeHashes].map((tokenHash) => revokeRuntimeTokenEverywhere(tokenHash))); return res.json({ revoked }); } catch (err) {