Skip to content
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

🐛 fix csp for local env not recognizing localhost a valid #1591

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

michenly
Copy link
Contributor

@michenly michenly commented Dec 21, 2023

WHY are these changes introduced?

The CSP bug was found when using ngrok as public domain for local development (needed for customer account API development)

Steps to re-produce:

  1. Follow public domain using ngrok with this guide
  2. In a seperate terminal, start hydrogen with npm run dev:app
  3. Goto above domain to see the errors in browser

Screenshot 2023-12-21 at 1 40 59 PM

WHAT is this pull request doing?

With a public domain, localhost:* SHOULD have been applied to enable the use of localhost:3100 for assets build. This rule was never hit while using localhost:3000 as domain since self rule has precedent.

This PR fix the rule by adding http protocol to the policy.

HOW to test your changes?

Follow the above steps to reproduce and see the errors for all asset related CSP errors goes away in browser.

Post-merge steps

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

@michenly michenly self-assigned this Dec 21, 2023
@github-actions github-actions bot had a problem deploying to preview December 21, 2023 19:03 Failure
@Shopify Shopify deleted a comment from github-actions bot Dec 21, 2023
@michenly michenly merged commit 970073e into main Dec 21, 2023
10 checks passed
@michenly michenly deleted the mc-csp-local-fix branch December 21, 2023 21:04
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.

2 participants