From ebf1f3c90cbebc7f2e6df0185595f00b8b68d73f Mon Sep 17 00:00:00 2001 From: Oliver Bartsch Date: Mon, 11 May 2026 22:16:49 +0200 Subject: [PATCH 1/2] [TASK] Add api key encryption --- .../FormDataProvider/DecryptApiKey.php | 45 +++++ Classes/Crypto/ApiKeyEncryption.php | 166 ++++++++++++++++ .../SymfonyAiCompilerPass.php | 6 +- .../ProviderConfigurationRepository.php | 5 + .../Exception/ApiKeyEncryptionException.php | 18 ++ Classes/Hooks/EncryptApiKey.php | 50 +++++ .../SymfonyAi/SymfonyAiPlatformAdapter.php | 14 +- Classes/Updates/EncryptApiKeysUpgrade.php | 108 +++++++++++ Configuration/Services.yaml | 9 + README.md | 15 ++ .../ApiKeyEncryptionPersistenceTest.php | 146 +++++++++++++++ Tests/Unit/Crypto/ApiKeyEncryptionTest.php | 177 ++++++++++++++++++ Tests/Unit/Hooks/EncryptApiKeyTest.php | 100 ++++++++++ composer.json | 6 +- ext_localconf.php | 7 + ext_tables.sql | 2 +- 16 files changed, 862 insertions(+), 12 deletions(-) create mode 100644 Classes/Backend/FormDataProvider/DecryptApiKey.php create mode 100644 Classes/Crypto/ApiKeyEncryption.php create mode 100644 Classes/Exception/ApiKeyEncryptionException.php create mode 100644 Classes/Hooks/EncryptApiKey.php create mode 100644 Classes/Updates/EncryptApiKeysUpgrade.php create mode 100644 Tests/Functional/Crypto/ApiKeyEncryptionPersistenceTest.php create mode 100644 Tests/Unit/Crypto/ApiKeyEncryptionTest.php create mode 100644 Tests/Unit/Hooks/EncryptApiKeyTest.php diff --git a/Classes/Backend/FormDataProvider/DecryptApiKey.php b/Classes/Backend/FormDataProvider/DecryptApiKey.php new file mode 100644 index 0000000..b7229fd --- /dev/null +++ b/Classes/Backend/FormDataProvider/DecryptApiKey.php @@ -0,0 +1,45 @@ +encryption->isEncrypted($value)) { + return $result; + } + + $result['databaseRow']['api_key'] = $this->encryption->decrypt($value); + return $result; + } +} diff --git a/Classes/Crypto/ApiKeyEncryption.php b/Classes/Crypto/ApiKeyEncryption.php new file mode 100644 index 0000000..800b26f --- /dev/null +++ b/Classes/Crypto/ApiKeyEncryption.php @@ -0,0 +1,166 @@ +isEncrypted($plaintext) || $this->isEndpointUrl($plaintext)) { + return $plaintext; + } + + if ($this->coreCipherAvailable()) { + return self::PREFIX_V2 . $this->encryptViaCore($plaintext); + } + return self::PREFIX_V1 . $this->encryptViaSecretbox($plaintext); + } + + /** + * The api_key column doubles as an endpoint URL for providers that + * expose a local HTTP service (Ollama, LM Studio, OpenAI-compatible + * proxies). Those values aren't secrets and shouldn't be encrypted. + */ + public function isEndpointUrl(string $value): bool + { + return str_starts_with($value, 'http://') || str_starts_with($value, 'https://'); + } + + public function decrypt(string $value): string + { + if ($value === '') { + return $value; + } + if (str_starts_with($value, self::PREFIX_V2)) { + return $this->decryptViaCore(substr($value, strlen(self::PREFIX_V2))); + } + if (str_starts_with($value, self::PREFIX_V1)) { + return $this->decryptViaSecretbox(substr($value, strlen(self::PREFIX_V1))); + } + return $value; + } + + public function isEncrypted(string $value): bool + { + return str_starts_with($value, self::PREFIX_ANY); + } + + private function coreCipherAvailable(): bool + { + return class_exists(CipherService::class) && class_exists(KeyFactory::class); + } + + private function encryptViaCore(string $plaintext): string + { + $keyFactory = GeneralUtility::makeInstance(KeyFactory::class); + $cipher = GeneralUtility::makeInstance(CipherService::class); + $sharedKey = $keyFactory->deriveSharedKeyFromEncryptionKey(self::class); + return $cipher->encrypt($plaintext, $sharedKey)->encode(); + } + + private function decryptViaCore(string $payload): string + { + try { + $keyFactory = GeneralUtility::makeInstance(KeyFactory::class); + $cipher = GeneralUtility::makeInstance(CipherService::class); + $sharedKey = $keyFactory->deriveSharedKeyFromEncryptionKey(self::class); + return $cipher->decrypt(CipherValue::fromSerialized($payload), $sharedKey); + } catch (CipherDecryptionFailedException | CipherException $e) { + throw new ApiKeyEncryptionException( + 'AiM API key could not be decrypted via core CipherService: ' . $e->getMessage(), + 1773874410, + $e, + ); + } + } + + private function encryptViaSecretbox(string $plaintext): string + { + $key = $this->deriveSecretboxKey(); + $nonce = random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES); + $ciphertext = sodium_crypto_secretbox($plaintext, $nonce, $key); + sodium_memzero($key); + + return base64_encode($nonce . $ciphertext); + } + + private function decryptViaSecretbox(string $payload): string + { + $bytes = base64_decode($payload, true); + if ($bytes === false || strlen($bytes) <= SODIUM_CRYPTO_SECRETBOX_NONCEBYTES) { + throw new ApiKeyEncryptionException('AiM API key ciphertext is malformed.', 1773874400); + } + + $nonce = substr($bytes, 0, SODIUM_CRYPTO_SECRETBOX_NONCEBYTES); + $ciphertext = substr($bytes, SODIUM_CRYPTO_SECRETBOX_NONCEBYTES); + $key = $this->deriveSecretboxKey(); + $plaintext = sodium_crypto_secretbox_open($ciphertext, $nonce, $key); + sodium_memzero($key); + + if ($plaintext === false) { + throw new ApiKeyEncryptionException( + 'AiM API key could not be decrypted. The system encryption key may have changed.', + 1773874401, + ); + } + + return $plaintext; + } + + private function deriveSecretboxKey(): string + { + $systemKey = (string)($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] ?? ''); + if ($systemKey === '') { + throw new ApiKeyEncryptionException( + 'AiM API key encryption requires $GLOBALS[\'TYPO3_CONF_VARS\'][\'SYS\'][\'encryptionKey\'] to be set.', + 1773874402, + ); + } + + return sodium_crypto_generichash( + self::KEY_DOMAIN . "\0" . $systemKey, + '', + SODIUM_CRYPTO_SECRETBOX_KEYBYTES, + ); + } +} diff --git a/Classes/DependencyInjection/SymfonyAiCompilerPass.php b/Classes/DependencyInjection/SymfonyAiCompilerPass.php index 7de2503..b3100b7 100644 --- a/Classes/DependencyInjection/SymfonyAiCompilerPass.php +++ b/Classes/DependencyInjection/SymfonyAiCompilerPass.php @@ -195,7 +195,7 @@ private function resolveNamespace(string $packageName): ?string private function buildBridgeDefinition(array $package): ?array { $namespace = $package['namespace']; - $factoryClass = $namespace . '\\PlatformFactory'; + $factoryClass = $namespace . '\\Factory'; if (!class_exists($factoryClass)) { return null; @@ -397,7 +397,7 @@ private function deriveName(string $packageName): string } /** - * Detect the primary authentication parameter of a PlatformFactory::create() method. + * Detect the primary authentication parameter of a Factory::createProvider() method. * * Most bridges use `apiKey`. Some (Ollama, LM Studio) use `endpoint`. * We check the first parameter name via reflection. @@ -405,7 +405,7 @@ private function deriveName(string $packageName): string private function detectFactoryParam(string $factoryClass): string { try { - $method = new \ReflectionMethod($factoryClass, 'create'); + $method = new \ReflectionMethod($factoryClass, 'createProvider'); foreach ($method->getParameters() as $param) { $name = $param->getName(); if ($name === 'endpoint' || $name === 'hostUrl' || $name === 'baseUrl') { diff --git a/Classes/Domain/Repository/ProviderConfigurationRepository.php b/Classes/Domain/Repository/ProviderConfigurationRepository.php index b0b554b..d0f8982 100644 --- a/Classes/Domain/Repository/ProviderConfigurationRepository.php +++ b/Classes/Domain/Repository/ProviderConfigurationRepository.php @@ -12,6 +12,7 @@ namespace B13\Aim\Domain\Repository; +use B13\Aim\Crypto\ApiKeyEncryption; use B13\Aim\Domain\Model\ProviderConfiguration; use B13\Aim\Domain\Model\ProviderConfigurationFactory; use TYPO3\CMS\Core\Database\Connection; @@ -26,6 +27,7 @@ class ProviderConfigurationRepository public function __construct( private readonly ConnectionPool $connectionPool, + private readonly ApiKeyEncryption $encryption, ) {} public function findByUid(int $uid): ?ProviderConfiguration @@ -188,6 +190,9 @@ protected function map(array $rows): array protected function mapSingleRow(array $row): ProviderConfiguration { + if (isset($row['api_key']) && $row['api_key'] !== '') { + $row['api_key'] = $this->encryption->decrypt((string)$row['api_key']); + } return ProviderConfigurationFactory::fromRow($row); } diff --git a/Classes/Exception/ApiKeyEncryptionException.php b/Classes/Exception/ApiKeyEncryptionException.php new file mode 100644 index 0000000..ee99154 --- /dev/null +++ b/Classes/Exception/ApiKeyEncryptionException.php @@ -0,0 +1,18 @@ +encryption->encrypt($value); + } +} diff --git a/Classes/Provider/SymfonyAi/SymfonyAiPlatformAdapter.php b/Classes/Provider/SymfonyAi/SymfonyAiPlatformAdapter.php index 7b05d0c..8a6e041 100644 --- a/Classes/Provider/SymfonyAi/SymfonyAiPlatformAdapter.php +++ b/Classes/Provider/SymfonyAi/SymfonyAiPlatformAdapter.php @@ -37,7 +37,7 @@ use Symfony\AI\Platform\Message\Content\Image; use Symfony\AI\Platform\Message\Message; use Symfony\AI\Platform\Message\MessageBag; -use Symfony\AI\Platform\PlatformInterface; +use Symfony\AI\Platform\ProviderInterface; use Symfony\AI\Platform\TokenUsage\TokenUsageInterface; /** @@ -61,13 +61,13 @@ class SymfonyAiPlatformAdapter implements ToolCallingCapableInterface, EmbeddingCapableInterface { - /** @var array Platforms cached by configuration key */ + /** @var array Providers cached by configuration key */ private array $platforms = []; private readonly string $maxTokensKey; /** - * @param string $factoryClass Fully-qualified class name of the bridge's PlatformFactory + * @param string $factoryClass Fully-qualified class name of the bridge's Factory * @param string $factoryParam Name of the factory parameter to pass the config value to ('apiKey' or 'endpoint') */ public function __construct( @@ -237,16 +237,16 @@ public function processEmbeddingRequest(EmbeddingRequest $request): EmbeddingRes } /** - * Lazily create and cache a Platform instance per provider configuration. + * Lazily create and cache a Provider instance per provider configuration. */ - private function getPlatform(ProviderConfiguration $config): PlatformInterface + private function getPlatform(ProviderConfiguration $config): ProviderInterface { $cacheKey = $config->uid > 0 ? (string)$config->uid : md5($config->apiKey . $config->model); if (!isset($this->platforms[$cacheKey])) { $factoryClass = $this->factoryClass; $this->platforms[$cacheKey] = match ($this->factoryParam) { - 'endpoint' => $factoryClass::create(endpoint: $config->apiKey), - default => $factoryClass::create(apiKey: $config->apiKey), + 'endpoint' => $factoryClass::createProvider(endpoint: $config->apiKey), + default => $factoryClass::createProvider(apiKey: $config->apiKey), }; } return $this->platforms[$cacheKey]; diff --git a/Classes/Updates/EncryptApiKeysUpgrade.php b/Classes/Updates/EncryptApiKeysUpgrade.php new file mode 100644 index 0000000..390a0f8 --- /dev/null +++ b/Classes/Updates/EncryptApiKeysUpgrade.php @@ -0,0 +1,108 @@ +findRowsToEncrypt() !== []; + } + + public function executeUpdate(): bool + { + $connection = $this->connectionPool->getConnectionForTable(self::TABLE); + + foreach ($this->findRowsToEncrypt() as $row) { + $connection->update( + self::TABLE, + ['api_key' => $this->encryption->encrypt((string)$row['api_key'])], + ['uid' => (int)$row['uid']], + ['api_key' => Connection::PARAM_STR], + ); + } + + return true; + } + + /** + * Returns plaintext rows that hold a real API key (not an endpoint URL). + * + * @return list + */ + private function findRowsToEncrypt(): array + { + $qb = $this->connectionPool->getQueryBuilderForTable(self::TABLE); + $qb->getRestrictions()->removeAll(); + + $rows = $qb->select('uid', 'api_key') + ->from(self::TABLE) + ->where( + $qb->expr()->neq('api_key', $qb->createNamedParameter('')), + $qb->expr()->notLike( + 'api_key', + $qb->createNamedParameter(ApiKeyEncryption::PREFIX_ANY . '%'), + ), + $qb->expr()->notLike('api_key', $qb->createNamedParameter('http://%')), + $qb->expr()->notLike('api_key', $qb->createNamedParameter('https://%')), + ) + ->executeQuery() + ->fetchAllAssociative(); + + return array_map( + static fn(array $row): array => ['uid' => (int)$row['uid'], 'api_key' => (string)$row['api_key']], + $rows, + ); + } +} diff --git a/Configuration/Services.yaml b/Configuration/Services.yaml index bf02247..e313a70 100644 --- a/Configuration/Services.yaml +++ b/Configuration/Services.yaml @@ -12,3 +12,12 @@ services: B13\Aim\Domain\Repository\ProviderConfigurationRepository: public: true + + B13\Aim\Crypto\ApiKeyEncryption: + public: true + + B13\Aim\Hooks\EncryptApiKey: + public: true + + B13\Aim\Backend\FormDataProvider\DecryptApiKey: + public: true diff --git a/README.md b/README.md index 52d2a2e..89b5dce 100644 --- a/README.md +++ b/README.md @@ -342,6 +342,21 @@ Install a bridge, flush caches. The provider appears automatically in the backen AiM provides a complete governance system for AI usage, built on native TYPO3 mechanisms. +### API key encryption + +Provider API keys stored in `tx_aim_configuration.api_key` are encrypted using a key derived from `$GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']`. + +| TYPO3 version | Cipher | Implementation | +|---|---|---| +| v14+ | XChaCha20-Poly1305 AEAD | Core `\TYPO3\CMS\Core\Crypto\Cipher\CipherService` | +| v12 / v13 | XSalsa20-Poly1305 secretbox | Local libsodium implementation (CipherService not yet available) | + +Stored values carry a version prefix (`aim:enc:v1:` for the v12/v13 path, `aim:enc:v2:` for the v14 path) so decryption auto-selects the right routine even after an upgrade. Encryption is transparent: a DataHandler hook encrypts on save, a FormDataProvider decrypts for the backend edit form, and the repository decrypts on read. Legacy plaintext rows from earlier AiM versions are migrated via the **"[AiM] Encrypt stored provider API keys"** upgrade wizard in the Install Tool. + +For providers that put an **endpoint URL** in the `api_key` field instead of a real secret (Ollama, LM Studio, self-hosted OpenAI-compatible proxies), AiM detects the `http://` / `https://` prefix and skips encryption — the URL stays plaintext both in the column and in DB exports. + +If `SYS/encryptionKey` is rotated, existing API keys become unreadable and must be re-entered. + ### Provider restrictions Restrict provider configurations to specific backend user groups via the `be_groups` field on each configuration record. Only members of the listed groups (or admins) can use that configuration. diff --git a/Tests/Functional/Crypto/ApiKeyEncryptionPersistenceTest.php b/Tests/Functional/Crypto/ApiKeyEncryptionPersistenceTest.php new file mode 100644 index 0000000..6c23ac7 --- /dev/null +++ b/Tests/Functional/Crypto/ApiKeyEncryptionPersistenceTest.php @@ -0,0 +1,146 @@ +get(ApiKeyEncryption::class); + $uid = $this->insertRow($encryption->encrypt('sk-plaintext-secret')); + + $config = $this->get(ProviderConfigurationRepository::class)->findByUid($uid); + + self::assertNotNull($config); + self::assertSame('sk-plaintext-secret', $config->apiKey); + } + + #[Test] + public function repositoryReturnsLegacyPlaintextUnchanged(): void + { + $uid = $this->insertRow('sk-legacy-plaintext'); + + $config = $this->get(ProviderConfigurationRepository::class)->findByUid($uid); + + self::assertNotNull($config); + self::assertSame('sk-legacy-plaintext', $config->apiKey); + } + + #[Test] + public function upgradeWizardIsRegisteredInTheInstallToolRegistry(): void + { + $registry = $this->get(UpgradeWizardRegistry::class); + self::assertTrue($registry->hasUpgradeWizard('aimEncryptApiKeys')); + self::assertInstanceOf(EncryptApiKeysUpgrade::class, $registry->getUpgradeWizard('aimEncryptApiKeys')); + } + + #[Test] + public function upgradeWizardEncryptsLegacyPlaintextRows(): void + { + $uid = $this->insertRow('sk-legacy-from-old-version'); + + $wizard = $this->get(UpgradeWizardRegistry::class)->getUpgradeWizard('aimEncryptApiKeys'); + self::assertTrue($wizard->updateNecessary()); + self::assertTrue($wizard->executeUpdate()); + self::assertFalse($wizard->updateNecessary()); + + self::assertStringStartsWith(ApiKeyEncryption::PREFIX_ANY, $this->fetchRawApiKey($uid)); + + $config = $this->get(ProviderConfigurationRepository::class)->findByUid($uid); + self::assertNotNull($config); + self::assertSame('sk-legacy-from-old-version', $config->apiKey); + } + + #[Test] + public function upgradeWizardLeavesEndpointUrlsAsPlaintext(): void + { + $uid = $this->insertRow('http://host.docker.internal:11434'); + + $wizard = $this->get(UpgradeWizardRegistry::class)->getUpgradeWizard('aimEncryptApiKeys'); + self::assertFalse($wizard->updateNecessary()); + self::assertTrue($wizard->executeUpdate()); + + self::assertSame('http://host.docker.internal:11434', $this->fetchRawApiKey($uid)); + + $config = $this->get(ProviderConfigurationRepository::class)->findByUid($uid); + self::assertNotNull($config); + self::assertSame('http://host.docker.internal:11434', $config->apiKey); + } + + #[Test] + public function upgradeWizardLeavesAlreadyEncryptedRowsAlone(): void + { + $encryption = $this->get(ApiKeyEncryption::class); + $uid = $this->insertRow($encryption->encrypt('sk-already-encrypted')); + $before = $this->fetchRawApiKey($uid); + + $wizard = $this->get(UpgradeWizardRegistry::class)->getUpgradeWizard('aimEncryptApiKeys'); + self::assertFalse($wizard->updateNecessary()); + self::assertTrue($wizard->executeUpdate()); + + self::assertSame($before, $this->fetchRawApiKey($uid)); + } + + private function insertRow(string $apiKey): int + { + $connection = GeneralUtility::makeInstance(ConnectionPool::class) + ->getConnectionForTable(self::TABLE); + $connection->insert(self::TABLE, [ + 'pid' => 0, + 'ai_provider' => 'test', + 'title' => 'Test', + 'api_key' => $apiKey, + 'model' => 'test-model', + ]); + return (int)$connection->lastInsertId(); + } + + private function fetchRawApiKey(int $uid): string + { + $qb = GeneralUtility::makeInstance(ConnectionPool::class) + ->getQueryBuilderForTable(self::TABLE); + $qb->getRestrictions()->removeAll(); + return (string)$qb->select('api_key') + ->from(self::TABLE) + ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid, Connection::PARAM_INT))) + ->executeQuery() + ->fetchOne(); + } +} diff --git a/Tests/Unit/Crypto/ApiKeyEncryptionTest.php b/Tests/Unit/Crypto/ApiKeyEncryptionTest.php new file mode 100644 index 0000000..2d7f454 --- /dev/null +++ b/Tests/Unit/Crypto/ApiKeyEncryptionTest.php @@ -0,0 +1,177 @@ +originalKey = (string)($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] ?? ''); + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = str_repeat('a', 96); + } + + protected function tearDown(): void + { + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = $this->originalKey; + parent::tearDown(); + } + + #[Test] + public function encryptDecryptRoundTrip(): void + { + $service = new ApiKeyEncryption(); + $plaintext = 'sk-proj-abc123-very-secret'; + + $encrypted = $service->encrypt($plaintext); + + self::assertNotSame($plaintext, $encrypted); + self::assertTrue($service->isEncrypted($encrypted)); + self::assertSame($plaintext, $service->decrypt($encrypted)); + } + + #[Test] + public function encryptPicksV2OnTypo3V14(): void + { + if (!class_exists(CipherService::class)) { + self::markTestSkipped('TYPO3 CipherService not available — only present on v14+.'); + } + $encrypted = (new ApiKeyEncryption())->encrypt('sk-test'); + self::assertStringStartsWith(ApiKeyEncryption::PREFIX_V2, $encrypted); + } + + #[Test] + public function decryptOfV1PayloadStillWorksOnV14(): void + { + // A value persisted by v12/v13 must remain readable after upgrade. + // We build a v1 payload directly so the test is independent of which + // path encrypt() currently picks. + $v1 = $this->makeV1Payload('sk-from-old-typo3'); + self::assertSame('sk-from-old-typo3', (new ApiKeyEncryption())->decrypt($v1)); + } + + #[Test] + public function encryptLeavesEndpointUrlsUntouched(): void + { + $service = new ApiKeyEncryption(); + $endpoint = 'http://host.docker.internal:11434'; + + $result = $service->encrypt($endpoint); + + self::assertSame($endpoint, $result); + self::assertFalse($service->isEncrypted($result)); + } + + #[Test] + public function encryptLeavesHttpsEndpointUrlsUntouched(): void + { + $service = new ApiKeyEncryption(); + $endpoint = 'https://my-self-hosted-llm.example.com:8443'; + + self::assertSame($endpoint, $service->encrypt($endpoint)); + } + + #[Test] + public function encryptIsIdempotent(): void + { + $service = new ApiKeyEncryption(); + $encrypted = $service->encrypt('sk-test'); + + self::assertSame($encrypted, $service->encrypt($encrypted)); + } + + #[Test] + public function encryptOfEmptyStringStaysEmpty(): void + { + $service = new ApiKeyEncryption(); + self::assertSame('', $service->encrypt('')); + } + + #[Test] + public function decryptOfLegacyPlaintextReturnsInputUnchanged(): void + { + $service = new ApiKeyEncryption(); + self::assertSame('sk-legacy-plaintext', $service->decrypt('sk-legacy-plaintext')); + } + + #[Test] + public function encryptUsesFreshNonceEachTime(): void + { + $service = new ApiKeyEncryption(); + $first = $service->encrypt('sk-same-input'); + $second = $service->encrypt('sk-same-input'); + + self::assertNotSame($first, $second); + self::assertSame('sk-same-input', $service->decrypt($first)); + self::assertSame('sk-same-input', $service->decrypt($second)); + } + + #[Test] + public function decryptFailsWhenSystemKeyChanged(): void + { + $service = new ApiKeyEncryption(); + $encrypted = $service->encrypt('sk-test'); + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = str_repeat('b', 96); + + $this->expectException(ApiKeyEncryptionException::class); + $service->decrypt($encrypted); + } + + #[Test] + public function isEncryptedDetectsBothPrefixes(): void + { + $service = new ApiKeyEncryption(); + self::assertFalse($service->isEncrypted('sk-plaintext')); + self::assertTrue($service->isEncrypted(ApiKeyEncryption::PREFIX_V1 . 'whatever')); + self::assertTrue($service->isEncrypted(ApiKeyEncryption::PREFIX_V2 . 'whatever')); + } + + #[Test] + public function encryptFailsWithoutSystemEncryptionKeyOnV1Path(): void + { + if (class_exists(CipherService::class)) { + self::markTestSkipped('On v14 the missing-key error is raised by core, not by us.'); + } + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = ''; + + $this->expectException(ApiKeyEncryptionException::class); + $this->expectExceptionCode(1773874402); + (new ApiKeyEncryption())->encrypt('sk-test'); + } + + /** + * Builds a v1 payload by replicating the libsodium secretbox path, + * so the test does not depend on which encryption path the service + * picks at runtime. + */ + private function makeV1Payload(string $plaintext): string + { + $key = sodium_crypto_generichash( + "aim:apikey:v1\0" . $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'], + '', + SODIUM_CRYPTO_SECRETBOX_KEYBYTES, + ); + $nonce = random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES); + $ciphertext = sodium_crypto_secretbox($plaintext, $nonce, $key); + return ApiKeyEncryption::PREFIX_V1 . base64_encode($nonce . $ciphertext); + } +} diff --git a/Tests/Unit/Hooks/EncryptApiKeyTest.php b/Tests/Unit/Hooks/EncryptApiKeyTest.php new file mode 100644 index 0000000..be7fdf1 --- /dev/null +++ b/Tests/Unit/Hooks/EncryptApiKeyTest.php @@ -0,0 +1,100 @@ +originalKey = (string)($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] ?? ''); + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = str_repeat('a', 96); + } + + protected function tearDown(): void + { + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = $this->originalKey; + parent::tearDown(); + } + + #[Test] + public function encryptsPlaintextApiKeyOnSave(): void + { + $hook = new EncryptApiKey(new ApiKeyEncryption()); + $fieldArray = ['api_key' => 'sk-secret', 'title' => 'My Provider']; + + $hook->processDatamap_postProcessFieldArray('new', 'tx_aim_configuration', 'NEW1', $fieldArray, $this->createDataHandlerMock()); + + self::assertTrue((new ApiKeyEncryption())->isEncrypted($fieldArray['api_key'])); + self::assertSame('My Provider', $fieldArray['title']); + } + + #[Test] + public function leavesAlreadyEncryptedValueUntouched(): void + { + $encryption = new ApiKeyEncryption(); + $hook = new EncryptApiKey($encryption); + $alreadyEncrypted = $encryption->encrypt('sk-secret'); + $fieldArray = ['api_key' => $alreadyEncrypted]; + + $hook->processDatamap_postProcessFieldArray('update', 'tx_aim_configuration', 1, $fieldArray, $this->createDataHandlerMock()); + + self::assertSame($alreadyEncrypted, $fieldArray['api_key']); + } + + #[Test] + public function ignoresOtherTables(): void + { + $hook = new EncryptApiKey(new ApiKeyEncryption()); + $fieldArray = ['api_key' => 'sk-secret']; + + $hook->processDatamap_postProcessFieldArray('new', 'tt_content', 'NEW1', $fieldArray, $this->createDataHandlerMock()); + + self::assertSame('sk-secret', $fieldArray['api_key']); + } + + #[Test] + public function ignoresUpdatesThatDoNotTouchApiKey(): void + { + $hook = new EncryptApiKey(new ApiKeyEncryption()); + $fieldArray = ['title' => 'Renamed']; + + $hook->processDatamap_postProcessFieldArray('update', 'tx_aim_configuration', 1, $fieldArray, $this->createDataHandlerMock()); + + self::assertSame(['title' => 'Renamed'], $fieldArray); + } + + #[Test] + public function ignoresEmptyApiKey(): void + { + $hook = new EncryptApiKey(new ApiKeyEncryption()); + $fieldArray = ['api_key' => '']; + + $hook->processDatamap_postProcessFieldArray('update', 'tx_aim_configuration', 1, $fieldArray, $this->createDataHandlerMock()); + + self::assertSame('', $fieldArray['api_key']); + } + + private function createDataHandlerMock(): DataHandler + { + return $this->createMock(DataHandler::class); + } +} diff --git a/composer.json b/composer.json index f1c0952..04e60f1 100644 --- a/composer.json +++ b/composer.json @@ -5,6 +5,7 @@ "license": "GPL-2.0-or-later", "require": { "php": "^8.1", + "ext-sodium": "*", "typo3/cms-core": "^12.4 || ^13.4 || ^14.0", "typo3/cms-backend": "^12.4 || ^13.4 || ^14.0" }, @@ -12,8 +13,11 @@ "phpunit/phpunit": "^11.0", "typo3/testing-framework": "^9.0" }, + "conflict": { + "symfony/ai-platform": "<0.8" + }, "suggest": { - "symfony/ai-platform": "Required to use Symfony AI bridges as providers (OpenAI, Anthropic, Gemini, Mistral, Ollama, etc.)", + "symfony/ai-platform": "^0.8 — required to use Symfony AI bridges as providers (OpenAI, Anthropic, Gemini, Mistral, Ollama, etc.)", "typo3/cms-dashboard": "Enables dashboard widgets for AI usage analytics" }, "autoload": { diff --git a/ext_localconf.php b/ext_localconf.php index b83b7ca..9f0037d 100644 --- a/ext_localconf.php +++ b/ext_localconf.php @@ -2,9 +2,16 @@ declare(strict_types=1); +use B13\Aim\Backend\FormDataProvider\DecryptApiKey; use B13\Aim\Hooks\DefaultProviderHook; +use B13\Aim\Hooks\EncryptApiKey; $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['processDatamapClass'][DefaultProviderHook::class] = DefaultProviderHook::class; +$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['processDatamapClass'][EncryptApiKey::class] = EncryptApiKey::class; + +$GLOBALS['TYPO3_CONF_VARS']['SYS']['formEngine']['formDataGroup']['tcaDatabaseRecord'][DecryptApiKey::class] = [ + 'depends' => [\TYPO3\CMS\Backend\Form\FormDataProvider\DatabaseEditRow::class], +]; // Register AI capability permissions for backend user groups $GLOBALS['TYPO3_CONF_VARS']['BE']['customPermOptions']['aim'] = [ diff --git a/ext_tables.sql b/ext_tables.sql index 027d60b..7167b24 100644 --- a/ext_tables.sql +++ b/ext_tables.sql @@ -3,7 +3,7 @@ CREATE TABLE tx_aim_configuration ( title varchar(255) DEFAULT '' NOT NULL, description text, `default` tinyint(4) unsigned DEFAULT '0' NOT NULL, - api_key varchar(255) DEFAULT '' NOT NULL, + api_key text, model varchar(255) DEFAULT '' NOT NULL, total_cost double(10,6) DEFAULT '0.000000' NOT NULL, cost_currency varchar(10) DEFAULT 'USD' NOT NULL, From ad79cece768d5da066cfdcd981fa3f705994cc33 Mon Sep 17 00:00:00 2001 From: Oliver Bartsch Date: Tue, 12 May 2026 10:49:54 +0200 Subject: [PATCH 2/2] [TASK] Add command to rotate api keys --- Classes/Command/RotateApiKeys.php | 194 +++++++++++++++++ Classes/Crypto/ApiKeyEncryption.php | 25 +++ README.md | 10 +- .../Functional/Command/RotateApiKeysTest.php | 197 ++++++++++++++++++ 4 files changed, 425 insertions(+), 1 deletion(-) create mode 100644 Classes/Command/RotateApiKeys.php create mode 100644 Tests/Functional/Command/RotateApiKeysTest.php diff --git a/Classes/Command/RotateApiKeys.php b/Classes/Command/RotateApiKeys.php new file mode 100644 index 0000000..9c00f65 --- /dev/null +++ b/Classes/Command/RotateApiKeys.php @@ -0,0 +1,194 @@ +setHelp( + 'When $TYPO3_CONF_VARS[SYS][encryptionKey] has been rotated, existing encrypted ' + . 'API keys can no longer be read. This command takes the previous value of the ' + . 'system key, decrypts each stored API key with it, and re-encrypts using the ' + . 'current system key.' + ) + ->addOption( + 'old-key', + null, + InputOption::VALUE_REQUIRED, + 'The previous value of $TYPO3_CONF_VARS[SYS][encryptionKey].', + ) + ->addOption( + 'dry-run', + null, + InputOption::VALUE_NONE, + 'Report what would change without writing.', + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $oldKey = (string)$input->getOption('old-key'); + if ($oldKey === '') { + $output->writeln('--old-key is required.'); + return Command::FAILURE; + } + $dryRun = (bool)$input->getOption('dry-run'); + + [$toRotate, $unrecoverable, $alreadyCurrent, $unencrypted] = $this->classifyRows($oldKey); + + if ($unrecoverable !== []) { + $output->writeln('The following rows cannot be decrypted with the supplied old key:'); + foreach ($unrecoverable as $uid => $message) { + $output->writeln(sprintf(' - uid=%d: %s', $uid, $message)); + } + $output->writeln('Aborting without writes. Verify the --old-key value.'); + return Command::FAILURE; + } + + if ($toRotate === []) { + $output->writeln('Nothing to rotate.'); + $this->writeSkipBreakdown($output, $alreadyCurrent, $unencrypted); + return Command::SUCCESS; + } + + if ($dryRun) { + $output->writeln(sprintf( + '[dry-run] Would re-encrypt %d row(s).', + count($toRotate), + )); + $this->writeSkipBreakdown($output, $alreadyCurrent, $unencrypted); + return Command::SUCCESS; + } + + $connection = $this->connectionPool->getConnectionForTable(self::TABLE); + foreach ($toRotate as $uid => $plaintext) { + $connection->update( + self::TABLE, + ['api_key' => $this->encryption->encrypt($plaintext)], + ['uid' => $uid], + ['api_key' => Connection::PARAM_STR], + ); + } + + $output->writeln(sprintf( + 'Re-encrypted %d API key(s).', + count($toRotate), + )); + $this->writeSkipBreakdown($output, $alreadyCurrent, $unencrypted); + return Command::SUCCESS; + } + + private function writeSkipBreakdown(OutputInterface $output, int $alreadyCurrent, int $unencrypted): void + { + if ($alreadyCurrent > 0) { + $output->writeln(sprintf( + ' - %d row(s) already use the current system key', + $alreadyCurrent, + )); + } + if ($unencrypted > 0) { + $output->writeln(sprintf( + ' - %d row(s) are not encrypted (endpoint URLs or empty values)', + $unencrypted, + )); + } + } + + /** + * @return array{0: array, 1: array, 2: int, 3: int} + * [uid => plaintext to re-encrypt], [uid => failure message], already-current count, unencrypted count + */ + private function classifyRows(string $oldKey): array + { + $toRotate = []; + $unrecoverable = []; + $alreadyCurrent = 0; + $unencrypted = 0; + + foreach ($this->fetchAllRows() as $row) { + $uid = (int)$row['uid']; + $value = (string)$row['api_key']; + + if ($value === '' || $this->encryption->isEndpointUrl($value) || !$this->encryption->isEncrypted($value)) { + $unencrypted++; + continue; + } + + try { + $this->encryption->decrypt($value); + $alreadyCurrent++; + continue; + } catch (ApiKeyEncryptionException) { + // Fall through to old-key attempt + } + + try { + $toRotate[$uid] = $this->encryption->decryptWithSystemKey($value, $oldKey); + } catch (ApiKeyEncryptionException $e) { + $unrecoverable[$uid] = $e->getMessage(); + } + } + + return [$toRotate, $unrecoverable, $alreadyCurrent, $unencrypted]; + } + + /** + * @return list + */ + private function fetchAllRows(): array + { + $qb = $this->connectionPool->getQueryBuilderForTable(self::TABLE); + $qb->getRestrictions()->removeAll(); + return $qb->select('uid', 'api_key') + ->from(self::TABLE) + ->executeQuery() + ->fetchAllAssociative(); + } +} diff --git a/Classes/Crypto/ApiKeyEncryption.php b/Classes/Crypto/ApiKeyEncryption.php index 800b26f..b64fe66 100644 --- a/Classes/Crypto/ApiKeyEncryption.php +++ b/Classes/Crypto/ApiKeyEncryption.php @@ -80,6 +80,31 @@ public function decrypt(string $value): string return $value; } + /** + * Decrypts a value using a system encryption key other than the one + * currently in $TYPO3_CONF_VARS — needed by the rotation command after + * SYS/encryptionKey has changed but stored ciphertexts still use the old + * derivation. + * + * Both decryption paths (v1 secretbox and v2 CipherService) read the + * system key from globals, so we swap it temporarily and restore it via + * try/finally. The swap is local to this call. + */ + public function decryptWithSystemKey(string $value, string $systemKey): string + { + if ($value === '' || !$this->isEncrypted($value)) { + return $value; + } + + $previous = $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] ?? null; + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = $systemKey; + try { + return $this->decrypt($value); + } finally { + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = $previous; + } + } + public function isEncrypted(string $value): bool { return str_starts_with($value, self::PREFIX_ANY); diff --git a/README.md b/README.md index 89b5dce..a14f9d5 100644 --- a/README.md +++ b/README.md @@ -355,7 +355,15 @@ Stored values carry a version prefix (`aim:enc:v1:` for the v12/v13 path, `aim:e For providers that put an **endpoint URL** in the `api_key` field instead of a real secret (Ollama, LM Studio, self-hosted OpenAI-compatible proxies), AiM detects the `http://` / `https://` prefix and skips encryption — the URL stays plaintext both in the column and in DB exports. -If `SYS/encryptionKey` is rotated, existing API keys become unreadable and must be re-entered. +If `SYS/encryptionKey` is rotated, existing API keys can no longer be decrypted with the new key. Run the rotation command *before* the rotation takes effect, or right after with the old value still in hand: + +```bash +vendor/bin/typo3 aim:rotateApiKeys --old-key='' +``` + +The command decrypts each stored key with the supplied old value, re-encrypts with the current one, and reports the result. It is idempotent (re-running with the same old key is a no-op) and aborts without writes if any row cannot be decrypted with the supplied value. Add `--dry-run` to preview. + +Without the previous key value, encrypted API keys cannot be recovered. This is by design. Save the old `SYS/encryptionKey` somewhere safe before rotating. ### Provider restrictions diff --git a/Tests/Functional/Command/RotateApiKeysTest.php b/Tests/Functional/Command/RotateApiKeysTest.php new file mode 100644 index 0000000..a5f51d3 --- /dev/null +++ b/Tests/Functional/Command/RotateApiKeysTest.php @@ -0,0 +1,197 @@ +get(CommandRegistry::class); + self::assertTrue($registry->has('aim:rotateApiKeys')); + self::assertInstanceOf(RotateApiKeys::class, $registry->get('aim:rotateApiKeys')); + } + + #[Test] + public function rotatesEncryptedRowsFromOldKeyToCurrent(): void + { + $encryption = $this->get(ApiKeyEncryption::class); + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = self::OLD_KEY; + $uid = $this->insertRow($encryption->encrypt('sk-real-secret')); + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = self::NEW_KEY; + // The old ciphertext must NOT decrypt with the new key + $this->assertThrows(fn() => $encryption->decrypt($this->fetchRawApiKey($uid))); + + $exit = $this->runCommand(['--old-key' => self::OLD_KEY]); + self::assertSame(0, $exit); + + // After rotation, the value decrypts with the current key + self::assertSame('sk-real-secret', $encryption->decrypt($this->fetchRawApiKey($uid))); + } + + #[Test] + public function isIdempotentOnSecondRun(): void + { + $encryption = $this->get(ApiKeyEncryption::class); + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = self::OLD_KEY; + $uid = $this->insertRow($encryption->encrypt('sk-real-secret')); + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = self::NEW_KEY; + + self::assertSame(0, $this->runCommand(['--old-key' => self::OLD_KEY])); + $afterFirst = $this->fetchRawApiKey($uid); + + $tester = $this->runCommandAndReturnTester(['--old-key' => self::OLD_KEY]); + self::assertSame(0, $tester->getStatusCode()); + $display = $tester->getDisplay(); + self::assertStringContainsString('Nothing to rotate', $display); + self::assertStringContainsString('1 row(s) already use the current system key', $display); + + // Same ciphertext (no double re-encryption) + self::assertSame($afterFirst, $this->fetchRawApiKey($uid)); + self::assertSame('sk-real-secret', $encryption->decrypt($afterFirst)); + } + + #[Test] + public function endpointUrlsAreSkippedAndReportedDistinctly(): void + { + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = self::NEW_KEY; + $uid = $this->insertRow('http://host.docker.internal:11434'); + + $tester = $this->runCommandAndReturnTester(['--old-key' => self::OLD_KEY]); + self::assertSame(0, $tester->getStatusCode()); + + $display = $tester->getDisplay(); + self::assertStringContainsString('Nothing to rotate', $display); + self::assertStringContainsString('1 row(s) are not encrypted', $display); + self::assertStringNotContainsString('already use the current system key', $display); + self::assertSame('http://host.docker.internal:11434', $this->fetchRawApiKey($uid)); + } + + #[Test] + public function abortsAndReportsWhenOldKeyIsWrong(): void + { + $encryption = $this->get(ApiKeyEncryption::class); + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = self::OLD_KEY; + $uid = $this->insertRow($encryption->encrypt('sk-real-secret')); + $beforeRotation = $this->fetchRawApiKey($uid); + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = self::NEW_KEY; + + $tester = $this->runCommandAndReturnTester(['--old-key' => 'definitely-not-the-old-key']); + self::assertSame(1, $tester->getStatusCode()); + self::assertStringContainsString('cannot be decrypted', $tester->getDisplay()); + self::assertStringContainsString('Aborting without writes', $tester->getDisplay()); + + // Row was not touched + self::assertSame($beforeRotation, $this->fetchRawApiKey($uid)); + } + + #[Test] + public function failsWithoutOldKey(): void + { + $tester = $this->runCommandAndReturnTester([]); + self::assertSame(1, $tester->getStatusCode()); + self::assertStringContainsString('--old-key is required', $tester->getDisplay()); + } + + #[Test] + public function dryRunDoesNotWrite(): void + { + $encryption = $this->get(ApiKeyEncryption::class); + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = self::OLD_KEY; + $uid = $this->insertRow($encryption->encrypt('sk-real-secret')); + $beforeRotation = $this->fetchRawApiKey($uid); + + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = self::NEW_KEY; + + $tester = $this->runCommandAndReturnTester([ + '--old-key' => self::OLD_KEY, + '--dry-run' => true, + ]); + self::assertSame(0, $tester->getStatusCode()); + self::assertStringContainsString('[dry-run]', $tester->getDisplay()); + self::assertSame($beforeRotation, $this->fetchRawApiKey($uid)); + } + + private function runCommand(array $options): int + { + return $this->runCommandAndReturnTester($options)->getStatusCode(); + } + + private function runCommandAndReturnTester(array $options): CommandTester + { + $tester = new CommandTester($this->get(RotateApiKeys::class)); + $tester->execute($options); + return $tester; + } + + private function insertRow(string $apiKey): int + { + $connection = GeneralUtility::makeInstance(ConnectionPool::class) + ->getConnectionForTable(self::TABLE); + $connection->insert(self::TABLE, [ + 'pid' => 0, + 'ai_provider' => 'test', + 'title' => 'Test', + 'api_key' => $apiKey, + 'model' => 'test-model', + ]); + return (int)$connection->lastInsertId(); + } + + private function fetchRawApiKey(int $uid): string + { + $qb = GeneralUtility::makeInstance(ConnectionPool::class) + ->getQueryBuilderForTable(self::TABLE); + $qb->getRestrictions()->removeAll(); + return (string)$qb->select('api_key') + ->from(self::TABLE) + ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid, Connection::PARAM_INT))) + ->executeQuery() + ->fetchOne(); + } + + private function assertThrows(callable $callable): void + { + try { + $callable(); + } catch (\Throwable) { + return; + } + self::fail('Expected an exception but none was thrown.'); + } +}