test(core): Expand secret field coverage from review

- Administrator secret custom field preserved via updateActiveAdministrator (the
  alias-input corruption regression).
- Config arg strategy input carries code and derived entityType/field.
- Strengthen the owning-entity assertion to rule out the customFields wrapper.
- Transformer to()/unconfigured/undecryptable-value paths; GCM tamper and malformed
  ciphertext.

Relates to #2648

Author: michaelbromley

packages/core/e2e/config-arg-secret.e2e-spec.ts

@@ -8,6 +8,9 @@ import {
     mergeConfig,
     PaymentMethod,
     PaymentMethodHandler,
+    RequestContext,
+    SecretAccessInput,
+    SecretAccessStrategy,
     TransactionalConnection,
 } from '@vendure/core';
 import { createTestEnvironment } from '@vendure/testing';
@@ -58,6 +61,16 @@ const secretCollectionFilter = new CollectionFilter({
 const PLAINTEXT_KEY = 'sk_live_supersecret';
 const FILTER_PLAINTEXT_KEY = 'ck_live_collectionsecret';
 
+// Captures the input passed to the strategy so a test can assert the derived owner (entityType/field)
+// and code for a config arg, while preserving the default permission-based reveal decision.
+let capturedSecretAccessInput: SecretAccessInput | undefined;
+class CapturingSecretAccessStrategy implements SecretAccessStrategy {
+    canAccessSecret(ctx: RequestContext, input: SecretAccessInput): boolean {
+        capturedSecretAccessInput = input;
+        return ctx.userHasPermissions([Permission.ReadSecret]);
+    }
+}
+
 // #2648 — `secret` config arg values must be encrypted at rest and never returned in plaintext
 // to a caller without the ReadSecret permission.
 describe('secret config args', () => {
@@ -71,6 +84,7 @@ describe('secret config args', () => {
             },
             systemOptions: {
                 encryptionStrategy: new DefaultEncryptionStrategy({ secret: 'test-encryption-key' }),
+                secretAccessStrategy: new CapturingSecretAccessStrategy(),
             },
         }),
     );
@@ -141,6 +155,20 @@ describe('secret config args', () => {
         expect(apiKey?.value).toBe(PLAINTEXT_KEY);
     });
 
+    it('provides the code and derived owner (entityType/field) to the strategy for a config arg', async () => {
+        capturedSecretAccessInput = undefined;
+        await adminClient.asSuperAdmin();
+        await adminClient.query(getPaymentMethodDocument, { id: paymentMethodId });
+        const captured = capturedSecretAccessInput as SecretAccessInput | undefined;
+        expect(captured?.kind).toBe('configArg');
+        if (captured?.kind === 'configArg') {
+            expect(captured.code).toBe(secretPaymentHandler.code);
+            expect(captured.argName).toBe('apiKey');
+            expect(captured.entityType).toBe('PaymentMethod');
+            expect(captured.field).toBe('handler');
+        }
+    });
+
     it('stores the secret arg encrypted at rest', async () => {
         const connection = server.app.get(TransactionalConnection);
         const stored = await connection.rawConnection

packages/core/e2e/custom-field-secret.e2e-spec.ts

@@ -73,6 +73,7 @@ describe('secret custom fields', () => {
                     { name: 'secretKey', type: 'string', secret: true },
                     { name: 'note', type: 'string' },
                 ],
+                Administrator: [{ name: 'apiToken', type: 'string', secret: true }],
             },
             systemOptions: {
                 encryptionStrategy: new DefaultEncryptionStrategy({ secret: 'test-encryption-key' }),
@@ -82,6 +83,7 @@ describe('secret custom fields', () => {
     );
 
     const manager = { emailAddress: 'cf-secret-manager@test.com', password: 'test-password' };
+    let managerAdminId: string;
 
     beforeAll(async () => {
         await server.init({
@@ -99,7 +101,7 @@ describe('secret custom fields', () => {
                 channelIds: ['T_1'],
             },
         });
-        await adminClient.query(createAdministratorDocument, {
+        const { createAdministrator } = await adminClient.query(createAdministratorDocument, {
             input: {
                 emailAddress: manager.emailAddress,
                 firstName: 'CF',
@@ -108,6 +110,7 @@ describe('secret custom fields', () => {
                 roleIds: [createRole.id],
             },
         });
+        managerAdminId = createAdministrator.id;
     }, TEST_SETUP_TIMEOUT_MS);
 
     afterAll(async () => {
@@ -170,9 +173,69 @@ describe('secret custom fields', () => {
         const captured = capturedSecretAccessInput as SecretAccessInput | undefined;
         expect(captured?.kind).toBe('customField');
         const entity = captured?.kind === 'customField' ? captured.entity : undefined;
+        // Must be the owning Product entity, not the customFields wrapper. The entity holds its custom
+        // fields under a nested `customFields` object; the wrapper instead spreads them at the top
+        // level (so it would have `secretKey` directly and no nested `customFields`).
+        expect((entity as any)?.customFields?.secretKey).toBe('sk_entity_check');
+        expect((entity as any)?.secretKey).toBeUndefined();
         expect(entity?.id).toBeTruthy();
-        // The owning entity carries a nested `customFields` object; the wrapper does not.
-        expect((entity as any)?.customFields).toBeDefined();
+    });
+
+    // Gabriel review — secret custom fields on an entity edited via an alias input type (here
+    // `updateActiveAdministrator`, an admin saving their own profile) must be preserved on a
+    // placeholder resubmit, not corrupted. This is the common case that the Product-only suites missed.
+    it('preserves a secret custom field edited via updateActiveAdministrator (alias input)', async () => {
+        const SET_ADMIN_TOKEN = gql`
+            mutation SetAdminToken($input: UpdateAdministratorInput!) {
+                updateAdministrator(input: $input) {
+                    id
+                }
+            }
+        `;
+        const GET_ADMIN_TOKEN = gql`
+            query GetAdminToken($id: ID!) {
+                administrator(id: $id) {
+                    id
+                    customFields {
+                        apiToken
+                    }
+                }
+            }
+        `;
+        const UPDATE_ACTIVE_ADMIN = gql`
+            mutation UpdateActiveAdmin($input: UpdateActiveAdministratorInput!) {
+                updateActiveAdministrator(input: $input) {
+                    id
+                }
+            }
+        `;
+        // As SuperAdmin, set the manager's secret token.
+        await adminClient.asSuperAdmin();
+        await adminClient.query(SET_ADMIN_TOKEN, {
+            input: { id: managerAdminId, customFields: { apiToken: 'admin_secret_token' } },
+        });
+
+        // The manager (no ReadSecret) sees the placeholder and saves their own profile back with it.
+        await adminClient.asUserWithCredentials(manager.emailAddress, manager.password);
+        const { activeAdministrator } = await adminClient.query(gql`
+            query {
+                activeAdministrator {
+                    id
+                    customFields {
+                        apiToken
+                    }
+                }
+            }
+        `);
+        expect(activeAdministrator.customFields.apiToken).toBe(REDACTED_SECRET_PLACEHOLDER);
+        await adminClient.query(UPDATE_ACTIVE_ADMIN, {
+            input: { firstName: 'Renamed', customFields: { apiToken: REDACTED_SECRET_PLACEHOLDER } },
+        });
+
+        // The stored secret must be preserved, not overwritten with the encrypted placeholder.
+        await adminClient.asSuperAdmin();
+        const { administrator } = await adminClient.query(GET_ADMIN_TOKEN, { id: managerAdminId });
+        expect(administrator.customFields.apiToken).toBe('admin_secret_token');
     });
 
     it(
@@ -192,6 +255,6 @@ describe('secret custom fields', () => {
                     customFields: { secretKey: REDACTED_SECRET_PLACEHOLDER },
                 },
             });
-        }, 'A value must be provided for the secret argument "secretKey"'),
+        }, 'A value must be provided for the secret field "secretKey"'),
     );
 });

packages/core/src/config/system/default-encryption-strategy.spec.ts

@@ -58,4 +58,20 @@ describe('DefaultEncryptionStrategy', () => {
         const encrypted = a.encrypt('secret-value');
         expect(() => b.decrypt(encrypted)).toThrow();
     });
+
+    it('rejects tampered ciphertext (GCM integrity)', () => {
+        const strategy = configured();
+        const encrypted = strategy.encrypt('secret-value');
+        // Flip the last character of the ciphertext's data segment.
+        const parts = encrypted.split(':');
+        const data = parts[parts.length - 1];
+        const flipped = (data.slice(0, -1) + (data.endsWith('A') ? 'B' : 'A')) as string;
+        parts[parts.length - 1] = flipped;
+        expect(() => strategy.decrypt(parts.join(':'))).toThrow();
+    });
+
+    it('throws a clear error on a malformed ciphertext', () => {
+        const strategy = configured();
+        expect(() => strategy.decrypt('enc:v1:')).toThrow(/not a well-formed ciphertext/);
+    });
 });

packages/core/src/entity/value-transformers.spec.ts

@@ -1,3 +1,4 @@
+import { REDACTED_SECRET_PLACEHOLDER } from '@vendure/common/lib/shared-constants';
 import { describe, expect, it } from 'vitest';
 
 import { EncryptionStrategy } from '../config/system/encryption-strategy';
@@ -6,7 +7,8 @@ import { EncryptedFieldTransformer } from './value-transformers';
 
 /**
  * A strategy that throws if asked to decrypt anything not produced by encrypt(), modelling a custom
- * strategy that honours the interface contract strictly (decrypt only handles ciphertext).
+ * strategy that honours the interface contract strictly (decrypt only handles ciphertext). A value of
+ * `enc:corrupt` models ciphertext that cannot be decrypted (e.g. corrupted or wrong-key data).
  */
 class StrictStrategy implements EncryptionStrategy {
     encrypt(plaintext: string) {
@@ -16,7 +18,11 @@ class StrictStrategy implements EncryptionStrategy {
         if (!this.isEncrypted(ciphertext)) {
             throw new Error('decrypt() called on a non-ciphertext value');
         }
-        return ciphertext.slice('enc:'.length);
+        const payload = ciphertext.slice('enc:'.length);
+        if (payload === 'corrupt') {
+            throw new Error('cannot decrypt');
+        }
+        return payload;
     }
     isEncrypted(value: string) {
         return value.startsWith('enc:');
@@ -29,6 +35,15 @@ class StrictStrategy implements EncryptionStrategy {
 describe('EncryptedFieldTransformer', () => {
     const transformer = new EncryptedFieldTransformer(() => new StrictStrategy());
 
+    it('encrypts on write', () => {
+        expect(transformer.to('secret')).toBe('enc:secret');
+    });
+
+    it('passes null and empty through on write without encrypting', () => {
+        expect(transformer.to(null)).toBe(null);
+        expect(transformer.to('')).toBe('');
+    });
+
     it('decrypts ciphertext on read', () => {
         expect(transformer.from('enc:secret')).toBe('secret');
     });
@@ -37,8 +52,18 @@ describe('EncryptedFieldTransformer', () => {
         expect(transformer.from('legacy-plaintext')).toBe('legacy-plaintext');
     });
 
-    it('passes null and empty through', () => {
+    it('returns the placeholder (not throwing) when a single value cannot be decrypted', () => {
+        expect(transformer.from('enc:corrupt')).toBe(REDACTED_SECRET_PLACEHOLDER);
+    });
+
+    it('passes null and empty through on read', () => {
         expect(transformer.from(null)).toBe(null);
         expect(transformer.from('')).toBe('');
     });
+
+    it('throws when no EncryptionStrategy is configured', () => {
+        const unconfigured = new EncryptedFieldTransformer(() => undefined);
+        expect(() => unconfigured.to('secret')).toThrow(/no EncryptionStrategy is configured/);
+        expect(() => unconfigured.from('enc:secret')).toThrow(/no EncryptionStrategy is configured/);
+    });
 });