cubeops: run the container image as an unprivileged user - #1447
cubeops: run the container image as an unprivileged user#1447dwin-gharibi wants to merge 1 commit into
Conversation
Signed-off-by: Dwin Gharibi <dwin.gharibi@email.kntu.ac.ir>
| && mkdir -p /data/log/CubeOps \ | ||
| && chown -R cube:cube /data/log/CubeOps | ||
|
|
There was a problem hiding this comment.
This pre-creates and chowns only the log directory, but CubeOps writes to another tree at runtime that this image does not set up for uid 100.
CubeOps/internal/service/openclaw.go:1250-1251 hardcodes two host state roots — /data/shared/agenthub/openclaw and /data/shared/agenthub/openclaw-snapshots — and PrepareOpenclawStateDir (os.MkdirAll) is called unconditionally on every agenthub instance create/clone when persistence_mode=shared_files (CubeOps/internal/service/agenthub.go:492-498, 1241); CopyOpenclawStateDir and os.RemoveAll also touch those trees. Neither root exists in the Alpine base image, so MkdirAll("/data/shared/agenthub/openclaw/<id>") walks up and tries to create /data — writable by root but not by uid 100 → EACCES.
Today (uid 0) those calls succeed, silently creating useless ephemeral dirs in the container layer; after this change the same agenthub calls start returning 500s in the k8s image whenever the paths aren't host-mounted. That is a behavior change this PR's risk section doesn't cover. Please either:
- create +
chown cube:cubethe two roots here alongside the log dir (preserves the prior ephemeral behavior), and/or - document that
shared_filesagenthub persistence requires/data/shared/agenthubmounted writable by uid 100.
(Related but pre-existing and out of scope: CopyOpenclawStateDir at openclaw.go:1307 invokes rsync, which this image never installs — the docker-cli apk is unused while the actually-needed rsync is missing.)
Review: cubeops: run the container image as an unprivileged user (PR #1447)AI-generated review — verified against the base branch workspace. Not a human approval. Verdict: Approve with one substantive gap to address or document. The change is minimal, well-reasoned, matches the existing What's good
Main finding (inline on
|
Closes #1446.
Motivation
CubeOps/Dockerfilehad noUSER, so the service ran as uid 0. It holds the JWT signing secret and themaster encryption key, and it needs no host privileges — and both sibling control-plane images already run
unprivileged (
CubeAPIwith acubeuser,cube-lifecycle-manageron distrolessnonroot). CubeOps wasthe outlier.
What this changes
CubeOps/Dockerfileruntime stage, following the patternCubeAPI/Dockerfile:73-81already establishes:The log directory is created and chowned because
config.LogDirdefaults to/data/log/CubeOps(
internal/config/config.go:98-99) and a non-root user cannot create it at runtime. Operators who pointCUBE_OPS_LOG_DIRelsewhere must ensure that path is writable by uid 100 — or mount it with the rightownership.
Nothing else changes: same base image, same packages, same entrypoint and port.
Why CubeOps can drop root
I checked rather than assuming:
grep -rn "exec.Command" CubeOps --include='*.go'finds onlyrsync(
internal/service/openclaw.go:1307). Thedocker-clipackage in the image appears unused.grep -rn "docker.sock" deploy/kubernetes/chart/returns nothing).
Testing
The full image build needs network access to fetch modules and hit the same
proxy.golang.org … 403 Forbiddenthat blocksgo mod downloadlocally, so I verified the runtime stagedirectly by building it with a stub binary in place of the
COPY --from=builder:So the entrypoint runs unprivileged and the default log directory is writable by that user. The build-stage
half of the Dockerfile is untouched by this change.
CI:
build-checkbuilds the image in the builder environment, which has the network access this needs.Risk / rollout
Ownership of a mounted log volume matters after this. If a deployment bind-mounts or PVC-mounts
/data/log/CubeOpsfrom the host, that directory must be writable by uid 100 or CubeOps will fail to openits log file on start. The chart should either set an
fsGroupor pre-chown the path — worth checkingalongside this merge.
No other behaviour change.
Not fixed here
CubeMaster/docker/Dockerfilealso lacks aUSER. CubeMaster does host-level work and may legitimatelyneed root; that needs its own audit rather than a copy of this patch.
docker-clipackage and the missingrsync(used atopenclaw.go:1307) are separate issuesI noticed while checking privileges.