From 60f90eb8beb4f873f089b0875c02f684c6b10296 Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Wed, 22 Apr 2026 14:31:11 -0300 Subject: [PATCH] config: harden resolveSSHKeyPath against relative paths + drop stray test keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior commit's test (TestLoadDefaultsResolveFirecrackerAndGenerateSSHKey) was contained, but every OTHER config test called Load(paths.Layout{ConfigDir: ...}) without SSHDir or StateDir set. Under the new code path that meant resolveSSHKeyPath produced a relative target ("ssh/id_ed25519") which go test happily wrote against the package's own source directory — and I caught that in the commit after the fact, in the form of internal/config/ssh/ showing up as tracked files. Two changes: - resolveSSHKeyPath now errors if the resolved path is not absolute. paths.Resolve always produces an absolute SSHDir in production; this just stops a fumbled layout from silently scribbling into cwd. - Every existing config test that was relying on the old ConfigDir/ssh path gets an explicit SSHDir: t.TempDir() added, restoring the key-generation surface under tempdir isolation. Delete internal/config/ssh/ — those files were test-generated by the prior commit and have no business in the repo. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/config/config.go | 3 +++ internal/config/config_test.go | 14 +++++++------- internal/config/ssh/id_ed25519 | 3 --- internal/config/ssh/id_ed25519.pub | 1 - 4 files changed, 10 insertions(+), 11 deletions(-) delete mode 100644 internal/config/ssh/id_ed25519 delete mode 100644 internal/config/ssh/id_ed25519.pub diff --git a/internal/config/config.go b/internal/config/config.go index 8713c89..0f1ae63 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -269,6 +269,9 @@ func resolveSSHKeyPath(layout paths.Layout, configured string) (string, error) { if sshDir == "" { sshDir = filepath.Join(layout.StateDir, "ssh") } + if !filepath.IsAbs(sshDir) { + return "", fmt.Errorf("ssh key dir must be absolute; got %q (check paths.Resolve populated SSHDir / StateDir)", sshDir) + } return ensureDefaultSSHKey(filepath.Join(sshDir, "id_ed25519")) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9a756b3..da85358 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -69,7 +69,7 @@ default_dns = "9.9.9.9" t.Fatalf("write config.toml: %v", err) } - cfg, err := Load(paths.Layout{ConfigDir: configDir}) + cfg, err := Load(paths.Layout{ConfigDir: configDir, SSHDir: t.TempDir()}) if err != nil { t.Fatalf("Load: %v", err) } @@ -106,7 +106,7 @@ default_dns = "9.9.9.9" func TestLoadAppliesLogLevelEnvOverride(t *testing.T) { t.Setenv("BANGER_LOG_LEVEL", "warn") - cfg, err := Load(paths.Layout{ConfigDir: t.TempDir()}) + cfg, err := Load(paths.Layout{ConfigDir: t.TempDir(), SSHDir: t.TempDir()}) if err != nil { t.Fatalf("Load: %v", err) } @@ -130,7 +130,7 @@ mode = "0644" if err := os.WriteFile(filepath.Join(configDir, "config.toml"), data, 0o644); err != nil { t.Fatal(err) } - cfg, err := Load(paths.Layout{ConfigDir: configDir}) + cfg, err := Load(paths.Layout{ConfigDir: configDir, SSHDir: t.TempDir()}) if err != nil { t.Fatalf("Load: %v", err) } @@ -193,7 +193,7 @@ func TestLoadRejectsInvalidFileSyncEntries(t *testing.T) { if err := os.WriteFile(filepath.Join(configDir, "config.toml"), []byte(tc.toml+"\n"), 0o644); err != nil { t.Fatal(err) } - _, err := Load(paths.Layout{ConfigDir: configDir}) + _, err := Load(paths.Layout{ConfigDir: configDir, SSHDir: t.TempDir()}) if err == nil { t.Fatalf("Load: want error containing %q", tc.want) } @@ -216,7 +216,7 @@ system_overlay_size = "12G" if err := os.WriteFile(filepath.Join(configDir, "config.toml"), data, 0o644); err != nil { t.Fatal(err) } - cfg, err := Load(paths.Layout{ConfigDir: configDir}) + cfg, err := Load(paths.Layout{ConfigDir: configDir, SSHDir: t.TempDir()}) if err != nil { t.Fatalf("Load: %v", err) } @@ -237,7 +237,7 @@ system_overlay_size = "12G" func TestLoadEmptyVMDefaultsLeavesZeros(t *testing.T) { // No [vm_defaults] block → cfg.VMDefaults is the zero value, // which the resolver will map to auto or builtin. - cfg, err := Load(paths.Layout{ConfigDir: t.TempDir()}) + cfg, err := Load(paths.Layout{ConfigDir: t.TempDir(), SSHDir: t.TempDir()}) if err != nil { t.Fatalf("Load: %v", err) } @@ -259,7 +259,7 @@ func TestLoadRejectsNegativeVMDefaults(t *testing.T) { if err := os.WriteFile(filepath.Join(configDir, "config.toml"), []byte(body+"\n"), 0o644); err != nil { t.Fatal(err) } - if _, err := Load(paths.Layout{ConfigDir: configDir}); err == nil { + if _, err := Load(paths.Layout{ConfigDir: configDir, SSHDir: t.TempDir()}); err == nil { t.Fatal("expected error") } }) diff --git a/internal/config/ssh/id_ed25519 b/internal/config/ssh/id_ed25519 deleted file mode 100644 index 6e6936e..0000000 --- a/internal/config/ssh/id_ed25519 +++ /dev/null @@ -1,3 +0,0 @@ ------BEGIN PRIVATE KEY----- -MC4CAQAwBQYDK2VwBCIEIOeClGP/5JANJJpar5grOSE0RcaqMedAT5Nc6BcyCphM ------END PRIVATE KEY----- diff --git a/internal/config/ssh/id_ed25519.pub b/internal/config/ssh/id_ed25519.pub deleted file mode 100644 index bdfd01b..0000000 --- a/internal/config/ssh/id_ed25519.pub +++ /dev/null @@ -1 +0,0 @@ -ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOpF+WjNdlBLZYI3sbPST2lhxzrsfELwRXT58vkNL3xK