`swapDevices` with `randomEncryption` enabled don't work if `device` contains `:` character

davidkna commented 2 years ago

Describe the bug

Since the last unstable update, swapDevices when used with a device that contain a colon like /dev/md/host:name or /disk/by-id/md-uuid-00000000:00000000:00000000:00000000 with swapDevices.*.randomEncryption = true fail with:

error: store path ...-unit-script-mkswap-dev-md-host:name-start contains illegal character ':'

Steps To Reproduce

Steps to reproduce the behavior:

Set

swapDevices = [{
# Or any path with a colon that if tested
device = "/dev/md/disk:name";
randomEncryption.enable = true;
}];

Expected behavior

Colons don't leak into the script name, and device paths with a colon are supported (again) in this scenario.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

Add any other context about the problem here.

Notify maintainers

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 5.15.16, NixOS, 22.05 (Quokka)`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.5.1`
 - channels(david): `""`
 - channels(root): `"nixos-22.05pre348581.c07b471b52b"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

corpix commented 2 years ago

Same for devices identified by PCI address (openstack based clouds):

could reproduce on current nixpkgs master

error: store path 'vssk89wfn38af4pvnn4fg4cmdh2i07av-unit-script-mkfs-dev-disk-by--x2dpath-pci--x2d0000:02:02.0-start' contains illegal character ':'

UPD: I have duct-taped my problem with following patch:

diff --git nixos/modules/tasks/filesystems.nix nixos/modules/tasks/filesystems.nix
index f3da6771197..ebf3f64817b 100644
--- nixos/modules/tasks/filesystems.nix
+++ nixos/modules/tasks/filesystems.nix
@@ -302,7 +302,7 @@ in
             mountPoint' = "${escapeSystemdPath fs.mountPoint}.mount";
             device'  = escapeSystemdPath fs.device;
             device'' = "${device'}.device";
-          in nameValuePair "mkfs-${device'}"
+          in nameValuePair "mkfs-${replaceChars [":"] ["-"] device'}"
           { description = "Initialisation of Filesystem ${fs.device}";
             wantedBy = [ mountPoint' ];
             before = [ mountPoint' "systemd-fsck@${device'}.service" ];

First thought was: why can't we patch escapeSystemdPath to escape colons? Answer: no, we can't because it follows the semantics of systemd-escape which is not escaping colons. This will break matching between auto-generated (by systemd) units (like .device). I don't see a good solution for this. But... why should Nix deny colons in paths?

Atemu commented 5 months ago

Can you still repro this? I couldn't repro this using pkgs.writeShellScriptBin ":foo" "" which should be what NixOS does internally. Pretty much any call to stdenv.mkDerivation should be passed through sanitizeDerivationName which should take care of issues like this.

NixOS / nixpkgs