fix: validate tui forms and protect edits

This commit is contained in:
mirivlad 2026-08-14 03:27:41 +08:00
parent 37c4fd34a9
commit 16f4f9937f
5 changed files with 404 additions and 24 deletions

View File

@ -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

View File

@ -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 ]"

View File

@ -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)
}
}
}

View File

@ -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"

View File

@ -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 {