Skip to content

Fix recursive JSON merge patch stack overflow#43

Closed
zknpr wants to merge 1 commit intomainfrom
json-merge-patch-recursion-fix-10449259625165348147
Closed

Fix recursive JSON merge patch stack overflow#43
zknpr wants to merge 1 commit intomainfrom
json-merge-patch-recursion-fix-10449259625165348147

Conversation

@zknpr
Copy link
Copy Markdown
Owner

@zknpr zknpr commented Feb 7, 2026

Fixed a recursive stack overflow vulnerability in JSON merge patch utilities by implementing a maximum recursion depth check. Added regression tests to verify the fix.


PR created automatically by Jules for task 10449259625165348147 started by @zknpr

This commit addresses a security vulnerability where deeply nested or cyclic JSON objects could cause a stack overflow in `generateMergePatch` and `applyMergePatch`.

- Introduces a `MAX_DEPTH` limit of 1000 for recursion in `src/core/json-utils.ts`.
- Updates `generateMergePatch` and `applyMergePatch` to throw an error if the depth limit is exceeded.
- Adds comprehensive tests in `tests/unit/json_utils_security.test.ts` to verify the fix and ensure no regressions.

This change ensures the extension is robust against malicious or accidentally deep JSON structures.

Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Feb 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sq-lite-explorer Ready Ready Preview, Comment Feb 7, 2026 1:41pm

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello @zknpr, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of JSON merge patch utilities by introducing a maximum recursion depth. This change prevents potential stack overflow vulnerabilities that could arise from processing excessively deep or cyclically referenced JSON structures during patch generation or application, thereby improving application stability. New tests have been added to validate this fix.

Highlights

  • Recursion Depth Limit: Introduced a MAX_DEPTH constant (set to 1000) to control recursion depth in JSON merge patch operations.
  • Vulnerability Fix: Implemented a recursion depth check in generateMergePatch and applyMergePatch functions, causing them to throw an error if the predefined MAX_DEPTH is exceeded, preventing potential stack overflows.
  • Regression Tests: Added new regression tests to tests/unit/json_utils_security.test.ts to validate the recursion depth limit for both generating and applying JSON merge patches, including scenarios with deep nesting and cyclic references.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • src/core/json-utils.ts
    • Added a MAX_DEPTH constant to define the maximum allowed recursion level.
    • Modified generateMergePatch to accept a depth parameter, incrementing it on recursive calls and throwing an error if MAX_DEPTH is surpassed.
    • Modified applyMergePatch to accept a depth parameter, incrementing it on recursive calls and throwing an error if MAX_DEPTH is surpassed.
  • tests/unit/json_utils_security.test.ts
    • Added a new test file dedicated to JSON merge patch security.
    • Included tests to verify that generateMergePatch correctly throws an error when processing deeply nested objects exceeding MAX_DEPTH.
    • Included tests to verify that applyMergePatch correctly throws an error when processing deeply nested objects exceeding MAX_DEPTH.
    • Added a test case to ensure the depth limit effectively handles cyclic references during patch generation.
Activity
  • PR was automatically created by Jules for task 10449259625165348147, initiated by @zknpr.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request effectively addresses a potential stack overflow vulnerability in the JSON merge patch utilities by introducing a recursion depth limit. The changes in generateMergePatch and applyMergePatch are correct and well-implemented. The addition of a new security-focused test suite (json_utils_security.test.ts) is excellent, as it includes regression tests for deep recursion and cyclic references, ensuring the fix is robust. I've suggested some improvements to the new tests to make them more concise by using assert.throws, which is a common pattern for asserting exceptions in Node.js tests.

Comment on lines +17 to +22
try {
generateMergePatch(original, modified);
assert.fail('Should have thrown depth limit error');
} catch (e: any) {
assert.match(e.message, /JSON merge patch depth limit exceeded/);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better readability and conciseness, you can use assert.throws() to verify that a function throws an error. This avoids the need for a try...catch block and assert.fail().

For example:

assert.throws(
    () => generateMergePatch(original, modified),
    /JSON merge patch depth limit exceeded/
);

Comment on lines +35 to +40
try {
applyMergePatch(target, patch);
assert.fail('Should have thrown depth limit error');
} catch (e: any) {
assert.match(e.message, /JSON apply merge patch depth limit exceeded/);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the previous test, you can use assert.throws() here for a more concise assertion. This will also resolve the inconsistent indentation within the try...catch block.

For example:

assert.throws(
    () => applyMergePatch(target, patch),
    /JSON apply merge patch depth limit exceeded/
);

Comment on lines +49 to +54
try {
generateMergePatch(original, modified);
assert.fail('Should have thrown depth limit error');
} catch (e: any) {
assert.match(e.message, /JSON merge patch depth limit exceeded/);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

You can also simplify this test case by using assert.throws(). This will also resolve the inconsistent indentation within the try...catch block.

For example:

assert.throws(
    () => generateMergePatch(original, modified),
    /JSON merge patch depth limit exceeded/
);

@zknpr
Copy link
Copy Markdown
Owner Author

zknpr commented Feb 7, 2026

Merged as part of v1.3.0 release in PR #65

@zknpr zknpr closed this Feb 7, 2026
@google-labs-jules
Copy link
Copy Markdown
Contributor

Merged as part of v1.3.0 release in PR #65

Thank you for the update. I am glad to hear the security fix has been included in the v1.3.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant