From c1cede8797fc07ee3d76959aeaebaf73617d4b50 Mon Sep 17 00:00:00 2001 From: zdyxry Date: Sun, 25 Jun 2023 13:37:40 +0800 Subject: [PATCH 01/26] Auto cleanup temporary files --- try | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/try b/try index 787d488a..aaa91706 100755 --- a/try +++ b/try @@ -22,7 +22,11 @@ TRY_VERSION="0.1.0" try() { START_DIR="$PWD" - [ "$SANDBOX_DIR" ] || SANDBOX_DIR=$(mktemp -d) + if [ -d "$SANDBOX_DIR" ]; then + SANDBOX_DIR_EXISTS=1 + else + SANDBOX_DIR=$(mktemp -d) + fi export SANDBOX_DIR mkdir -p "$SANDBOX_DIR/upperdir" "$SANDBOX_DIR/workdir" "$SANDBOX_DIR/temproot" @@ -39,6 +43,13 @@ try() { mount_and_execute=$(mktemp) export chroot_executable=$(mktemp) export try_mount_log=$(mktemp) + + # Cleanup the temporary files + CLEANUP="rm -rf $mount_and_execute $chroot_executable $try_mount_log" + if [ "$SANDBOX_DIR_EXISTS" != "1" ]; then + CLEANUP="$CLEANUP $SANDBOX_DIR" + fi + cat >"$mount_and_execute" <<"EOF" #!/bin/sh @@ -77,7 +88,6 @@ if ! [ -f "$SANDBOX_DIR/temproot/$chroot_executable" ]; then cp $chroot_executable "$SANDBOX_DIR/temproot/$chroot_executable" fi - unshare --root="$SANDBOX_DIR/temproot" /bin/bash "$chroot_executable" EOF @@ -91,6 +101,8 @@ EOF chmod +x "$mount_and_execute" "$chroot_executable" + trap "$CLEANUP" EXIT + # --mount: mounting and unmounting filesystems will not affect the rest of the system outside the unshare # --map-root-user: map to the superuser UID and GID in the newly created user namespace. # --user: the process will have a distinct set of UIDs, GIDs and capabilities. @@ -109,7 +121,7 @@ EOF (commit) commit "$SANDBOX_DIR";; (interactive) summary "$SANDBOX_DIR" >&2 - if [ "$?" -eq 0 ] + if [ "$exitcode" -eq 0 ] && [ "$?" -eq 0 ] then echo read -p "Commit these changes? [y/N] " DO_COMMIT >&2 @@ -121,6 +133,7 @@ EOF fi ;; esac + $CLEANUP } ################################################################################ @@ -278,3 +291,4 @@ case "$1" in esac exit $exitcode + From 8667191ef74ad18a773e5e0381cf151abcbf3d4a Mon Sep 17 00:00:00 2001 From: zdyxry Date: Tue, 27 Jun 2023 13:05:27 +0800 Subject: [PATCH 02/26] Perform cleanup action only after executing commit --- test/run_tests.sh | 43 +++++++++++++++++++++++++++++++++++++++++++ try | 25 ++++++++++++++++++------- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index 3f8e5149..71ebd38a 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -145,6 +145,47 @@ test_untar_D_flag_commit() fi } +test_untar_D_flag_commit_without_cleanup() +{ + local shell=$1 + cp $RESOURCE_DIR/* "$2/" + # Will always commit the result in case of try + if [ "$shell" == "bash" ]; then + $shell gunzip $2/file.txt.gz + else + try_example_dir=$(mktemp -d) + $shell -D $try_example_dir gunzip $2/file.txt.gz + if ! [ -d "$try_example_dir" ]; then + echo "try_example_dir does not exist" + return 1 + fi + $shell commit $try_example_dir + if ! [ -d "$try_example_dir" ]; then + echo "try_example_dir does not exist" + return 1 + fi + fi +} + +test_touch_and_rm_with_cleanup() +{ + TMPDIR="/tmp" + local shell=$1 + cp $RESOURCE_DIR/* "$2/" + # Will always commit the result in case of try + if [ "$shell" == "bash" ]; then + $shell $MISC_SCRIPT_DIR/touch_echo_and_rm.sh $2/file_1.txt $2/file_2.txt $2/file.txt.gz + else + tmp_file_count1=$(ls $TMPDIR | wc -l) + $shell -y $MISC_SCRIPT_DIR/touch_echo_and_rm.sh $2/file_1.txt $2/file_2.txt $2/file.txt.gz + tmp_file_count2=$(ls $TMPDIR | wc -l) + # We save try_mount_log in /tmp/ dir + if [ $tmp_file_count1 -ne $(($tmp_file_count2 - 1)) ]; then + return 1 + fi + fi +} + test_touch_and_rm_no_flag() { local shell=$1 @@ -219,6 +260,8 @@ if [ "$#" -eq 0 ]; then run_test test_touch_and_rm_no_flag # run_test test_touch_and_rm_n_flag_commit run_test test_touch_and_rm_D_flag_commit + run_test test_touch_and_rm_with_cleanup + run_test test_untar_D_flag_commit_without_cleanup else for testname in $@ diff --git a/try b/try index aaa91706..769dcbc4 100755 --- a/try +++ b/try @@ -45,7 +45,7 @@ try() { export try_mount_log=$(mktemp) # Cleanup the temporary files - CLEANUP="rm -rf $mount_and_execute $chroot_executable $try_mount_log" + CLEANUP="$mount_and_execute $chroot_executable" if [ "$SANDBOX_DIR_EXISTS" != "1" ]; then CLEANUP="$CLEANUP $SANDBOX_DIR" fi @@ -101,8 +101,6 @@ EOF chmod +x "$mount_and_execute" "$chroot_executable" - trap "$CLEANUP" EXIT - # --mount: mounting and unmounting filesystems will not affect the rest of the system outside the unshare # --map-root-user: map to the superuser UID and GID in the newly created user namespace. # --user: the process will have a distinct set of UIDs, GIDs and capabilities. @@ -118,24 +116,37 @@ EOF case "$NO_COMMIT" in (quiet) ;; (show) printf "%s\n" "$SANDBOX_DIR";; - (commit) commit "$SANDBOX_DIR";; + (commit) commit "$SANDBOX_DIR" + cleanup $CLEANUP;; (interactive) summary "$SANDBOX_DIR" >&2 - if [ "$exitcode" -eq 0 ] && [ "$?" -eq 0 ] + if [ "$?" -eq 0 ] then echo read -p "Commit these changes? [y/N] " DO_COMMIT >&2 case "$DO_COMMIT" in - (y|Y|yes|YES) commit "$SANDBOX_DIR";; + (y|Y|yes|YES) commit "$SANDBOX_DIR" + cleanup $CLEANUP;; (*) printf "Not committing.\n" >&2 echo "$SANDBOX_DIR";; esac fi ;; esac - $CLEANUP } +################################################################################ +# Cleanup temporary files +################################################################################ + +cleanup() { + if [ -n "$CLEANUP" ]; then + echo "Cleaning up temporary files: $CLEANUP" + rm -rf $CLEANUP + fi +} + + ################################################################################ # Summarize an overlay ################################################################################ From f632f631ec4cec9f95754befdb9ac57f2d9be331 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Tue, 27 Jun 2023 16:18:08 -0400 Subject: [PATCH 03/26] cleanup logic, formatting --- try | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/try b/try index a81f4074..e17d0de8 100755 --- a/try +++ b/try @@ -22,10 +22,11 @@ TRY_VERSION="0.1.0" try() { START_DIR="$PWD" - if [ -d "$SANDBOX_DIR" ]; then - SANDBOX_DIR_EXISTS=1 - else + CLEANUP="" + if ! [ -d "$SANDBOX_DIR" ] + then SANDBOX_DIR=$(mktemp -d) + CLEANUP="$CLEANUP $SANDBOX_DIR" fi export SANDBOX_DIR mkdir -p "$SANDBOX_DIR/upperdir" "$SANDBOX_DIR/workdir" "$SANDBOX_DIR/temproot" @@ -35,7 +36,8 @@ try() { for top_dir in /* do ## Only make the directory if the original is a directory too - if [ -d "$top_dir" ]; then + if [ -d "$top_dir" ] + then mkdir "$SANDBOX_DIR"/upperdir/"$top_dir" "$SANDBOX_DIR"/workdir"/$top_dir" "$SANDBOX_DIR"/temproot/"$top_dir" fi done @@ -46,10 +48,7 @@ try() { export script_to_execute=$(mktemp) # Cleanup the temporary files - CLEANUP="$mount_and_execute $chroot_executable $script_to_execute" - if [ "$SANDBOX_DIR_EXISTS" != "1" ]; then - CLEANUP="$CLEANUP $SANDBOX_DIR" - fi + CLEANUP="$CLEANUP $mount_and_execute $chroot_executable $script_to_execute" cat >"$mount_and_execute" <<"EOF" #!/bin/sh @@ -58,7 +57,8 @@ try() { for top_dir in /* do ## If the directory is not a mountpoint - if [ -d "$top_dir" ] && ! mountpoint -q "$top_dir"; then + if [ -d "$top_dir" ] && ! mountpoint -q "$top_dir" + then ## TODO: The mount -t overlay overlay -o lowerdir=/"$top_dir",upperdir="$SANDBOX_DIR"/upperdir/"$top_dir",workdir="$SANDBOX_DIR"/workdir/"$top_dir" "$SANDBOX_DIR"/temproot/"$top_dir" 2>> "$try_mount_log" || echo "Warning: Failed mounting $top_dir as an overlay, see "$try_mount_log"" 1>&2 fi @@ -85,7 +85,8 @@ mount --rbind /dev "$SANDBOX_DIR/temproot/dev" mount --rbind --read-only /run "$SANDBOX_DIR/temproot/run" 2> /dev/null ## Check if chroot_executable exists, #29 -if ! [ -f "$SANDBOX_DIR/temproot/$chroot_executable" ]; then +if ! [ -f "$SANDBOX_DIR/temproot/$chroot_executable" ] +then cp $chroot_executable "$SANDBOX_DIR/temproot/$chroot_executable" fi @@ -123,7 +124,7 @@ EOF (quiet) ;; (show) printf "%s\n" "$SANDBOX_DIR";; (commit) commit "$SANDBOX_DIR" - cleanup $CLEANUP;; + cleanup;; (interactive) summary "$SANDBOX_DIR" >&2 if [ "$?" -eq 0 ] @@ -132,7 +133,7 @@ EOF read -p "Commit these changes? [y/N] " DO_COMMIT >&2 case "$DO_COMMIT" in (y|Y|yes|YES) commit "$SANDBOX_DIR" - cleanup $CLEANUP;; + cleanup;; (*) printf "Not committing.\n" >&2 echo "$SANDBOX_DIR";; esac @@ -146,8 +147,8 @@ EOF ################################################################################ cleanup() { - if [ -n "$CLEANUP" ]; then - echo "Cleaning up temporary files: $CLEANUP" + if [ -n "$CLEANUP" ] + then rm -rf $CLEANUP fi } @@ -179,7 +180,8 @@ summary() { echo echo "Changes detected in the following files:" echo - while IFS= read -r changed_file; do + while IFS= read -r changed_file + do local_file="${changed_file#$SANDBOX_DIR/upperdir}" ## KK 2023-06-20 Could print local_file instead of changed file for ## cleaner output. @@ -210,7 +212,8 @@ commit() { # changed_files from summary. changed_files=$(find "$SANDBOX_DIR/upperdir/" -type f -o \( -type c -size 0 \) -o -type d | ignore_changes) - while IFS= read -r changed_file; do + while IFS= read -r changed_file + do local_file="${changed_file#$SANDBOX_DIR/upperdir}" if [ -d "$changed_file" ] && ! [ -d "${local_file}" ] then # new directory From 6b988d33c3f0e849021899f8d1775ed51b82616f Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Tue, 27 Jun 2023 16:25:52 -0400 Subject: [PATCH 04/26] debugging in CI, yolo --- try | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/try b/try index e17d0de8..1867df57 100755 --- a/try +++ b/try @@ -147,8 +147,10 @@ EOF ################################################################################ cleanup() { - if [ -n "$CLEANUP" ] + if [ "$CLEANUP" ] then + echo "cleaning up $CLEANUP" + ls /tmp rm -rf $CLEANUP fi } From d539936edbfcbe5b8f2ef2b5b674c47c233ae410 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Tue, 27 Jun 2023 16:28:12 -0400 Subject: [PATCH 05/26] use `=` with test, not `==` --- test/run_tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index fffd62d6..55873d2d 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -94,7 +94,7 @@ test_untar_D_flag_commit_without_cleanup() local shell=$1 cp $RESOURCE_DIR/* "$2/" # Will always commit the result in case of try - if [ "$shell" == "bash" ]; then + if [ "$shell" = "bash" ]; then $shell gunzip $2/file.txt.gz else try_example_dir=$(mktemp -d) @@ -117,7 +117,7 @@ test_touch_and_rm_with_cleanup() local shell=$1 cp $RESOURCE_DIR/* "$2/" # Will always commit the result in case of try - if [ "$shell" == "bash" ]; then + if [ "$shell" = "bash" ]; then $shell $MISC_SCRIPT_DIR/touch_echo_and_rm.sh $2/file_1.txt $2/file_2.txt $2/file.txt.gz else tmp_file_count1=$(ls $TMPDIR | wc -l) From 6f339fe991cc2bbebfb93d56a95eef0fd568df67 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 05:00:05 -0700 Subject: [PATCH 06/26] explicitly unmount the overlay --- try | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/try b/try index 1867df57..1df52ba6 100755 --- a/try +++ b/try @@ -53,6 +53,7 @@ try() { cat >"$mount_and_execute" <<"EOF" #!/bin/sh +MOUNTED="" # actually mount the overlays for top_dir in /* do @@ -60,7 +61,13 @@ do if [ -d "$top_dir" ] && ! mountpoint -q "$top_dir" then ## TODO: The - mount -t overlay overlay -o lowerdir=/"$top_dir",upperdir="$SANDBOX_DIR"/upperdir/"$top_dir",workdir="$SANDBOX_DIR"/workdir/"$top_dir" "$SANDBOX_DIR"/temproot/"$top_dir" 2>> "$try_mount_log" || echo "Warning: Failed mounting $top_dir as an overlay, see "$try_mount_log"" 1>&2 + mount -t overlay overlay -o lowerdir=/"$top_dir",upperdir="$SANDBOX_DIR"/upperdir/"$top_dir",workdir="$SANDBOX_DIR"/workdir/"$top_dir" "$SANDBOX_DIR"/temproot/"$top_dir" 2>> "$try_mount_log" + if [ "$?" -eq 0 ] + then + MOUNTED="$MOUNTED $SANDBOX_DIR/temproot/$top_dir" + else + $echo "Warning: Failed mounting $top_dir as an overlay, see "$try_mount_log"" >&2 + fi fi done @@ -73,16 +80,22 @@ done # then we want to exclude the root partition "/" for mount_dir in $(findmnt --real -r -o target -n | grep -v "^/$") do - mount -t overlay overlay -o lowerdir="$mount_dir",upperdir="$SANDBOX_DIR"/upperdir"$mount_dir",workdir="$SANDBOX_DIR"/workdir"$mount_dir" "$SANDBOX_DIR"/temproot"$mount_dir" 2>> "$try_mount_log" || echo "Warning: Failed mounting $mount_dir as an overlay, see "$try_mount_log"" 1>&2 + mount -t overlay overlay -o lowerdir="$mount_dir",upperdir="$SANDBOX_DIR"/upperdir"$mount_dir",workdir="$SANDBOX_DIR"/workdir"$mount_dir" "$SANDBOX_DIR"/temproot"$mount_dir" 2>> "$try_mount_log" + if [ "$?" -eq 0 ] + then + MOUNTED="$MOUNTED $SANDBOX_DIR/temproot$mount_dir" + else + echo "Warning: Failed mounting $mount_dir as an overlay, see "$try_mount_log"" 1>&2 + fi done ## Bind the udev mount so that the containerized process has access to /dev ## KK 2023-05-06 Are there any security/safety implications by binding the whole /dev? ## Maybe we just want to bind a few files in it like /dev/null, /dev/zero? -mount --rbind /dev "$SANDBOX_DIR/temproot/dev" +mount --rbind /dev "$SANDBOX_DIR/temproot/dev" && MOUNTED="$MOUNTED $SANDBOX_DIR/temproot/dev" ## KK 2023-06-20 Redirecting to /dev/null to suppress a yet uninvestigated but ## seemingly not impactful warning. -mount --rbind --read-only /run "$SANDBOX_DIR/temproot/run" 2> /dev/null +mount --rbind --read-only /run "$SANDBOX_DIR/temproot/run" 2>/dev/null && MOUNTED="$MOUNTED $SANDBOX_DIR/temproot/run" ## Check if chroot_executable exists, #29 if ! [ -f "$SANDBOX_DIR/temproot/$chroot_executable" ] @@ -91,6 +104,15 @@ then fi unshare --root="$SANDBOX_DIR/temproot" /bin/bash "$chroot_executable" +exitcode="$?" + +# unmount +for m in $MOUNTED +do + umount "$m" +done + +exit $exitcode EOF cat >"$chroot_executable" < Date: Wed, 28 Jun 2023 05:03:50 -0700 Subject: [PATCH 07/26] debugging --- try | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/try b/try index 1df52ba6..4e2b5deb 100755 --- a/try +++ b/try @@ -109,7 +109,7 @@ exitcode="$?" # unmount for m in $MOUNTED do - umount "$m" + umount "$m" || echo "Couldn't unmount $m" >&2 done exit $exitcode @@ -171,8 +171,6 @@ EOF cleanup() { if [ "$CLEANUP" ] then - echo "cleaning up $CLEANUP" - ls /tmp rm -rf $CLEANUP fi } From f105015872d4108422ea8540c5086b7f8381e203 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 10:54:00 -0400 Subject: [PATCH 08/26] fsync---a shot in the dark --- try | 1 + 1 file changed, 1 insertion(+) diff --git a/try b/try index 4e2b5deb..a42f8f45 100755 --- a/try +++ b/try @@ -107,6 +107,7 @@ unshare --root="$SANDBOX_DIR/temproot" /bin/bash "$chroot_executable" exitcode="$?" # unmount +fsync for m in $MOUNTED do umount "$m" || echo "Couldn't unmount $m" >&2 From 55412a468a56e75db3013bfbcea39e708396ba47 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 11:00:40 -0400 Subject: [PATCH 09/26] s/fsync/sync/ --- try | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/try b/try index a42f8f45..fd32f09b 100755 --- a/try +++ b/try @@ -107,7 +107,7 @@ unshare --root="$SANDBOX_DIR/temproot" /bin/bash "$chroot_executable" exitcode="$?" # unmount -fsync +sync for m in $MOUNTED do umount "$m" || echo "Couldn't unmount $m" >&2 From a8c93fe18dc6b1fc5e8e0bbe3bdb517602e70ad7 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 11:11:00 -0400 Subject: [PATCH 10/26] debug test --- test/run_tests.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/test/run_tests.sh b/test/run_tests.sh index 55873d2d..61ec69b6 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -123,6 +123,7 @@ test_touch_and_rm_with_cleanup() tmp_file_count1=$(ls $TMPDIR | wc -l) $shell -y $MISC_SCRIPT_DIR/touch_echo_and_rm.sh $2/file_1.txt $2/file_2.txt $2/file.txt.gz tmp_file_count2=$(ls $TMPDIR | wc -l) + echo "original count ${tmp_file_count1} final count ${tmp_file_count2}" # We save try_mount_log in /tmp/ dir if [ $tmp_file_count1 -ne $(($tmp_file_count2 - 1)) ]; then return 1 From 9b980afd30ff46dcec1227cb5cd66f2a2a733dd3 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 11:26:21 -0400 Subject: [PATCH 11/26] the script didn't even exist?! --- test/run_tests.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index 61ec69b6..a93904b9 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -116,12 +116,18 @@ test_touch_and_rm_with_cleanup() TMPDIR="/tmp" local shell=$1 cp $RESOURCE_DIR/* "$2/" + SCRIPT=$(mktemp) + cat >$SCRIPT < Date: Wed, 28 Jun 2023 11:28:57 -0400 Subject: [PATCH 12/26] durrrrr --- test/run_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index a93904b9..34157101 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -121,7 +121,7 @@ test_touch_and_rm_with_cleanup() touch $1 cat $2 rm $3 - EOF +EOF # Will always commit the result in case of try if [ "$shell" = "bash" ]; then $shell $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz From d4bb11f00634c1b51962f4b682dfa4e2c08785f1 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 11:31:39 -0400 Subject: [PATCH 13/26] debug test --- test/run_tests.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index 34157101..3077970f 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -44,6 +44,7 @@ run_test() return 1 fi + echo echo -n "Running $test..." # Run test @@ -117,11 +118,12 @@ test_touch_and_rm_with_cleanup() local shell=$1 cp $RESOURCE_DIR/* "$2/" SCRIPT=$(mktemp) - cat >$SCRIPT <$SCRIPT <<'EOF' touch $1 cat $2 rm $3 EOF + cat $SCRIPT # Will always commit the result in case of try if [ "$shell" = "bash" ]; then $shell $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz From 7369fba93e819f18e16c5fe50b231e5ccf1b26d6 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 11:36:34 -0400 Subject: [PATCH 14/26] debugging --- test/run_tests.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/run_tests.sh b/test/run_tests.sh index 3077970f..66ef83e8 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -128,10 +128,13 @@ EOF if [ "$shell" = "bash" ]; then $shell $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz else + orig_tmp=$(ls $TMPDIR) tmp_file_count1=$(ls $TMPDIR | wc -l) $shell -y $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz tmp_file_count2=$(ls $TMPDIR | wc -l) echo "original count ${tmp_file_count1} final count ${tmp_file_count2}" + echo DIFF: + diff --color -u <(echo "$orig_tmp") <(ls $TMPDIR) # We save try_mount_log in /tmp/ dir if [ $tmp_file_count1 -ne $(($tmp_file_count2 - 1)) ]; then return 1 From f5c54dd4b59162171baa434cc4973d5257f01cc1 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 11:39:50 -0400 Subject: [PATCH 15/26] test was faulty --- test/run_tests.sh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index 66ef83e8..f4bd920c 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -129,15 +129,12 @@ EOF $shell $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz else orig_tmp=$(ls $TMPDIR) - tmp_file_count1=$(ls $TMPDIR | wc -l) $shell -y $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz - tmp_file_count2=$(ls $TMPDIR | wc -l) - echo "original count ${tmp_file_count1} final count ${tmp_file_count2}" - echo DIFF: - diff --color -u <(echo "$orig_tmp") <(ls $TMPDIR) + # We save try_mount_log in /tmp/ dir - if [ $tmp_file_count1 -ne $(($tmp_file_count2 - 1)) ]; then - return 1 + if diff --color -u <(echo "$orig_tmp") <(ls $TMPDIR) + then + return 47 fi fi } From 59c48fffc14ab2cbb9f6786d3a0ae5caca3e6e7d Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 11:42:02 -0400 Subject: [PATCH 16/26] debugging --- test/run_tests.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index f4bd920c..0f0b1624 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -129,7 +129,8 @@ EOF $shell $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz else orig_tmp=$(ls $TMPDIR) - $shell -y $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz + ls $2 + $shell -y -- $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz # We save try_mount_log in /tmp/ dir if diff --color -u <(echo "$orig_tmp") <(ls $TMPDIR) From 1b2e1ccb4206bbb89963c2f08ffdbeb1a214bf32 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 11:52:22 -0400 Subject: [PATCH 17/26] overhaul tests --- test/run_tests.sh | 70 ++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index 0f0b1624..c5798b89 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -53,11 +53,11 @@ run_test() if [ $test_try_ec -eq 0 ]; then echo -ne '\t\t\t' - echo "$test are identical" >> $output_dir/result_status + echo "$test passed" >> $output_dir/result_status echo -e '\tOK' else - echo -n " (!) EC mismatch" - echo "$test are not identical" >> $output_dir/result_status + echo -n " non-zero ec ($test_try_ec)" + echo "$test failed" >> $output_dir/result_status echo -e '\t\tFAIL' fi } @@ -92,51 +92,47 @@ test_untar_D_flag_commit() test_untar_D_flag_commit_without_cleanup() { - local shell=$1 - cp $RESOURCE_DIR/* "$2/" - # Will always commit the result in case of try - if [ "$shell" = "bash" ]; then - $shell gunzip $2/file.txt.gz - else - try_example_dir=$(mktemp -d) - $shell -D $try_example_dir gunzip $2/file.txt.gz - if ! [ -d "$try_example_dir" ]; then - echo "try_example_dir does not exist" - return 1 - fi - $shell commit $try_example_dir - if ! [ -d "$try_example_dir" ]; then - echo "try_example_dir does not exist" - return 1 - fi + local try_workspace=$1 + cp $RESOURCE_DIR/* "$try_workspace/" + cd "$try_workspace/" + + try_example_dir=$(mktemp -d) + "$try" -D $try_example_dir gunzip file.txt.gz + if ! [ -d "$try_example_dir" ]; then + echo "try_example_dir disappeared with no commit" + return 1 + fi + "$try" commit $try_example_dir + if ! [ -d "$try_example_dir" ]; then + echo "try_example_dir disappeared after manual commit" + return 1 fi } test_touch_and_rm_with_cleanup() { - TMPDIR="/tmp" - local shell=$1 - cp $RESOURCE_DIR/* "$2/" + local try_workspace=$1 + cp $RESOURCE_DIR/* "$try_workspace/" + cd "$try_workspace/" + + : ${TMPDIR=/tmp} + SCRIPT=$(mktemp) cat >$SCRIPT <<'EOF' touch $1 cat $2 rm $3 EOF - cat $SCRIPT - # Will always commit the result in case of try - if [ "$shell" = "bash" ]; then - $shell $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz - else - orig_tmp=$(ls $TMPDIR) - ls $2 - $shell -y -- $SCRIPT $2/file_1.txt $2/file_2.txt $2/file.txt.gz - - # We save try_mount_log in /tmp/ dir - if diff --color -u <(echo "$orig_tmp") <(ls $TMPDIR) - then - return 47 - fi + + orig_tmp=$(ls "$TMPDIR") + $shell -y -- $SCRIPT file_1.txt file_2.txt file.txt.gz + new_tmp=$(ls "$TMPDIR") + + if ! diff -q <(echo "$orig_tmp") <(echo "$new_tmp") + then + echo "temporary directory was not cleaned up; diff:" + diff --color -u <(echo "$orig_tmp") <(echo "$new_tmp") + return 47 fi } From 603685c646bdfbd98fe39a61516a5df6b6be5c42 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 12:00:39 -0400 Subject: [PATCH 18/26] cleaning up test harness --- test/run_tests.sh | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index c5798b89..c4316d85 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -34,6 +34,9 @@ cleanup() mkdir "$try_workspace" } +TOTAL_TESTS=0 +PASSED_TEST=0 +FAILING_TESTS="" run_test() { cleanup @@ -48,14 +51,17 @@ run_test() echo -n "Running $test..." # Run test + : $((TOTAL_TESTS += 1)) $test "$try_workspace" test_try_ec=$? if [ $test_try_ec -eq 0 ]; then + : $((PASSED_TESTS += 1)) echo -ne '\t\t\t' echo "$test passed" >> $output_dir/result_status echo -e '\tOK' else + FAILING_TESTS="$FAILING_TESTS $test" echo -n " non-zero ec ($test_try_ec)" echo "$test failed" >> $output_dir/result_status echo -e '\t\tFAIL' @@ -253,16 +259,10 @@ case "$distro" in ;; esac -echo -e "\n====================| Test Summary |====================\n" -echo "> Below follow the identical outputs:" -grep "are identical" "$output_dir"/result_status | awk '{print $1}' | tee $output_dir/passed.log - -echo "> Below follow the non-identical outputs:" -grep "are not identical" "$output_dir"/result_status | awk '{print $1}' | tee $output_dir/failed.log -echo "========================================================" -TOTAL_TESTS=$(cat "$output_dir"/result_status | wc -l | xargs) -PASSED_TESTS=$(grep -c "are identical" "$output_dir"/result_status) -echo "Summary: ${PASSED_TESTS}/${TOTAL_TESTS} tests passed." | tee $output_dir/results.log +echo +echo "====================| Test Summary |====================" +echo "Failing tests:${FAILING_TESTS}" +echo "Summary: ${PASSED_TESTS}/${TOTAL_TESTS} tests passed." echo "========================================================" if [ $PASSED_TESTS -ne $TOTAL_TESTS ] From ffde3f49b9d78838e066366985ee412b5c906899 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Wed, 28 Jun 2023 13:49:28 -0400 Subject: [PATCH 19/26] cleanup tests --- test/run_tests.sh | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index c4316d85..daadbda4 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -68,7 +68,7 @@ run_test() fi } -test_untar_no_flag() +test_unzip_no_flag() { local try_workspace=$1 cp $RESOURCE_DIR/file.txt.gz "$try_workspace/" @@ -77,11 +77,11 @@ test_untar_no_flag() ## Set up expected output echo 'Hello World!' >expected.out - "$try" -y gunzip file.txt.gz + "$try" -y gunzip file.txt.gz || return 1 diff -q expected.out file.txt } -test_untar_D_flag_commit() +test_unzip_D_flag_commit() { local try_workspace=$1 cp $RESOURCE_DIR/file.txt.gz "$try_workspace/" @@ -91,24 +91,24 @@ test_untar_D_flag_commit() echo 'Hello World!' >expected.out try_example_dir=$(mktemp -d) - "$try" -D $try_example_dir gunzip file.txt.gz + "$try" -D $try_example_dir gunzip file.txt.gz || return 1 $try commit $try_example_dir diff -q expected.out file.txt } -test_untar_D_flag_commit_without_cleanup() +test_unzip_D_flag_commit_without_cleanup() { local try_workspace=$1 cp $RESOURCE_DIR/* "$try_workspace/" cd "$try_workspace/" try_example_dir=$(mktemp -d) - "$try" -D $try_example_dir gunzip file.txt.gz + "$try" -D $try_example_dir gunzip file.txt.gz || return 1 if ! [ -d "$try_example_dir" ]; then echo "try_example_dir disappeared with no commit" return 1 fi - "$try" commit $try_example_dir + "$try" commit $try_example_dir || return 1 if ! [ -d "$try_example_dir" ]; then echo "try_example_dir disappeared after manual commit" return 1 @@ -123,22 +123,15 @@ test_touch_and_rm_with_cleanup() : ${TMPDIR=/tmp} - SCRIPT=$(mktemp) - cat >$SCRIPT <<'EOF' - touch $1 - cat $2 - rm $3 -EOF - orig_tmp=$(ls "$TMPDIR") - $shell -y -- $SCRIPT file_1.txt file_2.txt file.txt.gz + "$try" -y -- "touch file_1.txt; echo test > file_2.txt; rm file.txt.gz" || return 1 new_tmp=$(ls "$TMPDIR") if ! diff -q <(echo "$orig_tmp") <(echo "$new_tmp") then echo "temporary directory was not cleaned up; diff:" diff --color -u <(echo "$orig_tmp") <(echo "$new_tmp") - return 47 + return 1 fi } @@ -152,7 +145,7 @@ test_touch_and_rm_no_flag() touch expected1.txt echo 'test' >expected2.txt - "$try" -y "touch file_1.txt; echo test > file_2.txt; rm file.txt.gz" + "$try" -y "touch file_1.txt; echo test > file_2.txt; rm file.txt.gz" || return 1 diff -q expected1.txt file_1.txt && diff -q expected2.txt file_2.txt && @@ -170,7 +163,7 @@ test_touch_and_rm_D_flag_commit() echo 'test' >expected2.txt try_example_dir=$(mktemp -d) - "$try" -D $try_example_dir "touch file_1.txt; echo test > file_2.txt; rm file.txt.gz" + "$try" -D $try_example_dir "touch file_1.txt; echo test > file_2.txt; rm file.txt.gz" || return 1 $try commit $try_example_dir diff -q expected1.txt file_1.txt && @@ -186,7 +179,7 @@ test_pipeline() ## Set up expected output echo 'TesT' >expected.out - "$try" 'echo test | tr t T' > out.txt + "$try" 'echo test | tr t T' > out.txt || return 1 diff -q expected.out out.txt } @@ -203,7 +196,7 @@ test_cmd_sbst_and_var() echo $(pwd) EOF - "$try" sh script.sh >out.txt + "$try" sh script.sh >out.txt || return 1 diff -q expected.out out.txt } @@ -221,12 +214,12 @@ test_fail() # We run all tests composed with && to exit on the first that fails if [ "$#" -eq 0 ]; then - run_test test_untar_no_flag - run_test test_untar_D_flag_commit + run_test test_unzip_no_flag + run_test test_unzip_D_flag_commit run_test test_touch_and_rm_no_flag run_test test_touch_and_rm_D_flag_commit run_test test_touch_and_rm_with_cleanup - run_test test_untar_D_flag_commit_without_cleanup + run_test test_unzip_D_flag_commit_without_cleanup run_test test_pipeline run_test test_cmd_sbst_and_var From 52ea6f1a606ac100b9ef82ddf50ff289b57d5d30 Mon Sep 17 00:00:00 2001 From: Konstantinos Kallas Date: Thu, 29 Jun 2023 15:24:22 -0400 Subject: [PATCH 20/26] Remove sandbox cleanup logic --- try | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/try b/try index fd32f09b..279b1344 100755 --- a/try +++ b/try @@ -53,7 +53,6 @@ try() { cat >"$mount_and_execute" <<"EOF" #!/bin/sh -MOUNTED="" # actually mount the overlays for top_dir in /* do @@ -62,10 +61,8 @@ do then ## TODO: The mount -t overlay overlay -o lowerdir=/"$top_dir",upperdir="$SANDBOX_DIR"/upperdir/"$top_dir",workdir="$SANDBOX_DIR"/workdir/"$top_dir" "$SANDBOX_DIR"/temproot/"$top_dir" 2>> "$try_mount_log" - if [ "$?" -eq 0 ] + if ! [ "$?" -eq 0 ] then - MOUNTED="$MOUNTED $SANDBOX_DIR/temproot/$top_dir" - else $echo "Warning: Failed mounting $top_dir as an overlay, see "$try_mount_log"" >&2 fi fi @@ -81,10 +78,8 @@ done for mount_dir in $(findmnt --real -r -o target -n | grep -v "^/$") do mount -t overlay overlay -o lowerdir="$mount_dir",upperdir="$SANDBOX_DIR"/upperdir"$mount_dir",workdir="$SANDBOX_DIR"/workdir"$mount_dir" "$SANDBOX_DIR"/temproot"$mount_dir" 2>> "$try_mount_log" - if [ "$?" -eq 0 ] + if ! [ "$?" -eq 0 ] then - MOUNTED="$MOUNTED $SANDBOX_DIR/temproot$mount_dir" - else echo "Warning: Failed mounting $mount_dir as an overlay, see "$try_mount_log"" 1>&2 fi done @@ -92,10 +87,10 @@ done ## Bind the udev mount so that the containerized process has access to /dev ## KK 2023-05-06 Are there any security/safety implications by binding the whole /dev? ## Maybe we just want to bind a few files in it like /dev/null, /dev/zero? -mount --rbind /dev "$SANDBOX_DIR/temproot/dev" && MOUNTED="$MOUNTED $SANDBOX_DIR/temproot/dev" +mount --rbind /dev "$SANDBOX_DIR/temproot/dev" ## KK 2023-06-20 Redirecting to /dev/null to suppress a yet uninvestigated but ## seemingly not impactful warning. -mount --rbind --read-only /run "$SANDBOX_DIR/temproot/run" 2>/dev/null && MOUNTED="$MOUNTED $SANDBOX_DIR/temproot/run" +mount --rbind --read-only /run "$SANDBOX_DIR/temproot/run" 2>/dev/null ## Check if chroot_executable exists, #29 if ! [ -f "$SANDBOX_DIR/temproot/$chroot_executable" ] @@ -104,16 +99,6 @@ then fi unshare --root="$SANDBOX_DIR/temproot" /bin/bash "$chroot_executable" -exitcode="$?" - -# unmount -sync -for m in $MOUNTED -do - umount "$m" || echo "Couldn't unmount $m" >&2 -done - -exit $exitcode EOF cat >"$chroot_executable" < Date: Thu, 29 Jun 2023 15:27:49 -0400 Subject: [PATCH 21/26] test nit --- test/run_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index daadbda4..4b64e54a 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -118,7 +118,7 @@ test_unzip_D_flag_commit_without_cleanup() test_touch_and_rm_with_cleanup() { local try_workspace=$1 - cp $RESOURCE_DIR/* "$try_workspace/" + cp $RESOURCE_DIR/file.txt.gz "$try_workspace/" cd "$try_workspace/" : ${TMPDIR=/tmp} From 5a77768d0be72bf6bf886b0e3e2112b21593a99a Mon Sep 17 00:00:00 2001 From: Konstantinos Kallas Date: Thu, 29 Jun 2023 15:43:58 -0400 Subject: [PATCH 22/26] tiny bug --- test/run_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index 66ac2865..df99d72b 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -35,7 +35,7 @@ cleanup() } TOTAL_TESTS=0 -PASSED_TEST=0 +PASSED_TESTS=0 FAILING_TESTS="" test_read_from_run_dir() From 5f90836926139a701f1265d90e4d8083b0b74204 Mon Sep 17 00:00:00 2001 From: Konstantinos Kallas Date: Thu, 29 Jun 2023 15:44:07 -0400 Subject: [PATCH 23/26] Debugging info before cleanup --- try | 1 + 1 file changed, 1 insertion(+) diff --git a/try b/try index 152aec7f..b9bcabab 100755 --- a/try +++ b/try @@ -251,6 +251,7 @@ EOF ################################################################################ cleanup() { + echo "Before cleanup: $CLEANUP" 1>&2 if [ "$CLEANUP" ] then rm -rf $CLEANUP From 97d58fc313acfd52867b5e0dfe2294819cda1536 Mon Sep 17 00:00:00 2001 From: Konstantinos Kallas Date: Thu, 29 Jun 2023 15:52:36 -0400 Subject: [PATCH 24/26] Add the ignore_file in the CLEANUP --- try | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/try b/try index 80005a56..b2207ce5 100755 --- a/try +++ b/try @@ -21,7 +21,7 @@ TRY_VERSION="0.1.0" try() { START_DIR="$PWD" - CLEANUP="" + CLEANUP="$IGNORE_FILE" if ! command -v findmnt > /dev/null then From d205146a58b3888d762bd21dd85ff81821b354d9 Mon Sep 17 00:00:00 2001 From: Konstantinos Kallas Date: Thu, 6 Jul 2023 12:32:33 -0400 Subject: [PATCH 25/26] revert cleanup for now --- try | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/try b/try index b2207ce5..a2887807 100755 --- a/try +++ b/try @@ -21,7 +21,6 @@ TRY_VERSION="0.1.0" try() { START_DIR="$PWD" - CLEANUP="$IGNORE_FILE" if ! command -v findmnt > /dev/null then @@ -36,7 +35,6 @@ try() { else ## Create a new sandbox if one was not given SANDBOX_DIR=$(mktemp -d) - CLEANUP="$CLEANUP $SANDBOX_DIR" fi ## If the sandbox is not valid we exit early @@ -76,10 +74,6 @@ try() { export chroot_executable=$(mktemp) export try_mount_log=$(mktemp) export script_to_execute=$(mktemp) - - # Cleanup the temporary files - CLEANUP="$CLEANUP $mount_and_execute $chroot_executable $script_to_execute" - cat >"$mount_and_execute" <<"EOF" #!/bin/sh @@ -227,8 +221,7 @@ EOF case "$NO_COMMIT" in (quiet) ;; (show) printf "%s\n" "$SANDBOX_DIR";; - (commit) commit "$SANDBOX_DIR" - cleanup;; + (commit) commit "$SANDBOX_DIR";; (interactive) summary "$SANDBOX_DIR" >&2 if [ "$?" -eq 0 ] @@ -236,8 +229,7 @@ EOF echo read -p "Commit these changes? [y/N] " DO_COMMIT >&2 case "$DO_COMMIT" in - (y|Y|yes|YES) commit "$SANDBOX_DIR" - cleanup;; + (y|Y|yes|YES) commit "$SANDBOX_DIR";; (*) printf "Not committing.\n" >&2 echo "$SANDBOX_DIR";; esac @@ -246,19 +238,6 @@ EOF esac } -################################################################################ -# Cleanup temporary files -################################################################################ - -cleanup() { - echo "Before cleanup: $CLEANUP" 1>&2 - if [ "$CLEANUP" ] - then - rm -rf $CLEANUP - fi -} - - ################################################################################ # Summarize an overlay ################################################################################ @@ -510,4 +489,3 @@ case "$1" in esac exit $exitcode - From eb1c178bc11301ef16c64a0e3bf23393029a2ac5 Mon Sep 17 00:00:00 2001 From: Konstantinos Kallas Date: Thu, 6 Jul 2023 12:35:56 -0400 Subject: [PATCH 26/26] remove cleanup test --- test/run_tests.sh | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/test/run_tests.sh b/test/run_tests.sh index d50f843a..42cd40f8 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -128,25 +128,28 @@ test_unzip_D_flag_commit_without_cleanup() fi } -test_touch_and_rm_with_cleanup() -{ - local try_workspace=$1 - cp $RESOURCE_DIR/file.txt.gz "$try_workspace/" - cd "$try_workspace/" - - : ${TMPDIR=/tmp} - - orig_tmp=$(ls "$TMPDIR") - "$try" -y -- "touch file_1.txt; echo test > file_2.txt; rm file.txt.gz" || return 1 - new_tmp=$(ls "$TMPDIR") +# KK 2023-07-06 This test checks whether try has correctly cleaned up its temporary directories +# but is not working now. I am leaving it in so that its logic can be reused for a new test. +# +# test_touch_and_rm_with_cleanup() +# { +# local try_workspace=$1 +# cp $RESOURCE_DIR/file.txt.gz "$try_workspace/" +# cd "$try_workspace/" + +# : ${TMPDIR=/tmp} + +# orig_tmp=$(ls "$TMPDIR") +# "$try" -y -- "touch file_1.txt; echo test > file_2.txt; rm file.txt.gz" || return 1 +# new_tmp=$(ls "$TMPDIR") - if ! diff -q <(echo "$orig_tmp") <(echo "$new_tmp") - then - echo "temporary directory was not cleaned up; diff:" - diff --color -u <(echo "$orig_tmp") <(echo "$new_tmp") - return 1 - fi -} +# if ! diff -q <(echo "$orig_tmp") <(echo "$new_tmp") +# then +# echo "temporary directory was not cleaned up; diff:" +# diff --color -u <(echo "$orig_tmp") <(echo "$new_tmp") +# return 1 +# fi +# } test_touch_and_rm_no_flag() { @@ -405,7 +408,7 @@ if [ "$#" -eq 0 ]; then run_test test_unzip_D_flag_commit run_test test_touch_and_rm_no_flag run_test test_touch_and_rm_D_flag_commit - run_test test_touch_and_rm_with_cleanup + # run_test test_touch_and_rm_with_cleanup run_test test_unzip_D_flag_commit_without_cleanup run_test test_reuse_sandbox run_test test_reuse_problematic_sandbox