From 6fa8e2f8aed6f45f8df93322005a754fbf3c0034 Mon Sep 17 00:00:00 2001 From: Noa Flaherty Date: Sat, 28 Feb 2026 23:29:37 -0500 Subject: [PATCH 1/3] feat: cutover decision authorization to principal-based and remove isTrusted Replace the isTrusted compatibility bypass in all runtime types and callsites with principal-based authorization. Decision authorization is now purely principal-based: actor.guardianPrincipalId must match request.guardianPrincipalId for any applied decision. There is no longer a trusted bypass path. Key changes: - Remove isTrusted from ActorContext type, replace with guardianPrincipalId - Add three-step principal validation in applyCanonicalGuardianDecision() - Add guardianPrincipalId filter to canonical guardian store queries - Update guardian-reply-router to use principal-based request discovery - Update all callsites (HTTP routes, daemon handlers, session process, inbound message handler) to resolve and pass guardianPrincipalId - Replace isTrusted-based guardianReplyText conditionals with channel checks Closes #10960 --- .../approvals/guardian-decision-primitive.ts | 53 ++++++++++--------- .../approvals/guardian-request-resolvers.ts | 12 +++-- .../src/daemon/handlers/guardian-actions.ts | 7 ++- assistant/src/daemon/handlers/sessions.ts | 7 ++- assistant/src/daemon/session-process.ts | 4 +- .../src/memory/canonical-guardian-store.ts | 4 ++ .../src/runtime/guardian-reply-router.ts | 31 ++++++----- .../src/runtime/routes/conversation-routes.ts | 20 +++---- .../runtime/routes/guardian-action-routes.ts | 8 ++- .../runtime/routes/inbound-message-handler.ts | 2 +- 10 files changed, 88 insertions(+), 60 deletions(-) diff --git a/assistant/src/approvals/guardian-decision-primitive.ts b/assistant/src/approvals/guardian-decision-primitive.ts index e921d288686..587a345f2d7 100644 --- a/assistant/src/approvals/guardian-decision-primitive.ts +++ b/assistant/src/approvals/guardian-decision-primitive.ts @@ -18,7 +18,8 @@ * 9. Kind-specific resolver dispatch via the resolver registry * * Security invariants enforced here: - * - Decision application is identity-bound to expected guardian identity + * - Decision authorization is purely principal-based: + * actor.guardianPrincipalId must match request.guardianPrincipalId * - Decisions are first-response-wins (CAS-like stale protection) * - `approve_always` is rejected/downgraded for guardian-on-behalf requests * - Scoped grant minting only on explicit approve for requests with tool metadata @@ -349,44 +350,46 @@ export async function applyCanonicalGuardianDecision( return { applied: false, reason: 'invalid_action', detail: `invalid action: ${action}` }; } - // 2c. Validate identity: actor must match guardian_external_user_id - // unless the actor is trusted (desktop). - // - // Channel tool-approval requests must always be identity-bound. Treat - // missing guardianExternalUserId as unauthorized (fail-closed) so a - // non-guardian actor can never approve an unbound request. - if ( - !actorContext.isTrusted && - request.kind === 'tool_approval' && - !request.guardianExternalUserId - ) { + // 2c. Principal-based authorization: actor.guardianPrincipalId must match + // request.guardianPrincipalId for any applied decision. This is the single + // authorization gate — there is no trusted bypass. + + if (!request.guardianPrincipalId) { log.warn( { - event: 'canonical_decision_missing_guardian_binding', + event: 'canonical_decision_missing_request_principal', requestId, kind: request.kind, sourceType: request.sourceType, }, - 'Canonical tool approval missing guardian binding; rejecting decision', + 'Canonical request missing guardianPrincipalId; rejecting decision', + ); + return { applied: false, reason: 'identity_mismatch', detail: 'request missing guardianPrincipalId' }; + } + + if (!actorContext.guardianPrincipalId) { + log.warn( + { + event: 'canonical_decision_missing_actor_principal', + requestId, + actorChannel: actorContext.channel, + }, + 'Actor missing guardianPrincipalId; rejecting decision', ); - return { applied: false, reason: 'identity_mismatch', detail: 'missing guardian binding' }; + return { applied: false, reason: 'identity_mismatch', detail: 'actor missing guardianPrincipalId' }; } - if ( - request.guardianExternalUserId && - !actorContext.isTrusted && - actorContext.externalUserId !== request.guardianExternalUserId - ) { + if (actorContext.guardianPrincipalId !== request.guardianPrincipalId) { log.warn( { - event: 'canonical_decision_identity_mismatch', + event: 'canonical_decision_principal_mismatch', requestId, - expectedGuardian: request.guardianExternalUserId, - actualActor: actorContext.externalUserId, + expectedPrincipal: request.guardianPrincipalId, + actualPrincipal: actorContext.guardianPrincipalId, }, - 'Actor identity does not match expected guardian', + 'Actor principal does not match request principal', ); - return { applied: false, reason: 'identity_mismatch' }; + return { applied: false, reason: 'identity_mismatch', detail: 'principal mismatch' }; } // 2d. Check expiry diff --git a/assistant/src/approvals/guardian-request-resolvers.ts b/assistant/src/approvals/guardian-request-resolvers.ts index 4d9f2746a8a..da61085e9a7 100644 --- a/assistant/src/approvals/guardian-request-resolvers.ts +++ b/assistant/src/approvals/guardian-request-resolvers.ts @@ -43,8 +43,8 @@ export interface ActorContext { externalUserId: string | undefined; /** Channel the decision arrived on. */ channel: string; - /** Whether the actor is a trusted/desktop context. */ - isTrusted: boolean; + /** Principal ID for authorization — must match the request's guardianPrincipalId. */ + guardianPrincipalId: string | undefined; } /** The decision being applied. */ @@ -374,7 +374,9 @@ const accessRequestResolver: GuardianRequestResolver = { return { ok: true, applied: true, - ...(ctx.actor.isTrusted ? { guardianReplyText: `Access denied for ${requesterLabel}.` } : {}), + // Desktop actors (vellum channel) receive inline reply text; channel + // actors get replies delivered via the channel delivery context. + ...(ctx.actor.channel === 'vellum' ? { guardianReplyText: `Access denied for ${requesterLabel}.` } : {}), }; } @@ -528,7 +530,9 @@ const accessRequestResolver: GuardianRequestResolver = { return { ok: true, applied: true, - ...(ctx.actor.isTrusted ? { guardianReplyText: verificationReplyText } : {}), + // Desktop actors (vellum channel) receive inline reply text; channel + // actors get replies delivered via the channel delivery context. + ...(ctx.actor.channel === 'vellum' ? { guardianReplyText: verificationReplyText } : {}), }; }, }; diff --git a/assistant/src/daemon/handlers/guardian-actions.ts b/assistant/src/daemon/handlers/guardian-actions.ts index 09d7d0b2124..e409ca8afd3 100644 --- a/assistant/src/daemon/handlers/guardian-actions.ts +++ b/assistant/src/daemon/handlers/guardian-actions.ts @@ -3,6 +3,7 @@ import { } from '../../approvals/guardian-decision-primitive.js'; import { getCanonicalGuardianRequest } from '../../memory/canonical-guardian-store.js'; import type { ApprovalAction } from '../../runtime/channel-approval-types.js'; +import { resolveLocalIpcGuardianContext } from '../../runtime/local-actor-identity.js'; import { listGuardianDecisionPrompts } from '../../runtime/routes/guardian-action-routes.js'; import type { GuardianActionDecision, GuardianActionsPendingRequest } from '../ipc-protocol.js'; import { defineHandlers, log } from './shared.js'; @@ -45,13 +46,15 @@ export const guardianActionsHandlers = defineHandlers({ } } + // Resolve the local IPC actor's principal via the vellum guardian binding. + const localGuardianCtx = resolveLocalIpcGuardianContext('vellum'); const canonicalResult = await applyCanonicalGuardianDecision({ requestId: msg.requestId, action: msg.action as ApprovalAction, actorContext: { - externalUserId: undefined, + externalUserId: localGuardianCtx.guardianExternalUserId, channel: 'vellum', - isTrusted: true, + guardianPrincipalId: localGuardianCtx.guardianPrincipalId ?? undefined, }, userText: undefined, }); diff --git a/assistant/src/daemon/handlers/sessions.ts b/assistant/src/daemon/handlers/sessions.ts index 82809c98568..4eaec6d4a56 100644 --- a/assistant/src/daemon/handlers/sessions.ts +++ b/assistant/src/daemon/handlers/sessions.ts @@ -600,13 +600,16 @@ export async function handleUserMessage( ])); if (pendingRequestIdsForConversation.length > 0) { + // Resolve the local IPC actor's principal via the vellum guardian binding + // for principal-based authorization in the canonical decision primitive. + const localCtx = resolveLocalIpcGuardianContext(ipcChannel); const routerResult = await routeGuardianReply({ messageText: messageText.trim(), channel: ipcChannel, actor: { - externalUserId: undefined, + externalUserId: localCtx.guardianExternalUserId, channel: ipcChannel, - isTrusted: true, + guardianPrincipalId: localCtx.guardianPrincipalId ?? undefined, }, conversationId: msg.sessionId, pendingRequestIds: pendingRequestIdsForConversation, diff --git a/assistant/src/daemon/session-process.ts b/assistant/src/daemon/session-process.ts index e4593e727a7..22f7c889620 100644 --- a/assistant/src/daemon/session-process.ts +++ b/assistant/src/daemon/session-process.ts @@ -383,9 +383,9 @@ export async function processMessage( messageText: trimmedContent, channel: 'vellum', actor: { - externalUserId: undefined, + externalUserId: session.guardianContext?.guardianExternalUserId, channel: 'vellum', - isTrusted: true, + guardianPrincipalId: session.guardianContext?.guardianPrincipalId ?? undefined, }, conversationId: session.conversationId, pendingRequestIds: canonicalPendingRequestIdsForConversation, diff --git a/assistant/src/memory/canonical-guardian-store.ts b/assistant/src/memory/canonical-guardian-store.ts index ade5bccdea2..c1e2c71d31c 100644 --- a/assistant/src/memory/canonical-guardian-store.ts +++ b/assistant/src/memory/canonical-guardian-store.ts @@ -271,6 +271,7 @@ export function getCanonicalGuardianRequestByCode(code: string): CanonicalGuardi export interface ListCanonicalGuardianRequestsFilters { status?: CanonicalRequestStatus; guardianExternalUserId?: string; + guardianPrincipalId?: string; requesterExternalUserId?: string; conversationId?: string; sourceType?: string; @@ -289,6 +290,9 @@ export function listCanonicalGuardianRequests(filters?: ListCanonicalGuardianReq if (filters?.guardianExternalUserId) { conditions.push(eq(canonicalGuardianRequests.guardianExternalUserId, filters.guardianExternalUserId)); } + if (filters?.guardianPrincipalId) { + conditions.push(eq(canonicalGuardianRequests.guardianPrincipalId, filters.guardianPrincipalId)); + } if (filters?.conversationId) { conditions.push(eq(canonicalGuardianRequests.conversationId, filters.conversationId)); } diff --git a/assistant/src/runtime/guardian-reply-router.ts b/assistant/src/runtime/guardian-reply-router.ts index f7049220ca6..ff39c88bc40 100644 --- a/assistant/src/runtime/guardian-reply-router.ts +++ b/assistant/src/runtime/guardian-reply-router.ts @@ -195,8 +195,8 @@ function findPendingCanonicalRequests( }); } - // For desktop/trusted actors without an externalUserId, query by - // conversationId so the NL path can discover pending requests. + // Actors without an externalUserId: scope by conversationId so the NL + // path can discover pending requests bound to this conversation. if (conversationId) { return listCanonicalGuardianRequests({ status: 'pending', @@ -204,10 +204,14 @@ function findPendingCanonicalRequests( }); } - // Trusted actors without a conversationId: return all pending requests - // so desktop sessions can always discover pending guardian work. - if (actor.isTrusted) { - return listCanonicalGuardianRequests({ status: 'pending' }); + // Actors with a guardianPrincipalId but no externalUserId or + // conversationId: query by principal so desktop sessions can still + // discover pending guardian work via their bound principal. + if (actor.guardianPrincipalId) { + return listCanonicalGuardianRequests({ + status: 'pending', + guardianPrincipalId: actor.guardianPrincipalId, + }); } return []; @@ -299,22 +303,21 @@ export async function routeGuardianReply( // silently defaulting to approve_once. if (!codeResult.remainingText || codeResult.remainingText.trim().length === 0) { // Identity check: only expose request details to the assigned guardian - // or trusted (desktop) actors. Mirrors the identity check in + // principal. Mirrors the principal check in // applyCanonicalGuardianDecision to prevent leaking request details // (toolName, questionText) to unauthorized senders. if ( - request.guardianExternalUserId && - !actor.isTrusted && - actor.externalUserId !== request.guardianExternalUserId + request.guardianPrincipalId && + actor.guardianPrincipalId !== request.guardianPrincipalId ) { log.warn( { - event: 'router_code_only_identity_mismatch', + event: 'router_code_only_principal_mismatch', requestId: request.id, - expectedGuardian: request.guardianExternalUserId, - actualActor: actor.externalUserId, + expectedPrincipal: request.guardianPrincipalId, + actualPrincipal: actor.guardianPrincipalId, }, - 'Code-only clarification blocked: actor identity does not match expected guardian', + 'Code-only clarification blocked: actor principal does not match request principal', ); return { decisionApplied: false, diff --git a/assistant/src/runtime/routes/conversation-routes.ts b/assistant/src/runtime/routes/conversation-routes.ts index 0b34cfaf4a7..8d804c91ea6 100644 --- a/assistant/src/runtime/routes/conversation-routes.ts +++ b/assistant/src/runtime/routes/conversation-routes.ts @@ -87,6 +87,8 @@ async function tryConsumeCanonicalGuardianReply(params: { approvalConversationGenerator?: ApprovalConversationGenerator; /** Verified actor identity from actor-token middleware. */ verifiedActorExternalUserId?: string; + /** Verified actor principal ID for principal-based authorization. */ + verifiedActorPrincipalId?: string; }): Promise<{ consumed: boolean; messageId?: string }> { const { conversationId, @@ -98,6 +100,7 @@ async function tryConsumeCanonicalGuardianReply(params: { onEvent, approvalConversationGenerator, verifiedActorExternalUserId, + verifiedActorPrincipalId, } = params; const trimmedContent = content.trim(); @@ -114,12 +117,7 @@ async function tryConsumeCanonicalGuardianReply(params: { actor: { externalUserId: verifiedActorExternalUserId, channel: sourceChannel, - // When a verified identity is available, disable the trusted bypass so - // that the identity-match checks in applyCanonicalGuardianDecision - // actually run. Only fall back to isTrusted when no verified identity - // was resolved (defensive — shouldn't happen for vellum since - // verification runs upstream). - isTrusted: !verifiedActorExternalUserId, + guardianPrincipalId: verifiedActorPrincipalId, }, conversationId, pendingRequestIds, @@ -523,12 +521,15 @@ export async function handleSendMessage( ? smDeps.resolveAttachments(attachmentIds) : []; - // Resolve the verified actor's external user ID for inline approval - // routing. Uses the guardianExternalUserId from the verified context - // (actor-token or local-fallback) rather than hardcoding undefined. + // Resolve the verified actor's external user ID and principal for inline + // approval routing. Uses the guardianExternalUserId and guardianPrincipalId + // from the verified context (actor-token or local-fallback). const verifiedActorExternalUserId = actorVerification?.ok ? actorVerification.guardianContext.guardianExternalUserId : undefined; + const verifiedActorPrincipalId = actorVerification?.ok + ? actorVerification.guardianContext.guardianPrincipalId ?? undefined + : undefined; // Try to consume the message as a canonical guardian approval/rejection reply. // On failure, degrade to the existing queue/auto-deny path rather than @@ -544,6 +545,7 @@ export async function handleSendMessage( onEvent, approvalConversationGenerator: deps.approvalConversationGenerator, verifiedActorExternalUserId, + verifiedActorPrincipalId, }); if (inlineReplyResult.consumed) { return Response.json( diff --git a/assistant/src/runtime/routes/guardian-action-routes.ts b/assistant/src/runtime/routes/guardian-action-routes.ts index b8ad820ed74..834a63480d4 100644 --- a/assistant/src/runtime/routes/guardian-action-routes.ts +++ b/assistant/src/runtime/routes/guardian-action-routes.ts @@ -124,13 +124,19 @@ export async function handleGuardianActionDecision(req: Request, server: ServerW ? tokenResult.claims.guardianPrincipalId : tokenResult.guardianContext.guardianExternalUserId; + // Resolve the actor's principal ID: from the token claims if present, + // otherwise from the vellum guardian binding (local fallback). + const actorPrincipalId = tokenResult.claims + ? tokenResult.claims.guardianPrincipalId + : tokenResult.guardianContext.guardianPrincipalId ?? undefined; + const canonicalResult = await applyCanonicalGuardianDecision({ requestId, action: action as ApprovalAction, actorContext: { externalUserId: actorExternalUserId, channel: 'vellum', - isTrusted: true, + guardianPrincipalId: actorPrincipalId, }, userText: undefined, }); diff --git a/assistant/src/runtime/routes/inbound-message-handler.ts b/assistant/src/runtime/routes/inbound-message-handler.ts index 37a39ea1104..27de6feb14b 100644 --- a/assistant/src/runtime/routes/inbound-message-handler.ts +++ b/assistant/src/runtime/routes/inbound-message-handler.ts @@ -980,7 +980,7 @@ export async function handleChannelInbound( actor: { externalUserId: canonicalSenderId ?? rawSenderId!, channel: sourceChannel, - isTrusted: false, + guardianPrincipalId: guardianCtx.guardianPrincipalId ?? undefined, }, conversationId: result.conversationId, callbackData: body.callbackData, From e74a7ec96f366e7b10db3a581a42fc9d87abe867 Mon Sep 17 00:00:00 2001 From: Noa Flaherty Date: Sat, 28 Feb 2026 23:40:39 -0500 Subject: [PATCH 2/3] fix: resolve cross-channel principal mismatch and non-vellum channel fallback - conversation-routes.ts: Fall back to session.guardianContext for verifiedActorExternalUserId and verifiedActorPrincipalId when actorVerification is null (non-vellum channels). - guardian-decision-primitive.ts: Allow cross-channel guardian decisions by checking actor principal against the assistant's canonical vellum binding principal when direct principal match fails. - guardian-reply-router.ts: Add guardianPrincipalId filter to the conversation fallback query in findPendingCanonicalRequests so guardians only see requests they are authorized to act on. Co-Authored-By: Claude Opus 4.6 --- .../approvals/guardian-decision-primitive.ts | 37 ++++++++++++++++--- .../src/runtime/guardian-reply-router.ts | 3 ++ .../src/runtime/routes/conversation-routes.ts | 4 +- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/assistant/src/approvals/guardian-decision-primitive.ts b/assistant/src/approvals/guardian-decision-primitive.ts index 587a345f2d7..03318575791 100644 --- a/assistant/src/approvals/guardian-decision-primitive.ts +++ b/assistant/src/approvals/guardian-decision-primitive.ts @@ -36,6 +36,7 @@ import { type GuardianApprovalRequest, updateApprovalDecision, } from '../memory/channel-guardian-store.js'; +import { getActiveBinding } from '../memory/guardian-bindings.js'; import { DAEMON_INTERNAL_ASSISTANT_ID } from '../runtime/assistant-scope.js'; import type { ApprovalAction, @@ -380,16 +381,40 @@ export async function applyCanonicalGuardianDecision( } if (actorContext.guardianPrincipalId !== request.guardianPrincipalId) { - log.warn( + // Cross-channel fallback: when a request was created with a channel + // binding's principal (e.g. Telegram) but the guardian responds from + // vellum/desktop, the actor's principal is the vellum binding's + // canonical principal. Allow the decision if the actor matches the + // assistant's canonical (vellum) principal — this is the single + // assistant-scoped guardian identity used for decision authorization + // across all channels and surfaces. + const vellumBinding = getActiveBinding(DAEMON_INTERNAL_ASSISTANT_ID, 'vellum'); + const canonicalPrincipal = vellumBinding?.guardianPrincipalId; + + if (!canonicalPrincipal || actorContext.guardianPrincipalId !== canonicalPrincipal) { + log.warn( + { + event: 'canonical_decision_principal_mismatch', + requestId, + expectedPrincipal: request.guardianPrincipalId, + actualPrincipal: actorContext.guardianPrincipalId, + canonicalPrincipal: canonicalPrincipal ?? null, + }, + 'Actor principal does not match request principal or canonical principal', + ); + return { applied: false, reason: 'identity_mismatch', detail: 'principal mismatch' }; + } + + log.info( { - event: 'canonical_decision_principal_mismatch', + event: 'canonical_decision_cross_channel_principal_match', requestId, - expectedPrincipal: request.guardianPrincipalId, - actualPrincipal: actorContext.guardianPrincipalId, + requestPrincipal: request.guardianPrincipalId, + actorPrincipal: actorContext.guardianPrincipalId, + canonicalPrincipal, }, - 'Actor principal does not match request principal', + 'Actor principal matches canonical vellum principal (cross-channel authorization)', ); - return { applied: false, reason: 'identity_mismatch', detail: 'principal mismatch' }; } // 2d. Check expiry diff --git a/assistant/src/runtime/guardian-reply-router.ts b/assistant/src/runtime/guardian-reply-router.ts index ff39c88bc40..4fc85873042 100644 --- a/assistant/src/runtime/guardian-reply-router.ts +++ b/assistant/src/runtime/guardian-reply-router.ts @@ -197,10 +197,13 @@ function findPendingCanonicalRequests( // Actors without an externalUserId: scope by conversationId so the NL // path can discover pending requests bound to this conversation. + // Include guardianPrincipalId filter when available so the guardian only + // sees requests they are authorized to act on. if (conversationId) { return listCanonicalGuardianRequests({ status: 'pending', conversationId, + ...(actor.guardianPrincipalId ? { guardianPrincipalId: actor.guardianPrincipalId } : {}), }); } diff --git a/assistant/src/runtime/routes/conversation-routes.ts b/assistant/src/runtime/routes/conversation-routes.ts index 8d804c91ea6..fcefc6ac908 100644 --- a/assistant/src/runtime/routes/conversation-routes.ts +++ b/assistant/src/runtime/routes/conversation-routes.ts @@ -526,10 +526,10 @@ export async function handleSendMessage( // from the verified context (actor-token or local-fallback). const verifiedActorExternalUserId = actorVerification?.ok ? actorVerification.guardianContext.guardianExternalUserId - : undefined; + : session.guardianContext?.guardianExternalUserId; const verifiedActorPrincipalId = actorVerification?.ok ? actorVerification.guardianContext.guardianPrincipalId ?? undefined - : undefined; + : session.guardianContext?.guardianPrincipalId ?? undefined; // Try to consume the message as a canonical guardian approval/rejection reply. // On failure, degrade to the existing queue/auto-deny path rather than From aa325721773a8f8023917dcf9af60049feeb5260 Mon Sep 17 00:00:00 2001 From: Noa Flaherty Date: Sat, 28 Feb 2026 23:47:28 -0500 Subject: [PATCH 3/3] fix: add cross-channel principal fallback to code-only clarification check Extract isAuthorizedGuardianPrincipal() shared helper from the inline cross-channel fallback logic in applyCanonicalGuardianDecision, and use it in the router's code-only clarification identity check. This ensures a desktop guardian entering a request code for a cross-channel request (e.g. Telegram-originated) sees the clarification context instead of getting "Request not found". Co-Authored-By: Claude Opus 4.6 --- .../approvals/guardian-decision-primitive.ts | 71 ++++++++++++------- .../src/runtime/guardian-reply-router.ts | 14 ++-- 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/assistant/src/approvals/guardian-decision-primitive.ts b/assistant/src/approvals/guardian-decision-primitive.ts index 03318575791..a804e8e8c59 100644 --- a/assistant/src/approvals/guardian-decision-primitive.ts +++ b/assistant/src/approvals/guardian-decision-primitive.ts @@ -275,6 +275,39 @@ export function mintCanonicalRequestGrant(params: { return { minted: false }; } +// --------------------------------------------------------------------------- +// Cross-channel principal authorization helper +// --------------------------------------------------------------------------- + +/** + * Check whether an actor's guardian principal is authorized to act on a + * request that was bound to a (possibly different) guardian principal. + * + * Authorization passes when: + * 1. The actor's principal directly matches the request's principal, OR + * 2. The actor's principal matches the assistant's canonical (vellum) + * principal — this covers the cross-channel case where a request was + * created with a channel binding's principal (e.g. Telegram) but the + * guardian responds from vellum/desktop. + * + * Returns `true` when the actor is authorized, `false` otherwise. + */ +export function isAuthorizedGuardianPrincipal( + actorPrincipalId: string, + requestPrincipalId: string, +): boolean { + if (actorPrincipalId === requestPrincipalId) { + return true; + } + + // Cross-channel fallback: check if the actor matches the assistant's + // canonical (vellum) principal. + const vellumBinding = getActiveBinding(DAEMON_INTERNAL_ASSISTANT_ID, 'vellum'); + const canonicalPrincipal = vellumBinding?.guardianPrincipalId; + + return !!canonicalPrincipal && actorPrincipalId === canonicalPrincipal; +} + // --------------------------------------------------------------------------- // Canonical guardian decision primitive // --------------------------------------------------------------------------- @@ -380,38 +413,26 @@ export async function applyCanonicalGuardianDecision( return { applied: false, reason: 'identity_mismatch', detail: 'actor missing guardianPrincipalId' }; } - if (actorContext.guardianPrincipalId !== request.guardianPrincipalId) { - // Cross-channel fallback: when a request was created with a channel - // binding's principal (e.g. Telegram) but the guardian responds from - // vellum/desktop, the actor's principal is the vellum binding's - // canonical principal. Allow the decision if the actor matches the - // assistant's canonical (vellum) principal — this is the single - // assistant-scoped guardian identity used for decision authorization - // across all channels and surfaces. - const vellumBinding = getActiveBinding(DAEMON_INTERNAL_ASSISTANT_ID, 'vellum'); - const canonicalPrincipal = vellumBinding?.guardianPrincipalId; - - if (!canonicalPrincipal || actorContext.guardianPrincipalId !== canonicalPrincipal) { - log.warn( - { - event: 'canonical_decision_principal_mismatch', - requestId, - expectedPrincipal: request.guardianPrincipalId, - actualPrincipal: actorContext.guardianPrincipalId, - canonicalPrincipal: canonicalPrincipal ?? null, - }, - 'Actor principal does not match request principal or canonical principal', - ); - return { applied: false, reason: 'identity_mismatch', detail: 'principal mismatch' }; - } + if (!isAuthorizedGuardianPrincipal(actorContext.guardianPrincipalId, request.guardianPrincipalId)) { + log.warn( + { + event: 'canonical_decision_principal_mismatch', + requestId, + expectedPrincipal: request.guardianPrincipalId, + actualPrincipal: actorContext.guardianPrincipalId, + }, + 'Actor principal does not match request principal or canonical principal', + ); + return { applied: false, reason: 'identity_mismatch', detail: 'principal mismatch' }; + } + if (actorContext.guardianPrincipalId !== request.guardianPrincipalId) { log.info( { event: 'canonical_decision_cross_channel_principal_match', requestId, requestPrincipal: request.guardianPrincipalId, actorPrincipal: actorContext.guardianPrincipalId, - canonicalPrincipal, }, 'Actor principal matches canonical vellum principal (cross-channel authorization)', ); diff --git a/assistant/src/runtime/guardian-reply-router.ts b/assistant/src/runtime/guardian-reply-router.ts index 4fc85873042..83aff0eddc9 100644 --- a/assistant/src/runtime/guardian-reply-router.ts +++ b/assistant/src/runtime/guardian-reply-router.ts @@ -20,6 +20,7 @@ import { applyCanonicalGuardianDecision, type CanonicalDecisionResult, + isAuthorizedGuardianPrincipal, } from '../approvals/guardian-decision-primitive.js'; import type { ActorContext, ChannelDeliveryContext } from '../approvals/guardian-request-resolvers.js'; import { @@ -306,12 +307,15 @@ export async function routeGuardianReply( // silently defaulting to approve_once. if (!codeResult.remainingText || codeResult.remainingText.trim().length === 0) { // Identity check: only expose request details to the assigned guardian - // principal. Mirrors the principal check in - // applyCanonicalGuardianDecision to prevent leaking request details - // (toolName, questionText) to unauthorized senders. + // principal. Uses the shared cross-channel principal helper (same logic + // as applyCanonicalGuardianDecision) to prevent leaking request details + // (toolName, questionText) to unauthorized senders while allowing + // cross-channel lookups (e.g. desktop guardian entering a code for a + // Telegram-originated request). if ( request.guardianPrincipalId && - actor.guardianPrincipalId !== request.guardianPrincipalId + actor.guardianPrincipalId && + !isAuthorizedGuardianPrincipal(actor.guardianPrincipalId, request.guardianPrincipalId) ) { log.warn( { @@ -320,7 +324,7 @@ export async function routeGuardianReply( expectedPrincipal: request.guardianPrincipalId, actualPrincipal: actor.guardianPrincipalId, }, - 'Code-only clarification blocked: actor principal does not match request principal', + 'Code-only clarification blocked: actor principal does not match request or canonical principal', ); return { decisionApplied: false,