From 8d5ed1ba4c6cd9ffc0f468999dc763a6ab3e9614 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 14 Oct 2026 22:15:16 +0000 Subject: [PATCH] Print an empty unlocker or vault list as [] in JSON (closes #148) `secret unlocker list --json` printed "unlockers": null for a vault with no unlockers, and `secret vault list --json` printed "vaults": null when vaults.d held no vaults. Both lists were nil slices, which encoding/json writes as null. Each is now made as an empty slice before it is filled, so an empty list prints as []. vault.ListVaults therefore returns an empty slice instead of nil for an empty vaults.d; its other callers only range over the result or check its length. Model: opus-5-5 --- internal/cli/unlockers.go | 3 ++- internal/cli/unlockers_list_test.go | 38 ++++++++++++++++++++++++-- internal/cli/vault_list_test.go | 42 +++++++++++++++++++++++++++++ internal/vault/management.go | 5 ++-- 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 internal/cli/vault_list_test.go diff --git a/internal/cli/unlockers.go b/internal/cli/unlockers.go index 87b3d7e..2e1e3de 100644 --- a/internal/cli/unlockers.go +++ b/internal/cli/unlockers.go @@ -314,7 +314,8 @@ func (cli *Instance) UnlockersList(jsonOutput bool) error { return err } - var unlockers []UnlockerInfo + // Not nil, so that a vault with no unlockers prints [] in JSON, not null + unlockers := make([]UnlockerInfo, 0, len(unlockerMetadata)) for _, unlockerID := range slices.Sorted(maps.Keys(unlockerMetadata)) { metadata := unlockerMetadata[unlockerID] diff --git a/internal/cli/unlockers_list_test.go b/internal/cli/unlockers_list_test.go index 0d72990..e8f3f2a 100644 --- a/internal/cli/unlockers_list_test.go +++ b/internal/cli/unlockers_list_test.go @@ -1,13 +1,14 @@ // Unlocker List Tests // -// Tests for `secret unlocker list` behavior when an unlocker's metadata -// cannot be read or used: +// Tests for `secret unlocker list`: // // - 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 @@ -321,3 +322,36 @@ 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()) +} diff --git a/internal/cli/vault_list_test.go b/internal/cli/vault_list_test.go new file mode 100644 index 0000000..8a216dd --- /dev/null +++ b/internal/cli/vault_list_test.go @@ -0,0 +1,42 @@ +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()) +} diff --git a/internal/vault/management.go b/internal/vault/management.go index f5f81f7..c3ed373 100644 --- a/internal/vault/management.go +++ b/internal/vault/management.go @@ -140,8 +140,9 @@ func ListVaults(fs afero.Fs, stateDir string) ([]string, error) { return nil, fmt.Errorf("failed to read vaults directory: %w", err) } - // Extract vault names - var vaults []string + // Extract vault names. Not nil, so that no vaults prints [] in JSON, + // not null. + vaults := make([]string, 0, len(entries)) for _, entry := range entries { if entry.IsDir() { -- 2.54.0