Compare commits
1
Commits
next
...
526a6be17b
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
526a6be17b |
@@ -25,6 +25,12 @@ 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), `import`, `version list`/`promote`/`rm`,
|
||||||
|
`encrypt` and `decrypt`. Before, `secret rm ..` deleted the whole
|
||||||
|
vault and `secret rm .` every secret in it.
|
||||||
- 2026-10-02: A plain `docker build .` builds again: the size tests
|
- 2026-10-02: A plain `docker build .` builds again: the size tests
|
||||||
skip a case that needs more locked memory than the process can
|
skip a case that needs more locked memory than the process can
|
||||||
lock, and run every case under `script/cibuild`. The image stamps the
|
lock, and run every case under `script/cibuild`. The image stamps the
|
||||||
@@ -96,8 +102,7 @@ Bring the repo into policy compliance in one commit:
|
|||||||
buffer.Bytes() to GPGEncryptFunc and EncryptWithPassphrase.
|
buffer.Bytes() 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,182 @@
|
|||||||
|
package cli_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"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
|
||||||
|
}
|
||||||
|
|
||||||
|
// requireRejectedAndUnchanged runs a command against the two test vaults
|
||||||
|
// and requires that it fails with vault.ErrInvalidSecretName and leaves
|
||||||
|
// everything under the state directory as it was. The error alone proves
|
||||||
|
// nothing: it could be returned after the vault had already been deleted.
|
||||||
|
func requireRejectedAndUnchanged(t *testing.T, run func(c *cli.Instance) error) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
fs := newTwoVaultFs(t)
|
||||||
|
before := snapshotStateDir(t, fs)
|
||||||
|
|
||||||
|
vaultDir := testStateDir + "/vaults.d/default"
|
||||||
|
require.Contains(t, before, vaultDir+"/secrets.d/x/")
|
||||||
|
require.Contains(t, before, vaultDir+"/unlockers.d/passphrase/")
|
||||||
|
|
||||||
|
err := run(cli.NewCLIInstanceWithStateDir(fs, testStateDir))
|
||||||
|
|
||||||
|
require.Equal(t, before, snapshotStateDir(t, fs))
|
||||||
|
require.ErrorIs(t, err, vault.ErrInvalidSecretName)
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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 // subtests use t.Setenv via newTwoVaultFs
|
||||||
|
func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
|
||||||
|
cmd := &cobra.Command{}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
command string
|
||||||
|
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", 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 default:.. work", func(c *cli.Instance) error {
|
||||||
|
return c.MoveSecret(cmd, "default:..", "work", 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", 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, tt.run)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -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 {
|
||||||
@@ -757,6 +767,16 @@ func (cli *Instance) MoveSecret(
|
|||||||
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 {
|
||||||
|
err := vault.ValidateSecretName(source)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
err = vault.ValidateSecretName(dest)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
@@ -815,6 +835,16 @@ func (cli *Instance) moveSecretCrossVault(
|
|||||||
destVaultName, destSecretName string,
|
destVaultName, destSecretName string,
|
||||||
force bool,
|
force bool,
|
||||||
) error {
|
) error {
|
||||||
|
err := vault.ValidateSecretName(srcSecretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
err = vault.ValidateSecretName(destSecretName)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// Get source vault
|
// Get source vault
|
||||||
srcVault := vault.NewVault(cli.fs, cli.stateDir, srcVaultName)
|
srcVault := vault.NewVault(cli.fs, cli.stateDir, srcVaultName)
|
||||||
|
|
||||||
|
|||||||
@@ -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 {
|
||||||
|
|||||||
+20
-10
@@ -110,6 +110,20 @@ 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': must match pattern [a-z0-9.\\-_/]+",
|
||||||
|
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 +138,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)
|
||||||
@@ -628,13 +640,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