From d2479a9598871968e7b940cec6c4e2b04bcfd62a Mon Sep 17 00:00:00 2001 From: "ankitatripathi.mp@gmail.com" Date: Fri, 14 Aug 2026 15:00:27 +0530 Subject: [PATCH 1/3] fix: dry-run correctness for array diffs, null existing, ignore-fields, and included-connections --- src/tools/auth0/handlers/connections.ts | 7 +- src/tools/auth0/handlers/databases.ts | 2 +- src/tools/auth0/handlers/default.ts | 2 - src/tools/calculateDryRunChanges.ts | 23 ++-- .../tools/auth0/handlers/connections.tests.js | 64 ++++++++++ test/tools/auth0/handlers/databases.tests.js | 110 ++++++++++++++++++ test/tools/calculateDryRunChanges.test.ts | 63 ++++++++++ 7 files changed, 257 insertions(+), 14 deletions(-) diff --git a/src/tools/auth0/handlers/connections.ts b/src/tools/auth0/handlers/connections.ts index dee99a8db..517104d7a 100644 --- a/src/tools/auth0/handlers/connections.ts +++ b/src/tools/auth0/handlers/connections.ts @@ -923,7 +923,12 @@ export default class ConnectionsHandler extends DefaultAPIHandler { }; }); - const proposedChanges = await super.dryRunChanges({ ...assets, connections: formatted }); + let proposedChanges = await super.dryRunChanges({ ...assets, connections: formatted }); + + const includedConnections = (assets.include && assets.include.connections) || []; + const excludedConnections = (assets.exclude && assets.exclude.connections) || []; + proposedChanges = filterExcluded(proposedChanges, excludedConnections); + proposedChanges = filterIncluded(proposedChanges, includedConnections); return addExcludedConnectionPropertiesToChanges({ proposedChanges, diff --git a/src/tools/auth0/handlers/databases.ts b/src/tools/auth0/handlers/databases.ts index 858b72a9c..148e7cf76 100644 --- a/src/tools/auth0/handlers/databases.ts +++ b/src/tools/auth0/handlers/databases.ts @@ -627,7 +627,7 @@ export default class DatabaseHandler extends DefaultAPIHandler { assets: formatted, existing: existingDatabasesConnections, identifiers: this.identifiers, - ignoreDryRunFields: this.ignoreDryRunFields, + ignoreDryRunFields: this.getEffectiveIgnoreDryRunFields(), }); } diff --git a/src/tools/auth0/handlers/default.ts b/src/tools/auth0/handlers/default.ts index 2c304f38e..d466f0c59 100644 --- a/src/tools/auth0/handlers/default.ts +++ b/src/tools/auth0/handlers/default.ts @@ -310,7 +310,6 @@ export default class APIHandler { return calculateDryRunChanges({ type: this.type, assets: typeAssets, - // @ts-ignore TODO: investigate what happens when `existing` is null existing, identifiers: this.identifiers, ignoreDryRunFields: this.getEffectiveIgnoreDryRunFields(), @@ -348,7 +347,6 @@ export default class APIHandler { return calculateDryRunChanges({ type: this.type, assets: typeAssets, - // @ts-ignore TODO: investigate what happens when `existing` is null existing, identifiers: this.identifiers, ignoreDryRunFields: this.getEffectiveIgnoreDryRunFields(), diff --git a/src/tools/calculateDryRunChanges.ts b/src/tools/calculateDryRunChanges.ts index 0a0281466..328dda7ba 100644 --- a/src/tools/calculateDryRunChanges.ts +++ b/src/tools/calculateDryRunChanges.ts @@ -133,6 +133,10 @@ export const exportDiffLog = async (fileName: string, resourceTypeName?: string) } }; +function formatDiffValue(value: unknown): string { + return typeof value === 'object' && value !== null ? JSON.stringify(value) : String(value); +} + /** * Compares two objects and returns an array of human-readable difference strings. * Only considers keys present in `localObj` — extra keys in `remoteObj` are ignored. @@ -232,7 +236,9 @@ export function getObjectDifferences( ); differences.push(...nestedDifferences); } else if (item !== normalizedRemoteValue[index]) { - const message = `Array item difference at [${currentPath}[${index}]]: local:${item} vs remote:${normalizedRemoteValue[index]}`; + const message = `Array item difference at [${currentPath}[${index}]]: local:${formatDiffValue( + item + )} vs remote:${formatDiffValue(normalizedRemoteValue[index])}`; differences.push(message); } }); @@ -273,13 +279,10 @@ export function getObjectDifferences( return; } - // Compare primitive values — omit array indices from path to reduce log noise + // Compare primitive values if (localValue !== remoteValue) { - let arrayPathRegex = new RegExp(/\[\d+\]/g); - if (!arrayPathRegex.test(currentPath)) { - const message = `Value difference for [${currentPath}]: local:${localValue} vs remote:${remoteValue}`; - differences.push(message); - } + const message = `Value difference for [${currentPath}]: local:${localValue} vs remote:${remoteValue}`; + differences.push(message); } }); @@ -341,9 +344,9 @@ export function calculateDryRunChanges({ const localAssets: Asset[] = (Array.isArray(assets) ? [...assets] : [assets]).map((asset) => type === 'tenant' ? normalizeTenantForDryRun(asset) : asset ); - const remoteAssets: Asset[] = (Array.isArray(existing) ? [...existing] : [existing]).map( - (asset) => (type === 'tenant' ? normalizeTenantForDryRun(asset) : asset) - ); + const remoteAssets: Asset[] = ( + Array.isArray(existing) ? [...existing] : existing ? [existing] : [] + ).map((asset) => (type === 'tenant' ? normalizeTenantForDryRun(asset) : asset)); // Helper: returns true if a local and remote asset share at least one identifier value const assetsMatch = (localAsset: Asset, remoteAsset: Asset) => diff --git a/test/tools/auth0/handlers/connections.tests.js b/test/tools/auth0/handlers/connections.tests.js index 515deae56..607993f53 100644 --- a/test/tools/auth0/handlers/connections.tests.js +++ b/test/tools/auth0/handlers/connections.tests.js @@ -2836,3 +2836,67 @@ describe('#addExcludedConnectionPropertiesToChanges', () => { }); // Expect no change }); }); + +describe('#connections dryRunChanges', () => { + // Regression tests for INCLUDED_CONNECTIONS dry-run bug: + // dryRunChanges was not applying filterIncluded, causing phantom DELETEs for + // connections outside AUTH0_INCLUDED_CONNECTIONS. + + const config = (key) => ({ AUTH0_CLIENT_ID: 'client_id' }[key]); + + it('should not report phantom deletes for connections outside AUTH0_INCLUDED_CONNECTIONS', async () => { + const auth0 = { + connections: { + list: (params) => + mockPagedData(params, 'connections', [ + { id: 'con_google', name: 'google-oauth2', strategy: 'google-oauth2' }, + { id: 'con_email', name: 'email', strategy: 'email' }, + ]), + }, + clients: { + list: (params) => mockPagedData(params, 'clients', []), + }, + }; + + const handler = new connections.default({ client: pageClient(auth0), config }); + + // Local config only manages google-oauth2; include list restricts to it + const assets = { + connections: [{ name: 'google-oauth2', strategy: 'google-oauth2' }], + include: { connections: ['google-oauth2'] }, + }; + + const changes = await handler.dryRunChanges(assets); + + // 'email' exists on the tenant but is outside the include list — + // it must not appear as a DELETE in dry-run + expect(changes.del).to.have.length(0); + }); + + it('should report deletes normally when no include list is configured', async () => { + const auth0 = { + connections: { + list: (params) => + mockPagedData(params, 'connections', [ + { id: 'con_google', name: 'google-oauth2', strategy: 'google-oauth2' }, + { id: 'con_email', name: 'email', strategy: 'email' }, + ]), + }, + clients: { + list: (params) => mockPagedData(params, 'clients', []), + }, + }; + + const handler = new connections.default({ client: pageClient(auth0), config }); + + // No include filter — all connections are in scope + const assets = { + connections: [{ name: 'google-oauth2', strategy: 'google-oauth2' }], + }; + + const changes = await handler.dryRunChanges(assets); + + // 'email' is in scope and not in local config — should appear as DELETE + expect(changes.del.some((c) => c.name === 'email')).to.be.true; + }); +}); diff --git a/test/tools/auth0/handlers/databases.tests.js b/test/tools/auth0/handlers/databases.tests.js index d48afd9ff..0b7992336 100644 --- a/test/tools/auth0/handlers/databases.tests.js +++ b/test/tools/auth0/handlers/databases.tests.js @@ -3022,3 +3022,113 @@ describe('#databases handler with enabled clients integration', () => { }); }); }); + +describe('#databases dryRunChanges', () => { + // Regression tests for #1450: AUTH0_IGNORE_DRY_RUN_FIELDS was silently a no-op + // for databases because dryRunChanges used this.ignoreDryRunFields (constructor + // defaults only) instead of this.getEffectiveIgnoreDryRunFields() (which merges + // in the AUTH0_IGNORE_DRY_RUN_FIELDS config value). + + const pool = { + addEachTask: (data) => { + if (data.data && data.data.length) data.generator(data.data[0]); + return { promise: () => null }; + }, + addSingleTask: (task) => { + const result = task.generator(task.data); + return { promise: () => Promise.resolve(result) }; + }, + }; + + it('should suppress diffs for fields listed in AUTH0_IGNORE_DRY_RUN_FIELDS', async () => { + const auth0 = { + connections: { + // Remote includes options: {} so getFormattedOptions doesn't produce a spurious diff + list: (params) => + mockPagedData(params, 'connections', [ + { + id: 'con_1', + name: 'test-db', + strategy: 'auth0', + options: {}, + noisy_field: 'remote_value', + }, + ]), + }, + clients: { + list: (params) => mockPagedData(params, 'clients', []), + }, + actions: { + list: (params) => mockPagedData(params, 'actions', []), + }, + pool, + }; + + const config = (key) => { + if (key === 'AUTH0_IGNORE_DRY_RUN_FIELDS') return { databases: ['noisy_field'] }; + if (key === 'AUTH0_CLIENT_ID') return 'client_id'; + }; + + const handler = new databases.default({ client: pageClient(auth0), config }); + + const assets = { + databases: [ + { + name: 'test-db', + strategy: 'auth0', + options: {}, + noisy_field: 'local_value', // differs from remote — but must be ignored + }, + ], + }; + + const changes = await handler.dryRunChanges(assets); + + // noisy_field is configured to be ignored — no update should be reported + expect(changes.update).to.have.length(0); + }); + + it('should report diffs for fields not in AUTH0_IGNORE_DRY_RUN_FIELDS', async () => { + const auth0 = { + connections: { + list: (params) => + mockPagedData(params, 'connections', [ + { + id: 'con_1', + name: 'test-db', + strategy: 'auth0', + options: {}, + tracked_field: 'remote_value', + }, + ]), + }, + clients: { + list: (params) => mockPagedData(params, 'clients', []), + }, + actions: { + list: (params) => mockPagedData(params, 'actions', []), + }, + pool, + }; + + // No AUTH0_IGNORE_DRY_RUN_FIELDS configured + const config = (key) => ({ AUTH0_CLIENT_ID: 'client_id' }[key]); + + const handler = new databases.default({ client: pageClient(auth0), config }); + + const assets = { + databases: [ + { + name: 'test-db', + strategy: 'auth0', + options: {}, + tracked_field: 'local_value', // differs from remote — should be detected + }, + ], + }; + + const changes = await handler.dryRunChanges(assets); + + expect(changes.update).to.have.length(1); + }); +}); diff --git a/test/tools/calculateDryRunChanges.test.ts b/test/tools/calculateDryRunChanges.test.ts index 7d53f4f58..58ac0324c 100644 --- a/test/tools/calculateDryRunChanges.test.ts +++ b/test/tools/calculateDryRunChanges.test.ts @@ -544,6 +544,45 @@ describe('#utils calculateDryRunChanges', () => { expect(changes.update[0].client_id).to.equal('cli_abc'); }); + it('should classify asset as update when a field inside an array item changes', () => { + // Regression test for #1451: value change inside array item was silently dropped + const changes = calculateDryRunChanges({ + type: 'organizations', + assets: [ + { + name: 'acme', + connections: [{ connection_id: 'con_abc', assign_membership_on_login: true }], + }, + ], + existing: [ + { + name: 'acme', + connections: [{ connection_id: 'con_abc', assign_membership_on_login: false }], + }, + ], + identifiers: ['id', 'name'], + ignoreDryRunFields: [], + }); + + expect(changes.update).to.have.length(1); + expect(changes.update[0].name).to.equal('acme'); + }); + + it('should treat null existing as empty and classify all local assets as creates', () => { + // Regression test for null crash: [null] was used as remoteAssets, causing TypeError + const changes = calculateDryRunChanges({ + type: 'clients', + assets: [{ name: 'My App', app_type: 'spa' }], + existing: null, + identifiers: ['client_id', 'name'], + ignoreDryRunFields: [], + }); + + expect(changes.create).to.have.length(1); + expect(changes.update).to.have.length(0); + expect(changes.del).to.have.length(0); + }); + it('should use _clientName in the UPDATE identifier for clientGrants', () => { const changes = calculateDryRunChanges({ type: 'clientGrants', @@ -660,6 +699,30 @@ describe('#getObjectDifferences', () => { ); expect(diffs.length).to.be.greaterThan(0); }); + + it('should report value differences for fields inside array items', () => { + // Regression test for #1451: array index path guard was suppressing these diffs + const diffs = getObjectDifferences( + { connections: [{ connection_id: 'con_abc', assign_membership_on_login: true }] }, + { connections: [{ connection_id: 'con_abc', assign_membership_on_login: false }] }, + 'acme', + 'organizations' + ); + expect(diffs.length).to.be.greaterThan(0); + expect(diffs.some((d) => d.includes('assign_membership_on_login'))).to.be.true; + }); + + it('should serialize object array items as JSON in diff messages, not as [object Object]', () => { + // Regression test for #1452: objects were rendered via .toString() → [object Object] + const diffs = getObjectDifferences( + { connections: [{ connection_id: 'con_abc', assign_membership_on_login: false }] }, + { connections: [] }, + 'acme', + 'organizations' + ); + expect(diffs.some((d) => d.includes('[object Object]'))).to.be.false; + expect(diffs.some((d) => d.includes('connection_id'))).to.be.true; + }); }); describe('#calculateDryRunChanges - diff log labels', () => { From 62b1c820815e1c8098a84110c43cf53b28cac1f7 Mon Sep 17 00:00:00 2001 From: "ankitatripathi.mp@gmail.com" Date: Tue, 18 Aug 2026 14:44:21 +0530 Subject: [PATCH 2/3] fix: resolve merge conflicts after upstream pull --- src/tools/calculateDryRunChanges.ts | 12 ++++--- test/tools/calculateDryRunChanges.test.ts | 39 +++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/tools/calculateDryRunChanges.ts b/src/tools/calculateDryRunChanges.ts index 328dda7ba..3403ad766 100644 --- a/src/tools/calculateDryRunChanges.ts +++ b/src/tools/calculateDryRunChanges.ts @@ -120,6 +120,14 @@ function normalizeArrayValues(values: any[]): any[] { .sort((left, right) => JSON.stringify(left).localeCompare(JSON.stringify(right))); } +/** + * Serializes a value to a human-readable string for use in diff messages. + * Objects and arrays are JSON-stringified to avoid "[object Object]" output. + */ +function formatDiffValue(value: unknown): string { + return typeof value === 'object' && value !== null ? JSON.stringify(value) : String(value); +} + /** * Writes the accumulated diff log to a JSON file on disk. * Useful for CI pipelines that want a machine-readable dry-run report. @@ -133,10 +141,6 @@ export const exportDiffLog = async (fileName: string, resourceTypeName?: string) } }; -function formatDiffValue(value: unknown): string { - return typeof value === 'object' && value !== null ? JSON.stringify(value) : String(value); -} - /** * Compares two objects and returns an array of human-readable difference strings. * Only considers keys present in `localObj` — extra keys in `remoteObj` are ignored. diff --git a/test/tools/calculateDryRunChanges.test.ts b/test/tools/calculateDryRunChanges.test.ts index 58ac0324c..3752a04f1 100644 --- a/test/tools/calculateDryRunChanges.test.ts +++ b/test/tools/calculateDryRunChanges.test.ts @@ -605,6 +605,45 @@ describe('#utils calculateDryRunChanges', () => { const updatedGrant = changes.update[0] as any; expect(updatedGrant._clientName).to.equal('My M2M App'); }); + + it('should classify asset as update when a field inside an array item changes', () => { + // Regression test for #1451: array index path guard was suppressing diffs for + // paths containing "[N]", causing field changes inside array items to go undetected. + const changes = calculateDryRunChanges({ + type: 'organizations', + assets: [ + { + name: 'acme', + connections: [{ connection_id: 'con_abc', assign_membership_on_login: true }], + }, + ], + existing: [ + { + name: 'acme', + connections: [{ connection_id: 'con_abc', assign_membership_on_login: false }], + }, + ], + identifiers: ['id', 'name'], + ignoreDryRunFields: [], + }); + expect(changes.update).to.have.length(1); + expect(changes.update[0].name).to.equal('acme'); + }); + + it('should treat null existing as empty and classify all local assets as creates', () => { + // Regression test for null crash: when getType() returns null (e.g. 403 or empty tenant), + // existing is null — must not crash and must classify all local assets as creates. + const changes = calculateDryRunChanges({ + type: 'clients', + assets: [{ name: 'My App', app_type: 'spa' }], + existing: null, + identifiers: ['client_id', 'name'], + ignoreDryRunFields: [], + }); + expect(changes.create).to.have.length(1); + expect(changes.update).to.have.length(0); + expect(changes.del).to.have.length(0); + }); }); describe('#getObjectDifferences', () => { From 86a20294800f96e75d70fd54f17cdaf12cdb93c1 Mon Sep 17 00:00:00 2001 From: "ankitatripathi.mp@gmail.com" Date: Fri, 21 Aug 2026 14:27:27 +0530 Subject: [PATCH 3/3] test: remove duplicate test cases in calculateDryRunChanges --- test/tools/calculateDryRunChanges.test.ts | 39 ----------------------- 1 file changed, 39 deletions(-) diff --git a/test/tools/calculateDryRunChanges.test.ts b/test/tools/calculateDryRunChanges.test.ts index 3752a04f1..f1bd3adce 100644 --- a/test/tools/calculateDryRunChanges.test.ts +++ b/test/tools/calculateDryRunChanges.test.ts @@ -544,45 +544,6 @@ describe('#utils calculateDryRunChanges', () => { expect(changes.update[0].client_id).to.equal('cli_abc'); }); - it('should classify asset as update when a field inside an array item changes', () => { - // Regression test for #1451: value change inside array item was silently dropped - const changes = calculateDryRunChanges({ - type: 'organizations', - assets: [ - { - name: 'acme', - connections: [{ connection_id: 'con_abc', assign_membership_on_login: true }], - }, - ], - existing: [ - { - name: 'acme', - connections: [{ connection_id: 'con_abc', assign_membership_on_login: false }], - }, - ], - identifiers: ['id', 'name'], - ignoreDryRunFields: [], - }); - - expect(changes.update).to.have.length(1); - expect(changes.update[0].name).to.equal('acme'); - }); - - it('should treat null existing as empty and classify all local assets as creates', () => { - // Regression test for null crash: [null] was used as remoteAssets, causing TypeError - const changes = calculateDryRunChanges({ - type: 'clients', - assets: [{ name: 'My App', app_type: 'spa' }], - existing: null, - identifiers: ['client_id', 'name'], - ignoreDryRunFields: [], - }); - - expect(changes.create).to.have.length(1); - expect(changes.update).to.have.length(0); - expect(changes.del).to.have.length(0); - }); - it('should use _clientName in the UPDATE identifier for clientGrants', () => { const changes = calculateDryRunChanges({ type: 'clientGrants',