Add missing security headers #2362
Conversation
group-income
|
||||||||||||||||||||||||||||
| Project |
group-income
|
| Branch Review |
add-security-headers
|
| Run status |
|
| Run duration | 10m 46s |
| Commit |
|
| Committer | Snowteamer |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
10
|
Skipped
|
0
|
Passing
|
112
|
| View all changes introduced in this branch ↗︎ | |
backend/server.js
Outdated
| // Don't share context with potentially untrusted sites. | ||
| 'Cross-Origin-Opener-Policy': 'same-origin', | ||
| // Block loading the page in a frame. | ||
| 'X-Frame-Options': 'deny', |
There was a problem hiding this comment.
This should be DENY (uppercase)
backend/server.js
Outdated
| @@ -24,6 +24,23 @@ import { GIMessage } from '~/shared/domains/chelonia/GIMessage.js' | |||
|
|
|||
| const { CONTRACTS_VERSION, GI_VERSION } = process.env | |||
|
|
|||
| const securityHeaders = { | |||
| // This is the most likely to break things now; it may need changing when sandboxing or federation is implemented. | |||
| 'Content-Security-Policy': "default-src 'none'; script-src 'unsafe-eval'; script-src-elem 'self'; script-src-attr 'none'; style-src 'self'; style-src-elem 'self'; style-src-attr 'none'; img-src 'self' blob:; font-src 'self'; connect-src 'self'; media-src 'self'; prefetch-src 'self'; worker-src 'self'; frame-ancestors 'none'; form-action 'self'; upgrade-insecure-requests; manifest-src 'self'", | |||
There was a problem hiding this comment.
This will cause issues with the emoji images, since those are loaded externally. See #2334. Either img-src will need to be expanded, or the sheet be embedded locally (preferred solution).
There was a problem hiding this comment.
Is using img-src 'self' blob: https://unpkg.com OK for now? Otherwise, installing and redistributing emoji sheets via dist/assets seems a bit tricky regarding e.g. licensing issues.
There was a problem hiding this comment.
In terms of solving the CSP issue, yes (although you need to check manually, it could interfere with Cross-Origin-Embedder-Policy as well).
The preferred (to me) solution would be embedding the sheet. There could be some licensing concerns, but I'm not sure those concerns are alleviated by hotlinking. This is best left as a question to a lawyer or to the copyright holder. (Sidenote: fonts designs generally aren't under copyright in the US, but I'm guessing emoji designs likely are, and even if they weren't, there's other places to consider).
There was a problem hiding this comment.
An alternative solution is to drop the Apple emojis and use an emoji set with a free license. E.g., https://github.com/mozilla/fxemoji
|
Running the app locally and opening
|
There was a problem hiding this comment.
Nice work @snowteamer, it's coming along, still a few more browsers we need to test on.
BTW, if you'd like to test on BrowserStack, I'll send you credentials.
The site doesn't load on Safari on macOS:
And the other error your noticed in Chrome related to the favicon not being set also needs to be fixed, as we need the red dot to be visible when DMs are sent.
Fix #2291
Emoji sheet, audio asset loading and file upload in chat manually tested on:
Note that the favicon badge gets blocked in Chrome, but maybe it's acceptable, so as to avoid allowing
data:URLs?