From f3b344d731d82f2369fc7b82ec0ac65ead37fae5 Mon Sep 17 00:00:00 2001 From: Luan Vu Date: Sat, 21 Mar 2026 08:35:31 +0700 Subject: [PATCH] fix(pkg-helper): fix apk-packages persist file not writable in Docker (#324) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .runtime directory on the data volume may be owned by goclaw:goclaw (from older images or Docker volume initialization). pkg-helper runs as root but without CAP_DAC_OVERRIDE, so it cannot create files in goclaw-owned directories. This caused persistAdd() to fail silently — runtime-installed system packages (bash, pandoc, etc.) were lost on container recreate. Fix: set .runtime directory ownership to root:goclaw (mode 0750) so pkg-helper can write apk-packages while goclaw can still traverse. Three layers for robustness: - Dockerfile: pre-create .runtime with correct split ownership in image - docker-entrypoint.sh: fix ownership on existing volumes (upgrade path) - pkg-helper: self-healing ensurePersistDir() at startup as defense-in-depth Subdirs (pip/, npm-global/, pip-cache/) remain goclaw-owned since those are written by the app process, not pkg-helper. Fixes #323 Co-authored-by: Luvu182 <208665161+Luvu182@users.noreply.github.com> --- Dockerfile | 17 ++++++++++++----- cmd/pkg-helper/main.go | 30 ++++++++++++++++++++++++++++++ docker-entrypoint.sh | 18 +++++++++++++----- 3 files changed, 55 insertions(+), 10 deletions(-) diff --git a/Dockerfile b/Dockerfile index a48b9b3f..1dd95394 100644 --- a/Dockerfile +++ b/Dockerfile @@ -97,11 +97,18 @@ RUN set -eux; \ RUN chmod +x /app/docker-entrypoint.sh && \ chmod 755 /app/pkg-helper && chown root:root /app/pkg-helper -# Create data directories (owned by goclaw user). -# Binaries and entrypoint stay root-owned (readable by all). -RUN mkdir -p /app/workspace /app/data /app/skills /app/tsnet-state /app/.goclaw \ - && chown -R goclaw:goclaw /app/workspace /app/data /app/skills /app/tsnet-state /app/.goclaw \ - && chown goclaw:goclaw /app/bundled-skills +# Create data directories. +# .runtime has split ownership: root owns the dir (so pkg-helper can write apk-packages), +# while pip/npm subdirs are goclaw-owned (runtime installs by the app process). +RUN mkdir -p /app/workspace /app/data/.runtime/pip /app/data/.runtime/npm-global/lib \ + /app/data/.runtime/pip-cache /app/skills /app/tsnet-state /app/.goclaw \ + && touch /app/data/.runtime/apk-packages \ + && chown -R goclaw:goclaw /app/workspace /app/skills /app/tsnet-state /app/.goclaw \ + && chown goclaw:goclaw /app/bundled-skills /app/data \ + && chown root:goclaw /app/data/.runtime /app/data/.runtime/apk-packages \ + && chmod 0750 /app/data/.runtime \ + && chmod 0640 /app/data/.runtime/apk-packages \ + && chown -R goclaw:goclaw /app/data/.runtime/pip /app/data/.runtime/npm-global /app/data/.runtime/pip-cache # Default environment ENV GOCLAW_CONFIG=/app/config.json \ diff --git a/cmd/pkg-helper/main.go b/cmd/pkg-helper/main.go index 43e38d12..cfc35b42 100644 --- a/cmd/pkg-helper/main.go +++ b/cmd/pkg-helper/main.go @@ -12,6 +12,7 @@ import ( "os" "os/exec" "os/signal" + "path/filepath" "regexp" "strings" "syscall" @@ -62,6 +63,9 @@ func main() { slog.Warn("pkg-helper: chmod socket failed", "error", err) } + // Ensure persist directory is writable by root (self-healing for upgrades). + ensurePersistDir() + // Graceful shutdown on SIGTERM/SIGINT. sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGTERM, syscall.SIGINT) @@ -223,3 +227,29 @@ func apkListFile() string { } return runtimeDir + "/apk-packages" } + +// ensurePersistDir ensures the apk persist file's parent directory is writable by root. +// On existing volumes the directory may be goclaw-owned (from older images); fix ownership +// using CAP_CHOWN so pkg-helper can create/write the persist file. +func ensurePersistDir() { + dir := filepath.Dir(apkListFile()) + fi, err := os.Stat(dir) + if err != nil { + // Directory doesn't exist — entrypoint should have created it. + return + } + if !fi.IsDir() { + return + } + + // Try to fix ownership to root:goclaw (gid 1000) if not already root-owned. + // CAP_CHOWN is available even when CAP_DAC_OVERRIDE is dropped. + if stat, ok := fi.Sys().(*syscall.Stat_t); ok && stat.Uid != 0 { + if err := os.Chown(dir, 0, 1000); err != nil { + slog.Warn("pkg-helper: cannot fix persist dir ownership", "dir", dir, "error", err) + } else { + os.Chmod(dir, 0750) //nolint:errcheck + slog.Info("pkg-helper: fixed persist dir ownership", "dir", dir) + } + } +} diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 99807c3e..5bf0bcd4 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -7,14 +7,23 @@ RUNTIME_DIR="/app/data/.runtime" # Non-fatal: on first start with a fresh volume the directory may not be # writable yet (volume initialisation race on some Docker runtimes). # The app starts fine without .runtime; package installs will fail gracefully. -mkdir -p "$RUNTIME_DIR/pip" "$RUNTIME_DIR/npm-global/lib" || true +mkdir -p "$RUNTIME_DIR/pip" "$RUNTIME_DIR/npm-global/lib" "$RUNTIME_DIR/pip-cache" || true + +# Fix .runtime ownership for split root/goclaw access. +# .runtime itself must be root-owned so pkg-helper (root) can write apk-packages. +# Subdirs pip/, npm-global/, pip-cache/ must be goclaw-owned for runtime installs. +# This also handles upgrades from older images where .runtime was fully goclaw-owned. +if [ "$(id -u)" = "0" ] && [ -d "$RUNTIME_DIR" ]; then + chown root:goclaw "$RUNTIME_DIR" 2>/dev/null || true + chmod 0750 "$RUNTIME_DIR" 2>/dev/null || true + chown -R goclaw:goclaw "$RUNTIME_DIR/pip" "$RUNTIME_DIR/npm-global" "$RUNTIME_DIR/pip-cache" 2>/dev/null || true +fi # Python: allow agent to pip install to writable target dir export PYTHONPATH="$RUNTIME_DIR/pip:${PYTHONPATH:-}" export PIP_TARGET="$RUNTIME_DIR/pip" export PIP_BREAK_SYSTEM_PACKAGES=1 export PIP_CACHE_DIR="$RUNTIME_DIR/pip-cache" -mkdir -p "$RUNTIME_DIR/pip-cache" || true # Node.js: allow agent to npm install -g to writable prefix # NODE_PATH includes both pre-installed system globals and runtime-installed globals. @@ -23,11 +32,10 @@ export NODE_PATH="/usr/local/lib/node_modules:$RUNTIME_DIR/npm-global/lib/node_m export PATH="$RUNTIME_DIR/npm-global/bin:$RUNTIME_DIR/pip/bin:$PATH" # System packages: re-install on-demand packages persisted across recreates. -# In Docker: entrypoint runs as root (then drops via su-exec). -# Outside Docker: may run as non-root — skip privileged operations gracefully. +# After chown above, root owns .runtime and can create this file. APK_LIST="$RUNTIME_DIR/apk-packages" -touch "$APK_LIST" 2>/dev/null || true if [ "$(id -u)" = "0" ]; then + touch "$APK_LIST" 2>/dev/null || true chown root:goclaw "$APK_LIST" 2>/dev/null || true chmod 0640 "$APK_LIST" 2>/dev/null || true fi