Security: run web container as non-root user and add no-new-privileges#1606
Security: run web container as non-root user and add no-new-privileges#1606Oykunle wants to merge 2 commits into
Conversation
|
Issue #1607 was opened to address the limitations listed in the PR description. |
huss
left a comment
There was a problem hiding this comment.
@Oykunle Thank you for working on this issue. I made one code comment to consider. I also wanted to ask a few overall questions:
- I would like to ask how you tested the user within the Docker OED web container and the file owner. I'm not saying there is an issue but when I opened a terminal on that container it started as root and files were owned by the normal user (such as node). Thanks.
- You applied these changes to the web container. Do you have thoughts on changes to the database container? Should they apply to the cypress container (though security on this is probably less important)?
I did some testing for production and non (developer) running and did not find any issues. I could not test a new clone because of the startup issue with OED development. Once that is resolved I should be able to finish all testing.
Let me know if anything is not clear or you have questions/thoughts.
|
For verifying the user and file ownership, I tested it directly using: docker compose run --rm --entrypoint sh web -lc 'whoami && id' I also checked the app directory: docker compose run --rm --entrypoint sh web -lc 'ls -ld /usr/src/app' and confirmed it’s owned by appuser:appgroup. I also did a quick write test (touch / rm) in that directory to make sure there were no permission issues with the mounted volume. For the terminal showing root, I suspect that depends on how the shell/session is opened. In my checks, the runtime execution context (via docker compose run ...) is using the non-root user. For the database container, I didn’t apply the same changes here since it has different persistence and filesystem requirements, and I didn’t want to expand the scope of this PR without validating that more carefully. I’d be open to exploring that separately if we want to extend the hardening there. For the cypress container, PR #1559 already introduced similar protections, so I treated the web container as the main missing piece. Also good to hear your testing didn’t surface any issues. Let me know if there’s anything else you’d like me to check or validate. |
|
@Oykunle I've spent a little time trying to figure out the user and owner of files.
Note I did all of these after removing the current oed docker containers ( I welcome any and all thoughts at this point. |
|
Thanks again for taking the time to check this. After your comment, I re-tested from a completely clean state and was initially able to reproduce the same behavior you saw (container running as root). It turned out the Dockerfile in this branch was not in the expected state at that point, so the non-root user setup was not actually being applied. I’ve now restored the intended Dockerfile changes and re-verified everything from scratch: docker compose down -v Then checked the running container directly: docker exec -it $(docker ps -qf "name=web") whoami This now returns: So the web container is now correctly running as the non-root user, and file ownership is aligned with that user. I also verified write access in /usr/src/app to ensure there are no permission issues. Regarding the earlier observation where some shells appeared as root, the difference seems to come from how the shell is opened. In some cases, docker exec may default to root, but the container’s runtime user defined in the Dockerfile (USER appuser) is now correctly applied. Let me know if you’d like me to check anything else. |
|
@Oykunle Thank you for the additional information. My experience was that until I did: It stayed as root but afterward it was the appuser. Given this can you elaborate on these:
If it is needed then OED needs to work out how an upgrade for this change would be put into effect for existing sites. Developers might be less critical but that needs some thinking too. Any thoughts? |
Thanks for checking this out. From what I saw while testing, the main thing that makes the difference is rebuilding the web image:
That’s what actually picks up the updated Dockerfile with So all three together are a good, safe way to do it, but the rebuild is really the important part. I also agree with your point, existing setups will keep running as root unless they rebuild the image. So this change won’t take effect automatically. For upgrades, I think the best approach would be to clearly document this (maybe in release notes) with the exact commands to run. Developers might not be as critical, but they’d still need to rebuild locally if they want to see the updated behavior. |
Description
This PR improves container security by addressing privilege escalation risks in the OED web container.
Previously, the web container ran as the root user, which increases the potential impact if the container is compromised. This change ensures the container runs as a non-root user and prevents privilege escalation.
Changes include:
appuser) and group (appgroup) with fixed UID/GID10001no-new-privileges:trueto the web service configurationContext:
PR #1559 introduced similar protections for the cypress container but did not extend them to the web container due to concerns about development behavior. This PR extends that hardening effort to the web container.
Validation:
appuser(uid/gid10001) usingdocker exec/usr/src/app/usr/src/appto confirm no permission issuesType of change
Checklist
Limitations