Merge branch 'master' into feat/add_logging_info

Author: Volodymyr Malyhin

registry/server/migrations/20260113105120_sanitize_versioning_secrets.ts

@@ -0,0 +1,47 @@
+import { Knex } from 'knex';
+
+import { EntityTypes } from '../versioning/interfaces';
+import { sanitizeSnapshot, Snapshot } from '../versioning/utils/secretSanitizer';
+
+const ENTITY_TYPES = [EntityTypes.settings, EntityTypes.settings_domain_value, EntityTypes.auth_entities];
+
+function sanitizeIfChanged(json: string | null, entityType: string, secretKeys: Set<string>): string | null {
+    if (!json) {
+        return null;
+    }
+    const sanitized = JSON.stringify(sanitizeSnapshot(entityType, JSON.parse(json) as Snapshot, secretKeys));
+    return sanitized !== json ? sanitized : null;
+}
+
+export async function up(knex: Knex): Promise<void> {
+    const secretSettings: { key: string }[] = await knex('settings').select('key').where('secret', true);
+    const secretKeys = new Set(secretSettings.map((s) => s.key));
+
+    let count = 0;
+    const rows = await knex('versioning')
+        .select('id', 'entity_type', 'data', 'data_after')
+        .whereIn('entity_type', ENTITY_TYPES);
+
+    for (const row of rows) {
+        const data = sanitizeIfChanged(row.data, row.entity_type, secretKeys);
+        const dataAfter = sanitizeIfChanged(row.data_after, row.entity_type, secretKeys);
+
+        if (data || dataAfter) {
+            const update: Record<string, string> = {};
+            if (data) {
+                update.data = data;
+            }
+            if (dataAfter) {
+                update.data_after = dataAfter;
+            }
+            await knex('versioning').where('id', row.id).update(update);
+            count++;
+        }
+    }
+
+    console.log(`Sanitized ${count} versioning records`);
+}
+
+export async function down(): Promise<void> {
+    throw new Error('Irreversible migration: secret values cannot be recovered');
+}

registry/server/versioning/services/Versioning.ts

@@ -5,6 +5,7 @@ import versioningConfig from '../config';
 import { extractInsertedId, formatDate } from '../../util/db';
 import * as errors from '../errors';
 import { EntityTypes, OperationConfig, VersionRow, VersionRowData, VersionRowParsed } from '../interfaces';
+import { sanitizeSnapshot, stripSecretFields } from '../utils/secretSanitizer';
 import Transaction = Knex.Transaction;
 
 export * from '../interfaces';
@@ -66,14 +67,21 @@ export class Versioning {
                 });
             }
 
-            const data = currentData === null ? null : JSON.stringify(currentData);
-            const data_after = newData === null ? null : JSON.stringify(newData);
+            const rawData = currentData === null ? null : JSON.stringify(currentData);
+            const rawDataAfter = newData === null ? null : JSON.stringify(newData);
 
-            if (data === data_after) {
+            if (rawData === rawDataAfter) {
                 await commit();
                 return null;
             }
 
+            const secretKeys = await this.getSecretSettingKeys(trx);
+            const sanitize = (snapshot: typeof currentData) =>
+                snapshot ? JSON.stringify(sanitizeSnapshot(updatedConfig.type, snapshot, secretKeys)) : null;
+
+            const data = sanitize(currentData);
+            const data_after = sanitize(newData);
+
             const logRecord: VersionRowData = {
                 entity_type: updatedConfig.type,
                 entity_id: updatedConfig.id.toString(),
@@ -127,46 +135,55 @@ export class Versioning {
                         .delete()
                         .transacting(trx);
                 } else if (versionRow.data_after === null) {
-                    // Deletion operation, so we need to create everything the was deleted
+                    // Deletion operation, so we need to create everything that was deleted
+                    // Strip secret marker fields to avoid inserting placeholders - secrets remain lost
                     const dataToRestore = versionRow.data;
+                    const strippedData = stripSecretFields(dataToRestore.data);
                     await this.db(versionRow.entity_type)
                         .insert({
-                            ...dataToRestore.data,
+                            ...strippedData,
                             [entityConfig.idColumn]: versionRow.entity_id,
                         })
                         .transacting(trx);
 
                     for (const relation of entityConfig.related) {
                         const relatedItems = dataToRestore.related[relation.type].map((v: any) => ({
-                            ...v,
+                            ...stripSecretFields(v),
                             [relation.key]: versionRow.entity_id,
                         }));
                         await this.db.batchInsert(relation.type, relatedItems).transacting(trx);
                     }
                 } else {
                     // We have an update operation
+                    // Strip secret marker fields to preserve current DB values for secrets
                     const dataToRestore = versionRow.data;
+                    const strippedData = stripSecretFields(dataToRestore.data);
                     for (const relation of entityConfig.related) {
                         await this.db(relation.type)
                             .where(relation.key, versionRow.entity_id)
                             .delete()
                             .transacting(trx);
 
                         const relatedItems = dataToRestore.related[relation.type].map((v: any) => ({
-                            ...v,
+                            ...stripSecretFields(v),
                             [relation.key]: versionRow.entity_id,
                         }));
                         await this.db.batchInsert(relation.type, relatedItems).transacting(trx);
                     }
                     await this.db(versionRow.entity_type)
                         .where(entityConfig.idColumn, versionRow.entity_id)
-                        .update(dataToRestore.data)
+                        .update(strippedData)
                         .transacting(trx);
                 }
             },
         );
     }
 
