1
0
forked from sneak/secret

refactor: return errors from NewCLIInstance instead of panicking

Change NewCLIInstance() and NewCLIInstanceWithFs() to return
(*Instance, error) instead of panicking on DetermineStateDir failure.

Callers in RunE contexts propagate the error. Callers in command
construction (for shell completion) use log.Fatalf. Test callers
use t.Fatalf.

Addresses review feedback on PR #18.
This commit is contained in:
clawbot 2026-02-19 23:53:29 -08:00
parent 36ece2fca7
commit 6be4601763
12 changed files with 156 additions and 42 deletions

View File

@ -17,30 +17,30 @@ type Instance struct {
} }
// NewCLIInstance creates a new CLI instance with the real filesystem // NewCLIInstance creates a new CLI instance with the real filesystem
func NewCLIInstance() *Instance { func NewCLIInstance() (*Instance, error) {
fs := afero.NewOsFs() fs := afero.NewOsFs()
stateDir, err := secret.DetermineStateDir("") stateDir, err := secret.DetermineStateDir("")
if err != nil { if err != nil {
panic(fmt.Sprintf("cannot determine state directory: %v", err)) return nil, fmt.Errorf("cannot determine state directory: %w", err)
} }
return &Instance{ return &Instance{
fs: fs, fs: fs,
stateDir: stateDir, stateDir: stateDir,
} }, nil
} }
// NewCLIInstanceWithFs creates a new CLI instance with the given filesystem (for testing) // NewCLIInstanceWithFs creates a new CLI instance with the given filesystem (for testing)
func NewCLIInstanceWithFs(fs afero.Fs) *Instance { func NewCLIInstanceWithFs(fs afero.Fs) (*Instance, error) {
stateDir, err := secret.DetermineStateDir("") stateDir, err := secret.DetermineStateDir("")
if err != nil { if err != nil {
panic(fmt.Sprintf("cannot determine state directory: %v", err)) return nil, fmt.Errorf("cannot determine state directory: %w", err)
} }
return &Instance{ return &Instance{
fs: fs, fs: fs,
stateDir: stateDir, stateDir: stateDir,
} }, nil
} }
// NewCLIInstanceWithStateDir creates a new CLI instance with custom state directory (for testing) // NewCLIInstanceWithStateDir creates a new CLI instance with custom state directory (for testing)

View File

@ -25,7 +25,10 @@ func TestCLIInstanceStateDir(t *testing.T) {
func TestCLIInstanceWithFs(t *testing.T) { func TestCLIInstanceWithFs(t *testing.T) {
// Test creating CLI instance with custom filesystem // Test creating CLI instance with custom filesystem
fs := afero.NewMemMapFs() fs := afero.NewMemMapFs()
cli := NewCLIInstanceWithFs(fs) cli, err := NewCLIInstanceWithFs(fs)
if err != nil {
t.Fatalf("failed to initialize CLI: %v", err)
}
// The state directory should be determined automatically // The state directory should be determined automatically
stateDir := cli.GetStateDir() stateDir := cli.GetStateDir()

View File

@ -22,7 +22,10 @@ func newEncryptCmd() *cobra.Command {
inputFile, _ := cmd.Flags().GetString("input") inputFile, _ := cmd.Flags().GetString("input")
outputFile, _ := cmd.Flags().GetString("output") outputFile, _ := cmd.Flags().GetString("output")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
cli.cmd = cmd cli.cmd = cmd
return cli.Encrypt(args[0], inputFile, outputFile) return cli.Encrypt(args[0], inputFile, outputFile)
@ -45,7 +48,10 @@ func newDecryptCmd() *cobra.Command {
inputFile, _ := cmd.Flags().GetString("input") inputFile, _ := cmd.Flags().GetString("input")
outputFile, _ := cmd.Flags().GetString("output") outputFile, _ := cmd.Flags().GetString("output")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
cli.cmd = cmd cli.cmd = cmd
return cli.Decrypt(args[0], inputFile, outputFile) return cli.Decrypt(args[0], inputFile, outputFile)

View File

@ -38,7 +38,10 @@ func newGenerateMnemonicCmd() *cobra.Command {
`mnemonic phrase that can be used with 'secret init' ` + `mnemonic phrase that can be used with 'secret init' ` +
`or 'secret import'.`, `or 'secret import'.`,
RunE: func(cmd *cobra.Command, _ []string) error { RunE: func(cmd *cobra.Command, _ []string) error {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.GenerateMnemonic(cmd) return cli.GenerateMnemonic(cmd)
}, },
@ -56,7 +59,10 @@ func newGenerateSecretCmd() *cobra.Command {
secretType, _ := cmd.Flags().GetString("type") secretType, _ := cmd.Flags().GetString("type")
force, _ := cmd.Flags().GetBool("force") force, _ := cmd.Flags().GetBool("force")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.GenerateSecret(cmd, args[0], length, secretType, force) return cli.GenerateSecret(cmd, args[0], length, secretType, force)
}, },

View File

@ -1,6 +1,7 @@
package cli package cli
import ( import (
"log"
"encoding/json" "encoding/json"
"fmt" "fmt"
"io" "io"
@ -40,7 +41,10 @@ type InfoOutput struct {
// newInfoCmd returns the info command // newInfoCmd returns the info command
func newInfoCmd() *cobra.Command { func newInfoCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
var jsonOutput bool var jsonOutput bool

View File

@ -1,6 +1,7 @@
package cli package cli
import ( import (
"log"
"fmt" "fmt"
"log/slog" "log/slog"
"os" "os"
@ -27,7 +28,10 @@ func NewInitCmd() *cobra.Command {
// RunInit is the exported function that handles the init command // RunInit is the exported function that handles the init command
func RunInit(cmd *cobra.Command, _ []string) error { func RunInit(cmd *cobra.Command, _ []string) error {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
return cli.Init(cmd) return cli.Init(cmd)
} }

View File

@ -1,6 +1,7 @@
package cli package cli
import ( import (
"log"
"encoding/json" "encoding/json"
"fmt" "fmt"
"io" "io"
@ -44,7 +45,10 @@ func newAddCmd() *cobra.Command {
force, _ := cmd.Flags().GetBool("force") force, _ := cmd.Flags().GetBool("force")
secret.Debug("Got force flag", "force", force) secret.Debug("Got force flag", "force", force)
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
cli.cmd = cmd // Set the command for stdin access cli.cmd = cmd // Set the command for stdin access
secret.Debug("Created CLI instance, calling AddSecret") secret.Debug("Created CLI instance, calling AddSecret")
@ -58,7 +62,10 @@ func newAddCmd() *cobra.Command {
} }
func newGetCmd() *cobra.Command { func newGetCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
cmd := &cobra.Command{ cmd := &cobra.Command{
Use: "get <secret-name>", Use: "get <secret-name>",
Short: "Retrieve a secret from the vault", Short: "Retrieve a secret from the vault",
@ -66,7 +73,10 @@ func newGetCmd() *cobra.Command {
ValidArgsFunction: getSecretNamesCompletionFunc(cli.fs, cli.stateDir), ValidArgsFunction: getSecretNamesCompletionFunc(cli.fs, cli.stateDir),
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
version, _ := cmd.Flags().GetString("version") version, _ := cmd.Flags().GetString("version")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.GetSecretWithVersion(cmd, args[0], version) return cli.GetSecretWithVersion(cmd, args[0], version)
}, },
@ -93,7 +103,10 @@ func newListCmd() *cobra.Command {
filter = args[0] filter = args[0]
} }
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.ListSecrets(cmd, jsonOutput, quietOutput, filter) return cli.ListSecrets(cmd, jsonOutput, quietOutput, filter)
}, },
@ -115,7 +128,10 @@ func newImportCmd() *cobra.Command {
sourceFile, _ := cmd.Flags().GetString("source") sourceFile, _ := cmd.Flags().GetString("source")
force, _ := cmd.Flags().GetBool("force") force, _ := cmd.Flags().GetBool("force")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.ImportSecret(cmd, args[0], sourceFile, force) return cli.ImportSecret(cmd, args[0], sourceFile, force)
}, },
@ -129,7 +145,10 @@ func newImportCmd() *cobra.Command {
} }
func newRemoveCmd() *cobra.Command { func newRemoveCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
cmd := &cobra.Command{ cmd := &cobra.Command{
Use: "remove <secret-name>", Use: "remove <secret-name>",
Aliases: []string{"rm"}, Aliases: []string{"rm"},
@ -139,7 +158,10 @@ func newRemoveCmd() *cobra.Command {
Args: cobra.ExactArgs(1), Args: cobra.ExactArgs(1),
ValidArgsFunction: getSecretNamesCompletionFunc(cli.fs, cli.stateDir), ValidArgsFunction: getSecretNamesCompletionFunc(cli.fs, cli.stateDir),
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.RemoveSecret(cmd, args[0], false) return cli.RemoveSecret(cmd, args[0], false)
}, },
@ -149,7 +171,10 @@ func newRemoveCmd() *cobra.Command {
} }
func newMoveCmd() *cobra.Command { func newMoveCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
cmd := &cobra.Command{ cmd := &cobra.Command{
Use: "move <source> <destination>", Use: "move <source> <destination>",
Aliases: []string{"mv", "rename"}, Aliases: []string{"mv", "rename"},
@ -172,7 +197,10 @@ The source secret is deleted after successful copy.`,
}, },
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
force, _ := cmd.Flags().GetBool("force") force, _ := cmd.Flags().GetBool("force")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.MoveSecret(cmd, args[0], args[1], force) return cli.MoveSecret(cmd, args[0], args[1], force)
}, },

View File

@ -113,7 +113,10 @@ func TestAddSecretVariousSizes(t *testing.T) {
cmd.SetIn(stdin) cmd.SetIn(stdin)
// Create CLI instance // Create CLI instance
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
t.Fatalf("failed to initialize CLI: %v", err)
}
cli.fs = fs cli.fs = fs
cli.stateDir = stateDir cli.stateDir = stateDir
cli.cmd = cmd cli.cmd = cmd
@ -230,7 +233,10 @@ func TestImportSecretVariousSizes(t *testing.T) {
cmd := &cobra.Command{} cmd := &cobra.Command{}
// Create CLI instance // Create CLI instance
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
t.Fatalf("failed to initialize CLI: %v", err)
}
cli.fs = fs cli.fs = fs
cli.stateDir = stateDir cli.stateDir = stateDir
@ -318,7 +324,10 @@ func TestAddSecretBufferGrowth(t *testing.T) {
cmd.SetIn(stdin) cmd.SetIn(stdin)
// Create CLI instance // Create CLI instance
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
t.Fatalf("failed to initialize CLI: %v", err)
}
cli.fs = fs cli.fs = fs
cli.stateDir = stateDir cli.stateDir = stateDir
cli.cmd = cmd cli.cmd = cmd
@ -377,7 +386,10 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
cmd.SetIn(slowReader) cmd.SetIn(slowReader)
// Create CLI instance // Create CLI instance
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
t.Fatalf("failed to initialize CLI: %v", err)
}
cli.fs = fs cli.fs = fs
cli.stateDir = stateDir cli.stateDir = stateDir
cli.cmd = cmd cli.cmd = cmd

View File

@ -1,6 +1,7 @@
package cli package cli
import ( import (
"log"
"encoding/json" "encoding/json"
"fmt" "fmt"
"os" "os"
@ -96,7 +97,10 @@ func newUnlockerListCmd() *cobra.Command {
RunE: func(cmd *cobra.Command, _ []string) error { RunE: func(cmd *cobra.Command, _ []string) error {
jsonOutput, _ := cmd.Flags().GetBool("json") jsonOutput, _ := cmd.Flags().GetBool("json")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
cli.cmd = cmd cli.cmd = cmd
return cli.UnlockersList(jsonOutput) return cli.UnlockersList(jsonOutput)
@ -153,7 +157,10 @@ to access the same vault. This provides flexibility and backup access options.`,
Args: cobra.ExactArgs(1), Args: cobra.ExactArgs(1),
ValidArgs: strings.Split(supportedTypes, ", "), ValidArgs: strings.Split(supportedTypes, ", "),
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
unlockerType := args[0] unlockerType := args[0]
// Validate unlocker type // Validate unlocker type
@ -186,7 +193,10 @@ to access the same vault. This provides flexibility and backup access options.`,
} }
func newUnlockerRemoveCmd() *cobra.Command { func newUnlockerRemoveCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
cmd := &cobra.Command{ cmd := &cobra.Command{
Use: "remove <unlocker-id>", Use: "remove <unlocker-id>",
Aliases: []string{"rm"}, Aliases: []string{"rm"},
@ -198,7 +208,10 @@ func newUnlockerRemoveCmd() *cobra.Command {
ValidArgsFunction: getUnlockerIDsCompletionFunc(cli.fs, cli.stateDir), ValidArgsFunction: getUnlockerIDsCompletionFunc(cli.fs, cli.stateDir),
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
force, _ := cmd.Flags().GetBool("force") force, _ := cmd.Flags().GetBool("force")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.UnlockersRemove(args[0], force, cmd) return cli.UnlockersRemove(args[0], force, cmd)
}, },
@ -210,7 +223,10 @@ func newUnlockerRemoveCmd() *cobra.Command {
} }
func newUnlockerSelectCmd() *cobra.Command { func newUnlockerSelectCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
return &cobra.Command{ return &cobra.Command{
Use: "select <unlocker-id>", Use: "select <unlocker-id>",
@ -218,7 +234,10 @@ func newUnlockerSelectCmd() *cobra.Command {
Args: cobra.ExactArgs(1), Args: cobra.ExactArgs(1),
ValidArgsFunction: getUnlockerIDsCompletionFunc(cli.fs, cli.stateDir), ValidArgsFunction: getUnlockerIDsCompletionFunc(cli.fs, cli.stateDir),
RunE: func(_ *cobra.Command, args []string) error { RunE: func(_ *cobra.Command, args []string) error {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.UnlockerSelect(args[0]) return cli.UnlockerSelect(args[0])
}, },

View File

@ -1,6 +1,7 @@
package cli package cli
import ( import (
"log"
"encoding/json" "encoding/json"
"fmt" "fmt"
"os" "os"
@ -41,7 +42,10 @@ func newVaultListCmd() *cobra.Command {
RunE: func(cmd *cobra.Command, _ []string) error { RunE: func(cmd *cobra.Command, _ []string) error {
jsonOutput, _ := cmd.Flags().GetBool("json") jsonOutput, _ := cmd.Flags().GetBool("json")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.ListVaults(cmd, jsonOutput) return cli.ListVaults(cmd, jsonOutput)
}, },
@ -58,7 +62,10 @@ func newVaultCreateCmd() *cobra.Command {
Short: "Create a new vault", Short: "Create a new vault",
Args: cobra.ExactArgs(1), Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.CreateVault(cmd, args[0]) return cli.CreateVault(cmd, args[0])
}, },
@ -66,7 +73,10 @@ func newVaultCreateCmd() *cobra.Command {
} }
func newVaultSelectCmd() *cobra.Command { func newVaultSelectCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
return &cobra.Command{ return &cobra.Command{
Use: "select <name>", Use: "select <name>",
@ -74,7 +84,10 @@ func newVaultSelectCmd() *cobra.Command {
Args: cobra.ExactArgs(1), Args: cobra.ExactArgs(1),
ValidArgsFunction: getVaultNamesCompletionFunc(cli.fs, cli.stateDir), ValidArgsFunction: getVaultNamesCompletionFunc(cli.fs, cli.stateDir),
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.SelectVault(cmd, args[0]) return cli.SelectVault(cmd, args[0])
}, },
@ -82,7 +95,10 @@ func newVaultSelectCmd() *cobra.Command {
} }
func newVaultImportCmd() *cobra.Command { func newVaultImportCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
return &cobra.Command{ return &cobra.Command{
Use: "import <vault-name>", Use: "import <vault-name>",
@ -96,7 +112,10 @@ func newVaultImportCmd() *cobra.Command {
vaultName = args[0] vaultName = args[0]
} }
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.VaultImport(cmd, vaultName) return cli.VaultImport(cmd, vaultName)
}, },
@ -104,7 +123,10 @@ func newVaultImportCmd() *cobra.Command {
} }
func newVaultRemoveCmd() *cobra.Command { func newVaultRemoveCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
cmd := &cobra.Command{ cmd := &cobra.Command{
Use: "remove <name>", Use: "remove <name>",
Aliases: []string{"rm"}, Aliases: []string{"rm"},
@ -115,7 +137,10 @@ func newVaultRemoveCmd() *cobra.Command {
ValidArgsFunction: getVaultNamesCompletionFunc(cli.fs, cli.stateDir), ValidArgsFunction: getVaultNamesCompletionFunc(cli.fs, cli.stateDir),
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
force, _ := cmd.Flags().GetBool("force") force, _ := cmd.Flags().GetBool("force")
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
return cli.RemoveVault(cmd, args[0], force) return cli.RemoveVault(cmd, args[0], force)
}, },

View File

@ -1,6 +1,7 @@
package cli package cli
import ( import (
"log"
"fmt" "fmt"
"path/filepath" "path/filepath"
"strings" "strings"
@ -18,7 +19,10 @@ const (
// newVersionCmd returns the version management command // newVersionCmd returns the version management command
func newVersionCmd() *cobra.Command { func newVersionCmd() *cobra.Command {
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
log.Fatalf("failed to initialize CLI: %v", err)
}
return VersionCommands(cli) return VersionCommands(cli)
} }

View File

@ -266,7 +266,10 @@ func TestGetSecretWithVersion(t *testing.T) {
func TestVersionCommandStructure(t *testing.T) { func TestVersionCommandStructure(t *testing.T) {
// Test that version commands are properly structured // Test that version commands are properly structured
cli := NewCLIInstance() cli, err := NewCLIInstance()
if err != nil {
t.Fatalf("failed to initialize CLI: %v", err)
}
cmd := VersionCommands(cli) cmd := VersionCommands(cli)
assert.Equal(t, "version", cmd.Use) assert.Equal(t, "version", cmd.Use)