Set node as user in Dockerfile + some refactoring#91
Open
OiYouYeahYou wants to merge 1 commit into
Open
Conversation
The most important part of these changes is deprivileging the user from `root` to `node` to mitigate escalation attacks. https://github.com/nodejs/docker-node/blob/main/docs/BestPractices.md#non-root-user Also, this refactors for simplicity by reducing much of the repetition
OiYouYeahYou
commented
Mar 8, 2025
| @@ -1,22 +1,17 @@ | |||
| FROM node:20-alpine AS development-dependencies-env | |||
| COPY . /app | |||
Author
There was a problem hiding this comment.
I'm uncertain if this was a typo or intentional. I can see that maybe in some cases there's a postbuild in the install, but this copy busts the cache basically every time
OiYouYeahYou
commented
Mar 8, 2025
| # Build app and prepare `node_modules` for production | ||
| # This second install will remove the omitted deps automatically | ||
| RUN npm run build \ | ||
| && npm ci --omit=dev |
Author
There was a problem hiding this comment.
Putting this in a the same stage as the previous one to reduce network, disk and time usage
OiYouYeahYou
commented
Mar 8, 2025
Contributor
|
@OiYouYeahYou I'm no Docker expert, but I'll try to get someone on the team to take a look Would you update all of the |
Member
|
CC/ @rubys Do we need any of these changes in https://github.com/fly-apps/dockerfile-node as well? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The most important part of these changes is deprivileging the user from
roottonodeto mitigate escalation attacks.The refactoring I've done is in the attempt of being a simple but solid Dockerfile for new and novice users by making it a little easier to understand with fewer repeated commands. Any resultant changes to built images should be trivial at best, which I've also checked with Go Dive to confirm.
There are other Dockerfiles in this repo which could be updated, but I want to open up with this first to test the waters.