[BUGFIX] Fix file ownership of config files created in arm_user_files_setup.sh by dkreth · Pull Request #1660 · automatic-ripping-machine/automatic-ripping-machine

dkreth · 2026-01-02T19:12:10Z

Description

When config files are copied from /opt/arm/setup/ to /etc/arm/config/, they retain ownership (see man page for cp). If the startup script runs as root, then the config files are created in /etc/arm/config and owned by root. This prevents the arm user from modifying the config files via the web UI, causing a 500 error when attempting to save settings in the ARM UI.

To fix this, we add a chown command after cp to ensure config files are owned by the arm user, matching the ownership requirements of the /etc/arm/config directory.

Fixes #1639 #1599

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Docker

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have tested that my fix is effective

Changelog:

Add a chown command after cp to ensure config files are owned by the arm user

When config files are copied from /opt/arm/setup/ to /etc/arm/config/, they retain ownership (see man page for cp). If the startup script runs as root, then the config files are created in /etc/arm/config and owned by root. This prevents the arm user from modifying the config files via the web UI, causing a 500 error when attempting to save settings. Add chown command after cp to ensure config files are owned by the arm user, matching the ownership requirements of the config directory.

microtechno9000

agree with change to remove depreciated cp command

The container uses coreutils 8.32, which doesn't support --update=none (introduced in coreutils 9.1). Reverting to --no-clobber to maintain compatibility while keeping the chown fix. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

sonarqubecloud · 2026-01-28T14:35:57Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

dkreth · 2026-01-28T14:39:01Z

I reverted the change to remove the deprecated cp command. The container uses coreutils 8.32, which doesn't support --update=none (introduced in coreutils 9.1).

dkreth · 2026-01-28T14:45:17Z

Just tested this.

Script Output:

  Config not found! Creating config file: /etc/arm/config/arm.yaml
  Config not found! Creating config file: /etc/arm/config/apprise.yaml

File Ownership Result:

  -rw-r--r-- 1 arm  arm   4845 Jan 28 09:40 apprise.yaml
  -rwxr-xr-x 1 arm  arm  12649 Jan 28 09:40 arm.yaml

So both arm.yaml and apprise.yaml are owned by arm:arm.

dkreth · 2026-01-28T15:15:28Z

Problem

I just found an issue that was fixed by this commit but may still be present according to this issue from August 2024.

So... I'm concerned that my fix that's being introducing in this PR (adding the chown command) will cause the GitHub issue above to regress/reoccur.

@microtechno9000 I had a couple ideas, but curious on your thoughts here.

Option 1

I looked for a flag we could pass to the cp command, but didn't see one that fits our use case. The --no-preserve=ownership flag won't help here. The issue isn't copying ownership from the source, it's that cp creates the new file owned by whoever runs the command (root).

Option 2

Another option would be to create the file as the arm user instead of root

su arm -c "cp --no-clobber '/opt/arm/setup/${conf}' '${thisConf}'"

I believe this would work because the script already runs as root, so su arm works directly, but @microtechno9000 can you verify my assumption here?

If so, I think this would be the cleanest solution:

No chown call = no NFS/SFTP permission failures
File created with correct ownership from the start
Works as long as arm user has write access to the directory (which I believe is already verified by check_folder_ownership)

Option 3

Alternatively, the simplest option would be to just attempt the cp command and bailout if it fails. There's no chance at a reversion of issue #822, but it also will fail silently.

cp --no-clobber "/opt/arm/setup/${conf}" "${thisConf}"
chown arm:arm "${thisConf}" 2>/dev/null || true

microtechno9000 · 2026-02-04T11:48:55Z

Problem

I just found an issue that was fixed by this commit but may still be present according to this issue from August 2024.

So... I'm concerned that my fix that's being introducing in this PR (adding the chown command) will cause the GitHub issue above to regress/reoccur.

@microtechno9000 I had a couple ideas, but curious on your thoughts here.

Option 1

I looked for a flag we could pass to the cp command, but didn't see one that fits our use case. The --no-preserve=ownership flag won't help here. The issue isn't copying ownership from the source, it's that cp creates the new file owned by whoever runs the command (root).

Option 2

Another option would be to create the file as the arm user instead of root
su arm -c "cp --no-clobber '/opt/arm/setup/${conf}' '${thisConf}'"
I believe this would work because the script already runs as root, so su arm works directly, but @microtechno9000 can you verify my assumption here?

If so, I think this would be the cleanest solution:
* No `chown` call = no NFS/SFTP permission failures

* File created with correct ownership from the start

* Works as long as `arm` user has write access to the directory (which I believe is already verified by `check_folder_ownership`)
Option 3

Alternatively, the simplest option would be to just attempt the cp command and bailout if it fails. There's no chance at a reversion of issue #822, but it also will fail silently.
cp --no-clobber "/opt/arm/setup/${conf}" "${thisConf}"
chown arm:arm "${thisConf}" 2>/dev/null || true

This has indeed been an issue that keeps coming up, in particular because ARM uses a privileged container due to needing access to CD/DVD/Bluray drives and GPU compute. There are ways around this, but not easily.

