Nextcloud adminPassFile

MunsMan commented 3 months ago

Describe the bug

Unable to build the Nextcloud Service, if the password starts with a “-”. Similar issue reported by NextCloud issue. I guess the password was not properly escaped, but it took me hours to figure out, why my service wasn't work.

Steps To Reproduce

Steps to reproduce the behavior:

Storing a password starting with “-” in a sops-nix file
Setup Nextcloud for the first time with that password as adminPassFile

Expected behavior

Should escape the password properly and just work.

Additional context

Aug 21 23:48:30 mininix systemd[1]: Starting nextcloud-setup.service...
Aug 21 23:48:31 mininix nextcloud-setup-start[8795]:
Aug 21 23:48:31 mininix nextcloud-setup-start[8795]:   The "--admin-pass" option requires a value.
Aug 21 23:48:31 mininix nextcloud-setup-start[8795]:

Notify maintainers

Sadly, there are no active maintainers listed, resulting in pinging randomly using git-blame… @Ma27 @bachp @pennae @stuebinm

Metadata

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.6.40, NixOS, 24.11 (Vicuna), 24.11.20240716.ad0b5ee`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.5`
 - nixpkgs: `/nix/store/70y8v9qfn34sdfsnwh3l9772lk4rpfiy-source`

Add a :+1: reaction to issues you find important.

bjornfor commented 3 months ago

I had a look and the Nextcloud module and found lots of quoting (good). However:

In, mkExport = { arg, value }: "export ${arg}=${value}";, should ${value} be quoted?
Do we know that Nextcloud itself handles a password that starts with a dash?

MunsMan commented 3 months ago

I think that ${value} might need some quotes as well. I just found this PR, which describes the quotation off occ commands. It might be, that many special characters need to be escaped. In the Nextcloud docs are no mention, of special password formats. From the logs, it looks like that the occ install command is not escaped properly because in the end, Nextcloud can't be installed.

stuebinm commented 3 months ago

what you probably want to do is change how installFlags are handled; currently these are simply concatenated with whitespace. Changing that to use -adminpass=... syntax instead ought to fix the issue.

eyeballing it (i have not tested this, nor do i intend to spend much time on this),

            installFlags = concatStringsSep " \\\n    "
-              (mapAttrsToList (k: v: "${k} ${toString v}") {
+              (mapAttrsToList (k: v: "${k}=${toString v}") {
              "--database" = ''"${c.dbtype}"'';

ought to be enough (I'm honestly not sure why these arguments are handled with this much abstraction on top, but I won't question it).

Ma27 commented 3 months ago

The thing is, while this command runs, everybody can read the admin password since the cmdline is world-readable (also it's only read on the very first install because Nextcloud demands it and ignored later). So I think it's better anyways to immediately change the password in the UI after provisioning.

If @stuebinm's fix works, I'm happy to accept a PR (preferably with a test) for this.

'm honestly not sure why these arguments are handled with this much abstraction on top, but I won't question it

Probably just grown historically :shrug:

NixOS / nixpkgs