On Wed, Dec 07, 2022 at 10:26:36AM +0100, Stefan Sperling wrote:
> As found by Mikhail, gotd regress test script run as root when the
> test suite is invoked via doas(1). This happens because doas sets
> the $LOGNAME environment veriable to the target user (root). Unlike
> su(1) which leaves $LOGNAME set to the name of the original user.
> 
> This patch adds a sanity check to prevent running test scripts as root,
> and switches to $USERNAME which works with both doas(1) and su(1).
> 
> This does not fix 'su -', which leaves $USERNAME unset and also sets
> $LOGNAME to root. That is no big deal; people can just avoid 'su -',
> or pass their intended GOTD_TEST_USER on the make command line.
> 
> ok?

OK kn

> 
>  make it easier to run gotd regress via doas(1)
>  
> diff b1142068fd26fa24285c533f10e3afcb61719759 32014c96a26e67fa0bf2fc74139ec6b68336c966
> commit - b1142068fd26fa24285c533f10e3afcb61719759
> commit + 32014c96a26e67fa0bf2fc74139ec6b68336c966
> blob - cd20ee9bf8631fcd757767edf78df5f410e56161
> blob + 1f10771a98fe00cf4c3029d94a41ae32341873e7
> --- regress/gotd/Makefile
> +++ regress/gotd/Makefile
> @@ -12,7 +12,7 @@ GOTD_TEST_USER?=${LOGNAME}
>  GOTD_TEST_REPO!?=mktemp -d "$(GOTD_TEST_ROOT)/gotd-test-repo-XXXXXXXXX"
>  GOTD_TEST_REPO_URL=ssh://${GOTD_DEVUSER}@127.0.0.1/test-repo
>  
> -GOTD_TEST_USER?=${LOGNAME}
> +GOTD_TEST_USER?=${USERNAME}
>  GOTD_TEST_USER_HOME!=userinfo $(GOTD_TEST_USER) | awk '/^dir/ {print $$2}'
>  
>  # gotd.conf parameters
> @@ -34,8 +34,12 @@ ensure_root:
>  
>  ensure_root:
>  	@if [[ `id -u` -ne 0 ]]; then \
> -		echo gotd test suite must be run by root >&2; \
> +		echo gotd test suite must be started by root >&2; \
>  		false; \
> +	fi ; \
> +	if [[ "$(GOTD_TEST_USER)" = "root" ]]; then \

Inside [[ you do not need to quote, see 'man -Otag=[[ ksh'.

The only difference quoting can make is on the right operator, which is
taken as pattern in [[, unless force it to be a literal string with quotes.

I'd also use == not =, but again, that's just a style nit.  If the rest
of the tests already look like that, there's little point in deviating
from it.

> +		echo GOTD_TEST_USER must be a non-root user >&2; \
> +		false; \
>  	fi
>  
>  start_gotd_ro: ensure_root
>