Skip to content

Commit 7dadbcf

Browse files
ZordrakZordrak
authored
Merge pull request #527 from tfutils/fix/524-readonly-config-dir
Fix #524: add writability check for TFENV_CONFIG_DIR before install lock
2 parents 6b7e0b8 + 437a260 commit 7dadbcf

2 files changed

Lines changed: 309 additions & 4 deletions

File tree

‎libexec/tfenv-install‎

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,57 @@ if [ "${TFENV_SKIP_REMOTE_CHECK:-0}" -eq 0 ]; then
8686
[ -n "${remote_version}" ] && version="${remote_version}" || log 'error' "No versions matching '${requested:-$version}' found in remote";
8787
fi;
8888

89+
# Attempt to create the config dir and capture any error for precise diagnostics.
90+
# A read-only config dir (e.g. Homebrew Cellar on CI runners) causes mkdir-based
91+
# locking to fail with EACCES, previously misreported as lock contention (#524).
92+
declare mkdir_err;
93+
mkdir_err="$(mkdir -p "${TFENV_CONFIG_DIR}" 2>&1)";
94+
declare mkdir_rc="${?}";
95+
96+
declare config_dir_reason="";
97+
98+
if [ "${mkdir_rc}" -ne 0 ]; then
99+
# mkdir failed — determine exactly why and report precisely
100+
if [ -e "${TFENV_CONFIG_DIR}" ] && [ ! -d "${TFENV_CONFIG_DIR}" ]; then
101+
config_dir_reason="${TFENV_CONFIG_DIR} exists but is not a directory";
102+
else
103+
# Walk up the path to find the first ancestor that exists
104+
declare check_path="${TFENV_CONFIG_DIR}";
105+
while [ ! -e "${check_path}" ]; do
106+
check_path="$(dirname "${check_path}")";
107+
done;
108+
if [ ! -d "${check_path}" ]; then
109+
config_dir_reason="ancestor ${check_path} exists but is not a directory";
110+
elif [ ! -w "${check_path}" ]; then
111+
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'))";
112+
else
113+
# Writable ancestor exists but mkdir still failed — relay the actual error
114+
config_dir_reason="mkdir failed: ${mkdir_err}";
115+
fi;
116+
fi;
117+
elif [ ! -w "${TFENV_CONFIG_DIR}" ]; then
118+
# Directory exists but is not writable by this user
119+
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'))";
120+
fi;
121+
122+
if [ -n "${config_dir_reason}" ]; then
123+
log 'info' "TFENV_CONFIG_DIR (${TFENV_CONFIG_DIR}): ${config_dir_reason}";
124+
if [[ "${TFENV_FORCE_INTERACTIVE:-}" == "1" || -t 0 ]]; then
125+
echo "tfenv: Use ~/.tfenv instead? [y/N]" >&2;
126+
read -r answer;
127+
if [[ "${answer}" =~ ^[Yy]$ ]]; then
128+
TFENV_CONFIG_DIR="${HOME}/.tfenv";
129+
export TFENV_CONFIG_DIR;
130+
mkdir -p "${TFENV_CONFIG_DIR}" || log 'error' "Failed to create ${TFENV_CONFIG_DIR}";
131+
log 'info' "Falling back to TFENV_CONFIG_DIR=${TFENV_CONFIG_DIR}";
132+
else
133+
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\")";
134+
fi;
135+
else
136+
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\")";
137+
fi;
138+
fi;
139+
89140
dst_path="${TFENV_CONFIG_DIR}/versions/${version}";
90141
if [ -f "${dst_path}/terraform" ]; then
91142
echo "Terraform v${version} is already installed";
@@ -98,10 +149,6 @@ declare lockdir="${TFENV_CONFIG_DIR}/.install-lock-${version}";
98149
declare lock_retries=0;
99150
declare lock_max_retries=60;
100151

101-
# Ensure the config dir exists so the lock mkdir can succeed on a first install.
102-
# Without this, mkdir fails with ENOENT and the loop below misreads it as contention.
103-
mkdir -p "${TFENV_CONFIG_DIR}" || log 'error' "Failed to create ${TFENV_CONFIG_DIR}";
104-
105152
cleanup_lock() {
106153
rmdir "${lockdir}" 2>/dev/null || true;
107154
};

