chore(local): make sg handle empty secret file gracefully (#63614)

Previously, `sg` would trip on an empty `sg.secrets.json`. It now treats
it the same way as if the file wasn't there at all. Improved usage text
as I was there already.

## Test plan

CI + added a unit test + local test.

<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->

dev/sg/internal/secrets/store.go

@@ -75,7 +75,15 @@ func LoadFromFile(filepath string) (*Store, error) {
 	}
 	defer f.Close()
 	dec := json.NewDecoder(f)
-	return s, dec.Decode(&s.m)
+	if err := dec.Decode(&s.m); err != nil {
+		// Ignore EOF which is returned when the file is empty, we just pretend the file isn't there.
+		// Note that invalid JSON might still return "unexpected EOF" and
+		// we let that one get through.
+		if !errors.Is(err, io.EOF) {
+			return nil, err
+		}
+	}
+	return s, nil
 }
 
 // Write serializes the store content in the given writer.

dev/sg/internal/secrets/store_test.go

@@ -18,24 +18,64 @@ func TestSecrets(t *testing.T) {
 		store := newStore("")
 		err := store.Put("foo", data)
 		if err != nil {
-			t.Fatalf("want no error, got %v", err)
+			t.Fatalf("want no error, got %q", err)
 		}
 
 		want := data
 		got := mySecrets{}
 		err = store.Get("foo", &got)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatalf("want no error when getting secret, but got: %q", err)
 		}
 		if diff := cmp.Diff(want, got); diff != "" {
 			t.Fatalf("wrong secret data. (-want +got):\n%s", diff)
 		}
 	})
 
+	t.Run("LoadFile returns an error on invalid JSON", func(t *testing.T) {
+		f, err := os.CreateTemp(os.TempDir(), "secrets*.json")
+		if err != nil {
+			t.Fatalf("couldn't create temp secret file: %q", err)
+		}
+		if _, err := f.WriteString(`{"foo":1`); err != nil {
+			t.Fatalf("couldn't write in temp secret file: %q", err)
+		}
+		f.Close()
+		filepath := f.Name()
+		t.Cleanup(func() {
+			_ = os.Remove(filepath)
+		})
+
+		_, err = LoadFromFile(filepath)
+		if err == nil {
+			t.Fatal("want an error but got none")
+		}
+	})
+
+	t.Run("LoadFile doesn't fail when file is empty", func(t *testing.T) {
+		f, err := os.CreateTemp(os.TempDir(), "secrets*.json")
+		if err != nil {
+			t.Fatalf("couldn't create temp secret file: %q", err)
+		}
+		f.Close()
+		filepath := f.Name()
+		t.Cleanup(func() {
+			_ = os.Remove(filepath)
+		})
+
+		got, err := LoadFromFile(filepath)
+		if err != nil {
+			t.Fatalf("want no error when loading an empty file, but got %q instead", err)
+		}
+		if got == nil {
+			t.Fatal("want store to not be nil")
+		}
+	})
+
 	t.Run("SaveFile and LoadFile", func(t *testing.T) {
 		f, err := os.CreateTemp(os.TempDir(), "secrets*.json")
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatalf("couldn't create temp secret file: %q", err)
 		}
 		f.Close()
 		filepath := f.Name()
@@ -47,19 +87,21 @@ func TestSecrets(t *testing.T) {
 		// Assign a secret and save it
 		s, err := LoadFromFile(filepath)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatalf("couldn't load temp secret file: %q", err)
 		}
 		data := map[string]any{"key": "val"}
-		s.Put("foo", data)
+		if err := s.Put("foo", data); err != nil {
+			t.Fatalf("want no error when putting secret, got %q", err)
+		}
 		err = s.SaveFile()
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatalf("failed to save secrets: %q", err)
 		}
 
 		// Fetch it back and compare
 		got, err := LoadFromFile(filepath)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatalf("couldn't load temp secret file: %q", err)
 		}
 		if diff := cmp.Diff(s.m, got.m); diff != "" {
 			t.Fatalf("(-want +got):\n%s", diff)

dev/sg/sg_secret.go

@@ -20,20 +20,18 @@ var (
 	secretCommand = &cli.Command{
 		Name:  "secret",
 		Usage: "Manipulate secrets stored in memory and in file",
-		UsageText: `
-# List all secrets stored in your local configuration.
+		UsageText: `# List all secrets stored in your local configuration.
 sg secret list
 
-# Remove the secrets associated with buildkite (sg ci build) - supports autocompletion for
-# ease of use
+# Remove the secrets associated with buildkite (sg ci build) - (supports bash autocompletion).
 sg secret reset buildkite
 `,
 		Category: category.Env,
 		Subcommands: []*cli.Command{
 			{
 				Name:         "reset",
-				ArgsUsage:    "<...key>",
-				Usage:        "Remove a specific secret from secrets file",
+				ArgsUsage:    "key1 key2 ...",
+				Usage:        "Remove a individual secret(s) from secrets file (see 'list' for getting the keys)",
 				Action:       resetSecretExec,
 				BashComplete: completions.CompleteArgs(bashCompleteSecrets),
 			},