|
|
|
|
@@ -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`.
|
