diff --git a/cmd/config/set_app_secret.go b/cmd/config/set_app_secret.go index 19040d460..2360c552a 100644 --- a/cmd/config/set_app_secret.go +++ b/cmd/config/set_app_secret.go @@ -159,17 +159,42 @@ func setAppSecretNonInteractive(f *cmdutil.Factory, opts *SetAppSecretOptions, m // ── Interactive (human at a TTY) path ──────────────────────────────────────── +// secretPrompter supplies the three interactive steps (profile pick, confirm, +// secret entry). The default implementation is backed by huh; tests inject fakes +// to drive runInteractive's orchestration without a real terminal. +type secretPrompter struct { + selectProfile func(multi *core.MultiAppConfig, profileOverride string) (*core.AppConfig, error) + confirm func(target *errs.ErrTarget) (bool, error) + readInput func() (string, error) +} + +// defaultSecretPrompter wires the huh-backed interactive steps. +func defaultSecretPrompter() secretPrompter { + return secretPrompter{ + selectProfile: selectTargetProfile, + confirm: confirmRotate, + readInput: promptHiddenInput, + } +} + // setAppSecretInteractive walks a human through profile selection, an explicit // confirmation, and a hidden secret entry, then runs the same verify-before-write -// and storage logic. Failures are rendered as readable lines (not JSON). +// and storage logic. func setAppSecretInteractive(f *cmdutil.Factory, multi *core.MultiAppConfig, profileOverride string) error { - app, err := selectTargetProfile(multi, profileOverride) + return runInteractive(f, multi, profileOverride, defaultSecretPrompter()) +} + +// runInteractive orchestrates the interactive flow against the supplied prompter, +// then runs the same verify-before-write / storage logic as the agent path. +// Failures are rendered as readable lines (not the agent JSON envelope). +func runInteractive(f *cmdutil.Factory, multi *core.MultiAppConfig, profileOverride string, p secretPrompter) error { + app, err := p.selectProfile(multi, profileOverride) if err != nil { return err } target := buildTarget(multi, app) - confirmed, err := confirmRotate(target) + confirmed, err := p.confirm(target) if err != nil { return err } @@ -178,7 +203,7 @@ func setAppSecretInteractive(f *cmdutil.Factory, multi *core.MultiAppConfig, pro return nil // explicit decline is a clean no-op (exit 0) } - newSecret, err := promptNewSecret() + newSecret, err := p.readInput() if err != nil { return err } @@ -187,8 +212,8 @@ func setAppSecretInteractive(f *cmdutil.Factory, multi *core.MultiAppConfig, pro // Render a readable line for humans instead of the agent JSON envelope. code := output.ExitCodeOf(err) humanMsg := err.Error() - if p, ok := errs.ProblemOf(err); ok { - humanMsg = p.Message + if pr, ok := errs.ProblemOf(err); ok { + humanMsg = pr.Message } fmt.Fprintf(f.IOStreams.ErrOut, "✗ %s\n", humanMsg) fmt.Fprintln(f.IOStreams.ErrOut, " run the command again to retry") @@ -221,19 +246,7 @@ func selectTargetProfile(multi *core.MultiAppConfig, profileOverride string) (*c return &multi.Apps[0], nil } - activeApp := multi.CurrentAppConfig("") - options := make([]huh.Option[int], 0, len(multi.Apps)) - selected := 0 - for i := range multi.Apps { - a := &multi.Apps[i] - label := fmt.Sprintf("%s (%s)", a.ProfileName(), a.AppId) - if activeApp != nil && activeApp.ProfileName() == a.ProfileName() { - label += " [active]" - selected = i - } - options = append(options, huh.NewOption(label, i)) - } - + options, selected := profileSelectOptions(multi) form := huh.NewForm( huh.NewGroup( huh.NewSelect[int](). @@ -251,6 +264,24 @@ func selectTargetProfile(multi *core.MultiAppConfig, profileOverride string) (*c return &multi.Apps[selected], nil } +// profileSelectOptions builds the huh picker options for multiple profiles, +// labelling and pre-selecting the active one. Pure logic, unit-tested. +func profileSelectOptions(multi *core.MultiAppConfig) ([]huh.Option[int], int) { + activeApp := multi.CurrentAppConfig("") + options := make([]huh.Option[int], 0, len(multi.Apps)) + selected := 0 + for i := range multi.Apps { + a := &multi.Apps[i] + label := fmt.Sprintf("%s (%s)", a.ProfileName(), a.AppId) + if activeApp != nil && activeApp.ProfileName() == a.ProfileName() { + label += " [active]" + selected = i + } + options = append(options, huh.NewOption(label, i)) + } + return options, selected +} + // confirmRotate shows a y/N confirmation for the resolved target. func confirmRotate(target *errs.ErrTarget) (bool, error) { activeSeg := "" @@ -277,22 +308,16 @@ func confirmRotate(target *errs.ErrTarget) (bool, error) { return confirmed, nil } -// promptNewSecret reads the new secret with a hidden input (matches +// promptHiddenInput reads the new secret with a hidden input (matches // config init's existing app-secret prompt style). -func promptNewSecret() (string, error) { +func promptHiddenInput() (string, error) { var secret string form := huh.NewForm( huh.NewGroup( huh.NewInput(). Title("Enter the new app secret"). EchoMode(huh.EchoModePassword). - Validate(func(s string) error { - if strings.TrimSpace(s) == "" { - //nolint:forbidigo // huh inline form-validation message shown in the TUI, not a final CLI error - return fmt.Errorf("app secret must not be empty") - } - return nil - }). + Validate(validateAppSecret). Value(&secret), ), ).WithTheme(cmdutil.ThemeFeishu()) @@ -305,6 +330,16 @@ func promptNewSecret() (string, error) { return strings.TrimSpace(secret), nil } +// validateAppSecret is the huh input validator: the new secret must be non-empty. +// Pure logic, unit-tested. +func validateAppSecret(s string) error { + if strings.TrimSpace(s) == "" { + //nolint:forbidigo // huh inline form-validation message shown in the TUI, not a final CLI error + return fmt.Errorf("app secret must not be empty") + } + return nil +} + // ── Shared verify / write / output ─────────────────────────────────────────── // buildTarget assembles the structured target identity for the resolved profile. diff --git a/cmd/config/set_app_secret_test.go b/cmd/config/set_app_secret_test.go index 4a6a9fb6a..b99d466ec 100644 --- a/cmd/config/set_app_secret_test.go +++ b/cmd/config/set_app_secret_test.go @@ -406,6 +406,154 @@ func TestSelectTargetProfile_SingleProfile(t *testing.T) { } } +// TestProfileSelectOptions verifies the multi-profile picker options pre-select +// the active profile. +func TestProfileSelectOptions(t *testing.T) { + multi := &core.MultiAppConfig{Apps: []core.AppConfig{ + {Name: "first", AppId: "cli_first", AppSecret: core.PlainSecret("test-secret"), Brand: core.BrandFeishu}, + {Name: "second", AppId: "cli_second", AppSecret: core.PlainSecret("test-secret"), Brand: core.BrandFeishu}, + }} + opts, selected := profileSelectOptions(multi) + if len(opts) != 2 { + t.Fatalf("options = %d, want 2", len(opts)) + } + active := multi.CurrentAppConfig("") + wantIdx := 0 + for i := range multi.Apps { + if active != nil && multi.Apps[i].ProfileName() == active.ProfileName() { + wantIdx = i + } + } + if selected != wantIdx { + t.Errorf("preselected index = %d, want %d (active)", selected, wantIdx) + } +} + +// TestValidateAppSecret verifies the interactive secret input validator. +func TestValidateAppSecret(t *testing.T) { + if err := validateAppSecret("a-real-secret"); err != nil { + t.Errorf("non-empty secret should pass, got %v", err) + } + for _, s := range []string{"", " ", "\t\n "} { + if err := validateAppSecret(s); err == nil { + t.Errorf("empty/whitespace %q should fail validation", s) + } + } +} + +// TestRunInteractive_Cancel verifies that declining the confirm step writes +// nothing and exits cleanly (exit 0) with a readable notice. +func TestRunInteractive_Cancel(t *testing.T) { + rt := &fakeRT{tatHandler: func(*http.Request) (*http.Response, error) { + t.Error("verify must not run on cancel") + return jsonResp(500, `{}`), nil + }} + const appID = "cli_run_cancel" + f, spy := setAppSecretFactory(t, rt, appID) + errBuf := &bytes.Buffer{} + f.IOStreams.ErrOut = errBuf + multi, err := core.LoadMultiAppConfig() + if err != nil { + t.Fatalf("load config: %v", err) + } + + readCalled := false + p := secretPrompter{ + selectProfile: func(m *core.MultiAppConfig, _ string) (*core.AppConfig, error) { return m.CurrentAppConfig(""), nil }, + confirm: func(*errs.ErrTarget) (bool, error) { return false, nil }, + readInput: func() (string, error) { readCalled = true; return "", nil }, + } + if err := runInteractive(f, multi, "", p); err != nil { + t.Fatalf("cancel should return nil (clean exit 0), got %v", err) + } + if readCalled { + t.Error("readInput must not be called after a declined confirm") + } + if spy.setCalls != 0 { + t.Errorf("keychain.Set called %d times, want 0 on cancel", spy.setCalls) + } + if !strings.Contains(errBuf.String(), "cancelled") { + t.Errorf("want 'cancelled' notice on stderr, got %q", errBuf.String()) + } +} + +// TestRunInteractive_VerifyInvalid verifies that an invalid secret in the +// interactive flow writes nothing, exits 3, and renders a readable line (not JSON). +func TestRunInteractive_VerifyInvalid(t *testing.T) { + rt := &fakeRT{tatHandler: func(*http.Request) (*http.Response, error) { + return jsonResp(400, `{"error":"invalid_client","error_description":"bad","code":20002}`), nil + }} + const appID = "cli_run_invalid" + f, spy := setAppSecretFactory(t, rt, appID) + errBuf := &bytes.Buffer{} + f.IOStreams.ErrOut = errBuf + multi, err := core.LoadMultiAppConfig() + if err != nil { + t.Fatalf("load config: %v", err) + } + + p := secretPrompter{ + selectProfile: func(m *core.MultiAppConfig, _ string) (*core.AppConfig, error) { return m.CurrentAppConfig(""), nil }, + confirm: func(*errs.ErrTarget) (bool, error) { return true, nil }, + readInput: func() (string, error) { return "test-secret", nil }, + } + err = runInteractive(f, multi, "", p) + if err == nil { + t.Fatal("want error for invalid secret, got nil") + } + if got := output.ExitCodeOf(err); got != output.ExitAuth { + t.Errorf("exit code = %d, want %d (ExitAuth)", got, output.ExitAuth) + } + if spy.setCalls != 0 { + t.Errorf("keychain.Set called %d times, want 0 on invalid secret", spy.setCalls) + } + out := errBuf.String() + if !strings.Contains(out, "invalid") { + t.Errorf("want a readable invalid-secret message on stderr, got %q", out) + } + // Interactive failures must be readable lines, not the agent JSON envelope. + if strings.Contains(out, `"error"`) || strings.Contains(out, `"subtype"`) { + t.Errorf("interactive failure must not be a JSON envelope, got %q", out) + } +} + +// TestRunInteractive_Success verifies the happy interactive path verifies and +// writes via the shared logic and emits the success envelope. +func TestRunInteractive_Success(t *testing.T) { + const appID = "cli_run_ok" + kc := newMapKeychain() + _ = kc.Set("lark-cli", "appsecret"+":"+appID, "test-secret") + kc.setCalls = 0 + keychainSecret := core.SecretInput{Ref: &core.SecretRef{Source: "keychain", ID: "appsecret" + ":" + appID}} + f, _ := setAppSecretFactoryFull(t, kc, appID, keychainSecret) + outBuf := &bytes.Buffer{} + f.IOStreams.Out = outBuf + f.IOStreams.IsTerminal = false + multi, err := core.LoadMultiAppConfig() + if err != nil { + t.Fatalf("load config: %v", err) + } + + p := secretPrompter{ + selectProfile: func(m *core.MultiAppConfig, _ string) (*core.AppConfig, error) { return m.CurrentAppConfig(""), nil }, + confirm: func(*errs.ErrTarget) (bool, error) { return true, nil }, + readInput: func() (string, error) { return "test-secret", nil }, + } + if err := runInteractive(f, multi, "", p); err != nil { + t.Fatalf("success path returned error: %v", err) + } + if kc.setCalls == 0 { + t.Error("keychain.Set not called on success") + } + var env map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(outBuf.String())), &env); err != nil { + t.Fatalf("stdout is not JSON: %v (%q)", err, outBuf.String()) + } + if env["ok"] != true { + t.Errorf("ok = %v, want true", env["ok"]) + } +} + // ── Task 5: verify-before-write ─────────────────────────────────────────────── // setAppSecretFactory sets up an isolated config factory with the given