M5: Cutover decision authorization + remove isTrusted by noanflaherty · Pull Request #10990 · vellum-ai/vellum-assistant

noanflaherty · 2026-03-01T04:29:55Z

Summary

Remove the isTrusted compatibility bypass from all runtime types and callsites. 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.

Changes

Authorization logic (`guardian-decision-primitive.ts`)

Replaced the two isTrusted bypass conditions with three-step principal validation:
1. Reject when request guardianPrincipalId is missing
2. Reject when actor guardianPrincipalId is missing
3. Reject when actor principal does not match request principal
Updated security invariants comment to document the new single authorization gate

Type changes (`guardian-request-resolvers.ts`)

Removed isTrusted: boolean from ActorContext interface
Added guardianPrincipalId: string | undefined to ActorContext
Changed guardianReplyText conditionals from ctx.actor.isTrusted to ctx.actor.channel === 'vellum'

Store changes (`canonical-guardian-store.ts`)

Added guardianPrincipalId to ListCanonicalGuardianRequestsFilters interface and query implementation

Guardian reply router (`guardian-reply-router.ts`)

Updated findPendingCanonicalRequests to use guardianPrincipalId instead of isTrusted for fallback discovery
Updated code-only clarification identity check to use principal comparison

Callsite updates (all pass `guardianPrincipalId` instead of `isTrusted`)

conversation-routes.ts: Added verifiedActorPrincipalId parameter to tryConsumeCanonicalGuardianReply
guardian-action-routes.ts: Resolved actorPrincipalId from token claims or guardian context
guardian-actions.ts (daemon): Resolved local IPC principal via resolveLocalIpcGuardianContext
sessions.ts (daemon): Resolved local IPC principal via resolveLocalIpcGuardianContext
session-process.ts: Used session.guardianContext?.guardianPrincipalId
inbound-message-handler.ts: Used guardianCtx.guardianPrincipalId

Test plan

Verify no production isTrusted references remain (confirmed: only unrelated local variables in memory modules)
Guardian decisions via HTTP routes require matching principal
Guardian decisions via IPC daemon handlers require matching principal
Guardian reply routing uses principal-based request discovery
Requests missing principal are rejected with identity_mismatch

Closes #10960

…Trusted 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

noanflaherty · 2026-03-01T04:30:08Z

⚠️ Human Attention Needed

Security-sensitive change: This PR removes the isTrusted bypass from the guardian decision authorization path. After this change, all guardian decisions (regardless of channel or trust level) require a matching guardianPrincipalId between the actor and the canonical request. There is no longer any bypass path.

Key areas to review carefully:

guardian-decision-primitive.ts (lines ~80-110): The three-step principal validation replaces the old isTrusted bypass. Verify that the rejection conditions and log messages are correct.
guardian-request-resolvers.ts: The ActorContext type change affects all downstream consumers. Verify the channel === 'vellum' replacement for guardianReplyText conditionals is semantically equivalent.
All callsites: Each callsite now resolves guardianPrincipalId from its context (token claims, guardian context, session context). Verify each resolution path provides the correct principal.

Risk: If any callsite fails to provide a guardianPrincipalId, or if the backfill (M2) didn't populate guardianPrincipalId on existing canonical requests, decisions will be rejected with identity_mismatch. This is the intended fail-closed behavior but could cause temporary disruption if earlier milestones have gaps.

…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 <noreply@anthropic.com>

noanflaherty · 2026-03-01T04:41:40Z

@codex review this PR again — the previous issues have been fixed in commit e74a7ec

noanflaherty · 2026-03-01T04:41:41Z

@devin review this PR again — the previous issues have been fixed in commit e74a7ec

…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 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e74a7ec96f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-01T04:48:04Z

