From 116e1661319cf49f38b4c4624f13aafc70cc17de Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 2 Oct 2026 13:20:35 -0700 Subject: [PATCH 1/4] scripts: rename test environment vars Problem: flux-pam tests use FLUX_PAM_TEST_* environment variables to point prolog and housekeeping to mock systemctl and loginctl programs. This introduces an unnecessary risk since the documented allowed-environment glob for prolog and housekeeping is FLUX_*, which allows these environment variables to leak inadvertently in production. While there is no way these variables could be set by an untrusted user, defense-in-depth dictates that these test-only variables should not share the same prefix as actual Flux environment variables. Rename the test environment variables with a leading underscore `_FLUX_PAM_TEST_*` so they don't match the standard pattern. Update affected tests. Assisted-by: Claude:Opus-5 --- src/scripts/flux-pam-housekeeping.in | 2 +- src/scripts/flux-pam-prolog.in | 4 ++-- t/t0002-prolog-housekeeping.t | 6 +++--- t/t0004-prolog-real-systemd.t | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/scripts/flux-pam-housekeeping.in b/src/scripts/flux-pam-housekeeping.in index 969685f..6c0c5ed 100755 --- a/src/scripts/flux-pam-housekeeping.in +++ b/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(): diff --git a/src/scripts/flux-pam-prolog.in b/src/scripts/flux-pam-prolog.in index 287cf97..ef0bbaa 100755 --- a/src/scripts/flux-pam-prolog.in +++ b/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(): diff --git a/t/t0002-prolog-housekeeping.t b/t/t0002-prolog-housekeeping.t index ee33d85..b65bd2e 100755 --- a/t/t0002-prolog-housekeeping.t +++ b/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,7 @@ 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} ' test_expect_success 're-configure flux with pam.manage-user-slice enabled' ' flux config load <<-'EOT' diff --git a/t/t0004-prolog-real-systemd.t b/t/t0004-prolog-real-systemd.t index 6821232..6b161b0 100755 --- a/t/t0004-prolog-real-systemd.t +++ b/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 From bde3ddfd836d1318d355e1bd7c4ac58326e5cdad Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 2 Oct 2026 13:56:49 -0700 Subject: [PATCH 2/4] build: strip test path overrides from installed scripts Problem: The installed prolog and housekeeping scripts carry the _FLUX_PAM_TEST_* lookups that redirect systemctl and loginctl at a mock. Renaming them out of the FLUX_* namespace keeps flux-imp from passing them through, but the mechanism is still present in code that runs as root, where it has no use: the testsuite runs the scripts from the build tree, never the installed copies. Generate an .inst copy of each script at build time with every lookup replaced by the path found by configure. Install this version so it has no possibility of overriding paths of programs run as root. Assisted-by: Claude:Opus-5 --- src/scripts/Makefile.am | 47 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/src/scripts/Makefile.am b/src/scripts/Makefile.am index 6d893ee..e9b660f 100644 --- a/src/scripts/Makefile.am +++ b/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 $@ From ed0153bb1da52d3d918e0f547b060039da30c82a Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 2 Oct 2026 13:57:06 -0700 Subject: [PATCH 3/4] .gitignore: ignore generated scripts Problem: The generated prolog and housekeeping scripts show up as untracked, and the .inst copies built alongside them now do too. Add all three patterns to .gitignore. Assisted-by: Claude:Opus-5 --- .gitignore | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 3523288..c12eac3 100644 --- a/.gitignore +++ b/.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/ From 1d9d5b023f4f2a990a6d78e86c248ab5be9f14ef Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 2 Oct 2026 13:57:06 -0700 Subject: [PATCH 4/4] testsuite: check that installed scripts use literal paths Problem: Nothing verifies that the installed prolog and housekeeping scripts have the test path overrides stripped. Add checks against the generated scripts in inst/ to ensure the environment overrides have been stripped and they are still valid Python. Assisted-by: Claude:Opus-5 --- t/t0002-prolog-housekeeping.t | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/t/t0002-prolog-housekeeping.t b/t/t0002-prolog-housekeeping.t index b65bd2e..dc9e26b 100755 --- a/t/t0002-prolog-housekeeping.t +++ b/t/t0002-prolog-housekeeping.t @@ -58,6 +58,34 @@ 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} ' +# 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' [access]