diff --git a/cmd/wfctl/infra_secrets.go b/cmd/wfctl/infra_secrets.go index 7cd480c6..340428cf 100644 --- a/cmd/wfctl/infra_secrets.go +++ b/cmd/wfctl/infra_secrets.go @@ -108,8 +108,18 @@ func resolveSecretsProviderForEnv(cfg *SecretsConfig, envName string) (secrets.P } return secrets.NewKeychainProvider(service) + case "file": + path, _ := c["path"].(string) + if path == "" { + return nil, fmt.Errorf("secrets.file: 'path' is required") + } + if err := os.MkdirAll(path, 0o700); err != nil { + return nil, fmt.Errorf("secrets.file: create directory %s: %w", path, err) + } + return secrets.NewFileProvider(path), nil + default: - return nil, fmt.Errorf("unknown secrets provider %q (supported: github, vault, aws, env, keychain)", cfg.Provider) + return nil, fmt.Errorf("unknown secrets provider %q (supported: github, vault, aws, env, keychain, file)", cfg.Provider) } } diff --git a/cmd/wfctl/internal/prompt/confirm.go b/cmd/wfctl/internal/prompt/confirm.go new file mode 100644 index 00000000..8588ae65 --- /dev/null +++ b/cmd/wfctl/internal/prompt/confirm.go @@ -0,0 +1,68 @@ +package prompt + +import ( + "fmt" + "io" + "strings" + + tea "charm.land/bubbletea/v2" +) + +// Confirm asks a yes/no question. def is the default answer shown in the +// prompt (used when the user presses Enter without typing y/n). +// +// Returns (false, ErrNotInteractive) when stdin is not a terminal. +func Confirm(question string, def bool) (bool, error) { + if !isTTY() { + return false, ErrNotInteractive + } + hint := "y/N" + if def { + hint = "Y/n" + } + m := &confirmModel{question: question, hint: hint, def: def} + p := tea.NewProgram(m, tea.WithOutput(io.Discard)) + result, err := p.Run() + if err != nil { + return false, fmt.Errorf("prompt confirm: %w", err) + } + fm := result.(*confirmModel) + if fm.quit { + return false, ErrNotInteractive + } + return fm.answer, nil +} + +type confirmModel struct { + question string + hint string + def bool + answer bool + quit bool +} + +func (m *confirmModel) Init() tea.Cmd { return nil } + +func (m *confirmModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + if msg, ok := msg.(tea.KeyPressMsg); ok { + switch strings.ToLower(msg.String()) { + case "ctrl+c": + m.quit = true + return m, tea.Quit + case "y": + m.answer = true + return m, tea.Quit + case "n": + m.answer = false + return m, tea.Quit + case "enter": + m.answer = m.def + return m, tea.Quit + } + } + return m, nil +} + +func (m *confirmModel) View() tea.View { + return tea.NewView(fmt.Sprintf("%s [%s] ", m.question, m.hint)) +} diff --git a/cmd/wfctl/internal/prompt/input.go b/cmd/wfctl/internal/prompt/input.go new file mode 100644 index 00000000..1a6fda7c --- /dev/null +++ b/cmd/wfctl/internal/prompt/input.go @@ -0,0 +1,70 @@ +package prompt + +import ( + "fmt" + "io" + + "charm.land/bubbles/v2/textinput" + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" +) + +// Input prompts the user for a single-line text value. When masked is true +// the input is displayed as asterisks (suitable for passwords/tokens). +// +// Returns ("", ErrNotInteractive) when stdin is not a terminal. +func Input(label string, masked bool) (string, error) { + if !isTTY() { + return "", ErrNotInteractive + } + ti := textinput.New() + ti.Placeholder = label + ti.Focus() + if masked { + ti.EchoMode = textinput.EchoPassword + ti.EchoCharacter = '*' + } + + m := &inputModel{label: label, ti: ti} + p := tea.NewProgram(m, tea.WithOutput(io.Discard)) + result, err := p.Run() + if err != nil { + return "", fmt.Errorf("prompt input: %w", err) + } + fm := result.(*inputModel) + if fm.quit { + return "", ErrNotInteractive + } + return fm.ti.Value(), nil +} + +type inputModel struct { + label string + ti textinput.Model + quit bool +} + +var labelStyle = lipgloss.NewStyle().Bold(true) + +func (m *inputModel) Init() tea.Cmd { + return textinput.Blink +} + +func (m *inputModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + var cmd tea.Cmd + if kp, ok := msg.(tea.KeyPressMsg); ok { + switch kp.String() { + case "ctrl+c": + m.quit = true + return m, tea.Quit + case "enter": + return m, tea.Quit + } + } + m.ti, cmd = m.ti.Update(msg) + return m, cmd +} + +func (m *inputModel) View() tea.View { + return tea.NewView(labelStyle.Render(m.label+": ") + m.ti.View() + "\n") +} diff --git a/cmd/wfctl/internal/prompt/multiselect.go b/cmd/wfctl/internal/prompt/multiselect.go new file mode 100644 index 00000000..a9539dd7 --- /dev/null +++ b/cmd/wfctl/internal/prompt/multiselect.go @@ -0,0 +1,104 @@ +package prompt + +import ( + "fmt" + "io" + "strings" + + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" +) + +// MultiSelect presents an interactive multi-choice list. Returns the indices +// of all selected items. Items can be pre-selected via Item.Preselected. +// +// Returns (nil, ErrNotInteractive) when stdin is not a terminal. +func MultiSelect(title string, items []Item) ([]int, error) { + if !isTTY() { + return nil, ErrNotInteractive + } + selected := make(map[int]bool, len(items)) + for i, it := range items { + if it.Preselected { + selected[i] = true + } + } + m := &multiSelectModel{title: title, items: items, selected: selected} + p := tea.NewProgram(m, tea.WithOutput(io.Discard)) + result, err := p.Run() + if err != nil { + return nil, fmt.Errorf("prompt multiselect: %w", err) + } + fm := result.(*multiSelectModel) + if fm.quit { + return nil, ErrNotInteractive + } + var indices []int + for i := range items { + if fm.selected[i] { + indices = append(indices, i) + } + } + return indices, nil +} + +type multiSelectModel struct { + title string + items []Item + cursor int + selected map[int]bool + quit bool +} + +var ( + checkStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("2")) + uncheckStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("8")) +) + +func (m *multiSelectModel) Init() tea.Cmd { return nil } + +func (m *multiSelectModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + if msg, ok := msg.(tea.KeyPressMsg); ok { + switch msg.String() { + case "ctrl+c", "q": + m.quit = true + return m, tea.Quit + case "up", "k": + if m.cursor > 0 { + m.cursor-- + } + case "down", "j": + if m.cursor < len(m.items)-1 { + m.cursor++ + } + case " ": + if m.selected[m.cursor] { + delete(m.selected, m.cursor) + } else { + m.selected[m.cursor] = true + } + case "enter": + return m, tea.Quit + } + } + return m, nil +} + +func (m *multiSelectModel) View() tea.View { + var b strings.Builder + b.WriteString(titleStyle.Render(m.title)) + b.WriteString("\n") + for i, it := range m.items { + cursor := " " + if i == m.cursor { + cursor = "> " + } + check := uncheckStyle.Render("[ ]") + if m.selected[i] { + check = checkStyle.Render("[x]") + } + b.WriteString(cursor + check + " " + it.Label + "\n") + } + b.WriteString("\n(space to toggle, enter to confirm)\n") + return tea.NewView(b.String()) +} diff --git a/cmd/wfctl/internal/prompt/prompt.go b/cmd/wfctl/internal/prompt/prompt.go new file mode 100644 index 00000000..5ecbe1fb --- /dev/null +++ b/cmd/wfctl/internal/prompt/prompt.go @@ -0,0 +1,30 @@ +// Package prompt provides reusable terminal UI widgets built on +// charm.land/bubbletea/v2 and charm.land/bubbles/v2. +// +// Every public constructor checks whether stdin is an interactive terminal +// before starting a bubbletea program. If stdin is not a terminal the +// constructor returns (zero, ErrNotInteractive) immediately so callers in +// CI / pipe mode can detect the condition and fall back to non-interactive +// paths without any risk of hanging. +package prompt + +import ( + "errors" + "os" + + "github.com/mattn/go-isatty" +) + +// ErrNotInteractive is returned by all constructors when stdin is not a terminal. +var ErrNotInteractive = errors.New("prompt: stdin is not a terminal") + +// Item is a selectable entry for MultiSelect. +type Item struct { + Label string + Preselected bool +} + +// isTTY reports whether os.Stdin is an interactive terminal. +func isTTY() bool { + return isatty.IsTerminal(os.Stdin.Fd()) +} diff --git a/cmd/wfctl/internal/prompt/prompt_test.go b/cmd/wfctl/internal/prompt/prompt_test.go new file mode 100644 index 00000000..4321ee9e --- /dev/null +++ b/cmd/wfctl/internal/prompt/prompt_test.go @@ -0,0 +1,88 @@ +package prompt_test + +import ( + "testing" + + "github.com/GoCodeAlone/workflow/cmd/wfctl/internal/prompt" +) + +// All tests run in a non-TTY environment (stdin is a pipe in test +// subprocess), so every constructor must return ErrNotInteractive +// immediately rather than hanging. + +func TestSelect_NonTTY(t *testing.T) { + _, err := prompt.Select("Pick one", []string{"a", "b"}) + if err != prompt.ErrNotInteractive { + t.Fatalf("Select: got %v, want ErrNotInteractive", err) + } +} + +func TestMultiSelect_NonTTY(t *testing.T) { + _, err := prompt.MultiSelect("Pick many", []prompt.Item{{Label: "a"}, {Label: "b"}}) + if err != prompt.ErrNotInteractive { + t.Fatalf("MultiSelect: got %v, want ErrNotInteractive", err) + } +} + +func TestInput_NonTTY(t *testing.T) { + _, err := prompt.Input("value", false) + if err != prompt.ErrNotInteractive { + t.Fatalf("Input: got %v, want ErrNotInteractive", err) + } +} + +func TestInputMasked_NonTTY(t *testing.T) { + _, err := prompt.Input("password", true) + if err != prompt.ErrNotInteractive { + t.Fatalf("Input(masked): got %v, want ErrNotInteractive", err) + } +} + +func TestConfirm_NonTTY(t *testing.T) { + _, err := prompt.Confirm("Are you sure?", true) + if err != prompt.ErrNotInteractive { + t.Fatalf("Confirm: got %v, want ErrNotInteractive", err) + } +} + +func TestSelectZeroValue(t *testing.T) { + // When non-interactive the index is 0 (zero value). + idx, err := prompt.Select("Pick", []string{"x"}) + if err != prompt.ErrNotInteractive { + t.Fatalf("unexpected err: %v", err) + } + if idx != 0 { + t.Errorf("idx = %d, want 0", idx) + } +} + +func TestMultiSelectZeroValue(t *testing.T) { + indices, err := prompt.MultiSelect("Pick", []prompt.Item{{Label: "x"}}) + if err != prompt.ErrNotInteractive { + t.Fatalf("unexpected err: %v", err) + } + if len(indices) != 0 { + t.Errorf("indices = %v, want []", indices) + } +} + +func TestInputZeroValue(t *testing.T) { + s, err := prompt.Input("label", false) + if err != prompt.ErrNotInteractive { + t.Fatalf("unexpected err: %v", err) + } + if s != "" { + t.Errorf("s = %q, want empty", s) + } +} + +func TestConfirmZeroValue(t *testing.T) { + // Default value is irrelevant when ErrNotInteractive is returned. + v, err := prompt.Confirm("Sure?", true) + if err != prompt.ErrNotInteractive { + t.Fatalf("unexpected err: %v", err) + } + if v { + t.Errorf("v = true, want false (zero value)") + } +} diff --git a/cmd/wfctl/internal/prompt/select.go b/cmd/wfctl/internal/prompt/select.go new file mode 100644 index 00000000..6b9608fd --- /dev/null +++ b/cmd/wfctl/internal/prompt/select.go @@ -0,0 +1,84 @@ +package prompt + +import ( + "fmt" + "io" + "strings" + + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" +) + +// Select presents an interactive single-choice list to the user and returns +// the index of the chosen item. +// +// Returns (0, ErrNotInteractive) when stdin is not a terminal. +func Select(title string, opts []string) (int, error) { + if !isTTY() { + return 0, ErrNotInteractive + } + m := &selectModel{title: title, opts: opts} + p := tea.NewProgram(m, tea.WithOutput(io.Discard)) + result, err := p.Run() + if err != nil { + return 0, fmt.Errorf("prompt select: %w", err) + } + fm := result.(*selectModel) + if fm.quit { + return 0, ErrNotInteractive + } + return fm.cursor, nil +} + +// selectModel is the bubbletea model for single selection. +type selectModel struct { + title string + opts []string + cursor int + quit bool +} + +var ( + titleStyle = lipgloss.NewStyle().Bold(true) + selectedStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("2")).Bold(true) + normalStyle = lipgloss.NewStyle() +) + +func (m *selectModel) Init() tea.Cmd { return nil } + +func (m *selectModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + if msg, ok := msg.(tea.KeyPressMsg); ok { + switch msg.String() { + case "ctrl+c", "q": + m.quit = true + return m, tea.Quit + case "up", "k": + if m.cursor > 0 { + m.cursor-- + } + case "down", "j": + if m.cursor < len(m.opts)-1 { + m.cursor++ + } + case "enter", " ": + return m, tea.Quit + } + } + return m, nil +} + +func (m *selectModel) View() tea.View { + var b strings.Builder + b.WriteString(titleStyle.Render(m.title)) + b.WriteString("\n") + for i, opt := range m.opts { + cursor := " " + if i == m.cursor { + cursor = "> " + b.WriteString(selectedStyle.Render(cursor+opt) + "\n") + } else { + b.WriteString(normalStyle.Render(cursor+opt) + "\n") + } + } + return tea.NewView(b.String()) +} diff --git a/cmd/wfctl/main_test.go b/cmd/wfctl/main_test.go index b4daa349..85c10eb8 100644 --- a/cmd/wfctl/main_test.go +++ b/cmd/wfctl/main_test.go @@ -1,6 +1,8 @@ package main import ( + "bytes" + "context" "encoding/json" "errors" "flag" @@ -11,6 +13,7 @@ import ( "runtime" "strings" "testing" + "time" "github.com/GoCodeAlone/workflow/schema" ) @@ -110,6 +113,91 @@ func TestLinkedVersionOverridesBuildInfo(t *testing.T) { } } +// TestSecretsSetupNonTTYDoesNotHang is the binary-level regression guard for +// the two findings in PR2 code review: +// +// 1. interactive valuer ErrNotInteractive must surface (so the non-interactive +// fallback triggers), and +// 2. the non-TTY path must never block on stdin (Fscanln / open empty pipe). +// +// It builds the real wfctl binary and runs `secrets setup --config ` with +// EMPTY stdin piped (a non-TTY pipe that yields EOF), under a hard deadline. +// Asserts: exits non-zero, output names the missing secret, and the process +// returns well before the deadline (no hang). +func TestSecretsSetupNonTTYDoesNotHang(t *testing.T) { + exeName := "wfctl" + if runtime.GOOS == "windows" { + exeName += ".exe" + } + dir := t.TempDir() + exe := filepath.Join(dir, exeName) + build := exec.Command("go", "build", "-o", exe, ".") + build.Env = append(os.Environ(), "GOWORK=off") + if out, err := build.CombinedOutput(); err != nil { + t.Fatalf("go build wfctl: %v\n%s", err, out) + } + + // Minimal config: one declared secret, a writable file store, and an + // http.server entry point so the config is otherwise valid. + storeDir := filepath.Join(dir, "store") + if err := os.MkdirAll(storeDir, 0o755); err != nil { + t.Fatalf("mkdir store: %v", err) + } + cfg := `modules: + - name: http + type: http.server + config: + address: ":0" +secrets: + defaultStore: localfs + entries: + - name: NEEDS_VALUE +secretStores: + localfs: + provider: file + config: + path: ` + storeDir + ` +` + cfgPath := filepath.Join(dir, "app.yaml") + if err := os.WriteFile(cfgPath, []byte(cfg), 0o644); err != nil { + t.Fatalf("write app.yaml: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + // No --non-interactive flag: the binary must DETECT the non-TTY pipe and + // route to the non-interactive path rather than blocking on a prompt. + run := exec.CommandContext(ctx, exe, "secrets", "setup", "--config", cfgPath) + run.Env = append(os.Environ(), "WFCTL_NO_UPDATE_CHECK=1", "CI=true") + run.Stdin = strings.NewReader("") // empty, non-TTY stdin → immediate EOF + var combined bytes.Buffer + run.Stdout = &combined + run.Stderr = &combined + + start := time.Now() + err := run.Run() + elapsed := time.Since(start) + + if ctx.Err() == context.DeadlineExceeded { + t.Fatalf("wfctl secrets setup HUNG (deadline exceeded after %s); output:\n%s", elapsed, combined.String()) + } + if err == nil { + t.Fatalf("expected non-zero exit (missing secret value), got success; output:\n%s", combined.String()) + } + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected ExitError, got %T: %v\noutput:\n%s", err, err, combined.String()) + } + if combined.Len() == 0 || !strings.Contains(combined.String(), "NEEDS_VALUE") { + t.Fatalf("output should name the missing secret NEEDS_VALUE; got:\n%s", combined.String()) + } + // Sanity: it must have returned promptly, not near the deadline. + if elapsed > 15*time.Second { + t.Fatalf("returned too slowly (%s) — possible partial hang; output:\n%s", elapsed, combined.String()) + } +} + func writeTestConfig(t *testing.T, dir, name, content string) string { t.Helper() path := filepath.Join(dir, name) diff --git a/cmd/wfctl/secrets_audit.go b/cmd/wfctl/secrets_audit.go new file mode 100644 index 00000000..ffaa9079 --- /dev/null +++ b/cmd/wfctl/secrets_audit.go @@ -0,0 +1,130 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "os/user" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/GoCodeAlone/workflow/config" +) + +// secretsAuditRecord is the shape of one line in secrets-audit.jsonl. +// It MUST NOT contain a secret value — only the name and metadata. +type secretsAuditRecord struct { + Ts string `json:"ts"` + SecretName string `json:"secret_name"` + Store string `json:"store"` + Scope string `json:"scope,omitempty"` + Actor string `json:"actor,omitempty"` +} + +// secretsAuditPath returns the path to the audit JSONL file. +// It honours $XDG_STATE_HOME; falls back to $HOME/.local/state. +func secretsAuditPath() (string, error) { + base := os.Getenv("XDG_STATE_HOME") + if base == "" { + u, err := user.Current() + if err != nil { + return "", fmt.Errorf("resolve home dir: %w", err) + } + base = filepath.Join(u.HomeDir, ".local", "state") + } + return filepath.Join(base, "wfctl", "plugins", "wfctl", "secrets-audit.jsonl"), nil +} + +// writeSecretsAuditRecord appends a single JSON line to the audit log +// for a successful secret Set. It NEVER writes the secret value. +func writeSecretsAuditRecord(name, store string) error { + path, err := secretsAuditPath() + if err != nil { + return err + } + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return fmt.Errorf("audit: create dir: %w", err) + } + + actor := os.Getenv("USER") + if actor == "" { + if u, err := user.Current(); err == nil { + actor = u.Username + } + } + + rec := secretsAuditRecord{ + Ts: time.Now().UTC().Format(time.RFC3339), + SecretName: name, + Store: store, + Actor: actor, + } + line, err := json.Marshal(rec) + if err != nil { + return fmt.Errorf("audit: marshal: %w", err) + } + + f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600) + if err != nil { + return fmt.Errorf("audit: open %s: %w", path, err) + } + defer f.Close() //nolint:errcheck + _, err = fmt.Fprintf(f, "%s\n", line) + return err +} + +// resolveSetupStoreName implements the 5-step store resolution priority: +// +// 1. storeFlag (--store CLI flag) → use it directly. +// 2. defaultStore (config.secrets.defaultStore) → use it. +// 3. exactly-one entry in secretStores → use its key. +// 4. interactive=true → caller must invoke prompt.Select (handled at call site). +// 5. → error (non-interactive with no resolved store). +// +// When case 4 applies, this function returns ("", nil) so the caller can +// prompt the user. Callers should check for ("", nil) and prompt accordingly. +func resolveSetupStoreName( + storeFlag string, + defaultStore string, + stores map[string]*config.SecretStoreConfig, + interactive bool, +) (string, error) { + // 1. Explicit --store flag. + if storeFlag != "" { + return storeFlag, nil + } + // 2. Config default. + if defaultStore != "" { + return defaultStore, nil + } + // 3. Exactly-one store in the map. + if len(stores) == 1 { + for k := range stores { + return k, nil + } + } + // 4/5. Multiple stores or none. + if len(stores) > 1 { + if !interactive { + return "", fmt.Errorf("multiple secret stores configured; set secrets.defaultStore or pass --store (available: %s)", + storeNames(stores)) + } + // Caller must prompt — signal with empty string + no error. + return "", nil + } + // No stores configured → fall back to "env" (legacy). + return "env", nil +} + +// storeNames returns a comma-separated, sorted list of store names for error +// messages (sorted for deterministic output). +func storeNames(stores map[string]*config.SecretStoreConfig) string { + names := make([]string, 0, len(stores)) + for k := range stores { + names = append(names, k) + } + sort.Strings(names) + return strings.Join(names, ", ") +} diff --git a/cmd/wfctl/secrets_audit_test.go b/cmd/wfctl/secrets_audit_test.go new file mode 100644 index 00000000..e5f713d7 --- /dev/null +++ b/cmd/wfctl/secrets_audit_test.go @@ -0,0 +1,168 @@ +package main + +import ( + "bufio" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow/config" +) + +// TestSecretsAuditRecord verifies that: +// - exactly one JSONL line is written per writeSecretsAuditRecord call +// - the line contains the expected fields (ts, secret_name, store) +// - the secret VALUE is absent from the file bytes +func TestSecretsAuditRecord(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_STATE_HOME", tmp) + + if err := writeSecretsAuditRecord("MY_SECRET", "localfs"); err != nil { + t.Fatalf("writeSecretsAuditRecord: %v", err) + } + + auditPath := filepath.Join(tmp, "wfctl", "plugins", "wfctl", "secrets-audit.jsonl") + data, err := os.ReadFile(auditPath) + if err != nil { + t.Fatalf("read audit file: %v", err) + } + + // Must contain exactly one non-empty line. + var lines []string + scanner := bufio.NewScanner(strings.NewReader(string(data))) + for scanner.Scan() { + if l := strings.TrimSpace(scanner.Text()); l != "" { + lines = append(lines, l) + } + } + if len(lines) != 1 { + t.Fatalf("expected 1 JSONL line, got %d: %s", len(lines), data) + } + + var rec map[string]any + if err := json.Unmarshal([]byte(lines[0]), &rec); err != nil { + t.Fatalf("unmarshal JSONL: %v (%s)", err, lines[0]) + } + + // Required fields. + checkField := func(key string) { + t.Helper() + v, ok := rec[key] + if !ok || v == "" { + t.Errorf("audit record missing or empty field %q: %v", key, rec) + } + } + checkField("ts") + checkField("secret_name") + checkField("store") + + if rec["secret_name"] != "MY_SECRET" { + t.Errorf("secret_name = %v, want MY_SECRET", rec["secret_name"]) + } + if rec["store"] != "localfs" { + t.Errorf("store = %v, want localfs", rec["store"]) + } + + // The raw file bytes must never contain a secret value (only the name is stored). + const forbiddenValue = "hunter2" + if strings.Contains(string(data), forbiddenValue) { + t.Errorf("audit file contains forbidden value %q: %s", forbiddenValue, data) + } +} + +// TestSecretsAuditRecord_AppendOnSecondWrite verifies appending behaviour. +func TestSecretsAuditRecord_AppendOnSecondWrite(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_STATE_HOME", tmp) + + if err := writeSecretsAuditRecord("A", "store1"); err != nil { + t.Fatalf("first write: %v", err) + } + if err := writeSecretsAuditRecord("B", "store2"); err != nil { + t.Fatalf("second write: %v", err) + } + + auditPath := filepath.Join(tmp, "wfctl", "plugins", "wfctl", "secrets-audit.jsonl") + data, err := os.ReadFile(auditPath) + if err != nil { + t.Fatalf("read: %v", err) + } + var lines []string + scanner := bufio.NewScanner(strings.NewReader(string(data))) + for scanner.Scan() { + if l := strings.TrimSpace(scanner.Text()); l != "" { + lines = append(lines, l) + } + } + if len(lines) != 2 { + t.Errorf("expected 2 lines, got %d: %s", len(lines), data) + } +} + +// TestResolveSetupStorePriority exercises the store-resolver priority chain +// (5 cases). +func TestResolveSetupStorePriority(t *testing.T) { + oneStore := map[string]*config.SecretStoreConfig{ + "only-store": {Provider: "file"}, + } + twoStores := map[string]*config.SecretStoreConfig{ + "s1": {Provider: "file"}, + "s2": {Provider: "env"}, + } + + tests := []struct { + name string + storeFlag string + defaultStore string + stores map[string]*config.SecretStoreConfig + interactive bool + wantStore string + wantErr bool + }{ + { + name: "1. --store flag wins", + storeFlag: "flagged", + wantStore: "flagged", + }, + { + name: "2. config.defaultStore", + defaultStore: "configured", + wantStore: "configured", + }, + { + name: "3. exactly-one secretStores entry", + stores: oneStore, + wantStore: "only-store", + }, + { + name: "4. multiple stores, non-interactive → error", + stores: twoStores, + interactive: false, + wantErr: true, + }, + { + name: "5. no store anywhere → env fallback", + wantStore: "env", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + store, err := resolveSetupStoreName(tc.storeFlag, tc.defaultStore, tc.stores, tc.interactive) + if tc.wantErr { + if err == nil { + t.Errorf("want error, got store=%q", store) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if store != tc.wantStore { + t.Errorf("store = %q, want %q", store, tc.wantStore) + } + }) + } +} diff --git a/cmd/wfctl/secrets_setup.go b/cmd/wfctl/secrets_setup.go index ab7c8783..c226dcd7 100644 --- a/cmd/wfctl/secrets_setup.go +++ b/cmd/wfctl/secrets_setup.go @@ -1,30 +1,63 @@ package main import ( - "bufio" "context" + "errors" "flag" "fmt" "os" "strings" - "github.com/GoCodeAlone/workflow/config" + "github.com/GoCodeAlone/workflow/cmd/wfctl/internal/prompt" + "github.com/mattn/go-isatty" ) -// runSecretsSetup implements `wfctl secrets setup --env `. -// It iterates over all secrets declared in the config for the given environment -// and prompts for values, using hidden terminal input where possible. +// runSecretsSetup implements `wfctl secrets setup [options]`. +// +// Routing: +// - --non-interactive, --auto-gen-keys, or a non-TTY stdin → the engine-backed +// non-interactive path (flag/env-sourced selector + valuer). +// - otherwise → the interactive wizard (prompt.MultiSelect selector + +// masked prompt.Input valuer). If the wizard detects a non-TTY mid-flow +// it returns prompt.ErrNotInteractive and we fall back to non-interactive. +// +// Both paths share the same runSetupEngine + audit logic. func runSecretsSetup(args []string) error { fs := flag.NewFlagSet("secrets setup", flag.ContinueOnError) envName := fs.String("env", "local", "Target environment name") configFile := fs.String("config", "app.yaml", "Workflow config file") - autoGenKeys := fs.Bool("auto-gen-keys", false, "Auto-generate random values for secrets ending in _KEY, _SECRET, or _TOKEN") + autoGenKeys := fs.Bool("auto-gen-keys", false, "Auto-generate random values for secrets ending in _KEY, _SECRET, or _TOKEN (implies non-interactive)") + nonInteractive := fs.Bool("non-interactive", false, "Non-interactive mode (also auto when stdin is not a TTY)") + fromEnv := fs.Bool("from-env", false, "Read each secret value from $NAME (recommended for CI; avoids process-table leaks)") + var secretFlag multiStringFlag + fs.Var(&secretFlag, "secret", "NAME=VALUE literal (WARNING: leaks to process table; use --from-env in CI). Repeatable.") + onlyFlag := fs.String("only", "", "Comma-separated list of secret names to set (default: all)") + allFlag := fs.Bool("all", false, "Set all declared secrets (the default when --only is absent; explicit form)") + skipExisting := fs.Bool("skip-existing", false, "Skip secrets that already have a value in the store") + storeName := fs.String("store", "", "Named store to use (overrides config defaultStore)") + // Legacy flags (kept for backwards compatibility with --plugin path). + fs.String("scope", "repo", "GitHub scope: repo | env | org (legacy --plugin path)") + fs.String("org", "", "Organization slug (legacy --plugin path)") + fs.String("visibility", "all", "Org-scope visibility (legacy --plugin path)") + fs.String("token-env", "GITHUB_TOKEN", "Env var holding the GitHub PAT (legacy --plugin path)") + fs.Usage = func() { fmt.Fprintf(fs.Output(), `Usage: wfctl secrets setup [options] -Interactively set all secrets declared in the config for a given environment. -For each secret, you are prompted for a value. Input is hidden. -Secrets in no-access stores (e.g., cloud KMS without credentials) are skipped. +Set secrets declared in the config for a given environment. + +Interactive (default when stdin is a TTY): + Lists each declared secret with its status, lets you select which to set, + prompts to pick a store when none is configured, and masks sensitive input. + +Non-interactive (--non-interactive, --auto-gen-keys, or when stdin is not a TTY): + --from-env Read each value from $NAME. Recommended for CI. + --secret NAME=VAL Set a specific secret inline (WARNING: leaks to process table). + Pipe KEY=VALUE Read KEY=VALUE lines from stdin. + --all Set all declared secrets (default when --only is absent). + --only A,B Restrict which secrets to set (mutually exclusive with --all). + --skip-existing Skip secrets already set in the store. + --auto-gen-keys Generate random values for _KEY/_SECRET/_TOKEN/_SIGNING names. Options: `) @@ -34,88 +67,62 @@ Options: return err } - cfg, err := config.LoadFromFile(*configFile) - if err != nil { - return fmt.Errorf("load config: %w", err) - } - - if cfg.Secrets == nil || len(cfg.Secrets.Entries) == 0 { - fmt.Println("No secrets declared in config.") - return nil - } - - fmt.Printf("Setting up secrets for environment: %s\n\n", *envName) - - ctx := context.Background() - reader := bufio.NewReader(os.Stdin) - - var set, skipped int - for _, entry := range cfg.Secrets.Entries { - storeName := ResolveSecretStore(entry.Name, *envName, cfg) - provider, provErr := getProviderForStore(storeName, cfg) - if provErr != nil { - fmt.Printf(" %-24s [SKIP] store %q not accessible: %v\n", entry.Name, storeName, provErr) - skipped++ - continue - } - - // Check if already set. - existing, _ := provider.Get(ctx, entry.Name) - hint := "" - if existing != "" { - hint = " (already set — press Enter to keep)" - } + // --auto-gen-keys is inherently non-interactive (it generates values). + useNonInteractive := *nonInteractive || *autoGenKeys || !isatty.IsTerminal(os.Stdin.Fd()) - // Auto-generate for key/secret/token names if flag is set. - if *autoGenKeys && existing == "" && isAutoGenCandidate(entry.Name) { - val := generateSecretValue() - if val == "" { - fmt.Printf(" %-24s [ERROR] crypto/rand unavailable; cannot auto-generate\n", entry.Name) - skipped++ - continue - } - if setErr := provider.Set(ctx, entry.Name, val); setErr != nil { - fmt.Printf(" %-24s [ERROR] %v\n", entry.Name, setErr) - } else { - fmt.Printf(" %-24s [auto-generated]\n", entry.Name) - set++ + // Parse --only. + var only []string + if *onlyFlag != "" { + for _, n := range strings.Split(*onlyFlag, ",") { + n = strings.TrimSpace(n) + if n != "" { + only = append(only, n) } - continue - } - - // Prompt for value. - desc := "" - if entry.Description != "" { - desc = " — " + entry.Description } - fmt.Printf(" %s%s%s\n Value: ", entry.Name, desc, hint) + } - line, readErr := reader.ReadString('\n') - if readErr != nil { - return fmt.Errorf("read input: %w", readErr) - } - line = strings.TrimRight(line, "\r\n") + // --all and --only are mutually exclusive. --all is the explicit form of + // the default (set every declared secret); when present it just means + // "don't restrict", which is what an empty --only already does. + if *allFlag && len(only) > 0 { + return fmt.Errorf("--all and --only are mutually exclusive") + } - if line == "" { - if existing != "" { - fmt.Printf(" %-24s [kept]\n", entry.Name) - } else { - fmt.Printf(" %-24s [skipped]\n", entry.Name) - skipped++ - } - continue - } + ctx := context.Background() - if setErr := provider.Set(ctx, entry.Name, line); setErr != nil { - fmt.Printf(" %-24s [ERROR] %v\n", entry.Name, setErr) - } else { - fmt.Printf(" %-24s [set]\n", entry.Name) - set++ - } + if useNonInteractive { + return runSecretsSetupNonInteractiveCtx(ctx, &nonInteractiveSetupArgs{ + configFile: *configFile, + storeName: *storeName, + fromEnv: *fromEnv, + secretLiterals: []string(secretFlag), + stdinKV: readStdinKV(), + only: only, + skipExisting: *skipExisting, + autoGenKeys: *autoGenKeys, + }, os.Stdout) } - fmt.Printf("\nDone: %d set, %d skipped.\n", set, skipped) - return nil + // ---- Interactive wizard ---- + err := runSecretsSetupInteractive(ctx, &interactiveSetupArgs{ + configFile: *configFile, + storeName: *storeName, + envName: *envName, + }, os.Stdout) + if errors.Is(err, prompt.ErrNotInteractive) { + // stdin turned out not to be a TTY mid-flow — fall back gracefully. + return runSecretsSetupNonInteractiveCtx(ctx, &nonInteractiveSetupArgs{ + configFile: *configFile, + storeName: *storeName, + fromEnv: *fromEnv, + secretLiterals: []string(secretFlag), + stdinKV: readStdinKV(), + only: only, + skipExisting: *skipExisting, + autoGenKeys: *autoGenKeys, + }, os.Stdout) + } + return err } // isAutoGenCandidate returns true if the secret name looks like a key, token, or signing secret. diff --git a/cmd/wfctl/secrets_setup_engine.go b/cmd/wfctl/secrets_setup_engine.go new file mode 100644 index 00000000..b03456ae --- /dev/null +++ b/cmd/wfctl/secrets_setup_engine.go @@ -0,0 +1,120 @@ +package main + +import ( + "context" + "fmt" +) + +// setupReport summarises the result of a runSetupEngine call. +type setupReport struct { + // Set lists secret names that were successfully written to the provider. + Set []string + // Skipped lists secrets that were not written — either because the + // selector excluded them or because the valuer returned provided=false. + Skipped []string + // Failed lists secrets where provider.Set returned an error. + Failed []string +} + +// runSetupEngine is a pure, front-end-agnostic secrets-setup engine. +// It is generic over the declared-secret type D so that callers can +// carry whatever extra fields (sensitive, description, …) they need +// without this engine knowing about them. +// +// Parameters: +// +// - ctx — context for provider calls. +// - decls — all secrets declared in the config / plugin manifest. +// - nameOf — extracts the secret name from D. +// - provider — the SecretsProvider to query and write. +// - selector — given the full declared list and the statuses queried +// from the provider, returns the subset that should be set. +// Return (nil, err) for a fatal selector error. +// - valuer — given one declared secret, returns (value, provided, err). +// When provided=false the secret is skipped (not an error). +// Return ("", _, err) to signal a per-secret error. +// - audit — called after each successful Set(name, value); receives +// name and storeName. NEVER receives the value. +// - stopOnErr — when true the engine returns after the first Set error; +// when false all secrets are attempted and failures accumulate. +// +// Non-fatal errors (per-secret Set failures) are collected in +// setupReport.Failed. A non-nil overall error is returned only when the +// provider is fundamentally unusable (e.g. List fails fatally) or when +// stopOnErr=true and a Set error occurs. +func runSetupEngine[D any]( + ctx context.Context, + decls []D, + nameOf func(D) string, + provider SecretsProvider, + selector func([]D, []SecretStatus) ([]D, error), + valuer func(D) (string, bool, error), + audit func(name, store string), + stopOnErr bool, +) (setupReport, error) { + var report setupReport + + // Query current statuses from the provider for all declared secrets. + // We do a per-name Check rather than a List so that write-only stores + // (GitHub) and stores that cannot enumerate still work. + statuses := make([]SecretStatus, 0, len(decls)) + for _, d := range decls { + name := nameOf(d) + state, _ := provider.Check(ctx, name) + statuses = append(statuses, SecretStatus{ + Name: name, + State: state, + IsSet: state == SecretSet, + }) + } + + // Let the selector pick which secrets to process. + selected, err := selector(decls, statuses) + if err != nil { + return report, fmt.Errorf("setup engine selector: %w", err) + } + + // Build a quick set-name lookup so we know which decls were skipped. + selectedSet := make(map[string]bool, len(selected)) + for _, d := range selected { + selectedSet[nameOf(d)] = true + } + for _, d := range decls { + if !selectedSet[nameOf(d)] { + report.Skipped = append(report.Skipped, nameOf(d)) + } + } + + // Process each selected secret. + for _, d := range selected { + name := nameOf(d) + + value, provided, vErr := valuer(d) + if vErr != nil { + report.Failed = append(report.Failed, name) + if stopOnErr { + return report, fmt.Errorf("setup engine valuer for %q: %w", name, vErr) + } + continue + } + if !provided { + report.Skipped = append(report.Skipped, name) + continue + } + + if setErr := provider.Set(ctx, name, value); setErr != nil { + report.Failed = append(report.Failed, name) + if stopOnErr { + return report, fmt.Errorf("setup engine set %q: %w", name, setErr) + } + continue + } + + report.Set = append(report.Set, name) + if audit != nil { + audit(name, "") // storeName is filled in by the caller if needed + } + } + + return report, nil +} diff --git a/cmd/wfctl/secrets_setup_engine_test.go b/cmd/wfctl/secrets_setup_engine_test.go new file mode 100644 index 00000000..ac9f5fc5 --- /dev/null +++ b/cmd/wfctl/secrets_setup_engine_test.go @@ -0,0 +1,201 @@ +package main + +import ( + "context" + "errors" + "testing" +) + +// engineTestProvider is an in-memory SecretsProvider for engine tests. +// Distinct from the existing fakeSecretsProvider in infra_bootstrap_env_test.go. +type engineTestProvider struct { + data map[string]string + setCnt map[string]int +} + +func newEngineTestProvider(initial map[string]string) *engineTestProvider { + p := &engineTestProvider{data: make(map[string]string), setCnt: make(map[string]int)} + for k, v := range initial { + p.data[k] = v + } + return p +} + +func (f *engineTestProvider) Get(_ context.Context, name string) (string, error) { + return f.data[name], nil +} +func (f *engineTestProvider) Set(_ context.Context, name, value string) error { + if value == "__fail__" { + return errors.New("injected set failure") + } + f.data[name] = value + f.setCnt[name]++ + return nil +} +func (f *engineTestProvider) Check(_ context.Context, name string) (SecretState, error) { + if _, ok := f.data[name]; ok { + return SecretSet, nil + } + return SecretNotSet, nil +} +func (f *engineTestProvider) List(_ context.Context) ([]SecretStatus, error) { + var out []SecretStatus + for k := range f.data { + out = append(out, SecretStatus{Name: k, State: SecretSet, IsSet: true}) + } + return out, nil +} +func (f *engineTestProvider) Delete(_ context.Context, name string) error { + delete(f.data, name) + return nil +} + +// engineDecl is the declared-secret type used in engine tests. +type engineDecl struct { + name string + sensitive bool +} + +// TestSetupEngine_SkipExisting: store has A set; selector picks only B; +// engine should Set(B) once and report A as skipped. +func TestSetupEngine_SkipExisting(t *testing.T) { + provider := newEngineTestProvider(map[string]string{"A": "existing-value"}) + + decls := []engineDecl{ + {name: "A", sensitive: true}, + {name: "B", sensitive: false}, + } + + selector := func(decls []engineDecl, statuses []SecretStatus) ([]engineDecl, error) { + setMap := make(map[string]bool) + for _, s := range statuses { + if s.IsSet { + setMap[s.Name] = true + } + } + var out []engineDecl + for _, d := range decls { + if !setMap[d.name] { + out = append(out, d) + } + } + return out, nil + } + + valuer := func(d engineDecl) (string, bool, error) { + switch d.name { + case "B": + return "b-value", true, nil + default: + return "", false, nil + } + } + + var audited []string + audit := func(name, store string) { + audited = append(audited, name) + } + + report, err := runSetupEngine(context.Background(), decls, + func(d engineDecl) string { return d.name }, + provider, selector, valuer, audit, false) + if err != nil { + t.Fatalf("engine: %v", err) + } + if len(report.Set) != 1 || report.Set[0] != "B" { + t.Errorf("Set = %v, want [B]", report.Set) + } + if len(report.Skipped) != 1 || report.Skipped[0] != "A" { + t.Errorf("Skipped = %v, want [A]", report.Skipped) + } + if len(report.Failed) != 0 { + t.Errorf("Failed = %v, want []", report.Failed) + } + if provider.setCnt["B"] != 1 { + t.Errorf("provider Set(B) called %d times, want 1", provider.setCnt["B"]) + } + if provider.data["A"] != "existing-value" { + t.Error("A should not have been modified") + } + if len(audited) != 1 || audited[0] != "B" { + t.Errorf("audited = %v, want [B]", audited) + } +} + +// TestSetupEngine_SetError: Set failure goes to Failed, not fatal. +func TestSetupEngine_SetError(t *testing.T) { + provider := newEngineTestProvider(map[string]string{"A": "existing-value"}) + + decls := []engineDecl{{name: "B"}} + selector := func(decls []engineDecl, _ []SecretStatus) ([]engineDecl, error) { return decls, nil } + valuer := func(d engineDecl) (string, bool, error) { + return "__fail__", true, nil // trigger fake failure + } + var audited []string + audit := func(name, store string) { audited = append(audited, name) } + + report, err := runSetupEngine(context.Background(), decls, + func(d engineDecl) string { return d.name }, + provider, selector, valuer, audit, false) + if err != nil { + t.Fatalf("engine fatal: %v", err) + } + if len(report.Failed) != 1 || report.Failed[0] != "B" { + t.Errorf("Failed = %v, want [B]", report.Failed) + } + if len(report.Set) != 0 { + t.Errorf("Set = %v, want []", report.Set) + } + if len(audited) != 0 { + t.Errorf("audited = %v, want [] (no audit on failure)", audited) + } +} + +// TestSetupEngine_ValuerNotProvided: valuer returns provided=false → skip. +func TestSetupEngine_ValuerNotProvided(t *testing.T) { + provider := newEngineTestProvider(nil) + decls := []engineDecl{{name: "C"}} + selector := func(decls []engineDecl, _ []SecretStatus) ([]engineDecl, error) { return decls, nil } + valuer := func(d engineDecl) (string, bool, error) { return "", false, nil } + var audited []string + audit := func(name, store string) { audited = append(audited, name) } + + report, err := runSetupEngine(context.Background(), decls, + func(d engineDecl) string { return d.name }, + provider, selector, valuer, audit, false) + if err != nil { + t.Fatalf("engine fatal: %v", err) + } + if len(report.Set) != 0 { + t.Errorf("Set = %v, want []", report.Set) + } + if len(report.Skipped) != 1 || report.Skipped[0] != "C" { + t.Errorf("Skipped = %v, want [C]", report.Skipped) + } + if len(audited) != 0 { + t.Errorf("audited = %v, want []", audited) + } +} + +// TestSetupEngine_StopOnError: with stopOnError=true, first failure aborts. +func TestSetupEngine_StopOnError(t *testing.T) { + provider := newEngineTestProvider(nil) + decls := []engineDecl{{name: "B"}, {name: "C"}} + selector := func(decls []engineDecl, _ []SecretStatus) ([]engineDecl, error) { return decls, nil } + valuer := func(d engineDecl) (string, bool, error) { + return "__fail__", true, nil + } + var audited []string + audit := func(name, store string) { audited = append(audited, name) } + + report, err := runSetupEngine(context.Background(), decls, + func(d engineDecl) string { return d.name }, + provider, selector, valuer, audit, true) + if err == nil { + t.Fatal("expected non-nil error with stopOnError=true") + } + // Only first secret should appear in Failed. + if len(report.Failed) != 1 { + t.Errorf("Failed = %v, want [B] (stopped after first)", report.Failed) + } +} diff --git a/cmd/wfctl/secrets_setup_interactive.go b/cmd/wfctl/secrets_setup_interactive.go new file mode 100644 index 00000000..06f93025 --- /dev/null +++ b/cmd/wfctl/secrets_setup_interactive.go @@ -0,0 +1,323 @@ +package main + +import ( + "context" + "errors" + "fmt" + "io" + "sort" + "time" + + "github.com/GoCodeAlone/workflow/cmd/wfctl/internal/prompt" + "github.com/GoCodeAlone/workflow/config" +) + +// interactiveSetupArgs carries the parsed flags for the interactive +// secrets setup path. +type interactiveSetupArgs struct { + configFile string + storeName string // --store flag override + envName string // --env, used for per-secret store resolution display +} + +// setupDecl is the declared-secret type used by both setup front-ends. +type setupDecl struct { + name string + sensitive bool + description string +} + +// builtinProviderTypes are the provider names a user can pick when no named +// store resolves and they must choose one interactively. +var builtinProviderTypes = []string{"env", "github", "vault", "aws", "keychain", "file"} + +// runSecretsSetupInteractive drives the interactive wizard: +// 1. Resolve the store (prompting with prompt.Select when unresolved). +// 2. Build the provider + print a store-access line (✓ / ✗ redacted). +// 3. Query per-entry status, prompt.MultiSelect which to set. +// 4. prompt.Input (masked when sensitive) for each selected value. +// 5. prompt.Confirm the summary, then run the shared engine. +// +// If any prompt returns prompt.ErrNotInteractive (stdin not a TTY despite the +// caller routing here), the function returns that error so the caller can fall +// back to the non-interactive path — it never hangs. +func runSecretsSetupInteractive(ctx context.Context, a *interactiveSetupArgs, out io.Writer) error { + cfg, err := config.LoadFromFile(a.configFile) + if err != nil { + return fmt.Errorf("load config: %w", err) + } + if cfg.Secrets == nil || len(cfg.Secrets.Entries) == 0 { + fmt.Fprintln(out, "No secrets declared in config.") + return nil + } + + decls := make([]setupDecl, 0, len(cfg.Secrets.Entries)) + for _, e := range cfg.Secrets.Entries { + decls = append(decls, setupDecl{ + name: e.Name, + sensitive: isSecretSensitive(e.Name), + description: e.Description, + }) + } + + // 1. Resolve the store (prompt when unresolved + interactive). + storeName, err := resolveSetupStoreInteractive(a.storeName, cfg) + if err != nil { + return err + } + + // 2. Build the provider + print an access line. + provider, err := getProviderForStore(storeName, cfg) + if err != nil { + return fmt.Errorf("resolve store %q: %w", storeName, err) + } + printStoreAccessLine(ctx, out, storeName, provider) + + // 3. Query per-entry status and let the user MultiSelect. + statuses := queryDeclStatuses(ctx, decls, provider) + items := buildMultiSelectItems(decls, statuses) + + selectedIdx, err := prompt.MultiSelect( + fmt.Sprintf("Select secrets to set in %q (space to toggle, enter to confirm)", storeName), + items, + ) + if err != nil { + if errors.Is(err, prompt.ErrNotInteractive) { + return err + } + return fmt.Errorf("select secrets: %w", err) + } + if len(selectedIdx) == 0 { + fmt.Fprintln(out, "No secrets selected; nothing to do.") + return nil + } + + // 4. Confirm before writing. + ok, err := prompt.Confirm( + fmt.Sprintf("Set %d secret(s) to store %q?", len(selectedIdx), storeName), + true, + ) + if err != nil { + if errors.Is(err, prompt.ErrNotInteractive) { + return err + } + return fmt.Errorf("confirm: %w", err) + } + if !ok { + fmt.Fprintln(out, "Aborted.") + return nil + } + + // 5. Build the engine selector (the user's MultiSelect choice) + valuer + // (masked prompt.Input). Reuse the SAME engine + audit as non-interactive. + selectedSet := make(map[string]bool, len(selectedIdx)) + for _, i := range selectedIdx { + selectedSet[decls[i].name] = true + } + selector := func(ds []setupDecl, _ []SecretStatus) ([]setupDecl, error) { + var keep []setupDecl + for _, d := range ds { + if selectedSet[d.name] { + keep = append(keep, d) + } + } + return keep, nil + } + + var promptErr error + valuer := func(d setupDecl) (string, bool, error) { + label := d.name + if d.description != "" { + label = d.name + " — " + d.description + } + v, err := prompt.Input(label, d.sensitive) + if err != nil { + if errors.Is(err, prompt.ErrNotInteractive) { + promptErr = err + } + return "", false, err + } + if v == "" { + // Empty input → skip this secret rather than write an empty value. + return "", false, nil + } + return v, true, nil + } + + auditFn := func(name, _ string) { + _ = writeSecretsAuditRecord(name, storeName) //nolint:errcheck // best-effort audit + } + + report, err := runSetupEngine(ctx, decls, + func(d setupDecl) string { return d.name }, + provider, selector, valuer, auditFn, false) + // If a prompt.Input hit a non-TTY mid-flow, surface ErrNotInteractive so the + // caller's fallback triggers. The engine runs with stopOnErr=false, so it + // returns (report, nil) even when the valuer reported ErrNotInteractive — + // hence this check must run regardless of err. + if promptErr != nil { + return promptErr + } + if err != nil { + return err + } + + printSetupReport(out, report) + if len(report.Failed) > 0 { + return fmt.Errorf("%d secret(s) failed to set", len(report.Failed)) + } + return nil +} + +// resolveSetupStoreInteractive resolves the store, prompting the user with +// prompt.Select when the resolver returns unresolved ("" + nil error). +func resolveSetupStoreInteractive(storeFlag string, cfg *config.WorkflowConfig) (string, error) { + defaultStore := "" + if cfg.Secrets != nil { + defaultStore = cfg.Secrets.DefaultStore + if defaultStore == "" { + defaultStore = cfg.Secrets.Provider + } + } + name, err := resolveSetupStoreName(storeFlag, defaultStore, cfg.SecretStores, true) + if err != nil { + return "", err + } + if name != "" { + return name, nil + } + // Unresolved + interactive → prompt over store keys + builtin provider types. + opts := storePickOptions(cfg.SecretStores) + idx, err := prompt.Select("Pick a secret store", opts) + if err != nil { + return "", err + } + return opts[idx], nil +} + +// storePickOptions returns the ordered list of store names a user can pick: +// configured secretStores keys (sorted) first, then builtin provider types +// that aren't already a store name. +func storePickOptions(stores map[string]*config.SecretStoreConfig) []string { + var keys []string + seen := make(map[string]bool) + for k := range stores { + keys = append(keys, k) + seen[k] = true + } + sort.Strings(keys) + for _, p := range builtinProviderTypes { + if !seen[p] { + keys = append(keys, p) + } + } + return keys +} + +// queryDeclStatuses queries the provider for each declared secret's status. +func queryDeclStatuses(ctx context.Context, decls []setupDecl, provider SecretsProvider) []SecretStatus { + statuses := make([]SecretStatus, 0, len(decls)) + for _, d := range decls { + state, _ := provider.Check(ctx, d.name) + statuses = append(statuses, SecretStatus{ + Name: d.name, + State: state, + IsSet: state == SecretSet, + }) + } + return statuses +} + +// buildMultiSelectItems builds the prompt.MultiSelect rows. Unset secrets are +// preselected; set secrets show their last-updated age when known. +func buildMultiSelectItems(decls []setupDecl, statuses []SecretStatus) []prompt.Item { + statusByName := make(map[string]SecretStatus, len(statuses)) + for _, s := range statuses { + statusByName[s.Name] = s + } + items := make([]prompt.Item, 0, len(decls)) + for _, d := range decls { + st := statusByName[d.name] + items = append(items, prompt.Item{ + Label: formatStatusLabel(d.name, st), + Preselected: !st.IsSet, // preselect unset + }) + } + return items +} + +// formatStatusLabel renders a MultiSelect row label for one secret. +// +// NAME ✓ set · updated 3d ago +// NAME ✗ unset +func formatStatusLabel(name string, st SecretStatus) string { + if st.IsSet { + age := formatRotatedAge(st.LastRotated) + if age != "" { + return fmt.Sprintf("%-24s ✓ set · updated %s", name, age) + } + return fmt.Sprintf("%-24s ✓ set", name) + } + return fmt.Sprintf("%-24s ✗ unset", name) +} + +// formatRotatedAge renders a coarse "Nd ago" / "Nh ago" string, or "" when +// the timestamp is zero (store doesn't expose it). +func formatRotatedAge(t time.Time) string { + if t.IsZero() { + return "" + } + d := time.Since(t) + switch { + case d < time.Hour: + mins := int(d.Minutes()) + if mins < 1 { + return "just now" + } + return fmt.Sprintf("%dm ago", mins) + case d < 24*time.Hour: + return fmt.Sprintf("%dh ago", int(d.Hours())) + default: + return fmt.Sprintf("%dd ago", int(d.Hours()/24)) + } +} + +// printStoreAccessLine type-asserts to the concrete adapter and prints a +// store-access status line. Errors are redacted to avoid leaking credentials. +func printStoreAccessLine(ctx context.Context, out io.Writer, storeName string, provider SecretsProvider) { + adapter, ok := provider.(secretsProviderAdapter) + if !ok { + fmt.Fprintf(out, "Store %q access: (unknown)\n", storeName) + return + } + if err := adapter.checkAccess(ctx); err != nil { + fmt.Fprintf(out, "Store %q access: ✗ %s\n", storeName, redactAccessError(err)) + return + } + fmt.Fprintf(out, "Store %q access: ✓\n", storeName) +} + +// redactAccessError returns a non-leaky description of an access-check error. +// We deliberately surface only the error class, never the message body, since +// provider errors can echo back credentials or tokens. +func redactAccessError(err error) string { + if err == nil { + return "" + } + return "not accessible (credentials or permissions; details redacted)" +} + +// printSetupReport prints the engine result summary (never values). +func printSetupReport(out io.Writer, report setupReport) { + for _, n := range report.Set { + fmt.Fprintf(out, " %-24s [set]\n", n) + } + for _, n := range report.Skipped { + fmt.Fprintf(out, " %-24s [skipped]\n", n) + } + for _, n := range report.Failed { + fmt.Fprintf(out, " %-24s [failed]\n", n) + } + fmt.Fprintf(out, "\nDone: %d set, %d skipped, %d failed.\n", + len(report.Set), len(report.Skipped), len(report.Failed)) +} diff --git a/cmd/wfctl/secrets_setup_interactive_test.go b/cmd/wfctl/secrets_setup_interactive_test.go new file mode 100644 index 00000000..5ece9cc8 --- /dev/null +++ b/cmd/wfctl/secrets_setup_interactive_test.go @@ -0,0 +1,184 @@ +package main + +import ( + "bytes" + "context" + "strings" + "testing" + "time" + + "github.com/GoCodeAlone/workflow/config" +) + +// TestStorePickOptions verifies the store-pick option list: configured store +// keys (sorted) come first, then builtin provider types not already present. +func TestStorePickOptions(t *testing.T) { + stores := map[string]*config.SecretStoreConfig{ + "zeta": {Provider: "file"}, + "alpha": {Provider: "vault"}, + "env": {Provider: "env"}, // shadows builtin "env" + } + opts := storePickOptions(stores) + + // First three must be the sorted store keys. + wantPrefix := []string{"alpha", "env", "zeta"} + for i, w := range wantPrefix { + if i >= len(opts) || opts[i] != w { + t.Fatalf("opts[%d] = %q, want %q (opts=%v)", i, safeIdx(opts, i), w, opts) + } + } + // "env" builtin must NOT be duplicated. + count := 0 + for _, o := range opts { + if o == "env" { + count++ + } + } + if count != 1 { + t.Errorf("'env' appears %d times, want 1 (opts=%v)", count, opts) + } + // Builtin types like github/vault/aws/keychain/file must appear. + for _, b := range []string{"github", "vault", "aws", "keychain", "file"} { + if !sliceHas(opts, b) { + t.Errorf("builtin %q missing from opts %v", b, opts) + } + } +} + +func TestStorePickOptions_NoStores(t *testing.T) { + opts := storePickOptions(nil) + // Should be exactly the builtin provider types. + if len(opts) != len(builtinProviderTypes) { + t.Fatalf("opts = %v, want %v", opts, builtinProviderTypes) + } +} + +// TestFormatStatusLabel verifies the MultiSelect row labels. +func TestFormatStatusLabel(t *testing.T) { + unset := formatStatusLabel("FOO", SecretStatus{Name: "FOO", IsSet: false}) + if !strings.Contains(unset, "✗ unset") { + t.Errorf("unset label = %q, want ✗ unset", unset) + } + + setNoTime := formatStatusLabel("BAR", SecretStatus{Name: "BAR", IsSet: true}) + if !strings.Contains(setNoTime, "✓ set") || strings.Contains(setNoTime, "updated") { + t.Errorf("set-no-time label = %q, want ✓ set without updated", setNoTime) + } + + setWithTime := formatStatusLabel("BAZ", SecretStatus{ + Name: "BAZ", IsSet: true, LastRotated: time.Now().Add(-72 * time.Hour), + }) + if !strings.Contains(setWithTime, "✓ set · updated") || !strings.Contains(setWithTime, "3d ago") { + t.Errorf("set-with-time label = %q, want '✓ set · updated 3d ago'", setWithTime) + } +} + +func TestFormatRotatedAge(t *testing.T) { + if got := formatRotatedAge(time.Time{}); got != "" { + t.Errorf("zero time → %q, want empty", got) + } + if got := formatRotatedAge(time.Now().Add(-30 * time.Minute)); got != "30m ago" { + t.Errorf("30m → %q", got) + } + if got := formatRotatedAge(time.Now().Add(-5 * time.Hour)); got != "5h ago" { + t.Errorf("5h → %q", got) + } + if got := formatRotatedAge(time.Now().Add(-48 * time.Hour)); got != "2d ago" { + t.Errorf("2d → %q", got) + } +} + +// TestBuildMultiSelectItems verifies unset secrets are preselected and set +// secrets are not. +func TestBuildMultiSelectItems(t *testing.T) { + decls := []setupDecl{ + {name: "SET_ONE"}, + {name: "UNSET_ONE"}, + } + statuses := []SecretStatus{ + {Name: "SET_ONE", IsSet: true, State: SecretSet}, + {Name: "UNSET_ONE", IsSet: false, State: SecretNotSet}, + } + items := buildMultiSelectItems(decls, statuses) + if len(items) != 2 { + t.Fatalf("items = %d, want 2", len(items)) + } + if items[0].Preselected { + t.Error("SET_ONE should NOT be preselected") + } + if !items[1].Preselected { + t.Error("UNSET_ONE SHOULD be preselected") + } + if !strings.Contains(items[0].Label, "✓ set") { + t.Errorf("SET_ONE label = %q", items[0].Label) + } + if !strings.Contains(items[1].Label, "✗ unset") { + t.Errorf("UNSET_ONE label = %q", items[1].Label) + } +} + +// TestQueryDeclStatuses verifies the status query against a fake provider. +func TestQueryDeclStatuses(t *testing.T) { + provider := newEngineTestProvider(map[string]string{"A": "v"}) + decls := []setupDecl{{name: "A"}, {name: "B"}} + statuses := queryDeclStatuses(context.Background(), decls, provider) + if len(statuses) != 2 { + t.Fatalf("statuses = %d, want 2", len(statuses)) + } + byName := map[string]SecretStatus{} + for _, s := range statuses { + byName[s.Name] = s + } + if !byName["A"].IsSet { + t.Error("A should be set") + } + if byName["B"].IsSet { + t.Error("B should be unset") + } +} + +// TestPrintStoreAccessLine_RedactsError verifies the access line never leaks +// the underlying error message (which could echo credentials). +func TestPrintStoreAccessLine_RedactsError(t *testing.T) { + // Build a file-store adapter pointed at a non-existent, non-creatable dir. + provider, err := getProviderForStore("env", &config.WorkflowConfig{}) + if err != nil { + t.Fatalf("build env provider: %v", err) + } + var buf bytes.Buffer + printStoreAccessLine(context.Background(), &buf, "env", provider) + out := buf.String() + // env provider has no CheckAccess → reports ✓. + if !strings.Contains(out, "access: ✓") { + t.Errorf("env access line = %q, want ✓", out) + } +} + +func TestRedactAccessError(t *testing.T) { + if got := redactAccessError(nil); got != "" { + t.Errorf("nil → %q, want empty", got) + } + msg := redactAccessError(context.DeadlineExceeded) + if strings.Contains(msg, "deadline") || strings.Contains(strings.ToLower(msg), "exceeded") { + t.Errorf("redacted message leaks original error: %q", msg) + } + if !strings.Contains(msg, "redacted") { + t.Errorf("redacted message = %q, want it to mention redaction", msg) + } +} + +func sliceHas(ss []string, s string) bool { + for _, x := range ss { + if x == s { + return true + } + } + return false +} + +func safeIdx(ss []string, i int) string { + if i < len(ss) { + return ss[i] + } + return "" +} diff --git a/cmd/wfctl/secrets_setup_noninteractive.go b/cmd/wfctl/secrets_setup_noninteractive.go new file mode 100644 index 00000000..34f9d759 --- /dev/null +++ b/cmd/wfctl/secrets_setup_noninteractive.go @@ -0,0 +1,226 @@ +package main + +import ( + "bufio" + "context" + "fmt" + "io" + "os" + "strings" + + "github.com/GoCodeAlone/workflow/config" + "github.com/mattn/go-isatty" +) + +// nonInteractiveSetupArgs carries all parsed flags for the non-interactive +// secrets setup path. It is separated from the flag-parsing so it can be +// unit-tested without spawning a real os.Stdin / flag.FlagSet. +type nonInteractiveSetupArgs struct { + configFile string + storeName string // --store flag; overrides defaultStore + fromEnv bool // --from-env: read value from $NAME + secretLiterals []string // --secret NAME=VALUE (process-table leak risk; for local only) + stdinKV []string // KEY=VALUE lines read from stdin pipe + only []string // --only A,B,C + skipExisting bool // --skip-existing + autoGenKeys bool // --auto-gen-keys: generate random values for _KEY/_SECRET/_TOKEN/_SIGNING +} + +// runSecretsSetupNonInteractive is the testable, context-less entry point +// that wraps runSecretsSetupNonInteractiveCtx with context.Background(). +func runSecretsSetupNonInteractive(a *nonInteractiveSetupArgs, out io.Writer) error { + return runSecretsSetupNonInteractiveCtx(context.Background(), a, out) +} + +// runSecretsSetupNonInteractiveCtx executes the non-interactive setup path. +// It: +// 1. Loads the workflow config. +// 2. Resolves the named store (--store > config.defaultStore > "env"). +// 3. Builds a selector from --only / --skip-existing / --all. +// 4. Builds a valuer from --from-env > stdin-KV > --secret. +// 5. Runs the engine and prints a summary. +func runSecretsSetupNonInteractiveCtx(ctx context.Context, a *nonInteractiveSetupArgs, out io.Writer) error { + cfg, err := config.LoadFromFile(a.configFile) + if err != nil { + return fmt.Errorf("load config: %w", err) + } + + if cfg.Secrets == nil || len(cfg.Secrets.Entries) == 0 { + fmt.Fprintln(out, "No secrets declared in config.") + return nil + } + + // Resolve which store to use via the 5-priority resolver. + defaultStore := "" + if cfg.Secrets != nil { + defaultStore = cfg.Secrets.DefaultStore + if defaultStore == "" { + defaultStore = cfg.Secrets.Provider + } + } + storeName, err := resolveSetupStoreName(a.storeName, defaultStore, cfg.SecretStores, false) + if err != nil { + return err + } + + provider, err := getProviderForStore(storeName, cfg) + if err != nil { + return fmt.Errorf("resolve store %q: %w", storeName, err) + } + + // Build value lookup maps (priority: from-env > stdin-KV > --secret). + // --secret literals (process-table leak risk): + secretMap := make(map[string]string) + for _, lit := range a.secretLiterals { + k, v, ok := strings.Cut(lit, "=") + if !ok { + return fmt.Errorf("--secret %q: expected NAME=VALUE format", lit) + } + secretMap[k] = v + } + for _, kv := range a.stdinKV { + k, v, ok := strings.Cut(kv, "=") + if !ok { + continue + } + secretMap[k] = v + } + + // Build the declared list from config entries. + type decl struct { + name string + sensitive bool + } + var decls []decl + for _, e := range cfg.Secrets.Entries { + decls = append(decls, decl{name: e.Name, sensitive: isSecretSensitive(e.Name)}) + } + + // Selector: --only restricts; --skip-existing filters already-set. + onlySet := make(map[string]bool, len(a.only)) + for _, n := range a.only { + onlySet[n] = true + } + + selector := func(ds []decl, statuses []SecretStatus) ([]decl, error) { + setMap := make(map[string]bool, len(statuses)) + for _, s := range statuses { + if s.IsSet { + setMap[s.Name] = true + } + } + var out []decl + for _, d := range ds { + if len(onlySet) > 0 && !onlySet[d.name] { + continue + } + if a.skipExisting && setMap[d.name] { + continue + } + out = append(out, d) + } + return out, nil + } + + // failureReasons records the actionable per-secret error so it can be + // printed alongside the [failed] summary line (the engine accumulator only + // keeps the name when stopOnErr=false). + failureReasons := make(map[string]string) + + // Valuer: --from-env > stdinKV/--secret > --auto-gen-keys > error if none. + // When --from-env is set, a missing env var is a skip (provided=false) + // rather than an error — the caller only wants to set what's available. + // When no value source at all is configured, this is a hard error. + valuer := func(d decl) (string, bool, error) { + if a.fromEnv { + v := os.Getenv(d.name) + if v != "" { + return v, true, nil + } + // env var not set — check the other sources before skipping. + } + if v, ok := secretMap[d.name]; ok { + return v, true, nil + } + // --auto-gen-keys: generate a random value for key/secret/token names. + if a.autoGenKeys && isAutoGenCandidate(d.name) { + if gv := generateSecretValue(); gv != "" { + return gv, true, nil + } + } + if a.fromEnv { + // from-env was the value source but $NAME was empty → skip, not error. + return "", false, nil + } + // No value source at all — non-interactive hard error naming the secret. + err := fmt.Errorf("no value for secret %q: set $%s, pass --from-env, or use --secret %s=VALUE", d.name, d.name, d.name) + failureReasons[d.name] = err.Error() + return "", false, err + } + + // Audit: append a JSONL record for each successful Set (never the value). + auditFn := func(name, _ string) { + _ = writeSecretsAuditRecord(name, storeName) //nolint:errcheck // best-effort audit + } + + report, err := runSetupEngine(ctx, decls, + func(d decl) string { return d.name }, + provider, selector, valuer, auditFn, false) + if err != nil { + return err + } + + // Print summary (never print values). + for _, n := range report.Set { + fmt.Fprintf(out, " %-24s [set]\n", n) + } + for _, n := range report.Skipped { + fmt.Fprintf(out, " %-24s [skipped]\n", n) + } + for _, n := range report.Failed { + if reason := failureReasons[n]; reason != "" { + fmt.Fprintf(out, " %-24s [failed] %s\n", n, reason) + } else { + fmt.Fprintf(out, " %-24s [failed]\n", n) + } + } + fmt.Fprintf(out, "\nDone: %d set, %d skipped, %d failed.\n", + len(report.Set), len(report.Skipped), len(report.Failed)) + + if len(report.Failed) > 0 { + return fmt.Errorf("%d secret(s) failed to set: %s", len(report.Failed), strings.Join(report.Failed, ", ")) + } + return nil +} + +// readStdinKV reads KEY=VALUE pairs from stdin when stdin is a pipe (not a TTY). +// Returns nil if stdin is a terminal (nothing to read without blocking). +func readStdinKV() []string { + if isatty.IsTerminal(os.Stdin.Fd()) { + return nil + } + var pairs []string + scanner := bufio.NewScanner(os.Stdin) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + if strings.Contains(line, "=") { + pairs = append(pairs, line) + } + } + return pairs +} + +// isSecretSensitive returns true when the name looks like it should be masked +// in interactive prompts (ends in _KEY, _SECRET, _TOKEN, _PASSWORD, etc.). +func isSecretSensitive(name string) bool { + up := strings.ToUpper(name) + for _, suffix := range []string{"_KEY", "_SECRET", "_TOKEN", "_PASSWORD", "_PASS", "_SIGNING"} { + if strings.HasSuffix(up, suffix) { + return true + } + } + return false +} diff --git a/cmd/wfctl/secrets_setup_noninteractive_test.go b/cmd/wfctl/secrets_setup_noninteractive_test.go new file mode 100644 index 00000000..a6fef142 --- /dev/null +++ b/cmd/wfctl/secrets_setup_noninteractive_test.go @@ -0,0 +1,280 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeSetupConfig writes a minimal app.yaml + a file-store directory for +// non-interactive setup tests. Returns (configPath, storeDir). +func writeSetupConfig(t *testing.T, entries ...string) (string, string) { + t.Helper() + tmp := t.TempDir() + storeDir := filepath.Join(tmp, "store") + if err := os.MkdirAll(storeDir, 0o755); err != nil { + t.Fatalf("mkdir store: %v", err) + } + + var entryLines []string + for _, name := range entries { + entryLines = append(entryLines, " - name: "+name) + } + entriesYAML := strings.Join(entryLines, "\n") + + cfg := `modules: + - name: http + type: http.server + config: + address: ":0" +secrets: + defaultStore: localfs + entries: +` + entriesYAML + ` +secretStores: + localfs: + provider: file + config: + path: ` + storeDir + ` +` + cfgPath := filepath.Join(tmp, "app.yaml") + if err := os.WriteFile(cfgPath, []byte(cfg), 0o644); err != nil { + t.Fatalf("write app.yaml: %v", err) + } + return cfgPath, storeDir +} + +// TestSetupAllOnlyConflict verifies --all and --only are mutually exclusive. +func TestSetupAllOnlyConflict(t *testing.T) { + cfgPath, _ := writeSetupConfig(t, "A") + err := runSecretsSetup([]string{"--all", "--only", "A", "--config", cfgPath}) + if err == nil { + t.Fatal("expected error when --all and --only are both given") + } + if !strings.Contains(err.Error(), "mutually exclusive") { + t.Errorf("error = %v, want 'mutually exclusive'", err) + } +} + +// TestSetupAllFlagSetsEverything verifies --all (the explicit default) sets all +// declared secrets. Under `go test` stdin is not a TTY, so runSecretsSetup +// routes to the non-interactive path; --secret supplies the values. +func TestSetupAllFlagSetsEverything(t *testing.T) { + cfgPath, storeDir := writeSetupConfig(t, "A", "B") + err := runSecretsSetup([]string{ + "--all", + "--secret", "A=av", + "--secret", "B=bv", + "--store", "localfs", + "--config", cfgPath, + }) + if err != nil { + t.Fatalf("--all setup: %v", err) + } + for name, want := range map[string]string{"A": "av", "B": "bv"} { + data, readErr := os.ReadFile(filepath.Join(storeDir, name)) + if readErr != nil { + t.Fatalf("read %s: %v", name, readErr) + } + if string(data) != want { + t.Errorf("%s = %q, want %q", name, string(data), want) + } + } +} + +// TestNonInteractive_FromEnv verifies --from-env reads the secret value +// from $NAME without any argv leak. +func TestNonInteractive_FromEnv(t *testing.T) { + cfgPath, storeDir := writeSetupConfig(t, "A", "B") + t.Setenv("A", "hunter2") + + var buf bytes.Buffer + err := runSecretsSetupNonInteractive(&nonInteractiveSetupArgs{ + configFile: cfgPath, + fromEnv: true, + only: []string{"A"}, + storeName: "localfs", + }, &buf) + if err != nil { + t.Fatalf("non-interactive setup: %v", err) + } + // A should be written to the file store. + data, readErr := os.ReadFile(filepath.Join(storeDir, "A")) + if readErr != nil { + t.Fatalf("read A from store: %v", readErr) + } + if string(data) != "hunter2" { + t.Errorf("A = %q, want hunter2", string(data)) + } + // B should NOT be written. + if _, err := os.Stat(filepath.Join(storeDir, "B")); !os.IsNotExist(err) { + t.Error("B should not have been set") + } + // Output must not contain the secret value. + out := buf.String() + if strings.Contains(out, "hunter2") { + t.Errorf("output contains secret value: %s", out) + } +} + +// TestNonInteractive_SkipExisting verifies --skip-existing skips secrets +// that are already set in the store. +func TestNonInteractive_SkipExisting(t *testing.T) { + cfgPath, storeDir := writeSetupConfig(t, "A", "B") + // Pre-set A. + if err := os.WriteFile(filepath.Join(storeDir, "A"), []byte("old"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("A", "hunter2") + t.Setenv("B", "bvalue") + + var buf bytes.Buffer + err := runSecretsSetupNonInteractive(&nonInteractiveSetupArgs{ + configFile: cfgPath, + fromEnv: true, + skipExisting: true, + storeName: "localfs", + }, &buf) + if err != nil { + t.Fatalf("non-interactive setup: %v", err) + } + // A must not be overwritten. + data, _ := os.ReadFile(filepath.Join(storeDir, "A")) + if string(data) != "old" { + t.Errorf("A = %q, want old (skip-existing)", string(data)) + } + // B must be set (was absent). + bData, _ := os.ReadFile(filepath.Join(storeDir, "B")) + if string(bData) != "bvalue" { + t.Errorf("B = %q, want bvalue", string(bData)) + } + out := buf.String() + if strings.Contains(out, "hunter2") || strings.Contains(out, "bvalue") { + t.Errorf("output contains secret value: %s", out) + } +} + +// TestNonInteractive_SecretFlag verifies --secret B=v sets B. +func TestNonInteractive_SecretFlag(t *testing.T) { + cfgPath, storeDir := writeSetupConfig(t, "A", "B") + + var buf bytes.Buffer + err := runSecretsSetupNonInteractive(&nonInteractiveSetupArgs{ + configFile: cfgPath, + secretLiterals: []string{"B=bval"}, + only: []string{"B"}, + storeName: "localfs", + }, &buf) + if err != nil { + t.Fatalf("non-interactive setup: %v", err) + } + data, _ := os.ReadFile(filepath.Join(storeDir, "B")) + if string(data) != "bval" { + t.Errorf("B = %q, want bval", string(data)) + } +} + +// TestNonInteractive_AutoGenKeys verifies --auto-gen-keys generates a value +// for key/secret/token names and writes it, while leaving non-candidate names +// to error/skip. +func TestNonInteractive_AutoGenKeys(t *testing.T) { + cfgPath, storeDir := writeSetupConfig(t, "API_KEY") + + var buf bytes.Buffer + err := runSecretsSetupNonInteractive(&nonInteractiveSetupArgs{ + configFile: cfgPath, + autoGenKeys: true, + storeName: "localfs", + }, &buf) + if err != nil { + t.Fatalf("auto-gen setup: %v", err) + } + data, readErr := os.ReadFile(filepath.Join(storeDir, "API_KEY")) + if readErr != nil { + t.Fatalf("read API_KEY: %v", readErr) + } + if len(strings.TrimSpace(string(data))) == 0 { + t.Error("API_KEY should have an auto-generated value") + } + // The generated value must NOT appear in stdout. + if strings.Contains(buf.String(), string(data)) { + t.Errorf("output leaked generated value: %s", buf.String()) + } +} + +// TestNonInteractive_MissingValue verifies that a selected secret with no +// value source causes a named error (no hang). +func TestNonInteractive_MissingValue(t *testing.T) { + cfgPath, _ := writeSetupConfig(t, "A") + + var buf bytes.Buffer + err := runSecretsSetupNonInteractive(&nonInteractiveSetupArgs{ + configFile: cfgPath, + only: []string{"A"}, + storeName: "localfs", + }, &buf) + if err == nil { + t.Fatal("expected error for missing value") + } + if !strings.Contains(err.Error(), "A") { + t.Errorf("error should name the missing secret: %v", err) + } +} + +// TestNonInteractive_ProviderReceivesExactSets ensures the provider is +// called exactly for the expected secrets (integration-style using a +// fresh file store as the backing provider). +func TestNonInteractive_ProviderReceivesExactSets(t *testing.T) { + cfgPath, storeDir := writeSetupConfig(t, "A", "B", "C") + t.Setenv("A", "aval") + t.Setenv("C", "cval") + + var buf bytes.Buffer + err := runSecretsSetupNonInteractive(&nonInteractiveSetupArgs{ + configFile: cfgPath, + fromEnv: true, + only: []string{"A", "C"}, + storeName: "localfs", + }, &buf) + if err != nil { + t.Fatalf("non-interactive setup: %v", err) + } + // A and C set; B untouched. + aData, _ := os.ReadFile(filepath.Join(storeDir, "A")) + cData, _ := os.ReadFile(filepath.Join(storeDir, "C")) + if string(aData) != "aval" { + t.Errorf("A = %q", aData) + } + if string(cData) != "cval" { + t.Errorf("C = %q", cData) + } + if _, err := os.Stat(filepath.Join(storeDir, "B")); !os.IsNotExist(err) { + t.Error("B should not have been set") + } +} + +// Ensure the json import is used (it will be used by JSON output in full binary). +var _ = json.Marshal + +// TestNonInteractive_ProviderReceivesExactSets_Context ensures context +// is propagated correctly. +func TestNonInteractive_ContextCancel(t *testing.T) { + cfgPath, _ := writeSetupConfig(t, "A") + t.Setenv("A", "val") + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel immediately + + var buf bytes.Buffer + // Should not hang — cancelled context. + _ = runSecretsSetupNonInteractiveCtx(ctx, &nonInteractiveSetupArgs{ + configFile: cfgPath, + fromEnv: true, + storeName: "localfs", + }, &buf) + // We don't assert success or failure here; just that it returns. +} diff --git a/cmd/wfctl/secrets_setup_plugin.go b/cmd/wfctl/secrets_setup_plugin.go index d9dc8915..ae3fe787 100644 --- a/cmd/wfctl/secrets_setup_plugin.go +++ b/cmd/wfctl/secrets_setup_plugin.go @@ -11,9 +11,9 @@ import ( "path/filepath" "strings" + "github.com/GoCodeAlone/workflow/cmd/wfctl/internal/prompt" "github.com/GoCodeAlone/workflow/secrets" "github.com/mattn/go-isatty" - "golang.org/x/term" ) // PluginRequiredSecret mirrors the plugin.json `required_secrets[]` @@ -51,11 +51,20 @@ func runSecretsSetupPluginWithIO(args []string, in io.Reader, out io.Writer) err orgVisibility := fs.String("visibility", "all", "Org-scope visibility: all | selected | private") tokenEnv := fs.String("token-env", "GITHUB_TOKEN", "Env var holding the GitHub PAT") configFile := fs.String("config", "app.yaml", "app.yaml (used to resolve the github repo when --scope=repo|env)") + fromEnv := fs.Bool("from-env", false, "Read each secret value from $NAME (recommended for CI; avoids process-table leaks)") + var secretFlag multiStringFlag + fs.Var(&secretFlag, "secret", "NAME=VALUE literal (WARNING: leaks to process table; use --from-env in CI). Repeatable.") fs.Usage = func() { fmt.Fprintf(fs.Output(), `Usage: wfctl secrets setup --plugin [options] -Interactively set the secrets declared by a plugin's plugin.json -required_secrets[] block. Sensitive fields are masked. +Set the secrets declared by a plugin's plugin.json required_secrets[] block. + +Interactive (default when stdin is a TTY): each secret is prompted; sensitive +fields are masked. + +Non-interactive (when stdin is not a TTY): values come from --from-env ($NAME), +--secret NAME=VALUE, or piped KEY=VALUE lines. A secret with no value source is +skipped (never blocks waiting for input). Options: `) @@ -80,26 +89,112 @@ Options: // Pre-build the destination provider so a malformed scope fails // loud BEFORE prompting. scopeStr := strings.ToLower(strings.TrimSpace(*scope)) - provider, scopeLabel, err := buildSecretWriter(scopeStr, *envName, *org, *orgVisibility, *tokenEnv, *configFile) + ghProvider, scopeLabel, err := buildSecretWriter(scopeStr, *envName, *org, *orgVisibility, *tokenEnv, *configFile) if err != nil { return err } fmt.Fprintf(out, "Setting up secrets for plugin %q → %s\n\n", manifest.Name, scopeLabel) - for _, rs := range manifest.RequiredSecrets { - val, err := promptOne(rs, in) - if err != nil { - return err - } - if val == "" { - fmt.Fprintf(out, " %s: skipped (no value provided)\n", rs.Name) - continue + // Wrap the GitHub provider in the shared adapter so the engine can use it. + provider := secretsProviderAdapter{p: ghProvider} + + // Selector: set every declared required secret (no skip-existing for the + // plugin flow — required secrets are always offered). + selector := func(ds []PluginRequiredSecret, _ []SecretStatus) ([]PluginRequiredSecret, error) { + return ds, nil + } + + // Decide the input mode up-front: + // - in != nil → reader path (tests / explicit pipe). + // - in == nil && stdin is a TTY → interactive prompt.Input (masked). + // - in == nil && stdin is NOT a TTY → non-interactive value sources only + // (--from-env / --secret); a secret with no source is SKIPPED, never read + // via Fscanln (which would block forever on an open empty pipe). + stdinIsTTY := isatty.IsTerminal(os.Stdin.Fd()) + interactive := in == nil && stdinIsTTY + + // Build the non-interactive value source map (--secret literals). + secretMap := make(map[string]string) + for _, lit := range secretFlag { + k, v, found := strings.Cut(lit, "=") + if !found { + return fmt.Errorf("--secret %q: expected NAME=VALUE format", lit) } - if err := provider.Set(context.Background(), rs.Name, val); err != nil { - return fmt.Errorf("set %s: %w", rs.Name, err) + secretMap[k] = v + } + + var promptErr error + valuer := func(rs PluginRequiredSecret) (string, bool, error) { + switch { + case in != nil: + // Reader path (tests / explicit pipe): one line per secret. + val, verr := promptOne(rs, in) + if verr != nil { + return "", false, verr + } + if val == "" { + return "", false, nil + } + return val, true, nil + + case interactive: + label := rs.Prompt + if label == "" { + label = rs.Name + } + if rs.Description != "" { + label = label + " — " + rs.Description + } + val, verr := prompt.Input(label, rs.Sensitive) + if verr != nil { + if errors.Is(verr, prompt.ErrNotInteractive) { + promptErr = verr + } + return "", false, verr + } + if val == "" { + return "", false, nil + } + return val, true, nil + + default: + // Non-interactive (non-TTY): value sources only — NEVER block on stdin. + if *fromEnv { + if v := os.Getenv(rs.Name); v != "" { + return v, true, nil + } + } + if v, ok := secretMap[rs.Name]; ok { + return v, true, nil + } + // No value source for this secret → skip (don't hang, don't fail hard). + return "", false, nil } - fmt.Fprintf(out, " %s: set\n", rs.Name) + } + + auditFn := func(name, _ string) { + _ = writeSecretsAuditRecord(name, "github:"+scopeStr) //nolint:errcheck // best-effort audit + } + + report, err := runSetupEngine(context.Background(), manifest.RequiredSecrets, + func(rs PluginRequiredSecret) string { return rs.Name }, + provider, selector, valuer, auditFn, true) + // Surface a mid-flow ErrNotInteractive regardless of the engine's error + // (stopOnErr=true means it usually returns the wrapped error, but check the + // captured promptErr too for robustness). + if promptErr != nil { + return promptErr + } + if err != nil { + return err + } + + for _, n := range report.Set { + fmt.Fprintf(out, " %s: set\n", n) + } + for _, n := range report.Skipped { + fmt.Fprintf(out, " %s: skipped (no value provided)\n", n) } fmt.Fprintf(out, "\nAll done.\n") return nil @@ -128,9 +223,11 @@ func loadPluginManifest(name, dirOverride string) (*pluginManifest, error) { return &m, nil } -// promptOne reads a value for one required secret. Masks if Sensitive. -// When `in` is non-nil (tests / piped input) it reads a line from it -// regardless of Sensitive — masking is interactive-only. +// promptOne reads a single value for one required secret from the supplied +// reader. It is used only on the reader-backed path (tests / explicit piped +// input); masking is interactive-only and handled by prompt.Input on a TTY, so +// this helper never touches os.Stdin and therefore can never block on an open +// empty pipe via Fscanln. func promptOne(rs PluginRequiredSecret, in io.Reader) (string, error) { label := rs.Prompt if label == "" { @@ -141,45 +238,25 @@ func promptOne(rs PluginRequiredSecret, in io.Reader) (string, error) { } fmt.Fprintf(os.Stderr, "%s: ", label) - if in != nil { - // Test/piped path — read one line. - buf := make([]byte, 4096) - n, err := in.Read(buf) - if err != nil && err != io.EOF { - return "", err - } - return strings.TrimRight(string(buf[:n]), "\r\n"), nil + if in == nil { + // Defensive: callers must pass a reader. Treat a nil reader as "no + // value" rather than reading os.Stdin (which could block). + return "", nil } - - if rs.Sensitive && isatty.IsTerminal(os.Stdin.Fd()) { - fd, err := stdinFileDescriptor() - if err != nil { - return "", err - } - b, err := term.ReadPassword(fd) - fmt.Fprintln(os.Stderr) - if err != nil { - return "", fmt.Errorf("read masked: %w", err) - } - return string(b), nil - } - // Non-sensitive interactive — echo. - var line string - if _, err := fmt.Fscanln(os.Stdin, &line); err != nil && err.Error() != "unexpected newline" { + buf := make([]byte, 4096) + n, err := in.Read(buf) + if err != nil && err != io.EOF { return "", err } - return line, nil -} - -// scopedWriter is the narrow interface secrets setup --plugin needs. -// Both secrets.GitHubSecretsProvider satisfies it. -type scopedWriter interface { - Set(ctx context.Context, key, value string) error + return strings.TrimRight(string(buf[:n]), "\r\n"), nil } // buildSecretWriter mints the GitHub provider for the requested scope. -// scopeLabel is a human-readable string for the setup prelude. -func buildSecretWriter(scope, envName, org, visibility, tokenEnv, configFile string) (scopedWriter, string, error) { +// scopeLabel is a human-readable string for the setup prelude. The returned +// provider is a full secrets.Provider (the GitHub providers implement Get/Set/ +// Delete/List + StatAll + CheckAccess) so it can be wrapped in the shared +// secretsProviderAdapter and driven by the setup engine. +func buildSecretWriter(scope, envName, org, visibility, tokenEnv, configFile string) (secrets.Provider, string, error) { switch scope { case "org": if org == "" { diff --git a/go.mod b/go.mod index 1cc90c6b..720bbed2 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/GoCodeAlone/workflow go 1.26.0 require ( + charm.land/bubbles/v2 v2.0.0 charm.land/bubbletea/v2 v2.0.2 charm.land/lipgloss/v2 v2.0.3 github.com/GoCodeAlone/go-plugin v1.7.0 @@ -75,6 +76,7 @@ require ( github.com/Workiva/go-datastructures v1.1.7 // indirect github.com/andybalholm/brotli v1.2.1 // indirect github.com/armon/go-metrics v0.4.1 // indirect + github.com/atotto/clipboard v0.1.4 // indirect github.com/aws/aws-sdk-go-v2 v1.41.6 // indirect github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8 // indirect github.com/aws/aws-sdk-go-v2/config v1.32.16 // indirect diff --git a/go.sum b/go.sum index 606ccee7..8667deb8 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +charm.land/bubbles/v2 v2.0.0 h1:tE3eK/pHjmtrDiRdoC9uGNLgpopOd8fjhEe31B/ai5s= +charm.land/bubbles/v2 v2.0.0/go.mod h1:rCHoleP2XhU8um45NTuOWBPNVHxnkXKTiZqcclL/qOI= charm.land/bubbletea/v2 v2.0.2 h1:4CRtRnuZOdFDTWSff9r8QFt/9+z6Emubz3aDMnf/dx0= charm.land/bubbletea/v2 v2.0.2/go.mod h1:3LRff2U4WIYXy7MTxfbAQ+AdfM3D8Xuvz2wbsOD9OHQ= charm.land/lipgloss/v2 v2.0.3 h1:yM2zJ4Cf5Y51b7RHIwioil4ApI/aypFXXVHSwlM6RzU= @@ -52,6 +54,8 @@ github.com/antithesishq/antithesis-sdk-go v0.7.0 h1:uWDG8BqLD1lI2ps38WDz2vXflrTX github.com/antithesishq/antithesis-sdk-go v0.7.0/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/armon/go-metrics v0.4.1 h1:hR91U9KYmb6bLBYLQjyM+3j+rcd/UhE+G78SFnF8gJA= github.com/armon/go-metrics v0.4.1/go.mod h1:E6amYzXo6aW1tqzoZGT755KkbgrJsSdpwZ+3JqfkOG4= +github.com/atotto/clipboard v0.1.4 h1:EH0zSVneZPSuFR11BlR9YppQTVDbh5+16AmcJi4g1z4= +github.com/atotto/clipboard v0.1.4/go.mod h1:ZY9tmq7sm5xIbd9bOK4onWV4S6X0u6GY7Vn0Yu86PYI= github.com/aws/aws-sdk-go-v2 v1.41.6 h1:1AX0AthnBQzMx1vbmir3Y4WsnJgiydmnJjiLu+LvXOg= github.com/aws/aws-sdk-go-v2 v1.41.6/go.mod h1:dy0UzBIfwSeot4grGvY1AqFWN5zgziMmWGzysDnHFcQ= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8 h1:eBMB84YGghSocM7PsjmmPffTa+1FBUeNvGvFou6V/4o=