From 7b3b6a2b2077b766908c663b7fe3cdacde74a48d Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Tue, 15 Sep 2026 22:07:43 +0200 Subject: [PATCH 1/2] doc/ts: describe variable loss when conflict unload follows setenv When conflict_unload is enabled, the conflicting loaded module is unloaded at the time the conflict, family or module unload command is evaluated. A variable set by the loading modulefile before this command is unset by this unload if the conflicting modulefile also sets it. Document this behavior as another reason to declare dependencies prior environment changes and cover it in the testsuite. Assisted-by: Claude:claude-fable-5-1 Signed-off-by: Xavier Delaruelle --- doc/source/modulefile.rst | 7 ++- testsuite/modulefiles.4/conun/1 | 16 ++++++ testsuite/modulefiles.4/foo/1.0 | 7 +++ .../modules.50-cmds/660-conflict_unload.exp | 55 +++++++++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/doc/source/modulefile.rst b/doc/source/modulefile.rst index 49194f469..4c420abe4 100644 --- a/doc/source/modulefile.rst +++ b/doc/source/modulefile.rst @@ -2276,7 +2276,12 @@ reloaded or refreshed. This is especially important when the modulefile updates an environment variable also altered by other modulefiles like :envvar:`PATH`. As the order of the path elements in such variable defines priority, it is important that this order does not change depending on the way -the modulefiles are loaded. +the modulefiles are loaded. Moreover when :mconfig:`conflict_unload` is +enabled, a conflicting loaded modulefile is unloaded at the time the +:mfcmd:`conflict`, :mfcmd:`family` or :mfcmd:`module unload` command +is evaluated. Environment changes made prior this command by the loading +modulefile are overridden by this unload. For instance a variable set by the +loading modulefile ends up unset if it is also set by the unloaded modulefile. :command:`module` keeps environment consistent which means a modulefile cannot be loaded if its requirements are not loaded or if a conflicting module is diff --git a/testsuite/modulefiles.4/conun/1 b/testsuite/modulefiles.4/conun/1 index d9ea46f2b..6da854014 100644 --- a/testsuite/modulefiles.4/conun/1 +++ b/testsuite/modulefiles.4/conun/1 @@ -67,6 +67,22 @@ if {[info exists env(TESTSUITE_CONFLICT_UNLOAD)]} { depun_of_conun_is_sticky1 - depun_of_conun_is_super_sticky1 { conflict conun } + setenv_before_conflict1 { + setenv TSCONUN conun + conflict foo + } + setenv_after_conflict1 { + conflict foo + setenv TSCONUN conun + } + setenv_before_unload1 { + setenv TSCONUN conun + module unload foo + } + setenv_before_family1 { + setenv TSCONUN conun + family conun + } implicit_default_off1 { conflict conun } diff --git a/testsuite/modulefiles.4/foo/1.0 b/testsuite/modulefiles.4/foo/1.0 index c6a6b8d3b..6f2e3c590 100644 --- a/testsuite/modulefiles.4/foo/1.0 +++ b/testsuite/modulefiles.4/foo/1.0 @@ -61,6 +61,13 @@ if {[info exists env(TESTSUITE_CONFLICT_UNLOAD)]} { depre_of_conun_is_conflict_of_reqlo1 { conflict bar/1 } + setenv_before_conflict1 - setenv_after_conflict1 - setenv_before_unload1 { + setenv TSCONUN foo + } + setenv_before_family1 { + setenv TSCONUN foo + family conun + } } } if {[info exists env(TESTSUITE_MODULEPATH_OPT)]} { diff --git a/testsuite/modules.50-cmds/660-conflict_unload.exp b/testsuite/modules.50-cmds/660-conflict_unload.exp index 91cdb2178..366b96cae 100644 --- a/testsuite/modules.50-cmds/660-conflict_unload.exp +++ b/testsuite/modules.50-cmds/660-conflict_unload.exp @@ -910,6 +910,61 @@ set tserr [msg_top_load_conun conun/1 {} conun/2 {} {} bar/1] testouterr_cmd bash {load conun/1} $ans $tserr +# +# environment variable set by loading module prior conflict declaration +# + +unsetenv_loaded_module +unsetenv_var __MODULES_LMPREREQ +unsetenv_var __MODULES_LMCONFLICT +unsetenv_var __MODULES_LMTAG +unsetenv_var __MODULES_LMALTNAME +unsetenv_var MODULES_FAMILY_CONUN + +setenv_loaded_module [list foo/1.0] [list $mp/foo/1.0] +setenv_var TSCONUN foo + +# conflicting module unload occurs when conflict is evaluated, so variable +# set by loading module before that is unset by this unload +setenv_var TESTSUITE_CONFLICT_UNLOAD setenv_before_conflict1 +set ans [list] +lappend ans [list set __MODULES_LMCONFLICT conun/1&foo] +lappend ans [list set _LMFILES_ $mp/conun/1] +lappend ans [list set LOADEDMODULES conun/1] +lappend ans [list unset TSCONUN] +set tserr [msg_top_load_conun conun/1 {} {foo/1.0} {} {} {} {}] +testouterr_cmd bash {load conun/1} $ans $tserr + +setenv_var TESTSUITE_CONFLICT_UNLOAD setenv_before_unload1 +testouterr_cmd bash {load conun/1} $ans $tserr + +# variable is preserved if set after conflict declaration +setenv_var TESTSUITE_CONFLICT_UNLOAD setenv_after_conflict1 +set ans [list] +lappend ans [list set __MODULES_LMCONFLICT conun/1&foo] +lappend ans [list set _LMFILES_ $mp/conun/1] +lappend ans [list set LOADEDMODULES conun/1] +lappend ans [list set TSCONUN conun] +testouterr_cmd bash {load conun/1} $ans $tserr + +setenv_var TESTSUITE_CONFLICT_UNLOAD setenv_before_family1 +setenv_var MODULES_FAMILY_CONUN foo +setenv_var __MODULES_LMALTNAME foo/1.0&al|conun +set ans [list] +lappend ans [list set MODULES_FAMILY_CONUN conun] +lappend ans [list set __MODULES_LMCONFLICT conun/1&conun] +lappend ans [list set __MODULES_LMALTNAME conun/1&al|conun] +lappend ans [list set _LMFILES_ $mp/conun/1] +lappend ans [list set LOADEDMODULES conun/1] +lappend ans [list unset TSCONUN] +lappend ans [list set LMOD_FAMILY_CONUN conun] +testouterr_cmd bash {load conun/1} $ans $tserr + +unsetenv_var MODULES_FAMILY_CONUN +unsetenv_var __MODULES_LMALTNAME +unsetenv_var TSCONUN + + # # Disabled implicit default # From ada610c35c8a666365c979a5ca09e98889161fc3 Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Tue, 15 Sep 2026 22:14:08 +0200 Subject: [PATCH 2/2] Resolve relative modulepath from cwd during modulefile evaluation A relative path entry in MODULEPATH is made absolute by the procedure which uses the directory of the modulefile being evaluated as base when an evaluation is ongoing. A module loaded from within a modulefile, with a "module load", "prereq" or "depends-on" command, is thus searched in a wrong directory and reported as not found. Relative modulepath entries refer to the current working directory of the module command, whatever the evaluation context. Add an option to the path resolution procedure to force this base directory and use it when resolving modulepath entries. The "is-used" modulefile command resolves its arguments relatively to the modulefile directory, so a raw relative MODULEPATH entry passed as argument does not match the entry once resolved from the current working directory. Check argument against the raw MODULEPATH content first, as "module unuse" does, to keep such argument recognized as used. Assisted-by: Claude:claude-fable-5-1 Signed-off-by: Xavier Delaruelle --- NEWS.rst | 4 ++ tcl/mfcmd.tcl | 11 ++-- tcl/modfind.tcl.in | 4 +- tcl/util.tcl | 5 +- testsuite/modulefiles.4/bar/1 | 18 +++++ .../745-modulepath-relative.exp | 66 +++++++++++++++++++ 6 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 testsuite/modules.50-cmds/745-modulepath-relative.exp diff --git a/NEWS.rst b/NEWS.rst index 5601b8078..2955df0f8 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -255,6 +255,10 @@ Modules 5.7.0 (not yet released) shell meta-characters. Completion candidates were passed to ``compgen -W`` which evaluates command substitution syntax. (fix `CVE-2026-85013`_ found by AISLE in partnership with Red Hat) +* Fix resolution of relative path entries in :envvar:`MODULEPATH` when a + module is loaded during a modulefile evaluation. Such entry was resolved + against the directory of the evaluating modulefile rather than against the + current working directory. .. _CVE-2026-85013: https://github.com/envmodules/modules/security/advisories/GHSA-8hrw-p88g-qhmg diff --git a/tcl/mfcmd.tcl b/tcl/mfcmd.tcl index ef09513e3..0e7c89662 100644 --- a/tcl/mfcmd.tcl +++ b/tcl/mfcmd.tcl @@ -1336,11 +1336,14 @@ proc is-saved {args} { # test at least one of the directories passed as argument is set in MODULEPATH proc is-used {args} { set modpathlist [getModulePathList] + set rawmodpathlist [getModulePathList returnempty 0 0] foreach path $args { - # transform given path in an absolute path to compare with dirs - # registered in the MODULEPATH env var which are returned absolute. - set abspath [getAbsolutePath $path] - if {$abspath in $modpathlist} { + # given path is first checked against the raw MODULEPATH content, as a + # relative entry there refers to the current working directory whereas + # given path is transformed in an absolute path relatively to the + # directory of the modulefile being evaluated + if {$path in $rawmodpathlist || [getAbsolutePath $path] in\ + $modpathlist} { return 1 } } diff --git a/tcl/modfind.tcl.in b/tcl/modfind.tcl.in index 3e48ccebd..52ad0e175 100644 --- a/tcl/modfind.tcl.in +++ b/tcl/modfind.tcl.in @@ -507,8 +507,10 @@ proc getModulePathList {{behavior returnempty} {resolv_var 1} {set_abs 1}} { if {$resolv_var} { set modpath [resolvStringWithEnv $modpath] } + # relative modulepath refers to current working directory of module + # command, not to the directory of the modulefile being evaluated if {$set_abs} { - set modpath [getAbsolutePath $modpath] + set modpath [getAbsolutePath $modpath 1] } lappendNoDup modpath_list $modpath } diff --git a/tcl/util.tcl b/tcl/util.tcl index 03873e530..cb3a8b696 100644 --- a/tcl/util.tcl +++ b/tcl/util.tcl @@ -94,9 +94,10 @@ proc runCommand {cmd args} { } } -proc getAbsolutePath {path} { +proc getAbsolutePath {path {from_cwd 0}} { # currently executing a modulefile or rc, so get the directory of this file - if {[currentState modulefile] ne {}} { + # unless resolution from current working directory is requested + if {!$from_cwd && [currentState modulefile] ne {}} { set curdir [file dirname [currentState modulefile]] # elsewhere get module command current working directory } else { diff --git a/testsuite/modulefiles.4/bar/1 b/testsuite/modulefiles.4/bar/1 index 615fbf555..e3c626638 100644 --- a/testsuite/modulefiles.4/bar/1 +++ b/testsuite/modulefiles.4/bar/1 @@ -78,3 +78,21 @@ if {[info exists env(TESTSUITE_LCOMPAT)]} { } } } +if {[info exists env(TESTSUITE_RELATIVE_MODULEPATH)]} { + switch -- $env(TESTSUITE_RELATIVE_MODULEPATH) { + load1 { + module load foo/1.0 + } + prereq1 { + prereq foo/1.0 + } + depon1 { + depends-on foo/1.0 + } + isused1 { + setenv TS_ISUSED_RAW [is-used $env(MODULEPATH)] + setenv TS_ISUSED_REL [is-used ..] + setenv TS_ISUSED_UNK [is-used [file tail $env(MODULEPATH)]] + } + } +} diff --git a/testsuite/modules.50-cmds/745-modulepath-relative.exp b/testsuite/modules.50-cmds/745-modulepath-relative.exp new file mode 100644 index 000000000..e2d7f0f3a --- /dev/null +++ b/testsuite/modules.50-cmds/745-modulepath-relative.exp @@ -0,0 +1,66 @@ +############################################################################## +# Modules Revision 3.0 +# Providing a flexible user environment +# +# File: modules.50-cmds/%M% +# Revision: %I% +# First Edition: 2026/09/15 +# Last Mod.: %U%, %G% +# +# Authors: Xavier Delaruelle, xavier.delaruelle@cea.fr +# +# Description: Testuite testsequence +# Command: load +# Modulefiles: bar, foo +# Sub-Command: is-used +# +# Comment: %C{ +# Test module load made during a modulefile evaluation when +# modulepath is set as a relative path +# }C% +# +############################################################################## + +skip_if_quick_mode + +set mp $modpath.4 +set mpre $modpathre.4 + +# modulepath relative to current working directory of module command +set relmp [file tail $env(TESTSUITEDIR)]/[file tail $mp] + +setenv_var MODULES_AUTO_HANDLING 1 + +set ans [list] +lappend ans [list set __MODULES_LMPREREQ bar/1&foo/1.0] +lappend ans [list set _LMFILES_ $mp/foo/1.0:$mp/bar/1] +lappend ans [list set LOADEDMODULES foo/1.0:bar/1] +lappend ans [list set __MODULES_LMTAG foo/1.0&auto-loaded] +set tserr [msg_top_load bar/1 {} foo/1.0 {}] + +# is-used given the raw relative entry or a path relative to the modulefile +# directory, which is not the current working directory +set ans2 [list] +lappend ans2 [list set TS_ISUSED_UNK 0] +lappend ans2 [list set TS_ISUSED_RAW 1] +lappend ans2 [list set TS_ISUSED_REL 1] +lappend ans2 [list set _LMFILES_ $mp/bar/1] +lappend ans2 [list set LOADEDMODULES bar/1] + +foreach modulepath [list $relmp ./$relmp] { + setenv_path_var MODULEPATH $modulepath + foreach tscase {load1 prereq1 depon1} { + setenv_var TESTSUITE_RELATIVE_MODULEPATH $tscase + testouterr_cmd bash {load bar/1} $ans $tserr + } + + setenv_var TESTSUITE_RELATIVE_MODULEPATH isused1 + testouterr_cmd bash {load bar/1} $ans2 {} +} + + +# +# Cleanup +# + +reset_test_env