-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
nixos/immich: restrict media filesystem permissions #361627
Conversation
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.
We should rather set serviceConfig.UMask
.
d38f602
to
ed020bb
Compare
That does seem more elegant, but will it fix up existing installations? Seems like a shame to leave some unknown number of users with world-readable data. |
I don't think we should mess with existing installations. What if someone manually adjusted the permissions? |
ed020bb
to
8efc344
Compare
Applied suggestions. Does also excluding group permissions serve a security purpose, or is it just on principle of minimizing surface area? |
I guess it's the latter. I'm just following what most other NixOS services with a umask do. |
immich appears to create this directory with permissions 0755 by default, which needlessly exposes user data to other processes.
8efc344
to
fd21ef2
Compare
@ofborg test immich |
1 similar comment
@ofborg test immich |
immich appears to create this directory with permissions 0755 by default, which needlessly exposes user data to other processes.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.