diff --git a/internal/tui/app.go b/internal/tui/app.go index babc4e1..2e9d443 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -100,6 +100,10 @@ type serverDeletedMsg struct { err error } +type discardFormMsg struct { + origin screen +} + type importDoneMsg struct { servers []*model.Server count int @@ -225,6 +229,8 @@ type confirmState struct { consequence string verb string parent screen + complete screen + completeSet bool focus confirmChoice pending bool action func() tea.Cmd @@ -486,6 +492,18 @@ func (m *tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { delete(m.selected, msg.alias) return m, nil + case discardFormMsg: + m.finishConfirm() + switch msg.origin { + case screenForm: + m.form = nil + case screenForwardForm: + m.forwardForm = nil + case screenTemplateForm: + m.templateForm = nil + } + return m, nil + case forwardDeleteConfirmMsg: m.beginConfirm(confirmState{ title: "Delete port forward?", @@ -966,6 +984,10 @@ func (m *tuiModel) updateTemplates(msg tea.KeyMsg) (tea.Model, tea.Cmd) { func (m *tuiModel) updateTemplateForm(msg tea.KeyMsg) (tea.Model, tea.Cmd) { if msg.Type == tea.KeyEsc { + if m.templateForm != nil && m.templateForm.Dirty() { + m.confirmDiscard("Command template", screenTemplateForm, screenTemplates) + return m, nil + } m.screen = screenTemplates m.templateForm = nil return m, nil @@ -1081,6 +1103,10 @@ func (m *tuiModel) updateForm(msg tea.KeyMsg) (tea.Model, tea.Cmd) { } return m, cmd } + if m.form != nil && m.form.Dirty() { + m.confirmDiscard("Server profile", screenForm, screenList) + return m, nil + } m.screen = screenList m.form = nil @@ -1496,10 +1522,29 @@ func (m *tuiModel) finishConfirm() { if m.confirm == nil { return } - m.screen = m.confirm.parent + destination := m.confirm.parent + if m.confirm.completeSet { + destination = m.confirm.complete + } + m.screen = destination m.confirm = nil } +func (m *tuiModel) confirmDiscard(title string, origin, destination screen) { + m.beginConfirm(confirmState{ + title: "Discard unsaved changes?", + target: title, + consequence: "Your edits on this form will be lost.", + verb: "Discard", + parent: origin, + complete: destination, + completeSet: true, + action: func() tea.Cmd { + return func() tea.Msg { return discardFormMsg{origin: origin} } + }, + }) +} + func (m *tuiModel) confirmServerDelete(server *model.Server) { alias := server.Alias m.beginConfirm(confirmState{ @@ -1594,6 +1639,10 @@ func (m *tuiModel) screenOwnsPrintableInput() bool { func (m *tuiModel) updateForwardForm(msg tea.KeyMsg) (tea.Model, tea.Cmd) { if msg.Type == tea.KeyEsc { + if m.forwardForm != nil && m.forwardForm.Dirty() { + m.confirmDiscard("Port forward", screenForwardForm, screenForwardList) + return m, nil + } m.screen = screenForwardList m.forwardForm = nil return m, nil diff --git a/internal/tui/form.go b/internal/tui/form.go index 5414a32..41cd52f 100644 --- a/internal/tui/form.go +++ b/internal/tui/form.go @@ -2,6 +2,7 @@ package tui import ( "fmt" + "strconv" "strings" "time" @@ -62,6 +63,12 @@ type formModel struct { showGroupList bool authList list.Model showAuthList bool + initial formSnapshot +} + +type formSnapshot struct { + values []string + password string } func newFormModel(w, h int) *formModel { @@ -85,6 +92,7 @@ func newFormModel(w, h int) *formModel { inputs[i].Placeholder = placeholderForLabel(label) inputs[i].CharLimit = 128 } + inputs[3].SetValue("22") pw := textinput.New() pw.Placeholder = "optional" @@ -122,6 +130,7 @@ func newFormModel(w, h int) *formModel { } fm.updateFocus() + fm.initial = fm.snapshot() return fm } @@ -202,9 +211,31 @@ func newEditFormModel(s *model.Server, w, h int) *formModel { } } fm.updateFocus() + fm.initial = fm.snapshot() return fm } +func (fm *formModel) snapshot() formSnapshot { + values := make([]string, len(fm.inputs)) + for i := range fm.inputs { + values[i] = fm.inputs[i].Value() + } + return formSnapshot{values: values, password: fm.password.Value()} +} + +func (fm *formModel) Dirty() bool { + current := fm.snapshot() + if current.password != fm.initial.password || len(current.values) != len(fm.initial.values) { + return true + } + for i := range current.values { + if current.values[i] != fm.initial.values[i] { + return true + } + } + return false +} + func (fm *formModel) Init() tea.Cmd { return nil } @@ -389,6 +420,14 @@ func (fm *formModel) updateFocus() { func (fm *formModel) labelAt(index int) string { if index >= 0 && index < len(fm.labels) { + switch index { + case 0: + return "Alias *" + case 2: + return "Host *" + case 3: + return "Port *" + } if index == 5 { return "Auth Method (/ pick)" } @@ -409,6 +448,10 @@ func (fm *formModel) runTest() tea.Cmd { fm.err = nil fm.saved = false + if _, err := parsePort(fm.inputs[3].Value()); err != nil { + fm.testing = false + return func() tea.Msg { return testDoneMsg{ok: false, err: err.Error()} } + } s := fm.buildServer() pw := fm.password.Value() @@ -434,6 +477,9 @@ func (fm *formModel) runSave() tea.Cmd { fm.saved = false fm.testResult = "" + if _, err := parsePort(fm.inputs[3].Value()); err != nil { + return func() tea.Msg { return saveDoneMsg{err: err} } + } s := fm.buildServer() pw := fm.password.Value() @@ -482,8 +528,7 @@ func parseRouteHops(input string) model.Route { } func (fm *formModel) buildServer() *model.Server { - port := 22 - fmt.Sscanf(fm.inputs[3].Value(), "%d", &port) + port, _ := parsePort(fm.inputs[3].Value()) authMethod := model.AuthMethod(fm.inputs[5].Value()) if authMethod == "" { authMethod = model.AuthKey @@ -508,6 +553,18 @@ func (fm *formModel) buildServer() *model.Server { } } +func parsePort(value string) (int, error) { + value = strings.TrimSpace(value) + port, err := strconv.Atoi(value) + if err != nil { + return 0, fmt.Errorf("Port must be a number from 1 to 65535") + } + if port < 1 || port > 65535 { + return 0, fmt.Errorf("Port must be between 1 and 65535") + } + return port, nil +} + func (fm *formModel) View() string { var b strings.Builder @@ -602,9 +659,9 @@ func (fm *formModel) View() string { if fm.saved { b.WriteString("\n" + successStyle.Render("✓ Saved.") + "\n") } - if fm.err != nil { - b.WriteString("\n" + errorStyle.Render(fmt.Sprintf("✗ Error: %v", fm.err)) + "\n") - } + } + if fm.err != nil { + b.WriteString("\n" + errorStyle.Render(fmt.Sprintf("✗ Error: %v", fm.err)) + "\n") } testBtn := "[ Test ]" diff --git a/internal/tui/form_validation_test.go b/internal/tui/form_validation_test.go new file mode 100644 index 0000000..94fdd56 --- /dev/null +++ b/internal/tui/form_validation_test.go @@ -0,0 +1,195 @@ +package tui + +import ( + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + "github.com/mirivlad/sshkeeper/internal/model" +) + +func TestParsePortRejectsInvalidValues(t *testing.T) { + for _, value := range []string{"", "abc", "0", "65536", "22x"} { + t.Run(value, func(t *testing.T) { + if _, err := parsePort(value); err == nil { + t.Fatalf("parsePort(%q) succeeded", value) + } + }) + } + if port, err := parsePort("22"); err != nil || port != 22 { + t.Fatalf("parsePort(22) = %d, %v", port, err) + } +} + +func TestServerFormPreservesInvalidPortAndDoesNotSave(t *testing.T) { + fm := newFormModel(80, 24) + fm.inputs[0].SetValue("prod") + fm.inputs[2].SetValue("prod.example") + fm.inputs[3].SetValue("abc") + oldSave := SaveServer + t.Cleanup(func() { SaveServer = oldSave }) + saves := 0 + SaveServer = func(*model.Server, string, string) error { + saves++ + return nil + } + + cmd := fm.runSave() + if cmd == nil { + t.Fatal("expected validation result command") + } + updated, _ := fm.Update(cmd()) + fm = updated.(*formModel) + if saves != 0 || fm.inputs[3].Value() != "abc" { + t.Fatalf("invalid input was lost or saved: saves=%d value=%q", saves, fm.inputs[3].Value()) + } + if fm.err == nil || !strings.Contains(fm.err.Error(), "Port") { + t.Fatalf("missing actionable port error: %v", fm.err) + } + if view := fm.View(); !strings.Contains(view, "Port must be a number") { + t.Fatalf("validation error is not rendered:\n%s", view) + } +} + +func TestDirtyServerFormRequiresDiscardConfirmation(t *testing.T) { + oldList := ListServers + t.Cleanup(func() { ListServers = oldList }) + ListServers = func() ([]*model.Server, error) { return nil, nil } + + m := New(nil) + m.screen = screenForm + m.form = newFormModel(80, 24) + m.form.inputs[0].SetValue("prod") + + updated, cmd := m.updateForm(tea.KeyMsg{Type: tea.KeyEsc}) + m = updated.(*tuiModel) + if cmd != nil || m.screen != screenConfirm || m.confirm == nil || m.confirm.parent != screenForm { + t.Fatalf("dirty form did not open discard confirmation: screen=%v confirm=%#v", m.screen, m.confirm) + } + updated, _ = m.updateConfirm(tea.KeyMsg{Type: tea.KeyEnter}) + m = updated.(*tuiModel) + if m.screen != screenForm || m.form == nil || m.form.inputs[0].Value() != "prod" { + t.Fatalf("Cancel did not preserve form: screen=%v form=%#v", m.screen, m.form) + } + + updated, _ = m.updateForm(tea.KeyMsg{Type: tea.KeyEsc}) + m = updated.(*tuiModel) + updated, _ = m.updateConfirm(tea.KeyMsg{Type: tea.KeyTab}) + m = updated.(*tuiModel) + updated, cmd = m.updateConfirm(tea.KeyMsg{Type: tea.KeyEnter}) + m = updated.(*tuiModel) + if cmd == nil { + t.Fatal("expected discard command") + } + updated, _ = m.Update(cmd()) + m = updated.(*tuiModel) + if m.screen != screenList || m.form != nil || m.confirm != nil { + t.Fatalf("discard did not return to list: screen=%v form=%v confirm=%v", m.screen, m.form, m.confirm) + } +} + +func TestCleanFormsExitWithoutConfirmation(t *testing.T) { + oldList := ListServers + t.Cleanup(func() { ListServers = oldList }) + ListServers = func() ([]*model.Server, error) { return nil, nil } + + tests := []struct { + name string + screen screen + setup func(*tuiModel) + exit func(*tuiModel) (tea.Model, tea.Cmd) + want screen + }{ + { + name: "server", + screen: screenForm, + setup: func(m *tuiModel) { m.form = newFormModel(80, 24) }, + exit: func(m *tuiModel) (tea.Model, tea.Cmd) { return m.updateForm(tea.KeyMsg{Type: tea.KeyEsc}) }, + want: screenList, + }, + { + name: "forward", + screen: screenForwardForm, + setup: func(m *tuiModel) { + m.forwardScreen = newForwardScreenModel(1, "prod", 80, 24) + m.forwardForm = newForwardFormModel(1, 80, 24) + }, + exit: func(m *tuiModel) (tea.Model, tea.Cmd) { + return m.updateForwardForm(tea.KeyMsg{Type: tea.KeyEsc}) + }, + want: screenForwardList, + }, + { + name: "template", + screen: screenTemplateForm, + setup: func(m *tuiModel) { m.templateForm = newTemplateFormModel(nil, 80, 24) }, + exit: func(m *tuiModel) (tea.Model, tea.Cmd) { + return m.updateTemplateForm(tea.KeyMsg{Type: tea.KeyEsc}) + }, + want: screenTemplates, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := New(nil) + m.screen = tt.screen + tt.setup(m) + updated, _ := tt.exit(m) + m = updated.(*tuiModel) + if m.screen != tt.want || m.confirm != nil { + t.Fatalf("clean exit: screen=%v confirm=%v", m.screen, m.confirm) + } + }) + } +} + +func TestDirtyForwardAndTemplateFormsRequireConfirmation(t *testing.T) { + t.Run("forward", func(t *testing.T) { + m := New(nil) + m.screen = screenForwardForm + m.forwardScreen = newForwardScreenModel(1, "prod", 80, 24) + m.forwardForm = newForwardFormModel(1, 80, 24) + m.forwardForm.nameInput.SetValue("postgres") + updated, _ := m.updateForwardForm(tea.KeyMsg{Type: tea.KeyEsc}) + m = updated.(*tuiModel) + if m.screen != screenConfirm || m.confirm == nil || m.confirm.parent != screenForwardForm { + t.Fatalf("dirty forward form did not confirm: screen=%v confirm=%#v", m.screen, m.confirm) + } + }) + + t.Run("template", func(t *testing.T) { + m := New(nil) + m.screen = screenTemplateForm + m.templateForm = newTemplateFormModel(nil, 80, 24) + m.templateForm.inputs[0].SetValue("uptime") + updated, _ := m.updateTemplateForm(tea.KeyMsg{Type: tea.KeyEsc}) + m = updated.(*tuiModel) + if m.screen != screenConfirm || m.confirm == nil || m.confirm.parent != screenTemplateForm { + t.Fatalf("dirty template form did not confirm: screen=%v confirm=%#v", m.screen, m.confirm) + } + }) +} + +func TestRequiredFieldsAreMarked(t *testing.T) { + serverView := newFormModel(100, 30).View() + for _, want := range []string{"Alias *", "Host *", "Port *"} { + if !strings.Contains(serverView, want) { + t.Fatalf("server form missing %q:\n%s", want, serverView) + } + } + + forwardView := newForwardFormModel(1, 100, 30).View() + for _, want := range []string{"Name *", "Listen Port *", "Target Host *", "Target Port *"} { + if !strings.Contains(forwardView, want) { + t.Fatalf("forward form missing %q:\n%s", want, forwardView) + } + } + + templateView := newTemplateFormModel(nil, 100, 30).View() + for _, want := range []string{"Name *", "Command *"} { + if !strings.Contains(templateView, want) { + t.Fatalf("template form missing %q:\n%s", want, templateView) + } + } +} diff --git a/internal/tui/forward.go b/internal/tui/forward.go index ed13cdc..548b2d4 100644 --- a/internal/tui/forward.go +++ b/internal/tui/forward.go @@ -161,6 +161,14 @@ type forwardFormModel struct { typeIdx int // 0=local, 1=remote, 2=socks width int height int + initial forwardFormSnapshot +} + +type forwardFormSnapshot struct { + name string + description string + values []string + forwardType model.ForwardType } var forwardTypes = []forwardTypeItem{ @@ -186,7 +194,7 @@ func newForwardFormModel(serverID int64, w, h int) *forwardFormModel { inputs[i].CharLimit = 128 } - return &forwardFormModel{ + fm := &forwardFormModel{ serverID: serverID, inputs: inputs, focusIdx: 0, @@ -197,6 +205,9 @@ func newForwardFormModel(serverID int64, w, h int) *forwardFormModel { width: w, height: h, } + fm.updateFocus() + fm.initial = fm.snapshot() + return fm } func newForwardEditModel(serverID int64, fwd *model.Forward, w, h int) *forwardFormModel { @@ -211,9 +222,37 @@ func newForwardEditModel(serverID int64, fwd *model.Forward, w, h int) *forwardF fm.inputs[1].SetValue(strconv.Itoa(fwd.LocalPort)) fm.inputs[2].SetValue(fwd.RemoteAddr) fm.inputs[3].SetValue(strconv.Itoa(fwd.RemotePort)) + fm.updateFocus() + fm.initial = fm.snapshot() return fm } +func (fm *forwardFormModel) snapshot() forwardFormSnapshot { + values := make([]string, len(fm.inputs)) + for i := range fm.inputs { + values[i] = fm.inputs[i].Value() + } + return forwardFormSnapshot{ + name: fm.nameInput.Value(), + description: fm.descInput.Value(), + values: values, + forwardType: fm.currentType, + } +} + +func (fm *forwardFormModel) Dirty() bool { + current := fm.snapshot() + if current.name != fm.initial.name || current.description != fm.initial.description || current.forwardType != fm.initial.forwardType || len(current.values) != len(fm.initial.values) { + return true + } + for i := range current.values { + if current.values[i] != fm.initial.values[i] { + return true + } + } + return false +} + func typeIndex(t model.ForwardType) int { switch t { case model.ForwardLocal: @@ -258,7 +297,7 @@ func (fm *forwardFormModel) labelForField(idx int) string { if idx < 0 || idx >= len(labels) { return "" } - return labels[idx] + return labels[idx] + " *" } func (fm *forwardFormModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { @@ -369,7 +408,7 @@ func (fm *forwardFormModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { func (fm *forwardFormModel) updateFocus() { fm.nameInput.Blur() - fm.nameInput.Prompt = blurredStyle.Render("Name: ") + fm.nameInput.Prompt = blurredStyle.Render("Name *: ") fm.descInput.Blur() fm.descInput.Prompt = blurredStyle.Render("Description: ") for i := range fm.inputs { @@ -381,7 +420,7 @@ func (fm *forwardFormModel) updateFocus() { switch { case fm.focusIdx == 0: fm.nameInput.Focus() - fm.nameInput.Prompt = focusedStyle.Render("Name> ") + fm.nameInput.Prompt = focusedStyle.Render("Name *> ") case fm.focusIdx == 1: fm.descInput.Focus() fm.descInput.Prompt = focusedStyle.Render("Description> ") @@ -400,10 +439,11 @@ func (fm *forwardFormModel) runSave() tea.Cmd { name := strings.TrimSpace(fm.nameInput.Value()) desc := strings.TrimSpace(fm.descInput.Value()) - localPort := 0 - fmt.Sscanf(fm.inputs[1].Value(), "%d", &localPort) + localPort, err := parseNamedPort("Listen port", fm.inputs[1].Value()) + if err != nil { + return saveDoneMsg{err: err} + } remotePort := 0 - fmt.Sscanf(fm.inputs[3].Value(), "%d", &remotePort) localAddr := strings.TrimSpace(fm.inputs[0].Value()) remoteAddr := strings.TrimSpace(fm.inputs[2].Value()) @@ -411,10 +451,6 @@ func (fm *forwardFormModel) runSave() tea.Cmd { if name == "" { return saveDoneMsg{err: fmt.Errorf("name is required")} } - if localPort < 1 || localPort > 65535 { - return saveDoneMsg{err: fmt.Errorf("invalid listen port %d: must be 1-65535", localPort)} - } - switch fm.currentType { case model.ForwardLocal: if localAddr == "" { @@ -423,15 +459,17 @@ func (fm *forwardFormModel) runSave() tea.Cmd { if remoteAddr == "" { return saveDoneMsg{err: fmt.Errorf("target host is required for local forward")} } - if remotePort < 1 || remotePort > 65535 { - return saveDoneMsg{err: fmt.Errorf("invalid target port %d: must be 1-65535", remotePort)} + remotePort, err = parseNamedPort("Target port", fm.inputs[3].Value()) + if err != nil { + return saveDoneMsg{err: err} } case model.ForwardRemote: if remoteAddr == "" { return saveDoneMsg{err: fmt.Errorf("remote listen address is required")} } - if remotePort < 1 || remotePort > 65535 { - return saveDoneMsg{err: fmt.Errorf("invalid remote port %d: must be 1-65535", remotePort)} + remotePort, err = parseNamedPort("Remote port", fm.inputs[3].Value()) + if err != nil { + return saveDoneMsg{err: err} } if localAddr == "" { localAddr = "127.0.0.1" @@ -467,11 +505,22 @@ func (fm *forwardFormModel) runSave() tea.Cmd { if SaveForward == nil { return saveDoneMsg{err: fmt.Errorf("forward storage is unavailable")} } - err := SaveForward(fwd) + err = SaveForward(fwd) return saveDoneMsg{err: err} } } +func parseNamedPort(label, value string) (int, error) { + port, err := strconv.Atoi(strings.TrimSpace(value)) + if err != nil { + return 0, fmt.Errorf("%s must be a number from 1 to 65535", label) + } + if port < 1 || port > 65535 { + return 0, fmt.Errorf("%s must be between 1 and 65535", label) + } + return port, nil +} + func (fm *forwardFormModel) View() string { var b strings.Builder title := "Add Port Forward" diff --git a/internal/tui/template_form.go b/internal/tui/template_form.go index 2e61763..a96d666 100644 --- a/internal/tui/template_form.go +++ b/internal/tui/template_form.go @@ -21,6 +21,7 @@ type templateFormModel struct { saved bool width int height int + initial []string } func newTemplateFormModel(t *model.CommandTemplate, w, h int) *templateFormModel { @@ -44,9 +45,31 @@ func newTemplateFormModel(t *model.CommandTemplate, w, h int) *templateFormModel inputs[2].SetValue(t.Description) } tf.updateFocus() + tf.initial = tf.snapshot() return tf } +func (tf *templateFormModel) snapshot() []string { + values := make([]string, len(tf.inputs)) + for i := range tf.inputs { + values[i] = tf.inputs[i].Value() + } + return values +} + +func (tf *templateFormModel) Dirty() bool { + current := tf.snapshot() + if len(current) != len(tf.initial) { + return true + } + for i := range current { + if current[i] != tf.initial[i] { + return true + } + } + return false +} + func (tf *templateFormModel) Init() tea.Cmd { return nil } @@ -89,14 +112,21 @@ func (tf *templateFormModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { func (tf *templateFormModel) updateFocus() { for i := range tf.inputs { tf.inputs[i].Blur() - tf.inputs[i].Prompt = blurredStyle.Render(tf.labels[i] + ": ") + tf.inputs[i].Prompt = blurredStyle.Render(tf.labelAt(i) + ": ") } if tf.focusIdx < len(tf.inputs) { tf.inputs[tf.focusIdx].Focus() - tf.inputs[tf.focusIdx].Prompt = focusedStyle.Render(tf.labels[tf.focusIdx] + "> ") + tf.inputs[tf.focusIdx].Prompt = focusedStyle.Render(tf.labelAt(tf.focusIdx) + "> ") } } +func (tf *templateFormModel) labelAt(index int) string { + if index == 0 || index == 1 { + return tf.labels[index] + " *" + } + return tf.labels[index] +} + func (tf *templateFormModel) save() tea.Cmd { return func() tea.Msg { if SaveCommandTemplate == nil {