From 2f611a0038497e5376010c2ac165f8eadf2dc8b3 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 18 Jun 2026 18:13:04 -0400 Subject: [PATCH 1/4] fix(init): show credential destination context Closes #349 --- docs/init-ux-contract.md | 22 ++ internal/cmd/credentialcmd/credentialcmd.go | 60 +++- .../cmd/credentialcmd/credentialcmd_test.go | 267 ++++++++++++++++++ .../init_credential_destination.go | 119 ++++++++ .../init_reviewer_credential_status.go | 33 ++- 5 files changed, 485 insertions(+), 16 deletions(-) create mode 100644 internal/cmd/credentialcmd/init_credential_destination.go diff --git a/docs/init-ux-contract.md b/docs/init-ux-contract.md index b3afda6..a2c0267 100644 --- a/docs/init-ux-contract.md +++ b/docs/init-ux-contract.md @@ -191,6 +191,25 @@ uses those keys. Final commit remains the only write boundary for staged secret values, and the final readiness summary should continue to report follow-up credential work without leaking values. +All credential-bearing init flows should show equivalent non-secret destination +context before collecting secret values. This includes Git credentials, reviewer +PAT/GitHub App credentials, and LLM API keys handled by the shared credential +collector. + +Destination summaries should include: + +- credential storage ref +- resolved secrets-management profile label/id when a named profile applies +- resolved backend display label, including platform-specific automatic OS + default copy such as `Automatic OS default (macOS Keychain)` +- configured 1Password vault, item title prefix, item tag, item field title, and + other non-secret routing labels when present + +Destination summaries must be non-fatal. If a profile/backend destination cannot +be resolved, the flow should show non-secret unavailable copy and continue to the +existing credential choice. They must never read or display backend token values; +environment variable names may be shown only as backend-auth env var names. + ## Draft-Local Reuse Rules LLM runtimes and reviewer entities are reusable **within the current interactive @@ -300,6 +319,9 @@ Instead: profile is using its Git account or a subscription runtime - any advanced path should still explain that these labels are non-secret pointers to keyring entries, not the secrets themselves +- users who need to change where secrets are stored should configure/select a + secrets-management profile rather than entering GitHub App IDs, private keys, + or API keys in the top-level secrets-management workflow ## Global Settings Area diff --git a/internal/cmd/credentialcmd/credentialcmd.go b/internal/cmd/credentialcmd/credentialcmd.go index d352f4f..db2a3d2 100644 --- a/internal/cmd/credentialcmd/credentialcmd.go +++ b/internal/cmd/credentialcmd/credentialcmd.go @@ -222,6 +222,8 @@ type initPromptContext struct { ExistingProfileNames []string DefaultProfileName string ExistingConfig config.File + BackendArg string + BackendFlagSet bool GitScopes map[string]initGitScopeDraft ProfileGitScopes map[string]string ReviewerEntities map[string]initReviewerEntityDraft @@ -633,6 +635,7 @@ const ( type initCredentialSecretPrompt struct { Entry initCredentialPlanEntry + Destination string TargetHasRequired bool TargetHasAnyKeys bool ClipboardSupported bool @@ -641,6 +644,7 @@ type initCredentialSecretPrompt struct { type initSecretValuePrompt struct { Entry initCredentialPlanEntry Key string + Destination string Optional bool TargetHasKey bool ClipboardSupported bool @@ -3325,13 +3329,18 @@ func (p huhInitSecretPrompter) ChooseCredentialAction(prompt initCredentialSecre huh.NewOption("Back to main menu", initCredentialSecretActionBack), ) choice := options[0].Value + fields := []huh.Field{} + if destination := strings.TrimSpace(prompt.Destination); destination != "" { + fields = append(fields, huh.NewNote().Description(destination)) + } + fields = append(fields, + huh.NewSelect[initCredentialSecretAction](). + Title(initCredentialSecretPromptTitle(prompt)). + Options(options...). + Value(&choice), + ) form := huh.NewForm( - huh.NewGroup( - huh.NewSelect[initCredentialSecretAction](). - Title(initCredentialSecretPromptTitle(prompt)). - Options(options...). - Value(&choice), - ), + huh.NewGroup(fields...), ) back, err := runBackableInitForm(form, p.stdin, p.stderr) if err != nil { @@ -3373,13 +3382,18 @@ func (p huhInitSecretPrompter) ChooseSecretSource(prompt initSecretValuePrompt) } options = append(options, huh.NewOption("Back to credential choices", initSecretSourceBack)) choice := options[0].Value + fields := []huh.Field{} + if destination := strings.TrimSpace(prompt.Destination); destination != "" { + fields = append(fields, huh.NewNote().Description(destination)) + } + fields = append(fields, + huh.NewSelect[initSecretSource](). + Title(initSecretSourcePromptTitle(prompt)). + Options(options...). + Value(&choice), + ) form := huh.NewForm( - huh.NewGroup( - huh.NewSelect[initSecretSource](). - Title(initSecretSourcePromptTitle(prompt)). - Options(options...). - Value(&choice), - ), + huh.NewGroup(fields...), ) back, err := runBackableInitForm(form, p.stdin, p.stderr) if err != nil { @@ -3398,14 +3412,22 @@ func initSecretSourcePromptTitle(prompt initSecretValuePrompt) string { func (p huhInitSecretPrompter) PasteSecret(prompt initSecretValuePrompt) (string, error) { var value string action := initDetailActionEdit + description := "Secret input is hidden." + if destination := strings.TrimSpace(prompt.Destination); destination != "" { + description = destination + "\n" + description + } field := huh.NewInput(). Title(fmt.Sprintf("Paste %s%s", prompt.Key, initSecretsProfilePromptSuffix(prompt.Entry.SecretsProfile))). - Description("Secret input is hidden."). + Description(description). Value(&value). EchoMode(huh.EchoModePassword). Validate(validateRequiredText("secret value is required")) if prompt.Key == credentials.GitHubAppPrivateKeyKey { - field.Description("Secret input is hidden. Clipboard is recommended for multi-line private keys.") + privateKeyDescription := "Secret input is hidden. Clipboard is recommended for multi-line private keys." + if destination := strings.TrimSpace(prompt.Destination); destination != "" { + privateKeyDescription = destination + "\n" + privateKeyDescription + } + field.Description(privateKeyDescription) } form := huh.NewForm( huh.NewGroup( @@ -5742,10 +5764,17 @@ func collectInteractiveInitSecrets(_ *cobra.Command, opts *root.Options, deps in if initCredentialEntryDeferredByDecision(plan.credentialDecisions, entry) { continue } + destination := initCredentialDestinationDescription(initCredentialDestinationContext{ + Entry: entry, + Config: plan.cfg, + BackendArg: plan.backendArg, + BackendFlagSet: plan.backendFlagSet, + }) credentialChoices: for { action, err := prompter.ChooseCredentialAction(initCredentialSecretPrompt{ Entry: entry, + Destination: destination, ClipboardSupported: deps.clipboardSupported(), }) if err != nil { @@ -5778,6 +5807,7 @@ func collectInteractiveInitSecrets(_ *cobra.Command, opts *root.Options, deps in if action == initCredentialSecretActionSetNow && targetHasAnyKeys { action, err = prompter.ChooseCredentialAction(initCredentialSecretPrompt{ Entry: entry, + Destination: destination, TargetHasRequired: targetHasRequired, TargetHasAnyKeys: targetHasAnyKeys, ClipboardSupported: deps.clipboardSupported(), @@ -5818,6 +5848,7 @@ func collectInteractiveInitSecrets(_ *cobra.Command, opts *root.Options, deps in source, err := prompter.ChooseSecretSource(initSecretValuePrompt{ Entry: entry, Key: spec.Key, + Destination: destination, Optional: !spec.Required, TargetHasKey: targetHasKey, ClipboardSupported: deps.clipboardSupported(), @@ -5852,6 +5883,7 @@ func collectInteractiveInitSecrets(_ *cobra.Command, opts *root.Options, deps in value, err := prompter.PasteSecret(initSecretValuePrompt{ Entry: entry, Key: spec.Key, + Destination: destination, Optional: !spec.Required, TargetHasKey: targetHasKey, ClipboardSupported: deps.clipboardSupported(), diff --git a/internal/cmd/credentialcmd/credentialcmd_test.go b/internal/cmd/credentialcmd/credentialcmd_test.go index b09dd1f..5af6739 100644 --- a/internal/cmd/credentialcmd/credentialcmd_test.go +++ b/internal/cmd/credentialcmd/credentialcmd_test.go @@ -3519,6 +3519,86 @@ func TestCollectInteractiveInitSecretsRecordsDraftWritesBeforeApply(t *testing.T } } +func TestCollectInteractiveInitSecretsPassesDestinationToSharedCredentialPrompts(t *testing.T) { + cfg := config.File{ + Secrets: config.SecretsConfig{ + Profiles: map[string]config.SecretsProfile{ + "team-vault": { + Label: "Team Vault", + Backend: config.SecretsProfileBackend{Kind: config.SecretsBackendKind(credstore.BackendFile)}, + }, + }, + }, + } + resolved, err := credentials.ResolveSecretsProfileForProfile(cfg, config.Profile{SecretsProfile: "team-vault"}) + if err != nil { + t.Fatalf("ResolveSecretsProfileForProfile: %v", err) + } + refs := []config.CredentialRef{ + {Purpose: "git", Ref: "codereview/work", Mode: string(config.GitAuthModePAT)}, + {Purpose: "llm", Ref: "codereview/work-llm", Mode: string(config.LLMAuthAPIKey), Provider: string(config.LLMProviderAnthropic)}, + {Purpose: "reviewer_credentials", Ref: "codereview/work-reviewer", Mode: string(config.GitAuthModePAT)}, + } + entries := make([]initCredentialPlanEntry, 0, len(refs)) + for _, ref := range refs { + specs, err := credentials.KeySpecsForPurpose(ref) + if err != nil { + t.Fatalf("KeySpecsForPurpose(%#v): %v", ref, err) + } + entries = append(entries, initCredentialPlanEntry{ + Ref: ref, + SecretsProfile: resolved, + KeySpecs: specs, + MissingRequiredKeys: initCredentialRequiredKeys(initCredentialPlanEntry{KeySpecs: specs}), + State: initCredentialPlanStateMissingRequired, + }) + } + prompter := &fakeInitSecretPrompter{ + actions: []initCredentialSecretAction{ + initCredentialSecretActionSetNow, + initCredentialSecretActionSetNow, + initCredentialSecretActionSetNow, + }, + sources: []initSecretSource{ + initSecretSourcePaste, + initSecretSourcePaste, + initSecretSourcePaste, + }, + pastes: []string{"git-token", "llm-key", "reviewer-token"}, + } + workspace, err := collectInteractiveInitSecrets(&cobra.Command{}, &root.Options{}, initDeps{ + secretPrompter: prompter, + clipboardSupported: func() bool { return false }, + openResolvedStore: func(credentials.ResolvedSecretsProfile, string, bool, config.File) (initStore, error) { + return newFakeInitStore(nil), nil + }, + }, initWorkspaceDraft{ + cfg: cfg, + credentialPlan: entries, + }) + if err != nil { + t.Fatalf("collectInteractiveInitSecrets: %v", err) + } + if len(workspace.writes) != 3 { + t.Fatalf("writes = %#v, want three staged credential refs", workspace.writes) + } + for _, prompt := range prompter.actionPrompts { + if !strings.Contains(prompt.Destination, "Destination: "+prompt.Entry.Ref.Ref+" via Team Vault (Encrypted file)") { + t.Fatalf("action prompt destination = %q for %#v", prompt.Destination, prompt.Entry.Ref) + } + } + for _, prompt := range prompter.sourcePrompts { + if !strings.Contains(prompt.Destination, "Destination: "+prompt.Entry.Ref.Ref+" via Team Vault (Encrypted file)") { + t.Fatalf("source prompt destination = %q for %#v", prompt.Destination, prompt.Entry.Ref) + } + } + for _, prompt := range prompter.pastePrompts { + if !strings.Contains(prompt.Destination, "Destination: "+prompt.Entry.Ref.Ref+" via Team Vault (Encrypted file)") { + t.Fatalf("paste prompt destination = %q for %#v", prompt.Destination, prompt.Entry.Ref) + } + } +} + func TestCollectInteractiveInitSecretsSourceBackReturnsToCredentialChoices(t *testing.T) { store := newFakeInitStore(nil) prompter := &fakeInitSecretPrompter{ @@ -14122,6 +14202,193 @@ func TestInitCredentialSecretPromptTitleReviewerKeys(t *testing.T) { } } +func TestInitCredentialDestinationDescriptionLegacyAutoUsesPlatformCopy(t *testing.T) { + t.Setenv(credentials.BackendEnvVar(), "") + entry := initCredentialPlanEntry{ + Ref: config.CredentialRef{Purpose: "git", Ref: "codereview/work"}, + SecretsProfile: credentials.ResolvedSecretsProfile{ + ID: config.LegacyProjectedSecretsProfileID, + Label: "Legacy default", + Backend: config.ProjectedLegacySecretsBackendKind, + Source: config.EffectiveSecretsProfileSourceProjectedLegacy, + SelectionSource: credentials.SecretsProfileSelectionLegacyDefault, + }, + } + + description := initCredentialDestinationDescription(initCredentialDestinationContext{ + Entry: entry, + Config: config.File{ + Profiles: map[string]config.Profile{"work": basicProfile("work")}, + }, + }) + + for _, want := range []string{ + "Destination: codereview/work via Legacy default", + initAutomaticOSDefaultSecretsBackendLabel(), + "Change destination by configuring/selecting a secrets-management profile", + } { + if !strings.Contains(description, want) { + t.Fatalf("description = %q, want %q", description, want) + } + } +} + +func TestInitCredentialDestinationDescriptionNamedOnePasswordShowsRoutingWithoutTokenValues(t *testing.T) { + serviceTokenEnv := strings.Join([]string{"OP_SERVICE_ACCOUNT", "TOKEN"}, "_") + connectTokenEnv := strings.Join([]string{"OP_CONNECT", "TOKEN"}, "_") + t.Setenv(serviceTokenEnv, "sentinel-service-token-value") + t.Setenv(connectTokenEnv, "sentinel-connect-token-value") + cfg := config.File{ + Secrets: config.SecretsConfig{ + Profiles: map[string]config.SecretsProfile{ + "team-vault": { + Label: "Team Vault", + Backend: config.SecretsProfileBackend{ + Kind: config.SecretsBackendKind(credstore.BackendOP), + OnePassword: &config.SecretsProfileOnePasswordConfig{ + VaultID: "Engineering", + ItemTitlePrefix: "cr-", + ItemTag: "code-review", + ItemFieldTitle: "credential", + ServiceTokenEnv: serviceTokenEnv, + }, + }, + }, + }, + }, + } + resolved, err := credentials.ResolveSecretsProfileForProfile(cfg, config.Profile{SecretsProfile: "team-vault"}) + if err != nil { + t.Fatalf("ResolveSecretsProfileForProfile: %v", err) + } + + description := initCredentialDestinationDescription(initCredentialDestinationContext{ + Entry: initCredentialPlanEntry{ + Ref: config.CredentialRef{Purpose: "reviewer_credentials", Ref: "codereview/rianjs-bot", Mode: string(config.GitAuthModePAT)}, + SecretsProfile: resolved, + }, + Config: cfg, + }) + + for _, want := range []string{ + "Destination: codereview/rianjs-bot via Team Vault (1Password service account)", + "Secrets profile: team-vault", + "1Password vault: Engineering", + "1Password item title prefix: cr-", + "1Password item tag: code-review", + "1Password item field title: credential", + "1Password backend auth env var: " + serviceTokenEnv, + } { + if !strings.Contains(description, want) { + t.Fatalf("description = %q, want %q", description, want) + } + } + for _, leaked := range []string{"sentinel-service-token-value", "sentinel-connect-token-value"} { + if strings.Contains(description, leaked) { + t.Fatalf("description leaked token value %q: %s", leaked, description) + } + } +} + +func TestInitCredentialDestinationDescriptionOnePasswordConnectDoesNotReadTokenValue(t *testing.T) { + connectTokenEnv := strings.Join([]string{"CUSTOM_CONNECT", "TOKEN"}, "_") + t.Setenv(connectTokenEnv, "sentinel-connect-token-value") + cfg := config.File{ + Secrets: config.SecretsConfig{ + Profiles: map[string]config.SecretsProfile{ + "connect-vault": { + Label: "Connect Vault", + Backend: config.SecretsProfileBackend{ + Kind: config.SecretsBackendKind(credstore.BackendOPConnect), + OnePassword: &config.SecretsProfileOnePasswordConfig{ + VaultID: "Engineering", + ConnectHost: "https://connect.example", + ConnectTokenEnv: connectTokenEnv, + }, + }, + }, + }, + }, + } + resolved, err := credentials.ResolveSecretsProfileForProfile(cfg, config.Profile{SecretsProfile: "connect-vault"}) + if err != nil { + t.Fatalf("ResolveSecretsProfileForProfile: %v", err) + } + + description := initCredentialDestinationDescription(initCredentialDestinationContext{ + Entry: initCredentialPlanEntry{ + Ref: config.CredentialRef{Purpose: "llm", Ref: "codereview/work-llm", Mode: string(config.LLMAuthAPIKey), Provider: string(config.LLMProviderAnthropic)}, + SecretsProfile: resolved, + }, + Config: cfg, + }) + + for _, want := range []string{ + "Destination: codereview/work-llm via Connect Vault (1Password Connect)", + "1Password Connect host: https://connect.example", + "1Password backend auth env var: " + connectTokenEnv, + } { + if !strings.Contains(description, want) { + t.Fatalf("description = %q, want %q", description, want) + } + } + if strings.Contains(description, "sentinel-connect-token-value") { + t.Fatalf("description leaked Connect token value: %s", description) + } +} + +func TestInitCredentialDestinationDescriptionUnavailableIsNonFatal(t *testing.T) { + description := initCredentialDestinationDescription(initCredentialDestinationContext{ + Entry: initCredentialPlanEntry{ + Ref: config.CredentialRef{Purpose: "llm", Ref: "codereview/work-llm"}, + SecretsProfile: credentials.ResolvedSecretsProfile{ + ID: "missing-profile", + Label: "Missing Profile", + Backend: string(credstore.BackendOPConnect), + Source: config.EffectiveSecretsProfileSourceConfigured, + }, + }, + Config: config.File{}, + }) + + for _, want := range []string{ + "Destination: codereview/work-llm", + "credential destination unavailable", + "Change destination by configuring/selecting a secrets-management profile", + } { + if !strings.Contains(description, want) { + t.Fatalf("description = %q, want %q", description, want) + } + } +} + +func TestHuhInitSecretPrompterAccessibleShowsCredentialDestination(t *testing.T) { + t.Setenv("TERM", "dumb") + var stderr bytes.Buffer + prompter := huhInitSecretPrompter{ + stdin: strings.NewReader("\n"), + stderr: &stderr, + } + _, err := prompter.ChooseCredentialAction(initCredentialSecretPrompt{ + Entry: initCredentialPlanEntry{ + Ref: config.CredentialRef{Purpose: "git", Ref: "codereview/work"}, + SecretsProfile: credentials.ResolvedSecretsProfile{ + ID: "team-vault", + Label: "Team Vault", + Backend: string(credstore.BackendFile), + Source: config.EffectiveSecretsProfileSourceConfigured, + }, + }, + Destination: "Destination: codereview/work via Team Vault (Encrypted file)", + }) + if err != nil { + t.Fatalf("ChooseCredentialAction: %v", err) + } + if got := stderr.String(); !strings.Contains(got, "Destination: codereview/work via Team Vault (Encrypted file)") { + t.Fatalf("stderr = %q, want credential destination note", got) + } +} + func TestInitReviewerCredentialStatusStates(t *testing.T) { entry := initCredentialPlanEntry{ Ref: config.CredentialRef{ diff --git a/internal/cmd/credentialcmd/init_credential_destination.go b/internal/cmd/credentialcmd/init_credential_destination.go new file mode 100644 index 0000000..486be00 --- /dev/null +++ b/internal/cmd/credentialcmd/init_credential_destination.go @@ -0,0 +1,119 @@ +package credentialcmd + +import ( + "fmt" + "strings" + + "github.com/open-cli-collective/cli-common/credstore" + + "github.com/open-cli-collective/codereview-cli/internal/config" + "github.com/open-cli-collective/codereview-cli/internal/credentials" +) + +type initCredentialDestinationContext struct { + Entry initCredentialPlanEntry + Config config.File + BackendArg string + BackendFlagSet bool +} + +func initCredentialDestinationDescription(ctx initCredentialDestinationContext) string { + ref := strings.TrimSpace(ctx.Entry.Ref.Ref) + if ref == "" { + ref = "(standard credential location)" + } + destination, details := initCredentialDestinationDetails(ctx, ref) + lines := []string{"Destination: " + destination} + lines = append(lines, details...) + lines = append(lines, "Change destination by configuring/selecting a secrets-management profile; secret values are collected separately.") + return strings.Join(lines, "\n") +} + +func initCredentialDestinationDetails(ctx initCredentialDestinationContext, ref string) (string, []string) { + resolved := ctx.Entry.SecretsProfile + if resolved.IsNamed() { + return initNamedCredentialDestinationDetails(ctx, ref) + } + return initLegacyCredentialDestinationDetails(ctx, ref) +} + +func initNamedCredentialDestinationDetails(ctx initCredentialDestinationContext, ref string) (string, []string) { + resolved := ctx.Entry.SecretsProfile + displayName := strings.TrimSpace(resolved.DisplayName()) + if displayName == "" { + displayName = "selected secrets-management profile" + } + profile, ok := ctx.Config.Secrets.Profiles[strings.TrimSpace(resolved.ID)] + if !ok { + return fmt.Sprintf("%s via %s", ref, displayName), []string{"credential destination unavailable."} + } + backendKind := profile.Backend.Kind + if strings.TrimSpace(string(backendKind)) == "" { + backendKind = config.SecretsBackendKind(resolved.Backend) + } + backendLabel := initSecretsBackendDisplayLabel(backendKind) + lines := []string{} + if strings.TrimSpace(resolved.ID) != "" { + lines = append(lines, "Secrets profile: "+strings.TrimSpace(resolved.ID)) + } + if strings.TrimSpace(string(backendKind)) != "" { + lines = append(lines, "Backend kind: "+strings.TrimSpace(string(backendKind))) + } + lines = append(lines, initOnePasswordDestinationDetails(profile.Backend)...) + return fmt.Sprintf("%s via %s (%s)", ref, displayName, backendLabel), lines +} + +func initLegacyCredentialDestinationDetails(ctx initCredentialDestinationContext, ref string) (string, []string) { + displayName := strings.TrimSpace(ctx.Entry.SecretsProfile.DisplayName()) + if displayName == "" { + displayName = "Legacy default" + } + backend, source, err := credentials.BackendMetadata(ctx.BackendArg, ctx.BackendFlagSet, ctx.Config) + if err != nil { + return fmt.Sprintf("%s via %s", ref, displayName), []string{"credential destination unavailable."} + } + return fmt.Sprintf("%s via %s (%s)", ref, displayName, initCredentialBackendMetadataLabel(backend, source)), nil +} + +func initCredentialBackendMetadataLabel(backend credstore.Backend, source credstore.Source) string { + if source == credstore.SourceAuto { + return initAutomaticOSDefaultSecretsBackendLabel() + } + if strings.TrimSpace(string(backend)) == "" { + return initAutomaticOSDefaultSecretsBackendLabel() + } + return initSecretsBackendDisplayLabel(config.SecretsBackendKind(backend)) +} + +func initOnePasswordDestinationDetails(backend config.SecretsProfileBackend) []string { + if !config.IsOnePasswordSecretsBackend(backend.Kind) || backend.OnePassword == nil { + return nil + } + onePassword := backend.OnePassword + lines := []string{} + if value := strings.TrimSpace(onePassword.VaultID); value != "" { + lines = append(lines, "1Password vault: "+value) + } + if value := strings.TrimSpace(onePassword.ItemTitlePrefix); value != "" { + lines = append(lines, "1Password item title prefix: "+value) + } + if value := strings.TrimSpace(onePassword.ItemTag); value != "" { + lines = append(lines, "1Password item tag: "+value) + } + if value := strings.TrimSpace(onePassword.ItemFieldTitle); value != "" { + lines = append(lines, "1Password item field title: "+value) + } + if value := strings.TrimSpace(onePassword.ConnectHost); value != "" { + lines = append(lines, "1Password Connect host: "+value) + } + if value := strings.TrimSpace(onePassword.ServiceTokenEnv); value != "" { + lines = append(lines, "1Password backend auth env var: "+value) + } + if value := strings.TrimSpace(onePassword.ConnectTokenEnv); value != "" { + lines = append(lines, "1Password backend auth env var: "+value) + } + if value := strings.TrimSpace(onePassword.DesktopAccountID); value != "" { + lines = append(lines, "1Password desktop account id: "+value) + } + return lines +} diff --git a/internal/cmd/credentialcmd/init_reviewer_credential_status.go b/internal/cmd/credentialcmd/init_reviewer_credential_status.go index 36dfc00..85b4ad2 100644 --- a/internal/cmd/credentialcmd/init_reviewer_credential_status.go +++ b/internal/cmd/credentialcmd/init_reviewer_credential_status.go @@ -24,6 +24,7 @@ const ( type initReviewerCredentialStatus struct { Ref config.CredentialRef SecretsProfile credentials.ResolvedSecretsProfile + Destination string Keys []initReviewerCredentialKeyStatus Unavailable string } @@ -36,6 +37,10 @@ type initReviewerCredentialKeyStatus struct { func currentInteractiveInitReviewerEntityPromptContext(opts *root.Options, deps initDeps, session initSessionDraft) initPromptContext { ctx := currentInteractiveInitInventoryPromptContext(session) + if opts != nil { + ctx.BackendArg = opts.Backend + } + ctx.BackendFlagSet = session.backendFlagSet ctx.ReviewerCredentialStatuses = buildInteractiveInitReviewerCredentialStatuses(opts, deps, session) return ctx } @@ -84,7 +89,18 @@ func buildInteractiveInitReviewerCredentialStatuses(opts *root.Options, deps ini existing = keys } } - statuses = append(statuses, initReviewerCredentialStatusFromEntry(entry, session.writes[entry.Ref.Ref], session.credentialDecisions, existing, unavailable)) + status := initReviewerCredentialStatusFromEntry(entry, session.writes[entry.Ref.Ref], session.credentialDecisions, existing, unavailable) + backendArg := "" + if opts != nil { + backendArg = opts.Backend + } + status.Destination = initCredentialDestinationDescription(initCredentialDestinationContext{ + Entry: entry, + Config: session.cfg, + BackendArg: backendArg, + BackendFlagSet: session.backendFlagSet, + }) + statuses = append(statuses, status) } return statuses } @@ -277,6 +293,15 @@ func synthesizeReviewerCredentialStatus(ctx initPromptContext, ref config.Creden State: keyState, }) } + status.Destination = initCredentialDestinationDescription(initCredentialDestinationContext{ + Entry: initCredentialPlanEntry{ + Ref: ref, + SecretsProfile: status.SecretsProfile, + }, + Config: ctx.ExistingConfig, + BackendArg: ctx.BackendArg, + BackendFlagSet: ctx.BackendFlagSet, + }) return status } @@ -302,7 +327,11 @@ func reviewerCredentialAuthModeForKind(kind initReviewerEntityKind) config.GitAu func initReviewerCredentialStatusDescription(status initReviewerCredentialStatus) string { var lines []string - lines = append(lines, "Destination: "+initReviewerCredentialDestinationDescription(status)) + destination := strings.TrimSpace(status.Destination) + if destination == "" { + destination = "Destination: " + initReviewerCredentialDestinationDescription(status) + } + lines = append(lines, strings.Split(destination, "\n")...) if strings.TrimSpace(status.Unavailable) != "" { lines = append(lines, strings.TrimSpace(status.Unavailable)+".") } From a3b6296f9e5503d5e1ba18479ffe61d7fc59a671 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 18 Jun 2026 18:16:49 -0400 Subject: [PATCH 2/4] fix(init): use raw backend for destination summaries --- internal/cmd/credentialcmd/credentialcmd.go | 6 +- .../cmd/credentialcmd/credentialcmd_test.go | 82 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/internal/cmd/credentialcmd/credentialcmd.go b/internal/cmd/credentialcmd/credentialcmd.go index db2a3d2..16f48d8 100644 --- a/internal/cmd/credentialcmd/credentialcmd.go +++ b/internal/cmd/credentialcmd/credentialcmd.go @@ -5764,10 +5764,14 @@ func collectInteractiveInitSecrets(_ *cobra.Command, opts *root.Options, deps in if initCredentialEntryDeferredByDecision(plan.credentialDecisions, entry) { continue } + destinationBackend := "" + if opts != nil { + destinationBackend = opts.Backend + } destination := initCredentialDestinationDescription(initCredentialDestinationContext{ Entry: entry, Config: plan.cfg, - BackendArg: plan.backendArg, + BackendArg: destinationBackend, BackendFlagSet: plan.backendFlagSet, }) credentialChoices: diff --git a/internal/cmd/credentialcmd/credentialcmd_test.go b/internal/cmd/credentialcmd/credentialcmd_test.go index 5af6739..3d76ca0 100644 --- a/internal/cmd/credentialcmd/credentialcmd_test.go +++ b/internal/cmd/credentialcmd/credentialcmd_test.go @@ -3599,6 +3599,47 @@ func TestCollectInteractiveInitSecretsPassesDestinationToSharedCredentialPrompts } } +func TestCollectInteractiveInitSecretsDestinationUsesRawRuntimeBackend(t *testing.T) { + t.Setenv(credentials.BackendEnvVar(), "") + prompter := &fakeInitSecretPrompter{ + actions: []initCredentialSecretAction{initCredentialSecretActionDefer}, + } + _, err := collectInteractiveInitSecrets(&cobra.Command{}, &root.Options{Backend: string(credstore.BackendMemory)}, initDeps{ + secretPrompter: prompter, + clipboardSupported: func() bool { return false }, + }, initWorkspaceDraft{ + cfg: config.File{}, + credentialPlan: []initCredentialPlanEntry{{ + Ref: config.CredentialRef{Purpose: "git", Ref: "codereview/work", Mode: string(config.GitAuthModePAT)}, + SecretsProfile: credentials.ResolvedSecretsProfile{ + ID: config.LegacyProjectedSecretsProfileID, + Label: "Legacy default", + Backend: config.ProjectedLegacySecretsBackendKind, + Source: config.EffectiveSecretsProfileSourceProjectedLegacy, + SelectionSource: credentials.SecretsProfileSelectionLegacyDefault, + }, + KeySpecs: []credentials.KeySpec{{Key: credentials.GitTokenKey, Required: true}}, + MissingRequiredKeys: []string{credentials.GitTokenKey}, + State: initCredentialPlanStateMissingRequired, + }}, + backendFlagSet: true, + backendArg: " --backend memory", + }) + if err != nil { + t.Fatalf("collectInteractiveInitSecrets: %v", err) + } + if len(prompter.actionPrompts) != 1 { + t.Fatalf("action prompts = %d, want 1", len(prompter.actionPrompts)) + } + destination := prompter.actionPrompts[0].Destination + if !strings.Contains(destination, "Destination: codereview/work via Legacy default (In-memory store)") { + t.Fatalf("destination = %q, want raw runtime backend metadata", destination) + } + if strings.Contains(destination, "credential destination unavailable") { + t.Fatalf("destination = %q, want available runtime backend summary", destination) + } +} + func TestCollectInteractiveInitSecretsSourceBackReturnsToCredentialChoices(t *testing.T) { store := newFakeInitStore(nil) prompter := &fakeInitSecretPrompter{ @@ -14337,6 +14378,47 @@ func TestInitCredentialDestinationDescriptionOnePasswordConnectDoesNotReadTokenV } } +func TestInitCredentialDestinationDescriptionOnePasswordDesktopShowsAccountID(t *testing.T) { + cfg := config.File{ + Secrets: config.SecretsConfig{ + Profiles: map[string]config.SecretsProfile{ + "desktop-vault": { + Label: "Desktop Vault", + Backend: config.SecretsProfileBackend{ + Kind: config.SecretsBackendKind(credstore.BackendOPDesktop), + OnePassword: &config.SecretsProfileOnePasswordConfig{ + VaultID: "Engineering", + DesktopAccountID: "account-123", + }, + }, + }, + }, + }, + } + resolved, err := credentials.ResolveSecretsProfileForProfile(cfg, config.Profile{SecretsProfile: "desktop-vault"}) + if err != nil { + t.Fatalf("ResolveSecretsProfileForProfile: %v", err) + } + + description := initCredentialDestinationDescription(initCredentialDestinationContext{ + Entry: initCredentialPlanEntry{ + Ref: config.CredentialRef{Purpose: "git", Ref: "codereview/work", Mode: string(config.GitAuthModePAT)}, + SecretsProfile: resolved, + }, + Config: cfg, + }) + + for _, want := range []string{ + "Destination: codereview/work via Desktop Vault (1Password desktop app)", + "1Password vault: Engineering", + "1Password desktop account id: account-123", + } { + if !strings.Contains(description, want) { + t.Fatalf("description = %q, want %q", description, want) + } + } +} + func TestInitCredentialDestinationDescriptionUnavailableIsNonFatal(t *testing.T) { description := initCredentialDestinationDescription(initCredentialDestinationContext{ Entry: initCredentialPlanEntry{ From 5dbef42bb99b681a2f292177a43c6590f9a36cf7 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 18 Jun 2026 18:21:27 -0400 Subject: [PATCH 3/4] test(init): cover credential destination prompt paths --- internal/cmd/credentialcmd/credentialcmd.go | 24 ++-- .../cmd/credentialcmd/credentialcmd_test.go | 103 +++++++++++++++++- 2 files changed, 112 insertions(+), 15 deletions(-) diff --git a/internal/cmd/credentialcmd/credentialcmd.go b/internal/cmd/credentialcmd/credentialcmd.go index 16f48d8..ac63b6f 100644 --- a/internal/cmd/credentialcmd/credentialcmd.go +++ b/internal/cmd/credentialcmd/credentialcmd.go @@ -3412,23 +3412,12 @@ func initSecretSourcePromptTitle(prompt initSecretValuePrompt) string { func (p huhInitSecretPrompter) PasteSecret(prompt initSecretValuePrompt) (string, error) { var value string action := initDetailActionEdit - description := "Secret input is hidden." - if destination := strings.TrimSpace(prompt.Destination); destination != "" { - description = destination + "\n" + description - } field := huh.NewInput(). Title(fmt.Sprintf("Paste %s%s", prompt.Key, initSecretsProfilePromptSuffix(prompt.Entry.SecretsProfile))). - Description(description). + Description(initSecretPasteDescription(prompt)). Value(&value). EchoMode(huh.EchoModePassword). Validate(validateRequiredText("secret value is required")) - if prompt.Key == credentials.GitHubAppPrivateKeyKey { - privateKeyDescription := "Secret input is hidden. Clipboard is recommended for multi-line private keys." - if destination := strings.TrimSpace(prompt.Destination); destination != "" { - privateKeyDescription = destination + "\n" + privateKeyDescription - } - field.Description(privateKeyDescription) - } form := huh.NewForm( huh.NewGroup( huh.NewSelect[string](). @@ -6847,6 +6836,17 @@ func readSecretIngress(r io.Reader, stdin bool, envVar, stdinFlag, envFlag strin return value, nil } +func initSecretPasteDescription(prompt initSecretValuePrompt) string { + description := "Secret input is hidden." + if prompt.Key == credentials.GitHubAppPrivateKeyKey { + description = "Secret input is hidden. Clipboard is recommended for multi-line private keys." + } + if destination := strings.TrimSpace(prompt.Destination); destination != "" { + description = destination + "\n" + description + } + return description +} + func readOptionalSecretIngress(r io.Reader, stdin bool, envVar, stdinFlag, envFlag string) (string, bool, error) { if stdin && envVar != "" { return "", false, fmt.Errorf("only one of %s or %s may be set", stdinFlag, envFlag) diff --git a/internal/cmd/credentialcmd/credentialcmd_test.go b/internal/cmd/credentialcmd/credentialcmd_test.go index 3d76ca0..4a5eb62 100644 --- a/internal/cmd/credentialcmd/credentialcmd_test.go +++ b/internal/cmd/credentialcmd/credentialcmd_test.go @@ -3640,6 +3640,45 @@ func TestCollectInteractiveInitSecretsDestinationUsesRawRuntimeBackend(t *testin } } +func TestCollectInteractiveInitSecretsDestinationUsesInferredDefaultBackend(t *testing.T) { + t.Setenv(credentials.BackendEnvVar(), "") + prompter := &fakeInitSecretPrompter{ + actions: []initCredentialSecretAction{initCredentialSecretActionDefer}, + } + _, err := collectInteractiveInitSecrets(&cobra.Command{}, &root.Options{}, initDeps{ + secretPrompter: prompter, + clipboardSupported: func() bool { return false }, + }, initWorkspaceDraft{ + cfg: config.File{}, + credentialPlan: []initCredentialPlanEntry{{ + Ref: config.CredentialRef{Purpose: "git", Ref: "codereview/work", Mode: string(config.GitAuthModePAT)}, + SecretsProfile: credentials.ResolvedSecretsProfile{ + ID: config.LegacyProjectedSecretsProfileID, + Label: "Legacy default", + Backend: config.ProjectedLegacySecretsBackendKind, + Source: config.EffectiveSecretsProfileSourceProjectedLegacy, + SelectionSource: credentials.SecretsProfileSelectionLegacyDefault, + }, + KeySpecs: []credentials.KeySpec{{Key: credentials.GitTokenKey, Required: true}}, + MissingRequiredKeys: []string{credentials.GitTokenKey}, + State: initCredentialPlanStateMissingRequired, + }}, + }) + if err != nil { + t.Fatalf("collectInteractiveInitSecrets: %v", err) + } + if len(prompter.actionPrompts) != 1 { + t.Fatalf("action prompts = %d, want 1", len(prompter.actionPrompts)) + } + destination := prompter.actionPrompts[0].Destination + if !strings.Contains(destination, initAutomaticOSDefaultSecretsBackendLabel()) { + t.Fatalf("destination = %q, want inferred automatic backend copy", destination) + } + if strings.Contains(destination, "credential destination unavailable") { + t.Fatalf("destination = %q, want available inferred backend summary", destination) + } +} + func TestCollectInteractiveInitSecretsSourceBackReturnsToCredentialChoices(t *testing.T) { store := newFakeInitStore(nil) prompter := &fakeInitSecretPrompter{ @@ -14471,6 +14510,44 @@ func TestHuhInitSecretPrompterAccessibleShowsCredentialDestination(t *testing.T) } } +func TestHuhInitSecretPrompterAccessibleSecretSourceShowsCredentialDestination(t *testing.T) { + t.Setenv("TERM", "dumb") + var stderr bytes.Buffer + prompter := huhInitSecretPrompter{ + stdin: strings.NewReader("\n"), + stderr: &stderr, + } + _, err := prompter.ChooseSecretSource(initSecretValuePrompt{ + Entry: initCredentialPlanEntry{ + Ref: config.CredentialRef{Purpose: "git", Ref: "codereview/work"}, + }, + Key: credentials.GitTokenKey, + Destination: "Destination: codereview/work via Team Vault (Encrypted file)", + }) + if err != nil { + t.Fatalf("ChooseSecretSource: %v", err) + } + if got := stderr.String(); !strings.Contains(got, "Destination: codereview/work via Team Vault (Encrypted file)") { + t.Fatalf("stderr = %q, want credential destination note", got) + } +} + +func TestInitSecretPasteDescriptionKeepsDestinationForGitHubAppPrivateKey(t *testing.T) { + description := initSecretPasteDescription(initSecretValuePrompt{ + Key: credentials.GitHubAppPrivateKeyKey, + Destination: "Destination: codereview/rianjs-bot via Team Vault (Encrypted file)", + }) + + for _, want := range []string{ + "Destination: codereview/rianjs-bot via Team Vault (Encrypted file)", + "Clipboard is recommended for multi-line private keys", + } { + if !strings.Contains(description, want) { + t.Fatalf("description = %q, want %q", description, want) + } + } +} + func TestInitReviewerCredentialStatusStates(t *testing.T) { entry := initCredentialPlanEntry{ Ref: config.CredentialRef{ @@ -14629,8 +14706,17 @@ func TestInitReviewerCredentialStatusShowsExistingPATAndSecretsProfileDestinatio DefaultProfile: "work-1password", Profiles: map[string]config.SecretsProfile{ "work-1password": { - Label: "Work Vault", - Backend: config.SecretsProfileBackend{Kind: config.SecretsBackendKind(credstore.BackendOPDesktop)}, + Label: "Work Vault", + Backend: config.SecretsProfileBackend{ + Kind: config.SecretsBackendKind(credstore.BackendOPDesktop), + OnePassword: &config.SecretsProfileOnePasswordConfig{ + VaultID: "Engineering", + ItemTitlePrefix: "cr-", + ItemTag: "code-review", + ItemFieldTitle: "credential", + DesktopAccountID: "account-123", + }, + }, }, }, }, @@ -14667,7 +14753,18 @@ func TestInitReviewerCredentialStatusShowsExistingPATAndSecretsProfileDestinatio t.Fatalf("opened secrets profiles = %#v, want work-1password", opened) } description := initReviewerCredentialStatusDescription(status) - for _, want := range []string{"Destination: codereview/work-reviewer via Work Vault", string(credstore.BackendOPDesktop)} { + if strings.TrimSpace(status.Destination) == "" { + t.Fatalf("status.Destination is empty; description fell back to legacy formatter: %q", description) + } + for _, want := range []string{ + "Destination: codereview/work-reviewer via Work Vault (1Password desktop app)", + string(credstore.BackendOPDesktop), + "1Password vault: Engineering", + "1Password item title prefix: cr-", + "1Password item tag: code-review", + "1Password item field title: credential", + "1Password desktop account id: account-123", + } { if !strings.Contains(description, want) { t.Fatalf("description = %q, want %q", description, want) } From 8afb1c360865dc00f1c0b48a84cfcd1d895a5466 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 18 Jun 2026 18:26:18 -0400 Subject: [PATCH 4/4] chore(init): clarify destination summary labels --- internal/cmd/credentialcmd/credentialcmd.go | 8 ++++---- internal/cmd/credentialcmd/credentialcmd_test.go | 4 ++-- .../cmd/credentialcmd/init_credential_destination.go | 12 ++++++------ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/cmd/credentialcmd/credentialcmd.go b/internal/cmd/credentialcmd/credentialcmd.go index ac63b6f..75ed048 100644 --- a/internal/cmd/credentialcmd/credentialcmd.go +++ b/internal/cmd/credentialcmd/credentialcmd.go @@ -6837,14 +6837,14 @@ func readSecretIngress(r io.Reader, stdin bool, envVar, stdinFlag, envFlag strin } func initSecretPasteDescription(prompt initSecretValuePrompt) string { - description := "Secret input is hidden." + hiddenInputNotice := "Secret input is hidden." if prompt.Key == credentials.GitHubAppPrivateKeyKey { - description = "Secret input is hidden. Clipboard is recommended for multi-line private keys." + hiddenInputNotice = "Secret input is hidden. Clipboard is recommended for multi-line private keys." } if destination := strings.TrimSpace(prompt.Destination); destination != "" { - description = destination + "\n" + description + return destination + "\n" + hiddenInputNotice } - return description + return hiddenInputNotice } func readOptionalSecretIngress(r io.Reader, stdin bool, envVar, stdinFlag, envFlag string) (string, bool, error) { diff --git a/internal/cmd/credentialcmd/credentialcmd_test.go b/internal/cmd/credentialcmd/credentialcmd_test.go index 4a5eb62..bf4e697 100644 --- a/internal/cmd/credentialcmd/credentialcmd_test.go +++ b/internal/cmd/credentialcmd/credentialcmd_test.go @@ -14357,7 +14357,7 @@ func TestInitCredentialDestinationDescriptionNamedOnePasswordShowsRoutingWithout "1Password item title prefix: cr-", "1Password item tag: code-review", "1Password item field title: credential", - "1Password backend auth env var: " + serviceTokenEnv, + "1Password service account token env var: " + serviceTokenEnv, } { if !strings.Contains(description, want) { t.Fatalf("description = %q, want %q", description, want) @@ -14406,7 +14406,7 @@ func TestInitCredentialDestinationDescriptionOnePasswordConnectDoesNotReadTokenV for _, want := range []string{ "Destination: codereview/work-llm via Connect Vault (1Password Connect)", "1Password Connect host: https://connect.example", - "1Password backend auth env var: " + connectTokenEnv, + "1Password Connect token env var: " + connectTokenEnv, } { if !strings.Contains(description, want) { t.Fatalf("description = %q, want %q", description, want) diff --git a/internal/cmd/credentialcmd/init_credential_destination.go b/internal/cmd/credentialcmd/init_credential_destination.go index 486be00..80c01db 100644 --- a/internal/cmd/credentialcmd/init_credential_destination.go +++ b/internal/cmd/credentialcmd/init_credential_destination.go @@ -53,11 +53,11 @@ func initNamedCredentialDestinationDetails(ctx initCredentialDestinationContext, } backendLabel := initSecretsBackendDisplayLabel(backendKind) lines := []string{} - if strings.TrimSpace(resolved.ID) != "" { - lines = append(lines, "Secrets profile: "+strings.TrimSpace(resolved.ID)) + if value := strings.TrimSpace(resolved.ID); value != "" { + lines = append(lines, "Secrets profile: "+value) } - if strings.TrimSpace(string(backendKind)) != "" { - lines = append(lines, "Backend kind: "+strings.TrimSpace(string(backendKind))) + if value := strings.TrimSpace(string(backendKind)); value != "" { + lines = append(lines, "Backend kind: "+value) } lines = append(lines, initOnePasswordDestinationDetails(profile.Backend)...) return fmt.Sprintf("%s via %s (%s)", ref, displayName, backendLabel), lines @@ -107,10 +107,10 @@ func initOnePasswordDestinationDetails(backend config.SecretsProfileBackend) []s lines = append(lines, "1Password Connect host: "+value) } if value := strings.TrimSpace(onePassword.ServiceTokenEnv); value != "" { - lines = append(lines, "1Password backend auth env var: "+value) + lines = append(lines, "1Password service account token env var: "+value) } if value := strings.TrimSpace(onePassword.ConnectTokenEnv); value != "" { - lines = append(lines, "1Password backend auth env var: "+value) + lines = append(lines, "1Password Connect token env var: "+value) } if value := strings.TrimSpace(onePassword.DesktopAccountID); value != "" { lines = append(lines, "1Password desktop account id: "+value)