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

New pre commit hook #3

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
54c8a4b
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 8, 2022
f04be76
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 8, 2022
b2d4132
Adding the pre-commit hook and instructions on how to set it up
rajk1000 Sep 8, 2022
ed5efd3
comment out the set -x in the pre-commit script
rajk1000 Sep 8, 2022
17f7397
removing scan out file to o do more testing
rajk1000 Sep 8, 2022
304b34e
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 8, 2022
50088b0
removing scan out file to o do more testing
rajk1000 Sep 8, 2022
8ad9d5e
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 8, 2022
9598897
removing scan out file to o do more testing
rajk1000 Sep 8, 2022
e254add
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 8, 2022
b09a056
removing scan out file to o do more testing
rajk1000 Sep 8, 2022
0d34c4b
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 8, 2022
dc86fcb
modifying pre-commit hook after review
rajk1000 Sep 8, 2022
f6f77fc
modifying bit more indent fixes pre-commit hook after review
rajk1000 Sep 8, 2022
a75009e
removing scan out file to o do more testing
rajk1000 Sep 9, 2022
8834a8b
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 9, 2022
0b9ea55
adding more comments to pre-commit hook after review
rajk1000 Sep 9, 2022
7e4b03b
removing scan out file to o do more testing
rajk1000 Sep 9, 2022
5606ca2
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 9, 2022
71e40bd
adding some variable cuddling to pre-commit hook after review
rajk1000 Sep 9, 2022
62405ab
removing scan out file to o do more testing
rajk1000 Sep 9, 2022
b05dc3d
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 9, 2022
9c16a79
adding some tweaks to pre-commit hook after review
rajk1000 Sep 9, 2022
59927b2
Adding tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 9, 2022
181033f
removing scan out file to o do more testing
rajk1000 Sep 9, 2022
a010147
adding in main to do more testing
rajk1000 Sep 9, 2022
6207827
adding in mod file
rajk1000 Sep 9, 2022
c5b0f7a
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 9, 2022
a94c175
removing in mod file
rajk1000 Sep 9, 2022
eb5f427
removing scan out file to o do more testing
rajk1000 Sep 9, 2022
3cb661a
adding in a comment only
rajk1000 Sep 9, 2022
90d746c
Adding in insecure code
rajk1000 Sep 9, 2022
fb615a1
removing scan out file to o do more testing
rajk1000 Sep 9, 2022
26190c3
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 9, 2022
eb3a548
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 9, 2022
daae91c
removing scan out file to o do more testing
rajk1000 Sep 9, 2022
4ff1f44
changed name of bucket
rajk1000 Sep 9, 2022
04f160b
Added in ability to pass in TFSEC_PRECOMMIT_ARGS to tfsec
rajk1000 Sep 13, 2022
1a09a2e
stuff dding in pre-commit hook that runs tfsec on new terraform files
rajk1000 Sep 14, 2022
681ce63
removing scan out file to o do more testing
rajk1000 Sep 14, 2022
d00efff
comment to trigger tfsec
rajk1000 Sep 14, 2022
1fba480
removing scan out file to o do more testing
rajk1000 Sep 15, 2022
ca27cd6
Adding correct tfsec-scan-output.txt file to check the diff working ok
rajk1000 Sep 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
#!/bin/bash
#set -x
PREFIX="pre-commit:"
RESULTS_FILE=tfsec-scan-output.txt

Choose a reason for hiding this comment

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

Quote your variables please.

Also, it would help if there were some useful comments - reading comments aids understanding.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

TFSEC_RESULTS_DIR=/tmp/tfsec-results

Choose a reason for hiding this comment

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

Ditto.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

TFSEC_WORKING_DIR=/tmp/tfsec-files

Choose a reason for hiding this comment

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

Ditto.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok


initialise() {
mkdir -p $TFSEC_WORKING_DIR

Choose a reason for hiding this comment

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

Indentation should be two spaces.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

mkdir -p $TFSEC_RESULTS_DIR
rm $TFSEC_WORKING_DIR/* 2> /dev/null

Choose a reason for hiding this comment

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

Remove stray space.

Why are you redirecting STDERR to /dev/null?

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok. Redirecting as it gives an error if this is run for the first time

Choose a reason for hiding this comment

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

Righto, maybe you could:

  • see if the directory exists
  • if yes, delete it & recreate it
  • if no, create it

rm $TFSEC_RESULTS_DIR/* 2> /dev/null

Choose a reason for hiding this comment

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

Ditto.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

ALLOW_TFSEC_ISSUES="no"
}

populateGlobalVars() {
stagedFileList=$(git diff --diff-filter=d --cached --name-only)

Choose a reason for hiding this comment

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

All variables should be upper case.

It's generally a good idea to quote VAR="$(some command)"

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

tfFileList=$(echo "$stagedFileList" | grep -E '\.(tf)$')
if [[ ! -z "$tfFileList" ]] && [ ${#tfFileList[@]} -gt 0 ];

Choose a reason for hiding this comment

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

Is there a reason why you mixed [[ ]] and [ ] ..?

Copy link
Owner Author

Choose a reason for hiding this comment

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

no reason. changed now to [[

then
TERRAFORM_FILES_STAGED="yes"
else
TERRAFORM_FILES_STAGED="no"
fi

Choose a reason for hiding this comment

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

Remove stray space.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

}

copyTerraformFilesToBeScanned() {

echo "$PREFIX Performing a tfsec scan on the terraform files you have staged (only terraform .tf files will be parsed)."
echo "$PREFIX The list of files to be scanned by tfsec will be as follows.."

#browse through ALL files , not only the TF files
for value in ${stagedFileList}
do
# copy all TF files to /tmp/tfsec-files
if [[ $value == *.tf ]]
then
echo "$PREFIX FILE: $value"
cp "$value" "$TFSEC_WORKING_DIR"

Choose a reason for hiding this comment

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

B0rked indentation.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

fi

# check if the tfsec-scan-output file has been staged
if [[ $value == $RESULTS_FILE ]]
then
# by staging this file, the user has indicated that they want to allow through the commit despite the tfsec issues.
ALLOW_TFSEC_ISSUES="yes"
# also copy this to the same directory so that a diff can take place later between this staged file and the latest scan output
cp "$value" "$TFSEC_WORKING_DIR"

fi
done
}

removeTimingStatsFromScanOutput() {
#remove any timing stats from the file, as these are problematic when doing a diff

Choose a reason for hiding this comment

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

Add a space after each #

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

#Firstly, save the file before any sed replacement just in case we wish to troubleshoot
cp $TFSEC_RESULTS_DIR/$RESULTS_FILE $TFSEC_RESULTS_DIR/tfsec-scan-output-orig.txt
sed -i '/^ disk i/d' $TFSEC_RESULTS_DIR/$RESULTS_FILE

Choose a reason for hiding this comment

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

Quoting and cuddling are a must here.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

sed -i '/^ parsing/d' $TFSEC_RESULTS_DIR/$RESULTS_FILE
sed -i '/^ adaptation/d' $TFSEC_RESULTS_DIR/$RESULTS_FILE
sed -i '/^ checks/d' $TFSEC_RESULTS_DIR/$RESULTS_FILE
sed -i '/^ total/d' $TFSEC_RESULTS_DIR/$RESULTS_FILE
}

Choose a reason for hiding this comment

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

Two blank lines here, but elsewhere you use a single blank line between code blocks.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok


informUserThatWeAllowCommitThroughDespiteTfsecIssues() {
echo "$PREFIX Your Commit is being let through despite tfsec issues!!!"

Choose a reason for hiding this comment

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

A pile of echo statements is calling out for a sub-procedure (there is a my-info example in the style guide)

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

echo "$PREFIX You have indicated that we should let the commit go through, but please review the tfsec issues detected with the team!"
echo "$PREFIX The tfsec-scan-output.txt file has details of these tfsec issues shown and has been committed too"
}

informUserThatWeDenyCommitBecauseTfSecNotUptoDate() {
echo "$PREFIX Your Commit is disallowed and has been aborted!!!"
echo "$PREFIX Even though you have GIT add(ed) your $RESULTS_FILE file, it is not up to date."
echo "$PREFIX Please check the latest issues in $TFSEC_RESULTS_DIR/$RESULTS_FILE and see if you wish to still ignore"
echo "$PREFIX If you still wish to ignore issues, please GIT add the latest file $TFSEC_RESULTS_DIR/$RESULTS_FILE"
}


informUserThatWeDenyCommitBecauseOutstandingTfSecIssues() {
echo "$PREFIX Your Commit is disallowed and has been aborted!!!"
echo "$PREFIX You should aim to fix the above reported tfsec issues."
echo "$PREFIX The output of the tfsec scan was also written to the file $TFSEC_RESULTS_DIR/$RESULTS_FILE"
echo "$PREFIX NOTE : If you cannot fix and instead want these to be reviewed, please GIT add the following file"
echo "$PREFIX NOTE : (The file to GIT add is $TFSEC_RESULTS_DIR/$RESULTS_FILE)"
echo "$PREFIX NOTE : If you GIT add the above file, the commit will be allowed through"
}


initialise;

Choose a reason for hiding this comment

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

I don't think the ; are needed.

Also, if this is the start of the main block, you should have a comment to suggest things have started.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

populateGlobalVars;

if [ "$TERRAFORM_FILES_STAGED" = "yes" ]; then

Choose a reason for hiding this comment

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

then should be beneath the if

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

Choose a reason for hiding this comment

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

Are you using [ ] or [[ ]]..?

I think [[ ]] is safer, but I can't remember why.

Copy link
Owner Author

Choose a reason for hiding this comment

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

[[ now


copyTerraformFilesToBeScanned;

#Do the latest tfsec scan on the files that have been staged
if ! tfsec $TFSEC_WORKING_DIR --no-colour --out $TFSEC_RESULTS_DIR/$RESULTS_FILE; then

Choose a reason for hiding this comment

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

Quoting and cuddling, blah, blah.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok


removeTimingStatsFromScanOutput;

if [ "$ALLOW_TFSEC_ISSUES" = "yes" ]
then
#Check that the file added is exactly the same as the latest tfsec issues. They must be identical in order for the commit to be allowed through
diffResult=$(diff $TFSEC_RESULTS_DIR/$RESULTS_FILE $TFSEC_WORKING_DIR/$RESULTS_FILE)

Choose a reason for hiding this comment

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

Ditto.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

if [ $? -eq 0 ]
then
informUserThatWeAllowCommitThroughDespiteTfsecIssues;

Choose a reason for hiding this comment

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

Not a fan of Camel case function names.

Also, could they be shorter?

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

exit 0
else
informUserThatWeDenyCommitBecauseTfSecNotUptoDate;
exit 1
fi
else
informUserThatWeDenyCommitBecauseOutstandingTfSecIssues;

Choose a reason for hiding this comment

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

Indentation

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok

exit 1
fi
fi
fi
12 changes: 11 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1 +1,11 @@
# tfsec-testing

## Instructions on using git hooks in this repository.
---
*Please run the following command to set the git hooks directory as .githooks/*
---
git config --local core.hooksPath .githooks/

*The pre-commit hook*
---
This pre-commit hooks runs *tfsec* on any Terraform (.tf) files included as part of the commit.
If it finds *tfsec* issues reported, it will then disallow the commit. To override this, read the full instructions on what to do. This is output by the pre-commit hook
2 changes: 1 addition & 1 deletion main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ resource "aws_s3_bucket" "foo-bucket" {
acl = "private"
}

#hello world
#hello world commenting
module "raj_otherstuff" {
source = "./modules/otherstuff"
}