Skip to content

Limit the scope of fixing permissions #1142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Limit the scope of fixing permissions #1142

wants to merge 1 commit into from

Conversation

kode54
Copy link

@kode54 kode54 commented Mar 3, 2025

Permissions should only be fixed for mutable data directories, not the Docker image itself.

@kode54 kode54 force-pushed the kode54-perm-fix-1 branch 2 times, most recently from 6290ccf to 0cbfc3f Compare March 3, 2025 08:16
@GlassedSilver
Copy link
Collaborator

Have you considered this?

#921 (comment)

@kode54
Copy link
Author

kode54 commented Mar 5, 2025

Guess I'll just use my own fork forever then.

@kode54 kode54 closed this Mar 5, 2025
@GlassedSilver
Copy link
Collaborator

I only asked a question, not sure why you jump to any premature conclusions, especially because I'm only casually spotting and reading the PR, I don't really maintain the repo for lack of knowhow, I'm grateful for any contributions as long as they are fit for general distribution.

@martadinata666
Copy link
Collaborator

martadinata666 commented Mar 5, 2025 •
edited
Loading

Let's take a sample from docker-compose, change to nightly tag and set UID&GID, and override entrypoint.sh from this PR

version: "2"
services:
    ytdl_material:
        environment: 
            ytdl_mongodb_connection_string: 'mongodb://ytdl-mongo-db:27017'
            ytdl_use_local_db: 'false'
            write_ytdl_config: 'true'
            UID: 2000
            GID: 3000
        restart: always
        depends_on:
            - ytdl-mongo-db
        volumes:
            - ./appdata:/app/appdata
            - ./audio:/app/audio
            - ./video:/app/video
            - ./subscriptions:/app/subscriptions
            - ./users:/app/users
            - ./entrypoint-test.sh:/app/entrypoint.sh
        ports:
            - "8998:17442"
        image: tzahi12345/youtubedl-material:nightly
    ytdl-mongo-db:
        # If you are using a Raspberry Pi, use mongo:4.4.18
        image: mongo:4
        logging:
            driver: "none"          
        container_name: mongo-db
        restart: always
        volumes:
            - ./db/:/data/db

Run result, when the videos downloading works, the nodejs throws errors because of this permission thing. But somehow, the web app works when I'm not really diving into web test properly to find the exact error.

