Skip to content

Fix #524: add writability check for TFENV_CONFIG_DIR before install lock #527

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.

Sign up for GitHub

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

Merged
Zordrak merged 6 commits into from
Apr 28, 2026
Merged

Fix #524: add writability check for TFENV_CONFIG_DIR before install lock #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
28f06a0
Fix #524: add writability check for TFENV_CONFIG_DIR before install lock
Zordrak Apr 28, 2026
6631ecb
Fix BSD script(1) quoting in install_lock tests 3 and 4
Zordrak Apr 28, 2026
a9ba09e
Replace script(1) PTY hack with TFENV_FORCE_INTERACTIVE testing seam
Zordrak Apr 28, 2026
904473d
Fix: let mkdir -p fail silently so interactive fallback fires when co…
Zordrak Apr 28, 2026
ce0722b
Improve config dir error handling with precise diagnostics
Zordrak Apr 28, 2026
437a260
Add complete test coverage for config dir diagnostics
Zordrak Apr 28, 2026
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
55 changes: 51 additions & 4 deletions libexec/tfenv-install
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,57 @@ if [ "${TFENV_SKIP_REMOTE_CHECK:-0}" -eq 0 ]; then
[ -n "${remote_version}" ] && version="${remote_version}" || log 'error' "No versions matching '${requested:-$version}' found in remote";
fi;

# Attempt to create the config dir and capture any error for precise diagnostics.
# A read-only config dir (e.g. Homebrew Cellar on CI runners) causes mkdir-based
# locking to fail with EACCES, previously misreported as lock contention (#524).
declare mkdir_err;
mkdir_err="$(mkdir -p "${TFENV_CONFIG_DIR}" 2>&1)";
declare mkdir_rc="${?}";

declare config_dir_reason="";

if [ "${mkdir_rc}" -ne 0 ]; then
# mkdir failed — determine exactly why and report precisely
if [ -e "${TFENV_CONFIG_DIR}" ] && [ ! -d "${TFENV_CONFIG_DIR}" ]; then
config_dir_reason="${TFENV_CONFIG_DIR} exists but is not a directory";
else
# Walk up the path to find the first ancestor that exists
declare check_path="${TFENV_CONFIG_DIR}";
while [ ! -e "${check_path}" ]; do
check_path="$(dirname "${check_path}")";
done;
if [ ! -d "${check_path}" ]; then
config_dir_reason="ancestor ${check_path} exists but is not a directory";
elif [ ! -w "${check_path}" ]; then
config_dir_reason="cannot create ${TFENV_CONFIG_DIR} — parent ${check_path} is not writable (owner: $(stat -c '%U' "${check_path}" 2>/dev/null || stat -f '%Su' "${check_path}" 2>/dev/null || echo 'unknown'))";
else
# Writable ancestor exists but mkdir still failed — relay the actual error
config_dir_reason="mkdir failed: ${mkdir_err}";
fi;
fi;
elif [ ! -w "${TFENV_CONFIG_DIR}" ]; then
# Directory exists but is not writable by this user
config_dir_reason="exists but is not writable (owner: $(stat -c '%U' "${TFENV_CONFIG_DIR}" 2>/dev/null || stat -f '%Su' "${TFENV_CONFIG_DIR}" 2>/dev/null || echo 'unknown'), mode: $(stat -c '%a' "${TFENV_CONFIG_DIR}" 2>/dev/null || stat -f '%Lp' "${TFENV_CONFIG_DIR}" 2>/dev/null || echo 'unknown'))";
fi;

if [ -n "${config_dir_reason}" ]; then
log 'info' "TFENV_CONFIG_DIR (${TFENV_CONFIG_DIR}): ${config_dir_reason}";
if [[ "${TFENV_FORCE_INTERACTIVE:-}" == "1" || -t 0 ]]; then
echo "tfenv: Use ~/.tfenv instead? [y/N]" >&2;
read -r answer;
if [[ "${answer}" =~ ^[Yy]$ ]]; then
TFENV_CONFIG_DIR="${HOME}/.tfenv";
export TFENV_CONFIG_DIR;
mkdir -p "${TFENV_CONFIG_DIR}" || log 'error' "Failed to create ${TFENV_CONFIG_DIR}";
log 'info' "Falling back to TFENV_CONFIG_DIR=${TFENV_CONFIG_DIR}";
else
log 'error' "TFENV_CONFIG_DIR (${TFENV_CONFIG_DIR}): ${config_dir_reason}. Set TFENV_CONFIG_DIR to a writable path (e.g. export TFENV_CONFIG_DIR=\"\${HOME}/.tfenv\")";
fi;
else
log 'error' "TFENV_CONFIG_DIR (${TFENV_CONFIG_DIR}): ${config_dir_reason}. Set TFENV_CONFIG_DIR to a writable path (e.g. export TFENV_CONFIG_DIR=\"\${HOME}/.tfenv\")";
fi;
fi;

