diff --git a/assistant/src/__tests__/guardian-actions-endpoint.test.ts b/assistant/src/__tests__/guardian-actions-endpoint.test.ts index 42fabbf4ed9..c1278e5f55e 100644 --- a/assistant/src/__tests__/guardian-actions-endpoint.test.ts +++ b/assistant/src/__tests__/guardian-actions-endpoint.test.ts @@ -317,7 +317,7 @@ describe('HTTP handleGuardianActionDecision', () => { const actorContext = call.actorContext as Record; expect(actorContext.externalUserId).toBeUndefined(); expect(actorContext.channel).toBe('vellum'); - expect(actorContext.isTrusted).toBe(true); + expect(actorContext.guardianPrincipalId).toBeDefined(); }); }); @@ -611,7 +611,7 @@ describe('IPC guardian_action_decision', () => { const actorContext = call.actorContext as Record; expect(actorContext.externalUserId).toBeUndefined(); expect(actorContext.channel).toBe('vellum'); - expect(actorContext.isTrusted).toBe(true); + expect(actorContext.guardianPrincipalId).toBeDefined(); }); }); diff --git a/assistant/src/__tests__/guardian-decision-primitive-canonical.test.ts b/assistant/src/__tests__/guardian-decision-primitive-canonical.test.ts index a5f2aa3fffc..f37d9853046 100644 --- a/assistant/src/__tests__/guardian-decision-primitive-canonical.test.ts +++ b/assistant/src/__tests__/guardian-decision-primitive-canonical.test.ts @@ -64,11 +64,14 @@ afterAll(() => { // Helpers // --------------------------------------------------------------------------- +/** Consistent test principal used across all test actors and requests. */ +const TEST_PRINCIPAL_ID = 'test-principal-id'; + function guardianActor(overrides: Partial = {}): ActorContext { return { externalUserId: 'guardian-1', channel: 'telegram', - isTrusted: false, + guardianPrincipalId: TEST_PRINCIPAL_ID, ...overrides, }; } @@ -77,7 +80,7 @@ function trustedActor(overrides: Partial = {}): ActorContext { return { externalUserId: undefined, channel: 'desktop', - isTrusted: true, + guardianPrincipalId: TEST_PRINCIPAL_ID, ...overrides, }; } @@ -120,6 +123,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceChannel: 'telegram', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', inputDigest: 'sha256:abc', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -153,6 +157,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceChannel: 'telegram', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', inputDigest: 'sha256:abc', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -178,6 +183,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceType: 'voice', sourceChannel: 'twilio', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, callSessionId: 'call-1', pendingQuestionId: 'pq-1', questionText: 'What is the gate code?', @@ -199,21 +205,22 @@ describe('applyCanonicalGuardianDecision', () => { expect(resolved!.answerText).toBe('1234'); }); - // ── Identity mismatch ────────────────────────────────────────────── + // ── Principal mismatch ────────────────────────────────────────────── - test('rejects decision when actor does not match guardian', async () => { + test('rejects decision when actor principal does not match request principal', async () => { const req = createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); const result = await applyCanonicalGuardianDecision({ requestId: req.id, action: 'approve_once', - actorContext: guardianActor({ externalUserId: 'imposter-99' }), + actorContext: guardianActor({ guardianPrincipalId: 'wrong-principal' }), }); expect(result.applied).toBe(false); @@ -225,12 +232,13 @@ describe('applyCanonicalGuardianDecision', () => { expect(unchanged!.status).toBe('pending'); }); - test('trusted actor bypasses identity check', async () => { + test('matching principal authorizes decision (cross-channel same principal)', async () => { const req = createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'desktop', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', inputDigest: 'sha256:abc', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -243,24 +251,48 @@ describe('applyCanonicalGuardianDecision', () => { }); expect(result.applied).toBe(true); - // No grant minted because trusted actor has no externalUserId if (!result.applied) return; + // No grant minted because trusted actor has no externalUserId expect(result.grantMinted).toBe(false); }); - test('rejects non-trusted decision when tool approval has no guardian binding', async () => { + test('rejects decision when request has no guardianPrincipalId', async () => { + // unknown_kind is not in DECISIONABLE_KINDS so it can be created without + // guardianPrincipalId, but the decision primitive still rejects because + // the request is missing its principal binding. + const req = createCanonicalGuardianRequest({ + kind: 'unknown_kind', + sourceType: 'channel', + conversationId: 'conv-1', + guardianExternalUserId: 'guardian-1', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + }); + + const result = await applyCanonicalGuardianDecision({ + requestId: req.id, + action: 'approve_once', + actorContext: guardianActor({ guardianPrincipalId: 'some-principal' }), + }); + + expect(result.applied).toBe(false); + if (result.applied) return; + expect(result.reason).toBe('identity_mismatch'); + }); + + test('rejects decision when actor has no guardianPrincipalId', async () => { const req = createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'channel', conversationId: 'conv-1', - // No guardianExternalUserId — open request + guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); const result = await applyCanonicalGuardianDecision({ requestId: req.id, action: 'approve_once', - actorContext: guardianActor({ externalUserId: 'anyone' }), + actorContext: guardianActor({ guardianPrincipalId: undefined }), }); expect(result.applied).toBe(false); @@ -276,6 +308,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -324,6 +357,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -351,6 +385,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceChannel: 'telegram', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', inputDigest: 'sha256:abc', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -385,6 +420,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() - 10_000).toISOString(), // already expired }); @@ -405,6 +441,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, // No expiresAt }); @@ -426,6 +463,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceChannel: 'telegram', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'file_read', inputDigest: 'sha256:def', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -446,6 +484,7 @@ describe('applyCanonicalGuardianDecision', () => { sourceType: 'voice', sourceChannel: 'twilio', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, callSessionId: 'call-99', pendingQuestionId: 'pq-99', questionText: 'What is the password?', @@ -464,12 +503,13 @@ describe('applyCanonicalGuardianDecision', () => { expect(resolved!.answerText).toBe('secret123'); }); - test('succeeds even with no resolver for unknown kind', async () => { + test('succeeds for non-decisionable kind with matching principal', async () => { const req = createCanonicalGuardianRequest({ kind: 'unknown_kind', sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -485,7 +525,7 @@ describe('applyCanonicalGuardianDecision', () => { expect(resolved!.status).toBe('approved'); }); - test('trusted desktop actor still mints scoped grant for approved canonical request', async () => { + test('desktop actor with matching principal mints scoped grant for approved canonical request', async () => { const req = createCanonicalGuardianRequest({ kind: 'unknown_kind', sourceType: 'voice', @@ -494,6 +534,7 @@ describe('applyCanonicalGuardianDecision', () => { callSessionId: 'call-voice-1', toolName: 'host_bash', inputDigest: 'sha256:voice-digest-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -530,6 +571,7 @@ describe('mintCanonicalRequestGrant', () => { sourceType: 'channel', sourceChannel: 'telegram', conversationId: 'conv-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', inputDigest: 'sha256:abc', }); @@ -549,6 +591,7 @@ describe('mintCanonicalRequestGrant', () => { sourceType: 'channel', sourceChannel: 'telegram', conversationId: 'conv-2', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', inputDigest: 'sha256:xyz', }); @@ -570,6 +613,7 @@ describe('mintCanonicalRequestGrant', () => { const req = createCanonicalGuardianRequest({ kind: 'pending_question', sourceType: 'voice', + guardianPrincipalId: TEST_PRINCIPAL_ID, // No toolName or inputDigest }); @@ -586,6 +630,7 @@ describe('mintCanonicalRequestGrant', () => { const req = createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'channel', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', // No inputDigest }); diff --git a/assistant/src/__tests__/guardian-routing-invariants.test.ts b/assistant/src/__tests__/guardian-routing-invariants.test.ts index 01c46260a05..c88ed7b95bf 100644 --- a/assistant/src/__tests__/guardian-routing-invariants.test.ts +++ b/assistant/src/__tests__/guardian-routing-invariants.test.ts @@ -5,7 +5,7 @@ * its key architectural invariants: * * 1. All decision paths route through `applyCanonicalGuardianDecision` - * 2. Identity checks are enforced before decisions are applied + * 2. Principal-based authorization is enforced before decisions are applied * 3. Stale/expired/already-resolved decisions are rejected * 4. Code-only messages return clarification (not auto-approve) * 5. Disambiguation with multiple pending requests stays fail-closed @@ -88,11 +88,14 @@ afterAll(() => { // Helpers // --------------------------------------------------------------------------- +/** Consistent test principal used across all test actors and requests. */ +const TEST_PRINCIPAL_ID = 'test-principal-id'; + function guardianActor(overrides: Partial = {}): ActorContext { return { externalUserId: 'guardian-1', channel: 'telegram', - isTrusted: false, + guardianPrincipalId: TEST_PRINCIPAL_ID, ...overrides, }; } @@ -101,7 +104,7 @@ function trustedActor(overrides: Partial = {}): ActorContext { return { externalUserId: undefined, channel: 'desktop', - isTrusted: true, + guardianPrincipalId: TEST_PRINCIPAL_ID, ...overrides, }; } @@ -223,25 +226,26 @@ describe('routing invariant: all decision paths reference applyCanonicalGuardian }); // =========================================================================== -// SECTION 2: Identity enforcement invariants +// SECTION 2: Principal-based authorization invariants // =========================================================================== -describe('routing invariant: identity checks enforced before decisions', () => { +describe('routing invariant: principal-based authorization enforced before decisions', () => { beforeEach(() => resetTables()); - test('non-matching actor identity is rejected by canonical primitive', async () => { + test('mismatching actor principal is rejected by canonical primitive', async () => { const req = createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); const result = await applyCanonicalGuardianDecision({ requestId: req.id, action: 'approve_once', - actorContext: guardianActor({ externalUserId: 'imposter-99' }), + actorContext: guardianActor({ guardianPrincipalId: 'wrong-principal' }), }); expect(result.applied).toBe(false); @@ -253,12 +257,13 @@ describe('routing invariant: identity checks enforced before decisions', () => { expect(unchanged!.status).toBe('pending'); }); - test('trusted (desktop) actor bypasses identity check', async () => { + test('matching principal authorizes desktop actor', async () => { const req = createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'desktop', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -271,19 +276,19 @@ describe('routing invariant: identity checks enforced before decisions', () => { expect(result.applied).toBe(true); }); - test('request with no guardian binding rejects non-trusted actor', async () => { + test('actor without guardianPrincipalId is rejected', async () => { const req = createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'channel', conversationId: 'conv-1', - // No guardianExternalUserId — open request + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); const result = await applyCanonicalGuardianDecision({ requestId: req.id, action: 'approve_once', - actorContext: guardianActor({ externalUserId: 'anyone' }), + actorContext: guardianActor({ guardianPrincipalId: undefined }), }); expect(result.applied).toBe(false); @@ -291,12 +296,13 @@ describe('routing invariant: identity checks enforced before decisions', () => { expect(result.reason).toBe('identity_mismatch'); }); - test('identity mismatch on code-only message blocks detail leakage', async () => { + test('principal mismatch on code-only message blocks detail leakage', async () => { createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'ABC123', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -304,7 +310,7 @@ describe('routing invariant: identity checks enforced before decisions', () => { const result = await routeGuardianReply(replyCtx({ messageText: 'ABC123', - actor: guardianActor({ externalUserId: 'imposter' }), + actor: guardianActor({ guardianPrincipalId: 'wrong-principal' }), conversationId: 'conv-1', })); @@ -329,6 +335,7 @@ describe('routing invariant: stale/expired/already-resolved decisions rejected', sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() - 10_000).toISOString(), // already expired }); @@ -349,6 +356,7 @@ describe('routing invariant: stale/expired/already-resolved decisions rejected', sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -393,6 +401,7 @@ describe('routing invariant: stale/expired/already-resolved decisions rejected', sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'ABC123', expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -423,6 +432,7 @@ describe('routing invariant: stale/expired/already-resolved decisions rejected', sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() - 10_000).toISOString(), // already expired }); @@ -451,6 +461,7 @@ describe('routing invariant: code-only messages return clarification', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'A1B2C3', toolName: 'shell', questionText: 'Run shell command: ls -la', @@ -482,6 +493,7 @@ describe('routing invariant: code-only messages return clarification', () => { sourceChannel: 'voice', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, callSessionId: 'call-1', pendingQuestionId: 'pq-1', requestCode: 'A2B3C4', @@ -513,6 +525,7 @@ describe('routing invariant: code-only messages return clarification', () => { sourceChannel: 'voice', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, callSessionId: 'call-2', pendingQuestionId: 'pq-2', requestCode: 'B2C3D4', @@ -544,6 +557,7 @@ describe('routing invariant: code-only messages return clarification', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'A1B2C3', toolName: 'shell', inputDigest: 'sha256:abc', @@ -570,6 +584,7 @@ describe('routing invariant: code-only messages return clarification', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'D4E5F6', expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -601,6 +616,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'DDD444', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -628,6 +644,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'GGG777', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -648,12 +665,13 @@ describe('routing invariant: disambiguation stays fail-closed', () => { expect(unchanged!.status).toBe('pending'); }); - test('explicit empty pendingRequestIds hint stays fail-closed for trusted actors', async () => { + test('explicit empty pendingRequestIds hint stays fail-closed for desktop actors', async () => { createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'channel', conversationId: 'conv-other', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'HHH888', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -678,6 +696,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'EEE555', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -688,6 +707,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'FFF666', toolName: 'file_write', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -719,6 +739,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'AAA111', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -729,6 +750,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'BBB222', toolName: 'file_write', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -771,6 +793,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceChannel: 'voice', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, callSessionId: 'call-answer', pendingQuestionId: 'pq-answer', requestCode: 'ABC123', @@ -784,6 +807,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceChannel: 'voice', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, callSessionId: 'call-approval', pendingQuestionId: 'pq-approval', requestCode: 'DEF456', @@ -815,6 +839,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'CCC333', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -849,6 +874,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', requestCode: 'GO1234', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -876,6 +902,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: '111AAA', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -885,6 +912,7 @@ describe('routing invariant: disambiguation stays fail-closed', () => { sourceType: 'channel', conversationId: 'conv-2', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: '222BBB', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -951,6 +979,7 @@ describe('routing invariant: approve_always downgraded for guardian-on-behalf', sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', inputDigest: 'sha256:abc', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -983,6 +1012,7 @@ describe('routing invariant: callback buttons route through canonical primitive' sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, toolName: 'shell', inputDigest: 'sha256:abc', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -1009,6 +1039,7 @@ describe('routing invariant: callback buttons route through canonical primitive' sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); registerPendingToolApprovalInteraction(req.id, 'conv-1', 'shell'); @@ -1032,6 +1063,7 @@ describe('routing invariant: callback buttons route through canonical primitive' sourceType: 'channel', conversationId: 'conv-other', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); registerPendingToolApprovalInteraction(req.id, 'conv-other', 'shell'); @@ -1044,8 +1076,8 @@ describe('routing invariant: callback buttons route through canonical primitive' // Should be consumed — conversationId scoping was removed because in // cross-channel flows the guardian's conversation differs from the - // requester's. Identity validation in the canonical decision primitive - // (guardianExternalUserId match) is the correct security boundary. + // requester's. Principal validation in the canonical decision primitive + // is the correct security boundary. expect(result.consumed).toBe(true); expect(result.decisionApplied).toBe(true); @@ -1056,14 +1088,14 @@ describe('routing invariant: callback buttons route through canonical primitive' }); // =========================================================================== -// SECTION 9: Destination hints do not bypass identity binding for tool_approval +// SECTION 9: Destination hints do not bypass principal binding for tool_approval // =========================================================================== -describe('routing invariant: destination hints do not bypass tool_approval identity binding', () => { +describe('routing invariant: destination hints do not bypass tool_approval principal binding', () => { beforeEach(() => resetTables()); - test('explicit pendingRequestIds still fail closed when guardianExternalUserId is missing', async () => { - // Voice-originated tool approval with missing guardian identity binding. + test('explicit pendingRequestIds still fail closed when guardianPrincipalId does not match', async () => { + // Voice-originated tool approval with a different principal than the actor. const req = createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'voice', @@ -1071,8 +1103,8 @@ describe('routing invariant: destination hints do not bypass tool_approval ident conversationId: 'conv-voice-1', toolName: 'shell', requestCode: 'NL1234', + guardianPrincipalId: 'request-principal', expiresAt: new Date(Date.now() + 60_000).toISOString(), - // guardianExternalUserId intentionally omitted }); registerPendingToolApprovalInteraction(req.id, 'conv-voice-1', 'shell'); @@ -1081,7 +1113,7 @@ describe('routing invariant: destination hints do not bypass tool_approval ident const result = await routeGuardianReply(replyCtx({ messageText: 'approve', channel: 'telegram', - actor: guardianActor({ externalUserId: 'guardian-tg-user' }), + actor: guardianActor({ guardianPrincipalId: 'different-principal' }), conversationId: 'conv-guardian-chat', pendingRequestIds: [req.id], approvalConversationGenerator: undefined, @@ -1095,8 +1127,8 @@ describe('routing invariant: destination hints do not bypass tool_approval ident expect(resolved!.status).toBe('pending'); }); - test('without destination hints, missing guardianExternalUserId means no pending requests found', async () => { - // Voice-originated request: no guardianExternalUserId + test('without destination hints, unbound principal means no pending requests found', async () => { + // Voice-originated request: different principal const req = createCanonicalGuardianRequest({ kind: 'tool_approval', sourceType: 'voice', @@ -1104,6 +1136,7 @@ describe('routing invariant: destination hints do not bypass tool_approval ident conversationId: 'conv-voice-2', toolName: 'shell', requestCode: 'NL5678', + guardianPrincipalId: 'voice-principal', expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -1143,6 +1176,7 @@ describe('routing invariant: invite handoff bypass for access requests', () => { sourceChannel: 'telegram', conversationId: 'conv-access-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'INV001', toolName: 'ingress_access_request', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -1171,6 +1205,7 @@ describe('routing invariant: invite handoff bypass for access requests', () => { sourceType: 'channel', sourceChannel: 'telegram', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, expiresAt: new Date(Date.now() + 60_000).toISOString(), }); @@ -1193,6 +1228,7 @@ describe('routing invariant: invite handoff bypass for access requests', () => { sourceType: 'channel', conversationId: 'conv-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'TAP001', toolName: 'shell', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -1219,6 +1255,7 @@ describe('routing invariant: invite handoff bypass for access requests', () => { sourceChannel: 'telegram', conversationId: 'conv-access-2', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'A00B01', toolName: 'ingress_access_request', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -1239,13 +1276,14 @@ describe('routing invariant: invite handoff bypass for access requests', () => { expect(resolved!.status).toBe('approved'); }); - test('trusted desktop access-request approval returns a verification code reply', async () => { + test('desktop access-request approval returns a verification code reply', async () => { const req = createCanonicalGuardianRequest({ kind: 'access_request', sourceType: 'channel', sourceChannel: 'telegram', conversationId: 'conv-access-desktop', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requestCode: 'C0D3A5', toolName: 'ingress_access_request', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -1276,6 +1314,7 @@ describe('routing invariant: invite handoff bypass for access requests', () => { sourceChannel: 'telegram', conversationId: 'conv-access-desktop-nl', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: TEST_PRINCIPAL_ID, requesterExternalUserId: 'requester-1', requesterChatId: 'chat-1', requestCode: 'A1B2C3', diff --git a/assistant/src/__tests__/no-is-trusted-guard.test.ts b/assistant/src/__tests__/no-is-trusted-guard.test.ts new file mode 100644 index 00000000000..3bb849227cf --- /dev/null +++ b/assistant/src/__tests__/no-is-trusted-guard.test.ts @@ -0,0 +1,77 @@ +/** + * Guard test: `isTrusted` must not appear in production code. + * + * The authorization model was migrated from a boolean `isTrusted` flag to + * principal-based authorization (`guardianPrincipalId` matching). This guard + * ensures the legacy pattern is never reintroduced in production source files. + * + * The invariant: `actor.guardianPrincipalId === request.guardianPrincipalId` + * (with cross-channel fallback via the vellum canonical principal). + * + * Allowed exceptions: + * - Variable names like `isTrustedActor` or `isTrustedContact` that refer + * to trust-class checks (e.g. `trustClass === 'guardian'`), NOT to a + * boolean `isTrusted` property on ActorContext. + * - Test files (__tests__/) — may reference `isTrusted` in test descriptions + * or comments about the migration. + */ + +import { execSync } from 'node:child_process'; +import { resolve } from 'node:path'; + +import { describe, expect, test } from 'bun:test'; + +const repoRoot = resolve(__dirname, '..', '..', '..'); + +describe('isTrusted guard', () => { + test('isTrusted property must not exist in production ActorContext usage', () => { + // Search for `isTrusted` used as a property (e.g., `.isTrusted`, `isTrusted:`, + // `isTrusted =`) in production TypeScript files, excluding tests, node_modules, + // and the allowed trust-class variable pattern. + const raw = execSync( + [ + 'grep -rn "isTrusted" assistant/src/ --include="*.ts"', + 'grep -v "__tests__"', + 'grep -v "node_modules"', + ].join(' | ') + ' || true', + { encoding: 'utf-8', cwd: repoRoot }, + ); + + // Filter in JS: strip allowed token names from each line, then check if + // `isTrusted` still appears. This avoids the grep -v approach which could + // mask forbidden usage on lines that also contain allowed tokens. + const ALLOWED_TOKENS = ['isTrustedActor', 'isTrustedContact', 'isTrustedTrustClass']; + const offending = raw + .trim() + .split('\n') + .filter((line) => { + if (!line) return false; + let stripped = line; + for (const token of ALLOWED_TOKENS) { + stripped = stripped.replaceAll(token, ''); + } + return stripped.includes('isTrusted'); + }); + + if (offending.length > 0) { + throw new Error( + 'Found `isTrusted` references in production code. Authorization must use ' + + '`guardianPrincipalId` matching instead. Offending lines:\n' + + offending.join('\n'), + ); + } + }); + + test('ActorContext interface must not declare isTrusted field', () => { + // Verify the ActorContext type definition does not include isTrusted + const result = execSync( + [ + 'grep -n "isTrusted" assistant/src/approvals/guardian-request-resolvers.ts', + 'true', + ].join(' || '), + { encoding: 'utf-8', cwd: repoRoot }, + ); + + expect(result.trim()).toBe(''); + }); +}); diff --git a/assistant/src/__tests__/tool-approval-handler.test.ts b/assistant/src/__tests__/tool-approval-handler.test.ts index 928c17ddbc1..02a9db78c8e 100644 --- a/assistant/src/__tests__/tool-approval-handler.test.ts +++ b/assistant/src/__tests__/tool-approval-handler.test.ts @@ -286,7 +286,7 @@ describe('ToolApprovalHandler / pre-exec gate grant check', () => { expect(result.allowed).toBe(true); }); - test('undefined actor role (desktop/trusted) bypasses grant check', async () => { + test('undefined actor role (desktop) bypasses grant check', async () => { const toolName = 'bash'; const input = { command: 'deploy' }; diff --git a/assistant/src/__tests__/tool-grant-request-escalation.test.ts b/assistant/src/__tests__/tool-grant-request-escalation.test.ts index 4fd72ca6149..5fb7486ec9b 100644 --- a/assistant/src/__tests__/tool-grant-request-escalation.test.ts +++ b/assistant/src/__tests__/tool-grant-request-escalation.test.ts @@ -162,7 +162,7 @@ function guardianActor(overrides: Partial = {}): ActorContext { return { externalUserId: 'guardian-1', channel: 'telegram', - isTrusted: false, + guardianPrincipalId: 'test-principal-id', ...overrides, }; } @@ -361,6 +361,7 @@ describe('applyCanonicalGuardianDecision / tool_grant_request', () => { conversationId: 'conv-1', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', inputDigest: 'sha256:testdigest', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -390,6 +391,7 @@ describe('applyCanonicalGuardianDecision / tool_grant_request', () => { conversationId: 'conv-1', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', inputDigest: 'sha256:testdigest', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -417,6 +419,7 @@ describe('applyCanonicalGuardianDecision / tool_grant_request', () => { conversationId: 'conv-1', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', inputDigest: 'sha256:testdigest', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -425,7 +428,7 @@ describe('applyCanonicalGuardianDecision / tool_grant_request', () => { const result = await applyCanonicalGuardianDecision({ requestId: req.id, action: 'approve_once', - actorContext: guardianActor({ externalUserId: 'imposter-99' }), + actorContext: guardianActor({ guardianPrincipalId: 'imposter-principal' }), }); expect(result.applied).toBe(false); @@ -564,6 +567,7 @@ describe('inline wait-and-resume', () => { conversationId: 'conv-1', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', inputDigest: 'sha256:waitgrant', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -606,6 +610,7 @@ describe('inline wait-and-resume', () => { conversationId: 'conv-1', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', inputDigest: 'sha256:denywait', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -645,6 +650,7 @@ describe('inline wait-and-resume', () => { conversationId: 'conv-1', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', inputDigest: 'sha256:timeoutwait', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -679,6 +685,7 @@ describe('inline wait-and-resume', () => { conversationId: 'conv-1', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', inputDigest: 'sha256:abortwait', expiresAt: new Date(Date.now() + 60_000).toISOString(), diff --git a/assistant/src/__tests__/trusted-contact-inline-approval-integration.test.ts b/assistant/src/__tests__/trusted-contact-inline-approval-integration.test.ts index e2fa550aab6..63e8dec79e4 100644 --- a/assistant/src/__tests__/trusted-contact-inline-approval-integration.test.ts +++ b/assistant/src/__tests__/trusted-contact-inline-approval-integration.test.ts @@ -221,7 +221,7 @@ function guardianActor(overrides: Partial = {}): ActorContext { return { externalUserId: 'guardian-1', channel: 'telegram', - isTrusted: false, + guardianPrincipalId: 'test-principal-id', ...overrides, }; } @@ -381,6 +381,7 @@ describe('(b) prompt-path flow: confirmation_request bridges to guardian', () => conversationId: 'conv-bridge-1', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', status: 'pending', expiresAt: new Date(Date.now() + 5 * 60_000).toISOString(), @@ -419,6 +420,7 @@ describe('(b) prompt-path flow: confirmation_request bridges to guardian', () => conversationId: 'conv-unified-1', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', status: 'pending', expiresAt: new Date(Date.now() + 5 * 60_000).toISOString(), @@ -508,6 +510,7 @@ describe('(c) no-binding flow: trusted contact fails fast without guardian bindi conversationId: 'conv-nobinding', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', status: 'pending', expiresAt: new Date(Date.now() + 5 * 60_000).toISOString(), @@ -607,6 +610,7 @@ describe('(d) unknown actor flow: fail-closed with no interactive approval', () conversationId: 'conv-unknown', requesterExternalUserId: 'unknown-user', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', status: 'pending', expiresAt: new Date(Date.now() + 5 * 60_000).toISOString(), @@ -794,6 +798,7 @@ describe('(f) timeout/stale flow: stale guardian decision after inline wait time requesterExternalUserId: 'requester-1', requesterChatId: 'requester-chat-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', inputDigest: 'sha256:stale', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -842,6 +847,7 @@ describe('(f) timeout/stale flow: stale guardian decision after inline wait time requesterExternalUserId: 'requester-1', requesterChatId: 'requester-chat-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', inputDigest: 'sha256:fresh', expiresAt: new Date(Date.now() + 60_000).toISOString(), @@ -974,6 +980,7 @@ describe('cross-milestone integration checks', () => { conversationId: 'conv-consistency', requesterExternalUserId: 'requester-1', guardianExternalUserId: 'guardian-1', + guardianPrincipalId: 'test-principal-id', toolName: 'bash', status: 'pending', expiresAt: new Date(Date.now() + 5 * 60_000).toISOString(), diff --git a/assistant/src/approvals/guardian-decision-primitive.ts b/assistant/src/approvals/guardian-decision-primitive.ts index a804e8e8c59..d73a8ab6ed6 100644 --- a/assistant/src/approvals/guardian-decision-primitive.ts +++ b/assistant/src/approvals/guardian-decision-primitive.ts @@ -386,7 +386,7 @@ export async function applyCanonicalGuardianDecision( // 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. + // authorization gate — principal identity must always match. if (!request.guardianPrincipalId) { log.warn( diff --git a/assistant/src/approvals/guardian-request-resolvers.ts b/assistant/src/approvals/guardian-request-resolvers.ts index da61085e9a7..5f131dd3947 100644 --- a/assistant/src/approvals/guardian-request-resolvers.ts +++ b/assistant/src/approvals/guardian-request-resolvers.ts @@ -39,7 +39,7 @@ const log = getLogger('guardian-request-resolvers'); /** Actor context for the entity making the decision. */ export interface ActorContext { - /** External user ID of the deciding actor (undefined for desktop/trusted). */ + /** External user ID of the deciding actor (undefined for desktop actors). */ externalUserId: string | undefined; /** Channel the decision arrived on. */ channel: string;