Skip to content

[core] Disable local level of gEnv #22775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jblomer wants to merge 3 commits into
base: master
Choose a base branch
from
+137 −40
Open

[core] Disable local level of gEnv #22775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ccb07e9
[core] make test fixture for TEnv unit tests
jblomer Jul 8, 2026
3f49096
[core] fix up saving TEnv to custom user directory
jblomer Jul 8, 2026
b8076f5
[core] disable local level of gEnv
jblomer Jul 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions core/base/inc/TEnv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class TWriteEnvParser;
enum EEnvLevel {

@silverweed silverweed Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add a brief explanation of what each level means

kEnvGlobal,
kEnvUser,
kEnvLocal,
kEnvLocal, // For gEnv, the local level is disabled by default
kEnvChange,
kEnvAll
};
Expand Down Expand Up @@ -79,9 +79,10 @@ friend class TWriteEnvParser;
class TEnv : public TObject {

private:
THashList *fTable; // hash table containing env records
TString fRcName; // resource file base name
Bool_t fIgnoreDup; // ignore duplicates, don't issue warning
THashList *fTable; // hash table containing env records
TString fRcName; // resource file base name
Bool_t fIgnoreDup; // ignore duplicates, don't issue warning

@silverweed silverweed Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably initialize all these fields inline

Bool_t fIsLocalLevelDisabled; //! By default, gEnv does not allow use of the local level

TEnv(const TEnv&) = delete;
TEnv& operator=(const TEnv&) = delete;
Expand All @@ -90,7 +91,7 @@ class TEnv : public TObject {
const char *GetUserDirectory() const;

public:
TEnv(const char *name="");
TEnv(const char *name = "", bool disableLocalLevel = false);
virtual ~TEnv();

THashList *GetTable() const { return fTable; }
Expand Down Expand Up @@ -120,6 +121,8 @@ class TEnv : public TObject {
virtual void PrintEnv(EEnvLevel level = kEnvAll) const;
Bool_t IgnoreDuplicates(Bool_t ignore);

Bool_t IsLocalLevelDisabled() const { return fIsLocalLevelDisabled; }

ClassDefOverride(TEnv,2) // Handle ROOT configuration resources
};

Expand Down
39 changes: 34 additions & 5 deletions core/base/src/TEnv.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ Three types of config files are read: global, user and local files. The
global file is `$ROOTSYS/etc/system<name>` (or `ROOTETCDIR/system<name>`)
the user file is `$HOME/<name>` and the local file is `./<name>`.

For gEnv, the local level is disabled to prevent environment poisoning.

@silverweed silverweed Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a bit obscure what we mean by "environment poisoning". Could we make this comment a little more explicit on why it is a safety measure?

The behavior can be changed to be backwards-compatible by setting
`ROOTENV_UNSAFE_USE_LOCAL=1`.

@silverweed silverweed Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intuitively I would think that ROOTENV_UNSAFE_USE_LOCAL=0 would mean "explicitly disable local env". It would be beneficial to either make it that way or warn the user if we find a value 0 (or false) inside this variable that it's not doing what they think.


By setting the shell variable `ROOTENV_NO_HOME=1` the reading of
the `$HOME/<name>` resource file will be skipped. This might be useful
in case the home directory resides on an auto-mounted remote file
Expand Down Expand Up @@ -413,13 +417,19 @@ const char *TEnv::GetUserDirectory() const
/// In case the environment variable ROOTENV_USER_PATH is specified,
/// and ROOTENV_NO_HOME is not set, then `$ROOTENV_USER_PATH/<name>`
/// is considered instead of `$HOME/<name>`.
/// The file corresponding to `kEnvLocal` is read only if the local level
/// is not disabled. The local level is disabled by default for gEnv but
/// not for other instances of TEnv. Setting the `ROOTENV_UNSAFE_USE_LOCAL`
/// environment variable to a non-empty value will enable the local level for
/// gEnv.
/// If environment variables have to be avoided, a `rootlogon.C` script
/// can be created where where the environment can be set through an
/// invocation of TEnv::ReadFile.

TEnv::TEnv(const char *name)
TEnv::TEnv(const char *name, bool disableLocalLevel)
{
fIgnoreDup = kFALSE;
fIsLocalLevelDisabled = disableLocalLevel;

if (!name || !name[0] || !gSystem)
fTable = nullptr;
Expand All @@ -436,10 +446,12 @@ TEnv::TEnv(const char *name)
gSystem->PrependPathName(GetUserDirectory(), temp);
ReadFile(temp.Data(), kEnvUser);
if (strcmp(GetUserDirectory(), gSystem->WorkingDirectory())) {
ReadFile(name, kEnvLocal);
if (!IsLocalLevelDisabled())
ReadFile(name, kEnvLocal);
}
} else {
ReadFile(name, kEnvLocal);
if (!IsLocalLevelDisabled())
ReadFile(name, kEnvLocal);
}
}
}
Expand Down Expand Up @@ -616,6 +628,11 @@ Int_t TEnv::ReadFile(const char *fname, EEnvLevel level)
return -1;
}

