-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
xdg-autostart: Add readOnly option #6629
base: master
Are you sure you want to change the base?
Conversation
modules/misc/xdg-autostart.nix
Outdated
xdg.configFile = if cfg.readOnly then { | ||
autostart.source = linkedDesktopEntries; | ||
} else | ||
listToAttrs (map mapDesktopEntry cfg.entries); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the recursive
option for HM's file options would be cleaner, I think. Recursive = true would link all files inside source
recursively, whereas recursive = false would link autostart
directly to source
.
This also allows us to remove mapDesktopEntry
above.
xdg.configFile = if cfg.readOnly then { | |
autostart.source = linkedDesktopEntries; | |
} else | |
listToAttrs (map mapDesktopEntry cfg.entries); | |
xdg.configFile."autostart" = { | |
recursive = !cfg.readOnly; | |
source = linkedDesktopEntries; | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback, using recursive
didn't occur to me but that does seem to work as intended so I've changed the implementation and added a test.
d16ecfc
to
48c0cc8
Compare
When `readOnly` is set to `true` the autostart entries are linked from a readonly directory in the nix store and `XDG_CONFIG_HOME/autostart` is a link to that directory, so that programs cannot install arbitrary autostart services.
@@ -35,6 +41,9 @@ in { | |||
}; | |||
|
|||
config = mkIf (cfg.enable && cfg.entries != [ ]) { | |||
xdg.configFile = listToAttrs (map mapDesktopEntry cfg.entries); | |||
xdg.configFile.autostart = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I personally like to emphasize attrset keys in NixOS options by passing them in quotes. This is not a blocker for merging though.
xdg.configFile.autostart = { | |
xdg.configFile."autostart" = { |
Description
Some programs add a
.desktop
file toXDG_CONFIG_HOME/autostart
when they are started (can't remember from the top of my head which program did for me, and I've already deleted the file), but I want to be able to explicitly grant programs permission to autostart in my home manager configuration. This PR adds axdg.autostart.readOnly
option to make that possible.When
readOnly
is set totrue
the autostart entries are linked from a readonly directory in the nix store andXDG_CONFIG_HOME/autostart
is a link to that directory, so that programs cannot install arbitrary autostart services.I haven't added a test yet because I first wanted to check whether this has a chance of being merged and if we really need the
readOnly
option, or if we should just always makeXDG_CONFIG_HOME/autostart
readonly, because something similar was done without an option for backwards compatibility in #5553.Checklist
Change is backwards compatible.
Code formatted with
./format
.Code tested through
nix-shell --pure tests -A run.all
or
nix build --reference-lock-file flake.lock ./tests#test-all
using Flakes.Test cases updated/added. See example.
Commit messages are formatted like
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
Maintainer CC @Scrumplex