From d750b978a2d348a1470eff8e9ce5a5bcfa006c05 Mon Sep 17 00:00:00 2001 From: Sem Van Broekhoven <144097969+dotsem@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:55:05 +0200 Subject: [PATCH] fix: service config not saving after creating or editing --- internal/config/config_file.go | 9 ++--- internal/config/editor.go | 5 +-- internal/config/manager.go | 16 ++------- internal/ssh/keygen.go | 5 ++- internal/ssh/presets.go | 2 +- internal/ssh/presets_test.go | 9 +++++ internal/tui/service_form.go | 51 ++++++++++++++-------------- internal/tui/service_form_test.go | 35 ++++++++++++++++++++ internal/tui/service_overlay.go | 14 ++++++-- internal/utils/path.go | 18 ++++++++++ internal/utils/path_test.go | 55 +++++++++++++++++++++++++++++++ 11 files changed, 170 insertions(+), 49 deletions(-) create mode 100644 internal/utils/path.go create mode 100644 internal/utils/path_test.go diff --git a/internal/config/config_file.go b/internal/config/config_file.go index 0e1adf9..a958f16 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -6,6 +6,7 @@ import ( "path/filepath" "reflect" "strings" + "tusshi/internal/utils" "github.com/kevinburke/ssh_config" ) @@ -14,7 +15,7 @@ import ( // registers it with the primary SSH config via an Include directive, // and maps it internally for display and interaction. func (m *Manager) AddConfigFile(targetPath string) error { - absTarget := expandTilde(targetPath) + absTarget := utils.ExpandTilde(targetPath) if abs, err := filepath.Abs(absTarget); err == nil { absTarget = abs } @@ -61,12 +62,12 @@ func (m *Manager) AddConfigFile(targetPath string) error { // updates all internal tracking indices, redirects child hosts, and updates // the corresponding Include directive inside the primary config. func (m *Manager) RenameConfigFile(oldPath, newPath string) error { - absOld := expandTilde(oldPath) + absOld := utils.ExpandTilde(oldPath) if abs, err := filepath.Abs(absOld); err == nil { absOld = abs } - absNew := expandTilde(newPath) + absNew := utils.ExpandTilde(newPath) if abs, err := filepath.Abs(absNew); err == nil { absNew = abs } @@ -114,7 +115,7 @@ func (m *Manager) RenameConfigFile(oldPath, newPath string) error { // its associated Include directive inside the primary configuration. // It fails if any host connections are still defined inside the file. func (m *Manager) DeleteConfigFile(targetPath string) error { - absTarget := expandTilde(targetPath) + absTarget := utils.ExpandTilde(targetPath) if abs, err := filepath.Abs(absTarget); err == nil { absTarget = abs } diff --git a/internal/config/editor.go b/internal/config/editor.go index 8360326..177791f 100644 --- a/internal/config/editor.go +++ b/internal/config/editor.go @@ -8,6 +8,7 @@ import ( "path/filepath" "reflect" "strings" + "tusshi/internal/utils" "github.com/kevinburke/ssh_config" ) @@ -15,7 +16,7 @@ import ( // AddHost appends a new Host connection block to a specific target configuration file // and serializes the modified AST back to disk. func (m *Manager) AddHost(targetFile string, h *Host) error { - absTarget := expandTilde(targetFile) + absTarget := utils.ExpandTilde(targetFile) if abs, err := filepath.Abs(absTarget); err == nil { absTarget = abs } @@ -193,7 +194,7 @@ func (m *Manager) DeleteHost(alias string) error { // MoveHost transfers a host block from one configuration file to another. func (m *Manager) MoveHost(alias string, targetFile string) error { - absTarget := expandTilde(targetFile) + absTarget := utils.ExpandTilde(targetFile) if abs, err := filepath.Abs(absTarget); err == nil { absTarget = abs } diff --git a/internal/config/manager.go b/internal/config/manager.go index 56071b4..0aa60a6 100644 --- a/internal/config/manager.go +++ b/internal/config/manager.go @@ -8,6 +8,7 @@ import ( "reflect" "slices" "strings" + "tusshi/internal/utils" "github.com/kevinburke/ssh_config" ) @@ -28,7 +29,7 @@ type Manager struct { // NewManager creates and initializes a Manager with the specified primary config file. // It performs basic path expansion on the provided primary path. func NewManager(primaryPath string) *Manager { - absPath := expandTilde(primaryPath) + absPath := utils.ExpandTilde(primaryPath) if abs, err := filepath.Abs(absPath); err == nil { absPath = abs } @@ -184,7 +185,7 @@ func (m *Manager) loadPath(path string, depth int) error { // resolveAndLoadIncludes matches globs and recursively loads matched files. func (m *Manager) resolveAndLoadIncludes(pattern string, depth int) { - expanded := expandTilde(pattern) + expanded := utils.ExpandTilde(pattern) if !filepath.IsAbs(expanded) { expanded = filepath.Join(filepath.Dir(m.PrimaryPath), expanded) } @@ -220,17 +221,6 @@ func (m *Manager) buildGlobalConfig() *ssh_config.Config { return &ssh_config.Config{Hosts: mergedHosts} } -// expandTilde replaces ~/ prefix with the user home directory path. -func expandTilde(path string) string { - if strings.HasPrefix(path, "~/") { - home, err := os.UserHomeDir() - if err == nil { - return filepath.Join(home, path[2:]) - } - } - return path -} - // FindConfigFile searches FileOrder for a path matching the given name, // base name, or extensionless nickname. func (m *Manager) FindConfigFile(name string) (string, bool) { diff --git a/internal/ssh/keygen.go b/internal/ssh/keygen.go index 98208b3..0a45eb2 100644 --- a/internal/ssh/keygen.go +++ b/internal/ssh/keygen.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "strings" + "tusshi/internal/utils" ) // Key type identifiers supported by ssh-keygen. @@ -25,6 +26,8 @@ func GenerateKey(path, keyType, comment string) error { keyType = KeyTypeED25519 } + path = utils.ExpandTilde(path) + args := []string{"-t", keyType, "-f", path, "-N", "", "-C", comment} if keyType == KeyTypeRSA { args = append(args, "-b", "4096") @@ -41,7 +44,7 @@ func GenerateKey(path, keyType, comment string) error { // ReadPublicKey reads the public key file corresponding to the given private key path. func ReadPublicKey(privateKeyPath string) (string, error) { - pubPath := privateKeyPath + ".pub" + pubPath := utils.ExpandTilde(privateKeyPath) + ".pub" data, err := os.ReadFile(pubPath) // #nosec G304 — user-provided path from TUI if err != nil { return "", fmt.Errorf("reading public key %q: %w", pubPath, err) diff --git a/internal/ssh/presets.go b/internal/ssh/presets.go index 030b072..693df7b 100644 --- a/internal/ssh/presets.go +++ b/internal/ssh/presets.go @@ -19,7 +19,7 @@ var Presets = []ServicePreset{ {Name: "GitHub", KeyName: "github", HostName: "github.com", User: defaultUserGit}, {Name: "GitLab", KeyName: "gitlab", HostName: "gitlab.com", User: defaultUserGit}, // TODO: add more - {Name: "Custom", KeyName: "service", HostName: "service", User: defaultUserGit}, + {Name: "Custom", KeyName: "service", HostName: PresetCustom, User: defaultUserGit}, } // FindPreset returns the preset matching the given name, hostname, or keyname. diff --git a/internal/ssh/presets_test.go b/internal/ssh/presets_test.go index 84e1579..dec96ea 100644 --- a/internal/ssh/presets_test.go +++ b/internal/ssh/presets_test.go @@ -27,6 +27,15 @@ func TestFindPreset(t *testing.T) { assert.Equal(t, "git", preset.User) }) + t.Run("returns built-in custom preset", func(t *testing.T) { + preset, ok := ssh.FindPreset(ssh.PresetCustom) + assert.True(t, ok) + assert.Equal(t, "Custom", preset.Name) + assert.Equal(t, "service", preset.KeyName) + assert.Equal(t, ssh.PresetCustom, preset.HostName) + assert.Equal(t, "git", preset.User) + }) + t.Run("returns false for unknown preset", func(t *testing.T) { _, ok := ssh.FindPreset("unknown-service") assert.False(t, ok) diff --git a/internal/tui/service_form.go b/internal/tui/service_form.go index cb8a72d..baae160 100644 --- a/internal/tui/service_form.go +++ b/internal/tui/service_form.go @@ -7,6 +7,8 @@ import ( "strings" tussh "tusshi/internal/ssh" + "tusshi/internal/utils" + "tusshi/internal/validation" "github.com/charmbracelet/huh" ) @@ -68,7 +70,7 @@ func BuildServiceForm(s *ServiceFormState) *huh.Form { Placeholder("~/.ssh/id_ed25519"). Value(&s.KeyPath). Validate(func(v string) error { - expanded := expandTildePath(v) + expanded := utils.ExpandTilde(v) if _, err := os.Stat(expanded); err != nil { return fmt.Errorf("file not found: %s", expanded) } @@ -78,7 +80,8 @@ func BuildServiceForm(s *ServiceFormState) *huh.Form { inputHostAlias := huh.NewInput(). Title("Host Alias"). Description("Name used in SSH config (e.g. github or github-work)"). - Value(&s.HostAlias) + Value(&s.HostAlias). + Validate(validation.ValidateAlias) inputHostName := huh.NewInput(). Title("HostName"). @@ -95,16 +98,16 @@ func BuildServiceForm(s *ServiceFormState) *huh.Form { return } if s.PresetAlias != s.lastPreset || s.KeyType != s.lastKeyType { - if preset, ok := tussh.FindPreset(s.PresetAlias); ok { - s.HostAlias = preset.HostName - s.HostName = preset.HostName - s.HostUser = preset.User - } else if s.PresetAlias == tussh.PresetCustom { + if s.PresetAlias == tussh.PresetCustom { if s.lastPreset != "" { s.HostAlias = "" s.HostName = "" s.HostUser = "git" } + } else if preset, ok := tussh.FindPreset(s.PresetAlias); ok { + s.HostAlias = preset.HostName + s.HostName = preset.HostName + s.HostUser = preset.User } s.KeyPath = s.ProvideDefaultKeyPath() @@ -173,23 +176,31 @@ func BuildServiceForm(s *ServiceFormState) *huh.Form { // ApplyPreset fills HostAlias, HostName, and HostUser from the selected preset if still empty on submit. func (s *ServiceFormState) ApplyPreset() { - if preset, ok := tussh.FindPreset(s.PresetAlias); ok { - s.HostAlias = preset.HostName - s.HostName = preset.HostName - s.HostUser = preset.User - } else if s.PresetAlias == tussh.PresetCustom { + if s.PresetAlias == tussh.PresetCustom { if s.HostUser == "" { s.HostUser = "git" } + } else if preset, ok := tussh.FindPreset(s.PresetAlias); ok { + if s.HostAlias == "" { + s.HostAlias = preset.HostName + } + if s.HostName == "" { + s.HostName = preset.HostName + } + if s.HostUser == "" { + s.HostUser = preset.User + } } - s.KeyPath = s.ProvideDefaultKeyPath() + if s.KeyPath == "" { + s.KeyPath = s.ProvideDefaultKeyPath() + } } // ProvideDefaultKeyPath generates a non-colliding default SSH key path. func (s *ServiceFormState) ProvideDefaultKeyPath() string { keyBaseName := "" - if preset, ok := tussh.FindPreset(s.PresetAlias); ok && preset.KeyName != "" { + if preset, ok := tussh.FindPreset(s.PresetAlias); ok && s.PresetAlias != tussh.PresetCustom && preset.KeyName != "" { keyBaseName = preset.KeyName } else { keyBaseName = s.HostAlias @@ -230,15 +241,5 @@ func (s *ServiceFormState) ProvideDefaultKeyPath() string { // ResolvedKeyPath returns the expanded absolute path for the configured key. func (s *ServiceFormState) ResolvedKeyPath() string { - return expandTildePath(s.KeyPath) -} - -func expandTildePath(path string) string { - if strings.HasPrefix(path, "~/") { - home, err := os.UserHomeDir() - if err == nil { - return filepath.Join(home, path[2:]) - } - } - return path + return utils.ExpandTilde(s.KeyPath) } diff --git a/internal/tui/service_form_test.go b/internal/tui/service_form_test.go index 72bb8a3..48351f7 100644 --- a/internal/tui/service_form_test.go +++ b/internal/tui/service_form_test.go @@ -42,6 +42,41 @@ func TestServiceFormState(t *testing.T) { assert.Contains(t, state.KeyPath, "id_ed25519_gitlab") }) + t.Run("ApplyPreset does not overwrite user-entered fields when populated", func(t *testing.T) { + state := &tui.ServiceFormState{ + Action: "add", + PresetAlias: "github.com", + KeySource: "existing", + HostAlias: "github-work", + HostName: "github.mycorp.internal", + HostUser: "custom-git", + KeyPath: "~/.ssh/custom_key", + } + + state.ApplyPreset() + + assert.Equal(t, "github-work", state.HostAlias) + assert.Equal(t, "github.mycorp.internal", state.HostName) + assert.Equal(t, "custom-git", state.HostUser) + assert.Equal(t, "~/.ssh/custom_key", state.KeyPath) + }) + + t.Run("ApplyPreset handles custom preset without overriding alias and name", func(t *testing.T) { + state := &tui.ServiceFormState{ + Action: "add", + PresetAlias: tussh.PresetCustom, + HostAlias: "bitbucket.org", + HostName: "bitbucket.org", + } + + state.ApplyPreset() + + assert.Equal(t, "bitbucket.org", state.HostAlias) + assert.Equal(t, "bitbucket.org", state.HostName) + assert.Equal(t, "git", state.HostUser) + assert.Contains(t, state.KeyPath, "id_ed25519_bitbucket.org") + }) + t.Run("ProvideDefaultKeyPath generates non-colliding key path", func(t *testing.T) { state := &tui.ServiceFormState{ HostAlias: "github-work", diff --git a/internal/tui/service_overlay.go b/internal/tui/service_overlay.go index 88a99d8..9df9d3a 100644 --- a/internal/tui/service_overlay.go +++ b/internal/tui/service_overlay.go @@ -8,6 +8,7 @@ import ( "tusshi/internal/ssh" "tusshi/internal/tui/components" "tusshi/internal/tui/theme" + "tusshi/internal/utils" "github.com/atotto/clipboard" tea "github.com/charmbracelet/bubbletea" @@ -19,7 +20,7 @@ func (c *cmdContext) OpenServiceForm(action string, targetHost *config.Host) { Action: action, KeySource: keySourceGenerate, KeyType: ssh.KeyTypeED25519, - PresetAlias: "github", + PresetAlias: "github.com", } if action == actionEdit && targetHost != nil { @@ -31,6 +32,8 @@ func (c *cmdContext) OpenServiceForm(action string, targetHost *config.Host) { state.KeySource = keySourceExisting if preset, ok := ssh.FindPreset(targetHost.Alias); ok { state.PresetAlias = preset.HostName + } else if preset, ok := ssh.FindPreset(targetHost.Name); ok { + state.PresetAlias = preset.HostName } else { state.PresetAlias = ssh.PresetCustom } @@ -75,7 +78,7 @@ func (c *cmdContext) DeleteService(alias string) { return } - keyPath := expandTildePath(found.IdentityFile) + keyPath := utils.ExpandTilde(found.IdentityFile) var hasKeyFile bool if keyPath != "" { if _, err := os.Stat(keyPath); err == nil { @@ -161,11 +164,16 @@ func (m *Model) executeServiceFormSubmit(s *ServiceFormState) { } } + keyPath := s.KeyPath + if keyPath == "" { + keyPath = resolved + } + h := &config.Host{ Alias: s.HostAlias, Name: s.HostName, User: s.HostUser, - IdentityFile: resolved, + IdentityFile: keyPath, IsService: true, Properties: make(map[string]string), } diff --git a/internal/utils/path.go b/internal/utils/path.go new file mode 100644 index 0000000..309b747 --- /dev/null +++ b/internal/utils/path.go @@ -0,0 +1,18 @@ +package utils + +import ( + "os" + "path/filepath" + "strings" +) + +// ExpandTilde replaces ~/ prefix with the user home directory path. +func ExpandTilde(path string) string { + if strings.HasPrefix(path, "~/") { + home, err := os.UserHomeDir() + if err == nil { + return filepath.Join(home, path[2:]) + } + } + return path +} diff --git a/internal/utils/path_test.go b/internal/utils/path_test.go new file mode 100644 index 0000000..adfa89d --- /dev/null +++ b/internal/utils/path_test.go @@ -0,0 +1,55 @@ +package utils_test + +import ( + "os" + "path/filepath" + "testing" + "tusshi/internal/utils" + + "github.com/stretchr/testify/assert" +) + +func TestExpandTilde(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Fatalf("os.UserHomeDir() failed: %v", err) + } + + tests := []struct { + name string + input string + expected string + }{ + { + name: "absolute path", + input: "/var/log/baboon", + expected: "/var/log/baboon", + }, + { + name: "empty path", + input: "", + expected: "", + }, + { + name: "home directory prefix", + input: "~/Documents", + expected: filepath.Join(home, "Documents"), + }, + { + name: "ssh config path", + input: "~/.ssh/config", + expected: filepath.Join(home, ".ssh/config"), + }, + { + name: "tilde without slash", + input: "~", + expected: "~", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, utils.ExpandTilde(tt.input)) + }) + } +}