From 017b8d73bf2060fd40b17da0f6bbecebb0015b52 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 18:07:56 +0200 Subject: [PATCH] Type-check and lint the macOS build from Linux (closes #50) script/lint-darwin (make lint-darwin; run by script/check, and its commands by the Dockerfile lint stage) runs go vet and golangci-lint with GOOS=darwin and cgo off. Compiling cgo for macOS needs Apple's SDK, so the three functions that call go-keychain, which is cgo there, move to keychainunlocker_cgo.go; a macOS build without cgo gets keychainunlocker_nocgo.go and the macse stub, whose errors name the missing macOS build with cgo. The rest of the keychain unlocker and its plain-Go tests are now checked; their findings are fixed without changing behaviour, and lines over 88 columns in the unchecked files are wrapped. Model: opus-5-5 --- Dockerfile | 5 +- Dockerfile.lint | 16 +- Makefile | 8 +- README.md | 14 +- TODO.md | 34 +- internal/macse/macse_darwin.go | 12 +- internal/macse/macse_stub.go | 12 +- internal/macse/macse_test.go | 9 +- internal/secret/derivation_index_test.go | 47 +- internal/secret/keychainunlocker.go | 406 +++++------ internal/secret/keychainunlocker_cgo.go | 104 +++ internal/secret/keychainunlocker_nocgo.go | 30 + internal/secret/keychainunlocker_test.go | 15 +- internal/secret/pgpunlock_test.go | 780 +++++++++++++--------- internal/secret/seunlocker_darwin.go | 124 ++-- internal/secret/seunlocker_test.go | 28 +- internal/secret/validation_darwin_test.go | 149 +---- script/check | 6 +- script/lint-darwin | 26 + 19 files changed, 1056 insertions(+), 769 deletions(-) create mode 100644 internal/secret/keychainunlocker_cgo.go create mode 100644 internal/secret/keychainunlocker_nocgo.go create mode 100755 script/lint-darwin diff --git a/Dockerfile b/Dockerfile index 8e852de..de226b1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,8 +14,11 @@ ARG CHECK_EPOCH COPY . . RUN make fmt-check -# Not make lint: script/lint is a docker build, which cannot run in here. +# Not make lint or make lint-darwin: script/lint and script/lint-darwin are +# docker builds, which cannot run in here. These are their commands. RUN golangci-lint run --config .golangci.yml ./... +RUN GOOS=darwin CGO_ENABLED=0 go vet ./... +RUN GOOS=darwin CGO_ENABLED=0 golangci-lint run --config .golangci.yml ./... # Build stage — tests and compilation # golang 1.24.13-alpine (2026-03-10) diff --git a/Dockerfile.lint b/Dockerfile.lint index da5b64b..0ce5e52 100644 --- a/Dockerfile.lint +++ b/Dockerfile.lint @@ -1,6 +1,6 @@ -# Lint image, built by script/lint: golangci-lint runs as a build step, so a -# successful build is a clean lint. Works where the docker daemon is remote -# and bind mounts are impossible. +# Lint image, built by script/lint and script/lint-darwin: golangci-lint runs +# as a build step, so a successful build is a clean lint. Works where the +# docker daemon is remote and bind mounts are impossible. # golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07 FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps @@ -17,3 +17,13 @@ FROM deps AS lint COPY . . RUN golangci-lint run --config .golangci.yml ./... + +# script/lint-darwin rebuilds this stage on every run, by this name. It +# checks the code as a macOS build compiles it, but with cgo off, which +# leaves out the files that need cgo on macOS (see script/lint-darwin). +FROM deps AS lint-darwin + +COPY . . + +RUN GOOS=darwin CGO_ENABLED=0 go vet ./... +RUN GOOS=darwin CGO_ENABLED=0 golangci-lint run --config .golangci.yml ./... diff --git a/Makefile b/Makefile index 041b902..ff671ce 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ export CGO_ENABLED=1 -.PHONY: default bootstrap setup build test lint fmt fmt-check check docker \ - docker-run clean install hooks +.PHONY: default bootstrap setup build test lint lint-darwin fmt fmt-check \ + check docker docker-run clean install hooks default: check @@ -24,6 +24,10 @@ fmt: lint: @script/lint +# Type-check and lint the macOS build from Linux (see script/lint-darwin) +lint-darwin: + @script/lint-darwin + check: @script/check diff --git a/README.md b/README.md index 24dd19a..c746a73 100644 --- a/README.md +++ b/README.md @@ -498,6 +498,10 @@ secret decrypt encryption/mykey --input document.txt.age --output document.txt - **macOS**: Full support including Keychain and Secure Enclave integration - **Linux**: Full support (excluding macOS-specific features) +The keychain and Secure Enclave unlockers need a macOS build with cgo. A macOS +build without cgo, such as one cross-compiled from Linux, offers them but fails +to add or use them. + ## Security Considerations ### Threat Model @@ -560,10 +564,16 @@ them. We provide: - `script/lint` — run `golangci-lint` in docker only: builds `Dockerfile.lint`, where the linter is a build step that runs on every call, also on an unchanged tree +- `script/lint-darwin` — run `go vet` and `golangci-lint` in docker on + the code as a macOS build compiles it (`GOOS=darwin`), which a Linux + build never compiles; cgo is off, so the keychain unlocker's calls into + the keychain (`internal/secret/keychainunlocker_cgo.go`, and + `keychainunlocker_test.go`) and the Secure Enclave bindings + (`internal/macse`) are not checked - `script/fmt` — format all Go code (writes) - `script/fmt-check` — check formatting without writing -- `script/check` — run `script/test`, `script/lint`, and - `script/fmt-check` +- `script/check` — run `script/test`, `script/lint`, + `script/lint-darwin`, and `script/fmt-check` - `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 diff --git a/TODO.md b/TODO.md index 401fbca..40eef4c 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,27 @@ Bring the repo into policy compliance in one commit: # Completed Steps +- 2026-10-04: `script/lint-darwin` (`make lint-darwin`) runs `go vet` and + `golangci-lint` in docker on the code as a macOS build compiles it + (`GOOS=darwin`), with cgo off + (https://git.eeqj.de/sneak/secret/issues/50). `script/check` runs it, and + the `Dockerfile` lint stage runs its commands, so `script/cibuild` does too. + Before, CI on Linux never compiled the files built only for macOS. Compiling + cgo code for macOS needs Apple's SDK headers, and both `internal/macse` and + `github.com/keybase/go-keychain` are cgo on macOS. So the three functions + that call `go-keychain` moved from `keychainunlocker.go` to + `keychainunlocker_cgo.go`, built only with cgo on macOS like + `macse_darwin.go`. A macOS build without cgo, which before did not compile, + gets `keychainunlocker_nocgo.go` and the `macse` stub instead, whose errors + say the keychain or Secure Enclave needs a macOS build with cgo. The check + covers the rest of the keychain unlocker, the Secure Enclave unlocker and + the macOS-only tests other than `keychainunlocker_test.go`, whose lint + findings are fixed. For the length and complexity limits, parts of + `GetIdentity`, `getLongTermPrivateKey` and `CreateKeychainUnlocker` moved + into functions of their own, and the Secure Enclave unlocker derives the + long-term key from the mnemonic through the same function as the keychain + unlocker instead of a copy of it. Lines over 88 columns in the files the + check cannot see are wrapped. - 2026-10-04: `secret rm`, `secret version rm`, `secret vault remove` and `secret unlocker remove` ask `[y/N]` before removing anything (https://git.eeqj.de/sneak/secret/issues/39), naming what they remove: the @@ -297,11 +318,14 @@ Bring the repo into policy compliance in one commit: - Cover mnemonic-vs-xprv identity consistency in `pkg/agehd/agehd_test.go` `TestMnemonicVsXPRVConsistency` (was an in-code FIXME removed for godox). -- Darwin-gated files (`internal/secret/keychainunlocker.go`, - `seunlocker_darwin.go`, `internal/macse/macse_darwin.go`, related - tests) are not linted on the Linux CI runner and still contain lines - over the new 88-column limit; they will surface if lint ever runs on - macOS. +- CI does not compile, lint or test the files built only with cgo on + macOS, since compiling them needs Apple's SDK: + `internal/secret/keychainunlocker_cgo.go` (the three functions that call + `go-keychain`) with `keychainunlocker_test.go`, and `internal/macse` + (`macse_darwin.go`, `macse_test.go`, the Objective-C sources). Lint has + never run on them, so it would likely find more there than the line + lengths. No macOS test runs in CI. A macOS runner would cover all of it + (asked on https://git.eeqj.de/sneak/secret/issues/50). - Merge secure-enclave-unlocker to main once review is done. - 1.0 critical security blockers (from repo TODO.md): - Command injection: GPG key IDs passed unescaped to exec.Command diff --git a/internal/macse/macse_darwin.go b/internal/macse/macse_darwin.go index 4d77c3a..04cc57b 100644 --- a/internal/macse/macse_darwin.go +++ b/internal/macse/macse_darwin.go @@ -38,7 +38,8 @@ const ( ) // CreateKey creates a new P-256 non-exportable key in the Secure Enclave via sc_auth. -// Returns the uncompressed public key bytes (65 bytes) and the identity hash (for deletion). +// Returns the uncompressed public key bytes (65 bytes) and the identity hash +// (for deletion). func CreateKey(label string) (publicKey []byte, hash string, err error) { pubKeyBuf := make([]C.uint8_t, p256UncompressedKeySize) pubKeyLen := C.int(p256UncompressedKeySize) @@ -57,7 +58,8 @@ func CreateKey(label string) (publicKey []byte, hash string, err error) { return nil, "", fmt.Errorf("secure enclave: %s", C.GoString(&errBuf[0])) } - pk := C.GoBytes(unsafe.Pointer(&pubKeyBuf[0]), pubKeyLen) //nolint:nlreturn // CGo result extraction + //nolint:nlreturn // CGo result extraction + pk := C.GoBytes(unsafe.Pointer(&pubKeyBuf[0]), pubKeyLen) h := C.GoString(&hashBuf[0]) return pk, h, nil @@ -83,7 +85,8 @@ func Encrypt(label string, plaintext []byte) ([]byte, error) { return nil, fmt.Errorf("secure enclave: %s", C.GoString(&errBuf[0])) } - out := C.GoBytes(unsafe.Pointer(&ciphertextBuf[0]), ciphertextLen) //nolint:nlreturn // CGo result extraction + //nolint:nlreturn // CGo result extraction + out := C.GoBytes(unsafe.Pointer(&ciphertextBuf[0]), ciphertextLen) return out, nil } @@ -107,7 +110,8 @@ func Decrypt(label string, ciphertext []byte) ([]byte, error) { return nil, fmt.Errorf("secure enclave: %s", C.GoString(&errBuf[0])) } - out := C.GoBytes(unsafe.Pointer(&plaintextBuf[0]), plaintextLen) //nolint:nlreturn // CGo result extraction + //nolint:nlreturn // CGo result extraction + out := C.GoBytes(unsafe.Pointer(&plaintextBuf[0]), plaintextLen) return out, nil } diff --git a/internal/macse/macse_stub.go b/internal/macse/macse_stub.go index 90fb917..f813ee9 100644 --- a/internal/macse/macse_stub.go +++ b/internal/macse/macse_stub.go @@ -1,28 +1,28 @@ -//go:build !darwin +//go:build !darwin || !cgo // Package macse provides Go bindings for macOS Secure Enclave operations. package macse import "errors" -var errNotSupported = errors.New("secure enclave is only supported on macOS") +var errNotSupported = errors.New("secure enclave needs a macOS build with cgo") -// CreateKey is not supported on non-darwin platforms. +// CreateKey fails: the Secure Enclave needs a macOS build with cgo. func CreateKey(_ string) ([]byte, string, error) { return nil, "", errNotSupported } -// Encrypt is not supported on non-darwin platforms. +// Encrypt fails: the Secure Enclave needs a macOS build with cgo. func Encrypt(_ string, _ []byte) ([]byte, error) { return nil, errNotSupported } -// Decrypt is not supported on non-darwin platforms. +// Decrypt fails: the Secure Enclave needs a macOS build with cgo. func Decrypt(_ string, _ []byte) ([]byte, error) { return nil, errNotSupported } -// DeleteKey is not supported on non-darwin platforms. +// DeleteKey fails: the Secure Enclave needs a macOS build with cgo. func DeleteKey(_ string) error { return errNotSupported } diff --git a/internal/macse/macse_test.go b/internal/macse/macse_test.go index c56625c..954e296 100644 --- a/internal/macse/macse_test.go +++ b/internal/macse/macse_test.go @@ -1,5 +1,4 @@ -//go:build darwin -// +build darwin +//go:build darwin && cgo package macse @@ -45,7 +44,8 @@ func TestCreateAndDeleteKey(t *testing.T) { // Verify valid uncompressed P-256 public key if len(pubKey) != p256UncompressedKeySize { - t.Fatalf("expected public key length %d, got %d", p256UncompressedKeySize, len(pubKey)) + t.Fatalf("expected public key length %d, got %d", + p256UncompressedKeySize, len(pubKey)) } if pubKey[0] != 0x04 { @@ -83,7 +83,8 @@ func TestEncryptDecryptRoundTrip(t *testing.T) { }() // Test data simulating an age private key - plaintext := []byte("AGE-SECRET-KEY-1QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ") + plaintext := []byte("AGE-SECRET-KEY-1" + + "QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ") // Encrypt ciphertext, err := Encrypt(testKeyLabel, plaintext) diff --git a/internal/secret/derivation_index_test.go b/internal/secret/derivation_index_test.go index 590ed5f..648feb6 100644 --- a/internal/secret/derivation_index_test.go +++ b/internal/secret/derivation_index_test.go @@ -1,5 +1,6 @@ //go:build darwin +//nolint:testpackage // white-box test of unexported getLongTermPrivateKey package secret import ( @@ -27,22 +28,44 @@ type realVault struct { func (v *realVault) GetDirectory() (string, error) { return filepath.Join(v.stateDir, "vaults.d", v.name), nil } -func (v *realVault) GetName() string { return v.name } +func (v *realVault) GetName() string { return v.name } + +//nolint:ireturn // implements VaultInterface func (v *realVault) GetFilesystem() afero.Fs { return v.fs } // Unused by getLongTermPrivateKey — these satisfy VaultInterface. -func (v *realVault) AddSecret(string, *memguard.LockedBuffer, bool) error { panic("not used") } -func (v *realVault) GetCurrentUnlocker() (Unlocker, error) { panic("not used") } -func (v *realVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) { panic("not used") } -func (v *realVault) SetMnemonic(*memguard.LockedBuffer) { panic("not used") } -func (v *realVault) SetUnlockPassphrase(*memguard.LockedBuffer) { panic("not used") } -func (v *realVault) CreatePassphraseUnlocker(*memguard.LockedBuffer) (*PassphraseUnlocker, error) { +func (v *realVault) AddSecret(string, *memguard.LockedBuffer, bool) error { + panic("not used") +} + +//nolint:ireturn // implements VaultInterface +func (v *realVault) GetCurrentUnlocker() (Unlocker, error) { + panic("not used") +} + +func (v *realVault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) { + panic("not used") +} + +func (v *realVault) SetMnemonic(*memguard.LockedBuffer) { + panic("not used") +} + +func (v *realVault) SetUnlockPassphrase(*memguard.LockedBuffer) { + panic("not used") +} + +func (v *realVault) CreatePassphraseUnlocker( + *memguard.LockedBuffer, +) (*PassphraseUnlocker, error) { panic("not used") } // createRealVault sets up a complete vault directory structure on an in-memory // filesystem, identical to what vault.CreateVault produces. -func createRealVault(t *testing.T, fs afero.Fs, stateDir, name string, derivationIndex uint32) *realVault { +func createRealVault( + t *testing.T, fs afero.Fs, stateDir, name string, derivationIndex uint32, +) *realVault { t.Helper() vaultDir := filepath.Join(stateDir, "vaults.d", name) @@ -55,7 +78,8 @@ func createRealVault(t *testing.T, fs afero.Fs, stateDir, name string, derivatio } metaBytes, err := json.Marshal(metadata) require.NoError(t, err) - require.NoError(t, afero.WriteFile(fs, filepath.Join(vaultDir, "vault-metadata.json"), metaBytes, FilePerms)) + require.NoError(t, afero.WriteFile(fs, + filepath.Join(vaultDir, "vault-metadata.json"), metaBytes, FilePerms)) return &realVault{name: name, stateDir: stateDir, fs: fs} } @@ -63,7 +87,9 @@ func createRealVault(t *testing.T, fs afero.Fs, stateDir, name string, derivatio func TestGetLongTermPrivateKeyUsesVaultDerivationIndex(t *testing.T) { t.Parallel() - const testMnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" + //nolint:dupword // BIP39 test mnemonic repeats words by design + const testMnemonic = "abandon abandon abandon abandon abandon abandon " + + "abandon abandon abandon abandon abandon about" // Derive expected keys at two different indices to prove they differ. key0, err := agehd.DeriveIdentity(testMnemonic, 0) @@ -82,6 +108,7 @@ func TestGetLongTermPrivateKeyUsesVaultDerivationIndex(t *testing.T) { result, err := getLongTermPrivateKey(fs, vault, mnemonic, nil) require.NoError(t, err) + defer result.Destroy() assert.Equal(t, key5.String(), string(result.Bytes()), diff --git a/internal/secret/keychainunlocker.go b/internal/secret/keychainunlocker.go index b829734..97aebc9 100644 --- a/internal/secret/keychainunlocker.go +++ b/internal/secret/keychainunlocker.go @@ -1,11 +1,11 @@ //go:build darwin -// +build darwin package secret import ( "encoding/hex" "encoding/json" + "errors" "fmt" "log/slog" "os" @@ -17,23 +17,40 @@ import ( "filippo.io/age" "git.eeqj.de/sneak/secret/pkg/agehd" "github.com/awnumar/memguard" - keychain "github.com/keybase/go-keychain" "github.com/spf13/afero" ) const ( agePrivKeyPassphraseLength = 64 // KEYCHAIN_APP_IDENTIFIER is the service name used for keychain items - KEYCHAIN_APP_IDENTIFIER = "berlin.sneak.app.secret" //nolint:revive // ALL_CAPS is intentional for this constant + // + //nolint:revive // ALL_CAPS is intentional for this constant + KEYCHAIN_APP_IDENTIFIER = "berlin.sneak.app.secret" + + // keychainUnlockerType is the metadata type string for keychain unlockers. + keychainUnlockerType = "keychain" + + // macOSFlag is the unlocker metadata flag of the macOS-only unlockers. + macOSFlag = "macos" ) // keychainItemNameRegex validates keychain item names // Allows alphanumeric characters, dots, hyphens, and underscores only var keychainItemNameRegex = regexp.MustCompile(`^[A-Za-z0-9._-]+$`) +var ( + errNotMacOS = errors.New( + "keychain unlockers are only supported on macOS") + errKeychainItemNameEmpty = errors.New("keychain item name cannot be empty") + errInvalidKeychainItemName = errors.New("invalid keychain item name format") + errUnsupportedCurrentUnlocker = errors.New( + "unsupported current unlocker type for keychain unlocker creation") +) + // KeychainUnlockerMetadata extends UnlockerMetadata with keychain-specific data type KeychainUnlockerMetadata struct { UnlockerMetadata + // Keychain item name KeychainItemName string `json:"keychainItemName"` } @@ -45,6 +62,17 @@ type KeychainUnlocker struct { fs afero.Fs } +// NewKeychainUnlocker creates a new KeychainUnlocker instance +func NewKeychainUnlocker( + fs afero.Fs, directory string, metadata UnlockerMetadata, +) *KeychainUnlocker { + return &KeychainUnlocker{ + Directory: directory, + Metadata: metadata, + fs: fs, + } +} + // GetIdentity implements Unlocker interface for Keychain-based unlockers func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) { DebugWith("Getting keychain unlocker identity", @@ -52,50 +80,20 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) { slog.String("unlocker_type", k.GetType()), ) - // Step 1: Get keychain item name - keychainItemName, err := k.GetKeychainItemName() + keychainData, err := k.readKeychainData() if err != nil { - Debug("Failed to get keychain item name", "error", err, "unlocker_id", k.GetID()) - - return nil, fmt.Errorf("failed to get keychain item name: %w", err) - } - - // Step 2: Retrieve data from keychain - Debug("Retrieving data from macOS keychain", "keychain_item", keychainItemName) - keychainDataBytes, err := retrieveFromKeychain(keychainItemName) - if err != nil { - Debug("Failed to retrieve data from keychain", "error", err, "keychain_item", keychainItemName) - - return nil, fmt.Errorf("failed to retrieve data from keychain: %w", err) - } - - DebugWith("Retrieved data from keychain", - slog.String("unlocker_id", k.GetID()), - slog.Int("data_length", len(keychainDataBytes)), - ) - - // Move the keychain data into locked memory; this wipes keychainDataBytes - keychainDataBuffer := memguard.NewBufferFromBytes(keychainDataBytes) - defer keychainDataBuffer.Destroy() - - // Step 3: Parse keychain data - keychainData, err := decodeKeychainData(keychainDataBuffer) - if err != nil { - Debug("Failed to parse keychain data", "error", err, "unlocker_id", k.GetID()) - - return nil, fmt.Errorf("failed to parse keychain data: %w", err) + return nil, err } defer keychainData.AgePrivKeyPassphrase.Destroy() - Debug("Parsed keychain data successfully", "unlocker_id", k.GetID()) - // Step 4: Read the encrypted age private key from filesystem agePrivKeyPath := filepath.Join(k.Directory, "priv.age") Debug("Reading encrypted age private key", "path", agePrivKeyPath) encryptedAgePrivKeyData, err := afero.ReadFile(k.fs, agePrivKeyPath) if err != nil { - Debug("Failed to read encrypted age private key", "error", err, "path", agePrivKeyPath) + Debug("Failed to read encrypted age private key", + "error", err, "path", agePrivKeyPath) return nil, fmt.Errorf("failed to read encrypted age private key: %w", err) } @@ -106,12 +104,17 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) { ) // Step 5: Decrypt the age private key using the passphrase from keychain - Debug("Decrypting age private key with keychain passphrase", "unlocker_id", k.GetID()) - agePrivKeyBuffer, err := DecryptWithPassphrase(encryptedAgePrivKeyData, keychainData.AgePrivKeyPassphrase) - if err != nil { - Debug("Failed to decrypt age private key with keychain passphrase", "error", err, "unlocker_id", k.GetID()) + Debug("Decrypting age private key with keychain passphrase", + "unlocker_id", k.GetID()) - return nil, fmt.Errorf("failed to decrypt age private key with keychain passphrase: %w", err) + agePrivKeyBuffer, err := DecryptWithPassphrase( + encryptedAgePrivKeyData, keychainData.AgePrivKeyPassphrase) + if err != nil { + Debug("Failed to decrypt age private key with keychain passphrase", + "error", err, "unlocker_id", k.GetID()) + + return nil, fmt.Errorf( + "failed to decrypt age private key with keychain passphrase: %w", err) } defer agePrivKeyBuffer.Destroy() @@ -140,7 +143,7 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) { // GetType implements Unlocker interface func (k *KeychainUnlocker) GetType() string { - return "keychain" + return keychainUnlockerType } // GetMetadata implements Unlocker interface @@ -174,58 +177,105 @@ func (k *KeychainUnlocker) Remove() error { // Step 1: Get keychain item name keychainItemName, err := k.GetKeychainItemName() if err != nil { - Debug("Failed to get keychain item name during removal", "error", err, "unlocker_id", k.GetID()) + Debug("Failed to get keychain item name during removal", + "error", err, "unlocker_id", k.GetID()) return fmt.Errorf("failed to get keychain item name: %w", err) } // Step 2: Remove from keychain Debug("Removing keychain item", "keychain_item", keychainItemName) - if err := deleteFromKeychain(keychainItemName); err != nil { - Debug("Failed to remove keychain item", "error", err, "keychain_item", keychainItemName) + + err = deleteFromKeychain(keychainItemName) + if err != nil { + Debug("Failed to remove keychain item", + "error", err, "keychain_item", keychainItemName) return fmt.Errorf("failed to remove keychain item: %w", err) } // Step 3: Remove directory Debug("Removing keychain unlocker directory", "directory", k.Directory) - if err := RemoveDirAtomic(k.fs, k.Directory); err != nil { - Debug("Failed to remove keychain unlocker directory", "error", err, "directory", k.Directory) + + err = RemoveDirAtomic(k.fs, k.Directory) + if err != nil { + Debug("Failed to remove keychain unlocker directory", + "error", err, "directory", k.Directory) return fmt.Errorf("failed to remove keychain unlocker directory: %w", err) } - Debug("Successfully removed keychain unlocker", "unlocker_id", k.GetID(), "keychain_item", keychainItemName) + Debug("Successfully removed keychain unlocker", + "unlocker_id", k.GetID(), "keychain_item", keychainItemName) return nil } -// NewKeychainUnlocker creates a new KeychainUnlocker instance -func NewKeychainUnlocker(fs afero.Fs, directory string, metadata UnlockerMetadata) *KeychainUnlocker { - return &KeychainUnlocker{ - Directory: directory, - Metadata: metadata, - fs: fs, - } -} - // GetKeychainItemName returns the keychain item name from metadata func (k *KeychainUnlocker) GetKeychainItemName() (string, error) { // Load the metadata metadataPath := filepath.Join(k.Directory, "unlocker-metadata.json") + metadataData, err := afero.ReadFile(k.fs, metadataPath) if err != nil { return "", fmt.Errorf("failed to read keychain metadata: %w", err) } var keychainMetadata KeychainUnlockerMetadata - if err := json.Unmarshal(metadataData, &keychainMetadata); err != nil { + + err = json.Unmarshal(metadataData, &keychainMetadata) + if err != nil { return "", fmt.Errorf("failed to parse keychain metadata: %w", err) } return keychainMetadata.KeychainItemName, nil } +// readKeychainData reads and parses the data this unlocker keeps in the +// keychain (steps 1 to 3 of GetIdentity). The caller must destroy the +// returned AgePrivKeyPassphrase. +func (k *KeychainUnlocker) readKeychainData() (*KeychainData, error) { + // Step 1: Get keychain item name + keychainItemName, err := k.GetKeychainItemName() + if err != nil { + Debug("Failed to get keychain item name", "error", err, "unlocker_id", k.GetID()) + + return nil, fmt.Errorf("failed to get keychain item name: %w", err) + } + + // Step 2: Retrieve data from keychain + Debug("Retrieving data from macOS keychain", "keychain_item", keychainItemName) + + keychainDataBytes, err := retrieveFromKeychain(keychainItemName) + if err != nil { + Debug("Failed to retrieve data from keychain", + "error", err, "keychain_item", keychainItemName) + + return nil, fmt.Errorf("failed to retrieve data from keychain: %w", err) + } + + DebugWith("Retrieved data from keychain", + slog.String("unlocker_id", k.GetID()), + slog.Int("data_length", len(keychainDataBytes)), + ) + + // Move the keychain data into locked memory; this wipes keychainDataBytes + keychainDataBuffer := memguard.NewBufferFromBytes(keychainDataBytes) + defer keychainDataBuffer.Destroy() + + // Step 3: Parse keychain data + keychainData, err := decodeKeychainData(keychainDataBuffer) + if err != nil { + Debug("Failed to parse keychain data", "error", err, "unlocker_id", k.GetID()) + + return nil, fmt.Errorf("failed to parse keychain data: %w", err) + } + + Debug("Parsed keychain data successfully", "unlocker_id", k.GetID()) + + return keychainData, nil +} + // generateKeychainUnlockerName generates a unique name for the keychain unlocker func generateKeychainUnlockerName(vaultName string) (string, error) { hostname, err := os.Hostname() @@ -247,31 +297,7 @@ func getLongTermPrivateKey( fs afero.Fs, vault VaultInterface, mnemonic, passphrase *memguard.LockedBuffer, ) (*memguard.LockedBuffer, error) { if mnemonic != nil { - // Read vault metadata to get the correct derivation index - vaultDir, err := vault.GetDirectory() - if err != nil { - return nil, fmt.Errorf("failed to get vault directory: %w", err) - } - - metadataPath := filepath.Join(vaultDir, "vault-metadata.json") - metadataBytes, err := afero.ReadFile(fs, metadataPath) - if err != nil { - return nil, fmt.Errorf("failed to read vault metadata: %w", err) - } - - var metadata VaultMetadata - if err := json.Unmarshal(metadataBytes, &metadata); err != nil { - return nil, fmt.Errorf("failed to parse vault metadata: %w", err) - } - - // Use mnemonic with the vault's actual derivation index - ltIdentity, err := agehd.DeriveIdentity(mnemonic.String(), metadata.DerivationIndex) - if err != nil { - return nil, fmt.Errorf("failed to derive long-term key from mnemonic: %w", err) - } - - // Return the private key in a secure buffer - return memguard.NewBufferFromBytes([]byte(ltIdentity.String())), nil + return deriveLongTermPrivateKey(fs, vault, mnemonic) } // Get the vault to access current unlocker @@ -292,34 +318,43 @@ func getLongTermPrivateKey( // Get encrypted long-term key from current unlocker, handling different types var encryptedLtPrivKey []byte + switch currentUnlocker := currentUnlocker.(type) { case *PassphraseUnlocker: // Read the encrypted long-term private key from passphrase unlocker - encryptedLtPrivKey, err = afero.ReadFile(fs, filepath.Join(currentUnlocker.GetDirectory(), "longterm.age")) + encryptedLtPrivKey, err = afero.ReadFile(fs, + filepath.Join(currentUnlocker.GetDirectory(), "longterm.age")) if err != nil { - return nil, fmt.Errorf("failed to read encrypted long-term key from current passphrase unlocker: %w", err) + return nil, fmt.Errorf("failed to read encrypted long-term key "+ + "from current passphrase unlocker: %w", err) } case *PGPUnlocker: // Read the encrypted long-term private key from PGP unlocker - encryptedLtPrivKey, err = afero.ReadFile(fs, filepath.Join(currentUnlocker.GetDirectory(), "longterm.age")) + encryptedLtPrivKey, err = afero.ReadFile(fs, + filepath.Join(currentUnlocker.GetDirectory(), "longterm.age")) if err != nil { - return nil, fmt.Errorf("failed to read encrypted long-term key from current PGP unlocker: %w", err) + return nil, fmt.Errorf("failed to read encrypted long-term key "+ + "from current PGP unlocker: %w", err) } case *KeychainUnlocker: - // Read the encrypted long-term private key from another keychain unlocker - encryptedLtPrivKey, err = afero.ReadFile(fs, filepath.Join(currentUnlocker.GetDirectory(), "longterm.age")) + // Read the encrypted long-term private key from another keychain + // unlocker + encryptedLtPrivKey, err = afero.ReadFile(fs, + filepath.Join(currentUnlocker.GetDirectory(), "longterm.age")) if err != nil { - return nil, fmt.Errorf("failed to read encrypted long-term key from current keychain unlocker: %w", err) + return nil, fmt.Errorf("failed to read encrypted long-term key "+ + "from current keychain unlocker: %w", err) } default: - return nil, fmt.Errorf("unsupported current unlocker type for keychain unlocker creation") + return nil, errUnsupportedCurrentUnlocker } // Decrypt long-term private key using current unlocker - ltPrivKeyBuffer, err := DecryptWithIdentity(encryptedLtPrivKey, currentUnlockerIdentity) + ltPrivKeyBuffer, err := DecryptWithIdentity( + encryptedLtPrivKey, currentUnlockerIdentity) if err != nil { return nil, fmt.Errorf("failed to decrypt long-term private key: %w", err) } @@ -328,6 +363,43 @@ func getLongTermPrivateKey( return ltPrivKeyBuffer, nil } +// deriveLongTermPrivateKey derives the long-term private key from mnemonic at +// the vault's derivation index, for getLongTermPrivateKey and +// getLongTermKeyForSE. +func deriveLongTermPrivateKey( + fs afero.Fs, vault VaultInterface, mnemonic *memguard.LockedBuffer, +) (*memguard.LockedBuffer, error) { + // Read vault metadata to get the correct derivation index + vaultDir, err := vault.GetDirectory() + if err != nil { + return nil, fmt.Errorf("failed to get vault directory: %w", err) + } + + metadataPath := filepath.Join(vaultDir, "vault-metadata.json") + + metadataBytes, err := afero.ReadFile(fs, metadataPath) + if err != nil { + return nil, fmt.Errorf("failed to read vault metadata: %w", err) + } + + var metadata VaultMetadata + + err = json.Unmarshal(metadataBytes, &metadata) + if err != nil { + return nil, fmt.Errorf("failed to parse vault metadata: %w", err) + } + + // Use mnemonic with the vault's actual derivation index + ltIdentity, err := agehd.DeriveIdentity(mnemonic.String(), metadata.DerivationIndex) + if err != nil { + return nil, fmt.Errorf( + "failed to derive long-term key from mnemonic: %w", err) + } + + // Return the private key in a secure buffer + return memguard.NewBufferFromBytes([]byte(ltIdentity.String())), nil +} + // CreateKeychainUnlocker creates a new keychain unlocker and stores it in the // vault. The long-term key comes from mnemonic when it is not nil, else from // the current unlocker, as getLongTermPrivateKey describes. @@ -335,7 +407,8 @@ func CreateKeychainUnlocker( fs afero.Fs, stateDir string, mnemonic, passphrase *memguard.LockedBuffer, ) (*KeychainUnlocker, error) { // Check if we're on macOS - if err := checkMacOSAvailable(); err != nil { + err := checkMacOSAvailable() + if err != nil { return nil, err } @@ -377,10 +450,12 @@ func CreateKeychainUnlocker( // 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)) defer agePrivKeyBuffer.Destroy() - encryptedAgePrivKey, err := EncryptWithPassphrase(agePrivKeyBuffer, agePrivKeyPassphrase) + encryptedAgePrivKey, err := EncryptWithPassphrase( + agePrivKeyBuffer, agePrivKeyPassphrase) if err != nil { return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err) } @@ -393,9 +468,11 @@ func CreateKeychainUnlocker( defer ltPrivKeyData.Destroy() // Step 5: Encrypt long-term private key to the new age unlocker - encryptedLtPrivKeyToAge, err := EncryptToRecipient(ltPrivKeyData, ageIdentity.Recipient()) + 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) + return nil, fmt.Errorf( + "failed to encrypt long-term private key to age unlocker: %w", err) } // Step 6: Prepare keychain data @@ -411,12 +488,23 @@ func CreateKeychainUnlocker( } defer keychainDataBuffer.Destroy() + return writeKeychainUnlocker(fs, unlockerDir, keychainItemName, ageRecipient, + encryptedAgePrivKey, encryptedLtPrivKeyToAge, keychainDataBuffer) +} + +// writeKeychainUnlocker writes a new keychain unlocker into unlockerDir and +// stores its data in the keychain (steps 7 and 8 of CreateKeychainUnlocker). +func writeKeychainUnlocker( + fs afero.Fs, unlockerDir, keychainItemName, ageRecipient string, + encryptedAgePrivKey, encryptedLtPrivKey []byte, + keychainDataBuffer *memguard.LockedBuffer, +) (*KeychainUnlocker, error) { // Step 7: Prepare enhanced metadata keychainMetadata := KeychainUnlockerMetadata{ UnlockerMetadata: UnlockerMetadata{ - Type: "keychain", + Type: keychainUnlockerType, CreatedAt: time.Now(), - Flags: []string{"keychain", "macos"}, + Flags: []string{keychainUnlockerType, macOSFlag}, }, KeychainItemName: keychainItemName, } @@ -429,27 +517,29 @@ func CreateKeychainUnlocker( // 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 { + err := WriteFileAtomic(fs, filepath.Join(dir, "pub.txt"), []byte(ageRecipient)) + if 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 { + err = WriteFileAtomic(fs, filepath.Join(dir, "priv.age"), encryptedAgePrivKey) + if 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 { + 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) } - if err := storeInKeychain(keychainItemName, keychainDataBuffer); err != nil { + err = storeInKeychain(keychainItemName, keychainDataBuffer) + if 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 { + err = WriteFileAtomic(fs, filepath.Join(dir, "unlocker-metadata.json"), + metadataBytes) + if err != nil { return fmt.Errorf("failed to write unlocker metadata: %w", err) } @@ -469,111 +559,21 @@ func CreateKeychainUnlocker( // checkMacOSAvailable verifies that we're running on macOS func checkMacOSAvailable() error { if runtime.GOOS != "darwin" { - return fmt.Errorf("keychain unlockers are only supported on macOS, current OS: %s", runtime.GOOS) + return fmt.Errorf("%w, current OS: %s", errNotMacOS, runtime.GOOS) } return nil } -// validateKeychainItemName validates that a keychain item name is safe for command execution +// validateKeychainItemName validates that a keychain item name is safe for +// command execution func validateKeychainItemName(itemName string) error { if itemName == "" { - return fmt.Errorf("keychain item name cannot be empty") + return errKeychainItemNameEmpty } if !keychainItemNameRegex.MatchString(itemName) { - return fmt.Errorf("invalid keychain item name format: %s", itemName) - } - - return nil -} - -// storeInKeychain stores data in the macOS keychain using keybase/go-keychain -func storeInKeychain(itemName string, data *memguard.LockedBuffer) error { - if data == nil { - return fmt.Errorf("data buffer is nil") - } - if err := validateKeychainItemName(itemName); err != nil { - return fmt.Errorf("invalid keychain item name: %w", err) - } - - item := keychain.NewItem() - item.SetSecClass(keychain.SecClassGenericPassword) - item.SetService(KEYCHAIN_APP_IDENTIFIER) - item.SetAccount(itemName) - item.SetLabel(fmt.Sprintf("%s - %s", KEYCHAIN_APP_IDENTIFIER, itemName)) - item.SetDescription("Secret vault keychain data") - item.SetData(data.Bytes()) - item.SetSynchronizable(keychain.SynchronizableNo) - // Use AccessibleWhenUnlockedThisDeviceOnly for better security and to trigger auth - item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly) - - // First try to delete any existing item - deleteItem := keychain.NewItem() - deleteItem.SetSecClass(keychain.SecClassGenericPassword) - deleteItem.SetService(KEYCHAIN_APP_IDENTIFIER) - deleteItem.SetAccount(itemName) - _ = keychain.DeleteItem(deleteItem) // Ignore error as item might not exist - - // Add the new item - if err := keychain.AddItem(item); err != nil { - return fmt.Errorf("failed to store item in keychain: %w", err) - } - - return nil -} - -// retrieveFromKeychain retrieves data from the macOS keychain using keybase/go-keychain -func retrieveFromKeychain(itemName string) ([]byte, error) { - if err := validateKeychainItemName(itemName); err != nil { - return nil, fmt.Errorf("invalid keychain item name: %w", err) - } - - query := keychain.NewItem() - query.SetSecClass(keychain.SecClassGenericPassword) - query.SetService(KEYCHAIN_APP_IDENTIFIER) - query.SetAccount(itemName) - query.SetMatchLimit(keychain.MatchLimitOne) - query.SetReturnData(true) - - results, err := keychain.QueryItem(query) - if err != nil { - return nil, fmt.Errorf("failed to retrieve item from keychain: %w", err) - } - - if len(results) == 0 { - return nil, fmt.Errorf("keychain item not found: %s", itemName) - } - - return results[0].Data, nil -} - -// deleteFromKeychain removes an item from the macOS keychain using keybase/go-keychain -// If the item doesn't exist, this function returns nil (not an error) since the goal -// is to ensure the item is gone, and it already being gone satisfies that goal. -func deleteFromKeychain(itemName string) error { - if err := validateKeychainItemName(itemName); err != nil { - return fmt.Errorf("invalid keychain item name: %w", err) - } - - item := keychain.NewItem() - item.SetSecClass(keychain.SecClassGenericPassword) - item.SetService(KEYCHAIN_APP_IDENTIFIER) - item.SetAccount(itemName) - - if err := keychain.DeleteItem(item); err != nil { - // If the item doesn't exist, that's not an error - the goal is to ensure - // the item is gone, and it already being gone satisfies that goal. - // This is important for cleaning up unlocker directories when the keychain - // item has already been removed (e.g., manually by user, or synced vault - // from a different machine). - if err == keychain.ErrorItemNotFound { - Debug("Keychain item not found during deletion, ignoring", "item_name", itemName) - - return nil - } - - return fmt.Errorf("failed to delete item from keychain: %w", err) + return fmt.Errorf("%w: %s", errInvalidKeychainItemName, itemName) } return nil diff --git a/internal/secret/keychainunlocker_cgo.go b/internal/secret/keychainunlocker_cgo.go new file mode 100644 index 0000000..86bdfde --- /dev/null +++ b/internal/secret/keychainunlocker_cgo.go @@ -0,0 +1,104 @@ +//go:build darwin && cgo + +package secret + +import ( + "fmt" + + "github.com/awnumar/memguard" + keychain "github.com/keybase/go-keychain" +) + +// The keychain unlocker's only calls into go-keychain, which is cgo on macOS. +// A macOS build without cgo gets keychainunlocker_nocgo.go instead. + +// storeInKeychain stores data in the macOS keychain using keybase/go-keychain +func storeInKeychain(itemName string, data *memguard.LockedBuffer) error { + if data == nil { + return fmt.Errorf("data buffer is nil") + } + if err := validateKeychainItemName(itemName); err != nil { + return fmt.Errorf("invalid keychain item name: %w", err) + } + + item := keychain.NewItem() + item.SetSecClass(keychain.SecClassGenericPassword) + item.SetService(KEYCHAIN_APP_IDENTIFIER) + item.SetAccount(itemName) + item.SetLabel(fmt.Sprintf("%s - %s", KEYCHAIN_APP_IDENTIFIER, itemName)) + item.SetDescription("Secret vault keychain data") + item.SetData(data.Bytes()) + item.SetSynchronizable(keychain.SynchronizableNo) + // Use AccessibleWhenUnlockedThisDeviceOnly for better security and to trigger auth + item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly) + + // First try to delete any existing item + deleteItem := keychain.NewItem() + deleteItem.SetSecClass(keychain.SecClassGenericPassword) + deleteItem.SetService(KEYCHAIN_APP_IDENTIFIER) + deleteItem.SetAccount(itemName) + _ = keychain.DeleteItem(deleteItem) // Ignore error as item might not exist + + // Add the new item + if err := keychain.AddItem(item); err != nil { + return fmt.Errorf("failed to store item in keychain: %w", err) + } + + return nil +} + +// retrieveFromKeychain retrieves data from the macOS keychain using keybase/go-keychain +func retrieveFromKeychain(itemName string) ([]byte, error) { + if err := validateKeychainItemName(itemName); err != nil { + return nil, fmt.Errorf("invalid keychain item name: %w", err) + } + + query := keychain.NewItem() + query.SetSecClass(keychain.SecClassGenericPassword) + query.SetService(KEYCHAIN_APP_IDENTIFIER) + query.SetAccount(itemName) + query.SetMatchLimit(keychain.MatchLimitOne) + query.SetReturnData(true) + + results, err := keychain.QueryItem(query) + if err != nil { + return nil, fmt.Errorf("failed to retrieve item from keychain: %w", err) + } + + if len(results) == 0 { + return nil, fmt.Errorf("keychain item not found: %s", itemName) + } + + return results[0].Data, nil +} + +// deleteFromKeychain removes an item from the macOS keychain using keybase/go-keychain +// If the item doesn't exist, this function returns nil (not an error) since the goal +// is to ensure the item is gone, and it already being gone satisfies that goal. +func deleteFromKeychain(itemName string) error { + if err := validateKeychainItemName(itemName); err != nil { + return fmt.Errorf("invalid keychain item name: %w", err) + } + + item := keychain.NewItem() + item.SetSecClass(keychain.SecClassGenericPassword) + item.SetService(KEYCHAIN_APP_IDENTIFIER) + item.SetAccount(itemName) + + if err := keychain.DeleteItem(item); err != nil { + // If the item doesn't exist, that's not an error - the goal is to ensure + // the item is gone, and it already being gone satisfies that goal. + // This is important for cleaning up unlocker directories when the keychain + // item has already been removed (e.g., manually by user, or synced vault + // from a different machine). + if err == keychain.ErrorItemNotFound { + Debug("Keychain item not found during deletion, ignoring", "item_name", itemName) + + return nil + } + + return fmt.Errorf("failed to delete item from keychain: %w", err) + } + + return nil +} diff --git a/internal/secret/keychainunlocker_nocgo.go b/internal/secret/keychainunlocker_nocgo.go new file mode 100644 index 0000000..6a486f9 --- /dev/null +++ b/internal/secret/keychainunlocker_nocgo.go @@ -0,0 +1,30 @@ +//go:build darwin && !cgo + +package secret + +import ( + "errors" + + "github.com/awnumar/memguard" +) + +// In a macOS build without cgo, these take the place of the functions in +// keychainunlocker_cgo.go: go-keychain is cgo on macOS, so they can only fail. + +var errKeychainNotSupported = errors.New( + "keychain unlockers need a macOS build with cgo") + +// storeInKeychain fails: the keychain needs a macOS build with cgo. +func storeInKeychain(_ string, _ *memguard.LockedBuffer) error { + return errKeychainNotSupported +} + +// retrieveFromKeychain fails: the keychain needs a macOS build with cgo. +func retrieveFromKeychain(_ string) ([]byte, error) { + return nil, errKeychainNotSupported +} + +// deleteFromKeychain fails: the keychain needs a macOS build with cgo. +func deleteFromKeychain(_ string) error { + return errKeychainNotSupported +} diff --git a/internal/secret/keychainunlocker_test.go b/internal/secret/keychainunlocker_test.go index 981ee6e..3df251b 100644 --- a/internal/secret/keychainunlocker_test.go +++ b/internal/secret/keychainunlocker_test.go @@ -1,5 +1,4 @@ -//go:build darwin -// +build darwin +//go:build darwin && cgo package secret @@ -35,7 +34,8 @@ func TestKeychainStoreRetrieveDelete(t *testing.T) { // Test 2: Retrieve data from keychain retrievedData, err := retrieveFromKeychain(testItemName) require.NoError(t, err, "Failed to retrieve data from keychain") - assert.Equal(t, testData, string(retrievedData), "Retrieved data doesn't match stored data") + assert.Equal(t, testData, string(retrievedData), + "Retrieved data doesn't match stored data") // Test 3: Update existing item (store again with different data) newTestData := "updated-test-data-67890" @@ -48,7 +48,8 @@ func TestKeychainStoreRetrieveDelete(t *testing.T) { // Verify updated data retrievedData, err = retrieveFromKeychain(testItemName) require.NoError(t, err, "Failed to retrieve updated data from keychain") - assert.Equal(t, newTestData, string(retrievedData), "Retrieved data doesn't match updated data") + assert.Equal(t, newTestData, string(retrievedData), + "Retrieved data doesn't match updated data") // Test 4: Delete from keychain err = deleteFromKeychain(testItemName) @@ -93,7 +94,8 @@ func TestKeychainInvalidItemName(t *testing.T) { for _, name := range invalidNames { err := storeInKeychain(name, testData) assert.Error(t, err, "Expected error for invalid name: %s", name) - assert.Contains(t, err.Error(), "invalid keychain item name", "Error should mention invalid name for: %s", name) + assert.Contains(t, err.Error(), "invalid keychain item name", + "Error should mention invalid name for: %s", name) } // Test valid names (should not error on validation) @@ -180,5 +182,6 @@ func TestDeleteNonExistentKeychainItem(t *testing.T) { // This is important for cleaning up unlocker directories when the keychain item // has already been removed (e.g., manually by user, or on a different machine) err := deleteFromKeychain(testItemName) - assert.NoError(t, err, "Deleting non-existent keychain item should not return an error") + assert.NoError(t, err, + "Deleting non-existent keychain item should not return an error") } diff --git a/internal/secret/pgpunlock_test.go b/internal/secret/pgpunlock_test.go index d3e48ac..164948a 100644 --- a/internal/secret/pgpunlock_test.go +++ b/internal/secret/pgpunlock_test.go @@ -4,7 +4,9 @@ package secret_test import ( "bytes" + "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -22,23 +24,24 @@ import ( "github.com/spf13/afero" ) -// Register vault with secret package for testing -func init() { - // Register the vault.GetCurrentVault function with the secret package - secret.RegisterGetCurrentVaultFunc(func(fs afero.Fs, stateDir string) (secret.VaultInterface, error) { - return vault.GetCurrentVault(fs, stateDir) - }) -} +// pgpUnlockerType is the type of a PGP unlocker. +const pgpUnlockerType = "pgp" + +var errNilDataBuffer = errors.New("data buffer is nil") // setupNonInteractiveGPG creates a custom GPG environment for testing func setupNonInteractiveGPG(t *testing.T, _, passphrase, gnupgHomeDir string) { + t.Helper() + // Create GPG config file for non-interactive operation gpgConfPath := filepath.Join(gnupgHomeDir, "gpg.conf") gpgConfContent := `batch no-tty pinentry-mode loopback ` - if err := os.WriteFile(gpgConfPath, []byte(gpgConfContent), 0o600); err != nil { + + err := os.WriteFile(gpgConfPath, []byte(gpgConfContent), 0o600) + if err != nil { t.Fatalf("Failed to write GPG config file: %v", err) } @@ -47,11 +50,15 @@ pinentry-mode loopback origDecryptFunc := secret.GPGDecryptFunc // Set custom GPG functions for this test - secret.GPGEncryptFunc = func(data *memguard.LockedBuffer, keyID string) ([]byte, error) { + secret.GPGEncryptFunc = func( + data *memguard.LockedBuffer, keyID string, + ) ([]byte, error) { if data == nil { - return nil, fmt.Errorf("data buffer is nil") + return nil, errNilDataBuffer } - cmd := exec.Command("gpg", + + //nolint:gosec // G204: test runs gpg with test-controlled arguments + cmd := exec.CommandContext(t.Context(), "gpg", "--homedir", gnupgHomeDir, "--batch", "--yes", @@ -63,11 +70,13 @@ pinentry-mode loopback "-r", keyID) var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout cmd.Stderr = &stderr cmd.Stdin = bytes.NewReader(data.Bytes()) - if err := cmd.Run(); err != nil { + err := cmd.Run() + if err != nil { return nil, fmt.Errorf("GPG encryption failed: %w\nStderr: %s", err, stderr.String()) } @@ -75,7 +84,8 @@ pinentry-mode loopback } secret.GPGDecryptFunc = func(encryptedData []byte) (*memguard.LockedBuffer, error) { - cmd := exec.Command("gpg", + //nolint:gosec // G204: test runs gpg with test-controlled arguments + cmd := exec.CommandContext(t.Context(), "gpg", "--homedir", gnupgHomeDir, "--batch", "--yes", @@ -85,11 +95,13 @@ pinentry-mode loopback "--decrypt") var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout cmd.Stderr = &stderr cmd.Stdin = bytes.NewReader(encryptedData) - if err := cmd.Run(); err != nil { + err := cmd.Run() + if err != nil { return nil, fmt.Errorf("GPG decryption failed: %w\nStderr: %s", err, stderr.String()) } @@ -105,20 +117,24 @@ pinentry-mode loopback } // runGPGWithPassphrase executes a GPG command with the specified passphrase -func runGPGWithPassphrase(gnupgHome, passphrase string, args []string, input io.Reader) ([]byte, error) { - cmdArgs := []string{ +func runGPGWithPassphrase( + ctx context.Context, + gnupgHome, passphrase string, args []string, input io.Reader, +) ([]byte, error) { + cmdArgs := append([]string{ "--homedir=" + gnupgHome, "--batch", "--yes", "--pinentry-mode", "loopback", "--passphrase", passphrase, - } - cmdArgs = append(cmdArgs, args...) + }, args...) - cmd := exec.Command("gpg", cmdArgs...) + //nolint:gosec // G204: test runs gpg with test-controlled arguments + cmd := exec.CommandContext(ctx, "gpg", cmdArgs...) cmd.Stdin = input var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout cmd.Stderr = &stderr @@ -130,14 +146,96 @@ func runGPGWithPassphrase(gnupgHome, passphrase string, args []string, input io. return stdout.Bytes(), nil } +// generateTestGPGKey generates a GPG key protected by passphrase in +// gnupgHomeDir and returns its key ID and fingerprint. +func generateTestGPGKey( + t *testing.T, tempDir, gnupgHomeDir, passphrase string, +) (string, string) { + t.Helper() + + // Create GPG batch file for key generation + batchFile := filepath.Join(tempDir, "gen-key-batch") + batchContent := `%echo Generating a test key +Key-Type: RSA +Key-Length: 2048 +Name-Real: Test User +Name-Email: test@example.com +Expire-Date: 0 +Passphrase: ` + passphrase + ` +%commit +%echo Key generation completed +` + + err := os.WriteFile(batchFile, []byte(batchContent), 0o600) + if err != nil { + t.Fatalf("Failed to write batch file: %v", err) + } + + // Generate GPG key with batch mode + t.Log("Generating GPG key...") + + _, err = runGPGWithPassphrase(t.Context(), gnupgHomeDir, passphrase, + []string{"--gen-key", batchFile}, nil) + if err != nil { + t.Fatalf("Failed to generate GPG key: %v", err) + } + + t.Log("GPG key generated successfully") + + // Get the key ID and fingerprint + output, err := runGPGWithPassphrase(t.Context(), gnupgHomeDir, passphrase, + []string{"--list-secret-keys", "--with-colons", "--fingerprint"}, nil) + if err != nil { + t.Fatalf("Failed to list GPG keys: %v", err) + } + + // Parse output to get key ID and fingerprint + var keyID, fingerprint string + + for line := range strings.SplitSeq(string(output), "\n") { + if strings.HasPrefix(line, "sec:") { + fields := strings.Split(line, ":") + if len(fields) >= 5 { + keyID = fields[4] + } + } else if strings.HasPrefix(line, "fpr:") { + fields := strings.Split(line, ":") + if len(fields) >= 10 && fields[9] != "" { + fingerprint = fields[9] + + break + } + } + } + + if keyID == "" { + t.Fatalf("Failed to find GPG key ID in output: %s", output) + } + + if fingerprint == "" { + t.Fatalf("Failed to find GPG fingerprint in output: %s", output) + } + + t.Logf("Generated GPG key ID: %s", keyID) + t.Logf("Generated GPG fingerprint: %s", fingerprint) + + return keyID, fingerprint +} + +//nolint:paralleltest // t.Setenv forbids parallel subtests func TestPGPUnlockerWithRealFS(t *testing.T) { // Check if gpg is available - if _, err := exec.LookPath("gpg"); err != nil { + _, err := exec.LookPath("gpg") + if err != nil { t.Log("GPG not available, PGP unlock key tests may not fully function") // Continue anyway to test what we can } - // Create a temporary directory for our tests + // Create a temporary directory for our tests. Not t.TempDir: its longer + // path would put gpg-agent's socket in GNUPGHOME past the 104-byte limit + // macOS sets on socket paths. + // + //nolint:usetesting // see the comment above tempDir, err := os.MkdirTemp("", "secret-pgp-test-") if err != nil { t.Fatalf("Failed to create temp dir: %v", err) @@ -146,7 +244,9 @@ func TestPGPUnlockerWithRealFS(t *testing.T) { // Create a temporary GNUPGHOME gnupgHomeDir := filepath.Join(tempDir, "gnupg") - if err := os.MkdirAll(gnupgHomeDir, 0o700); err != nil { + + err = os.MkdirAll(gnupgHomeDir, 0o700) + if err != nil { t.Fatalf("Failed to create GNUPGHOME: %v", err) } @@ -159,64 +259,7 @@ func TestPGPUnlockerWithRealFS(t *testing.T) { // Setup non-interactive GPG with custom functions setupNonInteractiveGPG(t, tempDir, testPassphrase, gnupgHomeDir) - // Create GPG batch file for key generation - batchFile := filepath.Join(tempDir, "gen-key-batch") - batchContent := `%echo Generating a test key -Key-Type: RSA -Key-Length: 2048 -Name-Real: Test User -Name-Email: test@example.com -Expire-Date: 0 -Passphrase: ` + testPassphrase + ` -%commit -%echo Key generation completed -` - if err := os.WriteFile(batchFile, []byte(batchContent), 0o600); err != nil { - t.Fatalf("Failed to write batch file: %v", err) - } - - // Generate GPG key with batch mode - t.Log("Generating GPG key...") - _, err = runGPGWithPassphrase(gnupgHomeDir, testPassphrase, - []string{"--gen-key", batchFile}, nil) - if err != nil { - t.Fatalf("Failed to generate GPG key: %v", err) - } - t.Log("GPG key generated successfully") - - // Get the key ID and fingerprint - output, err := runGPGWithPassphrase(gnupgHomeDir, testPassphrase, - []string{"--list-secret-keys", "--with-colons", "--fingerprint"}, nil) - if err != nil { - t.Fatalf("Failed to list GPG keys: %v", err) - } - - // Parse output to get key ID and fingerprint - var keyID, fingerprint string - lines := strings.Split(string(output), "\n") - for _, line := range lines { - if strings.HasPrefix(line, "sec:") { - fields := strings.Split(line, ":") - if len(fields) >= 5 { - keyID = fields[4] - } - } else if strings.HasPrefix(line, "fpr:") { - fields := strings.Split(line, ":") - if len(fields) >= 10 && fields[9] != "" { - fingerprint = fields[9] - break - } - } - } - - if keyID == "" { - t.Fatalf("Failed to find GPG key ID in output: %s", output) - } - if fingerprint == "" { - t.Fatalf("Failed to find GPG fingerprint in output: %s", output) - } - t.Logf("Generated GPG key ID: %s", keyID) - t.Logf("Generated GPG fingerprint: %s", fingerprint) + keyID, fingerprint := generateTestGPGKey(t, tempDir, gnupgHomeDir, testPassphrase) // Set the GPG_AGENT_INFO to empty to ensure gpg-agent doesn't interfere t.Setenv("GPG_AGENT_INFO", "") @@ -224,12 +267,6 @@ Passphrase: ` + testPassphrase + ` // Use the real filesystem fs := afero.NewOsFs() - // Test data - testMnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" - - mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic)) - defer mnemonic.Destroy() - // Set test environment variables t.Setenv(secret.EnvGPGKeyID, keyID) @@ -239,162 +276,20 @@ Passphrase: ` + testPassphrase + ` // Test creation of a PGP unlock key through a vault t.Run("CreatePGPUnlocker", func(t *testing.T) { - // Set a limited test timeout to avoid hanging - timer := time.AfterFunc(30*time.Second, func() { - t.Fatalf("Test timed out after 30 seconds") - }) - defer timer.Stop() - - // Create a test vault directory structure - vlt, err := vault.CreateVault(fs, stateDir, vaultName, mnemonic) - if err != nil { - t.Fatalf("Failed to create vault: %v", err) - } - - // Set the current vault - err = vault.SelectVault(fs, stateDir, vaultName) - if err != nil { - t.Fatalf("Failed to select vault: %v", err) - } - - // Derive long-term key from mnemonic - ltIdentity, err := agehd.DeriveIdentity(testMnemonic, 0) - if err != nil { - t.Fatalf("Failed to derive long-term key: %v", err) - } - - // Get the vault directory - vaultDir, err := vlt.GetDirectory() - if err != nil { - t.Fatalf("Failed to get vault directory: %v", err) - } - - // Write long-term public key - ltPubKeyPath := filepath.Join(vaultDir, "pub.age") - if err := afero.WriteFile(fs, ltPubKeyPath, []byte(ltIdentity.Recipient().String()), secret.FilePerms); err != nil { - t.Fatalf("Failed to write long-term public key: %v", err) - } - - // Unlock the vault - vlt.Unlock(ltIdentity) - - // Create a passphrase unlocker first (to have current unlocker) - passphraseBuffer := memguard.NewBufferFromBytes([]byte("test-passphrase")) - defer passphraseBuffer.Destroy() - passUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer) - if err != nil { - t.Fatalf("Failed to create passphrase unlocker: %v", err) - } - - // Verify passphrase unlocker was created - if passUnlocker == nil { - t.Fatal("Passphrase unlocker is nil") - } - - // Now create a PGP unlock key (this will use our custom GPGEncryptFunc) - pgpUnlocker, err := secret.CreatePGPUnlocker(fs, stateDir, keyID, fingerprint, mnemonic, nil) - if err != nil { - t.Fatalf("Failed to create PGP unlock key: %v", err) - } - - // Verify the PGP unlock key was created - if pgpUnlocker == nil { - t.Fatal("PGP unlock key is nil") - } - - // Check if the key has the correct type - if pgpUnlocker.GetType() != "pgp" { - t.Errorf("Expected PGP unlock key type 'pgp', got '%s'", pgpUnlocker.GetType()) - } - - // Check if the key ID includes the GPG fingerprint - if !strings.Contains(pgpUnlocker.GetID(), fingerprint) { - t.Errorf("PGP unlock key ID '%s' does not contain GPG fingerprint '%s'", pgpUnlocker.GetID(), fingerprint) - } - - // Check if the key directory exists - unlockerDir := pgpUnlocker.GetDirectory() - keyExists, err := afero.DirExists(fs, unlockerDir) - if err != nil { - t.Fatalf("Failed to check if PGP key directory exists: %v", err) - } - if !keyExists { - t.Errorf("PGP unlock key directory does not exist: %s", unlockerDir) - } - - // Check if required files exist - recipientPath := filepath.Join(unlockerDir, "pub.txt") - recipientExists, err := afero.Exists(fs, recipientPath) - if err != nil { - t.Fatalf("Failed to check if recipient file exists: %v", err) - } - if !recipientExists { - t.Errorf("PGP unlock key recipient file does not exist: %s", recipientPath) - } - - privKeyPath := filepath.Join(unlockerDir, "priv.age.gpg") - privKeyExists, err := afero.Exists(fs, privKeyPath) - if err != nil { - t.Fatalf("Failed to check if private key file exists: %v", err) - } - if !privKeyExists { - t.Errorf("PGP unlock key private key file does not exist: %s", privKeyPath) - } - - metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json") - metadataExists, err := afero.Exists(fs, metadataPath) - if err != nil { - t.Fatalf("Failed to check if metadata file exists: %v", err) - } - if !metadataExists { - t.Errorf("PGP unlock key metadata file does not exist: %s", metadataPath) - } - - longtermPath := filepath.Join(unlockerDir, "longterm.age") - longtermExists, err := afero.Exists(fs, longtermPath) - if err != nil { - t.Fatalf("Failed to check if longterm key file exists: %v", err) - } - if !longtermExists { - t.Errorf("PGP unlock key longterm key file does not exist: %s", longtermPath) - } - - // Read and verify metadata - metadataBytes, err := afero.ReadFile(fs, metadataPath) - if err != nil { - t.Fatalf("Failed to read metadata: %v", err) - } - - var metadata struct { - ID string `json:"id"` - Type string `json:"type"` - CreatedAt time.Time `json:"createdAt"` - Flags []string `json:"flags"` - GPGKeyID string `json:"gpgKeyId"` - } - - if err := json.Unmarshal(metadataBytes, &metadata); err != nil { - t.Fatalf("Failed to parse metadata: %v", err) - } - - if metadata.Type != "pgp" { - t.Errorf("Expected metadata type 'pgp', got '%s'", metadata.Type) - } - - if metadata.GPGKeyID != fingerprint { - t.Errorf("Expected GPG fingerprint '%s', got '%s'", fingerprint, metadata.GPGKeyID) - } + testCreatePGPUnlocker(t, fs, stateDir, vaultName, keyID, fingerprint) }) // Set up key directory for individual tests unlockerDir := filepath.Join(tempDir, "unlocker") - if err := os.MkdirAll(unlockerDir, secret.DirPerms); err != nil { + + err = os.MkdirAll(unlockerDir, secret.DirPerms) + if err != nil { t.Fatalf("Failed to create unlocker directory: %v", err) } // Set up test metadata metadata := secret.UnlockerMetadata{ - Type: "pgp", + Type: pgpUnlockerType, CreatedAt: time.Now(), Flags: []string{"gpg", "encrypted"}, } @@ -404,105 +299,340 @@ Passphrase: ` + testPassphrase + ` // Test getting GPG key ID t.Run("GetGPGKeyID", func(t *testing.T) { - // Create PGP metadata with GPG key ID - type PGPUnlockerMetadata struct { - secret.UnlockerMetadata - GPGKeyID string `json:"gpgKeyId"` - } - - pgpMetadata := PGPUnlockerMetadata{ - UnlockerMetadata: metadata, - GPGKeyID: fingerprint, - } - - // Write metadata file - metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json") - metadataBytes, err := json.MarshalIndent(pgpMetadata, "", " ") - if err != nil { - t.Fatalf("Failed to marshal metadata: %v", err) - } - if err := afero.WriteFile(fs, metadataPath, metadataBytes, secret.FilePerms); err != nil { - t.Fatalf("Failed to write metadata: %v", err) - } - - // Get GPG key ID - retrievedKeyID, err := unlocker.GetGPGKeyID() - if err != nil { - t.Fatalf("Failed to get GPG key ID: %v", err) - } - - // Verify key ID (should be the fingerprint) - if retrievedKeyID != fingerprint { - t.Errorf("Expected GPG fingerprint '%s', got '%s'", fingerprint, retrievedKeyID) - } + testGetGPGKeyID(t, fs, unlocker, unlockerDir, metadata, fingerprint) }) // Test getting identity from PGP unlocker t.Run("GetIdentity", func(t *testing.T) { - // Generate an age identity for testing - ageIdentity, err := age.GenerateX25519Identity() - if err != nil { - t.Fatalf("Failed to generate age identity: %v", err) - } - - // Write the recipient - recipientPath := filepath.Join(unlockerDir, "pub.txt") - if err := afero.WriteFile(fs, recipientPath, []byte(ageIdentity.Recipient().String()), secret.FilePerms); err != nil { - t.Fatalf("Failed to write recipient: %v", err) - } - - // GPG encrypt the private key using our custom encrypt function - privKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String())) - defer privKeyBuffer.Destroy() - encryptedOutput, err := secret.GPGEncryptFunc(privKeyBuffer, keyID) - if err != nil { - t.Fatalf("Failed to encrypt with GPG: %v", err) - } - - // Write the encrypted data to a file - encryptedPath := filepath.Join(unlockerDir, "priv.age.gpg") - if err := afero.WriteFile(fs, encryptedPath, encryptedOutput, secret.FilePerms); err != nil { - t.Fatalf("Failed to write encrypted private key: %v", err) - } - - // Now try to get the identity - this will use our custom GPGDecryptFunc - identity, err := unlocker.GetIdentity() - if err != nil { - t.Fatalf("Failed to get identity: %v", err) - } - - // Verify the identity matches - expectedPubKey := ageIdentity.Recipient().String() - actualPubKey := identity.Recipient().String() - if actualPubKey != expectedPubKey { - t.Errorf("Expected public key '%s', got '%s'", expectedPubKey, actualPubKey) - } + testPGPUnlockerGetIdentity(t, fs, unlocker, unlockerDir, keyID) }) // Test removing the unlocker t.Run("RemoveUnlocker", func(t *testing.T) { - // Ensure unlocker directory exists before removal - keyExists, err := afero.DirExists(fs, unlockerDir) - if err != nil { - t.Fatalf("Failed to check if unlocker directory exists: %v", err) - } - if !keyExists { - t.Fatalf("Unlocker directory does not exist: %s", unlockerDir) - } - - // Remove unlocker - err = unlocker.Remove() - if err != nil { - t.Fatalf("Failed to remove unlocker: %v", err) - } - - // Verify directory is gone - keyExists, err = afero.DirExists(fs, unlockerDir) - if err != nil { - t.Fatalf("Failed to check if unlocker directory exists: %v", err) - } - if keyExists { - t.Errorf("Unlocker directory still exists after removal: %s", unlockerDir) - } + testRemovePGPUnlocker(t, fs, unlocker, unlockerDir) }) } + +// testCreatePGPUnlocker creates a vault with a passphrase unlocker, then a +// PGP unlocker for the GPG key keyID, and checks the PGP unlocker's files +// and metadata. +func testCreatePGPUnlocker( + t *testing.T, fs afero.Fs, stateDir, vaultName, keyID, fingerprint string, +) { + t.Helper() + + // Set a limited test timeout to avoid hanging + timer := time.AfterFunc(30*time.Second, func() { + t.Fatalf("Test timed out after 30 seconds") + }) + defer timer.Stop() + + mnemonic := testMnemonicBuffer(t) + + // Create a test vault directory structure + vlt, err := vault.CreateVault(fs, stateDir, vaultName, mnemonic) + if err != nil { + t.Fatalf("Failed to create vault: %v", err) + } + + // Set the current vault + err = vault.SelectVault(fs, stateDir, vaultName) + if err != nil { + t.Fatalf("Failed to select vault: %v", err) + } + + // Derive long-term key from mnemonic + ltIdentity, err := agehd.DeriveIdentity(testMnemonic, 0) + if err != nil { + t.Fatalf("Failed to derive long-term key: %v", err) + } + + // Get the vault directory + vaultDir, err := vlt.GetDirectory() + if err != nil { + t.Fatalf("Failed to get vault directory: %v", err) + } + + // Write long-term public key + ltPubKeyPath := filepath.Join(vaultDir, "pub.age") + + err = afero.WriteFile(fs, ltPubKeyPath, + []byte(ltIdentity.Recipient().String()), secret.FilePerms) + if err != nil { + t.Fatalf("Failed to write long-term public key: %v", err) + } + + // Unlock the vault + vlt.Unlock(ltIdentity) + + // Create a passphrase unlocker first (to have current unlocker) + passphraseBuffer := memguard.NewBufferFromBytes([]byte("test-passphrase")) + defer passphraseBuffer.Destroy() + + passUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer) + if err != nil { + t.Fatalf("Failed to create passphrase unlocker: %v", err) + } + + // Verify passphrase unlocker was created + if passUnlocker == nil { + t.Fatal("Passphrase unlocker is nil") + } + + // Now create a PGP unlock key (this will use our custom GPGEncryptFunc) + pgpUnlocker, err := secret.CreatePGPUnlocker( + fs, stateDir, keyID, fingerprint, mnemonic, nil) + if err != nil { + t.Fatalf("Failed to create PGP unlock key: %v", err) + } + + // Verify the PGP unlock key was created + if pgpUnlocker == nil { + t.Fatal("PGP unlock key is nil") + } + + // Check if the key has the correct type + if pgpUnlocker.GetType() != pgpUnlockerType { + t.Errorf("Expected PGP unlock key type 'pgp', got '%s'", pgpUnlocker.GetType()) + } + + // Check if the key ID includes the GPG fingerprint + if !strings.Contains(pgpUnlocker.GetID(), fingerprint) { + t.Errorf("PGP unlock key ID '%s' does not contain GPG fingerprint '%s'", + pgpUnlocker.GetID(), fingerprint) + } + + checkPGPUnlockerFiles(t, fs, pgpUnlocker.GetDirectory()) + checkPGPUnlockerMetadata(t, fs, pgpUnlocker.GetDirectory(), fingerprint) +} + +// checkPGPUnlockerFiles checks that the PGP unlocker in unlockerDir has all +// its files. +func checkPGPUnlockerFiles(t *testing.T, fs afero.Fs, unlockerDir string) { + t.Helper() + + // Check if the key directory exists + keyExists, err := afero.DirExists(fs, unlockerDir) + if err != nil { + t.Fatalf("Failed to check if PGP key directory exists: %v", err) + } + + if !keyExists { + t.Errorf("PGP unlock key directory does not exist: %s", unlockerDir) + } + + // Check if required files exist + recipientPath := filepath.Join(unlockerDir, "pub.txt") + + recipientExists, err := afero.Exists(fs, recipientPath) + if err != nil { + t.Fatalf("Failed to check if recipient file exists: %v", err) + } + + if !recipientExists { + t.Errorf("PGP unlock key recipient file does not exist: %s", recipientPath) + } + + privKeyPath := filepath.Join(unlockerDir, "priv.age.gpg") + + privKeyExists, err := afero.Exists(fs, privKeyPath) + if err != nil { + t.Fatalf("Failed to check if private key file exists: %v", err) + } + + if !privKeyExists { + t.Errorf("PGP unlock key private key file does not exist: %s", privKeyPath) + } + + metadataPath := filepath.Join(unlockerDir, unlockerMetadataFile) + + metadataExists, err := afero.Exists(fs, metadataPath) + if err != nil { + t.Fatalf("Failed to check if metadata file exists: %v", err) + } + + if !metadataExists { + t.Errorf("PGP unlock key metadata file does not exist: %s", metadataPath) + } + + longtermPath := filepath.Join(unlockerDir, "longterm.age") + + longtermExists, err := afero.Exists(fs, longtermPath) + if err != nil { + t.Fatalf("Failed to check if longterm key file exists: %v", err) + } + + if !longtermExists { + t.Errorf("PGP unlock key longterm key file does not exist: %s", longtermPath) + } +} + +// checkPGPUnlockerMetadata checks that the metadata of the PGP unlocker in +// unlockerDir names its type and the GPG key by fingerprint. +func checkPGPUnlockerMetadata( + t *testing.T, fs afero.Fs, unlockerDir, fingerprint string, +) { + t.Helper() + + // Read and verify metadata + metadataPath := filepath.Join(unlockerDir, unlockerMetadataFile) + + metadataBytes, err := afero.ReadFile(fs, metadataPath) + if err != nil { + t.Fatalf("Failed to read metadata: %v", err) + } + + var metadata struct { + ID string `json:"id"` + Type string `json:"type"` + CreatedAt time.Time `json:"createdAt"` + Flags []string `json:"flags"` + GPGKeyID string `json:"gpgKeyId"` + } + + err = json.Unmarshal(metadataBytes, &metadata) + if err != nil { + t.Fatalf("Failed to parse metadata: %v", err) + } + + if metadata.Type != pgpUnlockerType { + t.Errorf("Expected metadata type 'pgp', got '%s'", metadata.Type) + } + + if metadata.GPGKeyID != fingerprint { + t.Errorf("Expected GPG fingerprint '%s', got '%s'", fingerprint, metadata.GPGKeyID) + } +} + +// testGetGPGKeyID writes PGP unlocker metadata holding the GPG fingerprint +// into unlockerDir and checks that unlocker reads it back. +func testGetGPGKeyID( + t *testing.T, fs afero.Fs, unlocker *secret.PGPUnlocker, + unlockerDir string, metadata secret.UnlockerMetadata, fingerprint string, +) { + t.Helper() + + // Create PGP metadata with GPG key ID + type PGPUnlockerMetadata struct { + secret.UnlockerMetadata + + GPGKeyID string `json:"gpgKeyId"` + } + + pgpMetadata := PGPUnlockerMetadata{ + UnlockerMetadata: metadata, + GPGKeyID: fingerprint, + } + + // Write metadata file + metadataPath := filepath.Join(unlockerDir, unlockerMetadataFile) + + metadataBytes, err := json.MarshalIndent(pgpMetadata, "", " ") + if err != nil { + t.Fatalf("Failed to marshal metadata: %v", err) + } + + err = afero.WriteFile(fs, metadataPath, metadataBytes, secret.FilePerms) + if err != nil { + t.Fatalf("Failed to write metadata: %v", err) + } + + // Get GPG key ID + retrievedKeyID, err := unlocker.GetGPGKeyID() + if err != nil { + t.Fatalf("Failed to get GPG key ID: %v", err) + } + + // Verify key ID (should be the fingerprint) + if retrievedKeyID != fingerprint { + t.Errorf("Expected GPG fingerprint '%s', got '%s'", fingerprint, retrievedKeyID) + } +} + +// testPGPUnlockerGetIdentity writes an age identity encrypted to the GPG key +// keyID into unlockerDir and checks that unlocker decrypts it. +func testPGPUnlockerGetIdentity( + t *testing.T, fs afero.Fs, unlocker *secret.PGPUnlocker, + unlockerDir, keyID string, +) { + t.Helper() + + // Generate an age identity for testing + ageIdentity, err := age.GenerateX25519Identity() + if err != nil { + t.Fatalf("Failed to generate age identity: %v", err) + } + + // Write the recipient + recipientPath := filepath.Join(unlockerDir, "pub.txt") + + err = afero.WriteFile(fs, recipientPath, + []byte(ageIdentity.Recipient().String()), secret.FilePerms) + if err != nil { + t.Fatalf("Failed to write recipient: %v", err) + } + + // GPG encrypt the private key using our custom encrypt function + privKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String())) + defer privKeyBuffer.Destroy() + + encryptedOutput, err := secret.GPGEncryptFunc(privKeyBuffer, keyID) + if err != nil { + t.Fatalf("Failed to encrypt with GPG: %v", err) + } + + // Write the encrypted data to a file + encryptedPath := filepath.Join(unlockerDir, "priv.age.gpg") + + err = afero.WriteFile(fs, encryptedPath, encryptedOutput, secret.FilePerms) + if err != nil { + t.Fatalf("Failed to write encrypted private key: %v", err) + } + + // Now try to get the identity - this will use our custom GPGDecryptFunc + identity, err := unlocker.GetIdentity() + if err != nil { + t.Fatalf("Failed to get identity: %v", err) + } + + // Verify the identity matches + expectedPubKey := ageIdentity.Recipient().String() + actualPubKey := identity.Recipient().String() + + if actualPubKey != expectedPubKey { + t.Errorf("Expected public key '%s', got '%s'", expectedPubKey, actualPubKey) + } +} + +// testRemovePGPUnlocker removes unlocker and checks that unlockerDir is gone. +func testRemovePGPUnlocker( + t *testing.T, fs afero.Fs, unlocker *secret.PGPUnlocker, unlockerDir string, +) { + t.Helper() + + // Ensure unlocker directory exists before removal + keyExists, err := afero.DirExists(fs, unlockerDir) + if err != nil { + t.Fatalf("Failed to check if unlocker directory exists: %v", err) + } + + if !keyExists { + t.Fatalf("Unlocker directory does not exist: %s", unlockerDir) + } + + // Remove unlocker + err = unlocker.Remove() + if err != nil { + t.Fatalf("Failed to remove unlocker: %v", err) + } + + // Verify directory is gone + keyExists, err = afero.DirExists(fs, unlockerDir) + if err != nil { + t.Fatalf("Failed to check if unlocker directory exists: %v", err) + } + + if keyExists { + t.Errorf("Unlocker directory still exists after removal: %s", unlockerDir) + } +} diff --git a/internal/secret/seunlocker_darwin.go b/internal/secret/seunlocker_darwin.go index d1bd945..eb2d580 100644 --- a/internal/secret/seunlocker_darwin.go +++ b/internal/secret/seunlocker_darwin.go @@ -1,5 +1,4 @@ //go:build darwin -// +build darwin package secret @@ -13,7 +12,6 @@ import ( "filippo.io/age" "git.eeqj.de/sneak/secret/internal/macse" - "git.eeqj.de/sneak/secret/pkg/agehd" "github.com/awnumar/memguard" "github.com/spf13/afero" ) @@ -32,6 +30,7 @@ const ( // SecureEnclaveUnlockerMetadata extends UnlockerMetadata with SE-specific data. type SecureEnclaveUnlockerMetadata struct { UnlockerMetadata + SEKeyLabel string `json:"seKeyLabel"` SEKeyHash string `json:"seKeyHash"` } @@ -43,6 +42,19 @@ type SecureEnclaveUnlocker struct { fs afero.Fs } +// NewSecureEnclaveUnlocker creates a new SecureEnclaveUnlocker instance. +func NewSecureEnclaveUnlocker( + fs afero.Fs, + directory string, + metadata UnlockerMetadata, +) *SecureEnclaveUnlocker { + return &SecureEnclaveUnlocker{ + Directory: directory, + Metadata: metadata, + fs: fs, + } +} + // GetIdentity implements Unlocker interface for SE-based unlockers. // Decrypts the vault's long-term private key directly using the Secure Enclave. func (s *SecureEnclaveUnlocker) GetIdentity() (*age.X25519Identity, error) { @@ -58,6 +70,7 @@ func (s *SecureEnclaveUnlocker) GetIdentity() (*age.X25519Identity, error) { // Read ECIES-encrypted long-term private key from disk encryptedPath := filepath.Join(s.Directory, seLongtermFilename) + encryptedData, err := afero.ReadFile(s.fs, encryptedPath) if err != nil { return nil, fmt.Errorf( @@ -140,7 +153,9 @@ func (s *SecureEnclaveUnlocker) Remove() error { if seKeyHash != "" { Debug("Deleting SE key", "hash", seKeyHash) - if err := macse.DeleteKey(seKeyHash); err != nil { + + err = macse.DeleteKey(seKeyHash) + if err != nil { Debug("Failed to delete SE key", "error", err, "hash", seKeyHash) return fmt.Errorf("failed to delete SE key: %w", err) @@ -148,7 +163,9 @@ func (s *SecureEnclaveUnlocker) Remove() error { } Debug("Removing SE unlocker directory", "directory", s.Directory) - if err := RemoveDirAtomic(s.fs, s.Directory); err != nil { + + err = RemoveDirAtomic(s.fs, s.Directory) + if err != nil { return fmt.Errorf("failed to remove SE unlocker directory: %w", err) } @@ -158,34 +175,24 @@ func (s *SecureEnclaveUnlocker) Remove() error { } // getSEKeyInfo reads the SE key label and hash from metadata. -func (s *SecureEnclaveUnlocker) getSEKeyInfo() (label string, hash string, err error) { +func (s *SecureEnclaveUnlocker) getSEKeyInfo() (string, string, error) { metadataPath := filepath.Join(s.Directory, "unlocker-metadata.json") + metadataData, err := afero.ReadFile(s.fs, metadataPath) if err != nil { return "", "", fmt.Errorf("failed to read SE metadata: %w", err) } var seMetadata SecureEnclaveUnlockerMetadata - if err := json.Unmarshal(metadataData, &seMetadata); err != nil { + + err = json.Unmarshal(metadataData, &seMetadata) + if err != nil { return "", "", fmt.Errorf("failed to parse SE metadata: %w", err) } return seMetadata.SEKeyLabel, seMetadata.SEKeyHash, nil } -// NewSecureEnclaveUnlocker creates a new SecureEnclaveUnlocker instance. -func NewSecureEnclaveUnlocker( - fs afero.Fs, - directory string, - metadata UnlockerMetadata, -) *SecureEnclaveUnlocker { - return &SecureEnclaveUnlocker{ - Directory: directory, - Metadata: metadata, - fs: fs, - } -} - // generateSEKeyLabel generates a unique label for the SE CTK identity. func generateSEKeyLabel(vaultName string) (string, error) { hostname, err := os.Hostname() @@ -214,7 +221,8 @@ func CreateSecureEnclaveUnlocker( stateDir string, mnemonic, passphrase *memguard.LockedBuffer, ) (*SecureEnclaveUnlocker, error) { - if err := checkMacOSAvailable(); err != nil { + err := checkMacOSAvailable() + if err != nil { return nil, err } @@ -231,6 +239,7 @@ func CreateSecureEnclaveUnlocker( // Step 1: Create P-256 key in the Secure Enclave via sc_auth Debug("Creating Secure Enclave key", "label", seKeyLabel) + _, seKeyHash, err := macse.CreateKey(seKeyLabel) if err != nil { return nil, fmt.Errorf("failed to create SE key: %w", err) @@ -263,14 +272,14 @@ func CreateSecureEnclaveUnlocker( return nil, fmt.Errorf("failed to get vault directory: %w", err) } - unlockerDirName := fmt.Sprintf("se-%s", filepath.Base(seKeyLabel)) + unlockerDirName := "se-" + filepath.Base(seKeyLabel) unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerDirName) seMetadata := SecureEnclaveUnlockerMetadata{ UnlockerMetadata: UnlockerMetadata{ Type: seUnlockerType, CreatedAt: time.Now().UTC(), - Flags: []string{seUnlockerType, "macos"}, + Flags: []string{seUnlockerType, macOSFlag}, }, SEKeyLabel: seKeyLabel, SEKeyHash: seKeyHash, @@ -283,20 +292,7 @@ func CreateSecureEnclaveUnlocker( // 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 + return writeSEUnlockerFiles(fs, dir, encryptedLtKey, metadataBytes) }) if err != nil { return nil, err @@ -309,6 +305,29 @@ func CreateSecureEnclaveUnlocker( }, nil } +// writeSEUnlockerFiles writes the files of a new SE unlocker into dir: the +// SE-encrypted long-term key, then the metadata. +func writeSEUnlockerFiles( + fs afero.Fs, dir string, encryptedLtKey, metadataBytes []byte, +) error { + err := WriteFileAtomic(fs, filepath.Join(dir, seLongtermFilename), + encryptedLtKey) + if err != nil { + return fmt.Errorf( + "failed to write SE-encrypted long-term key: %w", + err, + ) + } + + err = WriteFileAtomic(fs, + filepath.Join(dir, "unlocker-metadata.json"), metadataBytes) + if err != nil { + return fmt.Errorf("failed to write metadata: %w", err) + } + + return nil +} + // getLongTermKeyForSE retrieves the vault's long-term private key, derived // from mnemonic when it is not nil, else through the current unlocker, which // is given passphrase when it is a passphrase unlocker. @@ -318,37 +337,7 @@ func getLongTermKeyForSE( mnemonic, passphrase *memguard.LockedBuffer, ) (*memguard.LockedBuffer, error) { if mnemonic != nil { - // Read vault metadata to get the correct derivation index - vaultDir, err := vault.GetDirectory() - if err != nil { - return nil, fmt.Errorf("failed to get vault directory: %w", err) - } - - metadataPath := filepath.Join(vaultDir, "vault-metadata.json") - metadataBytes, err := afero.ReadFile(fs, metadataPath) - if err != nil { - return nil, fmt.Errorf("failed to read vault metadata: %w", err) - } - - var metadata VaultMetadata - if err := json.Unmarshal(metadataBytes, &metadata); err != nil { - return nil, fmt.Errorf("failed to parse vault metadata: %w", err) - } - - // Use mnemonic with the vault's actual derivation index - ltIdentity, err := agehd.DeriveIdentity( - mnemonic.String(), - metadata.DerivationIndex, - ) - - if err != nil { - return nil, fmt.Errorf( - "failed to derive long-term key from mnemonic: %w", - err, - ) - } - - return memguard.NewBufferFromBytes([]byte(ltIdentity.String())), nil + return deriveLongTermPrivateKey(fs, vault, mnemonic) } currentUnlocker, err := vault.GetCurrentUnlocker() @@ -373,6 +362,7 @@ func getLongTermKeyForSE( currentUnlocker.GetDirectory(), "longterm.age", ) + encryptedLtKey, err := afero.ReadFile(fs, longtermPath) if err != nil { return nil, fmt.Errorf( diff --git a/internal/secret/seunlocker_test.go b/internal/secret/seunlocker_test.go index cc778b1..47c5063 100644 --- a/internal/secret/seunlocker_test.go +++ b/internal/secret/seunlocker_test.go @@ -1,6 +1,6 @@ //go:build darwin -// +build darwin +//nolint:testpackage // white-box test of unexported Secure Enclave helpers package secret import ( @@ -13,12 +13,14 @@ import ( ) func TestNewSecureEnclaveUnlocker(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() dir := "/tmp/test-se-unlocker" metadata := UnlockerMetadata{ - Type: "secure-enclave", + Type: seUnlockerType, CreatedAt: time.Date(2026, 1, 15, 10, 30, 0, 0, time.UTC), - Flags: []string{"secure-enclave", "macos"}, + Flags: []string{seUnlockerType, "macos"}, } unlocker := NewSecureEnclaveUnlocker(fs, dir, metadata) @@ -35,9 +37,11 @@ func TestNewSecureEnclaveUnlocker(t *testing.T) { } func TestSecureEnclaveUnlockerImplementsInterface(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() metadata := UnlockerMetadata{ - Type: "secure-enclave", + Type: seUnlockerType, CreatedAt: time.Now().UTC(), } @@ -48,9 +52,11 @@ func TestSecureEnclaveUnlockerImplementsInterface(t *testing.T) { } func TestSecureEnclaveUnlockerGetIDFormat(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() metadata := UnlockerMetadata{ - Type: "secure-enclave", + Type: seUnlockerType, CreatedAt: time.Date(2026, 3, 10, 14, 30, 0, 0, time.UTC), } @@ -63,6 +69,8 @@ func TestSecureEnclaveUnlockerGetIDFormat(t *testing.T) { } func TestGenerateSEKeyLabel(t *testing.T) { + t.Parallel() + label, err := generateSEKeyLabel("test-vault") require.NoError(t, err) @@ -72,6 +80,8 @@ func TestGenerateSEKeyLabel(t *testing.T) { } func TestSecureEnclaveUnlockerGetIdentityMissingFile(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() dir := "/tmp/test-se-unlocker-missing" @@ -84,10 +94,12 @@ func TestSecureEnclaveUnlockerGetIdentityMissingFile(t *testing.T) { "seKeyLabel": "berlin.sneak.app.secret.se.test", "seKeyHash": "abc123" }` - require.NoError(t, afero.WriteFile(fs, dir+"/unlocker-metadata.json", []byte(metadataJSON), FilePerms)) + require.NoError(t, afero.WriteFile( + fs, dir+"/unlocker-metadata.json", []byte(metadataJSON), FilePerms, + )) metadata := UnlockerMetadata{ - Type: "secure-enclave", + Type: seUnlockerType, CreatedAt: time.Date(2026, 1, 15, 10, 30, 0, 0, time.UTC), } @@ -96,6 +108,6 @@ func TestSecureEnclaveUnlockerGetIdentityMissingFile(t *testing.T) { // GetIdentity should fail because the encrypted longterm key file is missing identity, err := unlocker.GetIdentity() assert.Nil(t, identity) - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "failed to read SE-encrypted long-term key") } diff --git a/internal/secret/validation_darwin_test.go b/internal/secret/validation_darwin_test.go index 6c6c32d..1769868 100644 --- a/internal/secret/validation_darwin_test.go +++ b/internal/secret/validation_darwin_test.go @@ -1,5 +1,6 @@ //go:build darwin +//nolint:testpackage // white-box test of unexported validateKeychainItemName package secret import ( @@ -7,138 +8,46 @@ import ( ) func TestValidateKeychainItemName(t *testing.T) { + t.Parallel() + tests := []struct { name string itemName string wantErr bool }{ // Valid cases - { - name: "valid simple name", - itemName: "my-secret-key", - wantErr: false, - }, - { - name: "valid name with dots", - itemName: "com.example.app.key", - wantErr: false, - }, - { - name: "valid name with underscores", - itemName: "my_secret_key_123", - wantErr: false, - }, - { - name: "valid alphanumeric", - itemName: "Secret123Key", - wantErr: false, - }, - { - name: "valid with hyphen at start", - itemName: "-my-key", - wantErr: false, - }, - { - name: "valid with dot at start", - itemName: ".hidden-key", - wantErr: false, - }, + {name: "valid simple name", itemName: "my-secret-key", wantErr: false}, + {name: "valid name with dots", itemName: "com.example.app.key", wantErr: false}, + {name: "valid name with underscores", itemName: "my_secret_key_123", wantErr: false}, + {name: "valid alphanumeric", itemName: "Secret123Key", wantErr: false}, + {name: "valid with hyphen at start", itemName: "-my-key", wantErr: false}, + {name: "valid with dot at start", itemName: ".hidden-key", wantErr: false}, // Invalid cases - { - name: "empty item name", - itemName: "", - wantErr: true, - }, - { - name: "item name with spaces", - itemName: "my secret key", - wantErr: true, - }, - { - name: "item name with semicolon", - itemName: "key;rm -rf /", - wantErr: true, - }, - { - name: "item name with pipe", - itemName: "key|cat /etc/passwd", - wantErr: true, - }, - { - name: "item name with backticks", - itemName: "key`whoami`", - wantErr: true, - }, - { - name: "item name with dollar sign", - itemName: "key$(whoami)", - wantErr: true, - }, - { - name: "item name with quotes", - itemName: "key\"name", - wantErr: true, - }, - { - name: "item name with single quotes", - itemName: "key'name", - wantErr: true, - }, - { - name: "item name with backslash", - itemName: "key\\name", - wantErr: true, - }, - { - name: "item name with newline", - itemName: "key\nname", - wantErr: true, - }, - { - name: "item name with carriage return", - itemName: "key\rname", - wantErr: true, - }, - { - name: "item name with ampersand", - itemName: "key&echo test", - wantErr: true, - }, - { - name: "item name with redirect", - itemName: "key>/tmp/test", - wantErr: true, - }, - { - name: "item name with null byte", - itemName: "key\x00name", - wantErr: true, - }, - { - name: "item name with parentheses", - itemName: "key(test)", - wantErr: true, - }, - { - name: "item name with brackets", - itemName: "key[test]", - wantErr: true, - }, - { - name: "item name with asterisk", - itemName: "key*", - wantErr: true, - }, - { - name: "item name with question mark", - itemName: "key?", - wantErr: true, - }, + {name: "empty item name", itemName: "", wantErr: true}, + {name: "item name with spaces", itemName: "my secret key", wantErr: true}, + {name: "item name with semicolon", itemName: "key;rm -rf /", wantErr: true}, + {name: "item name with pipe", itemName: "key|cat /etc/passwd", wantErr: true}, + {name: "item name with backticks", itemName: "key`whoami`", wantErr: true}, + {name: "item name with dollar sign", itemName: "key$(whoami)", wantErr: true}, + {name: "item name with quotes", itemName: "key\"name", wantErr: true}, + {name: "item name with single quotes", itemName: "key'name", wantErr: true}, + {name: "item name with backslash", itemName: "key\\name", wantErr: true}, + {name: "item name with newline", itemName: "key\nname", wantErr: true}, + {name: "item name with carriage return", itemName: "key\rname", wantErr: true}, + {name: "item name with ampersand", itemName: "key&echo test", wantErr: true}, + {name: "item name with redirect", itemName: "key>/tmp/test", wantErr: true}, + {name: "item name with null byte", itemName: "key\x00name", wantErr: true}, + {name: "item name with parentheses", itemName: "key(test)", wantErr: true}, + {name: "item name with brackets", itemName: "key[test]", wantErr: true}, + {name: "item name with asterisk", itemName: "key*", wantErr: true}, + {name: "item name with question mark", itemName: "key?", wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := validateKeychainItemName(tt.itemName) if (err != nil) != tt.wantErr { t.Errorf("validateKeychainItemName() error = %v, wantErr %v", err, tt.wantErr) diff --git a/script/check b/script/check index cc046f7..1150ff0 100755 --- a/script/check +++ b/script/check @@ -1,7 +1,6 @@ #!/bin/sh -# script/check: run all checks (test, lint, fmt-check). Our own -# extension to scripts-to-rule-them-all. Must not modify any files. -# Generic: usually needs no adaptation. +# script/check: run all checks (test, lint, lint-darwin, fmt-check). Our +# own extension to scripts-to-rule-them-all. Must not modify any files. set -eu SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" @@ -9,6 +8,7 @@ SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" main() { "$SCRIPT_DIR/test" "$SCRIPT_DIR/lint" + "$SCRIPT_DIR/lint-darwin" "$SCRIPT_DIR/fmt-check" } diff --git a/script/lint-darwin b/script/lint-darwin new file mode 100755 index 0000000..01cc83d --- /dev/null +++ b/script/lint-darwin @@ -0,0 +1,26 @@ +#!/bin/sh +# script/lint-darwin: type-check (go vet) and lint the code as a macOS +# build compiles it, from Linux, in docker only. CI runs on Linux, which +# never compiles the files built only for macOS. Builds the lint-darwin +# stage of Dockerfile.lint, rebuilt on every run as script/lint does. +# +# Cgo is off: compiling cgo code for macOS needs Apple's SDK headers. That +# leaves out the files built only with cgo on macOS: the keychain unlocker's +# calls into the keychain (keychainunlocker_cgo.go, and +# keychainunlocker_test.go) and the Secure Enclave bindings (internal/macse). +# Nothing on Linux checks those. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + docker build \ + --progress=plain \ + --target lint-darwin \ + --no-cache-filter=lint-darwin \ + --output=type=cacheonly \ + -f Dockerfile.lint . +} + +main "$@"