Check vault names in every command that takes one (closes #68)

A vault name may use only lowercase ASCII letters, digits, `.`, `-` and
`_`, and must not be empty, `.` or `..`; the error now states that rule.
`vault create`, `vault import`, `vault select`, `vault remove`, both
vault names of `mv` and shell completion of a `vault:secret` argument
check the name as typed before building any path from it. Before,
`vault import ..` wrote a long-term key and an unlocker into the state
directory itself, and `vault select ..` made that the current vault.

Model: opus-5-5

Dockerfile

@@ -6,6 +6,11 @@ WORKDIR /src
 COPY go.mod go.sum ./
 RUN go mod download
 
+# script/cibuild sets CHECK_EPOCH to the current time, so the RUN steps
+# below run again on each build, an unchanged tree included, while the
+# steps above stay cached. ARG is per stage: the build stage declares it too.
+ARG CHECK_EPOCH
+
 COPY . .
 
 RUN make fmt-check
@@ -25,6 +30,9 @@ WORKDIR /build
 COPY go.mod go.sum ./
 RUN go mod download
 
+# As in the lint stage: the RUN steps below run again on each script/cibuild.
+ARG CHECK_EPOCH
+
 COPY . .
 
 RUN make test

README.md

@@ -526,7 +526,8 @@ them. We provide:
 - `script/docker` — build the Docker image tagged with the project name
 - `script/cibuild` — CI entrypoint: `docker build --ulimit
   memlock=-1:-1 .` (memguard needs mlock; the Dockerfile runs the
-  checks)
+  checks), with a new `CHECK_EPOCH` build argument on every run so the
+  checks run again on an unchanged tree
 - `script/precommit` — pre-commit checks: `go mod tidy` verification,
   then `script/check`
 - `script/install-precommit` — install the git pre-commit hook that

TODO.md

@@ -29,10 +29,30 @@ Bring the repo into policy compliance in one commit:
   `.`, `-` and `_`, and must not be empty, `.` or `..`
   (https://git.eeqj.de/sneak/secret/issues/68); the error and `README.md`
   state the rule. `vault create`, `vault import`, `vault select`,
-  `vault remove` and both vault names of `mv` check the name as typed with
+  `vault remove`, both vault names of `mv` and shell completion of a
+  `vault:secret` argument check the name as typed with
   `vault.ValidateVaultName` before building any path from it. Before,
   `vault import ..` wrote a long-term key and an unlocker into the state
   directory itself, and `vault select ..` made that the current vault.
+- 2026-10-04: `script/cibuild` runs the checks again on an unchanged
+  tree (https://git.eeqj.de/sneak/secret/issues/54). It passes the
+  current time as the `CHECK_EPOCH` build argument, which both the lint
+  and the build stage of the `Dockerfile` declare after their module
+  download, so the `RUN` steps below the argument run again on each
+  build while the base images and module downloads stay cached. Before,
+  a second run on the same tree took every check from the build cache
+  and reported success having run nothing.
+- 2026-10-04: A failed unlocker add no longer leaves a partial unlocker
+  directory (https://git.eeqj.de/sneak/secret/issues/48).
+  `secret unlocker add pgp` resolves the GPG key's fingerprint once, for
+  its duplicate check, and passes it to `CreatePGPUnlocker` to record.
+  `CreatePGPUnlocker` and `CreateKeychainUnlocker` get the long-term key
+  and encrypt everything before writing anything. All four unlocker
+  types write their files through `secret.WriteDir`: a new unlocker is
+  built in a temporary directory, renamed into place when complete and
+  removed on a failure. One added under the directory name of an
+  existing unlocker is still written into that directory in place
+  (https://git.eeqj.de/sneak/secret/issues/71).
 - 2026-10-04: `secret unlocker select` and `secret unlocker remove`
   skip, with the warning `unlocker list` gives, an unlocker directory
   whose metadata file cannot be checked for, read or parsed, instead of
@@ -137,13 +157,10 @@ Bring the repo into policy compliance in one commit:
     - from `init` or `vault create` killed after the passphrase prompt
       but before the unlocker is written, a vault with no unlocker,
       which `vault create` has already made the current vault;
-    - from an unlocker add stopped before its metadata is written, a
-      directory that `unlocker list` warns about and `unlocker rm`
-      removes only by its directory name;
-    - data under a `.tmp-` name in the state directory: a secret or
-      version being added, or the secret, version, unlocker or vault
-      being removed, encrypted keys included. Nothing deletes it; it
-      must be deleted by hand
+    - data under a `.tmp-` name in the state directory: a secret,
+      version or unlocker being added, or the secret, version, unlocker
+      or vault being removed, encrypted keys included. Nothing deletes
+      it; it must be deleted by hand
       (https://git.eeqj.de/sneak/secret/issues/75).
 - 2026-10-03: The checks run before changing a vault now stop with an
   error naming the path and cause when they cannot read what they

internal/cli/completions.go

@@ -123,7 +123,9 @@ func getVaultNamesCompletionFunc(fs afero.Fs, stateDir string) func(
 }
 
 // completeVaultQualifiedSecrets completes "vault:secret" references once a
-// colon is present in the input
+// colon is present in the input. It completes nothing when the vault part
+// is not a valid vault name, so that a name such as ".." cannot list a
+// directory outside vaults.d.
 func completeVaultQualifiedSecrets(
 	fs afero.Fs, stateDir, toComplete string,
 ) []string {
@@ -134,6 +136,10 @@ func completeVaultQualifiedSecrets(
 	vaultName := parts[0]
 	secretPrefix := parts[1]
 
+	if vault.ValidateVaultName(vaultName) != nil {
+		return nil
+	}
+
 	vlt := vault.NewVault(fs, stateDir, vaultName)
 
 	secrets, err := vlt.ListSecrets()

internal/cli/completions_test.go

@@ -0,0 +1,41 @@
+//nolint:testpackage // white-box test of unexported internals
+package cli
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestVaultSecretCompletionRejectsInvalidVaultName is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/68: completing a `vault:secret`
+// argument lists nothing when the vault part is not a valid vault name, even
+// where that name, joined onto vaults.d, leads to a secrets.d directory.
+func TestVaultSecretCompletionRejectsInvalidVaultName(t *testing.T) {
+	t.Parallel()
+
+	const (
+		stateDir = "/state"
+		dirPerm  = 0o700
+	)
+
+	fs := afero.NewMemMapFs()
+
+	// The vault "work" holds the secret "x". So does every directory an
+	// invalid name below would lead to from vaults.d.
+	for _, vaultName := range []string{"work", ".", "..", "a/b"} {
+		secretDir := filepath.Join(stateDir, "vaults.d", vaultName, "secrets.d", "x")
+		require.NoError(t, fs.MkdirAll(secretDir, dirPerm))
+	}
+
+	assert.Equal(t, []string{"work:x"},
+		completeVaultQualifiedSecrets(fs, stateDir, "work:"))
+
+	for _, toComplete := range []string{".:", "..:", "a/b:"} {
+		assert.Empty(t, completeVaultQualifiedSecrets(fs, stateDir, toComplete),
+			"completing %q", toComplete)
+	}
+}

internal/cli/unlockers.go

@@ -685,7 +685,8 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to get current vault: %w", err)
 	}
 
-	// Resolve the GPG key ID to its fingerprint
+	// Resolve the GPG key ID to its fingerprint, once: the duplicate check
+	// and the new unlocker's metadata both use this result
 	fingerprint, err := secret.ResolveGPGKeyFingerprint(gpgKeyID)
 	if err != nil {
 		return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
@@ -706,7 +707,8 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 		return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
 	}
 
-	pgpUnlocker, err := secret.CreatePGPUnlocker(cli.fs, cli.stateDir, gpgKeyID)
+	pgpUnlocker, err := secret.CreatePGPUnlocker(
+		cli.fs, cli.stateDir, gpgKeyID, fingerprint)
 	if err != nil {
 		return err
 	}

internal/cli/unlockers_add_test.go

@@ -0,0 +1,35 @@
+//nolint:testpackage // white-box test of unexported internals
+package cli
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// unknownTestGPGUserID is a GPG user ID that no key in the test keyring has.
+const unknownTestGPGUserID = "not-in-keyring@example.com"
+
+// TestAddPGPUnlockerUnknownKey asserts that adding a PGP unlocker for a key
+// the keyring does not hold fails at looking up the key's fingerprint and
+// leaves no new unlocker directory. The error must come from the lookup: a
+// lookup moved after anything is written would also come after getting the
+// vault's long-term key, which fails first on every platform but macOS
+// (https://git.eeqj.de/sneak/secret/issues/88).
+//
+//nolint:paralleltest // t.Setenv (GNUPGHOME) forbids parallel tests
+func TestAddPGPUnlockerUnknownKey(t *testing.T) {
+	newTestGPGKey(t)
+
+	base := newListTestVault(t, 1)
+	instance, cmd := newTestInstance(base)
+	cmd.Flags().String("keyid", unknownTestGPGUserID, "")
+
+	err := instance.addPGPUnlocker(cmd)
+
+	require.ErrorContains(t, err, "failed to resolve GPG key fingerprint")
+	assertDirEntries(t, base,
+		filepath.Join(testVaultDir(listTestVaultName), listTestUnlockersDirName),
+		listTestUnlockerDirOne)
+}

internal/secret/atomic.go

@@ -1,6 +1,7 @@
 package secret
 
 import (
+	"errors"
 	"fmt"
 	"path/filepath"
 
@@ -61,6 +62,52 @@ func TempDirFor(fs afero.Fs, target string) (string, error) {
 	return dir, nil
 }
 
+// WriteDir calls write to write the files of the directory dir. When dir does
+// not exist yet, write writes them into a temporary directory from TempDirFor,
+// which is then renamed to dir, so that neither a failure nor a crash leaves
+// dir half-written; on a failure the temporary directory is removed, and a
+// failure to remove it is returned along with the first. A directory cannot be
+// renamed over one that has files in it, so when dir already exists, write
+// writes into it in place; dir is then never removed.
+func WriteDir(fs afero.Fs, dir string, write func(dir string) error) error {
+	exists, err := afero.Exists(fs, dir)
+	if err != nil {
+		return fmt.Errorf("failed to check for %s: %w", dir, err)
+	}
+
+	if exists {
+		return write(dir)
+	}
+
+	// Create the directory the finished one is renamed into
+	err = fs.MkdirAll(filepath.Dir(dir), DirPerms)
+	if err != nil {
+		return fmt.Errorf("failed to create %s: %w", filepath.Dir(dir), err)
+	}
+
+	tmp, err := TempDirFor(fs, dir)
+	if err != nil {
+		return err
+	}
+
+	err = write(tmp)
+	if err == nil {
+		err = fs.Rename(tmp, dir)
+	}
+
+	if err != nil {
+		removeErr := fs.RemoveAll(tmp)
+		if removeErr != nil {
+			err = errors.Join(err,
+				fmt.Errorf("failed to remove %s: %w", tmp, removeErr))
+		}
+
+		return err
+	}
+
+	return nil
+}
+
 // RemoveDirAtomic deletes the directory dir so that it disappears in one
 // rename: dir is moved into a new directory from TempDirFor, which is then
 // deleted. A crash part-way leaves only that temporary directory behind.

internal/secret/atomic_test.go

@@ -35,6 +35,10 @@ const currentFile = "current"
 // unlockerMetadataFile is the file a new unlocker writes last.
 const unlockerMetadataFile = "unlocker-metadata.json"
 
+// privKeyFile is the file that holds the encrypted private key of a version
+// or of a passphrase unlocker.
+const privKeyFile = "priv.age"
+
 // unlockerPassphrase protects the passphrase unlockers the tests create.
 //
 //nolint:gosec // G101: test data, not a real credential
@@ -453,7 +457,7 @@ func TestVersionSaveIsWholeOrAbsent(t *testing.T) {
 
 				if exists {
 					assert.ElementsMatch(t,
-						[]string{"pub.age", "value.age", "priv.age", "metadata.age"},
+						[]string{"pub.age", "value.age", privKeyFile, "metadata.age"},
 						dirNames(t, base, versionDir),
 						"version directory visible before it was complete")
 				}
@@ -496,7 +500,7 @@ func TestVersionSaveFailureLeavesNothing(t *testing.T) {
 			writeLongTermKey(t, base, stateDir)
 
 			fs := hookFs{Fs: base, before: func(op, path string) error {
-				if op == opRename && filepath.Base(path) == "priv.age" {
+				if op == opRename && filepath.Base(path) == privKeyFile {
 					return errInjected
 				}
 
@@ -642,37 +646,119 @@ func TestPassphraseUnlockerGetsKeyFirst(t *testing.T) {
 	require.Error(t, err)
 }
 
-// TestPassphraseUnlockerWritesMetadataLast checks that the last file a new
-// passphrase unlocker writes in its directory is its metadata: an unlocker
-// directory without metadata is never used, so one interrupted earlier
-// cannot be.
-func TestPassphraseUnlockerWritesMetadataLast(t *testing.T) {
+// TestPassphraseUnlockerIsWholeOrAbsent checks, before every change that
+// creating a passphrase unlocker makes, that the unlocker's directory either
+// does not exist or holds all of its files: a crash or a failure at any point
+// leaves no partial unlocker.
+//
+//nolint:paralleltest // t.Setenv forbids t.Parallel
+func TestPassphraseUnlockerIsWholeOrAbsent(t *testing.T) {
 	t.Setenv(secret.EnvMnemonic, testMnemonic)
 
-	base := afero.NewMemMapFs()
-	vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
-	require.NoError(t, err)
+	files := []string{"pub.age", privKeyFile, "longterm.age", unlockerMetadataFile}
 
-	vaultDir, err := vlt.GetDirectory()
-	require.NoError(t, err)
+	for _, tfs := range testFilesystems {
+		t.Run(tfs.name, func(t *testing.T) {
+			base, stateDir := tfs.open(t)
+			vlt, err := vault.CreateVault(base, stateDir, testVaultName)
+			require.NoError(t, err)
 
-	unlockerDir := filepath.Join(vaultDir, "unlockers.d", "passphrase")
+			vaultDir, err := vlt.GetDirectory()
+			require.NoError(t, err)
 
-	var last string
+			unlockerDir := filepath.Join(vaultDir, "unlockers.d", "passphrase")
 
-	fs := hookFs{Fs: base, before: func(_, path string) error {
-		if filepath.Dir(path) == unlockerDir {
-			last = filepath.Base(path)
-		}
+			fs := hookFs{Fs: base, before: func(string, string) error {
+				exists, err := afero.DirExists(base, unlockerDir)
+				require.NoError(t, err)
 
-		return nil
-	}}
+				if exists {
+					assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir),
+						"unlocker directory visible before it was complete")
+				}
 
-	passphrase := memguard.NewBufferFromBytes([]byte(unlockerPassphrase))
-	defer passphrase.Destroy()
+				return nil
+			}}
 
-	_, err = vault.NewVault(fs, testVaultStateDir, testVaultName).
-		CreatePassphraseUnlocker(passphrase)
-	require.NoError(t, err)
-	assert.Equal(t, unlockerMetadataFile, last)
+			passphrase := memguard.NewBufferFromBytes([]byte(unlockerPassphrase))
+			defer passphrase.Destroy()
+
+			_, err = vault.NewVault(fs, stateDir, testVaultName).
+				CreatePassphraseUnlocker(passphrase)
+			require.NoError(t, err)
+			assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir))
+		})
+	}
+}
+
+// TestWriteDirFailureLeavesNothing makes writing a new directory fail after
+// a file has been written in it, and checks that neither the directory nor
+// its temporary directory is left behind; and, when the temporary directory
+// cannot be removed either, that both failures are reported.
+func TestWriteDirFailureLeavesNothing(t *testing.T) {
+	t.Parallel()
+
+	for _, tfs := range testFilesystems {
+		t.Run(tfs.name, func(t *testing.T) {
+			t.Parallel()
+
+			base, dir := tfs.open(t)
+			listed := filepath.Join(dir, "unlockers.d")
+			target := filepath.Join(listed, "new")
+
+			writeThenFail := func(tmp string) error {
+				require.NoError(t, secret.WriteFileAtomic(base,
+					filepath.Join(tmp, unlockerMetadataFile), []byte("{}")))
+
+				return errInjected
+			}
+
+			err := secret.WriteDir(base, target, writeThenFail)
+			require.ErrorIs(t, err, errInjected)
+
+			// Nothing in the directory that is listed, nor beside it
+			assert.Empty(t, dirNames(t, base, listed))
+			assert.Equal(t, []string{"unlockers.d"}, dirNames(t, base, dir))
+
+			fs := hookFs{Fs: base, before: func(op, _ string) error {
+				if op == opRemove {
+					return os.ErrPermission
+				}
+
+				return nil
+			}}
+
+			err = secret.WriteDir(fs, target, writeThenFail)
+			require.ErrorIs(t, err, errInjected)
+			require.ErrorIs(t, err, os.ErrPermission)
+			assert.Empty(t, dirNames(t, base, listed))
+		})
+	}
+}
+
+// TestWriteDirKeepsExistingDir makes writing into a directory that already
+// exists fail, and checks that the directory, with what was in it, is still
+// there: WriteDir writes into it in place and never removes it.
+func TestWriteDirKeepsExistingDir(t *testing.T) {
+	t.Parallel()
+
+	for _, tfs := range testFilesystems {
+		t.Run(tfs.name, func(t *testing.T) {
+			t.Parallel()
+
+			fs, dir := tfs.open(t)
+			target := filepath.Join(dir, "unlockers.d", "passphrase")
+			require.NoError(t, fs.MkdirAll(target, secret.DirPerms))
+			require.NoError(t, secret.WriteFileAtomic(fs,
+				filepath.Join(target, unlockerMetadataFile), []byte("{}")))
+
+			err := secret.WriteDir(fs, target, func(got string) error {
+				assert.Equal(t, target, got)
+
+				return errInjected
+			})
+			require.ErrorIs(t, err, errInjected)
+			assert.Equal(t, []string{unlockerMetadataFile}, dirNames(t, fs, target))
+		})
+	}
 }

internal/secret/keychainunlocker.go

@@ -341,16 +341,13 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to generate keychain item name: %w", err)
 	}
 
-	// Create unlocker directory using the keychain item name as the directory name
+	// The unlocker directory is named after the keychain item
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get vault directory: %w", err)
 	}
 
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", keychainItemName)
-	if err := fs.MkdirAll(unlockerDir, DirPerms); err != nil {
-		return nil, fmt.Errorf("failed to create unlocker directory: %w", err)
-	}
 
 	// Step 1: Generate a new age keypair for the keychain unlocker
 	ageIdentity, err := age.GenerateX25519Identity()
@@ -358,6 +355,8 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to generate age keypair: %w", err)
 	}
 
+	ageRecipient := ageIdentity.Recipient().String()
+
 	// Step 2: Generate a random passphrase for encrypting the age private key
 	agePrivKeyPassphrase, err := generateRandomPassphrase(agePrivKeyPassphraseLength)
 	if err != nil {
@@ -365,14 +364,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	}
 	defer agePrivKeyPassphrase.Destroy()
 
-	// Step 3: Store age recipient as plaintext
-	ageRecipient := ageIdentity.Recipient().String()
-	recipientPath := filepath.Join(unlockerDir, "pub.txt")
-	if err := WriteFileAtomic(fs, recipientPath, []byte(ageRecipient)); err != nil {
-		return nil, fmt.Errorf("failed to write age recipient: %w", err)
-	}
-
-	// Step 4: Encrypt age private key with the generated passphrase and store on disk
+	// Step 3: Encrypt age private key with the generated passphrase
 	// Create a secure buffer for the private key
 	agePrivKeyStr := ageIdentity.String()
 	agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr))
@@ -383,31 +375,20 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err)
 	}
 
-	agePrivKeyPath := filepath.Join(unlockerDir, "priv.age")
-	if err := WriteFileAtomic(fs, agePrivKeyPath, encryptedAgePrivKey); err != nil {
-		return nil, fmt.Errorf("failed to write encrypted age private key: %w", err)
-	}
-
-	// Step 5: Get or derive the long-term private key
+	// Step 4: Get or derive the long-term private key
 	ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
 	if err != nil {
 		return nil, err
 	}
 	defer ltPrivKeyData.Destroy()
 
-	// Step 6: Encrypt long-term private key to the new age unlocker
+	// Step 5: Encrypt long-term private key to the new age unlocker
 	encryptedLtPrivKeyToAge, err := EncryptToRecipient(ltPrivKeyData, ageIdentity.Recipient())
 	if err != nil {
 		return nil, fmt.Errorf("failed to encrypt long-term private key to age unlocker: %w", err)
 	}
 
-	// Write encrypted long-term private key
-	ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
-	if err := WriteFileAtomic(fs, ltPrivKeyPath, encryptedLtPrivKeyToAge); err != nil {
-		return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
-	}
-
-	// Step 7: Prepare keychain data
+	// Step 6: Prepare keychain data
 	keychainData := KeychainData{
 		AgePublicKey:         ageRecipient,
 		AgePrivKeyPassphrase: agePrivKeyPassphrase,
@@ -420,12 +401,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	}
 	defer keychainDataBuffer.Destroy()
 
-	// Step 8: Store data in keychain
-	if err := storeInKeychain(keychainItemName, keychainDataBuffer); err != nil {
-		return nil, fmt.Errorf("failed to store data in keychain: %w", err)
-	}
-
-	// Step 9: Create and write enhanced metadata
+	// Step 7: Prepare enhanced metadata
 	keychainMetadata := KeychainUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
 			Type:      "keychain",
@@ -440,10 +416,37 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err)
 	}
 
-	if err := WriteFileAtomic(fs,
-		filepath.Join(unlockerDir, "unlocker-metadata.json"),
-		metadataBytes); err != nil {
-		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
+	// Step 8: Write the unlocker's files and store the data in the keychain,
+	// the metadata last
+	err = WriteDir(fs, unlockerDir, func(dir string) error {
+		pubPath := filepath.Join(dir, "pub.txt")
+		if err := WriteFileAtomic(fs, pubPath, []byte(ageRecipient)); err != nil {
+			return fmt.Errorf("failed to write age recipient: %w", err)
+		}
+
+		privPath := filepath.Join(dir, "priv.age")
+		if err := WriteFileAtomic(fs, privPath, encryptedAgePrivKey); err != nil {
+			return fmt.Errorf("failed to write encrypted age private key: %w", err)
+		}
+
+		ltKeyPath := filepath.Join(dir, "longterm.age")
+		if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtPrivKeyToAge); err != nil {
+			return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
+		}
+
+		if err := storeInKeychain(keychainItemName, keychainDataBuffer); err != nil {
+			return fmt.Errorf("failed to store data in keychain: %w", err)
+		}
+
+		metadataPath := filepath.Join(dir, "unlocker-metadata.json")
+		if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
+			return fmt.Errorf("failed to write unlocker metadata: %w", err)
+		}
+
+		return nil
+	})
+	if err != nil {
+		return nil, err
 	}
 
 	return &KeychainUnlocker{

internal/secret/pgpunlock_test.go

@@ -290,7 +290,7 @@ Passphrase: ` + testPassphrase + `
 		}
 
 		// Now create a PGP unlock key (this will use our custom GPGEncryptFunc)
-		pgpUnlocker, err := secret.CreatePGPUnlocker(fs, stateDir, keyID)
+		pgpUnlocker, err := secret.CreatePGPUnlocker(fs, stateDir, keyID, fingerprint)
 		if err != nil {
 			t.Fatalf("Failed to create PGP unlock key: %v", err)
 		}

internal/secret/pgpunlocker.go

@@ -222,20 +222,13 @@ func generatePGPUnlockerName() (string, error) {
 	return fmt.Sprintf("%s-pgp-%s", hostname, enrollmentDate), nil
 }
 
-// preparePGPUnlockerDir checks GPG availability and creates the
-// unlocker directory in the current vault, returning the vault and the
-// directory path.
+// pgpUnlockerDir returns the current vault and the directory in it for a
+// new PGP unlocker, named after the host and the day.
 //
 //nolint:ireturn // the vault is only available behind VaultInterface
-func preparePGPUnlockerDir(
+func pgpUnlockerDir(
 	fs afero.Fs, stateDir string,
 ) (VaultInterface, string, error) {
-	// Check if GPG is available
-	err := checkGPGAvailable()
-	if err != nil {
-		return nil, "", err
-	}
-
 	// Get current vault
 	vault, err := GetCurrentVault(fs, stateDir)
 	if err != nil {
@@ -248,27 +241,29 @@ func preparePGPUnlockerDir(
 		return nil, "", fmt.Errorf("failed to generate unlocker name: %w", err)
 	}
 
-	// Create unlocker directory using the generated name
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, "", fmt.Errorf("failed to get vault directory: %w", err)
 	}
 
-	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerName)
-
-	err = fs.MkdirAll(unlockerDir, DirPerms)
-	if err != nil {
-		return nil, "", fmt.Errorf("failed to create unlocker directory: %w", err)
-	}
-
-	return vault, unlockerDir, nil
+	return vault, filepath.Join(vaultDir, "unlockers.d", unlockerName), nil
 }
 
