Skip to content
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.

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

Support not running as root #24

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support not running as root #24

wants to merge 1 commit into from

Conversation

chadmed
Copy link
Member

@chadmed chadmed commented Mar 1, 2025

We don't need to be running as root. Let's run as speakersafetyd by default.

@chadmed
Copy link
Member Author

chadmed commented Mar 1, 2025

Should help with #23 and #19

@@ -4,6 +4,9 @@ Description=Speaker Protection Daemon
[Service]
Type=simple
ExecStart=/usr/bin/speakersafetyd -c /usr/share/speakersafetyd/ -b /var/lib/speakersafetyd/blackbox -m 7
User=speakersafetyd
Copy link
Member

Choose a reason for hiding this comment

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

It might make sense to use DynamicUser for this, so distributions won't have to manage yet another static service user. Something like:

DynamicUser=yes
User=speakersafetyd
Group=speakersafetyd
RuntimeDirectory=/run/speakersafetyd
StateDirectory=/var/lib/speakersafetyd

(completely untested to be clear)

Choose a reason for hiding this comment

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

Nay on dynamic user, speakersafetyd generates a "blackbox" file on crash, and services using dynamic user are not supposed to leave non-temporary files on exit.

We don't need to be running as root. Let's run as speakersafetyd
by default.

Signed-off-by: James Calligeros <[email protected]>
@dkwo
Copy link

dkwo commented Mar 2, 2025

Thanks a lot! I've been running like so for some time on my Void machine, and it works well.
Out of abundance of caution, I gave it the sys_nice capability, since it uses uclamp: do you think this is needed?

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.

4 participants