Skip to content

Optimize Nut-server Docker configuration and security - #1

Open
eskwisit wants to merge 1 commit into
mainfrom
optimize-nut-server-17297820138796096883
Open

Optimize Nut-server Docker configuration and security#1
eskwisit wants to merge 1 commit into
mainfrom
optimize-nut-server-17297820138796096883

Conversation

@eskwisit

@eskwisit eskwisit commented Mar 5, 2026

Copy link
Copy Markdown
Owner

I have optimized the Docker configuration and repository structure for the nut-server. Key improvements include:

  1. Security: The container now runs as a non-root user (nut, UID 1000) by default.
  2. Image Optimization: Multiple RUN layers have been consolidated, and build-time dependencies (like gcc and libssl-dev) are now purged in the same layer to minimize final image size.
  3. Streamlined Build: The nut codebase is now fetched using curl and extracted directly into /app, avoiding temporary ZIP files.
  4. Dependency Management: GUI-related packages are automatically removed from requirements.txt to keep the server-only image slim.
  5. Repository Best Practices: Added a .dockerignore file to prevent local development artifacts from bloating the build context.
  6. Documentation Update: The README.md has been updated with the new volume paths and notes on host directory permissions for the non-root user.

PR created automatically by Jules for task 17297820138796096883 started by @eskwisit

- Consolidate multiple RUN layers into a single instruction to reduce image size.
- Implement a non-root user 'nut' (UID 1000) for improved security.
- Refactor the fetching and patching logic using curl and tar.
- Add a .dockerignore file to exclude unnecessary files from the build context.
- Update the README.md to reflect the new non-root user and revised volume paths (/app/conf, /app/_NSPOUT).
- Ensure build-time dependencies (gcc, -dev libraries) are removed after installation.
- Patch requirements.txt to remove GUI dependencies and add markupsafe==2.0.1 for stability.

Co-authored-by: eskwisit <47784621+eskwisit@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@devin-ai-integration

Copy link
Copy Markdown

Code review — Optimize Nut-server Docker configuration and security

Reviewed by building both main and this branch and running the resulting images (empty /titles bind mount, Docker 27.4.1, amd64).

