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` and both
vault names of `mv` 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,11 +6,6 @@ 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
@@ -30,9 +25,6 @@ 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,8 +526,7 @@ 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), with a new `CHECK_EPOCH` build argument on every run so the
-  checks run again on an unchanged tree
+  checks)
 - `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,30 +29,10 @@ 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`, both vault names of `mv` and shell completion of a
-  `vault:secret` argument check the name as typed with
+  `vault remove` and both vault names of `mv` 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
@@ -157,10 +137,13 @@ 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;
-    - 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
+    - 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
       (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,9 +123,7 @@ func getVaultNamesCompletionFunc(fs afero.Fs, stateDir string) func(
 }
 
 // completeVaultQualifiedSecrets completes "vault:secret" references once a
-// 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.
+// colon is present in the input
 func completeVaultQualifiedSecrets(
 	fs afero.Fs, stateDir, toComplete string,
 ) []string {
@@ -136,10 +134,6 @@ 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

@@ -1,41 +0,0 @@
-//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,8 +685,7 @@ 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, once: the duplicate check
-	// and the new unlocker's metadata both use this result
+	// Resolve the GPG key ID to its fingerprint
 	fingerprint, err := secret.ResolveGPGKeyFingerprint(gpgKeyID)
 	if err != nil {
 		return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
@@ -707,8 +706,7 @@ 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, fingerprint)
+	pgpUnlocker, err := secret.CreatePGPUnlocker(cli.fs, cli.stateDir, gpgKeyID)
 	if err != nil {
 		return err
 	}

internal/cli/unlockers_add_test.go

@@ -1,35 +0,0 @@
-//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,7 +1,6 @@
 package secret
 
 import (
-	"errors"
 	"fmt"
 	"path/filepath"
 
@@ -62,52 +61,6 @@ 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,10 +35,6 @@ 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
@@ -457,7 +453,7 @@ func TestVersionSaveIsWholeOrAbsent(t *testing.T) {
 
 				if exists {
 					assert.ElementsMatch(t,
-						[]string{"pub.age", "value.age", privKeyFile, "metadata.age"},
+						[]string{"pub.age", "value.age", "priv.age", "metadata.age"},
 						dirNames(t, base, versionDir),
 						"version directory visible before it was complete")
 				}
@@ -500,7 +496,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) == privKeyFile {
+				if op == opRename && filepath.Base(path) == "priv.age" {
 					return errInjected
 				}
 
@@ -646,21 +642,15 @@ func TestPassphraseUnlockerGetsKeyFirst(t *testing.T) {
 	require.Error(t, err)
 }
 
-// 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) {
+// 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) {
 	t.Setenv(secret.EnvMnemonic, testMnemonic)
 
-	files := []string{"pub.age", privKeyFile, "longterm.age", unlockerMetadataFile}
-
-	for _, tfs := range testFilesystems {
-		t.Run(tfs.name, func(t *testing.T) {
-			base, stateDir := tfs.open(t)
-			vlt, err := vault.CreateVault(base, stateDir, testVaultName)
+	base := afero.NewMemMapFs()
+	vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
 	require.NoError(t, err)
 
 	vaultDir, err := vlt.GetDirectory()
@@ -668,13 +658,11 @@ func TestPassphraseUnlockerIsWholeOrAbsent(t *testing.T) {
 
 	unlockerDir := filepath.Join(vaultDir, "unlockers.d", "passphrase")
 
-			fs := hookFs{Fs: base, before: func(string, string) error {
-				exists, err := afero.DirExists(base, unlockerDir)
-				require.NoError(t, err)
+	var last string
 
-				if exists {
-					assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir),
-						"unlocker directory visible before it was complete")
+	fs := hookFs{Fs: base, before: func(_, path string) error {
+		if filepath.Dir(path) == unlockerDir {
+			last = filepath.Base(path)
 		}
 
 		return nil
@@ -683,82 +671,8 @@ func TestPassphraseUnlockerIsWholeOrAbsent(t *testing.T) {
 	passphrase := memguard.NewBufferFromBytes([]byte(unlockerPassphrase))
 	defer passphrase.Destroy()
 
-			_, err = vault.NewVault(fs, stateDir, testVaultName).
+	_, err = vault.NewVault(fs, testVaultStateDir, 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))
-		})
-	}
+	assert.Equal(t, unlockerMetadataFile, last)
 }

internal/secret/keychainunlocker.go

@@ -341,13 +341,16 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to generate keychain item name: %w", err)
 	}
 
-	// The unlocker directory is named after the keychain item
+	// Create unlocker directory using the keychain item name as the directory 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", 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()
@@ -355,8 +358,6 @@ 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 {
@@ -364,7 +365,14 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	}
 	defer agePrivKeyPassphrase.Destroy()
 
-	// Step 3: Encrypt age private key with the generated passphrase
+	// 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
 	// Create a secure buffer for the private key
 	agePrivKeyStr := ageIdentity.String()
 	agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr))
@@ -375,20 +383,31 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err)
 	}
 
-	// Step 4: Get or derive the long-term private key
+	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
 	ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
 	if err != nil {
 		return nil, err
 	}
 	defer ltPrivKeyData.Destroy()
 
-	// Step 5: Encrypt long-term private key to the new age unlocker
+	// Step 6: 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)
 	}
 
-	// Step 6: Prepare keychain data
+	// 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
 	keychainData := KeychainData{
 		AgePublicKey:         ageRecipient,
 		AgePrivKeyPassphrase: agePrivKeyPassphrase,
@@ -401,7 +420,12 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 	}
 	defer keychainDataBuffer.Destroy()
 
-	// Step 7: Prepare enhanced metadata
+	// 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
 	keychainMetadata := KeychainUnlockerMetadata{
 		UnlockerMetadata: UnlockerMetadata{
 			Type:      "keychain",
@@ -416,37 +440,10 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
 		return nil, fmt.Errorf("failed to marshal 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
+	if err := WriteFileAtomic(fs,
+		filepath.Join(unlockerDir, "unlocker-metadata.json"),
+		metadataBytes); err != nil {
+		return nil, fmt.Errorf("failed to write unlocker metadata: %w", 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, fingerprint)
+		pgpUnlocker, err := secret.CreatePGPUnlocker(fs, stateDir, keyID)
 		if err != nil {
 			t.Fatalf("Failed to create PGP unlock key: %v", err)
 		}

internal/secret/pgpunlocker.go

@@ -222,13 +222,20 @@ func generatePGPUnlockerName() (string, error) {
 	return fmt.Sprintf("%s-pgp-%s", hostname, enrollmentDate), nil
 }
 
-// pgpUnlockerDir returns the current vault and the directory in it for a
-// new PGP unlocker, named after the host and the day.
+// preparePGPUnlockerDir checks GPG availability and creates the
+// unlocker directory in the current vault, returning the vault and the
+// directory path.
 //
 //nolint:ireturn // the vault is only available behind VaultInterface
-func pgpUnlockerDir(
+func preparePGPUnlockerDir(
 	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 {
@@ -241,29 +248,27 @@ func pgpUnlockerDir(
 		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)
 	}
 
-	return vault, filepath.Join(vaultDir, "unlockers.d", unlockerName), nil
-}
+	unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerName)
 
-// 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, gpgKeyID, fingerprint string,
-) (*PGPUnlocker, error) {
-	err := checkGPGAvailable()
+	err = fs.MkdirAll(unlockerDir, DirPerms)
 	if err != nil {
-		return nil, err
+		return nil, "", fmt.Errorf("failed to create unlocker directory: %w", err)
 	}
 
-	vault, unlockerDir, err := pgpUnlockerDir(fs, stateDir)
+	return vault, unlockerDir, nil
+}
+
+// CreatePGPUnlocker creates a new PGP unlocker and stores it in the vault
+func CreatePGPUnlocker(
+	fs afero.Fs, stateDir string, gpgKeyID string,
+) (*PGPUnlocker, error) {
+	vault, unlockerDir, err := preparePGPUnlockerDir(fs, stateDir)
 	if err != nil {
 		return nil, err
 	}
@@ -274,13 +279,77 @@ func CreatePGPUnlocker(
 		return nil, fmt.Errorf("failed to generate age keypair: %w", err)
 	}
 
-	// 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)
+	// 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)
 	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{
@@ -296,85 +365,13 @@ func CreatePGPUnlocker(
 		return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err)
 	}
 
-	// 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, err
-	}
-
-	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)
+		filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes)
 	if err != nil {
-		return fmt.Errorf("failed to write unlocker metadata: %w", err)
+		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
 	}
 
-	return nil
+	return &pgpMetadata, nil
 }
 
 // validateGPGKeyID validates that a GPG key ID is safe for command execution

internal/secret/pgpunlocker_test.go

@@ -1,67 +0,0 @@
-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: Prepare the unlocker directory's path and metadata
+	// Step 4: Create unlocker directory and write files
 	vaultDir, err := vault.GetDirectory()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get vault directory: %w", err)
@@ -262,7 +262,23 @@ 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,
@@ -278,25 +294,9 @@ func CreateSecureEnclaveUnlocker(
 		return nil, fmt.Errorf("failed to marshal 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")
+	metadataPath := filepath.Join(unlockerDir, "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 nil, fmt.Errorf("failed to write metadata: %w", err)
 	}
 
 	return &SecureEnclaveUnlocker{

internal/vault/unlockers.go

@@ -357,14 +357,26 @@ 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()
@@ -375,6 +387,15 @@ 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(),
@@ -386,13 +407,11 @@ func (v *Vault) CreatePassphraseUnlocker(
 		return nil, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	// 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)
-	})
+	metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
+
+	err = secret.WriteFileAtomic(v.fs, metadataPath, metadataBytes)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to write unlocker metadata: %w", err)
 	}
 
 	// Create the unlocker instance
@@ -438,14 +457,12 @@ func (v *Vault) readUnlockerMetadata(unlockerDir string) (UnlockerMetadata, erro
 	return metadata, nil
 }
 
-// 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(
+// writeUnlockerKeypair writes the unlocker's public key and its
+// passphrase-encrypted private key into the unlocker directory.
+func (v *Vault) writeUnlockerKeypair(
 	unlockerDir string,
 	unlockerIdentity *age.X25519Identity,
 	passphrase *memguard.LockedBuffer,
-	encryptedLtPrivKey, metadataBytes []byte,
 ) error {
 	// Write public key
 	pubKeyPath := filepath.Join(unlockerDir, "pub.age")
@@ -475,17 +492,5 @@ func (v *Vault) writeUnlockerFiles(
 		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,17 +4,13 @@
 # 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 \
-        --build-arg CHECK_EPOCH="$(date +%s)" .
+    docker build --ulimit memlock=-1:-1 .
 }
 
 main "$@"