From 617008e8f125c0cf42825c7ef1b05f900e845fe0 Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Wed, 22 Apr 2026 17:00:34 -0300 Subject: [PATCH] =?UTF-8?q?config:=20normalize=20ssh=5Fkey=5Fpath=20?= =?UTF-8?q?=E2=80=94=20expand=20~/,=20reject=20non-absolute?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: resolveSSHKeyPath returned a configured ssh_key_path verbatim. That meant: - ssh_key_path = "~/.ssh/id_ed25519" kept the literal "~" — downstream readers (internal/guest/ssh.go, internal/daemon/image_seed.go, internal/daemon/vm_authsync.go, internal/cli/ssh.go) do raw os.ReadFile on the path and fail at runtime with a path that looks fine but isn't. - ssh_key_path = "id_ed25519" (relative) silently worked or didn't depending on the daemon's cwd — the daemon process's cwd is not the user's shell cwd, so behavior was non-obvious. Fix: add normalizeSSHKeyPath() run over configured values. It: - expands "~/..." against $HOME - rejects bare "~" (ambiguous) - rejects "~user/..." (we don't do user-tilde) - rejects relative paths outright - returns filepath.Clean'd absolute paths Tests cover the accepting case (home-anchored expansion) and every rejection branch via a table-driven subtests. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/config/config.go | 40 ++++++++++++++++++++++++++- internal/config/config_test.go | 49 ++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 0f1ae63..24cac8c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -258,7 +258,7 @@ func validateFileSyncMode(mode string) error { func resolveSSHKeyPath(layout paths.Layout, configured string) (string, error) { configured = strings.TrimSpace(configured) if configured != "" { - return configured, nil + return normalizeSSHKeyPath(configured) } // Key lives under the state dir, not the config dir. The daemon's // ensureVMSSHClientConfig scrubs ConfigDir/ssh on every Open as @@ -275,6 +275,44 @@ func resolveSSHKeyPath(layout paths.Layout, configured string) (string, error) { return ensureDefaultSSHKey(filepath.Join(sshDir, "id_ed25519")) } +// normalizeSSHKeyPath validates and canonicalises a user-configured +// ssh_key_path. Accepts: +// +// - absolute paths ("/home/me/keys/id_ed25519") +// - home-anchored paths ("~/keys/id_ed25519") — expanded against $HOME +// +// Rejects: +// +// - bare "~" (ambiguous — expand to what?) +// - "~other/foo" (we only expand the current user's home) +// - relative paths ("id_ed25519", "./keys/id_ed25519") — these are +// ambiguous because the daemon's cwd isn't the user's shell cwd, +// and readers in internal/guest + internal/cli do raw os.ReadFile +// on the path without re-resolving against a known anchor +func normalizeSSHKeyPath(raw string) (string, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", nil + } + if raw == "~" { + return "", fmt.Errorf("ssh_key_path %q: bare '~' is not supported, point at a specific key file", raw) + } + if strings.HasPrefix(raw, "~") && !strings.HasPrefix(raw, "~/") { + return "", fmt.Errorf("ssh_key_path %q: only '~/' is expanded, not '~user/'", raw) + } + if strings.HasPrefix(raw, "~/") { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("ssh_key_path %q: expand ~/: %w", raw, err) + } + raw = filepath.Join(home, strings.TrimPrefix(raw, "~/")) + } + if !filepath.IsAbs(raw) { + return "", fmt.Errorf("ssh_key_path %q: must be absolute (start with '/') or home-anchored (start with '~/')", raw) + } + return filepath.Clean(raw), nil +} + func ensureDefaultSSHKey(path string) (string, error) { if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { return "", err diff --git a/internal/config/config_test.go b/internal/config/config_test.go index da85358..66a0379 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -50,6 +50,55 @@ func TestLoadDefaultsResolveFirecrackerAndGenerateSSHKey(t *testing.T) { } } +func TestLoadSSHKeyPathExpandsHomeAnchored(t *testing.T) { + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + + configDir := t.TempDir() + data := []byte("ssh_key_path = \"~/mykeys/id_ed25519\"\n") + if err := os.WriteFile(filepath.Join(configDir, "config.toml"), data, 0o644); err != nil { + t.Fatalf("write config.toml: %v", err) + } + + cfg, err := Load(paths.Layout{ConfigDir: configDir, SSHDir: t.TempDir()}) + if err != nil { + t.Fatalf("Load: %v", err) + } + want := filepath.Join(homeDir, "mykeys", "id_ed25519") + if cfg.SSHKeyPath != want { + t.Fatalf("SSHKeyPath = %q, want %q", cfg.SSHKeyPath, want) + } +} + +func TestLoadRejectsInvalidSSHKeyPath(t *testing.T) { + cases := []struct { + name string + raw string + want string + }{ + {"relative bare", "id_ed25519", "must be absolute"}, + {"relative with dot", "./keys/id_ed25519", "must be absolute"}, + {"bare tilde", "~", "bare '~' is not supported"}, + {"user-tilde", "~other/id_ed25519", "only '~/' is expanded"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + configDir := t.TempDir() + data := []byte("ssh_key_path = \"" + tc.raw + "\"\n") + if err := os.WriteFile(filepath.Join(configDir, "config.toml"), data, 0o644); err != nil { + t.Fatalf("write config.toml: %v", err) + } + _, err := Load(paths.Layout{ConfigDir: configDir, SSHDir: t.TempDir()}) + if err == nil { + t.Fatalf("Load %q: want error containing %q", tc.raw, tc.want) + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("Load %q: error = %v, want contains %q", tc.raw, err, tc.want) + } + }) + } +} + func TestLoadAppliesConfigOverrides(t *testing.T) { configDir := t.TempDir() data := []byte(`