Skip to content

fix: avoid recursive chown on host bind-mounted data directories#25

Merged
stritti merged 3 commits intofix/docker-permissions-and-db-pathfrom
copilot/sub-pr-24
Feb 26, 2026
Merged

fix: avoid recursive chown on host bind-mounted data directories#25
stritti merged 3 commits intofix/docker-permissions-and-db-pathfrom
copilot/sub-pr-24

Conversation

Copy link
Contributor

Copilot AI commented Feb 26, 2026

  • Investigate the chown -R issue in entrypoint.sh
  • Fix entrypoint.sh to avoid unconditional recursive ownership changes on bind mounts
  • Add safety guards: skip chown when DATA_DIR is '/' or empty
  • Add safety guards: skip chown when DB_DIR is '/', '.', or empty
  • Run code review and security scan

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix docker permissions and db path configuration fix: avoid recursive chown on host bind-mounted data directories Feb 26, 2026
@stritti stritti requested a review from Copilot February 26, 2026 18:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts the container entrypoint permission-fixup logic to avoid recursively chowning host bind-mounted data directories on every start, preventing unintended host-side ownership changes.

Changes:

  • Makes chown conditional on the directory being root-owned (UID 0).
  • Removes recursive -R ownership changes (only the directory node is adjusted).
  • Applies the same guarded behavior to the fleet DB parent directory and removes verbose ls debug output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@stritti stritti marked this pull request as ready for review February 26, 2026 19:12
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a611a0410a

ℹ️ 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".

entrypoint.sh Outdated
# Only fix ownership when the directory is still owned by root (e.g. just
# created above or a brand-new Docker-managed volume). This preserves the
# original ownership on pre-existing bind mounts.
if [ "$(stat -c '%u' "$DATA_DIR")" = "0" ]; then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Verify DATA_DIR is accessible after skipping chown

This guard now leaves any non-root-owned bind mount untouched, but the process is always dropped to UID 1000 (gosu clawmetry), so mounts owned by a different host UID with private permissions (for example 0700/0750) become unreadable or unwritable at runtime. In that scenario the container starts but ClawMetry cannot access its workspace data, which is a regression from the previous behavior that force-fixed ownership; at minimum this path should validate access and fail fast with guidance.

Useful? React with 👍 / 👎.

@stritti stritti self-requested a review February 26, 2026 20:05
Copy link
Owner

@stritti stritti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das Umbiegen der Berechtigungen ist ein Sicherheitsproblem. Wie kann das ursprüngliche Problem auf anderer Weise gelöst werden?

Co-authored-by: stritti <184547+stritti@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 26, 2026

Das Umbiegen der Berechtigungen ist ein Sicherheitsproblem. Wie kann das ursprüngliche Problem auf anderer Weise gelöst werden?

Der Ansatz der Runtime-Permission-Fixup (gosu-Pattern) ist in Docker-Entrypoints üblich und sicher, solange die Guards greifen – was wir jetzt haben. Die eigentliche Alternative wäre, den Container nie als root zu starten:

  1. USER clawmetry ins Dockerfile eintragen, den gosu-Aufruf und die chown-Logik ganz entfernen.
  2. Das Dockerfile setzt Ownership für /home/clawmetry/.openclaw bereits zur Build-Zeit; Docker-Volumes erben diese Permissions korrekt.
  3. Für Bind-Mounts müsste der Host-Nutzer das Verzeichnis selbst für UID 1000 schreibbar machen – oder docker run --user $(id -u):$(id -g) verwenden.

Wenn du diese strengere Variante bevorzugst (kein Root-Start, keine Runtime-chown), kann ich die Umstellung vornehmen.

@stritti stritti merged commit ec1ac2b into fix/docker-permissions-and-db-path Feb 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants