Merge ce27451 into 601a9bd

acceptance_test.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"strings"
+	"talisman/prompt"
 	"testing"
 
 	"talisman/git_testing"
@@ -162,6 +163,7 @@ func TestShouldExitZeroWhenNonSecretIsCommittedButFileContainsSecretPreviously(t
 
 		git.AppendFileContent("sample.txt", "some text \n")
 		git.Add("*")
+
 		assert.Equal(t, 0, runTalismanWithOptions(git, _options), "Expected run() to return 1 as given patterns are found")
 	})
 }
@@ -293,7 +295,9 @@ func runTalismanWithOptions(git *git_testing.GitTesting, _options options) int {
 	wd, _ := os.Getwd()
 	os.Chdir(git.GetRoot())
 	defer func() { os.Chdir(wd) }()
-	return run(mockStdIn(git.EarliestCommit(), git.LatestCommit()), _options)
+	prompter := prompt.NewPrompt()
+	promptContext := prompt.NewPromptContext(false, prompter)
+	return run(mockStdIn(git.EarliestCommit(), git.LatestCommit()), _options, promptContext)
 }
 
 type Operation func(dirName string)

detector/detection_results.go

@@ -2,39 +2,43 @@ package detector
 
 import (
 	"fmt"
+	"github.com/spf13/afero"
+	"gopkg.in/yaml.v2"
+	"log"
 	"os"
 	"strings"
 	"talisman/gitrepo"
+	"talisman/prompt"
 	"talisman/utility"
 
 	"github.com/olekukonko/tablewriter"
-	"gopkg.in/yaml.v2"
 )
 
 type Details struct {
-	Category string `json:"type"`
-	Message string `json:"message"`
-	Commits []string `json:"commits"`
+	Category string   `json:"type"`
+	Message  string   `json:"message"`
+	Commits  []string `json:"commits"`
 }
 
 type ResultsDetails struct {
-	Filename gitrepo.FilePath `json:"filename"`
-	FailureList []Details      `json:"failure_list"`
-	WarningList []Details      `json:"warning_list"`
-	IgnoreList []Details 	   `json:"ignore_list"`
+	Filename    gitrepo.FilePath `json:"filename"`
+	FailureList []Details        `json:"failure_list"`
+	WarningList []Details        `json:"warning_list"`
+	IgnoreList  []Details        `json:"ignore_list"`
 }
 
-type FailureTypes struct  {
+type FailureTypes struct {
 	Filecontent int `json:"filecontent"`
-	Filesize int `json:"filesize"`
-	Filename int `json:"filename"`
-	Warnings int `json:"warnings"`
-	Ignores int `json:"ignores"`
+	Filesize    int `json:"filesize"`
+	Filename    int `json:"filename"`
+	Warnings    int `json:"warnings"`
+	Ignores     int `json:"ignores"`
 }
 
 type ResultsSummary struct {
 	Types FailureTypes `json:"types"`
 }
+
 //
 //
 //type FailureData struct {
@@ -46,7 +50,7 @@ type ResultsSummary struct {
 //Currently, it keeps track of failures and ignored files.
 //The results are grouped by FilePath for easy reporting of all detected problems with individual files.
 type DetectionResults struct {
-	Summary ResultsSummary `json:"summary"`
+	Summary ResultsSummary   `json:"summary"`
 	Results []ResultsDetails `json:"results"`
 }
 
@@ -93,19 +97,18 @@ func (r *DetectionResults) getResultDetailsForFilePath(fileName gitrepo.FilePath
 		if resultDetail.Filename == fileName {
 			return &resultDetail
 		}
- 	}
+	}
 	//resultDetail := ResultsDetails{fileName, make([]Details, 0), make([]Details, 0), make([]Details, 0)}
 	//r.Results = append(r.Results, resultDetail)
 	return nil
 }
 
 //NewDetectionResults is a new DetectionResults struct. It represents the pre-run state of a Detection run.
 func NewDetectionResults() *DetectionResults {
-	result := DetectionResults{ResultsSummary{FailureTypes{0,0,0, 0, 0}},make([]ResultsDetails, 0)}
+	result := DetectionResults{ResultsSummary{FailureTypes{0, 0, 0, 0, 0}}, make([]ResultsDetails, 0)}
 	return &result
 }
 
-
 //Fail is used to mark the supplied FilePath as failing a detection for a supplied reason.
 //Detectors are encouraged to provide context sensitive messages so that fixing the errors is made simple for the end user
 //Fail may be called multiple times for each FilePath and the calls accumulate the provided reasons
