Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2650577d26 |
@@ -113,7 +113,9 @@ automatically switch to another vault if removing the current one.
|
|||||||
Adds a secret to the current vault. Reads the secret value from stdin.
|
Adds a secret to the current vault. Reads the secret value from stdin.
|
||||||
- `--force, -f`: Overwrite existing secret
|
- `--force, -f`: Overwrite existing secret
|
||||||
|
|
||||||
**Secret Name Format:** `[a-z0-9\.\-\_\/]+`
|
**Secret Name Format:** only ASCII letters, digits, `.`, `-`, `_` and `/`
|
||||||
|
are allowed, and a name must not be empty, start with `.` or `/`, end with
|
||||||
|
`/`, contain `//`, or have `..` as a path segment.
|
||||||
- Forward slashes (`/`) are converted to percent signs (`%`) for storage
|
- Forward slashes (`/`) are converted to percent signs (`%`) for storage
|
||||||
- Examples: `database/password`, `api.key`, `ssh_private_key`
|
- Examples: `database/password`, `api.key`, `ssh_private_key`
|
||||||
|
|
||||||
|
|||||||
@@ -25,6 +25,13 @@ Bring the repo into policy compliance in one commit:
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-03: Every command that builds a path from a secret name
|
||||||
|
checks the name first with `vault.ValidateSecretName` and touches
|
||||||
|
nothing when it is invalid: `rm`, `mv` (both names, within a vault
|
||||||
|
and between vaults, before switching the current vault), `import`,
|
||||||
|
`version list`/`promote`/`rm`, `encrypt` and `decrypt`. The error
|
||||||
|
and `README.md` state the naming rule. Before, `secret rm ..`
|
||||||
|
deleted the whole vault and `secret rm .` every secret in it.
|
||||||
- 2026-10-03: The keychain unlocker's age key passphrase stays in
|
- 2026-10-03: The keychain unlocker's age key passphrase stays in
|
||||||
locked memory: it is generated into a locked buffer, and the
|
locked memory: it is generated into a locked buffer, and the
|
||||||
keychain JSON is written and read by `KeychainData` code in
|
keychain JSON is written and read by `KeychainData` code in
|
||||||
@@ -100,8 +107,7 @@ Bring the repo into policy compliance in one commit:
|
|||||||
to GPGEncryptFunc and EncryptWithPassphrase.
|
to GPGEncryptFunc and EncryptWithPassphrase.
|
||||||
- Race conditions: no file locking in vault/secrets.go:142-176;
|
- Race conditions: no file locking in vault/secrets.go:142-176;
|
||||||
non-atomic writes can leave the vault inconsistent.
|
non-atomic writes can leave the vault inconsistent.
|
||||||
- Input validation: dots in secret names risk path traversal
|
- Input validation: no maximum secret size (DoS).
|
||||||
(vault/secrets.go:75-99); no maximum secret size (DoS).
|
|
||||||
- Timing attacks: bytes.Equal passphrase compare (cli/init.go:
|
- Timing attacks: bytes.Equal passphrase compare (cli/init.go:
|
||||||
209-216); non-constant-time public key compare (vault.go:95-100).
|
209-216); non-constant-time public key compare (vault.go:95-100).
|
||||||
- High priority:
|
- High priority:
|
||||||
|
|||||||
@@ -122,6 +122,11 @@ func (cli *Instance) resolveEncryptionKey(
|
|||||||
|
|
||||||
// Encrypt encrypts data using an age secret key stored in a secret
|
// Encrypt encrypts data using an age secret key stored in a secret
|
||||||
func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
|
func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
|
||||||
|
err := vault.ValidateSecretName(secretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// Get current vault
|
// Get current vault
|
||||||
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -191,6 +196,11 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
|
|||||||
|
|
||||||
// Decrypt decrypts data using an age secret key stored in a secret
|
// Decrypt decrypts data using an age secret key stored in a secret
|
||||||
func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
|
func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
|
||||||
|
err := vault.ValidateSecretName(secretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// Get current vault
|
// Get current vault
|
||||||
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -0,0 +1,267 @@
|
|||||||
|
package cli_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"maps"
|
||||||
|
"os"
|
||||||
|
"slices"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"git.eeqj.de/sneak/secret/internal/cli"
|
||||||
|
"git.eeqj.de/sneak/secret/internal/secret"
|
||||||
|
"git.eeqj.de/sneak/secret/internal/vault"
|
||||||
|
"github.com/awnumar/memguard"
|
||||||
|
"github.com/spf13/afero"
|
||||||
|
"github.com/spf13/cobra"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
// testStateDir is the in-memory state directory of the test vaults.
|
||||||
|
testStateDir = "/test/state"
|
||||||
|
|
||||||
|
// testPassphrase protects the passphrase unlocker of each test vault.
|
||||||
|
testPassphrase = "test-passphrase"
|
||||||
|
|
||||||
|
// testVersion is a version name in the format the vault uses.
|
||||||
|
testVersion = "20260101.001"
|
||||||
|
|
||||||
|
// missingFile is an import source that does not exist, so an import
|
||||||
|
// that opened it before checking the name would fail with another error.
|
||||||
|
missingFile = "/no/such/file"
|
||||||
|
)
|
||||||
|
|
||||||
|
// newTwoVaultFs returns an in-memory filesystem holding the vaults "work"
|
||||||
|
// and "default", the current one. Each holds the secret "x" and a
|
||||||
|
// passphrase unlocker, so both secrets.d and unlockers.d have contents.
|
||||||
|
//
|
||||||
|
//nolint:ireturn // afero.Fs is the filesystem abstraction used throughout
|
||||||
|
func newTwoVaultFs(t *testing.T) afero.Fs {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
t.Setenv(secret.EnvMnemonic, testMnemonic)
|
||||||
|
|
||||||
|
fs := afero.NewMemMapFs()
|
||||||
|
|
||||||
|
for _, name := range []string{"work", "default"} {
|
||||||
|
vlt, err := vault.CreateVault(fs, testStateDir, name)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
_, err = vlt.CreatePassphraseUnlocker(
|
||||||
|
memguard.NewBufferFromBytes([]byte(testPassphrase)))
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return fs
|
||||||
|
}
|
||||||
|
|
||||||
|
// snapshotStateDir maps every file under the state directory to its
|
||||||
|
// contents, and every directory, written with a trailing "/", to "". Two
|
||||||
|
// snapshots are equal only if nothing in it was added, removed or changed.
|
||||||
|
func snapshotStateDir(t *testing.T, fs afero.Fs) map[string]string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
tree := map[string]string{}
|
||||||
|
|
||||||
|
err := afero.Walk(fs, testStateDir, func(
|
||||||
|
path string, info os.FileInfo, err error,
|
||||||
|
) error {
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if info.IsDir() {
|
||||||
|
tree[path+"/"] = ""
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
content, err := afero.ReadFile(fs, path)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
tree[path] = string(content)
|
||||||
|
|
||||||
|
return nil
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
return tree
|
||||||
|
}
|
||||||
|
|
||||||
|
// newFsFromSnapshot returns a new in-memory filesystem holding exactly the
|
||||||
|
// directories and files recorded by snapshotStateDir.
|
||||||
|
//
|
||||||
|
//nolint:ireturn // afero.Fs is the filesystem abstraction used throughout
|
||||||
|
func newFsFromSnapshot(t *testing.T, tree map[string]string) afero.Fs {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
fs := afero.NewMemMapFs()
|
||||||
|
|
||||||
|
// In sorted order every directory comes before its contents.
|
||||||
|
for _, path := range slices.Sorted(maps.Keys(tree)) {
|
||||||
|
dir, isDir := strings.CutSuffix(path, "/")
|
||||||
|
if isDir {
|
||||||
|
require.NoError(t, fs.MkdirAll(dir, secret.DirPerms))
|
||||||
|
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
err := afero.WriteFile(fs, path, []byte(tree[path]), secret.FilePerms)
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return fs
|
||||||
|
}
|
||||||
|
|
||||||
|
// requireRejectedAndUnchanged runs a command on a copy of the state
|
||||||
|
// directory recorded in before. It requires exactly the error
|
||||||
|
// vault.ValidateSecretName gives for the rejected name, so that a later
|
||||||
|
// check rejecting the name does not count, and everything under the state
|
||||||
|
// directory as it was: the error alone proves nothing, since it could come
|
||||||
|
// after the vault had already been deleted.
|
||||||
|
func requireRejectedAndUnchanged(
|
||||||
|
t *testing.T, before map[string]string, rejected string,
|
||||||
|
run func(c *cli.Instance) error,
|
||||||
|
) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
fs := newFsFromSnapshot(t, before)
|
||||||
|
|
||||||
|
err := run(cli.NewCLIInstanceWithStateDir(fs, testStateDir))
|
||||||
|
|
||||||
|
require.Equal(t, before, snapshotStateDir(t, fs))
|
||||||
|
require.ErrorIs(t, err, vault.ErrInvalidSecretName)
|
||||||
|
require.EqualError(t, err, vault.ValidateSecretName(rejected).Error())
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestInvalidSecretNameLeavesVaultsUnchanged is a regression test for
|
||||||
|
// https://git.eeqj.de/sneak/secret/issues/33, where `secret rm ..` deleted
|
||||||
|
// the whole vault, and `secret rm .` or `secret rm ""` every secret in it.
|
||||||
|
// Moves and imports use --force, so that only the name check stands in
|
||||||
|
// the way.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
|
||||||
|
func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
|
||||||
|
// Creating a passphrase unlocker is slow by design, so the vaults are
|
||||||
|
// created once and each case runs on its own copy of them.
|
||||||
|
before := snapshotStateDir(t, newTwoVaultFs(t))
|
||||||
|
|
||||||
|
vaultDir := testStateDir + "/vaults.d/default"
|
||||||
|
require.Contains(t, before, vaultDir+"/secrets.d/x/")
|
||||||
|
require.Contains(t, before, vaultDir+"/unlockers.d/passphrase/")
|
||||||
|
require.Equal(t, "default", before[testStateDir+"/currentvault"])
|
||||||
|
|
||||||
|
cmd := &cobra.Command{}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
command string
|
||||||
|
rejected string // the secret name the command must reject
|
||||||
|
run func(c *cli.Instance) error
|
||||||
|
}{
|
||||||
|
{"rm ..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.RemoveSecret(cmd, "..", false)
|
||||||
|
}},
|
||||||
|
{"rm .", ".", func(c *cli.Instance) error {
|
||||||
|
return c.RemoveSecret(cmd, ".", false)
|
||||||
|
}},
|
||||||
|
{`rm ""`, "", func(c *cli.Instance) error {
|
||||||
|
return c.RemoveSecret(cmd, "", false)
|
||||||
|
}},
|
||||||
|
{"rm ../../etc", "../../etc", func(c *cli.Instance) error {
|
||||||
|
return c.RemoveSecret(cmd, "../../etc", false)
|
||||||
|
}},
|
||||||
|
{"mv --force .. x", "..", func(c *cli.Instance) error {
|
||||||
|
return c.MoveSecret(cmd, "..", "x", true)
|
||||||
|
}},
|
||||||
|
{"mv --force x ..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.MoveSecret(cmd, "x", "..", true)
|
||||||
|
}},
|
||||||
|
{`mv --force x ""`, "", func(c *cli.Instance) error {
|
||||||
|
return c.MoveSecret(cmd, "x", "", true)
|
||||||
|
}},
|
||||||
|
// "work" is not the current vault: a move within it must not
|
||||||
|
// select it when a name is rejected.
|
||||||
|
{"mv --force work:.. work:x", "..", func(c *cli.Instance) error {
|
||||||
|
return c.MoveSecret(cmd, "work:..", "work:x", true)
|
||||||
|
}},
|
||||||
|
{"mv --force work:x work:..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.MoveSecret(cmd, "work:x", "work:..", true)
|
||||||
|
}},
|
||||||
|
{"mv --force default:.. work", "..", func(c *cli.Instance) error {
|
||||||
|
return c.MoveSecret(cmd, "default:..", "work", true)
|
||||||
|
}},
|
||||||
|
{"mv --force default:.. work:y", "..", func(c *cli.Instance) error {
|
||||||
|
return c.MoveSecret(cmd, "default:..", "work:y", true)
|
||||||
|
}},
|
||||||
|
{"mv --force default:x work:..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.MoveSecret(cmd, "default:x", "work:..", true)
|
||||||
|
}},
|
||||||
|
{"import --force ..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.ImportSecret(cmd, "..", missingFile, true)
|
||||||
|
}},
|
||||||
|
{"import --force .", ".", func(c *cli.Instance) error {
|
||||||
|
return c.ImportSecret(cmd, ".", missingFile, true)
|
||||||
|
}},
|
||||||
|
{"import --force ../../etc", "../../etc", func(c *cli.Instance) error {
|
||||||
|
return c.ImportSecret(cmd, "../../etc", missingFile, true)
|
||||||
|
}},
|
||||||
|
{"version list ..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.ListVersions(cmd, "..")
|
||||||
|
}},
|
||||||
|
{"version promote ..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.PromoteVersion(cmd, "..", testVersion)
|
||||||
|
}},
|
||||||
|
{"version rm ..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.RemoveVersion(cmd, "..", testVersion)
|
||||||
|
}},
|
||||||
|
{"encrypt ..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.Encrypt("..", "", "")
|
||||||
|
}},
|
||||||
|
{"decrypt ..", "..", func(c *cli.Instance) error {
|
||||||
|
return c.Decrypt("..", "", "")
|
||||||
|
}},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.command, func(t *testing.T) {
|
||||||
|
requireRejectedAndUnchanged(t, before, tt.rejected, tt.run)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestMoveToVaultNameRenamesInCurrentVault checks that `secret mv x work`,
|
||||||
|
// where "work" is also the name of a vault, renames the secret "x" to "work"
|
||||||
|
// in the current vault and changes nothing else.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
|
||||||
|
func TestMoveToVaultNameRenamesInCurrentVault(t *testing.T) {
|
||||||
|
before := snapshotStateDir(t, newTwoVaultFs(t))
|
||||||
|
fs := newFsFromSnapshot(t, before)
|
||||||
|
|
||||||
|
c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
|
||||||
|
err := c.MoveSecret(&cobra.Command{}, "x", "work", false)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// Expected: the state as before, with everything under the current
|
||||||
|
// vault's secrets.d/x/ now under secrets.d/work/.
|
||||||
|
oldDir := testStateDir + "/vaults.d/default/secrets.d/x/"
|
||||||
|
newDir := testStateDir + "/vaults.d/default/secrets.d/work/"
|
||||||
|
want := map[string]string{}
|
||||||
|
|
||||||
|
for path, content := range before {
|
||||||
|
rest, found := strings.CutPrefix(path, oldDir)
|
||||||
|
if found {
|
||||||
|
path = newDir + rest
|
||||||
|
}
|
||||||
|
|
||||||
|
want[path] = content
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Contains(t, want, newDir)
|
||||||
|
require.Equal(t, want, snapshotStateDir(t, fs))
|
||||||
|
}
|
||||||
+39
-17
@@ -603,6 +603,11 @@ func printSecretsTable(
|
|||||||
func (cli *Instance) ImportSecret(
|
func (cli *Instance) ImportSecret(
|
||||||
cmd *cobra.Command, secretName, sourceFile string, force bool,
|
cmd *cobra.Command, secretName, sourceFile string, force bool,
|
||||||
) error {
|
) error {
|
||||||
|
err := vault.ValidateSecretName(secretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// Get current vault
|
// Get current vault
|
||||||
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -649,6 +654,11 @@ func (cli *Instance) ImportSecret(
|
|||||||
|
|
||||||
// RemoveSecret removes a secret from the vault
|
// RemoveSecret removes a secret from the vault
|
||||||
func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool) error {
|
func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool) error {
|
||||||
|
err := vault.ValidateSecretName(secretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// Get current vault
|
// Get current vault
|
||||||
currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -702,13 +712,8 @@ func (cli *Instance) MoveSecret(
|
|||||||
srcVaultName, srcSecretName, srcQualified := ParseVaultSecretRef(source)
|
srcVaultName, srcSecretName, srcQualified := ParseVaultSecretRef(source)
|
||||||
destVaultName, destSecretName, destQualified := ParseVaultSecretRef(dest)
|
destVaultName, destSecretName, destQualified := ParseVaultSecretRef(dest)
|
||||||
|
|
||||||
// If neither is qualified, this is a simple within-vault rename
|
|
||||||
if !srcQualified && !destQualified {
|
|
||||||
return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Cross-vault move requires source to be qualified
|
// Cross-vault move requires source to be qualified
|
||||||
if !srcQualified {
|
if !srcQualified && destQualified {
|
||||||
return errCrossVaultSourceUnqualified
|
return errCrossVaultSourceUnqualified
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -716,31 +721,46 @@ func (cli *Instance) MoveSecret(
|
|||||||
// Format: "work:secret default" means move to vault "default"
|
// Format: "work:secret default" means move to vault "default"
|
||||||
// Format: "work:secret default:newname" means move to vault "default"
|
// Format: "work:secret default:newname" means move to vault "default"
|
||||||
// with a new name
|
// with a new name
|
||||||
if !destQualified {
|
if srcQualified && !destQualified {
|
||||||
// Check if dest is actually a vault name
|
// Check if dest is actually a vault name
|
||||||
vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
|
vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
|
||||||
if err == nil && slices.Contains(vaults, dest) {
|
if err == nil && slices.Contains(vaults, dest) {
|
||||||
// dest is a vault name, use source secret name
|
// dest is a vault name, use source secret name
|
||||||
destVaultName = dest
|
destVaultName = dest
|
||||||
destSecretName = srcSecretName
|
destSecretName = srcSecretName
|
||||||
}
|
} else {
|
||||||
|
// dest is a secret name in source vault
|
||||||
// If destVaultName is still empty, dest is a secret name in source vault
|
|
||||||
if destVaultName == "" {
|
|
||||||
destVaultName = srcVaultName
|
destVaultName = srcVaultName
|
||||||
destSecretName = dest
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// If destination secret name is empty, use source secret name
|
// If destination secret name is empty, use source secret name. A plain
|
||||||
if destSecretName == "" {
|
// rename keeps it empty, so that the check below rejects it.
|
||||||
|
if srcQualified && destSecretName == "" {
|
||||||
destSecretName = srcSecretName
|
destSecretName = srcSecretName
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check both names, for every form of the move, before selecting a vault
|
||||||
|
// below, so that a rejected move leaves the current vault as it was.
|
||||||
|
err := vault.ValidateSecretName(srcSecretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
err = vault.ValidateSecretName(destSecretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// If neither is qualified, this is a simple within-vault rename
|
||||||
|
if !srcQualified && !destQualified {
|
||||||
|
return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
|
||||||
|
}
|
||||||
|
|
||||||
// Same vault? Use simple rename if possible (optimization)
|
// Same vault? Use simple rename if possible (optimization)
|
||||||
if srcVaultName == destVaultName {
|
if srcVaultName == destVaultName {
|
||||||
// Select the vault and do a simple move
|
// Select the vault and do a simple move
|
||||||
err := vault.SelectVault(cli.fs, cli.stateDir, srcVaultName)
|
err = vault.SelectVault(cli.fs, cli.stateDir, srcVaultName)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to select vault '%s': %w", srcVaultName, err)
|
return fmt.Errorf("failed to select vault '%s': %w", srcVaultName, err)
|
||||||
}
|
}
|
||||||
@@ -753,7 +773,8 @@ func (cli *Instance) MoveSecret(
|
|||||||
cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force)
|
cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force)
|
||||||
}
|
}
|
||||||
|
|
||||||
// moveSecretWithinVault handles rename within the current vault
|
// moveSecretWithinVault handles rename within the current vault. Its caller,
|
||||||
|
// MoveSecret, has already checked both secret names.
|
||||||
func (cli *Instance) moveSecretWithinVault(
|
func (cli *Instance) moveSecretWithinVault(
|
||||||
cmd *cobra.Command, source, dest string, force bool,
|
cmd *cobra.Command, source, dest string, force bool,
|
||||||
) error {
|
) error {
|
||||||
@@ -808,7 +829,8 @@ func (cli *Instance) moveSecretWithinVault(
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// moveSecretCrossVault handles moving between different vaults
|
// moveSecretCrossVault handles moving between different vaults. Its caller,
|
||||||
|
// MoveSecret, has already checked both secret names.
|
||||||
func (cli *Instance) moveSecretCrossVault(
|
func (cli *Instance) moveSecretCrossVault(
|
||||||
cmd *cobra.Command,
|
cmd *cobra.Command,
|
||||||
srcVaultName, srcSecretName,
|
srcVaultName, srcSecretName,
|
||||||
|
|||||||
@@ -112,6 +112,11 @@ func VersionCommands(cli *Instance) *cobra.Command {
|
|||||||
func (cli *Instance) ListVersions(cmd *cobra.Command, secretName string) error {
|
func (cli *Instance) ListVersions(cmd *cobra.Command, secretName string) error {
|
||||||
secret.Debug("ListVersions called", "secret_name", secretName)
|
secret.Debug("ListVersions called", "secret_name", secretName)
|
||||||
|
|
||||||
|
err := vault.ValidateSecretName(secretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// Get current vault
|
// Get current vault
|
||||||
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -239,6 +244,11 @@ func formatVersionTime(t *time.Time) string {
|
|||||||
func (cli *Instance) PromoteVersion(
|
func (cli *Instance) PromoteVersion(
|
||||||
cmd *cobra.Command, secretName string, version string,
|
cmd *cobra.Command, secretName string, version string,
|
||||||
) error {
|
) error {
|
||||||
|
err := vault.ValidateSecretName(secretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// Get current vault
|
// Get current vault
|
||||||
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -282,6 +292,11 @@ func (cli *Instance) PromoteVersion(
|
|||||||
func (cli *Instance) RemoveVersion(
|
func (cli *Instance) RemoveVersion(
|
||||||
cmd *cobra.Command, secretName string, version string,
|
cmd *cobra.Command, secretName string, version string,
|
||||||
) error {
|
) error {
|
||||||
|
err := vault.ValidateSecretName(secretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// Get current vault
|
// Get current vault
|
||||||
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -310,64 +310,6 @@ func TestPerSecretKeyFunctionality(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
// For testing purposes only
|
|
||||||
func isValidSecretName(name string) bool {
|
|
||||||
if name == "" {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
// 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
|
|
||||||
char != '_' && // underscore
|
|
||||||
char != '/' { // slash
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return true
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestSecretNameValidation(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
tests := []struct {
|
|
||||||
name string
|
|
||||||
valid bool
|
|
||||||
}{
|
|
||||||
{"valid-name", true},
|
|
||||||
{"valid.name", true},
|
|
||||||
{"valid_name", true},
|
|
||||||
{"valid/path/name", true},
|
|
||||||
{"123valid", true},
|
|
||||||
{"", false},
|
|
||||||
{"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
|
|
||||||
}
|
|
||||||
|
|
||||||
for _, test := range tests {
|
|
||||||
t.Run(test.name, func(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
result := isValidSecretName(test.name)
|
|
||||||
if result != test.valid {
|
|
||||||
t.Errorf(
|
|
||||||
"isValidSecretName(%q) = %v, want %v",
|
|
||||||
test.name,
|
|
||||||
result,
|
|
||||||
test.valid,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
})
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestSecretGetValueWithEnvMnemonicUsesVaultDerivationIndex(t *testing.T) {
|
func TestSecretGetValueWithEnvMnemonicUsesVaultDerivationIndex(t *testing.T) {
|
||||||
// This test demonstrates the bug where GetValue uses hardcoded index 0
|
// This test demonstrates the bug where GetValue uses hardcoded index 0
|
||||||
// instead of the vault's actual derivation index when using environment mnemonic
|
// instead of the vault's actual derivation index when using environment mnemonic
|
||||||
|
|||||||
@@ -29,10 +29,11 @@ var (
|
|||||||
// ErrNilValueBuffer indicates a nil value buffer was supplied.
|
// ErrNilValueBuffer indicates a nil value buffer was supplied.
|
||||||
ErrNilValueBuffer = errors.New("value buffer is nil")
|
ErrNilValueBuffer = errors.New("value buffer is nil")
|
||||||
|
|
||||||
// ErrInvalidSecretName indicates a secret name that does not match
|
// ErrInvalidSecretName indicates a secret name that breaks the naming
|
||||||
// the allowed pattern [a-z0-9.\-_/]+. Composed as
|
// rule: only ASCII letters, digits, '.', '-', '_' and '/'; not empty;
|
||||||
// "invalid secret name '<name>': must match pattern [a-z0-9.\-_/]+",
|
// no leading '.' or '/', no trailing '/', no '//', no '..' path segment.
|
||||||
// or as "invalid secret name: <name>" by GetSecretObject.
|
// Composed by ValidateSecretName as
|
||||||
|
// "invalid secret name '<name>': <the rule>".
|
||||||
ErrInvalidSecretName = errors.New("invalid secret name")
|
ErrInvalidSecretName = errors.New("invalid secret name")
|
||||||
|
|
||||||
// ErrSecretExists indicates the secret already exists and --force
|
// ErrSecretExists indicates the secret already exists and --force
|
||||||
|
|||||||
+26
-12
@@ -79,6 +79,7 @@ func (v *Vault) ListSecrets() ([]string, error) {
|
|||||||
// - No leading or trailing slashes
|
// - No leading or trailing slashes
|
||||||
// - No double slashes
|
// - No double slashes
|
||||||
// - No names starting with dots
|
// - No names starting with dots
|
||||||
|
// - No ".." path segments
|
||||||
func isValidSecretName(name string) bool {
|
func isValidSecretName(name string) bool {
|
||||||
if name == "" {
|
if name == "" {
|
||||||
return false
|
return false
|
||||||
@@ -110,6 +111,22 @@ func isValidSecretName(name string) bool {
|
|||||||
return matched
|
return matched
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ValidateSecretName returns an error wrapping ErrInvalidSecretName when
|
||||||
|
// name is not a valid secret name. Call it on the name exactly as the user
|
||||||
|
// gave it, before building any path from it.
|
||||||
|
func ValidateSecretName(name string) error {
|
||||||
|
if !isValidSecretName(name) {
|
||||||
|
return fmt.Errorf(
|
||||||
|
"%w '%s': only ASCII letters, digits, '.', '-', '_' and '/' are allowed, "+
|
||||||
|
"and a name must not be empty, start with '.' or '/', end with '/', "+
|
||||||
|
"contain '//', or have '..' as a path segment",
|
||||||
|
ErrInvalidSecretName, name,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// AddSecret adds a secret to this vault
|
// AddSecret adds a secret to this vault
|
||||||
func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool) error {
|
func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool) error {
|
||||||
if value == nil {
|
if value == nil {
|
||||||
@@ -124,13 +141,11 @@ func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool)
|
|||||||
)
|
)
|
||||||
|
|
||||||
// Validate secret name
|
// Validate secret name
|
||||||
if !isValidSecretName(name) {
|
err := ValidateSecretName(name)
|
||||||
|
if err != nil {
|
||||||
secret.Debug("Invalid secret name provided", "secret_name", name)
|
secret.Debug("Invalid secret name provided", "secret_name", name)
|
||||||
|
|
||||||
return fmt.Errorf(
|
return err
|
||||||
"%w '%s': must match pattern [a-z0-9.\\-_/]+",
|
|
||||||
ErrInvalidSecretName, name,
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
secret.Debug("Secret name validation passed", "secret_name", name)
|
secret.Debug("Secret name validation passed", "secret_name", name)
|
||||||
@@ -358,8 +373,9 @@ func (v *Vault) UnlockVault() (*age.X25519Identity, error) {
|
|||||||
|
|
||||||
// GetSecretObject retrieves a Secret object with metadata loaded from this vault
|
// GetSecretObject retrieves a Secret object with metadata loaded from this vault
|
||||||
func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
|
func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
|
||||||
if !isValidSecretName(name) {
|
err := ValidateSecretName(name)
|
||||||
return nil, fmt.Errorf("%w: %s", ErrInvalidSecretName, name)
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// First check if the secret exists by checking for the metadata file
|
// First check if the secret exists by checking for the metadata file
|
||||||
@@ -628,13 +644,11 @@ func (v *Vault) updatePreviousVersion(
|
|||||||
// version exist, and resolves an empty version to the current one.
|
// version exist, and resolves an empty version to the current one.
|
||||||
func (v *Vault) resolveSecretVersion(name, version string) (string, error) {
|
func (v *Vault) resolveSecretVersion(name, version string) (string, error) {
|
||||||
// Validate secret name to prevent path traversal
|
// Validate secret name to prevent path traversal
|
||||||
if !isValidSecretName(name) {
|
err := ValidateSecretName(name)
|
||||||
|
if err != nil {
|
||||||
secret.Debug("Invalid secret name provided", "secret_name", name)
|
secret.Debug("Invalid secret name provided", "secret_name", name)
|
||||||
|
|
||||||
return "", fmt.Errorf(
|
return "", err
|
||||||
"%w '%s': must match pattern [a-z0-9.\\-_/]+",
|
|
||||||
ErrInvalidSecretName, name,
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get vault directory
|
// Get vault directory
|
||||||
|
|||||||
Reference in New Issue
Block a user