Skip to content

Commit 857c097

Browse files
leggetterclaudecursoragent
authored
fix: reject project list/use for CI-scoped keys with clear errors (#308)
* fix(cli): reject project list/use for CI-scoped keys with clear errors Validate credentials before cross-project commands, omit X-Team-ID on ListProjects, improve login and MCP flows for single-project CI keys, and strip legacy workspace/team keys on profile save. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix(cli): address review feedback on login prefix and --type validation Only show CI-scoped login prefix when validate confirms no user_id; validate --type before the credentials check so invalid flags fail offline (fixes CI). Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com> * chore(acceptance): move destination and gateway tags to slice 2 Rebalance CI matrix runtimes (~5.3 min slice 0 vs ~3 min slice 2). Move ConnectionListResponse to helpers so gateway tests compile without connection_list in the same build. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(cli): align project-scoped credential errors with API Rename ErrProjectScopedCredentials (ErrCIScopedCredentials alias), update messages for single-project scope, and match CLI_PROJECT_SCOPED from API. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 488ee7e commit 857c097

21 files changed

Lines changed: 539 additions & 19 deletions

.github/workflows/test-acceptance.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ jobs:
1414
include:
1515
- slice: "0"
1616
api_key_secret: HOOKDECK_CLI_TESTING_API_KEY
17-
tags: "basic connection source destination gateway mcp listen project_use connection_list connection_upsert connection_error_hints connection_oauth_aws connection_update"
17+
tags: "basic connection source mcp listen project_use connection_list connection_upsert connection_error_hints connection_oauth_aws connection_update"
1818
- slice: "1"
1919
api_key_secret: HOOKDECK_CLI_TESTING_API_KEY_2
2020
tags: "request event"
2121
- slice: "2"
2222
api_key_secret: HOOKDECK_CLI_TESTING_API_KEY_3
23-
tags: "attempt metrics issue transformation"
23+
tags: "attempt metrics issue transformation destination gateway"
2424
runs-on: ubuntu-latest
2525
env:
2626
ACCEPTANCE_SLICE: ${{ matrix.slice }}

pkg/cmd/project_list.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ func (lc *projectListCmd) runProjectListCmd(cmd *cobra.Command, args []string) e
6060
}
6161
}
6262

63+
if err := project.EnsureUserAssociatedCredentials(&Config); err != nil {
64+
return err
65+
}
66+
6367
projects, err := project.ListProjects(&Config)
6468
if err != nil {
6569
return err

pkg/cmd/project_use.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ func (lc *projectUseCmd) runProjectUseCmd(cmd *cobra.Command, args []string) err
5252
return err
5353
}
5454

55+
if err := project.EnsureUserAssociatedCredentials(&Config); err != nil {
56+
return err
57+
}
58+
5559
projects, err := project.ListProjects(&Config)
5660
if err != nil {
5761
return err

pkg/config/profile.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,34 @@ func (p *Profile) SaveProfile() error {
3030
}
3131
p.Config.viper.Set(p.getConfigField("project_type"), projectType)
3232
p.Config.viper.Set(p.getConfigField("guest_url"), p.GuestURL)
33+
34+
if err := p.removeLegacyConfigKeys(); err != nil {
35+
return err
36+
}
37+
3338
return p.Config.writeConfig()
3439
}
3540

