Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified the scan logic for pre-commit hooks #107

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
42d2b74
Merge pull request #4 from thoughtworks/master
sriharsha-y Jan 10, 2019
f1c7561
Made changes to report failures in .talismanrc as 'Warnings'
aaquibzama-tw Jan 22, 2019
b471c01
Corrected the recursive Hex detection on .talismanrc.
sriharsha-y Feb 3, 2019
403e882
Merge branch 'master' of https://github.com/thoughtworks/talisman int…
sriharsha-y Feb 3, 2019
3c6efb9
Merge branch 'thoughtworks-master' into temp_master
sriharsha-y Feb 3, 2019
4aabdab
Corrected Failures variable in DetectionResults for rendering correct…
sriharsha-y Feb 3, 2019
9f9167e
Added heading for warnings table.
sriharsha-y Feb 3, 2019
dd41779
Removed unnecessary debug logs.
sriharsha-y Feb 3, 2019
221e2b1
Adds functionality to generate scan report in JSON format
aaquibzama-tw Feb 18, 2019
5f43eea
Adds functionality to generate scan report in JSON format
aaquibzama-tw Feb 18, 2019
de2bb3d
Resolved conflicts
aaquibzama-tw Feb 18, 2019
8701828
Modified the name of reports folder
aaquibzama-tw Feb 19, 2019
004fa84
Adds functionality to save reports in custom location
aaquibzama-tw Feb 19, 2019
0db9212
Implemented code review comments
aaquibzama-tw Feb 20, 2019
adb2f8b
Fixed build failure
aaquibzama-tw Feb 20, 2019
8fa8cff
Updated the directory name for reports folder
aaquibzama-tw Feb 20, 2019
f8ab3cc
Merge pull request #7 from thoughtworks/master
aaquibzama-tw Mar 5, 2019
6bd7ceb
Modified the scan logic for pre-commit hooks
aaquibzama-tw Mar 5, 2019
5101160
Fixed build failure
aaquibzama-tw Mar 5, 2019
b2ebfe9
Stops the scan in scenarios where the checksum is matching
aaquibzama-tw Mar 7, 2019
87b0652
Stops the scan in scenarios where the checksum is matching
aaquibzama-tw Mar 7, 2019
7d5b31a
Modified the scan logic for pre-commit hooks
aaquibzama-tw Mar 5, 2019
3c70730
Stops the scan in scenarios where the checksum is matching
aaquibzama-tw Mar 7, 2019
532ed67
Merge branch 'master' of https://github.com/aaquibzama-tw/talisman
aaquibzama-tw Mar 7, 2019
432b05c
Implemented review fest
aaquibzama-tw Mar 7, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Find the instructions below.
## [Recommended approach]
## Installation as a global hook template

We recommend installing Talisman as a git hook template, as that will cause
We recommend installing Talisman as a **pre-commit git hook template**, as that will cause
Talisman to be present, not only in your existing git repositories, but also in any new repository that you 'init' or
'clone'.

Expand Down Expand Up @@ -226,6 +226,10 @@ In the above example, the file *danger.pem* has been flagged as a security breac
* The filename matches one of the pre-configured patterns.
* The file contains an awsSecretKey which is scanned and flagged by Talisman

If you have installed Talisman as a pre-commit hook, it will scan only the _diff_ within each commit. This means that it would only report errors for parts of the file that were changed.

In case you have installed Talisman as a pre-push hook, it will scan the complete file in which changes are made. As mentioned above, it is recommended that you use Talisman as a **pre-commit hook**.

## Validations
The following detectors execute against the changesets to detect secrets/sensitive information:

Expand Down Expand Up @@ -321,7 +325,6 @@ In case you want to store the reports in some other location, it can be provided
* `talisman --scan --rd=/Users/username/Desktop`

<i>Talisman currently does not support ignoring of files for scanning.</i>

# Uninstallation
The uninstallation process depends on how you had installed Talisman.
You could have chosen to install as a global hook template or at a single repository.
Expand Down
41 changes: 36 additions & 5 deletions acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ const awsAccessKeyIDExample string = "accessKey=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEX
const talismanRCDataWithIgnoreDetectorWithFilename = `
fileignoreconfig:
- filename: private.pem
checksum: 05db785bf1e1712f69b81eeb9956bd797b956e7179ebe3cb7bb2cd9be037a24c
checksum: 05db785bf1e1712f69b81eeb9956bd797b956e7179ebe3cb7bb2cd9be37a24c
ignore_detectors: [filename]
`

const talismanRCDataWithIgnoreDetectorWithFilecontent = `
fileignoreconfig:
- filename: private.pem
checksum: 05db785bf1e1712f69b81eeb9956bd797b956e7179ebe3cb7bb2cd9be037a24c
checksum: 05db785bf1e1712f69b81eeb9956bd797b956e7179ebe3cb7bb2cd9be37a24c
ignore_detectors: [filecontent]
`

