From 3fd30bb9e6cc76f03d2815db46fe0065a490d071 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 15 Feb 2026 14:03:28 -0800 Subject: [PATCH] Validate secret name in GetSecretVersion to prevent path traversal Add isValidSecretName() check at the top of GetSecretVersion(), matching the existing validation in AddSecret(). Without this, crafted secret names containing path traversal sequences (e.g. '../../../etc/passwd') could be used to read files outside the vault directory. Add regression tests for both GetSecretVersion and GetSecret. Closes #13 --- internal/vault/path_traversal_test.go | 66 +++++++++++++++++++++++++++ internal/vault/secrets.go | 6 +++ 2 files changed, 72 insertions(+) create mode 100644 internal/vault/path_traversal_test.go diff --git a/internal/vault/path_traversal_test.go b/internal/vault/path_traversal_test.go new file mode 100644 index 0000000..499ba31 --- /dev/null +++ b/internal/vault/path_traversal_test.go @@ -0,0 +1,66 @@ +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", + } + + 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") +} diff --git a/internal/vault/secrets.go b/internal/vault/secrets.go index 3452e0d..a7b3387 100644 --- a/internal/vault/secrets.go +++ b/internal/vault/secrets.go @@ -319,6 +319,12 @@ 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 {