Attaching to ytdl_material-1
ytdl_material-1  | [entrypoint] setup permission, this may take a while
ytdl_material-1  | 
ytdl_material-1  | > [email protected] start
ytdl_material-1  | > pm2-runtime --raw pm2.config.js
ytdl_material-1  | 
ytdl_material-1  | 2025-03-05T02:52:26: PM2 log: Launching in no daemon mode
ytdl_material-1  | 2025-03-05T02:52:26: PM2 log: [Watch] Start watching YoutubeDL-Material
ytdl_material-1  | 2025-03-05T02:52:26: PM2 log: App [YoutubeDL-Material:0] starting in -fork mode-
ytdl_material-1  | 2025-03-05T02:52:26: PM2 log: App [YoutubeDL-Material:0] online
ytdl_material-1  | 2025-03-05T02:52:28.679Z ERROR: Failed to get config file
ytdl_material-1  | 2025-03-05T02:52:28.682Z ERROR: Failed to get config file
ytdl_material-1  | 2025-03-05T02:52:28.709Z INFO: Cannot find config file. Creating one with default values...
ytdl_material-1  | 2025-03-05T02:52:28.720Z INFO: Config items set using ENV variables.
ytdl_material-1  | TypeError: Cannot use 'in' operator to search for 'YoutubeDLMaterial' in null
ytdl_material-1  |     at Function.Object.byString (/app/config.js:41:15)
ytdl_material-1  |     at Object.exports.getConfigItem (/app/config.js:106:24)
ytdl_material-1  |     at setupTelegramBot (/app/notifications.js:168:41)
ytdl_material-1  |     at Object.<anonymous> (/app/notifications.js:155:1)
ytdl_material-1  |     at Module._compile (node:internal/modules/cjs/loader:1103:14)
ytdl_material-1  |     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1157:10)
ytdl_material-1  |     at Module.load (node:internal/modules/cjs/loader:981:32)
ytdl_material-1  |     at Function.Module._load (node:internal/modules/cjs/loader:822:12)
ytdl_material-1  |     at Module.require (node:internal/modules/cjs/loader:1005:19)
ytdl_material-1  |     at Module.Hook._require.Module.require (/usr/local/nvm/versions/node/v16.14.2/lib/node_modules/pm2/node_modules/require-in-the-middle/index.js:101:39)
ytdl_material-1  |     at require (node:internal/modules/cjs/helpers:102:18)
ytdl_material-1  |     at Object.<anonymous> (/app/downloader.js:16:27)
ytdl_material-1  |     at Module._compile (node:internal/modules/cjs/loader:1103:14)
ytdl_material-1  |     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1157:10)
ytdl_material-1  |     at Module.load (node:internal/modules/cjs/loader:981:32)
ytdl_material-1  |     at Function.Module._load (node:internal/modules/cjs/loader:822:12)
ytdl_material-1  |     at Module.require (node:internal/modules/cjs/loader:1005:19)
ytdl_material-1  |     at Module.Hook._require.Module.require (/usr/local/nvm/versions/node/v16.14.2/lib/node_modules/pm2/node_modules/require-in-the-middle/index.js:101:39)
ytdl_material-1  |     at require (node:internal/modules/cjs/helpers:102:18)
ytdl_material-1  |     at Object.<anonymous> (/app/app.js:25:24)
ytdl_material-1  |     at Module._compile (node:internal/modules/cjs/loader:1103:14)
ytdl_material-1  |     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1157:10)
ytdl_material-1  |     at Module.load (node:internal/modules/cjs/loader:981:32)
ytdl_material-1  |     at Function.Module._load (node:internal/modules/cjs/loader:822:12)
ytdl_material-1  |     at Object.<anonymous> (/usr/local/nvm/versions/node/v16.14.2/lib/node_modules/pm2/lib/ProcessContainerFork.js:33:23)
ytdl_material-1  |     at Module._compile (node:internal/modules/cjs/loader:1103:14)
ytdl_material-1  |     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1157:10)
ytdl_material-1  |     at Module.load (node:internal/modules/cjs/loader:981:32)
ytdl_material-1  | TypeError: Cannot use 'in' operator to search for 'YoutubeDLMaterial' in null
ytdl_material-1  |     at Function.Object.byString (/app/config.js:41:15)
ytdl_material-1  |     at Object.exports.getConfigItem (/app/config.js:106:24)
ytdl_material-1  |     at /app/notifications.js:157:41
ytdl_material-1  |     at Object.next (/app/node_modules/rxjs/dist/cjs/internal/Subscriber.js:161:21)
ytdl_material-1  |     at SafeSubscriber.Subscriber._next (/app/node_modules/rxjs/dist/cjs/internal/Subscriber.js:101:26)
ytdl_material-1  |     at SafeSubscriber.Subscriber.next (/app/node_modules/rxjs/dist/cjs/internal/Subscriber.js:72:18)
ytdl_material-1  |     at BehaviorSubject._subscribe (/app/node_modules/rxjs/dist/cjs/internal/BehaviorSubject.js:36:44)
ytdl_material-1  |     at BehaviorSubject.Observable._trySubscribe (/app/node_modules/rxjs/dist/cjs/internal/Observable.js:41:25)
ytdl_material-1  |     at BehaviorSubject.Subject._trySubscribe (/app/node_modules/rxjs/dist/cjs/internal/Subject.js:120:47)
ytdl_material-1  |     at /app/node_modules/rxjs/dist/cjs/internal/Observable.js:35:31
ytdl_material-1  |     at Object.errorContext (/app/node_modules/rxjs/dist/cjs/internal/util/errorContext.js:22:9)
ytdl_material-1  |     at BehaviorSubject.Observable.subscribe (/app/node_modules/rxjs/dist/cjs/internal/Observable.js:26:24)
ytdl_material-1  |     at Object.<anonymous> (/app/notifications.js:156:27)
ytdl_material-1  |     at Module._compile (node:internal/modules/cjs/loader:1103:14)
ytdl_material-1  |     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1157:10)
ytdl_material-1  |     at Module.load (node:internal/modules/cjs/loader:981:32)
ytdl_material-1  |     at Function.Module._load (node:internal/modules/cjs/loader:822:12)
ytdl_material-1  |     at Module.require (node:internal/modules/cjs/loader:1005:19)
ytdl_material-1  |     at Module.Hook._require.Module.require (/usr/local/nvm/versions/node/v16.14.2/lib/node_modules/pm2/node_modules/require-in-the-middle/index.js:101:39)
ytdl_material-1  |     at require (node:internal/modules/cjs/helpers:102:18)
ytdl_material-1  |     at Object.<anonymous> (/app/downloader.js:16:27)
ytdl_material-1  |     at Module._compile (node:internal/modules/cjs/loader:1103:14)
ytdl_material-1  |     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1157:10)
ytdl_material-1  |     at Module.load (node:internal/modules/cjs/loader:981:32)
ytdl_material-1  |     at Function.Module._load (node:internal/modules/cjs/loader:822:12)
ytdl_material-1  |     at Module.require (node:internal/modules/cjs/loader:1005:19)
ytdl_material-1  |     at Module.Hook._require.Module.require (/usr/local/nvm/versions/node/v16.14.2/lib/node_modules/pm2/node_modules/require-in-the-middle/index.js:101:39)
ytdl_material-1  |     at require (node:internal/modules/cjs/helpers:102:18)
ytdl_material-1  | 2025-03-05T02:52:28: PM2 log: App [YoutubeDL-Material:0] exited with code [1] via signal [SIGINT]
ytdl_material-1  | 2025-03-05T02:52:28: PM2 log: App [YoutubeDL-Material:0] starting in -fork mode-
ytdl_material-1  | 2025-03-05T02:52:28: PM2 log: App [YoutubeDL-Material:0] online
ytdl_material-1  | 2025-03-05T02:52:29.965Z INFO: Config items set using ENV variables.
ytdl_material-1  | 2025-03-05T02:52:30.102Z INFO: Beginning migration: 4.1->4.2+
ytdl_material-1  | 2025-03-05T02:52:30.145Z ERROR: Invalid Operation, no operations specified
ytdl_material-1  | 2025-03-05T02:52:30.146Z ERROR: Migration failed: 4.1->4.2+
ytdl_material-1  | 2025-03-05T02:52:30.146Z INFO: Beginning migration: 4.2->4.3+
ytdl_material-1  | 2025-03-05T02:52:30.615Z INFO: 4.2->4.3+ migration complete!
ytdl_material-1  | 2025-03-05T02:52:30.615Z INFO: Checking if tasks manager role permissions exist for admin user...
ytdl_material-1  | 2025-03-05T02:52:30.618Z INFO: Task manager permissions check complete!
ytdl_material-1  | 2025-03-05T02:52:30.618Z INFO: Checking if archives have been migrated...
ytdl_material-1  | 2025-03-05T02:52:30.621Z INFO: Archives migration complete!
ytdl_material-1  | 2025-03-05T02:52:30.622Z WARN: Failed to get youtube-dl binary details at location 'appdata/youtube-dl.json'. Generating file...
ytdl_material-1  | 2025-03-05T02:52:30.978Z WARN: Updating yt-dlp binary to 'appdata/bin/yt-dlp', downloading...
ytdl_material-1  | 2025-03-05T02:52:33.141Z INFO: YoutubeDL-Material v4.3.2 started on PORT 17442
ytdl_material-1  | 2025-03-05T02:52:33.141Z INFO: Skipping subscription check as no valid subscriptions exist.

