[PATCH] tests: Make sure that git does not access $HOME

[PATCH] tests: Make sure that git does not access $HOME

[Jason]

From: "Jason A. Donenfeld" <Jason at zx2c4.com>

With the latest changes to prevent git from accessing configuration
files that it should not, it's important to be sure that we won't
have further breakage in the future.

Use strace to implement a test to make sure cgit does not access()
anything built from $HOME.

Signed-off-by: Jason A. Donenfeld <Jason at zx2c4.com>
---
 tests/t0109-gitconfig.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100755 tests/t0109-gitconfig.sh

diff --git a/tests/t0109-gitconfig.sh b/tests/t0109-gitconfig.sh
new file mode 100755
index 0000000..b68866f
--- /dev/null
+++ b/tests/t0109-gitconfig.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+test_description='Ensure that git does not access $HOME'
+. ./setup.sh
+
+test -n "$(which strace 2>/dev/null)" || {
+	skip_all='Skipping access validation tests: strace not found'
+	test_done
+	exit
+}
+
+test_expect_success 'no access to $HOME' '
+	non_existant_path="/path/to/some/place/that/does/not/possibly/exist/$(date +%N)"
+	strace -E HOME="$non_existant_path" -E CGIT_CONFIG="$PWD/cgitrc" \
+	-E QUERY_STRING="url=foo/commit" -e access -f cgit 2>&1 >/dev/null | \
+	test_must_fail grep "$non_existant_path"
+'
+
+test_done
-- 
1.8.1.5

[PATCH] tests: Make sure that git does not access $HOME

[Jason]

Stemming from John's concern, we now have a test case in the tests
branch to ensure we don't have further gitconfig breakage down the
line.

[PATCH] tests: Make sure that git does not access $HOME

[john]

On Mon, Apr 08, 2013 at 10:20:12PM +0200, Jason A. Donenfeld wrote:
> From: "Jason A. Donenfeld" <Jason at zx2c4.com>
> 
> With the latest changes to prevent git from accessing configuration
> files that it should not, it's important to be sure that we won't
> have further breakage in the future.
> 
> Use strace to implement a test to make sure cgit does not access()
> anything built from $HOME.
> 
> Signed-off-by: Jason A. Donenfeld <Jason at zx2c4.com>
> ---
>  tests/t0109-gitconfig.sh | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>  create mode 100755 tests/t0109-gitconfig.sh
> 
> diff --git a/tests/t0109-gitconfig.sh b/tests/t0109-gitconfig.sh
> new file mode 100755
> index 0000000..b68866f
> --- /dev/null
> +++ b/tests/t0109-gitconfig.sh
> @@ -0,0 +1,19 @@
> +#!/bin/sh
> +
> +test_description='Ensure that git does not access $HOME'
> +. ./setup.sh
> +
> +test -n "$(which strace 2>/dev/null)" || {
> +	skip_all='Skipping access validation tests: strace not found'
> +	test_done
> +	exit
> +}
> +
> +test_expect_success 'no access to $HOME' '
> +	non_existant_path="/path/to/some/place/that/does/not/possibly/exist/$(date +%N)"
> +	strace -E HOME="$non_existant_path" -E CGIT_CONFIG="$PWD/cgitrc" \
> +	-E QUERY_STRING="url=foo/commit" -e access -f cgit 2>&1 >/dev/null | \

Can we avoid a pipe here and use a temporary file instead?  In fact,
using "-o filename" to strace seems like the best way to make sure we
get exactly the right output.

It would also be a bit more readable like this:

	strace -E HOME="$non_existant_path" \
		-E CGIT_CONFIG="$PWD/cgitrc" \
		-E QUERY_STRING="url=foo/commit" \
		-e access \
		-o strace.out \
		-f cgit &&

> +	test_must_fail grep "$non_existant_path"
> +'
> +
> +test_done
> -- 
> 1.8.1.5

[PATCH] tests: Make sure that git does not access $HOME

[Jason]

On Mon, Apr 8, 2013 at 10:32 PM, John Keeping <john at keeping.me.uk> wrote:
> Can we avoid a pipe here and use a temporary file instead?

Sure, no problem. Why do you prefer a temporary file over a pipe?

[PATCH] tests: Make sure that git does not access $HOME

[john]

On Mon, Apr 08, 2013 at 10:35:09PM +0200, Jason A. Donenfeld wrote:
> On Mon, Apr 8, 2013 at 10:32 PM, John Keeping <john at keeping.me.uk> wrote:
> > Can we avoid a pipe here and use a temporary file instead?
> 
> Sure, no problem. Why do you prefer a temporary file over a pipe?

The pipe hides the exit status of whatever is on its LHS, so if
cgit/strace crashes or exits with an error, piping the output completely
loses that, which is particularly bad if we're then grep'ing for a
negative.