Tempfile and temp directory cleanups (#473)

* Tempfile and temp directory cleanups
    Add comments about mktemp on different platforms
    Be more careful about tempfile cleanups
    Don't use find to locate files to cleanup
    Use shorter SECRETS_EXTENSION and SECRETS_DIR env settings in tests
    Set TMPDIR in tests again
    Show DESTDIR used when testing git-secret install
    Change filename passed to mktemp -t in tests
This commit is contained in:
Josh Rabinowitz 2019-05-21 12:06:51 -04:00 committed by GitHub
parent c20e61313b
commit 10fa2a7be2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 82 additions and 25 deletions

View File

@ -12,7 +12,13 @@ function __replace_in_file_freebsd {
function __temp_file_freebsd {
: "${TMPDIR:=/tmp}"
local filename
filename=$(mktemp -t _git_secret_XXX )
# man mktemp on FreeBSD:
# ...
# If the -t prefix option is given, mktemp will generate a template string
# based on the prefix and the TMPDIR environment variable if set. The
# default location if TMPDIR is not set is /tmp. "
filename=$(mktemp -t _git_secret )
echo "$filename";
}

View File

@ -10,7 +10,21 @@ function __replace_in_file_linux {
function __temp_file_linux {
: "${TMPDIR:=/tmp}"
local filename
filename=$(mktemp -t _git_secret_XXX ) # makes a filename like /tmp/_git_secret_6Hw
# man mktemp on CentOS 7:
# mktemp [OPTION]... [TEMPLATE]
# ...
# -p DIR, --tmpdir[=DIR]
# interpret TEMPLATE relative to DIR; if DIR is not specified,
# use $TMPDIR if set, else /tmp. With this option, TEMPLATE
# must not be an absolute name; unlike with -t, TEMPLATE may
# contain slashes, but mktemp creates only the final component
# ...
# -t interpret TEMPLATE as a single file name component,
# relative to a directory: $TMPDIR, if set; else the directory
# specified via -p; else /tmp [deprecated]
filename=$(mktemp -p "${TMPDIR}" _git_secret.XXXXX )
# makes a filename like /$TMPDIR/_git_secret.ONIHo
echo "$filename"
}

View File

@ -10,7 +10,16 @@ function __replace_in_file_osx {
function __temp_file_osx {
: "${TMPDIR:=/tmp}"
local filename
filename=$(mktemp -t _git_secret ) # makes a filename like '/var/folders/nz/vv4_91234569k3tkvyszvwg90009gn/T/_git_secret.HhvUPlUI'
# man mktemp on OSX:
# ...
# "If the -t prefix option is given, mktemp will generate a template string
# based on the prefix and the _CS_DARWIN_USER_TEMP_DIR configuration vari-
# able if available. Fallback locations if _CS_DARWIN_USER_TEMP_DIR is not
# available are TMPDIR and /tmp."
filename=$(mktemp -t _git_secret )
# On OSX this can make a filename like
# '/var/folders/nz/vv4_91234569k3tkvyszvwg90009gn/T/_git_secret.HhvUPlUI'
echo "$filename";
}

View File

@ -71,7 +71,6 @@ function tell {
local start_key_cnt
start_key_cnt=$(get_gpg_key_count)
for email in "${emails[@]}"; do
# This file will be removed automatically:
_temporary_file # note that `_temporary_file` will export `temporary_filename` var.
# shellcheck disable=2154
local keyfile="$temporary_filename"
@ -105,6 +104,9 @@ function tell {
$SECRETS_GPG_COMMAND "${args[@]}"
fi
exit_code=$?
rm -f "$keyfile" || _abort "error removing temporary keyfile: $keyfile"
if [[ "$exit_code" -ne 0 ]]; then
_abort "problem importing public key for '$email' with gpg: exit code $exit_code"
fi

View File

@ -13,7 +13,10 @@ FIXTURES_DIR="$BATS_TEST_DIRNAME/fixtures"
TEST_GPG_HOMEDIR="$BATS_TMPDIR"
TEST_GPG_OUTPUT_FILE=$(TMPDIR="$BATS_TMPDIR" mktemp -t '_git_secret_test_output_XXX')
# TODO: factor out tempdir creation. On osx TEST_GPG_OUTPUT_FILE, still has 'XXXXXX's, like
# /var/folders/mm/_f0j67x10l92b4zznyx4ylzh00017w/T/gitsecret_output.XXXXXX.RaqyGYqL
TEST_GPG_OUTPUT_FILE=$(TMPDIR="$BATS_TMPDIR" mktemp -t 'gitsecret_output.XXXXXX')
#echo "# TEST_GPG_OUTPUT_FILE=$TEST_GPG_OUTPUT_FILE" >&3
# shellcheck disable=SC2016
AWK_GPG_GET_FP='
@ -103,7 +106,7 @@ function install_fixture_key {
cp "$FIXTURES_DIR/gpg/${1}/public.key" "$public_key"
$GPGTEST --import "$public_key" >> "$TEST_GPG_OUTPUT_FILE" 2>&1
rm -f "$public_key"
rm -f "$public_key" || _abort "Couldn't delete public key: $public_key"
}
@ -127,7 +130,7 @@ function install_fixture_full_key {
install_fixture_key "$1"
rm -f "$private_key"
rm -f "$private_key" || _abort "Couldn't delete private key: $private_key"
# return fingerprint to delete it later:
echo "$fingerprint"
}
@ -273,11 +276,26 @@ function unset_current_state {
rm "$TEST_GPG_OUTPUT_FILE"
# removes gpg homedir:
find "$TEST_GPG_HOMEDIR" \
-regex ".*\/random_seed\|.*\.gpg\|.*\.kbx.?\|.*private-keys.*\|.*test_sub_dir\|.*S.gpg-agent\|.*file_to_hide.*" \
-exec rm -rf {} +
## old code to remove tmp gpg homedir: TODO: remove.
#find "$TEST_GPG_HOMEDIR" \
# -regex ".*\/random_seed\|.*\.gpg\|.*\.kbx.?\|.*private-keys.*\|.*test_sub_dir\|.*S.gpg-agent\|.*file_to_hide.*" \
# -exec rm -rf {} +
# new code to remove temporary gpg homedir artifacts.
# For #360, 'find and rm only relevant files when test fails'.
# ${VAR:?} will cause command to fail if VAR is 0 length, as per shellcheck SC2115
rm -vrf "${TEST_GPG_HOMEDIR:?}/private-keys*" 2>&1 | sed 's/^/# unset_current_state: rm /'
rm -vrf "${TEST_GPG_HOMEDIR:?}/*.kbx" 2>&1 | sed 's/^/# unset_current_state: rm /'
rm -vrf "${TEST_GPG_HOMEDIR:?}/*.kbx~" 2>&1 | sed 's/^/# unset_current_state: rm /'
rm -vrf "${TEST_GPG_HOMEDIR:?}/*.gpg" 2>&1 | sed 's/^/# unset_current_state: rm /'
rm -vrf "${TEST_GPG_HOMEDIR:?}/${TEST_DEFAULT_FILENAME}" 2>&1 | sed 's/^/# unset_current_state: rm /'
rm -vrf "${TEST_GPG_HOMEDIR:?}/${TEST_SECOND_FILENAME}" 2>&1 | sed 's/^/# unset_current_state: rm /'
rm -vrf "${TEST_GPG_HOMEDIR:?}/${TEST_THIRD_FILENAME}" 2>&1 | sed 's/^/# unset_current_state: rm /'
# return to the base dir:
cd "$SECRET_PROJECT_ROOT" || exit 1
}
# show output if we wind up manually removing the test output file in a trap
trap 'if [[ -f "$TEST_GPG_OUTPUT_FILE" ]]; then if [[ -n "$SECRETS_TEST_VERBOSE" ]]; then echo "git-secret: test: cleaning up: $TEST_GPG_OUTPUT_FILE"; fi; rm -f "$TEST_GPG_OUTPUT_FILE"; fi;' EXIT

View File

@ -63,7 +63,7 @@ function teardown {
@test "run 'changes' with source file missing" {
local password=$(test_user_password "$TEST_DEFAULT_USER")
rm "$FILE_TO_HIDE"
rm "$FILE_TO_HIDE" || _abort "error removing: $FILE_TO_HIDE"
run git secret changes -d "$TEST_GPG_HOMEDIR" -p "$password" "$FILE_TO_HIDE"
[ "$status" -ne 0 ]
@ -72,7 +72,7 @@ function teardown {
@test "run 'changes' with hidden file missing" {
local password=$(test_user_password "$TEST_DEFAULT_USER")
local encrypted_file=$(_get_encrypted_filename "$FILE_TO_HIDE")
rm "$encrypted_file"
rm "$encrypted_file" || _abort "error removing: $encrypted_file"
run git secret changes -d "$TEST_GPG_HOMEDIR" -p "$password" "$FILE_TO_HIDE"
[ "$status" -ne 0 ]
@ -167,6 +167,8 @@ function teardown {
local num_lines=$(echo "$output" | wc -l)
[[ "$num_lines" -eq 1 ]]
rm -f "$THIRD_FILE_TO_HIDE"
}
@test "run 'changes' with bad arg" {

View File

@ -111,8 +111,9 @@ function teardown {
run git secret hide
[ "$status" -eq 0 ]
# cd back
# cd back and clean up
cd ".."
rm -rf "$root_dir"
}
@test "run 'hide' with missing file" {
@ -286,6 +287,8 @@ function teardown {
[[ "$output" == *"removing unencrypted files"* ]]
[[ "$output" == *"$FILE_TO_HIDE"* ]]
[[ "$output" == *"$second_file"* ]]
rm -rf "$root_dir"
}

View File

@ -4,7 +4,7 @@
INSTALL_BASE="${TMPDIR}/git-secret-test-install"
@test "install git-secret to '$TMPDIR'" {
@test "install git-secret to DESTDIR='$INSTALL_BASE'" {
rm -f "${INSTALL_BASE}/usr/bin/git-secret"
@ -14,5 +14,7 @@ INSTALL_BASE="${TMPDIR}/git-secret-test-install"
DESTDIR="${INSTALL_BASE}" run make install
[ -x "${INSTALL_BASE}/usr/bin/git-secret" ]
rm -rf "${INSTALL_BASE}"
}

View File

@ -105,7 +105,7 @@ function _has_line {
[ -f "$encrypted_file" ]
# Cleaning up:
rm -r "$folder"
rm -rf "$folder"
}

View File

@ -4,25 +4,26 @@
set -e
#TEST_DIR="/tmp/tempdir with spaces"
TEST_DIR="/tmp/tempdir"
TEST_DIR=/tmp/git-secret-test
rm -rf "${TEST_DIR}"
mkdir "${TEST_DIR}"
echo "Created dir: ${TEST_DIR}"
chmod 0700 "${TEST_DIR}"
(
cd "${TEST_DIR}"
# test with non-standard SECRETS_DIR (normally .gitsecret) and SECRETS_EXTENSION (normally .secret)
export SECRETS_DIR=.gitsecret-testdir
export SECRETS_EXTENSION=.secret2
#export SECRETS_VERBOSE=''
export SECRETS_DIR=.gitsec
export SECRETS_EXTENSION=.sec
#export TMPDIR="${TEST_DIR}" # uncommenting this line seems to cause #451
#echo "# TMPDIR is $TMPDIR"
export TMPDIR="${TEST_DIR}"
echo "# TMPDIR is $TMPDIR"
# bats expects diagnostic lines to be sent to fd 3, matching regex '^ #' (IE, like: `echo '# message here' >&3`)
# bats ... 3>&1 shows diagnostic output when errors occur.
# bats expects diagnostic lines to be sent to fd 3, matching regex '^ #'
# (IE, like: `echo '# message here' >&3`).
# bats ... 3>&1 shows diagnostic output
bats "${SECRET_PROJECT_ROOT}/tests/" 3>&1
)