dst_path="${TFENV_CONFIG_DIR}/versions/${version}";
if [ -f "${dst_path}/terraform" ]; then
echo "Terraform v${version} is already installed";
Expand All @@ -98,10 +149,6 @@ declare lockdir="${TFENV_CONFIG_DIR}/.install-lock-${version}";
declare lock_retries=0;
declare lock_max_retries=60;

# Ensure the config dir exists so the lock mkdir can succeed on a first install.
# Without this, mkdir fails with ENOENT and the loop below misreads it as contention.
mkdir -p "${TFENV_CONFIG_DIR}" || log 'error' "Failed to create ${TFENV_CONFIG_DIR}";

cleanup_lock() {
rmdir "${lockdir}" 2>/dev/null || true;
};
Expand Down
258 changes: 258 additions & 0 deletions test/test_install_lock.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,258 @@
#!/usr/bin/env bash

# Source common test setup
source "$(dirname "${0}")/test_common.sh";

#####################
# Begin Script Body #
#####################

declare -a errors=();
declare test_version='1.6.1';

log 'info' '### Test Suite: install_lock';

##############################################################################
# Test 1: Install with non-existent TFENV_CONFIG_DIR (regression test #487/#525)
##############################################################################
log 'info' '## install_lock: install with non-existent TFENV_CONFIG_DIR';
cleanup || log 'error' 'Cleanup failed?!';
(
declare fresh_config_dir;
fresh_config_dir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_test')";
rm -rf "${fresh_config_dir}";
# Confirm it does not exist
[ ! -d "${fresh_config_dir}" ] || exit 1;
TFENV_CONFIG_DIR="${fresh_config_dir}" tfenv install "${test_version}" || exit 1;
[ -f "${fresh_config_dir}/versions/${test_version}/terraform" ] || exit 1;
rm -rf "${fresh_config_dir}";
) && log 'info' '## install_lock: non-existent config dir passed' \
|| error_and_proceed 'install with non-existent TFENV_CONFIG_DIR failed';

##############################################################################
# Test 2: Install with read-only TFENV_CONFIG_DIR, non-interactive (#524)
##############################################################################
log 'info' '## install_lock: read-only config dir, non-interactive';
cleanup || log 'error' 'Cleanup failed?!';
(
declare ro_dir;
ro_dir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_ro')";
chmod 555 "${ro_dir}";
declare output;
output="$(TFENV_CONFIG_DIR="${ro_dir}" tfenv install "${test_version}" < /dev/null 2>&1)";
declare rc="${?}";
chmod 755 "${ro_dir}";
rm -rf "${ro_dir}";
[ "${rc}" -ne 0 ] || exit 1;
echo "${output}" | grep -q 'not writable' || exit 1;
# Verify diagnostics include ownership info
echo "${output}" | grep -q 'owner:' || exit 1;
) && log 'info' '## install_lock: read-only non-interactive passed' \
|| error_and_proceed 'read-only TFENV_CONFIG_DIR non-interactive did not fail with expected error';

##############################################################################
# Test 3: Install with read-only TFENV_CONFIG_DIR, interactive fallback accepted
##############################################################################
log 'info' '## install_lock: read-only config dir, interactive fallback accepted';
cleanup || log 'error' 'Cleanup failed?!';
(
declare ro_dir;
ro_dir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_ro2')";
chmod 555 "${ro_dir}";
declare fallback_home;
fallback_home="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_home')";
# Use TFENV_FORCE_INTERACTIVE to bypass the [[ -t 0 ]] check so we can
# pipe input directly without needing script(1) and PTY allocation, which
# behaves differently on GNU vs BSD and is unreliable in CI.
declare output;
output="$(printf 'y\n' | TFENV_FORCE_INTERACTIVE=1 TFENV_CONFIG_DIR="${ro_dir}" HOME="${fallback_home}" "${TFENV_ROOT}/bin/tfenv" install "${test_version}" 2>&1)";
declare rc="${?}";
chmod 755 "${ro_dir}";
rm -rf "${ro_dir}";
if [ "${rc}" -ne 0 ]; then
echo "UNEXPECTED FAILURE output: ${output}" >&2;
rm -rf "${fallback_home}";
exit 1;
fi;
[ -f "${fallback_home}/.tfenv/versions/${test_version}/terraform" ] || {
echo "terraform binary not found in fallback dir" >&2;
rm -rf "${fallback_home}";
exit 1;
};
rm -rf "${fallback_home}";
) && log 'info' '## install_lock: interactive fallback accepted passed' \
|| error_and_proceed 'read-only TFENV_CONFIG_DIR interactive fallback (y) did not install to ~/.tfenv';

