Canonical Identity Binding Cutover: Remove isTrusted

[noanflaherty]

Summary

Remove the isTrusted compatibility bypass from all guardian decision authorization paths and replace it with canonical identity binding using guardianPrincipalId. All decisions are now authorized purely by principal matching: actor.guardianPrincipalId === request.guardianPrincipalId, with a cross-channel fallback via the assistant's canonical vellum principal.

Changes

• Added guardianPrincipalId column to channel_guardian_bindings and canonical_guardian_requests tables (M1)
• Backfilled existing rows with principal IDs and expired unresolvable pending requests (M2)
• Propagated principal through all runtime contexts: actor-trust-resolver, guardian-context-resolver, local-actor-identity, session-runtime-assembly (M3)
• Bound principal at all canonical request creation sites and all createBinding callsites, with IntegrityError guard for decisionable kinds (M4)
• Replaced isTrusted bypass in applyCanonicalGuardianDecision with 3-step principal validation, extracted isAuthorizedGuardianPrincipal shared helper (M5)
• Added guard test to prevent isTrusted reintroduction, updated all tests to principal-match pattern (M6)

Milestone PRs (merged into feature branch)

• M1: Schema + storage plumbing for guardianPrincipalId #10965: M1: Schema + storage plumbing for guardianPrincipalId
• M2: Backfill + startup invariants for guardian principal #10969: M2: Backfill + startup invariants for guardian principal
• M3: Propagate principal through runtime contexts #10978: M3: Propagate principal through runtime contexts
• M4: Bind principal at all canonical request creation sites #10980: M4: Bind principal at all canonical request creation sites
• M5: Cutover decision authorization + remove isTrusted #10990: M5: Cutover decision authorization + remove isTrusted
• feat: M6 — tests + guardrails + cleanup for principal-based auth (#10961) #11002: M6: Tests + guardrails + cleanup

Project issue

Closes #10955

Test plan

• Verify isTrusted does not appear in production code: grep -rn "isTrusted" assistant/src/ --include="*.ts" | grep -v __tests__ | grep -v node_modules
• Verify all decisionable canonical requests persist guardianPrincipalId
• Verify cross-surface approvals: request created in channel, decision applied from vellum/guardian thread
• Verify principal mismatch correctly blocks unauthorized decisions
• Run full test suite: cd assistant && bun test

Generated with Claude Code

---

[noanflaherty]

@codex review

[noanflaherty]

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

[noanflaherty]

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

[noanflaherty]

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

[noanflaherty]

PR Review: Canonical Identity Binding Cutover

I did a focused review of the identity-binding + isTrusted removal path. Two issues stood out.

1) [P1] Authorization is still effectively a compatibility shim in one direction

• File: assistant/src/approvals/guardian-decision-primitive.ts (isAuthorizedGuardianPrincipal, around lines 299-317)
• Issue: isAuthorizedGuardianPrincipal returns true when either side matches the vellum canonical principal, even when actorPrincipalId !== requestPrincipalId.

Current logic:

• allow on direct principal match
• allow if actor principal is canonical vellum principal
• allow if request principal is canonical vellum principal

That third case means a request bound to vellum canonical principal can be approved by any other channel principal, even if principals do not match. This weakens the strict principal-binding invariant and keeps a compatibility bypass in place.

Why this matters:

• The PR goal is to rely on canonical identity binding and retire isTrusted behavior.
• This helper preserves implicit trust bridging rather than enforcing a single bound identity.

Suggested fix:

• Prefer strict principal equality in the authorization gate.
• If cross-channel approvals are required, unify all bindings for the same guardian onto one principal (migration/backfill) instead of runtime OR-based fallback.

2) [P2] New principal requirement broke multiple touched tests

The new createCanonicalGuardianRequest guard requires guardianPrincipalId for decisionable kinds (tool_approval, tool_grant_request, pending_question). Several test fixtures in this PR still create tool_approval rows without it.

• File: assistant/src/__tests__/guardian-actions-endpoint.test.ts
  • createTestCanonicalRequest() (around lines 90-114) defaults to kind: 'tool_approval' but does not set guardianPrincipalId.
• File: assistant/src/__tests__/guardian-principal-id-roundtrip.test.ts
  • multiple tests create tool_approval without guardianPrincipalId (e.g. around lines 129-143, 178-182).

Repro:

cd assistant
bun test src/__tests__/guardian-actions-endpoint.test.ts

This currently fails with IntegrityError: Cannot create decisionable canonical request ... without guardianPrincipalId.

---

What I ran

• bun test src/__tests__/guardian-decision-primitive-canonical.test.ts
• bun test src/__tests__/guardian-routing-invariants.test.ts
• bun test src/__tests__/guardian-actions-endpoint.test.ts (fails as above)

[noanflaherty]

Both issues addressed in commit 59c9010:

P1: Replaced the isAuthorizedGuardianPrincipal runtime OR-based fallback with strict principal equality. All channel bindings are now unified onto the canonical vellum principal at creation time (channel-guardian-service.ts) and in the backfill migration (126, re-keyed to v2). The isAuthorizedGuardianPrincipal function has been removed entirely — authorization is now a single actorContext.guardianPrincipalId !== request.guardianPrincipalId check.

P2: Added guardianPrincipalId to the createTestCanonicalRequest helper in guardian-actions-endpoint.test.ts (defaults to 'test-principal'). All 4 guardian test files pass (111 tests, 0 failures).

[noanflaherty]

Follow-up review on 59c9010 (merge readiness)

I re-reviewed after the latest fix commit and reran the affected suites.

✅ Previously raised issues are addressed

• Principal authorization is now strict equality in the canonical decision path.
• Channel principal unification/backfill is implemented in migration/service paths.
• Test fixture IntegrityError failures were resolved (missing guardianPrincipalId in decisionable request fixtures).

⚠️ One remaining issue before merge

[P2] TypeScript union narrowing regression

• File: assistant/src/runtime/guardian-reply-router.ts (around line 507)
• Issue: resolverReplyText is read from canonicalResult without narrowing on applied.

Current expression:

const hasResolverReplyText = Boolean(result.canonicalResult?.resolverReplyText);

CanonicalDecisionResult is a union where resolverReplyText only exists on the applied: true branch, so this causes a TS error.

Suggested fix (matches the safe pattern already used on main):

const hasResolverReplyText = Boolean(
  result.canonicalResult?.applied && result.canonicalResult.resolverReplyText,
);

Test signal from this pass

I reran targeted suites on the PR head:

• guardian-actions-endpoint.test.ts ✅
• guardian-principal-id-roundtrip.test.ts ✅
• guardian-decision-primitive-canonical.test.ts ✅
• guardian-routing-invariants.test.ts ✅
• no-is-trusted-guard.test.ts ✅
• actor-token-service.test.ts ✅
• channel-guardian.test.ts ✅

Merge recommendation

This is very close, but I would fix the single guardian-reply-router.ts narrowing issue first, then merge.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 15 additional findings in Devin Review.

[noanflaherty]

Addressed in #11084