NIGH-152 Log Findings to CircleCI UI + Make Github Token Optional (#40)

* make github token optional and log findings to circle

* wip

* fix bug caused by comments on outdated code

* uncomment

* add outdated comment to test

* add info log statement if github token not given

* fix auto-import syntax

Author: alan20854

cmd/nightfalldlp/main.go

@@ -90,10 +90,12 @@ func CreateDiffReviewerClient() (diffreviewer.DiffReviewer, error) {
 		return github.NewAuthenticatedGithubService(githubToken), nil
 	case usingCircleCi():
 		githubToken, ok := os.LookupEnv(githubTokenEnvVar)
-		if !ok {
-			return nil, fmt.Errorf("could not find required %s environment variable", githubTokenEnvVar)
+		if !ok || githubToken == "" {
+			circleService := circleci.NewCircleCiService()
+			circleService.GetLogger().Info("Github Token not found - findings will only be posted to CircleCI UI")
+			return circleService, nil
 		}
-		return circleci.NewCircleCiService(githubToken), nil
+		return circleci.NewCircleCiServiceWithGithubComments(githubToken), nil
 	default:
 		return nil, errors.New("current environment unknown")
 	}

internal/clients/diffreviewer/circleci/circleci_service.go

@@ -39,6 +39,8 @@ const (
 	GithubCommentRightSide = "RIGHT"
 )
 
+var errSensitiveItemsFound = errors.New("potentially sensitive items found")
+
 // Service contains the github client that makes Github api calls
 type Service struct {
 	GithubClient githubintf.GithubClient
@@ -55,7 +57,14 @@ type prDetails struct {
 }
 
 // NewCircleCiService creates a new CircleCi service
-func NewCircleCiService(token string) diffreviewer.DiffReviewer {
+func NewCircleCiService() diffreviewer.DiffReviewer {
+	return &Service{
+		Logger: circlelogger.NewDefaultCircleLogger(),
+	}
+}
+
+// NewCircleCiServiceWithGithubComments creates a new CircleCi service with an authenticated Github client
+func NewCircleCiServiceWithGithubComments(token string) diffreviewer.DiffReviewer {
 	return &Service{
 		GithubClient: gc.NewAuthenticatedClient(token),
 		Logger:       circlelogger.NewDefaultCircleLogger(),
@@ -198,6 +207,10 @@ func (s *Service) WriteComments(comments []*diffreviewer.Comment) error {
 		s.Logger.Info("no sensitive items found")
 		return nil
 	}
+	s.logCommentsToCircle(comments)
+	if s.GithubClient == nil {
+		return errSensitiveItemsFound
+	}
 	if s.PrDetails.PrNumber != nil {
 		existingComments, _, err := s.GithubClient.PullRequestsService().ListComments(
 			context.Background(),
@@ -221,7 +234,6 @@ func (s *Service) WriteComments(comments []*diffreviewer.Comment) error {
 			)
 			if err != nil {
 				s.Logger.Error(fmt.Sprintf("Error writing comment to pull request: %s", err.Error()))
-				s.Logger.Error(*c.Body) // log comment that was not written to circle ui
 			}
 		}
 	} else {
@@ -236,12 +248,22 @@ func (s *Service) WriteComments(comments []*diffreviewer.Comment) error {
 			)
 			if err != nil {
 				s.Logger.Error(fmt.Sprintf("Error writing comment to commit: %s", err.Error()))
-				s.Logger.Error(*c.Body)
 			}
 		}
 	}
 	// returning error to fail circleCI step
-	return errors.New("potentially sensitive items found")
+	return errSensitiveItemsFound
+}
+
+func (s *Service) logCommentsToCircle(comments []*diffreviewer.Comment) {
+	for _, comment := range comments {
+		s.Logger.Error(fmt.Sprintf(
+			"%s at %s on line %d",
+			comment.Body,
+			comment.FilePath,
+			comment.LineNumber,
+		))
+	}
 }
 
 func (s *Service) createGithubPullRequestComments(comments []*diffreviewer.Comment) []*github.PullRequestComment {

internal/clients/diffreviewer/circleci/circleci_service_test.go

@@ -2,22 +2,21 @@ package circleci
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/http/httptest"
 	"os"
 	"path"
 	"testing"
 
-	"github.com/nightfallai/nightfall_code_scanner/internal/mocks/clients/githubpullrequests_mock"
-
 	"github.com/golang/mock/gomock"
 	"github.com/google/go-github/v31/github"
 	"github.com/nightfallai/nightfall_code_scanner/internal/clients/diffreviewer"
 	circlelogger "github.com/nightfallai/nightfall_code_scanner/internal/clients/logger/circle_logger"
 	"github.com/nightfallai/nightfall_code_scanner/internal/mocks/clients/gitdiff_mock"
 	"github.com/nightfallai/nightfall_code_scanner/internal/mocks/clients/githubclient_mock"
+	"github.com/nightfallai/nightfall_code_scanner/internal/mocks/clients/githubpullrequests_mock"
 	"github.com/nightfallai/nightfall_code_scanner/internal/mocks/clients/githubrepositories_mock"
+	logger_mock "github.com/nightfallai/nightfall_code_scanner/internal/mocks/logger"
 	"github.com/nightfallai/nightfall_code_scanner/internal/nightfallconfig"
 	nightfallAPI "github.com/nightfallai/nightfall_go_client/generated"
 	"github.com/stretchr/testify/suite"
@@ -226,15 +225,77 @@ func (c *circleCiTestSuite) TestGetDiff() {
 	c.Equal(expectedFileDiffs, fileDiffs, "invalid fileDiff return value")
 }
 
+func (c *circleCiTestSuite) TestWriteCircleComments() {
+	tp := c.initTestParams()
+	ctrl := gomock.NewController(c.T())
+	defer ctrl.Finish()
+	mockLogger := logger_mock.NewLogger(ctrl)
+	testCircleService := &Service{
+		Logger: mockLogger,
+		PrDetails: prDetails{
+			CommitSha: commitSha,
+			Owner:     testOwner,
+			Repo:      testRepo,
+		},
+	}
+	tp.cs = testCircleService
+
+	testComments, _ := makeTestGithubRepositoryComments(
+		"testComment",
+		"/comments.txt",
+		tp.cs.PrDetails.CommitSha,
+		60,
+	)
+	emptyComments := []*diffreviewer.Comment{}
+
+	tests := []struct {
+		giveComments []*diffreviewer.Comment
+		wantErr      error
+		desc         string
+	}{
+		{
+			giveComments: testComments,
+			wantErr:      errSensitiveItemsFound,
+			desc:         "single batch comments test",
+		},
+		{
+			giveComments: emptyComments,
+			wantErr:      nil,
+			desc:         "no comments test",
+		},
+	}
+
+	for _, tt := range tests {
+		if len(tt.giveComments) == 0 {
+			mockLogger.EXPECT().Info("no sensitive items found")
+		}
+		for _, comment := range tt.giveComments {
+			mockLogger.EXPECT().Error(fmt.Sprintf(
+				"%s at %s on line %d",
+				comment.Body,
+				comment.FilePath,
+				comment.LineNumber,
+			))
+		}
+		err := tp.cs.WriteComments(tt.giveComments)
+		if len(tt.giveComments) == 0 {
+			c.NoError(err, fmt.Sprintf("unexpected error writing comments for %s test", tt.desc))
+		} else {
+			c.EqualError(err, tt.wantErr.Error(), fmt.Sprintf("invalid error writing comments for %s test", tt.desc))
+		}
+	}
+}
+
 func (c *circleCiTestSuite) TestWritePullRequestComments() {
 	tp := c.initTestParams()
 	ctrl := gomock.NewController(c.T())
 	defer ctrl.Finish()
 	mockClient := githubclient_mock.NewGithubClient(tp.ctrl)
 	mockPullRequests := githubpullrequests_mock.NewGithubPullRequests(tp.ctrl)
+	mockLogger := logger_mock.NewLogger(ctrl)
 	testCircleService := &Service{
 		GithubClient: mockClient,
-		Logger:       circlelogger.NewDefaultCircleLogger(),
+		Logger:       mockLogger,
 		PrDetails: prDetails{
 			CommitSha: commitSha,
 			Owner:     testOwner,
@@ -261,7 +322,7 @@ func (c *circleCiTestSuite) TestWritePullRequestComments() {
 		{
 			giveComments:       testComments,
 			giveGithubComments: testGithubComments,
-			wantError:          errors.New("potentially sensitive items found"),
+			wantError:          errSensitiveItemsFound,
 			desc:               "single batch comments test",
 		},
 		{
@@ -281,6 +342,9 @@ func (c *circleCiTestSuite) TestWritePullRequestComments() {
 			*testCircleService.PrDetails.PrNumber,
 			&github.PullRequestListCommentsOptions{},
 		)
+		if len(tt.giveComments) == 0 {
+			mockLogger.EXPECT().Info("no sensitive items found")
+		}
 		for _, gc := range tt.giveGithubComments {
 			mockClient.EXPECT().PullRequestsService().Return(mockPullRequests)
 			mockPullRequests.EXPECT().CreateComment(
@@ -290,6 +354,12 @@ func (c *circleCiTestSuite) TestWritePullRequestComments() {
 				*testCircleService.PrDetails.PrNumber,
 				gc,
 			)
+			mockLogger.EXPECT().Error(fmt.Sprintf(
+				"%s at %s on line %d",
+				gc.GetBody(),
+				gc.GetPath(),
+				gc.GetLine(),
+			))
 		}
 		err := tp.cs.WriteComments(tt.giveComments)
 		if len(tt.giveComments) > 0 {
@@ -336,9 +406,10 @@ func (c *circleCiTestSuite) TestWriteRepositoryComments() {
 	defer ctrl.Finish()
 	mockClient := githubclient_mock.NewGithubClient(tp.ctrl)
 	mockRepositories := githubrepositories_mock.NewGithubRepositories(tp.ctrl)
+	mockLogger := logger_mock.NewLogger(ctrl)
 	testCircleService := &Service{
 		GithubClient: mockClient,
-		Logger:       circlelogger.NewDefaultCircleLogger(),
+		Logger:       mockLogger,
 		PrDetails: prDetails{
 			CommitSha: commitSha,
 			Owner:     testOwner,
@@ -364,7 +435,7 @@ func (c *circleCiTestSuite) TestWriteRepositoryComments() {
 		{
 			giveComments:       testComments,
 			giveGithubComments: testGithubComments,
-			wantError:          errors.New("potentially sensitive items found"),
+			wantError:          errSensitiveItemsFound,
 			desc:               "single batch comments test",
 		},
 		{
@@ -376,7 +447,10 @@ func (c *circleCiTestSuite) TestWriteRepositoryComments() {
 	}
 
 	for _, tt := range tests {
-		for _, gc := range tt.giveGithubComments {
+		if len(tt.giveGithubComments) == 0 {
+			mockLogger.EXPECT().Info("no sensitive items found")
+		}
+		for index, gc := range tt.giveGithubComments {
 			mockClient.EXPECT().RepositoriesService().Return(mockRepositories)
 			mockRepositories.EXPECT().CreateComment(
 				context.Background(),
@@ -385,6 +459,12 @@ func (c *circleCiTestSuite) TestWriteRepositoryComments() {
 				testCircleService.PrDetails.CommitSha,
 				gc,
 			)
+			mockLogger.EXPECT().Error(fmt.Sprintf(
+				"%s at %s on line %d",
+				gc.GetBody(),
+				gc.GetPath(),
+				tt.giveComments[index].LineNumber,
+			))
 		}
 		err := tp.cs.WriteComments(tt.giveComments)
 		if len(tt.giveComments) > 0 {

internal/clients/logger/logger.go

@@ -1,5 +1,7 @@
 package logger
 
+//go:generate go run github.com/golang/mock/mockgen -destination=../../mocks/logger/logger_mock.go -source=../logger/logger.go -package=logger_mock -mock_names=Logger=Logger
+
 // Logger is the interface for logging content
 type Logger interface {
 	Debug(msg string)