-// CreatePGPUnlocker creates a new PGP unlocker and stores it in the vault
+// CreatePGPUnlocker creates a new PGP unlocker and stores it in the vault.
+// It encrypts to the GPG key gpgKeyID and records fingerprint, that key's
+// fingerprint as ResolveGPGKeyFingerprint returns it, in the metadata.
+// Everything that can fail short of writing a file is done before anything
+// is written, and the files are written through WriteDir, so a failure
+// leaves no partial unlocker.
 func CreatePGPUnlocker(
-	fs afero.Fs, stateDir string, gpgKeyID string,
+	fs afero.Fs, stateDir, gpgKeyID, fingerprint string,
 ) (*PGPUnlocker, error) {
-	vault, unlockerDir, err := preparePGPUnlockerDir(fs, stateDir)
+	err := checkGPGAvailable()
+	if err != nil {
+		return nil, err
+	}
+
+	vault, unlockerDir, err := pgpUnlockerDir(fs, stateDir)
 	if err != nil {
 		return nil, err
 	}
@@ -279,77 +274,13 @@ func CreatePGPUnlocker(
 		return nil, fmt.Errorf("failed to generate age keypair: %w", err)
 	}
 
-	// Step 2: Store age recipient as plaintext
-	ageRecipient := ageIdentity.Recipient().String()
-	recipientPath := filepath.Join(unlockerDir, "pub.txt")
-
-	err = WriteFileAtomic(fs, recipientPath, []byte(ageRecipient))
-	if err != nil {
-		return nil, fmt.Errorf("failed to write age recipient: %w", err)
-	}
-
-	// Step 3: Get or derive the long-term private key
-	ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
+	// Step 2: Encrypt the long-term private key to the new keypair, and the
+	// keypair's private key to the GPG key
+	encryptedLtPrivKey, encryptedAgePrivKey, err := encryptPGPUnlockerKeys(
+		fs, vault, ageIdentity, gpgKeyID)
 	if err != nil {
 		return nil, err
 	}
-	defer ltPrivKeyData.Destroy()
-
-	// Step 7: Encrypt long-term private key to the new age unlocker
-	encryptedLtPrivKeyToAge, err := EncryptToRecipient(
-		ltPrivKeyData, ageIdentity.Recipient())
-	if err != nil {
-		return nil, fmt.Errorf(
-			"failed to encrypt long-term private key to age unlocker: %w", err)
-	}
-
-	// Write encrypted long-term private key
-	ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
-
-	err = WriteFileAtomic(fs, ltPrivKeyPath, encryptedLtPrivKeyToAge)
-	if err != nil {
-		return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
-	}
-
-	// Step 8: Encrypt age private key to the GPG key ID
-	// Use memguard to protect the private key in memory
-	agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String()))
-	defer agePrivateKeyBuffer.Destroy()
-
-	encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to encrypt age private key with GPG: %w", err)
-	}
-
-	agePrivKeyPath := filepath.Join(unlockerDir, "priv.age.gpg")
-
-	err = WriteFileAtomic(fs, agePrivKeyPath, encryptedAgePrivKey)
-	if err != nil {
-		return nil, fmt.Errorf("failed to write encrypted age private key: %w", err)
-	}
-
-	// Steps 9-10: Resolve the fingerprint and write enhanced metadata
-	pgpMetadata, err := writePGPUnlockerMetadata(fs, unlockerDir, gpgKeyID)
-	if err != nil {
-		return nil, err
-	}
-
-	return &PGPUnlocker{
-		Directory: unlockerDir,
-		Metadata:  pgpMetadata.UnlockerMetadata,
-		fs:        fs,
-	}, nil
-}
-
-// writePGPUnlockerMetadata resolves the GPG key fingerprint and writes
-// the unlocker metadata file, returning the metadata written.
-func writePGPUnlockerMetadata(
-	fs afero.Fs, unlockerDir string, gpgKeyID string,
-) (*PGPUnlockerMetadata, error) {
-	fingerprint, err := ResolveGPGKeyFingerprint(gpgKeyID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
-	}
 
 	pgpMetadata := PGPUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
@@ -365,13 +296,85 @@ func writePGPUnlockerMetadata(
 		return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err)
 	}
 
-	err = WriteFileAtomic(fs,
-		filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes)
+	// Step 3: Write the unlocker's files, the metadata last
+	err = WriteDir(fs, unlockerDir, func(dir string) error {
+		return writePGPUnlockerFiles(fs, dir, ageIdentity.Recipient(),
+			encryptedLtPrivKey, encryptedAgePrivKey, metadataBytes)
+	})
 	if err != nil {
-		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
+		return nil, err
 	}
 
-	return &pgpMetadata, nil
+	return &PGPUnlocker{
+		Directory: unlockerDir,
+		Metadata:  pgpMetadata.UnlockerMetadata,
+		fs:        fs,
+	}, nil
+}
+
+// encryptPGPUnlockerKeys returns the vault's long-term private key encrypted
+// to the new PGP unlocker's age keypair, and that keypair's private key
+// encrypted to the GPG key gpgKeyID.
+func encryptPGPUnlockerKeys(
+	fs afero.Fs, vault VaultInterface,
+	ageIdentity *age.X25519Identity, gpgKeyID string,
+) ([]byte, []byte, error) {
+	// Get or derive the long-term private key
+	ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
+	if err != nil {
+		return nil, nil, err
+	}
+	defer ltPrivKeyData.Destroy()
+
+	encryptedLtPrivKey, err := EncryptToRecipient(
+		ltPrivKeyData, ageIdentity.Recipient())
+	if err != nil {
+		return nil, nil, fmt.Errorf(
+			"failed to encrypt long-term private key to age unlocker: %w", err)
+	}
+
+	// Use memguard to protect the private key in memory
+	agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String()))
+	defer agePrivateKeyBuffer.Destroy()
+
+	encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
+	if err != nil {
+		return nil, nil, fmt.Errorf(
+			"failed to encrypt age private key with GPG: %w", err)
+	}
+
+	return encryptedLtPrivKey, encryptedAgePrivKey, nil
+}
+
+// writePGPUnlockerFiles writes the files of a PGP unlocker into dir, the
+// metadata last.
+func writePGPUnlockerFiles(
+	fs afero.Fs, dir string, ageRecipient *age.X25519Recipient,
+	encryptedLtPrivKey, encryptedAgePrivKey, metadataBytes []byte,
+) error {
+	err := WriteFileAtomic(fs, filepath.Join(dir, "pub.txt"),
+		[]byte(ageRecipient.String()))
+	if err != nil {
+		return fmt.Errorf("failed to write age recipient: %w", err)
+	}
+
+	err = WriteFileAtomic(fs, filepath.Join(dir, "longterm.age"), encryptedLtPrivKey)
+	if err != nil {
+		return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
+	}
+
+	err = WriteFileAtomic(fs, filepath.Join(dir, "priv.age.gpg"), encryptedAgePrivKey)
+	if err != nil {
+		return fmt.Errorf("failed to write encrypted age private key: %w", err)
+	}
+
+	err = WriteFileAtomic(fs,
+		filepath.Join(dir, "unlocker-metadata.json"), metadataBytes)
+	if err != nil {
+		return fmt.Errorf("failed to write unlocker metadata: %w", err)
+	}
+
+	return nil
 }
 
 // validateGPGKeyID validates that a GPG key ID is safe for command execution

