From b3bb53efaa5a46a7140e3c1705dde36c21cc4db1 Mon Sep 17 00:00:00 2001
From: Jake Coffman <jakecoffman@github.com>
Date: Thu, 7 Sep 2023 16:11:50 -0500
Subject: [PATCH] fix updater confused by blank commit (#176)

---
 cmd/dependabot/internal/cmd/update.go |  2 +-
 internal/infra/run.go                 |  4 ++--
 internal/infra/updater.go             | 13 +++++++++----
 internal/infra/updater_test.go        | 21 +++++++++++++++++++++
 internal/model/job.go                 |  2 +-
 internal/server/api.go                |  2 +-
 6 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/cmd/dependabot/internal/cmd/update.go b/cmd/dependabot/internal/cmd/update.go
index b4f2fe6..745e549 100644
--- a/cmd/dependabot/internal/cmd/update.go
+++ b/cmd/dependabot/internal/cmd/update.go
@@ -198,7 +198,7 @@ func readArguments(cmd *cobra.Command) (*model.Input, error) {
 				Provider:    provider,
 				Repo:        repo,
 				Directory:   directory,
-				Commit:      &commit,
+				Commit:      commit,
 				Branch:      nil,
 				Hostname:    nil,
 				APIEndpoint: nil,
diff --git a/internal/infra/run.go b/internal/infra/run.go
index 902094f..47594ec 100644
--- a/internal/infra/run.go
+++ b/internal/infra/run.go
@@ -69,7 +69,7 @@ func (p *RunParams) Validate() error {
 	if p.Job == nil {
 		return fmt.Errorf("job is required")
 	}
-	if p.Job.Source.Commit != nil && *p.Job.Source.Commit != "" && !gitShaRegex.MatchString(*p.Job.Source.Commit) {
+	if p.Job.Source.Commit != "" && !gitShaRegex.MatchString(p.Job.Source.Commit) {
 		return fmt.Errorf("commit must be a SHA, or not provided")
 	}
 	return nil
@@ -137,7 +137,7 @@ func Run(params RunParams) error {
 }
 
 func generateOutput(params RunParams, api *server.API, outFile *os.File) ([]byte, error) {
-	if params.Job.Source.Commit == nil {
+	if params.Job.Source.Commit == "" {
 		// store the SHA we worked with for reproducible tests
 		params.Job.Source.Commit = api.Actual.Input.Job.Source.Commit
 	}
diff --git a/internal/infra/updater.go b/internal/infra/updater.go
index 5e330be..477a8bd 100644
--- a/internal/infra/updater.go
+++ b/internal/infra/updater.go
@@ -109,11 +109,11 @@ func putUpdaterInputs(ctx context.Context, cli *client.Client, cert, id string,
 		return fmt.Errorf("failed to copy cert to container: %w", err)
 	}
 
-	data, err := json.Marshal(FileFetcherJobFile{Job: job})
+	data, err := JobFile{Job: job}.ToJSON()
 	if err != nil {
 		return fmt.Errorf("failed to marshal job file: %w", err)
 	}
-	if t, err := tarball(guestInputDir, string(data)); err != nil {
+	if t, err := tarball(guestInputDir, data); err != nil {
 		return fmt.Errorf("failed create input tarball: %w", err)
 	} else if err = cli.CopyToContainer(ctx, id, "/", t, opt); err != nil {
 		return fmt.Errorf("failed to copy input to container: %w", err)
@@ -278,11 +278,16 @@ func (u *Updater) Close() error {
 	})
 }
 
-// FileFetcherJobFile  is the payload passed to file updater containers.
-type FileFetcherJobFile struct {
+// JobFile  is the payload passed to file updater containers.
+type JobFile struct {
 	Job *model.Job `json:"job"`
 }
 
+func (j JobFile) ToJSON() (string, error) {
+	data, err := json.Marshal(j)
+	return string(data), err
+}
+
 func tarball(name, contents string) (*bytes.Buffer, error) {
 	var buf bytes.Buffer
 	t := tar.NewWriter(&buf)
diff --git a/internal/infra/updater_test.go b/internal/infra/updater_test.go
index f70a7f1..da037ef 100644
--- a/internal/infra/updater_test.go
+++ b/internal/infra/updater_test.go
@@ -1,8 +1,10 @@
 package infra
 
 import (
+	"github.com/dependabot/cli/internal/model"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -48,3 +50,22 @@ func Test_mountOptions(t *testing.T) {
 		}
 	}
 }
+
+func TestJobFile_ToJSON(t *testing.T) {
+	t.Run("empty commit doesn't pass in empty string", func(t *testing.T) {
+		job := JobFile{
+			Job: &model.Job{
+				Source: model.Source{
+					Commit: "",
+				},
+			},
+		}
+		json, err := job.ToJSON()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if strings.Contains(json, `"commit"`) {
+			t.Errorf("expected JSON to not contain commit: %v", json)
+		}
+	})
+}
diff --git a/internal/model/job.go b/internal/model/job.go
index aaedf9b..ce4c06a 100644
--- a/internal/model/job.go
+++ b/internal/model/job.go
@@ -57,7 +57,7 @@ type Source struct {
 	Repo      string  `json:"repo" yaml:"repo,omitempty"`
 	Directory string  `json:"directory" yaml:"directory,omitempty"`
 	Branch    *string `json:"branch" yaml:"branch,omitempty"`
-	Commit    *string `json:"commit" yaml:"commit,omitempty"`
+	Commit    string  `json:"commit,omitempty" yaml:"commit,omitempty"`
 
 	Hostname    *string `json:"hostname" yaml:"hostname,omitempty"`         // Must be provided if APIEndpoint is
 	APIEndpoint *string `json:"api-endpoint" yaml:"api-endpoint,omitempty"` // Must be provided if Hostname is
diff --git a/internal/server/api.go b/internal/server/api.go
index b5a694c..0c33394 100644
--- a/internal/server/api.go
+++ b/internal/server/api.go
@@ -198,7 +198,7 @@ func (a *API) pushResult(kind string, actual *model.UpdateWrapper) error {
 
 	if msg, ok := actual.Data.(model.MarkAsProcessed); ok {
 		// record the commit SHA so the test is reproducible
-		a.Actual.Input.Job.Source.Commit = &msg.BaseCommitSha
+		a.Actual.Input.Job.Source.Commit = msg.BaseCommitSha
 	}
 
 	return nil