diff --git a/internal/repo/users.go b/internal/repo/users.go index 6fb2a8f4..e0782473 100644 --- a/internal/repo/users.go +++ b/internal/repo/users.go @@ -280,24 +280,12 @@ func (r *UsersRepo) UpdateUserCredentials(id int64, update UserCredentialUpdate) user.GongfengRefreshToken = "" user.GongfengTokenExpiresAt = time.Time{} } - if update.CodeBuddyAPIKeyClear { - user.CodeBuddyAPIKey = "" - } else if value := strings.TrimSpace(update.CodeBuddyAPIKey); value != "" { - user.CodeBuddyAPIKey = value - } + user.CodeBuddyAPIKey = updatedSecret(user.CodeBuddyAPIKey, update.CodeBuddyAPIKey, update.CodeBuddyAPIKeyClear) if update.CodeBuddyInternetEnvironment != nil { user.CodeBuddyInternetEnvironment = strings.TrimSpace(*update.CodeBuddyInternetEnvironment) } - if update.CodexAPIKeyClear { - user.CodexAPIKey = "" - } else if value := strings.TrimSpace(update.CodexAPIKey); value != "" { - user.CodexAPIKey = value - } - if update.ClaudeCodeAPIKeyClear { - user.ClaudeCodeAPIKey = "" - } else if value := strings.TrimSpace(update.ClaudeCodeAPIKey); value != "" { - user.ClaudeCodeAPIKey = value - } + user.CodexAPIKey = updatedSecret(user.CodexAPIKey, update.CodexAPIKey, update.CodexAPIKeyClear) + user.ClaudeCodeAPIKey = updatedSecret(user.ClaudeCodeAPIKey, update.ClaudeCodeAPIKey, update.ClaudeCodeAPIKeyClear) if update.CodexBaseURL != nil { user.CodexBaseURL = strings.TrimSpace(*update.CodexBaseURL) } @@ -367,6 +355,18 @@ func nullableString(v string) any { return v } +// updatedSecret applies a credential form update to a stored secret: clear +// wins, a blank value keeps the stored one, anything else replaces it trimmed. +func updatedSecret(current, value string, clear bool) string { + if clear { + return "" + } + if value = strings.TrimSpace(value); value != "" { + return value + } + return current +} + func applyAgentCLICredentialUpdates(user *domain.User, updates map[string]AgentCLICredentialUpdate) error { if len(updates) == 0 { return nil @@ -382,11 +382,7 @@ func applyAgentCLICredentialUpdates(user *domain.User, updates map[string]AgentC } cred := merged[kind] previousKey := cred.APIKey - if update.APIKeyClear { - cred.APIKey = "" - } else if value := strings.TrimSpace(update.APIKey); value != "" { - cred.APIKey = value - } + cred.APIKey = updatedSecret(cred.APIKey, update.APIKey, update.APIKeyClear) // Cursor's catalog is the list the key's plan offered (saved by the // chat model list); another key starts from that key's own list. if kind == "cursor" && cred.APIKey != previousKey && update.Models == nil { diff --git a/internal/repo/users_updated_secret_test.go b/internal/repo/users_updated_secret_test.go new file mode 100644 index 00000000..10f9a08b --- /dev/null +++ b/internal/repo/users_updated_secret_test.go @@ -0,0 +1,27 @@ +package repo + +import "testing" + +func TestUpdatedSecret(t *testing.T) { + cases := []struct { + name string + current string + value string + clear bool + want string + }{ + {name: "clear drops stored secret", current: "old", clear: true, want: ""}, + {name: "clear wins over new value", current: "old", value: "new", clear: true, want: ""}, + {name: "empty value keeps stored secret", current: "old", want: "old"}, + {name: "blank value keeps stored secret", current: "old", value: " \t\n", want: "old"}, + {name: "new value replaces trimmed", current: "old", value: " new ", want: "new"}, + {name: "new value fills empty secret", value: "new", want: "new"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := updatedSecret(tc.current, tc.value, tc.clear); got != tc.want { + t.Fatalf("updatedSecret(%q, %q, %v) = %q, want %q", tc.current, tc.value, tc.clear, got, tc.want) + } + }) + } +} diff --git a/internal/repo/users_updated_secret_wiring_test.go b/internal/repo/users_updated_secret_wiring_test.go new file mode 100644 index 00000000..9aba6938 --- /dev/null +++ b/internal/repo/users_updated_secret_wiring_test.go @@ -0,0 +1,145 @@ +package repo + +import ( + "agentflow/internal/domain" + "testing" +) + +// The unit test for updatedSecret covers the helper itself; these cases pin +// how UpdateUserCredentials wires each stored secret to its own form value and +// clear flag, so a swapped argument or a dropped "keep stored" path fails. + +func seedSecretWiringUser(t *testing.T, st *Store) domain.User { + t.Helper() + if err := st.PutUser(domain.User{LoginName: "secret-wiring"}); err != nil { + t.Fatal(err) + } + user, ok := st.UserByLoginName("secret-wiring") + if !ok { + t.Fatal("missing user") + } + base := "https://gateway.example/v1" + if _, err := st.UpdateUserCredentials(user.ID, UserCredentialUpdate{ + CodeBuddyAPIKey: "cb-old", + CodexAPIKey: "codex-old", + ClaudeCodeAPIKey: "claude-old", + AgentCLICredentials: map[string]AgentCLICredentialUpdate{ + "opencode": {APIKey: "oc-old", BaseURL: &base}, + }, + }); err != nil { + t.Fatal(err) + } + return user +} + +type storedSecrets struct { + codeBuddy, codex, claude, opencode string +} + +func readStoredSecrets(t *testing.T, st *Store, id int64) storedSecrets { + t.Helper() + got, ok := st.User(id) + if !ok { + t.Fatal("missing saved user") + } + return storedSecrets{ + codeBuddy: got.CodeBuddyAPIKey, + codex: got.CodexAPIKey, + claude: got.ClaudeCodeAPIKey, + opencode: got.AgentCLICredentials["opencode"].APIKey, + } +} + +func TestUpdateUserCredentialsSecretWiring(t *testing.T) { + seeded := storedSecrets{codeBuddy: "cb-old", codex: "codex-old", claude: "claude-old", opencode: "oc-old"} + newBase := "https://other.example/v1" + cases := []struct { + name string + update UserCredentialUpdate + want storedSecrets + }{ + { + name: "unrelated update keeps every stored secret", + update: UserCredentialUpdate{ + AgentCLICredentials: map[string]AgentCLICredentialUpdate{"opencode": {BaseURL: &newBase}}, + }, + want: seeded, + }, + { + name: "blank values keep every stored secret", + update: UserCredentialUpdate{CodeBuddyAPIKey: " ", CodexAPIKey: "\t", ClaudeCodeAPIKey: " \n", AgentCLICredentials: map[string]AgentCLICredentialUpdate{"opencode": {APIKey: " "}}}, + want: seeded, + }, + { + name: "clear CodeBuddy only", + update: UserCredentialUpdate{CodeBuddyAPIKeyClear: true}, + want: storedSecrets{codeBuddy: "", codex: "codex-old", claude: "claude-old", opencode: "oc-old"}, + }, + { + name: "clear Codex only", + update: UserCredentialUpdate{CodexAPIKeyClear: true}, + want: storedSecrets{codeBuddy: "cb-old", codex: "", claude: "claude-old", opencode: "oc-old"}, + }, + { + name: "clear Claude Code only", + update: UserCredentialUpdate{ClaudeCodeAPIKeyClear: true}, + want: storedSecrets{codeBuddy: "cb-old", codex: "codex-old", claude: "", opencode: "oc-old"}, + }, + { + name: "replace CodeBuddy only, trimmed", + update: UserCredentialUpdate{CodeBuddyAPIKey: " cb-new "}, + want: storedSecrets{codeBuddy: "cb-new", codex: "codex-old", claude: "claude-old", opencode: "oc-old"}, + }, + { + name: "replace Codex only, trimmed", + update: UserCredentialUpdate{CodexAPIKey: " codex-new "}, + want: storedSecrets{codeBuddy: "cb-old", codex: "codex-new", claude: "claude-old", opencode: "oc-old"}, + }, + { + name: "replace Claude Code only, trimmed", + update: UserCredentialUpdate{ClaudeCodeAPIKey: " claude-new "}, + want: storedSecrets{codeBuddy: "cb-old", codex: "codex-old", claude: "claude-new", opencode: "oc-old"}, + }, + { + name: "clear wins over a new value on the same field", + update: UserCredentialUpdate{CodexAPIKey: "codex-new", CodexAPIKeyClear: true, ClaudeCodeAPIKey: "claude-new"}, + want: storedSecrets{codeBuddy: "cb-old", codex: "", claude: "claude-new", opencode: "oc-old"}, + }, + { + name: "clear agent CLI key wins over a new value", + update: UserCredentialUpdate{ + AgentCLICredentials: map[string]AgentCLICredentialUpdate{"opencode": {APIKey: "oc-new", APIKeyClear: true}}, + }, + want: storedSecrets{codeBuddy: "cb-old", codex: "codex-old", claude: "claude-old", opencode: ""}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + st, err := OpenTest() + if err != nil { + t.Fatal(err) + } + defer st.Close() + user := seedSecretWiringUser(t, st) + if got := readStoredSecrets(t, st, user.ID); got != seeded { + t.Fatalf("seeded secrets = %+v, want %+v", got, seeded) + } + updated, err := st.UpdateUserCredentials(user.ID, tc.update) + if err != nil { + t.Fatal(err) + } + returned := storedSecrets{ + codeBuddy: updated.CodeBuddyAPIKey, + codex: updated.CodexAPIKey, + claude: updated.ClaudeCodeAPIKey, + opencode: updated.AgentCLICredentials["opencode"].APIKey, + } + if returned != tc.want { + t.Fatalf("returned secrets = %+v, want %+v", returned, tc.want) + } + if got := readStoredSecrets(t, st, user.ID); got != tc.want { + t.Fatalf("stored secrets = %+v, want %+v", got, tc.want) + } + }) + } +}