41+
func (p *Profile) removeLegacyConfigKeys() error {
42+
legacyKeys := []string{"workspace_id", "workspace_mode", "team_id", "team_mode"}
43+
configFile := p.Config.viper.ConfigFileUsed()
44+
var err error
45+
for _, key := range legacyKeys {
46+
p.Config.viper, err = removeKey(p.Config.viper, p.getConfigField(key))
47+
if err != nil {
48+
return err
49+
}
50+
p.Config.viper, err = removeKey(p.Config.viper, key)
51+
if err != nil {
52+
return err
53+
}
54+
}
55+
if configFile != "" {
56+
p.Config.viper.SetConfigFile(configFile)
57+
}
58+
return nil
59+
}
60+
3661
func (p *Profile) RemoveProfile() error {
3762
var err error
3863
runtimeViper := p.Config.viper

pkg/config/profile_credentials_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package config
22

33
import (
4+
"os"
5+
"path/filepath"
46
"testing"
57

68
"github.com/hookdeck/hookdeck-cli/pkg/hookdeck"
9+
"github.com/stretchr/testify/assert"
710
"github.com/stretchr/testify/require"
811
)
912

@@ -83,3 +86,39 @@ func TestProfile_ApplyCIClient(t *testing.T) {
8386
require.Equal(t, ProjectTypeGateway, p.ProjectType)
8487
require.Empty(t, p.GuestURL)
8588
}
89+
90+
func TestSaveProfile_RemovesLegacyWorkspaceKeys(t *testing.T) {
91+
dir := t.TempDir()
92+
path := filepath.Join(dir, "config.toml")
93+
content := `profile = "default"
94+
workspace_id = "legacy_team"
95+
workspace_mode = "inbound"
96+
team_id = "legacy_team"
97+
team_mode = "inbound"
98+
99+
[default]
100+
api_key = "hk_test_123456789012"
101+
project_id = "proj_new"
102+
project_mode = "inbound"
103+
workspace_id = "legacy_profile_team"
104+
workspace_mode = "inbound"
105+
team_id = "legacy_profile_team"
106+
team_mode = "inbound"
107+
`
108+
require.NoError(t, os.WriteFile(path, []byte(content), 0600))
109+
110+
c, err := LoadConfigFromFile(path)
111+
require.NoError(t, err)
112+
c.Profile.Config = c
113+
114+
require.NoError(t, c.Profile.SaveProfile())
115+
116+
raw, err := os.ReadFile(path)
117+
require.NoError(t, err)
118+
tomlText := string(raw)
119+
assert.NotContains(t, tomlText, "workspace_id")
120+
assert.NotContains(t, tomlText, "workspace_mode")
121+
assert.NotContains(t, tomlText, "team_id")
122+
assert.NotContains(t, tomlText, "team_mode")
123+
assert.Contains(t, tomlText, "project_id")
124+
}

pkg/gateway/mcp/server_test.go

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ func newTestClient(baseURL string, apiKey string) *hookdeck.Client {
3939
func connectInMemory(t *testing.T, client *hookdeck.Client) *mcpsdk.ClientSession {
4040
t.Helper()
4141
cfg := &config.Config{}
42+
if client != nil && client.BaseURL != nil {
43+
cfg.APIBaseURL = client.BaseURL.String()
44+
}
4245
srv := NewServer(client, cfg)
4346

4447
serverTransport, clientTransport := mcpsdk.NewInMemoryTransports()
@@ -92,6 +95,23 @@ func listResponse(models ...map[string]any) map[string]any {
9295
// mockAPI creates an httptest server that handles specific API paths.
9396
func mockAPI(t *testing.T, handlers map[string]http.HandlerFunc) *httptest.Server {
9497
t.Helper()
98+
if handlers == nil {
99+
handlers = map[string]http.HandlerFunc{}
100+
}
101+
if _, ok := handlers["/2025-07-01/cli-auth/validate"]; !ok {
102+
handlers["/2025-07-01/cli-auth/validate"] = func(w http.ResponseWriter, r *http.Request) {
103+
json.NewEncoder(w).Encode(map[string]any{
104+
"user_id": "usr_test",
105+
"user_name": "Test User",
106+
"user_email": "u@example.com",
107+
"organization_name": "Test Org",
108+
"organization_id": "org_test",
109+
"team_id": "proj_test123",
110+
"team_name_no_org": "Production",
111+
"team_mode": "console",
112+
})
113+
}
114+
}
95115
mux := http.NewServeMux()
96116
for pattern, handler := range handlers {
97117
mux.HandleFunc(pattern, handler)
@@ -1116,14 +1136,86 @@ func TestMetricsTool_UnknownAction(t *testing.T) {
11161136
// ---------------------------------------------------------------------------
11171137

11181138
func TestLoginTool_AlreadyAuthenticated(t *testing.T) {
1119-
client := newTestClient("https://api.hookdeck.com", "test-key")
1139+
api := mockAPI(t, map[string]http.HandlerFunc{
1140+
"/2025-07-01/cli-auth/validate": func(w http.ResponseWriter, r *http.Request) {
1141+
json.NewEncoder(w).Encode(map[string]any{
1142+
"user_id": "usr_1",
1143+
"user_name": "Test User",
1144+
"user_email": "u@example.com",
1145+
"organization_name": "Org",
1146+
"organization_id": "org_1",
1147+
"team_id": "tm_1",
1148+
"team_name_no_org": "Proj",
1149+
"team_mode": "inbound",
1150+
})
1151+
},
1152+
})
1153+
client := newTestClient(api.URL, "test-key")
11201154
session := connectInMemory(t, client)
11211155

11221156
result := callTool(t, session, "hookdeck_login", map[string]any{})
11231157
assert.False(t, result.IsError)
11241158
assert.Contains(t, textContent(t, result), "Already authenticated")
11251159
}
11261160

1161+
func TestLoginTool_CIScopedKeyStartsLogin(t *testing.T) {
1162+
api := mockAPI(t, map[string]http.HandlerFunc{
1163+
"/2025-07-01/cli-auth/validate": func(w http.ResponseWriter, r *http.Request) {
1164+
json.NewEncoder(w).Encode(map[string]any{
1165+
"organization_name": "Org",
1166+
"organization_id": "org_1",
1167+
"team_id": "tm_ci",
1168+
"team_name_no_org": "CI Project",
1169+
"team_mode": "inbound",
1170+
})
1171+
},
1172+
"/2025-07-01/cli-auth": func(w http.ResponseWriter, r *http.Request) {
1173+
json.NewEncoder(w).Encode(map[string]any{
1174+
"browser_url": "https://hookdeck.com/auth?code=ci-upgrade",
1175+
"poll_url": "http://" + r.Host + "/2025-07-01/cli-auth/poll?key=ci-upgrade",
1176+
})
1177+
},
1178+
"/2025-07-01/cli-auth/poll": func(w http.ResponseWriter, r *http.Request) {
1179+
json.NewEncoder(w).Encode(map[string]any{"claimed": false})
1180+
},
1181+
})
1182+
client := newTestClient(api.URL, "test-key")
1183+
session := connectInMemory(t, client)
1184+
1185+
result := callTool(t, session, "hookdeck_login", map[string]any{})
1186+
assert.False(t, result.IsError)
1187+
text := textContent(t, result)
1188+
assert.NotContains(t, text, "Already authenticated")
1189+
assert.Contains(t, text, "Login initiated")
1190+
assert.Contains(t, text, "scoped to one project")
1191+
}
1192+
1193+
func TestLoginTool_UnauthorizedKeyNoScopedPrefix(t *testing.T) {
1194+
api := mockAPI(t, map[string]http.HandlerFunc{
1195+
"/2025-07-01/cli-auth/validate": func(w http.ResponseWriter, r *http.Request) {
1196+
w.WriteHeader(http.StatusUnauthorized)
1197+
_, _ = w.Write([]byte("Unauthorized"))
1198+
},
1199+
"/2025-07-01/cli-auth": func(w http.ResponseWriter, r *http.Request) {
1200+
json.NewEncoder(w).Encode(map[string]any{
1201+
"browser_url": "https://hookdeck.com/auth?code=revoked",
1202+
"poll_url": "http://" + r.Host + "/2025-07-01/cli-auth/poll?key=revoked",
1203+
})
1204+
},
1205+
"/2025-07-01/cli-auth/poll": func(w http.ResponseWriter, r *http.Request) {
1206+
json.NewEncoder(w).Encode(map[string]any{"claimed": false})
1207+
},
1208+
})
1209+
client := newTestClient(api.URL, "test-key")
1210+
session := connectInMemory(t, client)
1211+
1212+
result := callTool(t, session, "hookdeck_login", map[string]any{})
1213+
assert.False(t, result.IsError)
1214+
text := textContent(t, result)
1215+
assert.Contains(t, text, "Login initiated")
1216+
assert.NotContains(t, text, "scoped to one project")
1217+
}
1218+
11271219
func TestLoginTool_ReauthStartsFreshLogin(t *testing.T) {
11281220
api := mockAPI(t, map[string]http.HandlerFunc{
11291221
"/2025-07-01/cli-auth": func(w http.ResponseWriter, r *http.Request) {

pkg/gateway/mcp/tool_login.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,19 @@ func handleLogin(srv *Server) mcpsdk.ToolHandler {
7171
client.ProjectName = ""
7272
}
7373

74-
// Already authenticated — nothing to do.
74+
// Already authenticated with a user-associated key — nothing to do.
75+
loginPrefix := ""
7576
if client.APIKey != "" {
76-
return TextResult("Already authenticated. All Hookdeck tools are available."), nil
77+
lacks_user, err := project.CredentialsLackUserAssociation(client)
78+
if err != nil && !hookdeck.IsUnauthorizedError(err) {
79+
return ErrorResult(fmt.Sprintf("Failed to verify credentials: %s", err)), nil
80+
}
81+
if err == nil && !lacks_user {
82+
return TextResult("Already authenticated. All Hookdeck tools are available."), nil
83+
}
84+
if err == nil && lacks_user {
85+
loginPrefix = "Current credentials are scoped to one project; opening browser sign-in for full access.\n\n"
86+
}
7787
}
7888

7989
// If a login flow is already in progress, check its status.
@@ -187,7 +197,8 @@ func handleLogin(srv *Server) mcpsdk.ToolHandler {
187197

188198
// Return the URL immediately so the agent can show it to the user.
189199
return TextResult(fmt.Sprintf(
190-
"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.",
200+
"%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.",
201+
loginPrefix,
191202
session.BrowserURL,
192203
)), nil
193204
}

pkg/gateway/mcp/tool_projects.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ type projectEntry struct {
4343
}
4444

4545
func projectsList(client *hookdeck.Client) (*mcpsdk.CallToolResult, error) {
46+
if err := project.EnsureUserAssociatedClient(client); err != nil {
47+
return ErrorResult(listProjectsFailureMessage(err)), nil
48+
}
49+
4650
projects, err := client.ListProjects()
4751
if err != nil {
4852
return ErrorResult(listProjectsFailureMessage(err)), nil
@@ -71,6 +75,10 @@ func projectsUse(client *hookdeck.Client, in input) (*mcpsdk.CallToolResult, err
7175
return ErrorResult("project_id is required for the use action"), nil
7276
}
7377

78+
if err := project.EnsureUserAssociatedClient(client); err != nil {
79+
return ErrorResult(listProjectsFailureMessage(err)), nil
80+
}
81+
7482
projects, err := client.ListProjects()
7583
if err != nil {
7684
return ErrorResult(listProjectsFailureMessage(err)), nil

pkg/gateway/mcp/tool_projects_errors.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strings"
77

88
"github.com/hookdeck/hookdeck-cli/pkg/hookdeck"
9+
"github.com/hookdeck/hookdeck-cli/pkg/project"
910
)
1011

1112
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 {
1920
}
2021

2122
func shouldSuggestReauthAfterListProjectsFailure(err error) bool {
23+
if errors.Is(err, project.ErrProjectScopedCredentials) {
24+
return true
25+
}
2226
var apiErr *hookdeck.APIError
2327
if errors.As(err, &apiErr) {
2428
if apiErr.StatusCode == http.StatusForbidden || apiErr.StatusCode == http.StatusUnauthorized {
2529
return true
2630
}
31+
msg := strings.ToUpper(apiErr.Message)
32+
if strings.Contains(msg, "CLI_PROJECT_SCOPED") || strings.Contains(msg, "CLI_USER_REQUIRED") {
33+
return true
34+
}
2735
return strings.Contains(strings.ToLower(apiErr.Message), "fatal")
2836
}
2937
// ListProjects wraps some failures as plain fmt.Errorf with the status code

pkg/gateway/mcp/tool_projects_errors_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/hookdeck/hookdeck-cli/pkg/hookdeck"
8+
"github.com/hookdeck/hookdeck-cli/pkg/project"
89
"github.com/stretchr/testify/assert"
910
)
1011

@@ -14,6 +15,16 @@ func TestShouldSuggestReauthAfterListProjectsFailure(t *testing.T) {
1415
err error
1516
want bool
1617
}{
18+
{
19+
name: "project-scoped credentials",
20+
err: project.ErrProjectScopedCredentials,
21+
want: true,
22+
},
23+
{
24+
name: "APIError 403 CLI_PROJECT_SCOPED",
25+
err: &hookdeck.APIError{StatusCode: 403, Message: "CLI_PROJECT_SCOPED: cannot list all projects"},
26+
want: true,
27+
},
1728
{
1829
name: "APIError 403",
1930
err: &hookdeck.APIError{StatusCode: 403, Message: "not allowed"},

0 commit comments

Comments
 (0)