Repository: cli/cli
Pull request #9610 (2024-09-12 16:22:43)

Disable auth check for `attestation trusted-root` command
The `gh attestation trusted-root` command currently uses an authentication check to ensure that a GitHub token is present. When running in GH Actions, you may see an error like the following:

```
gh: To use GitHub CLI in a GitHub Actions workflow, set the GH_TOKEN environment variable. Example:
  env:
    GH_TOKEN: ${{ github.token }}
Error: Process completed with exit code 4.
```

Since this command doesn't actually interact with any GH APIs, there is no reason to force a token to be present.

This change disables the auth check for this command.

Fixes https://github.com/cli/cli/issues/9614

cliAutomation: Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message.
williammartin: > Since this command doesn't actually interact with any GH APIs, there is no reason to force a token to be present.

Is this true for tenancy environments? Does it matter if it's not? @kommendorkapten
kommendorkapten: Yes you are correct @williammartin, when tenancy is enabled, an authenticated call to the `/meta` endpoint is made.
kommendorkapten: @bdehamer If you look at diff for `trusted_root` here: https://github.com/cli/cli/compare/bdehamer/disable-attestation-trusted-root-auth-check...wm-fs/error-on-not-authed

you'll see a check I think you may want to add so it fails with a nicer error message if tenancy is on.

If we are planning to do similar modifications to other attestation sub commands, we would need a similar check for that too.

Patch:
diff --git a/internal/config/config.go b/internal/config/config.go
index 29b66b73b..1b56d30b2 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -217,6 +217,12 @@ func (c *AuthConfig) ActiveToken(hostname string) (string, string) {
 	return token, source
 }
 
