Merge branch 'main' into fix/issue-3
This commit is contained in:
commit
206651f89a
14
AGENTS.md
14
AGENTS.md
@ -141,3 +141,17 @@ Version: 2025-06-08
|
||||
- Local application imports
|
||||
|
||||
Each group should be separated by a blank line.
|
||||
|
||||
## Go-Specific Guidelines
|
||||
|
||||
1. **No `panic`, `log.Fatal`, or `os.Exit` in library code.** Always propagate errors via return values.
|
||||
|
||||
2. **Constructors return `(*T, error)`, not just `*T`.** Callers must handle errors, not crash.
|
||||
|
||||
3. **Wrap errors** with `fmt.Errorf("context: %w", err)` for debuggability.
|
||||
|
||||
4. **Never modify linter config** (`.golangci.yml`) to suppress findings. Fix the code.
|
||||
|
||||
5. **All PRs must pass `make check` with zero failures.** No exceptions, no "pre-existing issue" excuses.
|
||||
|
||||
6. **Pin external dependencies by commit hash**, not mutable tags.
|
||||
|
||||
@ -17,24 +17,30 @@ type Instance struct {
|
||||
}
|
||||
|
||||
// NewCLIInstance creates a new CLI instance with the real filesystem
|
||||
func NewCLIInstance() *Instance {
|
||||
func NewCLIInstance() (*Instance, error) {
|
||||
fs := afero.NewOsFs()
|
||||
stateDir := secret.DetermineStateDir("")
|
||||
stateDir, err := secret.DetermineStateDir("")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("cannot determine state directory: %w", err)
|
||||
}
|
||||
|
||||
return &Instance{
|
||||
fs: fs,
|
||||
stateDir: stateDir,
|
||||
}
|
||||
}, nil
|
||||
}
|
||||
|
||||
// NewCLIInstanceWithFs creates a new CLI instance with the given filesystem (for testing)
|
||||
func NewCLIInstanceWithFs(fs afero.Fs) *Instance {
|
||||
stateDir := secret.DetermineStateDir("")
|
||||
func NewCLIInstanceWithFs(fs afero.Fs) (*Instance, error) {
|
||||
stateDir, err := secret.DetermineStateDir("")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("cannot determine state directory: %w", err)
|
||||
}
|
||||
|
||||
return &Instance{
|
||||
fs: fs,
|
||||
stateDir: stateDir,
|
||||
}
|
||||
}, nil
|
||||
}
|
||||
|
||||
// NewCLIInstanceWithStateDir creates a new CLI instance with custom state directory (for testing)
|
||||
|
||||
@ -25,7 +25,10 @@ func TestCLIInstanceStateDir(t *testing.T) {
|
||||
func TestCLIInstanceWithFs(t *testing.T) {
|
||||
// Test creating CLI instance with custom filesystem
|
||||
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
|
||||
stateDir := cli.GetStateDir()
|
||||
@ -41,7 +44,10 @@ func TestDetermineStateDir(t *testing.T) {
|
||||
testEnvDir := "/test-env-dir"
|
||||
t.Setenv(secret.EnvStateDir, testEnvDir)
|
||||
|
||||
stateDir := secret.DetermineStateDir("")
|
||||
stateDir, err := secret.DetermineStateDir("")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if stateDir != testEnvDir {
|
||||
t.Errorf("Expected state directory %q from environment, got %q", testEnvDir, stateDir)
|
||||
}
|
||||
@ -49,7 +55,10 @@ func TestDetermineStateDir(t *testing.T) {
|
||||
// Test with custom config dir
|
||||
_ = os.Unsetenv(secret.EnvStateDir)
|
||||
customConfigDir := "/custom-config"
|
||||
stateDir = secret.DetermineStateDir(customConfigDir)
|
||||
stateDir, err = secret.DetermineStateDir(customConfigDir)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
expectedDir := filepath.Join(customConfigDir, secret.AppID)
|
||||
if stateDir != expectedDir {
|
||||
t.Errorf("Expected state directory %q with custom config, got %q", expectedDir, stateDir)
|
||||
|
||||
@ -22,7 +22,10 @@ func newEncryptCmd() *cobra.Command {
|
||||
inputFile, _ := cmd.Flags().GetString("input")
|
||||
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
|
||||
|
||||
return cli.Encrypt(args[0], inputFile, outputFile)
|
||||
@ -45,7 +48,10 @@ func newDecryptCmd() *cobra.Command {
|
||||
inputFile, _ := cmd.Flags().GetString("input")
|
||||
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
|
||||
|
||||
return cli.Decrypt(args[0], inputFile, outputFile)
|
||||
|
||||
@ -38,7 +38,10 @@ func newGenerateMnemonicCmd() *cobra.Command {
|
||||
`mnemonic phrase that can be used with 'secret init' ` +
|
||||
`or 'secret import'.`,
|
||||
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)
|
||||
},
|
||||
@ -56,7 +59,10 @@ func newGenerateSecretCmd() *cobra.Command {
|
||||
secretType, _ := cmd.Flags().GetString("type")
|
||||
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)
|
||||
},
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"log"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
@ -40,7 +41,10 @@ type InfoOutput struct {
|
||||
|
||||
// newInfoCmd returns the info command
|
||||
func newInfoCmd() *cobra.Command {
|
||||
cli := NewCLIInstance()
|
||||
cli, err := NewCLIInstance()
|
||||
if err != nil {
|
||||
log.Fatalf("failed to initialize CLI: %v", err)
|
||||
}
|
||||
|
||||
var jsonOutput bool
|
||||
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"log"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"os"
|
||||
@ -27,7 +28,10 @@ func NewInitCmd() *cobra.Command {
|
||||
|
||||
// RunInit is the exported function that handles the init command
|
||||
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)
|
||||
}
|
||||
|
||||
@ -1047,7 +1047,6 @@ func test12SecretNameFormats(t *testing.T, tempDir, testMnemonic string, runSecr
|
||||
// Test invalid secret names
|
||||
invalidNames := []string{
|
||||
"", // empty
|
||||
"UPPERCASE", // uppercase not allowed
|
||||
"with space", // spaces not allowed
|
||||
"with@symbol", // special characters not allowed
|
||||
"with#hash", // special characters not allowed
|
||||
@ -1073,7 +1072,7 @@ func test12SecretNameFormats(t *testing.T, tempDir, testMnemonic string, runSecr
|
||||
|
||||
// Some of these might not be invalid after all (e.g., leading/trailing slashes might be stripped, .hidden might be allowed)
|
||||
// For now, just check the ones we know should definitely fail
|
||||
definitelyInvalid := []string{"", "UPPERCASE", "with space", "with@symbol", "with#hash", "with$dollar"}
|
||||
definitelyInvalid := []string{"", "with space", "with@symbol", "with#hash", "with$dollar"}
|
||||
shouldFail := false
|
||||
for _, invalid := range definitelyInvalid {
|
||||
if invalidName == invalid {
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"log"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
@ -44,7 +45,10 @@ func newAddCmd() *cobra.Command {
|
||||
force, _ := cmd.Flags().GetBool("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
|
||||
secret.Debug("Created CLI instance, calling AddSecret")
|
||||
|
||||
@ -58,7 +62,10 @@ func newAddCmd() *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{
|
||||
Use: "get <secret-name>",
|
||||
Short: "Retrieve a secret from the vault",
|
||||
@ -66,7 +73,10 @@ func newGetCmd() *cobra.Command {
|
||||
ValidArgsFunction: getSecretNamesCompletionFunc(cli.fs, cli.stateDir),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
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)
|
||||
},
|
||||
@ -93,7 +103,10 @@ func newListCmd() *cobra.Command {
|
||||
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)
|
||||
},
|
||||
@ -115,7 +128,10 @@ func newImportCmd() *cobra.Command {
|
||||
sourceFile, _ := cmd.Flags().GetString("source")
|
||||
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)
|
||||
},
|
||||
@ -129,7 +145,10 @@ func newImportCmd() *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{
|
||||
Use: "remove <secret-name>",
|
||||
Aliases: []string{"rm"},
|
||||
@ -139,7 +158,10 @@ func newRemoveCmd() *cobra.Command {
|
||||
Args: cobra.ExactArgs(1),
|
||||
ValidArgsFunction: getSecretNamesCompletionFunc(cli.fs, cli.stateDir),
|
||||
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)
|
||||
},
|
||||
@ -149,7 +171,10 @@ func newRemoveCmd() *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{
|
||||
Use: "move <source> <destination>",
|
||||
Aliases: []string{"mv", "rename"},
|
||||
@ -172,7 +197,10 @@ The source secret is deleted after successful copy.`,
|
||||
},
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
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)
|
||||
},
|
||||
|
||||
@ -113,7 +113,10 @@ func TestAddSecretVariousSizes(t *testing.T) {
|
||||
cmd.SetIn(stdin)
|
||||
|
||||
// Create CLI instance
|
||||
cli := NewCLIInstance()
|
||||
cli, err := NewCLIInstance()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to initialize CLI: %v", err)
|
||||
}
|
||||
cli.fs = fs
|
||||
cli.stateDir = stateDir
|
||||
cli.cmd = cmd
|
||||
@ -230,7 +233,10 @@ func TestImportSecretVariousSizes(t *testing.T) {
|
||||
cmd := &cobra.Command{}
|
||||
|
||||
// Create CLI instance
|
||||
cli := NewCLIInstance()
|
||||
cli, err := NewCLIInstance()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to initialize CLI: %v", err)
|
||||
}
|
||||
cli.fs = fs
|
||||
cli.stateDir = stateDir
|
||||
|
||||
@ -318,7 +324,10 @@ func TestAddSecretBufferGrowth(t *testing.T) {
|
||||
cmd.SetIn(stdin)
|
||||
|
||||
// Create CLI instance
|
||||
cli := NewCLIInstance()
|
||||
cli, err := NewCLIInstance()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to initialize CLI: %v", err)
|
||||
}
|
||||
cli.fs = fs
|
||||
cli.stateDir = stateDir
|
||||
cli.cmd = cmd
|
||||
@ -377,7 +386,10 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
|
||||
cmd.SetIn(slowReader)
|
||||
|
||||
// Create CLI instance
|
||||
cli := NewCLIInstance()
|
||||
cli, err := NewCLIInstance()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to initialize CLI: %v", err)
|
||||
}
|
||||
cli.fs = fs
|
||||
cli.stateDir = stateDir
|
||||
cli.cmd = cmd
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"log"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
@ -96,7 +97,10 @@ func newUnlockerListCmd() *cobra.Command {
|
||||
RunE: func(cmd *cobra.Command, _ []string) error {
|
||||
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
|
||||
|
||||
return cli.UnlockersList(jsonOutput)
|
||||
@ -153,7 +157,10 @@ to access the same vault. This provides flexibility and backup access options.`,
|
||||
Args: cobra.ExactArgs(1),
|
||||
ValidArgs: strings.Split(supportedTypes, ", "),
|
||||
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]
|
||||
|
||||
// Validate unlocker type
|
||||
@ -186,7 +193,10 @@ to access the same vault. This provides flexibility and backup access options.`,
|
||||
}
|
||||
|
||||
func newUnlockerRemoveCmd() *cobra.Command {
|
||||
cli := NewCLIInstance()
|
||||
cli, err := NewCLIInstance()
|
||||
if err != nil {
|
||||
log.Fatalf("failed to initialize CLI: %v", err)
|
||||
}
|
||||
cmd := &cobra.Command{
|
||||
Use: "remove <unlocker-id>",
|
||||
Aliases: []string{"rm"},
|
||||
@ -198,7 +208,10 @@ func newUnlockerRemoveCmd() *cobra.Command {
|
||||
ValidArgsFunction: getUnlockerIDsCompletionFunc(cli.fs, cli.stateDir),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
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)
|
||||
},
|
||||
@ -210,7 +223,10 @@ func newUnlockerRemoveCmd() *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{
|
||||
Use: "select <unlocker-id>",
|
||||
@ -218,7 +234,10 @@ func newUnlockerSelectCmd() *cobra.Command {
|
||||
Args: cobra.ExactArgs(1),
|
||||
ValidArgsFunction: getUnlockerIDsCompletionFunc(cli.fs, cli.stateDir),
|
||||
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])
|
||||
},
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"log"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
@ -41,7 +42,10 @@ func newVaultListCmd() *cobra.Command {
|
||||
RunE: func(cmd *cobra.Command, _ []string) error {
|
||||
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)
|
||||
},
|
||||
@ -58,7 +62,10 @@ func newVaultCreateCmd() *cobra.Command {
|
||||
Short: "Create a new vault",
|
||||
Args: cobra.ExactArgs(1),
|
||||
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])
|
||||
},
|
||||
@ -66,7 +73,10 @@ func newVaultCreateCmd() *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{
|
||||
Use: "select <name>",
|
||||
@ -74,7 +84,10 @@ func newVaultSelectCmd() *cobra.Command {
|
||||
Args: cobra.ExactArgs(1),
|
||||
ValidArgsFunction: getVaultNamesCompletionFunc(cli.fs, cli.stateDir),
|
||||
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])
|
||||
},
|
||||
@ -82,7 +95,10 @@ func newVaultSelectCmd() *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{
|
||||
Use: "import <vault-name>",
|
||||
@ -96,7 +112,10 @@ func newVaultImportCmd() *cobra.Command {
|
||||
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)
|
||||
},
|
||||
@ -104,7 +123,10 @@ func newVaultImportCmd() *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{
|
||||
Use: "remove <name>",
|
||||
Aliases: []string{"rm"},
|
||||
@ -115,7 +137,10 @@ func newVaultRemoveCmd() *cobra.Command {
|
||||
ValidArgsFunction: getVaultNamesCompletionFunc(cli.fs, cli.stateDir),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
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)
|
||||
},
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"log"
|
||||
"fmt"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
@ -18,7 +19,10 @@ const (
|
||||
|
||||
// newVersionCmd returns the version management command
|
||||
func newVersionCmd() *cobra.Command {
|
||||
cli := NewCLIInstance()
|
||||
cli, err := NewCLIInstance()
|
||||
if err != nil {
|
||||
log.Fatalf("failed to initialize CLI: %v", err)
|
||||
}
|
||||
|
||||
return VersionCommands(cli)
|
||||
}
|
||||
|
||||
@ -266,7 +266,10 @@ func TestGetSecretWithVersion(t *testing.T) {
|
||||
|
||||
func TestVersionCommandStructure(t *testing.T) {
|
||||
// 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)
|
||||
|
||||
assert.Equal(t, "version", cmd.Use)
|
||||
|
||||
@ -28,16 +28,17 @@ func generateRandomString(length int, charset string) (string, error) {
|
||||
return string(result), nil
|
||||
}
|
||||
|
||||
// DetermineStateDir determines the state directory based on environment variables and OS
|
||||
func DetermineStateDir(customConfigDir string) string {
|
||||
// DetermineStateDir determines the state directory based on environment variables and OS.
|
||||
// It returns an error if no usable directory can be determined.
|
||||
func DetermineStateDir(customConfigDir string) (string, error) {
|
||||
// Check for environment variable first
|
||||
if envStateDir := os.Getenv(EnvStateDir); envStateDir != "" {
|
||||
return envStateDir
|
||||
return envStateDir, nil
|
||||
}
|
||||
|
||||
// Use custom config dir if provided
|
||||
if customConfigDir != "" {
|
||||
return filepath.Join(customConfigDir, AppID)
|
||||
return filepath.Join(customConfigDir, AppID), nil
|
||||
}
|
||||
|
||||
// Use os.UserConfigDir() which handles platform-specific directories:
|
||||
@ -47,10 +48,13 @@ func DetermineStateDir(customConfigDir string) string {
|
||||
configDir, err := os.UserConfigDir()
|
||||
if err != nil {
|
||||
// Fallback to a reasonable default if we can't determine user config dir
|
||||
homeDir, _ := os.UserHomeDir()
|
||||
homeDir, homeErr := os.UserHomeDir()
|
||||
if homeErr != nil {
|
||||
return "", fmt.Errorf("unable to determine state directory: config dir: %w, home dir: %w", err, homeErr)
|
||||
}
|
||||
|
||||
return filepath.Join(homeDir, ".config", AppID)
|
||||
return filepath.Join(homeDir, ".config", AppID), nil
|
||||
}
|
||||
|
||||
return filepath.Join(configDir, AppID)
|
||||
return filepath.Join(configDir, AppID), nil
|
||||
}
|
||||
|
||||
50
internal/secret/helpers_test.go
Normal file
50
internal/secret/helpers_test.go
Normal file
@ -0,0 +1,50 @@
|
||||
package secret
|
||||
|
||||
import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestDetermineStateDir_ErrorsWhenHomeDirUnavailable(t *testing.T) {
|
||||
// Clear all env vars that could provide a home/config directory.
|
||||
// On Darwin, os.UserHomeDir may still succeed via the password
|
||||
// database, so we also test via an explicit empty-customConfigDir
|
||||
// path to exercise the fallback branch.
|
||||
t.Setenv(EnvStateDir, "")
|
||||
t.Setenv("HOME", "")
|
||||
t.Setenv("XDG_CONFIG_HOME", "")
|
||||
|
||||
result, err := DetermineStateDir("")
|
||||
// On systems where both lookups fail, we must get an error.
|
||||
// On systems where the OS provides a fallback (e.g. macOS pw db),
|
||||
// result should still be valid (non-empty, not root-relative).
|
||||
if err != nil {
|
||||
// Good — the error case is handled.
|
||||
return
|
||||
}
|
||||
if result == "/.config/"+AppID || result == "" {
|
||||
t.Errorf("DetermineStateDir returned dangerous/empty path %q without error", result)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDetermineStateDir_UsesEnvVar(t *testing.T) {
|
||||
t.Setenv(EnvStateDir, "/custom/state")
|
||||
result, err := DetermineStateDir("")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if result != "/custom/state" {
|
||||
t.Errorf("expected /custom/state, got %q", result)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDetermineStateDir_UsesCustomConfigDir(t *testing.T) {
|
||||
t.Setenv(EnvStateDir, "")
|
||||
result, err := DetermineStateDir("/my/config")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
expected := "/my/config/" + AppID
|
||||
if result != expected {
|
||||
t.Errorf("expected %q, got %q", expected, result)
|
||||
}
|
||||
}
|
||||
@ -257,9 +257,10 @@ func isValidSecretName(name string) bool {
|
||||
if name == "" {
|
||||
return false
|
||||
}
|
||||
// Valid characters for secret names: lowercase letters, numbers, dash, dot, underscore, slash
|
||||
// Valid characters for secret names: letters, numbers, dash, dot, underscore, slash
|
||||
for _, char := range name {
|
||||
if (char < 'a' || char > 'z') && // lowercase letters
|
||||
(char < 'A' || char > 'Z') && // uppercase letters
|
||||
(char < '0' || char > '9') && // numbers
|
||||
char != '-' && // dash
|
||||
char != '.' && // dot
|
||||
@ -283,7 +284,9 @@ func TestSecretNameValidation(t *testing.T) {
|
||||
{"valid/path/name", true},
|
||||
{"123valid", true},
|
||||
{"", false},
|
||||
{"Invalid-Name", false}, // uppercase not allowed
|
||||
{"Valid-Upper-Name", true}, // uppercase allowed
|
||||
{"2025-11-21-ber1app1-vaultik-test-bucket-AKI", true}, // real-world uppercase key ID
|
||||
{"MixedCase/Path/Name", true}, // mixed case with path
|
||||
{"invalid name", false}, // space not allowed
|
||||
{"invalid@name", false}, // @ not allowed
|
||||
}
|
||||
|
||||
96
internal/vault/path_traversal_test.go
Normal file
96
internal/vault/path_traversal_test.go
Normal file
@ -0,0 +1,96 @@
|
||||
package vault
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"git.eeqj.de/sneak/secret/internal/secret"
|
||||
"github.com/awnumar/memguard"
|
||||
"github.com/spf13/afero"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestGetSecretVersionRejectsPathTraversal verifies that GetSecretVersion
|
||||
// validates the secret name and rejects path traversal attempts.
|
||||
// This is a regression test for https://git.eeqj.de/sneak/secret/issues/13
|
||||
func TestGetSecretVersionRejectsPathTraversal(t *testing.T) {
|
||||
testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
|
||||
t.Setenv(secret.EnvMnemonic, testMnemonic)
|
||||
t.Setenv(secret.EnvUnlockPassphrase, "test-passphrase")
|
||||
|
||||
fs := afero.NewMemMapFs()
|
||||
stateDir := "/test/state"
|
||||
|
||||
vlt, err := CreateVault(fs, stateDir, "test-vault")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Add a legitimate secret so the vault is set up
|
||||
value := memguard.NewBufferFromBytes([]byte("legitimate-secret"))
|
||||
err = vlt.AddSecret("legit", value, false)
|
||||
require.NoError(t, err)
|
||||
|
||||
// These names contain path traversal and should be rejected
|
||||
maliciousNames := []string{
|
||||
"../../../etc/passwd",
|
||||
"..%2f..%2fetc/passwd",
|
||||
".secret",
|
||||
"../sibling-vault/secrets.d/target",
|
||||
"foo/../bar",
|
||||
"a/../../etc/passwd",
|
||||
}
|
||||
|
||||
for _, name := range maliciousNames {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
_, err := vlt.GetSecretVersion(name, "")
|
||||
assert.Error(t, err, "GetSecretVersion should reject malicious name: %s", name)
|
||||
assert.Contains(t, err.Error(), "invalid secret name",
|
||||
"error should indicate invalid name for: %s", name)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetSecretRejectsPathTraversal verifies GetSecret (which calls GetSecretVersion)
|
||||
// also rejects path traversal names.
|
||||
func TestGetSecretRejectsPathTraversal(t *testing.T) {
|
||||
testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
|
||||
t.Setenv(secret.EnvMnemonic, testMnemonic)
|
||||
t.Setenv(secret.EnvUnlockPassphrase, "test-passphrase")
|
||||
|
||||
fs := afero.NewMemMapFs()
|
||||
stateDir := "/test/state"
|
||||
|
||||
vlt, err := CreateVault(fs, stateDir, "test-vault")
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = vlt.GetSecret("../../../etc/passwd")
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "invalid secret name")
|
||||
}
|
||||
|
||||
// TestGetSecretObjectRejectsPathTraversal verifies GetSecretObject
|
||||
// also validates names and rejects path traversal attempts.
|
||||
func TestGetSecretObjectRejectsPathTraversal(t *testing.T) {
|
||||
testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
|
||||
t.Setenv(secret.EnvMnemonic, testMnemonic)
|
||||
t.Setenv(secret.EnvUnlockPassphrase, "test-passphrase")
|
||||
|
||||
fs := afero.NewMemMapFs()
|
||||
stateDir := "/test/state"
|
||||
|
||||
vlt, err := CreateVault(fs, stateDir, "test-vault")
|
||||
require.NoError(t, err)
|
||||
|
||||
maliciousNames := []string{
|
||||
"../../../etc/passwd",
|
||||
"foo/../bar",
|
||||
"a/../../etc/passwd",
|
||||
}
|
||||
|
||||
for _, name := range maliciousNames {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
_, err := vlt.GetSecretObject(name)
|
||||
assert.Error(t, err, "GetSecretObject should reject: %s", name)
|
||||
assert.Contains(t, err.Error(), "invalid secret name")
|
||||
})
|
||||
}
|
||||
}
|
||||
@ -67,7 +67,7 @@ func (v *Vault) ListSecrets() ([]string, error) {
|
||||
return secrets, nil
|
||||
}
|
||||
|
||||
// isValidSecretName validates secret names according to the format [a-z0-9\.\-\_\/]+
|
||||
// isValidSecretName validates secret names according to the format [a-zA-Z0-9\.\-\_\/]+
|
||||
// but with additional restrictions:
|
||||
// - No leading or trailing slashes
|
||||
// - No double slashes
|
||||
@ -92,8 +92,15 @@ func isValidSecretName(name string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// Check for path traversal via ".." components
|
||||
for _, part := range strings.Split(name, "/") {
|
||||
if part == ".." {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// Check the basic pattern
|
||||
matched, _ := regexp.MatchString(`^[a-z0-9\.\-\_\/]+$`, name)
|
||||
matched, _ := regexp.MatchString(`^[a-zA-Z0-9\.\-\_\/]+$`, name)
|
||||
|
||||
return matched
|
||||
}
|
||||
@ -319,6 +326,13 @@ func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) {
|
||||
slog.String("version", version),
|
||||
)
|
||||
|
||||
// Validate secret name to prevent path traversal
|
||||
if !isValidSecretName(name) {
|
||||
secret.Debug("Invalid secret name provided", "secret_name", name)
|
||||
|
||||
return nil, fmt.Errorf("invalid secret name '%s': must match pattern [a-z0-9.\\-_/]+", name)
|
||||
}
|
||||
|
||||
// Get vault directory
|
||||
vaultDir, err := v.GetDirectory()
|
||||
if err != nil {
|
||||
@ -454,6 +468,10 @@ func (v *Vault) UnlockVault() (*age.X25519Identity, error) {
|
||||
|
||||
// GetSecretObject retrieves a Secret object with metadata loaded from this vault
|
||||
func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
|
||||
if !isValidSecretName(name) {
|
||||
return nil, fmt.Errorf("invalid secret name: %s", name)
|
||||
}
|
||||
|
||||
// First check if the secret exists by checking for the metadata file
|
||||
vaultDir, err := v.GetDirectory()
|
||||
if err != nil {
|
||||
|
||||
42
internal/vault/secrets_name_test.go
Normal file
42
internal/vault/secrets_name_test.go
Normal file
@ -0,0 +1,42 @@
|
||||
package vault
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestIsValidSecretNameUppercase(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
valid bool
|
||||
}{
|
||||
// Lowercase (existing behavior)
|
||||
{"valid-name", true},
|
||||
{"valid.name", true},
|
||||
{"valid_name", true},
|
||||
{"valid/path/name", true},
|
||||
{"123valid", true},
|
||||
|
||||
// Uppercase (new behavior - issue #2)
|
||||
{"Valid-Upper-Name", true},
|
||||
{"2025-11-21-ber1app1-vaultik-test-bucket-AKI", true},
|
||||
{"MixedCase/Path/Name", true},
|
||||
{"ALLUPPERCASE", true},
|
||||
{"ABC123", true},
|
||||
|
||||
// Still invalid
|
||||
{"", false},
|
||||
{"invalid name", false},
|
||||
{"invalid@name", false},
|
||||
{".dotstart", false},
|
||||
{"/leading-slash", false},
|
||||
{"trailing-slash/", false},
|
||||
{"double//slash", false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := isValidSecretName(tt.name)
|
||||
if result != tt.valid {
|
||||
t.Errorf("isValidSecretName(%q) = %v, want %v", tt.name, result, tt.valid)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user