remove test environment variables from installed prolog and housekeeping scripts

.gitignore

@@ -42,8 +42,13 @@ m4/ltsugar.m4
 m4/ltversion.m4
 m4/lt~obsolete.m4
 
-# Generated Makefile 
-# (meta build system like autotools, 
+# Generated Makefile
+# (meta build system like autotools,
 # can automatically generate from config.status script
 # (which is called by configure script))
 Makefile
+
+# Generated scripts, and the stripped copies that get installed
+/src/scripts/flux-pam-prolog
+/src/scripts/flux-pam-housekeeping
+/src/scripts/inst/

src/scripts/Makefile.am

@@ -1,8 +1,51 @@
 fluxlibexecprologdir = $(fluxlibexecdir)/prolog.d
 fluxlibexechousekeepingdir = $(fluxlibexecdir)/housekeeping.d
-fluxlibexecprolog_SCRIPTS = flux-pam-prolog
-fluxlibexechousekeeping_SCRIPTS = flux-pam-housekeeping
+fluxlibexecprolog_SCRIPTS = inst/flux-pam-prolog
+fluxlibexechousekeeping_SCRIPTS = inst/flux-pam-housekeeping
 
 EXTRA_DIST = \
 	flux-pam-prolog.in \
 	flux-pam-housekeeping.in
+
+CLEANFILES = \
+	$(fluxlibexecprolog_SCRIPTS) \
+	$(fluxlibexechousekeeping_SCRIPTS)
+
+# CLEANFILES removes the scripts but leaves the directory holding them.
+clean-local:
+	-rmdir inst 2>/dev/null || :
+
+# The scripts consult _FLUX_PAM_TEST_* to redirect systemctl and loginctl
+# at a mock. The testsuite runs them from the build tree, so the installed
+# copies have no use for the overrides: replace each lookup with the path
+# configure found, leaving nothing in the installed script that can steer
+# what it executes.
+#
+# Built into inst/ rather than under a suffix so automake installs them
+# under their own names, applying $(transform) as it does for any script.
+#
+# Generated from the configured script, not from the .in template, so the
+# @SYSTEMCTL@ and @LOGINCTL@ substitutions are already in place.
+#
+# The path may be empty: configure only looks for systemctl and loginctl
+# when libsystemd is present, so a build without it substitutes "". The
+# scripts are installed either way and must still have the lookups
+# removed, so match an empty path too.
+STRIP_TEST_HOOKS = \
+	$(SED) -E \
+	  's|^([A-Z]+) = os\.environ\.get\("_FLUX_PAM_TEST_[A-Z]+", ("[^"]*")\)$$|\1 = \2|'
+
+# Fail if a lookup survives the rewrite. A pattern that stops matching,
+# say after the assignments are reformatted, would otherwise ship the
+# overrides while the build still succeeds. .DELETE_ON_ERROR removes the
+# partial target, so a failed strip cannot leave one behind.
+.DELETE_ON_ERROR:
+
+inst/%: %
+	$(AM_V_at)$(MKDIR_P) inst
+	$(AM_V_GEN)$(STRIP_TEST_HOOKS) $< >$@
+	$(AM_V_at)if grep -q '_FLUX_PAM_TEST_' $@; then \
+	  echo "$@: failed to strip test path overrides" >&2; \
+	  exit 1; \
+	fi
+	$(AM_V_at)chmod +x $@

src/scripts/flux-pam-housekeeping.in

@@ -27,7 +27,7 @@ import sys
 import flux.job
 import flux.pam
 
-SYSTEMCTL = os.environ.get("FLUX_PAM_TEST_SYSTEMCTL", "@SYSTEMCTL@")
+SYSTEMCTL = os.environ.get("_FLUX_PAM_TEST_SYSTEMCTL", "@SYSTEMCTL@")
 
 
 def main():

src/scripts/flux-pam-prolog.in

@@ -28,8 +28,8 @@ import sys
 import flux.job
 import flux.pam
 
-SYSTEMCTL = os.environ.get("FLUX_PAM_TEST_SYSTEMCTL", "@SYSTEMCTL@")
-LOGINCTL = os.environ.get("FLUX_PAM_TEST_LOGINCTL", "@LOGINCTL@")
+SYSTEMCTL = os.environ.get("_FLUX_PAM_TEST_SYSTEMCTL", "@SYSTEMCTL@")
+LOGINCTL = os.environ.get("_FLUX_PAM_TEST_LOGINCTL", "@LOGINCTL@")
 
 
 def main():

t/t0002-prolog-housekeeping.t

