fix: properly check write access for fine-grained PATs (#4)

jwm4 · web-flow · commit e4ba4c0a27d3 · 2026-01-02T16:17:35.000-05:00
* fix: update repo references from jwm4 to ambient-code

Update remaining references to the old jwm4 repo:
- review_roadmap/main.py: footer link in generated roadmaps
- tests/test_main.py: test assertion for footer URL
- ADR/phase4_plan.md: pip install example URL

* fix: properly check write access for fine-grained PATs

The write access check was incorrectly passing for tokens that lacked
write permissions because it only checked the repository permissions
object, which reflects user access rather than token-specific access.

Changes:
- Add WriteAccessStatus enum and WriteAccessResult model for detailed
  access check results (GRANTED, DENIED, UNCERTAIN)
- For classic PATs: verify OAuth scopes via X-OAuth-Scopes header
- For fine-grained PATs: perform live write test using reactions
  (create and immediately delete an 'eyes' reaction on the PR)
- Improve error messages with troubleshooting guidance
- Add comprehensive GitHub token setup documentation to README

The live write test allows definitive verification for fine-grained PATs,
which cannot be checked via headers since they don't include scope info
and the permissions object reflects user access, not token configuration.
diff --git a/README.md b/README.md
@@ -37,7 +37,7 @@ cp env.example .env
 
 | Variable | Description |
 |----------|-------------|
-| `GITHUB_TOKEN` | GitHub personal access token for API access |
+| `GITHUB_TOKEN` | GitHub personal access token for API access ([see below](#github-token-setup)) |
 | `ANTHROPIC_API_KEY` | Anthropic API key (required if provider is `anthropic`) |
 | `OPENAI_API_KEY` | OpenAI API key (required if provider is `openai`) |
 | `GOOGLE_API_KEY` | Google API key (required if provider is `google`) |
@@ -50,6 +50,56 @@ cp env.example .env
 
 > **Note**: The `.env` file values are overridden by any matching environment variables in your shell.  If you do not want to have a .env file, you can just skip these steps and set these variables in your environment instead.
 
+### GitHub Token Setup
+
+A GitHub personal access token (PAT) is required to fetch PR data. The type of token and permissions needed depend on your use case:
+
+#### Read-Only Access (default)
+
+For generating roadmaps without posting comments, a token with **read access to repositories** is sufficient.
+
+#### Write Access (for `--post` flag)
+
+To post roadmaps as PR comments using the `--post` flag, your token needs **write access**. The specific requirements differ by token type:
+
+| Token Type | Required Permissions |
+|------------|---------------------|
+| **Fine-grained PAT** | Repository access → Pull requests: **Read and write** |
+| **Classic PAT** | `repo` scope (full repo access) or `public_repo` (public repos only) |
+
+#### Creating a Fine-Grained Personal Access Token (Recommended)
+
+Fine-grained tokens are more secure because they limit access to specific repositories.
+
+1. Go to [GitHub Settings → Developer settings → Personal access tokens → Fine-grained tokens](https://github.com/settings/tokens?type=beta)
+2. Click **"Generate new token"**
+3. Configure the token:
+   - **Token name**: e.g., `review-roadmap`
+   - **Expiration**: Choose an appropriate duration
+   - **Repository access**: Select "Only select repositories" and choose the repos you want to analyze
+   - **Permissions**:
+     - **Pull requests**: Read-only (for basic usage) or Read and write (for `--post`)
+     - **Contents**: Read-only (required to fetch file contents for context expansion)
+4. Click **"Generate token"** and copy the token value
+
+#### Creating a Classic Personal Access Token
+
+Classic tokens are simpler but grant broader access.
+
+1. Go to [GitHub Settings → Developer settings → Personal access tokens → Tokens (classic)](https://github.com/settings/tokens)
+2. Click **"Generate new token"** → **"Generate new token (classic)"**
+3. Configure the token:
+   - **Note**: e.g., `review-roadmap`
+   - **Expiration**: Choose an appropriate duration
+   - **Scopes**:
+     - `repo` — Full access (works for both public and private repos)
+     - Or `public_repo` — Only for public repositories
+4. Click **"Generate token"** and copy the token value
+
+#### Important: Token Scopes vs. User Permissions
+
+Even if you have collaborator access to a repository, your token must also have the correct **OAuth scopes** to perform write operations. A common mistake is using a token with read-only scopes while expecting write access to work based on your user permissions. The tool checks both your repository permissions and your token's scopes before attempting to post comments.
+
 ## Usage
 
 Run the tool using the CLI:
@@ -69,7 +119,7 @@ review_roadmap {PR link in the form owner/repo/pr_number or just a URL to the PR
 
 You can use both `-o` and `-p` together—the roadmap will be generated once and saved to both the file and the PR comment.
 
-> **Note:** To use `--post`, your `GITHUB_TOKEN` must have write access to the repository. For fine-grained personal access tokens, ensure the **"Pull requests"** permission is set to **"Read and write"**. For classic tokens, the `repo` scope is required.
+> **Note:** The `--post` flag requires a token with write access. See [GitHub Token Setup](#github-token-setup) for details.
 
 ### Examples
 
diff --git a/review_roadmap/github/client.py b/review_roadmap/github/client.py
@@ -10,7 +10,10 @@
 import httpx
 
 from review_roadmap.config import settings
-from review_roadmap.models import PRContext, PRMetadata, FileDiff, PRComment
+from review_roadmap.models import (
+    PRContext, PRMetadata, FileDiff, PRComment,
+    WriteAccessResult, WriteAccessStatus,
+)
 
 
 class GitHubClient:
@@ -220,17 +223,60 @@ def get_file_content(self, owner: str, repo: str, path: str, ref: str) -> str:
         raw_resp.raise_for_status()
         return raw_resp.text
 
-    def check_write_access(self, owner: str, repo: str) -> bool:
-        """Check if the authenticated user has write access to the repository.
+    def _test_write_with_reaction(self, owner: str, repo: str, pr_number: int) -> bool:
+        """Test write access by creating and immediately deleting a reaction.
 
-        Used to validate permissions before attempting to post a PR comment.
+        This is a non-destructive way to verify the token can write to the PR.
+        Uses the 'eyes' emoji (👀) as it's unobtrusive if cleanup fails.
 
         Args:
             owner: Repository owner.
             repo: Repository name.
+            pr_number: PR number to test against.
 
         Returns:
-            True if the user has push or admin access, False otherwise.
+            True if write access is confirmed, False otherwise.
+        """
+        try:
+            # Create a reaction on the PR (PRs are issues in GitHub's API)
+            create_resp = self.client.post(
+                f"/repos/{owner}/{repo}/issues/{pr_number}/reactions",
+                json={"content": "eyes"}
+            )
+            
+            if create_resp.status_code in (200, 201):
+                # Successfully created - now clean it up
+                reaction_id = create_resp.json().get("id")
+                if reaction_id:
+                    self.client.delete(
+                        f"/repos/{owner}/{repo}/issues/{pr_number}/reactions/{reaction_id}"
+                    )
+                return True
+            elif create_resp.status_code == 403:
+                return False
+            else:
+                # Unexpected status - treat as uncertain
+                return False
+        except Exception:
+            return False
+
+    def check_write_access(
+        self, owner: str, repo: str, pr_number: Optional[int] = None
+    ) -> WriteAccessResult:
+        """Check if the authenticated token can write to the repository.
+
+        Validates:
+        1. The user has push/admin permissions on the repository
+        2. The token has the required OAuth scopes (for classic PATs/OAuth tokens)
+        3. For fine-grained PATs: performs a live test using reactions if pr_number provided
+
+        Args:
+            owner: Repository owner.
+            repo: Repository name.
+            pr_number: Optional PR number for live write test (used for fine-grained PATs).
+
+        Returns:
+            WriteAccessResult with status, confidence level, and explanation.
 
         Raises:
             httpx.HTTPStatusError: If the repository request fails.
@@ -239,8 +285,75 @@ def check_write_access(self, owner: str, repo: str) -> bool:
         resp.raise_for_status()
         repo_data = resp.json()
         
+        # Check user's role-based permissions
         permissions = repo_data.get("permissions", {})
-        return permissions.get("push", False) or permissions.get("admin", False)
+        has_role_access = permissions.get("push", False) or permissions.get("admin", False)
+        
+        if not has_role_access:
+            return WriteAccessResult(
+                status=WriteAccessStatus.DENIED,
+                is_fine_grained_pat=False,
+                message="Your user account does not have write access to this repository."
+            )
+        
+        # Check token's OAuth scopes (only present for classic PATs and OAuth tokens)
+        oauth_scopes_header = resp.headers.get("X-OAuth-Scopes")
+        
+        if oauth_scopes_header is not None:
+            # Classic PAT or OAuth token - can verify scopes definitively
+            scopes = [s.strip() for s in oauth_scopes_header.split(",") if s.strip()]
+            is_private = repo_data.get("private", False)
+            
+            if is_private:
+                has_token_scope = "repo" in scopes
+                required_scope = "repo"
+            else:
+                has_token_scope = "repo" in scopes or "public_repo" in scopes
+                required_scope = "repo or public_repo"
+            
+            if has_token_scope:
+                return WriteAccessResult(
+                    status=WriteAccessStatus.GRANTED,
+                    is_fine_grained_pat=False,
+                    message="Classic token with correct scopes verified."
+                )
+            else:
+                return WriteAccessResult(
+                    status=WriteAccessStatus.DENIED,
+                    is_fine_grained_pat=False,
+                    message=f"Token lacks required scope ({required_scope}). Current scopes: {', '.join(scopes) or 'none'}"
+                )
+        
+        # No X-OAuth-Scopes header = fine-grained PAT
+        # Try a live write test if we have a PR number
+        if pr_number is not None:
+            if self._test_write_with_reaction(owner, repo, pr_number):
+                return WriteAccessResult(
+                    status=WriteAccessStatus.GRANTED,
+                    is_fine_grained_pat=True,
+                    message="Fine-grained PAT write access verified via live test."
+                )
+            else:
+                return WriteAccessResult(
+                    status=WriteAccessStatus.DENIED,
+                    is_fine_grained_pat=True,
+                    message=(
+                        "Fine-grained PAT detected but write test failed. "
+                        f"Ensure your token has 'Pull requests: Read and write' permission "
+                        f"for {owner}/{repo}."
+                    )
+                )
+        
+        # No PR number provided - can't do live test
+        return WriteAccessResult(
+            status=WriteAccessStatus.UNCERTAIN,
+            is_fine_grained_pat=True,
+            message=(
+                "Fine-grained PAT detected. Cannot verify if this token is configured "
+                f"for {owner}/{repo}. If posting fails, ensure your token has "
+                "'Pull requests: Read and write' permission for this specific repository."
+            )
+        )
 
     def post_pr_comment(self, owner: str, repo: str, pr_number: int, body: str) -> Dict[str, Any]:
         """Post a comment on a pull request.
diff --git a/review_roadmap/main.py b/review_roadmap/main.py
@@ -5,6 +5,7 @@
 from review_roadmap.agent.graph import build_graph
 from review_roadmap.config import settings
 from review_roadmap.logging import configure_logging
+from review_roadmap.models import WriteAccessStatus
 
 app = typer.Typer(add_completion=False)
 console = Console()
@@ -61,14 +62,20 @@ def generate(
     if post:
         console.print(f"[bold blue]Checking write access for {owner}/{repo}...[/bold blue]")
         try:
-            has_access = gh_client.check_write_access(owner, repo)
-            if not has_access:
+            # Pass pr_number to enable live write test for fine-grained PATs
+            access_result = gh_client.check_write_access(owner, repo, pr_number)
+            
+            if access_result.status == WriteAccessStatus.DENIED:
                 console.print(
-                    "[red]Error: Your GitHub token does not have write access to this repository.[/red]\n"
+                    f"[red]Error: {access_result.message}[/red]\n"
                     "[yellow]To use --post, your token needs 'Pull requests: Read and write' permission.[/yellow]"
                 )
                 raise typer.Exit(code=1)
-            console.print("[green]Write access confirmed.[/green]")
+            elif access_result.status == WriteAccessStatus.UNCERTAIN:
+                console.print(f"[yellow]Warning: {access_result.message}[/yellow]")
+                console.print("[yellow]Proceeding, but posting may fail...[/yellow]")
+            else:
+                console.print("[green]Write access confirmed.[/green]")
         except typer.Exit:
             raise
         except Exception as e:
@@ -108,6 +115,12 @@ def generate(
             console.print(f"[bold green]Roadmap posted to PR #{pr_number}[/bold green]")
         except Exception as e:
             console.print(f"[red]Error posting comment to PR: {e}[/red]")
+            if "403" in str(e):
+                console.print(
+                    "\n[yellow]This usually means your token lacks write access to this repository.[/yellow]\n"
+                    "[yellow]If using a fine-grained PAT, ensure it is configured for this specific repository[/yellow]\n"
+                    f"[yellow]with 'Pull requests: Read and write' permission for {owner}/{repo}.[/yellow]"
+                )
             raise typer.Exit(code=1)
 
     # If neither output nor post, print to console
diff --git a/review_roadmap/models.py b/review_roadmap/models.py
@@ -4,10 +4,38 @@
 fetched from GitHub's API, including PR metadata, file diffs, and comments.
 """
 
+from enum import Enum
 from typing import List, Optional
 from pydantic import BaseModel, Field
 
 
+class WriteAccessStatus(str, Enum):
+    """Result of checking write access to a repository."""
+    
+    GRANTED = "granted"
+    """Write access confirmed with high confidence (classic PAT with correct scopes)."""
+    
+    DENIED = "denied"
+    """Write access definitely not available (missing permissions or scopes)."""
+    
+    UNCERTAIN = "uncertain"
+    """Cannot reliably determine access (fine-grained PAT - may or may not work)."""
+
+
+class WriteAccessResult(BaseModel):
+    """Result of a write access check.
+    
+    Attributes:
+        status: The access status (granted, denied, or uncertain).
+        is_fine_grained_pat: True if the token appears to be a fine-grained PAT.
+        message: Human-readable explanation of the result.
+    """
+    
+    status: WriteAccessStatus
+    is_fine_grained_pat: bool = False
+    message: str = ""
+
+
 class FileDiff(BaseModel):
     """Represents a single file change in a Pull Request.
 
diff --git a/tests/test_github_client.py b/tests/test_github_client.py
diff --git a/tests/test_main.py b/tests/test_main.py