UID & GID result

root@ea36507a6a2f:/app# ls -aln
total 564
drwxr-xr-x   1 2000 3000   4096 Mar  5 02:52 .
drwxr-xr-x   1    0    0   4096 Mar  5 02:52 ..
-rw-r--r--   1 2000 3000     49 Feb 27 23:54 .dockerignore
-rw-r--r--   1 2000 3000    293 Feb 27 23:54 .eslintrc.json
drwxr-xr-x   1 1000 1000   4096 Feb 27 23:56 .npm
-rw-r--r--   1 1000 1000  70948 Feb 27 23:54 app.js
drwxr-xr-x   1 2000 3000    294 Mar  5 02:52 appdata
-rw-r--r--   1 1000 1000   3768 Feb 27 23:54 archive.js
drwxr-xr-x   1 2000 3000      0 Mar  5 02:52 audio
drwxr-xr-x   1 2000 3000   4096 Feb 27 23:54 authentication
-rw-r--r--   1 1000 1000   4865 Feb 27 23:54 categories.js
-rw-r--r--   1 1000 1000   9516 Feb 27 23:54 config.js
-rw-r--r--   1 1000 1000  10976 Feb 27 23:54 consts.js
-rw-r--r--   1 1000 1000  30119 Feb 27 23:54 db.js
-rw-r--r--   1 1000 1000  29050 Feb 27 23:54 downloader.js
-rw-r--r--   1 1000 1000    148 Feb 27 23:54 ecosystem.config.js
-rwxrwxr-x   1 2000 3000    528 Mar  3 10:42 entrypoint.sh
-rw-r--r--   1 1000 1000  13052 Feb 27 23:54 files.js
drwxr-xr-x   1 1000 1000   4096 Feb 27 23:54 fix-scripts
-rw-r--r--   1 1000 1000    869 Feb 27 23:54 logger.js
drwxr-xr-x 423 1000 1000  16384 Feb 27 23:56 node_modules
-rw-r--r--   1 1000 1000  10455 Feb 27 23:54 notifications.js
-rw-r--r--   1 2000 3000 197136 Feb 27 23:56 package-lock.json
-rw-r--r--   1 2000 3000   1731 Feb 27 23:54 package.json
drwxr-xr-x   5 2000 3000   4096 Mar  5 02:52 pm2
-rw-r--r--   1 1000 1000    195 Feb 27 23:54 pm2.config.js
drwxr-xr-x   3 1000 1000   4096 Feb 27 23:59 public
drwxr-xr-x   1 2000 3000      0 Mar  5 02:52 subscriptions
-rw-r--r--   1 1000 1000  24657 Feb 27 23:54 subscriptions.js
-rw-r--r--   1 1000 1000  13741 Feb 27 23:54 tasks.js
drwxr-xr-x   1 2000 3000   4096 Feb 27 23:54 test
-rw-r--r--   1 1000 1000   4674 Feb 27 23:54 twitch.js
drwxr-xr-x   1 2000 3000      0 Mar  5 02:52 users
-rw-r--r--   1 1000 1000  19454 Feb 27 23:54 utils.js
-rw-r--r--   1 2000 3000     79 Feb 27 23:54 version.json
drwxr-xr-x   1 2000 3000      0 Mar  5 02:52 video
-rw-r--r--   1 1000 1000   7021 Feb 27 23:54 youtube-dl.js

