From 3573f61b863e3e3110f87b8addb1db3ff63a071d Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 27 Aug 2025 12:10:06 -0600 Subject: [PATCH 1/4] Add agent-task command with OAuth token validation Introduces a new `agent-task` command under pkg/cmd/agent with strict OAuth (device flow) token validation. Includes comprehensive tests for token source and host validation, and registers the command in the root command set. --- pkg/cmd/agent-task/agent_task.go | 62 ++++++++++ pkg/cmd/agent-task/agent_task_test.go | 172 ++++++++++++++++++++++++++ pkg/cmd/root/root.go | 2 + 3 files changed, 236 insertions(+) create mode 100644 pkg/cmd/agent-task/agent_task.go create mode 100644 pkg/cmd/agent-task/agent_task_test.go diff --git a/pkg/cmd/agent-task/agent_task.go b/pkg/cmd/agent-task/agent_task.go new file mode 100644 index 000000000..1e916d034 --- /dev/null +++ b/pkg/cmd/agent-task/agent_task.go @@ -0,0 +1,62 @@ +package agent + +import ( + "errors" + "fmt" + "strings" + + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/go-gh/v2/pkg/auth" + "github.com/spf13/cobra" +) + +// NewCmdAgentTask creates the base `agent-task` command. +func NewCmdAgentTask(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "agent-task", + Aliases: []string{"agent-tasks", "agent", "agents"}, + Short: "Manage agent tasks (preview)", + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + return requireOAuthToken(f) + }, + // This is required to run this root command. We want to + // run it to test PersistentPreRunE behavior. + RunE: func(cmd *cobra.Command, args []string) error { + return nil + }, + } + return cmd +} + +// requireOAuthToken ensures an OAuth (device flow) token is present and valid. +// agent-task subcommands inherit this check via PersistentPreRunE. +func requireOAuthToken(f *cmdutil.Factory) error { + cfg, err := f.Config() + if err != nil { + return err + } + + authCfg := cfg.Authentication() + host, _ := authCfg.DefaultHost() + if host == "" { + return errors.New("no default host configured; run 'gh auth login'") + } + + if auth.IsEnterprise(host) { + return errors.New("agent tasks are not supported on this host") + } + + token, source := authCfg.ActiveToken(host) + + // Tokens from sources "oauth_token" and "keyring" are likely + // minted through our device flow. + tokenSourceIsDeviceFlow := source == "oauth_token" || source == "keyring" + // Tokens with "gho_" prefix are OAuth tokens. + tokenIsOAuth := strings.HasPrefix(token, "gho_") + + // Reject if the token is not from a device flow source or is not an OAuth token + if !tokenSourceIsDeviceFlow || !tokenIsOAuth { + return fmt.Errorf("this command requires an OAuth token. Re-authenticate with: gh auth login") + } + return nil +} diff --git a/pkg/cmd/agent-task/agent_task_test.go b/pkg/cmd/agent-task/agent_task_test.go new file mode 100644 index 000000000..76b2651f1 --- /dev/null +++ b/pkg/cmd/agent-task/agent_task_test.go @@ -0,0 +1,172 @@ +package agent + +import ( + "testing" + + "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/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/require" +) + +// setupMockOAuthConfig configures a blank config with a default host and optional token behavior. +func setupMockOAuthConfig(t *testing.T, tokenSource string) gh.Config { + t.Helper() + c := config.NewBlankConfig() + switch tokenSource { + case "oauth_token": + // valid OAuth device flow token stored in config + c.Set("github.com", "oauth_token", "gho_OAUTH123") + case "keyring": + // valid OAuth device flow token stored in keyring + c.Set("github.com", "oauth_token", "gho_OAUTH123") + case "GH_TOKEN": + // classic style token stored in config (will fail prefix check) + c.Set("github.com", "oauth_token", "ghp_CLASSIC123") + case "GH_ENTERPRISE_TOKEN": + // enterprise style token stored in config (will fail prefix check) + c.Set("something.ghes.com", "oauth_token", "ghe_ENTERPRISE123") + } + return c +} + +func TestOAuthTokenAccepted(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, stdout, _ := iostreams.Test() + f.IOStreams = ios + f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, "oauth_token"), nil } + + cmd := NewCmdAgentTask(f) + err := cmd.Execute() + require.NoError(t, err) + require.Equal(t, "", stdout.String()) +} + +func TestKeyringOAuthTokenAccepted(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, stdout, _ := iostreams.Test() + f.IOStreams = ios + f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, "keyring"), nil } + + cmd := NewCmdAgentTask(f) + err := cmd.Execute() + require.NoError(t, err) + require.Equal(t, "", stdout.String()) +} + +func TestEnvVarTokenRejected(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, "GH_TOKEN"), nil } + cmd := NewCmdAgentTask(f) + err := cmd.Execute() + require.Error(t, err) + require.Contains(t, err.Error(), "requires an OAuth token") +} + +func TestEnterpriseTokenIgnored(t *testing.T) { + // This test ignores the test helper because we want to test a specific config state + t.Run("enterprise token alone is ignored and rejected", func(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + f.Config = func() (gh.Config, error) { + return func() gh.Config { + c := config.NewBlankConfig() + c.Set("something.ghes.com", "oauth_token", "ghe_ENTERPRISE123") + return c + }(), nil + } + + cmd := NewCmdAgentTask(f) + err := cmd.Execute() + require.Error(t, err) + }) + + t.Run("github.com oauth is accepted and enterprise token ignored", func(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + f.Config = func() (gh.Config, error) { + return func() gh.Config { + c := config.NewBlankConfig() + c.Set("something.ghes.com", "oauth_token", "ghe_ENTERPRISE123") + c.Set("github.com", "oauth_token", "gho_OAUTH123") + return c + }(), nil + } + + cmd := NewCmdAgentTask(f) + err := cmd.Execute() + require.NoError(t, err) + }) + +} + +func TestEnterpriseHostRejected(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + + f.Config = func() (gh.Config, error) { + return &ghmock.ConfigMock{ + AuthenticationFunc: func() gh.AuthConfig { + c := &config.AuthConfig{} + c.SetDefaultHost("something.ghes.com", "GH_HOST") + return c + }, + }, nil + } + + cmd := NewCmdAgentTask(f) + err := cmd.Execute() + require.Error(t, err) + require.Contains(t, err.Error(), "not supported on this host") +} + +func TestEmptyHostRejected(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + + f.Config = func() (gh.Config, error) { + return &ghmock.ConfigMock{ + AuthenticationFunc: func() gh.AuthConfig { + c := &config.AuthConfig{} + c.SetDefaultHost("", "GH_HOST") + return c + }, + }, nil + } + + cmd := NewCmdAgentTask(f) + err := cmd.Execute() + require.Error(t, err) + require.Contains(t, err.Error(), "no default host configured") +} + +func TestNoAuthRejected(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + // No token configured + f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, ""), nil } + + cmd := NewCmdAgentTask(f) + err := cmd.Execute() + require.Error(t, err) +} + +func TestAliasAreSet(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, "oauth_token"), nil } + + cmd := NewCmdAgentTask(f) + + require.ElementsMatch(t, []string{"agent-tasks", "agent", "agents"}, cmd.Aliases) +} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 6a709c336..27f028e44 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -8,6 +8,7 @@ import ( "github.com/MakeNowJust/heredoc" accessibilityCmd "github.com/cli/cli/v2/pkg/cmd/accessibility" actionsCmd "github.com/cli/cli/v2/pkg/cmd/actions" + agentTaskCmd "github.com/cli/cli/v2/pkg/cmd/agent-task" aliasCmd "github.com/cli/cli/v2/pkg/cmd/alias" "github.com/cli/cli/v2/pkg/cmd/alias/shared" apiCmd "github.com/cli/cli/v2/pkg/cmd/api" @@ -126,6 +127,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) (*cobra.Command, cmd.AddCommand(versionCmd.NewCmdVersion(f, version, buildDate)) cmd.AddCommand(accessibilityCmd.NewCmdAccessibility(f)) cmd.AddCommand(actionsCmd.NewCmdActions(f)) + cmd.AddCommand(agentTaskCmd.NewCmdAgentTask(f)) cmd.AddCommand(aliasCmd.NewCmdAlias(f)) cmd.AddCommand(authCmd.NewCmdAuth(f)) cmd.AddCommand(attestationCmd.NewCmdAttestation(f)) From 2128a297b37550b6c98e78f03a6ae1452799a2af Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 27 Aug 2025 17:06:19 -0600 Subject: [PATCH 2/4] Show help on agent-task command execution Changed the RunE function of the agent-task command to display help output instead of returning nil, improving user guidance when the command is run without arguments. --- pkg/cmd/agent-task/agent_task.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/agent_task.go b/pkg/cmd/agent-task/agent_task.go index 1e916d034..b53c6786d 100644 --- a/pkg/cmd/agent-task/agent_task.go +++ b/pkg/cmd/agent-task/agent_task.go @@ -22,7 +22,7 @@ func NewCmdAgentTask(f *cmdutil.Factory) *cobra.Command { // This is required to run this root command. We want to // run it to test PersistentPreRunE behavior. RunE: func(cmd *cobra.Command, args []string) error { - return nil + return cmd.Help() }, } return cmd From 1bc2710c883b27093115dba207febd78817a92aa Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 27 Aug 2025 17:07:35 -0600 Subject: [PATCH 3/4] Refactor test to use require.Empty assertion Replaces require.Equal with require.Empty in TestOAuthTokenAccepted for improved clarity when checking for empty output. --- pkg/cmd/agent-task/agent_task_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/agent_task_test.go b/pkg/cmd/agent-task/agent_task_test.go index 76b2651f1..d5e89c55f 100644 --- a/pkg/cmd/agent-task/agent_task_test.go +++ b/pkg/cmd/agent-task/agent_task_test.go @@ -41,7 +41,7 @@ func TestOAuthTokenAccepted(t *testing.T) { cmd := NewCmdAgentTask(f) err := cmd.Execute() require.NoError(t, err) - require.Equal(t, "", stdout.String()) + require.Empty(t, stdout.String()) } func TestKeyringOAuthTokenAccepted(t *testing.T) { From b939188e6ddc240d29420e95f14c1e109cf91479 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 27 Aug 2025 17:20:45 -0600 Subject: [PATCH 4/4] Refactor agent task tests into table-driven format Consolidates multiple individual test functions into a single table-driven test, improving maintainability and readability. This change makes it easier to add new test cases and ensures consistent test structure for agent task command authentication scenarios. --- pkg/cmd/agent-task/agent_task_test.go | 211 ++++++++++++-------------- 1 file changed, 94 insertions(+), 117 deletions(-) diff --git a/pkg/cmd/agent-task/agent_task_test.go b/pkg/cmd/agent-task/agent_task_test.go index d5e89c55f..dd4fe21b0 100644 --- a/pkg/cmd/agent-task/agent_task_test.go +++ b/pkg/cmd/agent-task/agent_task_test.go @@ -32,132 +32,109 @@ func setupMockOAuthConfig(t *testing.T, tokenSource string) gh.Config { return c } -func TestOAuthTokenAccepted(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, stdout, _ := iostreams.Test() - f.IOStreams = ios - f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, "oauth_token"), nil } - - cmd := NewCmdAgentTask(f) - err := cmd.Execute() - require.NoError(t, err) - require.Empty(t, stdout.String()) -} - -func TestKeyringOAuthTokenAccepted(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, stdout, _ := iostreams.Test() - f.IOStreams = ios - f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, "keyring"), nil } - - cmd := NewCmdAgentTask(f) - err := cmd.Execute() - require.NoError(t, err) - require.Equal(t, "", stdout.String()) -} - -func TestEnvVarTokenRejected(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, "GH_TOKEN"), nil } - cmd := NewCmdAgentTask(f) - err := cmd.Execute() - require.Error(t, err) - require.Contains(t, err.Error(), "requires an OAuth token") -} - -func TestEnterpriseTokenIgnored(t *testing.T) { - // This test ignores the test helper because we want to test a specific config state - t.Run("enterprise token alone is ignored and rejected", func(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - f.Config = func() (gh.Config, error) { - return func() gh.Config { - c := config.NewBlankConfig() - c.Set("something.ghes.com", "oauth_token", "ghe_ENTERPRISE123") - return c - }(), nil - } - - cmd := NewCmdAgentTask(f) - err := cmd.Execute() - require.Error(t, err) - }) - - t.Run("github.com oauth is accepted and enterprise token ignored", func(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - f.Config = func() (gh.Config, error) { - return func() gh.Config { +func TestNewCmdAgentTask(t *testing.T) { + tests := []struct { + name string + tokenSource string + customConfig func() (gh.Config, error) + wantErr bool + wantErrContains string + wantStdout string + }{ + { + name: "oauth token is accepted", + tokenSource: "oauth_token", + wantErr: false, + wantStdout: "", + }, + { + name: "keyring oauth token is accepted", + tokenSource: "keyring", + wantErr: false, + wantStdout: "", + }, + { + name: "env var token is rejected", + tokenSource: "GH_TOKEN", + wantErr: true, + wantErrContains: "requires an OAuth token", + }, + { + name: "enterprise token alone is ignored and rejected", + tokenSource: "GH_ENTERPRISE_TOKEN", + wantErr: true, + }, + { + name: "github.com oauth is accepted and enterprise token ignored", + customConfig: func() (gh.Config, error) { c := config.NewBlankConfig() c.Set("something.ghes.com", "oauth_token", "ghe_ENTERPRISE123") c.Set("github.com", "oauth_token", "gho_OAUTH123") - return c - }(), nil - } - - cmd := NewCmdAgentTask(f) - err := cmd.Execute() - require.NoError(t, err) - }) - -} - -func TestEnterpriseHostRejected(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - - f.Config = func() (gh.Config, error) { - return &ghmock.ConfigMock{ - AuthenticationFunc: func() gh.AuthConfig { - c := &config.AuthConfig{} - c.SetDefaultHost("something.ghes.com", "GH_HOST") - return c + return c, nil }, - }, nil + wantErr: false, + wantStdout: "", + }, + { + name: "enterprise host is rejected", + customConfig: func() (gh.Config, error) { + return &ghmock.ConfigMock{ + AuthenticationFunc: func() gh.AuthConfig { + c := &config.AuthConfig{} + c.SetDefaultHost("something.ghes.com", "GH_HOST") + return c + }, + }, nil + }, + wantErr: true, + wantErrContains: "not supported on this host", + }, + { + name: "empty host is rejected", + customConfig: func() (gh.Config, error) { + return &ghmock.ConfigMock{ + AuthenticationFunc: func() gh.AuthConfig { + c := &config.AuthConfig{} + c.SetDefaultHost("", "GH_HOST") + return c + }, + }, nil + }, + wantErr: true, + wantErrContains: "no default host configured", + }, + { + name: "no auth is rejected", + tokenSource: "", + wantErr: true, + }, } - cmd := NewCmdAgentTask(f) - err := cmd.Execute() - require.Error(t, err) - require.Contains(t, err.Error(), "not supported on this host") -} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, stdout, _ := iostreams.Test() + f.IOStreams = ios + if tt.customConfig != nil { + f.Config = tt.customConfig + } else { + f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, tt.tokenSource), nil } + } -func TestEmptyHostRejected(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios + cmd := NewCmdAgentTask(f) + err := cmd.Execute() - f.Config = func() (gh.Config, error) { - return &ghmock.ConfigMock{ - AuthenticationFunc: func() gh.AuthConfig { - c := &config.AuthConfig{} - c.SetDefaultHost("", "GH_HOST") - return c - }, - }, nil + if tt.wantErr { + require.Error(t, err) + if tt.wantErrContains != "" { + require.Contains(t, err.Error(), tt.wantErrContains) + } + } else { + require.NoError(t, err) + require.Equal(t, tt.wantStdout, stdout.String()) + } + }) } - - cmd := NewCmdAgentTask(f) - err := cmd.Execute() - require.Error(t, err) - require.Contains(t, err.Error(), "no default host configured") -} - -func TestNoAuthRejected(t *testing.T) { - f := &cmdutil.Factory{} - ios, _, _, _ := iostreams.Test() - f.IOStreams = ios - // No token configured - f.Config = func() (gh.Config, error) { return setupMockOAuthConfig(t, ""), nil } - - cmd := NewCmdAgentTask(f) - err := cmd.Execute() - require.Error(t, err) } func TestAliasAreSet(t *testing.T) {