Skip to content

feat: adding support for user not root to Docker#2489

Open
aycsi wants to merge 1 commit intospotDL:devfrom
aycsi:docker-feat
Open

feat: adding support for user not root to Docker#2489
aycsi wants to merge 1 commit intospotDL:devfrom
aycsi:docker-feat

Conversation

@aycsi
Copy link

@aycsi aycsi commented Sep 3, 2025

Docker container running as root #1729.

Description

Fixing the Docker container running as root issue at #1729.

Motivation and Context

So users can access and manage their downloaded music files without using sudo.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project, as much as possible
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my changes
  • All new and existing tests passed

@aycsi aycsi changed the base branch from master to dev September 3, 2025 21:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses issue #1729 by modifying the Docker setup to run the container as a non-root user instead of root, allowing users to access downloaded music files without requiring sudo permissions.

  • Added user/group ID customization through build arguments with sensible defaults
  • Updated docker-compose configuration to support user ID passing and improved volume mapping
  • Enhanced documentation with docker-compose usage instructions including user ID setup

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Dockerfile Creates a non-root user 'spotdl' with customizable UID/GID and sets proper permissions
docker-compose.yml Adds build context with UID/GID args and updates volume mapping to ./downloads
README.md Adds docker-compose usage documentation with user ID configuration instructions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 43 to +48

# Change Workdir to download location
WORKDIR /music

# Fix permissions for music directory
RUN chown spotdl:spotdl /music
Copy link

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting ownership on the VOLUME /music directory will not persist when the volume is mounted at runtime. The chown command here affects the image layer but gets overridden when a host directory is mounted to /music. Consider removing this step or documenting that host directory permissions need to be managed separately.

Suggested change
# Change Workdir to download location
WORKDIR /music
# Fix permissions for music directory
RUN chown spotdl:spotdl /music
# NOTE: If you mount a host directory to /music, you must ensure the directory is writable by the 'spotdl' user (UID/GID), as permissions set here will not persist.
# Change Workdir to download location
WORKDIR /music
# (chown on /music removed; see note above)

Copilot uses AI. Check for mistakes.
image: "spotdl/spotify-downloader"
volumes:
- .:/music
- ./downloads:/music
Copy link

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The volume mapping has changed from mounting the current directory (.) to ./downloads. This is a breaking change that should be documented in the description as it changes the expected directory structure for existing users.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for your PR and sorry for the late review.
I see that you've set a default volume mapping when using compose, one thing to note is that since the
Docker container would not be root anymore, it wouldn't be able to access the files under /music in the container because of permissions.

Copilot points it out:
https://github.com/spotDL/spotify-downloader/pull/2489/changes#r2320603547

So we have two options:

  1. Move from bind volumes to named volumes. This would add a little more complexity to UX since now we would have to docker cp files from the volume to the host.
  2. Revert changes and keep using root user, but keep using bind mounts.

cc: @Silverarmor, if we want to improve security by not using root user I personally think option no. 1 is the way to go, even with the added complexity.

nevertheless, @aycsi please let me know what you think or other solutions you can come up with!

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.

3 participants