diff --git a/README.md b/README.md
index 8e228c9..7474911 100644
--- a/README.md
+++ b/README.md
@@ -15,6 +15,23 @@ A flexible Go library for secure secret management with multiple backend provide
 - **Thread Safe**: Concurrent access protection with read/write mutexes
 - **Comprehensive API**: Full CRUD operations plus metadata and existence checks
 
+## Upgrading to v0.3.0
+
+v0.3.0 is a security and correctness release. It contains breaking changes, each
+of which replaces behaviour that failed silently:
+
+| Change | Why | What to do |
+|---|---|---|
+| `Provider.Metadata()` returns `(Metadata, error)` | Every failure previously returned an empty struct, so a broken command, a timeout and "not configured" were indistinguishable | Handle the new error |
+| External configs referencing `{{value}}`/`{{password}}` in a `cmd` are rejected | The value was interpolated into a shell command with no quoting — a command-injection sink that also corrupted ordinary passwords | Move the secret to an `InputTemplate` (stdin) |
+| An existing but zero-length vault file is an error | It was read as "no vault here", so the constructor initialized and immediately overwrote it, destroying every secret | Restore from backup, or delete the file to start fresh |
+| Operations on a closed vault return `ErrVaultClosed` | They dereferenced nil state and panicked | Nothing, unless you relied on the panic |
+| Vault IDs are charset-validated | An ID is interpolated into a filename, and `filepath.Clean` *resolves* traversal rather than sanitizing it | Use IDs matching `^[a-zA-Z0-9][a-zA-Z0-9-_.]*$` |
+| Encryption keys must be exactly 32 bytes | `aes.NewCipher` also accepts 16 and 24, silently downgrading an "AES256" vault to AES-128/192 | Regenerate short keys |
+| `DeriveKey` returns a parameter-tagged salt | Changing the scrypt cost would otherwise silently change every derived key | Pass the returned salt back verbatim rather than base64-decoding it first |
+
+Local vault files written by earlier versions are read without migration.
+
 ## Quick Start
 
 ```go
@@ -63,7 +80,7 @@ Stores secrets in an AES-256 encrypted file with configurable key sources.
 ```go
 provider, _, err := vault.New("my-vault",
     vault.WithProvider(vault.ProviderTypeAES256),
-    vault.WithAESPath("~/secrets.vault"),
+    vault.WithAESPath("~/.config/flow/vaults"), // a directory, not a file
 )
 ```
 
@@ -79,7 +96,7 @@ Uses the [age encryption tool](https://age-encryption.org/) with public key cryp
 ```go
 provider, _, err := vault.New("my-vault", 
     vault.WithProvider(vault.ProviderTypeAge),
-    vault.WithAgePath("~/secrets.age"),
+    vault.WithAgePath("~/.config/flow/vaults"), // a directory, not a file
 )
 ```
 
@@ -107,7 +124,7 @@ Stores secrets in plain text JSON files.
 ```go
 provider, _, err := vault.New("my-vault",
     vault.WithProvider(vault.ProviderTypeUnencrypted), 
-    vault.WithUnencryptedPath("~/dev-secrets.json"),
+    vault.WithUnencryptedPath("~/.config/flow/vaults"), // a directory, not a file
 )
 ```
 
@@ -122,10 +139,12 @@ config := &vault.Config{
     Type: vault.ProviderTypeExternal,
     External: &vault.ExternalConfig{
         Get: vault.CommandConfig{
-            CommandTemplate: "bw get password {{key}}",
+            CommandTemplate: "bw get password '{{key}}'",
         },
         Set: vault.CommandConfig{
-            CommandTemplate: "bw create item --name {{key}} --password {{value}}",
+            CommandTemplate: "bw create item",
+            // The secret is piped to the command's stdin, never placed in it.
+            InputTemplate: "{{value}}",
         },
         // ... other operations
     },
@@ -134,6 +153,14 @@ config := &vault.Config{
 provider, err := vault.NewExternalVaultProvider(config)
 ```
 
+> **The secret value is not available to command templates.** A rendered command
+> is parsed and run by a shell and the template engine does no quoting, so
+> interpolating a secret there is a command-injection sink and silently corrupts
+> any value containing shell metacharacters (`p@$$w0rd` has `$$` replaced by the
+> process ID; `correct horse battery` word-splits to `correct`). Configurations
+> referencing `{{value}}` or `{{password}}` in a `cmd` are rejected at load —
+> use an `InputTemplate` instead.
+
 **External Provider Examples**
 
 Ready-to-use configurations for popular CLI tools are available in the [`examples/`](./examples/) directory:
@@ -165,7 +192,7 @@ secrets, _ := provider.ListSecrets()
 exists, _ := provider.HasSecret("api-key")
 
 // Get vault metadata
-metadata := provider.Metadata()
+metadata, err := provider.Metadata()
 ```
 
 ### Configuration from File
diff --git a/aes.go b/aes.go
index 9ea13af..272407f 100644
--- a/aes.go
+++ b/aes.go
@@ -1,10 +1,8 @@
 package vault
 
 import (
-	"errors"
 	"fmt"
-	"os"
-	"path/filepath"
+	"sort"
 	"sync"
 	"time"
 
@@ -22,7 +20,10 @@ const (
 type AESState struct {
 	Metadata `yaml:"metadata"`
 
-	Version int               `json:"version"`
+	// yaml, not json: this struct is marshaled with gopkg.in/yaml.v3, which
+	// ignores json tags and would otherwise have keyed this field as "version"
+	// only by lowercasing coincidence.
+	Version int               `yaml:"version"`
 	ID      string            `yaml:"id"`
 	Secrets map[string]string `yaml:"secrets"`
 }
@@ -43,13 +44,22 @@ func GenerateEncryptionKey() (string, error) {
 	return crypto.GenerateKey()
 }
 
-// DeriveEncryptionKey derives an AES encryption key from a passphrase
+// DeriveEncryptionKey derives an AES encryption key from a passphrase.
+//
+// An empty sal requests a freshly generated salt. Pass the returned salt back
+// verbatim to re-derive the same key; it carries the parameters it was made
+// with, so future changes to the defaults cannot alter an existing key.
 func DeriveEncryptionKey(passphrase, sal string) (string, string, error) {
-	key, salt, err := crypto.DeriveKey([]byte(passphrase), []byte(sal))
+	var salt []byte
+	if sal != "" {
+		salt = []byte(sal)
+	}
+
+	key, salt2, err := crypto.DeriveKey([]byte(passphrase), salt)
 	if err != nil {
 		return "", "", fmt.Errorf("failed to derive encryption key: %w", err)
 	}
-	return key, salt, nil
+	return key, salt2, nil
 }
 
 // ValidateEncryptionKey checks if a key is valid by attempting to encrypt/decrypt test data
@@ -77,10 +87,14 @@ func NewAES256Vault(cfg *Config) (*AES256Vault, error) {
 		return nil, fmt.Errorf("AES configuration is required")
 	}
 
-	path := filepath.Join(
-		filepath.Clean(cfg.Aes.StoragePath),
-		filepath.Clean(fmt.Sprintf("%s-%s.%s", vaultFileBase, cfg.ID, aesVaultFileExt)),
-	)
+	if err := cfg.Validate(); err != nil {
+		return nil, err
+	}
+
+	path, err := resolveVaultPath(cfg.Aes.StoragePath, cfg.ID, aesVaultFileExt)
+	if err != nil {
+		return nil, err
+	}
 
 	vault := &AES256Vault{
 		id:       cfg.ID,
@@ -119,20 +133,16 @@ func (v *AES256Vault) init() error {
 		Secrets: make(map[string]string),
 	}
 
-	return v.save()
+	return withVaultLock(v.fullPath, v.save)
 }
 
 // load retrieves the AESState from the vault file, decrypts it, and unmarshals it into an AESState struct.
 func (v *AES256Vault) load() error {
-	data, err := os.ReadFile(filepath.Clean(v.fullPath))
+	data, exists, err := readVaultFile(v.fullPath)
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			return nil
-		}
-		return fmt.Errorf("%w: failed to read vault file %s: %w", ErrVaultNotFound, v.fullPath, err)
+		return err
 	}
-
-	if len(data) == 0 {
+	if !exists {
 		return nil
 	}
 
@@ -147,6 +157,10 @@ func (v *AES256Vault) load() error {
 	if err := yaml.Unmarshal([]byte(dataStr), &state); err != nil {
 		return fmt.Errorf("failed to unmarshal vault state: %w", err)
 	}
+	if err := checkVaultVersion(state.Version, aesCurrentVaultVersion, v.fullPath); err != nil {
+		return err
+	}
+
 	v.state = &state
 	return nil
 }
@@ -154,7 +168,7 @@ func (v *AES256Vault) load() error {
 // save encrypts and writes the vault contents to disk
 func (v *AES256Vault) save() error {
 	if v.state == nil {
-		return nil
+		return ErrVaultClosed
 	}
 
 	if v.dek == "" {
@@ -171,41 +185,52 @@ func (v *AES256Vault) save() error {
 		return fmt.Errorf("failed to encrypt vault state: %w", err)
 	}
 
-	// write to the file atomically
-	if err := os.MkdirAll(filepath.Dir(v.fullPath), 0750); err != nil {
-		return fmt.Errorf("failed to create vault directory: %w", err)
-	}
-	tempFile := v.fullPath + ".tmp"
-	if err := os.WriteFile(tempFile, []byte(encryptedDataStr), 0600); err != nil {
-		return fmt.Errorf("failed to write temp vault file: %w", err)
-	}
-
-	if err := os.Rename(tempFile, v.fullPath); err != nil {
-		_ = os.Remove(tempFile)
-		return fmt.Errorf("failed to move vault file: %w", err)
-	}
+	return writeVaultFileAtomic(v.fullPath, []byte(encryptedDataStr))
+}
 
-	return nil
+// mutate runs a read-modify-write cycle under the cross-process vault lock.
+//
+// Reloading inside the lock is the point: in-memory state is a snapshot taken
+// when the provider was constructed, and every save rewrites the whole file.
+// Writing that snapshot back without refreshing silently discards whatever
+// another process stored in the meantime.
+func (v *AES256Vault) mutate(apply func() error) error {
+	return withVaultLock(v.fullPath, func() error {
+		if err := v.load(); err != nil {
+			return err
+		}
+		if err := apply(); err != nil {
+			return err
+		}
+		return v.save()
+	})
 }
 
 func (v *AES256Vault) ID() string {
 	return v.id
 }
 
-func (v *AES256Vault) Metadata() Metadata {
+func (v *AES256Vault) Metadata() (Metadata, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
 	if v.state == nil {
-		return Metadata{}
+		return Metadata{}, ErrVaultClosed
 	}
-	return v.state.Metadata
+	return v.state.Metadata, nil
 }
 
 func (v *AES256Vault) GetSecret(key string) (Secret, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if err := ValidateSecretKey(key); err != nil {
+		return nil, err
+	}
+	if v.state == nil {
+		return nil, ErrVaultClosed
+	}
+
 	value, exists := v.state.Secrets[key]
 	if !exists {
 		return nil, ErrSecretNotFound
@@ -221,36 +246,54 @@ func (v *AES256Vault) SetSecret(key string, secret Secret) error {
 	if err := ValidateSecretKey(key); err != nil {
 		return err
 	}
-
-	if v.state.Secrets == nil {
-		v.state.Secrets = make(map[string]string)
+	if v.state == nil {
+		return ErrVaultClosed
 	}
 
-	v.state.Secrets[key] = secret.PlainTextString()
-	return v.save()
+	return v.mutate(func() error {
+		if v.state.Secrets == nil {
+			v.state.Secrets = make(map[string]string)
+		}
+		v.state.Secrets[key] = secret.PlainTextString()
+		return nil
+	})
 }
 
 func (v *AES256Vault) DeleteSecret(key string) error {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
-	_, exists := v.state.Secrets[key]
-	if !exists {
-		return ErrSecretNotFound
+	if err := ValidateSecretKey(key); err != nil {
+		return err
+	}
+	if v.state == nil {
+		return ErrVaultClosed
 	}
 
-	delete(v.state.Secrets, key)
-	return v.save()
+	// The existence check runs inside mutate, after the reload, so it sees the
+	// current on-disk contents rather than a stale snapshot.
+	return v.mutate(func() error {
+		if _, exists := v.state.Secrets[key]; !exists {
+			return ErrSecretNotFound
+		}
+		delete(v.state.Secrets, key)
+		return nil
+	})
 }
 
 func (v *AES256Vault) ListSecrets() ([]string, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if v.state == nil {
+		return nil, ErrVaultClosed
+	}
+
 	keys := make([]string, 0, len(v.state.Secrets))
 	for k := range v.state.Secrets {
 		keys = append(keys, k)
 	}
+	sort.Strings(keys)
 	return keys, nil
 }
 
@@ -258,6 +301,13 @@ func (v *AES256Vault) HasSecret(key string) (bool, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if err := ValidateSecretKey(key); err != nil {
+		return false, err
+	}
+	if v.state == nil {
+		return false, ErrVaultClosed
+	}
+
 	_, exists := v.state.Secrets[key]
 	return exists, nil
 }
@@ -267,6 +317,9 @@ func (v *AES256Vault) Close() error {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
+	if v.state != nil {
+		clearSecrets(v.state.Secrets)
+	}
 	v.dek = ""
 	v.state = nil
 
diff --git a/aes_key.go b/aes_key.go
index e15dfee..9df63fe 100644
--- a/aes_key.go
+++ b/aes_key.go
@@ -26,21 +26,37 @@ func NewKeyResolver(sources []KeySource) *KeyResolver {
 
 func (r *KeyResolver) ResolveKeys() ([]string, error) {
 	var keys []string
+	var failures []string
 
 	for _, source := range r.sources {
 		switch source.Type {
 		case envSource:
 			if key := r.fromEnvironment(source.Name); key != "" {
 				keys = append(keys, key)
+			} else {
+				failures = append(failures, fmt.Sprintf("env %s: not set or empty", source.Name))
 			}
 		case fileSource:
-			if key, err := r.fromFile(source.Path); err == nil && key != "" {
+			// Errors here used to be discarded outright (`err == nil &&`), so a
+			// typo'd path, a permissions problem or a truncated key file all
+			// surfaced as the generic "no encryption keys found" -- or worse,
+			// silently fell through to a different source.
+			key, err := r.fromFile(source.Path)
+			switch {
+			case err != nil:
+				failures = append(failures, fmt.Sprintf("file %s: %v", source.Path, err))
+			case key == "":
+				failures = append(failures, fmt.Sprintf("file %s: is empty", source.Path))
+			default:
 				keys = append(keys, key)
 			}
 		}
 	}
 
 	if len(keys) == 0 {
+		if len(failures) > 0 {
+			return nil, fmt.Errorf("%w: no encryption keys found (%s)", ErrNoAccess, strings.Join(failures, "; "))
+		}
 		return nil, fmt.Errorf("%w: no encryption keys found", ErrNoAccess)
 	}
 
diff --git a/age.go b/age.go
index 43b842b..4a97f99 100644
--- a/age.go
+++ b/age.go
@@ -4,8 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"os"
-	"path/filepath"
+	"sort"
 	"sync"
 	"time"
 
@@ -46,10 +45,14 @@ func NewAgeVault(cfg *Config) (*AgeVault, error) {
 		return nil, fmt.Errorf("age configuration is required")
 	}
 
-	path := filepath.Join(
-		filepath.Clean(cfg.Age.StoragePath),
-		filepath.Clean(fmt.Sprintf("%s-%s.%s", vaultFileBase, cfg.ID, ageVaultFileExt)),
-	)
+	if err := cfg.Validate(); err != nil {
+		return nil, err
+	}
+
+	path, err := resolveVaultPath(cfg.Age.StoragePath, cfg.ID, ageVaultFileExt)
+	if err != nil {
+		return nil, err
+	}
 
 	vault := &AgeVault{
 		mu:       sync.RWMutex{},
@@ -101,24 +104,31 @@ func (v *AgeVault) init() error {
 		return fmt.Errorf("no recipients available for encryption, please add at least one recipient")
 	}
 
+	// Creating a vault encrypted only to someone else's key produces a file
+	// that cannot be opened again -- including by the process that just wrote
+	// it. Catch that here rather than at the next load.
+	if !v.canDecryptWith(v.state.Recipients) {
+		return fmt.Errorf(
+			"%w: none of the configured recipients match your identity, "+
+				"so the vault would be unreadable as soon as it is written",
+			ErrInvalidRecipient,
+		)
+	}
+
 	if err := v.parseRecipients(); err != nil {
 		return fmt.Errorf("failed to parse recipients: %w", err)
 	}
 
-	return v.save()
+	return withVaultLock(v.fullPath, v.save)
 }
 
 // load reads the vault file and decrypts its contents
 func (v *AgeVault) load() error {
-	data, err := os.ReadFile(v.fullPath)
+	data, exists, err := readVaultFile(v.fullPath)
 	if err != nil {
-		if os.IsNotExist(err) {
-			return nil
-		}
-		return fmt.Errorf("failed to read vault file: %w", err)
+		return err
 	}
-
-	if len(data) == 0 {
+	if !exists {
 		return nil
 	}
 
@@ -132,6 +142,10 @@ func (v *AgeVault) load() error {
 		return fmt.Errorf("failed to unmarshal vault state: %w", err)
 	}
 
+	if err := checkVaultVersion(state.Version, ageCurrentVaultVersion, v.fullPath); err != nil {
+		return err
+	}
+
 	v.state = &state
 	if err := v.parseRecipients(); err != nil {
 		return fmt.Errorf("failed to parse recipients: %w", err)
@@ -143,7 +157,7 @@ func (v *AgeVault) load() error {
 // save encrypts and writes the vault contents to disk
 func (v *AgeVault) save() error {
 	if v.state == nil {
-		return nil
+		return ErrVaultClosed
 	}
 
 	if len(v.recipients) == 0 {
@@ -169,41 +183,48 @@ func (v *AgeVault) save() error {
 		return fmt.Errorf("failed to finalize encryption: %w", err)
 	}
 
-	// write to the file atomically
-	if err := os.MkdirAll(filepath.Dir(v.fullPath), 0750); err != nil {
-		return fmt.Errorf("failed to create vault directory: %w", err)
-	}
-	tempFile := v.fullPath + ".tmp"
-	if err := os.WriteFile(tempFile, buf.Bytes(), 0600); err != nil {
-		return fmt.Errorf("failed to write temp vault file: %w", err)
-	}
-
-	if err := os.Rename(tempFile, v.fullPath); err != nil {
-		_ = os.Remove(tempFile)
-		return fmt.Errorf("failed to move vault file: %w", err)
-	}
+	return writeVaultFileAtomic(v.fullPath, buf.Bytes())
+}
 
-	return nil
+// mutate runs a read-modify-write cycle under the cross-process vault lock.
+// See AES256Vault.mutate for why the reload inside the lock is required.
+func (v *AgeVault) mutate(apply func() error) error {
+	return withVaultLock(v.fullPath, func() error {
+		if err := v.load(); err != nil {
+			return err
+		}
+		if err := apply(); err != nil {
+			return err
+		}
+		return v.save()
+	})
 }
 
 func (v *AgeVault) ID() string {
 	return v.id
 }
 
-func (v *AgeVault) Metadata() Metadata {
+func (v *AgeVault) Metadata() (Metadata, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
 	if v.state == nil {
-		return Metadata{}
+		return Metadata{}, ErrVaultClosed
 	}
-	return v.state.Metadata
+	return v.state.Metadata, nil
 }
 
 func (v *AgeVault) GetSecret(key string) (Secret, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if err := ValidateSecretKey(key); err != nil {
+		return nil, err
+	}
+	if v.state == nil {
+		return nil, ErrVaultClosed
+	}
+
 	value, exists := v.state.Secrets[key]
 	if !exists {
 		return nil, ErrSecretNotFound
@@ -219,36 +240,54 @@ func (v *AgeVault) SetSecret(key string, value Secret) error {
 	if err := ValidateSecretKey(key); err != nil {
 		return err
 	}
-
-	if v.state.Secrets == nil {
-		v.state.Secrets = make(map[string]string)
+	if v.state == nil {
+		return ErrVaultClosed
 	}
 
-	v.state.Secrets[key] = value.PlainTextString()
-	return v.save()
+	return v.mutate(func() error {
+		if v.state.Secrets == nil {
+			v.state.Secrets = make(map[string]string)
+		}
+		v.state.Secrets[key] = value.PlainTextString()
+		return nil
+	})
 }
 
 func (v *AgeVault) DeleteSecret(key string) error {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
-	_, exists := v.state.Secrets[key]
-	if !exists {
-		return ErrSecretNotFound
+	if err := ValidateSecretKey(key); err != nil {
+		return err
+	}
+	if v.state == nil {
+		return ErrVaultClosed
 	}
 
-	delete(v.state.Secrets, key)
-	return v.save()
+	// The existence check runs inside mutate, after the reload, so it sees the
+	// current on-disk contents rather than a stale snapshot.
+	return v.mutate(func() error {
+		if _, exists := v.state.Secrets[key]; !exists {
+			return ErrSecretNotFound
+		}
+		delete(v.state.Secrets, key)
+		return nil
+	})
 }
 
 func (v *AgeVault) ListSecrets() ([]string, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if v.state == nil {
+		return nil, ErrVaultClosed
+	}
+
 	keys := make([]string, 0, len(v.state.Secrets))
 	for k := range v.state.Secrets {
 		keys = append(keys, k)
 	}
+	sort.Strings(keys)
 	return keys, nil
 }
 
@@ -256,15 +295,49 @@ func (v *AgeVault) HasSecret(key string) (bool, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if err := ValidateSecretKey(key); err != nil {
+		return false, err
+	}
+	if v.state == nil {
+		return false, ErrVaultClosed
+	}
+
 	_, exists := v.state.Secrets[key]
 	return exists, nil
 }
 
+// canDecryptWith reports whether any resolved identity appears in recipients.
+// Encrypting to a set that excludes your own key produces a vault you can never
+// reopen, and a single mistyped public key is enough to do it.
+func (v *AgeVault) canDecryptWith(recipients []string) bool {
+	own := make(map[string]struct{}, len(v.identities))
+	for _, id := range v.identities {
+		if x, ok := id.(*age.X25519Identity); ok {
+			own[x.Recipient().String()] = struct{}{}
+		}
+	}
+	if len(own) == 0 {
+		// An identity type we cannot map to a recipient string; do not block on
+		// a check we are unable to perform.
+		return true
+	}
+
+	for _, r := range recipients {
+		if _, ok := own[r]; ok {
+			return true
+		}
+	}
+	return false
+}
+
 func (v *AgeVault) Close() error {
 	// clear the secret state from memory
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
+	if v.state != nil {
+		clearSecrets(v.state.Secrets)
+	}
 	v.state = nil
 	v.recipients = nil
 	v.identities = nil
@@ -276,49 +349,77 @@ func (v *AgeVault) AddRecipient(publicKey string) error {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
-	if err := v.addRecipientToState(publicKey); err != nil {
-		return err
-	}
-	if err := v.parseRecipients(); err != nil {
-		return fmt.Errorf("failed to parse recipients: %w", err)
+	if v.state == nil {
+		return ErrVaultClosed
 	}
 
-	return v.save()
+	return v.mutate(func() error {
+		if err := v.addRecipientToState(publicKey); err != nil {
+			return err
+		}
+		return v.parseRecipients()
+	})
 }
 
 func (v *AgeVault) RemoveRecipient(publicKey string) error {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
-	// Don't allow removing the last recipient
-	if len(v.state.Recipients) <= 1 {
-		return fmt.Errorf("cannot remove the last recipient - at least one recipient is required for encryption")
+	if v.state == nil {
+		return ErrVaultClosed
 	}
 
-	found := false
-	for i, rec := range v.state.Recipients {
-		if rec == publicKey {
-			v.state.Recipients = append(v.state.Recipients[:i], v.state.Recipients[i+1:]...)
-			found = true
-			break
+	return v.mutate(func() error {
+		// Don't allow removing the last recipient
+		if len(v.state.Recipients) <= 1 {
+			return fmt.Errorf("cannot remove the last recipient - at least one recipient is required for encryption")
 		}
-	}
 
-	if !found {
-		return fmt.Errorf("recipient %s not found", publicKey)
-	}
+		remaining := make([]string, 0, len(v.state.Recipients))
+		found := false
+		for _, rec := range v.state.Recipients {
+			if rec == publicKey {
+				found = true
+				continue
+			}
+			remaining = append(remaining, rec)
+		}
 
-	if err := v.parseRecipients(); err != nil {
-		return fmt.Errorf("failed to parse recipients: %w", err)
-	}
+		if !found {
+			return fmt.Errorf("recipient %s not found", publicKey)
+		}
 
-	return v.save()
+		// Refusing here is the difference between "you removed a colleague" and
+		// "you locked yourself out permanently". The previous check only
+		// guarded the *last* recipient, not your own.
+		if !v.canDecryptWith(remaining) {
+			return fmt.Errorf(
+				"%w: removing %s would leave no recipient matching your identity, "+
+					"making the vault permanently unreadable",
+				ErrInvalidRecipient, publicKey,
+			)
+		}
+
+		// Commit to state only once the new set is known to parse, so a failure
+		// cannot leave in-memory recipients inconsistent with what is on disk.
+		previous := v.state.Recipients
+		v.state.Recipients = remaining
+		if err := v.parseRecipients(); err != nil {
+			v.state.Recipients = previous
+			return fmt.Errorf("failed to parse recipients: %w", err)
+		}
+		return nil
+	})
 }
 
 func (v *AgeVault) ListRecipients() ([]string, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if v.state == nil {
+		return nil, ErrVaultClosed
+	}
+
 	recipients := make([]string, len(v.state.Recipients))
 	copy(recipients, v.state.Recipients) // prevent modification of internal state
 	return recipients, nil
diff --git a/config.go b/config.go
index b25eccc..469e45d 100644
--- a/config.go
+++ b/config.go
@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
+	"time"
 )
 
 type ProviderType string
@@ -25,11 +27,17 @@ type Config struct {
 	External    *ExternalConfig    `json:"external,omitempty"`
 	Keyring     *KeyringConfig     `json:"keyring,omitempty"`
 	Unencrypted *UnencryptedConfig `json:"unencrypted,omitempty"`
+
+	// pendingLocalPath holds a WithLocalPath value until the provider type is
+	// known. Unexported so it never reaches the serialized config.
+	pendingLocalPath string
 }
 
 func (c *Config) Validate() error {
-	if c.ID == "" {
-		return fmt.Errorf("%w: vault ID is required", ErrInvalidConfig)
+	// The ID becomes part of a filename and of keyring entry names, so it needs
+	// a real charset check, not just a non-empty check.
+	if err := ValidateVaultID(c.ID); err != nil {
+		return err
 	}
 
 	switch c.Type {
@@ -214,12 +222,57 @@ type ExternalConfig struct {
 
 	// WorkingDir for command execution
 	WorkingDir string `json:"working_dir,omitempty"`
+
+	// NotFoundPattern is matched against a failing command's error output to tell
+	// "this secret does not exist" apart from a real failure (an expired session,
+	// a network error, a permissions problem). Without it, any non-zero exit is
+	// read as absence. Example: "ParameterNotFound".
+	NotFoundPattern string `json:"not_found_pattern,omitempty"`
 }
 
+// timeoutDuration parses the configured timeout. An empty timeout means no limit.
+func (c *ExternalConfig) timeoutDuration() (time.Duration, error) {
+	if c.Timeout == "" {
+		return 0, nil
+	}
+	return time.ParseDuration(c.Timeout)
+}
+
+// secretValueRefs matches a template action referencing the secret value. These
+// are rejected in command templates: the rendered command is executed by a shell
+// and the template engine performs no quoting, so interpolating a secret there is
+// a command-injection sink and silently corrupts values containing shell
+// metacharacters. Secrets must travel over stdin via an input template.
+var secretValueRefs = regexp.MustCompile(`{{[^}]*\b(value|password)\b[^}]*}}`)
+
 func (c *ExternalConfig) Validate() error {
 	if c.Get.CommandTemplate == "" || c.Set.CommandTemplate == "" {
 		return fmt.Errorf("%w: get and set args template required for external vault", ErrInvalidConfig)
 	}
+
+	cmdTemplates := map[string]string{
+		"get":      c.Get.CommandTemplate,
+		"set":      c.Set.CommandTemplate,
+		"delete":   c.Delete.CommandTemplate,
+		"list":     c.List.CommandTemplate,
+		"exists":   c.Exists.CommandTemplate,
+		"metadata": c.Metadata.CommandTemplate,
+	}
+	for op, tmpl := range cmdTemplates {
+		if secretValueRefs.MatchString(tmpl) {
+			return fmt.Errorf(
+				"%w: the %s command template references the secret value, which is unsafe: "+
+					"the command is run by a shell and the value is not quoted. "+
+					`Move it to an input template instead, e.g. "input": "{{ value }}"`,
+				ErrInvalidConfig, op,
+			)
+		}
+	}
+
+	if _, err := c.timeoutDuration(); err != nil {
+		return fmt.Errorf("%w: invalid timeout duration %q: %w", ErrInvalidConfig, c.Timeout, err)
+	}
+
 	return nil
 }
 
diff --git a/crypto/crypto.go b/crypto/crypto.go
index e30948d..397ce99 100644
--- a/crypto/crypto.go
+++ b/crypto/crypto.go
@@ -7,13 +7,44 @@ import (
 	"encoding/base64"
 	"fmt"
 	"io"
+	"strings"
 
 	"golang.org/x/crypto/scrypt"
 )
 
+const (
+	// KeyLen is the required key size. AES-256 means 32 bytes and nothing else:
+	// aes.NewCipher also accepts 16 and 24, so without an explicit check a short
+	// key silently downgrades an "AES256" vault to AES-128 or AES-192.
+	KeyLen = 32
+
+	// SaltLen is the size of a generated salt.
+	SaltLen = 32
+	// MinSaltLen is the smallest salt accepted for derivation.
+	MinSaltLen = 16
+
+	// scryptN is the CPU/memory cost. Memory used is roughly 128 * N * r, so
+	// this is ~64 MiB per derivation. The previous value of 1<<20 required
+	// ~1 GiB and several seconds, which thrashes or OOMs on a modest machine
+	// and makes concurrent derivations a trivial local denial of service.
+	scryptN = 1 << 16
+	scryptR = 8
+	scryptP = 1
+
+	// legacyScryptN is the original cost. Salts issued before parameters were
+	// recorded carry no parameter block, and must keep deriving the same key.
+	legacyScryptN = 1 << 20
+
+	saltPrefix = "scrypt"
+	saltSep    = "$"
+
+	// maxPlaintextLen bounds a single encryption.
+	maxPlaintextLen = 64 * 1024 * 1024
+)
+
 // GenerateKey generates a random 32 byte key and returns it as a base64 encoded string.
 func GenerateKey() (string, error) {
-	key := make([]byte, 32)
+	key := make([]byte, KeyLen)
 	_, err := rand.Read(key)
 	if err != nil {
 		return "", fmt.Errorf("error reading random bytes: %w", err)
@@ -21,23 +52,85 @@ func GenerateKey() (string, error) {
 	return EncodeValue(key), nil
 }
 
-// DeriveKey derives a 32 byte key from the provided password and salt and returns
-// the key and salt as base64 encoded strings.
-// If salt is nil, a random salt will be generated.
+// DeriveKey derives a 32 byte key from the provided password and salt, returning
+// the base64 encoded key and the salt that produced it.
+//
+// If salt is empty a fresh random one is generated. The returned salt carries
+// the parameters used ("scrypt$N=...,r=...,p=...$<base64>") so that changing the
+// defaults later cannot silently change the key derived from an existing salt.
+// Pass the returned salt back verbatim to re-derive the same key.
 func DeriveKey(password, salt []byte) (string, string, error) {
-	if salt == nil {
-		salt = make([]byte, 32)
-		if _, err := rand.Read(salt); err != nil {
+	// Deliberately len()==0 rather than salt==nil. []byte("") from a variable
+	// string is non-nil, so a nil check let the natural "I have no salt, make
+	// one" call fall through to an *unsalted*, fully deterministic derivation.
+	if len(salt) == 0 {
+		generated := make([]byte, SaltLen)
+		if _, err := rand.Read(generated); err != nil {
+			return "", "", fmt.Errorf("error generating salt: %w", err)
+		}
+		key, err := deriveScrypt(password, generated, scryptN)
+		if err != nil {
 			return "", "", err
 		}
+		return key, formatSalt(generated), nil
 	}
 
-	key, err := scrypt.Key(password, salt, 1048576, 8, 1, 32)
+	raw, n, err := parseSalt(salt)
+	if err != nil {
+		return "", "", err
+	}
+	key, err := deriveScrypt(password, raw, n)
 	if err != nil {
 		return "", "", err
 	}
+	return key, string(salt), nil
+}
+
+func formatSalt(raw []byte) string {
+	return fmt.Sprintf("%s%sN=%d,r=%d,p=%d%s%s",
+		saltPrefix, saltSep, scryptN, scryptR, scryptP, saltSep, EncodeValue(raw))
+}
+
+// parseSalt splits a salt into its raw bytes and cost parameter. A salt with no
+// parameter block predates them and is used as-is with the original cost.
+func parseSalt(salt []byte) ([]byte, int, error) {
+	s := string(salt)
+	if !strings.HasPrefix(s, saltPrefix+saltSep) {
+		if len(salt) < MinSaltLen {
+			return nil, 0, fmt.Errorf("salt must be at least %d bytes, got %d", MinSaltLen, len(salt))
+		}
+		return salt, legacyScryptN, nil
+	}
+
+	parts := strings.Split(s, saltSep)
+	if len(parts) != 3 {
+		return nil, 0, fmt.Errorf("malformed salt: expected %s$<params>$<base64>", saltPrefix)
+	}
+
+	var n, r, p int
+	if _, err := fmt.Sscanf(parts[1], "N=%d,r=%d,p=%d", &n, &r, &p); err != nil {
+		return nil, 0, fmt.Errorf("malformed salt parameters %q: %w", parts[1], err)
+	}
+	if r != scryptR || p != scryptP {
+		return nil, 0, fmt.Errorf("unsupported salt parameters r=%d p=%d", r, p)
+	}
 
-	return EncodeValue(key), EncodeValue(salt), nil
+	raw, err := DecodeValue(parts[2])
+	if err != nil {
+		return nil, 0, fmt.Errorf("malformed salt encoding: %w", err)
+	}
+	if len(raw) < MinSaltLen {
+		return nil, 0, fmt.Errorf("salt must be at least %d bytes, got %d", MinSaltLen, len(raw))
+	}
+	return raw, n, nil
+}
+
+func deriveScrypt(password, salt []byte, n int) (string, error) {
+	key, err := scrypt.Key(password, salt, n, scryptR, scryptP, KeyLen)
+	if err != nil {
+		return "", fmt.Errorf("error deriving key: %w", err)
+	}
+	return EncodeValue(key), nil
 }
 
 // EncodeValue encodes a byte slice as a base64 encoded string.
@@ -54,12 +147,27 @@ func DecodeValue(s string) ([]byte, error) {
 	return data, nil
 }
 
+// decodeKey decodes and length-checks an encryption key.
+func decodeKey(encryptionKey string) ([]byte, error) {
+	key, err := DecodeValue(encryptionKey)
+	if err != nil {
+		return nil, fmt.Errorf("error decoding master key: %w", err)
+	}
+	if len(key) != KeyLen {
+		return nil, fmt.Errorf(
+			"encryption key must be %d bytes, got %d; expected a base64 encoded 256-bit key",
+			KeyLen, len(key),
+		)
+	}
+	return key, nil
+}
+
 // EncryptValue encrypts a string using AES-256-GCM and returns the encrypted value as a base64 encoded string.
 // The encryption key used for encryption must be a base64 encoded string.
 func EncryptValue(encryptionKey string, text string) (string, error) {
-	decodedMasterKey, err := DecodeValue(encryptionKey)
+	decodedMasterKey, err := decodeKey(encryptionKey)
 	if err != nil {
-		return "", fmt.Errorf("error decoding master key: %w", err)
+		return "", err
 	}
 	block, err := aes.NewCipher(decodedMasterKey)
 	if err != nil {
@@ -73,7 +181,7 @@ func EncryptValue(encryptionKey string, text string) (string, error) {
 
 	plaintext := []byte(text)
 	// verify that the plaintext is not too long to fit in an int
-	if len(plaintext) > 64*1024*1024 {
+	if len(plaintext) > maxPlaintextLen {
 		return "", fmt.Errorf("plaintext too long to encrypt")
 	}
 
@@ -88,9 +196,9 @@ func EncryptValue(encryptionKey string, text string) (string, error) {
 // DecryptValue decrypts a string using AES-256-GCM and returns the decrypted value as a string.
 // The master key used for decryption must be a base64 encoded string.
 func DecryptValue(encryptionKey string, text string) (string, error) {
-	decodedMasterKey, err := DecodeValue(encryptionKey)
+	decodedMasterKey, err := decodeKey(encryptionKey)
 	if err != nil {
-		return "", fmt.Errorf("error decoding master key: %w", err)
+		return "", err
 	}
 	block, err := aes.NewCipher(decodedMasterKey)
 	if err != nil {
diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go
index c787353..204aab8 100644
--- a/crypto/crypto_test.go
+++ b/crypto/crypto_test.go
@@ -34,76 +34,91 @@ func TestGenerateKey(t *testing.T) {
 	}
 }
 
-func TestDeriveKeyWithProvidedSalt(t *testing.T) {
-	salt, err := crypto.GenerateKey()
-	if err != nil {
-		t.Fatalf("Failed to generate salt: %v", err)
-	}
-	decodedSalt, err := crypto.DecodeValue(salt)
-	if err != nil {
-		t.Fatalf("Failed to decode salt: %v", err)
-	}
-	if len(decodedSalt) == 0 {
-		t.Error("Decoded salt should not be empty")
-	}
+// The returned salt is passed back verbatim; it carries the parameters it was
+// produced with so a later change to the defaults cannot silently derive a
+// different key from the same salt.
+func TestDeriveKeyRoundTripsItsOwnSalt(t *testing.T) {
+	password := []byte("password")
 
-	inputPassword := []byte("password")
-	derivedKey, outSalt, err := crypto.DeriveKey(inputPassword, decodedSalt)
+	derivedKey, outSalt, err := crypto.DeriveKey(password, nil)
 	if err != nil {
-		t.Fatalf("Failed to derive key: %v", err)
-	}
-	if derivedKey == "" {
-		t.Error("Derived key should not be empty")
+		t.Fatalf("Failed to derive key without salt: %v", err)
 	}
-	if outSalt != salt {
-		t.Errorf("Output salt should equal input salt, got %s, expected %s", outSalt, salt)
+	if derivedKey == "" || outSalt == "" {
+		t.Fatal("Derived key and salt should not be empty")
 	}
 
 	decodedDerivedKey, err := crypto.DecodeValue(derivedKey)
 	if err != nil {
 		t.Fatalf("Failed to decode derived key: %v", err)
 	}
-	if len(decodedDerivedKey) == 0 {
-		t.Error("Decoded derived key should not be empty")
+	if len(decodedDerivedKey) != crypto.KeyLen {
+		t.Errorf("Derived key is %d bytes, want %d", len(decodedDerivedKey), crypto.KeyLen)
 	}
-}
 
-func TestDeriveKeyWithoutSalt(t *testing.T) {
-	inputPassword := []byte("password")
-	derivedKey, outSalt, err := crypto.DeriveKey(inputPassword, nil)
+	derivedKey2, outSalt2, err := crypto.DeriveKey(password, []byte(outSalt))
 	if err != nil {
-		t.Fatalf("Failed to derive key without salt: %v", err)
+		t.Fatalf("Failed to derive key with the returned salt: %v", err)
 	}
-	if derivedKey == "" {
-		t.Error("Derived key should not be empty")
+	if derivedKey != derivedKey2 {
+		t.Error("Keys derived with the same password and salt should be identical")
 	}
-	if outSalt == "" {
-		t.Error("Generated salt should not be empty")
+	if outSalt != outSalt2 {
+		t.Errorf("Salt should round trip unchanged, got %s, want %s", outSalt2, outSalt)
 	}
+}
 
-	decodedDerivedKey, err := crypto.DecodeValue(derivedKey)
+func TestDeriveKeyUsesDistinctSaltsPerCall(t *testing.T) {
+	password := []byte("password")
+
+	key1, salt1, err := crypto.DeriveKey(password, nil)
 	if err != nil {
-		t.Fatalf("Failed to decode derived key: %v", err)
+		t.Fatalf("Failed to derive first key: %v", err)
 	}
-	if len(decodedDerivedKey) == 0 {
-		t.Error("Decoded derived key should not be empty")
+	key2, salt2, err := crypto.DeriveKey(password, nil)
+	if err != nil {
+		t.Fatalf("Failed to derive second key: %v", err)
 	}
 
-	// Test reproducibility with same salt
-	decodedSalt, err := crypto.DecodeValue(outSalt)
-	if err != nil {
-		t.Fatalf("Failed to decode output salt: %v", err)
+	if salt1 == salt2 {
+		t.Error("Each derivation should generate a fresh salt")
+	}
+	if key1 == key2 {
+		t.Error("The same password with different salts must not produce the same key")
 	}
+}
+
+// []byte("") from a variable string is non-nil, so the old salt==nil check
+// never fired for the natural "I have no salt, generate one" call. That derived
+// an unsalted, fully deterministic key -- identical for every user with the
+// same passphrase, and precomputable.
+func TestDeriveKeyTreatsEmptySaltAsAbsentNotUnsalted(t *testing.T) {
+	password := []byte("password")
+	empty := ""
 
-	derivedKey2, outSalt2, err := crypto.DeriveKey(inputPassword, decodedSalt)
+	key1, salt1, err := crypto.DeriveKey(password, []byte(empty))
 	if err != nil {
-		t.Fatalf("Failed to derive key with same salt: %v", err)
+		t.Fatalf("Failed to derive key with empty salt: %v", err)
 	}
-	if derivedKey != derivedKey2 {
-		t.Error("Keys derived with same password and salt should be identical")
+	key2, salt2, err := crypto.DeriveKey(password, []byte(empty))
+	if err != nil {
+		t.Fatalf("Failed to derive second key with empty salt: %v", err)
 	}
-	if outSalt != outSalt2 {
-		t.Error("Output salt should be same when input salt is provided")
+
+	if salt1 == "" || salt2 == "" {
+		t.Fatal("An empty salt must produce a freshly generated one")
+	}
+	if salt1 == salt2 {
+		t.Error("An empty salt produced the same salt twice; it is not being generated")
+	}
+	if key1 == key2 {
+		t.Error("An empty salt derived a deterministic key; the salt is not being applied")
+	}
+}
+
+func TestDeriveKeyRejectsShortSalts(t *testing.T) {
+	if _, _, err := crypto.DeriveKey([]byte("password"), []byte("tiny")); err == nil {
+		t.Error("Expected a short salt to be rejected")
 	}
 }
 
@@ -246,6 +261,27 @@ func TestInvalidKeys(t *testing.T) {
 	}
 }
 
+// aes.NewCipher accepts 16, 24 and 32 byte keys, so without an explicit length
+// check a short key silently downgrades an "AES256" vault to AES-128 or -192.
+func TestNonAES256KeysAreRejected(t *testing.T) {
+	for _, size := range []int{8, 16, 24, 31, 33, 64} {
+		key := crypto.EncodeValue(make([]byte, size))
+
+		if _, err := crypto.EncryptValue(key, "data"); err == nil {
+			t.Errorf("EncryptValue accepted a %d byte key, want rejection", size)
+		}
+		if _, err := crypto.DecryptValue(key, crypto.EncodeValue(make([]byte, 64))); err == nil {
+			t.Errorf("DecryptValue accepted a %d byte key, want rejection", size)
+		}
+	}
+
+	// The correct size still works.
+	valid := crypto.EncodeValue(make([]byte, crypto.KeyLen))
+	if _, err := crypto.EncryptValue(valid, "data"); err != nil {
+		t.Errorf("EncryptValue rejected a %d byte key: %v", crypto.KeyLen, err)
+	}
+}
+
 func TestInvalidCiphertext(t *testing.T) {
 	key, err := crypto.GenerateKey()
 	if err != nil {
diff --git a/errors.go b/errors.go
index 21cf2b2..b12d0c0 100644
--- a/errors.go
+++ b/errors.go
@@ -14,6 +14,8 @@ var (
 	ErrDecryptionFailed = errors.New("decryption failed")
 	ErrInvalidRecipient = errors.New("invalid recipient")
 	ErrPathNotSecure    = errors.New("path is not secure")
+	ErrVaultClosed      = errors.New("vault is closed")
+	ErrVaultCorrupt     = errors.New("vault file is corrupt")
 )
 
 type VaultPathError struct {
diff --git a/examples/README.md b/examples/README.md
index 1254e00..5bffe01 100644
--- a/examples/README.md
+++ b/examples/README.md
@@ -12,13 +12,14 @@ This directory contains ready-to-use configurations for popular CLI tools.
 ## Quick Start
 
 ```bash
