Skip to content

AGENTS.md

Latest commit

 

History

History
149 lines (118 loc) · 12.9 KB

AGENTS.md

File metadata and controls

149 lines (118 loc) · 12.9 KB
Raw

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. When asked to review code, also follow REVIEW.md.

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.

Vulnerability fixes

  • The repository is public: the commit message, code comments, test names and PR description of a vulnerability fix describe the functional change without revealing that it fixes a vulnerability. No security wording (vulnerability, XSS, injection, CSRF, security, CVE...), no exploit, no affected versions. E.g. N°1234 - Fix display of special characters in the activity panel, not N°1234 - Fix XSS in the activity panel nor N°1234 - Security hardening.

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. Only Composer updates lib/composer/autoload_*.php (see Commands).
    • 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 --no-interaction --classmap-authoritative at the repository root (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 versioned next to some SCSS files (css/setup.css, which includes css/backoffice/, css/email.css, css/unauthenticated.css, portal CSS...). For css/setup.css, run php .make/build/commands/setupCssCompiler.php; for the others, tell the user. The back-office theme is compiled by the setup into env-*/: nothing to commit.
  • 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.
  • New classes go in sources/ (namespace Combodo\iTop\... matching the folder), not in core/ or application/. Module-specific code stays in its module.
  • 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 (see Backward compatibility).
  • PHPDoc: @since is mandatory on new classes and on new or changed methods and properties, e.g. @since 3.3.0 N°1234. If the target version is unknown, ask the user. 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.

Backward compatibility

Extensions use public and protected classes, methods and constants, events, configuration parameters, dictionary codes and REST/webservice APIs.

  • Never remove or rename them, nor change their signature or behavior. Do not add type hints to existing signatures, nor return types to fluent methods.
  • To replace one, keep it, tag it @deprecated <version> N°1234 - Use Xxx() instead and call DeprecatedCallsLog::NotifyDeprecatedPhpMethod() in it.

Performance

  • No query in a loop: prefer a single OQL query (IN, joins).
  • Use DBObjectSet::Count(), CountExceeds() or SetLimit() when a count or a subset is enough.
  • When iterating over a set, load only the needed attributes with DBObjectSet::OptimizeColumnLoad().

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 every other language file next to the en one (same folder): translate them, or use the English text with a ~~ suffix if unsure. When creating a new dictionary file, create it for every language listed in tests/php-unit-tests/integration-tests/DictionariesConsistencyTest.php (GetLanguagesData()).
  • When adding a new language, also declare it in GetLanguagesData(), otherwise DictionariesConsistencyTest fails.
  • 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.