Conversation
johnwatson484
left a comment
There was a problem hiding this comment.
Looks good, I think this will help a lot of our teams as I often see this discussed with varying decisions being made at the end.
It may also be worth opening a PR is the https://github.com/DEFRA/defra-docker-node and https://github.com/DEFRA/defra-docker-dotnetcore repos as the example Dockerfiles contradict this guidance?
I've added a few suggested tweaks too.
| **Why this works:** | ||
| - Files are owned by `root:root` | ||
| - The container runs as the `node` user (non-root) via `USER node` | ||
| - Since the `node` user doesn't own the files, it has no write permissions by default |
There was a problem hiding this comment.
I think this part is misleading as COPY will copy the files with permissions as they are. So if they have write access to that group/owner or everyone already, the COPY command won't alter that.
Perhaps adding a step to set the file permissions explicitly after but before switching to node user?
eg.
USER root
COPY --chown=root:root --from=development /home/node/app/ ./app/
COPY --chown=root:root --from=development /home/node/package*.json ./
RUN npm ci
RUN chmod -R a-w /home/node
USER node
| - This satisfies security scanning requirements in tools like Sonarqube without needing explicit `chmod` commands | ||
|
|
||
| **Approaches to avoid:** | ||
| - `COPY --chown=node:node` - gives write permissions to the running user |
There was a problem hiding this comment.
Depending on what the Node process is doing there may be scenarios where it needs to write data. Typically this would be to a mounted tmp or perhaps within a node_modules .cache directory, but that depends on the hosting environment for how these permissions can be set.
I don't think we need to go into specific use cases for explicit answers and we should absolutely favour no write permissions. But I think we should at least acknowledge that there are scenarios where write access is needed and that readers need to consider whether they have that need when following this guide.
| COPY --chown=root:root package*.json ./ | ||
| RUN npm install | ||
| COPY --chown=node:node app/ ./app/ | ||
| COPY --chown=root:root app/ ./app/ |
There was a problem hiding this comment.
Need to consider the impact on tests, nodemon etc here. If the node process is unable to write files then local development and testing may be compromised.
Perhaps suggest this only applies to the production build stage?
It will mean Sonar will continue to flag for a check of course. But I think that friction is better than the friction of not being able to develop in a container.
Description
Updates Docker guidance to address SonarCloud security vulnerability related to file permissions in COPY commands.
Problem
SonarCloud flags
COPY --chown=node:nodecommands with the error: "Make sure no write permissions are assigned to the copied resource." This occurs because giving the running user ownership of files grants write permissions, creating an unnecessary security risk - see https://rules.sonarsource.com/docker/type/Security%20Hotspot/RSPEC-6504/Solution
Changed all COPY commands from
--chown=node:nodeto--chown=root:rootin Dockerfile examples. This ensures:root:rootnode)chmodcommandsChanges Made
--chown=root:rootin both development and production stagesFiles Changed
docs/guides/docker_guidance.mdSecurity Impact
This change improves security posture by ensuring containerized applications follow the principle of least privilege - the running user cannot modify its own application files.