diff --git a/.github/workflows/test-acceptance.yml b/.github/workflows/test-acceptance.yml index 08f82187..11a40961 100644 --- a/.github/workflows/test-acceptance.yml +++ b/.github/workflows/test-acceptance.yml @@ -14,13 +14,13 @@ jobs: include: - slice: "0" api_key_secret: HOOKDECK_CLI_TESTING_API_KEY - tags: "basic connection source destination gateway mcp listen project_use connection_list connection_upsert connection_error_hints connection_oauth_aws connection_update" + tags: "basic connection source mcp listen project_use connection_list connection_upsert connection_error_hints connection_oauth_aws connection_update" - slice: "1" api_key_secret: HOOKDECK_CLI_TESTING_API_KEY_2 tags: "request event" - slice: "2" api_key_secret: HOOKDECK_CLI_TESTING_API_KEY_3 - tags: "attempt metrics issue transformation" + tags: "attempt metrics issue transformation destination gateway" runs-on: ubuntu-latest env: ACCEPTANCE_SLICE: ${{ matrix.slice }} diff --git a/pkg/cmd/project_list.go b/pkg/cmd/project_list.go index dcc56f9e..db620ee2 100644 --- a/pkg/cmd/project_list.go +++ b/pkg/cmd/project_list.go @@ -60,6 +60,10 @@ func (lc *projectListCmd) runProjectListCmd(cmd *cobra.Command, args []string) e } } + if err := project.EnsureUserAssociatedCredentials(&Config); err != nil { + return err + } + projects, err := project.ListProjects(&Config) if err != nil { return err diff --git a/pkg/cmd/project_use.go b/pkg/cmd/project_use.go index 7fc79ae0..4d6f1e31 100644 --- a/pkg/cmd/project_use.go +++ b/pkg/cmd/project_use.go @@ -52,6 +52,10 @@ func (lc *projectUseCmd) runProjectUseCmd(cmd *cobra.Command, args []string) err return err } + if err := project.EnsureUserAssociatedCredentials(&Config); err != nil { + return err + } + projects, err := project.ListProjects(&Config) if err != nil { return err diff --git a/pkg/config/profile.go b/pkg/config/profile.go index 2608f734..b1f8d782 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -30,9 +30,34 @@ func (p *Profile) SaveProfile() error { } p.Config.viper.Set(p.getConfigField("project_type"), projectType) p.Config.viper.Set(p.getConfigField("guest_url"), p.GuestURL) + + if err := p.removeLegacyConfigKeys(); err != nil { + return err + } + return p.Config.writeConfig() } +func (p *Profile) removeLegacyConfigKeys() error { + legacyKeys := []string{"workspace_id", "workspace_mode", "team_id", "team_mode"} + configFile := p.Config.viper.ConfigFileUsed() + var err error + for _, key := range legacyKeys { + p.Config.viper, err = removeKey(p.Config.viper, p.getConfigField(key)) + if err != nil { + return err + } + p.Config.viper, err = removeKey(p.Config.viper, key) + if err != nil { + return err + } + } + if configFile != "" { + p.Config.viper.SetConfigFile(configFile) + } + return nil +} + func (p *Profile) RemoveProfile() error { var err error runtimeViper := p.Config.viper diff --git a/pkg/config/profile_credentials_test.go b/pkg/config/profile_credentials_test.go index b55e9510..0a8d3e52 100644 --- a/pkg/config/profile_credentials_test.go +++ b/pkg/config/profile_credentials_test.go @@ -1,9 +1,12 @@ package config import ( + "os" + "path/filepath" "testing" "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -83,3 +86,39 @@ func TestProfile_ApplyCIClient(t *testing.T) { require.Equal(t, ProjectTypeGateway, p.ProjectType) require.Empty(t, p.GuestURL) } + +func TestSaveProfile_RemovesLegacyWorkspaceKeys(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + content := `profile = "default" +workspace_id = "legacy_team" +workspace_mode = "inbound" +team_id = "legacy_team" +team_mode = "inbound" + +[default] +api_key = "hk_test_123456789012" +project_id = "proj_new" +project_mode = "inbound" +workspace_id = "legacy_profile_team" +workspace_mode = "inbound" +team_id = "legacy_profile_team" +team_mode = "inbound" +` + require.NoError(t, os.WriteFile(path, []byte(content), 0600)) + + c, err := LoadConfigFromFile(path) + require.NoError(t, err) + c.Profile.Config = c + + require.NoError(t, c.Profile.SaveProfile()) + + raw, err := os.ReadFile(path) + require.NoError(t, err) + tomlText := string(raw) + assert.NotContains(t, tomlText, "workspace_id") + assert.NotContains(t, tomlText, "workspace_mode") + assert.NotContains(t, tomlText, "team_id") + assert.NotContains(t, tomlText, "team_mode") + assert.Contains(t, tomlText, "project_id") +} diff --git a/pkg/gateway/mcp/server_test.go b/pkg/gateway/mcp/server_test.go index 80fcc344..8ff121a0 100644 --- a/pkg/gateway/mcp/server_test.go +++ b/pkg/gateway/mcp/server_test.go @@ -39,6 +39,9 @@ func newTestClient(baseURL string, apiKey string) *hookdeck.Client { func connectInMemory(t *testing.T, client *hookdeck.Client) *mcpsdk.ClientSession { t.Helper() cfg := &config.Config{} + if client != nil && client.BaseURL != nil { + cfg.APIBaseURL = client.BaseURL.String() + } srv := NewServer(client, cfg) serverTransport, clientTransport := mcpsdk.NewInMemoryTransports() @@ -92,6 +95,23 @@ func listResponse(models ...map[string]any) map[string]any { // mockAPI creates an httptest server that handles specific API paths. func mockAPI(t *testing.T, handlers map[string]http.HandlerFunc) *httptest.Server { t.Helper() + if handlers == nil { + handlers = map[string]http.HandlerFunc{} + } + if _, ok := handlers["/2025-07-01/cli-auth/validate"]; !ok { + handlers["/2025-07-01/cli-auth/validate"] = func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "user_id": "usr_test", + "user_name": "Test User", + "user_email": "u@example.com", + "organization_name": "Test Org", + "organization_id": "org_test", + "team_id": "proj_test123", + "team_name_no_org": "Production", + "team_mode": "console", + }) + } + } mux := http.NewServeMux() for pattern, handler := range handlers { mux.HandleFunc(pattern, handler) @@ -1116,7 +1136,21 @@ func TestMetricsTool_UnknownAction(t *testing.T) { // --------------------------------------------------------------------------- func TestLoginTool_AlreadyAuthenticated(t *testing.T) { - client := newTestClient("https://api.hookdeck.com", "test-key") + api := mockAPI(t, map[string]http.HandlerFunc{ + "/2025-07-01/cli-auth/validate": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "user_id": "usr_1", + "user_name": "Test User", + "user_email": "u@example.com", + "organization_name": "Org", + "organization_id": "org_1", + "team_id": "tm_1", + "team_name_no_org": "Proj", + "team_mode": "inbound", + }) + }, + }) + client := newTestClient(api.URL, "test-key") session := connectInMemory(t, client) result := callTool(t, session, "hookdeck_login", map[string]any{}) @@ -1124,6 +1158,64 @@ func TestLoginTool_AlreadyAuthenticated(t *testing.T) { assert.Contains(t, textContent(t, result), "Already authenticated") } +func TestLoginTool_CIScopedKeyStartsLogin(t *testing.T) { + api := mockAPI(t, map[string]http.HandlerFunc{ + "/2025-07-01/cli-auth/validate": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "organization_name": "Org", + "organization_id": "org_1", + "team_id": "tm_ci", + "team_name_no_org": "CI Project", + "team_mode": "inbound", + }) + }, + "/2025-07-01/cli-auth": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "browser_url": "https://hookdeck.com/auth?code=ci-upgrade", + "poll_url": "http://" + r.Host + "/2025-07-01/cli-auth/poll?key=ci-upgrade", + }) + }, + "/2025-07-01/cli-auth/poll": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{"claimed": false}) + }, + }) + client := newTestClient(api.URL, "test-key") + session := connectInMemory(t, client) + + result := callTool(t, session, "hookdeck_login", map[string]any{}) + assert.False(t, result.IsError) + text := textContent(t, result) + assert.NotContains(t, text, "Already authenticated") + assert.Contains(t, text, "Login initiated") + assert.Contains(t, text, "scoped to one project") +} + +func TestLoginTool_UnauthorizedKeyNoScopedPrefix(t *testing.T) { + api := mockAPI(t, map[string]http.HandlerFunc{ + "/2025-07-01/cli-auth/validate": func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte("Unauthorized")) + }, + "/2025-07-01/cli-auth": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "browser_url": "https://hookdeck.com/auth?code=revoked", + "poll_url": "http://" + r.Host + "/2025-07-01/cli-auth/poll?key=revoked", + }) + }, + "/2025-07-01/cli-auth/poll": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{"claimed": false}) + }, + }) + client := newTestClient(api.URL, "test-key") + session := connectInMemory(t, client) + + result := callTool(t, session, "hookdeck_login", map[string]any{}) + assert.False(t, result.IsError) + text := textContent(t, result) + assert.Contains(t, text, "Login initiated") + assert.NotContains(t, text, "scoped to one project") +} + func TestLoginTool_ReauthStartsFreshLogin(t *testing.T) { api := mockAPI(t, map[string]http.HandlerFunc{ "/2025-07-01/cli-auth": func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/gateway/mcp/tool_login.go b/pkg/gateway/mcp/tool_login.go index 2c118ddd..8e314539 100644 --- a/pkg/gateway/mcp/tool_login.go +++ b/pkg/gateway/mcp/tool_login.go @@ -71,9 +71,19 @@ func handleLogin(srv *Server) mcpsdk.ToolHandler { client.ProjectName = "" } - // Already authenticated — nothing to do. + // Already authenticated with a user-associated key — nothing to do. + loginPrefix := "" if client.APIKey != "" { - return TextResult("Already authenticated. All Hookdeck tools are available."), nil + lacks_user, err := project.CredentialsLackUserAssociation(client) + if err != nil && !hookdeck.IsUnauthorizedError(err) { + return ErrorResult(fmt.Sprintf("Failed to verify credentials: %s", err)), nil + } + if err == nil && !lacks_user { + return TextResult("Already authenticated. All Hookdeck tools are available."), nil + } + if err == nil && lacks_user { + loginPrefix = "Current credentials are scoped to one project; opening browser sign-in for full access.\n\n" + } } // If a login flow is already in progress, check its status. @@ -187,7 +197,8 @@ func handleLogin(srv *Server) mcpsdk.ToolHandler { // Return the URL immediately so the agent can show it to the user. return TextResult(fmt.Sprintf( - "Login initiated. The user must open the following URL in their browser to authenticate:\n\n%s\n\nOnce the user completes authentication in the browser, all Hookdeck tools will become available.\nCall hookdeck_login again to check if authentication has completed.", + "%sLogin initiated. The user must open the following URL in their browser to authenticate:\n\n%s\n\nOnce the user completes authentication in the browser, all Hookdeck tools will become available.\nCall hookdeck_login again to check if authentication has completed.", + loginPrefix, session.BrowserURL, )), nil } diff --git a/pkg/gateway/mcp/tool_projects.go b/pkg/gateway/mcp/tool_projects.go index 57dd8826..514c7782 100644 --- a/pkg/gateway/mcp/tool_projects.go +++ b/pkg/gateway/mcp/tool_projects.go @@ -43,6 +43,10 @@ type projectEntry struct { } func projectsList(client *hookdeck.Client) (*mcpsdk.CallToolResult, error) { + if err := project.EnsureUserAssociatedClient(client); err != nil { + return ErrorResult(listProjectsFailureMessage(err)), nil + } + projects, err := client.ListProjects() if err != nil { return ErrorResult(listProjectsFailureMessage(err)), nil @@ -71,6 +75,10 @@ func projectsUse(client *hookdeck.Client, in input) (*mcpsdk.CallToolResult, err return ErrorResult("project_id is required for the use action"), nil } + if err := project.EnsureUserAssociatedClient(client); err != nil { + return ErrorResult(listProjectsFailureMessage(err)), nil + } + projects, err := client.ListProjects() if err != nil { return ErrorResult(listProjectsFailureMessage(err)), nil diff --git a/pkg/gateway/mcp/tool_projects_errors.go b/pkg/gateway/mcp/tool_projects_errors.go index 921fd8a2..08f19d68 100644 --- a/pkg/gateway/mcp/tool_projects_errors.go +++ b/pkg/gateway/mcp/tool_projects_errors.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + "github.com/hookdeck/hookdeck-cli/pkg/project" ) const listProjectsReauthHint = `This may happen if the stored key is a dashboard or single-project API key that cannot list all teams/projects. Try hookdeck_login with reauth: true so the user can sign in via the browser and replace the credential with a full CLI session, then retry hookdeck_projects.` @@ -19,11 +20,18 @@ func listProjectsFailureMessage(err error) string { } func shouldSuggestReauthAfterListProjectsFailure(err error) bool { + if errors.Is(err, project.ErrProjectScopedCredentials) { + return true + } var apiErr *hookdeck.APIError if errors.As(err, &apiErr) { if apiErr.StatusCode == http.StatusForbidden || apiErr.StatusCode == http.StatusUnauthorized { return true } + msg := strings.ToUpper(apiErr.Message) + if strings.Contains(msg, "CLI_PROJECT_SCOPED") || strings.Contains(msg, "CLI_USER_REQUIRED") { + return true + } return strings.Contains(strings.ToLower(apiErr.Message), "fatal") } // ListProjects wraps some failures as plain fmt.Errorf with the status code diff --git a/pkg/gateway/mcp/tool_projects_errors_test.go b/pkg/gateway/mcp/tool_projects_errors_test.go index 836bc6bb..0f80f93d 100644 --- a/pkg/gateway/mcp/tool_projects_errors_test.go +++ b/pkg/gateway/mcp/tool_projects_errors_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + "github.com/hookdeck/hookdeck-cli/pkg/project" "github.com/stretchr/testify/assert" ) @@ -14,6 +15,16 @@ func TestShouldSuggestReauthAfterListProjectsFailure(t *testing.T) { err error want bool }{ + { + name: "project-scoped credentials", + err: project.ErrProjectScopedCredentials, + want: true, + }, + { + name: "APIError 403 CLI_PROJECT_SCOPED", + err: &hookdeck.APIError{StatusCode: 403, Message: "CLI_PROJECT_SCOPED: cannot list all projects"}, + want: true, + }, { name: "APIError 403", err: &hookdeck.APIError{StatusCode: 403, Message: "not allowed"}, diff --git a/pkg/hookdeck/projects.go b/pkg/hookdeck/projects.go index 5a46b4d7..19580b68 100644 --- a/pkg/hookdeck/projects.go +++ b/pkg/hookdeck/projects.go @@ -11,7 +11,7 @@ type Project struct { } func (c *Client) ListProjects() ([]Project, error) { - res, err := c.Get(context.Background(), APIPathPrefix+"/teams", "", nil) + res, err := c.clientForCLIAuthValidate().Get(context.Background(), APIPathPrefix+"/teams", "", nil) if err != nil { return []Project{}, err } diff --git a/pkg/hookdeck/projects_test.go b/pkg/hookdeck/projects_test.go new file mode 100644 index 00000000..4e2f8d74 --- /dev/null +++ b/pkg/hookdeck/projects_test.go @@ -0,0 +1,43 @@ +package hookdeck + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestListProjects_omitsTeamAndProjectHeadersWhenConfigHasProjectID(t *testing.T) { + var sawTeamHeader bool + var sawProjectHeader bool + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawTeamHeader = r.Header.Get("X-Team-ID") != "" + sawProjectHeader = r.Header.Get("X-Project-ID") != "" + if r.URL.Path != APIPathPrefix+"/teams" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode([]Project{{Id: "tm_1", Name: "[Org] Proj", Mode: "inbound"}}) + })) + t.Cleanup(server.Close) + + baseURL, err := url.Parse(server.URL) + require.NoError(t, err) + + client := &Client{ + BaseURL: baseURL, + APIKey: "test_key", + ProjectID: "stale_team_should_not_be_sent", + } + + projects, err := client.ListProjects() + require.NoError(t, err) + require.False(t, sawTeamHeader, "list projects must not send X-Team-ID") + require.False(t, sawProjectHeader, "list projects must not send X-Project-ID") + require.Len(t, projects, 1) +} diff --git a/pkg/login/client_login.go b/pkg/login/client_login.go index f491f600..200c5a46 100644 --- a/pkg/login/client_login.go +++ b/pkg/login/client_login.go @@ -7,6 +7,7 @@ import ( "os" log "github.com/sirupsen/logrus" + "golang.org/x/term" "github.com/briandowns/spinner" @@ -14,11 +15,15 @@ import ( configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" "github.com/hookdeck/hookdeck-cli/pkg/open" + "github.com/hookdeck/hookdeck-cli/pkg/project" "github.com/hookdeck/hookdeck-cli/pkg/validators" ) var openBrowser = open.Browser var canOpenBrowser = open.CanOpenBrowser +var stdinIsTerminal = func() bool { + return term.IsTerminal(int(os.Stdin.Fd())) +} // Login function is used to obtain credentials via hookdeck dashboard. func Login(config *configpkg.Config, input io.Reader) error { @@ -41,7 +46,7 @@ func Login(config *configpkg.Config, input io.Reader) error { // or we would re-enter this branch only). fmt.Fprintln(os.Stdout, "Your saved API key is no longer valid. Starting browser sign-in...") config.Profile.APIKey = "" - } else { + } else if response.UserID != "" { message := SuccessMessage(response.UserName, response.UserEmail, response.OrganizationName, response.ProjectName, response.ProjectMode == "console") ansi.StopSpinner(s, message, os.Stdout) @@ -54,7 +59,16 @@ func Login(config *configpkg.Config, input io.Reader) error { return err } + config.RefreshCachedAPIClient() + return nil + } else { + ansi.StopSpinner(s, "", os.Stdout) + if !stdinIsTerminal() { + return project.ErrProjectScopedCredentials + } + fmt.Fprintln(os.Stdout, "Your saved key is scoped to a single project (CI). Starting browser sign-in...") + config.Profile.APIKey = "" } } diff --git a/pkg/login/client_login_test.go b/pkg/login/client_login_test.go index a1fe1df3..5439fd51 100644 --- a/pkg/login/client_login_test.go +++ b/pkg/login/client_login_test.go @@ -11,6 +11,7 @@ import ( configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + "github.com/hookdeck/hookdeck-cli/pkg/project" "github.com/stretchr/testify/require" ) @@ -119,3 +120,136 @@ api_key = "hk_test_oldkey_abcdefghij" require.Equal(t, 1, pollHits, "poll should run once with immediate claimed=true") require.Equal(t, "hk_test_newkey_abcdefghij", cfg.Profile.APIKey) } + +func TestLogin_ciKeyHeadlessFailsFast(t *testing.T) { + configpkg.ResetAPIClientForTesting() + t.Cleanup(configpkg.ResetAPIClientForTesting) + + oldStdinIsTerminal := stdinIsTerminal + stdinIsTerminal = func() bool { return false } + t.Cleanup(func() { stdinIsTerminal = oldStdinIsTerminal }) + + var sawCLIAuthPost bool + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/cli-auth/validate") { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(hookdeck.ValidateAPIKeyResponse{ + ProjectID: "tm_ci", + ProjectMode: "inbound", + OrganizationName: "Org", + OrganizationID: "org_1", + ProjectName: "CI", + }) + return + } + if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli-auth") { + sawCLIAuthPost = true + } + t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + })) + t.Cleanup(ts.Close) + + configPath := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" + +[default] +api_key = "hk_test_cikey_abcdefghij" +`), 0o600)) + + cfg, err := configpkg.LoadConfigFromFile(configPath) + require.NoError(t, err) + cfg.APIBaseURL = ts.URL + cfg.DeviceName = "test-device" + cfg.LogLevel = "error" + cfg.TelemetryDisabled = true + + err = Login(cfg, strings.NewReader("\n")) + require.ErrorIs(t, err, project.ErrProjectScopedCredentials) + require.False(t, sawCLIAuthPost) +} + +func TestLogin_ciKeyStartsBrowserFlow(t *testing.T) { + configpkg.ResetAPIClientForTesting() + t.Cleanup(configpkg.ResetAPIClientForTesting) + + oldStdinIsTerminal := stdinIsTerminal + stdinIsTerminal = func() bool { return true } + t.Cleanup(func() { stdinIsTerminal = oldStdinIsTerminal }) + + oldCan := canOpenBrowser + oldOpen := openBrowser + canOpenBrowser = func() bool { return false } + openBrowser = func(string) error { return nil } + t.Cleanup(func() { + canOpenBrowser = oldCan + openBrowser = oldOpen + }) + + var sawCLIAuthPost bool + pollHits := 0 + var serverURL string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/cli-auth/validate"): + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(hookdeck.ValidateAPIKeyResponse{ + ProjectID: "tm_ci", + ProjectMode: "inbound", + OrganizationName: "Org", + OrganizationID: "org_1", + ProjectName: "CI", + }) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli-auth"): + sawCLIAuthPost = true + pollURL := serverURL + hookdeck.APIPathPrefix + "/cli-auth/poll?key=pollkey" + body, encErr := json.Marshal(map[string]string{ + "browser_url": "https://example.test/auth", + "poll_url": pollURL, + }) + require.NoError(t, encErr) + _, _ = w.Write(body) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/cli-auth/poll"): + pollHits++ + resp := map[string]interface{}{ + "claimed": true, + "key": "hk_test_userkey_abcdefghij", + "user_id": "usr_1", + "team_id": "tm_1", + "team_mode": "inbound", + "team_name": "Proj", + "user_name": "U", + "user_email": "u@example.com", + "organization_name": "Org", + "organization_id": "org_1", + "client_id": "cl_1", + } + enc, encErr := json.Marshal(resp) + require.NoError(t, encErr) + _, _ = w.Write(enc) + default: + t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + } + })) + serverURL = ts.URL + t.Cleanup(ts.Close) + + configPath := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" + +[default] +api_key = "hk_test_cikey_abcdefghij" +`), 0o600)) + + cfg, err := configpkg.LoadConfigFromFile(configPath) + require.NoError(t, err) + cfg.APIBaseURL = ts.URL + cfg.DeviceName = "test-device" + cfg.LogLevel = "error" + cfg.TelemetryDisabled = true + + err = Login(cfg, strings.NewReader("\n")) + require.NoError(t, err) + require.True(t, sawCLIAuthPost) + require.Equal(t, 1, pollHits) + require.Equal(t, "hk_test_userkey_abcdefghij", cfg.Profile.APIKey) +} diff --git a/pkg/project/credentials.go b/pkg/project/credentials.go new file mode 100644 index 00000000..63fdfc23 --- /dev/null +++ b/pkg/project/credentials.go @@ -0,0 +1,65 @@ +package project + +import ( + "errors" + "fmt" + + "github.com/hookdeck/hookdeck-cli/pkg/config" + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" +) + +// ErrProjectScopedCredentials is returned when the stored CLI key is valid but +// limited to a single project (for example after hookdeck ci). +var ErrProjectScopedCredentials = errors.New( + "this credential is scoped to a single project and cannot list all projects; " + + "keys from hookdeck ci are project-scoped; " + + "run hookdeck login in an interactive terminal, hookdeck login --cli-key with a product CLI key, or hookdeck_login via MCP for account-wide access", +) + +// ErrCIScopedCredentials is an alias for ErrProjectScopedCredentials. +var ErrCIScopedCredentials = ErrProjectScopedCredentials + +// ValidateCredentials calls GET /cli-auth/validate without sending X-Team-ID. +func ValidateCredentials(config *config.Config) (*hookdeck.ValidateAPIKeyResponse, error) { + return config.GetAPIClient().ValidateAPIKey() +} + +// EnsureUserAssociatedCredentials rejects project-scoped keys before cross-project calls. +func EnsureUserAssociatedCredentials(config *config.Config) error { + response, err := ValidateCredentials(config) + if err != nil { + return err + } + if response.UserID == "" { + return ErrProjectScopedCredentials + } + return nil +} + +// EnsureUserAssociatedClient rejects project-scoped keys for an in-memory API client (MCP). +func EnsureUserAssociatedClient(client *hookdeck.Client) error { + if client == nil || client.APIKey == "" { + return fmt.Errorf("not authenticated") + } + response, err := client.ValidateAPIKey() + if err != nil { + return err + } + if response.UserID == "" { + return ErrProjectScopedCredentials + } + return nil +} + +// CredentialsLackUserAssociation reports whether validate succeeded for a project-scoped key +// (no user_id — our proxy for single-project credential scope today). +func CredentialsLackUserAssociation(client *hookdeck.Client) (bool, error) { + if client == nil || client.APIKey == "" { + return false, nil + } + response, err := client.ValidateAPIKey() + if err != nil { + return false, err + } + return response.UserID == "", nil +} diff --git a/pkg/project/credentials_test.go b/pkg/project/credentials_test.go new file mode 100644 index 00000000..83a92aca --- /dev/null +++ b/pkg/project/credentials_test.go @@ -0,0 +1,44 @@ +package project + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + "github.com/stretchr/testify/require" +) + +func TestEnsureUserAssociatedCredentials_rejectsProjectScopedKey(t *testing.T) { + configpkg.ResetAPIClientForTesting() + t.Cleanup(configpkg.ResetAPIClientForTesting) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, hookdeck.APIPathPrefix+"/cli-auth/validate", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(hookdeck.ValidateAPIKeyResponse{ + ProjectID: "tm_ci", + ProjectMode: "inbound", + OrganizationName: "Org", + OrganizationID: "org_1", + ProjectName: "CI Project", + }) + })) + t.Cleanup(ts.Close) + + cfg := &configpkg.Config{ + APIBaseURL: ts.URL, + LogLevel: "error", + TelemetryDisabled: true, + } + cfg.Profile = configpkg.Profile{ + Name: "default", + APIKey: "hk_test_ci_key_abcdefghij", + Config: cfg, + } + + err := EnsureUserAssociatedCredentials(cfg) + require.ErrorIs(t, err, ErrProjectScopedCredentials) +} diff --git a/test/acceptance/README.md b/test/acceptance/README.md index cff8d60b..f4ecd2fd 100644 --- a/test/acceptance/README.md +++ b/test/acceptance/README.md @@ -72,7 +72,7 @@ go test -tags="basic connection source destination gateway mcp listen project_us Same commands as CI; use when debugging a subset or running in parallel: ```bash # Slice 0 (same tags as CI job 0) -ACCEPTANCE_SLICE=0 HOOKDECK_CLI_TELEMETRY_DISABLED=1 go test -tags="basic connection source destination gateway mcp listen project_use connection_list connection_upsert connection_error_hints connection_oauth_aws connection_update" ./test/acceptance/... -v -timeout 12m +ACCEPTANCE_SLICE=0 HOOKDECK_CLI_TELEMETRY_DISABLED=1 go test -tags="basic connection source mcp listen project_use connection_list connection_upsert connection_error_hints connection_oauth_aws connection_update" ./test/acceptance/... -v -timeout 12m # Slice 1 (same tags as CI job 1) ACCEPTANCE_SLICE=1 HOOKDECK_CLI_TELEMETRY_DISABLED=1 go test -tags="request event" ./test/acceptance/... -v -timeout 12m @@ -114,9 +114,9 @@ Use the same `-tags` as "Run all" if you want to skip the full acceptance set. A Tests are partitioned by **feature build tags** so CI and local runs can execute three matrix slices in parallel (each slice uses its own Hookdeck project and config file). -- **Slice 0 features:** `basic`, `connection`, `source`, `destination`, `gateway`, `mcp`, `listen`, `project_use`, `connection_list`, `connection_upsert`, `connection_error_hints`, `connection_oauth_aws`, `connection_update` +- **Slice 0 features:** `basic`, `connection`, `source`, `mcp`, `listen`, `project_use`, `connection_list`, `connection_upsert`, `connection_error_hints`, `connection_oauth_aws`, `connection_update` - **Slice 1 features:** `request`, `event` -- **Slice 2 features:** `attempt`, `metrics`, `issue`, `transformation` +- **Slice 2 features:** `attempt`, `metrics`, `issue`, `transformation`, `destination`, `gateway` - **Telemetry job:** `telemetry` only — separate CI job with telemetry **not** disabled (see [CI/CD](#cicd)) The CI workflow (`.github/workflows/test-acceptance.yml`) runs three matrix jobs plus `acceptance-telemetry`. Matrix jobs set `HOOKDECK_CLI_TELEMETRY_DISABLED=1`; the telemetry job does not. No test names or regexes are listed in YAML. diff --git a/test/acceptance/connection_list_test.go b/test/acceptance/connection_list_test.go index 350b3f45..64eee6f2 100644 --- a/test/acceptance/connection_list_test.go +++ b/test/acceptance/connection_list_test.go @@ -11,12 +11,6 @@ import ( "github.com/stretchr/testify/require" ) -// ConnectionListResponse wraps the list response with pagination -type ConnectionListResponse struct { - Models []Connection `json:"models"` - Pagination map[string]interface{} `json:"pagination"` -} - // TestConnectionListFilters tests the various filtering flags for connection list func TestConnectionListFilters(t *testing.T) { if testing.Short() { diff --git a/test/acceptance/helpers.go b/test/acceptance/helpers.go index aa572b2b..247ee16d 100644 --- a/test/acceptance/helpers.go +++ b/test/acceptance/helpers.go @@ -726,6 +726,12 @@ type Connection struct { Rules []map[string]interface{} `json:"rules"` } +// ConnectionListResponse wraps the list response with pagination. +type ConnectionListResponse struct { + Models []Connection `json:"models"` + Pagination map[string]interface{} `json:"pagination"` +} + // Source represents a Hookdeck source for testing type Source struct { ID string `json:"id"` diff --git a/test/acceptance/project_use_test.go b/test/acceptance/project_use_test.go index ecd1a5e7..4e3ff77a 100644 --- a/test/acceptance/project_use_test.go +++ b/test/acceptance/project_use_test.go @@ -330,3 +330,21 @@ func TestProjectListFilterByOrgAndProject(t *testing.T) { assert.NotEmpty(t, item.Type, "item %d should have type", i) } } + +// TestProjectListFailsWithCIKeyAcceptance asserts that project list fails with a clear +// message when the config holds a CI-scoped key (from hookdeck ci), not a 500 Fatal Error. +func TestProjectListFailsWithCIKeyAcceptance(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + + stdout, stderr, err := cli.Run("project", "list") + require.Error(t, err, "project list should fail with CI-scoped credentials") + + combined := stdout + stderr + assert.NotContains(t, combined, "Fatal Error") + assert.NotContains(t, combined, "status=500") + assert.Contains(t, combined, "single project") +} diff --git a/test/acceptance/run_parallel.sh b/test/acceptance/run_parallel.sh index 2e4b00c6..9c7beb65 100755 --- a/test/acceptance/run_parallel.sh +++ b/test/acceptance/run_parallel.sh @@ -29,9 +29,9 @@ SLICE1_LOG="$LOG_DIR/slice1.log" SLICE2_LOG="$LOG_DIR/slice2.log" TELEMETRY_LOG="$LOG_DIR/telemetry.log" -SLICE0_TAGS="basic connection source destination gateway mcp listen project_use connection_list connection_upsert connection_error_hints connection_oauth_aws connection_update" +SLICE0_TAGS="basic connection source mcp listen project_use connection_list connection_upsert connection_error_hints connection_oauth_aws connection_update" SLICE1_TAGS="request event" -SLICE2_TAGS="attempt metrics issue transformation" +SLICE2_TAGS="attempt metrics issue transformation destination gateway" run_slice0() { ACCEPTANCE_SLICE=0 HOOKDECK_CLI_TELEMETRY_DISABLED=1 go test -tags="$SLICE0_TAGS" ./test/acceptance/... -v -timeout 12m > "$SLICE0_LOG" 2>&1