Expand Down Expand Up @@ -93,13 +93,13 @@ func TestAddingSecretKeyShouldExitOneIfPEMFileIsPresentInTheGitHistory(t *testin
_options := options{
debug: false,
githook: PrePush,
scan: true,
scan: false,
}
git.SetupBaselineFiles("simple-file")
git.CreateFileWithContents("private.pem", "secret")
git.CreateFileWithContents(".talismanrc", talismanRCDataWithFileNameAndCorrectChecksum)
git.AddAndcommit("private.pem", "add private key")
assert.Equal(t, 1, runTalismanWithOptions(git, _options), "Expected run() to return 0 and pass as pem file was ignored")
assert.Equal(t, 0, runTalismanWithOptions(git, _options), "Expected run() to return 0 and pass as pem file was ignored")
})
}

Expand All @@ -108,7 +108,7 @@ func TestScanningSimpleFileShouldExitZero(t *testing.T) {
_options := options{
debug: false,
githook: PrePush,
scan: true,
scan: false,
}
git.SetupBaselineFiles("simple-file")
assert.Equal(t, 0, runTalismanWithOptions(git, _options), "Expected run() to return 0 and pass as pem file was ignored")
Expand All @@ -130,6 +130,37 @@ func TestChecksumCalculatorShouldExitOne(t *testing.T) {
})
}

func TestShouldExitOneWhenSecretIsCommitted(t *testing.T) {
withNewTmpGitRepo(func(git *git_testing.GitTesting) {
_options := options{
debug: false,
githook: PreCommit,
scan: false,
}
git.SetupBaselineFiles("simple-file")
git.CreateFileWithContents("sample.txt", "password=somepassword \n")
git.Add("*")
assert.Equal(t, 1, runTalismanWithOptions(git, _options), "Expected run() to return 1 as given patterns are found")
})
}

func TestShouldExitZeroWhenNonSecretIsCommittedButFileContainsSecretPreviously(t *testing.T) {
withNewTmpGitRepo(func(git *git_testing.GitTesting) {
_options := options{
debug: false,
githook: PreCommit,
scan: false,
}
git.SetupBaselineFiles("simple-file")
git.CreateFileWithContents("sample.txt", "password=somepassword \n")
git.AddAndcommit("*", "Initial Commit With Secret")

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")
})
}

// Need to work on this test case as talismanrc does not yet support comments
// func TestAddingSecretKeyShouldExitZeroIfPEMFilesAreIgnoredAndCommented(t *testing.T) {
// withNewTmpGitRepo(func(git *git_testing.GitTesting) {
Expand Down
14 changes: 14 additions & 0 deletions detector/checksum_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,20 @@ func NewChecksumCompare(gitAdditions []git_repo.Addition, talismanRCIgnoreConfig
return &cc
}

func (cc *ChecksumCompare) IsScanNotRequired(addition git_repo.Addition) bool {
currentCollectiveChecksum := utility.CollectiveSHA256Hash([]string{string(addition.Path)})
declaredCheckSum := ""
for _, ignore := range cc.ignoreConfig.FileIgnoreConfig {
if addition.Matches(ignore.FileName) {
currentCollectiveChecksum = utility.CollectiveSHA256Hash([]string{ignore.FileName})
declaredCheckSum = ignore.Checksum
}

}
return currentCollectiveChecksum == declaredCheckSum

}

