Skip to content

Conversation

@narbit
Copy link
Contributor

@narbit narbit commented Sep 19, 2025

Fixes #267545

Copilot AI review requested due to automatic review settings September 19, 2025 23:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes localhost redirect URLs from OAuth configuration in favor of loopback IP (127.0.0.1) redirect URLs only. This change improves security by preventing potential localhost-based attacks while maintaining the same functionality through the more secure loopback IP addresses.

Key Changes

  • Removes http://localhost/ and http://localhost:${DEFAULT_AUTH_FLOW_PORT}/ from OAuth redirect URI configurations
  • Retains http://127.0.0.1/ and http://127.0.0.1:${DEFAULT_AUTH_FLOW_PORT}/ for local development authentication flows
  • Updates corresponding test assertions to match the new configuration

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/vs/base/common/oauth.ts Removes localhost redirect URIs from dynamic registration, keeping only loopback IP addresses
src/vs/base/test/common/oauth.test.ts Updates test expectations to match the removal of localhost redirect URIs

@narbit
Copy link
Contributor Author

narbit commented Sep 19, 2025

Addresses #267545

@narbit
Copy link
Contributor Author

narbit commented Sep 22, 2025

@narbit please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM. We only use 127.0.0.1 when we spin up the local server so this makes sense.

@vs-code-engineering vs-code-engineering bot added this to the September 2025 milestone Sep 22, 2025
@TylerLeonhardt TylerLeonhardt merged commit e6617b9 into microsoft:main Sep 23, 2025
17 checks passed
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Nov 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default loopback redirect for enhanced security and compatibility

4 participants