fix: add non-root USER directive to all Dockerfiles#73
Conversation
Dockerfile
Outdated
| rm -rf /tmp/kirocli /tmp/kirocli.zip | ||
|
|
||
| RUN mkdir -p /home/agent/.local/share/kiro-cli /home/agent/.kiro | ||
| RUN useradd -m -s /bin/bash agent |
chaodu-agent
left a comment
There was a problem hiding this comment.
Nice work on the non-root enforcement! A few things to address before merging.
Dockerfile
Outdated
| rm -rf /tmp/kirocli /tmp/kirocli.zip | ||
|
|
||
| RUN mkdir -p /home/agent/.local/share/kiro-cli /home/agent/.kiro | ||
| RUN useradd -m -s /bin/bash agent |
There was a problem hiding this comment.
As Neil mentioned — useradd doesn't guarantee UID 1000. If the base image already has a user at UID 1000, this will get a different UID and break the K8s securityContext (runAsUser: 1000).
Suggest explicitly setting it:
RUN useradd -m -s /bin/bash -u 1000 agent|
|
||
| COPY --from=builder /build/target/release/agent-broker /usr/local/bin/agent-broker | ||
|
|
||
| USER node |
There was a problem hiding this comment.
USER node — but WORKDIR is /home/agent (line 18). The node user's home is /home/node, and it may not have write permission to /home/agent.
Either:
- Change
WORKDIRto/home/nodeand update paths accordingly, or - Add
RUN chown -R node:node /home/agentbeforeUSER node
Same issue applies to Dockerfile.codex and Dockerfile.gemini.
| @@ -20,6 +20,10 @@ spec: | |||
| labels: | |||
| {{- include "agent-broker.selectorLabels" . | nindent 8 }} | |||
| spec: | |||
| {{- with .Values.podSecurityContext }} | |||
| securityContext: | |||
| {{- toYaml . | nindent 8 }} | |||
There was a problem hiding this comment.
Consider also adding container-level securityContext for defense in depth:
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]Pod-level covers runAsNonRoot/UID, but container-level locks down privilege escalation and capabilities.
Learnings from
|
|
Addressed the review feedback in two commits: 9202ad0 — Harden Dockerfiles and securityContext
caa3619 — Use built-in
Intentionally deferred:
|
|
Note on home directory split: The default (kiro) preset uses
We considered unifying to The Helm helper |
caa3619 to
e2fa092
Compare
…Context - Dockerfile: useradd -u 1000, --chown=agent:agent, curl --retry, HEALTHCHECK - Dockerfile.claude/codex/gemini: use built-in node user /home/node, --chown=node:node, npm --retry, HEALTHCHECK - Helm chart: podSecurityContext + containerSecurityContext, preset-aware home helper - k8s manifests: pod + container securityContext
e2fa092 to
1c9fa70
Compare
…Context (openabdev#73) - Dockerfile: useradd -u 1000, --chown=agent:agent, curl --retry, HEALTHCHECK - Dockerfile.claude/codex/gemini: use built-in node user /home/node, --chown=node:node, npm --retry, HEALTHCHECK - Helm chart: podSecurityContext + containerSecurityContext, preset-aware home helper - k8s manifests: pod + container securityContext Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Summary
Harden all Dockerfiles with non-root users, proper file ownership, health checks, and network retry logic. Add container-level security context to K8s manifests.
Changes
Dockerfile (kiro / default preset)
useradd -m -s /bin/bash -u 1000 agent— explicit UID matching K8s securityContextCOPY --chown=agent:agent— correct ownership on copied binaryHEALTHCHECK— process-alive check viapgrepcurl --retry 3 --retry-delay 5— retry logic for kiro-cli download/home/agent(debian base, no Node.js needed — keeps image ~200MB smaller)Dockerfile.claude / Dockerfile.codex / Dockerfile.gemini
nodeuser (UID 1000) fromnode:22-bookworm-slimCOPY --chown=node:node— correct ownership on copied binarymkdir /home/agent— use/home/nodedirectlyHEALTHCHECK— process-alive check viapgrepnpm install --retry 3— retry logic for package installs/home/node(node base image, required for npm-based ACP adapters)K8s & Helm
securityContext:runAsNonRoot, UID/GID/fsGroup 1000securityContext:allowPrivilegeEscalation: false, drop ALL capabilitiesagent-broker.agent.homeresolves/home/node(codex/claude/gemini) vs/home/agent(default) based onagent.presetHome directory decision
debian:bookworm-slimagent/home/agentnode:22-bookworm-slimnode/home/nodenode:22-bookworm-slimnode/home/nodenode:22-bookworm-slimnode/home/nodeUnifying to a single home path was considered but rejected —
useradd agenton the node base gets UID 1001 (1000 is taken bynode), breakingrunAsUser: 1000. PVC is unaffected since both users are UID 1000.Intentionally deferred
ghCLI at build time — tracked in Security: Define a non-root USER in Dockerfile #47Related
Closes #47
See also #45