N°8632 - Various fixes (#814)

* N°8632 - Check existence of parameter file within iTop

* N°8632 - block parameter file from request

* log on error

* PHP CS fixer

* N°8632 - param files must be outside iTop

* PHP CS fixer

* Update webservices/export.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update tests/php-unit-tests/unitary-tests/application/utilsTest.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* ✅ Fix CI

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

.gitignore

@@ -58,6 +58,9 @@ tests/*/vendor/*
 /tests/php-unit-tests/phpunit.xml
 /tests/php-unit-tests/postbuild_integration.xml
 
+# PHP CS Fixer: Cache file
+/.php-cs-fixer.cache
+
 
 # Jetbrains
 /.idea/**

application/utils.inc.php

@@ -181,6 +181,9 @@ class utils
 
 	protected static function LoadParamFile($sParamFile)
 	{
+		if (utils::RealPath($sParamFile, APPROOT) !== false) {
+			throw new Exception("File '".utils::HtmlEntities($sParamFile)."' should be outside iTop");
+		}
 		if (!file_exists($sParamFile)) {
 			throw new Exception("Could not find the parameter file: '".utils::HtmlEntities($sParamFile)."'");
 		}

datamodels/2.x/itop-backup/check-backup.php

@@ -199,15 +199,15 @@ function RaiseAlarm($sMessage)
 //////////
 // Main
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	echo "Error: ".$e->GetMessage()."\n";
-	exit;
-}
-
 if (utils::IsModeCLI()) {
-	SetupUtils::CheckPhpAndExtensionsForCli(new CLIPage('Check backup utility'));
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		echo 'Error: '.$e->GetMessage()."\n";
+		exit;
+	}
+	$oP = new CLIPage('Check backup utility');
+	SetupUtils::CheckPhpAndExtensionsForCli($oP);
 
 	echo date('Y-m-d H:i:s')." - running check-backup utility\n";
 	try {

datamodels/2.x/itop-backup/common.cli-execution.php

@@ -88,16 +88,15 @@ if (utils::IsModeCLI()) {
 	$oP = new CLIPage(GetOperationName());
 
 	SetupUtils::CheckPhpAndExtensionsForCli($oP);
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		ExitError($oP, $e->GetMessage());
+	}
 } else {
 	$oP = new WebPage(GetOperationName());
 }
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	ExitError($oP, $e->GetMessage());
-}
-
 ExecuteMainOperation($oP);
 
 $oP->output();

synchro/synchro_exec.php

@@ -67,18 +67,18 @@ function ReadMandatoryParam($oP, $sParam, $sSanitizationFilter = 'parameter')
 if (utils::IsModeCLI()) {
 	$oP = new CLIPage(Dict::S("TitleSynchroExecution"));
 	SetupUtils::CheckPhpAndExtensionsForCli($oP, -2);
+
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		$oP->p('Error: '.$e->GetMessage());
+		$oP->output();
+		exit - 2;
+	}
 } else {
 	$oP = new WebPage(Dict::S("TitleSynchroExecution"));
 }
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	$oP->p("Error: ".$e->GetMessage());
-	$oP->output();
-	exit -2;
-}
-
 if (utils::IsModeCLI()) {
 	$sAuthUser = ReadMandatoryParam($oP, 'auth_user', 'raw_data');
 	$sAuthPwd = ReadMandatoryParam($oP, 'auth_pwd', 'raw_data');

synchro/synchro_import.php

@@ -224,18 +224,18 @@ function ChangeDateFormat($sProposedDate, $sFormat, $bDateOnly)
 if (utils::IsModeCLI()) {
 	$oP = new CLIPage(Dict::S('TitleSynchroExecution'));
 	SetupUtils::CheckPhpAndExtensionsForCli($oP, -2);
+
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		$oP->p('Error: '.$e->GetMessage());
+		$oP->output();
+		exit - 2;
+	}
 } else {
 	$oP = new CLILikeWebPage(Dict::S('TitleSynchroExecution'));
 }
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	$oP->p("Error: ".$e->GetMessage());
-	$oP->output();
-	exit -2;
-}
-
 if (utils::IsModeCLI()) {
 	// Next steps:
 	//   specific arguments: 'csvfile'

tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php

@@ -123,7 +123,7 @@ abstract class ItopDataTestCase extends ItopTestCase
 	{
 		parent::setUp();
 
-		\IssueLog::Error($this->getName());
+		\IssueLog::Info("Running phpunit test: ".$this->getName());
 
 		$this->PrepareEnvironment();
 
@@ -1446,9 +1446,6 @@ abstract class ItopDataTestCase extends ItopTestCase
 	{
 		$sConfigPath = MetaModel::GetConfig()->GetLoadedFile();
 		clearstatcache();
-		echo sprintf("rights via ls on %s:\n %s \n", $sConfigPath, exec("ls -al $sConfigPath"));
-		$sFilePermOutput = substr(sprintf('%o', fileperms('/etc/passwd')), -4);
-		echo sprintf("rights via fileperms on %s:\n %s \n", $sConfigPath, $sFilePermOutput);
 
 		$this->sConfigTmpBackupFile = tempnam(sys_get_temp_dir(), "config_");
 		MetaModel::GetConfig()->WriteToFile($this->sConfigTmpBackupFile);

tests/php-unit-tests/unitary-tests/application/utilsTest.php

@@ -996,4 +996,46 @@ HTML,
 			],
 		];
 	}
+
+	public function testLoadParamFile()
+	{
+		$sTmpFileInsideItop = APPROOT.'data/test/testLoadParamFile.params';
+		$sDir = dirname($sTmpFileInsideItop);
+		if (!is_dir($sDir)) {
+			mkdir($sDir, 0777, true);
+		}
+		$sParamName = 'IP1';
+		$sParamValue = 'IV1';
+		$sParams = <<<INI
+# comment
+$sParamName = $sParamValue
+INI;
+		file_put_contents($sTmpFileInsideItop, $sParams);
+
+		try {
+			$this->expectException(\Exception::class);
+			$this->expectExceptionMessage("File '$sTmpFileInsideItop' should be outside iTop");
+			self::InvokeNonPublicStaticMethod(utils::class, 'LoadParamFile', [$sTmpFileInsideItop]);
+			self::assertNotEquals($sParamValue, utils::ReadParam($sParamName, null), "utils::LoadParamFile() should NOT have loaded the file: $sTmpFileInsideItop");
+		} finally {
+			if (file_exists($sTmpFileInsideItop)) {
+				unlink($sTmpFileInsideItop);
+			}
+		}
+
+		$sParamName = 'OP2';
+		$sParamValue = 'OV2';
+
+		$sTmpFileOutsideItop = tempnam(sys_get_temp_dir(), 'utils-test');
+		$sParams = <<<INI
+# comment
+$sParamName = $sParamValue
+INI;
+
+		file_put_contents($sTmpFileOutsideItop, $sParams);
+		self::InvokeNonPublicStaticMethod(utils::class, 'LoadParamFile', [$sTmpFileOutsideItop]);
+		self::assertEquals($sParamValue, utils::ReadParam($sParamName, null), "utils::LoadParamFile() should have loaded the file: $sTmpFileOutsideItop");
+
+		unlink($sTmpFileOutsideItop);
+	}
 }

webservices/cron.php

@@ -446,18 +446,16 @@ function ReSyncProcesses($oP, $bVerbose, $bDebug)
 //
 
 set_time_limit(0); // Some background actions may really take long to finish (like backup)
-
-$bIsModeCLI = utils::IsModeCLI();
-if ($bIsModeCLI) {
-	$oP = new CLIPage("iTop - cron");
-
-	SetupUtils::CheckPhpAndExtensionsForCli($oP, EXIT_CODE_FATAL);
-} else {
-	$oP = new WebPage("iTop - cron");
-}
-
 try {
-	utils::UseParamFile();
+	$bIsModeCLI = utils::IsModeCLI();
+	if ($bIsModeCLI) {
+		$oP = new CLIPage("iTop - cron");
+
+		SetupUtils::CheckPhpAndExtensionsForCli($oP, EXIT_CODE_FATAL);
+		utils::UseParamFile();
+	} else {
+		$oP = new WebPage("iTop - cron");
+	}
 
 	$bVerbose = utils::ReadParam('verbose', false, true /* Allow CLI */);
 	$bDebug = utils::ReadParam('debug', false, true /* Allow CLI */);

