From ebe651762f83c4f01ef1a718d63e4f60113ffbf6 Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Wed, 22 Apr 2026 14:29:34 -0300 Subject: [PATCH] config: put the default SSH key under the state dir, not ConfigDir/ssh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: every daemon Open deleted the freshly-generated default SSH key before returning, so the next VM create failed reading it. Sequence: 1. Open → config.Load → resolveSSHKeyPath generates ~/.config/banger/ssh/id_ed25519 2. Open → ensureVMSSHClientConfig → syncVMSSHClientConfig scrubs ~/.config/banger/ssh entirely as a migration step for the pre-opt-in layout (commit 108f7a0) The scrub was added for a file that used to live at ConfigDir/ssh/ssh_config, but it os.RemoveAll'd the whole ConfigDir/ssh dir — including the id_ed25519 the key generator had just put there. Fix: point the default key at layout.SSHDir (a StateDir-rooted path that paths.Ensure already creates). The scrub can keep cleaning up ConfigDir/ssh because nothing banger writes under it anymore. Users whose ssh_key_path is explicitly set in config.toml are unaffected — configured wins. Users on the default path will get a fresh key at StateDir/ssh/id_ed25519 on their next daemon Open; existing VMs' authorized_keys re-sync on next start/create through ensureAuthorizedKeyOnWorkDisk, so no manual intervention is needed beyond restarting the daemon. Regression test pins the new placement and asserts the legacy path stays empty. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/config/config.go | 11 ++++++++++- internal/config/config_test.go | 13 +++++++++++-- internal/config/ssh/id_ed25519 | 3 +++ internal/config/ssh/id_ed25519.pub | 1 + 4 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 internal/config/ssh/id_ed25519 create mode 100644 internal/config/ssh/id_ed25519.pub diff --git a/internal/config/config.go b/internal/config/config.go index d2916a5..8713c89 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -260,7 +260,16 @@ func resolveSSHKeyPath(layout paths.Layout, configured string) (string, error) { if configured != "" { return configured, nil } - return ensureDefaultSSHKey(filepath.Join(layout.ConfigDir, "ssh", "id_ed25519")) + // Key lives under the state dir, not the config dir. The daemon's + // ensureVMSSHClientConfig scrubs ConfigDir/ssh on every Open as + // part of migrating off the pre-state-dir layout — putting the + // default key there would race with that cleanup (create → delete + // → next VM create fails to read the key). + sshDir := strings.TrimSpace(layout.SSHDir) + if sshDir == "" { + sshDir = filepath.Join(layout.StateDir, "ssh") + } + return ensureDefaultSSHKey(filepath.Join(sshDir, "id_ed25519")) } func ensureDefaultSSHKey(path string) (string, error) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d4ffb6d..9a756b3 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -12,6 +12,7 @@ import ( func TestLoadDefaultsResolveFirecrackerAndGenerateSSHKey(t *testing.T) { configDir := t.TempDir() + sshDir := t.TempDir() binDir := t.TempDir() firecrackerPath := filepath.Join(binDir, "firecracker") if err := os.WriteFile(firecrackerPath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { @@ -19,7 +20,7 @@ func TestLoadDefaultsResolveFirecrackerAndGenerateSSHKey(t *testing.T) { } t.Setenv("PATH", binDir) - cfg, err := Load(paths.Layout{ConfigDir: configDir}) + cfg, err := Load(paths.Layout{ConfigDir: configDir, SSHDir: sshDir}) if err != nil { t.Fatalf("Load: %v", err) } @@ -27,7 +28,11 @@ func TestLoadDefaultsResolveFirecrackerAndGenerateSSHKey(t *testing.T) { if cfg.FirecrackerBin != firecrackerPath { t.Fatalf("FirecrackerBin = %q, want %q", cfg.FirecrackerBin, firecrackerPath) } - wantKey := filepath.Join(configDir, "ssh", "id_ed25519") + // Default key lives under SSHDir (state dir), NOT ConfigDir/ssh. + // ConfigDir/ssh gets scrubbed by ensureVMSSHClientConfig on every + // daemon Open, so regression-guard that the generator never picks + // that path again. + wantKey := filepath.Join(sshDir, "id_ed25519") if cfg.SSHKeyPath != wantKey { t.Fatalf("SSHKeyPath = %q, want %q", cfg.SSHKeyPath, wantKey) } @@ -36,6 +41,10 @@ func TestLoadDefaultsResolveFirecrackerAndGenerateSSHKey(t *testing.T) { t.Fatalf("stat %s: %v", path, err) } } + legacyKey := filepath.Join(configDir, "ssh", "id_ed25519") + if _, err := os.Stat(legacyKey); err == nil { + t.Fatalf("key was also generated at legacy path %s; config.Load must not write under ConfigDir/ssh anymore", legacyKey) + } if cfg.DefaultImageName != "debian-bookworm" { t.Fatalf("DefaultImageName = %q, want debian-bookworm", cfg.DefaultImageName) } diff --git a/internal/config/ssh/id_ed25519 b/internal/config/ssh/id_ed25519 new file mode 100644 index 0000000..6e6936e --- /dev/null +++ b/internal/config/ssh/id_ed25519 @@ -0,0 +1,3 @@ +-----BEGIN PRIVATE KEY----- +MC4CAQAwBQYDK2VwBCIEIOeClGP/5JANJJpar5grOSE0RcaqMedAT5Nc6BcyCphM +-----END PRIVATE KEY----- diff --git a/internal/config/ssh/id_ed25519.pub b/internal/config/ssh/id_ed25519.pub new file mode 100644 index 0000000..bdfd01b --- /dev/null +++ b/internal/config/ssh/id_ed25519.pub @@ -0,0 +1 @@ +ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOpF+WjNdlBLZYI3sbPST2lhxzrsfELwRXT58vkNL3xK