+// HasActiveToken returns true when a token for the hostname is present.
+func (c *AuthConfig) HasActiveToken(hostname string) bool {
+	token, _ := c.ActiveToken(hostname)
+	return token != ""
+}
+
 // HasEnvToken returns true when a token has been specified in an
 // environment variable, else returns false.
 func (c *AuthConfig) HasEnvToken() bool {
diff --git a/internal/gh/gh.go b/internal/gh/gh.go
index c39734075..e4431fdab 100644
--- a/internal/gh/gh.go
+++ b/internal/gh/gh.go
@@ -93,6 +93,9 @@ type Migration interface {
 // with knowledge on how to access encrypted storage when neccesarry.
 // Behavior is scoped to authentication specific tasks.
 type AuthConfig interface {
+	// HasActiveToken returns true when a token for the hostname is present.
+	HasActiveToken(hostname string) bool
+
 	// ActiveToken will retrieve the active auth token for the given hostname, searching environment variables,
 	// general configuration, and finally encrypted storage.
 	ActiveToken(hostname string) (token string, source string)


Test patch:
diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go
index ed000ff18..61245c650 100644
--- a/internal/config/auth_config_test.go
+++ b/internal/config/auth_config_test.go
@@ -52,8 +52,32 @@ func TestTokenFromKeyringForUserErrorsIfUsernameIsBlank(t *testing.T) {
 	require.ErrorContains(t, err, "username cannot be blank")
 }
 
+func TestHasActiveToken(t *testing.T) {
+	// Given the user has logged in for a host
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.Login("github.com", "test-user", "test-token", "", false)
+	require.NoError(t, err)
+
+	// When we check if that host has an active token
+	hasActiveToken := authCfg.HasActiveToken("github.com")
+
+	// Then there is an active token
+	require.True(t, hasActiveToken, "expected there to be an active token")
+}
+
+func TestHasNoActiveToken(t *testing.T) {
+	// Given there are no users logged in for a host
+	authCfg := newTestAuthConfig(t)
+
+	// When we check if any host has an active token
+	hasActiveToken := authCfg.HasActiveToken("github.com")
+
+	// Then there is no active token
+	require.False(t, hasActiveToken, "expected there to be no active token")
+}
+
 func TestTokenStoredInConfig(t *testing.T) {
-	// When the user has logged in insecurely
+	// Given the user has logged in insecurely
 	authCfg := newTestAuthConfig(t)
 	_, err := authCfg.Login("github.com", "test-user", "test-token", "", false)
 	require.NoError(t, err)
diff --git a/pkg/cmd/attestation/trustedroot/trustedroot.go b/pkg/cmd/attestation/trustedroot/trustedroot.go
index c9c3fdb04..6f741dcd4 100644
--- a/pkg/cmd/attestation/trustedroot/trustedroot.go
+++ b/pkg/cmd/attestation/trustedroot/trustedroot.go
@@ -69,6 +69,15 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com
 			}
 
 			if ghinstance.IsTenancy(opts.Hostname) {
+				c, err := f.Config()
+				if err != nil {
+					return err
+				}
+
+				if !c.Authentication().HasActiveToken(opts.Hostname) {
+					return fmt.Errorf("not authenticated with %s", opts.Hostname)
+				}
+
 				hc, err := f.HttpClient()
 				if err != nil {
 					return err
@@ -94,6 +103,7 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com
 		},
 	}
 
+	cmdutil.DisableAuthCheck(&trustedRootCmd)
 	trustedRootCmd.Flags().StringVarP(&opts.TufUrl, "tuf-url", "", "", "URL to the TUF repository mirror")
 	trustedRootCmd.Flags().StringVarP(&opts.TufRootPath, "tuf-root", "", "", "Path to the TUF root.json file on disk")
 	trustedRootCmd.MarkFlagsRequiredTogether("tuf-url", "tuf-root")
diff --git a/pkg/cmd/attestation/trustedroot/trustedroot_test.go b/pkg/cmd/attestation/trustedroot/trustedroot_test.go
index 70b5ae2a1..c4a259436 100644
--- a/pkg/cmd/attestation/trustedroot/trustedroot_test.go
+++ b/pkg/cmd/attestation/trustedroot/trustedroot_test.go
@@ -3,6 +3,7 @@ package trustedroot
 import (
 	"bytes"
 	"fmt"
+	"net/http"
 	"strings"
 	"testing"
 
@@ -10,8 +11,13 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/cli/cli/v2/internal/config"
+	"github.com/cli/cli/v2/internal/gh"
+	ghmock "github.com/cli/cli/v2/internal/gh/mock"
+	"github.com/cli/cli/v2/pkg/cmd/attestation/api"
 	"github.com/cli/cli/v2/pkg/cmd/attestation/test"
 	"github.com/cli/cli/v2/pkg/cmdutil"
+	"github.com/cli/cli/v2/pkg/httpmock"
 	"github.com/cli/cli/v2/pkg/iostreams"
 )
 
@@ -19,6 +25,9 @@ func TestNewTrustedRootCmd(t *testing.T) {
 	testIO, _, _, _ := iostreams.Test()
 	f := &cmdutil.Factory{
 		IOStreams: testIO,
+		Config: func() (gh.Config, error) {
+			return &ghmock.ConfigMock{}, nil
+		},
 	}
 
 	testcases := []struct {
@@ -72,6 +81,83 @@ func TestNewTrustedRootCmd(t *testing.T) {
 	}
 }
 
+func TestNewTrustedRootWithTenancy(t *testing.T) {
+	testIO, _, _, _ := iostreams.Test()
+	var testReg httpmock.Registry
+	var metaResp = api.MetaResponse{
+		Domains: api.Domain{
+			ArtifactAttestations: api.ArtifactAttestations{
+				TrustDomain: "foo",
+			},
+		},
+	}
+	testReg.Register(httpmock.REST(http.MethodGet, "meta"),
+		httpmock.StatusJSONResponse(200, &metaResp))
+
+	httpClientFunc := func() (*http.Client, error) {
+		reg := &testReg
+		client := &http.Client{}
+		httpmock.ReplaceTripper(client, reg)
+		return client, nil
+	}
+
+	cli := "--hostname foo-bar.ghe.com"
+
+	t.Run("Host with NO auth configured", func(t *testing.T) {
+		f := &cmdutil.Factory{
+			IOStreams: testIO,
+			Config: func() (gh.Config, error) {
+				return &ghmock.ConfigMock{
+					AuthenticationFunc: func() gh.AuthConfig {
+						return &stubAuthConfig{hasActiveToken: false}
+					},
+				}, nil
+			},
+		}
+
+		cmd := NewTrustedRootCmd(f, func(_ *Options) error {
+			return nil
+		})
+
+		argv := strings.Split(cli, " ")
+		cmd.SetArgs(argv)
+		cmd.SetIn(&bytes.Buffer{})
+		cmd.SetOut(&bytes.Buffer{})
+		cmd.SetErr(&bytes.Buffer{})
+		_, err := cmd.ExecuteC()
+
+		assert.Error(t, err)
+		assert.ErrorContains(t, err, "not authenticated")
+	})
+
+	t.Run("Host with auth configured", func(t *testing.T) {
+		f := &cmdutil.Factory{
+			IOStreams: testIO,
+			Config: func() (gh.Config, error) {
+				return &ghmock.ConfigMock{
+					AuthenticationFunc: func() gh.AuthConfig {
+						return &stubAuthConfig{hasActiveToken: true}
+					},
+				}, nil
+			},
+			HttpClient: httpClientFunc,
+		}
+
+		cmd := NewTrustedRootCmd(f, func(_ *Options) error {
+			return nil
+		})
+
+		argv := strings.Split(cli, " ")
+		cmd.SetArgs(argv)
+		cmd.SetIn(&bytes.Buffer{})
+		cmd.SetOut(&bytes.Buffer{})
+		cmd.SetErr(&bytes.Buffer{})
+
+		_, err := cmd.ExecuteC()
+		assert.NoError(t, err)
+	})
+}
+
 var newTUFErrClient tufClientInstantiator = func(o *tuf.Options) (*tuf.Client, error) {
 	return nil, fmt.Errorf("failed to create TUF client")
 }
@@ -99,3 +185,14 @@ func TestGetTrustedRoot(t *testing.T) {
 	})
 
 }
+
+type stubAuthConfig struct {
+	config.AuthConfig
+	hasActiveToken bool
+}
+
+var _ gh.AuthConfig = (*stubAuthConfig)(nil)
+
+func (c *stubAuthConfig) HasActiveToken(host string) bool {
+	return c.hasActiveToken
+}