From 2441ffd69446946a20efa207fa996a16eb52e22e Mon Sep 17 00:00:00 2001 From: Denis Flaven Date: Tue, 15 Sep 2026 14:21:49 +0200 Subject: [PATCH 1/7] New tool search-any-object-by-oql --- src/Service/DatamodelService.php | 13 ++++++++++ src/Tools/core/get/iTopGetTools.php | 26 +++++++++++++++++++ templates/Anything-output.toon.twig | 2 +- .../searchAnyObjectByOql-input.json.twig | 8 ++++++ tests/phpunit/DatamodelServiceTest.php | 20 +++++++++++++- 5 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 templates/searchAnyObjectByOql-input.json.twig diff --git a/src/Service/DatamodelService.php b/src/Service/DatamodelService.php index 7824f0e..e11d1c8 100644 --- a/src/Service/DatamodelService.php +++ b/src/Service/DatamodelService.php @@ -208,5 +208,18 @@ protected function getWorkflowfromXml(string $className): array } return $workflow; } + + /** + * Extract the name of the selected class from an OQL query and check that the class is valid for the current datamodel + * @param string $oql + */ + public function getClassFromOQL(string $oql) + { + $aMatches = []; + if (!preg_match('/^SELECT ([_a-zA-Z][_a-zA-Z0-9]*)/', $oql, $aMatches)) { + throw new \InvalidArgumentException("Syntax error: '$oql' does not look like a valid OQL query."); + } + return $aMatches[1]; + } } diff --git a/src/Tools/core/get/iTopGetTools.php b/src/Tools/core/get/iTopGetTools.php index 87ea767..96d6d5a 100644 --- a/src/Tools/core/get/iTopGetTools.php +++ b/src/Tools/core/get/iTopGetTools.php @@ -114,4 +114,30 @@ public function searchUserRequestByCallerStatusStartDate(#[Schema(format: 'email ] ); } + + /** + * This tool searches for any object in iTop based on an OQL query. + * The OQL query must be a valid iTop OQL query that returns objects of the desired class. + * The result will be a list of objects of the same class, with their fields as specified in the datamodel. + * IMPORTANT: Do NOT assume that the datamodel is the "standard" one, use the tool get-class-schema - before creating the OQL query - + * to find the possible values for the class, its fields and its relations. + * @param string $oql The OQL query to execute. It must be a valid OQL query that returns objects of the desired class. + * @param int $limit The number of objects per page + * @param int $page The (one based) number of the page + */ + #[McpTool(name: TOOL_PREFIX.'search-any-object-by-oql', annotations: new ToolAnnotations(null, true, false, true, false))] + public function searchAnyObjectByOql(string $oql, int $limit = 20, int $page = 1 ): string + { + $className = $this->datamodel->getClassFromOQL($oql); + $outputFields = $this->datamodel->getListZlist($className); + return $this->runToolFromTemplates('searchAnyObjectByOql', 'Anything', + [ + 'class' => $className, + 'output_fields' => $outputFields, + 'oql' => $oql, + 'limit' => $limit, + 'page' => $page, + ] + ); + } } \ No newline at end of file diff --git a/templates/Anything-output.toon.twig b/templates/Anything-output.toon.twig index a1bfcc2..5c87b5b 100644 --- a/templates/Anything-output.toon.twig +++ b/templates/Anything-output.toon.twig @@ -10,7 +10,7 @@ class {{ className }}: {% endfor %} -{{ className }} { +{{ className }} {% if json.message is defined %}(Total number of objects {{json.message|lower}}){% endif %} { {% for obj in json.objects %} {{obj.key}}{% for code, value in obj.fields %}{% if code !== 'id' and value is not iterable %}, "{{ value }}"{% endif %}{% endfor ~%} {% endfor %} diff --git a/templates/searchAnyObjectByOql-input.json.twig b/templates/searchAnyObjectByOql-input.json.twig new file mode 100644 index 0000000..c66a395 --- /dev/null +++ b/templates/searchAnyObjectByOql-input.json.twig @@ -0,0 +1,8 @@ +{ + "operation":"core/get", + "class": "{{class}}", + "key": "{{oql}}", + "limit": {{limit}}, + "page": {{page}}, + "output_fields": "{{output_fields}}" +} \ No newline at end of file diff --git a/tests/phpunit/DatamodelServiceTest.php b/tests/phpunit/DatamodelServiceTest.php index 5063508..ada1863 100644 --- a/tests/phpunit/DatamodelServiceTest.php +++ b/tests/phpunit/DatamodelServiceTest.php @@ -32,7 +32,25 @@ public function testGetClassSchema(): void $service = new DatamodelService(__DIR__.'/../../data/datamodel-production.xml', $this->cache, 'FR FR'); $schema = $service->getClassSchema('Person'); $expected = 'name,status,org_id,org_name,email,phone,notify,function,cis_list,picture,first_name,employee_number,mobile_phone,location_id,location_name,manager_id,manager_name,team_list,user_list,tickets_list'; - $this->assertEquals(explode(',', $expected), array_keys($schema)); + $this->assertEquals(explode(',', $expected), array_keys($schema['fields'])); + } + + public function testGetClassFromOQLOk(): void + { + $service = new DatamodelService(__DIR__.'/../../data/datamodel-production.xml', $this->cache, 'FR FR'); + $this->assertEquals('Person', $service->getClassFromOQL("SELECT Person WHERE name='Foo'")); + } + + public function testGetClassFromOQLKo(): void + { + $service = new DatamodelService(__DIR__.'/../../data/datamodel-production.xml', $this->cache, 'FR FR'); + + $this->expectException("InvalidArgumentException"); + $this->assertEquals('Person', $service->getClassFromOQL("SELECT 123456 WHERE name='Foo'")); + + $this->expectException("InvalidArgumentException"); + $this->assertEquals('Person', $service->getClassFromOQL("NOT A SELECT Person")); + } } From d8f0b15950d7d88c5f04c6b0bb898ed2facb9899 Mon Sep 17 00:00:00 2001 From: Denis Flaven Date: Tue, 15 Sep 2026 19:04:19 +0200 Subject: [PATCH 2/7] Issue #7 - Escape user-controlled values --- src/Kernel.php | 5 ++++- src/Tools/core/get/iTopGetTools.php | 8 ++++---- src/Tools/iTopRestTools.php | 25 +++++++++++++++++++++++++ tests/phpunit/ToolsFromTemplateTest.php | 10 ++++++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/Kernel.php b/src/Kernel.php index c46a785..25b095b 100644 --- a/src/Kernel.php +++ b/src/Kernel.php @@ -11,7 +11,10 @@ class Kernel extends BaseKernel public function __construct(string $environment, string $debug) { - define('TOOL_PREFIX', $_ENV['APP_TOOLS_PREFIX']); + if (!defined('TOOL_PREFIX')) { + // Prevent redefinition when running unit tests + define('TOOL_PREFIX', $_ENV['APP_TOOLS_PREFIX']); + } parent::__construct($environment, $debug); } } diff --git a/src/Tools/core/get/iTopGetTools.php b/src/Tools/core/get/iTopGetTools.php index 96d6d5a..dde379e 100644 --- a/src/Tools/core/get/iTopGetTools.php +++ b/src/Tools/core/get/iTopGetTools.php @@ -41,7 +41,7 @@ public function getPersonFromEmail(#[Schema(format: 'email')] string $email): st { $this->mcpLogger->info('[Tool called] get-person-from-email'); return $this->runToolFromTemplates('getPersonFromEmail', 'Anything', [ - 'email' => $email, + 'email' => static::quoteString($email), 'fields' => $this->datamodel->getListZlist('Person'), ]); } @@ -54,7 +54,7 @@ public function getPersonFromFullname(string $fullname): string { $this->mcpLogger->info('[Tool called] get-person-from-fullname'); return $this->runToolFromTemplates('getPersonFromFullname', 'Anything', [ - 'fullname' => $fullname, + 'fullname' => static::quoteString($fullname), 'fields' => $this->datamodel->getListZlist('Person'), ]); } @@ -71,7 +71,7 @@ public function getPersonFromTelephone(string $telephone): string if (count($phones) === 0) { throw new \Exception('Datamodel issue: there seem to be no phone number attribute on the Person class!'); } - $conditions = array_map(function($item) use($telephone) { return "$item = '$telephone'"; }, $phones); + $conditions = array_map(function($item) use($me, $telephone) { return "$item = '".iTopGetTools::quoteString($telephone)."'"; }, $phones); $whereClause = implode(' OR ', $conditions); return $this->runToolFromTemplates('getPersonFromTelephone', 'Anything', [ 'query' => 'SELECT Person WHERE '.$whereClause, @@ -107,7 +107,7 @@ public function searchUserRequestByCallerStatusStartDate(#[Schema(format: 'email { return $this->runToolFromTemplates('searchUserRequestFromCallerStatusDate', 'UserRequest', [ - 'caller_email' => $caller_email, + 'caller_email' => static::quoteString($caller_email), 'statuses' => count($statuses) > 0 ? "'".implode("','", $statuses)."'" : '', 'start_date' => $start_date, 'condition' => $condition === 'greater_than' ? '>=' : '<=', diff --git a/src/Tools/iTopRestTools.php b/src/Tools/iTopRestTools.php index 1398397..b06cfd6 100644 --- a/src/Tools/iTopRestTools.php +++ b/src/Tools/iTopRestTools.php @@ -36,4 +36,29 @@ protected function postJsonToItop(string $json): string { return $this->iTopClient->postJsonToItop($json); } + + /** + * Quote the string to protect from user input breaking the JSON / OQL + * Based on MySQL's real_escape_string logic + * @param string $value + * @return string + */ + public static function quoteString(string $value) + { + $replacementMap = [ + "\0" => "\\0", + "\n" => "\\n", + "\r" => "\\r", + "\t" => "\\t", + chr(26) => "\\Z", + chr(8) => "\\b", + '"' => '\"', + "'" => "\'", + '_' => "\_", + "%" => "\%", + '\\' => '\\\\' + ]; + + return strtr($value, $replacementMap); + } } \ No newline at end of file diff --git a/tests/phpunit/ToolsFromTemplateTest.php b/tests/phpunit/ToolsFromTemplateTest.php index 065d3ef..9b25af3 100644 --- a/tests/phpunit/ToolsFromTemplateTest.php +++ b/tests/phpunit/ToolsFromTemplateTest.php @@ -117,4 +117,14 @@ protected function MockiTopClientThatWillReturn(string $inputData, array $output ->willReturn(json_encode($outputData)); return $mockiTopClient; } + + public function testQuoteString(): void + { + $this->assertEquals("O\\'Reilly", iTopGetTools::quoteString("O'Reilly")); + $this->assertEquals("Line1\\nLine2", iTopGetTools::quoteString("Line1\nLine2")); + $this->assertEquals("Tab\\tSeparated", iTopGetTools::quoteString("Tab\tSeparated")); + $this->assertEquals("Percent\\%Sign", iTopGetTools::quoteString("Percent%Sign")); + $this->assertEquals("Underscore\\_Test", iTopGetTools::quoteString("Underscore_Test")); + $this->assertEquals("Backslash\\\\Test", iTopGetTools::quoteString("Backslash\\Test")); + } } \ No newline at end of file From 0ada4ab9fe8f664276641ac1f47711311b6f033d Mon Sep 17 00:00:00 2001 From: Denis Flaven Date: Mon, 21 Sep 2026 15:46:14 +0200 Subject: [PATCH 3/7] Better parameters validation --- src/Service/DatamodelService.php | 20 +++++++++++++--- src/Tools/core/get/iTopGetTools.php | 10 ++++++-- tests/phpunit/DatamodelServiceTest.php | 33 +++++++++++++++++++++----- 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/Service/DatamodelService.php b/src/Service/DatamodelService.php index e11d1c8..9b3c59c 100644 --- a/src/Service/DatamodelService.php +++ b/src/Service/DatamodelService.php @@ -29,6 +29,12 @@ public function getClasses(): array return $classes; } + public function IsValidClass(string $className): bool + { + $classes = $this->getClasses(); + return isset($classes[$className]); + } + public function getClassSchema(string $className): array { $classSchema = $this->datamodelCache->get('ClassSchema-'.$className.'-'.$this->language, function(ItemInterface $item) use ($className) { @@ -216,10 +222,18 @@ protected function getWorkflowfromXml(string $className): array public function getClassFromOQL(string $oql) { $aMatches = []; - if (!preg_match('/^SELECT ([_a-zA-Z][_a-zA-Z0-9]*)/', $oql, $aMatches)) { - throw new \InvalidArgumentException("Syntax error: '$oql' does not look like a valid OQL query."); + $oql = trim($oql); + if (!preg_match('/^SELECT +([_a-zA-Z][_a-zA-Z0-9]*) +(?:JOIN|WHERE|AS)/', $oql, $aMatches)) { + if (!preg_match('/^SELECT +(?:[_a-zA-Z][_a-zA-Z0-9, ]*) +FROM +([_a-zA-Z][_a-zA-Z0-9]*)/', $oql, $aMatches)) { + throw new \InvalidArgumentException("Syntax error: '$oql' does not look like a valid OQL query."); + } + } + + $className = $aMatches[1]; + if (!$this->IsValidClass($className)) { + throw new \InvalidArgumentException("Class '$className' is not a valid class in the current datamodel."); } - return $aMatches[1]; + return $className; } } diff --git a/src/Tools/core/get/iTopGetTools.php b/src/Tools/core/get/iTopGetTools.php index dde379e..91da2ec 100644 --- a/src/Tools/core/get/iTopGetTools.php +++ b/src/Tools/core/get/iTopGetTools.php @@ -122,12 +122,18 @@ public function searchUserRequestByCallerStatusStartDate(#[Schema(format: 'email * IMPORTANT: Do NOT assume that the datamodel is the "standard" one, use the tool get-class-schema - before creating the OQL query - * to find the possible values for the class, its fields and its relations. * @param string $oql The OQL query to execute. It must be a valid OQL query that returns objects of the desired class. - * @param int $limit The number of objects per page - * @param int $page The (one based) number of the page + * @param int $limit The number of objects per page (> 0) + * @param int $page The (one based) number of the page (>= 1) */ #[McpTool(name: TOOL_PREFIX.'search-any-object-by-oql', annotations: new ToolAnnotations(null, true, false, true, false))] public function searchAnyObjectByOql(string $oql, int $limit = 20, int $page = 1 ): string { + if ($limit <= 0) { + throw new \InvalidArgumentException('Limit must be a positive integer'); + } + if ($page < 1) { + throw new \InvalidArgumentException('Page must be a positive integer (1 or greater)'); + } $className = $this->datamodel->getClassFromOQL($oql); $outputFields = $this->datamodel->getListZlist($className); return $this->runToolFromTemplates('searchAnyObjectByOql', 'Anything', diff --git a/tests/phpunit/DatamodelServiceTest.php b/tests/phpunit/DatamodelServiceTest.php index ada1863..7049bb2 100644 --- a/tests/phpunit/DatamodelServiceTest.php +++ b/tests/phpunit/DatamodelServiceTest.php @@ -4,6 +4,7 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Service\DatamodelService; use Symfony\Contracts\Cache\CacheInterface; +use PHPUnit\Framework\Attributes\DataProvider; class DatamodelServiceTest extends KernelTestCase @@ -38,19 +39,39 @@ public function testGetClassSchema(): void public function testGetClassFromOQLOk(): void { $service = new DatamodelService(__DIR__.'/../../data/datamodel-production.xml', $this->cache, 'FR FR'); + $this->assertEquals('Person', $service->getClassFromOQL(" SELECT Person WHERE name='Foo'")); + $this->assertEquals('Person', $service->getClassFromOQL("SELECT Person WHERE name='Foo'")); + + $this->assertEquals('Person', $service->getClassFromOQL("SELECT P FROM Person WHERE name='Foo'")); + + $this->assertEquals('Person', $service->getClassFromOQL("SELECT P,O FROM Person JOIN Organization AS O ON P.org_d = O.id WHERE name='Foo'")); + + $this->assertEquals('Person', $service->getClassFromOQL("SELECT Person AS P WHERE name='Foo'")); + + $this->assertEquals('Person', $service->getClassFromOQL("SELECT Person AS P JOIN Organization AS O WHERE P.name='Foo'")); } - public function testGetClassFromOQLKo(): void + /** + * @param string $oql + */ + #[DataProvider('oqlKoProvider')] + public function testGetClassFromOQLKo(string $oql): void { $service = new DatamodelService(__DIR__.'/../../data/datamodel-production.xml', $this->cache, 'FR FR'); $this->expectException("InvalidArgumentException"); - $this->assertEquals('Person', $service->getClassFromOQL("SELECT 123456 WHERE name='Foo'")); - - $this->expectException("InvalidArgumentException"); - $this->assertEquals('Person', $service->getClassFromOQL("NOT A SELECT Person")); - + $this->assertEquals('Person', $service->getClassFromOQL($oql)); + } + + public static function oqlKoProvider(): array + { + return [ + ["SELECT 123456 WHERE name='Foo'"], + ["NOT A SELECT Person"], + ["SELECT FROM Person WHERE name='Foo'"], + ["SELECT NotAValidClassName WHERE name='Foo'"], + ]; } } From 94ddd0d4e7fcd090c81afc1c2706f192d9be9359 Mon Sep 17 00:00:00 2001 From: Denis Flaven Date: Mon, 21 Sep 2026 15:47:18 +0200 Subject: [PATCH 4/7] Cleanup unused variable --- src/Tools/core/get/iTopGetTools.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/core/get/iTopGetTools.php b/src/Tools/core/get/iTopGetTools.php index 91da2ec..e5133b1 100644 --- a/src/Tools/core/get/iTopGetTools.php +++ b/src/Tools/core/get/iTopGetTools.php @@ -71,7 +71,7 @@ public function getPersonFromTelephone(string $telephone): string if (count($phones) === 0) { throw new \Exception('Datamodel issue: there seem to be no phone number attribute on the Person class!'); } - $conditions = array_map(function($item) use($me, $telephone) { return "$item = '".iTopGetTools::quoteString($telephone)."'"; }, $phones); + $conditions = array_map(function($item) use($telephone) { return "$item = '".iTopGetTools::quoteString($telephone)."'"; }, $phones); $whereClause = implode(' OR ', $conditions); return $this->runToolFromTemplates('getPersonFromTelephone', 'Anything', [ 'query' => 'SELECT Person WHERE '.$whereClause, From 107cb97d7434ef49b726e53b39aecb2ca4d0b8cc Mon Sep 17 00:00:00 2001 From: Denis Flaven Date: Mon, 21 Sep 2026 16:19:26 +0200 Subject: [PATCH 5/7] :white_check_mark: Fixed tests --- tests/phpunit/ToolsFromTemplateTest.php | 54 +++++++++++++++++++++---- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/tests/phpunit/ToolsFromTemplateTest.php b/tests/phpunit/ToolsFromTemplateTest.php index 9b25af3..6d1156d 100644 --- a/tests/phpunit/ToolsFromTemplateTest.php +++ b/tests/phpunit/ToolsFromTemplateTest.php @@ -52,19 +52,20 @@ public function testGetPersonFromEmail(): void ] ]; $mockiTopClient = $this->MockiTopClientThatWillReturn($input, $output); - $tools = new iTopGetTools($this->twigEnvironment, $mockiTopClient, $this->logger, $this->datamodel); - - $this->assertEquals( + + $expected = <<getPersonFromEmail('test@demo.com')); +EXPECTED; + + $tools = new iTopGetTools($this->twigEnvironment, $mockiTopClient, $this->logger, $this->datamodel); + $this->assertEquals($expected, $tools->getPersonFromEmail('test@demo.com')); } public function testGetPersonFromTelephone(): void @@ -100,13 +101,52 @@ public function testGetPersonFromTelephone(): void class Person: id, friendlyname, email, org_id -Person { +Person { 1, "Test Person", "test@demo.com", "1" } EXPECTED; $this->assertEquals(trim($expected), trim($tools->getPersonFromTelephone('123456789'))); } + + public function testSearchAnyObjectByOql() + { + // Mock the iTopClient service, to check that the templating works + $oql = "SELECT Person WHERE name LIKE '%Smith%'"; + $input = +<< [ + 'Person::1' => [ + 'class' => 'Person', + 'key' => 1, + 'fields' => ['friendlyname' => 'John Smith', 'email' => 'john.smith@demo.com'], + ], + ], + ]; + $mockiTopClient = $this->MockiTopClientThatWillReturn($input, $output); + + $expected = +<<twigEnvironment, $mockiTopClient, $this->logger, $this->datamodel); + $this->assertEquals(trim($expected), trim($tools->searchAnyObjectByOql($oql))); + } protected function MockiTopClientThatWillReturn(string $inputData, array $outputData) { From d0cfeb3abd302fef032c2a35fdee21abd37108de Mon Sep 17 00:00:00 2001 From: jf-cbd Date: Wed, 16 Sep 2026 17:11:14 +0200 Subject: [PATCH 6/7] :rocket: Add action to manage issues and PRs --- .github/workflows/add-to-dashboard.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .github/workflows/add-to-dashboard.yml diff --git a/.github/workflows/add-to-dashboard.yml b/.github/workflows/add-to-dashboard.yml new file mode 100644 index 0000000..3c99dcc --- /dev/null +++ b/.github/workflows/add-to-dashboard.yml @@ -0,0 +1,13 @@ +name: Add issue/PR to Combodo dashboard + +on: + pull_request: + types: [opened] + issues: + types: [opened] + +jobs: + route-to-project: + uses: Combodo/.github/.github/workflows/action.yml@master + secrets: + PR_AUTOMATICALLY_ADD_TO_PROJECT: ${{ secrets.PR_AUTOMATICALLY_ADD_TO_PROJECT }} From 95f8c783a473158357af9ed09bcd236e2f4d22f3 Mon Sep 17 00:00:00 2001 From: Denis Flaven Date: Mon, 21 Sep 2026 16:40:45 +0200 Subject: [PATCH 7/7] Better error message. --- src/Service/DatamodelService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service/DatamodelService.php b/src/Service/DatamodelService.php index 9b3c59c..f5ddb4d 100644 --- a/src/Service/DatamodelService.php +++ b/src/Service/DatamodelService.php @@ -231,7 +231,7 @@ public function getClassFromOQL(string $oql) $className = $aMatches[1]; if (!$this->IsValidClass($className)) { - throw new \InvalidArgumentException("Class '$className' is not a valid class in the current datamodel."); + throw new \InvalidArgumentException("Class '$className' is not a valid class in the current datamodel. Use the tool '".TOOL_PREFIX."list-all-classes' to get the list of valid classes."); } return $className; }