Thales Maciel
b1fbf695ca
Bug: syncVMSSHClientConfig did os.RemoveAll on $ConfigDir/ssh every daemon Open. The intent was to migrate off the pre-opt-in layout, where banger used to write $ConfigDir/ssh/ssh_config. But a user who sets ssh_key_path = "~/.config/banger/ssh/id_ed25519" in config.toml has their key live exactly in that dir — and the scrub deletes it along with every other file in the tree. This is the same class of bug that cost the default key untilebe6517moved it to StateDir, but that fix was scoped to the default path. A configured ssh_key_path pointed under the legacy dir still dies. Fix: replace os.RemoveAll with a narrow two-step cleanup: 1. Skip the cleanup entirely when the configured ssh_key_path resolves under the legacy dir. A user who pointed banger at a key there must keep the enclosing directory. 2. Otherwise, os.Remove the specific legacy file ($ConfigDir/ssh/ ssh_config) and then os.Remove the directory. The second os.Remove fails with ENOTEMPTY if the dir still holds anything (e.g. a user-managed sibling file we don't own). Both errors are swallowed — this is best-effort migration, not a hard failure. Tests pin all three paths: user key under legacy dir survives, legacy dir empties and is removed when the user moved on, and a user-managed sibling file in the legacy dir is preserved. Also fix stale doc claims in README.md and AGENTS.md — both still pointed at the old ~/.config/banger/ssh/id_ed25519 default, which moved to ~/.local/state/banger/ssh/id_ed25519 inebe6517. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
400 lines
13 KiB
Go
400 lines
13 KiB
Go
package daemon
|
|
|
|
import (
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"banger/internal/paths"
|
|
)
|
|
|
|
// A user-configured ssh_key_path that happens to live under the
|
|
// legacy $ConfigDir/ssh directory must survive the pre-opt-in
|
|
// migration cleanup. The old code did os.RemoveAll on the whole
|
|
// directory, which nuked the key. Pin the narrower behavior so a
|
|
// future refactor can't re-broaden the scrub.
|
|
func TestSyncVMSSHClientConfigPreservesUserKeyInLegacyDir(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
|
|
configDir := filepath.Join(homeDir, ".config", "banger")
|
|
legacyDir := filepath.Join(configDir, "ssh")
|
|
if err := os.MkdirAll(legacyDir, 0o700); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
userKey := filepath.Join(legacyDir, "id_ed25519")
|
|
if err := os.WriteFile(userKey, []byte("PRIVATE"), 0o600); err != nil {
|
|
t.Fatalf("WriteFile: %v", err)
|
|
}
|
|
// A stale ssh_config under the same dir from pre-opt-in era.
|
|
legacyConfig := filepath.Join(legacyDir, "ssh_config")
|
|
if err := os.WriteFile(legacyConfig, []byte("stale"), 0o644); err != nil {
|
|
t.Fatalf("WriteFile: %v", err)
|
|
}
|
|
|
|
layout := paths.Layout{
|
|
ConfigDir: configDir,
|
|
KnownHostsPath: filepath.Join(homeDir, ".local", "state", "banger", "ssh", "known_hosts"),
|
|
}
|
|
if err := syncVMSSHClientConfig(layout, userKey); err != nil {
|
|
t.Fatalf("syncVMSSHClientConfig: %v", err)
|
|
}
|
|
|
|
// The configured key must survive.
|
|
if _, err := os.Stat(userKey); err != nil {
|
|
t.Fatalf("user-configured key disappeared: %v", err)
|
|
}
|
|
// Enclosing directory must also survive (it contains the key).
|
|
if _, err := os.Stat(legacyDir); err != nil {
|
|
t.Fatalf("legacy dir removed despite containing the configured key: %v", err)
|
|
}
|
|
// The stale legacy ssh_config file can still be gone in this
|
|
// case — the user's key isn't ssh_config, so cleaning up the
|
|
// sibling file is fine. We don't assert either way, since the
|
|
// gate is "don't delete the user's key" not "always delete the
|
|
// sibling file."
|
|
}
|
|
|
|
// With ssh_key_path configured outside ConfigDir/ssh, the legacy
|
|
// migration step should scrub the old sibling file and then the
|
|
// (now-empty) directory — no os.RemoveAll on anything still in use.
|
|
func TestSyncVMSSHClientConfigNarrowsCleanupToLegacyFile(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
|
|
configDir := filepath.Join(homeDir, ".config", "banger")
|
|
legacyDir := filepath.Join(configDir, "ssh")
|
|
if err := os.MkdirAll(legacyDir, 0o700); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
// Simulate the pre-opt-in leftover: just the ssh_config file.
|
|
legacyConfig := filepath.Join(legacyDir, "ssh_config")
|
|
if err := os.WriteFile(legacyConfig, []byte("stale"), 0o644); err != nil {
|
|
t.Fatalf("WriteFile: %v", err)
|
|
}
|
|
|
|
// ssh_key_path lives in the state dir (the new default location).
|
|
stateDir := filepath.Join(homeDir, ".local", "state", "banger", "ssh")
|
|
if err := os.MkdirAll(stateDir, 0o700); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
userKey := filepath.Join(stateDir, "id_ed25519")
|
|
if err := os.WriteFile(userKey, []byte("PRIVATE"), 0o600); err != nil {
|
|
t.Fatalf("WriteFile: %v", err)
|
|
}
|
|
|
|
layout := paths.Layout{
|
|
ConfigDir: configDir,
|
|
KnownHostsPath: filepath.Join(homeDir, ".local", "state", "banger", "ssh", "known_hosts"),
|
|
}
|
|
if err := syncVMSSHClientConfig(layout, userKey); err != nil {
|
|
t.Fatalf("syncVMSSHClientConfig: %v", err)
|
|
}
|
|
|
|
// Legacy ssh_config file: gone.
|
|
if _, err := os.Stat(legacyConfig); !os.IsNotExist(err) {
|
|
t.Fatalf("legacy ssh_config survived cleanup: %v", err)
|
|
}
|
|
// Legacy dir: gone, since it was empty after the file removal.
|
|
if _, err := os.Stat(legacyDir); !os.IsNotExist(err) {
|
|
t.Fatalf("legacy dir survived cleanup when empty: %v", err)
|
|
}
|
|
// User's key: untouched.
|
|
if _, err := os.Stat(userKey); err != nil {
|
|
t.Fatalf("user key disappeared: %v", err)
|
|
}
|
|
}
|
|
|
|
// If the legacy dir contains UNEXPECTED files (not ssh_config, not
|
|
// the configured key), leave the dir alone. os.Remove on a non-
|
|
// empty dir errors with ENOTEMPTY, which we swallow. Regression
|
|
// guard so the cleanup can never escalate to recursive deletion.
|
|
func TestSyncVMSSHClientConfigLeavesUnexpectedLegacyContents(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
|
|
configDir := filepath.Join(homeDir, ".config", "banger")
|
|
legacyDir := filepath.Join(configDir, "ssh")
|
|
if err := os.MkdirAll(legacyDir, 0o700); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
// A user-managed file we have no business removing.
|
|
userFile := filepath.Join(legacyDir, "my-other-thing")
|
|
if err := os.WriteFile(userFile, []byte("mine"), 0o600); err != nil {
|
|
t.Fatalf("WriteFile: %v", err)
|
|
}
|
|
|
|
layout := paths.Layout{
|
|
ConfigDir: configDir,
|
|
KnownHostsPath: filepath.Join(homeDir, ".local", "state", "banger", "ssh", "known_hosts"),
|
|
}
|
|
// ssh_key_path lives elsewhere; cleanup would otherwise proceed.
|
|
stateKey := filepath.Join(homeDir, ".local", "state", "banger", "ssh", "id_ed25519")
|
|
if err := os.MkdirAll(filepath.Dir(stateKey), 0o700); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
if err := os.WriteFile(stateKey, []byte("PRIVATE"), 0o600); err != nil {
|
|
t.Fatalf("WriteFile: %v", err)
|
|
}
|
|
|
|
if err := syncVMSSHClientConfig(layout, stateKey); err != nil {
|
|
t.Fatalf("syncVMSSHClientConfig: %v", err)
|
|
}
|
|
|
|
if _, err := os.Stat(userFile); err != nil {
|
|
t.Fatalf("user-managed legacy-dir file disappeared: %v", err)
|
|
}
|
|
if _, err := os.Stat(legacyDir); err != nil {
|
|
t.Fatalf("legacy dir vanished despite non-empty contents: %v", err)
|
|
}
|
|
}
|
|
|
|
// Under the opt-in contract the daemon writes its own ssh_config file
|
|
// and never touches ~/.ssh/config on its own.
|
|
func TestSyncVMSSHClientConfigWritesBangerFileOnly(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
|
|
knownHostsPath := filepath.Join(homeDir, ".local", "state", "banger", "ssh", "known_hosts")
|
|
layout := paths.Layout{
|
|
ConfigDir: filepath.Join(homeDir, ".config", "banger"),
|
|
KnownHostsPath: knownHostsPath,
|
|
}
|
|
keyPath := filepath.Join(homeDir, ".config", "banger", "ssh", "id_ed25519")
|
|
|
|
if err := syncVMSSHClientConfig(layout, keyPath); err != nil {
|
|
t.Fatalf("syncVMSSHClientConfig: %v", err)
|
|
}
|
|
|
|
// Banger's own ssh_config file has the `Host *.vm` stanza.
|
|
bangerConfig, err := os.ReadFile(BangerSSHConfigPath(layout))
|
|
if err != nil {
|
|
t.Fatalf("ReadFile(banger ssh_config): %v", err)
|
|
}
|
|
for _, want := range []string{
|
|
"Host *.vm",
|
|
"IdentityFile " + keyPath,
|
|
"UserKnownHostsFile " + knownHostsPath,
|
|
"StrictHostKeyChecking accept-new",
|
|
} {
|
|
if !strings.Contains(string(bangerConfig), want) {
|
|
t.Fatalf("banger ssh_config missing %q:\n%s", want, bangerConfig)
|
|
}
|
|
}
|
|
|
|
// ~/.ssh/config must NOT have been created or modified.
|
|
if _, err := os.Stat(filepath.Join(homeDir, ".ssh", "config")); !os.IsNotExist(err) {
|
|
t.Fatalf("~/.ssh/config should be untouched; stat err = %v", err)
|
|
}
|
|
|
|
// Regression: the legacy posture (strict no + /dev/null) must not
|
|
// reappear in the banger file.
|
|
for _, must := range []string{
|
|
"StrictHostKeyChecking no",
|
|
"UserKnownHostsFile /dev/null",
|
|
} {
|
|
if strings.Contains(string(bangerConfig), must) {
|
|
t.Fatalf("banger ssh_config leaked legacy posture %q:\n%s", must, bangerConfig)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestInstallUserSSHIncludeAddsIncludeBlock(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
|
|
layout := paths.Layout{ConfigDir: filepath.Join(homeDir, ".config", "banger")}
|
|
if err := os.MkdirAll(layout.ConfigDir, 0o755); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
// Write a fake banger ssh_config so Install has something to include.
|
|
if err := os.WriteFile(BangerSSHConfigPath(layout), []byte("Host *.vm\n"), 0o644); err != nil {
|
|
t.Fatalf("WriteFile(banger ssh_config): %v", err)
|
|
}
|
|
|
|
if err := InstallUserSSHInclude(layout); err != nil {
|
|
t.Fatalf("InstallUserSSHInclude: %v", err)
|
|
}
|
|
got, err := os.ReadFile(filepath.Join(homeDir, ".ssh", "config"))
|
|
if err != nil {
|
|
t.Fatalf("ReadFile(~/.ssh/config): %v", err)
|
|
}
|
|
want := "Include " + BangerSSHConfigPath(layout)
|
|
if !strings.Contains(string(got), want) {
|
|
t.Fatalf("user config missing %q:\n%s", want, got)
|
|
}
|
|
if !strings.Contains(string(got), bangerSSHIncludeBegin) {
|
|
t.Fatalf("user config missing begin marker:\n%s", got)
|
|
}
|
|
}
|
|
|
|
func TestInstallUserSSHIncludeIsIdempotent(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
|
|
layout := paths.Layout{ConfigDir: filepath.Join(homeDir, ".config", "banger")}
|
|
if err := os.MkdirAll(layout.ConfigDir, 0o755); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
if err := os.WriteFile(BangerSSHConfigPath(layout), []byte("Host *.vm\n"), 0o644); err != nil {
|
|
t.Fatalf("WriteFile: %v", err)
|
|
}
|
|
for i := 0; i < 3; i++ {
|
|
if err := InstallUserSSHInclude(layout); err != nil {
|
|
t.Fatalf("InstallUserSSHInclude (%d): %v", i, err)
|
|
}
|
|
}
|
|
got, err := os.ReadFile(filepath.Join(homeDir, ".ssh", "config"))
|
|
if err != nil {
|
|
t.Fatalf("ReadFile: %v", err)
|
|
}
|
|
if n := strings.Count(string(got), bangerSSHIncludeBegin); n != 1 {
|
|
t.Fatalf("begin markers = %d, want 1:\n%s", n, got)
|
|
}
|
|
}
|
|
|
|
func TestInstallUserSSHIncludeMigratesLegacyInlineBlock(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
|
|
layout := paths.Layout{ConfigDir: filepath.Join(homeDir, ".config", "banger")}
|
|
if err := os.MkdirAll(layout.ConfigDir, 0o755); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
if err := os.WriteFile(BangerSSHConfigPath(layout), []byte("Host *.vm\n"), 0o644); err != nil {
|
|
t.Fatalf("WriteFile: %v", err)
|
|
}
|
|
|
|
sshDir := filepath.Join(homeDir, ".ssh")
|
|
if err := os.MkdirAll(sshDir, 0o700); err != nil {
|
|
t.Fatalf("MkdirAll(.ssh): %v", err)
|
|
}
|
|
legacy := strings.Join([]string{
|
|
"ServerAliveInterval 120",
|
|
"",
|
|
vmSSHConfigIncludeBegin,
|
|
"Host *.vm",
|
|
" User root",
|
|
" IdentityFile /some/old/key",
|
|
vmSSHConfigIncludeEnd,
|
|
"",
|
|
"Host other",
|
|
" HostName 192.0.2.5",
|
|
"",
|
|
}, "\n")
|
|
if err := os.WriteFile(filepath.Join(sshDir, "config"), []byte(legacy), 0o600); err != nil {
|
|
t.Fatalf("seed legacy config: %v", err)
|
|
}
|
|
|
|
if err := InstallUserSSHInclude(layout); err != nil {
|
|
t.Fatalf("InstallUserSSHInclude: %v", err)
|
|
}
|
|
got, err := os.ReadFile(filepath.Join(sshDir, "config"))
|
|
if err != nil {
|
|
t.Fatalf("ReadFile: %v", err)
|
|
}
|
|
gotStr := string(got)
|
|
// Legacy inline block must be gone.
|
|
if strings.Contains(gotStr, vmSSHConfigIncludeBegin) {
|
|
t.Fatalf("legacy inline block survived:\n%s", gotStr)
|
|
}
|
|
// New Include block must be present.
|
|
if !strings.Contains(gotStr, bangerSSHIncludeBegin) {
|
|
t.Fatalf("new include block missing:\n%s", gotStr)
|
|
}
|
|
// Unrelated stanzas must be preserved.
|
|
for _, want := range []string{"ServerAliveInterval 120", "Host other"} {
|
|
if !strings.Contains(gotStr, want) {
|
|
t.Fatalf("user config lost unrelated entry %q:\n%s", want, gotStr)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestUninstallUserSSHIncludeRemovesBothMarkerBlocks(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
|
|
sshDir := filepath.Join(homeDir, ".ssh")
|
|
if err := os.MkdirAll(sshDir, 0o700); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
seed := strings.Join([]string{
|
|
"Host keep",
|
|
" HostName 198.51.100.1",
|
|
"",
|
|
vmSSHConfigIncludeBegin,
|
|
"Host *.vm",
|
|
vmSSHConfigIncludeEnd,
|
|
"",
|
|
bangerSSHIncludeBegin,
|
|
"Include /tmp/banger-ssh-config",
|
|
bangerSSHIncludeEnd,
|
|
"",
|
|
}, "\n")
|
|
if err := os.WriteFile(filepath.Join(sshDir, "config"), []byte(seed), 0o600); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
|
|
if err := UninstallUserSSHInclude(); err != nil {
|
|
t.Fatalf("UninstallUserSSHInclude: %v", err)
|
|
}
|
|
got, err := os.ReadFile(filepath.Join(sshDir, "config"))
|
|
if err != nil {
|
|
t.Fatalf("ReadFile: %v", err)
|
|
}
|
|
gotStr := string(got)
|
|
for _, banned := range []string{vmSSHConfigIncludeBegin, bangerSSHIncludeBegin} {
|
|
if strings.Contains(gotStr, banned) {
|
|
t.Fatalf("residue of %q:\n%s", banned, gotStr)
|
|
}
|
|
}
|
|
if !strings.Contains(gotStr, "Host keep") {
|
|
t.Fatalf("lost unrelated entry:\n%s", gotStr)
|
|
}
|
|
}
|
|
|
|
func TestUninstallUserSSHIncludeIsNoOpWhenMissing(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
if err := UninstallUserSSHInclude(); err != nil {
|
|
t.Fatalf("UninstallUserSSHInclude on missing file: %v", err)
|
|
}
|
|
// Still no ~/.ssh/config.
|
|
if _, err := os.Stat(filepath.Join(homeDir, ".ssh", "config")); !os.IsNotExist(err) {
|
|
t.Fatalf("~/.ssh/config unexpectedly created; stat err = %v", err)
|
|
}
|
|
}
|
|
|
|
func TestUserSSHIncludeInstalledDetectsBothMarkers(t *testing.T) {
|
|
for _, tc := range []struct {
|
|
name string
|
|
seed string
|
|
wantIn bool
|
|
}{
|
|
{"missing file", "", false},
|
|
{"unrelated only", "Host other\n HostName 1.2.3.4\n", false},
|
|
{"legacy marker", vmSSHConfigIncludeBegin + "\nHost *.vm\n" + vmSSHConfigIncludeEnd + "\n", true},
|
|
{"new marker", bangerSSHIncludeBegin + "\nInclude /tmp/banger\n" + bangerSSHIncludeEnd + "\n", true},
|
|
} {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
homeDir := t.TempDir()
|
|
t.Setenv("HOME", homeDir)
|
|
if tc.seed != "" {
|
|
if err := os.MkdirAll(filepath.Join(homeDir, ".ssh"), 0o700); err != nil {
|
|
t.Fatalf("MkdirAll: %v", err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(homeDir, ".ssh", "config"), []byte(tc.seed), 0o600); err != nil {
|
|
t.Fatalf("WriteFile: %v", err)
|
|
}
|
|
}
|
|
got, err := UserSSHIncludeInstalled()
|
|
if err != nil {
|
|
t.Fatalf("UserSSHIncludeInstalled: %v", err)
|
|
}
|
|
if got != tc.wantIn {
|
|
t.Fatalf("got %v, want %v", got, tc.wantIn)
|
|
}
|
|
})
|
|
}
|
|
}
|