‎test/test_install_lock.sh‎

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
#!/usr/bin/env bash
2+
3+
# Source common test setup
4+
source "$(dirname "${0}")/test_common.sh";
5+
6+
#####################
7+
# Begin Script Body #
8+
#####################
9+
10+
declare -a errors=();
11+
declare test_version='1.6.1';
12+
13+
log 'info' '### Test Suite: install_lock';
14+
15+
##############################################################################
16+
# Test 1: Install with non-existent TFENV_CONFIG_DIR (regression test #487/#525)
17+
##############################################################################
18+
log 'info' '## install_lock: install with non-existent TFENV_CONFIG_DIR';
19+
cleanup || log 'error' 'Cleanup failed?!';
20+
(
21+
declare fresh_config_dir;
22+
fresh_config_dir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_test')";
23+
rm -rf "${fresh_config_dir}";
24+
# Confirm it does not exist
25+
[ ! -d "${fresh_config_dir}" ] || exit 1;
26+
TFENV_CONFIG_DIR="${fresh_config_dir}" tfenv install "${test_version}" || exit 1;
27+
[ -f "${fresh_config_dir}/versions/${test_version}/terraform" ] || exit 1;
28+
rm -rf "${fresh_config_dir}";
29+
) && log 'info' '## install_lock: non-existent config dir passed' \
30+
|| error_and_proceed 'install with non-existent TFENV_CONFIG_DIR failed';
31+
32+
##############################################################################
33+
# Test 2: Install with read-only TFENV_CONFIG_DIR, non-interactive (#524)
34+
##############################################################################
35+
log 'info' '## install_lock: read-only config dir, non-interactive';
36+
cleanup || log 'error' 'Cleanup failed?!';
37+
(
38+
declare ro_dir;
39+
ro_dir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_ro')";
40+
chmod 555 "${ro_dir}";
41+
declare output;
42+
output="$(TFENV_CONFIG_DIR="${ro_dir}" tfenv install "${test_version}" < /dev/null 2>&1)";
43+
declare rc="${?}";
44+
chmod 755 "${ro_dir}";
45+
rm -rf "${ro_dir}";
46+
[ "${rc}" -ne 0 ] || exit 1;
47+
echo "${output}" | grep -q 'not writable' || exit 1;
48+
# Verify diagnostics include ownership info
49+
echo "${output}" | grep -q 'owner:' || exit 1;
50+
) && log 'info' '## install_lock: read-only non-interactive passed' \
51+
|| error_and_proceed 'read-only TFENV_CONFIG_DIR non-interactive did not fail with expected error';
52+
53+
##############################################################################
54+
# Test 3: Install with read-only TFENV_CONFIG_DIR, interactive fallback accepted
55+
##############################################################################
56+
log 'info' '## install_lock: read-only config dir, interactive fallback accepted';
57+
cleanup || log 'error' 'Cleanup failed?!';
58+
(
59+
declare ro_dir;
60+
ro_dir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_ro2')";
61+
chmod 555 "${ro_dir}";
62+
declare fallback_home;
63+
fallback_home="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_home')";
64+
# Use TFENV_FORCE_INTERACTIVE to bypass the [[ -t 0 ]] check so we can
65+
# pipe input directly without needing script(1) and PTY allocation, which
66+
# behaves differently on GNU vs BSD and is unreliable in CI.
67+
declare output;
68+
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)";
69+
declare rc="${?}";
70+
chmod 755 "${ro_dir}";
71+
rm -rf "${ro_dir}";
72+
if [ "${rc}" -ne 0 ]; then
73+
echo "UNEXPECTED FAILURE output: ${output}" >&2;
74+
rm -rf "${fallback_home}";
75+
exit 1;
76+
fi;
77+
[ -f "${fallback_home}/.tfenv/versions/${test_version}/terraform" ] || {
78+
echo "terraform binary not found in fallback dir" >&2;
79+
rm -rf "${fallback_home}";
80+
exit 1;
81+
};
82+
rm -rf "${fallback_home}";
83+
) && log 'info' '## install_lock: interactive fallback accepted passed' \
84+
|| error_and_proceed 'read-only TFENV_CONFIG_DIR interactive fallback (y) did not install to ~/.tfenv';
85+
86+
##############################################################################
87+
# Test 4: Install with read-only TFENV_CONFIG_DIR, interactive fallback declined
88+
##############################################################################
89+
log 'info' '## install_lock: read-only config dir, interactive fallback declined';
90+
cleanup || log 'error' 'Cleanup failed?!';
91+
(
92+
declare ro_dir;
93+
ro_dir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_ro3')";
94+
chmod 555 "${ro_dir}";
95+
declare output;
96+
output="$(printf 'n\n' | TFENV_FORCE_INTERACTIVE=1 TFENV_CONFIG_DIR="${ro_dir}" "${TFENV_ROOT}/bin/tfenv" install "${test_version}" 2>&1)";
97+
declare rc="${?}";
98+
chmod 755 "${ro_dir}";
99+
rm -rf "${ro_dir}";
100+
[ "${rc}" -ne 0 ] || exit 1;
101+
) && log 'info' '## install_lock: interactive fallback declined passed' \
102+
|| error_and_proceed 'read-only TFENV_CONFIG_DIR interactive fallback (n) did not fail';
103+
104+
##############################################################################
105+
# Test 5: Non-existent config dir inside non-writable parent, non-interactive
106+
##############################################################################
107+
log 'info' '## install_lock: non-existent config dir, non-writable parent, non-interactive';
108+
cleanup || log 'error' 'Cleanup failed?!';
109+
(
110+
declare readonly_parent;
111+
readonly_parent="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_roprt')";
112+
chmod 555 "${readonly_parent}";
113+
declare output;
114+
output="$(TFENV_CONFIG_DIR="${readonly_parent}/tfenv-config" tfenv install "${test_version}" < /dev/null 2>&1)";
115+
declare rc="${?}";
116+
chmod 755 "${readonly_parent}";
117+
rm -rf "${readonly_parent}";
118+
[ "${rc}" -ne 0 ] || exit 1;
119+
echo "${output}" | grep -q 'not writable' || exit 1;
120+
# Verify diagnostics include ownership info and identify the parent
121+
echo "${output}" | grep -q 'owner:' || exit 1;
122+
echo "${output}" | grep -q 'parent' || exit 1;
123+
) && log 'info' '## install_lock: non-existent config dir, non-writable parent, non-interactive passed' \
124+
|| error_and_proceed 'non-existent config dir inside non-writable parent (non-interactive) did not fail with expected error';
125+
126+
##############################################################################
127+
# Test 6: Non-existent config dir inside non-writable parent, interactive fallback accepted
128+
##############################################################################
129+
log 'info' '## install_lock: non-existent config dir, non-writable parent, interactive fallback';
130+
cleanup || log 'error' 'Cleanup failed?!';
131+
(
132+
declare readonly_parent;
133+
readonly_parent="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_roprt2')";
134+
chmod 555 "${readonly_parent}";
135+
declare fallback_home;
136+
fallback_home="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_home2')";
137+
declare output;
138+
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)";
139+
declare rc="${?}";
140+
chmod 755 "${readonly_parent}";
141+
rm -rf "${readonly_parent}";
142+
if [ "${rc}" -ne 0 ]; then
143+
echo "UNEXPECTED FAILURE output: ${output}" >&2;
144+
rm -rf "${fallback_home}";
145+
exit 1;
146+
fi;
147+
[ -f "${fallback_home}/.tfenv/versions/${test_version}/terraform" ] || {
148+
echo "terraform binary not found in fallback dir" >&2;
149+
rm -rf "${fallback_home}";
150+
exit 1;
151+
};
152+
rm -rf "${fallback_home}";
153+
) && log 'info' '## install_lock: non-existent config dir, non-writable parent, interactive fallback passed' \
154+
|| error_and_proceed 'non-existent config dir inside non-writable parent (interactive y) did not install to ~/.tfenv';
155+
156+
##############################################################################
157+
# Test 7: Lock cleanup on normal exit
158+
##############################################################################
159+
log 'info' '## install_lock: lock cleanup after successful install';
160+
cleanup || log 'error' 'Cleanup failed?!';
161+
(
162+
tfenv install "${test_version}" || exit 1;
163+
# Verify no install lock directories remain
164+
declare lock_count;
165+
lock_count="$(find "${TFENV_CONFIG_DIR}" -maxdepth 1 -name '.install-lock-*' -type d 2>/dev/null | wc -l)";
166+
[ "${lock_count}" -eq 0 ] || exit 1;
167+
) && log 'info' '## install_lock: lock cleanup passed' \
168+
|| error_and_proceed 'install lock directory was not cleaned up after successful install';
169+
170+
##############################################################################
171+
# Test 8: Config dir path exists but is a FILE, not a directory
172+
##############################################################################
173+
log 'info' '## install_lock: config dir is a file, not a directory';
174+
cleanup || log 'error' 'Cleanup failed?!';
175+
(
176+
declare tmpdir;
177+
tmpdir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_notdir')";
178+
declare fakedir="${tmpdir}/fakedir";
179+
touch "${fakedir}";
180+
declare output;
181+
output="$(TFENV_CONFIG_DIR="${fakedir}" tfenv install "${test_version}" < /dev/null 2>&1)";
182+
declare rc="${?}";
183+
rm -rf "${tmpdir}";
184+
[ "${rc}" -ne 0 ] || exit 1;
185+
echo "${output}" | grep -q 'not a directory' || exit 1;
186+
) && log 'info' '## install_lock: config dir is a file passed' \
187+
|| error_and_proceed 'config dir that is a file did not fail with expected error';
188+
189+
##############################################################################
190+
# Test 9: Ancestor in path is a file, not a directory
191+
##############################################################################
192+
log 'info' '## install_lock: ancestor in path is a file, not a directory';
193+
cleanup || log 'error' 'Cleanup failed?!';
194+
(
195+
declare tmpdir;
196+
tmpdir="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_ancestor')";
197+
touch "${tmpdir}/blocker";
198+
declare output;
199+
output="$(TFENV_CONFIG_DIR="${tmpdir}/blocker/deep/config" tfenv install "${test_version}" < /dev/null 2>&1)";
200+
declare rc="${?}";
201+
rm -rf "${tmpdir}";
202+
[ "${rc}" -ne 0 ] || exit 1;
203+
echo "${output}" | grep -q 'ancestor' || exit 1;
204+
echo "${output}" | grep -q 'not a directory' || exit 1;
205+
) && log 'info' '## install_lock: ancestor is a file passed' \
206+
|| error_and_proceed 'ancestor that is a file did not fail with expected error';
207+
208+
##############################################################################
209+
# Test 10: Interactive fallback to ~/.tfenv fails when HOME is non-writable
210+
##############################################################################
211+
log 'info' '## install_lock: interactive fallback fails when HOME is non-writable';
212+
cleanup || log 'error' 'Cleanup failed?!';
213+
(
214+
declare ro_config;
215+
ro_config="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_rocfg')";
216+
chmod 555 "${ro_config}";
217+
declare ro_home;
218+
ro_home="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_rohome')";
219+
chmod 555 "${ro_home}";
220+
declare output;
221+
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)";
222+
declare rc="${?}";
223+
chmod 755 "${ro_config}";
224+
chmod 755 "${ro_home}";
225+
rm -rf "${ro_config}" "${ro_home}";
226+
[ "${rc}" -ne 0 ] || exit 1;
227+
echo "${output}" | grep -q 'Failed to create' || exit 1;
228+
) && log 'info' '## install_lock: interactive fallback fails with non-writable HOME passed' \
229+
|| error_and_proceed 'interactive fallback with non-writable HOME did not fail with expected error';
230+
231+
##############################################################################
232+
# Test 11: Non-existent config dir, non-writable parent, interactive decline
233+
##############################################################################
234+
log 'info' '## install_lock: non-existent config dir, non-writable parent, interactive decline';
235+
cleanup || log 'error' 'Cleanup failed?!';
236+
(
237+
declare readonly_parent;
238+
readonly_parent="$(mktemp -d 2>/dev/null || mktemp -d -t 'tfenv_lock_roprt3')";
239+
chmod 555 "${readonly_parent}";
240+
declare output;
241+
output="$(printf 'n\n' | TFENV_FORCE_INTERACTIVE=1 TFENV_CONFIG_DIR="${readonly_parent}/tfenv-config" "${TFENV_ROOT}/bin/tfenv" install "${test_version}" 2>&1)";
242+
declare rc="${?}";
243+
chmod 755 "${readonly_parent}";
244+
rm -rf "${readonly_parent}";
245+
[ "${rc}" -ne 0 ] || exit 1;
246+
) && log 'info' '## install_lock: non-writable parent, interactive decline passed' \
247+
|| error_and_proceed 'non-existent config dir, non-writable parent, interactive decline did not fail';
248+
249+
##############################################################################
250+
# Test 12: Lock cleanup on interrupted exit
251+
# Skipped — reliably testing signal-handler cleanup (SIGINT/SIGTERM during
252+
# install) is inherently racy and would produce flaky CI results. The
253+
# cleanup_lock trap is validated by manual testing and code review.
254+
##############################################################################
255+
log 'info' '## install_lock: signal cleanup — SKIPPED (inherently racy, see comment)';
256+
257+
finish_tests 'install_lock';
258+
# vim: set syntax=bash tabstop=2 softtabstop=2 shiftwidth=2 expandtab smarttab :

0 commit comments

Comments
 (0)