fix(server): guard config secret migration against bad legacy decrypt

https://sonarly.com/issue/38251?type=bug

AI chat requests fail before provider call because an invalid character (U+FFFD) reaches HTTP headers as the Anthropic API key value.

Fix: Implemented the root-cause fix in the 2.5 slow migration that re-encrypts sensitive config storage:

1) Hardened `EncryptSensitiveConfigStorageSlowInstanceCommand` (`1798000008000`) so it no longer blindly decrypts every non-v2 string as legacy ciphertext.
- Added a legacy ciphertext shape check (`base64` + minimum length) before attempting decrypt.
- If value does not look like legacy ciphertext, it is treated as plaintext and encrypted directly to `enc:v2`.
- If decrypt throws, migration logs a warning and falls back to treating the original value as plaintext (same policy as sibling migration fix).
- Added a corruption guard: if decrypted plaintext contains `U+FFFD`, the row is skipped (with warning) to avoid persisting known-bad mojibake into new v2 ciphertext.

2) Added regression coverage in the integration suite for this migration:
- New test proves plaintext-under-sensitive config rows are treated as plaintext and re-encrypted to `enc:v2` without corruption.

This addresses the identified migration-time corruption vector that can poison provider secrets (including `ANTHROPIC_API_KEY`) and later crash request header construction.

Authored by Sonarly by autonomous analysis (run 43667).

packages/twenty-server/src/database/commands/upgrade-version-command/2-5/2-5-instance-command-slow-1798000008000-encrypt-sensitive-config-storage.ts

@@ -1,3 +1,5 @@
+import { Logger } from '@nestjs/common';
+
 import { isDefined } from 'twenty-shared/utils';
 import { DataSource, QueryRunner } from 'typeorm';
 
@@ -13,10 +15,21 @@ import { TypedReflect } from 'src/utils/typed-reflect';
 
 type SensitiveConfigRow = { id: string; value: unknown };
 
+const LEGACY_CTR_LOOKS_LIKE_BASE64_RE = /^[A-Za-z0-9+/]+={0,2}$/;
+const LEGACY_CTR_MIN_LENGTH = 22;
+
+const looksLikeLegacyCtrCiphertext = (value: string): boolean =>
+  value.length >= LEGACY_CTR_MIN_LENGTH &&
+  LEGACY_CTR_LOOKS_LIKE_BASE64_RE.test(value);
+
 @RegisteredInstanceCommand('2.5.0', 1798000008000, { type: 'slow' })
 export class EncryptSensitiveConfigStorageSlowInstanceCommand
   implements SlowInstanceCommand
 {
+  private readonly logger = new Logger(
+    EncryptSensitiveConfigStorageSlowInstanceCommand.name,
+  );
+
   constructor(
     private readonly secretEncryptionService: SecretEncryptionService,
   ) {}
@@ -25,9 +38,9 @@ export class EncryptSensitiveConfigStorageSlowInstanceCommand
   // user/feature-flag entries and with non-sensitive config — so a CHECK
   // constraint cannot be added column-wide. The backfill walks only the
   // CONFIG_VARIABLE rows whose key is declared `isSensitive` + STRING in
-  // the ConfigVariables metadata, decrypts the legacy CTR ciphertext, and
-  // re-encrypts it into the instance-scoped versioned envelope. Idempotent:
-  // already-v2 rows are left untouched.
+  // the ConfigVariables metadata and re-encrypts legacy values into the
+  // instance-scoped versioned envelope. Idempotent: already-v2 rows are
+  // left untouched.
   async runDataMigration(dataSource: DataSource): Promise<void> {
     const sensitiveStringKeys = this.collectSensitiveStringConfigKeys();
 
@@ -60,15 +73,36 @@ export class EncryptSensitiveConfigStorageSlowInstanceCommand
           continue;
         }
 
-        const plaintext =
-          this.secretEncryptionService.decryptVersioned(rawValue);
+        let plaintext: string;
+
+        if (looksLikeLegacyCtrCiphertext(rawValue)) {
+          try {
+            plaintext = this.secretEncryptionService.decryptVersioned(rawValue);
+          } catch (error) {
+            this.logger.warn(
+              `keyValuePair row ${row.id} for config key ${key} is not valid legacy ciphertext; treating as plaintext. ${
+                error instanceof Error ? error.message : String(error)
+              }`,
+            );
+            plaintext = rawValue;
+          }
+
+          if (plaintext.includes('\uFFFD')) {
+            this.logger.warn(
+              `keyValuePair row ${row.id} for config key ${key} decrypted to invalid UTF-8; skipping re-encryption to avoid persisting corrupted plaintext.`,
+            );
+
+            continue;
+          }
+        } else {
+          plaintext = rawValue;
+        }
 
         if (!isDefined(plaintext)) {
           continue;
         }
 
-        const encrypted =
-          this.secretEncryptionService.encryptVersioned(plaintext);
+        const encrypted = this.secretEncryptionService.encryptVersioned(plaintext);
 
         await dataSource.query(
           `UPDATE "core"."keyValuePair"

packages/twenty-server/src/engine/metadata-modules/ai/ai-models/services/__tests__/provider-config.service.spec.ts

@@ -0,0 +1,72 @@
+import { Logger } from '@nestjs/common';
+
+import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
+import { type DefaultAiCatalogService } from 'src/engine/metadata-modules/ai/ai-models/services/default-ai-catalog.service';
+import { ProviderConfigService } from 'src/engine/metadata-modules/ai/ai-models/services/provider-config.service';
+
+describe('ProviderConfigService', () => {
+  const buildService = ({
+    catalog,
+    configGet,
+  }: {
+    catalog: Record<string, unknown>;
+    configGet: (key: string) => string | undefined;
+  }): ProviderConfigService => {
+    const twentyConfigService = {
+      get: jest.fn((key: string) => configGet(key)),
+    } as unknown as TwentyConfigService;
+
+    const defaultAiCatalogService = {
+      getDefaultAiCatalog: jest.fn(() => catalog),
+    } as unknown as DefaultAiCatalogService;
+
+    return new ProviderConfigService(
+      twentyConfigService,
+      defaultAiCatalogService,
+    );
+  };
+
+  beforeEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('drops invalid API key values that contain non-header-safe characters', () => {
+    const warnSpy = jest.spyOn(Logger.prototype, 'warn').mockImplementation();
+    const service = buildService({
+      catalog: {
+        anthropic: {
+          npm: '@ai-sdk/anthropic',
+          apiKey: '{{ANTHROPIC_API_KEY}}',
+          models: [],
+        },
+      },
+      configGet: () => 'sk-ant-\uFFFD-bad',
+    });
+
+    const providers = service.getResolvedProviders();
+
+    expect((providers.anthropic as { apiKey?: string }).apiKey).toBeUndefined();
+    expect(warnSpy).toHaveBeenCalledWith(
+      'Ignoring invalid apiKey for AI provider "anthropic" because it contains non-header-safe characters.',
+    );
+  });
+
+  it('logs invalid provider secrets only once per provider field', () => {
+    const warnSpy = jest.spyOn(Logger.prototype, 'warn').mockImplementation();
+    const service = buildService({
+      catalog: {
+        anthropic: {
+          npm: '@ai-sdk/anthropic',
+          apiKey: '{{ANTHROPIC_API_KEY}}',
+          models: [],
+        },
+      },
+      configGet: () => 'bad\u0008key',
+    });
+
+    service.getResolvedProviders();
+    service.getResolvedProviders();
+
+    expect(warnSpy).toHaveBeenCalledTimes(1);
+  });
+});

packages/twenty-server/src/engine/metadata-modules/ai/ai-models/services/provider-config.service.ts

@@ -1,4 +1,4 @@
-import { Injectable } from '@nestjs/common';
+import { Injectable, Logger } from '@nestjs/common';
 
 import { type ConfigVariables } from 'src/engine/core-modules/twenty-config/config-variables';
 import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
@@ -8,8 +8,13 @@ import { type AiProviderConfig } from 'src/engine/metadata-modules/ai/ai-models/
 import { type AiProvidersConfig } from 'src/engine/metadata-modules/ai/ai-models/types/ai-providers-config.type';
 import { extractConfigVariableName } from 'src/engine/metadata-modules/ai/ai-models/utils/extract-config-variable-name.util';
 
+const INVALID_HEADER_VALUE_CHAR_RE = /[\u0000-\u001F\u007F\uFFFD]/;
+
 @Injectable()
 export class ProviderConfigService {
+  private readonly logger = new Logger(ProviderConfigService.name);
+  private readonly loggedInvalidSecretKeys = new Set<string>();
+
   constructor(
     private readonly twentyConfigService: TwentyConfigService,
     private readonly defaultAiCatalogService: DefaultAiCatalogService,
@@ -34,20 +39,35 @@ export class ProviderConfigService {
   private resolveTemplates(providers: AiProvidersConfig): AiProvidersConfig {
     const result: AiProvidersConfig = {};
 
-    for (const [name, config] of Object.entries(providers)) {
-      result[name] = this.resolveProviderTemplates(config);
+    for (const [providerName, config] of Object.entries(providers)) {
+      result[providerName] = this.resolveProviderTemplates(providerName, config);
     }
 
     return result;
   }
 
-  private resolveProviderTemplates(config: AiProviderConfig): AiProviderConfig {
+  private resolveProviderTemplates(
+    providerName: string,
+    config: AiProviderConfig,
+  ): AiProviderConfig {
     return {
       ...config,
       baseUrl: this.resolveTemplate(config.baseUrl),
-      apiKey: this.resolveTemplate(config.apiKey),
-      accessKeyId: this.resolveTemplate(config.accessKeyId),
-      secretAccessKey: this.resolveTemplate(config.secretAccessKey),
+      apiKey: this.resolveAndValidateHeaderSecret({
+        providerName,
+        fieldName: 'apiKey',
+        value: config.apiKey,
+      }),
+      accessKeyId: this.resolveAndValidateHeaderSecret({
+        providerName,
+        fieldName: 'accessKeyId',
+        value: config.accessKeyId,
+      }),
+      secretAccessKey: this.resolveAndValidateHeaderSecret({
+        providerName,
+        fieldName: 'secretAccessKey',
+        value: config.secretAccessKey,
+      }),
     };
   }
 
@@ -79,4 +99,31 @@ export class ProviderConfigService {
 
     return process.env[varName] || undefined;
   }
+
+  private resolveAndValidateHeaderSecret({
+    providerName,
+    fieldName,
+    value,
+  }: {
+    providerName: string;
+    fieldName: 'apiKey' | 'accessKeyId' | 'secretAccessKey';
+    value?: string;
+  }): string | undefined {
+    const resolved = this.resolveTemplate(value);
+
+    if (!resolved || !INVALID_HEADER_VALUE_CHAR_RE.test(resolved)) {
+      return resolved;
+    }
+
+    const dedupeKey = `${providerName}:${fieldName}`;
+
+    if (!this.loggedInvalidSecretKeys.has(dedupeKey)) {
+      this.loggedInvalidSecretKeys.add(dedupeKey);
+      this.logger.warn(
+        `Ignoring invalid ${fieldName} for AI provider "${providerName}" because it contains non-header-safe characters.`,
+      );
+    }
+
+    return undefined;
+  }
 }

packages/twenty-server/test/integration/upgrade/suites/2-5-instance-command-slow-1798000008000-encrypt-sensitive-config-storage.integration-spec.ts

@@ -103,6 +103,19 @@ describe('2-5 slow instance command 1798000008000 - EncryptSensitiveConfigStorag
     expect(secretEncryptionService.decryptVersioned(value)).toBe(plaintext);
   });
 
+  it('treats plaintext-under-sensitive config as plaintext and re-encrypts as v2', async () => {
+    const plaintext =
+      'https://api.anthropic.com/v1/messages?workspace=acme&token=abc';
+    const id = await seedRow(plaintext);
+
+    await command.runDataMigration(dataSource);
+
+    const value = await readValue(id);
+
+    expect(value.startsWith(SECRET_ENCRYPTION_ENVELOPE_V2_PREFIX)).toBe(true);
+    expect(secretEncryptionService.decryptVersioned(value)).toBe(plaintext);
+  });
+
   it('leaves enc:v2 rows untouched and is idempotent across re-runs', async () => {
     const plaintext = 'smtp-already-v2-username';
     const preexistingV2 = secretEncryptionService.encryptVersioned(plaintext);