Skip to content

fix(net): prevent SSRF bypass via redirect in safeFetch#486

Open
HiddenPuppy wants to merge 1 commit intomainfrom
fix/406-safefetch-redirect-ssrf-bypass
Open

fix(net): prevent SSRF bypass via redirect in safeFetch#486
HiddenPuppy wants to merge 1 commit intomainfrom
fix/406-safefetch-redirect-ssrf-bypass

Conversation

@HiddenPuppy
Copy link
Copy Markdown
Collaborator

Problem

safeFetch runs SSRF guards (HTTPS-only check + assertNotPrivateHost) against the initial URL, then calls net.fetch(url) without setting redirect. Electron's net.fetch follows HTTP redirects (3xx) by default — the second request goes completely unchecked. An attacker-controlled HTTPS endpoint can respond with 302 Location: http://127.0.0.1/admin and bypass the SSRF guard entirely. Protocol downgrade (HTTPS → HTTP) is also allowed for the same reason.

Fix

Pass redirect: 'error' to net.fetch() so any 3xx response throws immediately instead of being followed. This is the simplest and safest option — safeFetch fetches static assets (images, files) that should never redirect through untrusted hops.

Verification

  • pnpm check passes (49 test files, 308 tests, all guards)
  • Unit test: asserts net.fetch is called with { redirect: 'error' }

Closes #406

safeFetch runs SSRF guards (HTTPS-only check + assertNotPrivateHost)
against the initial URL, but Electron's net.fetch follows HTTP redirects
(3xx) by default without re-running any checks. An attacker-controlled
endpoint can respond with 302 Location: http://127.0.0.1/admin to
bypass the SSRF guard entirely.

Fix: pass redirect:'error' to net.fetch so any 3xx response throws
immediately instead of being followed.

Closes #406
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 addresses a security vulnerability in the safeFetch utility where HTTP redirects could be used to bypass SSRF guards. By forcing net.fetch to reject redirects, the implementation ensures that requests remain within the scope of the initial security checks, preventing attackers from redirecting traffic to private hosts or downgrading protocols.

Highlights

  • SSRF Mitigation: Updated safeFetch to prevent SSRF bypasses by configuring net.fetch to reject HTTP redirects.
  • Security Hardening: Added redirect: 'error' to the fetch options, ensuring that any 3xx response triggers an immediate error instead of following potentially malicious redirects.
  • Testing: Added a new unit test to verify that net.fetch is correctly invoked with the redirect: 'error' option.
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 the 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 counterproductive. 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.

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Hi @HiddenPuppy,
Thanks for your pull request!
If the PR is ready, use the /auto-cc command to assign Reviewer to Review.
We will review it shortly.

Details

Instructions for interacting with me using comments are available here.
If you have questions or suggestions related to my behavior, please file an issue against the gh-ci-bot repository.

Copy link
Copy Markdown
Contributor

@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 enhances security in the safeFetch function by adding redirect: 'error' to the net.fetch call, preventing SSRF bypasses via 3xx redirects. A corresponding test case was also added. Feedback suggests further hardening the security by using the normalized parsed.href instead of the raw url string and improving the test by asserting the exact URL passed to the fetch call.

const timeout = setTimeout(() => controller.abort(), opts.timeoutMs ?? DEFAULT_TIMEOUT_MS);
try {
const res = await net.fetch(url, { signal: controller.signal });
const res = await net.fetch(url, { signal: controller.signal, redirect: 'error' });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-medium medium

To prevent potential SSRF bypasses via URL parser differentials, it is safer to use the normalized URL from the parsed object rather than the original url string. This ensures that the URL fetched by net.fetch is exactly the same one that was validated by the protocol and hostname checks. This is a recommended security practice when implementing SSRF guards to avoid discrepancies between how different libraries parse the same URL string.

Suggested change
const res = await net.fetch(url, { signal: controller.signal, redirect: 'error' });
const res = await net.fetch(parsed.href, { signal: controller.signal, redirect: 'error' });
References
  1. The style guide emphasizes guarding URLs and treating the Electron main process as a security boundary. Using normalized URLs prevents bypasses due to differences in how various components parse the same string. (link)

netFetchMock.mockResolvedValue(mockResponse(new ArrayBuffer(4)));

await safeFetch('https://cdn.example.com/img.png');
expect(netFetchMock).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ redirect: 'error' }));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Instead of using expect.any(String), it is better to assert the exact URL being passed to net.fetch. This ensures that the function is fetching the intended resource and that any normalization (like using parsed.href) is correctly applied, making the test more robust against regressions.

Suggested change
expect(netFetchMock).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ redirect: 'error' }));
expect(netFetchMock).toHaveBeenCalledWith('https://cdn.example.com/img.png', expect.objectContaining({ redirect: 'error' }));

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.

[Bug] safeFetch follows redirects without re-running SSRF check

1 participant