From a98d19e62c66242478222a827b6c2d4534fe766e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20Daba=C5=A1inskas?= Date: Wed, 15 Jul 2026 09:30:44 +0300 Subject: [PATCH 1/2] fix(mergedeep): preserve array shape when comparing empty targets When `compareDeep` encounters an empty target and a source array, it now returns additions as an array rather than an index-keyed object. This ensures consistent behavior whether the target is empty or contains existing entries. Previously, comparing `[]` with `[item]` would produce `additions: { 0: item }`, while comparing `[existing]` with `[existing, item]` would correctly produce `additions: [item]`. This inconsistency caused issues when applying diffs to repositories receiving their first array-based configuration. Also ensures the `deletions` property is always included in the result object for consistency. --- lib/mergeDeep.js | 17 +++++++++++++---- test/unit/lib/mergeDeep.test.js | 25 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/mergeDeep.js b/lib/mergeDeep.js index ab278e5c2..25314d31b 100644 --- a/lib/mergeDeep.js +++ b/lib/mergeDeep.js @@ -82,8 +82,17 @@ class MergeDeep { // If the target is empty, then all the source is added to additions if (t === undefined || t === null || (this.isEmpty(t) && !this.isEmpty(s))) { - additions = Object.assign(additions, s) - return ({ additions, modifications, hasChanges: true }) + if (firstInvocation && Array.isArray(s)) { + // Match the shape of the non-empty-target path, which unwinds the + // `__array` wrapper before returning: a top-level array diff must come + // back as an array, not as the index-keyed object Object.assign builds. + additions = s.slice() + } else { + // Assign into the container passed by recursive callers so they observe + // the additions through their own reference. + additions = Object.assign(additions, s) + } + return ({ additions, modifications, deletions, hasChanges: true }) } // Compare the entries in the objects or elements of the array @@ -92,8 +101,8 @@ class MergeDeep { // So any property in the target that is not in the source is not treated as a deletion for (const key in source) { // Skip prototype pollution vectors - if (key === "__proto__" || key === "constructor") { - continue; + if (key === '__proto__' || key === 'constructor') { + continue } // Logic specific for Github // API response includes urls for resources, or other ignorable fields; we can ignore them diff --git a/test/unit/lib/mergeDeep.test.js b/test/unit/lib/mergeDeep.test.js index dd8bd60ba..842204ffa 100644 --- a/test/unit/lib/mergeDeep.test.js +++ b/test/unit/lib/mergeDeep.test.js @@ -809,6 +809,7 @@ entries: ] }, modifications: {}, + deletions: {}, hasChanges: true } const ignorableFields = [] @@ -1882,4 +1883,28 @@ branches: expect(same.additions).toEqual({}) expect(same.modifications).toEqual({}) }) + + it('CompareDeep returns top-level array additions in the same shape for empty and non-empty targets', () => { + const mergeDeep = new MergeDeep(log, jest.fn(), []) + const ruleset = { name: 'first-ruleset', enforcement: 'active' } + + // Target already has an entry: additions come back as an array + const withExisting = mergeDeep.compareDeep( + [{ name: 'other', enforcement: 'active' }], + [{ name: 'other', enforcement: 'active' }, ruleset] + ) + expect(withExisting.additions).toEqual([ruleset]) + + // Empty target (e.g. a repo receiving its first ruleset) must produce the + // same shape, not an index-keyed object like { 0: { ... } } + const firstEntry = mergeDeep.compareDeep([], [ruleset]) + expect(firstEntry.additions).toEqual([ruleset]) + expect(firstEntry.hasChanges).toEqual(true) + }) + + it('CompareDeep always includes deletions in its result', () => { + const mergeDeep = new MergeDeep(log, jest.fn(), []) + const emptyTarget = mergeDeep.compareDeep([], [{ name: 'a' }]) + expect(emptyTarget).toHaveProperty('deletions') + }) }) From c1adf01ea3282fddfa10562d13d0d48ae0ba3e6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20Daba=C5=A1inskas?= Date: Wed, 15 Jul 2026 09:55:34 +0300 Subject: [PATCH 2/2] fix(mergedeep): skip prototype pollution in empty target comparison When comparing an empty target object against a source, the code now explicitly copies keys instead of using Object.assign. This prevents prototype pollution vectors like `__proto__` and `constructor` from being copied into the additions object. The fix mirrors the existing protection in the comparison loop and ensures that malicious keys parsed from JSON cannot pollute the prototype chain through the additions reference. --- lib/mergeDeep.js | 10 ++++++++-- test/unit/lib/mergeDeep.test.js | 9 +++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/mergeDeep.js b/lib/mergeDeep.js index 25314d31b..0dd87a259 100644 --- a/lib/mergeDeep.js +++ b/lib/mergeDeep.js @@ -89,8 +89,14 @@ class MergeDeep { additions = s.slice() } else { // Assign into the container passed by recursive callers so they observe - // the additions through their own reference. - additions = Object.assign(additions, s) + // the additions through their own reference. Copy keys explicitly to + // skip prototype pollution vectors, like the comparison loop does. + for (const key of Object.keys(s)) { + if (key === '__proto__' || key === 'constructor') { + continue + } + additions[key] = s[key] + } } return ({ additions, modifications, deletions, hasChanges: true }) } diff --git a/test/unit/lib/mergeDeep.test.js b/test/unit/lib/mergeDeep.test.js index 842204ffa..cb68e39c2 100644 --- a/test/unit/lib/mergeDeep.test.js +++ b/test/unit/lib/mergeDeep.test.js @@ -1907,4 +1907,13 @@ branches: const emptyTarget = mergeDeep.compareDeep([], [{ name: 'a' }]) expect(emptyTarget).toHaveProperty('deletions') }) + + it('CompareDeep skips prototype pollution vectors when the target is empty', () => { + const mergeDeep = new MergeDeep(log, jest.fn(), []) + // JSON.parse creates `__proto__` as an own key, unlike object literals + const source = JSON.parse('{"name": "a", "__proto__": { "polluted": true }}') + const result = mergeDeep.compareDeep({}, source) + expect(result.additions).toEqual({ name: 'a' }) + expect(result.additions.polluted).toBeUndefined() + }) })