TLDR;

  1. Don't set UID GID; it uses a 1000:1000 combo that most people use, as long as the permission line is up to 1000:1000, it doesn't chown anything.
  2. The reason the old PR to chown everything because some people just don't care managing permission, it expect docker image to solved it.
  3. The other way it just to line up application permission when using custom UID GID,let this:
            - ./appdata:/app/appdata
            - ./audio:/app/audio
            - ./video:/app/video
            - ./subscriptions:/app/subscriptions
            - ./users:/app/users

user-managed permission. Considering some people had large library sizes.

Thus at that point, number 2 solution chosen, resulting slow startup.

@kode54
Copy link
Author

kode54 commented Mar 5, 2025

Sorry, I was kind of busy and feeling bad earlier. I'll read over this and see what I can do.

@martadinata666 martadinata666 reopened this Mar 5, 2025
@kode54
Copy link
Author

kode54 commented Mar 5, 2025 •
edited
Loading

Oh, I see. There's a pm2.config.js that needs write permissions on first run. I'll account for that too. Oh, multiple .config.js files. Let's see.

@kode54
Limit the scope of fixing permissions
76e8cdf 
Permissions should only be fixed for mutable data directories, not the Docker image itself.

v2: Change to only ignore node_modules
v3: Throw in some extra restrictions for static content that is part of
    the Docker image
v4: Include .config.js files in the list as necessary
@kode54 kode54 force-pushed the kode54-perm-fix-1 branch from 0cbfc3f to 76e8cdf Compare March 5, 2025 03:44
@kode54
Copy link
Author

kode54 commented Mar 25, 2025

Dang it, I need to update this PR, because it isn't setting ownership for the .cache folder. :/

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

@martadinata666 martadinata666 Awaiting requested review from martadinata666

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kode54 @GlassedSilver @martadinata666