Update golangci-lint to v2.12.2 with canonical config (closes #30)

- Replace .golangci.yml with the canonical strict config (all linters enabled except the standard disable list; lll 88, funlen 80/50, cyclop 15, dupl 100; test files now linted) - Pin the Dockerfile lint stage to golangci/golangci-lint:v2.12.2 by tag and digest (Debian-based) - Fix all ~1550 findings surfaced by the new config: line wrapping, wsl_v5/nlreturn blank lines, noinlineerr splits, err113 sentinel errors, perfsprint/modernize rewrites, goconst constants, thelper, testifylint, noctx CommandContext, testpackage conversions, t.Parallel() where safe, and complexity/dupl helper extraction - Record the change and follow-up items in TODO.md User-visible strings -------------------- No user-visible string changes remain. Every error message this branch composes is byte-identical to the one main composes. The err113 sentinels are shaped so that fmt.Errorf reassembles the original text around them: a sentinel carries the fixed words of the message and the caller supplies the interpolated value in the position it has always occupied. Where the value sits in the middle of the sentence the sentinel therefore holds only a fragment (for example vault.ErrVaultNotFound is "does not exist", composed by its caller as "vault <name> does not exist"); each such sentinel documents the message it participates in. Verified mechanically rather than by inspection: every fmt.Errorf and errors.New call site in both trees was parsed, the Error() text of any sentinel passed to %w substituted in, and the resulting sets of composed message templates compared. All 350 templates main produces are still produced, character for character; the set of messages lost or altered is empty. unlocker list ------------- findUnlockerIDByMetadata now returns (string, error) instead of signalling failure with an empty ID. An unreadable unlockers.d is no longer indistinguishable from "no matching entry", so UnlockersList skips the entry with a warning naming the directory, as it did before the scan was extracted into a helper, rather than emitting a row under a synthesized fallback ID that no unlocker remove or unlocker select can match and that suppresses the current-unlocker marker. The duplicate-check and shell-completion callers skip on the same condition, matching their pre-extraction behavior. Covered by tests in internal/cli/unlockers_list_test.go.
2026-08-07 17:27:23 +00:00
parent 6e5e0db999
commit 397011a592
60 changed files with 6867 additions and 4875 deletions
--- a/internal/vault/secrets_version_test.go
+++ b/internal/vault/secrets_version_test.go
@@ -2,10 +2,14 @@
 //
 // Integration tests for vault-level version operations:
 //
-// - TestVaultAddSecretCreatesVersion: Tests that AddSecret creates proper version structure
-// - TestVaultAddSecretMultipleVersions: Tests creating multiple versions with force flag
-// - TestVaultGetSecretVersion: Tests retrieving specific versions and current version
-// - TestVaultVersionTimestamps: Tests timestamp logic (notBefore/notAfter) across versions
+// - TestVaultAddSecretCreatesVersion: Tests that AddSecret creates proper
+//   version structure
+// - TestVaultAddSecretMultipleVersions: Tests creating multiple versions with
+//   force flag
+// - TestVaultGetSecretVersion: Tests retrieving specific versions and current
+//   version
+// - TestVaultVersionTimestamps: Tests timestamp logic (notBefore/notAfter)
+//   across versions
 // - TestVaultGetNonExistentVersion: Tests error handling for invalid versions
 // - TestUpdateVersionMetadata: Tests metadata update functionality
 //
@@ -15,6 +19,7 @@
 // - Promotion doesn't modify timestamps
 // - Metadata remains encrypted and intact

+//nolint:testpackage // white-box test of unexported updateVersionMetadata
 package vault

 import (
@@ -30,33 +35,61 @@ import (
 	"github.com/stretchr/testify/require"
 )

+// testMnemonic is the mnemonic used to derive the vault long-term key.
+//
+//nolint:dupword // BIP39 test mnemonic intentionally repeats a word
+const testMnemonic = "abandon abandon abandon abandon abandon abandon abandon " +
+	"abandon abandon abandon abandon about"
+
+// envTestMnemonic is the (deliberately different) mnemonic placed in the
+// environment; the vault is unlocked manually with the derived key in
+// createTestVaultWithKey.
+//
+//nolint:dupword // BIP39-style test mnemonic intentionally repeats a word
+const envTestMnemonic = "abandon abandon abandon abandon abandon abandon " +
+	"abandon abandon abandon about"
+
+// Shared fixtures for white-box tests in this package.
+const (
+	testStateDir   = "/test/state"
+	testSecretPath = "test/secret"
+)
+
 // Helper function to add a secret to vault with proper buffer protection
-func addTestSecretToVault(t *testing.T, vault *Vault, name string, value []byte, force bool) {
+func addTestSecretToVault(
+	t *testing.T, vault *Vault, name string, value []byte, force bool,
+) {
 	t.Helper()
+
 	buffer := memguard.NewBufferFromBytes(value)
 	defer buffer.Destroy()
+
 	err := vault.AddSecret(name, buffer, force)
 	require.NoError(t, err)
 }

-// Helper function to create a vault with long-term key set up
-func createTestVaultWithKey(t *testing.T, fs afero.Fs, stateDir, vaultName string) *Vault {
+// Helper function to create a vault named "test" with its long-term key set
+// up and unlocked
+func createTestVaultWithKey(t *testing.T, fs afero.Fs) *Vault {
+	t.Helper()
+
 	// Set mnemonic for testing
-	t.Setenv(secret.EnvMnemonic, "abandon abandon abandon abandon abandon abandon abandon abandon abandon about")
+	t.Setenv(secret.EnvMnemonic, envTestMnemonic)

 	// Create vault
-	vault, err := CreateVault(fs, stateDir, vaultName)
+	vault, err := CreateVault(fs, testStateDir, "test")
 	require.NoError(t, err)

 	// Derive and store long-term key from mnemonic
-	mnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
-	ltIdentity, err := agehd.DeriveIdentity(mnemonic, 0)
+	ltIdentity, err := agehd.DeriveIdentity(testMnemonic, 0)
 	require.NoError(t, err)

 	// Store long-term public key in vault
 	vaultDir, _ := vault.GetDirectory()
 	ltPubKeyPath := filepath.Join(vaultDir, "pub.age")
-	err = afero.WriteFile(fs, ltPubKeyPath, []byte(ltIdentity.Recipient().String()), 0o600)
+
+	err = afero.WriteFile(fs, ltPubKeyPath,
+		[]byte(ltIdentity.Recipient().String()), 0o600)
 	require.NoError(t, err)

 	// Unlock the vault with the derived key
@@ -65,20 +98,19 @@ func createTestVaultWithKey(t *testing.T, fs afero.Fs, stateDir, vaultName strin
 	return vault
 }

+//nolint:paralleltest // createTestVaultWithKey uses t.Setenv
 func TestVaultAddSecretCreatesVersion(t *testing.T) {
 	fs := afero.NewMemMapFs()
-	stateDir := "/test/state"

 	// Create vault with long-term key
-	vault := createTestVaultWithKey(t, fs, stateDir, "test")
+	vault := createTestVaultWithKey(t, fs)

 	// Add a secret
-	secretName := "test/secret"
 	secretValue := []byte("initial-value")
 	expectedValue := make([]byte, len(secretValue))
 	copy(expectedValue, secretValue)

-	addTestSecretToVault(t, vault, secretName, secretValue, false)
+	addTestSecretToVault(t, vault, testSecretPath, secretValue, false)

 	// Check that version directory was created
 	vaultDir, _ := vault.GetDirectory()
@@ -97,32 +129,31 @@ func TestVaultAddSecretCreatesVersion(t *testing.T) {
 	assert.True(t, exists)

 	// Get the secret value
-	retrievedValue, err := vault.GetSecret(secretName)
+	retrievedValue, err := vault.GetSecret(testSecretPath)
 	require.NoError(t, err)
 	assert.Equal(t, expectedValue, retrievedValue)
 }

+//nolint:paralleltest // createTestVaultWithKey uses t.Setenv
 func TestVaultAddSecretMultipleVersions(t *testing.T) {
 	fs := afero.NewMemMapFs()
-	stateDir := "/test/state"

 	// Create vault with long-term key
-	vault := createTestVaultWithKey(t, fs, stateDir, "test")
-
-	secretName := "test/secret"
+	vault := createTestVaultWithKey(t, fs)

 	// Add first version
-	addTestSecretToVault(t, vault, secretName, []byte("version-1"), false)
+	addTestSecretToVault(t, vault, testSecretPath, []byte("version-1"), false)

 	// Try to add again without force - should fail
 	failBuffer := memguard.NewBufferFromBytes([]byte("version-2"))
 	defer failBuffer.Destroy()
-	err := vault.AddSecret(secretName, failBuffer, false)
-	assert.Error(t, err)
+
+	err := vault.AddSecret(testSecretPath, failBuffer, false)
+	require.Error(t, err)
 	assert.Contains(t, err.Error(), "already exists")

 	// Add with force - should create new version
-	addTestSecretToVault(t, vault, secretName, []byte("version-2"), true)
+	addTestSecretToVault(t, vault, testSecretPath, []byte("version-2"), true)

 	// Check that we have two versions
 	vaultDir, _ := vault.GetDirectory()
@@ -132,27 +163,25 @@ func TestVaultAddSecretMultipleVersions(t *testing.T) {
 	assert.Len(t, entries, 2)

 	// Current value should be version-2
-	value, err := vault.GetSecret(secretName)
+	value, err := vault.GetSecret(testSecretPath)
 	require.NoError(t, err)
 	assert.Equal(t, []byte("version-2"), value)
 }

+//nolint:paralleltest // createTestVaultWithKey uses t.Setenv
 func TestVaultGetSecretVersion(t *testing.T) {
 	fs := afero.NewMemMapFs()
-	stateDir := "/test/state"

 	// Create vault with long-term key
-	vault := createTestVaultWithKey(t, fs, stateDir, "test")
-
-	secretName := "test/secret"
+	vault := createTestVaultWithKey(t, fs)

 	// Add multiple versions
-	addTestSecretToVault(t, vault, secretName, []byte("version-1"), false)
+	addTestSecretToVault(t, vault, testSecretPath, []byte("version-1"), false)

 	// Small delay to ensure different version names
 	time.Sleep(10 * time.Millisecond)

-	addTestSecretToVault(t, vault, secretName, []byte("version-2"), true)
+	addTestSecretToVault(t, vault, testSecretPath, []byte("version-2"), true)

 	// Get versions list
 	vaultDir, _ := vault.GetDirectory()
@@ -163,58 +192,62 @@ func TestVaultGetSecretVersion(t *testing.T) {

 	// Get specific version (first one)
 	firstVersion := versions[1] // Last in list is first created
-	value, err := vault.GetSecretVersion(secretName, firstVersion)
+	value, err := vault.GetSecretVersion(testSecretPath, firstVersion)
 	require.NoError(t, err)
 	assert.Equal(t, []byte("version-1"), value)

 	// Get specific version (second one)
 	secondVersion := versions[0] // First in list is most recent
-	value, err = vault.GetSecretVersion(secretName, secondVersion)
+	value, err = vault.GetSecretVersion(testSecretPath, secondVersion)
 	require.NoError(t, err)
 	assert.Equal(t, []byte("version-2"), value)

 	// Get current (empty version)
-	value, err = vault.GetSecretVersion(secretName, "")
+	value, err = vault.GetSecretVersion(testSecretPath, "")
 	require.NoError(t, err)
 	assert.Equal(t, []byte("version-2"), value)
 }

+//nolint:paralleltest // createTestVaultWithKey uses t.Setenv
 func TestVaultVersionTimestamps(t *testing.T) {
 	fs := afero.NewMemMapFs()
-	stateDir := "/test/state"

 	// Create vault with long-term key
-	vault := createTestVaultWithKey(t, fs, stateDir, "test")
+	vault := createTestVaultWithKey(t, fs)

 	// Get long-term key
 	ltIdentity, err := vault.GetOrDeriveLongTermKey()
 	require.NoError(t, err)

-	secretName := "test/secret"
-
 	// Add first version
 	beforeFirst := time.Now()
+
 	v1Buffer := memguard.NewBufferFromBytes([]byte("version-1"))
 	defer v1Buffer.Destroy()
-	err = vault.AddSecret(secretName, v1Buffer, false)
+
+	err = vault.AddSecret(testSecretPath, v1Buffer, false)
 	require.NoError(t, err)
+
 	afterFirst := time.Now()

 	// Get first version metadata
 	vaultDir, _ := vault.GetDirectory()
 	secretDir := vaultDir + "/secrets.d/test%secret"
+
 	versions, err := secret.ListVersions(fs, secretDir)
 	require.NoError(t, err)
 	require.Len(t, versions, 1)

-	firstVersion := secret.NewVersion(vault, secretName, versions[0])
+	firstVersion := secret.NewVersion(vault, testSecretPath, versions[0])
 	err = firstVersion.LoadMetadata(ltIdentity)
 	require.NoError(t, err)

 	// Check first version timestamps
 	assert.NotNil(t, firstVersion.Metadata.CreatedAt)
-	assert.True(t, firstVersion.Metadata.CreatedAt.After(beforeFirst.Add(-time.Second)))
-	assert.True(t, firstVersion.Metadata.CreatedAt.Before(afterFirst.Add(time.Second)))
+	assert.True(t,
+		firstVersion.Metadata.CreatedAt.After(beforeFirst.Add(-time.Second)))
+	assert.True(t,
+		firstVersion.Metadata.CreatedAt.Before(afterFirst.Add(time.Second)))

 	assert.NotNil(t, firstVersion.Metadata.NotBefore)
 	assert.Equal(t, int64(1), firstVersion.Metadata.NotBefore.Unix()) // Epoch + 1
@@ -222,8 +255,11 @@ func TestVaultVersionTimestamps(t *testing.T) {

 	// Add second version
 	time.Sleep(10 * time.Millisecond)
+
 	beforeSecond := time.Now()
-	addTestSecretToVault(t, vault, secretName, []byte("version-2"), true)
+
+	addTestSecretToVault(t, vault, testSecretPath, []byte("version-2"), true)
+
 	afterSecond := time.Now()

 	// Get updated versions
@@ -232,56 +268,59 @@ func TestVaultVersionTimestamps(t *testing.T) {
 	require.Len(t, versions, 2)

 	// Reload first version metadata (should have notAfter now)
-	firstVersion = secret.NewVersion(vault, secretName, versions[1])
+	firstVersion = secret.NewVersion(vault, testSecretPath, versions[1])
 	err = firstVersion.LoadMetadata(ltIdentity)
 	require.NoError(t, err)

 	assert.NotNil(t, firstVersion.Metadata.NotAfter)
-	assert.True(t, firstVersion.Metadata.NotAfter.After(beforeSecond.Add(-time.Second)))
-	assert.True(t, firstVersion.Metadata.NotAfter.Before(afterSecond.Add(time.Second)))
+	assert.True(t,
+		firstVersion.Metadata.NotAfter.After(beforeSecond.Add(-time.Second)))
+	assert.True(t,
+		firstVersion.Metadata.NotAfter.Before(afterSecond.Add(time.Second)))

 	// Check second version timestamps
-	secondVersion := secret.NewVersion(vault, secretName, versions[0])
+	secondVersion := secret.NewVersion(vault, testSecretPath, versions[0])
 	err = secondVersion.LoadMetadata(ltIdentity)
 	require.NoError(t, err)

 	assert.NotNil(t, secondVersion.Metadata.NotBefore)
-	assert.True(t, secondVersion.Metadata.NotBefore.After(beforeSecond.Add(-time.Second)))
-	assert.True(t, secondVersion.Metadata.NotBefore.Before(afterSecond.Add(time.Second)))
+	assert.True(t,
+		secondVersion.Metadata.NotBefore.After(beforeSecond.Add(-time.Second)))
+	assert.True(t,
+		secondVersion.Metadata.NotBefore.Before(afterSecond.Add(time.Second)))
 	assert.Nil(t, secondVersion.Metadata.NotAfter) // Current version
 }

+//nolint:paralleltest // createTestVaultWithKey uses t.Setenv
 func TestVaultGetNonExistentVersion(t *testing.T) {
 	fs := afero.NewMemMapFs()
-	stateDir := "/test/state"

 	// Create vault with long-term key
-	vault := createTestVaultWithKey(t, fs, stateDir, "test")
+	vault := createTestVaultWithKey(t, fs)

 	// Add a secret
-	addTestSecretToVault(t, vault, "test/secret", []byte("value"), false)
+	addTestSecretToVault(t, vault, testSecretPath, []byte("value"), false)

 	// Try to get non-existent version
-	_, err := vault.GetSecretVersion("test/secret", "20991231.999")
-	assert.Error(t, err)
+	_, err := vault.GetSecretVersion(testSecretPath, "20991231.999")
+	require.Error(t, err)
 	assert.Contains(t, err.Error(), "not found")
 }

+//nolint:paralleltest // createTestVaultWithKey uses t.Setenv
 func TestUpdateVersionMetadata(t *testing.T) {
 	fs := afero.NewMemMapFs()
-	stateDir := "/test/state"

 	// Create vault with long-term key
-	vault := createTestVaultWithKey(t, fs, stateDir, "test")
+	vault := createTestVaultWithKey(t, fs)

 	// Get long-term key
 	ltIdentity, err := vault.GetOrDeriveLongTermKey()
 	require.NoError(t, err)

 	// Create a version manually to test updateVersionMetadata
-	secretName := "test/secret"
 	versionName := "20231215.001"
-	version := secret.NewVersion(vault, secretName, versionName)
+	version := secret.NewVersion(vault, testSecretPath, versionName)

 	// Set initial metadata
 	now := time.Now()
@@ -292,6 +331,7 @@ func TestUpdateVersionMetadata(t *testing.T) {
 	// Save version
 	testBuffer := memguard.NewBufferFromBytes([]byte("test-value"))
 	defer testBuffer.Destroy()
+
 	err = version.Save(testBuffer)
 	require.NoError(t, err)

@@ -301,7 +341,7 @@ func TestUpdateVersionMetadata(t *testing.T) {
 	require.NoError(t, err)

 	// Load and verify
-	version2 := secret.NewVersion(vault, secretName, versionName)
+	version2 := secret.NewVersion(vault, testSecretPath, versionName)
 	err = version2.LoadMetadata(ltIdentity)
 	require.NoError(t, err)