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

Remove duplicate caddyfile #89

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Remove duplicate caddyfile #89

wants to merge 2 commits into from

Conversation

maebeam
Copy link
Contributor

@maebeam maebeam commented Dec 9, 2021

This is not necessary because the frontend caddyfile has self and we no longer recommend the api.domain.com configuration by default.

@maebeam maebeam requested a review from a team as a code owner December 9, 2021 17:27
@maebeam
Copy link
Contributor Author

maebeam commented Dec 9, 2021

@tijno any thoughts on this change? Seems safe and clearly better to me

@tijno
Copy link
Contributor

tijno commented Dec 9, 2021

i can review in the morning - keeping caddyfiles in sync has been an issue in the past so reducing duppl files seems sensible.

am on mobile and want to look at impact on node setup.

will confirm in the morning

@maebeam
Copy link
Contributor Author

maebeam commented Dec 9, 2021

Thanks! No rush

@tijno
Copy link
Contributor

tijno commented Dec 10, 2021

I think there are 2 issues that will break things

  • FE set to bind to :80 but Docker/run dockerfile/compose uses :8080
  • Run has CSP for deso.run and api.deso.run, FE doesnt have these. deso.run is used for localhost setup, and is default in nginx config.

Besides that there are a bunch of other differences between the two caddy files - that are worth checking whether if changing them would break setups.

However - many of these may be legacy and no longer required. Maybe you could cast your eye over them and let me know.

Here is full list of the differences.

main Caddy section

  • [BREAKING] FE set to bind to :80 but Docker/run dockerfile/compose uses :8080
  • Run uses try_files {path} index.html but FE uses file index.html for anything but images/assets. Probably FE one is better and more secure as the Run one would allow access to any file that exists, before falling back to index.html
  • Run sets Access-Control-Allow-Methods and Access-Control-Allow-Origin, but FE doesnt - however settings are pretty wide so not sure this is issue
  • FE sets Cache-Control no-store - not sure this is needed for production, but as its SPA wont hurt either

connect-src

  • [BREAKING] Run has CSP for deso.run and api.deso.run, FE doesnt have these. deso.run is used for localhost setup, and is default in nginx setup.
  • Run is missig node.deso.org and related subdomains amp bithunt pulse - I think this is just because its not been updated since rebrand.
  • Run has api.bitpop.dev - not sure what this is used for - its missing from FE
  • Run has :* on explorer.bitclout.com - front end does not have the :* - not sure if all port access is needed
  • Run still has old api.bitclout.green, .blue and .navy - I dont think these are needed anymore.

script-src

  • Run has script-src https://cdn.jsdelivr.net/npm/sweetalert2@10, Not present in FE - i think maybe as its included in the build and no longer external

style-src

  • FE ads https://cdn.jsdelivr.net/npm/[email protected]/dist/css/bootstrap.min.css; which is not present in Run. I suspect this is legacy issue as its loaded from /vendor/bootstrap.min.css on FE

frame-src

  • FE adds https://geo.captcha-delivery.com - is this cloudflare related?
  • FE has https://youtube.com in addition to https://www.youtube.com
  • FE uses .tv for twitch, run uses .com

Other

  • FE adds frame-ancestors 'self' which is not present in Run

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