I think Option 2 is the best, it will avoid NFS permissions issues (NFS can be fun at the best of times) and avoid ARM changing permissions outside a container.

If you update the code to Option 2 I can merge in

microtechno9000

pending change, update to option 2 and remove 'chown'

mihawk90 · 2026-02-05T00:46:48Z

Option 4 is just using the install command that was made for this purpose:

❯ man install

NAME
       install - copy files and set attributes

SYNOPSIS
       install [OPTION]... [-T] SOURCE DEST
       install [OPTION]... SOURCE... DIRECTORY
       install [OPTION]... -t DIRECTORY SOURCE...
       install [OPTION]... -d DIRECTORY...

…

       -g, --group=GROUP
              set group ownership, instead of process' current group

       -m, --mode=MODE
              set permission mode (as in chmod), instead of rwxr-xr-x

       -o, --owner=OWNER
              set ownership (super-user only)

…

On a side note, the --no-clobber feels rather redundant when the copy is only executed if the file isn't present to begin with, i.e. the script is already checking that the file is not present before copying, and then uses an option to not overwrite existing files?

…ping-machine#1605, automatic-ripping-machine#1660 PR automatic-ripping-machine#1698 — Main feature chapters/filesize sorting: - Add chapters (Integer) and filesize (BigInteger) to Track model - Parse MakeMKV TINFO fields 8 (chapters) and 11 (filesize) - Sort main feature by chapters desc, length desc, filesize desc, track_number asc — fixes Disney Blu-ray playlist obfuscation - Add BigInteger to database layer exports - Migration: a4b5c6d7e8f9 PR automatic-ripping-machine#1605 — TV series disc label parsing: - Add parse_disc_label_for_identifiers() for labels like STARGATE_ATLANTIS_S1_D2 → S1D2 - Add normalize_series_name() and get_tv_folder_name() - Integrate into Job.build_final_path() for TV series - Config keys: USE_DISC_LABEL_FOR_TV, GROUP_TV_DISCS_UNDER_SERIES - Migration: b5c6d7e8f9a0 PR automatic-ripping-machine#1660 — Config file ownership fix: - Replace cp --no-clobber with install -o arm -g arm -m 644 in arm_user_files_setup.sh to set ownership atomically Includes 33 new tests covering all three features.

…e#1660) - Use 'install -o arm -g arm' for abcde.conf (was plain cp, created as root) - Restore arm.yaml ownership after sed -i overrides (sed resets to root)

dkreth and others added 3 commits January 2, 2026 12:35

fix: replace deprecated --no-clobber with --update=none

ec826b8

Bump version from 2.21.0 to 2.21.2

18389da

microtechno9000 approved these changes Jan 20, 2026

View reviewed changes

dkreth and others added 2 commits January 28, 2026 07:12

Merge branch 'main' into fix-file-ownership

49afab0

microtechno9000 requested changes Feb 4, 2026

View reviewed changes

microtechno9000 added Awaiting feedback Waiting for user to test fixes Bugfix Bugfixes in PR labels Feb 17, 2026

uprightbass360 mentioned this pull request Mar 3, 2026

feat: port upstream PRs #1698, #1605, #1660 uprightbass360/automatic-ripping-machine-neu#89

Merged

3 tasks

This was referenced Mar 15, 2026

fix: harden upstream ports (#60, #1605, #1639, #1660, #1664) uprightbass360/automatic-ripping-machine-neu#108

Closed

fix: harden upstream ports (#60, #1605, #1639, #1660, #1664) uprightbass360/automatic-ripping-machine-neu#109

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUGFIX] Fix file ownership of config files created in arm_user_files_setup.sh#1660

[BUGFIX] Fix file ownership of config files created in arm_user_files_setup.sh#1660
dkreth wants to merge 5 commits intoautomatic-ripping-machine:mainfrom
dkreth:fix-file-ownership

dkreth commented Jan 2, 2026 •

edited

Loading

Uh oh!

microtechno9000 left a comment

Uh oh!

sonarqubecloud bot commented Jan 28, 2026

Uh oh!

dkreth commented Jan 28, 2026

Uh oh!

dkreth commented Jan 28, 2026 •

edited

Loading

Uh oh!

dkreth commented Jan 28, 2026

Uh oh!

microtechno9000 commented Feb 4, 2026

Problem

Option 1

Option 2

Option 3

Uh oh!

microtechno9000 left a comment

Uh oh!

mihawk90 commented Feb 5, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dkreth commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Checklist:

Changelog:

Uh oh!

microtechno9000 left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 28, 2026

Quality Gate passed

Uh oh!

dkreth commented Jan 28, 2026

Uh oh!

dkreth commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkreth commented Jan 28, 2026

Problem

Option 1

Option 2

Option 3

Uh oh!

microtechno9000 commented Feb 4, 2026

Problem

Option 1

Option 2

Option 3

Uh oh!

microtechno9000 left a comment

Choose a reason for hiding this comment

Uh oh!

mihawk90 commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dkreth commented Jan 2, 2026 •

edited

Loading

dkreth commented Jan 28, 2026 •

edited

Loading

mihawk90 commented Feb 5, 2026 •

edited

Loading