Commands fail when currentvault names no vault (closes #156)
check / check (push) Waiting to run
check / check (push) Waiting to run
An empty currentvault, or one naming a vault that does not exist, made `secret list` report an empty vault with exit 0 and `secret add` fail on a missing directory. ResolveVaultSymlink now refuses an empty or invalid name, or one with no directory under vaults.d, with an error that wraps ErrVaultNotFound, names the file and says to run `secret vault select <name>`. Every command that uses the current vault gets it; `secret vault list` and `secret info` still run. newSizeTestVault wrote a full path into currentvault, which resolved only because the last path element was taken as the name; it now relies on CreateVault selecting the vault. Model: opus-5-5
This commit is contained in:
@@ -118,7 +118,11 @@ are allowed, and a name must not be empty, `.` or `..`.
|
||||
|
||||
#### `secret vault select <name>`
|
||||
|
||||
Switches to the specified vault for subsequent operations.
|
||||
Switches to the specified vault for subsequent operations. When `currentvault`
|
||||
is empty or names a vault that does not exist, every command that uses the
|
||||
current vault fails with an error that names the file and says to run
|
||||
`secret vault select <name>`. `secret vault list` still lists the vaults, and
|
||||
`secret info` still runs; both show no vault as current.
|
||||
|
||||
#### `secret vault remove <name> [--force]` / `secret vault rm` ⚠️ 🛑
|
||||
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
package cli_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
"github.com/spf13/cobra"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/secret/internal/cli"
|
||||
"sneak.berlin/go/secret/internal/secret"
|
||||
"sneak.berlin/go/secret/internal/vault"
|
||||
)
|
||||
|
||||
// TestListFailsWhenCurrentVaultDoesNotExist is a regression test for
|
||||
// https://git.eeqj.de/sneak/secret/issues/156, where a currentvault file
|
||||
// that was empty, held an invalid vault name, or named a vault that does
|
||||
// not exist, made `secret list` report an empty vault. Listing must fail
|
||||
// with an error that names the file and says to run `secret vault select`,
|
||||
// `secret vault list` must still list the vaults, and selecting a vault
|
||||
// must make listing work again. The name "." would make vaults.d itself the
|
||||
// vault, and "../vaults.d/work" would reach the vault work by a path.
|
||||
func TestListFailsWhenCurrentVaultDoesNotExist(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, name := range []string{"", ".", "../vaults.d/work", "gone"} {
|
||||
t.Run(strconv.Quote(name), func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
fs := newTwoVaultFs(t)
|
||||
currentVaultPath := filepath.Join(testStateDir, "currentvault")
|
||||
require.NoError(t, afero.WriteFile(fs, currentVaultPath,
|
||||
[]byte(name), secret.FilePerms))
|
||||
|
||||
c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
|
||||
|
||||
var out bytes.Buffer
|
||||
|
||||
cmd := &cobra.Command{}
|
||||
cmd.SetOut(&out)
|
||||
|
||||
err := c.ListSecrets(cmd, false, true, "")
|
||||
require.ErrorIs(t, err, vault.ErrVaultNotFound)
|
||||
require.ErrorContains(t, err, currentVaultPath)
|
||||
require.ErrorContains(t, err, "secret vault select")
|
||||
assert.Empty(t, out.String())
|
||||
|
||||
require.NoError(t, c.ListVaults(cmd, false))
|
||||
assert.Equal(t, "Available vaults:\n default\n work\n", out.String())
|
||||
|
||||
require.NoError(t, c.SelectVault(cmd, "default"))
|
||||
out.Reset()
|
||||
require.NoError(t, c.ListSecrets(cmd, false, true, ""))
|
||||
assert.Equal(t, "x\n", out.String())
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -36,17 +36,11 @@ func newSizeTestVault(t *testing.T) (afero.Fs, *vault.Vault) {
|
||||
|
||||
fs := afero.NewMemMapFs()
|
||||
|
||||
// Create vault
|
||||
// Create vault, which selects it as the current vault
|
||||
_, err := vault.CreateVault(fs, testStateDir, testVaultName,
|
||||
testMnemonicBuffer(t), nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Set current vault
|
||||
currentVaultPath := filepath.Join(testStateDir, "currentvault")
|
||||
vaultPath := filepath.Join(testStateDir, "vaults.d", testVaultName)
|
||||
err = afero.WriteFile(fs, currentVaultPath, []byte(vaultPath), 0o600)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Get vault and set up long-term key
|
||||
vlt, err := vault.GetCurrentVault(fs, testStateDir)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -55,7 +55,10 @@ func ValidateVaultName(name string) error {
|
||||
}
|
||||
|
||||
// ResolveVaultSymlink reads the currentvault file to get the path to the current vault
|
||||
// The file contains just the vault name (e.g., "default")
|
||||
// The file contains just the vault name (e.g., "default"). An empty or
|
||||
// invalid name, or the name of a vault that does not exist, is an error
|
||||
// wrapping ErrVaultNotFound that names the file and says to run
|
||||
// 'secret vault select <name>'.
|
||||
func ResolveVaultSymlink(fs afero.Fs, currentVaultPath string) (string, error) {
|
||||
secret.Debug("resolveVaultSymlink starting", "path", currentVaultPath)
|
||||
|
||||
@@ -74,6 +77,21 @@ func ResolveVaultSymlink(fs afero.Fs, currentVaultPath string) (string, error) {
|
||||
stateDir := filepath.Dir(currentVaultPath)
|
||||
absolutePath := filepath.Join(stateDir, "vaults.d", vaultName)
|
||||
|
||||
// An empty name would make vaults.d itself the vault's directory
|
||||
exists := false
|
||||
if isValidVaultName(vaultName) {
|
||||
exists, err = afero.DirExists(fs, absolutePath)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("failed to check if vault exists: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
if !exists {
|
||||
return "", fmt.Errorf("vault '%s' named in %s %w; run "+
|
||||
"'secret vault select <name>' to select a vault",
|
||||
vaultName, currentVaultPath, ErrVaultNotFound)
|
||||
}
|
||||
|
||||
secret.Debug("Resolved to absolute path", "absolute_path", absolutePath)
|
||||
|
||||
return absolutePath, nil
|
||||
|
||||
Reference in New Issue
Block a user