-# Test a configuration
-./test-provider.sh providers/bitwarden.json
-
-# Run the Go example
+# Run the Go example against a configuration
 go run main.go providers/pass.json
 ```
 
+These configurations are covered by `TestShippedExampleProvidersAreUsable`, which
+loads each one, renders every operation, and asserts that the secret value reaches
+the backend over stdin and never appears in a command string.
+
 ## Setup Instructions
 
 ### Authentication Requirements
@@ -54,7 +55,8 @@ Each configuration follows this pattern:
       "output": "{{output}}"
     },
     "set": {
-      "cmd": "subcommand {{key}} {{value}}"
+      "cmd": "subcommand {{key}}",
+      "input": "{{value}}"
     },
     "list": {
       "cmd": "list-subcommand"
@@ -78,9 +80,30 @@ Each configuration follows this pattern:
 
 ## Template Variables
 
-Available in `cmd` and `output` fields:
+In `cmd` fields:
 
-- `{{key}}` - The secret key/name
-- `{{value}}` - The secret value (for set operations)
+- `{{key}}` - The secret key/name (also available as `ref`, `id`, `name`)
 - `{{env["VariableName"]}}`- Environment variable value
-- `{{output}}` - Raw command output (for output templates)
+
+In `input` fields (piped to the command's stdin):
+
+- `{{value}}` - The secret value, on `set` only
+- `{{input}}` - The secret key
+- `{{env["VariableName"]}}`
+
+In `output` fields:
+
+- `{{output}}` - Raw command output
+
+### The secret value is never available to a `cmd` template
+
+A rendered command is parsed and executed by a shell, and the template engine
+performs no quoting. Interpolating a secret there is a command-injection sink and
+silently corrupts any value containing shell metacharacters -- `p@$$w0rd` has `$$`
+expanded to the process ID, and `correct horse battery` word-splits to `correct`.
+
+Configurations that reference `{{value}}` or `{{password}}` in any `cmd` are
+rejected at load. Pass the secret over stdin with an `input` template instead.
+
+Shell syntax in a `cmd` works normally: `$VAR`, `${VAR:-default}` and `$(...)` are
+resolved by the interpreter, with the configured `environment` in scope.
diff --git a/examples/main.go b/examples/main.go
index 89b8084..079116f 100644
--- a/examples/main.go
+++ b/examples/main.go
@@ -11,6 +11,16 @@ import (
 	"github.com/flowexec/vault"
 )
 
+func printMetadata(provider vault.Provider) {
+	fmt.Println("Getting vault metadata...")
+	metadata, err := provider.Metadata()
+	if err != nil {
+		fmt.Printf("Warning: could not read metadata: %v\n", err)
+		return
+	}
+	fmt.Printf("Metadata: %s\n", metadata.RawData)
+}
+
 func main() {
 	if len(os.Args) < 2 {
 		fmt.Println("Usage: go run main.go <provider-config.json>")
@@ -90,9 +100,7 @@ func main() {
 		}
 	}
 
-	fmt.Println("Getting vault metadata...")
-	metadata := provider.Metadata()
-	fmt.Printf("Metadata: %s\n", metadata.RawData)
+	printMetadata(provider)
 
 	fmt.Println("Cleaning up test secret...")
 	err = provider.DeleteSecret("test-key")
diff --git a/examples/providers/1password.json b/examples/providers/1password.json
index fac2bd2..297e15a 100644
--- a/examples/providers/1password.json
+++ b/examples/providers/1password.json
@@ -3,29 +3,27 @@
   "type": "external",
   "external": {
     "get": {
-      "cmd": "op read \"op://Private/{{key}}/password\"",
-      "output": "{{output}}"
+      "cmd": "op read --no-newline 'op://Private/{{key}}/password'"
     },
     "set": {
-      "cmd": "op item create --category Login --title {{key}} password={{value}} --vault Private"
+      "cmd": "if op item get --vault Private -- '{{key}}' >/dev/null 2>&1 </dev/null; then jq -Rs '{ fields: [ { id: \"password\", type: \"CONCEALED\", purpose: \"PASSWORD\", label: \"password\", value: . } ] }' | op item edit --vault Private -- '{{key}}' >/dev/null; else jq -Rs '{ title: \"{{key}}\", category: \"LOGIN\", fields: [ { id: \"password\", type: \"CONCEALED\", purpose: \"PASSWORD\", label: \"password\", value: . } ] }' | op item create --vault Private >/dev/null; fi",
+      "input": "{{value}}"
     },
     "delete": {
-      "cmd": "op item delete {{key}} --vault Private"
+      "cmd": "op item delete --vault Private -- '{{key}}' >/dev/null"
     },
     "list": {
       "cmd": "op item list --vault Private --format json",
-      "output": "{{ map(fromJSON(output), {.title}) | join(\"\\n\") }}"
+      "output": "{{ join(map(fromJSON(output), {.title}), \"\\n\") }}"
     },
+    "separator": "\n",
     "exists": {
-      "cmd": "op item get {{key}} --vault Private"
+      "cmd": "op item get --vault Private --format json -- '{{key}}' >/dev/null"
     },
     "metadata": {
-      "cmd": "op account list",
-      "output": "Account: {{output}}"
+      "cmd": "op whoami"
     },
-    "environment": {
-      "OP_SERVICE_ACCOUNT_TOKEN": "$OP_SERVICE_ACCOUNT_TOKEN"
-    },
-    "timeout": "30s"
+    "not_found_pattern": "isn't an item",
+    "timeout": "120s"
   }
-}
\ No newline at end of file
+}
diff --git a/examples/providers/aws-ssm.json b/examples/providers/aws-ssm.json
index 67b482e..0989d0c 100644
--- a/examples/providers/aws-ssm.json
+++ b/examples/providers/aws-ssm.json
@@ -3,32 +3,31 @@
   "type": "external",
   "external": {
     "get": {
-      "cmd": "aws ssm get-parameter --name /{{key}} --with-decryption --query Parameter.Value --output text",
-      "output": "{{output}}"
+      "cmd": "aws ssm get-parameter --name '/{{key}}' --with-decryption --query Parameter.Value --output text"
     },
     "set": {
-      "cmd": "aws ssm put-parameter --name /{{key}} --value {{value}} --type SecureString --overwrite"
+      "cmd": "aws ssm put-parameter --name '/{{key}}' --value file:///dev/stdin --type SecureString --overwrite --output json >/dev/null",
+      "input": "{{value}}"
     },
     "delete": {
-      "cmd": "aws ssm delete-parameter --name /{{key}}"
+      "cmd": "aws ssm delete-parameter --name '/{{key}}' --output json >/dev/null"
     },
     "list": {
-      "cmd": "aws ssm describe-parameters --query Parameters[].Name --output text",
-      "output": "{{output}}"
+      "cmd": "aws ssm get-parameters-by-path --path / --output json",
+      "output": "{{ join(map(fromJSON(output)[\"Parameters\"], {trimPrefix(.Name, \"/\")}), \"\\n\") }}"
     },
-    "listSeparator": "\t",
+    "separator": "\n",
     "exists": {
-      "cmd": "aws ssm get-parameter --name /{{key}}"
+      "cmd": "aws ssm get-parameter --name '/{{key}}' --query Parameter.Name --output text >/dev/null"
     },
     "metadata": {
       "cmd": "aws sts get-caller-identity --output json",
-      "output": "Account: {{fromJSON(output)[\"Account\"]}}, User: {{fromJSON(output)[\"Arn\"]}}"
+      "output": "AWS account {{ fromJSON(output)[\"Account\"] }} as {{ fromJSON(output)[\"Arn\"] }}"
     },
+    "not_found_pattern": "ParameterNotFound",
     "environment": {
-      "AWS_REGION": "$AWS_REGION",
-      "AWS_ACCESS_KEY_ID": "$AWS_ACCESS_KEY_ID",
-      "AWS_SECRET_ACCESS_KEY": "$AWS_SECRET_ACCESS_KEY"
+      "AWS_PAGER": ""
     },
     "timeout": "60s"
   }
-}
\ No newline at end of file
+}
diff --git a/examples/providers/bitwarden.json b/examples/providers/bitwarden.json
index fb5bde7..d88fbd3 100644
--- a/examples/providers/bitwarden.json
+++ b/examples/providers/bitwarden.json
@@ -3,29 +3,27 @@
   "type": "external",
   "external": {
     "get": {
-      "cmd": "bw get password {{key}}",
-      "output": "{{output}}"
+      "cmd": "bw --nointeraction list items | jq -r 'map(select(.name == \"{{key}}\")) | .[0].login.password // (\"secret not found\" | halt_error(1))'"
     },
     "set": {
-      "cmd": "if bw get item \"{{key}}\" >/dev/null 2>&1; then bw get item \"{{key}}\" | jq '.login.password=\"{{value}}\"' | bw encode | bw edit item $(bw get item \"{{key}}\" | jq -r .id); else echo '{\"object\":\"item\",\"type\":1,\"name\":\"{{key}}\",\"login\":{\"username\":\"{{key}}\",\"password\":\"{{value}}\"}}' | bw encode | bw create item; fi"
+      "cmd": "id=$(bw --nointeraction list items </dev/null | jq -r 'map(select(.name == \"{{key}}\")) | .[0].id // empty'); jq -Rs '{ object: \"item\", type: 1, name: \"{{key}}\", notes: null, favorite: false, login: { username: null, password: ., totp: null } }' | bw --nointeraction encode | { if [ -n \"$id\" ]; then bw --nointeraction edit item \"$id\"; else bw --nointeraction create item; fi; } >/dev/null",
+      "input": "{{value}}"
     },
     "delete": {
-      "cmd": "bw delete item $(bw get item {{key}} | jq -r .id)"
+      "cmd": "bw --nointeraction delete item --permanent \"$(bw --nointeraction list items | jq -r 'map(select(.name == \"{{key}}\")) | .[0].id')\" >/dev/null"
     },
     "list": {
-      "cmd": "bw list items --search \"\" --pretty",
-      "output": "{{ map(fromJSON(output), {.name}) | join(\"\\n\") }}"
+      "cmd": "bw --nointeraction list items",
+      "output": "{{ join(map(fromJSON(output), {.name}), \"\\n\") }}"
     },
+    "separator": "\n",
     "exists": {
-      "cmd": "bw get item {{key}}"
+      "cmd": "bw --nointeraction list items | jq -e 'map(select(.name == \"{{key}}\")) | length > 0' >/dev/null"
     },
     "metadata": {
-      "cmd": "bw status",
-      "output": "Status: {{output}}"
+      "cmd": "bw --nointeraction status"
     },
-    "environment": {
-      "BW_SESSION": "$BW_SESSION"
-    },
-    "timeout": "30s"
+    "not_found_pattern": "secret not found",
+    "timeout": "120s"
   }
-}
\ No newline at end of file
+}
diff --git a/examples/providers/pass.json b/examples/providers/pass.json
index b8f6d7a..58ac069 100644
--- a/examples/providers/pass.json
+++ b/examples/providers/pass.json
@@ -3,31 +3,27 @@
   "type": "external",
   "external": {
     "get": {
-      "cmd": "pass show {{key}}",
-      "output": "{{output}}"
+      "cmd": "pass show '{{key}}'"
     },
     "set": {
-      "cmd": "pass insert -e {{key}}",
+      "cmd": "pass insert -m -f '{{key}}' >/dev/null",
       "input": "{{value}}"
     },
     "delete": {
-      "cmd": "pass rm -f {{key}}"
+      "cmd": "pass rm -f '{{key}}' >/dev/null"
     },
     "list": {
-      "cmd": "pass ls",
-      "output": "{{output}}"
+      "cmd": "cd \"${PASSWORD_STORE_DIR:-$HOME/.password-store}\" && find . -name '*.gpg' -type f | sed -e 's|^\\./||' -e 's|\\.gpg$||'"
     },
+    "separator": "\n",
     "exists": {
-      "cmd": "pass show {{key}}"
+      "cmd": "test -f \"${PASSWORD_STORE_DIR:-$HOME/.password-store}/{{key}}.gpg\""
     },
     "metadata": {
-      "cmd": "pass git log --oneline -1",
-      "output": "Last change: {{output}}"
+      "cmd": "cat \"${PASSWORD_STORE_DIR:-$HOME/.password-store}/.gpg-id\"",
+      "output": "pass store recipients: {{ trim(output) }}"
     },
-    "environment": {
-      "PASSWORD_STORE_DIR": "$PASSWORD_STORE_DIR",
-      "GPG_TTY": "$(tty)"
-    },
-    "timeout": "30s"
+    "not_found_pattern": "is not in the password store",
+    "timeout": "120s"
   }
-}
\ No newline at end of file
+}
diff --git a/examples_test.go b/examples_test.go
new file mode 100644
index 0000000..ccac7cd
--- /dev/null
+++ b/examples_test.go
@@ -0,0 +1,155 @@
+package vault_test
+
+import (
+	"context"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/flowexec/vault"
+)
+
+// Every config shipped under examples/providers must load, validate, and render
+// each of its operations. Nothing previously exercised these files, which is how
+// three of them came to interpolate the secret straight into a shell command and
+// how the pass example's stdin-based set shipped permanently broken.
+func TestShippedExampleProvidersAreUsable(t *testing.T) {
+	files, err := filepath.Glob("examples/providers/*.json")
+	if err != nil {
+		t.Fatalf("Glob() error = %v", err)
+	}
+	if len(files) == 0 {
+		t.Fatal("no example provider configs found")
+	}
+
+	const secret = "s3cr3t-$(id)-'quoted'"
+
+	// Sample backend output per provider, shaped the way that CLI actually
+	// responds, so the output templates are genuinely exercised rather than
+	// handed something they happen to tolerate.
+	sampleOutput := map[string]struct{ list, metadata string }{
+		"aws-ssm.json": {
+			list:     `{"Parameters":[{"Name":"/alpha"},{"Name":"/beta"}]}`,
+			metadata: `{"Account":"123456789012","Arn":"arn:aws:iam::123456789012:user/dev"}`,
+		},
+		"1password.json": {
+			list:     `[{"title":"alpha"},{"title":"beta"}]`,
+			metadata: `dev@example.com`,
+		},
+		"bitwarden.json": {
+			list:     `[{"name":"alpha"},{"name":"beta"}]`,
+			metadata: `{"status":"unlocked"}`,
+		},
+		"pass.json": {
+			list:     "alpha\nbeta",
+			metadata: "AAAA1111",
+		},
+	}
+
+	for _, file := range files {
+		t.Run(filepath.Base(file), func(t *testing.T) {
+			samples, ok := sampleOutput[filepath.Base(file)]
+			if !ok {
+				t.Fatalf("no sample output defined for %s; add one so its templates are covered", file)
+			}
+
+			cfg, err := vault.LoadConfigJSON(file)
+			if err != nil {
+				t.Fatalf("LoadConfigJSON() error = %v", err)
+			}
+
+			provider, err := vault.NewExternalVaultProvider(&cfg)
+			if err != nil {
+				t.Fatalf("config was rejected: %v", err)
+			}
+
+			var (
+				lastCmd   string
+				lastInput string
+				nextOut   string
+			)
+			provider.SetExecutionFunc(func(
+				_ context.Context, cmd, input, _ string, _ []string,
+			) (string, error) {
+				lastCmd, lastInput = cmd, input
+				return nextOut, nil
+			})
+
+			// Set: the secret must reach stdin and never the command string.
+			if err := provider.SetSecret("test-key", vault.NewSecretValue([]byte(secret))); err != nil {
+				t.Fatalf("SetSecret() error = %v", err)
+			}
+			if lastInput != secret {
+				t.Errorf("set stdin = %q, want the secret %q", lastInput, secret)
+			}
+			if strings.Contains(lastCmd, secret) {
+				t.Errorf("the secret leaked into the set command: %q", lastCmd)
+			}
+
+			// Every other configured operation must render without error.
+			nextOut = "the-secret"
+			secretValue, err := provider.GetSecret("test-key")
+			if err != nil {
+				t.Errorf("GetSecret() error = %v", err)
+			} else if secretValue.PlainTextString() != "the-secret" {
+				t.Errorf("GetSecret() = %q, want %q", secretValue.PlainTextString(), "the-secret")
+			}
+
+			nextOut = ""
+			if err := provider.DeleteSecret("test-key"); err != nil {
+				t.Errorf("DeleteSecret() error = %v", err)
+			}
+
+			nextOut = samples.list
+			keys, err := provider.ListSecrets()
+			if err != nil {
+				t.Errorf("ListSecrets() error = %v", err)
+			} else if len(keys) != 2 || keys[0] != "alpha" || keys[1] != "beta" {
+				t.Errorf("ListSecrets() = %v, want [alpha beta]", keys)
+			}
+
+			nextOut = ""
+			if _, err := provider.HasSecret("test-key"); err != nil {
+				t.Errorf("HasSecret() error = %v", err)
+			}
+
+			nextOut = samples.metadata
+			if _, err := provider.Metadata(); err != nil {
+				t.Errorf("Metadata() error = %v", err)
+			}
+		})
+	}
+}
+
+// The rendered commands must not carry template or shell hazards that the
+// rendering pipeline would silently mangle.
+func TestShippedExampleProvidersRenderCleanCommands(t *testing.T) {
+	files, _ := filepath.Glob("examples/providers/*.json")
+
+	for _, file := range files {
+		t.Run(filepath.Base(file), func(t *testing.T) {
+			cfg, err := vault.LoadConfigJSON(file)
+			if err != nil {
+				t.Fatalf("LoadConfigJSON() error = %v", err)
+			}
+
+			for name, op := range map[string]vault.CommandConfig{
+				"get":      cfg.External.Get,
+				"set":      cfg.External.Set,
+				"delete":   cfg.External.Delete,
+				"list":     cfg.External.List,
+				"exists":   cfg.External.Exists,
+				"metadata": cfg.External.Metadata,
+			} {
+				if op.CommandTemplate == "" {
+					continue
+				}
+				// A backtick anywhere breaks the expression template, which
+				// wraps expressions in backticks itself.
+				if strings.Contains(op.CommandTemplate, "`") {
+					t.Errorf("%s command contains a backtick, which the template engine cannot carry", name)
+				}
+			}
+		})
+	}
+}
diff --git a/external.go b/external.go
index 5b57d73..d38f0a7 100644
--- a/external.go
+++ b/external.go
@@ -16,23 +16,33 @@ import (
 )
 
 type ExternalVaultProvider struct {
-	ctx     context.Context
 	mu      sync.RWMutex
 	id      string
 	execute func(ctx context.Context, cmd, input, dir string, envList []string) (string, error)
 
+	ctx     context.Context
+	timeout time.Duration
+	closed  bool
+
 	cfg *ExternalConfig
 }
 
 func NewExternalVaultProvider(cfg *Config) (*ExternalVaultProvider, error) {
 	if cfg.External == nil {
-		return nil, fmt.Errorf("external configuration is required")
+		return nil, fmt.Errorf("%w: external configuration is required", ErrInvalidConfig)
 	}
+	if err := cfg.Validate(); err != nil {
+		return nil, err
+	}
+
+	// Timeout is validated by ExternalConfig.Validate, so this cannot fail here.
+	timeout, _ := cfg.External.timeoutDuration()
 
 	vault := &ExternalVaultProvider{
 		ctx:     context.Background(),
 		id:      cfg.ID,
 		cfg:     cfg.External,
+		timeout: timeout,
 		execute: execute,
 	}
 
@@ -43,16 +53,41 @@ func (v *ExternalVaultProvider) ID() string {
 	return v.id
 }
 
+// SetContext replaces the context used for command execution, allowing callers to
+// cancel in-flight operations. The Provider interface does not thread a context
+// through its methods, so this is the supported way to make external commands
+// cancellable.
+func (v *ExternalVaultProvider) SetContext(ctx context.Context) {
+	if ctx == nil {
+		ctx = context.Background()
+	}
+	v.mu.Lock()
+	defer v.mu.Unlock()
+	v.ctx = ctx
+}
+
 func (v *ExternalVaultProvider) GetSecret(key string) (Secret, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	return v.getSecretLocked(key)
+}
+
+// getSecretLocked implements GetSecret and assumes the caller already holds at
+// least a read lock. HasSecret needs to delegate here rather than to GetSecret:
+// sync.RWMutex does not support recursive read locking, so a writer arriving
+// between the two RLock calls would deadlock both.
+func (v *ExternalVaultProvider) getSecretLocked(key string) (Secret, error) {
 	if err := ValidateSecretKey(key); err != nil {
 		return nil, err
 	}
 
+	if v.closed {
+		return nil, ErrVaultClosed
+	}
+
 	if v.cfg.Get.CommandTemplate == "" {
-		return nil, fmt.Errorf("get operation not configured")
+		return nil, fmt.Errorf("%w: get operation not configured", ErrInvalidConfig)
 	}
 
 	cmd, err := v.renderCmdTemplate(v.cfg.Get.CommandTemplate, key)
@@ -94,26 +129,38 @@ func (v *ExternalVaultProvider) SetSecret(key string, value Secret) error {
 		return err
 	}
 
+	if v.closed {
+		return ErrVaultClosed
+	}
+
 	if v.cfg.Set.CommandTemplate == "" {
-		return fmt.Errorf("set operation not configured")
+		return fmt.Errorf("%w: set operation not configured", ErrInvalidConfig)
 	}
 
-	cmd, err := v.renderCmdTemplateWithValue(v.cfg.Set.CommandTemplate, key, value.PlainTextString())
+	// The secret value is deliberately not available to the command template. The
+	// rendered command is parsed and run by a shell, and the template engine does
+	// no quoting, so interpolating a secret there is a command-injection sink and
+	// silently corrupts any value containing shell metacharacters. Values travel
+	// over stdin via the set input template instead. ExternalConfig.Validate
+	// rejects configs that still reference {{ value }} in the set command.
+	cmd, err := v.renderCmdTemplate(v.cfg.Set.CommandTemplate, key)
 	if err != nil {
 		return fmt.Errorf("failed to render set cmd: %w", err)
 	}
 
 	var input string
 	if v.cfg.Set.InputTemplate != "" {
-		input, err = v.renderInputTemplate(v.cfg.Get.InputTemplate, key)
+		input, err = v.renderInputTemplateWithValue(v.cfg.Set.InputTemplate, key, value.PlainTextString())
 		if err != nil {
 			return fmt.Errorf("failed to render input template: %w", err)
 		}
 	}
 
-	out, err := v.executeCommand(cmd, input)
-	if err != nil {
-		return fmt.Errorf("failed to set secret: %w stdErr: %s", err, out)
+	// The error from executeCommand already carries stderr. Do not add the command
+	// output here: a backend that echoes the failing command back would put the
+	// secret value into the returned error string.
+	if _, err := v.executeCommand(cmd, input); err != nil {
+		return fmt.Errorf("failed to set secret: %w", err)
 	}
 
 	return nil
@@ -127,8 +174,12 @@ func (v *ExternalVaultProvider) DeleteSecret(key string) error {
 		return err
 	}
 
+	if v.closed {
+		return ErrVaultClosed
+	}
+
 	if v.cfg.Delete.CommandTemplate == "" {
-		return fmt.Errorf("delete operation not configured")
+		return fmt.Errorf("%w: delete operation not configured", ErrInvalidConfig)
 	}
 
 	cmd, err := v.renderCmdTemplate(v.cfg.Delete.CommandTemplate, key)
@@ -138,7 +189,7 @@ func (v *ExternalVaultProvider) DeleteSecret(key string) error {
 
 	var input string
 	if v.cfg.Delete.InputTemplate != "" {
-		input, err = v.renderInputTemplate(v.cfg.Get.InputTemplate, key)
+		input, err = v.renderInputTemplate(v.cfg.Delete.InputTemplate, key)
 		if err != nil {
 			return fmt.Errorf("failed to render input template: %w", err)
 		}
@@ -155,8 +206,12 @@ func (v *ExternalVaultProvider) ListSecrets() ([]string, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if v.closed {
+		return nil, ErrVaultClosed
+	}
+
 	if v.cfg.List.CommandTemplate == "" {
-		return nil, fmt.Errorf("list operation not configured")
+		return nil, fmt.Errorf("%w: list operation not configured", ErrInvalidConfig)
 	}
 
 	cmd, err := v.renderCmdTemplate(v.cfg.List.CommandTemplate, "")
@@ -166,7 +221,7 @@ func (v *ExternalVaultProvider) ListSecrets() ([]string, error) {
 
 	var input string
 	if v.cfg.List.InputTemplate != "" {
-		input, err = v.renderInputTemplate(v.cfg.Get.InputTemplate, "")
+		input, err = v.renderInputTemplate(v.cfg.List.InputTemplate, "")
 		if err != nil {
 			return nil, fmt.Errorf("failed to render input template: %w", err)
 		}
@@ -196,7 +251,7 @@ func (v *ExternalVaultProvider) ListSecrets() ([]string, error) {
 		sep = "\n"
 	}
 	secrets := strings.Split(secretsList, sep)
-	var result []string
+	result := make([]string, 0, len(secrets))
 	for _, secret := range secrets {
 		secret = strings.TrimSpace(secret)
 		if secret != "" {
@@ -215,94 +270,127 @@ func (v *ExternalVaultProvider) HasSecret(key string) (bool, error) {
 		return false, err
 	}
 
+	if v.closed {
+		return false, ErrVaultClosed
+	}
+
 	if v.cfg.Exists.CommandTemplate != "" {
-		cmd, err := v.renderCmdTemplate(v.cfg.Exists.CommandTemplate, key)
-		if err != nil {
-			return false, fmt.Errorf("failed to render exists cmd: %w", err)
-		}
+		return v.hasSecretViaExistsCmd(key)
+	}
 
-		var input string
-		if v.cfg.Exists.InputTemplate != "" {
-			input, err = v.renderInputTemplate(v.cfg.Exists.InputTemplate, key)
-			if err != nil {
-				return false, fmt.Errorf("failed to render input template: %w", err)
-			}
+	if _, err := v.getSecretLocked(key); err != nil {
+		if v.isNotFoundErr(err) {
+			return false, nil
 		}
-
-		_, err = v.executeCommand(cmd, input)
-		// typically, exists commands return non-zero exit code if secret doesn't exist
-		return err == nil, nil
+		return false, err
 	}
+	return true, nil
+}
 
-	_, err := v.GetSecret(key)
+func (v *ExternalVaultProvider) hasSecretViaExistsCmd(key string) (bool, error) {
+	cmd, err := v.renderCmdTemplate(v.cfg.Exists.CommandTemplate, key)
 	if err != nil {
-		if strings.Contains(err.Error(), "not found") ||
-			strings.Contains(err.Error(), "not exist") ||
-			strings.Contains(err.Error(), "not in") {
-			return false, nil
+		return false, fmt.Errorf("failed to render exists cmd: %w", err)
+	}
+
+	var input string
+	if v.cfg.Exists.InputTemplate != "" {
+		input, err = v.renderInputTemplate(v.cfg.Exists.InputTemplate, key)
+		if err != nil {
+			return false, fmt.Errorf("failed to render input template: %w", err)
 		}
+	}
+
+	if _, err = v.executeCommand(cmd, input); err == nil {
+		return true, nil
+	}
+
+	// A non-zero exit conventionally means "absent", but it is also how an expired
+	// session, a network failure, or a permissions problem surfaces. NotFoundPattern
+	// lets a config say which failures actually mean absence.
+	if v.cfg.NotFoundPattern != "" && !strings.Contains(err.Error(), v.cfg.NotFoundPattern) {
 		return false, err
 	}
-	return true, nil
+	return false, nil
+}
+
+func (v *ExternalVaultProvider) isNotFoundErr(err error) bool {
+	if v.cfg.NotFoundPattern != "" {
+		return strings.Contains(err.Error(), v.cfg.NotFoundPattern)
+	}
+	msg := err.Error()
+	return strings.Contains(msg, "not found") ||
+		strings.Contains(msg, "not exist") ||
+		strings.Contains(msg, "not in")
 }
 
 func (v *ExternalVaultProvider) Close() error {
+	v.mu.Lock()
+	defer v.mu.Unlock()
+
+	v.closed = true
 	return nil
 }
 
 func (v *ExternalVaultProvider) SetExecutionFunc(
 	fn func(ctx context.Context, cmd, input, dir string, envList []string) (string, error),
 ) {
+	v.mu.Lock()
+	defer v.mu.Unlock()
 	v.execute = fn
 }
 
-func (v *ExternalVaultProvider) Metadata() Metadata {
+func (v *ExternalVaultProvider) Metadata() (Metadata, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if v.closed {
+		return Metadata{}, ErrVaultClosed
+	}
+
 	if v.cfg.Metadata.CommandTemplate == "" {
-		return Metadata{}
+		return Metadata{}, nil
 	}
 
 	cmd, err := v.renderCmdTemplate(v.cfg.Metadata.CommandTemplate, "")
 	if err != nil {
-		return Metadata{}
+		return Metadata{}, fmt.Errorf("failed to render metadata cmd: %w", err)
 	}
+
 	var input string
-	if v.cfg.List.InputTemplate != "" {
+	if v.cfg.Metadata.InputTemplate != "" {
 		input, err = v.renderInputTemplate(v.cfg.Metadata.InputTemplate, "")
 		if err != nil {
-			return Metadata{}
+			return Metadata{}, fmt.Errorf("failed to render input template: %w", err)
 		}
 	}
 
 	output, err := v.executeCommand(cmd, input)
 	if err != nil {
-		return Metadata{}
+		return Metadata{}, fmt.Errorf("failed to read metadata: %w", err)
 	}
 
 	var metadataOutput string
 	if v.cfg.Metadata.OutputTemplate != "" {
 		metadataOutput, err = v.renderOutputTemplate(v.cfg.Metadata.OutputTemplate, output)
 		if err != nil {
-			return Metadata{}
+			return Metadata{}, fmt.Errorf("failed to parse metadata output: %w", err)
 		}
 	} else {
 		metadataOutput = strings.TrimSpace(output)
 	}
 
-	return Metadata{RawData: metadataOutput}
+	return Metadata{RawData: metadataOutput}, nil
 }
 
 func (v *ExternalVaultProvider) executeCommand(cmd, input string) (string, error) {
 	ctx := v.ctx
-	if v.cfg.Timeout != "" {
+	if ctx == nil {
+		ctx = context.Background()
+	}
+	if v.timeout > 0 {
 		var cancel context.CancelFunc
-		dur, parseErr := time.ParseDuration(v.cfg.Timeout)
-		if parseErr != nil {
-			return "", fmt.Errorf("invalid timeout duration: %w", parseErr)
-		}
-		ctx, cancel = context.WithTimeout(v.ctx, dur)
+		ctx, cancel = context.WithTimeout(ctx, v.timeout)
 		defer cancel()
 	}
 
@@ -315,103 +403,71 @@ func (v *ExternalVaultProvider) executeCommand(cmd, input string) (string, error
 }
 
 func (v *ExternalVaultProvider) environmentToSlice() []string {
-	var envSlice []string
-	for key, value := range expandEnv(v.cfg.Environment) {
+	expanded := expandEnv(v.cfg.Environment)
+	envSlice := make([]string, 0, len(expanded))
+	for key, value := range expanded {
 		envSlice = append(envSlice, fmt.Sprintf("%s=%s", key, value))
 	}
 	return envSlice
 }
 
-func (v *ExternalVaultProvider) renderCmdTemplate(template, key string) (string, error) {
-	data := map[string]interface{}{
-		"env":      expandEnv(v.cfg.Environment),
-		"key":      key,
-		"ref":      key,
-		"id":       key,
-		"name":     key,
-		"template": template,
+// templateData is the variable set shared by the command and input templates.
+// The secret value is intentionally absent; only renderInputTemplateWithValue
+// adds it.
+func (v *ExternalVaultProvider) templateData(key string) map[string]interface{} {
+	return map[string]interface{}{
+		"env":  expandEnv(v.cfg.Environment),
+		"key":  key,
+		"ref":  key,
+		"id":   key,
+		"name": key,
 	}
+}
 
-	template = os.ExpandEnv(template)
-	tmpl := expression.NewTemplate(fmt.Sprintf("%s-args-template", v.id), data)
-	err := tmpl.Parse(template)
-	if err != nil {
-		return "", fmt.Errorf("parsing args template: %w", err)
+func (v *ExternalVaultProvider) render(name, template string, data map[string]interface{}) (string, error) {
+	// os.ExpandEnv is deliberately not applied here. It runs before the shell
+	// parses the command, so it destroys $VAR, ${VAR}, $1, $? and $@, makes a
+	// literal $ unwritable, and applies substitution before quoting -- the wrong
+	// order for injection safety. execute() already appends the configured
+	// environment to os.Environ(), so the interpreter resolves $VAR itself, with
+	// correct quoting semantics and with cfg.Environment actually in scope.
+	tmpl := expression.NewTemplate(fmt.Sprintf("%s-%s-template", v.id, name), data)
+	if err := tmpl.Parse(template); err != nil {
+		return "", fmt.Errorf("parsing %s template: %w", name, err)
 	}
 
 	result, err := tmpl.ExecuteToString()
 	if err != nil {
-		return "", fmt.Errorf("evaluating args template: %w", err)
+		return "", fmt.Errorf("evaluating %s template: %w", name, err)
 	}
 	return result, nil
 }
 
-func (v *ExternalVaultProvider) renderCmdTemplateWithValue(template, key, value string) (string, error) {
-	data := map[string]interface{}{
-		"env":      expandEnv(v.cfg.Environment),
-		"key":      key,
-		"ref":      key,
-		"id":       key,
-		"name":     key,
-		"value":    value,
-		"password": value,
-		"template": template,
-	}
-
-	template = os.ExpandEnv(template)
-	tmpl := expression.NewTemplate(fmt.Sprintf("%s-args-template", v.id), data)
-	err := tmpl.Parse(template)
-	if err != nil {
-		return "", fmt.Errorf("parsing args template: %w", err)
-	}
-
-	result, err := tmpl.ExecuteToString()
-	if err != nil {
-		return "", fmt.Errorf("evaluating args template: %w", err)
-	}
-	return result, nil
+func (v *ExternalVaultProvider) renderCmdTemplate(template, key string) (string, error) {
+	return v.render("args", template, v.templateData(key))
 }
 
 func (v *ExternalVaultProvider) renderInputTemplate(template, input string) (string, error) {
-	data := map[string]interface{}{
-		"env":      expandEnv(v.cfg.Environment),
-		"input":    input,
-		"template": template,
-	}
-
-	template = os.ExpandEnv(template)
-	tmpl := expression.NewTemplate(fmt.Sprintf("%s-input-template", v.id), data)
-	err := tmpl.Parse(template)
-	if err != nil {
-		return "", fmt.Errorf("parsing input template: %w", err)
-	}
+	return v.renderInputTemplateWithValue(template, input, "")
+}
 
-	result, err := tmpl.ExecuteToString()
-	if err != nil {
-		return "", fmt.Errorf("evaluating input template: %w", err)
-	}
-	return result, nil
+func (v *ExternalVaultProvider) renderInputTemplateWithValue(template, input, value string) (string, error) {
+	data := v.templateData(input)
+	data["input"] = input
+	data["value"] = value
+	data["password"] = value
+	return v.render("input", template, data)
 }
 
 func (v *ExternalVaultProvider) renderOutputTemplate(template, output string) (string, error) {
 	data := map[string]interface{}{
-		"env":      expandEnv(v.cfg.Environment),
-		"output":   output,
-		"template": template,
-	}
-
-	template = os.ExpandEnv(template)
-	tmpl := expression.NewTemplate(fmt.Sprintf("%s-output-template", v.id), data)
-	err := tmpl.Parse(template)
-	if err != nil {
-		return "", fmt.Errorf("parsing output template: %w", err)
+		"env":    expandEnv(v.cfg.Environment),
+		"output": output,
 	}
 
-	result, err := tmpl.ExecuteToString()
-	if err != nil {
-		return "", fmt.Errorf("evaluating output template: %w", err)
-	}
-	return result, nil
+	// Unlike command templates, output templates are never handed to a shell, so
+	// environment expansion here is safe and preserved for compatibility.
+	return v.render("output", os.ExpandEnv(template), data)
 }
 
 func execute(ctx context.Context, cmd, input, dir string, envList []string) (string, error) {
@@ -455,18 +511,25 @@ func execute(ctx context.Context, cmd, input, dir string, envList []string) (str
 		}
 		return stdErrBuffer.String(), fmt.Errorf("encountered an error executing command - %w", err)
 	}
-	output := stdOutBuffer.String()
-	if stderr := stdErrBuffer.String(); stderr != "" {
-		output += "\n" + stderr
-	}
-	return strings.TrimSpace(output), nil
+
+	// Only stdout is the result. Merging stderr in on success concatenates any
+	// warning the backend emits (e.g. "gpg: WARNING: unsafe permissions") onto
+	// the secret value itself. stderr is still returned on the error path above.
+	return strings.TrimSpace(stdOutBuffer.String()), nil
 }
 
+// expandEnv returns a new map with environment references expanded. It must not
+// mutate the input: the caller's map is the shared provider config, and the read
+// paths (GetSecret, ListSecrets, HasSecret, Metadata) hold only a read lock, so
+// writing to it concurrently is an unrecoverable "concurrent map writes" fault.
 func expandEnv(env map[string]string) map[string]string {
+	out := make(map[string]string, len(env))
 	for k, v := range env {
 		if strings.Contains(v, "$") || strings.Contains(v, "{") {
-			env[k] = os.ExpandEnv(v)
+			out[k] = os.ExpandEnv(v)
+		} else {
+			out[k] = v
 		}
 	}
-	return env
+	return out
 }
diff --git a/external_test.go b/external_test.go
index 54a967c..3319273 100644
--- a/external_test.go
+++ b/external_test.go
@@ -2,13 +2,61 @@ package vault_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"os"
+	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/flowexec/vault"
 )
 
+const testSecretValue = "s3cr3t"
+
+// validExternalConfig returns a config that satisfies ExternalConfig.Validate.
+// Get and Set command templates are mandatory, so every fixture needs them even
+// when the test only exercises another operation.
+func validExternalConfig() *vault.ExternalConfig {
+	return &vault.ExternalConfig{
+		Get: vault.CommandConfig{CommandTemplate: "vault kv get -format=json {{key}}"},
+		Set: vault.CommandConfig{CommandTemplate: "vault kv put {{key}}"},
+	}
+}
+
+func newTestProvider(t *testing.T, cfg *vault.ExternalConfig) *vault.ExternalVaultProvider {
+	t.Helper()
+	provider, err := vault.NewExternalVaultProvider(&vault.Config{
+		ID:       "test-vault",
+		Type:     vault.ProviderTypeExternal,
+		External: cfg,
+	})
+	if err != nil {
+		t.Fatalf("Failed to create provider: %v", err)
+	}
+	return provider
+}
+
+// execCapture records what the provider actually asked the shell to run. The
+// older mockCommandContext observes neither cmd nor input, so it cannot catch a
+// provider rendering the wrong template or leaking a secret into a command.
+type execCapture struct {
+	cmd, input, dir string
+	env             []string
+	calls           int
+}
+
+func capturingExec(c *execCapture, out string, err error) func(
+	context.Context, string, string, string, []string,
+) (string, error) {
+	return func(_ context.Context, cmd, input, dir string, envList []string) (string, error) {
+		c.cmd, c.input, c.dir, c.env = cmd, input, dir, envList
+		c.calls++
+		return out, err
+	}
+}
+
 func TestNewExternalVaultProvider(t *testing.T) {
 	tests := []struct {
 		name    string
@@ -18,16 +66,9 @@ func TestNewExternalVaultProvider(t *testing.T) {
 		{
 			name: "valid config",
 			config: &vault.Config{
-				ID:   "test-vault",
-				Type: vault.ProviderTypeExternal,
-				External: &vault.ExternalConfig{
-					Get: vault.CommandConfig{
-						CommandTemplate: "vault kv get -format=json {{key}}",
-					},
-					Set: vault.CommandConfig{
-						CommandTemplate: "vault kv put {{key}} value={{value}}",
-					},
-				},
+				ID:       "test-vault",
+				Type:     vault.ProviderTypeExternal,
+				External: validExternalConfig(),
 			},
 			wantErr: false,
 		},
@@ -39,6 +80,28 @@ func TestNewExternalVaultProvider(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "missing get and set templates",
+			config: &vault.Config{
+				ID:       "test-vault",
+				Type:     vault.ProviderTypeExternal,
+				External: &vault.ExternalConfig{},
+			},
+			wantErr: true,
+		},
+		{
+			name: "invalid timeout",
+			config: &vault.Config{
+				ID:   "test-vault",
+				Type: vault.ProviderTypeExternal,
+				External: func() *vault.ExternalConfig {
+					c := validExternalConfig()
+					c.Timeout = "not-a-duration"
+					return c
+				}(),
+			},
+			wantErr: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -48,240 +111,271 @@ func TestNewExternalVaultProvider(t *testing.T) {
 				t.Errorf("NewExternalVaultProvider() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-			if !tt.wantErr && provider == nil {
-				t.Error("NewExternalVaultProvider() returned nil provider")
+			if tt.wantErr {
+				return
+			}
+			if provider == nil {
+				t.Fatal("NewExternalVaultProvider() returned nil provider")
 			}
-			if !tt.wantErr && provider.ID() != tt.config.ID {
+			if provider.ID() != tt.config.ID {
 				t.Errorf("NewExternalVaultProvider() ID = %v, want %v", provider.ID(), tt.config.ID)
 			}
 		})
 	}
 }
 
-func TestExternalVaultProvider_GetSecret(t *testing.T) {
-	config := &vault.Config{
-		ID:   "test-vault",
-		Type: vault.ProviderTypeExternal,
-		External: &vault.ExternalConfig{
-			Get: vault.CommandConfig{
-				CommandTemplate: "vault kv get -format=json {{key}}",
-			},
-		},
+// A secret interpolated into a command template is a command-injection sink: the
+// rendered string is parsed and run by a shell and the template engine does no
+// quoting. Configs that try must be rejected at load, not silently accepted.
+func TestConfigRejectsSecretValueInCommandTemplates(t *testing.T) {
+	for _, tmpl := range []string{
+		"vault kv put {{key}} value={{value}}",
+		"vault kv put {{key}} value={{ value }}",
+		"vault kv put {{key}} pw={{password}}",
+	} {
+		cfg := validExternalConfig()
+		cfg.Set.CommandTemplate = tmpl
+
+		_, err := vault.NewExternalVaultProvider(&vault.Config{
+			ID: "test-vault", Type: vault.ProviderTypeExternal, External: cfg,
+		})
+		if err == nil {
+			t.Errorf("template %q was accepted, want rejection", tmpl)
+			continue
+		}
+		if !errors.Is(err, vault.ErrInvalidConfig) {
+			t.Errorf("template %q: error = %v, want ErrInvalidConfig", tmpl, err)
+		}
 	}
+}
 
-	provider, err := vault.NewExternalVaultProvider(config)
-	if err != nil {
-		t.Fatalf("Failed to create provider: %v", err)
-	}
+func TestSetSecret_ValueTravelsOverStdinNotTheCommand(t *testing.T) {
+	cfg := validExternalConfig()
+	cfg.Set.CommandTemplate = "store {{key}}"
+	cfg.Set.InputTemplate = "{{ value }}"
+	// A Get input template that must NOT be used for the set operation.
+	cfg.Get.InputTemplate = "WRONG-TEMPLATE"
 
-	tests := []struct {
-		name          string
-		key           string
-		mockOutputs   map[string]string
-		mockErrors    map[string]error
-		wantSecret    string
-		wantErr       bool
-		errorContains string
-	}{
-		{
-			name: "successful get",
-			key:  "test-key",
-			mockOutputs: map[string]string{
-				"vault kv get -format=json test-key": "secret-value",
-			},
-			wantSecret: "secret-value",
-			wantErr:    false,
-		},
-		{
-			name: "command fails",
-			key:  "test-key",
-			mockErrors: map[string]error{
-				"vault kv get -format=json test-key": fmt.Errorf("command failed"),
-			},
-			wantErr:       true,
-			errorContains: "failed to get secret",
-		},
-		{
-			name:          "invalid key",
-			key:           "",
-			wantErr:       true,
-			errorContains: "invalid secret key",
-		},
+	provider := newTestProvider(t, cfg)
+	rec := &execCapture{}
+	provider.SetExecutionFunc(capturingExec(rec, "", nil))
+
+	if err := provider.SetSecret("test-key", vault.NewSecretValue([]byte(testSecretValue))); err != nil {
+		t.Fatalf("SetSecret() error = %v", err)
 	}
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			testProvider := provider
-			if tt.name == "get operation not configured" {
-				testConfig := &vault.Config{
-					ID:       "test-vault",
-					Type:     vault.ProviderTypeExternal,
-					External: &vault.ExternalConfig{},
-				}
-				var err error
-				testProvider, err = vault.NewExternalVaultProvider(testConfig)
-				if err != nil {
-					t.Fatalf("Failed to create test provider: %v", err)
-				}
-			}
+	if rec.input != testSecretValue {
+		t.Errorf("stdin = %q, want %q (set input template was not rendered)", rec.input, testSecretValue)
+	}
+	if strings.Contains(rec.cmd, testSecretValue) {
+		t.Errorf("secret leaked into the command string: %q", rec.cmd)
+	}
+	if rec.cmd != "store test-key" {
+		t.Errorf("cmd = %q, want %q", rec.cmd, "store test-key")
+	}
+}
 
-			testProvider.SetExecutionFunc(mockCommandContext(tt.mockOutputs, tt.mockErrors))
+// A value containing shell metacharacters must round-trip byte-exact. Before the
+// fix, "p@$$w0rd" had $$ expanded to the PID and a different secret was stored.
+func TestSetSecret_ValueWithShellMetacharactersIsUntouched(t *testing.T) {
+	for _, value := range []string{
+		`p@$$w0rd`,
+		`correct horse battery`,
+		`hunter2; echo pwned`,
+		"back`tick`",
+		`quote'and"quote`,
+		"multi\nline",
+	} {
+		cfg := validExternalConfig()
+		cfg.Set.CommandTemplate = "store {{key}}"
+		cfg.Set.InputTemplate = "{{ value }}"
+
+		provider := newTestProvider(t, cfg)
+		rec := &execCapture{}
+		provider.SetExecutionFunc(capturingExec(rec, "", nil))
+
+		if err := provider.SetSecret("k", vault.NewSecretValue([]byte(value))); err != nil {
+			t.Fatalf("SetSecret(%q) error = %v", value, err)
+		}
+		if rec.input != value {
+			t.Errorf("stdin = %q, want %q", rec.input, value)
+		}
+		if strings.Contains(rec.cmd, value) {
+			t.Errorf("value %q leaked into command %q", value, rec.cmd)
+		}
+	}
+}
 
-			secret, err := testProvider.GetSecret(tt.key)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("GetSecret() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
+func TestSetSecret_InputTemplateSeesTheKey(t *testing.T) {
+	cfg := validExternalConfig()
+	cfg.Set.CommandTemplate = "store"
+	cfg.Set.InputTemplate = "{{ key }}:{{ value }}"
 
-			if err != nil && tt.errorContains != "" {
-				if !strings.Contains(err.Error(), tt.errorContains) {
-					t.Errorf("GetSecret() error = %v, want error containing %v", err, tt.errorContains)
-				}
-				return
-			}
+	provider := newTestProvider(t, cfg)
+	rec := &execCapture{}
+	provider.SetExecutionFunc(capturingExec(rec, "", nil))
 
-			if !tt.wantErr && secret.PlainTextString() != tt.wantSecret {
-				t.Errorf("GetSecret() secret = %v, want %v", secret.PlainTextString(), tt.wantSecret)
-			}
-		})
+	if err := provider.SetSecret("my-key", vault.NewSecretValue([]byte(testSecretValue))); err != nil {
+		t.Fatalf("SetSecret() error = %v", err)
+	}
+	if want := "my-key:" + testSecretValue; rec.input != want {
+		t.Errorf("stdin = %q, want %q", rec.input, want)
 	}
 }
 
-func TestExternalVaultProvider_SetSecret(t *testing.T) {
-	config := &vault.Config{
-		ID:   "test-vault",
-		Type: vault.ProviderTypeExternal,
-		External: &vault.ExternalConfig{
-			Set: vault.CommandConfig{
-				CommandTemplate: "vault kv put {{key}} value={{value}}",
-			},
-		},
-	}
+// Each operation must render its own input template. All of these previously
+// rendered Get's template instead.
+func TestOperationsRenderTheirOwnInputTemplate(t *testing.T) {
+	t.Run("delete", func(t *testing.T) {
+		cfg := validExternalConfig()
+		cfg.Get.InputTemplate = "WRONG"
+		cfg.Delete.CommandTemplate = "rm {{key}}"
+		cfg.Delete.InputTemplate = "delete:{{ input }}"
+
+		provider := newTestProvider(t, cfg)
+		rec := &execCapture{}
+		provider.SetExecutionFunc(capturingExec(rec, "", nil))
+
+		if err := provider.DeleteSecret("k"); err != nil {
+			t.Fatalf("DeleteSecret() error = %v", err)
+		}
+		if rec.input != "delete:k" {
+			t.Errorf("stdin = %q, want %q", rec.input, "delete:k")
+		}
+	})
 
-	provider, err := vault.NewExternalVaultProvider(config)
-	if err != nil {
-		t.Fatalf("Failed to create provider: %v", err)
+	t.Run("list", func(t *testing.T) {
+		cfg := validExternalConfig()
+		cfg.Get.InputTemplate = "WRONG"
+		cfg.List.CommandTemplate = "ls"
+		cfg.List.InputTemplate = "list-input"
+
+		provider := newTestProvider(t, cfg)
+		rec := &execCapture{}
+		provider.SetExecutionFunc(capturingExec(rec, "a\nb", nil))
+
+		if _, err := provider.ListSecrets(); err != nil {
+			t.Fatalf("ListSecrets() error = %v", err)
+		}
+		if rec.input != "list-input" {
+			t.Errorf("stdin = %q, want %q", rec.input, "list-input")
+		}
+	})
+
+	// Metadata previously gated on List's input template, so a configured
+	// metadata input was ignored unless list.input happened to be set too.
+	t.Run("metadata", func(t *testing.T) {
+		cfg := validExternalConfig()
+		cfg.Metadata.CommandTemplate = "status"
+		cfg.Metadata.InputTemplate = "meta-input"
+
+		provider := newTestProvider(t, cfg)
+		rec := &execCapture{}
+		provider.SetExecutionFunc(capturingExec(rec, "ok", nil))
+
+		if _, err := provider.Metadata(); err != nil {
+			t.Fatalf("Metadata() error = %v", err)
+		}
+		if rec.input != "meta-input" {
+			t.Errorf("stdin = %q, want %q", rec.input, "meta-input")
+		}
+	})
+}
+
+// Configs written against the old behaviour set only Get.InputTemplate; that
+// must keep working.
+func TestGetInputTemplateBackCompat(t *testing.T) {
+	cfg := validExternalConfig()
+	cfg.Get.InputTemplate = "{{ input }}"
+
+	provider := newTestProvider(t, cfg)
+	rec := &execCapture{}
+	provider.SetExecutionFunc(capturingExec(rec, "value", nil))
+
+	if _, err := provider.GetSecret("my-key"); err != nil {
+		t.Fatalf("GetSecret() error = %v", err)
 	}
+	if rec.input != "my-key" {
+		t.Errorf("stdin = %q, want %q", rec.input, "my-key")
+	}
+}
 
+func TestExternalVaultProvider_GetSecret(t *testing.T) {
 	tests := []struct {
 		name          string
 		key           string
-		value         string
-		mockOutputs   map[string]string
-		mockErrors    map[string]error
+		out           string
+		execErr       error
+		wantSecret    string
 		wantErr       bool
 		errorContains string
 	}{
+		{name: "successful get", key: "test-key", out: "secret-value", wantSecret: "secret-value"},
 		{
-			name:  "successful set",
-			key:   "test-key",
-			value: "test-value",
-			mockOutputs: map[string]string{
-				"vault kv put test-key value=test-value": "success",
-			},
-			wantErr: false,
-		},
-		{
-			name:  "command fails",
-			key:   "test-key",
-			value: "test-value",
-			mockErrors: map[string]error{
-				"vault kv put test-key value=test-value": fmt.Errorf("command failed"),
-			},
-			wantErr:       true,
-			errorContains: "failed to set secret",
+			name: "command fails", key: "test-key", execErr: fmt.Errorf("command failed"),
+			wantErr: true, errorContains: "failed to get secret",
 		},
+		{name: "invalid key", key: "", wantErr: true, errorContains: "invalid secret key"},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			testProvider := provider
-			testProvider.SetExecutionFunc(mockCommandContext(tt.mockOutputs, tt.mockErrors))
+			provider := newTestProvider(t, validExternalConfig())
+			provider.SetExecutionFunc(capturingExec(&execCapture{}, tt.out, tt.execErr))
 
-			secret := vault.NewSecretValue([]byte(tt.value))
-			err := testProvider.SetSecret(tt.key, secret)
+			secret, err := provider.GetSecret(tt.key)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("SetSecret() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("GetSecret() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-
-			if err != nil && tt.errorContains != "" {
-				if !strings.Contains(err.Error(), tt.errorContains) {
-					t.Errorf("SetSecret() error = %v, want error containing %v", err, tt.errorContains)
+			if err != nil {
+				if tt.errorContains != "" && !strings.Contains(err.Error(), tt.errorContains) {
+					t.Errorf("GetSecret() error = %v, want error containing %v", err, tt.errorContains)
 				}
+				return
+			}
+			if secret.PlainTextString() != tt.wantSecret {
+				t.Errorf("GetSecret() secret = %v, want %v", secret.PlainTextString(), tt.wantSecret)
 			}
 		})
 	}
 }
 
 func TestExternalVaultProvider_ListSecrets(t *testing.T) {
-	config := &vault.Config{
-		ID:   "test-vault",
-		Type: vault.ProviderTypeExternal,
-		External: &vault.ExternalConfig{
-			List: vault.CommandConfig{
-				CommandTemplate: "vault kv list",
-			},
-		},
-	}
-
-	provider, err := vault.NewExternalVaultProvider(config)
-	if err != nil {
-		t.Fatalf("Failed to create provider: %v", err)
-	}
-
 	tests := []struct {
 		name        string
-		mockOutputs map[string]string
-		mockErrors  map[string]error
+		out         string
+		execErr     error
 		wantSecrets []string
 		wantErr     bool
 	}{
-		{
-			name: "successful list",
-			mockOutputs: map[string]string{
-				"vault kv list": "secret1\nsecret2\nsecret3",
-			},
-			wantSecrets: []string{"secret1", "secret2", "secret3"},
-			wantErr:     false,
-		},
-		{
-			name: "empty list",
-			mockOutputs: map[string]string{
-				"vault kv list": "",
-			},
-			wantSecrets: []string{},
-			wantErr:     false,
-		},
-		{
-			name: "command fails",
-			mockErrors: map[string]error{
-				"vault kv list": fmt.Errorf("command failed"),
-			},
-			wantErr: true,
-		},
+		{name: "successful list", out: "secret1\nsecret2\nsecret3", wantSecrets: []string{"secret1", "secret2", "secret3"}},
+		{name: "empty list", out: "", wantSecrets: []string{}},
+		{name: "command fails", execErr: fmt.Errorf("command failed"), wantErr: true},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			testProvider := provider
-			testProvider.SetExecutionFunc(mockCommandContext(tt.mockOutputs, tt.mockErrors))
+			cfg := validExternalConfig()
+			cfg.List.CommandTemplate = "vault kv list"
+
+			provider := newTestProvider(t, cfg)
+			provider.SetExecutionFunc(capturingExec(&execCapture{}, tt.out, tt.execErr))
 
-			secrets, err := testProvider.ListSecrets()
+			secrets, err := provider.ListSecrets()
 			if (err != nil) != tt.wantErr {
 				t.Errorf("ListSecrets() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-
-			if !tt.wantErr {
-				if len(secrets) != len(tt.wantSecrets) {
-					t.Errorf("ListSecrets() returned %d secrets, want %d", len(secrets), len(tt.wantSecrets))
-					return
-				}
-				for i, secret := range secrets {
-					if secret != tt.wantSecrets[i] {
-						t.Errorf("ListSecrets() secret[%d] = %v, want %v", i, secret, tt.wantSecrets[i])
-					}
+			if tt.wantErr {
+				return
+			}
+			if len(secrets) != len(tt.wantSecrets) {
+				t.Fatalf("ListSecrets() returned %d secrets, want %d", len(secrets), len(tt.wantSecrets))
+			}
+			for i, secret := range secrets {
+				if secret != tt.wantSecrets[i] {
+					t.Errorf("ListSecrets() secret[%d] = %v, want %v", i, secret, tt.wantSecrets[i])
 				}
 			}
 		})
@@ -289,60 +383,28 @@ func TestExternalVaultProvider_ListSecrets(t *testing.T) {
 }
 
 func TestExternalVaultProvider_HasSecret(t *testing.T) {
-	config := &vault.Config{
-		ID:   "test-vault",
-		Type: vault.ProviderTypeExternal,
-		External: &vault.ExternalConfig{
-			Exists: vault.CommandConfig{
-				CommandTemplate: "vault kv get {{key}}",
-			},
-		},
-	}
-
-	provider, err := vault.NewExternalVaultProvider(config)
-	if err != nil {
-		t.Fatalf("Failed to create provider: %v", err)
-	}
-
 	tests := []struct {
-		name        string
-		key         string
-		mockOutputs map[string]string
-		mockErrors  map[string]error
-		wantExists  bool
-		wantErr     bool
+		name       string
+		key        string
+		execErr    error
+		wantExists bool
 	}{
-		{
-			name: "secret exists",
-			key:  "existing-key",
-			mockOutputs: map[string]string{
-				"vault kv get existing-key": "some-value",
-			},
-			wantExists: true,
-			wantErr:    false,
-		},
-		{
-			name: "secret does not exist",
-			key:  "nonexistent-key",
-			mockErrors: map[string]error{
-				"vault kv get nonexistent-key": fmt.Errorf("not found"),
-			},
-			wantExists: false,
-			wantErr:    false,
-		},
+		{name: "secret exists", key: "existing-key", wantExists: true},
+		{name: "secret does not exist", key: "nonexistent-key", execErr: fmt.Errorf("not found"), wantExists: false},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			testProvider := provider
-			testProvider.SetExecutionFunc(mockCommandContext(tt.mockOutputs, tt.mockErrors))
+			cfg := validExternalConfig()
+			cfg.Exists.CommandTemplate = "vault kv get {{key}}"
 
-			exists, err := testProvider.HasSecret(tt.key)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("HasSecret() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
+			provider := newTestProvider(t, cfg)
+			provider.SetExecutionFunc(capturingExec(&execCapture{}, "some-value", tt.execErr))
 
+			exists, err := provider.HasSecret(tt.key)
+			if err != nil {
+				t.Fatalf("HasSecret() error = %v", err)
+			}
 			if exists != tt.wantExists {
 				t.Errorf("HasSecret() = %v, want %v", exists, tt.wantExists)
 			}
@@ -350,83 +412,226 @@ func TestExternalVaultProvider_HasSecret(t *testing.T) {
 	}
 }
 
+// NotFoundPattern separates "absent" from "the backend is broken". Without it,
+// an expired session reports the secret as simply missing.
+func TestHasSecret_NotFoundPatternDistinguishesRealFailures(t *testing.T) {
+	cfg := validExternalConfig()
+	cfg.Exists.CommandTemplate = "check {{key}}"
+	cfg.NotFoundPattern = "ParameterNotFound"
+
+	t.Run("absent", func(t *testing.T) {
+		provider := newTestProvider(t, cfg)
+		provider.SetExecutionFunc(capturingExec(&execCapture{}, "", fmt.Errorf("ParameterNotFound: nope")))
+
+		exists, err := provider.HasSecret("k")
+		if err != nil {
+			t.Fatalf("HasSecret() error = %v", err)
+		}
+		if exists {
+			t.Error("HasSecret() = true, want false")
+		}
+	})
+
+	t.Run("real failure surfaces", func(t *testing.T) {
+		provider := newTestProvider(t, cfg)
+		provider.SetExecutionFunc(capturingExec(&execCapture{}, "", fmt.Errorf("ExpiredToken: session expired")))
+
+		if _, err := provider.HasSecret("k"); err == nil {
+			t.Error("HasSecret() error = nil, want the expired-session error to surface")
+		}
+	})
+}
+
+// HasSecret with no exists command delegates to the get path. Doing that through
+// the exported GetSecret would take a second read lock, which deadlocks if a
+// writer arrives in between, because sync.RWMutex is not reentrant.
+func TestHasSecret_WithoutExistsCommandDoesNotDeadlock(t *testing.T) {
+	provider := newTestProvider(t, validExternalConfig())
+	provider.SetExecutionFunc(capturingExec(&execCapture{}, "value", nil))
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		for i := 0; i < 50; i++ {
+			_, _ = provider.HasSecret("k")
+		}
+	}()
+	// Contend with writers so a queued writer sits between the two read locks.
+	for i := 0; i < 50; i++ {
+		_ = provider.SetSecret("k", vault.NewSecretValue([]byte("v")))
+	}
+	<-done
+}
+
 func TestExternalVaultProvider_Metadata(t *testing.T) {
-	tests := []struct {
-		name        string
-		config      *vault.ExternalConfig
-		mockOutputs map[string]string
-		mockErrors  map[string]error
-		wantRawData string
-	}{
-		{
-			name: "successful metadata retrieval",
-			config: &vault.ExternalConfig{
-				Metadata: vault.CommandConfig{
-					CommandTemplate: "vault status",
-				},
-			},
-			mockOutputs: map[string]string{
-				"vault status": "vault is healthy",
-			},
-			wantRawData: "vault is healthy",
-		},
-		{
-			name:        "no metadata command configured",
-			config:      &vault.ExternalConfig{},
-			wantRawData: "",
-		},
-		{
-			name: "metadata command fails",
-			config: &vault.ExternalConfig{
-				Metadata: vault.CommandConfig{
-					CommandTemplate: "vault status",
-				},
-			},
-			mockErrors: map[string]error{
-				"vault status": fmt.Errorf("command failed"),
-			},
-			wantRawData: "",
-		},
+	t.Run("successful retrieval", func(t *testing.T) {
+		cfg := validExternalConfig()
+		cfg.Metadata.CommandTemplate = "vault status"
+
+		provider := newTestProvider(t, cfg)
+		provider.SetExecutionFunc(capturingExec(&execCapture{}, "vault is healthy", nil))
+
+		metadata, err := provider.Metadata()
+		if err != nil {
+			t.Fatalf("Metadata() error = %v", err)
+		}
+		if metadata.RawData != "vault is healthy" {
+			t.Errorf("Metadata().RawData = %v, want %v", metadata.RawData, "vault is healthy")
+		}
+	})
+
+	t.Run("not configured is not an error", func(t *testing.T) {
+		provider := newTestProvider(t, validExternalConfig())
+		metadata, err := provider.Metadata()
+		if err != nil {
+			t.Fatalf("Metadata() error = %v", err)
+		}
+		if metadata.RawData != "" {
+			t.Errorf("Metadata().RawData = %v, want empty", metadata.RawData)
+		}
+	})
+
+	// Previously every failure path returned an empty Metadata{}, so a broken
+	// command, a timeout and "not configured" were indistinguishable.
+	t.Run("command failure surfaces as an error", func(t *testing.T) {
+		cfg := validExternalConfig()
+		cfg.Metadata.CommandTemplate = "vault status"
+
+		provider := newTestProvider(t, cfg)
+		provider.SetExecutionFunc(capturingExec(&execCapture{}, "", fmt.Errorf("command failed")))
+
+		if _, err := provider.Metadata(); err == nil {
+			t.Error("Metadata() error = nil, want the command failure to surface")
+		}
+	})
+}
+
+func TestClosedProviderReturnsErrVaultClosed(t *testing.T) {
+	cfg := validExternalConfig()
+	cfg.List.CommandTemplate = "ls"
+	cfg.Metadata.CommandTemplate = "status"
+
+	provider := newTestProvider(t, cfg)
+	provider.SetExecutionFunc(capturingExec(&execCapture{}, "ok", nil))
+	if err := provider.Close(); err != nil {
+		t.Fatalf("Close() error = %v", err)
 	}
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			config := &vault.Config{
-				ID:       "test-vault",
-				Type:     vault.ProviderTypeExternal,
-				External: tt.config,
-			}
+	if _, err := provider.GetSecret("k"); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("GetSecret() after Close = %v, want ErrVaultClosed", err)
+	}
+	if err := provider.SetSecret("k", vault.NewSecretValue([]byte("v"))); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("SetSecret() after Close = %v, want ErrVaultClosed", err)
+	}
+	if err := provider.DeleteSecret("k"); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("DeleteSecret() after Close = %v, want ErrVaultClosed", err)
+	}
+	if _, err := provider.ListSecrets(); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("ListSecrets() after Close = %v, want ErrVaultClosed", err)
+	}
+	if _, err := provider.HasSecret("k"); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("HasSecret() after Close = %v, want ErrVaultClosed", err)
+	}
+	if _, err := provider.Metadata(); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("Metadata() after Close = %v, want ErrVaultClosed", err)
+	}
+}
 
-			provider, err := vault.NewExternalVaultProvider(config)
-			if err != nil {
-				t.Fatalf("Failed to create provider: %v", err)
-			}
+// Exercises the real execute(), not a mock. stderr merged into stdout on success
+// concatenates backend warnings onto the secret value itself.
+func TestExecute_StderrIsNotMergedIntoTheSecret(t *testing.T) {
+	cfg := validExternalConfig()
+	cfg.Get.CommandTemplate = "printf 'the-secret'; printf 'gpg: WARNING: unsafe permissions' 1>&2"
 
-			testProvider := provider
-			testProvider.SetExecutionFunc(mockCommandContext(tt.mockOutputs, tt.mockErrors))
+	provider := newTestProvider(t, cfg)
 
-			metadata := testProvider.Metadata()
-			if metadata.RawData != tt.wantRawData {
-				t.Errorf("Metadata().RawData = %v, want %v", metadata.RawData, tt.wantRawData)
-			}
-		})
+	secret, err := provider.GetSecret("k")
+	if err != nil {
+		t.Fatalf("GetSecret() error = %v", err)
+	}
+	if got := secret.PlainTextString(); got != "the-secret" {
+		t.Errorf("GetSecret() = %q, want %q (stderr leaked into the value)", got, "the-secret")
 	}
 }
 
-// mockCommandContext creates mock commands for testing
-func mockCommandContext(
-	outputs map[string]string,
-	errors map[string]error,
-) func(ctx context.Context, cmd, input, dir string, envList []string) (string, error) {
-	return func(ctx context.Context, cmd, input, dir string, envList []string) (string, error) {
-		for _, output := range outputs {
-			return output, nil
+// End-to-end through the real shell, not a mock: a secret full of shell
+// metacharacters must survive a set/get round trip byte-exact. This is the
+// behaviour the injection fix exists to guarantee.
+func TestExternalProvider_RoundTripThroughRealShell(t *testing.T) {
+	dir := t.TempDir()
+	store := filepath.Join(dir, "secret.txt")
+
+	cfg := &vault.ExternalConfig{
+		Get: vault.CommandConfig{CommandTemplate: "cat '" + store + "'"},
+		Set: vault.CommandConfig{
+			CommandTemplate: "cat > '" + store + "'",
+			InputTemplate:   "{{ value }}",
+		},
+	}
+	provider := newTestProvider(t, cfg)
+
+	for _, value := range []string{
+		`p@$$w0rd`,
+		`correct horse battery`,
+		`hunter2; echo pwned > ` + filepath.Join(dir, "injected"),
+		"back`echo tick`",
+		`quote'and"quote`,
+		`glob*star?`,
+		`$(id)`,
+		`${HOME}`,
+	} {
+		if err := provider.SetSecret("k", vault.NewSecretValue([]byte(value))); err != nil {
+			t.Fatalf("SetSecret(%q) error = %v", value, err)
 		}
 
-		for range errors {
-			return "", fmt.Errorf("mock error")
+		secret, err := provider.GetSecret("k")
+		if err != nil {
+			t.Fatalf("GetSecret() after setting %q error = %v", value, err)
+		}
+		if got := secret.PlainTextString(); got != value {
+			t.Errorf("round trip: got %q, want %q", got, value)
 		}
+	}
+
+	// The injection attempt above must not have run.
+	if _, err := os.Stat(filepath.Join(dir, "injected")); !os.IsNotExist(err) {
+		t.Error("command injection succeeded: the payload created a file")
+	}
+}
+
+// expandEnv used to mutate the shared config map while callers held only a read
+// lock, which is an unrecoverable "concurrent map writes" fault. Run with -race.
+func TestConcurrentGetSecretDoesNotRaceOnEnvironment(t *testing.T) {
+	cfg := validExternalConfig()
+	cfg.Environment = map[string]string{
+		"HOME_REF": "$HOME",
+		"LITERAL":  "$(tty)",
+		"PLAIN":    "value",
+	}
+
+	provider := newTestProvider(t, cfg)
+	// Reads are concurrent by design, so the exec func must be stateless here --
+	// a shared execCapture would itself race and mask what we are testing.
+	provider.SetExecutionFunc(func(
+		_ context.Context, _, _, _ string, _ []string,
+	) (string, error) {
+		return "value", nil
+	})
+
+	var wg sync.WaitGroup
+	for i := 0; i < 50; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			_, _ = provider.GetSecret("k")
+			_, _ = provider.ListSecrets()
+			_, _ = provider.Metadata()
+		}()
+	}
+	wg.Wait()
 
-		return "mock", nil
+	// The config map itself must be unchanged: expansion returns a new map.
+	if got := cfg.Environment["LITERAL"]; got != "$(tty)" {
+		t.Errorf("config Environment was mutated: LITERAL = %q, want %q", got, "$(tty)")
 	}
 }
diff --git a/go.mod b/go.mod
index f36a8e9..381637c 100644
--- a/go.mod
+++ b/go.mod
@@ -4,6 +4,7 @@ go 1.24.0
 
 require (
 	filippo.io/age v1.2.1
+	github.com/gofrs/flock v0.13.0
 	github.com/jahvon/expression v0.1.3
 	github.com/zalando/go-keyring v0.2.6
 	golang.org/x/crypto v0.41.0
@@ -16,7 +17,6 @@ require (
 	github.com/danieljoos/wincred v1.2.2 // indirect
 	github.com/expr-lang/expr v1.17.5 // indirect
 	github.com/godbus/dbus/v5 v5.1.0 // indirect
-	golang.org/x/sys v0.35.0 // indirect
+	golang.org/x/sys v0.37.0 // indirect
 	golang.org/x/term v0.34.0 // indirect
-	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 )
diff --git a/go.sum b/go.sum
index 704d259..c2a4056 100644
--- a/go.sum
+++ b/go.sum
@@ -16,17 +16,16 @@ github.com/go-quicktest/qt v1.101.0 h1:O1K29Txy5P2OK0dGo59b7b0LR6wKfIhttaAhHUyn7
 github.com/go-quicktest/qt v1.101.0/go.mod h1:14Bz/f7NwaXPtdYEgzsx46kqSxVwTbzVZsDC26tQJow=
 github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
 github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
+github.com/gofrs/flock v0.13.0 h1:95JolYOvGMqeH31+FC7D2+uULf6mG61mEZ/A8dRYMzw=
+github.com/gofrs/flock v0.13.0/go.mod h1:jxeyy9R1auM5S6JYDBhDt+E2TCo7DkratH4Pgi8P+Z0=
 github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
 github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
 github.com/jahvon/expression v0.1.3 h1:ZOH6tj1zK9h+M+eDy/n1Fz6XL/vA3lCV6bzHe6YWAvY=
 github.com/jahvon/expression v0.1.3/go.mod h1:4HJB2k+epW5vFeptF6ILlXbFRQ+CuCyCSO4QdnGT3AE=
-github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
 github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
 github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
-github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
-github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
@@ -35,14 +34,14 @@ github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0t
 github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc=
 github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
 github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
-github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
-github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
+github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
+github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
 github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s=
 github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI=
 golang.org/x/crypto v0.41.0 h1:WKYxWedPGCTVVl5+WHSSrOBT0O8lx32+zxmHxijgXp4=
 golang.org/x/crypto v0.41.0/go.mod h1:pO5AFd7FA68rFak7rOAGVuygIISepHftHnr8dr6+sUc=
-golang.org/x/sys v0.35.0 h1:vz1N37gP5bs89s7He8XuIYXpyY0+QlsKmzipCbUtyxI=
-golang.org/x/sys v0.35.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
+golang.org/x/sys v0.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ=
+golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
 golang.org/x/term v0.34.0 h1:O/2T7POpk0ZZ7MAzMeWFSg6S5IpWd/RXDlM9hgM3DR4=
 golang.org/x/term v0.34.0/go.mod h1:5jC53AEywhIVebHgPVeg0mj8OD3VO9OzclacVrqpaAw=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
diff --git a/keyring.go b/keyring.go
index 47ed228..a33fecd 100644
--- a/keyring.go
+++ b/keyring.go
@@ -25,13 +25,22 @@ func NewKeyringVault(cfg *Config) (*KeyringVault, error) {
 		return nil, fmt.Errorf("keyring configuration is required")
 	}
 
+	if err := cfg.Validate(); err != nil {
+		return nil, err
+	}
+
 	vault := &KeyringVault{
 		id:      cfg.ID,
 		service: cfg.Keyring.Service,
 	}
 
-	// Try to load metadata or initialize if not exists
+	// Only a genuinely absent record means "new vault". Treating every error as
+	// absence meant a locked keychain, a DBus failure or a dismissed prompt made
+	// initMetadata() write a fresh record over the real Created timestamp.
 	if err := vault.loadMetadata(); err != nil {
+		if !errors.Is(err, keyring.ErrNotFound) {
+			return nil, fmt.Errorf("failed to read keyring vault metadata: %w", err)
+		}
 		if err := vault.initMetadata(); err != nil {
 			return nil, fmt.Errorf("failed to initialize keyring vault metadata: %w", err)
 		}
@@ -40,16 +49,25 @@ func NewKeyringVault(cfg *Config) (*KeyringVault, error) {
 	return vault, nil
 }
 
+// Keyring entry names are a flat namespace shared by every vault using the same
+// service, so the components are length-prefixed to make them unambiguous.
+// Plain "%s-secret-%s" collided: vault "a" with key "b-secret-c" and vault
+// "a-secret-b" with key "c" both produced "a-secret-b-secret-c", letting one
+// vault silently read, overwrite or delete another's secret.
+func (v *KeyringVault) namespaced(kind, key string) string {
+	return fmt.Sprintf("%d:%s:%s:%s", len(v.id), v.id, kind, key)
+}
+
 func (v *KeyringVault) metadataKey() string {
-	return fmt.Sprintf("%s-metadata", v.id)
+	return v.namespaced("metadata", "")
 }
 
 func (v *KeyringVault) secretKey(key string) string {
-	return fmt.Sprintf("%s-secret-%s", v.id, key)
+	return v.namespaced("secret", key)
 }
 
 func (v *KeyringVault) secretsListKey() string {
-	return fmt.Sprintf("%s-secrets-list", v.id)
+	return v.namespaced("secrets-list", "")
 }
 
 func (v *KeyringVault) initMetadata() error {
@@ -154,11 +172,11 @@ func (v *KeyringVault) ID() string {
 	return v.id
 }
 
-func (v *KeyringVault) Metadata() Metadata {
+func (v *KeyringVault) Metadata() (Metadata, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
-	return v.metadata
+	return v.metadata, nil
 }
 
 func (v *KeyringVault) GetSecret(key string) (Secret, error) {
diff --git a/keyring_test.go b/keyring_test.go
index 6e8d1b6..3cafc8c 100644
--- a/keyring_test.go
+++ b/keyring_test.go
@@ -253,7 +253,7 @@ func TestKeyringVault_Persistence(t *testing.T) {
 	}
 
 	// Verify metadata is accessible
-	metadata := vault2.Metadata()
+	metadata, _ := vault2.Metadata()
 	if metadata.Created.IsZero() {
 		t.Error("Expected creation time to be set")
 	}
@@ -273,7 +273,7 @@ func TestKeyringVault_Metadata(t *testing.T) {
 	}
 	defer vlt.Close()
 
-	metadata := vlt.Metadata()
+	metadata, _ := vlt.Metadata()
 	if metadata.Created.IsZero() {
 		t.Error("Expected creation time to be set")
 	}
@@ -289,7 +289,7 @@ func TestKeyringVault_Metadata(t *testing.T) {
 		t.Fatalf("Failed to set secret: %v", err)
 	}
 
-	newMetadata := vlt.Metadata()
+	newMetadata, _ := vlt.Metadata()
 	if !newMetadata.LastModified.After(oldModified) {
 		t.Error("Expected last modified time to be updated after setting secret")
 	}
diff --git a/local.go b/local.go
index 80cc138..2480111 100644
--- a/local.go
+++ b/local.go
@@ -14,9 +14,10 @@ const (
 	fileSource    = "file"
 )
 
-var (
-	DefaultVaultKeyEnv = "VAULT_KEY"
-)
+// DefaultVaultKeyEnv is the environment variable consulted when no key source
+// is configured. A const rather than a var: as a package-level var, any
+// dependency could repoint every default key lookup in the process.
+const DefaultVaultKeyEnv = "VAULT_KEY"
 
 type Metadata struct {
 	Created      time.Time `json:"created"`
@@ -30,27 +31,34 @@ func validateSecurePath(path string) error {
 		return fmt.Errorf("path cannot be empty")
 	}
 
-	// Check for directory traversal attempts
-	cleanPath := filepath.Clean(path)
-	if strings.Contains(cleanPath, "..") {
-		return NewVaultPathError(path)
-	}
-
 	// Check for null bytes
 	if strings.Contains(path, "\x00") {
 		return NewVaultPathError(path)
 	}
 
 	// Ensure the path is absolute after expansion
-	absPath, err := filepath.Abs(cleanPath)
+	absPath, err := filepath.Abs(filepath.Clean(path))
 	if err != nil {
 		return fmt.Errorf("failed to get absolute path: %w", err)
 	}
 
-	// Basic check that we're not accessing sensitive system directories
+	// Compare path elements rather than substrings. strings.Contains(clean, "..")
+	// rejected legitimate names like "my..backup" while catching almost nothing
+	// real, since Clean on an absolute path has already resolved any genuine ..
+	// elements away.
+	for _, elem := range strings.Split(absPath, string(filepath.Separator)) {
+		if elem == ".." {
+			return NewVaultPathError(path)
+		}
+	}
+
+	// Basic check that we're not writing into sensitive system directories.
+	// Compared as path prefixes, so "/etcetera" is no longer caught by "/etc".
+	// This is a guard rail, not a security boundary -- it is Unix-only and a
+	// symlink can still lead elsewhere.
 	systemDirs := []string{"/etc", "/sys", "/proc", "/dev"}
 	for _, sysDir := range systemDirs {
-		if strings.HasPrefix(absPath, sysDir) {
+		if absPath == sysDir || strings.HasPrefix(absPath, sysDir+string(filepath.Separator)) {
 			return NewVaultPathError(path)
 		}
 	}
@@ -65,38 +73,50 @@ func expandPath(path string) (string, error) {
 
 	var expandedPath string
 
-	switch path[0] {
-	case '~':
+	switch {
+	// Only "~" itself or a "~/..." prefix. Slicing path[1:] unconditionally
+	// turned "~user/x" into "<home>user/x", silently addressing the wrong file.
+	case path == "~":
 		homeDir, err := os.UserHomeDir()
 		if err != nil {
 			return "", fmt.Errorf("failed to get user home directory: %w", err)
 		}
-		expandedPath = homeDir + path[1:]
-	case '/':
-		expandedPath = path
-	case '.':
-		wd, err := os.Getwd()
+		expandedPath = homeDir
+	case strings.HasPrefix(path, "~/"):
+		homeDir, err := os.UserHomeDir()
 		if err != nil {
-			return "", fmt.Errorf("failed to get working directory: %w", err)
+			return "", fmt.Errorf("failed to get user home directory: %w", err)
 		}
-		expandedPath = wd + "/" + path[1:]
-	case '$':
-		envVar := path[1:]
-		if value, exists := os.LookupEnv(envVar); exists {
-			expandedPath = value
-		} else {
-			return "", fmt.Errorf("environment variable %s not found", envVar)
+		expandedPath = filepath.Join(homeDir, path[2:])
+	case strings.HasPrefix(path, "$"):
+		// Split the leading variable from the rest so "$HOME/vault" works. The
+		// previous code treated the whole remainder as the variable name and so
+		// looked up an env var literally called "HOME/vault". Resolving via
+		// os.ExpandEnv instead would be worse: an unset variable expands to ""
+		// and "$NOPE/vault" would silently become "/vault".
+		name, rest := path[1:], ""
+		if i := strings.IndexAny(name, `/\`); i >= 0 {
+			name, rest = name[:i], name[i+1:]
 		}
+		name = strings.TrimSuffix(strings.TrimPrefix(name, "{"), "}")
+
+		value, exists := os.LookupEnv(name)
+		if !exists || value == "" {
+			return "", fmt.Errorf("environment variable %s not found", name)
+		}
+		expandedPath = filepath.Join(value, rest)
+	case filepath.IsAbs(path):
+		expandedPath = path
 	default:
+		// Everything relative -- including "./x", "../x" and ".config/x" --
+		// joins against the working directory. The old code sliced path[1:] for
+		// anything starting with a dot, which dropped the leading dot from
+		// ".config/x" and silently swallowed the parent reference in "../x".
 		wd, err := os.Getwd()
 		if err != nil {
 			return "", fmt.Errorf("failed to get working directory: %w", err)
 		}
-		if wd[len(wd)-1] == '/' {
-			expandedPath = wd + path
-		} else {
-			expandedPath = wd + "/" + path
-		}
+		expandedPath = filepath.Join(wd, path)
 	}
 
 	if err := validateSecurePath(expandedPath); err != nil {
diff --git a/secret.go b/secret.go
index 3434c1c..867e9fb 100644
--- a/secret.go
+++ b/secret.go
@@ -1,10 +1,9 @@
 package vault
 
 import (
-	"crypto/rand"
 	"fmt"
 	"regexp"
-	"runtime"
+	"strings"
 )
 
 type Secret interface {
@@ -24,18 +23,25 @@ type Secret interface {
 // SecureBytes is a wrapper around []byte that provides secure memory handling
 type SecureBytes []byte
 
-// Zero securely clears the byte slice
+// Zero clears the byte slice.
+//
+// Note on what this can and cannot guarantee: it overwrites *this* buffer only.
+// Any Go string derived from it (see PlainTextString) is immutable and cannot be
+// zeroed, and the runtime may have copied the backing array during a heap move.
+// Treat this as reducing exposure, not eliminating it.
 func (s *SecureBytes) Zero() {
-	if s != nil && len(*s) > 0 {
-		// The series of steps below ensures that the memory is cleared securely. It prevents the compiler from
-		// optimizing away the zeroing operation and is recommended to securely clear sensitive data in Go.
-		_, _ = rand.Read(*s)
-		for i := range *s {
-			(*s)[i] = 0
-		}
-		*s = (*s)[:0]
-		runtime.GC()
+	if s == nil || len(*s) == 0 {
+		return
 	}
+	// The previous implementation filled with random bytes first and then forced
+	// a full runtime.GC() on every call. Neither helped: Go does not
+	// dead-store-eliminate writes through a pointer-reachable slice, and a
+	// forced GC per secret is a serious performance footgun in a library while
+	// still not guaranteeing that stale copies are collected or overwritten.
+	for i := range *s {
+		(*s)[i] = 0
+	}
+	*s = (*s)[:0]
 }
 
 // Copy creates a secure copy of the bytes
@@ -58,11 +64,21 @@ func NewSecretValue(value []byte) *SecretValue {
 	return &SecretValue{value: secureValue}
 }
 
+// PlainTextString returns the secret as a string.
+//
+// The result is an immutable Go string and therefore cannot be zeroed; Zero()
+// on this SecretValue will not reclaim it. Keep the returned value as
+// short-lived as possible, and prefer Bytes() when the caller can clear it.
 func (s *SecretValue) PlainTextString() string {
 	return string(s.value)
 }
 
-func (s *SecretValue) String() string {
+// String masks the secret so it cannot be printed by accident.
+//
+// Deliberately a value receiver: with a pointer receiver, formatting a
+// dereferenced copy (fmt.Sprintf("%v", *secret)) falls outside the method set
+// and prints the raw bytes instead of the mask.
+func (s SecretValue) String() string {
 	return "********"
 }
 
@@ -77,13 +93,50 @@ func (s *SecretValue) Zero() {
 	s.value.Zero()
 }
 
+// secretKeyPattern is compiled once. ValidateSecretKey runs on every get, set,
+// delete and existence check, so recompiling per call was pure waste.
+var secretKeyPattern = regexp.MustCompile(`^[a-zA-Z0-9-_.]+$`)
+
+// vaultIDPattern additionally forbids a leading dot, since a vault ID becomes
+// part of a filename.
+var vaultIDPattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9-_.]*$`)
+
 func ValidateSecretKey(reference string) error {
 	if reference == "" {
 		return ErrInvalidKey
 	}
-	re := regexp.MustCompile(`^[a-zA-Z0-9-_.]+$`)
-	if !re.MatchString(reference) {
+	if !secretKeyPattern.MatchString(reference) {
 		return fmt.Errorf("%w: must only contain alphanumeric characters, dashes, underscores, and/or dots", ErrInvalidKey)
 	}
+	// A leading dash makes the key look like a flag to any backend CLI the
+	// external provider shells out to -- a key of "-f" or "--vault" becomes an
+	// option rather than an argument.
+	if strings.HasPrefix(reference, "-") {
+		return fmt.Errorf("%w: must not start with a dash", ErrInvalidKey)
+	}
+	// "." and ".." are path elements. The external provider passes keys through
+	// to tools like pass, where they address entries within a store.
+	if reference == "." || reference == ".." {
+		return fmt.Errorf("%w: must not be %q", ErrInvalidKey, reference)
+	}
+	return nil
+}
+
+// ValidateVaultID checks an identifier that will be used to build filesystem
+// paths and keyring entry names.
+func ValidateVaultID(id string) error {
+	if id == "" {
+		return fmt.Errorf("%w: vault ID is required", ErrInvalidConfig)
+	}
+	if !vaultIDPattern.MatchString(id) {
+		return fmt.Errorf(
+			"%w: vault ID %q must start with a letter or digit and contain only "+
+				"alphanumeric characters, dashes, underscores, and/or dots",
+			ErrInvalidConfig, id,
+		)
+	}
+	if strings.Contains(id, "..") {
+		return fmt.Errorf("%w: vault ID %q must not contain %q", ErrInvalidConfig, id, "..")
+	}
 	return nil
 }
diff --git a/storage.go b/storage.go
new file mode 100644
index 0000000..ce2d544
--- /dev/null
+++ b/storage.go
@@ -0,0 +1,201 @@
+package vault
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"github.com/gofrs/flock"
+)
+
+const (
+	// vaultLockTimeout bounds how long a process waits for another process to
+	// finish its read-modify-write cycle before giving up.
+	vaultLockTimeout = 10 * time.Second
+	// vaultLockRetry is how often the lock is re-attempted while waiting.
+	vaultLockRetry = 50 * time.Millisecond
+	// vaultDirMode keeps the vault directory owner-only. Secrets live here, so
+	// the group-readable 0750 the providers used before is too permissive.
+	vaultDirMode = 0700
+	// vaultFileMode keeps the vault file owner-only.
+	vaultFileMode = 0600
+)
+
+// resolveVaultPath builds the on-disk path for a vault file and proves it stays
+// inside storagePath.
+//
+// filepath.Clean does not sanitize an ID embedded in a filename, it *resolves*
+// it: Clean("vault-../../../tmp/evil.enc") is "../tmp/evil.enc", because the
+// first ".." pops the literal "vault-.." element and the rest survive. Joining
+// that onto the storage directory escapes it, so a crafted vault ID could make
+// save() overwrite an arbitrary file. IDs are validated, and the result is then
+// checked against the base directory as a belt-and-braces second gate.
+func resolveVaultPath(storagePath, id, ext string) (string, error) {
+	if err := ValidateVaultID(id); err != nil {
+		return "", err
+	}
+
+	base, err := expandPath(storagePath)
+	if err != nil {
+		return "", fmt.Errorf("invalid vault storage path %q: %w", storagePath, err)
+	}
+
+	full := filepath.Join(base, fmt.Sprintf("%s-%s.%s", vaultFileBase, id, ext))
+
+	rel, err := filepath.Rel(base, full)
+	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
+		return "", NewVaultPathError(full)
+	}
+
+	return full, nil
+}
+
+// checkVaultVersion rejects a vault written by a newer version of the library.
+// The version field was recorded on every save but never read back, so a future
+// format change would have been parsed as though it were the current one.
+func checkVaultVersion(version, current int, path string) error {
+	if version > current {
+		return fmt.Errorf(
+			"%w: vault file %s is version %d but this build understands up to %d; upgrade to open it",
+			ErrVaultCorrupt, path, version, current,
+		)
+	}
+	return nil
+}
+
+// clearSecrets overwrites and removes every entry in a secrets map.
+//
+// This drops references promptly, but cannot be more than that: the values are
+// immutable Go strings whose backing bytes are not addressable, and the runtime
+// may already have copied them during a heap move. See SecretValue.Zero.
+func clearSecrets(secrets map[string]string) {
+	for k := range secrets {
+		delete(secrets, k)
+	}
+}
+
+// readVaultFile reads a vault file from disk.
+//
+// The boolean reports whether the file exists. Only a genuinely absent file
+// permits a caller to initialize a fresh vault; an existing but zero-length file
+// is reported as corrupt. Treating "empty" as "absent" is how a truncated vault
+// used to be silently reinitialized and then overwritten by the constructor,
+// destroying every secret without reporting an error.
+func readVaultFile(path string) ([]byte, bool, error) {
+	data, err := os.ReadFile(filepath.Clean(path))
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, false, nil
+		}
+		// Anything else -- a permissions problem, an I/O error -- must not be
+		// reported as "not found", which callers reasonably read as "create a
+		// new one".
+		return nil, false, fmt.Errorf("failed to read vault file %s: %w", path, err)
+	}
+
+	if len(data) == 0 {
+		return nil, true, fmt.Errorf(
+			"%w: vault file %s is empty; refusing to overwrite it. "+
+				"Restore it from a backup, or delete it to start a new vault",
+			ErrVaultCorrupt, path,
+		)
+	}
+
+	return data, true, nil
+}
+
+// writeVaultFileAtomic writes data to path via a temp file and a rename.
+func writeVaultFileAtomic(path string, data []byte) error {
+	dir := filepath.Dir(path)
+	if err := os.MkdirAll(dir, vaultDirMode); err != nil {
+		return fmt.Errorf("failed to create vault directory: %w", err)
+	}
+
+	// A random name via os.CreateTemp (which uses O_EXCL) rather than a fixed
+	// "<path>.tmp". The fixed name let two concurrent savers truncate each
+	// other's partial write, and let a pre-planted symlink at that predictable
+	// path redirect the write -- os.WriteFile follows symlinks and does not
+	// apply the 0600 mode to an already-existing target.
+	tmp, err := os.CreateTemp(dir, ".vault-*")
+	if err != nil {
+		return fmt.Errorf("failed to create temp vault file: %w", err)
+	}
+	tmpName := tmp.Name()
+
+	discard := func() {
+		_ = tmp.Close()
+		_ = os.Remove(tmpName)
+	}
+
+	if err := tmp.Chmod(vaultFileMode); err != nil {
+		discard()
+		return fmt.Errorf("failed to set vault file permissions: %w", err)
+	}
+	if _, err := tmp.Write(data); err != nil {
+		discard()
+		return fmt.Errorf("failed to write temp vault file: %w", err)
+	}
+	// Flush to stable storage before the rename. Without this, a crash can leave
+	// the renamed file present but zero-length -- exactly the corrupt state
+	// readVaultFile now has to reject.
+	if err := tmp.Sync(); err != nil {
+		discard()
+		return fmt.Errorf("failed to flush vault file: %w", err)
+	}
+	if err := tmp.Close(); err != nil {
+		_ = os.Remove(tmpName)
+		return fmt.Errorf("failed to close temp vault file: %w", err)
+	}
+
+	if err := os.Rename(tmpName, path); err != nil {
+		_ = os.Remove(tmpName)
+		return fmt.Errorf("failed to move vault file into place: %w", err)
+	}
+
+	syncDir(dir)
+	return nil
+}
+
+// syncDir flushes a directory entry so a completed rename survives a crash.
+// Best effort: not all platforms or filesystems support fsync on a directory
+// (Windows in particular), and by this point the data is already in place, so
+// there is nothing a caller could do with the error.
+func syncDir(dir string) {
+	d, err := os.Open(filepath.Clean(dir))
+	if err != nil {
+		return
+	}
+	defer func() { _ = d.Close() }()
+	_ = d.Sync()
+}
+
+// withVaultLock runs fn while holding an exclusive advisory lock on the vault.
+//
+// The per-instance RWMutex only serializes goroutines sharing one provider
+// value. It does nothing about a second provider in the same process, or -- the
+// common case -- a second `flow secret set` running concurrently. Because every
+// save rewrites the whole file from an in-memory snapshot, two unsynchronized
+// writers silently lose one of the two updates.
+func withVaultLock(vaultPath string, fn func() error) error {
+	if err := os.MkdirAll(filepath.Dir(vaultPath), vaultDirMode); err != nil {
+		return fmt.Errorf("failed to create vault directory: %w", err)
+	}
+
+	lock := flock.New(vaultPath + ".lock")
+	ctx, cancel := context.WithTimeout(context.Background(), vaultLockTimeout)
+	defer cancel()
+
+	locked, err := lock.TryLockContext(ctx, vaultLockRetry)
+	if err != nil {
+		return fmt.Errorf("failed to acquire vault lock for %s: %w", vaultPath, err)
+	}
+	if !locked {
+		return fmt.Errorf("timed out waiting for the vault lock on %s", vaultPath)
+	}
+	defer func() { _ = lock.Unlock() }()
+
+	return fn()
+}
diff --git a/storage_test.go b/storage_test.go
new file mode 100644
index 0000000..de750d0
--- /dev/null
+++ b/storage_test.go
@@ -0,0 +1,233 @@
+package vault_test
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/flowexec/vault"
+)
+
+const testVaultID = "v1"
+
+func unencryptedConfig(dir string) *vault.Config {
+	return &vault.Config{
+		ID:          testVaultID,
+		Type:        vault.ProviderTypeUnencrypted,
+		Unencrypted: &vault.UnencryptedConfig{StoragePath: dir},
+	}
+}
+
+func vaultFilePath(dir string) string {
+	return filepath.Join(dir, fmt.Sprintf("vault-%s.json", testVaultID))
+}
+
+// A zero-length vault file used to be read as "no vault here", which made the
+// constructor initialize a fresh one and immediately save over it -- destroying
+// every secret and reporting success.
+func TestEmptyVaultFileIsRejectedRatherThanOverwritten(t *testing.T) {
+	dir := t.TempDir()
+	path := vaultFilePath(dir)
+
+	if err := os.WriteFile(path, []byte{}, 0600); err != nil {
+		t.Fatalf("failed to seed empty vault file: %v", err)
+	}
+
+	_, err := vault.NewUnencryptedVault(unencryptedConfig(dir))
+	if err == nil {
+		t.Fatal("NewUnencryptedVault() accepted an empty vault file, want an error")
+	}
+	if !errors.Is(err, vault.ErrVaultCorrupt) {
+		t.Errorf("error = %v, want ErrVaultCorrupt", err)
+	}
+
+	// The crucial part: the file must be left exactly as it was.
+	info, statErr := os.Stat(path)
+	if statErr != nil {
+		t.Fatalf("vault file disappeared: %v", statErr)
+	}
+	if info.Size() != 0 {
+		t.Errorf("vault file was rewritten (size %d), want it left untouched", info.Size())
+	}
+}
+
+// A truncated vault must not be silently replaced with an empty one either.
+func TestTruncatedVaultFileIsNotSilentlyReinitialized(t *testing.T) {
+	dir := t.TempDir()
+
+	v, err := vault.NewUnencryptedVault(unencryptedConfig(dir))
+	if err != nil {
+		t.Fatalf("NewUnencryptedVault() error = %v", err)
+	}
+	if err := v.SetSecret("keep-me", vault.NewSecretValue([]byte("value"))); err != nil {
+		t.Fatalf("SetSecret() error = %v", err)
+	}
+
+	path := vaultFilePath(dir)
+	if err := os.WriteFile(path, []byte("{not json"), 0600); err != nil {
+		t.Fatalf("failed to corrupt vault file: %v", err)
+	}
+
+	if _, err := vault.NewUnencryptedVault(unencryptedConfig(dir)); err == nil {
+		t.Fatal("NewUnencryptedVault() accepted a corrupt vault file, want an error")
+	}
+
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("failed to read vault file: %v", err)
+	}
+	if string(data) != "{not json" {
+		t.Errorf("corrupt vault file was overwritten with %q", string(data))
+	}
+}
+
+// Every save rewrites the whole file from an in-memory snapshot taken when the
+// provider was built. Two providers writing without a shared lock silently lose
+// one of the two updates -- the per-instance RWMutex does nothing here.
+func TestConcurrentWritesFromSeparateProvidersDoNotLoseUpdates(t *testing.T) {
+	dir := t.TempDir()
+
+	if _, err := vault.NewUnencryptedVault(unencryptedConfig(dir)); err != nil {
+		t.Fatalf("NewUnencryptedVault() error = %v", err)
+	}
+
+	const writers = 8
+	var wg sync.WaitGroup
+	errs := make(chan error, writers)
+
+	for i := 0; i < writers; i++ {
+		wg.Add(1)
+		go func(n int) {
+			defer wg.Done()
+			// A separate provider instance per writer: distinct in-memory state,
+			// so only the cross-process file lock can serialize them.
+			v, err := vault.NewUnencryptedVault(unencryptedConfig(dir))
+			if err != nil {
+				errs <- fmt.Errorf("writer %d: open: %w", n, err)
+				return
+			}
+			key := fmt.Sprintf("key-%d", n)
+			if err := v.SetSecret(key, vault.NewSecretValue([]byte("value"))); err != nil {
+				errs <- fmt.Errorf("writer %d: set: %w", n, err)
+			}
+		}(i)
+	}
+	wg.Wait()
+	close(errs)
+	for err := range errs {
+		t.Fatalf("%v", err)
+	}
+
+	final, err := vault.NewUnencryptedVault(unencryptedConfig(dir))
+	if err != nil {
+		t.Fatalf("reopen error = %v", err)
+	}
+	keys, err := final.ListSecrets()
+	if err != nil {
+		t.Fatalf("ListSecrets() error = %v", err)
+	}
+	if len(keys) != writers {
+		t.Errorf("vault holds %d secrets (%v), want all %d -- updates were lost", len(keys), keys, writers)
+	}
+}
+
+func TestAtomicWriteLeavesNoTempFilesAndKeepsModeOwnerOnly(t *testing.T) {
+	dir := t.TempDir()
+
+	v, err := vault.NewUnencryptedVault(unencryptedConfig(dir))
+	if err != nil {
+		t.Fatalf("NewUnencryptedVault() error = %v", err)
+	}
+	for i := 0; i < 5; i++ {
+		if err := v.SetSecret(fmt.Sprintf("k%d", i), vault.NewSecretValue([]byte("v"))); err != nil {
+			t.Fatalf("SetSecret() error = %v", err)
+		}
+	}
+
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatalf("ReadDir() error = %v", err)
+	}
+	for _, e := range entries {
+		if strings.HasPrefix(e.Name(), ".vault-") || strings.HasSuffix(e.Name(), ".tmp") {
+			t.Errorf("temp file %q was left behind", e.Name())
+		}
+	}
+
+	info, err := os.Stat(vaultFilePath(dir))
+	if err != nil {
+		t.Fatalf("Stat() error = %v", err)
+	}
+	if perm := info.Mode().Perm(); perm != 0600 {
+		t.Errorf("vault file mode = %o, want 0600", perm)
+	}
+}
+
+// A fixed "<path>.tmp" name meant os.WriteFile would follow a symlink planted
+// there and write the vault contents through it.
+func TestAtomicWriteDoesNotFollowAPlantedTempSymlink(t *testing.T) {
+	dir := t.TempDir()
+	target := filepath.Join(t.TempDir(), "victim.txt")
+	if err := os.WriteFile(target, []byte("original"), 0600); err != nil {
+		t.Fatalf("failed to create target: %v", err)
+	}
+
+	if err := os.MkdirAll(dir, 0700); err != nil {
+		t.Fatalf("MkdirAll() error = %v", err)
+	}
+	if err := os.Symlink(target, vaultFilePath(dir)+".tmp"); err != nil {
+		t.Skipf("symlinks unavailable: %v", err)
+	}
+
+	v, err := vault.NewUnencryptedVault(unencryptedConfig(dir))
+	if err != nil {
+		t.Fatalf("NewUnencryptedVault() error = %v", err)
+	}
+	if err := v.SetSecret("k", vault.NewSecretValue([]byte("secret"))); err != nil {
+		t.Fatalf("SetSecret() error = %v", err)
+	}
+
+	data, err := os.ReadFile(target)
+	if err != nil {
+		t.Fatalf("ReadFile() error = %v", err)
+	}
+	if string(data) != "original" {
+		t.Errorf("the planted symlink was followed: target now holds %q", string(data))
+	}
+}
+
+func TestLocalProvidersReportClosedRatherThanPanicking(t *testing.T) {
+	dir := t.TempDir()
+
+	v, err := vault.NewUnencryptedVault(unencryptedConfig(dir))
+	if err != nil {
+		t.Fatalf("NewUnencryptedVault() error = %v", err)
+	}
+	if err := v.Close(); err != nil {
+		t.Fatalf("Close() error = %v", err)
+	}
+
+	// Every one of these dereferenced a nil state before the guards were added.
+	if _, err := v.GetSecret("k"); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("GetSecret() after Close = %v, want ErrVaultClosed", err)
+	}
+	if err := v.SetSecret("k", vault.NewSecretValue([]byte("v"))); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("SetSecret() after Close = %v, want ErrVaultClosed", err)
+	}
+	if err := v.DeleteSecret("k"); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("DeleteSecret() after Close = %v, want ErrVaultClosed", err)
+	}
+	if _, err := v.ListSecrets(); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("ListSecrets() after Close = %v, want ErrVaultClosed", err)
+	}
+	if _, err := v.HasSecret("k"); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("HasSecret() after Close = %v, want ErrVaultClosed", err)
+	}
+	if _, err := v.Metadata(); !errors.Is(err, vault.ErrVaultClosed) {
+		t.Errorf("Metadata() after Close = %v, want ErrVaultClosed", err)
+	}
+}
diff --git a/unencrypted.go b/unencrypted.go
index 681b741..4c4b040 100644
--- a/unencrypted.go
+++ b/unencrypted.go
@@ -2,10 +2,7 @@ package vault
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
-	"os"
-	"path/filepath"
 	"sort"
 	"sync"
 	"time"
@@ -39,10 +36,14 @@ func NewUnencryptedVault(cfg *Config) (*UnencryptedVault, error) {
 		return nil, fmt.Errorf("unencrypted configuration is required")
 	}
 
-	path := filepath.Join(
-		filepath.Clean(cfg.Unencrypted.StoragePath),
-		filepath.Clean(fmt.Sprintf("%s-%s.%s", vaultFileBase, cfg.ID, unencryptedVaultFileExt)),
-	)
+	if err := cfg.Validate(); err != nil {
+		return nil, err
+	}
+
+	path, err := resolveVaultPath(cfg.Unencrypted.StoragePath, cfg.ID, unencryptedVaultFileExt)
+	if err != nil {
+		return nil, err
+	}
 
 	vault := &UnencryptedVault{
 		id:       cfg.ID,
@@ -74,20 +75,16 @@ func (v *UnencryptedVault) init() error {
 		Secrets: make(map[string]string),
 	}
 
-	return v.save()
+	return withVaultLock(v.fullPath, v.save)
 }
 
 // load retrieves the vault contents from the file and parses it into the state.
 func (v *UnencryptedVault) load() error {
-	data, err := os.ReadFile(filepath.Clean(v.fullPath))
+	data, exists, err := readVaultFile(v.fullPath)
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			return nil
-		}
-		return fmt.Errorf("%w: failed to read vault file %s: %w", ErrVaultNotFound, v.fullPath, err)
+		return err
 	}
-
-	if len(data) == 0 {
+	if !exists {
 		return nil
 	}
 
@@ -97,6 +94,10 @@ func (v *UnencryptedVault) load() error {
 		return fmt.Errorf("failed to parse vault file: %w", err)
 	}
 
+	if err := checkVaultVersion(state.Version, unencryptedCurrentVaultVersion, v.fullPath); err != nil {
+		return err
+	}
+
 	v.state = &state
 	return nil
 }
@@ -104,7 +105,7 @@ func (v *UnencryptedVault) load() error {
 // save writes the vault contents to disk in JSON format
 func (v *UnencryptedVault) save() error {
 	if v.state == nil {
-		return nil
+		return ErrVaultClosed
 	}
 
 	v.state.LastModified = time.Now()
@@ -115,42 +116,48 @@ func (v *UnencryptedVault) save() error {
 		return fmt.Errorf("failed to marshal vault state: %w", err)
 	}
 
-	// Write to file atomically
-	if err := os.MkdirAll(filepath.Dir(v.fullPath), 0750); err != nil {
-		return fmt.Errorf("failed to create vault directory: %w", err)
-	}
-
-	tempFile := v.fullPath + ".tmp"
-	if err := os.WriteFile(tempFile, data, 0600); err != nil {
-		return fmt.Errorf("failed to write temp vault file: %w", err)
-	}
-
-	if err := os.Rename(tempFile, v.fullPath); err != nil {
-		_ = os.Remove(tempFile)
-		return fmt.Errorf("failed to move vault file: %w", err)
-	}
+	return writeVaultFileAtomic(v.fullPath, data)
+}
 
-	return nil
+// mutate runs a read-modify-write cycle under the cross-process vault lock.
+// See AES256Vault.mutate for why the reload inside the lock is required.
+func (v *UnencryptedVault) mutate(apply func() error) error {
+	return withVaultLock(v.fullPath, func() error {
+		if err := v.load(); err != nil {
+			return err
+		}
+		if err := apply(); err != nil {
+			return err
+		}
+		return v.save()
+	})
 }
 
 func (v *UnencryptedVault) ID() string {
 	return v.id
 }
 
-func (v *UnencryptedVault) Metadata() Metadata {
+func (v *UnencryptedVault) Metadata() (Metadata, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
 	if v.state == nil {
-		return Metadata{}
+		return Metadata{}, ErrVaultClosed
 	}
-	return v.state.Metadata
+	return v.state.Metadata, nil
 }
 
 func (v *UnencryptedVault) GetSecret(key string) (Secret, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if err := ValidateSecretKey(key); err != nil {
+		return nil, err
+	}
+	if v.state == nil {
+		return nil, ErrVaultClosed
+	}
+
 	value, exists := v.state.Secrets[key]
 	if !exists {
 		return nil, ErrSecretNotFound
@@ -166,32 +173,49 @@ func (v *UnencryptedVault) SetSecret(key string, secret Secret) error {
 	if err := ValidateSecretKey(key); err != nil {
 		return err
 	}
-
-	if v.state.Secrets == nil {
-		v.state.Secrets = make(map[string]string)
+	if v.state == nil {
+		return ErrVaultClosed
 	}
 
-	v.state.Secrets[key] = secret.PlainTextString()
-	return v.save()
+	return v.mutate(func() error {
+		if v.state.Secrets == nil {
+			v.state.Secrets = make(map[string]string)
+		}
+		v.state.Secrets[key] = secret.PlainTextString()
+		return nil
+	})
 }
 
 func (v *UnencryptedVault) DeleteSecret(key string) error {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
-	_, exists := v.state.Secrets[key]
-	if !exists {
-		return ErrSecretNotFound
+	if err := ValidateSecretKey(key); err != nil {
+		return err
+	}
+	if v.state == nil {
+		return ErrVaultClosed
 	}
 
-	delete(v.state.Secrets, key)
-	return v.save()
+	// The existence check runs inside mutate, after the reload, so it sees the
+	// current on-disk contents rather than a stale snapshot.
+	return v.mutate(func() error {
+		if _, exists := v.state.Secrets[key]; !exists {
+			return ErrSecretNotFound
+		}
+		delete(v.state.Secrets, key)
+		return nil
+	})
 }
 
 func (v *UnencryptedVault) ListSecrets() ([]string, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if v.state == nil {
+		return nil, ErrVaultClosed
+	}
+
 	keys := make([]string, 0, len(v.state.Secrets))
 	for k := range v.state.Secrets {
 		keys = append(keys, k)
@@ -206,6 +230,13 @@ func (v *UnencryptedVault) HasSecret(key string) (bool, error) {
 	v.mu.RLock()
 	defer v.mu.RUnlock()
 
+	if err := ValidateSecretKey(key); err != nil {
+		return false, err
+	}
+	if v.state == nil {
+		return false, ErrVaultClosed
+	}
+
 	_, exists := v.state.Secrets[key]
 	return exists, nil
 }
@@ -215,6 +246,9 @@ func (v *UnencryptedVault) Close() error {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
+	if v.state != nil {
+		clearSecrets(v.state.Secrets)
+	}
 	v.state = nil
 
 	return nil
diff --git a/unencrypted_test.go b/unencrypted_test.go
index a7c88c6..b080073 100644
--- a/unencrypted_test.go
+++ b/unencrypted_test.go
@@ -243,7 +243,7 @@ func TestUnencryptedVault_Persistence(t *testing.T) {
 	}
 
 	// Verify metadata is preserved
-	metadata := vault2.Metadata()
+	metadata, _ := vault2.Metadata()
 	if metadata.Created.IsZero() {
 		t.Error("Expected creation time to be preserved")
 	}
@@ -264,7 +264,7 @@ func TestUnencryptedVault_Metadata(t *testing.T) {
 	}
 	defer vlt.Close()
 
-	metadata := vlt.Metadata()
+	metadata, _ := vlt.Metadata()
 	if metadata.Created.IsZero() {
 		t.Error("Expected creation time to be set")
 	}
@@ -281,7 +281,7 @@ func TestUnencryptedVault_Metadata(t *testing.T) {
 		t.Fatalf("Failed to set secret: %v", err)
 	}
 
-	newMetadata := vlt.Metadata()
+	newMetadata, _ := vlt.Metadata()
 	if !newMetadata.LastModified.After(oldModified) {
 		t.Error("Expected last modified time to be updated after setting secret")
 	}
diff --git a/validation_test.go b/validation_test.go
new file mode 100644
index 0000000..dd50af5
--- /dev/null
+++ b/validation_test.go
@@ -0,0 +1,215 @@
+package vault_test
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/flowexec/vault"
+)
+
+// A vault ID is interpolated into a filename, and filepath.Clean *resolves*
+// traversal rather than sanitizing it: Clean("vault-../../../tmp/evil.enc")
+// yields "../tmp/evil.enc", which escapes the storage directory once joined.
+func TestVaultIDCannotEscapeTheStorageDirectory(t *testing.T) {
+	hostile := []string{
+		"../../../tmp/evil",
+		"..",
+		"../sibling",
+		"a/b",
+		`a\b`,
+		".hidden",
+		"-leading-dash",
+		"has space",
+		"semi;colon",
+		"",
+	}
+
+	for _, id := range hostile {
+		t.Run(id, func(t *testing.T) {
+			dir := t.TempDir()
+			_, err := vault.NewUnencryptedVault(&vault.Config{
+				ID:          id,
+				Type:        vault.ProviderTypeUnencrypted,
+				Unencrypted: &vault.UnencryptedConfig{StoragePath: dir},
+			})
+			if err == nil {
+				t.Fatalf("vault ID %q was accepted, want rejection", id)
+			}
+		})
+	}
+}
+
+// The traversal is only interesting if it would otherwise have written outside
+// the storage directory, so assert that directly.
+func TestTraversalVaultIDWritesNothingOutsideStorage(t *testing.T) {
+	base := t.TempDir()
+	storage := filepath.Join(base, "storage")
+	if err := os.MkdirAll(storage, 0700); err != nil {
+		t.Fatalf("MkdirAll() error = %v", err)
+	}
+
+	_, err := vault.NewUnencryptedVault(&vault.Config{
+		ID:          "../escaped",
+		Type:        vault.ProviderTypeUnencrypted,
+		Unencrypted: &vault.UnencryptedConfig{StoragePath: storage},
+	})
+	if err == nil {
+		t.Fatal("a traversing vault ID was accepted")
+	}
+
+	entries, err := os.ReadDir(base)
+	if err != nil {
+		t.Fatalf("ReadDir() error = %v", err)
+	}
+	for _, e := range entries {
+		if e.Name() != "storage" {
+			t.Errorf("something was written outside the storage directory: %q", e.Name())
+		}
+	}
+}
+
+func TestValidateSecretKey(t *testing.T) {
+	valid := []string{"key", "my-key", "my_key", "my.key", "a1", "A.B-C_d"}
+	for _, key := range valid {
+		if err := vault.ValidateSecretKey(key); err != nil {
+			t.Errorf("ValidateSecretKey(%q) = %v, want nil", key, err)
+		}
+	}
+
+	invalid := []struct {
+		key, why string
+	}{
+		{"", "empty"},
+		{"has space", "space"},
+		{"has/slash", "path separator"},
+		{"semi;colon", "shell metacharacter"},
+		{"$(id)", "command substitution"},
+		// A leading dash makes the key look like an option to any backend CLI
+		// the external provider shells out to.
+		{"-f", "leading dash"},
+		{"--vault", "leading dash"},
+		// "." and ".." are path elements; the external provider passes keys to
+		// tools like pass where they address entries within a store.
+		{".", "current directory"},
+		{"..", "parent directory"},
+	}
+	for _, tc := range invalid {
+		if err := vault.ValidateSecretKey(tc.key); err == nil {
+			t.Errorf("ValidateSecretKey(%q) = nil, want rejection (%s)", tc.key, tc.why)
+		}
+	}
+}
+
+// New() returned the concrete typed pointer even on error, which Go wraps in a
+// non-nil interface. `if provider != nil` then passed and the next call
+// panicked on a nil receiver.
+func TestNewReturnsATrulyNilProviderOnError(t *testing.T) {
+	provider, _, err := vault.New("test", vault.WithProvider(vault.ProviderTypeAge))
+	if err == nil {
+		t.Fatal("expected an error for an age vault with no configuration")
+	}
+	if provider != nil {
+		t.Errorf("New() returned a non-nil Provider (%T) alongside an error", provider)
+	}
+}
+
+func TestNewRejectsUnsupportedProviderType(t *testing.T) {
+	provider, _, err := vault.New("test", vault.WithProvider("nope"))
+	if err == nil {
+		t.Fatal("expected an error for an unsupported provider type")
+	}
+	if provider != nil {
+		t.Errorf("New() returned a non-nil Provider (%T) alongside an error", provider)
+	}
+	if !errors.Is(err, vault.ErrInvalidConfig) {
+		t.Errorf("error = %v, want ErrInvalidConfig", err)
+	}
+}
+
+// The masked String() must apply to a dereferenced copy too. With a pointer
+// receiver, fmt.Sprintf("%v", *secret) fell outside the method set and printed
+// the raw bytes.
+func TestSecretMasksItselfEvenWhenDereferenced(t *testing.T) {
+	secret := vault.NewSecretValue([]byte("top-secret-value"))
+
+	for name, rendered := range map[string]string{
+		"pointer":      fmt.Sprintf("%v", secret),
+		"dereferenced": fmt.Sprintf("%v", *secret),
+	} {
+		if strings.Contains(rendered, "top-secret-value") {
+			t.Errorf("%s formatting leaked the secret: %s", name, rendered)
+		}
+	}
+}
+
+// WithLocalPath switched on the provider type the moment it ran, so it was a
+// silent no-op unless WithProvider happened to be passed first -- surfacing
+// later as a confusing "storage path is required".
+func TestWithLocalPathIsOrderIndependent(t *testing.T) {
+	t.Run("path before provider", func(t *testing.T) {
+		dir := t.TempDir()
+		_, cfg, err := vault.New("v1",
+			vault.WithLocalPath(dir),
+			vault.WithProvider(vault.ProviderTypeUnencrypted),
+		)
+		if err != nil {
+			t.Fatalf("New() error = %v", err)
+		}
+		if cfg.Unencrypted == nil || cfg.Unencrypted.StoragePath != dir {
+			t.Errorf("storage path was not applied: %+v", cfg.Unencrypted)
+		}
+	})
+
+	t.Run("provider before path", func(t *testing.T) {
+		dir := t.TempDir()
+		_, cfg, err := vault.New("v1",
+			vault.WithProvider(vault.ProviderTypeUnencrypted),
+			vault.WithLocalPath(dir),
+		)
+		if err != nil {
+			t.Fatalf("New() error = %v", err)
+		}
+		if cfg.Unencrypted == nil || cfg.Unencrypted.StoragePath != dir {
+			t.Errorf("storage path was not applied: %+v", cfg.Unencrypted)
+		}
+	})
+}
+
+// The version field was written on every save but never read back, so a vault
+// from a future format would have been parsed as though it were current.
+func TestNewerVaultVersionIsRejected(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "vault-v1.json")
+
+	if err := os.WriteFile(path, []byte(`{"version":99,"id":"v1","secrets":{}}`), 0600); err != nil {
+		t.Fatalf("failed to seed vault file: %v", err)
+	}
+
+	_, err := vault.NewUnencryptedVault(&vault.Config{
+		ID:          "v1",
+		Type:        vault.ProviderTypeUnencrypted,
+		Unencrypted: &vault.UnencryptedConfig{StoragePath: dir},
+	})
+	if err == nil {
+		t.Fatal("a newer vault format version was accepted")
+	}
+	if !errors.Is(err, vault.ErrVaultCorrupt) {
+		t.Errorf("error = %v, want ErrVaultCorrupt", err)
+	}
+}
+
+func TestSecretZeroClearsTheBuffer(t *testing.T) {
+	secret := vault.NewSecretValue([]byte("top-secret-value"))
+	secret.Zero()
+
+	if got := secret.PlainTextString(); got != "" {
+		t.Errorf("PlainTextString() after Zero() = %q, want empty", got)
+	}
+	if got := secret.Bytes(); len(got) != 0 {
+		t.Errorf("Bytes() after Zero() = %v, want empty", got)
+	}
+}
diff --git a/vault.go b/vault.go
index a681699..4b764c7 100644
--- a/vault.go
+++ b/vault.go
@@ -14,8 +14,10 @@ type Provider interface {
 	// ID returns a unique identifier for this vault instance
 	ID() string
 
-	// Metadata returns vault metadata such as creation time
-	Metadata() Metadata
+	// Metadata returns vault metadata such as creation time. It returns an error
+	// rather than a zero value so that a failing backend command, a timeout, and
+	// "no metadata configured" stay distinguishable.
+	Metadata() (Metadata, error)
 
 	Close() error
 }
@@ -28,28 +30,51 @@ func New(id string, opts ...Option) (Provider, *Config, error) {
 	for _, opt := range opts {
 		opt(config)
 	}
+	// Resolved after every option has been applied, so WithLocalPath no longer
+	// depends on WithProvider having come first in the argument list.
+	config.applyPendingLocalPath()
+
 	if err := config.Validate(); err != nil {
 		return nil, config, err
 	}
 
+	// Each branch returns an explicit nil on error. Returning the typed pointer
+	// directly wrapped a nil *AgeVault (etc.) in a non-nil Provider interface,
+	// so the usual `if provider != nil` check passed and the next method call
+	// panicked on a nil receiver.
 	switch config.Type {
 	case ProviderTypeAge:
 		provider, err := NewAgeVault(config)
-		return provider, config, err
+		if err != nil {
+			return nil, config, err
+		}
+		return provider, config, nil
 	case ProviderTypeAES256:
 		provider, err := NewAES256Vault(config)
-		return provider, config, err
+		if err != nil {
+			return nil, config, err
+		}
+		return provider, config, nil
 	case ProviderTypeKeyring:
 		provider, err := NewKeyringVault(config)
-		return provider, config, err
+		if err != nil {
+			return nil, config, err
+		}
+		return provider, config, nil
 	case ProviderTypeUnencrypted:
 		provider, err := NewUnencryptedVault(config)
-		return provider, config, err
+		if err != nil {
+			return nil, config, err
+		}
+		return provider, config, nil
 	case ProviderTypeExternal:
 		provider, err := NewExternalVaultProvider(config)
-		return provider, config, err
+		if err != nil {
+			return nil, config, err
+		}
+		return provider, config, nil
 	}
-	return nil, nil, fmt.Errorf("unsupported vault type: %s", config.Type)
+	return nil, config, fmt.Errorf("%w: unsupported vault type: %s", ErrInvalidConfig, config.Type)
 }
 
 // WithProvider sets the vault provider type
@@ -99,18 +124,34 @@ func WithKeyringService(service string) Option {
 	}
 }
 
-// WithLocalPath sets the local vault storage path (works for Age, AES, and Unencrypted based on provider type)
+// WithLocalPath sets the local vault storage path (works for Age, AES, and Unencrypted based on provider type).
+//
+// The path is applied once all options have been processed, so it does not
+// matter whether WithProvider is passed before or after it. Previously this
+// switched on c.Type immediately and was a silent no-op unless WithProvider
+// happened to come first, surfacing later as "storage path is required".
 func WithLocalPath(path string) Option {
 	return func(c *Config) {
-		//nolint:exhaustive
-		switch c.Type {
-		case ProviderTypeAge:
-			WithAgePath(path)(c)
-		case ProviderTypeAES256:
-			WithAESPath(path)(c)
-		case ProviderTypeUnencrypted:
-			WithUnencryptedPath(path)(c)
-		}
+		c.pendingLocalPath = path
+	}
+}
+
+// applyPendingLocalPath routes a WithLocalPath value to the provider-specific
+// field now that the provider type is known.
+func (c *Config) applyPendingLocalPath() {
+	if c.pendingLocalPath == "" {
+		return
+	}
+	path := c.pendingLocalPath
+
+	//nolint:exhaustive
+	switch c.Type {
+	case ProviderTypeAge:
+		WithAgePath(path)(c)
+	case ProviderTypeAES256:
+		WithAESPath(path)(c)
+	case ProviderTypeUnencrypted:
+		WithUnencryptedPath(path)(c)
 	}
 }
 
diff --git a/vault_test.go b/vault_test.go
index e1dba44..8a3293d 100644
--- a/vault_test.go
+++ b/vault_test.go
@@ -276,7 +276,7 @@ func testPersistence(t *testing.T, v vault.Provider, provider vault.ProviderType
 	}
 
 	// Test that Metadata is preserved
-	metadata := newVault.Metadata()
+	metadata, _ := newVault.Metadata()
 	if metadata.Created.IsZero() {
 		t.Error("Metadata creation time should not be zero")
 	}