Docker: run as non-root user#30878
Conversation
Run the long-lived evcc process as a non-root user (default uid 1000) instead of root, reducing the blast radius of any in-container code execution. The entrypoint starts as root only to repair ownership of the database mount, then drops privileges via su-exec. Existing installations keep working without manual steps: the /root/.evcc mount is re-owned on first start and the database path is unchanged. PUID/PGID env vars (default 1000) align the runtime user with host ownership. The Home Assistant addon path stays root.
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The unconditional
chown -Ron/root/.evcc(and ondirname "$EVCC_DATABASE_DSN") on every start may be expensive and could unintentionally recurse over large/shared directories; consider narrowing the path or only fixing ownership when it’s actually wrong. - With the new logic, even arbitrary commands passed as
docker run ... evcc-image <cmd>now run under the non-root user; if you still want a way to drop into a root shell for debugging, you may want to special-case common shells or provide an explicit override.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The unconditional `chown -R` on `/root/.evcc` (and on `dirname "$EVCC_DATABASE_DSN"`) on every start may be expensive and could unintentionally recurse over large/shared directories; consider narrowing the path or only fixing ownership when it’s actually wrong.
- With the new logic, even arbitrary commands passed as `docker run ... evcc-image <cmd>` now run under the non-root user; if you still want a way to drop into a root shell for debugging, you may want to special-case common shells or provide an explicit override.
## Individual Comments
### Comment 1
<location path="packaging/docker/bin/entrypoint.sh" line_range="92-93" />
<code_context>
+ # user's home is set to /root so ~/.evcc/evcc.db keeps resolving to the
+ # existing mount (su-exec sets HOME from the passwd entry, not the env).
+ if [ "$(id -u)" = "0" ]; then
+ getent group evcc > /dev/null 2>&1 || addgroup -g "$PGID" evcc 2> /dev/null || addgroup evcc
+ getent passwd evcc > /dev/null 2>&1 || adduser -D -H -h /root -u "$PUID" -G evcc evcc 2> /dev/null || adduser -D -H -h /root -G evcc evcc
+
+ mkdir -p "$DATA_DIR"
</code_context>
<issue_to_address>
**issue:** The fallback addgroup/adduser behavior can ignore the requested PUID/PGID and lead to mismatched IDs.
If the requested PGID/UID are already in use under different names, `addgroup -g "$PGID" evcc` / `adduser ... -u "$PUID"` will fail, and the fallbacks will create a new user/group with different IDs. This breaks the expectation that PUID/PGID match the container user/group and the IDs used in `chown -R "$PUID:$PGID"`. It would be safer to fail loudly on this misconfiguration, or to look up and reuse existing user/group entries by UID/GID instead of creating new ones with mismatched IDs.
</issue_to_address>
### Comment 2
<location path="packaging/docker/bin/entrypoint.sh" line_range="95-100" />
<code_context>
+ mkdir -p "$DATA_DIR"
+ chown -R "$PUID:$PGID" "$DATA_DIR"
+ # fix a custom database directory if EVCC_DATABASE_DSN points elsewhere
+ [ -n "$EVCC_DATABASE_DSN" ] && chown -R "$PUID:$PGID" "$(dirname "$EVCC_DATABASE_DSN")" 2> /dev/null || true
+
+ RUNAS="su-exec $PUID:$PGID"
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The `chown` on `dirname "$EVCC_DATABASE_DSN"` can target unexpected paths and relies on subtle `&&`/`||` precedence.
1) If `EVCC_DATABASE_DSN` isn’t a plain filesystem path (e.g. `postgres://...` or `.`), `dirname` may resolve to an unexpected directory (including `.`), and `chown -R` will recursively change ownership there. Consider restricting this to known-safe locations (e.g. under `$DATA_DIR`) or validating that the resolved directory is within the intended mount.
2) The `[ -n ... ] && chown ... || true` pattern relies on `&&`/`||` precedence to behave correctly. An explicit `if [ -n "$EVCC_DATABASE_DSN" ]; then chown -R "$PUID:$PGID" "$(dirname "$EVCC_DATABASE_DSN")" || true; fi` is clearer and less error-prone if this line is modified later.
```suggestion
mkdir -p "$DATA_DIR"
chown -R "$PUID:$PGID" "$DATA_DIR"
# fix a custom database directory if EVCC_DATABASE_DSN points to a path under $DATA_DIR
if [ -n "$EVCC_DATABASE_DSN" ]; then
dbdir="$(dirname "$EVCC_DATABASE_DSN")"
case "$dbdir" in
"$DATA_DIR" | "$DATA_DIR"/*)
chown -R "$PUID:$PGID" "$dbdir" 2> /dev/null || true
;;
esac
fi
RUNAS="su-exec $PUID:$PGID"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
We‘ve tried this before. Should find old PR and cross-check before merging. |
|
Will their a permissions issue when user mounts their |
|
Yes, and it's worth calling out explicitly, because the usual workaround doesn't work as the entrypoint currently stands. A device passed with The normal escape hatch ( So to keep serial devices working the entrypoint would need to:
Otherwise we should at least document that serial‑device users have to pin 🤖 Generated with Claude Code |
|
Sketch of the entrypoint change (against the mkdir -p "$DATA_DIR"
chown -R "$PUID:$PGID" "$DATA_DIR"
[ -n "$EVCC_DATABASE_DSN" ] && chown -R "$PUID:$PGID" "$(dirname "$EVCC_DATABASE_DSN")" 2> /dev/null || true
+ # join the group(s) owning mounted serial devices (Modbus RTU, P1/SML readers);
+ # --device keeps the host owner (often root:dialout), so non-root needs the group
+ DEVICE_GIDS="$EVCC_DEVICE_GIDS"
+ for dev in /dev/ttyUSB* /dev/ttyACM* /dev/ttyAMA* /dev/serial/by-id/*; do
+ [ -e "$dev" ] || continue
+ DEVICE_GIDS="$DEVICE_GIDS $(stat -c '%g' "$dev")"
+ done
+ for gid in $DEVICE_GIDS; do
+ [ "$gid" = "$PGID" ] && continue
+ grp=$(getent group "$gid" | cut -d: -f1)
+ if [ -z "$grp" ]; then
+ grp="evccdev$gid"
+ addgroup -g "$gid" "$grp" 2> /dev/null || true
+ fi
+ addgroup evcc "$grp" 2> /dev/null || true
+ done
+
- RUNAS="su-exec $PUID:$PGID"
+ # no explicit gid: su-exec uid:gid wipes supplementary groups (setgroups), the
+ # uid-only form keeps them via getgrouplist; primary gid stays PGID from passwd
+ RUNAS="su-exec $PUID"Verified in Notes:
🤖 Generated with Claude Code |
|
Added the suggested device group. Updated docs PR evcc-io/docs#1088 accordingly. |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The unconditional
chown -R "$PUID:$PGID" "$DATA_DIR"(and optional EVCC_DATABASE_DSN dir) on every startup could be expensive for large volumes; consider narrowing this to the actual DB file or gating it (e.g. only when ownership is mismatched) to avoid repeated recursive traversals. - The serial device group join logic may add the same gid multiple times and print repeated log lines when multiple device paths share the same group; you could deduplicate
DEVICE_GIDSbefore the loop to keep the output and group management cleaner.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The unconditional `chown -R "$PUID:$PGID" "$DATA_DIR"` (and optional EVCC_DATABASE_DSN dir) on every startup could be expensive for large volumes; consider narrowing this to the actual DB file or gating it (e.g. only when ownership is mismatched) to avoid repeated recursive traversals.
- The serial device group join logic may add the same gid multiple times and print repeated log lines when multiple device paths share the same group; you could deduplicate `DEVICE_GIDS` before the loop to keep the output and group management cleaner.
## Individual Comments
### Comment 1
<location path="packaging/docker/bin/entrypoint.sh" line_range="93-96" />
<code_context>
+ # existing mount (su-exec sets HOME from the passwd entry, not the env).
+ if [ "$(id -u)" = "0" ]; then
+ getent group evcc > /dev/null 2>&1 || addgroup -g "$PGID" evcc 2> /dev/null || addgroup evcc
+ getent passwd evcc > /dev/null 2>&1 || adduser -D -H -h /root -u "$PUID" -G evcc evcc 2> /dev/null || adduser -D -H -h /root -G evcc evcc
+
+ mkdir -p "$DATA_DIR"
+ chown -R "$PUID:$PGID" "$DATA_DIR"
+ # fix a custom database directory if EVCC_DATABASE_DSN points elsewhere
+ [ -n "$EVCC_DATABASE_DSN" ] && chown -R "$PUID:$PGID" "$(dirname "$EVCC_DATABASE_DSN")" 2> /dev/null || true
</code_context>
<issue_to_address>
**issue:** Avoid potential mismatch between requested PUID/PGID and the actual evcc user/group IDs used for chown.
If `$PUID`/`$PGID` are already in use, `adduser -u "$PUID"` / `addgroup -g "$PGID"` may fail and the fallback will create `evcc` with different IDs. In that case, `chown -R "$PUID:$PGID"` won’t match the actual `evcc` user/group. To avoid this, derive the IDs from `id evcc` / `getent group evcc` after creation and use those values for `chown` instead of the requested PUID/PGID.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Pushed a docker image for testing docker hub. @StefanSchoof (and others): can you test if this is non-breaking for you? |
|
@naltatis I run my evcc not in docker. |
|
Throws |
|
Fixed |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the device group-detection loop,
for gid in $DEVICE_GIDS; dowill process duplicate gids (from multiple devices sharing a group), so consider deduplicating the list before the loop to avoid repeatedaddgroupcalls and repeated log lines. - The
chown -R "$PUID:$PGID" "$DATA_DIR"and subsequentchmod -Ron/root/.evccmay be expensive and override existing permissions on large or pre-populated mounts; you might want to guard these with a more targeted check (e.g., only adjust ownership when mismatched) to minimize surprises for existing volumes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the device group-detection loop, `for gid in $DEVICE_GIDS; do` will process duplicate gids (from multiple devices sharing a group), so consider deduplicating the list before the loop to avoid repeated `addgroup` calls and repeated log lines.
- The `chown -R "$PUID:$PGID" "$DATA_DIR"` and subsequent `chmod -R` on `/root/.evcc` may be expensive and override existing permissions on large or pre-populated mounts; you might want to guard these with a more targeted check (e.g., only adjust ownership when mismatched) to minimize surprises for existing volumes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Runs the evcc container process as a non-root user (default uid 1000) instead of root, limiting the impact of any in-container code execution. Existing installs keep working unchanged.
su-execafter re-chowning the database mount--deviceports (ttyUSB/ACM/AMA, serial/by-id);su-execruns uid-only so the groups survive,EVCC_DEVICE_GIDSoverrides/root/.evccmount unchanged; auto-migrates on first startPUID/PGIDenv (default 1000) to match host ownershipReal world testing wanted
Please try this change in the
naltatis/evcc:non-rootimage (nightly 25.6.2026) on docker hub and give feedback.