Skip to content

feat: Add RFC 6761–compliant localhost loopback checks so secure cookies work on localhost (fixes: #1676) #4038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Chriss4123
Copy link
Contributor

@Chriss4123 Chriss4123 commented Feb 14, 2025

fixes: #1676

Jira: https://usebruno.atlassian.net/browse/BRU-859

Description

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

This commit extends how cookies are treated in secure contexts by fully recognizing localhost and loopback IPs as trustworthy origins, matching the de facto behavior of all modern browsers and RFC 6761. Previously, tough-cookie defaulted secure to true only for https: and wss: URLs, causing cookies with secure set to never be sent to localhost.

What Changed

  1. New trustworthy-util.js

    • Implements isPotentiallyTrustworthy(url) by checking:
      • Schemes: https, wss, file
      • Loopback IPs: 127.0.0.1/8 and ::1
      • Hostnames: localhost and *.localhost
    • Adapted from Chromium’s IsLocalhost, IsLoopback and HostNoBracketsPiece, located at:
  2. cookies.js Update

    • Passes the { secure: isPotentiallyTrustworthy(url) } option to cookieJar.getCookiesSync().
    • This overrides tough-cookie’s built-in:
      let secure = options.secure;
      if (secure == null && (context.protocol == "https:" || context.protocol == "wss:")) {
        secure = true;
      }
      We do everything tough-cookie does by default—plus treat localhost/loopback the same as modern browsers. No existing functionality is removed, only expanded.

Testing this change

from flask import Flask, request, make_response

app = Flask(__name__)

@app.route('/setcookie')
def set_cookie():
    resp = make_response('')
    resp.set_cookie('example', '', secure=True)
    return resp

@app.route('/getcookie')
def get_cookie():
    return 'Cookie is set' if 'example' in request.cookies else 'No cookie set'

app.run()
  • Before: The secure cookie would not be sent to /getcookie after it was set by /setcookie due to secure being set to True.
  • After: The secure cookie would be sent to /getcookie which mirrors the behavior of modern browsers.

All modern browsers (Chrome et al.) and many other API testing clients (Postman et al.) handle http://localhost as a trustworthy origin. It only makes sense that this behavior also exists in Bruno.

naman-bruno
naman-bruno previously approved these changes Feb 21, 2025
@JoshuaHintze
Copy link

@sreelakshmi-bruno - Any chancel to get this merged in?

@sreelakshmi-bruno
Copy link
Collaborator

This will be merged and will be included in the upcoming release which will likely go out on coming Tuesday.

@sreelakshmi-bruno
Copy link
Collaborator

Apologies, we've postponed the release for this PR to April. Thank you for your patience!

@ramki-bruno
Copy link
Collaborator

@Chriss4123 I see that your PR on salesforce/tough-cookie is already merged. That alone is great news and thanks for your contributions.

Do you think we can just get straight to upgrading tough-cookie once their major release is out?

@Chriss4123
Copy link
Contributor Author

@Chriss4123 I see that your PR on salesforce/tough-cookie is already merged. That alone is great news and thanks for your contributions.

Do you think we can just get straight to upgrading tough-cookie once their major release is out?

Yes, that should work. Just keep in mind tough-cookie has been converted to TypeScript a while ago, however you are using an old version which still uses vanilla JS.

@ramki-bruno
Copy link
Collaborator

Since tough-cookie v6 is still in RC, I think we can go ahead with merging this for now.
I'm also adding few tests for this in #4709. Will add a note as well regarding upgrading it.

ramki-bruno
ramki-bruno previously approved these changes May 19, 2025
@ramki-bruno ramki-bruno force-pushed the feature/localhost-secure-context branch from fd73bdc to b198afb Compare May 19, 2025 12:44
@ramki-bruno
Copy link
Collaborator

Moved the util to @usebruno/common and fixed it for CLI as well.

@helloanoop
Copy link
Contributor

@ramki-bruno I'd like to keep the bruno-common platform agnostic as it gets bundled. Can you move the logic to bruno-requests ?

@ramki-bruno ramki-bruno force-pushed the feature/localhost-secure-context branch from b198afb to 88d943f Compare May 22, 2025 19:18
@ramki-bruno ramki-bruno force-pushed the feature/localhost-secure-context branch from 88d943f to 60f88a2 Compare May 23, 2025 07:05
@ramki-bruno ramki-bruno force-pushed the feature/localhost-secure-context branch from 60f88a2 to 2c3d2ff Compare May 23, 2025 08:21
@ramki-bruno
Copy link
Collaborator

Please use Merge commit

@Chriss4123 Chriss4123 dismissed stale reviews from ramki-bruno and naman-bruno via 2c3d2ff May 23, 2025 12:18
@lohit-bruno lohit-bruno merged commit 51f36b1 into usebruno:main May 26, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Cookies not sent
8 participants