From b0a31065e017f48a9fb9436915f1f934990f3c1e Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Tue, 12 May 2026 13:13:14 +0200 Subject: [PATCH 1/2] fix(security): harden temp-dir handling against symlink/env-override risks - learn.sh: replace predictable /tmp/bashunit_learn_$$ with mktemp -d - coverage.sh: replace $$-$RANDOM path with mktemp -d - parallel.sh: cleanup() refuses to rm -rf a TEMP_DIR_PARALLEL_TEST_SUITE that is not under */bashunit/parallel/* Adds regression test asserting cleanup leaves an out-of-tree path intact. --- CHANGELOG.md | 4 ++++ src/coverage.sh | 7 +++---- src/learn.sh | 8 +++++--- src/parallel.sh | 11 ++++++++++- tests/unit/parallel_test.sh | 12 +++++++++++- 5 files changed, 33 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bf3cd43..b0151b2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixed +- `bashunit learn` and coverage now create temp directories via `mktemp -d` (no predictable PID-based paths under `/tmp`) +- `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 + ### Internal - Codify global-slot return pattern for hot-path helpers; namespace mock/spy state under `_BASHUNIT_SPY_*` (#674) diff --git a/src/coverage.sh b/src/coverage.sh index ba228fa9..e3a66b38 100644 --- a/src/coverage.sh +++ b/src/coverage.sh @@ -75,11 +75,10 @@ function bashunit::coverage::init() { return 0 fi - # Create coverage data directory with unique name - # Use $$ (PID) + $RANDOM to avoid conflicts when tests call coverage::init + # Create coverage data directory with unique name via mktemp -d + # (avoids $$-$RANDOM collisions and symlink races in shared temp dirs) local coverage_dir - coverage_dir="${BASHUNIT_TEMP_DIR:-/tmp}/bashunit-coverage-$$-$RANDOM" - mkdir -p "$coverage_dir" + coverage_dir=$("${MKTEMP:-mktemp}" -d "${BASHUNIT_TEMP_DIR:-${TMPDIR:-/tmp}}/bashunit-coverage.XXXXXXXX") _BASHUNIT_COVERAGE_DATA_FILE="${coverage_dir}/hits.dat" _BASHUNIT_COVERAGE_TRACKED_FILES="${coverage_dir}/files.dat" diff --git a/src/learn.sh b/src/learn.sh index c57c18a9..ee8dd751 100644 --- a/src/learn.sh +++ b/src/learn.sh @@ -6,14 +6,14 @@ # Provides guided tutorials and exercises to learn bashunit ## -declare -r LEARN_TEMP_DIR="/tmp/bashunit_learn_$$" +LEARN_TEMP_DIR="" declare -r LEARN_PROGRESS_FILE="$HOME/.bashunit_learn_progress" ## # Initialize learning environment ## function bashunit::learn::init() { - mkdir -p "$LEARN_TEMP_DIR" + LEARN_TEMP_DIR=$("${MKTEMP:-mktemp}" -d "${TMPDIR:-/tmp}/bashunit_learn.XXXXXXXX") mkdir -p tests } @@ -21,7 +21,9 @@ function bashunit::learn::init() { # Cleanup learning environment ## function bashunit::learn::cleanup() { - rm -rf "$LEARN_TEMP_DIR" + if [ -n "${LEARN_TEMP_DIR:-}" ] && [ -d "$LEARN_TEMP_DIR" ]; then + rm -rf "$LEARN_TEMP_DIR" + fi } ## diff --git a/src/parallel.sh b/src/parallel.sh index bc142908..1edafe20 100755 --- a/src/parallel.sh +++ b/src/parallel.sh @@ -126,7 +126,16 @@ function bashunit::parallel::must_stop_on_failure() { function bashunit::parallel::cleanup() { # shellcheck disable=SC2153 - rm -rf "$TEMP_DIR_PARALLEL_TEST_SUITE" + local target="$TEMP_DIR_PARALLEL_TEST_SUITE" + case "$target" in + */bashunit/parallel/*) + rm -rf "$target" + ;; + *) + bashunit::internal_log "parallel::cleanup" "refused unsafe path:$target" + return 1 + ;; + esac } function bashunit::parallel::init() { diff --git a/tests/unit/parallel_test.sh b/tests/unit/parallel_test.sh index ca5e0508..26807878 100644 --- a/tests/unit/parallel_test.sh +++ b/tests/unit/parallel_test.sh @@ -20,7 +20,7 @@ function set_up() { _ORIGINAL_TEMP_DIR_PARALLEL="${TEMP_DIR_PARALLEL_TEST_SUITE:-}" _ORIGINAL_TEMP_FILE_STOP="${TEMP_FILE_PARALLEL_STOP_ON_FAILURE:-}" - export TEMP_DIR_PARALLEL_TEST_SUITE="$_TEST_TEMP_DIR/parallel_suite" + export TEMP_DIR_PARALLEL_TEST_SUITE="$_TEST_TEMP_DIR/bashunit/parallel/suite" export TEMP_FILE_PARALLEL_STOP_ON_FAILURE="$_TEST_TEMP_DIR/stop_on_failure" } @@ -147,6 +147,16 @@ function test_cleanup_removes_temp_directory() { assert_directory_not_exists "$TEMP_DIR_PARALLEL_TEST_SUITE" } +function test_cleanup_refuses_path_outside_bashunit_parallel() { + local unsafe_dir="$_TEST_TEMP_DIR/not_bashunit" + mkdir -p "$unsafe_dir" + export TEMP_DIR_PARALLEL_TEST_SUITE="$unsafe_dir" + + bashunit::parallel::cleanup + + assert_directory_exists "$unsafe_dir" +} + # === stop_on_failure tests === function test_mark_stop_on_failure_creates_file() { From 15adac39daba6872751cbaea165e7d768a2ed27a Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Tue, 12 May 2026 13:39:13 +0200 Subject: [PATCH 2/2] test(parallel): assert cleanup return code so strict mode does not abort --- tests/unit/parallel_test.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/parallel_test.sh b/tests/unit/parallel_test.sh index 26807878..a280e716 100644 --- a/tests/unit/parallel_test.sh +++ b/tests/unit/parallel_test.sh @@ -152,8 +152,10 @@ function test_cleanup_refuses_path_outside_bashunit_parallel() { mkdir -p "$unsafe_dir" export TEMP_DIR_PARALLEL_TEST_SUITE="$unsafe_dir" - bashunit::parallel::cleanup + local rc=0 + bashunit::parallel::cleanup || rc=$? + assert_same "1" "$rc" assert_directory_exists "$unsafe_dir" }