Skip to content

Adopt isPermissionError helper for gh CLI auth-error detection#32758

Merged
pelikhan merged 2 commits into
mainfrom
copilot/sergo-adopt-ispermissionerror-helper
May 17, 2026
Merged

Adopt isPermissionError helper for gh CLI auth-error detection#32758
pelikhan merged 2 commits into
mainfrom
copilot/sergo-adopt-ispermissionerror-helper

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

Four call sites in pkg/cli were reimplementing auth-error substring checks inline instead of using the existing isPermissionError helper in audit.go, causing the checked substring sets to drift independently across files.

Changes

pkg/cli/audit.go

  • Splits isPermissionError into an internal isPermissionErrorStr(s string) bool that operates on raw strings (enabling callers that check combined error+output messages) and a thin isPermissionError(err error) bool wrapper
  • Widens the canonical substring set to the union of all caller needs, adding "not logged into any GitHub hosts", "To use GitHub CLI in a GitHub Actions workflow", and "gh auth login" (previously only checked inline in logs_github_api.go)
  • Adds is403Error(err error) bool for 403 HTTP status checks

Call site consolidation

  • logs_download.go (×2): strings.Contains(err.Error(), "exit status 4")isPermissionError(err)
  • logs_github_api.go: five-clause inline auth check → isPermissionErrorStr(combinedMsg) (preserves combined error+output matching)
  • secrets.go, mcp_secrets.go: strings.Contains(err.Error(), "403")is403Error(err)
  • secrets.go: fixes inverted 403 guard logic (!is403Erroris403Error) to match the comment's stated intent

Before / After (logs_download.go)

// Before
if strings.Contains(err.Error(), "exit status 4") {
    return errors.New("GitHub CLI authentication required. Run 'gh auth login' first")
}

// After
if isPermissionError(err) {
    return errors.New("GitHub CLI authentication required. Run 'gh auth login' first")
}

Tests

  • Extends TestIsPermissionError with the three new substrings
  • Adds TestIsPermissionErrorStr covering the string-based helper
  • Adds TestIs403Error

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Adopt isPermissionError helper for gh CLI auth-error detection Adopt isPermissionError helper for gh CLI auth-error detection May 17, 2026
Copilot AI requested a review from pelikhan May 17, 2026 05:29
@pelikhan pelikhan marked this pull request as ready for review May 17, 2026 05:47
Copilot AI review requested due to automatic review settings May 17, 2026 05:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR centralizes GitHub CLI authentication and permission-error detection helpers across several pkg/cli call sites.

Changes:

  • Extracts string-based permission detection and adds a shared 403 helper.
  • Replaces inline auth/403 substring checks in logs and secrets flows.
  • Extends helper unit tests for additional auth and 403 cases.
Show a summary per file
File Description
pkg/cli/audit.go Adds shared string/error permission helpers and 403 detection.
pkg/cli/audit_test.go Adds coverage for new helper behavior.
pkg/cli/logs_github_api.go Uses the shared auth helper for workflow-run listing failures.
pkg/cli/logs_download.go Uses the shared auth helper for log/artifact download failures.
pkg/cli/mcp_secrets.go Uses the shared 403 helper for skipped secret checks.
pkg/cli/secrets.go Uses the shared 403 helper in secret availability checks.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 6/6 changed files
  • Comments generated: 3

strings.Contains(combinedMsg, "To use GitHub CLI in a GitHub Actions workflow") ||
strings.Contains(combinedMsg, "authentication required") ||
strings.Contains(outputMsg, "gh auth login") {
if isPermissionErrorStr(combinedMsg) {
Comment thread pkg/cli/secrets.go
Comment on lines +112 to 114
// If we get a 403 error, skip silently (no permission to check)
if is403Error(err) {
continue
Comment thread pkg/cli/audit_test.go
},
{
name: "GitHub Actions workflow auth error",
err: errors.New("To use GitHub CLI in a GitHub Actions workflow, set the GH_TOKEN environment variable"),
@pelikhan pelikhan merged commit 6de6dc2 into main May 17, 2026
4 checks passed
@pelikhan pelikhan deleted the copilot/sergo-adopt-ispermissionerror-helper branch May 17, 2026 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adopt isPermissionError helper for gh CLI auth-error detection (sergo)

3 participants