+    private async getSecretSettingKeys(trx: Transaction): Promise<Set<string>> {
+        const secretSettings = await this.db('settings').select('key').where('secret', true).transacting(trx);
+        return new Set(secretSettings.map((s: { key: string }) => s.key));
+    }
+
     private async getDataSnapshot(trx: Transaction, config: OperationConfig) {
         if (!config.id) {
             throw new Error(`Attempt to log operation without passing an ID! Passed ID: "${config.id}"`);

registry/server/versioning/utils/secretSanitizer.ts

@@ -0,0 +1,40 @@
+import { EntityTypes } from '../interfaces';
+
+export const SECRET_MARKER = '[SECRET]';
+export const LEGACY_MARKER = '[REDACTED]';
+
+export type Snapshot = { data: Record<string, any>; related: Record<string, any[]> };
+
+export function isSecretMarker(value: unknown): boolean {
+    return value === SECRET_MARKER || value === LEGACY_MARKER;
+}
+
+export function stripSecretFields(data: Record<string, any>): Record<string, any> {
+    return Object.fromEntries(Object.entries(data).filter(([_, value]) => !isSecretMarker(value)));
+}
+
+export function sanitizeSnapshot(entityType: string, snapshot: Snapshot, secretKeys: Set<string>): Snapshot {
+    const result: Snapshot = {
+        data: { ...snapshot.data },
+        related: Object.fromEntries(
+            Object.entries(snapshot.related).map(([key, arr]) => [key, arr.map((item) => ({ ...item }))]),
+        ),
+    };
+
+    if (entityType === EntityTypes.settings && result.data.secret) {
+        result.data.value = SECRET_MARKER;
+        if (result.data.default !== undefined) {
+            result.data.default = SECRET_MARKER;
+        }
+    }
+
+    if (entityType === EntityTypes.settings_domain_value && secretKeys.has(result.data.key)) {
+        result.data.value = SECRET_MARKER;
+    }
+
+    if (entityType === EntityTypes.auth_entities && result.data.secret) {
+        result.data.secret = SECRET_MARKER;
+    }
+
+    return result;
+}

registry/tests/versioning.spec.ts

@@ -1,6 +1,7 @@
 process.env.TZ = 'UTC';
 
 import supertest, { type Agent } from 'supertest';
+import * as bcrypt from 'bcrypt';
 import db from '../server/db';
 import { formatDate } from '../server/util/db';
 import { expect, request, requestWithAuth } from './common';
@@ -320,4 +321,136 @@ describe(`Tests ${basePath}`, () => {
             });
         });
     });
