bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Again, do not change the mode of all directories below $HOME


From: Jim Meyering
Subject: Re: [PATCH] Again, do not change the mode of all directories below $HOME.
Date: Tue, 22 Jul 2008 13:17:20 +0200

Ralf Wildenhues <address@hidden> wrote:
> * tests/CuTmpdir.pm (chmod_tree): Do not run chmod on undefined
> argument, can happen when the build path contains spaces.
> ---
>
> this fixes a regression of "make check" when source and build tree live
> in a directory with spaces in the name.  The regression was introduced
> some time after I posted a fix for the issue last time.

Thanks, Ralf!

Look at it as an incentive not to use directories with names
containing meta-characters ;-)

But seriously, considering the potential for damage and mischief,
perhaps it's time to add infrastructure that stops e.g., configure
in its tracks whenever it detects a potentially troublesome source or
build dir.

I applied your patch, then added a test to ensure there is no further
regression, in spite of the added cost to "make distcheck".  Finally,
I fixed a similar (and long-standing!) shell-under-quoting bug in
test-lib.sh that was exposed by the cleanup (trap-run) code not removing
a per-test directory for the tests that create unsearchable directories.
With a bad build directory containing an "a b" component, instead of
running e.g., chmod -R 700 "/.../a b/...", the pre-patch trap/cleanup
code would run chmod -R 700 "/.../a".

Here are the three change sets I've just pushed:

>From 1ee81530c09644492d5926b17174140c0a08ad22 Mon Sep 17 00:00:00 2001
From: Ralf Wildenhues <address@hidden>
Date: Tue, 22 Jul 2008 07:38:31 +0200
Subject: [PATCH] tests: again, do not change the mode of all directories below 
$HOME

* tests/CuTmpdir.pm (chmod_tree): Do not run chmod on undefined
argument, can happen when the build path contains spaces.
---
 tests/CuTmpdir.pm |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/CuTmpdir.pm b/tests/CuTmpdir.pm
index a7dd8b6..166e50b 100644
--- a/tests/CuTmpdir.pm
+++ b/tests/CuTmpdir.pm
@@ -45,7 +45,7 @@ sub chmod_1

 sub chmod_tree
 {
-  if (chdir $dir)
+  if (defined $dir && chdir $dir)
     {
       # Perform the equivalent of find . -type d -print0|xargs -0 chmod -R 700.
       my $options = {untaint => 1, wanted => \&chmod_1};
--
1.6.0.rc0.2.g9b02c


>From 9bb0d5766eeb200dae447a616903f14a0079aa63 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 22 Jul 2008 09:20:52 +0200
Subject: [PATCH] tests: ensure "make check" w/tainted build dir no longer 
impacts $HOME

* maint.mk (taint-distcheck): New rule.
(maintainer-distcheck): Make it.
---
 maint.mk |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/maint.mk b/maint.mk
index 19b7f12..eb241b6 100644
--- a/maint.mk
+++ b/maint.mk
@@ -667,6 +667,7 @@ cvs-check: vc-diff-check

 maintainer-distcheck:
        $(MAKE) distcheck
+       $(MAKE) taint-distcheck
        $(MAKE) my-distcheck


@@ -695,6 +696,36 @@ TMPDIR ?= /tmp
 t=$(TMPDIR)/$(PACKAGE)/test
 pfx=$(t)/i

+# More than once, tainted build and source directory names would
+# have caused at least one "make check" test to apply "chmod 700"
+# to all directories under $HOME.  Make sure it doesn't happen again.
+tp := $(shell echo "$(TMPDIR)/$(PACKAGE)-$$$$")
+t_prefix = $(tp)/a
+t_taint = '$(t_prefix) b'
+fake_home = $(tp)/home
+
+# Ensure that tests run from tainted build and src dir names work,
+# and don't affect anything in $HOME.  Create witness files in $HOME,
+# record their attributes, and build/test.  Then ensure that the
+# witnesses were not affected.
+taint-distcheck: $(DIST_ARCHIVES)
+       test -d $(t_taint) && chmod -R 700 $(t_taint) || :
+       -rm -rf $(t_taint) $(fake_home)
+       mkdir -p $(t_prefix) $(t_taint) $(fake_home)
+       GZIP=$(GZIP_ENV) $(AMTAR) -C $(t_taint) -zxf $(distdir).tar.gz
+       mkfifo $(fake_home)/fifo
+       touch $(fake_home)/f
+       mkdir -p $(fake_home)/d/e
+       ls -lR $(fake_home) $(t_prefix) > $(tp)/.ls-before
+       cd $(t_taint)/$(distdir)                        \
+         && ./configure                                \
+         && $(MAKE)                                    \
+         && HOME=$(fake_home) $(MAKE) check            \
+         && ls -lR $(fake_home) $(t_prefix) > $(tp)/.ls-after \
+         && diff $(tp)/.ls-before $(tp)/.ls-after      \
+         && test -d $(t_prefix)
+       rm -rf $(tp)
+
 # Verify that a twisted use of --program-transform-name=PROGRAM works.
 define install-transform-check
   rm -rf $(pfx);                                       \
--
1.6.0.rc0.2.g9b02c


>From f82c5ba71efd1228a58953b6efc5e0d84c73e8a4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 22 Jul 2008 11:23:08 +0200
Subject: [PATCH] tests: do not run chmod on a prefix of space-embedded tmpdir

* TESTS/test-lib.sh (remove_tmp_): New function.
(trap 0): Use it instead of open-coded (and misquoted) version.
---
 tests/test-lib.sh |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/test-lib.sh b/tests/test-lib.sh
index 2083d0c..f386933 100644
--- a/tests/test-lib.sh
+++ b/tests/test-lib.sh
@@ -271,10 +271,16 @@ cleanup_() { :; }
 t_=$("$abs_top_builddir/src/mktemp" -d --tmp="$test_dir_" 
cu-$this_test.XXXXXXXXXX)\
     || error_ "failed to create temporary directory in $test_dir_"

+remove_tmp_()
+{
+  local st=$?
+  cleanup_
+  cd "$test_dir_" && chmod -R u+rwx "$t_" && rm -rf "$t_" && exit $st
+}
+
 # Run each test from within a temporary sub-directory named after the
 # test itself, and arrange to remove it upon exception or normal exit.
-trap 'st=$?; cleanup_; d='"$t_"';
-    cd '"$test_dir_"' && chmod -R u+rwx "$d" && rm -rf "$d" && exit $st' 0
+trap remove_tmp_ 0
 trap '(exit $?); exit $?' 1 2 13 15

 cd "$t_" || error_ "failed to cd to $t_"
--
1.6.0.rc0.2.g9b02c




reply via email to

[Prev in Thread] Current Thread [Next in Thread]