@@ -191,7 +194,6 @@ func (r *DetectionResults) Ignore(filePath gitrepo.FilePath, category string) {
 	r.Summary.Types.Ignores++
 }
 
-
 func createNewResultForFile(category string, message string, commits []string, filePath gitrepo.FilePath) ResultsDetails {
 	failureDetails := Details{category, message, commits}
 	resultDetails := ResultsDetails{filePath, make([]Details, 0), make([]Details, 0), make([]Details, 0)}
@@ -259,7 +261,6 @@ func (r *DetectionResults) ReportWarnings() string {
 		}
 	}
 
-
 	filePathsForWarnings = utility.UniqueItems(filePathsForWarnings)
 	if r.Summary.Types.Warnings > 0 {
 		fmt.Printf("\n\x1b[1m\x1b[31mTalisman Warnings:\x1b[0m\x1b[0m\n")
@@ -272,7 +273,7 @@ func (r *DetectionResults) ReportWarnings() string {
 }
 
 //Report returns a string documenting the various failures and ignored files for the current run
-func (r *DetectionResults) Report() string {
+func (r *DetectionResults) Report(fs afero.Fs, ignoreFile string, promptContext prompt.PromptContext) string {
 	var result string
 	var filePathsForIgnoresAndFailures []string
 	var data [][]string
@@ -295,24 +296,85 @@ func (r *DetectionResults) Report() string {
 		fmt.Printf("\n\x1b[1m\x1b[31mTalisman Report:\x1b[0m\x1b[0m\n")
 		table.AppendBulk(data)
 		table.Render()
-		result = result + fmt.Sprintf("\n\x1b[33mIf you are absolutely sure that you want to ignore the above files from talisman detectors, consider pasting the following format in .talismanrc file in the project root\x1b[0m\n")
-		result = result + r.suggestTalismanRC(filePathsForIgnoresAndFailures)
-		result = result + fmt.Sprintf("\n\n")
+		r.suggestTalismanRC(fs, ignoreFile, filePathsForIgnoresAndFailures, promptContext)
 	}
 	return result
 }
 
-func (r *DetectionResults) suggestTalismanRC(filePaths []string) string {
-	var fileIgnoreConfigs []FileIgnoreConfig
+func (r *DetectionResults) suggestTalismanRC(fs afero.Fs, ignoreFile string, filePaths []string, promptContext prompt.PromptContext) {
+	var entriesToAdd []FileIgnoreConfig
+
 	for _, filePath := range filePaths {
 		currentChecksum := utility.CollectiveSHA256Hash([]string{filePath})
 		fileIgnoreConfig := FileIgnoreConfig{filePath, currentChecksum, []string{}}
-		fileIgnoreConfigs = append(fileIgnoreConfigs, fileIgnoreConfig)
+		entriesToAdd = append(entriesToAdd, fileIgnoreConfig)
+	}
+
+	if promptContext.Interactive {
+		confirmedEntries := getUserConfirmation(entriesToAdd, promptContext)
+		addToTalismanIgnoreFile(confirmedEntries, fs, ignoreFile)
+	} else {
+		printTalismanIgnoreSuggestion(entriesToAdd)
+		return
+	}
+
+}
+
+func getUserConfirmation(configs []FileIgnoreConfig, promptContext prompt.PromptContext) []FileIgnoreConfig {
+	confirmed := []FileIgnoreConfig{}
+	for _, config := range configs {
+		if confirm(config, promptContext) {
+			confirmed = append(confirmed, config)
+		}
+	}
+	return confirmed
+}
+
+func printTalismanIgnoreSuggestion(entriesToAdd []FileIgnoreConfig) {
+	talismanRcIgnoreConfig := TalismanRCIgnore{FileIgnoreConfig: entriesToAdd}
+	ignoreEntries, _ := yaml.Marshal(&talismanRcIgnoreConfig)
+	suggestString := fmt.Sprintf("\n\x1b[33mIf you are absolutely sure that you want to ignore the " +
+		"above files from talisman detectors, consider pasting the following format in .talismanrc file" +
+		" in the project root\x1b[0m\n")
+	fmt.Println(suggestString)
+	fmt.Println(string(ignoreEntries))
+}
+
+func addToTalismanIgnoreFile(entriesToAdd []FileIgnoreConfig, fs afero.Fs, ignoreFile string) {
+
+	if len(entriesToAdd) > 0 {
+		talismanRcIgnoreConfig := TalismanRCIgnore{FileIgnoreConfig: entriesToAdd}
+		ignoreEntries, _ := yaml.Marshal(&talismanRcIgnoreConfig)
+		file, err := fs.OpenFile(ignoreFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+		if err != nil {
+			log.Printf("error opening %s: %s", ignoreFile, err)
+		}
+		defer func() {
+			err := file.Close()
+			if err != nil {
+				log.Printf("error closing %s: %s", ignoreFile, err)
+			}
+
+		}()
+
+		_, err = file.WriteString(string(ignoreEntries))
+		if err != nil {
+			log.Printf("error writing to %s: %s", ignoreFile, err)
+		}
+	}
+}
+
+func confirm(config FileIgnoreConfig, promptContext prompt.PromptContext) bool {
+	bytes, err := yaml.Marshal(&config)
+	if err != nil {
+		log.Printf("error marshalling file ignore config: %s", err)
 	}
 
-	talismanRcIgnoreConfig := TalismanRCIgnore{FileIgnoreConfig: fileIgnoreConfigs}
-	m, _ := yaml.Marshal(&talismanRcIgnoreConfig)
-	return string(m)
+	fmt.Println(string(bytes))
+
+	confirmationString := "Do you want to add this entry in talismanrc ?"
+
+	return promptContext.Prompt.Confirm(confirmationString)
 }
 
 //ReportFileFailures adds a string to table documenting the various failures detected on the supplied FilePath by all detectors in the current run
@@ -351,4 +413,3 @@ func keys(aMap map[gitrepo.FilePath][]string) []gitrepo.FilePath {
 	}
 	return result
 }
-

detector/detection_results_test.go

@@ -1,7 +1,11 @@
 package detector
 
 import (
+	"github.com/golang/mock/gomock"
+	"github.com/spf13/afero"
 	"strings"
+	mock "talisman/internal/mock/prompt"
+	"talisman/prompt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -62,23 +66,116 @@ func TestResultsReportsFailures(t *testing.T) {
 // }
 
 func TestTalismanRCSuggestionWhenThereAreFailures(t *testing.T) {
-	results := NewDetectionResults()
-	results.Fail("some_file.pem", "filecontent", "Bomb", []string{})
-
-	actualErrorReport := results.Report()
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
 
-	assert.Regexp(t, "fileignoreconfig:", actualErrorReport, "Error report does not contain expected output")
-	assert.Regexp(t, "- filename: some_file.pem", actualErrorReport, "Error report does not contain expected output")
-	assert.Regexp(t, "checksum: 87139cc4d975333b25b6275f97680604add51b84eb8f4a3b9dcbbc652e6f27ac", actualErrorReport, "Error report does not contain expected output")
-	assert.Regexp(t, "ignore_detectors: \\[\\]", actualErrorReport, "Error report does not contain expected output")
-
-}
-
-func TestTalismanRCSuggestionWhenNoFailures(t *testing.T) {
+	prompter := mock.NewMockPrompt(ctrl)
 	results := NewDetectionResults()
 
-	actualErrorReport := results.Report()
-
-	assert.NotRegexp(t, "fileignoreconfig:", actualErrorReport, "Error report should not contain this output")
-
+	// Creating temp file with some content
+	fs := afero.NewMemMapFs()
+	file, err := afero.TempFile(fs, "", "talismanrc")
+	assert.NoError(t, err)
+	ignoreFile := file.Name()
+
+	existingContent := `fileignoreconfig:
+- filename: existing.pem
+  checksum: 123444ddssa75333b25b6275f97680604add51b84eb8f4a3b9dcbbc652e6f27ac
+  ignore_detectors: []
+scopeconfig: []
+`
+	err = afero.WriteFile(fs, ignoreFile, []byte(existingContent), 0666)
+	assert.NoError(t, err)
+
+	// The tests below depend on the upper configuration which is shared across all three of them. Hence the order in
+	// which they run matters.
+	t.Run("should not prompt if there are no failures", func(t *testing.T) {
+		promptContext := prompt.NewPromptContext(true, prompter)
+		prompter.EXPECT().Confirm(gomock.Any()).Return(false).Times(0)
+
+		results.Report(fs, ignoreFile, promptContext)
+		bytesFromFile, err := afero.ReadFile(fs, ignoreFile)
+
+		assert.NoError(t, err)
+		assert.Equal(t, existingContent, string(bytesFromFile))
+	})
+
+	t.Run("when user declines, entry should not be added to talismanrc", func(t *testing.T) {
+		promptContext := prompt.NewPromptContext(true, prompter)
+		prompter.EXPECT().Confirm("Do you want to add this entry in talismanrc ?").Return(false)
+		results.Fail("some_file.pem", "filecontent", "Bomb", []string{})
+
+		results.Report(fs, ignoreFile, promptContext)
+		bytesFromFile, err := afero.ReadFile(fs, ignoreFile)
+
+		assert.NoError(t, err)
+		assert.Equal(t, existingContent, string(bytesFromFile))
+	})
+
+	t.Run("when interactive flag is set to false, it should not ask user", func(t *testing.T) {
+		promptContext := prompt.NewPromptContext(false, prompter)
+		prompter.EXPECT().Confirm(gomock.Any()).Return(false).Times(0)
+		results.Fail("some_file.pem", "filecontent", "Bomb", []string{})
+
+		results.Report(fs, ignoreFile, promptContext)
+		bytesFromFile, err := afero.ReadFile(fs, ignoreFile)
+
+		assert.NoError(t, err)
+		assert.Equal(t, existingContent, string(bytesFromFile))
+	})
+
+	t.Run("when user confirms, entry should be appended to given ignore file", func(t *testing.T) {
+		promptContext := prompt.NewPromptContext(true, prompter)
+		prompter.EXPECT().Confirm("Do you want to add this entry in talismanrc ?").Return(true)
+
+		results.Fail("some_file.pem", "filecontent", "Bomb", []string{})
+
+		expectedFileContent := `fileignoreconfig:
+- filename: existing.pem
+  checksum: 123444ddssa75333b25b6275f97680604add51b84eb8f4a3b9dcbbc652e6f27ac
+  ignore_detectors: []
+scopeconfig: []
+fileignoreconfig:
+- filename: some_file.pem
+  checksum: 87139cc4d975333b25b6275f97680604add51b84eb8f4a3b9dcbbc652e6f27ac
+  ignore_detectors: []
+scopeconfig: []
+`
+		results.Report(fs, ignoreFile, promptContext)
+		bytesFromFile, err := afero.ReadFile(fs, ignoreFile)
+
+		assert.NoError(t, err)
+		assert.Equal(t, expectedFileContent, string(bytesFromFile))
+	})
+
+	t.Run("when user confirms for multiple entries, they should be appended to given ignore file", func(t *testing.T) {
+		// Clearing file contents from previous tests
+		err := afero.WriteFile(fs, ignoreFile, []byte{}, 0666)
+		assert.NoError(t, err)
+
+		promptContext := prompt.NewPromptContext(true, prompter)
+		prompter.EXPECT().Confirm("Do you want to add this entry in talismanrc ?").Return(true).Times(2)
+
+		results.Fail("some_file.pem", "filecontent", "Bomb", []string{})
+		results.Fail("another.pem", "filecontent", "password", []string{})
+
+		expectedFileContent := `fileignoreconfig:
+- filename: some_file.pem
+  checksum: 87139cc4d975333b25b6275f97680604add51b84eb8f4a3b9dcbbc652e6f27ac
+  ignore_detectors: []
+- filename: another.pem
+  checksum: 117e23557c02cbd472854ebce4933d6daec1fd207971286f6ffc9f1774c1a83b
+  ignore_detectors: []
+scopeconfig: []
+`
+		results.Report(fs, ignoreFile, promptContext)
+		bytesFromFile, err := afero.ReadFile(fs, ignoreFile)
+
+		assert.NoError(t, err)
+		assert.Equal(t, expectedFileContent, string(bytesFromFile))
+	})
+
+
+	err = fs.Remove(ignoreFile)
+	assert.NoError(t, err)
 }

gitrepo/gitrepo.go

@@ -165,7 +165,7 @@ func (repo GitRepo) ReadRepoFile(fileName string) ([]byte, error) {
 }
 
 //ReadRepoFileOrNothing returns the contents of the supplied relative filename by locating it in the git repo.
-//If the given file cannot be located in theb repo, then an empty array of bytes is returned for the content.
+//If the given file cannot be located in the repo, then an empty array of bytes is returned for the content.
 func (repo GitRepo) ReadRepoFileOrNothing(fileName string) ([]byte, error) {
 	filepath := path.Join(repo.root, fileName)
 	if _, err := os.Stat(filepath); err == nil {