diff --git a/core/base/inc/TEnv.h b/core/base/inc/TEnv.h index 50ee018258781..eb544b49e4206 100644 --- a/core/base/inc/TEnv.h +++ b/core/base/inc/TEnv.h @@ -24,7 +24,7 @@ class TWriteEnvParser; enum EEnvLevel { kEnvGlobal, kEnvUser, - kEnvLocal, + kEnvLocal, // For gEnv, the local level is disabled by default kEnvChange, kEnvAll }; @@ -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 + Bool_t fIsLocalLevelDisabled; //! By default, gEnv does not allow use of the local level TEnv(const TEnv&) = delete; TEnv& operator=(const TEnv&) = delete; @@ -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; } @@ -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 }; diff --git a/core/base/src/TEnv.cxx b/core/base/src/TEnv.cxx index 418a9ea3db86b..8a26c2d329473 100644 --- a/core/base/src/TEnv.cxx +++ b/core/base/src/TEnv.cxx @@ -17,6 +17,10 @@ Three types of config files are read: global, user and local files. The global file is `$ROOTSYS/etc/system` (or `ROOTETCDIR/system`) the user file is `$HOME/` and the local file is `./`. +For gEnv, the local level is disabled to prevent environment poisoning. +The behavior can be changed to be backwards-compatible by setting +`ROOTENV_USE_LOCAL=1`. + By setting the shell variable `ROOTENV_NO_HOME=1` the reading of the `$HOME/` resource file will be skipped. This might be useful in case the home directory resides on an auto-mounted remote file @@ -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/` /// is considered instead of `$HOME/`. +/// 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_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; @@ -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); } } } @@ -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); @@ -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); } @@ -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; @@ -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 { @@ -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); diff --git a/core/base/src/TROOT.cxx b/core/base/src/TROOT.cxx index 9ee7e332ca2b3..207ffb7269c9f 100644 --- a/core/base/src/TROOT.cxx +++ b/core/base/src/TROOT.cxx @@ -2148,7 +2148,8 @@ void TROOT::InitSystem() } // read default files - gEnv = new TEnv(".rootrc"); + const auto useLocalEnv = gSystem->Getenv("ROOTENV_USE_LOCAL"); + gEnv = new TEnv(".rootrc", /*disableLocalLevel=*/!useLocalEnv || !useLocalEnv[0]); ROOT::Internal::SetErrorSystemMsgHandler([](){ return gSystem->GetError(); }); SetErrorHandler(DefaultErrorHandler); diff --git a/core/base/test/TEnvTests.cxx b/core/base/test/TEnvTests.cxx index 074898751c94a..d9bdb01d871bc 100644 --- a/core/base/test/TEnvTests.cxx +++ b/core/base/test/TEnvTests.cxx @@ -1,5 +1,7 @@ #include "gtest/gtest.h" +#include "ROOT/TestSupport.hxx" + #include "TEnv.h" #include "TString.h" #include "TSystem.h" @@ -7,43 +9,105 @@ #include -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 /local with user-level config in and local-level config + // in /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 /local with user-level config in and local-level config - // in /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); + } }