fix: auto repair Docker mount permissions - #1263
Conversation
github-actions
Bot
added
ai
documentation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fb10d73e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| owner="$(stat -c '%u:%g' "$dir" 2>/dev/null || true)" | ||
| if [ "$owner" != "$APP_UID:$APP_GID" ]; then | ||
| if ! chown -R "$APP_UID:$APP_GID" "$dir"; then |
There was a problem hiding this comment.
When a bind-mounted directory itself is already owned by 1000:1000 but contains root-owned files from a previous run, restore, or manual copy (for example /app/data/stock_analysis.db), this guard skips the recursive chown. The following chmod -R u+rwX only changes the owner permission bits, so root-owned children remain unwritable after the process drops to dsa, and the advertised automatic permission repair still leaves database/report writes failing. Consider always applying the recursive ownership repair, or checking child ownership instead of only the mount point.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR makes the default Docker startup path “self-healing” after the switch to non-root execution by introducing a root entrypoint that repairs bind-mounted directory permissions for /app/data, /app/logs, and /app/reports, then drops privileges to the dsa user before starting the app.
Changes:
- Add a Docker entrypoint script that creates/fixes writable mount directories and then
gosu-execs the app asdsa. - Update the Dockerfile to install
gosu, wire the new entrypoint, and removeUSER dsaso the entrypoint can start as root. - Update runtime logging guidance + deployment docs, and add regression tests for the entrypoint/Dockerfile contract.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
docker/entrypoint.sh |
New root entrypoint to repair mount dirs and drop privileges via gosu. |
docker/Dockerfile |
Installs gosu, copies/execs entrypoint, removes USER dsa. |
main.py |
Updates the file-logging fallback warning text to reflect the new Docker behavior. |
tests/test_docker_entrypoint.py |
Adds tests asserting entrypoint syntax + Dockerfile/entrypoint contract. |
tests/test_main_schedule_mode.py |
Updates expectation for the changed logging fallback message. |
docs/full-guide.md |
Updates CN guide to state mounts are auto-repaired at startup and when it may not apply. |
docs/full-guide_EN.md |
Same as above for EN guide. |
docs/DEPLOY.md |
Updates CN deploy guide permissions section to reflect auto-repair entrypoint. |
docs/DEPLOY_EN.md |
Adds EN deploy guide permissions section reflecting auto-repair entrypoint. |
docs/CHANGELOG.md |
Adds an Unreleased fix entry describing the new Docker permission auto-repair. |
| warn "WARN: unable to make $dir writable for $APP_USER; check read-only, rootless, or NFS mount permissions if writes fail." | ||
| fi |
| if [ "$owner" != "$APP_UID:$APP_GID" ]; then | ||
| if ! chown -R "$APP_UID:$APP_GID" "$dir"; then | ||
| warn "WARN: unable to set ownership for $dir; check read-only, rootless, or NFS mount permissions if writes fail." | ||
| fi | ||
| fi | ||
|
|
||
| if ! chmod -R u+rwX "$dir"; then | ||
| warn "WARN: unable to make $dir writable for $APP_USER; check read-only, rootless, or NFS mount permissions if writes fail." | ||
| fi |
| def test_docker_entrypoint_repairs_ownership_and_user_permissions() -> None: | ||
| entrypoint = (REPO_ROOT / "docker" / "entrypoint.sh").read_text(encoding="utf-8") | ||
|
|
||
| assert 'chown -R "$APP_UID:$APP_GID" "$dir"' in entrypoint | ||
| assert 'chmod -R u+rwX "$dir"' in entrypoint |
🤖 自动审查报告
📁 修改的文件
🧠 AI 代码审查意见结论Ready to Merge 该 PR 解决了 Docker 容器在非 root 用户下运行时，主机挂载目录权限导致的应用启动失败问题，通过引入一个自修复的 Docker 启动入口脚本，显著改善了用户体验。代码变更、文档更新和测试覆盖全面，风险评估充分，验证结果良好。 结构化审查结果
必改项 (无)未发现任何阻断性问题。 建议项 (1 条)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba9132dcfd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| || python -c "import sys; sys.exit(0)" | ||
|
|
||
| # 启动入口先修复 bind mount 目录权限，再降权为 dsa 执行应用 | ||
| ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"] |
There was a problem hiding this comment.
This entrypoint drops privileges only for the initial container command; docker-compose exec starts a new process without going through the entrypoint. Since this commit also removes the Dockerfile USER dsa, the documented docker-compose -f ./docker/docker-compose.yml exec stock-analyzer python main.py --no-notify command now runs as the image default root unless users add -u dsa (Docker documents USER as setting the default user, while Compose exec has a separate --user override). A manual analysis run can therefore create root-owned database/log/report files on the bind mounts, and the already-running scheduled process as dsa can fail to update them until a restart repairs ownership.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过；非 root 镜像与默认 bind mount 之间的权限冲突会影响普通 Docker/Compose 升级路径，本 PR 有明确修复价值。
- 是否有对应 issue：无；描述中引用了 PR #1214 / #1262 作为背景，但未检测到 Fixes/Closes/Refs 形式的 issue 关联。
- PR 类型：fix；同时包含 docs/test，因为改动了 Docker 启动行为、部署文档和回归测试。
- description 完整性：完整；已覆盖背景、改动范围、兼容性风险、验证结果、文档更新和回滚方案。
- 是否可直接合入：可；当前 CI 为 success，改动同步更新了
docs/CHANGELOG.md和中英文部署文档，未发现阻断性行为风险；mergeable_state=blocked更像分支保护/权限状态，不单独构成阻断。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
efinance writes its search cache (search-cache.json) into its own package directory (site-packages/efinance/data), which is root-owned in the image. Since the container runs as the non-root dsa user (uid 1000), every A-share fetch failed with PermissionError, forcing EfinanceFetcher (the default priority-0 source) to fail and fall back. Pre-create and chown the efinance data dir to dsa at build time. The path is resolved dynamically to avoid hardcoding the Python version. This is image-internal static content, so the fix belongs at build time rather than the runtime mount-repair in the entrypoint (#1263), which only covers user-mounted volumes. Fixes #1748 Co-authored-by: gang.wu@ximalaya.com <gang.wu@ximalaya.com> Co-authored-by: mumu <42829555+ZhuLinsen@users.noreply.github.com>
efinance writes its search cache (search-cache.json) into its own package directory (site-packages/efinance/data), which is root-owned in the image. Since the container runs as the non-root dsa user (uid 1000), every A-share fetch failed with PermissionError, forcing EfinanceFetcher (the default priority-0 source) to fail and fall back. Pre-create and chown the efinance data dir to dsa at build time. The path is resolved dynamically to avoid hardcoding the Python version. This is image-internal static content, so the fix belongs at build time rather than the runtime mount-repair in the entrypoint (ZhuLinsen#1263), which only covers user-mounted volumes. Fixes ZhuLinsen#1748 Co-authored-by: gang.wu@ximalaya.com <gang.wu@ximalaya.com> Co-authored-by: mumu <42829555+ZhuLinsen@users.noreply.github.com>
efinance writes its search cache (search-cache.json) into its own package directory (site-packages/efinance/data), which is root-owned in the image. Since the container runs as the non-root dsa user (uid 1000), every A-share fetch failed with PermissionError, forcing EfinanceFetcher (the default priority-0 source) to fail and fall back. Pre-create and chown the efinance data dir to dsa at build time. The path is resolved dynamically to avoid hardcoding the Python version. This is image-internal static content, so the fix belongs at build time rather than the runtime mount-repair in the entrypoint (ZhuLinsen#1263), which only covers user-mounted volumes. Fixes ZhuLinsen#1748 Co-authored-by: gang.wu@ximalaya.com <gang.wu@ximalaya.com> Co-authored-by: mumu <42829555+ZhuLinsen@users.noreply.github.com>
PR Type
Background And Problem
PR #1214 switched the Docker image to run as the non-root
dsauser, which is a good security direction, but the default Docker / Compose examples still bind mount hostdata,logs, andreportsdirectories. That made ordinary upgrades fail unless users manually fixed host-side UID/GID permissions.PR #1262 made
logspermission failures non-fatal, but it did not remove the manual permission burden fordataandreportswrites. This PR makes the default Docker startup path self-healing instead of asking users to runchown/chmod.Scope Of Change
docker/entrypoint.shthat starts as root, creates/app/data,/app/logs, and/app/reports, checks whetherdsacan already write them, and only repairs permissions when needed.dsaviagosubefore running the existing command.chown/chmodwhen mounts are already writable; it also checks the configured SQLite database files and top-level mounted files to catch common root-owned leftovers from older images.docker/Dockerfileto installgosu, copy the entrypoint, and use it before the existingCMD.chowndirectories.chown/chmod.Compatibility And Risk
data,logs, andreportsare repaired automatically before the app runs as non-root.dsaand skips repair when the mount is already usable.dsa; only the entrypoint starts as root long enough to create/check/repair mount permissions and then drops privileges.--user/ Composeuser:, use read-only mounts, rootless Docker, NFS, or another storage backend that preventschown/chmodfrom inside the container.Verification Commands And Results
sh -n docker/entrypoint.sh python -m py_compile main.py tests/test_docker_entrypoint.py tests/test_main_schedule_mode.py python -m pytest tests/test_docker_entrypoint.py tests/test_main_schedule_mode.py -q ./scripts/ci_gate.sh flake8 ./scripts/ci_gate.sh offline-tests sh docker/entrypoint.sh sh -c 'test "$(id -u)" = "1000" && test "$(id -g)" = "1000"'Results:
21 passed1872 passed, 2 deselected, 166 subtests passedDocker build was attempted twice:
docker build -f docker/Dockerfile -t dsa-permission-entrypoint-test .Both attempts failed before building project layers because Docker Hub base image metadata requests timed out for
python:3.11-slim-bookworm/node:20-slim. This is recorded as an environment/network gap rather than a code failure.Documentation And Changelog
Updated:
docs/CHANGELOG.mddocs/full-guide.mddocs/full-guide_EN.mddocs/DEPLOY.mddocs/DEPLOY_EN.mdRollback Plan
Revert this PR commit to restore the previous Dockerfile behavior. If rollback is needed after release, users can still apply the existing manual host directory permission workaround from prior documentation.
Checklist
docs/CHANGELOG.md/ Relevant docs anddocs/CHANGELOG.mdare updated