fix(sdk): Teams adapter token security + migration guide by diberry · Pull Request #906 · bradygaster/squad

diberry · 2026-04-07T23:06:10Z

What

Identity-scoped token cache, logout mechanism, device-code timeout guard, JWT claim extraction, and migration guide for async adapter change.

Why

PR #768 introduced a breaking async change with no migration path. Token cache had security gaps: no tenant/app isolation, no revocation, no timeout guard, stale tokens reused on error.

How

Identity-scoped token cache — keyed by SHA-256 hash of \ enantId:clientId, not just tenantId. Different app registrations and different tenant configs get separate cache files. JWT \ id/\oid\ claims extracted and stored as audit metadata; \configTenantId\ and \clientId\ validated on load to reject cross-config reuse.
Explicit logout() — renamed from
evoke()\ for honest naming (public AAD clients can't revoke server-side). Local credential purge: clears memory + disk + identity caches.
15-minute device-code timeout guard — poll interval clamped to 2-30s to prevent both rapid polling and hangs.
Selective token clearing — only on permanent auth errors (\invalid_grant, \interaction_required, \consent_required, \invalid_client). Transient errors preserve tokens for retry.
Per-instance identity cache — \cachedUserId\ moved from module-level to adapter instance.
esetIdentityCaches()\ called on every token change (load, refresh, re-auth, logout).
Legacy migration — old \ eams-tokens.json\ auto-migrated to per-identity file, then deleted.
Migration guide — \docs/migration/teams-async-adapter.md\

Design decisions

Cache key = hash(tenantId + clientId) because we need the path before auth (bootstrap problem). Real \ id/\oid\ from JWT stored as metadata, not as cache key.
*\logout()\ not
evoke()* — public-client AAD refresh tokens can't be reliably revoked server-side.
Known limitation: default \organizations\ authority + same \clientId\ shares one cache per OS user. Documented in migration guide with recommendation to set explicit \ enantId.

Tests

37 new security tests (72 total) covering:

Cache key design: clientId in hash, tenantId in hash, traversal protection, determinism
JWT extraction: valid/invalid/malformed tokens, missing claims
Token CRUD: tenant isolation, clientId isolation, metadata validation, mismatch rejection
Logout lifecycle: disk cleanup, idempotency, cross-config isolation
Legacy migration, timeout constants, permanent vs transient error classification

Note on PR #883

PR #883 (channelId encoding, icacls warning, integration tests) is a separate set of fixes with no overlap.

Closes #862
Related: #895 (SDK Hardening PRD)

github-actions · 2026-04-07T23:06:22Z

🟡 Impact Analysis — PR #906

Risk tier: 🟡 MEDIUM

📊 Summary

Metric	Count
Files changed	5
Files added	3
Files modified	2
Files deleted	0
Modules touched	4

🎯 Risk Factors

5 files changed (≤5 → LOW)
4 modules touched (2-4 → MEDIUM)

📦 Modules Affected

docs (1 file)

docs/migration/teams-async-adapter.md

root (1 file)

.changeset/teams-adapter-security.md

squad-sdk (2 files)

packages/squad-sdk/src/platform/comms-teams.ts
packages/squad-sdk/src/platform/types.ts

tests (1 file)

test/comms-teams-security.test.ts

This report is generated automatically for every PR. See #733 for details.

Copilot

Pull request overview

This PR hardens the Teams (teams-graph) communication adapter’s credential handling and documents the breaking async factory change introduced in PR #768, focusing on multi-tenant safety and explicit logout semantics.

Changes:

Add tenant-scoped token storage (hashed per-tenant filename), revocation support, and device-code timeout/polling bounds to the Teams adapter.
Extend the CommunicationAdapter interface with an optional revoke() method for local credential purge.
Add a migration guide and a dedicated security test suite; include a changeset for the SDK patch release.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/comms-teams-security.test.ts	Adds security-focused tests for tenant isolation, revocation, migration, and device-code guard constants.
packages/squad-sdk/src/platform/types.ts	Adds optional `revoke?(): Promise<void>` to `CommunicationAdapter`.
packages/squad-sdk/src/platform/comms-teams.ts	Implements tenant-scoped token paths, migration, revoke(), device-code timeout guard, and permanent auth error handling.
docs/migration/teams-async-adapter.md	Documents the async `createCommunicationAdapter` migration and token security changes.
.changeset/teams-adapter-security.md	Declares a patch changeset for the SDK release notes.

github-actions · 2026-04-07T23:18:52Z

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 032df44

PR Scope: 📦🔧 Mixed (product + infrastructure)

⚠️ 3 item(s) to address before review

Status	Check	Details
✅	Single commit	1 commit — clean history
✅	Not in draft	Ready for review
✅	Branch up to date	Up to date with dev
❌	Copilot review	No Copilot review yet — it may still be processing
✅	Changeset present	Changeset file found
✅	Scope clean	No .squad/ or docs/proposals/ files
✅	No merge conflicts	No merge conflicts
❌	Copilot threads resolved	1 unresolved Copilot thread(s) — fix and resolve before merging
❌	CI passing	1 check(s) failing: test
✅	Issue linked	Issue reference found
✅	Protected files	No protected bootstrap files changed

Files Changed (5 files, +680 −40)

File	+/−
`.changeset/teams-adapter-security.md`	+5 −0
`docs/migration/teams-async-adapter.md`	+129 −0
`packages/squad-sdk/src/platform/comms-teams.ts`	+176 −40
`packages/squad-sdk/src/platform/types.ts`	+7 −0
`test/comms-teams-security.test.ts`	+363 −0

Total: +680 −40

This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

diberry · 2026-04-08T17:55:07Z

🔍 Squad Review — Kaylee (Engineering)

#	Check	Status	Notes
1	Changelog entry	✅	.changeset/teams-adapter-security.md (SDK patch)
2	Squashed to 1 commit	✅	1 commit
3	CI green	✅	All checks passed
4	Copilot comments resolved	✅	4/4 threads resolved with fixes or acknowledgments
5	No .squad/ files	✅	Clean
6	No unrelated files	✅	5 files — adapter, types, migration guide, tests, changeset
7	Tests for changes	✅	est/comms-teams-security.test.ts — 37 new tests (72 total)
8	Not a duplicate/reversal	✅	Unique scope (closes #862); #883 is separate per description

Verdict: ✅ Ready to merge

Review by Squad AI team (Kaylee — Engineering) · requested by Dina Berry

diberry

Review findings for PR #906

🔴 required: loadTokens validates accessToken and expiresAt shape but skips refreshToken — a cached file with a null/missing refresh token passes the guard, gets returned to ensureAuthenticated, and triggers a wasted AAD call that always fails before falling back to re-auth. One-liner fix: add && typeof parsed.refreshToken === 'string' to the shape check.

🔴 required: The key behavioral invariant — "permanent error clears disk tokens; transient error preserves them" — isn't exercised through ensureAuthenticated. A mock test for refresh failure routing would catch regressions in that if/else branch.

🟡 suggestion: Changeset description says "explicit revoke() for logout" but the method was renamed to logout(). This text becomes the changelog entry — worth fixing before merge.

🟡 suggestion: resetIdentityCaches() clears resolvedChatId on every successful token refresh. For adapters relying on dynamic resolution, this means one extra Graph API call per hour. Consider only clearing if the authenticated oid actually changes.

Add typeof refreshToken === 'string' to shape check in loadTokens to prevent wasted AAD round-trips on incomplete cached tokens. Fix changeset to say logout() instead of revoke(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

diberry · 2026-04-10T13:26:51Z

✅ CI validation passed on fork: diberry#142 — 8/8 checks green

The Copilot CLI uses `-p` for non-interactive prompts, not `--message`. Also switches from `gh copilot` to `copilot` directly to avoid Windows console window issues with .cmd batch files. Files changed (8 source + 1 test + 1 pre-existing fix): - watch/capabilities/execute.ts - watch/capabilities/decision-hygiene.ts - watch/capabilities/monitor-email.ts - watch/capabilities/monitor-teams.ts - watch/capabilities/retro.ts - watch/capabilities/wave-dispatch.ts - watch/index.ts - loop.ts - comms-teams.ts (pre-existing TOKEN_PATH typo from #906) - test/cli/watch-execute.test.ts Fixes #980 Closes #874 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The Copilot CLI uses -p for non-interactive prompts, not --message. Uses copilot directly on Windows to avoid .cmd console window issues, falls back to gh copilot on Linux/macOS for compatibility. Changes: - 8 source files: --message to -p, platform-aware cmd selection - loop.ts: aligned preflight check with actual invocation command - comms-teams.ts: fix pre-existing TOKEN_PATH typo from #906 - 2 test files updated: watch-execute + watch-capabilities - Added changeset for squad-cli + squad-sdk patch bumps All 67 tests pass (watch-execute: 16, watch-capabilities: 51). Fixes #980 Closes #874 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The Copilot CLI uses -p for non-interactive prompts, not --message. Switches default invocation from gh copilot to copilot directly (the standalone binary available on all platforms). Changes: - 8 source files: --message to -p, cmd changed to copilot - loop.ts: aligned preflight to check copilot --version - comms-teams.ts: fix pre-existing TOKEN_PATH typo from #906 - 2 test files updated (watch-execute + watch-capabilities) - Changeset added for squad-cli + squad-sdk patch bumps Fixes #980 Closes #874 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

) The Copilot CLI uses -p for non-interactive prompts, not --message. Switches default invocation from gh copilot to copilot directly (the standalone binary available on all platforms). Changes: - 8 source files: --message to -p, cmd changed to copilot - loop.ts: aligned preflight to check copilot --version - comms-teams.ts: fix pre-existing TOKEN_PATH typo from #906 - 2 test files updated (watch-execute + watch-capabilities) - Changeset added for squad-cli + squad-sdk patch bumps Fixes #980 Closes #874 Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(sdk): Teams adapter token security + migration guide * fix(sdk): add refreshToken validation to loadTokens + fix changeset text Add typeof refreshToken === 'string' to shape check in loadTokens to prevent wasted AAD round-trips on incomplete cached tokens. Fix changeset to say logout() instead of revoke(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Dina Berry <diberry@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Brady Gaster <41929050+bradygaster@users.noreply.github.com>

) The Copilot CLI uses -p for non-interactive prompts, not --message. Switches default invocation from gh copilot to copilot directly (the standalone binary available on all platforms). Changes: - 8 source files: --message to -p, cmd changed to copilot - loop.ts: aligned preflight to check copilot --version - comms-teams.ts: fix pre-existing TOKEN_PATH typo from #906 - 2 test files updated (watch-execute + watch-capabilities) - Changeset added for squad-cli + squad-sdk patch bumps Fixes #980 Closes #874 Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 7, 2026 23:06

Copilot started reviewing on behalf of diberry April 7, 2026 23:06 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Comment thread packages/squad-sdk/src/platform/comms-teams.ts Outdated

Comment thread packages/squad-sdk/src/platform/comms-teams.ts Outdated

Comment thread test/comms-teams-security.test.ts Outdated

Comment thread packages/squad-sdk/src/platform/comms-teams.ts

diberry force-pushed the squad/862-teams-adapter-security branch from 0335a57 to 032df44 Compare April 7, 2026 23:18

diberry force-pushed the squad/862-teams-adapter-security branch from 032df44 to b759ada Compare April 8, 2026 14:22

fix(sdk): Teams adapter token security + migration guide

37e811d

diberry force-pushed the squad/862-teams-adapter-security branch from b759ada to 37e811d Compare April 8, 2026 15:55

diberry commented Apr 10, 2026

View reviewed changes

diberry mentioned this pull request Apr 10, 2026

fix(sdk): Teams adapter token security diberry/squad#142

Draft

Merge branch 'dev' into squad/862-teams-adapter-security

e2a991b

bradygaster merged commit ec19f5e into dev Apr 12, 2026

tamirdresher mentioned this pull request Apr 16, 2026

fix: replace obsolete --message flag with -p in watch and loop #988

Merged

tamirdresher mentioned this pull request Apr 22, 2026

fix: restore 106 files accidentally reverted by PR #1023 merge #1027

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(sdk): Teams adapter token security + migration guide#906

fix(sdk): Teams adapter token security + migration guide#906
bradygaster merged 3 commits intodevfrom
squad/862-teams-adapter-security

diberry commented Apr 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 7, 2026

Uh oh!

diberry commented Apr 8, 2026

Uh oh!

diberry left a comment

Uh oh!

diberry commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

diberry commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

Design decisions

Tests

Note on PR #883

Uh oh!

github-actions Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟡 Impact Analysis — PR #906

📊 Summary

🎯 Risk Factors

📦 Modules Affected

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 7, 2026

🛫 PR Readiness Check

⚠️ 3 item(s) to address before review

Files Changed (5 files, +680 −40)

Uh oh!

diberry commented Apr 8, 2026

🔍 Squad Review — Kaylee (Engineering)

Uh oh!

diberry left a comment

Choose a reason for hiding this comment

Uh oh!

diberry commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

diberry commented Apr 7, 2026 •

edited

Loading

github-actions Bot commented Apr 7, 2026 •

edited

Loading