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

Restore http and https for font-src in CSP for app dev hot module reloading #995

Closed
wants to merge 1 commit into from

Conversation

cnathe
Copy link
Contributor

@cnathe cnathe commented Feb 19, 2025

Rationale

The related PR made some changes to the default CSP. The change to the font-src property broke some things in the client side application hot module reloading dev experience related to how our fonts/icons are loaded.

Screenshot 2025-02-18 at 11 27 04 AM

Related Pull Requests

Changes

  • Restore http and https for font-src in CSP for app dev hot module reloading

@labkey-willm
Copy link
Contributor

this file is now the source for what gets deployed to prod. We can't add things back in without making corresponding changes to the "Copy CSP Blocks" action to strip them out before copying to the Dockerfile and Chef repos. I can do that but I'll wait for @labkey-adam to weigh in.

@labkey-adam
Copy link
Contributor

I have a local change that will allow us to add to font-src programmatically, just like we do connect-src currently. I think that's the right solution and should avoid updating the Copy action. Assuming code reviews and approvals, I should be able to merge this today.

@labkey-adam
Copy link
Contributor

Questions: Does this handle the ws: complaints as well? Can we narrow the font-src host to https://localhost:3001? http: https: is very broad and may allow undesired resources (on TeamCity, say).

@cnathe
Copy link
Contributor Author

cnathe commented Feb 19, 2025

@labkey-willm @labkey-adam Perfect. I was hoping that this PR would be a start to you both coming up with a better solution! Adam, I can try your suggestion of "narrow the font-src to https://localhost:3001" if you want to send me a patch.

@labkey-adam
Copy link
Contributor

@labkey-willm @labkey-adam Perfect. I was hoping that this PR would be a start to you both coming up with a better solution! Adam, I can try your suggestion of "narrow the font-src to https://localhost:3001" if you want to send me a patch.

Largely untested code available in the PRs (platform and server) I just assigned. Note that you'll need to gradlew pickpg or similar to pick up the application.properties changes.

@cnathe cnathe closed this Feb 20, 2025
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.

3 participants