From a5963da4e614c9161040d50e6b26e785a54ff565 Mon Sep 17 00:00:00 2001 From: Jeppe Bundgaard Date: Thu, 12 Mar 2026 20:20:43 +0100 Subject: [PATCH] Cap expanded query terms, aliases, and hints to prevent amplification, add permission-context scoping for query cache, and encapsulate term capping logic in helper methods with relevant tests. --- .../system_search_openai_intent_parser.php | 85 ++++++++++-- .../app/classes/system_search_service.php | 65 ++++++++- .../SystemSearchOpenAiIntentParserTest.php | 44 ++++++ .../SystemSearchServiceIntentFlowTest.php | 126 ++++++++++++++++++ 4 files changed, 306 insertions(+), 14 deletions(-) diff --git a/services/nginx/app/classes/system_search_openai_intent_parser.php b/services/nginx/app/classes/system_search_openai_intent_parser.php index 5d6d347f..91aae9c9 100644 --- a/services/nginx/app/classes/system_search_openai_intent_parser.php +++ b/services/nginx/app/classes/system_search_openai_intent_parser.php @@ -12,6 +12,12 @@ class system_search_openai_intent_parser implements system_search_intent_parser_ private string $model = 'gpt-4.1-mini'; private float $temperature = 0.1; private int $timeoutSeconds = 10; + private int $maxAliases = 12; + private int $maxEntityHints = 8; + private int $maxAliasLength = 64; + private int $maxHintLength = 32; + private int $maxNormalizedQueryLength = 256; + private int $maxFallbackReasonLength = 160; /** * @var callable|null @@ -54,7 +60,7 @@ class system_search_openai_intent_parser implements system_search_intent_parser_ $cached = system_search_cache::getIntent($cacheHash); if (is_array($cached) && isset($cached['success'])) { $cached['source'] = 'cache'; - return $this->normalizeResult($cached); + return $this->normalizeResult($cached, $allowedEntityTypes); } try { @@ -62,7 +68,7 @@ class system_search_openai_intent_parser implements system_search_intent_parser_ $parsed = $this->parseResponse($raw); $parsed['source'] = 'openai'; system_search_cache::setIntent($cacheHash, $parsed, 3600); - return $this->normalizeResult($parsed); + return $this->normalizeResult($parsed, $allowedEntityTypes); } catch (Throwable $e) { return $this->failed($e->getMessage(), 'openai'); } @@ -130,14 +136,25 @@ class system_search_openai_intent_parser implements system_search_intent_parser_ 'type' => 'object', 'properties' => [ 'success' => ['type' => 'boolean'], - 'normalized_query' => ['type' => 'string'], + 'normalized_query' => [ + 'type' => 'string', + 'maxLength' => $this->maxNormalizedQueryLength, + ], 'aliases' => [ 'type' => 'array', - 'items' => ['type' => 'string'], + 'maxItems' => $this->maxAliases, + 'items' => [ + 'type' => 'string', + 'maxLength' => $this->maxAliasLength, + ], ], 'entity_hints' => [ 'type' => 'array', - 'items' => ['type' => 'string'], + 'maxItems' => $this->maxEntityHints, + 'items' => [ + 'type' => 'string', + 'maxLength' => $this->maxHintLength, + ], ], 'confidence' => [ 'type' => 'number', @@ -145,7 +162,12 @@ class system_search_openai_intent_parser implements system_search_intent_parser_ 'maximum' => 1, ], 'association_hint' => ['type' => 'boolean'], - 'fallback_reason' => ['type' => ['string', 'null']], + 'fallback_reason' => [ + 'anyOf' => [ + ['type' => 'string', 'maxLength' => $this->maxFallbackReasonLength], + ['type' => 'null'], + ], + ], ], 'required' => [ 'success', @@ -215,22 +237,63 @@ class system_search_openai_intent_parser implements system_search_intent_parser_ return $decoded; } - private function normalizeResult(array $result): array + private function normalizeResult(array $result, array $allowedEntityTypes = []): array { - $entityHints = array_values(array_unique(array_filter(array_map('strval', (array)($result['entity_hints'] ?? []))))); - $aliases = array_values(array_unique(array_filter(array_map('strval', (array)($result['aliases'] ?? []))))); + $normalizedQuery = trim((string)($result['normalized_query'] ?? '')); + if (mb_strlen($normalizedQuery) > $this->maxNormalizedQueryLength) { + $normalizedQuery = mb_substr($normalizedQuery, 0, $this->maxNormalizedQueryLength); + } + + $aliases = $this->sanitizeStringList((array)($result['aliases'] ?? []), $this->maxAliases, $this->maxAliasLength); + $entityHints = $this->sanitizeStringList((array)($result['entity_hints'] ?? []), $this->maxEntityHints, $this->maxHintLength); + if (!empty($allowedEntityTypes)) { + $entityHints = array_values(array_intersect($allowedEntityTypes, $entityHints)); + } + + $fallbackReason = null; + if (isset($result['fallback_reason']) && $result['fallback_reason'] !== null) { + $fallbackReason = trim((string)$result['fallback_reason']); + if (mb_strlen($fallbackReason) > $this->maxFallbackReasonLength) { + $fallbackReason = mb_substr($fallbackReason, 0, $this->maxFallbackReasonLength); + } + } + return [ 'success' => (bool)($result['success'] ?? false), - 'normalized_query' => (string)($result['normalized_query'] ?? ''), + 'normalized_query' => $normalizedQuery, 'aliases' => $aliases, 'entity_hints' => $entityHints, 'confidence' => max(0.0, min(1.0, (float)($result['confidence'] ?? 0.0))), 'association_hint' => (bool)($result['association_hint'] ?? false), - 'fallback_reason' => isset($result['fallback_reason']) ? (string)$result['fallback_reason'] : null, + 'fallback_reason' => $fallbackReason, 'source' => (string)($result['source'] ?? 'openai'), ]; } + private function sanitizeStringList(array $values, int $maxItems, int $maxLength): array + { + $result = []; + foreach ($values as $value) { + if (!is_string($value)) { + continue; + } + $item = trim(mb_strtolower($value)); + if ($item === '') { + continue; + } + if (mb_strlen($item) > $maxLength) { + $item = mb_substr($item, 0, $maxLength); + } + if (!in_array($item, $result, true)) { + $result[] = $item; + } + if (count($result) >= $maxItems) { + break; + } + } + return $result; + } + private function failed(string $reason, string $source): array { return [ diff --git a/services/nginx/app/classes/system_search_service.php b/services/nginx/app/classes/system_search_service.php index 7347e06a..cea59c71 100644 --- a/services/nginx/app/classes/system_search_service.php +++ b/services/nginx/app/classes/system_search_service.php @@ -11,6 +11,8 @@ class system_search_service private int $lowConfidenceResultThreshold = 5; private int $lowConfidenceTopScoreThreshold = 60; private int $defaultEntityFetchLimit = 200; + private int $maxExpandedTerms = 24; + private int $maxTermLength = 64; private array $tableColumnsCache = []; public function __construct(?system_search_intent_parser_i $intentParser = null) @@ -81,6 +83,7 @@ class system_search_service 'own_only' => $ownOnlyTypes, 'assoc' => $includeAssociations, 'dbg' => $debugIntent, + 'ctx' => $this->permissionContextFingerprint($permissionsCatalogAll, $permissionsCatalogOwn, $moduleConfigVisibility), 'v' => 2, ], JSON_UNESCAPED_UNICODE)); @@ -90,7 +93,7 @@ class system_search_service return $cached; } - $terms = $this->tokenize($query); + $terms = $this->limitTerms($this->tokenize($query)); $entityBoost = []; $initialResults = $this->executeLexicalSearch( $activeTypes, @@ -124,11 +127,11 @@ class system_search_service if (!empty($intent['success'])) { $intentMeta['status'] = 'ok'; - $expandedTerms = array_values(array_unique([ + $expandedTerms = $this->limitTerms([ ...$terms, ...$this->tokenize((string)($intent['normalized_query'] ?? '')), ...$this->tokenize(implode(' ', (array)($intent['aliases'] ?? []))), - ])); + ]); $intentMeta['expanded_terms'] = $expandedTerms; $boostedTypes = array_values(array_intersect($activeTypes, (array)($intent['entity_hints'] ?? []))); @@ -986,6 +989,62 @@ class system_search_service return array_values(array_unique($parts)); } + private function limitTerms(array $terms): array + { + $normalized = []; + foreach ($terms as $term) { + if (!is_string($term)) { + continue; + } + $value = trim(mb_strtolower($term)); + if ($value === '' || mb_strlen($value) < 2) { + continue; + } + if (mb_strlen($value) > $this->maxTermLength) { + $value = mb_substr($value, 0, $this->maxTermLength); + } + $normalized[] = $value; + if (count($normalized) >= $this->maxExpandedTerms) { + break; + } + } + return array_values(array_unique($normalized)); + } + + private function permissionContextFingerprint( + array $permissionsCatalogAll, + array $permissionsCatalogOwn, + array $moduleConfigVisibility + ): string { + $all = []; + foreach ($permissionsCatalogAll as $permission => $description) { + if (!is_string($permission) || $permission === '') { + continue; + } + $all[$permission] = is_string($description) ? $description : (string)$description; + } + ksort($all); + + $own = array_values(array_unique(array_filter(array_map(static fn($v) => is_string($v) ? trim($v) : '', $permissionsCatalogOwn)))); + sort($own); + + $visibility = []; + foreach ($moduleConfigVisibility as $module => $visible) { + if (!is_string($module) || $module === '') { + continue; + } + $visibility[$module] = (bool)$visible; + } + ksort($visibility); + + return md5(json_encode([ + 'all' => $all, + 'own' => $own, + 'visibility' => $visibility, + 'v' => 1, + ], JSON_UNESCAPED_UNICODE)); + } + private function normalizeTypes(array $types): array { $normalized = []; diff --git a/services/nginx/app/tests/Unit/Search/SystemSearchOpenAiIntentParserTest.php b/services/nginx/app/tests/Unit/Search/SystemSearchOpenAiIntentParserTest.php index 556dd7f4..5ca3afbf 100644 --- a/services/nginx/app/tests/Unit/Search/SystemSearchOpenAiIntentParserTest.php +++ b/services/nginx/app/tests/Unit/Search/SystemSearchOpenAiIntentParserTest.php @@ -190,3 +190,47 @@ it('uses parser cache for identical query and allowed type combinations', functi expect($second['source'])->toBe('cache'); expect($calls)->toBe(1); }); + +it('caps alias and hint payloads from OpenAI and filters hints to allowed types', function (): void { + $manyAliases = []; + for ($i = 0; $i < 40; $i++) { + $manyAliases[] = 'ALIAS_' . $i . '_' . str_repeat('x', 90); + } + + $parser = new system_search_openai_intent_parser( + function (array $payload, string $apiKey) use ($manyAliases): array { + return [ + 'output' => [ + [ + 'content' => [ + [ + 'text' => json_encode([ + 'success' => true, + 'normalized_query' => str_repeat('q', 500), + 'aliases' => $manyAliases, + 'entity_hints' => ['orders', 'invoices', 'made_up_type'], + 'confidence' => 0.7, + 'association_hint' => false, + 'fallback_reason' => null, + ], JSON_UNESCAPED_UNICODE), + ], + ], + ], + ], + ]; + }, + true, + 'test-key' + ); + + $result = $parser->parse('acme', ['orders', 'invoices']); + + expect(count($result['aliases']))->toBeLessThanOrEqual(12); + $longestAlias = 0; + foreach ($result['aliases'] as $alias) { + $longestAlias = max($longestAlias, mb_strlen((string)$alias)); + } + expect($longestAlias)->toBeLessThanOrEqual(64); + expect(mb_strlen((string)$result['normalized_query']))->toBeLessThanOrEqual(256); + expect($result['entity_hints'])->toBe(['orders', 'invoices']); +}); diff --git a/services/nginx/app/tests/Unit/Search/SystemSearchServiceIntentFlowTest.php b/services/nginx/app/tests/Unit/Search/SystemSearchServiceIntentFlowTest.php index 774c5d53..7b7b37d0 100644 --- a/services/nginx/app/tests/Unit/Search/SystemSearchServiceIntentFlowTest.php +++ b/services/nginx/app/tests/Unit/Search/SystemSearchServiceIntentFlowTest.php @@ -9,6 +9,63 @@ use classes\system_search_cache; use classes\system_search_service; use interfaces\system_search_intent_parser_i; +if (!class_exists('SystemSearchTestRedisAdapter')) { + class SystemSearchTestRedisAdapter + { + private array $store = []; + + public function get(string $key): mixed + { + return $this->store[$key] ?? null; + } + + public function set(string $key, string $value): void + { + $this->store[$key] = $value; + } + + public function setEx(string $key, string $value, int $ttl): void + { + $this->store[$key] = $value; + } + + public function delete(string $key): void + { + unset($this->store[$key]); + } + + public function expire(string $key, int $ttl): void + { + // TTL is not simulated in unit tests. + } + + public function set_array(string $key, array $value): void + { + $this->store[$key] = json_encode($value, JSON_UNESCAPED_UNICODE); + } + + public function get_array(string $key): ?array + { + $value = $this->store[$key] ?? null; + if (!is_string($value)) { + return null; + } + $decoded = json_decode($value, true); + return is_array($decoded) ? $decoded : null; + } + + public function clear_keys(string $pattern): void + { + $regex = '/^' . str_replace('\*', '.*', preg_quote($pattern, '/')) . '$/'; + foreach (array_keys($this->store) as $key) { + if (preg_match($regex, $key)) { + unset($this->store[$key]); + } + } + } + } +} + if (!class_exists('FakeSystemSearchIntentParser')) { class FakeSystemSearchIntentParser implements system_search_intent_parser_i { @@ -222,3 +279,72 @@ it('returns fallback parser metadata when parser fails gracefully', function (): expect($result['meta']['intent_parser']['status'])->toBe('fallback'); expect($result['meta']['intent_parser']['fallback_reason'])->toBe('openai_disabled'); }); + +it('scopes query cache by permission context to avoid cross-user cache leakage', function (): void { + system_search_cache::setAdapterForTests(new SystemSearchTestRedisAdapter()); + + $parser = new FakeSystemSearchIntentParser(); + $service = new TestableSystemSearchService($parser, [ + [['entity_type' => 'orders', 'entity_id' => '1', 'title' => 'first', 'score' => 95]], + [['entity_type' => 'orders', 'entity_id' => '2', 'title' => 'second', 'score' => 96]], + ]); + + $first = $service->search([ + 'query' => 'acme', + 'allowed_types' => ['orders'], + 'include_associations' => false, + 'permissions_catalog_own' => ['list_orders'], + 'module_config_visibility' => ['openAI' => true], + ]); + $second = $service->search([ + 'query' => 'acme', + 'allowed_types' => ['orders'], + 'include_associations' => false, + 'permissions_catalog_own' => ['list_own_orders'], + 'module_config_visibility' => ['openAI' => false], + ]); + + expect(count($service->lexicalCalls))->toBe(2); + expect($first['results'][0]['entity_id'])->toBe('1'); + expect($second['results'][0]['entity_id'])->toBe('2'); +}); + +it('caps AI-driven expanded terms to prevent query amplification', function (): void { + $aliases = []; + for ($i = 0; $i < 80; $i++) { + $aliases[] = 'alias_' . $i . '_' . str_repeat('x', 90); + } + + $parser = new FakeSystemSearchIntentParser([ + [ + 'success' => true, + 'normalized_query' => str_repeat('n', 500), + 'aliases' => $aliases, + 'entity_hints' => ['orders'], + 'confidence' => 0.8, + 'association_hint' => false, + 'fallback_reason' => null, + 'source' => 'openai', + ], + ]); + + $service = new TestableSystemSearchService($parser, [ + [['entity_type' => 'orders', 'entity_id' => '1', 'title' => 'weak', 'score' => 5]], + [['entity_type' => 'orders', 'entity_id' => '2', 'title' => 'strong', 'score' => 80]], + ]); + + $service->search([ + 'query' => 'a b', + 'allowed_types' => ['orders'], + 'include_associations' => false, + 'debug_intent' => true, + ]); + + $expanded = $service->lexicalCalls[1]['terms'] ?? []; + expect(count($expanded))->toBeLessThanOrEqual(24); + $maxLen = 0; + foreach ($expanded as $term) { + $maxLen = max($maxLen, mb_strlen((string)$term)); + } + expect($maxLen)->toBeLessThanOrEqual(64); +});