if (IsLocalLevelDisabled() && (level == kEnvLocal)) {
Error("ReadFile", "local level disabled, won't read");
return -1;
}

FILE *ifp;
if ((ifp = fopen(fname, "r"))) {
TReadEnvParser rp(this, ifp, level);
Expand Down Expand Up @@ -673,7 +690,8 @@ void TEnv::Save()
return;
}

SaveLevel(kEnvLocal); // Be default, new items will be put into Local.
if (!IsLocalLevelDisabled())
SaveLevel(kEnvLocal); // By default, new items will be put into Local.
SaveLevel(kEnvUser);
SaveLevel(kEnvGlobal);
}
Expand All @@ -693,6 +711,11 @@ void TEnv::SaveLevel(EEnvLevel level)
return;
}

if (IsLocalLevelDisabled() && (level == kEnvLocal)) {
Error("SaveLevel", "local level disabled, won't save");
return;
}

TString rootrcdir;
FILE *ifp, *ofp;

Expand All @@ -701,7 +724,8 @@ void TEnv::SaveLevel(EEnvLevel level)
sname += fRcName;
rootrcdir = gSystem->PrependPathName(TROOT::GetEtcDir(), sname);
} else if (level == kEnvUser) {
rootrcdir = gSystem->PrependPathName(GetUserDirectory(), fRcName);
TString sname = fRcName;
rootrcdir = gSystem->PrependPathName(GetUserDirectory(), sname);
} else if (level == kEnvLocal) {
rootrcdir = fRcName;
} else {
Expand Down Expand Up @@ -752,6 +776,11 @@ void TEnv::SaveLevel(EEnvLevel level)
void TEnv::SetValue(const char *name, const char *value, EEnvLevel level,
const char *type)
{
if (IsLocalLevelDisabled() && (level == kEnvLocal)) {
Error("SetValue", "local level disabled, won't set or change value");
return;
}

if (!fTable)
fTable = new THashList(1000);

Expand Down
3 changes: 2 additions & 1 deletion core/base/src/TROOT.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2148,7 +2148,8 @@ void TROOT::InitSystem()
}

// read default files
gEnv = new TEnv(".rootrc");
const auto useUnsafeLocalEnv = gSystem->Getenv("ROOTENV_UNSAFE_USE_LOCAL");
gEnv = new TEnv(".rootrc", /*disableLocalLevel=*/!useUnsafeLocalEnv || !useUnsafeLocalEnv[0]);

ROOT::Internal::SetErrorSystemMsgHandler([](){ return gSystem->GetError(); });
SetErrorHandler(DefaultErrorHandler);
Expand Down
122 changes: 93 additions & 29 deletions core/base/test/TEnvTests.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,113 @@
#include "gtest/gtest.h"

#include "ROOT/TestSupport.hxx"

#include "TEnv.h"
#include "TString.h"
#include "TSystem.h"
#include "TUUID.h"

#include <string>

TEST(TEnv, ROOTENV_USER_PATH)
class TEnvTest : public testing::Test {
protected:
TString fRcName;
TString fUserDir;
TString fUserConfig;
TString fLocalDir;
TString fLocalConfig;

void SetUp() override
{
fRcName = "testenv";

// Create directories structure <UUID>/local with user-level config in <UUID> and local-level config
// in <UUID>/local

const auto uuid = TUUID::UUIDv4();

fUserDir = uuid.AsString();
fUserConfig = fRcName;
gSystem->PrependPathName(fUserDir.Data(), fUserConfig);

fLocalDir = "local";
gSystem->PrependPathName(fUserDir, fLocalDir);
fLocalConfig = fRcName;
gSystem->PrependPathName(fLocalDir.Data(), fLocalConfig);

fLocalDir.ReplaceAll("\\", "\\\\");
ASSERT_EQ(0, gSystem->mkdir(fLocalDir.Data(), /*recursive=*/true));

gSystem->Setenv("ROOTENV_USER_PATH", fUserDir.Data());
}

void TearDown() override
{
gSystem->Unsetenv("ROOTENV_USER_PATH");

gSystem->Unlink((fLocalConfig + ".bak").Data());
gSystem->Unlink(fLocalConfig.Data());
gSystem->Unlink(fLocalDir.Data());
gSystem->Unlink((fUserConfig + ".bak").Data());
gSystem->Unlink(fUserConfig.Data());
gSystem->Unlink(fUserDir.Data());
}
};

TEST_F(TEnvTest, ROOTENV_USER_PATH)
{
auto uuid = TUUID::UUIDv4();

// Create directories structure <UUID>/local with user-level config in <UUID> and local-level config
// in <UUID>/local
const TString rcname = "testenv";
const TString userDir = uuid.AsString();
TString userConfig = rcname;
gSystem->PrependPathName(userDir.Data(), userConfig);
const TString userBak = userConfig + ".bak";
TString localDir = "local";
gSystem->PrependPathName(userDir, localDir);
TString localConfig = rcname;
gSystem->PrependPathName(localDir.Data(), localConfig);
TString homeConfig = rcname;
TString homeConfig = fRcName;
gSystem->PrependPathName(gSystem->HomeDirectory(), homeConfig);

localDir.ReplaceAll("\\", "\\\\");
ASSERT_EQ(0, gSystem->mkdir(localDir.Data(), /*recursive=*/true));
// Initially, neither the local nor the user config file exist
EXPECT_TRUE(gSystem->AccessPathName(fLocalConfig.Data()));
EXPECT_TRUE(gSystem->AccessPathName(fUserConfig.Data()));

gSystem->Setenv("ROOTENV_USER_PATH", userDir.Data());
EXPECT_TRUE(gSystem->AccessPathName(localConfig.Data()));
EXPECT_TRUE(gSystem->AccessPathName(userConfig.Data()));

TEnv env("testenv");
TEnv env(fRcName);
EXPECT_FALSE(env.IsLocalLevelDisabled());
env.SetValue("EnvRec");
EXPECT_EQ(1, env.GetValue("EnvRec", 0));
env.SaveLevel(kEnvUser);

EXPECT_TRUE(gSystem->AccessPathName(localConfig.Data()));
EXPECT_FALSE(gSystem->AccessPathName(userConfig.Data()));
// Now, the user config file should have been created in the custom directory
EXPECT_TRUE(gSystem->AccessPathName(fLocalConfig.Data()));
EXPECT_FALSE(gSystem->AccessPathName(fUserConfig.Data()));
EXPECT_TRUE(gSystem->AccessPathName(homeConfig.Data()));

EXPECT_EQ(0, gSystem->Unlink(localDir.Data()));
EXPECT_EQ(0, gSystem->Unlink(userConfig.Data()));
EXPECT_EQ(0, gSystem->Unlink(userBak.Data()));
EXPECT_EQ(0, gSystem->Unlink(userDir.Data()));
gSystem->Unsetenv("ROOTENV_USER_PATH");
TString pwd = gSystem->pwd();
EXPECT_TRUE(gSystem->cd(fLocalDir.Data()));
env.SaveLevel(kEnvLocal);
EXPECT_TRUE(gSystem->cd(pwd.Data()));

EXPECT_FALSE(gSystem->AccessPathName(fLocalConfig.Data()));
EXPECT_FALSE(gSystem->AccessPathName(fUserConfig.Data()));
EXPECT_TRUE(gSystem->AccessPathName(homeConfig.Data()));
}

TEST_F(TEnvTest, DisableLocalLevel)
{
EXPECT_TRUE(gEnv->IsLocalLevelDisabled());

gEnv->WriteFile(fLocalConfig.Data());
EXPECT_FALSE(gSystem->AccessPathName(fLocalConfig.Data()));

TEnv env(fRcName, /*disableLocalLevel=*/true);
{
ROOT::TestSupport::CheckDiagsRAII checkDiag;
checkDiag.requiredDiag(kError, "TEnv::ReadFile", "local level disabled, won't read", /*matchFullMessage=*/false);
EXPECT_EQ(-1, env.ReadFile(fLocalConfig.Data(), kEnvLocal));
}

{
ROOT::TestSupport::CheckDiagsRAII checkDiag;
checkDiag.requiredDiag(kError, "TEnv::SaveLevel", "local level disabled, won't save", /*matchFullMessage=*/false);
env.SaveLevel(kEnvLocal);
EXPECT_TRUE(gSystem->AccessPathName(fRcName));
}

{
ROOT::TestSupport::CheckDiagsRAII checkDiag;
checkDiag.requiredDiag(kError, "TEnv::SetValue", "local level disabled", /*matchFullMessage=*/false);
env.SetValue("xyz", kEnvLocal);
}
}
Loading