From e30311bc68677a1521f9218478e799dd82214825 Mon Sep 17 00:00:00 2001 From: Benjamin Koch Date: Mon, 8 Mar 2021 01:06:48 +0100 Subject: [PATCH 1/4] nixos/nextcloud: Conditionally enable ImageMagick PHP extension --- nixos/modules/services/web-apps/nextcloud.nix | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/nixos/modules/services/web-apps/nextcloud.nix b/nixos/modules/services/web-apps/nextcloud.nix index 5636415f6a0d..28524df078c5 100644 --- a/nixos/modules/services/web-apps/nextcloud.nix +++ b/nixos/modules/services/web-apps/nextcloud.nix @@ -10,7 +10,7 @@ let extensions = { enabled, all }: (with all; enabled - ++ [ imagick ] # Always enabled + ++ optional cfg.imagemagick imagick # Optionally enabled depending on caching settings ++ optional cfg.caching.apcu apcu ++ optional cfg.caching.redis redis @@ -303,6 +303,18 @@ in { }; }; + imagemagick = mkOption { + type = types.bool; + default = true; + description = '' + Whether to load the ImageMagick module into PHP. + This is used by the theming app and for generating previews of certain images (e.g. SVG and HEIF). + You may want to disable this for increased security. In that case, previews will still be available + for some images (e.g. JPEG and PNG). + See https://github.com/nextcloud/server/issues/13099 + ''; + }; + caching = { apcu = mkOption { type = types.bool; From 6e6f5f09234d3744eb87c92b95f4a795867676f9 Mon Sep 17 00:00:00 2001 From: Benjamin Koch Date: Tue, 9 Mar 2021 00:02:33 +0100 Subject: [PATCH 2/4] nixos/nextcloud: Rename option to services.nextcloud.disableImagemagick ... as was suggested in the related issue --- nixos/modules/services/web-apps/nextcloud.nix | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nixos/modules/services/web-apps/nextcloud.nix b/nixos/modules/services/web-apps/nextcloud.nix index 28524df078c5..9a541aba6e43 100644 --- a/nixos/modules/services/web-apps/nextcloud.nix +++ b/nixos/modules/services/web-apps/nextcloud.nix @@ -10,7 +10,7 @@ let extensions = { enabled, all }: (with all; enabled - ++ optional cfg.imagemagick imagick + ++ optional (!cfg.disableImagemagick) imagick # Optionally enabled depending on caching settings ++ optional cfg.caching.apcu apcu ++ optional cfg.caching.redis redis @@ -303,13 +303,13 @@ in { }; }; - imagemagick = mkOption { + disableImagemagick = mkOption { type = types.bool; - default = true; + default = false; description = '' - Whether to load the ImageMagick module into PHP. + Whether to not load the ImageMagick module into PHP. This is used by the theming app and for generating previews of certain images (e.g. SVG and HEIF). - You may want to disable this for increased security. In that case, previews will still be available + You may want to disable it for increased security. In that case, previews will still be available for some images (e.g. JPEG and PNG). See https://github.com/nextcloud/server/issues/13099 ''; From d83a43a396c7ec5ac2cd633262648306e25c6789 Mon Sep 17 00:00:00 2001 From: Benjamin Koch Date: Tue, 9 Mar 2021 01:44:18 +0100 Subject: [PATCH 3/4] nixos/nextcloud: Add test for services.nextcloud.disableImagemagick --- nixos/tests/nextcloud/basic.nix | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/nixos/tests/nextcloud/basic.nix b/nixos/tests/nextcloud/basic.nix index 0b8e1937128c..5d31165208cb 100644 --- a/nixos/tests/nextcloud/basic.nix +++ b/nixos/tests/nextcloud/basic.nix @@ -7,7 +7,7 @@ in { maintainers = [ globin eqyiel ]; }; - nodes = { + nodes = rec { # The only thing the client needs to do is download a file. client = { ... }: { services.davfs2.enable = true; @@ -47,6 +47,11 @@ in { environment.systemPackages = [ cfg.services.nextcloud.occ ]; }; + + nextcloudWithoutMagick = args@{ config, pkgs, lib, ... }: + lib.mkMerge + [ (nextcloud args) + { services.nextcloud.disableImagemagick = true; } ]; }; testScript = let @@ -69,7 +74,8 @@ in { diff <(echo 'hi') <(${pkgs.rclone}/bin/rclone cat nextcloud:test-shared-file) ''; in '' - start_all() + nextcloud.start() + client.start() nextcloud.wait_for_unit("multi-user.target") # This is just to ensure the nextcloud-occ program is working nextcloud.succeed("nextcloud-occ status") @@ -82,5 +88,13 @@ in { "${withRcloneEnv} ${diffSharedFile}" ) assert "hi" in client.succeed("cat /mnt/dav/test-shared-file") + + #client.succeed("nix path-info -r " + nextcloud.script + " | grep imagick") + #client.fail("nix path-info -r " + nextcloudWithoutMagick.script + " | grep imagick") + assert os.system("nix path-info -r " + nextcloud.script + " | grep imagick") == 0 + assert ( + os.system("nix path-info -r " + nextcloudWithoutMagick.script + " | grep imagick") + != 0 + ) ''; }) From 1bd7941a6b19a410ed4b38eaff8bf88592f21457 Mon Sep 17 00:00:00 2001 From: Benjamin Koch Date: Thu, 11 Mar 2021 02:56:11 +0100 Subject: [PATCH 4/4] nixos/nextcloud: Use exportReferencesGraph for imagick test --- nixos/tests/nextcloud/basic.nix | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/nixos/tests/nextcloud/basic.nix b/nixos/tests/nextcloud/basic.nix index 5d31165208cb..5074b6cdafef 100644 --- a/nixos/tests/nextcloud/basic.nix +++ b/nixos/tests/nextcloud/basic.nix @@ -54,7 +54,7 @@ in { { services.nextcloud.disableImagemagick = true; } ]; }; - testScript = let + testScript = { nodes, ... }: let withRcloneEnv = pkgs.writeScript "with-rclone-env" '' #!${pkgs.runtimeShell} export RCLONE_CONFIG_NEXTCLOUD_TYPE=webdav @@ -73,7 +73,17 @@ in { #!${pkgs.runtimeShell} diff <(echo 'hi') <(${pkgs.rclone}/bin/rclone cat nextcloud:test-shared-file) ''; + + findInClosure = what: drv: pkgs.runCommand "find-in-closure" { exportReferencesGraph = [ "graph" drv ]; inherit what; } '' + test -e graph + grep "$what" graph >$out || true + ''; + nextcloudUsesImagick = findInClosure "imagick" nodes.nextcloud.config.system.build.vm; + nextcloudWithoutDoesntUseIt = findInClosure "imagick" nodes.nextcloudWithoutMagick.config.system.build.vm; in '' + assert open("${nextcloudUsesImagick}").read() != "" + assert open("${nextcloudWithoutDoesntUseIt}").read() == "" + nextcloud.start() client.start() nextcloud.wait_for_unit("multi-user.target") @@ -88,13 +98,5 @@ in { "${withRcloneEnv} ${diffSharedFile}" ) assert "hi" in client.succeed("cat /mnt/dav/test-shared-file") - - #client.succeed("nix path-info -r " + nextcloud.script + " | grep imagick") - #client.fail("nix path-info -r " + nextcloudWithoutMagick.script + " | grep imagick") - assert os.system("nix path-info -r " + nextcloud.script + " | grep imagick") == 0 - assert ( - os.system("nix path-info -r " + nextcloudWithoutMagick.script + " | grep imagick") - != 0 - ) ''; })