From 8ead034d59364b82ada0b966357ed15acdd1de64 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Tue, 17 Mar 2020 14:47:09 -0700 Subject: [PATCH 1/2] test_helper: make test setup more hermetic When running tests locally outside of a container (e.g. git clone git-secrets; make test), if a user is already using git-secrets by default in their system config and default gitdir template, tests which expect not to have git-secrets installed will fail. Instead, let's remove all hooks when we create a directory without the testbench's template and ignore the system config which may contain patterns that conflict with the testbench. --- test/test_helper.bash | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test_helper.bash b/test/test_helper.bash index 9133e51..f44b0c3 100644 --- a/test/test_helper.bash +++ b/test/test_helper.bash @@ -1,4 +1,6 @@ #!/bin/bash +# Disallow any system-level git-secret pattern configs +export GIT_CONFIG_NOSYSTEM="true" export TEST_REPO="$BATS_TMPDIR/test-repo" export TEMP_HOME="$BATS_TMPDIR/home" export TEMPLATE_DIR="${BATS_TMPDIR}/template" @@ -31,6 +33,9 @@ setup_repo() { mkdir -p $TEST_REPO cd $TEST_REPO git init + # Uninstall any hooks present in the system template which could interfere + # with git-secrets + rm -fr .git/hooks/* git config --local --add secrets.patterns '@todo' git config --local --add secrets.patterns 'forbidden|me' git config --local --add secrets.patterns '#hash' From 334474857a24cea81ef93269f620df2536aefeca Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Thu, 27 Feb 2020 15:47:17 -0800 Subject: [PATCH 2/2] multifile grep: perform greps in series Passing a very long argument list to git-grep can cause it to fail; indeed, it's possible for the list of paths passed by git-secrets to either grep or git-grep to exceed the maximum number of arguments allowed in a user's environment (`getconf ARG_MAX`). Instead, let xargs check that the number of arguments won't exceed the system limit. Signed-off-by: Emily Shaffer --- git-secrets | 59 ++++++++++++++++++++++++++++++++++++++++++-- test/pre-commit.bats | 17 +++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/git-secrets b/git-secrets index 11be153..7dca35e 100755 --- a/git-secrets +++ b/git-secrets @@ -113,16 +113,71 @@ git_grep() { local files=("${@}") combined_patterns=$(load_combined_patterns) [ -z "${combined_patterns}" ] && return 1 - GREP_OPTIONS= LC_ALL=C git grep -nwHEI ${options} "${combined_patterns}" -- "${files[@]}" + + if [ ${#files[@]} -eq 0 ]; then + GREP_OPTIONS= LC_ALL=C git grep -nwHEI ${options} "${combined_patterns}" + return $? + fi + + # let xargs watch for system limit on arg count for us. xargs returns 123 if + # any call returned 1, but we care that all calls returned 1, so invert the + # output - xargs will return 0 if every call returned 0 + printf "%s\n" "${files[@]}" | + GREP_OPTIONS= LC_ALL=C xargs -P "$(nproc)" -d'\n' sh -c \ + 'git grep "$@" + rc=$? + case $rc in + 0) exit 1 ;; + 1) exit 0 ;; + *) exit 255 ;; + esac' - \ + -nwHEI "${options}" "${combined_patterns}" -- + status=$? + + # convert the xargs-ified return code to what grep would have returned + case $status in + 0) return 1 ;; + 123) return 0 ;; + *) return 2 ;; + esac } # Performs a regular grep, taking into account patterns and recursion. # Note: this function returns 1 on success, 0 on error. regular_grep() { local files=("${@}") patterns=$(load_patterns) action='skip' + local status=0 [ -z "${patterns}" ] && return 1 [ ${RECURSIVE} -eq 1 ] && action="recurse" - GREP_OPTIONS= LC_ALL=C grep -d "${action}" -nwHEI "${patterns}" "${files[@]}" + + if [ "${#files[@]}" -eq 1 ] && [ "${files[0]}" = "-" ]; then + GREP_OPTIONS= LC_ALL=C grep -d "${action}" -nwHEI "${patterns}" - + return $? + fi + + # let xargs watch for system limit on arg count for us. + # massage output so that xargs returns: + # 0 if all calls succeeded ("no match") + # 123 if any call failed with status 1-128 ("found a match") + # 124 if any call failed with status 255 ("error") + printf "%s\n" "${files[@]}" | + GREP_OPTIONS= LC_ALL=C xargs -P "$(nproc)" -d'\n' sh -c \ + 'grep "$@" + rc=$? + case $rc in + 0) exit 1 ;; + 1) exit 0 ;; + *) exit 255 ;; + esac' - \ + -d "${action}" -nwHEI "${patterns}" + status=$? + + # convert the xargs-ified return code to what grep would have returned + case $status in + 0) return 1 ;; + 123) return 0 ;; + *) return 2 ;; + esac } # Process the given status ($1) and output variables ($2). diff --git a/test/pre-commit.bats b/test/pre-commit.bats index 5ace267..2c58663 100644 --- a/test/pre-commit.bats +++ b/test/pre-commit.bats @@ -60,3 +60,20 @@ load test_helper [ "${lines[1]}" == "failure1.txt:1:another line... forbidden" ] [ "${lines[2]}" == "failure2.txt:1:me" ] } + +@test "Runs safely with args beyond the system argument length limit" { + setup_good_repo + repo_run git-secrets --install $TEST_REPO + cd $TEST_REPO + + FILENAME_LENGTH="$(getconf NAME_MAX .)" + (( FILE_COUNT = ( "$(getconf ARG_MAX)" / "$FILENAME_LENGTH" ) + 1 )) + + for (( i = 0; i < "$FILE_COUNT"; i++ )); do + >"$(printf "%0${FILENAME_LENGTH}d" "$i")" + done + + run git add . + run git commit -m 'This is fine' + [ $status -eq 0 ] +}