N°9597 - Switch environment to a build environment must not be available

[Lenaick]

Base information

Question	Answer
Related to a SourceForge thread / Another PR / A GitHub Issue / Combodo ticket?	N°9597
Type of change?	Bug fix

Symptom

When checking compatibility during setup, a build environment is created. It should not be possible to switch to this environment via the system

Proposed solution

Add an exception if the target environment of switch_env ends with *-build

Checklist before requesting a review

• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't
• Is the PR clear and detailed enough so anyone can understand without digging in the code?

[greptile-apps]

Greptile Summary

This PR blocks direct startup switches to build environments. The main changes are:

• Moves startup environment selection into StartupService.
• Rejects requested environments ending in -build.
• Updates Composer classmaps for the new service.
• Adds unit tests for default, valid, and build environment selection.

Confidence Score: 3/5

This should be fixed before merging.

• Existing session state can still select production-build without a switch_env request.

• Non-existent *-build request values now abort startup instead of falling back to the current environment.

• The core startup path is affected, so these failures can affect normal application entry points.

• sources/Service/Startup/StartupService.php

Important Files Changed

Filename	Overview
sources/Service/Startup/StartupService.php	Centralizes environment selection and adds the build-environment guard, but misses session-held build environments.
application/startup.inc.php	Delegates environment selection to the new service before starting MetaModel.

_{Reviews (1): Last reviewed commit: "N°9597 - Switch environment to a build e..." | Re-trigger Greptile}

[greptile-apps]

Validate session environment The new guard only checks the requested switch_env, but setup writes the build environment into $_SESSION['itop_env'] during RunTimeEnvironment::InitDataModel(). On the next normal application request with no switch_env, this branch returns production-build from the session and startup.inc.php starts MetaModel from conf/production-build/config-itop.php, so the build environment remains selectable through existing session state.

[greptile-apps]

Check existence first This rejects every *-build value before checking whether it is a real environment or whether a switch would occur. A request like ?switch_env=does-not-exist-build used to fall through to the current session/default environment because the config file did not exist; now it throws during startup and logs an exception for a value that would otherwise be ignored.

Suggested change

	if (static::IsBuildEnvironment($sSwitchEnv)) {
	$oException = new CoreException("Switching to environment '$sSwitchEnv' is not allowed since it is a build environment");
	IssueLog::Exception("Trying to switch to environment '$sSwitchEnv' is not allowed since it is a build environment", $oException);
	throw $oException;
	}
	if (
	($sSwitchEnv != null)
	&& file_exists(APPCONF.$sSwitchEnv.'/'.ITOP_CONFIG_FILE)
	&& (Session::Get('itop_env') !== $sSwitchEnv)
	) {
	if (
	($sSwitchEnv != null)
	&& file_exists(APPCONF.$sSwitchEnv.'/'.ITOP_CONFIG_FILE)
	&& (Session::Get('itop_env') !== $sSwitchEnv)
	) {
	if (static::IsBuildEnvironment($sSwitchEnv)) {
	$oException = new CoreException("Switching to environment '$sSwitchEnv' is not allowed since it is a build environment");
	IssueLog::Exception("Trying to switch to environment '$sSwitchEnv' is not allowed since it is a build environment", $oException);
	throw $oException;
	}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[odain-cbd]

what is the meaning of $bAllowCache? it seems not related to opcache/apccache.

maybe you could document what is its purpose.

[Lenaick]

This is a boolean that determines whether the APC cache can be used or not, in conjunction with the apc_cache.enabled parameter
In the case of the environment switch, the cache must not be used
I've updated the documentation

[Hipska]

Maybe a small explanation in the php doc for both parameters would be handy?

[odain-cbd]

maybe you could change all local method and remove staitc which is not so usefull. and not recommended by phpunit (no mock possible)