Include gpg's stderr in the error when gpg fails (closes #146)

When gpg failed while decrypting, encrypting or looking up a key
fingerprint, the error gave only gpg's exit status, so the user could
not tell why. cmd.Output keeps gpg's stderr in the *exec.ExitError it
returns; the three errors now end with that stderr, trimmed. When gpg
wrote nothing to stderr, the error is left as it was.

gpg's stderr can run to more than one line, and the error then does too.

The regression test gives gpg data that is not an OpenPGP message
instead of one encrypted to a missing key: gpg reports both the same
way, and this case starts no gpg-agent the test would have to stop.

Model: opus-5-5

README.md

@@ -204,12 +204,6 @@ Moves or renames a secret within the current vault.
 Lists all versions of a secret showing creation time, status, and validity
 period.
 
-`STATUS` is `current` for the secret's current version, `expired` for another
-version whose `NOT_AFTER` has passed, and `inactive` for any other version.
-Adding a version sets `NOT_AFTER` on the version that was current;
-`secret version promote` sets no times. A version that is not current therefore
-shows `inactive` when no version was added while it was current.
-
 #### `secret version promote <secret-name> <version>`
 
 Promotes a specific version to current by rewriting the secret's `current` file

internal/cli/unlockers.go

@@ -314,8 +314,7 @@ func (cli *Instance) UnlockersList(jsonOutput bool) error {
 		return err
 	}
 
-	// Not nil, so that a vault with no unlockers prints [] in JSON, not null
-	unlockers := make([]UnlockerInfo, 0, len(unlockerMetadata))
+	var unlockers []UnlockerInfo
 
 	for _, unlockerID := range slices.Sorted(maps.Keys(unlockerMetadata)) {
 		metadata := unlockerMetadata[unlockerID]

internal/cli/unlockers_list_test.go

@@ -1,14 +1,13 @@
 // Unlocker List Tests
 //
-// Tests for `secret unlocker list`:
+// Tests for `secret unlocker list` behavior when an unlocker's metadata
+// cannot be read or used:
 //
 //   - TestUnlockersListToleratesCorruptMetadata: one unlocker's corrupt
 //     metadata does not stop the others from being listed.
 //   - TestUnlockersListSkipsUnreadableMetadata: an unlocker whose metadata
 //     file cannot be checked for or read is left out, and the other is
 //     still listed.
-//   - TestUnlockersListEmptyJSON: a vault with no unlockers lists them as
-//     an empty JSON list.
 
 //nolint:testpackage // white-box test of unexported internals
 package cli
@@ -322,36 +321,3 @@ func TestUnlockersListSkipsUnreadableMetadata(t *testing.T) {
 		})
 	}
 }
-
-// TestUnlockersListEmptyJSON is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/148: listing the unlockers of a
-// vault that has none, as after its last unlocker is removed, printed
-// "unlockers": null in JSON form instead of an empty list.
-func TestUnlockersListEmptyJSON(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewMemMapFs()
-	vaultDir := filepath.Join(listTestStateDir, "vaults.d", listTestVaultName)
-
-	require.NoError(t, afero.WriteFile(
-		fs, filepath.Join(listTestStateDir, "currentvault"),
-		[]byte(listTestVaultName), listTestFilePerm,
-	))
-	require.NoError(t, fs.MkdirAll(
-		filepath.Join(vaultDir, listTestUnlockersDirName), listTestDirPerm,
-	))
-
-	var buf bytes.Buffer
-
-	cmd := &cobra.Command{}
-	cmd.SetOut(&buf)
-
-	instance := &Instance{fs: fs, stateDir: listTestStateDir, cmd: cmd}
-	require.NoError(t, instance.UnlockersList(true))
-
-	var decoded map[string]any
-
-	require.NoError(t, json.Unmarshal(buf.Bytes(), &decoded))
-	assert.Equal(t, []any{}, decoded["unlockers"],
-		"no unlockers must be printed as [], not %s", buf.String())
-}

internal/cli/vault_list_test.go

@@ -1,42 +0,0 @@
-package cli_test
-
-import (
-	"bytes"
-	"encoding/json"
-	"path/filepath"
-	"testing"
-
-	"github.com/spf13/afero"
-	"github.com/spf13/cobra"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/secret/internal/cli"
-	"sneak.berlin/go/secret/internal/secret"
-)
-
-// TestVaultListEmptyJSON is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/148: listing the vaults when
-// vaults.d holds none printed "vaults": null in JSON form instead of an
-// empty list.
-func TestVaultListEmptyJSON(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewMemMapFs()
-	require.NoError(t, fs.MkdirAll(
-		filepath.Join(testStateDir, "vaults.d"), secret.DirPerms,
-	))
-
-	var buf bytes.Buffer
-
-	cmd := &cobra.Command{}
-	cmd.SetOut(&buf)
-
-	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
-	require.NoError(t, c.ListVaults(cmd, true))
-
-	var decoded map[string]any
-
-	require.NoError(t, json.Unmarshal(buf.Bytes(), &decoded))
-	assert.Equal(t, []any{}, decoded["vaults"],
-		"no vaults must be printed as [], not %s", buf.String())
-}

