From 17cc2da1ce6b1ecb99411baaadd0ae9395d92135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=ADdia=20Tarcza?= <100163235+diatrcz@users.noreply.github.com> Date: Wed, 13 May 2026 15:48:04 +0200 Subject: [PATCH 1/2] build: add cloning to deepmerge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Lídia Tarcza <100163235+diatrcz@users.noreply.github.com> --- lib/helper.ts | 43 ++++++- lib/request-wrapper.ts | 2 +- test/unit/request-wrapper.test.js | 182 ++++++++++++++++++++++++++++++ 3 files changed, 221 insertions(+), 6 deletions(-) diff --git a/lib/helper.ts b/lib/helper.ts index 286c48af..6a751d2e 100644 --- a/lib/helper.ts +++ b/lib/helper.ts @@ -359,13 +359,46 @@ export function isJsonMimeType(mimeType: string) { const isObj = (val: unknown) => val && typeof val === 'object' && !Array.isArray(val); +/** + * Deep clone a value (object, array, or primitive) + * @param value - The value to clone + * @returns A deep clone of the value + */ +function deepClone(value: any): any { + if (value == null) { + return value; + } + + // Handle arrays - create a new array with cloned elements + if (Array.isArray(value)) { + return value.map((item) => deepClone(item)); + } + + // Handle objects - create a new object with cloned properties + if (typeof value === 'object') { + const cloned: any = {}; + Object.keys(value).forEach((key) => { + cloned[key] = deepClone(value[key]); + }); + return cloned; + } + + // Handle primitives (string, number, boolean, etc.) + return value; +} + export function deepMerge(target: any, source: any) { - const result = { ...target }; - Object.keys(source).forEach((key) => { - if (isObj(target[key]) && isObj(source[key])) { - result[key] = deepMerge(target[key], source[key]); + // Handle null/undefined inputs by treating them as empty objects + const safeTarget = target || {}; + const safeSource = source || {}; + + const result = { ...safeTarget }; + Object.keys(safeSource).forEach((key) => { + if (isObj(safeTarget[key]) && isObj(safeSource[key])) { + result[key] = deepMerge(safeTarget[key], safeSource[key]); } else { - result[key] = source[key]; + // Clone the source value to prevent mutation of the original + result[key] = deepClone(safeSource[key]); } }); return result; diff --git a/lib/request-wrapper.ts b/lib/request-wrapper.ts index 04034c07..604e81d9 100644 --- a/lib/request-wrapper.ts +++ b/lib/request-wrapper.ts @@ -246,7 +246,7 @@ export class RequestWrapper { * @throws Error */ public async sendRequest(parameters): Promise { - const options = deepMerge(parameters.defaultOptions || {}, parameters.options || {}); + const options = deepMerge(parameters.defaultOptions, parameters.options); const { path, body, form, formData, qs, method, serviceUrl, axiosOptions } = options; let { headers, url } = options; diff --git a/test/unit/request-wrapper.test.js b/test/unit/request-wrapper.test.js index e56dd34e..92735721 100644 --- a/test/unit/request-wrapper.test.js +++ b/test/unit/request-wrapper.test.js @@ -1294,6 +1294,188 @@ describe('gzipRequestBody', () => { expect(requestWrapperInstance.retryInterceptorId).toBeUndefined(); expect(requestWrapperInstance.raxConfig).toBeUndefined(); }); + + describe('Query Parameter Array Mutation', () => { + it('should not mutate original array query parameters after sendRequest', async () => { + // This simulates the Secrets Manager use case where groups is an array + const originalGroups = ['group-id-123']; + const originalParams = { + limit: 200, + offset: 0, + groups: originalGroups, + sort: 'name', + }; + + const parameters = { + defaultOptions: { + url: 'https://api.example.com/v1/secrets', + method: 'GET', + headers: {}, + }, + options: { + qs: originalParams, + }, + }; + + // Mock axios to prevent actual HTTP call + const mockAxiosResponse = { + status: 200, + statusText: 'OK', + headers: {}, + config: {}, + data: { secrets: [] }, + }; + + requestWrapperInstance.request = jest.fn().mockResolvedValue(mockAxiosResponse); + + try { + await requestWrapperInstance.sendRequest(parameters); + } catch (e) { + // Ignore errors, we're testing mutation + } + + // The critical test: the original array should still be an array + expect(Array.isArray(originalGroups)).toBe(true); + expect(originalGroups).toEqual(['group-id-123']); + expect(typeof originalGroups[0]).toBe('string'); + + // Also check the original params object + expect(Array.isArray(originalParams.groups)).toBe(true); + expect(originalParams.groups).toEqual(['group-id-123']); + }); + + it('should not mutate nested query parameters in defaultOptions', async () => { + const originalGroups = ['group-id-456', 'group-id-789']; + + const parameters = { + defaultOptions: { + url: 'https://api.example.com/v1/secrets', + method: 'GET', + headers: {}, + qs: { + groups: originalGroups, + limit: 100, + }, + }, + options: {}, + }; + + const mockAxiosResponse = { + status: 200, + statusText: 'OK', + headers: {}, + config: {}, + data: { secrets: [] }, + }; + + requestWrapperInstance.request = jest.fn().mockResolvedValue(mockAxiosResponse); + + try { + await requestWrapperInstance.sendRequest(parameters); + } catch (e) { + // Ignore errors + } + + // Original array should remain unchanged + expect(Array.isArray(originalGroups)).toBe(true); + expect(originalGroups).toEqual(['group-id-456', 'group-id-789']); + expect(originalGroups.length).toBe(2); + }); + + it('should handle multiple array parameters without mutation', async () => { + const groups = ['group-1', 'group-2']; + const tags = ['tag-a', 'tag-b', 'tag-c']; + + const parameters = { + defaultOptions: { + url: 'https://api.example.com/v1/secrets', + method: 'GET', + headers: {}, + }, + options: { + qs: { + groups, + tags, + limit: 50, + }, + }, + }; + + const mockAxiosResponse = { + status: 200, + statusText: 'OK', + headers: {}, + config: {}, + data: { secrets: [] }, + }; + + requestWrapperInstance.request = jest.fn().mockResolvedValue(mockAxiosResponse); + + try { + await requestWrapperInstance.sendRequest(parameters); + } catch (e) { + // Ignore errors + } + + // Both arrays should remain unchanged + expect(Array.isArray(groups)).toBe(true); + expect(groups).toEqual(['group-1', 'group-2']); + + expect(Array.isArray(tags)).toBe(true); + expect(tags).toEqual(['tag-a', 'tag-b', 'tag-c']); + }); + + it('should allow reusing the same parameters object for multiple requests', async () => { + const groups = ['group-id-xyz']; + + const parameters = { + defaultOptions: { + url: 'https://api.example.com/v1/secrets', + method: 'GET', + headers: {}, + }, + options: { + qs: { + groups, + offset: 0, + limit: 100, + }, + }, + }; + + const mockAxiosResponse = { + status: 200, + statusText: 'OK', + headers: {}, + config: {}, + data: { secrets: [] }, + }; + + requestWrapperInstance.request = jest.fn().mockResolvedValue(mockAxiosResponse); + + // First request + try { + await requestWrapperInstance.sendRequest(parameters); + } catch (e) { + // Ignore + } + + // Array should still be valid for second request + expect(Array.isArray(groups)).toBe(true); + expect(groups).toEqual(['group-id-xyz']); + + // Second request with same parameters + try { + await requestWrapperInstance.sendRequest(parameters); + } catch (e) { + // Ignore + } + + // Array should still be unchanged after second request + expect(Array.isArray(groups)).toBe(true); + expect(groups).toEqual(['group-id-xyz']); + }); + }); }); function makeCopy(obj) { From bb9c91c48eda409c1796b4e10df65421348988c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=ADdia=20Tarcza?= <100163235+diatrcz@users.noreply.github.com> Date: Wed, 13 May 2026 15:50:08 +0200 Subject: [PATCH 2/2] fix: add lint fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Lídia Tarcza <100163235+diatrcz@users.noreply.github.com> --- test/unit/request-wrapper.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/unit/request-wrapper.test.js b/test/unit/request-wrapper.test.js index 92735721..d65a4e45 100644 --- a/test/unit/request-wrapper.test.js +++ b/test/unit/request-wrapper.test.js @@ -1338,7 +1338,7 @@ describe('gzipRequestBody', () => { expect(Array.isArray(originalGroups)).toBe(true); expect(originalGroups).toEqual(['group-id-123']); expect(typeof originalGroups[0]).toBe('string'); - + // Also check the original params object expect(Array.isArray(originalParams.groups)).toBe(true); expect(originalParams.groups).toEqual(['group-id-123']); @@ -1346,7 +1346,7 @@ describe('gzipRequestBody', () => { it('should not mutate nested query parameters in defaultOptions', async () => { const originalGroups = ['group-id-456', 'group-id-789']; - + const parameters = { defaultOptions: { url: 'https://api.example.com/v1/secrets', @@ -1379,13 +1379,13 @@ describe('gzipRequestBody', () => { // Original array should remain unchanged expect(Array.isArray(originalGroups)).toBe(true); expect(originalGroups).toEqual(['group-id-456', 'group-id-789']); - expect(originalGroups.length).toBe(2); + expect(originalGroups).toHaveLength(2); }); it('should handle multiple array parameters without mutation', async () => { const groups = ['group-1', 'group-2']; const tags = ['tag-a', 'tag-b', 'tag-c']; - + const parameters = { defaultOptions: { url: 'https://api.example.com/v1/secrets', @@ -1420,14 +1420,14 @@ describe('gzipRequestBody', () => { // Both arrays should remain unchanged expect(Array.isArray(groups)).toBe(true); expect(groups).toEqual(['group-1', 'group-2']); - + expect(Array.isArray(tags)).toBe(true); expect(tags).toEqual(['tag-a', 'tag-b', 'tag-c']); }); it('should allow reusing the same parameters object for multiple requests', async () => { const groups = ['group-id-xyz']; - + const parameters = { defaultOptions: { url: 'https://api.example.com/v1/secrets',