techno-tim / littlelink-server

A lightweight, open source, stateless, and self-hosted alternative to linktree in a Docker container!
https://links.technotim.live
MIT License
901 stars 164 forks source link

[BUG] Server sending x-powered-by despite code setting it to disabled #211

Closed samip5 closed 2 years ago

samip5 commented 2 years ago

Buttons Relation

N/A

Bug Description

It seems that despite: https://github.com/techno-tim/littlelink-server/blob/8fff00eb76c13268f35004181de72157b0271de3/src/server.js#L58

x-powered-by: Express

My requests do get that header still?

Expected Result

I would have expected it to not show up.

Additional Information

Links: https://samip.dev

Images

No response

RangersMS commented 2 years ago

I have a fix for that here. It is working for me. I have build the docker image and run it and the header is gone. Build process went without any issue. Unfortunately yarn ci has something to complain about every what looks to me like the line break in every file. I guess it's some issue with my setup since it's complaining about almost every single file and I only touched three of them. But still I try to resolve the issue with yarn ci before I create a pull request.

This is an example of a file I haven't touched:

/usr/src/app/src/index.js
   1:32  error  Delete `␍`  prettier/prettier
   2:31  error  Delete `␍`  prettier/prettier
   3:1   error  Delete `␍`  prettier/prettier
   4:39  error  Delete `␍`  prettier/prettier
   5:1   error  Delete `␍`  prettier/prettier
   6:18  error  Delete `␍`  prettier/prettier
   7:40  error  Delete `␍`  prettier/prettier
   8:52  error  Delete `␍`  prettier/prettier
   9:10  error  Delete `␍`  prettier/prettier
  10:41  error  Delete `␍`  prettier/prettier
  11:22  error  Delete `␍`  prettier/prettier
  12:28  error  Delete `␍`  prettier/prettier
  13:6   error  Delete `␍`  prettier/prettier
  14:6   error  Delete `␍`  prettier/prettier
  15:47  error  Delete `␍`  prettier/prettier
  16:2   error  Delete `␍`  prettier/prettier
  17:1   error  Delete `␍`  prettier/prettier
  18:39  error  Delete `␍`  prettier/prettier
  19:1   error  Delete `␍`  prettier/prettier
  20:25  error  Delete `␍`  prettier/prettier
  21:43  error  Delete `␍`  prettier/prettier
  22:25  error  Delete `␍`  prettier/prettier
  23:15  error  Delete `␍`  prettier/prettier
  24:26  error  Delete `␍`  prettier/prettier
  25:14  error  Delete `␍`  prettier/prettier
  26:6   error  Delete `␍`  prettier/prettier
  27:46  error  Delete `␍`  prettier/prettier
  28:6   error  Delete `␍`  prettier/prettier

Maybe someone has a hint for me on how to resolve the yarn issue? Have tried to change between CLRF and LF without any difference. Have run the yarn commands on a Windows system in Docker with node:16.16.0-alpine.

Other than that I have used helmet to solve the issue. I don't know if @timothystewart6 likes that solution. It's adding a new dependency which should be avoidable since you could achieve the same solution with native express methods like server.disable('x-powered-by') or server.set('x-powered-by', false). But for some reason it's not working. As @samip5 pointed out the original source code already contains server.disable('x-powered-by').

On the other hand helmet helps to set various security related HTTP headers. Not just removing the x-powered-by header. For now I have only used helmet.hidePoweredBy() in order to avoid unnecessary changes for this issue. But you could extend the use of helmet and set other headers as well.

timothystewart6 commented 2 years ago

Run yarn lint --fix

That will fix lint

RangersMS commented 2 years ago

Thanks a lot! It worked. All tests passed. It also made git realize that 21 files changed. Obviously without any visible change. Am I supposed to commit all of them? Or was the fix just for my local environment so that I can run the test and I just discard these changes? Screenshot_2022-09-01_23-49-16_c1548dc5-74d5-458c-9389-665f272d8cd1

RangersMS commented 2 years ago

pr is open. missed to reference this issue.

RangersMS commented 2 years ago

ok, pr is closed. What about writing an own middleware which removes the header on every request. Instead of using helmet?

RangersMS commented 2 years ago

btw my workaround for that is to remove the header with my reverse proxy in front of littlelink. In my case traefik.

- "traefik.http.middlewares.littlelink-headers.headers.customresponseheaders.x-powered-by="
- "traefik.http.routers.littlelink.middlewares=littlelink-headers"

I'm actually thinking of adding this traefik middleware globally in front of every http router. I can't think of a reason to leave this header in any response. Like this example with a dedicated middleware:

static traefik config:

...
entryPoints:
  web:
    address: ":80"
    http:
      middlewares:
        - remove-x-powered-by-header@file

  websecure:
    address: ":443"
    http:
      middlewares:
        - remove-x-powered-by-header@file
...

x-powered-by-header.yml

http:
  middlewares:
    remove-x-powered-by-header:
      headers:
        customresponseheaders:
          x-powered-by: ""
timothystewart6 commented 2 years ago

I am actually fine with adding helmet for this if you want to reopen your PR. https://github.com/techno-tim/littlelink-server/pull/222 I just want to be sure it doesn't break anything else like resources cross domain like images.

RangersMS commented 2 years ago

I just used the function to remove the x-powered-by header as it was the minimum needed to resolve this issue. server.use(helmet.hidePoweredBy()); In my testing everything worked as expected.

If you use helmet like server.use(helmet()); it will add a bunch of other headers. I guess you had issues with Content Security Policy (CSP) as it will be set like this by default:

Content-Security-Policy: default-src 'self';base-uri 'self';font-src 'self' https: data:;form-action 'self';frame-ancestors 'self';img-src 'self' data:;object-src 'none';script-src 'self';script-src-attr 'none';style-src 'self' https: 'unsafe-inline';upgrade-insecure-requests

I'm 99.9999% sure my fix won't break anything. But just to be sure I'll just test it again.

PS: also fixed the root cause for my issue with prettier 🥳. Was a bad combination of my local git config on Windows messing up with LF and CLRF and using a devcontainer in VS Code. git config --global core.autocrlf false Resolving Git line ending issues in containers (resulting in many modified files)

timothystewart6 commented 2 years ago

Nice! yeah for sure! I am cool with reopening that PR to add helmet with only hidePoweredBy() function enabled, and then we can expand on it later and tighten it up. You're exactly right, it was the CSP rule that was blocking images from other domains!

RangersMS commented 2 years ago

Still had a local copy of that code. Build an image from that and it works as I expected. Profile picture from Twitter was loaded and x-powered-by header is missing. Screenshot_2022-09-10_21-39-35_e288377d-ea26-473b-8bb8-b662c4249350

Well let's see what happens when I reopen that PR after I deleted the feature branch. Had already deleted it and started a new one from scratch to find an other solution.

RangersMS commented 2 years ago

ok, can not be reopend. Have to do a new one. Might take a moment just to redo it and test it.

RangersMS commented 2 years ago

ok new PR is open with the helmet.hidePoweredBy() solution.

timothystewart6 commented 2 years ago

Closed by #https://github.com/techno-tim/littlelink-server/commit/e5d0eb047c9e05ecabdc680bbbb9a196c50f7afe