remove test environment variables from installed prolog and housekeeping scripts - #35
Merged
Merged
Conversation
grondo
force-pushed
the
harden-test-hooks
branch
3 times, most recently
from
October 2, 2026 22:54
95ca4e5 to
9c01f92
Compare
Contributor
Author
|
Excellent. Thanks for all the quick reviews. |
grondo
added
the
merge-when-passing
Contributor
Merge Queue Status
This pull request spent 8 minutes 28 seconds in the queue, including 4 minutes 13 seconds running CI. Required conditions to merge
|
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
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
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
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
mergify
Bot
force-pushed
the
harden-test-hooks
branch
from
October 3, 2026 00:27
9c01f92 to
1d9d5b0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem: The flux-pam tests use
FLUX_PAM_TEST_*environment variables to invoke mocksystemctlandloginctlcommands to get better coverage. These variables survive into the final installed scripts, and the prefix matches the suggestedallow-environmentglob in tbe IMP config:FLUX_*. While users probably can't influence the environment here, defense-in-depth dictates that these overrides are an open security hole.This PR renames the test env vars with a leading
_(_FLUX_PAM_TEST_*) and additionally removes support for the variables entirely in the installed scripts. This closes the hole even in the case non-stripped scripts get inadvertently installed.