##############################################################################
# Test 4: Install with read-only TFENV_CONFIG_DIR, interactive fallback declined
##############################################################################
log 'info' '## install_lock: read-only config dir, interactive fallback declined';
cleanup || log 'error' 'Cleanup failed?!';
(
declare ro_dir;
ro_dir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_ro3')";
chmod 555 "${ro_dir}";
declare output;
output="$(printf 'n\n' | TFENV_FORCE_INTERACTIVE=1 TFENV_CONFIG_DIR="${ro_dir}" "${TFENV_ROOT}/bin/tfenv" install "${test_version}" 2>&1)";
declare rc="${?}";
chmod 755 "${ro_dir}";
rm -rf "${ro_dir}";
[ "${rc}" -ne 0 ] || exit 1;
) && log 'info' '## install_lock: interactive fallback declined passed' \
|| error_and_proceed 'read-only TFENV_CONFIG_DIR interactive fallback (n) did not fail';

##############################################################################
# Test 5: Non-existent config dir inside non-writable parent, non-interactive
##############################################################################
log 'info' '## install_lock: non-existent config dir, non-writable parent, non-interactive';
cleanup || log 'error' 'Cleanup failed?!';
(
declare readonly_parent;
readonly_parent="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_roprt')";
chmod 555 "${readonly_parent}";
declare output;
output="$(TFENV_CONFIG_DIR="${readonly_parent}/tfenv-config" tfenv install "${test_version}" < /dev/null 2>&1)";
declare rc="${?}";
chmod 755 "${readonly_parent}";
rm -rf "${readonly_parent}";
[ "${rc}" -ne 0 ] || exit 1;
echo "${output}" | grep -q 'not writable' || exit 1;
# Verify diagnostics include ownership info and identify the parent
echo "${output}" | grep -q 'owner:' || exit 1;
echo "${output}" | grep -q 'parent' || exit 1;
) && log 'info' '## install_lock: non-existent config dir, non-writable parent, non-interactive passed' \
|| error_and_proceed 'non-existent config dir inside non-writable parent (non-interactive) did not fail with expected error';

##############################################################################
# Test 6: Non-existent config dir inside non-writable parent, interactive fallback accepted
##############################################################################
log 'info' '## install_lock: non-existent config dir, non-writable parent, interactive fallback';
cleanup || log 'error' 'Cleanup failed?!';
(
declare readonly_parent;
readonly_parent="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_roprt2')";
chmod 555 "${readonly_parent}";
declare fallback_home;
fallback_home="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_home2')";
declare output;
output="$(printf 'y\n' | TFENV_FORCE_INTERACTIVE=1 TFENV_CONFIG_DIR="${readonly_parent}/tfenv-config" HOME="${fallback_home}" "${TFENV_ROOT}/bin/tfenv" install "${test_version}" 2>&1)";
declare rc="${?}";
chmod 755 "${readonly_parent}";
rm -rf "${readonly_parent}";
if [ "${rc}" -ne 0 ]; then
echo "UNEXPECTED FAILURE output: ${output}" >&2;
rm -rf "${fallback_home}";
exit 1;
fi;
[ -f "${fallback_home}/.tfenv/versions/${test_version}/terraform" ] || {
echo "terraform binary not found in fallback dir" >&2;
rm -rf "${fallback_home}";
exit 1;
};
rm -rf "${fallback_home}";
) && log 'info' '## install_lock: non-existent config dir, non-writable parent, interactive fallback passed' \
|| error_and_proceed 'non-existent config dir inside non-writable parent (interactive y) did not install to ~/.tfenv';

##############################################################################
# Test 7: Lock cleanup on normal exit
##############################################################################
log 'info' '## install_lock: lock cleanup after successful install';
cleanup || log 'error' 'Cleanup failed?!';
(
tfenv install "${test_version}" || exit 1;
# Verify no install lock directories remain
declare lock_count;
lock_count="$(find "${TFENV_CONFIG_DIR}" -maxdepth 1 -name '.install-lock-*' -type d 2>/dev/null | wc -l)";
[ "${lock_count}" -eq 0 ] || exit 1;
) && log 'info' '## install_lock: lock cleanup passed' \
|| error_and_proceed 'install lock directory was not cleaned up after successful install';

