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

chore: remove upgrade_insecure_requests header in PocketIC gateway #2617

Closed
wants to merge 1 commit into from

Conversation

mraszyk
Copy link
Contributor

@mraszyk mraszyk commented Nov 14, 2024

This PR removes the upgrade_insecure_requests header in the PocketIC HTTP gateway if this gateways serves at http:// (context).

@mraszyk mraszyk requested a review from a team as a code owner November 14, 2024 18:19
@github-actions github-actions bot added the chore label Nov 14, 2024
@mraszyk mraszyk marked this pull request as draft November 14, 2024 18:23
Comment on lines +622 to +624
http_response
.headers_mut()
.remove(UPGRADE_INSECURE_REQUESTS);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamspofford-dfinity Could you please suggest how the headers should be modified here?

Choose a reason for hiding this comment

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

  • Upgrade-Insecure-Requests should not be blocked (or rather since it's a request header it shouldn't appear in the first place)
  • Strict-Transport-Security should be blocked
  • Content-Security-Policy, if it exists, should be modified to remove the upgrade-insecure-requests; directive, if it exists

Copy link
Contributor Author

@mraszyk mraszyk Nov 15, 2024

Choose a reason for hiding this comment

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

Modifying the content of the CSP header looks involved to me. I see the following alternatives:

  • block CSP entirely for localhost (this would make local testing of CSP impossible though);
  • modify the content of the CSP header when deploying an asset canister to the local network.

The latter seems to be the most robust option in my opinion.

@mraszyk mraszyk closed this Nov 18, 2024
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.

2 participants