Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/acp/permission-broker-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down
123 changes: 123 additions & 0 deletions src/main/compute/compute-approval-broker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<undefined>((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()
Expand Down Expand Up @@ -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<void>((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)
Expand Down
72 changes: 65 additions & 7 deletions src/main/compute/compute-approval-broker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export class ComputeApprovalBroker {
>()

private readonly providerGenerations = new Map<string, number>()
private readonly invalidatingProviders = new Set<string>()
private readonly inFlightRequests = new Map<string, Set<Promise<ComputeApprovalDecision>>>()

// Legacy fallback used only when no durable adapter is supplied.
private readonly conversationGrants = new Set<string>()
Expand Down Expand Up @@ -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<ComputeApprovalRequest, 'id'>,
ctx: ComputeApprovalContext
): Promise<ComputeApprovalDecision> {
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<ComputeApprovalRequest, 'id'>,
ctx: ComputeApprovalContext
): Promise<ComputeApprovalDecision> {
Expand All @@ -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'
Expand All @@ -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
}

Expand All @@ -169,14 +212,29 @@ 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<void> {
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)
}
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<ComputeApprovalDecision>
): void {
const requests = this.inFlightRequests.get(providerId)
requests?.delete(request)
if (requests?.size === 0) this.inFlightRequests.delete(providerId)
}

private async isProviderCurrent(
Expand Down
13 changes: 11 additions & 2 deletions src/main/compute/ipc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 }),
Expand Down
10 changes: 7 additions & 3 deletions src/main/compute/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Loading
Loading