webservices/export.php

@@ -43,10 +43,12 @@ const EXIT_CODE_ERROR = -1;
 const EXIT_CODE_FATAL = -2;
 
 try {
-	// Do this before loging, in order to allow setting user credentials from within the file
-	utils::UseParamFile();
+	if (utils::IsModeCLI()) {
+		// Do this before logging, in order to allow setting user credentials from within the file
+		utils::UseParamFile();
+	}
 } catch (Exception $e) {
-	echo "Error: ".$e->GetMessage()."<br/>\n";
+	echo "Error: ".$e->GetMessage()."\n";
 	exit(EXIT_CODE_FATAL);
 }
 

webservices/import.php

@@ -208,18 +208,18 @@ $oCtx = new ContextTag(ContextTag::TAG_IMPORT);
 if (utils::IsModeCLI()) {
 	$oP = new CLIPage("iTop - Bulk import");
 	SetupUtils::CheckPhpAndExtensionsForCli($oP, -2);
+
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		$oP->p('Error: '.$e->GetMessage());
+		$oP->output();
+		exit(-2);
+	}
 } else {
 	$oP = new CSVPage("iTop - Bulk import");
 }
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	$oP->p("Error: ".$e->GetMessage());
-	$oP->output();
-	exit(-2);
-}
-
 if (utils::IsModeCLI()) {
 	// Next steps:
 	//   specific arguments: 'csvfile'

webservices/rest.php

@@ -91,10 +91,8 @@ if (empty($sJsonString)) {
 
 $sProvider = '';
 
-$oKPI = new ExecutionKPI();
 try {
-	utils::UseParamFile();
-
+	$oKPI = new ExecutionKPI();
 	$oKPI->ComputeAndReport('Data model loaded');
 
 	// N°6358 - force credentials for REST calls