Skip to content

Commit 7b85bd2

Browse files
authored
fix(security): harden temp-dir handling against symlink/env-override (#676)
1 parent fe7415d commit 7b85bd2

5 files changed

Lines changed: 35 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### Fixed
6+
- `bashunit learn` and coverage now create temp directories via `mktemp -d` (no predictable PID-based paths under `/tmp`)
7+
- `bashunit::parallel::cleanup` refuses to `rm -rf` a `TEMP_DIR_PARALLEL_TEST_SUITE` whose path is not under `*/bashunit/parallel/*`, preventing accidental wipes from env overrides
8+
59
### Internal
610
- Codify global-slot return pattern for hot-path helpers; namespace mock/spy state under `_BASHUNIT_SPY_*` (#674)
711

src/coverage.sh

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,10 @@ function bashunit::coverage::init() {
7575
return 0
7676
fi
7777

78-
# Create coverage data directory with unique name
79-
# Use $$ (PID) + $RANDOM to avoid conflicts when tests call coverage::init
78+
# Create coverage data directory with unique name via mktemp -d
79+
# (avoids $$-$RANDOM collisions and symlink races in shared temp dirs)
8080
local coverage_dir
81-
coverage_dir="${BASHUNIT_TEMP_DIR:-/tmp}/bashunit-coverage-$$-$RANDOM"
82-
mkdir -p "$coverage_dir"
81+
coverage_dir=$("${MKTEMP:-mktemp}" -d "${BASHUNIT_TEMP_DIR:-${TMPDIR:-/tmp}}/bashunit-coverage.XXXXXXXX")
8382

8483
_BASHUNIT_COVERAGE_DATA_FILE="${coverage_dir}/hits.dat"
8584
_BASHUNIT_COVERAGE_TRACKED_FILES="${coverage_dir}/files.dat"

src/learn.sh

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,24 @@
66
# Provides guided tutorials and exercises to learn bashunit
77
##
88

9-
declare -r LEARN_TEMP_DIR="/tmp/bashunit_learn_$$"
9+
LEARN_TEMP_DIR=""
1010
declare -r LEARN_PROGRESS_FILE="$HOME/.bashunit_learn_progress"
1111

1212
##
1313
# Initialize learning environment
1414
##
1515
function bashunit::learn::init() {
16-
mkdir -p "$LEARN_TEMP_DIR"
16+
LEARN_TEMP_DIR=$("${MKTEMP:-mktemp}" -d "${TMPDIR:-/tmp}/bashunit_learn.XXXXXXXX")
1717
mkdir -p tests
1818
}
1919

2020
##
2121
# Cleanup learning environment
2222
##
2323
function bashunit::learn::cleanup() {
24-
rm -rf "$LEARN_TEMP_DIR"
24+
if [ -n "${LEARN_TEMP_DIR:-}" ] && [ -d "$LEARN_TEMP_DIR" ]; then
25+
rm -rf "$LEARN_TEMP_DIR"
26+
fi
2527
}
2628

2729
##

src/parallel.sh

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,16 @@ function bashunit::parallel::must_stop_on_failure() {
126126

127127
function bashunit::parallel::cleanup() {
128128
# shellcheck disable=SC2153
129-
rm -rf "$TEMP_DIR_PARALLEL_TEST_SUITE"
129+
local target="$TEMP_DIR_PARALLEL_TEST_SUITE"
130+
case "$target" in
131+
*/bashunit/parallel/*)
132+
rm -rf "$target"
133+
;;
134+
*)
135+
bashunit::internal_log "parallel::cleanup" "refused unsafe path:$target"
136+
return 1
137+
;;
138+
esac
130139
}
131140

132141
function bashunit::parallel::init() {

tests/unit/parallel_test.sh

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function set_up() {
2020
_ORIGINAL_TEMP_DIR_PARALLEL="${TEMP_DIR_PARALLEL_TEST_SUITE:-}"
2121
_ORIGINAL_TEMP_FILE_STOP="${TEMP_FILE_PARALLEL_STOP_ON_FAILURE:-}"
2222

23-
export TEMP_DIR_PARALLEL_TEST_SUITE="$_TEST_TEMP_DIR/parallel_suite"
23+
export TEMP_DIR_PARALLEL_TEST_SUITE="$_TEST_TEMP_DIR/bashunit/parallel/suite"
2424
export TEMP_FILE_PARALLEL_STOP_ON_FAILURE="$_TEST_TEMP_DIR/stop_on_failure"
2525
}
2626

@@ -147,6 +147,18 @@ function test_cleanup_removes_temp_directory() {
147147
assert_directory_not_exists "$TEMP_DIR_PARALLEL_TEST_SUITE"
148148
}
149149

150+
function test_cleanup_refuses_path_outside_bashunit_parallel() {
151+
local unsafe_dir="$_TEST_TEMP_DIR/not_bashunit"
152+
mkdir -p "$unsafe_dir"
153+
export TEMP_DIR_PARALLEL_TEST_SUITE="$unsafe_dir"
154+
155+
local rc=0
156+
bashunit::parallel::cleanup || rc=$?
157+
158+
assert_same "1" "$rc"
159+
assert_directory_exists "$unsafe_dir"
160+
}
161+
150162
# === stop_on_failure tests ===
151163

152164
function test_mark_stop_on_failure_creates_file() {

0 commit comments

Comments
 (0)