Verified working: image builds clean, starts as uid=1000(nut), HTTP server answers on 9000 (401 = nut's auth, expected), and the native extensions that depend on the purged -dev packages still import (pycurllibcurl/8.14.1, PIL, zstandard, Crypto). Image size drops 649 MB → 263 MB. Good change overall; the notes below are mostly hardening and one breaking-change/docs gap.


1. Breaking change for existing users is undocumented (highest impact)

Moving /root/nut/app silently breaks every existing docker run line in the wild:

-v /path/to/conf:/root/nut/conf   # now mounts into an unused dir; baked conf is used instead
-v /path/to/_NSPOUT:/root/nut/_NSPOUT

Nothing errors — users just find their customized nut.conf ignored and NSP output going into an anonymous volume. Since the published tag is eskwisit/nut-server:latest, anyone pulling gets this on the next docker pull.

Suggestion: call it out at the top of the README (and release notes) as a breaking change, and consider publishing it under a new major tag rather than only moving latest.

2. Empty/new host conf mount silently disables the /titles scan

Related, and it's pre-existing rather than introduced here, but the README actively instructs the mount that triggers it:

docker run -d -v /path/to/conf:/app/conf:rw eskwisit/nut-server

An empty host dir shadows the baked /app/conf/nut.conf, so nut falls back to its compiled defaults. Verified inside the running container:

$ docker exec nutconf python3 -c "from nut import Config; print(Config.paths.scan)"
['.']            # scans /app (the source tree), not /titles

The server still starts and serves nothing. Worth an entrypoint that seeds conf/nut.conf from a baked nut.default.conf when the mounted dir is empty — that also makes the config volume genuinely useful instead of a footgun. (Config.py loads conf/nut.default.conf first and then overlays conf/nut.conf, so keeping the default file and writing only the overlay is closer to upstream's intent than mv-ing it.)

3. Hardcoded UID/GID 1000 + useradd -m -d /app (mode 700)

  • /app ends up drwx------ nut, so docker run --user 1001 … fails outright: python3: can't open file '/app/nut.py': [Errno 13] Permission denied. Users on a NAS whose media is owned by a different UID have no escape hatch. A PUID/PGID entrypoint (LinuxServer.io style, or gosu) or at minimum chmod 755 /app would fix that.
  • Using the home dir as the code dir also drops skel files into the app tree (.bashrc, .profile, .bash_logout now sit next to nut.py). Prefer useradd --no-create-home plus an explicit WORKDIR /app.
  • README says "you may need to adjust permissions"; a concrete chown -R 1000:1000 /path/to/titles line would save support issues.

4. chown -R nut:nut /app makes the application code writable by the runtime user

This gives back a chunk of what running non-root buys you — a compromise in nut can persist by rewriting /app/*.py. nut only needs write access to titledb/, titles/, _NSPOUT/, conf/, and (currently) __pycache__. Confirmed via docker diff that the only extra writes are bytecode caches:

A /app/Fs/__pycache__/…   (etc.)

So: ENV PYTHONDONTWRITEBYTECODE=1, pre-create + chown only the four data dirs, and leave the source root-owned.

5. Build-dep purge/reinstall is fragile

&& apt-get purge -y --auto-remove curl gcc libssl-dev … \
&& apt-get install -y --no-install-recommends libcurl4 libjpeg62-turbo

It works today, but it hardcodes runtime library package names that are distro-release dependent — on the current python:3.10-slim (now trixie-based) libcurl4 resolves to libcurl4t64, and the next base bump can break the build or, worse, silently leave pycurl/Pillow importing a stale soname. Two more robust options:

  • apt-mark manual libcurl4t64 libjpeg62-turbo (or apt-mark auto the build deps) before the purge, instead of remove-then-reinstall; or
  • a proper multi-stage build: compile wheels in a builder stage, COPY --from=builder /usr/local/lib/python3.10/site-packages into a clean runtime stage. That also removes gcc from the layer history entirely, which the single-layer purge does not.

6. Download hardening

curl -L https://github.com/blawar/nut/archive/refs/tags/v3.3.tar.gz | tar -xz …
  • Use curl -fsSL: without -f, an HTTP 5xx/redirect-to-error page exits 0 and pipes HTML into tar, producing a confusing failure instead of a clean one.
  • The pipeline's exit status is tar's only; add SHELL ["/bin/bash", "-o", "pipefail", "-c"] so a mid-transfer curl failure fails the build.
  • Consider pinning the tarball's SHA256 (sha256sum -c) and the base image by digest — GitHub tag tarballs are not immutable, and this image ships code that handles user files.

7. Smaller items

  • mkdir -p /app/conf is a no-op — the tarball already provides conf/.
  • pyopenssl~=19.1.0 is in requirements.txt but nothing in the nut source imports it (grep -rn "import OpenSSL" → no hits), and it is currently broken anyway against the resolved cryptography version (AttributeError: module 'lib' has no attribute 'GEN_EMAIL' — reproduces on main too). Since you already sed out pyqt5/qt-range-slider, dropping pyopenssl is free size and one less broken dep. Same argument for pyusb, google-api-python-client, and google-auth-oauthlib in a server-only image.
  • The sed-based requirement patching is silent if upstream renames a package; grep -q guards (or a committed requirements.server.txt) would fail loudly instead of shipping a GUI dep.
  • .dockerignore has essentially no effect here since the Dockerfile has no COPY/ADD — harmless, but it doesn't do what the PR description claims.
  • No HEALTHCHECK. / returns 401, so use a TCP-style probe, e.g. CMD python3 -c "import socket;socket.create_connection(('127.0.0.1',9000),3)".
  • Keeping VOLUME for three paths means every docker run leaves three anonymous volumes behind; dropping VOLUME and documenting the bind mounts is usually friendlier.
  • No CI. A workflow that just runs docker build on PRs would have de-risked this change a lot.

8. README claim that doesn't hold (pre-existing, not caused by this PR)

"Very low memory usage" is not accurate. Measured steady-state RSS with an empty titles dir:

image peak RSS
main 6.12 GiB
this branch 6.27 GiB

On a 7.7 GB host, a second container gets OOM-killed (exit=137, OOMKilled=true). The cause is "autoUpdateTitleDb": true pulling the ~244 MB titledb and holding it parsed in memory. Since this PR is framed as an optimization pass, it'd be a good place to either default autoUpdateTitleDb to false (documenting how to re-enable), or drop the "very low memory usage" bullet and document a recommended --memory floor.


Verdict: ship-worthy after (1) is documented; (3) and (5) are the ones most likely to generate user-facing breakage later, and (2)/(8) are pre-existing issues this PR is well-placed to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant