Re: www/serf install permissions fix

["J. Lewis Muir" <jlmuir%imca-cat.org@localhost>]

On 04/05, J. Lewis Muir wrote:
> On 04/04, Greg Troxel wrote:
> > so I added a post-extract chmod.
> 
> OK, great!  Thank you!
> 
> > Please file a bug with serf so they can fix this, or at least not get it
> > wrong in the future.
> 
> Done:
> 
>   https://issues.apache.org/jira/browse/SERF-181
> 
> > Also, if you add a PKG_DEVELOPER test for writable files, we can chmod
> > those and get someone to file bugs too.
> 
> Below is my attempt at such a PKG_DEVELOPER test.  I have a few comments
> and questions about it:
> 
> 1. It checks for group- or world-writable files.  If it should only
>    check for world-writable files, obviously it's trivial to change it
>    to do that.
> 
> 2. I used find, grep, and sed directly in the _check-distfile-perms
>    target since mk/check/bsd.check-vars.mk adds them to USE_TOOLS.  It
>    seems that some mk/check/check-*.mk files use the tools directly
>    while others use the variable for them (e.g., find vs. ${FIND}).  Are
>    both correct?
> 
> 3. I used "[" (test) in the _check-distfile-perms target, but I did
>    not add that to USE_TOOLS.  Is this OK?  Other mk/check/check-*.mk
>    files use it without adding it to USE_TOOLS.  It's also usually a
>    shell built-in, but I'm not sure it's safe to assume that.  And it
>    gets added to USE_TOOLS in mk/bsd.prefs.mk (which is included by
>    (among others) mk/misc/common.mk which is included by (among others)
>    mk/bsd.pkg.mk).
> 
> 4. I added the include for check-distfile-perms.mk to
>    mk/check/bsd.check.mk in alphabetical order where the other
>    check-*.mk files were added.  However, I noticed that the include for
>    check-fakehome.mk is not in alphabetical order; it's the last to be
>    included.  Is alphabetical order correct, or should it be added at
>    the end after check-fakehome.mk?

Ping?

Lewis

> diff --git a/mk/check/bsd.check.mk b/mk/check/bsd.check.mk
> index 8db4197..925b798 100644
> --- a/mk/check/bsd.check.mk
> +++ b/mk/check/bsd.check.mk
> @@ -5,6 +5,9 @@
>  #
>  # There are some predefined hooks where checks can be added:
>  #
> +# post-extract-checks-hook
> +#	Is run after extracting the package distfiles.
> +#
>  # pre-configure-checks-hook
>  #	Is run before configuring the package.
>  #
> @@ -25,6 +28,7 @@
>  #    check-interpreter, check-shlibs, check-vulnerable, check-wrkref
>  #
>  
> +.include "check-distfile-perms.mk"
>  .include "check-files.mk"
>  .include "check-headers.mk"
>  .include "check-interpreter.mk"
> @@ -36,6 +40,7 @@
>  .include "check-wrkref.mk"
>  .include "check-fakehome.mk"
>  
> +post-extract-checks-hook \
>  pre-configure-checks-hook \
>  pre-build-checks-hook: .PHONY
>  	@${DO_NADA}
> diff --git a/mk/check/check-distfile-perms.mk b/mk/check/check-distfile-perms.mk
> new file mode 100644
> index 0000000..01770bd
> --- /dev/null
> +++ b/mk/check/check-distfile-perms.mk
> @@ -0,0 +1,52 @@
> +# $NetBSD$
> +#
> +# This file checks that extracted files are not group- nor world-writable.
> +#
> +# User-settable variables:
> +#
> +# CHECK_DISTFILE_PERMS
> +#	Specifies whether the check should be run at all.
> +#
> +#	Possible values: yes, no.
> +#
> +#	Default value: yes for PKG_DEVELOPER, no otherwise.
> +#
> +# Package-settable variables:
> +#
> +
> +_VARGROUPS+=					check-distfile-perms
> +_USER_VARS.check-distfile-perms=		CHECK_DISTFILE_PERMS
> +
> +.if ${PKG_DEVELOPER:Uno} != "no"
> +CHECK_DISTFILE_PERMS?=				yes
> +.else
> +CHECK_DISTFILE_PERMS?=				no
> +.endif
> +
> +.if ${CHECK_DISTFILE_PERMS:M[Yy][Ee][Ss]}
> +post-extract-checks-hook: _check-distfile-perms
> +.endif
> +
> +_CHECK_DISTFILE_PERMS_QUOTED_EXTRACT_DIRS=	# empty
> +.for f in ${EXTRACT_ONLY}
> +_CHECK_DISTFILE_PERMS_QUOTED_EXTRACT_DIRS+=	${EXTRACT_DIR.${f}:Q}
> +.endfor
> +
> +_check-distfile-perms: .PHONY
> +	@${STEP_MSG} "Checking for group- or world-writable extracted files"
> +	${RUN} 										\
> +	ok=yes; 									\
> +	for each in ${_CHECK_DISTFILE_PERMS_QUOTED_EXTRACT_DIRS}; do 			\
> +		cd ${WRKDIR:Q} &&							\
> +		if find "$$each" -perm -g=w -o -perm -o=w | grep . > /dev/null; then	\
> +			ok=no;								\
> +			break;								\
> +		fi;									\
> +	done;										\
> +	[ $$ok = yes ] && exit 0;							\
> +	${ERROR_MSG} "[check-distfile-perms.mk] group- or world-writable files:";	\
> +	for each in ${_CHECK_DISTFILE_PERMS_QUOTED_EXTRACT_DIRS}; do			\
> +		cd ${WRKDIR:Q} &&							\
> +		find "$$each" -perm -g=w -o -perm -o=w | sed  's/^/	/';		\
> +	done;										\
> +	exit 1
> diff --git a/mk/extract/extract.mk b/mk/extract/extract.mk
> index f23b7ef..f5ee3ea 100644
> --- a/mk/extract/extract.mk
> +++ b/mk/extract/extract.mk
> @@ -105,6 +105,7 @@ _REAL_EXTRACT_TARGETS+=	extract-dir
>  _REAL_EXTRACT_TARGETS+=	pre-extract
>  _REAL_EXTRACT_TARGETS+=	do-extract
>  _REAL_EXTRACT_TARGETS+=	post-extract
> +_REAL_EXTRACT_TARGETS+=	post-extract-checks-hook
>  _REAL_EXTRACT_TARGETS+=	extract-cookie
>  _REAL_EXTRACT_TARGETS+=	error-check
>