Skip to content
This repository has been archived by the owner on Jun 15, 2022. It is now read-only.

#74: Testing of seekrets rulesets using bats #87

Merged
merged 20 commits into from
Dec 9, 2016
Merged

Conversation

rogeruiz
Copy link
Contributor

@rogeruiz rogeruiz commented Nov 9, 2016

  • basic functional tests for git-seekrets

@alain-hoang alain-hoang changed the title [ work in progress ] WIP for #74. Testing of seekrets rulesets using bats Testing of seekrets rulesets using bats Nov 14, 2016
@alain-hoang alain-hoang changed the title Testing of seekrets rulesets using bats #74: Testing of seekrets rulesets using bats Nov 14, 2016
@LinuxBozo
Copy link
Contributor

@alain-hoang I would expect to see a couple more tests here, since the newrelic rule has explicit exceptions (unmatch), we should make sure that we're testing that those exceptions are ignored properly.

@alain-hoang
Copy link
Contributor

More tests updated to take into account unmatch rules for newrelic.

Also a new feature has been submitted upstream for being able to query for active hooks in a git repository #27

@alain-hoang
Copy link
Contributor

Waiting for feedback from upstream

* basic functional tests for git-seekrets
* Add tests for installation check
* Enable/disable tests for rulesets
* Tests for true positives in test repo
* Addresses repeatable installation for BATS
* Extra assurance that test runs are atomic
* Add more tests to cover new relic false matches and aws ids
@LinuxBozo LinuxBozo changed the base branch from seekrets-install to seekret December 5, 2016 19:09
pulled out version into variable gitseekretversion
@LinuxBozo LinuxBozo self-requested a review December 8, 2016 16:59
Copy link
Contributor

@LinuxBozo LinuxBozo left a comment

Choose a reason for hiding this comment

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

There seems to be some needed work here for failing tests.


@test "git-seekrets does not find newrelic false positives in test repo" {
run addFileWithFalseNewrelicSecrets
[ $(expr "${lines[1]}" : "Found Secrets: 0") -eq 0 ]
Copy link
Contributor

Choose a reason for hiding this comment

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

This test fails. Going by previous tests, shouldn't this be -ne 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

If a confirmed secret is found it would be expected that the return value would satisfy the condition -ne 0 as mentioned. This test is specifically trying to test a false positive which should not show up as a secret. If it does show up as a secret then this would be an invalid test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that logic, and the following:

#!/bin/bash
RIGHT="right"
if [ $(expr "${RIGHT}" : "right") -eq 0 ]; then
  echo "yep"
else
  echo "nope"
fi

You would then expect to see "yep" echo back on the terminal then, yes?

$ ./test-eq-0.sh
nope

Copy link
Contributor

Choose a reason for hiding this comment

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

I've put in an update that addresses this. I think my original thought was to capture the exit value of the expr command however the intention of the above code will test against the output generated from the expr command. I've updated the test appropriately. Thank you for pointing this out @LinuxBozo!


@test "git-seekrets only matches newrelic secrets in test repo" {
run addFileWithSomeNewrelicSecrets
[ $(expr "${lines[1]}" : "Found Secrets: 1") -eq 0 ]
Copy link
Contributor

Choose a reason for hiding this comment

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

This test fails. Going by previous tests, shouldn't this be -ne 0?

[ $status -gt 0 ]
}

@test "git-seekrets does not find secrets in test repo" {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is currently failing. Needs investigation into why and addressed.

Copy link
Contributor

Choose a reason for hiding this comment

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

False positive from outdated binary.

@alain-hoang
Copy link
Contributor

After looking into the test failures, it appears that the draft binary is being built with upstream which does not include the 18F changes necessary to work. I have put in an updated binary that is built against 18F's version of the git-seekret repo.

@LinuxBozo LinuxBozo merged commit 0e656b0 into seekret Dec 9, 2016
@LinuxBozo LinuxBozo deleted the seekrets-bats branch December 9, 2016 15:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants