EncryptedString PlaintextString branded string types (#21001)
## Summary closes https://github.com/twentyhq/core-team-issues/issues/2464 Introduces compile-time branded types to distinguish encrypted ciphertext from plaintext strings, preventing mix-ups like the one fixed in #20819 — but at the type level rather in addition to the one existing at runtime. ### Branded string primitives - Created `EncryptedString` and `PlaintextString` as hard nominal brands using `z.string().brand(...)`, making them non-assignable to each other or to raw `string` - Created `isEncryptedString` type predicate to narrow `string` to `EncryptedString` based on the `enc:v2:` envelope prefix - Retyped `SecretEncryptionService`: `encryptVersioned` accepts `PlaintextString`, `decryptVersioned` returns `PlaintextString` ### Entity typing - Typed encrypted columns across entities: `SigningKeyEntity.privateKey`, `TwoFactorAuthenticationMethodEntity.secret`, `ApplicationRegistrationVariableEntity.encryptedValue`, `ApplicationVariableEntity.value` - Parameterized JSONB types for connected account connection parameters (`ImapSmtpCaldavParams<Pwd>`) with reusable aliases `EncryptedImapSmtpCaldavParams` / `DecryptedImapSmtpCaldavParams` - Typed DTOs (`CreateApplicationRegistrationVariableInput`, `UpdateApplicationRegistrationVariablePayload`, `UpdateApplicationVariableEntityInput`) with `PlaintextString` ### ApplicationVariable always-encrypt uniformization - Retyped `ApplicationVariableEntity.value` to `EncryptedString | ''` — all values are now encrypted regardless of `isSecret` - Updated `ApplicationVariableEntityService` to always encrypt on write and always decrypt on read - Simplified `UpdateApplicationVariableActionHandlerService` by removing conditional encrypt/decrypt-on-isSecret-toggle logic - Added slow instance command (`2.9.0`) to backfill-encrypt existing `isSecret=false` plaintext rows and tighten the `CHECK` constraint ### ConfigStorageService refactor - Split `convertAndSecureValue` (which used `any`) into two well-typed methods: `convertAndDecrypt` and `convertAndEncrypt` - Introduced `isSensitiveStringValue` type predicate to narrow values before encryption/decryption ### What's next - Typeorm entity derivation to strictly type sitemap configuration as code + handler logic for encryption rotation - https://github.com/twentyhq/core-team-issues/issues/2465
This commit is contained in:
+8
-6
@@ -173,8 +173,7 @@ describe('ConfigStorageService', () => {
|
||||
|
||||
it('should decrypt sensitive string values', async () => {
|
||||
const key = 'SENSITIVE_CONFIG' as keyof ConfigVariables;
|
||||
const originalValue = 'sensitive-value';
|
||||
const encryptedValue = 'sensitive-value';
|
||||
const encryptedValue = 'enc:v2:deadbeef:sensitive-value';
|
||||
|
||||
const mockRecord = createMockKeyValuePair(key as string, encryptedValue);
|
||||
|
||||
@@ -197,7 +196,7 @@ describe('ConfigStorageService', () => {
|
||||
|
||||
const result = await service.get(key);
|
||||
|
||||
expect(result).toBe(originalValue);
|
||||
expect(result).toBe(encryptedValue);
|
||||
expect(secretEncryptionService.decryptVersioned).toHaveBeenCalledWith(
|
||||
encryptedValue,
|
||||
);
|
||||
@@ -538,7 +537,10 @@ describe('ConfigStorageService', () => {
|
||||
|
||||
it('should decrypt sensitive string values in loadAll', async () => {
|
||||
const configVars: KeyValuePairEntity[] = [
|
||||
createMockKeyValuePair('SENSITIVE_CONFIG', 'sensitive-value'),
|
||||
createMockKeyValuePair(
|
||||
'SENSITIVE_CONFIG',
|
||||
'enc:v2:deadbeef:sensitive-value',
|
||||
),
|
||||
createMockKeyValuePair('NORMAL_CONFIG', 'normal-value'),
|
||||
];
|
||||
|
||||
@@ -565,13 +567,13 @@ describe('ConfigStorageService', () => {
|
||||
|
||||
expect(result.size).toBe(2);
|
||||
expect(result.get('SENSITIVE_CONFIG' as keyof ConfigVariables)).toBe(
|
||||
'sensitive-value',
|
||||
'enc:v2:deadbeef:sensitive-value',
|
||||
);
|
||||
expect(result.get('NORMAL_CONFIG' as keyof ConfigVariables)).toBe(
|
||||
'normal-value',
|
||||
);
|
||||
expect(secretEncryptionService.decryptVersioned).toHaveBeenCalledWith(
|
||||
'sensitive-value',
|
||||
'enc:v2:deadbeef:sensitive-value',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
+58
-27
@@ -7,6 +7,8 @@ import {
|
||||
KeyValuePairEntity,
|
||||
KeyValuePairType,
|
||||
} from 'src/engine/core-modules/key-value-pair/key-value-pair.entity';
|
||||
import { isEncryptedString } from 'src/engine/core-modules/secret-encryption/branded-strings/is-encrypted-string.util';
|
||||
import { type PlaintextString } from 'src/engine/core-modules/secret-encryption/branded-strings/plaintext-string.type';
|
||||
import { SecretEncryptionService } from 'src/engine/core-modules/secret-encryption/secret-encryption.service';
|
||||
import { ConfigVariables } from 'src/engine/core-modules/twenty-config/config-variables';
|
||||
import { ConfigValueConverterService } from 'src/engine/core-modules/twenty-config/conversion/config-value-converter.service';
|
||||
@@ -47,31 +49,64 @@ export class ConfigStorageService implements ConfigStorageInterface {
|
||||
];
|
||||
}
|
||||
|
||||
private async convertAndSecureValue<T extends keyof ConfigVariables>(
|
||||
// oxlint-disable-next-line @typescripttypescript/no-explicit-any
|
||||
value: any,
|
||||
private isSensitiveStringValue(
|
||||
value: unknown,
|
||||
key: keyof ConfigVariables,
|
||||
): value is string {
|
||||
const metadata = this.getConfigMetadata(key);
|
||||
|
||||
return (
|
||||
typeof value === 'string' &&
|
||||
metadata?.isSensitive === true &&
|
||||
metadata.type === ConfigVariableType.STRING
|
||||
);
|
||||
}
|
||||
|
||||
private async convertAndDecrypt<T extends keyof ConfigVariables>(
|
||||
dbValue: unknown,
|
||||
key: T,
|
||||
isDecrypt = false,
|
||||
// oxlint-disable-next-line @typescripttypescript/no-explicit-any
|
||||
): Promise<any> {
|
||||
): Promise<ConfigVariables[T] | undefined> {
|
||||
try {
|
||||
const convertedValue = isDecrypt
|
||||
? this.configValueConverter.convertDbValueToAppValue(value, key)
|
||||
: this.configValueConverter.convertAppValueToDbValue(value, key);
|
||||
const convertedValue = this.configValueConverter.convertDbValueToAppValue(
|
||||
dbValue,
|
||||
key,
|
||||
);
|
||||
|
||||
const metadata = this.getConfigMetadata(key);
|
||||
const isSensitiveString =
|
||||
metadata?.isSensitive &&
|
||||
metadata.type === ConfigVariableType.STRING &&
|
||||
typeof convertedValue === 'string';
|
||||
|
||||
if (!isSensitiveString) {
|
||||
return convertedValue;
|
||||
if (
|
||||
this.isSensitiveStringValue(convertedValue, key) &&
|
||||
isEncryptedString(convertedValue)
|
||||
) {
|
||||
return this.secretEncryptionService.decryptVersioned(
|
||||
convertedValue,
|
||||
) as unknown as ConfigVariables[T];
|
||||
}
|
||||
|
||||
return isDecrypt
|
||||
? this.secretEncryptionService.decryptVersioned(convertedValue)
|
||||
: this.secretEncryptionService.encryptVersioned(convertedValue);
|
||||
return convertedValue;
|
||||
} catch (error) {
|
||||
throw new ConfigVariableException(
|
||||
`Failed to convert value for key ${key as string}: ${error.message}`,
|
||||
ConfigVariableExceptionCode.VALIDATION_FAILED,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private async convertAndEncrypt<T extends keyof ConfigVariables>(
|
||||
appValue: ConfigVariables[T],
|
||||
key: T,
|
||||
): Promise<KeyValuePairEntity['value']> {
|
||||
try {
|
||||
const convertedValue = this.configValueConverter.convertAppValueToDbValue(
|
||||
appValue,
|
||||
key,
|
||||
);
|
||||
|
||||
if (this.isSensitiveStringValue(convertedValue, key)) {
|
||||
return this.secretEncryptionService.encryptVersioned(
|
||||
convertedValue as PlaintextString,
|
||||
) as unknown as KeyValuePairEntity['value'];
|
||||
}
|
||||
|
||||
return convertedValue as KeyValuePairEntity['value'];
|
||||
} catch (error) {
|
||||
throw new ConfigVariableException(
|
||||
`Failed to convert value for key ${key as string}: ${error.message}`,
|
||||
@@ -96,7 +131,7 @@ export class ConfigStorageService implements ConfigStorageInterface {
|
||||
`Fetching config for ${key as string} in database: ${result?.value}`,
|
||||
);
|
||||
|
||||
return await this.convertAndSecureValue(result.value, key, true);
|
||||
return await this.convertAndDecrypt(result.value, key);
|
||||
} catch (error) {
|
||||
if (error instanceof ConfigVariableException) {
|
||||
throw error;
|
||||
@@ -114,7 +149,7 @@ export class ConfigStorageService implements ConfigStorageInterface {
|
||||
value: ConfigVariables[T],
|
||||
): Promise<void> {
|
||||
try {
|
||||
const dbValue = await this.convertAndSecureValue(value, key, false);
|
||||
const dbValue = await this.convertAndEncrypt(value, key);
|
||||
|
||||
const existingRecord = await this.keyValuePairRepository.findOne({
|
||||
where: this.getConfigVariableWhereClause(key as string),
|
||||
@@ -177,11 +212,7 @@ export class ConfigStorageService implements ConfigStorageInterface {
|
||||
const key = configVar.key as keyof ConfigVariables;
|
||||
|
||||
try {
|
||||
const value = await this.convertAndSecureValue(
|
||||
configVar.value,
|
||||
key,
|
||||
true,
|
||||
);
|
||||
const value = await this.convertAndDecrypt(configVar.value, key);
|
||||
|
||||
if (value !== undefined) {
|
||||
result.set(key, value);
|
||||
|
||||
Reference in New Issue
Block a user