fix(session-controller): ask the concrete access decision before cancelling
The recheck used allows(), the list-filter view that never sees call arguments, so a policy narrowing cancel to args.request refused a legitimate caller as access-withdrawn forever. Ask decide() with the principal and the request, keep the policy's own refusal reason, and cover both an args-independent role and an args-dependent condition.
This commit is contained in:
1 parent
258ff28b72
commit
44f0996974
2 files changed
+46
-8
No files matched your search
@@ -510,12 +510,25 @@ export class SessionController extends TypertRemoteService {
|
||||
@Remote('cancel')
|
||||
cancel(request: SessionCancelRequest): SessionCancelValue {
|
||||
// The Gateway admitted this call earlier; a caller that lost access meanwhile must not cancel.
|
||||
if (!this.ctx.remoteAccess.allows({ kind: 'session', id: request.sessionId })) {
|
||||
throw new RemoteError(
|
||||
'remote-access/denied',
|
||||
'remote access: session/cancel: access was withdrawn before cancellation',
|
||||
{ action: 'session/cancel', reason: 'access-withdrawn' },
|
||||
)
|
||||
// Ask the concrete decision rather than the list-filter view: a policy condition that reads
|
||||
// `args.request` never sees arguments through `allows()`, so using it here would refuse a
|
||||
// legitimate caller as withdrawn forever.
|
||||
const invocation = this.ctx.invocation
|
||||
const principal = principalOf(invocation)
|
||||
if (principal !== undefined) {
|
||||
const decision = this.ctx.remoteAccess.decide({
|
||||
principal,
|
||||
action: 'session/cancel',
|
||||
resources: [{ kind: 'session', id: request.sessionId }],
|
||||
args: { request },
|
||||
})
|
||||
if (decision.kind !== 'allow') {
|
||||
throw new RemoteError(
|
||||
'remote-access/denied',
|
||||
'remote access: session/cancel: access was withdrawn before cancellation',
|
||||
{ action: 'session/cancel', reason: decision.reason },
|
||||
)
|
||||
}
|
||||
}
|
||||
return this.commands.cancel(request)
|
||||
}
|
||||
|
||||
@@ -13,7 +13,7 @@ import {
|
||||
mountAgentLoopTestDependencies, mountAgentLoopTestHarness, type AgentLoopTestHarness,
|
||||
} from '@deepseek-ai/dsh-agent-loop-testkit'
|
||||
import TypertGatewayService from '@deepseek-ai/dsh-api-gateway'
|
||||
import type { RemoteAccessBinding, RemoteAccessRole } from '@deepseek-ai/dsh-api-remote-access'
|
||||
import type { RemoteAccessBinding, RemoteAccessCondition, RemoteAccessRole } from '@deepseek-ai/dsh-api-remote-access'
|
||||
import { createUserMessage } from '@deepseek-ai/dsh-llm'
|
||||
import { SessionId } from '@deepseek-ai/dsh-session'
|
||||
import { remoteMethods, type PeerId, type PeerScope, type Principal, type RemoteInvocation } from '@deepseek-ai/dsh-typert-protocol'
|
||||
@@ -335,12 +335,37 @@ describe('Session Controller access', () => {
|
||||
} catch (error) {
|
||||
failure = error
|
||||
}
|
||||
// The concrete decision reports the policy's own refusal reason.
|
||||
expect(failure).toMatchObject({
|
||||
code: 'remote-access/denied',
|
||||
details: { action: 'session/cancel', reason: 'access-withdrawn' },
|
||||
details: { action: 'session/cancel', reason: 'no-allow' },
|
||||
})
|
||||
})
|
||||
|
||||
it('cancels a shared Session for a caller that still holds the Session role', async () => {
|
||||
const { ctx, as } = await harness()
|
||||
guestPolicy(ctx, DRIVES_SHARED)
|
||||
expect(as(GUEST, 'cancel').sessionController.cancel({ sessionId: SHARED })).toEqual({ accepted: true })
|
||||
})
|
||||
|
||||
it('lets an args-dependent policy admit the caller\'s own cancel request', async () => {
|
||||
const { ctx, as } = await harness()
|
||||
// A policy may narrow cancel to the request it names; only a concrete decide() carries
|
||||
// `args.request`, so an allows()-based recheck would refuse this legitimate caller forever.
|
||||
const condition: RemoteAccessCondition<'session'> = ({ args }) => {
|
||||
const request = args?.request as { readonly sessionId?: unknown } | undefined
|
||||
return request?.sessionId === SHARED
|
||||
? { kind: 'allow' }
|
||||
: { kind: 'deny', reason: 'not-this-request' }
|
||||
}
|
||||
guestPolicy(ctx, {
|
||||
role: { name: 'test:session-cancel-args', resource: 'session', actions: ['session/cancel'] },
|
||||
resource: { kind: 'session', id: SHARED },
|
||||
condition,
|
||||
})
|
||||
expect(as(GUEST, 'cancel').sessionController.cancel({ sessionId: SHARED })).toEqual({ accepted: true })
|
||||
})
|
||||
|
||||
it('lets a non-owner read projections only in a Session its Session role names', async () => {
|
||||
const { ctx } = await harness()
|
||||
await ctx.plugin(TypertGatewayService)
|
||||
|
||||
Reference in new issue
Block a user