internal/cli/version.go

@@ -229,13 +229,9 @@ func printVersionRow(
 	}
 
 	// Determine status
-	status := "inactive"
-
-	switch {
-	case version == currentVersion:
+	status := "expired"
+	if version == currentVersion {
 		status = "current"
-	case sv.Metadata.NotAfter != nil && sv.Metadata.NotAfter.Before(time.Now()):
-		status = "expired"
 	}
 
 	// Format timestamps

internal/cli/version_test.go

@@ -5,8 +5,6 @@
 // - TestListVersionsCommand: Tests `secret version list` command output
 // - TestListVersionsNonExistentSecret: Tests error handling for missing secrets
 // - TestPromoteVersionCommand: Tests `secret version promote` command
-// - TestListVersionsStatusAfterPromote: Tests each version's status after
-//   promoting an older version
 // - TestPromoteNonExistentVersion: Tests error handling for invalid promotion
 // - TestPromoteHelpSaysCurrentFileIsRewritten: Tests `secret version promote
 //   --help` text
@@ -246,66 +244,6 @@ func TestPromoteVersionCommand(t *testing.T) {
 	assert.Equal(t, []byte("version-1"), promoted.Bytes())
 }
 
-func TestListVersionsStatusAfterPromote(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewMemMapFs()
-	cli := NewCLIInstanceWithStateDir(fs, testStateDir)
-	cli.Mnemonic = testMnemonicBuffer(t)
-
-	setupTestVault(t, fs)
-
-	vlt, err := vault.GetCurrentVault(fs, testStateDir)
-	require.NoError(t, err)
-
-	vlt.Mnemonic = cli.Mnemonic
-
-	addTestSecret(t, vlt, []byte("version-1"), false)
-	addTestSecret(t, vlt, []byte("version-2"), true)
-	addTestSecret(t, vlt, []byte("version-3"), true)
-
-	vaultDir, err := vlt.GetDirectory()
-	require.NoError(t, err)
-
-	// Newest first
-	versions, err := secret.ListVersions(fs,
-		filepath.Join(vaultDir, "secrets.d", "test%secret"))
-	require.NoError(t, err)
-	require.Len(t, versions, 3)
-
-	newest, middle, oldest := versions[0], versions[1], versions[2]
-
-	cmd := newRootCmd()
-
-	var buf bytes.Buffer
-	cmd.SetOut(&buf)
-	cmd.SetErr(&buf)
-
-	err = cli.PromoteVersion(cmd, "test/secret", oldest)
-	require.NoError(t, err)
-
-	buf.Reset()
-
-	err = cli.ListVersions(cmd, "test/secret")
-	require.NoError(t, err)
-
-	// STATUS is the fourth field of a row, since CREATED is a date and a time
-	statuses := map[string]string{}
-
-	for line := range strings.SplitSeq(buf.String(), "\n") {
-		fields := strings.Fields(line)
-		if len(fields) > 3 {
-			statuses[fields[0]] = fields[3]
-		}
-	}
-
-	assert.Equal(t, "current", statuses[oldest])
-	// Its NOT_AFTER was set when the newest version was added
-	assert.Equal(t, "expired", statuses[middle])
-	// It has no NOT_AFTER
-	assert.Equal(t, "inactive", statuses[newest])
-}
-
 func TestPromoteNonExistentVersion(t *testing.T) {
 	t.Parallel()
 

internal/secret/version.go

@@ -32,8 +32,7 @@ type VersionMetadata struct {
 	ID        string     `json:"id"`                  // ULID
 	CreatedAt *time.Time `json:"createdAt,omitempty"` // When version was created
 	NotBefore *time.Time `json:"notBefore,omitempty"` // When this version becomes active
-	// NotAfter is set when another version is added while this one is current
-	NotAfter *time.Time `json:"notAfter,omitempty"`
+	NotAfter  *time.Time `json:"notAfter,omitempty"`  // Expiry (nil = current)
 }
 
 // Version represents a version of a secret

internal/vault/management.go

@@ -140,9 +140,8 @@ func ListVaults(fs afero.Fs, stateDir string) ([]string, error) {
 		return nil, fmt.Errorf("failed to read vaults directory: %w", err)
 	}
 
-	// Extract vault names. Not nil, so that no vaults prints [] in JSON,
-	// not null.
-	vaults := make([]string, 0, len(entries))
+	// Extract vault names
+	var vaults []string
 
 	for _, entry := range entries {
 		if entry.IsDir() {