Apply suggestion from @greptile-apps[bot]

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

.gitignore

@@ -33,7 +33,6 @@ tests/*/vendor/*
 !/data/index.php
 !/data/web.config
 !/data/exclude.txt
-!/data/.compilation-symlinks
 
 # iTop extensions
 /extensions/**

AGENTS.md

@@ -0,0 +1,133 @@
+# AGENTS.md: iTop
+
+Instructions for AI coding agents and AI code reviewers working on this repository.
+iTop (by Combodo) is an open-source ITSM/CMDB web application: PHP backend, JS/HTML/SCSS front-end, XML datamodel.
+Write all code, identifiers, comments and review comments in English.
+
+## Critical rules (security first)
+
+These rules apply to every change. In code review, a violation is a P1 issue.
+
+### Input: sanitize with an explicit filter
+- Read request parameters only with `utils::ReadParam()` or `utils::ReadPostedParam()`, never from `$_GET`, `$_POST`, `$_REQUEST` or `$_COOKIE`.
+- Always pass the filter explicitly as a `utils::ENUM_SANITIZATION_FILTER_*` constant, the most restrictive that fits (`INTEGER`, `CLASS`, `FIELD_NAME`, `TRANSACTION_ID`, `URL`...).
+- `utils::ENUM_SANITIZATION_FILTER_RAW_DATA` requires a comment explaining why no other filter fits.
+
+### Output: escape at consumption time, in the right context
+- Sanitizing input is not escaping output. Escape where the value is rendered, never upstream in the backend, and never store escaped values.
+- PHP to HTML: `utils::EscapeHtml()`. Not `htmlentities()` nor `utils::HtmlEntities()` (they escape every non-ASCII char).
+- Twig: rely on auto-escaping; use `|escape('js')` in JS contexts and `|sanitize(constant('utils::ENUM_SANITIZATION_FILTER_VARIABLE_NAME'))` (or `..._ELEMENT_IDENTIFIER`) for generated identifiers. `|raw` only for trusted HTML, with a comment.
+- JS to DOM: escape untrusted values with `CombodoSanitizer.EscapeHtml()` before building HTML; prefer `.text()` to `.html()` or `innerHTML`.
+- User-provided rich HTML goes through `HTMLSanitizer::Sanitize()`.
+
+### CSRF
+- Never change state on a GET request.
+- A form that changes state embeds a transaction id from `utils::GetNewTransactionId()`; its handler calls `utils::IsTransactionValid()` before any modification.
+- Ajax calls use jQuery or `CombodoHTTP.Fetch()`, which send the `X-Combodo-Ajax` header checked by `pages/ajax.render.php`. Never use raw `fetch()` or `XMLHttpRequest`.
+
+### Data access and permissions
+- OQL: use placeholders (`WHERE name = :name`) and pass values in the `$aParams` argument of `DBObjectSearch::FromOQL()`. Never concatenate input into OQL or SQL; raw SQL values go through `CMDBSource::Quote()`.
+- Every new entry point (page, ajax operation, controller route, webservice) enforces authentication (`LoginWebPage::DoLogin()` / `DoLoginEx()`) and rights (`UserRights::IsActionAllowed()`, `UserRights::IsAdministrator()`...).
+- `AllowAllData()` bypasses user rights and scopes: only when strictly needed, with a comment.
+
+### Secrets and logs
+- Never log passwords, tokens, API keys, session ids or personal data. Log security events on `LogChannels::SECURITY`.
+
+### Security configuration parameters: secure by default
+- The default value of a configuration parameter is always the secure one.
+- An option that weakens a protection is opt-in: a `bool` named `security.disable_<protection>`, default `false`, whose description states the risk when set to `true` (model: `security.disable_inline_documents_sandbox` in `core/config.class.inc.php`). Do not add new `security.enable_*` parameters.
+
+## Project overview
+
+- Supported PHP versions: the `require.php` constraint in `composer.json`. Never use a language feature above its minimum version.
+- Branches: `develop` (next version), `support/x.y` (maintained versions).
+- Layout:
+    - `core/`: ORM, `MetaModel`, OQL, configuration (`core/config.class.inc.php`), logs. Legacy structure, meant to be replaced by `sources/` in the long term.
+    - `application/`: legacy UI layer, `utils` (`application/utils.inc.php`), exceptions (`application/exceptions/`). Legacy structure, meant to be replaced by `sources/` in the long term.
+    - `sources/`: namespaced code (`Combodo\iTop\...`, namespace matches the folder): services, controllers, UI blocks, forms.
+    - `pages/`: legacy entry points (`UI.php`, `ajax.render.php`).
+    - `datamodels/2.x/<module>/`: modules (`module.<module>.php`, `datamodel.<module>.xml`, `dictionaries/`).
+    - `dictionaries/`: core translations. `templates/`: Twig. `js/`: iTop JS. `css/`: SCSS and compiled CSS.
+    - `tests/`: PHPUnit (`php-unit-tests/`), PHP-CS-Fixer (`php-code-style/`), PHPStan (`php-static-analysis/`).
+- Never edit:
+    - `lib/` (Composer `vendor-dir`) and `node_modules/` (npm): third-party code.
+    - `env-*/`: generated by the datamodel compilation, not versioned.
+    - `data/`: temporary or cached files, not versioned.
+
+## Commands
+
+- Unit tests (needs an installed iTop instance, see `tests/php-unit-tests/README.md`): run `composer install` in `tests/php-unit-tests/`, then `tests/php-unit-tests/vendor/bin/phpunit --configuration tests/php-unit-tests/phpunit.xml.dist <path>`.
+- PHP code style: run `composer install` in `tests/php-code-style/`, then `tests/php-code-style/vendor/bin/php-cs-fixer fix --config tests/php-code-style/.php-cs-fixer.dist.php <path>`.
+- Static analysis: PHPStan, see `tests/php-static-analysis/README.md` (analyses `env-production`, needs a setup).
+- After adding, renaming or moving a PHP class in `core/`, `application/` or `sources/`: run `composer dump-autoload` (the classmap is authoritative: an unlisted class is not found) and commit `lib/composer/autoload_classmap.php` and `lib/composer/autoload_static.php`.
+- After changing SCSS: regenerate and commit the compiled CSS (the pre-commit hook rejects SCSS without CSS).
+- After changing datamodel XML: the datamodel must be recompiled by the iTop setup or toolkit. An agent cannot do it: tell the user.
+
+## Coding conventions
+
+### PHP
+- Formatting is enforced by PHP-CS-Fixer (PSR-12, tabs): do not hand-format.
+- Variables: mixed case with a type prefix: `$s` string, `$i` int, `$b` bool, `$a` array, `$o` object, `$f` float, no prefix for mixed types. Loop counters (`$i`, `$j`) are fine.
+- Functions and methods: UpperCamelCase starting with a verb (`GetInstance()`, `ReadParam()`). Classes: UpperCamelCase.
+- Array keys: snake_case, documented with PHPDoc array shapes (`array{key: type}`) or generics (`array<string, \CheckResult>`).
+- New code uses strict comparison (`===`). `==` only with a comment explaining why.
+- Test empty strings with `utils::IsNullOrEmptyString()`, not `empty()` (`"0"` is empty).
+- Type hints in new code only. Do not add them to existing signatures (it breaks extensions) nor return types to fluent methods.
+- PHPDoc: `@since` is mandatory on new classes and on new or changed methods and properties, e.g. `@since 3.3.0 N°1234 - Short description`. Use `bool`, not `boolean`.
+- Do not modify existing copyright headers.
+- Configuration parameters: lowercase, domain prefix, underscores (`email_transport_smtp.host`), declared in `core/config.class.inc.php`.
+
+### Services
+- A new service is a singleton: private constructor, `private static $oInstance`, `public static function GetInstance()`. Model: `sources/Service/Module/ModuleService.php`.
+
+### Exceptions and logs
+- Throw iTop exceptions (`CoreException` and its subclasses in `application/exceptions/`) rather than a bare `\Exception`.
+- Log a caught exception with `ExceptionLog::LogException($oException)`, not `IssueLog::Error($oException->getMessage())`.
+- Other logs: `IssueLog` with a `LogChannels::*` channel.
+- Never swallow an exception: an empty `catch` needs a comment explaining why.
+
+### JavaScript, Twig, SCSS
+- JS functions: UpperCamelCase starting with a verb. Follow the escaping and Ajax rules above.
+- Indentation follows `.editorconfig` (PHP, JS: tabs; SCSS, XML: 2 spaces; Twig: 4 spaces).
+
+### Translations
+- No hard-coded UI text: use `Dict::S()`, `Dict::Format()`, or `|dict_s` / `|dict_format` in Twig.
+- Add new entries in English in the `en` dictionary file (`dictionaries/en.dictionary.itop.*.php` or `datamodels/2.x/<module>/dictionaries/en.dict.<module>.php`).
+- Add new entries in other existing languages as well.
+- In other languages, a ~~ suffix marks an untranslated entry: never remove it without translating.
+
+### Commits
+- Start either with the ticket reference (`N°*`) or with one of the supported Gitmoji code (`:white_check_mark:`, `:green_heart:`, `:memo:`, `:bookmark:`), imperative present tense, first line 72 characters max, describe the functional change. More details in the body if needed.
+
+## Dependencies and licenses
+
+- Dependencies are versioned in the repository: Composer in `lib/`, npm in `node_modules/`. Change versions in `composer.json` / `package.json` and run Composer or npm; never edit the files directly.
+- Every new dependency must have an entry in `setup/licenses/community-licenses.xml`, with a license compatible with AGPL-3.0. For Composer packages, propose to the user to run `php .make/license/updateLicenses.php` (needs bash, composer and jq) and review its diff; npm packages and other bundled assets are added by hand.
+
+## Datamodel
+
+- The datamodel ("DM") is compiled from the XML of the modules (`datamodels/2.x/<module>/datamodel.<module>.xml`) and of installed extensions into `env-<environment>/` (usually `env-production/`), accessed at runtime through `MetaModel`.
+- Changing an existing class or attribute impacts every customer upgrade and their extensions (which alter the datamodel through XML deltas): treat it as high-impact and flag it.
+
+## Tests
+
+- Location: `tests/php-unit-tests/unitary-tests/`, mirroring the source path (tests for `application/foo.php` go in `unitary-tests/application/`). Cross-application data consistency: `integration-tests/`.
+- Naming: `MyClass` is tested by `MyClassTest`; `MyMethod()` by `testMyMethod()`.
+- Use the lightest base class that works:
+    1. `PHPUnit\Framework\TestCase`: pure logic.
+    2. `Combodo\iTop\Test\UnitTest\ItopTestCase`: iTop helpers, no datamodel.
+    3. `Combodo\iTop\Test\UnitTest\ItopDataTestCase`: starts the datamodel (`MetaModel`) and rolls back CRUD operations. Slow: only when the test needs the datamodel or iTop objects.
+    4. `Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase`: a non-standard datamodel.
+- Process isolation (`@runInSeparateProcess`, about 800 ms per `ItopDataTestCase` test) only when needed, on the test, never globally.
+- Do not leave static state behind (`ContextTag::AddContext()`, `EventService::RegisterListener()`, `Dict::Add()`): checked by `integration-tests/DetectStaticPollutionTest.php`.
+- `markTestSkipped()` only for a temporarily disabled test.
+- More details: `tests/php-unit-tests/README.md`.
+
+## Glossary
+
+- **DM / datamodel**: the application data model (classes, attributes, lifecycles, menus), compiled from XML and PHP into `env-*/`.
+- **Module**: a unit of iTop code under `datamodels/2.x/<module>/`. **Extension**: a packaged set of modules installed on top of iTop.
+- **OQL**: iTop's object query language.
+- **Transaction id**: iTop's CSRF token.
+- **Console / back-office**: the main UI for agents. **Portal**: the end-user UI (`datamodels/2.x/itop-portal-base/`).
+- **N°xxxx**: Combodo internal ticket number, used in commits and `@since`.

REVIEW.md

@@ -0,0 +1,30 @@
+# REVIEW.md: AI code review instructions for iTop
+
+For automated reviewers (GitHub Copilot code review, Greptile, ...). Project conventions and security rules are in `AGENTS.md`; this file says what to comment on and how.
+
+## Output
+- Write every comment in English.
+- Start each comment with its priority: `[P1-Security]`, `[P2-Regression]`, `[P2-Performance]`, `[P2-Architecture]` or `[P3]`.
+- Report every P1 and P2 issue. Report at most 3 `[P3]` comments per PR: keep the most useful ones.
+- Put test suggestions in a single `[Tests]` comment; it does not count toward the P3 limit.
+- If nothing is above P3, say so in one sentence. No praise, no file-by-file summary.
+
+## Priorities
+1. **P1 Security**: XSS, CSRF, OQL/SQL injection, missing authentication or rights check, sensitive data in logs, unsafe file or path handling, any breach of the "Critical rules" in `AGENTS.md`, a security option that is not secure by default or not named `security.disable_*`.
+2. **P2 Regression**: behavior change in an API extensions rely on (public classes and methods, extension interfaces, events, configuration parameters, REST/webservices); change to an existing datamodel class or attribute; removed or renamed dictionary entry.
+3. **P2 Performance**: queries inside loops, loading whole object sets when a count or a subset is enough, loading all attributes of an object in a loop instead of calling `\DBObjectSet::OptimizeColumnLoad`, repeated expensive calls.
+4. **P2 Architecture**: logic in the wrong layer, a new service that is not a singleton, a new PHP class outside of the `sources/` folder, a new PHP class in `core/`, `application/` or `sources/` without the updated `lib/composer/autoload_classmap.php`, a new dependency missing from `setup/licenses/community-licenses.xml`, SCSS changed without the compiled CSS.
+5. **P3 Other**: unclear names (variables, functions, classes), deviations from `AGENTS.md` conventions, typos and grammar in identifiers, comments and dictionary entries, UI labels that could be clearer or more consistent with existing ones.
+
+## Tests
+- When the PR changes behavior, propose the missing test cases: what to assert, in which file of `tests/php-unit-tests/unitary-tests/`.
+- Propose missing test cases to ensure maximum coverage of the new code, and that it is not broken by future changes.
+- Check readability, and that the base class is the lightest that works: `ItopDataTestCase` only if the test needs the datamodel. Flag unjustified process isolation and static state left behind.
+
+## Do not comment on
+- Formatting: indentation, spacing, line breaks, brace placement. PHP-CS-Fixer and `.editorconfig` handle it.
+- Third-party code in `lib/` and `node_modules/`, and generated code in `env-*/`. Only check whether `lib/composer/autoload_*.php` should have changed.
+- Code the PR does not touch, unless the change breaks it or it needs to be adjusted accordingly.
+
+## Vulnerability fixes
+This repository is public. If a change seems to fix an existing vulnerability, never describe the vulnerability, how to exploit it, or which versions are affected: neither in comments nor in the PR summary. Review it as an ordinary change. If the fix looks incomplete, describe only the missing check in neutral technical terms (for example "this value is not escaped here").

application/displayblock.class.inc.php

@@ -2075,17 +2075,8 @@ class MenuBlock extends DisplayBlock
 									$aAllowedProfiles = MetaModel::GetConfig()->Get('concurrent_lock_override_profiles');
 									$bCanKill = false;
 
-									$oUser = UserRights::GetUserObject();
-									$aUserProfiles = [];
-									if (!is_null($oUser)) {
-										$oProfileSet = $oUser->Get('profile_list');
-										while ($oProfile = $oProfileSet->Fetch()) {
-											$aUserProfiles[$oProfile->Get('profile')] = true;
-										}
-									}
-
 									foreach ($aAllowedProfiles as $sProfile) {
-										if (array_key_exists($sProfile, $aUserProfiles)) {
+										if (UserRights::HasProfile($sProfile)) {
 											$bCanKill = true;
 											break;
 										}

application/forms.class.inc.php

@@ -1202,7 +1202,6 @@ class DesignerComboField extends DesignerFormField
 	{
 		$sId = $this->oForm->GetFieldId($this->sCode);
 		$sName = $this->oForm->GetFieldName($this->sCode);
-		$sChecked = $this->defaultValue ? 'checked' : '';
 		$sMandatory = $this->bMandatory ? 'true' : 'false';
 		$sReadOnly = $this->IsReadOnly() ? 'disabled="disabled"' : '';
 		if ($this->IsSorted()) {
@@ -1245,8 +1244,7 @@ class DesignerComboField extends DesignerFormField
 				} else {
 					$sSelected = ($sKey == $this->defaultValue) ? 'selected' : '';
 				}
-				// Quick and dirty: display the menu parents as a tree
-				$sHtmlValue = str_replace(' ', ' ', $sDisplayValue);
+				$sHtmlValue = $this->PrepareValueForRendering($sDisplayValue);
 				$sHtml .= "<option value=\"".utils::EscapeHtml($sKey)."\" $sSelected>$sHtmlValue</option>";
 			}
 			$sHtml .= "</select></span>";
@@ -1263,6 +1261,14 @@ EOF
 
 	}
 
+	protected function PrepareValueForRendering($sValue = null): ?string
+	{
+		if (is_null($sValue)) {
+			$sValue = $this->defaultValue;
+		}
+		return $sValue;
+	}
+
 	public function ReadParam(&$aValues)
 	{
 		parent::ReadParam($aValues);

application/shortcut.class.inc.php

@@ -109,10 +109,18 @@ $('#shortcut_rename_dlg').dialog({
 	modal: true,
 	title: '$sDialogTitle',
 	buttons: [
-	{ text: "$sOkButtonLabel", click: ShortcutRenameOK},
-	{ text: "$sCancelButtonLabel", click: function() {
-		$(this).dialog( "close" ); $(this).remove();
-	} },
+        { 
+            text: "$sCancelButtonLabel",
+            click: function() {
+                $(this).dialog( "close" ); $(this).remove();
+            },
+            'class': 'ibo-button ibo-is-alternative ibo-is-neutral action cancel'
+        },
+        { 
+            text: "$sOkButtonLabel",
+             click: ShortcutRenameOK,
+            'class': 'ibo-is-regular ibo-is-primary'
+        },
 	],
 	close: function() { $(this).remove(); }
 });

application/utils.inc.php

@@ -1315,8 +1315,8 @@ class utils
 		}
 
 		if (is_null($sAuthUser)) {
-			$sAuthUser = self::ReadParam('auth_user', '', 'raw_data');
-			$sAuthPwd = self::ReadParam('auth_pwd', '', 'raw_data');
+			$sAuthUser = self::ReadParam('auth_user', '', true, 'raw_data');
+			$sAuthPwd = self::ReadParam('auth_pwd', '', true, 'raw_data');
 		}
 		$sParamFile = self::GetParamSourceFile('auth_user');
 		if (is_null($sParamFile)) {
@@ -1886,6 +1886,9 @@ SQL;
 			$aHTTPHeaders[] = trim($sHeaderString);
 		}
 		// Default options, can be overloaded/extended with the 4th parameter of this method, see above $aCurlOptions
+		$iCurlAllowedProtocols = self::ConvertProtocolsToCurlOption(
+			self::GetConfig()->Get('security.post_requests.allowed_protocols')
+		);
 		$aOptions = [
 			CURLOPT_RETURNTRANSFER => true,     // return the content of the request
 			CURLOPT_HEADER         => false,    // don't return the headers in the output
@@ -1908,6 +1911,9 @@ SQL;
 		$aAllOptions = $aCurlOptions + $aOptions;
 		$ch = curl_init($sUrl);
 		curl_setopt_array($ch, $aAllOptions);
+		if (!curl_setopt($ch, CURLOPT_PROTOCOLS, $iCurlAllowedProtocols)) {
+			throw new Exception('Unable to enforce allowed protocols for cURL request');
+		}
 		$response = curl_exec($ch);
 		$iErr = curl_errno($ch);
 		$sErrMsg = curl_error($ch);
@@ -1929,6 +1935,40 @@ SQL;
 		return $response;
 	}
 
+	private static function ConvertProtocolsToCurlOption(string $sProtocols): int
+	{
+		$aProtocols = explode(',', $sProtocols);
+
+		$iCurlProtocols = 0;
+		foreach ($aProtocols as $sProtocol) {
+			$iCurlProtocols |= match (strtolower(trim($sProtocol))) {
+				'dict' => defined('CURLPROTO_DICT') ? CURLPROTO_DICT : 0,
+				'file' => defined('CURLPROTO_FILE') ? CURLPROTO_FILE : 0,
+				'ftp' => defined('CURLPROTO_FTP') ? CURLPROTO_FTP : 0,
+				'ftps' => defined('CURLPROTO_FTPS') ? CURLPROTO_FTPS : 0,
+				'gopher' => defined('CURLPROTO_GOPHER') ? CURLPROTO_GOPHER : 0,
+				'http' => defined('CURLPROTO_HTTP') ? CURLPROTO_HTTP : 0,
+				'https' => defined('CURLPROTO_HTTPS') ? CURLPROTO_HTTPS : 0,
+				'imap' => defined('CURLPROTO_IMAP') ? CURLPROTO_IMAP : 0,
+				'imaps' => defined('CURLPROTO_IMAPS') ? CURLPROTO_IMAPS : 0,
+				'ldap' => defined('CURLPROTO_LDAP') ? CURLPROTO_LDAP : 0,
+				'ldaps' => defined('CURLPROTO_LDAPS') ? CURLPROTO_LDAPS : 0,
+				'mqtt' => defined('CURLPROTO_MQTT') ? CURLPROTO_MQTT : 0,
+				'pop3' => defined('CURLPROTO_POP3') ? CURLPROTO_POP3 : 0,
+				'pop3s' => defined('CURLPROTO_POP3S') ? CURLPROTO_POP3S : 0,
+				'rtsp' => defined('CURLPROTO_RTSP') ? CURLPROTO_RTSP : 0,
+				'scp' => defined('CURLPROTO_SCP') ? CURLPROTO_SCP : 0,
+				'sftp' => defined('CURLPROTO_SFTP') ? CURLPROTO_SFTP : 0,
+				'smtp' => defined('CURLPROTO_SMTP') ? CURLPROTO_SMTP : 0,
+				'smtps' => defined('CURLPROTO_SMTPS') ? CURLPROTO_SMTPS : 0,
+				'telnet' => defined('CURLPROTO_TELNET') ? CURLPROTO_TELNET : 0,
+				'tftp' => defined('CURLPROTO_TFTP') ? CURLPROTO_TFTP : 0,
+				default => throw new Exception("Unsupported protocol: $sProtocol"),
+			};
+		}
+		return $iCurlProtocols;
+	}
+
 	/**
 	 * Get a standard list of character sets
 	 *

core/config.class.inc.php

@@ -1747,11 +1747,19 @@ class Config
 			'source_of_value' => '',
 			'show_in_conf_sample' => false,
 		],
+		'security.disable_mandatory_authentication_for_inline_images' => [
+			'type'                => 'bool',
+			'description' => 'If true then image access does not require user authentication, provided the image\'s secret is valid. Note that setting this to true gives a less secure access to inline images!',
+			'default'             => false,
+			'value'               => false,
+			'source_of_value'     => '',
+			'show_in_conf_sample' => false,
+		],
 		'security.disable_joined_classes_filter' => [
 			'type'                => 'bool',
 			'description'         => 'If true, scope filters aren\'t applied to joined classes or union classes not directly listed in the SELECT clause.',
-			'default'             => true,
-			'value'               => true,
+			'default'             => false,
+			'value'               => false,
 			'source_of_value'     => '',
 			'show_in_conf_sample' => false,
 		],
@@ -1766,8 +1774,16 @@ class Config
 		'security.disable_exec_forced_login_for_all_enpoints' => [
 			'type'                => 'bool',
 			'description'         => 'If true, when no delegated authentication module is defined, no login will be forced on modules exec endpoints',
-			'default'             => true,
-			'value'               => true,
+			'default'             => false,
+			'value'               => false,
+			'source_of_value'     => '',
+			'show_in_conf_sample' => false,
+		],
+		'security.post_requests.allowed_protocols' => [
+			'type'                => 'string',
+			'description'         => 'List of allowed protocols that will be used for post requests. Allowed values: dict, file, ftp, ftps, gopher, http, https, imap, imaps, ldap, ldaps, mqtt, pop3, pop3s, rtsp, scp, sftp, smtp, smtps, telnet, tftp',
+			'default'             => 'http,https,ftp,ftps,sftp',
+			'value'               => 'http,https,ftp,ftps,sftp',
 			'source_of_value'     => '',
 			'show_in_conf_sample' => false,
 		],

core/dbobjectsearch.class.php

@@ -1704,7 +1704,7 @@ class DBObjectSearch extends DBSearch
 	 * @return array|mixed|\SQLObjectQuery|null
 	 * @throws \CoreException
 	 */
-	public function GetSQLQueryStructure($aAttToLoad, $bGetCount, $aGroupByExpr = null, $aSelectedClasses = null, $aSelectExpr = null)
+	public function GetSQLQueryStructure($aAttToLoad, $bGetCount, $aGroupByExpr = null, $aSelectedClasses = null, $aSelectExpr = null, $bSelectOnlyIds = false)
 	{
 		// Hide objects that are not visible to the current user
 		//
@@ -1775,6 +1775,8 @@ class DBObjectSearch extends DBSearch
 			$aContextData['aSelectExpr'] = $aSelectExpr;
 			$sRawId .= $bGetCount;
 			$aContextData['bGetCount'] = $bGetCount;
+			$sRawId .= 'ids:'.($bSelectOnlyIds ? '1' : '0');
+			$aContextData['bSelectOnlyIds'] = $bSelectOnlyIds;
 			if (is_array($aSelectedClasses)) {
 				$sRawId .= implode(',', $aSelectedClasses); // Unions may alter the list of selected columns
 			}
@@ -1828,7 +1830,7 @@ class DBObjectSearch extends DBSearch
 		if (!isset($oSQLQuery)) {
 			$oKPI = new ExecutionKPI();
 			$oSQLObjectQueryBuilder = new SQLObjectQueryBuilder($oSearch);
-			$oSQLQuery = $oSQLObjectQueryBuilder->BuildSQLQueryStruct($aAttToLoad, $bGetCount, $aModifierProperties, $aGroupByExpr, $aSelectedClasses, $aSelectExpr);
+			$oSQLQuery = $oSQLObjectQueryBuilder->BuildSQLQueryStruct($aAttToLoad, $bGetCount, $aModifierProperties, $aGroupByExpr, $aSelectedClasses, $aSelectExpr, $bSelectOnlyIds);
 			$oKPI->ComputeStats('BuildSQLQueryStruct', $sOqlQuery);
 
 			if (self::$m_bQueryCacheEnabled) {
@@ -1938,7 +1940,7 @@ class DBObjectSearch extends DBSearch
 		$oSearch = $this;
 		$aClassesToFilter = $this->GetSelectedClasses();
 
-		// Opt-in for joined classes filtering, otherwise only filter the selected class(es)
+		// Joined classes filtering can be disabled through the configuration.
 		if (MetaModel::GetConfig()->Get('security.disable_joined_classes_filter') === false) {
 			$aClassesToFilter = $this->GetJoinedClasses();
 		}

core/dbsearch.class.php

@@ -995,7 +995,7 @@ abstract class DBSearch
 	 * @internal
 	 *
 	 */
-	public function MakeSelectQuery($aOrderBy = [], $aArgs = [], $aAttToLoad = null, $aExtendedDataSpec = null, $iLimitCount = 0, $iLimitStart = 0, $bGetCount = false, $bBeautifulSQL = true)
+	public function MakeSelectQuery($aOrderBy = [], $aArgs = [], $aAttToLoad = null, $aExtendedDataSpec = null, $iLimitCount = 0, $iLimitStart = 0, $bGetCount = false, $bBeautifulSQL = true, $bSelectOnlyIds = false)
 	{
 		// Check the order by specification, and prefix with the class alias
 		// and make sure that the ordering columns are going to be selected
@@ -1036,7 +1036,7 @@ abstract class DBSearch
 			}
 		}
 
-		$oSQLQuery = $this->GetSQLQuery($aOrderBy, $aArgs, $aAttToLoad, $aExtendedDataSpec, $iLimitCount, $iLimitStart, $bGetCount);
+		$oSQLQuery = $this->GetSQLQuery($aOrderBy, $aArgs, $aAttToLoad, $aExtendedDataSpec, $iLimitCount, $iLimitStart, $bGetCount, null, null, $bSelectOnlyIds);
 
 		if ($this->m_bNoContextParameters) {
 			// Only internal parameters
@@ -1120,7 +1120,7 @@ abstract class DBSearch
 	 * @internal
 	 *
 	 */
-	protected function GetSQLQuery($aOrderBy, $aArgs, $aAttToLoad, $aExtendedDataSpec, $iLimitCount, $iLimitStart, $bGetCount, $aGroupByExpr = null, $aSelectExpr = null)
+	protected function GetSQLQuery($aOrderBy, $aArgs, $aAttToLoad, $aExtendedDataSpec, $iLimitCount, $iLimitStart, $bGetCount, $aGroupByExpr = null, $aSelectExpr = null, $bSelectOnlyIds = false)
 	{
 		$oSearch = $this->ApplyDataFilters();
 
@@ -1142,7 +1142,7 @@ abstract class DBSearch
 			}
 		}
 
-		$oSQLQuery = $oSearch->GetSQLQueryStructure($aAttToLoad, $bGetCount, $aGroupByExpr, null, $aSelectExpr);
+		$oSQLQuery = $oSearch->GetSQLQueryStructure($aAttToLoad, $bGetCount, $aGroupByExpr, null, $aSelectExpr, $bSelectOnlyIds);
 		$oSQLQuery->SetSourceOQL($oSearch->ToOQL());
 
 		// Join to an additional table, if required...
@@ -1177,7 +1177,8 @@ abstract class DBSearch
 		$bGetCount,
 		$aGroupByExpr = null,
 		$aSelectedClasses = null,
-		$aSelectExpr = null
+		$aSelectExpr = null,
+		$bSelectOnlyIds = false
 	);
 
 	/**

core/dbunionsearch.class.php

@@ -527,10 +527,10 @@ class DBUnionSearch extends DBSearch
 		throw new Exception('MakeUpdateQuery is not implemented for the unions!');
 	}
 
-	public function GetSQLQueryStructure($aAttToLoad, $bGetCount, $aGroupByExpr = null, $aSelectedClasses = null, $aSelectExpr = null)
+	public function GetSQLQueryStructure($aAttToLoad, $bGetCount, $aGroupByExpr = null, $aSelectedClasses = null, $aSelectExpr = null, $bSelectOnlyIds = false)
 	{
 		if (count($this->aSearches) == 1) {
-			return $this->aSearches[0]->GetSQLQueryStructure($aAttToLoad, $bGetCount, $aGroupByExpr, $aSelectedClasses, $aSelectExpr);
+			return $this->aSearches[0]->GetSQLQueryStructure($aAttToLoad, $bGetCount, $aGroupByExpr, $aSelectedClasses, $aSelectExpr, $bSelectOnlyIds);
 		}
 
 		$aSQLQueries = [];
@@ -610,7 +610,7 @@ class DBUnionSearch extends DBSearch
 					}
 				}
 			}
-			$oSubQuery = $oSearch->GetSQLQueryStructure($aQueryAttToLoad, false, $aQueryGroupByExpr, $aSearchSelectedClasses, $aQuerySelectExpr);
+			$oSubQuery = $oSearch->GetSQLQueryStructure($aQueryAttToLoad, false, $aQueryGroupByExpr, $aSearchSelectedClasses, $aQuerySelectExpr, $bSelectOnlyIds);
 			if (count($aSearchAliases) > 1) {
 				// Necessary to make sure that selected columns will match throughout all the queries
 				// (default order of selected fields depending on the order of JOINS)
@@ -683,7 +683,7 @@ class DBUnionSearch extends DBSearch
 			return $this;
 		}
 
-		// Opt-in for joined classes filtering, otherwise fallback on DBSearch filtering
+		// Joined classes filtering can be disabled through the configuration.
 		if (MetaModel::GetConfig()->Get('security.disable_joined_classes_filter') === true) {
 			return parent::ApplyDataFilters();
 		}

core/sqlobjectquerybuilder.class.inc.php

@@ -29,13 +29,14 @@ class SQLObjectQueryBuilder
 	 * @param array $aGroupByExpr
 	 * @param array $aSelectedClasses
 	 * @param array $aSelectExpr
+	 * @param bool $bSelectOnlyIds
 	 *
 	 * @return null|SQLObjectQuery
 	 * @throws \CoreException
 	 */
-	public function BuildSQLQueryStruct($aAttToLoad, $bGetCount, $aModifierProperties, $aGroupByExpr = null, $aSelectedClasses = null, $aSelectExpr = null)
+	public function BuildSQLQueryStruct($aAttToLoad, $bGetCount, $aModifierProperties, $aGroupByExpr = null, $aSelectedClasses = null, $aSelectExpr = null, $bSelectOnlyIds = false)
 	{
-		if ($bGetCount || !is_null($aGroupByExpr)) {
+		if ($bGetCount || !is_null($aGroupByExpr) || $bSelectOnlyIds) {
 			// Avoid adding all the fields for counts or "group by" requests
 			$aAttToLoad = [];
 			foreach ($this->oDBObjetSearch->GetSelectedClasses() as $sClassAlias => $sClass) {

datamodels/2.x/itop-attachments/ajax.itop-attachment.php

@@ -56,7 +56,7 @@ function RenderAttachments(AjaxPage $oPage, $iTransactionId)
 try {
 	require_once APPROOT.'/application/startup.inc.php';
 	require_once APPROOT.'/application/loginwebpage.class.inc.php';
-	LoginWebPage::DoLoginEx(null /* any portal */, false);
+	LoginWebPage::DoLogin(); // No user portal should access this endpoint.
 
 	$oPage = new AjaxPage("");
 
@@ -105,15 +105,6 @@ try {
 			$oPage->SetData($aResult);
 			break;
 
-		case 'remove':
-			$iAttachmentId = utils::ReadParam('att_id', '');
-			$oSearch = DBObjectSearch::FromOQL("SELECT Attachment WHERE id = :id");
-			$oSet = new DBObjectSet($oSearch, [], ['id' => $iAttachmentId]);
-			while ($oAttachment = $oSet->Fetch()) {
-				$oAttachment->DBDelete();
-			}
-			break;
-
 		case 'refresh_attachments_render':
 			$sTempId = utils::ReadParam('temp_id', '', false, 'transaction_id');
 			RenderAttachments($oPage, $sTempId);

datamodels/2.x/itop-structure/datamodel.itop-structure.xml

@@ -742,11 +742,9 @@
             {
                 $oAddon = UserRights::GetModuleInstance();
                 $aOrgs = $oAddon->GetUserOrgs($oUser,'Organization');
-                $oSet = $oUser->Get('profile_list');
-                $aProfiles = $oSet->GetColumnAsArray('profile');
 
                 // User is not allowed on the new Organization and has 'Portal user' Profile and is enabled
-                if ((count($aOrgs) > 0) && !in_array($this->Get('org_id'), $aOrgs) &&  in_array('Portal user',$aProfiles) && ($oUser->Get('status') === 'enabled'))
+                if ((count($aOrgs) > 0) && !in_array($this->Get('org_id'), $aOrgs) &&  UserRights::HasProfile('Portal user', $oUser) && ($oUser->Get('status') === 'enabled'))
                 {   // Let's block the Person modification,
                     // replace by $this->AddCheckWarning(...) if you don't want to block the modification
                     $this->AddCheckIssue(Dict::Format('Class:Person/Error:ChangingOrgDenied', $this->Get('org_id_friendlyname')));

js/icon_select.js

@@ -104,8 +104,8 @@ $(function()
 			$(document).on('click', function(){
 				me._cleanAllMenus();
 			});
-			var oMenu =
-				$('<ul class="menu-icon-select ibo-input-select-icon--menu">');
+			var oMenu = $('<ul class="menu-icon-select ibo-input-select-icon--menu">');
+			this.oButton.data('icon-select-menu', oMenu[0]);
 			for(var i in this.options.items)
 			{
 				var oItem = $('<li class="ibo-input-select-icon--menu--item"><div value="'+i+'"><img class="ibo-input-select-icon--menu--icon" src="'+this.options.items[i].icon+'" style="vertical-align: middle;"/>'+this.options.items[i].label+'<div></li>');

pages/ajax.document.php

@@ -64,7 +64,9 @@ try {
 			break;
 
 		case 'download_inlineimage':
-			LoginWebPage::DoLoginEx();
+			if (MetaModel::GetConfig()->Get('security.disable_mandatory_authentication_for_inline_images') === false) {
+				LoginWebPage::DoLoginEx();
+			}
 			$id = utils::ReadParam('id', '');
 			$sSecret = utils::ReadParam('s', '');
 			$iCacheSec = 31556926; // One year ahead: an inline image cannot change

pages/ajax.render.php

@@ -768,7 +768,7 @@ try {
 
 					// N°4129 - Prevent XSS attacks & other script executions
 					if (utils::GetConfig()->Get('security.disable_inline_documents_sandbox') === false) {
-						$oPage->add_header('Content-Security-Policy: sandbox;');
+						$oPage->add_header('Content-Security-Policy: sandbox; default-src \'none\'');
 					}
 
 					ormDocument::DownloadDocument($oPage, $sClass, $id, $sField, 'inline');
@@ -1239,13 +1239,32 @@ JS
 				$oSearch = new DBObjectSearch('Shortcut');
 				$aShortcuts = utils::ReadMultipleSelection($oSearch);
 				$iShortcut = $aShortcuts[0];
-				$oShortcut = MetaModel::GetObject('Shortcut', $iShortcut);
+				$oShortcutSearch = new DBObjectSearch('Shortcut');
+				$oShortcutSearch->AddCondition('user_id', UserRights::GetUserId(), '=');
+				$oShortcutSearch->AddCondition('id', $iShortcut, '=');
+				$oShortcutSet = new CMDBObjectSet($oShortcutSearch);
+
+				$oShortcut = $oShortcutSet->Fetch();
+
+				if ($oShortcut === null) {
+					throw new SecurityException(Dict::S('UI:ObjectDoesNotExist'));
+				}
+
 				$oShortcut->StartRenameDialog($oPage);
 				break;
 
 			case 'shortcut_rename_go':
 				$iShortcut = utils::ReadParam('id', 0);
-				$oShortcut = MetaModel::GetObject('Shortcut', $iShortcut);
+
+				$oShortcutSearch = new DBObjectSearch('Shortcut');
+				$oShortcutSearch->AddCondition('user_id', UserRights::GetUserId(), '=');
+				$oShortcutSearch->AddCondition('id', $iShortcut, '=');
+				$oShortcutSet = new CMDBObjectSet($oShortcutSearch);
+				$oShortcut = $oShortcutSet->Fetch();
+
+				if ($oShortcut === null) {
+					throw new SecurityException(Dict::S('UI:ObjectDoesNotExist'));
+				}
 
 				$sName = utils::ReadParam('attr_name', '', false, 'raw_data');
 				if (strlen($sName) > 0) {

pages/preferences.php

@@ -243,7 +243,7 @@ JS
 		);
 		$oShortcutsToolBar->AddSubBlock($oShortcutsRenameButton);
 		// - Delete button
-		$oShortcutsDeleteButton = ButtonUIBlockFactory::MakeForSecondaryAction(
+		$oShortcutsDeleteButton = ButtonUIBlockFactory::MakeForDestructiveAction(
 			Dict::S('UI:Button:Delete'),
 			null,
 			null,

sources/Application/WebPage/AjaxPage.php

@@ -346,12 +346,4 @@ class AjaxPage extends WebPage implements iTabbedPage
 	{
 		assert(false);
 	}
-
-	/**
-	 * @inheritDoc
-	 */
-	public static function FilterXSS($sHTML)
-	{
-		return str_ireplace(['<script', '</script>'], ['<!-- <removed-script', '</removed-script> -->'], $sHTML);
-	}
 }

sources/Application/WebPage/DownloadPage.php

@@ -47,15 +47,9 @@ class DownloadPage extends AjaxPage
 			header($s_header);
 		}
 
-		if (($this->sContentType == 'text/html') && ($this->sContentDisposition == 'inline')) {
-			// inline content != attachment && html => filter all scripts for malicious XSS scripts
-			$sContent = self::FilterXSS($this->sContent);
-		} else {
-			$sContent = $this->sContent;
-		}
 		$oKpi->ComputeAndReport(get_class($this).' output');
-		echo $sContent;
-		$oKpi->ComputeAndReport('Echoing ('.round(strlen($sContent) / 1024).' Kb)');
+		echo $this->sContent;
+		$oKpi->ComputeAndReport('Echoing ('.round(strlen($this->sContent) / 1024).' Kb)');
 		ExecutionKPI::ReportStats();
 	}
 }

templates/base/layouts/object/object-details/layout.html.twig

@@ -19,9 +19,9 @@
     {% endif %}
     <span class="ibo-object-details--object-class" data-role="ibo-object-details--object-class">
         {% if is_backoffice_menu_enabled('DataModelMenu') %}
-            <a href="{{ get_absolute_url_app_root() }}pages/schema.php?operation=details_class&class={{ oUIBlock.GetClassName() }}" target="_blank" data-tooltip-content="{{ 'UI:Layout:ObjectDetails:DatamodelSchemaLink:Tooltip' | dict_s }}" data-tooltip-html-enabled="true">{{ oUIBlock.GetClassLabel() }}</a>
+            <a class="ibo-object-details--object-class--label" href="{{ get_absolute_url_app_root() }}pages/schema.php?operation=details_class&class={{ oUIBlock.GetClassName() }}" target="_blank" data-tooltip-content="{{ 'UI:Layout:ObjectDetails:DatamodelSchemaLink:Tooltip' | dict_s }}" data-tooltip-html-enabled="true">{{ oUIBlock.GetClassLabel() }}</a>
         {% else %}
-            {{ oUIBlock.GetClassLabel() }}
+            <span class="ibo-object-details--object-class--label">{{ oUIBlock.GetClassLabel() }}</span>
         {% endif %}
     </span>
     {{ parent() }}

tests/php-unit-tests/integration-tests/login-tests/LoginWebPageTest.php

@@ -111,7 +111,7 @@ class LoginWebPageTest extends ItopDataTestCase
 		$this->assertStringContainsString('<title>iTop login</title>', $sPageContent, 'if itop is configured to force login when no there is no delegated authentication endpoints list, then login should be required.');
 	}
 
-	public function testWithoutDelegatedAuthenticationEndpointsListWithDefaultConfiguration()
+	public function testWithoutDelegatedAuthenticationEndpointsListRequiresLoginByDefault()
 	{
 		$sPageContent = $this->CallItopUri(
 			"pages/exec.php?exec_module=extension-without-delegated-authentication-endpoints-list&exec_page=src/Controller/File.php",
@@ -120,7 +120,24 @@ class LoginWebPageTest extends ItopDataTestCase
 			true
 		);
 
-		$this->assertStringContainsString('Yo', $sPageContent, 'by default (until N°9343) if no delegated authentication endpoints list is defined, not logged in persons should access pages');
+		$this->assertStringContainsString('<title>iTop login</title>', $sPageContent, 'by default, login should be required when no delegated authentication endpoints list is defined');
+	}
+
+	public function testWithoutDelegatedAuthenticationEndpointsListWithCompatibilityOptOut()
+	{
+		@chmod($this->oConfig->GetLoadedFile(), 0770);
+		$this->oConfig->Set('security.disable_exec_forced_login_for_all_enpoints', true, 'AnythingButEmptyOrUnknownValue');
+		$this->oConfig->WriteToFile();
+		@chmod($this->oConfig->GetLoadedFile(), 0444);
+
+		$sPageContent = $this->CallItopUri(
+			"pages/exec.php?exec_module=extension-without-delegated-authentication-endpoints-list&exec_page=src/Controller/File.php",
+			[],
+			[],
+			true
+		);
+
+		$this->assertStringContainsString('Yo !', $sPageContent, 'the compatibility opt-out should allow anonymous access when no delegated authentication endpoints list is defined');
 	}
 
 	public function testNotInDelegatedAuthenticationEndpointsList()

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

@@ -78,6 +78,11 @@ XML,
 </root>
 HTML
 			],
+			'DesignerComboField should not escape html characters to preserve indentation made with  ' => [
+				'\\DesignerComboField',
+				'  Foo & Bar',
+				'  Foo & Bar',
+			],
 		];
 	}
 }

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

@@ -72,6 +72,30 @@ class utilsTest extends ItopTestCase
 		];
 	}
 
+	/**
+	 * @dataProvider DoPostRequestAllowedProtocolsProvider
+	 */
+	public function testDoPostRequestRespectsAllowedProtocolsConfig(string $sAllowedProtocols, bool $bFileProtocolAllowed): void
+	{
+		// Config::Set changes the in-memory setting only; the test framework reloads config after this test class.
+		utils::GetConfig()->Set('security.post_requests.allowed_protocols', $sAllowedProtocols);
+		if (!$bFileProtocolAllowed) {
+			$this->expectException(\Exception::class);
+			$this->expectExceptionMessage('Problem opening URL');
+		} else {
+			$this->expectNotToPerformAssertions();
+		}
+		utils::DoPostRequest('file:///tmp', ['payload' => 'test']);
+	}
+
+	public function DoPostRequestAllowedProtocolsProvider(): array
+	{
+		return [
+			'file protocol is allowed' => ['file,http,https', true],
+			'file protocol is not allowed' => ['http,https', false],
+		];
+	}
+
 	/**
 	 * @dataProvider realPathDataProvider
 	 * @covers       utils::RealPath()

tests/php-unit-tests/unitary-tests/datamodels/2.x/authent-local/UserLocalTest.php

@@ -419,6 +419,7 @@ class UserLocalTest extends ItopDataTestCase
 		$this->assertInstanceOf(ormLinkSet::class, $oProfilesSet);
 		$this->assertEquals(0, $oProfilesSet->Count());
 		MetaModel::GetConfig()->Set('security.hide_administrators', false);
+		MetaModel::GetConfig()->Set('security.disable_joined_classes_filter', true);
 		$oProfilesSet = $this->GetAdminUserProfileList();
 		$this->assertIsObject($oProfilesSet);
 		$this->assertInstanceOf(ormLinkSet::class, $oProfilesSet);

tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/AttachmentAjaxEndpointTest.php

@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Combodo\iTop\Test\UnitTest\Module\ItopAttachment;
+
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+
+class AttachmentAjaxEndpointTest extends ItopDataTestCase
+{
+	public const USE_TRANSACTION = false;
+	private const AUTHENTICATION_PASSWORD = 'tagada-Secret,007';
+
+	protected function setUp(): void
+	{
+		parent::setUp();
+
+		$this->BackupConfiguration();
+		$this->AddLoginModeAndSaveConfiguration('url');
+	}
+
+	/**
+	 * @dataProvider AjaxEndpointAccessProvider
+	 */
+	public function testAjaxEndpointAccess(string $sProfile, int $iExpectedHttpCode): void
+	{
+		$sLogin = 'user-'.uniqid();
+		$this->CreateUser($sLogin, self::$aURP_Profiles[$sProfile], self::AUTHENTICATION_PASSWORD);
+
+		$iHttpCode = $this->CallAttachmentEndpointAs($sLogin);
+
+		$this->assertSame($iExpectedHttpCode, $iHttpCode);
+	}
+
+	public function AjaxEndpointAccessProvider(): array
+	{
+		return [
+			'console user' => ['Service Desk Agent', 200],
+			'portal user' => ['Portal user', 302], // redirect to portal
+		];
+	}
+
+	private function CallAttachmentEndpointAs(string $sLogin): int
+	{
+		$this->CallItopUri(
+			'env-production/itop-attachments/ajax.itop-attachment.php?operation=add&'.http_build_query([
+				'auth_user' => $sLogin,
+				'auth_pwd' => self::AUTHENTICATION_PASSWORD,
+			]),
+			[],
+			[
+				CURLOPT_HTTPHEADER => ['X-Combodo-Ajax:1'],
+				CURLOPT_POST => 0,
+			],
+			true
+		);
+
+		return $this->aLastCurlGetInfo['http_code'];
+	}
+}

tests/php-unit-tests/unitary-tests/pages/AjaxRenderTest.php

@@ -4,6 +4,7 @@ namespace Combodo\iTop\Test\UnitTest\Pages;
 
 use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
 use Dict;
+use ormDocument;
 use UserLocal;
 use UserRequest;
 
@@ -132,6 +133,47 @@ class AjaxRenderTest extends ItopDataTestCase
 		return $this->AcquireLockAsUser(self::$sLogin, self::$iTicketId);
 	}
 
+	public function testDisplayDocumentHasSandboxWhenConfigurationDoesNotDisableIt(): void
+	{
+		$sLogin = uniqid('AjaxRenderTest');
+		$this->CreateContactlessUser($sLogin, self::$aURP_Profiles['Administrator'], self::AUTHENTICATION_PASSWORD);
+		$iDocumentId = $this->GivenObjectInDB('DocumentFile', [
+			'name' => 'AjaxRenderTest_'.uniqid(),
+			'org_id' => $this->getTestOrgId(),
+			'file' => new ormDocument('<svg xmlns="http://www.w3.org/2000/svg"></svg>', 'image/svg+xml', 'test.svg'),
+		]);
+
+		$this->oiTopConfig->Set('security.disable_inline_documents_sandbox', false);
+		$this->SaveItopConfFile();
+		$sResponseHeaders = $this->GetDocumentResponseHeaders($sLogin, $iDocumentId);
+		$this->assertStringContainsString(
+			'sandbox',
+			$sResponseHeaders
+		);
+	}
+
+	private function GetDocumentResponseHeaders(string $sLogin, int $iDocumentId): string
+	{
+		$sResponse = $this->CallItopUri(
+			'pages/ajax.render.php?'.http_build_query([
+				'operation' => 'display_document',
+				'class' => 'DocumentFile',
+				'id' => $iDocumentId,
+				'field' => 'file',
+				'auth_user' => $sLogin,
+				'auth_pwd' => self::AUTHENTICATION_PASSWORD,
+			]),
+			[],
+			[
+				CURLOPT_HTTPHEADER => ['X-Combodo-Ajax:1'],
+				CURLOPT_HEADER => true,
+				CURLOPT_POST => 0,
+			]
+		);
+
+		return substr($sResponse, 0, $this->aLastCurlGetInfo['header_size']);
+	}
+
 	// Helper method to create a user with a specific profile
 	private function CreateUserWithProfile(int $iProfileId): UserLocal
 	{

tests/php-unit-tests/unitary-tests/synchro/DataSynchroTest.php

@@ -116,7 +116,7 @@ class DataSynchroTest extends ItopDataTestCase
 	 * @throws \MySQLException
 	 * @throws \OQLException
 	 */
-	public function RunDataSynchroTest($aUserLoginUsecase)
+	public function RunDataSynchroTest($aUserLoginUsecase, array $aAdditionalImportParams = [])
 	{
 		$sDescription = $aUserLoginUsecase['desc'];
 		$sTargetClass = $aUserLoginUsecase['target_class'];
@@ -273,13 +273,13 @@ class DataSynchroTest extends ItopDataTestCase
 				$sCSVTmpFile = tempnam(sys_get_temp_dir(), "CSV");
 				file_put_contents($sCSVTmpFile, $sCsvData);
 
-				$aParams = [
+				$aParams = array_merge($aAdditionalImportParams, [
 					'csvfile' => $sCSVTmpFile,
 					'data_source_id' => $iDataSourceId,
 					'separator' => ';',
 					'simulate' => 0,
 					'output' => 'details',
-				];
+				]);
 				list($iRetCode, $aOutputLines) = static::ExecSynchroImport($aParams, $bSynchroByHttp);
 
 				unlink($sCSVTmpFile);
@@ -390,6 +390,11 @@ class DataSynchroTest extends ItopDataTestCase
 		$this->RunDataSynchroTest($this->GetNominalUsecaseData());
 	}
 
+	public function testDataSynchroByCliWithMaxChunkSize()
+	{
+		$this->RunDataSynchroTest($this->GetNominalUsecaseData(), ['max_chunk_size' => 1 ]);
+	}
+
 	public function testDataSynchroByHttp()
 	{
 		$aUserLoginUsecase = $this->GetNominalUsecaseData();