From 6e9c3e2ecfbda4cddb0e64da556d898de1cb34ff Mon Sep 17 00:00:00 2001 From: Robin Braumann <50590409+bjorger@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:37:58 +0000 Subject: [PATCH] fix(core): Re-check user tool access on agent runtime cache hits (#37401) Co-authored-by: Cursor --- ...ent-execution-orchestrator.service.test.ts | 1 + .../agent-runtime-cache.service.test.ts | 136 ++++++++++++++++++ ...time-reconstruction-tool-filtering.test.ts | 35 +++++ .../agents/agent-runtime-cache.service.ts | 91 +++++++++++- .../agent-runtime-reconstruction.service.ts | 93 ++++++++++-- 5 files changed, 341 insertions(+), 15 deletions(-) diff --git a/packages/cli/src/modules/agents/__tests__/agent-execution-orchestrator.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-execution-orchestrator.service.test.ts index eec362d31a2..2752b0e0f8e 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-execution-orchestrator.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-execution-orchestrator.service.test.ts @@ -110,6 +110,7 @@ function makeRuntime(chunks: StreamChunk[] = [{ type: 'finish', finishReason: 's projectId, agentId, telemetryConfiguration: telemetryContext.configuration, + toolAccessCheckedAt: Date.now(), }; } diff --git a/packages/cli/src/modules/agents/__tests__/agent-runtime-cache.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-runtime-cache.service.test.ts index cb0a6b62df2..412f1e216ef 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-runtime-cache.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-runtime-cache.service.test.ts @@ -169,6 +169,142 @@ describe('AgentRuntimeCacheService', () => { } }); + it('re-checks baked-in tool access after the debounce window and rebuilds when a grant is revoked', async () => { + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + const { service, agentRepository, reconstructionService } = makeService(); + const user = mock({ id: 'user-a' }); + const snapshot = { credentialIds: ['cred-1'], workflowIds: ['wf-1'] }; + const originalRuntime = { ...makeRuntime(), userToolAccessSnapshot: snapshot }; + const rebuiltRuntime = makeRuntime(); + + agentRepository.findByIdAndProjectId.mockResolvedValue(makeAgent()); + reconstructionService.reconstructFromAgentEntity + .mockResolvedValueOnce(originalRuntime) + .mockResolvedValueOnce(rebuiltRuntime); + reconstructionService.userStillHasToolAccess.mockResolvedValue(true); + + const first = await service.getRuntime({ agentId, projectId, user }); + service.releaseRuntimeLease(first.agent); + + // Inside the debounce window: served without any re-check. + vi.setSystemTime(Date.now() + 30 * 1000); + const second = await service.getRuntime({ agentId, projectId, user }); + service.releaseRuntimeLease(second.agent); + expect(second.agent).toBe(originalRuntime.agent); + expect(reconstructionService.userStillHasToolAccess).not.toHaveBeenCalled(); + + // Past the window with grants intact: re-checked, same instance kept. + vi.setSystemTime(Date.now() + 2 * 60 * 1000); + const third = await service.getRuntime({ agentId, projectId, user }); + service.releaseRuntimeLease(third.agent); + expect(reconstructionService.userStillHasToolAccess).toHaveBeenCalledWith( + snapshot, + projectId, + user, + ); + expect(third.agent).toBe(originalRuntime.agent); + + // Grant revoked: the next re-check retires the runtime and rebuilds it. + reconstructionService.userStillHasToolAccess.mockResolvedValue(false); + vi.setSystemTime(Date.now() + 2 * 60 * 1000); + const fourth = await service.getRuntime({ agentId, projectId, user }); + + expect(fourth.agent).toBe(rebuiltRuntime.agent); + expect(originalRuntime.agent.close).toHaveBeenCalledOnce(); + expect(reconstructionService.reconstructFromAgentEntity).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); + + it('keeps the cached runtime when a tool-access re-check fails and retries after the debounce window', async () => { + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + const { service, agentRepository, reconstructionService } = makeService(); + const user = mock({ id: 'user-a' }); + const runtime = { + ...makeRuntime(), + userToolAccessSnapshot: { credentialIds: ['cred-1'], workflowIds: ['wf-1'] }, + }; + + agentRepository.findByIdAndProjectId.mockResolvedValue(makeAgent()); + reconstructionService.reconstructFromAgentEntity.mockResolvedValue(runtime); + reconstructionService.userStillHasToolAccess.mockRejectedValue( + new Error('database unavailable'), + ); + + const initial = await service.getRuntime({ agentId, projectId, user }); + service.releaseRuntimeLease(initial.agent); + vi.setSystemTime(Date.now() + 2 * 60 * 1000); + + const afterFailedRecheck = await service.getRuntime({ agentId, projectId, user }); + service.releaseRuntimeLease(afterFailedRecheck.agent); + + expect(afterFailedRecheck).toBe(initial); + expect(afterFailedRecheck.agent).toBe(runtime.agent); + expect(runtime.agent.close).not.toHaveBeenCalled(); + expect(reconstructionService.reconstructFromAgentEntity).toHaveBeenCalledOnce(); + + const withinDebounceWindow = await service.getRuntime({ agentId, projectId, user }); + service.releaseRuntimeLease(withinDebounceWindow.agent); + expect(reconstructionService.userStillHasToolAccess).toHaveBeenCalledOnce(); + + reconstructionService.userStillHasToolAccess.mockResolvedValue(true); + vi.setSystemTime(Date.now() + 2 * 60 * 1000); + const afterRetry = await service.getRuntime({ agentId, projectId, user }); + service.releaseRuntimeLease(afterRetry.agent); + + expect(afterRetry).toBe(initial); + expect(reconstructionService.userStillHasToolAccess).toHaveBeenCalledTimes(2); + expect(runtime.agent.close).not.toHaveBeenCalled(); + expect(reconstructionService.reconstructFromAgentEntity).toHaveBeenCalledOnce(); + } finally { + vi.useRealTimers(); + } + }); + + it('shares an in-flight tool-access re-check across concurrent cache hits', async () => { + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + const { service, agentRepository, reconstructionService } = makeService(); + const user = mock({ id: 'user-a' }); + const runtime = { + ...makeRuntime(), + userToolAccessSnapshot: { credentialIds: ['cred-1'], workflowIds: ['wf-1'] }, + }; + let resolveAccessCheck: (stillGranted: boolean) => void = () => {}; + const pendingAccessCheck = new Promise((resolve) => { + resolveAccessCheck = resolve; + }); + + agentRepository.findByIdAndProjectId.mockResolvedValue(makeAgent()); + reconstructionService.reconstructFromAgentEntity.mockResolvedValue(runtime); + reconstructionService.userStillHasToolAccess.mockReturnValue(pendingAccessCheck); + + const initial = await service.getRuntime({ agentId, projectId, user }); + service.releaseRuntimeLease(initial.agent); + vi.setSystemTime(Date.now() + 2 * 60 * 1000); + + const first = service.getRuntime({ agentId, projectId, user }); + const second = service.getRuntime({ agentId, projectId, user }); + + expect(reconstructionService.userStillHasToolAccess).toHaveBeenCalledOnce(); + resolveAccessCheck(true); + const [firstRuntime, secondRuntime] = await Promise.all([first, second]); + + expect(firstRuntime).toBe(secondRuntime); + expect(firstRuntime.agent).toBe(runtime.agent); + service.releaseRuntimeLease(firstRuntime.agent); + service.releaseRuntimeLease(secondRuntime.agent); + } finally { + vi.useRealTimers(); + } + }); + it('keeps draft runtimes separate by integration type', async () => { const { service, agentRepository, reconstructionService } = makeService(); const agent = makeAgent(); diff --git a/packages/cli/src/modules/agents/__tests__/agent-runtime-reconstruction-tool-filtering.test.ts b/packages/cli/src/modules/agents/__tests__/agent-runtime-reconstruction-tool-filtering.test.ts index 9ccf5fb77f3..914a392c396 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-runtime-reconstruction-tool-filtering.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-runtime-reconstruction-tool-filtering.test.ts @@ -280,6 +280,41 @@ describe('AgentRuntimeReconstructionService — per-user tool filtering', () => expect.arrayContaining(['Send Slack message', 'Lookup customer', 'custom_tool']), ); }); + + it('returns the granted credential and workflow ids so cached runtimes can re-check them', async () => { + vi.mocked(userHasScopes).mockResolvedValue(true); + const credentialsFinderService = mock(); + credentialsFinderService.findCredentialForUser.mockResolvedValue( + mock({ id: 'cred-1' }), + ); + const workflowRepository = mock(); + workflowRepository.findOneByAgentToolReference.mockResolvedValue( + mock({ id: 'wf-1' }), + ); + const workflowFinderService = mock(); + workflowFinderService.findWorkflowForUser.mockResolvedValue( + mock>>({ id: 'wf-1' }), + ); + const { service } = makeService({ + credentialsFinderService, + workflowFinderService, + workflowRepository, + }); + const entity = makeAgentEntity([nodeToolWithCredential, workflowTool, customTool]); + + const result = await service.reconstructFromAgentEntity( + entity, + mock(), + 'production', + undefined, + testUser, + ); + + expect(result.userToolAccessSnapshot).toEqual({ + credentialIds: ['cred-1'], + workflowIds: ['wf-1'], + }); + }); }); describe('AgentRuntimeReconstructionService.reconstructFromResolvedSource — per-user tool filtering', () => { diff --git a/packages/cli/src/modules/agents/agent-runtime-cache.service.ts b/packages/cli/src/modules/agents/agent-runtime-cache.service.ts index 32c143349fe..01606cbec5b 100644 --- a/packages/cli/src/modules/agents/agent-runtime-cache.service.ts +++ b/packages/cli/src/modules/agents/agent-runtime-cache.service.ts @@ -21,6 +21,7 @@ import { import { AgentSandboxRuntimeService } from './agent-sandbox-runtime.service'; import { buildAgentConfigurationTelemetry } from './agent-telemetry'; import { AgentRuntimeReconstructionService } from './agent-runtime-reconstruction.service'; +import type { UserToolAccessSnapshot } from './agent-runtime-reconstruction.service'; import type { Agent } from './entities/agent.entity'; import { AgentRepository } from './repositories/agent.repository'; import type { ToolRegistry } from './tool-registry'; @@ -43,12 +44,28 @@ export interface GetRuntimeParams { sandboxPrincipalHash?: AgentSandboxPrincipalHash; } +/** + * How long a passing tool-access re-check stays valid. Cache hits inside this + * window skip the DB checks entirely, so revoked access takes effect within + * one interval instead of whenever the runtime happens to rebuild. + */ +const TOOL_ACCESS_RECHECK_INTERVAL_MS = Time.minutes.toMilliseconds; + export interface AgentRuntime { agent: RuntimeAgent; agentId: string; toolRegistry: ToolRegistry; projectId: string; telemetryConfiguration: IAgentConfigurationTelemetryProperties; + /** + * Grants baked into the runtime's tool list by `filterToolsForUser`. + * The sliding TTL can keep an active runtime alive indefinitely, so these + * are re-checked on cache hits (debounced) rather than waiting for + * eviction. Absent when no user-gated tools made it into the runtime. + */ + userToolAccessSnapshot?: UserToolAccessSnapshot; + /** Epoch ms of the last passing tool-access re-check (build counts as one). */ + toolAccessCheckedAt: number; } interface RuntimeInitialization { @@ -66,6 +83,9 @@ export class AgentRuntimeCacheService { * TTL = 30 minutes of inactivity (sliding — each cache hit refreshes the * expiry) so actively used runtimes stay cached while idle agents are * evicted and their memory freed without an explicit shutdown step. + * Because the slide can keep a runtime alive indefinitely, user-scoped + * runtimes re-verify their baked-in tool access grants on hits (see + * `toolAccessStillCurrent`) so revocations don't outlive the cache. * * Separating draft and published with explicit prefixes prevents a draft * runtime from being mistakenly returned to a published-agent execution. @@ -81,6 +101,8 @@ export class AgentRuntimeCacheService { private readonly runtimeInitializations = new Map(); + private readonly toolAccessRechecks = new WeakMap>(); + private readonly activeRuntimeLeases = new WeakMap(); private readonly runtimesPendingClose = new WeakMap(); @@ -222,8 +244,20 @@ export class AgentRuntimeCacheService { const cached = this.runtimes.get(cacheKey); if (cached) { - this.runtimes.touch(cacheKey); - return this.acquireRuntimeLease(cached); + const accessStillCurrent = await this.toolAccessStillCurrent(cached, params); + const current = this.runtimes.get(cacheKey); + // The awaited re-check may race with cache invalidation or replacement. + if (current !== cached) { + if (current) return this.acquireRuntimeLease(current); + } else if (accessStillCurrent) { + this.runtimes.touch(cacheKey); + return this.acquireRuntimeLease(cached); + } else { + // Revoked grants: retire this runtime and rebuild below so the tool + // list is re-filtered against the user's current access. + this.runtimes.delete(cacheKey); + this.closeAgentResources(cached.agent, params.agentId); + } } const initialization = this.runtimeInitializations.get(cacheKey); @@ -255,6 +289,55 @@ export class AgentRuntimeCacheService { return this.acquireRuntimeLease(await runtimeInitialization.promise); } + /** + * Node/workflow tools are permission-filtered once at build time, so an + * actively used runtime kept alive by the sliding TTL would otherwise + * honor grants forever. Re-check the baked-in grants at most once per + * `TOOL_ACCESS_RECHECK_INTERVAL_MS`; hits inside the window are free. + */ + private async toolAccessStillCurrent( + runtime: AgentRuntime, + params: GetRuntimeParams, + ): Promise { + const { userToolAccessSnapshot } = runtime; + const { user } = params; + if (!userToolAccessSnapshot || !user) return true; + if (Date.now() - runtime.toolAccessCheckedAt < TOOL_ACCESS_RECHECK_INTERVAL_MS) return true; + + const inFlight = this.toolAccessRechecks.get(runtime); + if (inFlight) return await inFlight; + + const recheck = (async () => { + try { + const stillGranted = await this.agentRuntimeReconstructionService.userStillHasToolAccess( + userToolAccessSnapshot, + params.projectId, + user, + ); + if (stillGranted) runtime.toolAccessCheckedAt = Date.now(); + return stillGranted; + } catch (error) { + // Availability over freshness: a failing re-check must not take down + // the chat — serve the cached runtime and retry next interval. + runtime.toolAccessCheckedAt = Date.now(); + this.logger.warn('[AgentRuntimeCacheService] Failed to re-check tool access', { + agentId: runtime.agentId, + error: error instanceof Error ? error.message : String(error), + }); + return true; + } + })(); + this.toolAccessRechecks.set(runtime, recheck); + + try { + return await recheck; + } finally { + if (this.toolAccessRechecks.get(runtime) === recheck) { + this.toolAccessRechecks.delete(runtime); + } + } + } + private async reconstructRuntime(params: GetRuntimeParams): Promise { const { agentId, projectId, integrationType, usePublishedVersion, user, sandboxPrincipalHash } = params; @@ -287,7 +370,7 @@ export class AgentRuntimeCacheService { usePublishedVersion ? 'integrated' : 'manual', sandboxPrincipalHash, ); - const { agent: agentInstance, toolRegistry } = await reconstruction; + const { agent: agentInstance, toolRegistry, userToolAccessSnapshot } = await reconstruction; return { agent: agentInstance, @@ -295,6 +378,8 @@ export class AgentRuntimeCacheService { toolRegistry, projectId, telemetryConfiguration: buildAgentConfigurationTelemetry(agentData), + ...(userToolAccessSnapshot !== undefined ? { userToolAccessSnapshot } : {}), + toolAccessCheckedAt: Date.now(), }; } } diff --git a/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts b/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts index aac481ac8b9..2bba0521ac6 100644 --- a/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts +++ b/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts @@ -169,6 +169,20 @@ async function getWorkflowRunner(): Promise { return Container.get(WorkflowRunner); } +/** + * The access grants `filterToolsForUser` verified when it built a runtime's + * tool list. Its presence implies at least one node/workflow tool was kept, + * which always requires `workflow:execute` on the project. Cached runtimes + * re-check these grants periodically so access revoked after the build stops + * being honored (see `AgentRuntimeCacheService`). + */ +export interface UserToolAccessSnapshot { + /** Credential ids that passed a `credential:read` check. */ + credentialIds: string[]; + /** Workflow ids that passed a `workflow:execute` check. */ + workflowIds: string[]; +} + @Service() export class AgentRuntimeReconstructionService { constructor( @@ -204,7 +218,11 @@ export class AgentRuntimeReconstructionService { sandboxPrincipalHash?: AgentSandboxPrincipalHash, /** Pass false when the caller cannot resume a suspended run (workflow executions). */ supportsHitl?: boolean, - ): Promise<{ agent: RuntimeAgent; toolRegistry: ToolRegistry }> { + ): Promise<{ + agent: RuntimeAgent; + toolRegistry: ToolRegistry; + userToolAccessSnapshot?: UserToolAccessSnapshot; + }> { let config = agentEntity.schema; if (!config) { throw new UserError('Agent has no JSON config.'); @@ -215,11 +233,11 @@ export class AgentRuntimeReconstructionService { // chat, resume, task-now), drop node/workflow tools the user can't // execute or lacks credential/workflow access to before the runtime is // built, so denied tools never reach the LLM or the executor. + let userToolAccessSnapshot: UserToolAccessSnapshot | undefined; if (user && config.tools?.length) { - config = { - ...config, - tools: await this.filterToolsForUser(config.tools, agentEntity.projectId, user), - }; + const filtered = await this.filterToolsForUser(config.tools, agentEntity.projectId, user); + config = { ...config, tools: filtered.tools }; + userToolAccessSnapshot = filtered.snapshot; } const toolsByName: Record = {}; @@ -234,7 +252,7 @@ export class AgentRuntimeReconstructionService { agentEntity.projectId, ); - return await this.reconstructRuntime({ + const runtime = await this.reconstructRuntime({ config, memoryOwnerAgentId: agentEntity.id, projectId: agentEntity.projectId, @@ -254,6 +272,10 @@ export class AgentRuntimeReconstructionService { instrumentation, sandboxPrincipalHash, }); + return { + ...runtime, + ...(userToolAccessSnapshot !== undefined ? { userToolAccessSnapshot } : {}), + }; } /** @@ -269,10 +291,13 @@ export class AgentRuntimeReconstructionService { tools: AgentJsonToolConfig[], projectId: string, user: User, - ): Promise { + ): Promise<{ tools: AgentJsonToolConfig[]; snapshot?: UserToolAccessSnapshot }> { const canExecute = await userHasScopes(user, ['workflow:execute'], false, { projectId }); const filtered: AgentJsonToolConfig[] = []; + const grantedCredentialIds = new Set(); + const grantedWorkflowIds = new Set(); + let keptGatedTool = false; for (const ref of tools) { if (ref.type === 'custom') { filtered.push(ref); @@ -296,6 +321,8 @@ export class AgentRuntimeReconstructionService { ); if (accessibleCredentials.some((credential) => credential === null)) continue; + for (const id of credentialIds) grantedCredentialIds.add(id); + keptGatedTool = true; filtered.push(ref); continue; } @@ -311,10 +338,52 @@ export class AgentRuntimeReconstructionService { ); if (!accessibleWorkflow) continue; + grantedWorkflowIds.add(workflow.id); + keptGatedTool = true; filtered.push(ref); } - return filtered; + return { + tools: filtered, + ...(keptGatedTool + ? { + snapshot: { + credentialIds: [...grantedCredentialIds], + workflowIds: [...grantedWorkflowIds], + }, + } + : {}), + }; + } + + /** + * Re-run the access checks recorded in `snapshot` against current DB state. + * Returns false when any grant no longer holds — a runtime built from that + * snapshot is over-privileged and must be rebuilt so its tool list is + * re-filtered. + */ + async userStillHasToolAccess( + snapshot: UserToolAccessSnapshot, + projectId: string, + user: User, + ): Promise { + if (!(await userHasScopes(user, ['workflow:execute'], false, { projectId }))) return false; + + const credentials = await Promise.all( + snapshot.credentialIds.map( + async (id) => + await this.credentialsFinderService.findCredentialForUser(id, user, ['credential:read']), + ), + ); + if (credentials.some((credential) => credential === null)) return false; + + const workflows = await Promise.all( + snapshot.workflowIds.map( + async (id) => + await this.workflowFinderService.findWorkflowForUser(id, user, ['workflow:execute']), + ), + ); + return workflows.every((workflow) => Boolean(workflow)); } /** @@ -333,10 +402,10 @@ export class AgentRuntimeReconstructionService { ): Promise<{ agent: RuntimeAgent; toolRegistry: ToolRegistry }> { let config = params.config; if (params.user && config.tools?.length) { - config = { - ...config, - tools: await this.filterToolsForUser(config.tools, params.projectId, params.user), - }; + // Sub-agent runtimes are built per delegation and never cached, so the + // grant snapshot is not needed here. + const filtered = await this.filterToolsForUser(config.tools, params.projectId, params.user); + config = { ...config, tools: filtered.tools }; } const subAgentDelegation = await this.createSubAgentDelegationConfig(config, params.projectId);