+          request.guardianPrincipalId &&
+          actor.guardianPrincipalId !== request.guardianPrincipalId
        ) {


Apply canonical-principal fallback to code-only clarifications

The code-only clarification guard now rejects whenever actor.guardianPrincipalId !== request.guardianPrincipalId, but applyCanonicalGuardianDecision() (same PR) explicitly allows cross-channel authorization when the actor matches the assistant’s canonical vellum principal. In the common case where a request is created with a Telegram/SMS principal and the guardian replies from vellum, decisions are allowed but a code-only message is incorrectly blocked with “Request not found,” so authorized guardians cannot use the clarification flow before deciding. This check should mirror the canonical-principal fallback used by decision application.

Useful? React with 👍 / 👎.

noanflaherty · 2026-03-01T04:48:14Z

@codex review this PR again — the previous issues have been fixed in commit aa32572

noanflaherty · 2026-03-01T04:48:15Z

@devin review this PR again — the previous issues have been fixed in commit aa32572

chatgpt-codex-connector · 2026-03-01T04:52:27Z

Codex Review: Didn't find any major issues. What shall we delve into next?

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

devin-ai-integration

Devin Review found 1 new potential issue.

View 9 additional findings in Devin Review.

devin-ai-integration · 2026-03-01T04:55:02Z

        if (
-          request.guardianExternalUserId &&
-          !actor.isTrusted &&
-          actor.externalUserId !== request.guardianExternalUserId
+          request.guardianPrincipalId &&
+          actor.guardianPrincipalId &&
+          !isAuthorizedGuardianPrincipal(actor.guardianPrincipalId, request.guardianPrincipalId)


🟡 Code-only clarification identity check silently skips when actor principal is missing

The code-only clarification identity gate in guardian-reply-router.ts is weaker than the old code because the condition requires both request.guardianPrincipalId and actor.guardianPrincipalId to be truthy before checking authorization. When actor.guardianPrincipalId is undefined (e.g., a channel guardian whose binding was created before guardianPrincipalId was introduced), the entire check is skipped and request details (toolName, questionText) are exposed.

Comparison with old code

Old code:

if ( request.guardianExternalUserId && !actor.isTrusted && actor.externalUserId !== request.guardianExternalUserId )

The old code blocked when actor.externalUserId (always present for channel actors) didn't match the request's guardian, regardless of whether guardianPrincipalId existed.

New code:

if ( request.guardianPrincipalId && actor.guardianPrincipalId && !isAuthorizedGuardianPrincipal(...) )

When actor.guardianPrincipalId is falsy (legacy binding with null guardianPrincipalId), the second operand short-circuits the entire condition to false, and the block that returns 'Request not found.' is never reached.

By contrast, the decision gate in applyCanonicalGuardianDecision (guardian-decision-primitive.ts:404) correctly rejects when actor principal is missing. This read-only path should mirror that fail-closed behavior.

Impact: A channel actor on a legacy binding without guardianPrincipalId who enters a cross-channel request code can see clarification details that would have been hidden by the old identity check. Practical impact is limited to read-only info disclosure and only for legacy bindings.

Suggested change

if (

request.guardianExternalUserId &&

!actor.isTrusted &&

actor.externalUserId !== request.guardianExternalUserId

request.guardianPrincipalId &&

actor.guardianPrincipalId &&

!isAuthorizedGuardianPrincipal(actor.guardianPrincipalId, request.guardianPrincipalId)

if (

request.guardianPrincipalId &&

(!actor.guardianPrincipalId || !isAuthorizedGuardianPrincipal(actor.guardianPrincipalId, request.guardianPrincipalId))

) {

Was this helpful? React with 👍 or 👎 to provide feedback.

* feat: add guardianPrincipalId to schema and storage plumbing (#10965) Co-authored-by: Claude <noreply@anthropic.com> * M2: Backfill + startup invariants for guardian principal (#10969) * feat: backfill guardianPrincipalId and enforce startup invariants Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: restrict backfill expiration to expired requests and tighten desktop rebinding predicate Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * feat: propagate guardianPrincipalId through runtime contexts (#10978) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * M4: Bind principal at all canonical request creation sites (#10980) * feat: bind guardianPrincipalId at all canonical request creation sites Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add try/catch guards around createCanonicalGuardianRequest in all caller paths Wrap createCanonicalGuardianRequest calls in try/catch in three locations to handle IntegrityError when guardianPrincipalId is missing: 1. daemon/server.ts (makePendingInteractionRegistrar) - prevents crash when session.guardianContext is undefined during tool approval events 2. runtime/routes/conversation-routes.ts (makeHubPublisher) - same pattern for the HTTP hub publisher path 3. runtime/access-request-helper.ts - preserves the intentional no-binding fallback path (documented at file header) where access requests proceed without guardian identity All catch blocks log at debug level matching the existing pattern in daemon/handlers/sessions.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add try/catch in inbound-message-handler and fix misleading notified:true in access-request-helper - Wrap createCanonicalGuardianRequest call in inbound-message-handler.ts ingress escalation path with try/catch to prevent unhandled IntegrityError from crashing the HTTP handler with a 500. On failure, logs a warning and continues to the notification pipeline. - Fix catch block in access-request-helper.ts to return notified: false instead of notified: true, since the emitNotificationSignal call is never reached when the error is caught. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: pass guardianPrincipalId to all createBinding callsites Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * M5: Cutover decision authorization + remove isTrusted (#10990) * 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 * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * feat: M6 — tests + guardrails + cleanup for principal-based auth (#10961) (#11002) * feat: M6 — tests + guardrails + cleanup for principal-based auth (#10961) - Update 6 test files to use guardianPrincipalId principal-match pattern instead of legacy isTrusted boolean - Add guard test (no-is-trusted-guard.test.ts) to prevent isTrusted reintroduction in production code - Clean up 3 stale comments referencing trusted bypass and desktop/trusted with principal-based terminology - Verify no production isTrusted references remain (only allowed trust-class variable names: isTrustedActor, isTrustedContact, isTrustedTrustClass) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: correct guard test pipe and identity mismatch test field Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use JS-level allowlist filtering in guard test to prevent masking --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix: address holistic review — decidedByPrincipalId, access_request exempt, backfill expiry, code lookup guard Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: make isAuthorizedGuardianPrincipal symmetric for cross-channel approval Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address reviewer feedback — binding check, pre-bootstrap IPC, migration access_request exclusion - isAuthorizedGuardianPrincipal: verify actor's principal belongs to an active guardian binding before allowing cross-channel approval, closing the asymmetric authorization gap (Devin #3/#4) - local-actor-identity: eagerly create vellum binding via ensureVellumGuardianBinding in pre-bootstrap IPC path so downstream decisionable requests always have a guardianPrincipalId (Devin #5) - Migration 126: exclude access_request from expiry sweep in steps 3a and 3c since access_request is non-decisionable and proceeds via the invite flow (Codex P2 #2) - Update roundtrip tests to supply guardianPrincipalId for decisionable tool_approval requests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: unify channel binding principals + fix test fixtures for IntegrityError guard Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: align migration registry checkpoint key with migration function (_v1 -> _v2) * fix: narrow CanonicalDecisionResult union before accessing resolverReplyText --------- Co-authored-by: Claude <noreply@anthropic.com>

noanflaherty requested review from NgoHarrison and siddseethepalli as code owners March 1, 2026 04:29

noanflaherty self-assigned this Mar 1, 2026

This comment was marked as resolved.

Sign in to view

chatgpt-codex-connector Bot reviewed Mar 1, 2026

View reviewed changes

noanflaherty merged commit d4af4fd into feature/canonical-id-binding-cutover Mar 1, 2026
1 check passed

noanflaherty deleted the swarm/canonical-id-bind/task-5 branch March 1, 2026 04:54

devin-ai-integration Bot reviewed Mar 1, 2026

View reviewed changes

noanflaherty mentioned this pull request Mar 1, 2026

Canonical Identity Binding Cutover: Remove isTrusted #11006

Merged

5 tasks

Conversation

noanflaherty commented Mar 1, 2026 • edited by devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Authorization logic (guardian-decision-primitive.ts)

Type changes (guardian-request-resolvers.ts)

Store changes (canonical-guardian-store.ts)

Guardian reply router (guardian-reply-router.ts)

Callsite updates (all pass guardianPrincipalId instead of isTrusted)

Test plan

Uh oh!

noanflaherty commented Mar 1, 2026

⚠️ Human Attention Needed

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

noanflaherty commented Mar 1, 2026

Uh oh!

noanflaherty commented Mar 1, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

noanflaherty commented Mar 1, 2026

Uh oh!

noanflaherty commented Mar 1, 2026

Uh oh!

chatgpt-codex-connector Bot commented Mar 1, 2026

Uh oh!

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

noanflaherty commented Mar 1, 2026 •

edited by devin-ai-integration Bot

Loading

Authorization logic (`guardian-decision-primitive.ts`)

Type changes (`guardian-request-resolvers.ts`)

Store changes (`canonical-guardian-store.ts`)

Guardian reply router (`guardian-reply-router.ts`)

Callsite updates (all pass `guardianPrincipalId` instead of `isTrusted`)