[Regression on closed #293/#127] GithubStrategy still requests the `read:org` OAuth scope, which nothing in the codebase reads

オープン 初心者向け
  • 何か問題がありますか？
#744 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
2/5
見積もり時間
1〜3時間
初心者へのやさしさ
74/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
github, typescript
領域
authentication, backend-api-design, security

調査の方向性

src/auth/strategies/github.strategy.ts の GithubStrategy コンストラクターから始め、戦略オプションのテスト方法を src/auth/strategies/github.strategy.spec.ts で確認してください。現在要求されているスコープを確認し、説明されている回帰アサーションを追加してください。完了条件は、戦略が user:email のみを要求し、テストがそのスコープ配列と完全に一致することを検証することです。

索引モデルが issue の本文から書いたものです。

説明

bug security

Location

src/auth/strategies/github.strategy.ts:22 — GithubStrategy constructor

scope: ['user:email', 'read:org'],

Problem

The OAuth consent request still asks GitHub for read:org in addition to user:email. Nothing in the application consumes organization data from GitHub:

  • GithubStrategy.validate (lines 26-42) reads only profile.id, profile.username, profile.emails[0], profile.photos[0].
  • GithubSyncService calls exactly two Octokit endpoints — octokit.repos.get (line 130) and octokit.issues.listForRepo (line 217) — plus an unused octokit.pulls.get. Neither needs read:org, and both run under the platform's own GITHUB_API_TOKEN (octokit.provider.ts:50), not the user's OAuth token.
  • grep -rn 'read:org\|\.orgs\.' src/ finds the scope declaration and nothing else. The only other orgs hits are ReputationService's orgsContributedTo, which is derived from local bounty/repository rows, not from GitHub's organization API.

The user's accessToken is stored on GithubAccount (encrypted at rest) but is never used to make an API call anywhere in the codebase.

This is closed issue #127, re-filed and closed again as #293. The scope is still present.

Why it matters

read:org is not a trivial scope. It grants read access to the user's organization memberships (including private memberships that the user has deliberately hidden from their public profile), the organization's teams and team memberships, and its projects. For a contributor who belongs to a private organization at their employer, consenting to MergeFi discloses that membership and that org's team structure to this platform.

Two concrete consequences:

  1. Consent-screen friction and trust. GitHub's authorization page lists the requested permissions verbatim. A bounty platform asking to "Read org and team membership, read org projects" for no stated reason is a reasonable cause for a contributor — or an organization admin reviewing third-party app access — to decline. Organizations can and do block OAuth apps that request org scopes.
  2. Unnecessary blast radius. The token is persisted in github_accounts.accessToken. Should that column ever be read out (a misconfigured ENCRYPTION_KEY, a backup leak, a SQL-injection path), the stolen token carries org-read capability it never needed. Least privilege here is free: no feature depends on it.

Reproduction / failure path

  1. Configure real OAuth credentials and visit GET /api/auth/github as a user who is a private member of at least one GitHub organization.
  2. Observe GitHub's consent screen: alongside "Email addresses (read-only)" it lists organization and team membership read access.
  3. Grant access, then inspect the granted scopes for the app under GitHub -> Settings -> Applications -> Authorized OAuth Apps. read:org is held.
  4. Grep the codebase for any consumer of that capability — there is none. The scope is granted and never exercised.

Suggested fix

Drop the scope:

scope: ['user:email'],

user:email alone is sufficient for everything validate reads: profile.id, profile.username, profile.displayName, profile.profileUrl and profile.photos all come from the public user resource, and user:email is only needed for the private email address.

Add a regression test to src/auth/strategies/github.strategy.spec.ts that asserts the exact scope array passed to the Passport strategy, so a future change has to state its intent:

it('requests only the user:email scope', () => {
  // assert on the options object handed to super()
  expect(strategyOptions.scope).toEqual(['user:email']);
});

That test is what both previous fixes were missing — the scope has now been removed and reintroduced twice without anything failing.

If organization data is wanted later (for example, to verify a sponsor's org affiliation), add the scope back in the same change that adds the code reading it, and document in the README what it is used for so the consent screen can be explained to users.

主要言語
TypeScript
スター
2
フォーク
90
平均マージ
31分
マージ済み PR（30日）
20

環境構築

  • Dockerfile または Docker Compose ファイルあり
  • プルリクエストのテンプレートなし
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

MergeFi/backend のほかの issue

MergeFi/backend の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。