fix(docker): keep root runtime to avoid bind-mount permission denied#375
fix(docker): keep root runtime to avoid bind-mount permission denied#375dingyufei615 merged 1 commit intomasterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses critical permission issues encountered when Docker containers attempted to access host bind-mounted directories. By reverting the Docker user from Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This PR aims to resolve Docker bind-mount permission issues by changing the container's runtime user from appuser to root. However, running the container as root introduces a significant security risk by violating the principle of least privilege. While this approach addresses the functional requirement, it is not a security best practice. It is strongly recommended to adopt a more secure alternative, such as using an entrypoint script to adjust directory permissions at container startup and then switching to a non-root user to run the application. This ensures the application runs with the minimum necessary privileges without sacrificing security.
| # 若容器以内置 appuser 运行,常见场景会出现写入权限错误(Errno 13)。 | ||
| # 为保证开箱即用与历史行为一致,这里保持 root 运行主进程。 | ||
| # 如需非 root 运行,请在部署侧统一处理宿主机目录 UID/GID。 | ||
| USER root |
There was a problem hiding this comment.
The container is configured to run as the root user, which significantly increases the risk and impact of a container escape or application compromise by violating the Principle of Least Privilege. While this change aims to resolve bind-mount permission issues, running applications as root in a container is a severe security risk. If an attacker gains code execution, they would have full root access. A more secure approach is to maintain the appuser and use an entrypoint script. This script would run as root initially to chown the necessary bind-mounted directories to appuser, and then use a tool like su-exec or gosu to switch to appuser before executing the main application command. This ensures that the application runs with minimal privileges, with root only used for the brief permission adjustment.
USER appuser
Motivation
appusercaused host bind-mounted directories (e.g.prompts/,jsonl/,logs/,images/) to be unreadable/writable due to UID/GID mismatches, producing[Errno 13] Permission deniedat runtime.Description
USER appuserwithUSER rootinDockerfileand add comments explaining the bind-mount UID/GID issue and recommending deployment-side UID/GID handling or an entrypointchownif non-root execution is required.Testing
git diff,git commit) completed successfully and CI/pytestshould be run after this change is pushed for full validation.Codex Task