+
+    describe('Secret sanitization', () => {
+        const authEntitiesPath = '/api/v1/auth_entities';
+
+        function findVersionRecord(
+            versions: any[],
+            entityId: number | undefined,
+            filter: { hasData?: boolean; hasDataAfter?: boolean } = {},
+        ): any {
+            return versions.find((v: any) => {
+                if (v.entity_type !== 'auth_entities') return false;
+                if (String(v.entity_id) !== String(entityId)) return false;
+                if (filter.hasData !== undefined && (v.data !== null) !== filter.hasData) return false;
+                if (filter.hasDataAfter !== undefined && (v.data_after !== null) !== filter.hasDataAfter) return false;
+                return true;
+            });
+        }
+
+        it('should mask auth_entities secret in versioning API response', async () => {
+            let authEntityId: number | undefined;
+
+            try {
+                const createResp = await req
+                    .post(`${authEntitiesPath}/`)
+                    .send({
+                        identifier: 'test-api-auth-entity',
+                        secret: 'super-secret-password-hash',
+                        provider: 'local',
+                        role: 'admin',
+                    })
+                    .expect(200);
+
+                authEntityId = createResp.body.id;
+
+                const response = await req.get(basePath).expect(200);
+                const authVersion = findVersionRecord(response.body, authEntityId);
+
+                expect(authVersion).to.exist;
+                const dataAfter = JSON.parse(authVersion.data_after);
+
+                expect(dataAfter.data.secret).to.equal('[SECRET]');
+                expect(dataAfter.data.identifier).to.equal('test-api-auth-entity');
+                expect(dataAfter.data.provider).to.equal('local');
+            } finally {
+                if (authEntityId) {
+                    await req.delete(`${authEntitiesPath}/${authEntityId}`);
+                }
+            }
+        });
+
+        it('should detect auth_entities secret changes and log them with masked values', async () => {
+            let authEntityId: number | undefined;
+
+            try {
+                const createResp = await req
+                    .post(`${authEntitiesPath}/`)
+                    .send({
+                        identifier: 'test-api-auth-change',
+                        secret: 'initial-secret-hash',
+                        provider: 'local',
+                        role: 'admin',
+                    })
+                    .expect(200);
+
+                authEntityId = createResp.body.id;
+
+                await req.put(`${authEntitiesPath}/${authEntityId}`).send({ secret: 'new-secret-hash' }).expect(200);
+
+                const response = await req.get(basePath).expect(200);
+                const updateVersion = findVersionRecord(response.body, authEntityId, {
+                    hasData: true,
+                    hasDataAfter: true,
+                });
+
+                expect(updateVersion).to.exist;
+                const data = JSON.parse(updateVersion.data);
+                const dataAfter = JSON.parse(updateVersion.data_after);
+
+                expect(data.data.secret).to.equal('[SECRET]');
+                expect(dataAfter.data.secret).to.equal('[SECRET]');
+                expect(data.data.identifier).to.equal('test-api-auth-change');
+            } finally {
+                if (authEntityId) {
+                    await req.delete(`${authEntitiesPath}/${authEntityId}`);
+                }
+            }
+        });
+
+        it('should revert non-secret fields while preserving current auth secret', async () => {
+            let authEntityId: number | undefined;
+
+            try {
+                const createResp = await req
+                    .post(`${authEntitiesPath}/`)
+                    .send({
+                        identifier: 'test-api-auth-revert',
+                        secret: 'initial-secret-hash',
+                        provider: 'local',
+                        role: 'admin',
+                    })
+                    .expect(200);
+
+                authEntityId = createResp.body.id;
+
+                await req
+                    .put(`${authEntitiesPath}/${authEntityId}`)
+                    .send({ secret: 'new-secret-hash', role: 'readonly' })
+                    .expect(200);
+
+                const versionsResp = await req.get(basePath).expect(200);
+                const updateVersion = findVersionRecord(versionsResp.body, authEntityId, {
+                    hasData: true,
+                    hasDataAfter: true,
+                });
+
+                expect(updateVersion).to.exist;
+
+                await req.post(`${basePath}/${updateVersion.id}/revert`).expect(200);
+
+                const authResp = await req.get(`${authEntitiesPath}/${authEntityId}`).expect(200);
+                expect(authResp.body.role).to.equal('admin');
+
+                const [dbEntity] = await db('auth_entities').where('id', authEntityId);
+                expect(await bcrypt.compare('new-secret-hash', dbEntity.secret)).to.be.true;
+                expect(await bcrypt.compare('initial-secret-hash', dbEntity.secret)).to.be.false;
+            } finally {
+                if (authEntityId) {
+                    await req.delete(`${authEntitiesPath}/${authEntityId}`);
+                }
+            }
+        });
+    });
 });