Adapter hardening pass#441
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dancer
approved these changes
May 2, 2026
dancer
pushed a commit
that referenced
this pull request
May 4, 2026
…o helpers (#445) * docs: cover gchat verification, linear token encryption, shared crypto helpers The adapter hardening pass in #441 made gchat JWT verification required, added optional at-rest encryption for Linear OAuth tokens, and promoted the AES-256-GCM helpers into @chat-adapter/shared. Update the affected package READMEs and the adapter-authoring guide to match. * docs(adapter-shared): correct EncryptedTokenData field names --------- Co-authored-by: Ben Sabic <bensabic@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A round of correctness and configuration tightening across the platform adapters and the example app's OAuth install flows. Each change is small in surface area but several shift defaults, so I've called those out explicitly.
Summary of changes
@chat-adapter/gchat— require explicit verification configurationThe Google Chat adapter previously accepted the construction call when neither
googleChatProjectNumbernorpubsubAudiencewas set, logged a one-shot warning, and then processed every incoming webhook. That made the warning easy to miss in deployment logs and meant the safe configuration was the path of greatest effort.The constructor now refuses to start unless one of the following is true:
googleChatProjectNumber(orGOOGLE_CHAT_PROJECT_NUMBER) is configured for direct webhook JWT verification, orpubsubAudience(orGOOGLE_CHAT_PUBSUB_AUDIENCE) is configured for Pub/Sub push JWT verification, ordisableSignatureVerification: trueflag (orGOOGLE_CHAT_DISABLE_SIGNATURE_VERIFICATION=true) is set.The thrown
ValidationErrornames all three escape hatches so the resolution is obvious from the message alone. This mirrors what the Slack adapter already does forsigningSecret/webhookVerifier.Two transports share one HTTP endpoint here, so the constructor check alone is not sufficient: a deployment that only sets
googleChatProjectNumberwould still happily process Pub/Sub-shaped requests, and vice versa.handleWebhooknow treats each shape independently — if the verifier for the incoming shape is unconfigured anddisableSignatureVerificationis not set, the handler returns 401 instead of falling through to processing.This is a breaking change for any deployment that was relying on the old warn-and-process default. The fix is one env var. We chose loud-at-startup over silent-at-runtime intentionally — the example app (
examples/nextjs-chat/src/lib/adapters.ts) catches the throw and the gchat adapter will simply no-op until configured, so it doesn't take down the rest of the bot.@chat-adapter/github— multi-tenant bot identity auto-detectionIn multi-tenant App mode,
this.octokitis null at construction time and only populated lazily ingetOctokit()per installation. The previousinitialize()only fetched_botUserIdwhen a global Octokit existed — so multi-tenant bots ran with_botUserId === null, which meantparseAuthor()'sisMe: user.id === this._botUserIdcheck was always false (real GitHub user IDs are positive integers).For bots that respond to events that include their own replies, this caused self-message detection to silently fail and let the bot loop on its own messages.
The fix:
detectBotUserId(octokit)helper called eagerly the first time a per-installation Octokit is created ingetOctokit(), and synchronously insidehandleWebhook()once the installation ID is known soisMeworks on the very first dispatch.users.getAuthenticatedis the primary path; if that fails (some installation tokens don't have access to/user), it falls back toapps.getAuthenticated+users.getByUsernamefor${slug}[bot].removeReaction()'s ad-hoc lazy detection was deduplicated into the same helper.Single-tenant and PAT modes are unaffected —
initialize()still does the same single fetch it always did, just routed through the new helper.@chat-adapter/slack— constant-time compare on socket forwarding tokenhandleWebhookvalidated thex-slack-socket-tokenheader with!==, while every other auth check in the same file routed throughcrypto.timingSafeEqual. Switched to a smalltimingSafeStringEqualhelper that buffers both sides and length-checks before delegating totimingSafeEqual, matchingverifySignature's pattern.No behavior change for callers — same 401/200 outcomes for the same inputs.
@chat-adapter/linear— optional at-rest encryption for OAuth tokensThe Linear adapter persists per-organization
accessTokenandrefreshTokenvalues to whateverStateAdapterthe chat instance is configured with. Previously these went in plaintext; the Slack adapter has had AES-256-GCM token encryption support since the Slack OAuth work landed.This PR brings Linear to parity:
encryptionKeyconfig field (orLINEAR_ENCRYPTION_KEYenv var). Accepts the same hex-64 / base64-44 formats as the Slack adapter.setInstallation()encryptsaccessTokenandrefreshTokenbefore writing when a key is configured.getInstallation()decrypts on read; it tolerates legacy plaintext records (encryption key set, but stored value isn't anEncryptedTokenDataenvelope), which lets operators rotate encryption in without flushing existing installs.encryptionKey, behavior is identical to before — plaintext storage, no changes to the on-disk format.To avoid duplicating the AES-GCM primitives across Slack and Linear, the actual encryption helpers (
encryptToken,decryptToken,decodeKey,isEncryptedTokenData,EncryptedTokenData) moved frompackages/adapter-slack/src/crypto.tsintopackages/adapter-shared/src/crypto.ts. Slack'scrypto.tsre-exports from shared, so the public surface (including thedecodeKeyandEncryptedTokenDatare-exports from@chat-adapter/slack) is unchanged.@chat-adapter/shared— token encryption helpersNew module:
packages/adapter-shared/src/crypto.ts. AES-256-GCM with a randomly generated 12-byte IV per encryption, exported from the package entry point so any adapter that needs to encrypt tokens can pull it in instead of re-implementing.examples/nextjs-chat— OAuth state on Slack and Linear install flowsBoth
/api/slack/installand/api/linear/installnow generate a 32-byte opaquestatevalue, stash it in an HttpOnly + SameSite=Lax cookie scoped to the matching callback path with a 10-minute TTL, and redirect to the provider with the value in the authorize URL. The corresponding callback routes read the state cookie, consume it (delete on first read so it can't be replayed), andcrypto.timingSafeEqual-compare against the?state=query parameter. Mismatch returns 400 beforehandleOAuthCallbackis called.Logic is shared via
examples/nextjs-chat/src/lib/oauth-state.ts(issueOAuthState/consumeAndVerifyOAuthState) so any future install flow can adopt the same pattern with two function calls. There is no library-side change here — this is example-app only — but the pattern is what we'd recommend for real deployments.In-flight install flows started against the old code will fail with
OAuth state mismatchafter this lands; users just need to restart the flow.Tests
@chat-adapter/slack: two new index.test.ts cases for the timing-safe compare — same-length / wrong-content and length-mismatch — both expect 401.@chat-adapter/github: two new self-message-detection tests covering multi-tenant first-webhook detection and theapps.getAuthenticatedfallback path. The existing "should handle auth failure gracefully" test was updated to reflect the new fallback chain (rejects bothusers.getAuthenticatedandapps.getAuthenticated) and the new warning string.@chat-adapter/linear: four new tests under atoken encryptiondescribe — ciphertext-on-disk verification, plaintext-when-no-key (legacy behavior preserved), legacy-plaintext-tolerance for zero-downtime key rotation in, and key-length validation.@chat-adapter/gchat: replaced two old "fail-open with warning" tests with new ones covering the constructor throw, thedisableSignatureVerificationopt-in, and the two cross-transport rejection cases (Pub/Sub-shaped to agoogleChatProjectNumber-only adapter, and the inverse). The existing test helpers and module-level env var have been updated so the rest of the file's 240+ tests continue to construct adapters without verification config.@chat-adapter/integration-tests: the three replay/test contexts that construct gchat adapters with mock credentials now passdisableSignatureVerification: true, since these tests inject pre-built event payloads directly and don't exercise the JWT path.pnpm validate(knip + check + typecheck + test + build) is green across all 16 tasks. A changeset is included.