internal/secret/pgpunlocker_test.go

@@ -0,0 +1,67 @@
+package secret_test
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"git.eeqj.de/sneak/secret/internal/secret"
+	"git.eeqj.de/sneak/secret/internal/vault"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// The GPG key ID and fingerprint passed to CreatePGPUnlocker.
+const (
+	testGPGKeyID       = "0123456789ABCDEF"
+	testGPGFingerprint = "0123456789ABCDEF0123456789ABCDEF01234567"
+)
+
+// fakeGPGScript is a gpg for which `gpg --version` succeeds and anything
+// else fails.
+const fakeGPGScript = `#!/bin/sh
+[ "$*" = --version ]
+`
+
+// installFakeGPG makes fakeGPGScript the only gpg on PATH for the test.
+func installFakeGPG(t *testing.T) {
+	t.Helper()
+
+	dir := t.TempDir()
+
+	//nolint:gosec // G306: the script must be executable
+	err := os.WriteFile(filepath.Join(dir, "gpg"), []byte(fakeGPGScript), 0o700)
+	require.NoError(t, err)
+
+	t.Setenv("PATH", dir)
+}
+
+// TestCreatePGPUnlockerFailureWritesNothing makes CreatePGPUnlocker fail at
+// getting the vault's long-term key, which used to come after part of the
+// unlocker was written, and asserts that nothing is written. Getting the key
+// fails because on macOS there is no mnemonic and no current unlocker, and
+// on every other platform it always fails
+// (https://git.eeqj.de/sneak/secret/issues/88).
+func TestCreatePGPUnlockerFailureWritesNothing(t *testing.T) {
+	installFakeGPG(t)
+	t.Setenv(secret.EnvMnemonic, "")
+
+	base := afero.NewMemMapFs()
+	vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
+	require.NoError(t, err)
+
+	fs := hookFs{Fs: base, before: func(_, path string) error {
+		t.Errorf("changed %s", path)
+
+		return nil
+	}}
+
+	_, err = secret.CreatePGPUnlocker(
+		fs, testVaultStateDir, testGPGKeyID, testGPGFingerprint)
+	require.Error(t, err)
+
+	vaultDir, err := vlt.GetDirectory()
+	require.NoError(t, err)
+	assert.Empty(t, dirNames(t, base, filepath.Join(vaultDir, "unlockers.d")))
+}

internal/secret/seunlocker_darwin.go

@@ -254,7 +254,7 @@ func CreateSecureEnclaveUnlocker(
 		)
 	}
 
-	// Step 4: Create unlocker directory and write files
+	// Step 4: Prepare the unlocker directory's path and metadata
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get vault directory: %w", err)
@@ -262,23 +262,7 @@ func CreateSecureEnclaveUnlocker(
 
 	unlockerDirName := fmt.Sprintf("se-%s", filepath.Base(seKeyLabel))
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerDirName)
-	if err := fs.MkdirAll(unlockerDir, DirPerms); err != nil {
-		return nil, fmt.Errorf(
-			"failed to create unlocker directory: %w",
-			err,
-		)
-	}
 
