From d370a98d6815a52d9d85bb69b3210a4062ac2412 Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Mon, 25 May 2026 17:15:19 -0500 Subject: [PATCH 1/2] fix(config): wire --ssh-key-file flag into resolveSSHKeys Add tests that verify: - reading a public key from --ssh-key-file - reading multiple --ssh-key-file flags - merging --ssh-key and --ssh-key-file - error on nonexistent --ssh-key-file path All tests currently fail because the flag is not registered in BindFlags() and resolveSSHKeys() does not read files. resolveSSHKeys now reads public key files from --ssh-key-file paths and merges them with inline --ssh-key values. Both CLI flag sources share highest priority, followed by VIRTWORK_SSH_AUTHORIZED_KEYS env var, then YAML config. Returns an error if a specified key file cannot be read, rather than silently producing VMs without SSH access. Also registers --ssh-key-file in BindFlags for test coverage. Signed-off-by: Melvin Hillsman --- internal/config/config.go | 37 +++++++++++++---- internal/config/config_test.go | 73 ++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 7 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 74e6864..3d108bc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -87,6 +87,7 @@ func BindFlags(cmd *cobra.Command) { f.String("ssh-user", "", "SSH user for VMs") f.String("ssh-password", "", "SSH password for VMs") f.StringSlice("ssh-key", nil, "SSH authorized key (repeatable)") + f.StringSlice("ssh-key-file", nil, "SSH key file path (repeatable)") } // LoadConfig loads configuration from flags, environment variables, config file, @@ -160,7 +161,11 @@ func LoadConfig(cmd *cobra.Command) (*Config, error) { cfg.AuditDBPath = v.GetString("audit-db") // Handle SSH authorized keys: CLI flags, env var (comma-split), or YAML list - cfg.SSHAuthorizedKeys = resolveSSHKeys(v, cmd) + sshKeys, err := resolveSSHKeys(v, cmd) + if err != nil { + return nil, err + } + cfg.SSHAuthorizedKeys = sshKeys // Unmarshal workloads map if present in config file workloads := make(map[string]WorkloadConfig) @@ -184,15 +189,33 @@ func bindFlagIfSet(v *viper.Viper, cmd *cobra.Command, name string) { // resolveSSHKeys resolves SSH authorized keys from CLI flags, env vars, or config file. // Priority: CLI --ssh-key flags > VIRTWORK_SSH_AUTHORIZED_KEYS env var > YAML config. -func resolveSSHKeys(v *viper.Viper, cmd *cobra.Command) []string { - // CLI flags take highest priority +func resolveSSHKeys(v *viper.Viper, cmd *cobra.Command) ([]string, error) { + // CLI flags take highest priority — merge inline keys and file-based keys + var cliKeys []string + if cmd.Flags().Changed("ssh-key") { keys, _ := cmd.Flags().GetStringSlice("ssh-key") - if len(keys) > 0 { - return keys + cliKeys = append(cliKeys, keys...) + } + + if cmd.Flags().Changed("ssh-key-file") { + paths, _ := cmd.Flags().GetStringSlice("ssh-key-file") + for _, p := range paths { + data, err := os.ReadFile(p) + if err != nil { + return nil, fmt.Errorf("reading SSH key file %s: %w", p, err) + } + key := strings.TrimSpace(string(data)) + if key != "" { + cliKeys = append(cliKeys, key) + } } } + if len(cliKeys) > 0 { + return cliKeys, nil + } + // Check env var with comma splitting envVal := os.Getenv("VIRTWORK_SSH_AUTHORIZED_KEYS") if envVal != "" { @@ -205,10 +228,10 @@ func resolveSSHKeys(v *viper.Viper, cmd *cobra.Command) []string { } } if len(keys) > 0 { - return keys + return keys, nil } } // Fall back to YAML config list - return v.GetStringSlice("ssh-authorized-keys") + return v.GetStringSlice("ssh-authorized-keys"), nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 63565e4..43cf38a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -36,6 +36,13 @@ func writeConfigFile(dir, content string) string { return path } +func writeKeyFile(dir, name, content string) string { + path := filepath.Join(dir, name) + err := os.WriteFile(path, []byte(content), 0o600) + Expect(err).NotTo(HaveOccurred()) + return path +} + var _ = Describe("Config", func() { var cmd *cobra.Command @@ -328,6 +335,72 @@ ssh-authorized-keys: Expect(cfg.SSHAuthorizedKeys).To(ContainElement("ssh-rsa YAMLKEY1")) Expect(cfg.SSHAuthorizedKeys).To(ContainElement("ssh-ed25519 YAMLKEY2")) }) + + It("should read public key from --ssh-key-file", func() { + tmpDir, err := os.MkdirTemp("", "virtwork-config-test-*") + Expect(err).NotTo(HaveOccurred()) + defer func() { + _ = os.RemoveAll(tmpDir) + }() + + keyPath := writeKeyFile(tmpDir, "id_ed25519.pub", "ssh-ed25519 AAAAC3filekey user@host\n") + err1 := cmd.Flags().Set("ssh-key-file", keyPath) + Expect(err1).NotTo(HaveOccurred()) + + cfg, err := config.LoadConfig(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.SSHAuthorizedKeys).To(ContainElement("ssh-ed25519 AAAAC3filekey user@host")) + }) + + It("should read multiple --ssh-key-file flags", func() { + tmpDir, err := os.MkdirTemp("", "virtwork-config-test-*") + Expect(err).NotTo(HaveOccurred()) + defer func() { + _ = os.RemoveAll(tmpDir) + }() + + keyPath1 := writeKeyFile(tmpDir, "key1.pub", "ssh-rsa AAAA1 user1@host\n") + keyPath2 := writeKeyFile(tmpDir, "key2.pub", "ssh-ed25519 AAAA2 user2@host\n") + err1 := cmd.Flags().Set("ssh-key-file", keyPath1) + Expect(err1).NotTo(HaveOccurred()) + err2 := cmd.Flags().Set("ssh-key-file", keyPath2) + Expect(err2).NotTo(HaveOccurred()) + + cfg, err := config.LoadConfig(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.SSHAuthorizedKeys).To(HaveLen(2)) + Expect(cfg.SSHAuthorizedKeys).To(ContainElement("ssh-rsa AAAA1 user1@host")) + Expect(cfg.SSHAuthorizedKeys).To(ContainElement("ssh-ed25519 AAAA2 user2@host")) + }) + + It("should merge --ssh-key and --ssh-key-file", func() { + tmpDir, err := os.MkdirTemp("", "virtwork-config-test-*") + Expect(err).NotTo(HaveOccurred()) + defer func() { + _ = os.RemoveAll(tmpDir) + }() + + keyPath := writeKeyFile(tmpDir, "id.pub", "ssh-ed25519 FILEkey user@host\n") + err1 := cmd.Flags().Set("ssh-key", "ssh-rsa INLINEkey") + Expect(err1).NotTo(HaveOccurred()) + err2 := cmd.Flags().Set("ssh-key-file", keyPath) + Expect(err2).NotTo(HaveOccurred()) + + cfg, err := config.LoadConfig(cmd) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.SSHAuthorizedKeys).To(HaveLen(2)) + Expect(cfg.SSHAuthorizedKeys).To(ContainElement("ssh-rsa INLINEkey")) + Expect(cfg.SSHAuthorizedKeys).To(ContainElement("ssh-ed25519 FILEkey user@host")) + }) + + It("should return error for nonexistent --ssh-key-file", func() { + err1 := cmd.Flags().Set("ssh-key-file", "/nonexistent/key.pub") + Expect(err1).NotTo(HaveOccurred()) + + _, err := config.LoadConfig(cmd) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("/nonexistent/key.pub")) + }) }) Context("workload config", func() { From 8a5a026a373886ea1cf96ce84cc7fe79316e0318 Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Mon, 25 May 2026 18:49:20 -0500 Subject: [PATCH 2/2] fix(config): resolve gosec G304 on ssh-key-file ReadFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply filepath.Clean to normalize user-supplied key file paths and add nolint directive — this is a CLI tool where the user intentionally specifies the file path. Signed-off-by: Melvin Hillsman --- internal/config/config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 3d108bc..36528dd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,6 +6,7 @@ package config import ( "fmt" "os" + "path/filepath" "strings" "github.com/spf13/cobra" @@ -201,7 +202,7 @@ func resolveSSHKeys(v *viper.Viper, cmd *cobra.Command) ([]string, error) { if cmd.Flags().Changed("ssh-key-file") { paths, _ := cmd.Flags().GetStringSlice("ssh-key-file") for _, p := range paths { - data, err := os.ReadFile(p) + data, err := os.ReadFile(filepath.Clean(p)) //nolint:gosec // CLI user supplies the path intentionally if err != nil { return nil, fmt.Errorf("reading SSH key file %s: %w", p, err) }