security: add non-root users to token-spy and dashboard containers#431
Conversation
Lightheartdevs
left a comment
There was a problem hiding this comment.
Review: Needs Work (same issue as prior review)
The core Dockerfile changes are correct, but the upgrade-breaking permission bug we flagged previously is still present and unaddressed.
The problem
token-spy/compose.yaml mounts ./data/token-spy:/app/data as a bind mount. On existing installs, this host directory is owned by root:root (created by Docker running as root). The Dockerfile's chown -R dreamer:nogroup /app only affects the image layer — bind mounts override it. When the container starts as dreamer (UID 1000), it gets permission denied writing to /app/data.
This will break every existing token-spy install on upgrade with no clear error message.
How to fix
Add an entrypoint wrapper that checks/fixes ownership before dropping privileges:
COPY entrypoint.sh /entrypoint.sh
ENTRYPOINT ["/entrypoint.sh"]#!/bin/sh
# Fix ownership of bind-mounted data directory
if [ "$(stat -c '%u' /app/data 2>/dev/null)" != "$(id -u)" ]; then
chown -R dreamer:nogroup /app/data 2>/dev/null || true
fi
exec "$@"Or at minimum, document the migration step: sudo chown -R 1000:1000 ./data/token-spy
What's good
- Dashboard Dockerfile changes are thorough (nginx dirs, pid file, conf fixup)
- UID 1000 is a reasonable choice
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
|
The concept is right and this addresses a real finding from the security audit (M2 — containers running as root). Dashboard Dockerfile changes look solid. Blocker: The token-spy container bind-mounts Fix: Add an entrypoint wrapper that fixes ownership at runtime before dropping to the COPY entrypoint.sh /entrypoint.sh
ENTRYPOINT ["/entrypoint.sh"]#!/bin/sh
chown -R dreamer:dreamer /app/data 2>/dev/null || true
exec su-exec dreamer "$@"Or document a migration step for existing users. Either way, the bind mount ownership needs to be handled. |
Lightheartdevs
left a comment
There was a problem hiding this comment.
Audit Review
Dashboard Dockerfile: Looks good
- Creates user/group (UID/GID 1000)
- Chowns all nginx runtime directories (html, cache, log, conf.d, pid)
- Removes
userdirective from nginx.conf viased -i '/^user/d' - Correct approach, no concerns
Token-spy Dockerfile: Bind-mount permission bug still present
The same issue flagged in the March 20 review remains unresolved.
chown -R dreamer:nogroup /app in the Dockerfile only affects the image layer. At runtime, ./data/token-spy:/app/data mounts the host directory over /app/data. On existing installs, this directory is root:root (created by Docker running as root). The container running as UID 1000 will get EACCES writing to the database.
Fix options:
- Add an entrypoint script that checks ownership and runs
chownif needed (requires the entrypoint to start as root, thenexec gosu dreamer ...) - Document a migration step:
chown -R 1000:65534 data/token-spy/before upgrading - Use a named volume instead of a bind mount (Docker manages ownership)
Without one of these, merging this PR will break token-spy on every existing install.
Lightheartdevs
left a comment
There was a problem hiding this comment.
Audit: REQUEST CHANGES — bind-mount permission bug (flagged twice, still unresolved)
The non-root user setup is correct for the image layer:
- Dashboard: Creates user/group, chowns nginx runtime dirs, removes
userdirective from nginx.conf ✓ - Token-spy: Uses
adduser --system --no-create-home✓
Blocker: Bind-mount permission failure on existing installs.
token-spy/compose.yaml uses a bind mount ./data/token-spy:/app/data. On every existing install, this directory is owned by root:root. The chown in the Dockerfile only affects the image layer — bind mounts override it. When the container starts as UID 1000, it will get EACCES writing to /app/data.
This will silently break every existing token-spy installation on upgrade.
This has been flagged in two review cycles (March 20 and April 1) and remains unaddressed.
Fix options:
- Entrypoint wrapper: start as root, fix ownership, then
exec gosu dreamer ... - Init container or pre-start hook that runs
chown -R 1000:1000 data/token-spy/ - Switch from bind mount to named Docker volume
- Document required migration step in upgrade notes
The dashboard portion could potentially merge independently since nginx uses its own managed dirs, not user bind mounts.
Summary
Adds non-root user (UID 1000) to token-spy and dashboard Dockerfiles to follow container security best practices. Containers should not run as root to minimize attack surface.
Changes
token-spy
dreamersystem user (UID 1000)/appdirectorydashboard
dreameruser and group (UID/GID 1000)userdirective from nginx.conf (runs as current user)Security Impact
Both containers now run as UID 1000 (dreamer) instead of root, reducing the attack surface if a container is compromised.
Testing
Related