##############################################################################
# Test 8: Config dir path exists but is a FILE, not a directory
##############################################################################
log 'info' '## install_lock: config dir is a file, not a directory';
cleanup || log 'error' 'Cleanup failed?!';
(
declare tmpdir;
tmpdir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_notdir')";
declare fakedir="${tmpdir}/fakedir";
touch "${fakedir}";
declare output;
output="$(TFENV_CONFIG_DIR="${fakedir}" tfenv install "${test_version}" < /dev/null 2>&1)";
declare rc="${?}";
rm -rf "${tmpdir}";
[ "${rc}" -ne 0 ] || exit 1;
echo "${output}" | grep -q 'not a directory' || exit 1;
) && log 'info' '## install_lock: config dir is a file passed' \
|| error_and_proceed 'config dir that is a file did not fail with expected error';

##############################################################################
# Test 9: Ancestor in path is a file, not a directory
##############################################################################
log 'info' '## install_lock: ancestor in path is a file, not a directory';
cleanup || log 'error' 'Cleanup failed?!';
(
declare tmpdir;
tmpdir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_ancestor')";
touch "${tmpdir}/blocker";
declare output;
output="$(TFENV_CONFIG_DIR="${tmpdir}/blocker/deep/config" tfenv install "${test_version}" < /dev/null 2>&1)";
declare rc="${?}";
rm -rf "${tmpdir}";
[ "${rc}" -ne 0 ] || exit 1;
echo "${output}" | grep -q 'ancestor' || exit 1;
echo "${output}" | grep -q 'not a directory' || exit 1;
) && log 'info' '## install_lock: ancestor is a file passed' \
|| error_and_proceed 'ancestor that is a file did not fail with expected error';

##############################################################################
# Test 10: Interactive fallback to ~/.tfenv fails when HOME is non-writable
##############################################################################
log 'info' '## install_lock: interactive fallback fails when HOME is non-writable';
cleanup || log 'error' 'Cleanup failed?!';
(
declare ro_config;
ro_config="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_rocfg')";
chmod 555 "${ro_config}";
declare ro_home;
ro_home="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_rohome')";
chmod 555 "${ro_home}";
declare output;
output="$(printf 'y\n' | TFENV_FORCE_INTERACTIVE=1 TFENV_CONFIG_DIR="${ro_config}" HOME="${ro_home}" "${TFENV_ROOT}/bin/tfenv" install "${test_version}" 2>&1)";
declare rc="${?}";
chmod 755 "${ro_config}";
chmod 755 "${ro_home}";
rm -rf "${ro_config}" "${ro_home}";
[ "${rc}" -ne 0 ] || exit 1;
echo "${output}" | grep -q 'Failed to create' || exit 1;
) && log 'info' '## install_lock: interactive fallback fails with non-writable HOME passed' \
|| error_and_proceed 'interactive fallback with non-writable HOME did not fail with expected error';

##############################################################################
# Test 11: Non-existent config dir, non-writable parent, interactive decline
##############################################################################
log 'info' '## install_lock: non-existent config dir, non-writable parent, interactive decline';
cleanup || log 'error' 'Cleanup failed?!';
(
declare readonly_parent;
readonly_parent="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_roprt3')";
chmod 555 "${readonly_parent}";
declare output;
output="$(printf 'n\n' | TFENV_FORCE_INTERACTIVE=1 TFENV_CONFIG_DIR="${readonly_parent}/tfenv-config" "${TFENV_ROOT}/bin/tfenv" install "${test_version}" 2>&1)";
declare rc="${?}";
chmod 755 "${readonly_parent}";
rm -rf "${readonly_parent}";
[ "${rc}" -ne 0 ] || exit 1;
) && log 'info' '## install_lock: non-writable parent, interactive decline passed' \
|| error_and_proceed 'non-existent config dir, non-writable parent, interactive decline did not fail';

##############################################################################
# Test 12: Lock cleanup on interrupted exit
# Skipped — reliably testing signal-handler cleanup (SIGINT/SIGTERM during
# install) is inherently racy and would produce flaky CI results. The
# cleanup_lock trap is validated by manual testing and code review.
##############################################################################
log 'info' '## install_lock: signal cleanup — SKIPPED (inherently racy, see comment)';

finish_tests 'install_lock';
# vim: set syntax=bash tabstop=2 softtabstop=2 shiftwidth=2 expandtab smarttab :
Loading