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

HOTT-4467: Add CSP policy to Duty Calculator app #730

Merged
merged 2 commits into from Dec 12, 2023
Merged

HOTT-4467: Add CSP policy to Duty Calculator app #730

merged 2 commits into from Dec 12, 2023

Conversation

rasikasri
Copy link
Contributor

@rasikasri rasikasri commented Dec 7, 2023

Jira link

HOTT-4467

What?

I have added/removed/altered:

  • Added content security policy

Why?

I am doing this because:
We should add a content security policy to reduce the risk of XSS problems in the sites

Have you? (optional)

  • Reviewed view changes with stake holders
  • Validated mobile responsive behaviour of view changes

Deployment risks (optional)

  • Makes changes to our complex routing setup that may affect apis to proxying to backend

jebw
jebw previously approved these changes Dec 7, 2023
Copy link
Contributor

@jebw jebw left a comment

Choose a reason for hiding this comment

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

LGTM - only query is the change to the 404 response, could probably represent as text/plain rather than text/html but seems minor

@@ -1,5 +1,5 @@
Rails.application.routes.draw do
root to: proc { [404, {}, ['Not found.']] }
root to: proc { [404, {'Content-Type' => 'text/html'}, ['Not found.']] }
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be 'text/plain' ? Its not actually HTML?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

by default it is 'text/plain', but then it adds an inline style at some point, there is no style added and no csp issue either.

Copy link
Contributor

@jebw jebw left a comment

Choose a reason for hiding this comment

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

LGTM

@rasikasri rasikasri merged commit ee0f6be into main Dec 12, 2023
15 checks passed
@rasikasri rasikasri deleted the HOTT-4467 branch December 12, 2023 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants