From 3fd30bb9e6cc76f03d2815db46fe0065a490d071 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 15 Feb 2026 14:03:28 -0800 Subject: [PATCH 1/4] 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 { From 8eb25b98fd3f213c3539856414f885d76a9d03b1 Mon Sep 17 00:00:00 2001 From: user Date: Sun, 15 Feb 2026 14:17:33 -0800 Subject: [PATCH 2/4] fix: block .. path components in secret names and validate in GetSecretObject - isValidSecretName() now rejects names with '..' path components (e.g. foo/../bar) - GetSecretObject() now calls isValidSecretName() before building paths - Added test cases for mid-path traversal patterns --- internal/vault/path_traversal_test.go | 30 +++++++++++++++++++++++++++ internal/vault/secrets.go | 11 ++++++++++ 2 files changed, 41 insertions(+) diff --git a/internal/vault/path_traversal_test.go b/internal/vault/path_traversal_test.go index 499ba31..f244fe1 100644 --- a/internal/vault/path_traversal_test.go +++ b/internal/vault/path_traversal_test.go @@ -35,6 +35,8 @@ func TestGetSecretVersionRejectsPathTraversal(t *testing.T) { "..%2f..%2fetc/passwd", ".secret", "../sibling-vault/secrets.d/target", + "foo/../bar", + "a/../../etc/passwd", } for _, name := range maliciousNames { @@ -64,3 +66,31 @@ func TestGetSecretRejectsPathTraversal(t *testing.T) { 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") + }) + } +} diff --git a/internal/vault/secrets.go b/internal/vault/secrets.go index a7b3387..47b5b76 100644 --- a/internal/vault/secrets.go +++ b/internal/vault/secrets.go @@ -92,6 +92,13 @@ 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) @@ -460,6 +467,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 { From 6acd57d0ec31dce6fc51e02e6b28c177b05d868c Mon Sep 17 00:00:00 2001 From: clawbot Date: Thu, 19 Feb 2026 23:43:32 -0800 Subject: [PATCH 3/4] fix: suppress gosec G204 for validated GPG key ID inputs --- internal/secret/pgpunlocker.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/secret/pgpunlocker.go b/internal/secret/pgpunlocker.go index 1fedb17..bca1546 100644 --- a/internal/secret/pgpunlocker.go +++ b/internal/secret/pgpunlocker.go @@ -320,7 +320,9 @@ func ResolveGPGKeyFingerprint(keyID string) (string, error) { } // Use GPG to get the full fingerprint for the key - cmd := exec.Command("gpg", "--list-keys", "--with-colons", "--fingerprint", keyID) + cmd := exec.Command( // #nosec G204 -- keyID validated + "gpg", "--list-keys", "--with-colons", "--fingerprint", keyID, + ) output, err := cmd.Output() if err != nil { return "", fmt.Errorf("failed to resolve GPG key fingerprint: %w", err) @@ -359,7 +361,9 @@ func gpgEncryptDefault(data *memguard.LockedBuffer, keyID string) ([]byte, error return nil, fmt.Errorf("invalid GPG key ID: %w", err) } - cmd := exec.Command("gpg", "--trust-model", "always", "--armor", "--encrypt", "-r", keyID) + cmd := exec.Command( // #nosec G204 -- keyID validated + "gpg", "--trust-model", "always", "--armor", "--encrypt", "-r", keyID, + ) cmd.Stdin = strings.NewReader(data.String()) output, err := cmd.Output() From dc225bd0b1248587540e47f4505a20dbf9fede80 Mon Sep 17 00:00:00 2001 From: clawbot Date: Thu, 19 Feb 2026 23:44:38 -0800 Subject: [PATCH 4/4] fix: add blank line before return for nlreturn linter --- internal/vault/secrets.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/vault/secrets.go b/internal/vault/secrets.go index 47b5b76..1655982 100644 --- a/internal/vault/secrets.go +++ b/internal/vault/secrets.go @@ -329,6 +329,7 @@ func (v *Vault) GetSecretVersion(name string, version string) ([]byte, error) { // 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) }