Ensure NOT_FOUND error when querying private repos using insufficient scope

mislav · mislav · commit 2c3f02ee62fe · 2021-10-14T17:31:21.000+02:00
diff --git a/api/queries_repo.go b/api/queries_repo.go
@@ -241,12 +241,23 @@ func FetchRepository(client *Client, repo ghrepo.Interface, fields []string) (*R
 	}
 
 	var result struct {
-		Repository Repository
+		Repository *Repository
 	}
 	if err := client.GraphQL(repo.RepoHost(), query, variables, &result); err != nil {
 		return nil, err
 	}
-	return InitRepoHostname(&result.Repository, repo.RepoHost()), nil
+	// The GraphQL API should have returned an error in case of a missing repository, but this isn't
+	// guaranteed to happen when an authentication token with insufficient permissions is being used.
+	if result.Repository == nil {
+		return nil, GraphQLErrorResponse{
+			Errors: []GraphQLError{{
+				Type:    "NOT_FOUND",
+				Message: fmt.Sprintf("Could not resolve to a Repository with the name '%s/%s'.", repo.RepoOwner(), repo.RepoName()),
+			}},
+		}
+	}
+
+	return InitRepoHostname(result.Repository, repo.RepoHost()), nil
 }
 
 func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
@@ -280,16 +291,24 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
 		"name":  repo.RepoName(),
 	}
 
-	result := struct {
-		Repository Repository
-	}{}
-	err := client.GraphQL(repo.RepoHost(), query, variables, &result)
-
-	if err != nil {
+	var result struct {
+		Repository *Repository
+	}
+	if err := client.GraphQL(repo.RepoHost(), query, variables, &result); err != nil {
 		return nil, err
 	}
+	// The GraphQL API should have returned an error in case of a missing repository, but this isn't
+	// guaranteed to happen when an authentication token with insufficient permissions is being used.
+	if result.Repository == nil {
+		return nil, GraphQLErrorResponse{
+			Errors: []GraphQLError{{
+				Type:    "NOT_FOUND",
+				Message: fmt.Sprintf("Could not resolve to a Repository with the name '%s/%s'.", repo.RepoOwner(), repo.RepoName()),
+			}},
+		}
+	}
 
-	return InitRepoHostname(&result.Repository, repo.RepoHost()), nil
+	return InitRepoHostname(result.Repository, repo.RepoHost()), nil
 }
 
 func RepoDefaultBranch(client *Client, repo ghrepo.Interface) (string, error) {
diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go
@@ -10,6 +10,27 @@ import (
 	"github.com/cli/cli/v2/pkg/httpmock"
 )
 
+func TestGitHubRepo_notFound(t *testing.T) {
+	httpReg := &httpmock.Registry{}
+	defer httpReg.Verify(t)
+
+	httpReg.Register(
+		httpmock.GraphQL(`query RepositoryInfo\b`),
+		httpmock.StringResponse(`{ "data": { "repository": null } }`))
+
+	client := NewClient(ReplaceTripper(httpReg))
+	repo, err := GitHubRepo(client, ghrepo.New("OWNER", "REPO"))
+	if err == nil {
+		t.Fatal("GitHubRepo did not return an error")
+	}
+	if wants := "GraphQL error: Could not resolve to a Repository with the name 'OWNER/REPO'."; err.Error() != wants {
+		t.Errorf("GitHubRepo error: want %q, got %q", wants, err.Error())
+	}
+	if repo != nil {
+		t.Errorf("GitHubRepo: expected nil repo, got %v", repo)
+	}
+}
+
 func Test_RepoMetadata(t *testing.T) {
 	http := &httpmock.Registry{}
 	client := NewClient(ReplaceTripper(http))