@@ -14,8 +14,8 @@ PROLOG=${FLUX_BUILD_DIR}/src/scripts/flux-pam-prolog
 HOUSEKEEPING=${FLUX_BUILD_DIR}/src/scripts/flux-pam-housekeeping
 
 SCRIPTSDIR=${SHARNESS_TEST_SRCDIR}/scripts
-export FLUX_PAM_TEST_SYSTEMCTL=${SCRIPTSDIR}/mock-systemctl
-export FLUX_PAM_TEST_LOGINCTL=${SCRIPTSDIR}/mock-loginctl
+export _FLUX_PAM_TEST_SYSTEMCTL=${SCRIPTSDIR}/mock-systemctl
+export _FLUX_PAM_TEST_LOGINCTL=${SCRIPTSDIR}/mock-loginctl
 
 # Use temporary dir for lock files
 export FLUX_PAM_LOCK_DIR=$(pwd)/lock
@@ -56,7 +56,35 @@ broker_unsetenv() {
 test $(flux resource list -no {ncores} -i 0) -gt 1 && test_set_prereq MULTICORE
 
 test_expect_success 'mock-systemctl is executable' '
-	test -x ${FLUX_PAM_TEST_SYSTEMCTL}
+	test -x ${_FLUX_PAM_TEST_SYSTEMCTL}
+'
+# The scripts installed to prolog.d and housekeeping.d are built into
+# inst/ with the test path overrides replaced by the configured path, so
+# nothing in the code that runs as root can redirect systemctl or
+# loginctl. Check the generated artifact, since the build is what removes
+# them. A bare prefix match accepts the empty path a build without
+# libsystemd substitutes.
+INSTDIR=${FLUX_BUILD_DIR}/src/scripts/inst
+
+test_expect_success 'installed scripts bind a literal systemctl path' '
+	grep -E "^SYSTEMCTL = \".*\"$" ${INSTDIR}/flux-pam-prolog &&
+	grep -E "^LOGINCTL = \".*\"$" ${INSTDIR}/flux-pam-prolog &&
+	grep -E "^SYSTEMCTL = \".*\"$" ${INSTDIR}/flux-pam-housekeeping
+'
+test_expect_success 'installed scripts are valid python' '
+	flux python -c "import ast, sys
+for path in sys.argv[1:]:
+    ast.parse(open(path, \"rb\").read())" \
+		${INSTDIR}/flux-pam-prolog ${INSTDIR}/flux-pam-housekeeping
+'
+# The build-tree scripts keep the overrides: the tests below depend on
+# redirecting systemctl at a mock. Checked here so that a strip leaking
+# into the build tree fails once, rather than as a pile of downstream
+# failures with no obvious cause.
+test_expect_success 'build tree scripts keep the test path overrides' '
+	grep -q _FLUX_PAM_TEST_SYSTEMCTL ${PROLOG} &&
+	grep -q _FLUX_PAM_TEST_LOGINCTL ${PROLOG} &&
+	grep -q _FLUX_PAM_TEST_SYSTEMCTL ${HOUSEKEEPING}
 '
 test_expect_success 're-configure flux with pam.manage-user-slice enabled' '
 	flux config load <<-'EOT'

t/t0004-prolog-real-systemd.t

@@ -33,8 +33,8 @@ fi
 PROLOG=${FLUX_BUILD_DIR}/src/scripts/flux-pam-prolog
 
 SCRIPTSDIR=${SHARNESS_TEST_SRCDIR}/scripts
-export FLUX_PAM_TEST_SYSTEMCTL=${SCRIPTSDIR}/user-systemctl
-export FLUX_PAM_TEST_LOGINCTL=${SCRIPTSDIR}/mock-loginctl
+export _FLUX_PAM_TEST_SYSTEMCTL=${SCRIPTSDIR}/user-systemctl
+export _FLUX_PAM_TEST_LOGINCTL=${SCRIPTSDIR}/mock-loginctl
 
 export FLUX_PAM_LOCK_DIR=$(pwd)/lock
 export FLUX_PAM_SCRIPTS_DEBUG=1
@@ -61,10 +61,10 @@ slice_revert() {
 }
 
 test_expect_success 'user-systemctl wrapper is executable' '
-	test -x ${FLUX_PAM_TEST_SYSTEMCTL}
+	test -x ${_FLUX_PAM_TEST_SYSTEMCTL}
 '
 test_expect_success 'user systemd rejects a comma-joined DeviceAllow' '
-	test_must_fail ${FLUX_PAM_TEST_SYSTEMCTL} set-property --runtime \
+	test_must_fail ${_FLUX_PAM_TEST_SYSTEMCTL} set-property --runtime \
 	    ${SLICE} "DeviceAllow=/dev/null rw,/dev/zero rw" 2>comma.err &&
 	test_debug "cat comma.err" &&
 	grep -i "rwm flags" comma.err