Repository navigation
(janitor/dedupe): consolidate OAuth callback state hash and code validation - #7968
Merged
Merged
Conversation
…dation Four OAuth callbacks (gitlab, bitbucket, google, github user-connect) each computed an 8-char SHA-256 truncation of the signed OAuth state for safe Sentry logging, and three of them re-implemented the same RFC 6749 authorization-code validation. Consolidate both into one owner so a change to the observability hash or code alphabet applies everywhere.
Contributor
Author
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryBehavior-preserving dedupe of OAuth callback helpers across 5 files; verified equivalence of the state-hash and authorization-code validation logic at every call site. Files Reviewed (5 files)
Reviewed by deepseek-v4.1-flash · Input: 49.1K · Output: 5.9K · Cached: 321.4K Review guidance: REVIEW.md from base branch |
Contributor
Author
|
Meta Janitor: Safety review passed; requesting approval. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Meta-janitor safety approval for 214e0d8. Signal: #7968 (comment). GitHub auto-merge remains responsible for merging.
This was referenced Oct 19, 2026
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.
What
Four OAuth callback handlers duplicated two small, security-relevant helpers:
createHash('sha256').update(state).digest('hex').slice(0, 8)(the raw signed state embeds the owner, user id and nonce, so it must never be logged). Copied ingitlab-callback.ts,bitbucket-callback.ts,google/callback/route.ts, andgithub/user-connect/callback/route.ts.validOAuthCode(bitbucket, github) andsanitizeOAuthCode(google).This PR extracts both into a single owner,
apps/web/src/lib/integrations/oauth/callback-safety.ts, and rewires the four callers. No behavior change: the hash formula and the code-validation regex/length are byte-for-byte identical to before.Classification
Shared behavior (state hash) + shared data pipeline (code validation): one domain concept declared in multiple files that must change together.
Notes / adjacent findings (not changed)
provider_oauth_attempts.tskeeps its own full SHA-256 of the state for DB equality matching. That is intentionally different from the truncated observability hash, so it is left alone (documented in the new module).discord,dolthub,linear, andslackcallbacks still log the rawstatevalue to Sentry in their catch blocks. That is a potential leak of the signed state token and worth a separate security follow-up; it is out of scope for this dedupe PR.Verification
pnpm --filter web run typecheck— passes (exit 0).oxlinton the five touched files — 0 warnings, 0 errors.oxfmton the five touched files — clean.cleanupDbForTestinweb-shared/src/lib/drizzle.ts); no Postgres/Redis is available here.