-	// Write SE-encrypted long-term key
-	ltKeyPath := filepath.Join(unlockerDir, seLongtermFilename)
-	if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtKey); err != nil {
-		return nil, fmt.Errorf(
-			"failed to write SE-encrypted long-term key: %w",
-			err,
-		)
-	}
-
-	// Write metadata
 	seMetadata := SecureEnclaveUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
 			Type:      seUnlockerType,
@@ -294,9 +278,25 @@ func CreateSecureEnclaveUnlocker(
 		return nil, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
-	if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
-		return nil, fmt.Errorf("failed to write metadata: %w", err)
+	// Step 5: Write the SE-encrypted long-term key, then the metadata
+	err = WriteDir(fs, unlockerDir, func(dir string) error {
+		ltKeyPath := filepath.Join(dir, seLongtermFilename)
+		if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtKey); err != nil {
+			return fmt.Errorf(
+				"failed to write SE-encrypted long-term key: %w",
+				err,
+			)
+		}
+
+		metadataPath := filepath.Join(dir, "unlocker-metadata.json")
+		if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
+			return fmt.Errorf("failed to write metadata: %w", err)
+		}
+
+		return nil
+	})
+	if err != nil {
+		return nil, err
 	}
 
 	return &SecureEnclaveUnlocker{

internal/vault/unlockers.go

@@ -357,26 +357,14 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to get long-term key: %w", err)
 	}
 
-	// Create unlocker directory
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerTypePassphrase)
 
-	err = v.fs.MkdirAll(unlockerDir, secret.DirPerms)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create unlocker directory: %w", err)
-	}
-
 	// Generate new age keypair for unlocker
 	unlockerIdentity, err := age.GenerateX25519Identity()
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate unlocker: %w", err)
 	}
 
-	// Write the unlocker keypair (public and passphrase-encrypted private)
-	err = v.writeUnlockerKeypair(unlockerDir, unlockerIdentity, passphrase)
-	if err != nil {
-		return nil, err
-	}
-
 	// Encrypt long-term private key to this unlocker
 	ltPrivKeyBuffer := memguard.NewBufferFromBytes([]byte(ltIdentity.String()))
 	defer ltPrivKeyBuffer.Destroy()
@@ -387,15 +375,6 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to encrypt long-term private key: %w", err)
 	}
 
-	ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
-
-	err = secret.WriteFileAtomic(v.fs, ltPrivKeyPath, encryptedLtPrivKey)
-	if err != nil {
-		return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
-	}
-
-	// Write the metadata last: readers skip an unlocker directory without
-	// it, so an unlocker interrupted before this point is never used.
 	metadata := UnlockerMetadata{
 		Type:      unlockerTypePassphrase,
 		CreatedAt: time.Now(),
@@ -407,11 +386,13 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
-
-	err = secret.WriteFileAtomic(v.fs, metadataPath, metadataBytes)
+	// Write the unlocker's files, the metadata last
+	err = secret.WriteDir(v.fs, unlockerDir, func(dir string) error {
+		return v.writeUnlockerFiles(dir, unlockerIdentity, passphrase,
+			encryptedLtPrivKey, metadataBytes)
+	})
 	if err != nil {
-		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
+		return nil, err
 	}
 
 	// Create the unlocker instance
@@ -457,12 +438,14 @@ func (v *Vault) readUnlockerMetadata(unlockerDir string) (UnlockerMetadata, erro
 	return metadata, nil
 }
 
-// writeUnlockerKeypair writes the unlocker's public key and its
-// passphrase-encrypted private key into the unlocker directory.
-func (v *Vault) writeUnlockerKeypair(
+// writeUnlockerFiles writes the files of a passphrase unlocker into
+// unlockerDir: its public key, its passphrase-encrypted private key, the
+// long-term private key encrypted to it, and its metadata, last.
+func (v *Vault) writeUnlockerFiles(
 	unlockerDir string,
 	unlockerIdentity *age.X25519Identity,
 	passphrase *memguard.LockedBuffer,
+	encryptedLtPrivKey, metadataBytes []byte,
 ) error {
 	// Write public key
 	pubKeyPath := filepath.Join(unlockerDir, "pub.age")
@@ -492,5 +475,17 @@ func (v *Vault) writeUnlockerKeypair(
 		return fmt.Errorf("failed to write encrypted unlocker private key: %w", err)
 	}
 
+	err = secret.WriteFileAtomic(v.fs,
+		filepath.Join(unlockerDir, "longterm.age"), encryptedLtPrivKey)
+	if err != nil {
+		return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
+	}
+
+	err = secret.WriteFileAtomic(v.fs,
+		filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes)
+	if err != nil {
+		return fmt.Errorf("failed to write unlocker metadata: %w", err)
+	}
+
 	return nil
 }

script/cibuild

@@ -4,13 +4,17 @@
 # The Gitea workflow runs this on push. The memlock ulimit lets the tests
 # that lock large secrets in memory (memguard mlocks them) run; under the
 # lower limit of a plain `docker build .` they are skipped.
+# A cached build checks nothing: a new CHECK_EPOCH on every run makes the
+# Dockerfile's check steps run again on an unchanged tree, while its base
+# images and module downloads stay cached.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    docker build --ulimit memlock=-1:-1 .
+    docker build --ulimit memlock=-1:-1 \
+        --build-arg CHECK_EPOCH="$(date +%s)" .
 }
 
 main "$@"