diff --git a/src/main/acp/permission-broker-registry.test.ts b/src/main/acp/permission-broker-registry.test.ts index 83466faa5..258d0616d 100644 --- a/src/main/acp/permission-broker-registry.test.ts +++ b/src/main/acp/permission-broker-registry.test.ts @@ -130,6 +130,7 @@ describe('ACP permission broker with durable grants', () => { list: vi.fn().mockResolvedValue([]), listCached: vi.fn().mockReturnValue([]), revoke: vi.fn(), + extendUndo: vi.fn(), restore: vi.fn(), prune: vi.fn(), finalizeOwnerDeletion: vi.fn(), @@ -157,6 +158,7 @@ describe('ACP permission broker with durable grants', () => { list: vi.fn().mockResolvedValue([]), listCached: vi.fn().mockReturnValue([]), revoke: vi.fn(), + extendUndo: vi.fn(), restore: vi.fn(), prune: vi.fn(), finalizeOwnerDeletion: vi.fn(), diff --git a/src/main/compute/compute-approval-broker.test.ts b/src/main/compute/compute-approval-broker.test.ts index 80e155082..4568cbaee 100644 --- a/src/main/compute/compute-approval-broker.test.ts +++ b/src/main/compute/compute-approval-broker.test.ts @@ -138,6 +138,43 @@ describe('ComputeApprovalBroker', () => { expect(remember).not.toHaveBeenCalled() }) + it('does not create a pending approval after invalidation sweeps pending requests', async () => { + let finishGrantLookup: (() => void) | undefined + const resolveGrant = vi.fn( + () => + new Promise((resolve) => { + finishGrantLookup = () => resolve(undefined) + }) + ) + const broadcast = vi.fn() + const broker = new ComputeApprovalBroker({ + generateId: () => 'id-1', + broadcast, + permissionGrants: { resolve: resolveGrant, remember: vi.fn() } as never + }) + + const decision = broker.requestWithContext(makeRequest(), { + sessionId: 'session-1', + projectId: 'project-1', + operation: 'call_command', + ownerId: 'host-row-1' + }) + await vi.waitFor(() => expect(resolveGrant).toHaveBeenCalledOnce()) + + let invalidationCompleted = false + const invalidation = broker.invalidateProvider('ssh:biowulf').then(() => { + invalidationCompleted = true + }) + await Promise.resolve() + expect(invalidationCompleted).toBe(false) + + finishGrantLookup?.() + await invalidation + await expect(decision).resolves.toBe('deny') + expect(broadcast).not.toHaveBeenCalled() + broker.completeProviderInvalidation('ssh:biowulf') + }) + it('does not remember approval when the provider id belongs to a recreated host', async () => { const timer = makeTimer() const remember = vi.fn() @@ -168,6 +205,92 @@ describe('ComputeApprovalBroker', () => { expect(remember).not.toHaveBeenCalled() }) + it('drains an approval persistence tail before provider deletion proceeds', async () => { + const timer = makeTimer() + let releaseRemember: (() => void) | undefined + const remember = vi.fn( + () => + new Promise((resolve) => { + releaseRemember = resolve + }) + ) + const broker = new ComputeApprovalBroker({ + generateId: () => 'id-1', + broadcast: () => undefined, + setTimer: timer.set, + clearTimer: timer.clear, + permissionGrants: { resolve: vi.fn(), remember } as never, + isProviderCurrent: vi.fn().mockResolvedValue(true) + }) + + const decision = broker.requestWithContext(makeRequest(), { + sessionId: 'session-1', + projectId: 'project-1', + operation: 'call_command', + ownerId: 'host-row-1' + }) + await Promise.resolve() + broker.respond('id-1', 'project') + await vi.waitFor(() => expect(remember).toHaveBeenCalledOnce()) + + let invalidationCompleted = false + const invalidation = broker.invalidateProvider('ssh:biowulf').then(() => { + invalidationCompleted = true + }) + await Promise.resolve() + expect(invalidationCompleted).toBe(false) + + releaseRemember?.() + await invalidation + await expect(decision).resolves.toBe('deny') + broker.completeProviderInvalidation('ssh:biowulf') + }) + + it('denies new requests while provider deletion is draining', async () => { + const broker = new ComputeApprovalBroker({ + generateId: () => 'id-1', + broadcast: vi.fn(), + permissionGrants: { resolve: vi.fn(), remember: vi.fn() } as never + }) + + await broker.invalidateProvider('ssh:biowulf') + await expect( + broker.requestWithContext(makeRequest(), { + sessionId: 'session-1', + projectId: 'project-1', + operation: 'call_command', + ownerId: 'host-row-1' + }) + ).resolves.toBe('deny') + broker.completeProviderInvalidation('ssh:biowulf') + }) + + it('denies a stale Once request that reaches the broker after provider deletion completes', async () => { + const broadcast = vi.fn() + const remember = vi.fn() + const broker = new ComputeApprovalBroker({ + generateId: () => 'id-1', + broadcast, + permissionGrants: { resolve: vi.fn(), remember } as never, + isProviderCurrent: vi.fn().mockResolvedValue(false) + }) + + await broker.invalidateProvider('ssh:biowulf') + broker.completeProviderInvalidation('ssh:biowulf') + + const decision = broker.requestWithContext(makeRequest(), { + sessionId: 'session-1', + projectId: 'project-1', + operation: 'call_command', + ownerId: 'deleted-host-row' + }) + await vi.waitFor(() => expect(broadcast).toHaveBeenCalledOnce()) + broker.respond('id-1', 'once') + + await expect(decision).resolves.toBe('deny') + expect(remember).not.toHaveBeenCalled() + }) + it('does not auto-allow an existing grant for a replacement host with the same provider id', async () => { const broadcast = vi.fn() const isProviderCurrent = vi.fn().mockResolvedValue(false) diff --git a/src/main/compute/compute-approval-broker.ts b/src/main/compute/compute-approval-broker.ts index 550afa0b5..0598806c5 100644 --- a/src/main/compute/compute-approval-broker.ts +++ b/src/main/compute/compute-approval-broker.ts @@ -67,6 +67,8 @@ export class ComputeApprovalBroker { >() private readonly providerGenerations = new Map() + private readonly invalidatingProviders = new Set() + private readonly inFlightRequests = new Map>>() // Legacy fallback used only when no durable adapter is supplied. private readonly conversationGrants = new Set() @@ -99,7 +101,25 @@ export class ComputeApprovalBroker { // Like request(), but checks conversation and project grants first. If a grant matches, resolves // immediately without broadcasting. When the user responds with a scope that has memory, records it. - async requestWithContext( + requestWithContext( + info: Omit, + ctx: ComputeApprovalContext + ): Promise { + const providerId = info.provider_id + if (this.invalidatingProviders.has(providerId)) return Promise.resolve('deny') + + const request = this.requestWithContextOperation(info, ctx) + const requests = this.inFlightRequests.get(providerId) ?? new Set() + requests.add(request) + this.inFlightRequests.set(providerId, requests) + void request.then( + () => this.releaseInFlightRequest(providerId, request), + () => this.releaseInFlightRequest(providerId, request) + ) + return request + } + + private async requestWithContextOperation( info: Omit, ctx: ComputeApprovalContext ): Promise { @@ -126,22 +146,38 @@ export class ComputeApprovalBroker { // ── legacy project grant check (persistent) ─────────────────────────────────── if (this.deps.checkProjectGrant) { const hasProject = await this.deps.checkProjectGrant({ projectId, operation, providerId }) - if (hasProject) return 'project' + if (hasProject) { + return (await this.isProviderCurrent(providerId, ctx.ownerId, providerGeneration)) + ? 'project' + : 'deny' + } } // ── conversation grant check (session in-memory) ─────────────────────────────── const convKey = `${sessionId}:${operation}:${providerId}` - if (this.conversationGrants.has(convKey)) return 'conversation' + if (this.conversationGrants.has(convKey)) { + return (await this.isProviderCurrent(providerId, ctx.ownerId, providerGeneration)) + ? 'conversation' + : 'deny' + } // ── no grant — show approval card ───────────────────────────────────────────── + // Grant lookups above are asynchronous. Invalidation may have started after this operation + // entered the in-flight set but before it reached the approval card. Fail closed here so the + // invalidator cannot miss a newly-created pending request and wait on it indefinitely. + if ( + this.invalidatingProviders.has(providerId) || + (this.providerGenerations.get(providerId) ?? 0) !== providerGeneration + ) { + return 'deny' + } const decision = await this.request(info, ctx) if ((this.providerGenerations.get(providerId) ?? 0) !== providerGeneration) return 'deny' - const remembersDecision = - decision === 'conversation' || decision === 'project' || decision === 'global' + const allowsDecision = decision !== 'deny' if ( - remembersDecision && + allowsDecision && !(await this.isProviderCurrent(providerId, ctx.ownerId, providerGeneration)) ) { return 'deny' @@ -159,6 +195,13 @@ export class ComputeApprovalBroker { await this.deps.saveProjectGrant({ projectId, operation, providerId }) } + if ( + allowsDecision && + !(await this.isProviderCurrent(providerId, ctx.ownerId, providerGeneration)) + ) { + return 'deny' + } + return decision } @@ -169,7 +212,8 @@ export class ComputeApprovalBroker { // Host deletion begins by advancing its generation and denying every approval card that was // created for the old owner. A later host may reuse providerId, but it cannot reuse these calls. - invalidateProvider(providerId: string): void { + async invalidateProvider(providerId: string): Promise { + this.invalidatingProviders.add(providerId) this.providerGenerations.set(providerId, (this.providerGenerations.get(providerId) ?? 0) + 1) for (const key of this.conversationGrants) { if (key.endsWith(`:${providerId}`)) this.conversationGrants.delete(key) @@ -177,6 +221,20 @@ export class ComputeApprovalBroker { for (const [id, entry] of this.pending) { if (entry.providerId === providerId) this.settle(id, 'deny') } + await Promise.allSettled(Array.from(this.inFlightRequests.get(providerId) ?? [])) + } + + completeProviderInvalidation(providerId: string): void { + this.invalidatingProviders.delete(providerId) + } + + private releaseInFlightRequest( + providerId: string, + request: Promise + ): void { + const requests = this.inFlightRequests.get(providerId) + requests?.delete(request) + if (requests?.size === 0) this.inFlightRequests.delete(providerId) } private async isProviderCurrent( diff --git a/src/main/compute/ipc.test.ts b/src/main/compute/ipc.test.ts index c0f7457ec..30f062a79 100644 --- a/src/main/compute/ipc.test.ts +++ b/src/main/compute/ipc.test.ts @@ -325,7 +325,11 @@ describe('host delete guard', () => { const list = vi.fn(() => Promise.resolve([])) const hasActive = vi.fn(() => Promise.resolve(false)) const invalidateProvider = vi.fn() - const broker = { invalidateProvider } as unknown as ComputeApprovalBroker + const completeProviderInvalidation = vi.fn() + const broker = { + invalidateProvider, + completeProviderInvalidation + } as unknown as ComputeApprovalBroker const handlers = createComputeHandlers( mockRepository({ delete: del, list }), undefined, @@ -342,6 +346,7 @@ describe('host delete guard', () => { expect(invalidateProvider.mock.invocationCallOrder[0]).toBeLessThan( del.mock.invocationCallOrder[0] ) + expect(completeProviderInvalidation).toHaveBeenCalledWith('ssh:biowulf') }) it('allows deletion when no jobRepository is provided (backward compatibility)', async () => { @@ -369,7 +374,11 @@ describe('host delete guard', () => { const get = vi.fn().mockResolvedValue(null) const create = vi.fn().mockResolvedValue(sampleHost({ id: 'replacement-host' })) const invalidateProvider = vi.fn() - const broker = { invalidateProvider } as unknown as ComputeApprovalBroker + const completeProviderInvalidation = vi.fn() + const broker = { + invalidateProvider, + completeProviderInvalidation + } as unknown as ComputeApprovalBroker const permissionGrantRegistry = { prune } as unknown as PermissionGrantRegistry const handlers = createComputeHandlers( mockRepository({ delete: del, get, create }), diff --git a/src/main/compute/ipc.ts b/src/main/compute/ipc.ts index 5ac4a2322..d285cbbcb 100644 --- a/src/main/compute/ipc.ts +++ b/src/main/compute/ipc.ts @@ -338,9 +338,13 @@ const createComputeHandlers = ( ) } } - broker.invalidateProvider(providerId) - await repository.delete(providerId) - await permissionGrantRegistry?.prune({ kind: 'compute_provider', providerId }) + await broker.invalidateProvider(providerId) + try { + await repository.delete(providerId) + await permissionGrantRegistry?.prune({ kind: 'compute_provider', providerId }) + } finally { + broker.completeProviderInvalidation(providerId) + } }), sshConfigAliases: () => listSshAliases(), probe: (providerId) => service.probe(providerId), diff --git a/src/main/connectors/service.test.ts b/src/main/connectors/service.test.ts index 292e7e635..0b670ae59 100644 --- a/src/main/connectors/service.test.ts +++ b/src/main/connectors/service.test.ts @@ -125,6 +125,92 @@ describe('ConnectorService', () => { }) }) + it('does not dispatch a bundled call blocked while its approval is pending', async () => { + const fetchImpl = vi.fn() + let connectors = { + enabledIds: [] as string[], + autoAllowIds: [] as string[], + askToolIds: ['chemistry/pubchem_get_compounds'], + blockedToolIds: [] as string[] + } + let settleApproval: ((decision: 'once') => void) | undefined + const requestApproval = vi.fn( + () => + new Promise<'once'>((resolve) => { + settleApproval = resolve + }) + ) + const svc = new ConnectorService({ + engine: new ParserEngine({ fetchImpl }), + getConnectors: () => connectors, + getConnectorsFresh: async () => connectors, + resolveApiKey: () => undefined, + requestApproval + }) + + const call = svc.call('chemistry', 'pubchem_get_compounds', { cids: [1] }, internal) + await vi.waitFor(() => expect(requestApproval).toHaveBeenCalledOnce()) + connectors = { + ...connectors, + blockedToolIds: ['chemistry/pubchem_get_compounds'] + } + settleApproval?.('once') + + await expect(call).rejects.toThrow(/blocked by policy/) + expect(fetchImpl).not.toHaveBeenCalled() + }) + + it('does not dispatch from a stale cached Allow after durable policy becomes Block', async () => { + const fetchImpl = vi.fn() + const cached = { + enabledIds: [] as string[], + autoAllowIds: ['chemistry'], + askToolIds: [] as string[], + blockedToolIds: [] as string[] + } + const durable = { + ...cached, + blockedToolIds: ['chemistry/pubchem_get_compounds'] + } + const requestApproval = vi.fn() + const getConnectorsFresh = vi.fn().mockResolvedValue(durable) + const svc = new ConnectorService({ + engine: new ParserEngine({ fetchImpl }), + getConnectors: () => cached, + getConnectorsFresh, + resolveApiKey: () => undefined, + requestApproval + }) + + await expect( + svc.call('chemistry', 'pubchem_get_compounds', { cids: [1] }, internal) + ).rejects.toThrow(/blocked by policy/) + expect(getConnectorsFresh).toHaveBeenCalledOnce() + expect(requestApproval).not.toHaveBeenCalled() + expect(fetchImpl).not.toHaveBeenCalled() + }) + + it('does not reject a bundled call from stale cached Disabled after durable Enable', async () => { + const localHandler = vi.fn().mockResolvedValue({ ok: true }) + const cached = { + enabledIds: [] as string[], + autoAllowIds: [] as string[], + disabledConnectorIds: ['chemistry'] + } + const durable = { ...cached, disabledConnectorIds: [] as string[] } + const svc = new ConnectorService({ + getConnectors: () => cached, + getConnectorsFresh: vi.fn().mockResolvedValue(durable), + resolveApiKey: () => undefined, + localToolHandlers: { 'chemistry/pubchem_get_compounds': localHandler } + }) + + await expect( + svc.call('chemistry', 'pubchem_get_compounds', { cids: [1] }, internal) + ).resolves.toEqual({ ok: true }) + expect(localHandler).toHaveBeenCalledOnce() + }) + // Pins the ConnectorCallContext → ensureApproved → requestApproval seam. The connector service // already received the triggering session; a prior regression dropped it here, which made // ApprovalBroker → notification routing click on the wrong conversation (or none at all for @@ -361,6 +447,119 @@ describe('ConnectorService', () => { ) }) + it('does not discover or dispatch a custom server blocked while approval is pending', async () => { + const call = vi.fn() + const mcpClientManager = manager(call) + let connectors = { + enabledIds: [] as string[], + autoAllowIds: [] as string[], + askToolIds: ['myserver/do_thing'], + blockedToolIds: [] as string[], + customMcpServers: [ + { + id: '11111111-1111-4111-8111-111111111111', + name: 'myserver', + transport: 'stdio' as const, + command: 'server-command', + enabled: true + } + ] + } + let settleApproval: ((decision: 'once') => void) | undefined + const requestApproval = vi.fn( + () => + new Promise<'once'>((resolve) => { + settleApproval = resolve + }) + ) + const svc = new ConnectorService({ + mcpClientManager, + getConnectors: () => connectors, + getConnectorsFresh: async () => connectors, + resolveApiKey: () => undefined, + requestApproval + }) + + const pendingCall = svc.call('myserver', 'do_thing', {}, internal) + await vi.waitFor(() => expect(requestApproval).toHaveBeenCalledOnce()) + connectors = { ...connectors, blockedToolIds: ['myserver/do_thing'] } + settleApproval?.('once') + + await expect(pendingCall).rejects.toThrow(/blocked by policy/) + expect(mcpClientManager.listTools).not.toHaveBeenCalled() + expect(call).not.toHaveBeenCalled() + }) + + it('does not discover a custom server from a stale cached Allow after durable Block', async () => { + const call = vi.fn() + const mcpClientManager = manager(call) + const customMcpServers = [ + { + id: '11111111-1111-4111-8111-111111111111', + name: 'myserver', + transport: 'stdio' as const, + command: 'server-command', + enabled: true + } + ] + const cached = { + enabledIds: [] as string[], + autoAllowIds: ['myserver'], + askToolIds: [] as string[], + blockedToolIds: [] as string[], + customMcpServers + } + const durable = { + ...cached, + blockedToolIds: ['myserver/do_thing'] + } + const svc = new ConnectorService({ + mcpClientManager, + getConnectors: () => cached, + getConnectorsFresh: vi.fn().mockResolvedValue(durable), + resolveApiKey: () => undefined, + requestApproval: vi.fn() + }) + + await expect(svc.call('myserver', 'do_thing', {}, internal)).rejects.toThrow( + /blocked by policy/ + ) + expect(mcpClientManager.listTools).not.toHaveBeenCalled() + expect(call).not.toHaveBeenCalled() + }) + + it('discovers a newly durable custom server before its cached projection refreshes', async () => { + const call = vi.fn().mockResolvedValue({ ok: true }) + const mcpClientManager = manager(call) + const durable = { + enabledIds: [] as string[], + autoAllowIds: [] as string[], + customMcpServers: [ + { + id: '11111111-1111-4111-8111-111111111111', + name: 'myserver', + transport: 'stdio' as const, + command: 'server-command', + enabled: true + } + ] + } + const svc = new ConnectorService({ + mcpClientManager, + getConnectors: () => ({ + enabledIds: [], + autoAllowIds: [], + customMcpServers: [] + }), + getConnectorsFresh: vi.fn().mockResolvedValue(durable), + resolveApiKey: () => undefined + }) + + await expect(svc.call('myserver', 'do_thing', {}, internal)).resolves.toEqual({ ok: true }) + expect(mcpClientManager.listTools).toHaveBeenCalledOnce() + expect(call).toHaveBeenCalledOnce() + }) + it('routes a call to a remote (streamable_http) custom server with its url/headers', async () => { const call = vi.fn().mockResolvedValue({ ok: true }) const svc = new ConnectorService({ diff --git a/src/main/connectors/service.ts b/src/main/connectors/service.ts index 218c94936..c5e8f517d 100644 --- a/src/main/connectors/service.ts +++ b/src/main/connectors/service.ts @@ -9,6 +9,7 @@ import type { StoredConnectors, StoredCustomMcpServer } from '../settings/types' import type { PermissionGrantRegistry } from '../permission-grants/registry' import { ConnectorPermissionBroker } from '../permission-grants/connector-broker' import type { ConnectorPermissionRequest } from '../permission-grants/connector-broker' +import type { PermissionGrantScope } from '../../shared/permission-grants' import type { ApprovalDecision, ConnectorApprovalScope } from '../../shared/settings' import type { SpecialistProfileView } from '../../shared/specialist' @@ -25,6 +26,9 @@ type ConnectorServiceDeps = { engine?: ParserEngine mcpClientManager?: McpClientManagerLike getConnectors: () => StoredConnectors | undefined + // Re-read durable settings after an asynchronous approval/grant lookup so a policy change that + // completed while the call was waiting remains the final dispatch boundary. + getConnectorsFresh?: () => Promise resolveApiKey: (ref?: string) => string | undefined permissionGrantRegistry?: PermissionGrantRegistry // Human approval gate for a tool call that isn't pre-approved. A connector call sends data to an @@ -135,9 +139,12 @@ export class ConnectorService { ) } - isEnabled(connector: string): boolean { + isEnabled( + connector: string, + connectors: StoredConnectors | undefined = this.deps.getConnectors() + ): boolean { // Bundled connectors are enabled by default; only an explicit opt-out disables one. - return !(this.deps.getConnectors()?.disabledConnectorIds ?? []).includes(connector) + return !(connectors?.disabledConnectorIds ?? []).includes(connector) } // Invalidates every call that captured the previous custom-server configuration. While the @@ -176,7 +183,7 @@ export class ConnectorService { const isBundled = descriptor !== undefined || ALL_CONNECTOR_IDS.includes(connector) if (isBundled) return this.callBundled(connector, method, args, descriptor, context, access) - const custom = (this.deps.getConnectors()?.customMcpServers ?? []).find( + const custom = ((await this.currentConnectors())?.customMcpServers ?? []).find( (s) => s.name === connector ) if (!custom) { @@ -225,22 +232,19 @@ export class ConnectorService { context: ConnectorCallContext, access: ConnectorAccess ): Promise { - if (!access.bypassMainEnablement && !this.isEnabled(connector)) { - throw new ConnectorGateError('connector_disabled', `connector not enabled: ${connector}`) - } if (!descriptor) throw new ConnectorGateError('connector_unavailable', `unknown tool: ${connector}/${method}`) - if (!access.bypassMainPolicy) { - await this.ensureAuthorized(connector, connector, [connector], method, args, context) - } + const authorizedConnectors = access.bypassMainPolicy + ? undefined + : await this.ensureAuthorized(connector, connector, [connector], method, args, context) // Bundled tools that need privileged local behavior run here, after the same gate, instead of the // read-only HTTP engine. const localHandler = this.deps.localToolHandlers?.[`${connector}/${method}`] if (localHandler) return localHandler(args, context) - return this.engine.call(descriptor, args, this.credentials()) + return this.engine.call(descriptor, args, this.credentials(authorizedConnectors)) } private async callCustom( @@ -259,25 +263,18 @@ export class ConnectorService { if (!this.isCustomConfigRunnable(custom)) throw new ConnectorGateError('connector_unavailable') if (!this.deps.mcpClientManager) throw new ConnectorGateError('connector_runtime_unavailable') - const config = toCustomMcpConfig(custom) - const request = this.authorizationRequest( - custom.name, - custom.id, - [custom.id, custom.name], + // Approval must precede tools/list because even discovery connects the external server. The + // authorization state is retained across later policy rechecks so one Once approval never prompts + // twice merely because discovery itself was asynchronous. + let authorization = await this.authorizeCustomForCurrentPolicy( + custom, method, args, - context + context, + access, + generation ) - // A Block decision must short-circuit before tools/list starts or connects an external process. - const policyDecision = access.bypassMainPolicy - ? undefined - : this.permissionBroker.preflight(request) - // Approval must precede tools/list because even discovery connects the external server. Defer - // durable broad grants until the approved method is confirmed in the discovered catalog. - const deferredScope = access.bypassMainPolicy - ? undefined - : await this.permissionBroker.authorize(request, policyDecision, { deferRemember: true }) - this.assertCustomServerCurrent(custom, generation) + const config = toCustomMcpConfig(authorization.custom) let tools: Array<{ name: string }> try { @@ -298,13 +295,31 @@ export class ConnectorService { if (!tools.some((tool) => tool.name === method)) { throw new ConnectorGateError( 'connector_unavailable', - `unknown tool: ${custom.name}/${method}` + `unknown tool: ${authorization.custom.name}/${method}` ) } - this.assertCustomServerCurrent(custom, generation) - if (deferredScope) await this.permissionBroker.remember(request, deferredScope) + authorization = await this.authorizeCustomForCurrentPolicy( + authorization.custom, + method, + args, + context, + access, + generation, + authorization + ) + if (authorization.deferredScope) { + await this.permissionBroker.remember(authorization.request, authorization.deferredScope) + } - this.assertCustomServerCurrent(custom, generation) + await this.authorizeCustomForCurrentPolicy( + authorization.custom, + method, + args, + context, + access, + generation, + authorization + ) try { const result = await this.deps.mcpClientManager.call(config, method, args) @@ -359,17 +374,100 @@ export class ConnectorService { method: string, args: Record, context: ConnectorCallContext - ): Promise { - await this.permissionBroker.authorize( - this.authorizationRequest( + ): Promise { + let requireApprovalSatisfied = false + for (;;) { + const connectors = await this.currentConnectors() + if (!this.isEnabled(connectorLabel, connectors)) { + throw new ConnectorGateError( + 'connector_disabled', + `connector not enabled: ${connectorLabel}` + ) + } + const request = this.authorizationRequest( connectorLabel, capabilityServerId, policyIds, method, args, - context + context, + connectors ) - ) + const policyDecision = this.permissionBroker.preflight(request) + if (policyDecision === 'allow' || requireApprovalSatisfied) return connectors + + await this.permissionBroker.authorize(request, policyDecision) + requireApprovalSatisfied = true + } + } + + private async authorizeCustomForCurrentPolicy( + custom: StoredCustomMcpServer, + method: string, + args: Record, + context: ConnectorCallContext, + access: ConnectorAccess, + generation: number, + prior?: { + requireApprovalSatisfied: boolean + deferredScope?: PermissionGrantScope + } + ): Promise<{ + custom: StoredCustomMcpServer + request: ConnectorPermissionRequest + requireApprovalSatisfied: boolean + deferredScope?: PermissionGrantScope + }> { + let requireApprovalSatisfied = prior?.requireApprovalSatisfied ?? false + let deferredScope = prior?.deferredScope + + for (;;) { + const connectors = await this.currentConnectors() + const current = (connectors?.customMcpServers ?? []).find((server) => server.id === custom.id) + if (!current) throw new ConnectorGateError('connector_unavailable') + this.assertCustomServerCurrent(current, generation) + if (!access.bypassMainEnablement && !current.enabled) { + throw new ConnectorGateError('connector_disabled', `connector not enabled: ${current.name}`) + } + if (!this.isCustomConfigRunnable(current)) { + throw new ConnectorGateError('connector_unavailable') + } + + const request = this.authorizationRequest( + current.name, + current.id, + [current.id, current.name], + method, + args, + context, + connectors + ) + if (access.bypassMainPolicy) { + return { custom: current, request, requireApprovalSatisfied } + } + + const policyDecision = this.permissionBroker.preflight(request) + if (policyDecision === 'allow') { + return { custom: current, request, requireApprovalSatisfied } + } + if (requireApprovalSatisfied) { + return { + custom: current, + request, + requireApprovalSatisfied, + ...(deferredScope ? { deferredScope } : {}) + } + } + + deferredScope = await this.permissionBroker.authorize(request, policyDecision, { + deferRemember: true + }) + requireApprovalSatisfied = true + } + } + + private currentConnectors(): Promise { + return this.deps.getConnectorsFresh?.() ?? Promise.resolve(this.deps.getConnectors()) } private authorizationRequest( @@ -378,9 +476,9 @@ export class ConnectorService { policyIds: readonly string[], method: string, args: Record, - context: ConnectorCallContext + context: ConnectorCallContext, + connectors: StoredConnectors | undefined = this.deps.getConnectors() ): ConnectorPermissionRequest { - const c = this.deps.getConnectors() return { capability: { kind: 'mcp_tool', key: `mcp:${capabilityServerId}/${method}` }, context, @@ -389,15 +487,16 @@ export class ConnectorService { args, policy: { aliases: policyIds, - autoAllowIds: c?.autoAllowIds, - blockedToolIds: c?.blockedToolIds, - askToolIds: c?.askToolIds + autoAllowIds: connectors?.autoAllowIds, + blockedToolIds: connectors?.blockedToolIds, + askToolIds: connectors?.askToolIds } } } - private credentials(): ConnectorCredentials { - const c = this.deps.getConnectors() + private credentials( + c: StoredConnectors | undefined = this.deps.getConnectors() + ): ConnectorCredentials { return { ncbiEmail: c?.contactEmail, ncbiApiKey: this.deps.resolveApiKey(c?.ncbiApiKeyRef) } } } diff --git a/src/main/ipc.ts b/src/main/ipc.ts index e2061a308..0aa17ef0d 100644 --- a/src/main/ipc.ts +++ b/src/main/ipc.ts @@ -469,6 +469,7 @@ const registerIpcHandlers = async ({ }) const connectorService = new ConnectorService({ getConnectors: () => connectorsSnapshot, + getConnectorsFresh: () => settingsService.getConnectors(), resolveApiKey: (ref) => tryDecryptKey(ref), mcpClientManager, permissionGrantRegistry, diff --git a/src/main/permission-grants/ipc.test.ts b/src/main/permission-grants/ipc.test.ts index 56d3a9dd9..dd255176f 100644 --- a/src/main/permission-grants/ipc.test.ts +++ b/src/main/permission-grants/ipc.test.ts @@ -23,6 +23,11 @@ describe('permission grant IPC', () => { const registry = { list: vi.fn().mockResolvedValue([]), revoke: vi.fn().mockResolvedValue({ grants: [], conflicts: [] }), + extendUndo: vi.fn().mockResolvedValue({ + undoToken: 'undo-1', + expiresAt: 10, + revokedCount: 1 + }), restore: vi.fn().mockResolvedValue({ grants: [], conflicts: [] }), subscribe: vi.fn((next: () => void) => { listener = next @@ -40,16 +45,24 @@ describe('permission grant IPC', () => { expect([...handlers.keys()]).toEqual([ 'permissions:list', 'permissions:revoke', + 'permissions:extend-undo', 'permissions:restore' ]) expect(webRpc.channels()).toEqual( - expect.arrayContaining(['permissions:list', 'permissions:revoke', 'permissions:restore']) + expect.arrayContaining([ + 'permissions:list', + 'permissions:revoke', + 'permissions:extend-undo', + 'permissions:restore' + ]) ) await handlers.get('permissions:revoke')?.(undefined, { grants: [{ id: 'grant-1', revision: 2 }] }) + await handlers.get('permissions:extend-undo')?.(undefined, { undoToken: 'undo-1' }) await handlers.get('permissions:restore')?.(undefined, { undoToken: 'undo-1' }) expect(registry.revoke).toHaveBeenCalledWith({ grants: [{ id: 'grant-1', revision: 2 }] }) + expect(registry.extendUndo).toHaveBeenCalledWith({ undoToken: 'undo-1' }) expect(registry.restore).toHaveBeenCalledWith({ undoToken: 'undo-1' }) controller.invalidateProjection() diff --git a/src/main/permission-grants/ipc.ts b/src/main/permission-grants/ipc.ts index 2a41c899e..1a97d9bec 100644 --- a/src/main/permission-grants/ipc.ts +++ b/src/main/permission-grants/ipc.ts @@ -4,6 +4,8 @@ import type { PermissionGrantRestoreRequest, PermissionGrantRevokeRequest, PermissionGrantSnapshot, + PermissionGrantUndoExtendRequest, + PermissionGrantUndoReceipt, PermissionGrantsChangedEvent } from '../../shared/permission-grants' import type { Project } from '../../shared/projects' @@ -120,6 +122,16 @@ const registerPermissionGrantIpcHandlers = ( return mutationSnapshot(result) } ) + ipcMainHandle( + 'permissions:extend-undo', + async ( + _event, + request: PermissionGrantUndoExtendRequest + ): Promise => { + validateRestoreRequest(request) + return options.registry.extendUndo(request) + } + ) ipcMainHandle( 'permissions:restore', async ( diff --git a/src/main/permission-grants/reconciliation.test.ts b/src/main/permission-grants/reconciliation.test.ts index 0ad306918..3749fd5c3 100644 --- a/src/main/permission-grants/reconciliation.test.ts +++ b/src/main/permission-grants/reconciliation.test.ts @@ -13,25 +13,27 @@ describe('reconcilePermissionGrantOwners', () => { it('prunes only orphaned Session and dynamic soft-owner grants', async () => { const staleServerId = '11111111-1111-4111-8111-111111111111' const liveServerId = '22222222-2222-4222-8222-222222222222' - const list = vi - .fn() - .mockResolvedValue([ - record( - 'live-session', - { kind: 'execution', key: 'exec:agent/shell' }, - { kind: 'session', projectId: 'project-1', sessionId: 'session-live' } - ), - record( - 'stale-session', - { kind: 'execution', key: 'exec:agent/shell' }, - { kind: 'session', projectId: 'project-1', sessionId: 'session-stale' } - ), - record('live-custom', { kind: 'mcp_tool', key: `mcp:${liveServerId}/search` }), - record('stale-custom', { kind: 'mcp_tool', key: `mcp:${staleServerId}/search` }), - record('app-mcp', { kind: 'mcp_tool', key: 'mcp:open-science-notebook/notebook_execute' }), - record('live-compute', { kind: 'execution', key: 'exec:compute/ssh:live/call_command' }), - record('stale-compute', { kind: 'execution', key: 'exec:compute/ssh:stale/download' }) - ]) + const list = vi.fn().mockResolvedValue([ + record( + 'live-session', + { kind: 'execution', key: 'exec:agent/shell' }, + { kind: 'session', projectId: 'project-1', sessionId: 'session-live' } + ), + record( + 'stale-session', + { kind: 'execution', key: 'exec:agent/shell' }, + { kind: 'session', projectId: 'project-1', sessionId: 'session-stale' } + ), + record('live-custom', { kind: 'mcp_tool', key: `mcp:${liveServerId}/search` }), + record('stale-custom', { kind: 'mcp_tool', key: `mcp:${staleServerId}/search` }), + record('app-mcp', { kind: 'mcp_tool', key: 'mcp:open-science-notebook/notebook_execute' }), + record('live-compute', { kind: 'execution', key: 'exec:compute/ssh:live/call_command' }), + record('live-compute-slash', { + kind: 'execution', + key: 'exec:compute/ssh:cluster/team/submit_job' + }), + record('stale-compute', { kind: 'execution', key: 'exec:compute/ssh:stale/download' }) + ]) const prune = vi.fn().mockResolvedValue([]) await reconcilePermissionGrantOwners( @@ -39,7 +41,7 @@ describe('reconcilePermissionGrantOwners', () => { { sessions: [{ projectId: 'project-1', sessionId: 'session-live' }], customServerIds: [liveServerId], - computeProviderIds: ['ssh:live'] + computeProviderIds: ['ssh:live', 'ssh:cluster/team'] } ) diff --git a/src/main/permission-grants/reconciliation.ts b/src/main/permission-grants/reconciliation.ts index 8e97aba61..6b91051d9 100644 --- a/src/main/permission-grants/reconciliation.ts +++ b/src/main/permission-grants/reconciliation.ts @@ -57,7 +57,9 @@ const reconcilePermissionGrantOwners = async ( } if (record.capability.kind === 'execution') { - const providerId = /^exec:compute\/([^/]+)\//.exec(record.capability.key)?.[1] + // Compute provider ids embed the user-selected SSH alias. Treat only the final path segment as + // the operation so valid aliases containing '/' are reconciled against their complete id. + const providerId = /^exec:compute\/(.+)\/[^/]+$/.exec(record.capability.key)?.[1] if (computeProviderIds && providerId && !computeProviderIds.has(providerId)) { owners.set(`compute:${providerId}`, { kind: 'compute_provider', providerId }) } diff --git a/src/main/permission-grants/registry.test.ts b/src/main/permission-grants/registry.test.ts index 172ca28c7..ecdfb4f65 100644 --- a/src/main/permission-grants/registry.test.ts +++ b/src/main/permission-grants/registry.test.ts @@ -153,6 +153,32 @@ describe('PermissionGrantRegistry', () => { ).resolves.toMatchObject({ matchedScope: 'global' }) }) + it('does not authorize a grant revoked while scope liveness is being checked', async () => { + const client = await openClient() + let releaseLiveness: ((live: boolean) => void) | undefined + let deferLiveness = false + const registry = await createPermissionGrantRegistry({ + getClient: async () => client, + createId: () => 'grant-1', + isScopeLive: async () => + deferLiveness + ? new Promise((resolve) => { + releaseLiveness = resolve + }) + : true + }) + const capability = { kind: 'file_operation' as const, key: 'file:read' } + const grant = await registry.remember({ capability, scope: { kind: 'global' } }) + + deferLiveness = true + const resolving = registry.resolve(capability, {}) + await vi.waitFor(() => expect(releaseLiveness).toBeTypeOf('function')) + await registry.revoke({ grants: [{ id: grant.id, revision: grant.revision }] }) + releaseLiveness?.(true) + + await expect(resolving).resolves.toBeUndefined() + }) + it('revokes an exact revision and restores it through a one-time Undo receipt', async () => { const client = await openClient() await client.project.create({ data: { id: 'project-1', name: 'Project one' } }) @@ -280,6 +306,31 @@ describe('PermissionGrantRegistry', () => { ).resolves.toEqual([{ id: remembered.id, revision: remembered.revision }]) }) + it('extends a live Undo receipt from the authoritative clock', async () => { + const client = await openClient() + let now = 0 + const registry = await createPermissionGrantRegistry({ + getClient: async () => client, + createUndoToken: () => 'extended-undo', + now: () => new Date(now), + receiptTtlMs: 10 + }) + const capability = { kind: 'execution' as const, key: 'exec:local/python' } + const grant = await registry.remember({ capability, scope: { kind: 'global' } }) + await registry.revoke({ grants: [{ id: grant.id, revision: grant.revision }] }) + + now = 6 + await expect(registry.extendUndo({ undoToken: 'extended-undo' })).resolves.toEqual({ + undoToken: 'extended-undo', + expiresAt: 16, + revokedCount: 1 + }) + now = 15 + await registry.restore({ undoToken: 'extended-undo' }) + + await expect(registry.list()).resolves.toHaveLength(1) + }) + it('discards expired Undo receipts so their row snapshots cannot become live again', async () => { const client = await openClient() let now = 0 @@ -294,6 +345,7 @@ describe('PermissionGrantRegistry', () => { await registry.revoke({ grants: [{ id: grant.id, revision: grant.revision }] }) now = 20 + await expect(registry.extendUndo({ undoToken: 'expired-undo' })).resolves.toBeUndefined() await registry.restore({ undoToken: 'expired-undo' }) now = 0 await registry.restore({ undoToken: 'expired-undo' }) diff --git a/src/main/permission-grants/registry.ts b/src/main/permission-grants/registry.ts index 29d4366ca..2dc2355b7 100644 --- a/src/main/permission-grants/registry.ts +++ b/src/main/permission-grants/registry.ts @@ -13,6 +13,8 @@ import { type PermissionGrantOwner, type PermissionGrantRecord, type PermissionGrantScope, + type PermissionGrantUndoReceipt, + type ExtendPermissionGrantUndo, type RememberPermissionGrant, type RestorePermissionGrants, type RevokePermissionGrants @@ -50,6 +52,7 @@ type PermissionGrantRegistry = { list(): Promise listCached(): PermissionGrantRecord[] revoke(command: RevokePermissionGrants): Promise + extendUndo(command: ExtendPermissionGrantUndo): Promise restore(command: RestorePermissionGrants): Promise prune(owner: PermissionGrantOwner): Promise finalizeOwnerDeletion(owner: PermissionGrantOwner): Promise @@ -290,7 +293,13 @@ const createPermissionGrantRegistry = async ( for (const match of matches) { if (!options.isScopeLive || (await options.isScopeLive(match.grant.scope))) { - return { grant: match.grant, matchedScope: match.grant.scope.kind } + // Scope liveness can require storage I/O. A revoke/prune may complete while that check is + // pending, so re-read the cache before releasing authority instead of returning the stale + // record captured above. + const current = records.get(fingerprintFor(match.grant.capability, match.grant.scope)) + if (current?.id === match.grant.id && current.revision === match.grant.revision) { + return { grant: current, matchedScope: current.scope.kind } + } } } @@ -392,6 +401,23 @@ const createPermissionGrantRegistry = async ( }) }, + extendUndo(command) { + return runMutation(async () => { + const extendedAt = now().getTime() + purgeExpiredReceipts(extendedAt) + const receipt = receipts.get(command.undoToken) + if (!receipt) return undefined + + const expiresAt = extendedAt + receiptTtlMs + receipts.set(command.undoToken, { ...receipt, expiresAt }) + return { + undoToken: command.undoToken, + expiresAt, + revokedCount: receipt.rows.length + } + }) + }, + restore(command) { return runMutation(async () => { const receipt = receipts.get(command.undoToken) diff --git a/src/preload/index.d.ts b/src/preload/index.d.ts index 87d225656..e42a1003d 100644 --- a/src/preload/index.d.ts +++ b/src/preload/index.d.ts @@ -67,6 +67,8 @@ import type { PermissionGrantRestoreRequest, PermissionGrantRevokeRequest, PermissionGrantSnapshot, + PermissionGrantUndoExtendRequest, + PermissionGrantUndoReceipt, PermissionGrantsChangedEvent } from '../shared/permission-grants' import type { @@ -290,6 +292,9 @@ interface OpenScienceAPI { permissions: { list(): Promise revoke(request: PermissionGrantRevokeRequest): Promise + extendUndo( + request: PermissionGrantUndoExtendRequest + ): Promise restore(request: PermissionGrantRestoreRequest): Promise onChanged(listener: AcpListener): RemoveListener } diff --git a/src/preload/index.ts b/src/preload/index.ts index 56a659210..f7bad5d68 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -69,6 +69,8 @@ import type { PermissionGrantRestoreRequest, PermissionGrantRevokeRequest, PermissionGrantSnapshot, + PermissionGrantUndoExtendRequest, + PermissionGrantUndoReceipt, PermissionGrantsChangedEvent } from '../shared/permission-grants' import type { @@ -328,6 +330,9 @@ type OpenScienceAPI = { permissions: { list: () => Promise revoke: (request: PermissionGrantRevokeRequest) => Promise + extendUndo: ( + request: PermissionGrantUndoExtendRequest + ) => Promise restore: (request: PermissionGrantRestoreRequest) => Promise onChanged: (listener: AcpListener) => RemoveListener } @@ -810,6 +815,10 @@ const api: OpenScienceAPI = { list: () => ipcRenderer.invoke('permissions:list') as Promise, revoke: (request) => ipcRenderer.invoke('permissions:revoke', request) as Promise, + extendUndo: (request) => + ipcRenderer.invoke('permissions:extend-undo', request) as Promise< + PermissionGrantUndoReceipt | undefined + >, restore: (request) => ipcRenderer.invoke('permissions:restore', request) as Promise, onChanged: (listener) => onIpcMessage('permissions:changed', listener) diff --git a/src/renderer/src/components/PermissionUndoSnackbar.test.tsx b/src/renderer/src/components/PermissionUndoSnackbar.test.tsx index 8402978e3..8abf6d216 100644 --- a/src/renderer/src/components/PermissionUndoSnackbar.test.tsx +++ b/src/renderer/src/components/PermissionUndoSnackbar.test.tsx @@ -10,6 +10,7 @@ describe('PermissionUndoSnackbar', () => { let container: HTMLDivElement let root: Root const restore = vi.fn() + const extendUndo = vi.fn() beforeEach(() => { vi.useFakeTimers() @@ -21,7 +22,14 @@ describe('PermissionUndoSnackbar', () => { counts: { all: 0, global: 0, project: 0, session: 0 }, conflicts: [] }) - window.api = { permissions: { restore } } as unknown as Window['api'] + extendUndo.mockReset().mockImplementation(({ undoToken }: { undoToken: string }) => + Promise.resolve({ + undoToken, + expiresAt: Date.now() + 8_000, + revokedCount: 1 + }) + ) + window.api = { permissions: { extendUndo, restore } } as unknown as Window['api'] usePermissionGrantsStore.setState({ grants: [], counts: { all: 0, global: 0, project: 0, session: 0 }, @@ -88,7 +96,45 @@ describe('PermissionUndoSnackbar', () => { expect(stack?.querySelector('[data-slot="scroll-area-viewport"]')).toBeNull() }) - it('pauses automatic dismissal while the snackbar is hovered', async () => { + it('renews the authoritative receipt while automatic dismissal is paused by hover', async () => { + await act(async () => root.render()) + const snackbar = container.querySelector( + '[data-testid="permission-undo-snackbar"]' + ) + + await act(async () => snackbar?.dispatchEvent(new MouseEvent('mouseover', { bubbles: true }))) + await act(async () => vi.advanceTimersByTimeAsync(8_000)) + expect(extendUndo).toHaveBeenCalledWith({ undoToken: 'undo-1' }) + expect(container.querySelector('[data-testid="permission-undo-snackbar"]')).not.toBeNull() + + await act(async () => snackbar?.dispatchEvent(new MouseEvent('mouseout', { bubbles: true }))) + await act(async () => vi.advanceTimersByTimeAsync(7_999)) + expect(container.querySelector('[data-testid="permission-undo-snackbar"]')).not.toBeNull() + await act(async () => vi.advanceTimersByTimeAsync(1)) + expect(container.querySelector('[data-testid="permission-undo-snackbar"]')).toBeNull() + }) + + it('dismisses the action when its authoritative receipt cannot be renewed', async () => { + extendUndo.mockResolvedValueOnce(undefined) + await act(async () => root.render()) + const snackbar = container.querySelector( + '[data-testid="permission-undo-snackbar"]' + ) + + await act(async () => snackbar?.dispatchEvent(new MouseEvent('mouseover', { bubbles: true }))) + + expect(container.querySelector('[data-testid="permission-undo-snackbar"]')).toBeNull() + }) + + it('locally pauses a non-restorable explanation without trying to renew a receipt', async () => { + usePermissionGrantsStore.setState({ + undo: { + token: 'undo-1', + expiresAt: Date.now() + 5_000, + message: "Couldn't restore permission: owner no longer exists", + canRestore: false + } + }) await act(async () => root.render()) const snackbar = container.querySelector( '[data-testid="permission-undo-snackbar"]' @@ -96,6 +142,7 @@ describe('PermissionUndoSnackbar', () => { await act(async () => snackbar?.dispatchEvent(new MouseEvent('mouseover', { bubbles: true }))) await act(async () => vi.advanceTimersByTime(8_000)) + expect(extendUndo).not.toHaveBeenCalled() expect(container.querySelector('[data-testid="permission-undo-snackbar"]')).not.toBeNull() await act(async () => snackbar?.dispatchEvent(new MouseEvent('mouseout', { bubbles: true }))) diff --git a/src/renderer/src/components/PermissionUndoSnackbar.tsx b/src/renderer/src/components/PermissionUndoSnackbar.tsx index 9e4bff4e9..3fe44c0fc 100644 --- a/src/renderer/src/components/PermissionUndoSnackbar.tsx +++ b/src/renderer/src/components/PermissionUndoSnackbar.tsx @@ -8,11 +8,13 @@ import type { PermissionUndo } from '@/stores/permission-grants-store' const PermissionUndoItem = ({ undo, + extend, restore, dismiss, isRestoring }: { undo: PermissionUndo + extend: (token: string) => Promise restore: (token: string) => Promise dismiss: (token: string) => void isRestoring: boolean @@ -28,6 +30,29 @@ const PermissionUndoItem = ({ return () => window.clearTimeout(timer) }, [dismiss, paused, undo.expiresAt, undo.token]) + useEffect(() => { + if (!paused || undo.canRestore === false) return + let cancelled = false + let renewalTimer: number | undefined + const renew = async (): Promise => { + const expiresAt = await extend(undo.token) + if (cancelled) return + if (!expiresAt || expiresAt <= Date.now()) { + dismiss(undo.token) + return + } + renewalTimer = window.setTimeout( + () => void renew(), + Math.max(250, Math.floor((expiresAt - Date.now()) / 2)) + ) + } + void renew() + return () => { + cancelled = true + if (renewalTimer !== undefined) window.clearTimeout(renewalTimer) + } + }, [dismiss, extend, paused, undo.canRestore, undo.token]) + return (
{ const undo = usePermissionGrantsStore((state) => state.undo) const undoQueue = usePermissionGrantsStore((state) => state.undoQueue) const restore = usePermissionGrantsStore((state) => state.restore) + const extend = usePermissionGrantsStore((state) => state.extendUndo) const dismiss = usePermissionGrantsStore((state) => state.dismissUndo) const isRestoring = usePermissionGrantsStore((state) => state.isRestoring) @@ -117,6 +143,7 @@ const PermissionUndoSnackbar = (): React.JSX.Element | null => { { expect(usePermissionGrantsStore.getState()).toMatchObject({ version: 3, grants: [] }) }) + it('updates an Undo item from the authoritative extended receipt', async () => { + const expiresAt = Date.now() + 16_000 + const extendUndo = vi.fn().mockResolvedValue({ + undoToken: 'undo-1', + expiresAt, + revokedCount: 1 + }) + setPermissionApi({ extendUndo }) + usePermissionGrantsStore.setState({ + undo: { token: 'undo-1', expiresAt: Date.now() + 8_000, message: 'Revoked Shell' } + }) + + await expect(usePermissionGrantsStore.getState().extendUndo('undo-1')).resolves.toBe(expiresAt) + + expect(extendUndo).toHaveBeenCalledWith({ undoToken: 'undo-1' }) + expect(usePermissionGrantsStore.getState().undo?.expiresAt).toBe(expiresAt) + }) + + it('dismisses an Undo item when its receipt can no longer be extended', async () => { + setPermissionApi({ extendUndo: vi.fn().mockResolvedValue(undefined) }) + usePermissionGrantsStore.setState({ + undo: { token: 'undo-1', expiresAt: Date.now() + 8_000, message: 'Revoked Shell' } + }) + + await expect(usePermissionGrantsStore.getState().extendUndo('undo-1')).resolves.toBeUndefined() + + expect(usePermissionGrantsStore.getState().undo).toBeUndefined() + }) + it('restores a receipt once and clears the snackbar state', async () => { const restore = vi.fn().mockResolvedValue({ ...snapshot, conflicts: [] }) setPermissionApi({ restore }) diff --git a/src/renderer/src/stores/permission-grants-store.ts b/src/renderer/src/stores/permission-grants-store.ts index 82bb933aa..d0220459f 100644 --- a/src/renderer/src/stores/permission-grants-store.ts +++ b/src/renderer/src/stores/permission-grants-store.ts @@ -29,6 +29,7 @@ type PermissionGrantsStore = PermissionGrantSnapshot & { isRestoring: boolean load: () => Promise revoke: (grants: PermissionGrantView[]) => Promise + extendUndo: (token: string) => Promise restore: (token?: string) => Promise dismissUndo: (token?: string) => void listen: () => () => void @@ -139,10 +140,11 @@ const applyAuthoritativeSnapshot = ( ): PermissionGrantSnapshot => withoutPendingRevocations(incoming.version < state.version ? snapshotFromState(state) : incoming) +const allUndoItems = (state: Pick): PermissionUndo[] => + [state.undo, ...state.undoQueue].filter((item): item is PermissionUndo => Boolean(item)) + const undoItems = (state: Pick): PermissionUndo[] => - [state.undo, ...state.undoQueue].filter((item): item is PermissionUndo => - Boolean(item && item.expiresAt > Date.now()) - ) + allUndoItems(state).filter((item) => item.expiresAt > Date.now()) const withoutUndoToken = ( state: Pick, @@ -275,6 +277,31 @@ const usePermissionGrantsStore = create((set, get) => ({ } }, + extendUndo: async (token) => { + const undo = allUndoItems(get()).find((item) => item.token === token) + if (!undo || undo.canRestore === false) return undefined + + try { + const receipt = await window.api.permissions.extendUndo({ undoToken: token }) + if (!receipt) { + set((state) => withoutUndoToken(state, token)) + return undefined + } + set((state) => + nextUndoState( + allUndoItems(state).map((item) => + item.token === token ? { ...item, expiresAt: receipt.expiresAt } : item + ) + ) + ) + return receipt.expiresAt + } catch { + // A receipt that cannot be renewed must not remain as a visible but ineffective action. + set((state) => withoutUndoToken(state, token)) + return undefined + } + }, + restore: async (token) => { const undo = undoItems(get()).find((item) => item.token === (token ?? get().undo?.token)) if (!undo || undo.canRestore === false || undo.expiresAt <= Date.now()) { diff --git a/src/shared/permission-grants.ts b/src/shared/permission-grants.ts index 46e4f252e..8699dd25f 100644 --- a/src/shared/permission-grants.ts +++ b/src/shared/permission-grants.ts @@ -56,6 +56,16 @@ export type RestorePermissionGrants = { undoToken: string } +export type ExtendPermissionGrantUndo = { + undoToken: string +} + +export type PermissionGrantUndoReceipt = { + undoToken: string + expiresAt: number + revokedCount: number +} + export type PermissionGrantOwner = | { kind: 'project'; projectId: string } | { kind: 'session'; projectId: string; sessionId: string } @@ -69,11 +79,7 @@ export type PermissionGrantMutationConflict = { export type PermissionGrantMutationResult = { grants: PermissionGrantRecord[] - receipt?: { - undoToken: string - expiresAt: number - revokedCount: number - } + receipt?: PermissionGrantUndoReceipt conflicts: PermissionGrantMutationConflict[] } @@ -129,12 +135,12 @@ export type PermissionGrantRestoreRequest = { undoToken: string } +export type PermissionGrantUndoExtendRequest = { + undoToken: string +} + export type PermissionGrantMutationView = PermissionGrantSnapshot & { - receipt?: { - undoToken: string - expiresAt: number - revokedCount: number - } + receipt?: PermissionGrantUndoReceipt conflicts: PermissionGrantMutationConflict[] } diff --git a/src/shared/web-api-map.generated.ts b/src/shared/web-api-map.generated.ts index b081d1dc4..9b0fb99e5 100644 --- a/src/shared/web-api-map.generated.ts +++ b/src/shared/web-api-map.generated.ts @@ -70,6 +70,7 @@ export const WEB_INVOKE_CHANNELS = { 'notebookEnv.repair': 'notebook-env:repair', 'notifications.peekPendingOpenSession': 'notifications:peek-pending-open-session', 'notifications.takePendingOpenSession': 'notifications:take-pending-open-session', + 'permissions.extendUndo': 'permissions:extend-undo', 'permissions.list': 'permissions:list', 'permissions.restore': 'permissions:restore', 'permissions.revoke': 'permissions:revoke',