diff --git a/cmd/nightfalldlp/main.go b/cmd/nightfalldlp/main.go index 7cb25413..e747e1e8 100644 --- a/cmd/nightfalldlp/main.go +++ b/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") } diff --git a/internal/clients/diffreviewer/circleci/circleci_service.go b/internal/clients/diffreviewer/circleci/circleci_service.go index c28126b9..6fe14f83 100644 --- a/internal/clients/diffreviewer/circleci/circleci_service.go +++ b/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 { diff --git a/internal/clients/diffreviewer/circleci/circleci_service_test.go b/internal/clients/diffreviewer/circleci/circleci_service_test.go index f9648636..f7f61bcf 100644 --- a/internal/clients/diffreviewer/circleci/circleci_service_test.go +++ b/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 { diff --git a/internal/clients/logger/logger.go b/internal/clients/logger/logger.go index 11060351..9995b516 100644 --- a/internal/clients/logger/logger.go +++ b/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) diff --git a/internal/mocks/logger/logger_mock.go b/internal/mocks/logger/logger_mock.go new file mode 100644 index 00000000..dba97016 --- /dev/null +++ b/internal/mocks/logger/logger_mock.go @@ -0,0 +1,81 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: ../logger/logger.go + +// Package logger_mock is a generated GoMock package. +package logger_mock + +import ( + gomock "github.com/golang/mock/gomock" + reflect "reflect" +) + +// Logger is a mock of Logger interface +type Logger struct { + ctrl *gomock.Controller + recorder *LoggerMockRecorder +} + +// LoggerMockRecorder is the mock recorder for Logger +type LoggerMockRecorder struct { + mock *Logger +} + +// NewLogger creates a new mock instance +func NewLogger(ctrl *gomock.Controller) *Logger { + mock := &Logger{ctrl: ctrl} + mock.recorder = &LoggerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *Logger) EXPECT() *LoggerMockRecorder { + return m.recorder +} + +// Debug mocks base method +func (m *Logger) Debug(msg string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Debug", msg) +} + +// Debug indicates an expected call of Debug +func (mr *LoggerMockRecorder) Debug(msg interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Debug", reflect.TypeOf((*Logger)(nil).Debug), msg) +} + +// Info mocks base method +func (m *Logger) Info(msg string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Info", msg) +} + +// Info indicates an expected call of Info +func (mr *LoggerMockRecorder) Info(msg interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*Logger)(nil).Info), msg) +} + +// Warning mocks base method +func (m *Logger) Warning(msg string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Warning", msg) +} + +// Warning indicates an expected call of Warning +func (mr *LoggerMockRecorder) Warning(msg interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Warning", reflect.TypeOf((*Logger)(nil).Warning), msg) +} + +// Error mocks base method +func (m *Logger) Error(msg string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Error", msg) +} + +// Error indicates an expected call of Error +func (mr *LoggerMockRecorder) Error(msg interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*Logger)(nil).Error), msg) +}