//FilterIgnoresBasedOnChecksums filters the file ignores from the TalismanRCIgnore which doesn't have any checksum value or having mismatched checksum value from the .talsimanrc
func (cc *ChecksumCompare) FilterIgnoresBasedOnChecksums() TalismanRCIgnore {
finalIgnores := []FileIgnoreConfig{}
Expand Down
4 changes: 1 addition & 3 deletions detector/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ func (dc *Chain) Test(additions []git_repo.Addition, ignoreConfig TalismanRCIgno
repo := git_repo.RepoLocatedAt(wd)
gitTrackedFilesAsAdditions := repo.TrackedFilesAsAdditions()
gitTrackedFilesAsAdditions = append(gitTrackedFilesAsAdditions, additions...)
cc := NewChecksumCompare(gitTrackedFilesAsAdditions, ignoreConfig)
ignoreConfigFiltered := cc.FilterIgnoresBasedOnChecksums()
for _, v := range dc.detectors {
v.Test(additions, ignoreConfigFiltered, result)
v.Test(additions, ignoreConfig, result)
}
}
3 changes: 2 additions & 1 deletion detector/filecontent_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ func (fc *FileContentDetector) AggressiveMode() *FileContentDetector {
}

func (fc *FileContentDetector) Test(additions []git_repo.Addition, ignoreConfig TalismanRCIgnore, result *DetectionResults) {
cc := NewChecksumCompare(additions, ignoreConfig)
for _, addition := range additions {
if ignoreConfig.Deny(addition, "filecontent") {
if ignoreConfig.Deny(addition, "filecontent") || cc.IsScanNotRequired(addition) {
log.WithFields(log.Fields{
"filePath": addition.Path,
}).Info("Ignoring addition as it was specified to be ignored.")
Expand Down
3 changes: 2 additions & 1 deletion detector/filename_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@ func NewFileNameDetector(patternStrings ...string) Detector {

//Test tests the fileNames of the Additions to ensure that they don't look suspicious
func (fd FileNameDetector) Test(additions []git_repo.Addition, ignoreConfig TalismanRCIgnore, result *DetectionResults) {
cc := NewChecksumCompare(additions, ignoreConfig)
for _, addition := range additions {
if ignoreConfig.Deny(addition, "filename") {
if ignoreConfig.Deny(addition, "filename") || cc.IsScanNotRequired(addition){
log.WithFields(log.Fields{
"filePath": addition.Path,
}).Info("Ignoring addition as it was specified to be ignored.")
Expand Down
3 changes: 2 additions & 1 deletion detector/filesize_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ func NewFileSizeDetector(size int) Detector {
}

func (fd FileSizeDetector) Test(additions []git_repo.Addition, ignoreConfig TalismanRCIgnore, result *DetectionResults) {
cc := NewChecksumCompare(additions, ignoreConfig)
for _, addition := range additions {
if ignoreConfig.Deny(addition, "filesize") {
if ignoreConfig.Deny(addition, "filesize") || cc.IsScanNotRequired(addition) {
log.WithFields(log.Fields{
"filePath": addition.Path,
}).Info("Ignoring addition as it was specified to be ignored.")
Expand Down
3 changes: 2 additions & 1 deletion detector/pattern_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ type PatternDetector struct {

//Test tests the contents of the Additions to ensure that they don't look suspicious
func (detector PatternDetector) Test(additions []git_repo.Addition, ignoreConfig TalismanRCIgnore, result *DetectionResults) {
cc := NewChecksumCompare(additions, ignoreConfig)
for _, addition := range additions {
if ignoreConfig.Deny(addition, "filecontent") {
if ignoreConfig.Deny(addition, "filecontent") || cc.IsScanNotRequired(addition) {
log.WithFields(log.Fields{
"filePath": addition.Path,
}).Info("Ignoring addition as it was specified to be ignored.")
Expand Down
29 changes: 29 additions & 0 deletions git_repo/git_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ func RepoLocatedAt(path string) GitRepo {
return GitRepo{absoluteRoot}
}

//Gets all the staged files and collects the diff section in each file
func (repo GitRepo) GetDiffForStagedFiles() []Addition {
files := repo.stagedFiles()
result := make([]Addition, len(files))
for i, file := range files {
data := repo.fetchStagedDiff(file)
result[i] = NewAddition(file, data)
}

log.WithFields(log.Fields{
"additions": result,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try and make logging of result less verbose.
We can

  • Make this a debug log instead of info
    or
  • Make sure the member Data []byte of type Addition is not logged by default

}).Debug("Generating staged additions.")
return result
}

func (repo GitRepo) StagedAdditions() []Addition {
files := repo.stagedFiles()
result := make([]Addition, len(files))
Expand Down Expand Up @@ -208,6 +223,20 @@ func (repo *GitRepo) fetchStagedChanges() string {
return string(repo.executeRepoCommand("git", "diff", "--cached", "--name-status", "--diff-filter=ACM"))
}

//Fetches the currently staged diff and filters the command output to get only the modified sections of the file
func (repo *GitRepo) fetchStagedDiff(fileName string) []byte {
var result []byte
changes := strings.Split(string(repo.executeRepoCommand("git", "diff", "--staged", fileName)), "\n")
for _, c := range changes {
if !strings.HasPrefix(c, "+++") && !strings.HasPrefix(c, "---") && strings.HasPrefix(c, "+") {

result = append(result, strings.TrimPrefix(c, "+")...)
result = append(result, "\n"...)
}
}
return result
}

func (repo GitRepo) fetchRawOutgoingDiff(oldCommit string, newCommit string) string {
gitRange := oldCommit + ".." + newCommit
return string(repo.executeRepoCommand("git", "diff", gitRange, "--name-only", "--diff-filter=ACM"))
Expand Down
8 changes: 6 additions & 2 deletions git_testing/git_testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,18 @@ func (git *GitTesting) FileContents(filePath string) []byte {
}

func (git *GitTesting) AddAndcommit(fileName string, message string) {
git.ExecCommand("git", "add", fileName)
git.ExecCommand("git", "commit", fileName, "-m", message)
git.Add(fileName)
git.Commit(fileName, message)
}

func (git *GitTesting) Add(fileName string) {
git.ExecCommand("git", "add", fileName)
}

func (git *GitTesting) Commit(fileName string, message string) {
git.ExecCommand("git", "commit", fileName, "-m", message)
}

func (git *GitTesting) GetBlobDetails(fileName string) string {
var output []byte
object_hash_and_filename := ""
Expand Down
2 changes: 1 addition & 1 deletion pre_commit_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ func NewPreCommitHook() *PreCommitHook {
func (p *PreCommitHook) GetRepoAdditions() []git_repo.Addition {
wd, _ := os.Getwd()
repo := git_repo.RepoLocatedAt(wd)
return repo.StagedAdditions()
return repo.GetDiffForStagedFiles()
}