mirror of
https://github.com/Combodo/iTop.git
synced 2026-10-22 00:09:03 +02:00
Compare commits
7 Commits
issue/9239
...
feature/ai
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
16453be2df | ||
|
|
7514288cd8 | ||
|
|
6865c845f0 | ||
|
|
530c7b2ddd | ||
|
|
692c0723ab | ||
|
|
23619ec6cd | ||
|
|
0ba1bf829c |
133
AGENTS.md
Normal file
133
AGENTS.md
Normal file
@@ -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`.
|
||||
30
REVIEW.md
Normal file
30
REVIEW.md
Normal file
@@ -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").
|
||||
@@ -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);
|
||||
|
||||
@@ -1747,6 +1747,14 @@ 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.',
|
||||
|
||||
@@ -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>');
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -78,6 +78,11 @@ XML,
|
||||
</root>
|
||||
HTML
|
||||
],
|
||||
'DesignerComboField should not escape html characters to preserve indentation made with ' => [
|
||||
'\\DesignerComboField',
|
||||
' Foo & Bar',
|
||||
' Foo & Bar',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,100 +0,0 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Combodo\iTop\Test\UnitTest\Module\ItopCoreUpdate;
|
||||
|
||||
use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
|
||||
|
||||
class CoreUpdateAvailabilityTest extends ItopDataTestCase
|
||||
{
|
||||
public const USE_TRANSACTION = false;
|
||||
|
||||
private const AUTHENTICATION_PASSWORD = 'test-password-123';
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
parent::setUp();
|
||||
|
||||
$this->BackupConfiguration();
|
||||
$this->AddLoginModeAndSaveConfiguration('url');
|
||||
}
|
||||
|
||||
public function testFunctionalAdministratorCanAccessUpgradeWhenEnabled(): void
|
||||
{
|
||||
$this->SetUpgradeEnabled(true);
|
||||
|
||||
$sMenuPage = $this->CallAsFunctionalAdministrator();
|
||||
$this->assertSame(200, $this->GetLastHttpStatus());
|
||||
$this->assertStringContainsString('data-menu-id="iTopUpdate"', $sMenuPage);
|
||||
|
||||
$this->CallAsFunctionalAdministrator('core_update.select_update_file', ['maintenance' => 'true']);
|
||||
$this->assertSame(200, $this->GetLastHttpStatus());
|
||||
}
|
||||
|
||||
public function testUpgradeMenuIsHiddenWhenDisabled(): void
|
||||
{
|
||||
$this->SetUpgradeEnabled(false);
|
||||
|
||||
$sMenuPage = $this->CallAsFunctionalAdministrator();
|
||||
|
||||
$this->assertSame(200, $this->GetLastHttpStatus());
|
||||
$this->assertStringNotContainsString('data-menu-id="iTopUpdate"', $sMenuPage);
|
||||
}
|
||||
|
||||
public function testDirectUpgradePageAccessIsDeniedWhenDisabled(): void
|
||||
{
|
||||
$this->SetUpgradeEnabled(false);
|
||||
|
||||
$this->CallAsFunctionalAdministrator('core_update.select_update_file', ['maintenance' => 'true']);
|
||||
|
||||
$this->assertSame(403, $this->GetLastHttpStatus());
|
||||
}
|
||||
|
||||
public function testUpgradeAjaxEndpointIsDeniedWhenDisabled(): void
|
||||
{
|
||||
$this->SetUpgradeEnabled(false);
|
||||
|
||||
$this->CallAsFunctionalAdministrator('core_update_ajax.can_update_core');
|
||||
|
||||
$this->assertSame(403, $this->GetLastHttpStatus());
|
||||
}
|
||||
|
||||
private function SetUpgradeEnabled(bool $bEnabled): void
|
||||
{
|
||||
// Expected config parameter; it is not currently declared by itop-core-update.
|
||||
$this->oiTopConfig->Set('itop-core-update.enabled', $bEnabled);
|
||||
$this->SaveItopConfFile();
|
||||
}
|
||||
|
||||
private function CallAsFunctionalAdministrator(?string $sRoute = null, array $aParams = []): string
|
||||
{
|
||||
$sLogin = 'core-update-admin-'.bin2hex(random_bytes(6));
|
||||
$this->CreateContactlessUser(
|
||||
$sLogin,
|
||||
self::$aURP_Profiles['Administrator'],
|
||||
self::AUTHENTICATION_PASSWORD
|
||||
);
|
||||
|
||||
$aQuery = $aParams;
|
||||
if (null !== $sRoute) {
|
||||
$aQuery['route'] = $sRoute;
|
||||
}
|
||||
$aQuery['auth_user'] = $sLogin;
|
||||
$aQuery['auth_pwd'] = self::AUTHENTICATION_PASSWORD;
|
||||
$sUri = 'pages/UI.php';
|
||||
if ([] !== $aQuery) {
|
||||
$sUri .= '?'.http_build_query($aQuery);
|
||||
}
|
||||
|
||||
return $this->CallItopUri($sUri, [], [CURLOPT_POST => 0]);
|
||||
}
|
||||
|
||||
private function GetLastHttpStatus(): int
|
||||
{
|
||||
$this->assertNotNull($this->aLastCurlGetInfo);
|
||||
$this->assertArrayHasKey('http_code', $this->aLastCurlGetInfo);
|
||||
|
||||
return $this->aLastCurlGetInfo['http_code'];
|
||||
}
|
||||
}
|
||||
@@ -1,114 +0,0 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Combodo\iTop\Test\UnitTest\Module\ItopCoreUpdate;
|
||||
|
||||
use Combodo\iTop\CoreUpdate\Service\CoreUpdater;
|
||||
use Combodo\iTop\Test\UnitTest\ItopTestCase;
|
||||
|
||||
class SignatureValidationTest extends ItopTestCase
|
||||
{
|
||||
private const RESOURCES_DIRECTORY = __DIR__.'/resources/';
|
||||
|
||||
private string $sWorkDirectory = '';
|
||||
private string $sPublicKey;
|
||||
private string $sDestinationDirectory;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
parent::setUp();
|
||||
$this->RequireOnceItopFile('datamodels/2.x/itop-core-update/src/Service/CoreUpdater.php');
|
||||
|
||||
$this->sWorkDirectory = __DIR__.'/.signature-test-'.bin2hex(random_bytes(8));
|
||||
mkdir($this->sWorkDirectory, 0700, true);
|
||||
$this->sDestinationDirectory = $this->sWorkDirectory.'/installation';
|
||||
mkdir($this->sDestinationDirectory, 0700);
|
||||
|
||||
$this->sPublicKey = file_get_contents(self::RESOURCES_DIRECTORY.'dummy-public.pem');
|
||||
file_put_contents($this->sDestinationDirectory.'/installation-sentinel.txt', 'unchanged');
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
static::RecurseRmdir($this->sWorkDirectory);
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
public function testValidSignedPackageIsAccepted(): void
|
||||
{
|
||||
// Expected API: verify with an injected test public key before extracting to the supplied destination.
|
||||
CoreUpdater::ValidateAndExtractDownloadedFile(
|
||||
self::RESOURCES_DIRECTORY.'dummy-signed.zip',
|
||||
$this->sPublicKey,
|
||||
$this->sDestinationDirectory
|
||||
);
|
||||
|
||||
$this->assertSame(
|
||||
file_get_contents(self::RESOURCES_DIRECTORY.'package/file1'),
|
||||
file_get_contents($this->sDestinationDirectory.'/file1')
|
||||
);
|
||||
}
|
||||
|
||||
public function testPackageIsRejectedBeforeExtractionWhenManifestWasAlteredAfterSigning(): void
|
||||
{
|
||||
$this->AssertRejectedBeforeExtraction(self::RESOURCES_DIRECTORY.'dummy-altered-manifest.zip');
|
||||
}
|
||||
|
||||
public function testPackageIsRejectedBeforeExtractionWhenCoveredFileWasAltered(): void
|
||||
{
|
||||
$this->AssertRejectedBeforeExtraction(self::RESOURCES_DIRECTORY.'dummy-altered-content.zip');
|
||||
}
|
||||
|
||||
public function testPackageWithInvalidSignatureIsRejectedBeforeExtraction(): void
|
||||
{
|
||||
$this->AssertRejectedBeforeExtraction(self::RESOURCES_DIRECTORY.'dummy-badly-signed.zip');
|
||||
}
|
||||
|
||||
public function testPackageWithoutSignatureIsRejectedBeforeExtraction(): void
|
||||
{
|
||||
$this->AssertRejectedBeforeExtraction(self::RESOURCES_DIRECTORY.'dummy-unsigned.zip');
|
||||
}
|
||||
|
||||
public function testPackageWithoutManifestIsRejectedBeforeExtraction(): void
|
||||
{
|
||||
$this->AssertRejectedBeforeExtraction(self::RESOURCES_DIRECTORY.'dummy-missing-manifest.zip');
|
||||
}
|
||||
|
||||
private function AssertRejectedBeforeExtraction(string $sArchivePath): void
|
||||
{
|
||||
$aInstallationBefore = $this->SnapshotDirectory($this->sDestinationDirectory);
|
||||
$bRejected = false;
|
||||
try {
|
||||
CoreUpdater::ValidateAndExtractDownloadedFile(
|
||||
$sArchivePath,
|
||||
$this->sPublicKey,
|
||||
$this->sDestinationDirectory
|
||||
);
|
||||
} catch (\UnexpectedValueException $e) {
|
||||
$bRejected = true;
|
||||
}
|
||||
|
||||
$this->assertTrue($bRejected, 'The invalid update package was not rejected.');
|
||||
$this->assertSame($aInstallationBefore, $this->SnapshotDirectory($this->sDestinationDirectory));
|
||||
$this->assertFileDoesNotExist($this->sDestinationDirectory.'/file1');
|
||||
}
|
||||
|
||||
private function SnapshotDirectory(string $sDirectory): array
|
||||
{
|
||||
$aSnapshot = [];
|
||||
$oIterator = new RecursiveIteratorIterator(
|
||||
new RecursiveDirectoryIterator($sDirectory, RecursiveDirectoryIterator::SKIP_DOTS)
|
||||
);
|
||||
foreach ($oIterator as $oFile) {
|
||||
if ($oFile->isFile()) {
|
||||
$sRelativePath = substr($oFile->getPathname(), strlen($sDirectory) + 1);
|
||||
$aSnapshot[$sRelativePath] = hash_file('sha256', $oFile->getPathname());
|
||||
}
|
||||
}
|
||||
ksort($aSnapshot);
|
||||
|
||||
return $aSnapshot;
|
||||
}
|
||||
|
||||
}
|
||||
@@ -1,52 +0,0 @@
|
||||
<?php
|
||||
|
||||
// Usage: php build-package.php <content-dir> <private-key.pem> <output.zip>
|
||||
|
||||
[$script, $contentDir, $privateKeyPath, $outputZip] = $argv;
|
||||
|
||||
$contentDir = rtrim(realpath($contentDir), '/');
|
||||
$manifestName = 'manifest.xml';
|
||||
$signatureName = 'manifest.xml.sig'; // TODO: use the name the module expects
|
||||
|
||||
// 1. List files (excluding manifest and signature), sorted for a stable output
|
||||
$files = [];
|
||||
$iterator = new RecursiveIteratorIterator(
|
||||
new RecursiveDirectoryIterator($contentDir, FilesystemIterator::SKIP_DOTS)
|
||||
);
|
||||
foreach ($iterator as $file) {
|
||||
$relativePath = substr($file->getPathname(), strlen($contentDir) + 1);
|
||||
if (in_array($relativePath, [$manifestName, $signatureName], true)) {
|
||||
continue;
|
||||
}
|
||||
$files[] = $relativePath;
|
||||
}
|
||||
sort($files);
|
||||
|
||||
// 2. Build manifest.xml in your format
|
||||
$xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<files>\n";
|
||||
foreach ($files as $relativePath) {
|
||||
$fullPath = "$contentDir/$relativePath";
|
||||
$xml .= sprintf(
|
||||
"<file><path>%s</path><md5>%s</md5><size>%d</size></file>\n",
|
||||
htmlspecialchars($relativePath, ENT_XML1),
|
||||
md5_file($fullPath),
|
||||
filesize($fullPath)
|
||||
);
|
||||
}
|
||||
$xml .= "</files>\n";
|
||||
file_put_contents("$contentDir/$manifestName", $xml);
|
||||
|
||||
// 3. Sign the manifest
|
||||
$privateKey = openssl_pkey_get_private(file_get_contents($privateKeyPath));
|
||||
openssl_sign($xml, $signature, $privateKey, OPENSSL_ALGO_SHA256);
|
||||
file_put_contents("$contentDir/$signatureName", $signature); // or base64_encode($signature)
|
||||
|
||||
// 4. Zip everything
|
||||
$zip = new ZipArchive();
|
||||
$zip->open($outputZip, ZipArchive::CREATE | ZipArchive::OVERWRITE);
|
||||
foreach (array_merge($files, [$manifestName, $signatureName]) as $relativePath) {
|
||||
$zip->addFile("$contentDir/$relativePath", $relativePath);
|
||||
}
|
||||
$zip->close();
|
||||
|
||||
echo "Built $outputZip with ".count($files)." files\n";
|
||||
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
@@ -1,28 +0,0 @@
|
||||
-----BEGIN PRIVATE KEY-----
|
||||
MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQDYyavpa+/92vWV
|
||||
vm8VnqthJJ6J6cWZbeD/amvWQg3754Az66Gr4Lf0mTD+7iWbKzjcqANyDLOug3+J
|
||||
Hpxaj0ogx04lBi+MqFZZfYydD1S/xjrNwF4UnE0KfSPF5ZqgDjhJqhtevWlo7lxq
|
||||
KoI6902bmv1XVP8we+aUANQeCI0OI3VeMcIVZ76yEz7BDyrQ1t7Dj7H44gsR4MFQ
|
||||
P4K5QAHJ6+zW36K8/oiIzasvQ5bTVgVyx4UIlBHbw5YRpNdieX+NUFHQnMDUiZ0y
|
||||
1beJHujF24LUrmqAL95S3fADurxEsuDtTUow27z0ZLJ4vxmsGaGKYZT/9ZYJoHdu
|
||||
ux91Xdf9AgMBAAECggEAKqsO5gNzmfBVs+JmdMflDVqqxZQ4aylgRoVIskQQAouf
|
||||
BRkUmgLB0iewqAxxZGgOage7nd/4IZoKKQ/YUE6xlWAE9m6QP0wGqth2K5eP46r9
|
||||
ee+p1L6qvyzANLwrQeNqmr0bY/s92wRHNjIlrbttD/picKTvETjhujGrjjm6oZRm
|
||||
MwOvCJ+/NLOHQbSfmlVhEFQTwH+efCiEvegQYBeLDgBEx/K8K5W4KxbUSnbk0oAM
|
||||
TFEqmuGpSr11uN25W1I2OEKF0Ow2heEv9n9tDgdg5JbiX67HeSpGbrKs7bD6MpAh
|
||||
mrTEHKv6EC6q9RZEtJRnIHFRqMQd1HDzlGSdjebPFQKBgQD3+bG0522x0tU6qmXo
|
||||
gPZMS9iQlCDFZ3q899y3Peu3+ThZAG3MFj1pmBbl2whjuC65EF6cW//O04jqGl6b
|
||||
jNUwUgqcJMywes9JDQ2WWWQp81JAfyQAJ8gL5PHazi/ffK1B03Z9y47dQNy2UIAl
|
||||
4BvhWS2CBYeoPsdYWFbnpHLgFwKBgQDfzZwNzv7qgkREuVNRTAcBxcnnVMVrndXp
|
||||
3JFFDu3lt/JL+B0rxESwLkcgcEA3EUrY8y5Iq7NbnS6doVmZeCxi3sXbY/fEkyEy
|
||||
PHVmAUzHChxcsupZrOWoIxQzNbP8kJB22v9ocKaV1FfBVkkJ2WzWYjcl+DscKZ6X
|
||||
/EoZ4JbhCwKBgGhMiLcrRBMwN2lmdFiXrYCnwh4Ff8cn4BMNObOFAtktpMqiiIv3
|
||||
qrr4MmFZgCzZMiW6weCIPY8ToBm/H53TXXPmffnw7HsqHHeAWv+q95tBcY4ZK944
|
||||
3UZLdIJwiOg6O3FdbRA6Bwv+lRbFwzCrE270IFT42HcmeJgrB9qEa6DfAoGAJP9G
|
||||
dvnrJjVWt82zOlNJLT/XOsjGwh2biUePNE1dgoZAJdLkb6HIgR+FXpVv6CqRpv+8
|
||||
fQL1aYW4eRJ9T2OQYt/U8Ym7IeH4sOdRuQUp/hVrzt884oo0hPEXm8ea83uqhht5
|
||||
8Xf98fLidvGerWQmG+KsnzhD/UncD+ADF/gKuTUCgYAWM5ulgrgdo/ceum1JzMsh
|
||||
qRDoFN1ktXE8oZtoOvtWAECEzMk51QiuxCYikyE5jqg2Vys3LBZXqTvuyPzL/HDJ
|
||||
oB734PY8hoI1BYrJ0dKjQCPfImX2VMJQZ+Yk/21UaBwOIdoGDC1NVLNBlvjvjEEv
|
||||
IWp2vMPzX4CDAee+86uMjg==
|
||||
-----END PRIVATE KEY-----
|
||||
@@ -1,9 +0,0 @@
|
||||
-----BEGIN PUBLIC KEY-----
|
||||
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA2Mmr6Wvv/dr1lb5vFZ6r
|
||||
YSSeienFmW3g/2pr1kIN++eAM+uhq+C39Jkw/u4lmys43KgDcgyzroN/iR6cWo9K
|
||||
IMdOJQYvjKhWWX2MnQ9Uv8Y6zcBeFJxNCn0jxeWaoA44SaobXr1paO5caiqCOvdN
|
||||
m5r9V1T/MHvmlADUHgiNDiN1XjHCFWe+shM+wQ8q0Nbew4+x+OILEeDBUD+CuUAB
|
||||
yevs1t+ivP6IiM2rL0OW01YFcseFCJQR28OWEaTXYnl/jVBR0JzA1ImdMtW3iR7o
|
||||
xduC1K5qgC/eUt3wA7q8RLLg7U1KMNu89GSyeL8ZrBmhimGU//WWCaB3brsfdV3X
|
||||
/QIDAQAB
|
||||
-----END PUBLIC KEY-----
|
||||
Binary file not shown.
Binary file not shown.
@@ -1 +0,0 @@
|
||||
that's a file 1
|
||||
@@ -1 +0,0 @@
|
||||
nice content for file 1
|
||||
@@ -1 +0,0 @@
|
||||
nicer content for file 2
|
||||
@@ -1 +0,0 @@
|
||||
another file 1
|
||||
@@ -1,7 +0,0 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<files>
|
||||
<file><path>file1</path><md5>e7d0f0aa46a74bd08e0f0f268a19812b</md5><size>15</size></file>
|
||||
<file><path>folder1/file1</path><md5>f0d7b0cc26724909f6cb17f55040e054</md5><size>23</size></file>
|
||||
<file><path>folder1/file2</path><md5>1696bef05836b4787a86a6497781e161</md5><size>24</size></file>
|
||||
<file><path>folder2/file1</path><md5>d9966d42983e134e3f5249a5f2be66f7</md5><size>14</size></file>
|
||||
</files>
|
||||
Binary file not shown.
Reference in New Issue
Block a user