nixos/nextcloud: Remove --preserve-env in sudo

[Mynacol]

Description of changes

This helps supporting sudo-rs, which currently does not implement the --preserve-env flag and probably won't so in the foreseeable future [1].

The replacement just sets both environment variables behind the sudo invocation, which works with exec and both sudo implementations.

[1] trifectatechfoundation/sudo-rs#129

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[Mynacol]

Ping @Ma27 @SuperSandro2000
I didn't test this patch yet, I wanted to ask you first if you'd be open to it.

[Ma27]

Would be fine by me.

[Shawn8901]

It seems to break the nextcloud-occ when being run as nextcloud user

[nextcloud@tank:~]$ nextcloud-occ
/run/current-system/sw/bin/nextcloud-occ: line 8: /nix/store/266vrngrbyyc2hnn8j7lak2y4nm70sa2-nextcloud-29.0.2-with-apps/NEXTCLOUD_CONFIG_DIR=/persist/var/lib/nextcloud/config: No such file or directory

[nextcloud@tank:~]$

It also broke on system activation after cherry-picking the PR into my nixpkgs fork.

I try to take a deeper look later when I am at a desktop machine

[Ma27]

To be clear, I agree with the idea, I haven't taken a close look whether the new invocation is actually correct currently 😁

[Shawn8901]

From my side all good 😊, I am also the option that it's a nice idea to keep compatible with sudo-rs if possible, so I wanted to test it out.

Wrapper invocation for non nextcloud users work fine.

[SuperSandro2000]

Wouldn't this expose the pass env in the exec args and be displayed in programs like htop?

Why can't they implement this simple feature? This could be done in a few lines shell script.

[Ma27]

Wouldn't this expose the pass env in the exec args and be displayed in programs like htop?

Dammit, I forgot about OC_PASS. However, is there any reason this even exists? Just checked the sources and it's only used for adding a user or resetting someone's password, however other alternatives are also OK.

Can't remember having it used a single time.

[Shawn8901]

From my side all good 😊, I am also the option that it's a nice idea to keep compatible with sudo-rs if possible, so I wanted to test it out.

Wrapper invocation for non nextcloud users work fine.

To make it compatible with the exec command we have either to do

$sudo env NEXTCLOUD_CONFIG_DIR="${datadir}/config"   OC_PASS="$OC_PASS"

or

NEXTCLOUD_CONFIG_DIR="${datadir}/config"   OC_PASS="$OC_PASS" $sudo

Tho in case we keep OC_PASS , we should not expose it.

OC_PASS is used for --password-from-env (like @Ma27 already mentioned) --> https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/occ_command.html
That could be a nice thing for creating users declarative (in case we want to do that at some point as we could also query the available users and create it from a password file if not existing or so [just wild guessing for what we could use it in our context]).

[Ma27]

That could be a nice thing for creating users declarative

Just want to mention here that if this suggestion is to be some kind of ensure*-style option, then it isn't declarative and I'm strongly against adding it to this module. See also #248334

If the VM tests still pass with OC_PASS removed (i.e. installing Nextcloud still works without it), I'd suggest to just drop it.

[Shawn8901]

yeah, then we should remove it, totally agreeing from my end.

[Mynacol]

Alright, I tested this PR on my server. Directly setting env variables as the first attempt tried is also not implemented (warning: CLI-level env var list has not yet been implemented and will be ignored). Instead, I call env, which is always available and provides this feature.

I didn't think about OC_PASS being exposed, I dropped it, matching the last conclusion from you all.

[Mynacol]

@ofborg test nextcloud