fix(backend): Handle different GitHub token types in OAuth scope check#850
fix(backend): Handle different GitHub token types in OAuth scope check#850brendan-kellam merged 4 commits intomainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
WalkthroughThe PR adds GitHub token-type detection and conditional scope introspection to address permission syncing failures with app user tokens. It introduces token classification utilities, modifies scope retrieval to support optional introspection, adds repository listing capabilities, and implements error handling for missing permissions with re-authorization guidance. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
1 similar comment
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Line 13: Update the CHANGELOG entry line that reads "account driven permission
syncing" to use the hyphenated adjective "account-driven permission syncing";
locate the exact string in the CHANGELOG.md entry (the line beginning "- [EE]
Fixed issue where account driven permission syncing...") and replace "account
driven" with "account-driven".
🧹 Nitpick comments (1)
packages/backend/src/ee/accountPermissionSyncer.ts (1)
194-208: Consider extracting the HTTP error check into a shared utility.The error status checking pattern here duplicates logic from
isHttpErroringithub.ts(lines 80-85). Consider exporting that helper function and reusing it here for consistency.♻️ Optional refactor
In
packages/backend/src/github.ts, export the helper:-const isHttpError = (error: unknown, status: number): boolean => { +export const isHttpError = (error: unknown, status: number): boolean => {Then in this file:
import { createOctokitFromToken, getOAuthScopesForAuthenticatedUser as getGitHubOAuthScopesForAuthenticatedUser, getReposForAuthenticatedUser, + isHttpError, } from "../github.js";} catch (error) { - if (error && typeof error === 'object' && 'status' in error) { - const status = (error as { status: number }).status; - if (status === 401 || status === 403) { - throw new Error( - `GitHub API returned ${status} error. Your token may have expired or lacks the required permissions. ` + - `Please re-authorize with GitHub to grant the necessary access.` - ); - } + if (isHttpError(error, 401) || isHttpError(error, 403)) { + const status = (error as { status: number }).status; + throw new Error( + `GitHub API returned ${status} error. Your token may have expired or lacks the required permissions. ` + + `Please re-authorize with GitHub to grant the necessary access.` + ); } throw error; }
Summary
ghp_,gho_,ghu_,ghs_,github_pat_)getOAuthScopesForAuthenticatedUserto returnnullfor token types that don't support scope introspectionToken Type Support
x-oauth-scopesghp_gho_ghu_ghs_github_pat_Test plan
detectGitHubTokenTypeandsupportsOAuthScopeIntrospectiongho_) - should validate scopes via header🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features