From 650481a58ae1663aa44ecb805f70cafde469859a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix=20Pi=C3=A9dallu?= <felix@piedallu.me>
Date: Thu, 28 Sep 2023 11:23:59 +0200
Subject: [PATCH 01/25] ynh_safe_rm: Check if target is a symlink

When calling ynh_safe_rm to a broken symlink, the function was erroring out.
(test -e was following the symlink and returning false)
We need to also check if it is a symlink before exiting.
---
 helpers/helpers.v2.1.d/utils | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index be57a6d73..0cbd7c004 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -168,7 +168,7 @@ ynh_safe_rm() {
 
     if [[ -z "$target" ]]; then
         ynh_print_warn "ynh_safe_rm called with empty argument, ignoring."
-    elif [[ ! -e $target ]]; then
+    elif [[ ! -e "$target" ]] && [[ ! -L "$target" ]]; then
         ynh_print_info "'$target' wasn't deleted because it doesn't exist."
     elif ! _acceptable_path_to_delete "$target"; then
         ynh_print_warn "Not deleting '$target' because it is not an acceptable path to delete."

From 8846381d4758dd891ba7cc218778851669069109 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix=20Pi=C3=A9dallu?= <felix@piedallu.me>
Date: Fri, 28 Jun 2024 14:43:46 +0200
Subject: [PATCH 02/25] Rework _ynh_apply_default_permissions, only check if
 target is a child of install_dir.

---
 helpers/helpers.v2.1.d/utils | 47 ++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index be57a6d73..ff378a951 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -226,41 +226,36 @@ ynh_app_upgrading_from_version_before_or_equal_to() {
     dpkg --compare-versions $YNH_APP_CURRENT_VERSION le $version
 }
 
-# Check if we should enforce sane default permissions (= disable rwx for 'others')
-# on file/folders handled with ynh_setup_source and ynh_config_add
+# Apply sane permissions for files installed by ynh_setup_source and ynh_config_add.
 #
 # [internal]
 #
-# Having a file others-readable or a folder others-executable(=enterable)
-# is a security risk comparable to "chmod 777"
-#
-# Configuration files may contain secrets. Or even just being able to enter a
-# folder may allow an attacker to do nasty stuff (maybe a file or subfolder has
-# some write permission enabled for 'other' and the attacker may edit the
-# content or create files as leverage for priviledge escalation ...)
-#
-# The sane default should be to set ownership to $app:$app.
-# In specific case, you may want to set the ownership to $app:www-data
-# for example if nginx needs access to static files.
+# * Anything below $install_dir is chown $app:$app and chmod o-rwx,g-w
+# * The rest is considered as system configuration and chown root, chmod 400
 #
 _ynh_apply_default_permissions() {
     local target=$1
 
-    chmod o-rwx $target
-    chmod g-w $target
-    chown -R root:root $target
-    if ynh_system_user_exists --username=$app; then
-        chown $app:$app $target
+    is_subdir() {
+        # Returns false if child or parent is empty
+        child=$(realpath "$1" 2>/dev/null)
+        parent=$(realpath "$2" 2>/dev/null)
+        [[ "${child/$parent/}" != "$child" ]]
+    }
+
+    # App files can have files of their own
+    if ynh_system_user_exists --username="$app"; then
+        if is_subdir "$target" "$install_dir" ||  is_subdir "$target" "$data_dir"; then
+            chmod -R u=rwX,g=rX,o=X "$target"
+            chown -R "$app:$app" "$target"
+            chown "$app:www-data" "$target"
+            return
+        fi
     fi
 
-    # Crons should be owned by root
-    # Also we don't want systemd conf, nginx conf or others stuff to be owned by the app,
-    # otherwise they could self-edit their own systemd conf and escalate privilege
-    if echo "$target" | grep -q '^/etc/cron\|/etc/php\|/etc/nginx/conf.d\|/etc/fail2ban\|/etc/systemd/system'
-    then
-        chmod 400 $target
-        chown root:root $target
-    fi
+    # Other files are considered system
+    chmod -R 400 "$target"
+    chown -R root:root "$target"
 }
 
 int_to_bool() {

From d9d404a5b24f1745f414a1bce47fda3c44007d1e Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 28 Jun 2024 16:06:40 +0200
Subject: [PATCH 03/25] ynh_setup_source: apply default perms *after*
 extracting files to hopefully remove the need to manually chown/chmod

---
 helpers/helpers.v2.1.d/sources | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/helpers/helpers.v2.1.d/sources b/helpers/helpers.v2.1.d/sources
index ced0e05c7..c0c2fb863 100644
--- a/helpers/helpers.v2.1.d/sources
+++ b/helpers/helpers.v2.1.d/sources
@@ -182,10 +182,6 @@ ynh_setup_source() {
     # Extract source into the app dir
     mkdir --parents "$dest_dir"
 
-    if [ -n "${install_dir:-}" ] && [ "$dest_dir" == "$install_dir" ]; then
-        _ynh_apply_default_permissions $dest_dir
-    fi
-
     if [[ "$src_extract" == "false" ]]; then
         if [[ -z "$src_rename" ]]
         then
@@ -258,4 +254,8 @@ ynh_setup_source() {
         done
     fi
     rm -rf /var/cache/yunohost/files_to_keep_during_setup_source/
+
+    if [ -n "${install_dir:-}" ] && [ "$dest_dir" == "$install_dir" ]; then
+        _ynh_apply_default_permissions $dest_dir
+    fi
 }

From 3608c5678c145ce4b02caca350c913ef67a1a41f Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 28 Jun 2024 16:45:43 +0200
Subject: [PATCH 04/25] Proper 'if' cases to distinguish between $install_dir
 vs regular files in $install_dir and $data_dir

---
 helpers/helpers.v2.1.d/utils | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index ff378a951..6c63fe82d 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -236,7 +236,7 @@ ynh_app_upgrading_from_version_before_or_equal_to() {
 _ynh_apply_default_permissions() {
     local target=$1
 
-    is_subdir() {
+    is_in_dir() {
         # Returns false if child or parent is empty
         child=$(realpath "$1" 2>/dev/null)
         parent=$(realpath "$2" 2>/dev/null)
@@ -245,17 +245,27 @@ _ynh_apply_default_permissions() {
 
     # App files can have files of their own
     if ynh_system_user_exists --username="$app"; then
-        if is_subdir "$target" "$install_dir" ||  is_subdir "$target" "$data_dir"; then
-            chmod -R u=rwX,g=rX,o=X "$target"
-            chown -R "$app:$app" "$target"
-            chown "$app:www-data" "$target"
+        # If this is a file in $install_dir or $data_dir : it should be owned and read+writable by $app only
+        if [ -f "$target" ] && (([[ -z "${install_dir:-}" ]] is_in_dir "$target" "$install_dir") || ([[ -z "${install_dir:-}" ]] is_in_dir "$target" "$data_dir"))
+        then
+            chmod 600 "$target"
+            chown "$app:$app" "$target"
+            return
+        fi
+        # If this is the install dir (so far this is the only way this helper is called with a directory)
+        if [ "$target" ==  "$install_dir" ]
+        then
+            # Files inside should be owned by $app/www-data with rw-r----- (+x for folders or files that already have +x)
+            chmod -R u=rwX,g=r-X,o=--- "$target"
+            # We set the group to www-data because most apps do serve static assets that need to be readable by nginx ...
+            chown -R "$app:www-data" "$target"
             return
         fi
     fi
 
     # Other files are considered system
-    chmod -R 400 "$target"
-    chown -R root:root "$target"
+    chmod 400 "$target"
+    chown root:root "$target"
 }
 
 int_to_bool() {

From 3b26ccc2a542c67769af5b7150137500dd1eb26f Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 28 Jun 2024 16:55:39 +0200
Subject: [PATCH 05/25] Properly handle case where $parent is empty to simplify
 condition

---
 helpers/helpers.v2.1.d/utils | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index 6c63fe82d..af2859d78 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -237,23 +237,24 @@ _ynh_apply_default_permissions() {
     local target=$1
 
     is_in_dir() {
-        # Returns false if child or parent is empty
-        child=$(realpath "$1" 2>/dev/null)
-        parent=$(realpath "$2" 2>/dev/null)
+        # Returns false if parent is empty
+        [ -n "$2" ] || return 1
+        local child=$(realpath "$1" 2>/dev/null)
+        local parent=$(realpath "$2" 2>/dev/null)
         [[ "${child/$parent/}" != "$child" ]]
     }
 
     # App files can have files of their own
     if ynh_system_user_exists --username="$app"; then
         # If this is a file in $install_dir or $data_dir : it should be owned and read+writable by $app only
-        if [ -f "$target" ] && (([[ -z "${install_dir:-}" ]] is_in_dir "$target" "$install_dir") || ([[ -z "${install_dir:-}" ]] is_in_dir "$target" "$data_dir"))
+        if [ -f "$target" ] && (is_in_dir "$target" "${install_dir:-}" || is_in_dir "$target" "${data_dir:-}")
         then
             chmod 600 "$target"
             chown "$app:$app" "$target"
             return
         fi
         # If this is the install dir (so far this is the only way this helper is called with a directory)
-        if [ "$target" ==  "$install_dir" ]
+        if [ "$target" ==  "${install_dir:-}" ]
         then
             # Files inside should be owned by $app/www-data with rw-r----- (+x for folders or files that already have +x)
             chmod -R u=rwX,g=r-X,o=--- "$target"

From 75d704297470d358af7d6864df76f050f5fda7b8 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <4533074+alexAubin@users.noreply.github.com>
Date: Fri, 28 Jun 2024 16:56:28 +0200
Subject: [PATCH 06/25] Update helpers/helpers.v2.1.d/utils: use regex matching
 to check if path is child from a parent path

---
 helpers/helpers.v2.1.d/utils | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index af2859d78..2ee0b5d38 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -241,7 +241,7 @@ _ynh_apply_default_permissions() {
         [ -n "$2" ] || return 1
         local child=$(realpath "$1" 2>/dev/null)
         local parent=$(realpath "$2" 2>/dev/null)
-        [[ "${child/$parent/}" != "$child" ]]
+        [[ "${child}" =~ ^$parent ]]
     }
 
     # App files can have files of their own

From 8b8768fd776f9d15efd5aeb9d65cd943c0bfc920 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 28 Jun 2024 18:09:35 +0200
Subject: [PATCH 07/25] Only set www-data as group for webapps

---
 helpers/helpers.v2.1.d/utils | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index 2ee0b5d38..f03e67804 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -258,8 +258,13 @@ _ynh_apply_default_permissions() {
         then
             # Files inside should be owned by $app/www-data with rw-r----- (+x for folders or files that already have +x)
             chmod -R u=rwX,g=r-X,o=--- "$target"
+            local group="$app"
             # We set the group to www-data because most apps do serve static assets that need to be readable by nginx ...
-            chown -R "$app:www-data" "$target"
+            # The fact that the app is a webapp is infered by the fact that $domain and $path are defined
+            if [[ -n "${domain:-}" ]] && [[ -n "${path:-}" ]] then
+                group="www-data"
+            fi
+            chown -R "$app:$group" "$target"
             return
         fi
     fi

From ae3018cdd0d97f4f56117fdc80bacb9637e5426e Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 28 Jun 2024 18:39:12 +0200
Subject: [PATCH 08/25] Infer the necessity to use www-data as group from the
 presence of alias or root in nginx.conf

---
 helpers/helpers.v2.1.d/utils | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index f03e67804..05a859498 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -256,14 +256,15 @@ _ynh_apply_default_permissions() {
         # If this is the install dir (so far this is the only way this helper is called with a directory)
         if [ "$target" ==  "${install_dir:-}" ]
         then
-            # Files inside should be owned by $app/www-data with rw-r----- (+x for folders or files that already have +x)
-            chmod -R u=rwX,g=r-X,o=--- "$target"
             local group="$app"
-            # We set the group to www-data because most apps do serve static assets that need to be readable by nginx ...
-            # The fact that the app is a webapp is infered by the fact that $domain and $path are defined
-            if [[ -n "${domain:-}" ]] && [[ -n "${path:-}" ]] then
+            # We set the group to www-data for webapps that do serve static assets, which therefore need to be readable by nginx ...
+            # The fact that the app needs this is infered by the existence of an nginx.conf and the presence of "alias" or "root" directive
+            if grep -q '^\s*alias\s\|^\s*root\s' "$YNH_APP_BASEDIR/conf/nginx.conf" 2>/dev/null then
                 group="www-data"
             fi
+            # Files inside should be owned by $app with rw-r----- (+x for folders or files that already have +x)
+            # The group needs read/dirtraversal (in particular if it's www-data)
+            chmod -R u=rwX,g=r-X,o=--- "$target"
             chown -R "$app:$group" "$target"
             return
         fi

From 656ff823a914a15741be2e756d3f5dbcdb893184 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 28 Jun 2024 18:56:18 +0200
Subject: [PATCH 09/25] Also handle files in /etc/$app

---
 helpers/helpers.v2.1.d/utils | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index 05a859498..73b32a3f3 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -247,7 +247,7 @@ _ynh_apply_default_permissions() {
     # App files can have files of their own
     if ynh_system_user_exists --username="$app"; then
         # If this is a file in $install_dir or $data_dir : it should be owned and read+writable by $app only
-        if [ -f "$target" ] && (is_in_dir "$target" "${install_dir:-}" || is_in_dir "$target" "${data_dir:-}")
+        if [ -f "$target" ] && (is_in_dir "$target" "${install_dir:-}" || is_in_dir "$target" "${data_dir:-}" || is_in_dir "$target" "/etc/$app")
         then
             chmod 600 "$target"
             chown "$app:$app" "$target"

From ef68485c5fc4fea221e13e05b6ed638ab519dc2f Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 28 Jun 2024 19:24:07 +0200
Subject: [PATCH 10/25] Use the group defined in the manifest by default

---
 helpers/helpers.v2.1.d/utils | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index 73b32a3f3..ad7a7620d 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -256,11 +256,19 @@ _ynh_apply_default_permissions() {
         # If this is the install dir (so far this is the only way this helper is called with a directory)
         if [ "$target" ==  "${install_dir:-}" ]
         then
-            local group="$app"
-            # We set the group to www-data for webapps that do serve static assets, which therefore need to be readable by nginx ...
-            # The fact that the app needs this is infered by the existence of an nginx.conf and the presence of "alias" or "root" directive
-            if grep -q '^\s*alias\s\|^\s*root\s' "$YNH_APP_BASEDIR/conf/nginx.conf" 2>/dev/null then
-                group="www-data"
+            # Read the group from the install_dir manifest resource
+            local group="$(ynh_read_manifest '.resources.install_dir.group' | sed 's/null//g' | sed "s/__APP__/$app/g" | cut -f1 -d:)"
+            if [[ -z "$group" ]]
+            then
+                # We set the group to www-data for webapps that do serve static assets, which therefore need to be readable by nginx ...
+                # The fact that the app needs this is infered by the existence of an nginx.conf and the presence of "alias" or "root" directive
+                if grep -q '^\s*alias\s\|^\s*root\s' "$YNH_APP_BASEDIR/conf/nginx.conf" 2>/dev/null;
+                then
+                    group="www-data"
+                # Or default to "$app"
+                else
+                    group="$app"
+                fi
             fi
             # Files inside should be owned by $app with rw-r----- (+x for folders or files that already have +x)
             # The group needs read/dirtraversal (in particular if it's www-data)

From 1dfc47d1d7784a355b8ba8e7213313efaacb813a Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 28 Jun 2024 20:21:56 +0200
Subject: [PATCH 11/25] helpers2.1: in logrotate, make sure to also chown $app
 the log dir

---
 helpers/helpers.v2.1.d/logrotate | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/helpers/helpers.v2.1.d/logrotate b/helpers/helpers.v2.1.d/logrotate
index 07e62261f..4f2d063b1 100644
--- a/helpers/helpers.v2.1.d/logrotate
+++ b/helpers/helpers.v2.1.d/logrotate
@@ -24,9 +24,11 @@ ynh_config_add_logrotate() {
 
     for stuff in $logfile
     do
-        mkdir --parents $(dirname "$stuff")
         # Make sure the permissions of the parent dir are correct (otherwise the config file could be ignored and the corresponding logs never rotated)
-        chmod 750 $(dirname "$stuff")
+        local dir=$(dirname "$stuff")
+        mkdir --parents $dir
+        chmod 750 $dir
+        chown $app:$app $dir
     done
 
     local tempconf="$(mktemp)"

From 7b2959a3ebd76c50cd87287c0d3f36cd3c7345a0 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 29 Jun 2024 17:18:20 +0200
Subject: [PATCH 12/25] helpers2.1: forgot to rename the apt call in mongodb
 helpers

---
 helpers/helpers.v2.1.d/mongodb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/helpers/helpers.v2.1.d/mongodb b/helpers/helpers.v2.1.d/mongodb
index b0d1fe981..3fb851277 100644
--- a/helpers/helpers.v2.1.d/mongodb
+++ b/helpers/helpers.v2.1.d/mongodb
@@ -235,7 +235,10 @@ ynh_install_mongo() {
         mongo_debian_release=buster
     fi
 
-    ynh_install_extra_app_dependencies --repo="deb http://repo.mongodb.org/apt/debian $mongo_debian_release/mongodb-org/$mongo_version main" --package="mongodb-org mongodb-org-server mongodb-org-tools mongodb-mongosh" --key="https://www.mongodb.org/static/pgp/server-$mongo_version.asc"
+    ynh_apt_install_dependencies_from_extra_repository \
+        --repo="deb http://repo.mongodb.org/apt/debian $mongo_debian_release/mongodb-org/$mongo_version main" \
+        --package="mongodb-org mongodb-org-server mongodb-org-tools mongodb-mongosh" \
+        --key="https://www.mongodb.org/static/pgp/server-$mongo_version.asc"
     mongodb_servicename=mongod
 
     # Make sure MongoDB is started and enabled

From 1ab3a79d3951ce9f062110361b9643495a90ac86 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 29 Jun 2024 18:06:40 +0200
Subject: [PATCH 13/25] Update changelog for 11.2.18

---
 debian/changelog | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/debian/changelog b/debian/changelog
index 2ea7acce1..f3d38c9c7 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+yunohost (11.2.18) stable; urgency=low
+
+  - helpers2.1: Rework _ynh_apply_default_permissions to hopefully remove the necessity to chown/chmod in the app scripts ([#1883](http://github.com/YunoHost/yunohost/pull/1883))
+  - helpers2.1: in logrotate, make sure to also chown $app the log dir (1dfc47d1d)
+  - helpers2.1: forgot to rename the apt call in mongodb helpers (7b2959a3e)
+  - helpers2.1: in ynh_safe_rm, check if target is not a broken symlink before erorring out ([#1716](http://github.com/YunoHost/yunohost/pull/1716))
+
+  Thanks to all contributors <3 ! (Félix Piédallu)
+
+ -- Alexandre Aubin <alex.aubin@mailoo.org>  Sat, 29 Jun 2024 18:05:04 +0200
+
 yunohost (11.2.17.1) stable; urgency=low
 
   - helpers2.1: fix __PATH__/ handling (997388dc)

From 3e1c9ebaf7eca7f8ba59da0eb02cd71782b4e45d Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 29 Jun 2024 19:21:08 +0200
Subject: [PATCH 14/25] Fix getopts error handling ...

---
 helpers/helpers.v2.1.d/getopts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/helpers/helpers.v2.1.d/getopts b/helpers/helpers.v2.1.d/getopts
index 4df68e7ae..ca517eebd 100644
--- a/helpers/helpers.v2.1.d/getopts
+++ b/helpers/helpers.v2.1.d/getopts
@@ -102,9 +102,9 @@ ynh_handle_getopts_args() {
             getopts ":$getopts_parameters" parameter || true
 
             if [ "$parameter" = "?" ]; then
-                ynh_die "Invalid argument: -${OPTARG:-}"
+                ynh_die "Invalid argument: ${1:-}"
             elif [ "$parameter" = ":" ]; then
-                ynh_die "-$OPTARG parameter requires an argument."
+                ynh_die "${1:-} parameter requires an argument."
             else
                 local shift_value=1
                 # Use the long option, corresponding to the short option read by getopts, as a variable

From a349fc0334d7c40a35dfc9dc7ecc8723e5fc8edd Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 29 Jun 2024 20:04:19 +0200
Subject: [PATCH 15/25] apps: tweaks to be more robust and prevent the stupid
 flood of 'sh: 0: getcwd() failed: No such file or directory' when running an
 app upgrade/remove from /var/www/$app, sometimes making it look like the
 upgrade failed when it didnt

---
 src/service.py      | 6 ++++--
 src/utils/system.py | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/service.py b/src/service.py
index 5e49dfc8a..977b9a902 100644
--- a/src/service.py
+++ b/src/service.py
@@ -688,13 +688,15 @@ def _get_services():
     ]
     for name in services_with_package_condition:
         package = services[name]["ignore_if_package_is_not_installed"]
-        if os.system(f"dpkg --list | grep -q 'ii *{package}'") != 0:
+        if check_output(f"dpkg-query --show --showformat='${{db:Status-Status}}' '{package}' 2>/dev/null || true") != "installed":
             del services[name]
 
     php_fpm_versions = check_output(
-        r"dpkg --list | grep -P 'ii  php\d.\d-fpm' | awk '{print $2}' | grep -o -P '\d.\d' || true"
+        r"dpkg --list | grep -P 'ii  php\d.\d-fpm' | awk '{print $2}' | grep -o -P '\d.\d' || true",
+        cwd="/tmp"
     )
     php_fpm_versions = [v for v in php_fpm_versions.split("\n") if v.strip()]
+
     for version in php_fpm_versions:
         # Skip php 7.3 which is most likely dead after buster->bullseye migration
         # because users get spooked
diff --git a/src/utils/system.py b/src/utils/system.py
index 27ef98dd1..5bea7f971 100644
--- a/src/utils/system.py
+++ b/src/utils/system.py
@@ -159,7 +159,7 @@ def ynh_packages_version(*args, **kwargs):
 
 
 def dpkg_is_broken():
-    if check_output("dpkg --audit") != "":
+    if check_output("dpkg --audit", cwd="/tmp/") != "":
         return True
     # If dpkg is broken, /var/lib/dpkg/updates
     # will contains files like 0001, 0002, ...

From c2d69f7f84c0a164b9c03df1852ba854c8aa6181 Mon Sep 17 00:00:00 2001
From: alexAubin <4533074+alexAubin@users.noreply.github.com>
Date: Sat, 29 Jun 2024 18:05:02 +0000
Subject: [PATCH 16/25] :art: Format Python code with Black

---
 src/service.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/service.py b/src/service.py
index 977b9a902..b0dc82827 100644
--- a/src/service.py
+++ b/src/service.py
@@ -688,12 +688,17 @@ def _get_services():
     ]
     for name in services_with_package_condition:
         package = services[name]["ignore_if_package_is_not_installed"]
-        if check_output(f"dpkg-query --show --showformat='${{db:Status-Status}}' '{package}' 2>/dev/null || true") != "installed":
+        if (
+            check_output(
+                f"dpkg-query --show --showformat='${{db:Status-Status}}' '{package}' 2>/dev/null || true"
+            )
+            != "installed"
+        ):
             del services[name]
 
     php_fpm_versions = check_output(
         r"dpkg --list | grep -P 'ii  php\d.\d-fpm' | awk '{print $2}' | grep -o -P '\d.\d' || true",
-        cwd="/tmp"
+        cwd="/tmp",
     )
     php_fpm_versions = [v for v in php_fpm_versions.split("\n") if v.strip()]
 

From d47c87e57d0bf7714416ac2ce07b400cb527a179 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 29 Jun 2024 20:08:14 +0200
Subject: [PATCH 17/25] helpers2.1: wrmbgl

---
 helpers/helpers.v2.1.d/utils | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/helpers/helpers.v2.1.d/utils b/helpers/helpers.v2.1.d/utils
index 00c67c792..859cd29d9 100644
--- a/helpers/helpers.v2.1.d/utils
+++ b/helpers/helpers.v2.1.d/utils
@@ -257,7 +257,7 @@ _ynh_apply_default_permissions() {
         if [ "$target" ==  "${install_dir:-}" ]
         then
             # Read the group from the install_dir manifest resource
-            local group="$(ynh_read_manifest '.resources.install_dir.group' | sed 's/null//g' | sed "s/__APP__/$app/g" | cut -f1 -d:)"
+            local group="$(ynh_read_manifest 'resources.install_dir.group' | sed 's/null//g' | sed "s/__APP__/$app/g" | cut -f1 -d:)"
             if [[ -z "$group" ]]
             then
                 # We set the group to www-data for webapps that do serve static assets, which therefore need to be readable by nginx ...

From e5b575901a77b2fea13d3f81aed64366566efc8c Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 29 Jun 2024 20:31:28 +0200
Subject: [PATCH 18/25] apps: be more robust when an app upgrade succeeds but
 for some reason is marked with 'broke the system' ... ending up in
 inconsistent state between the app settings vs the app scritpts (for example
 in v1->v2 transitions but not only)

---
 src/app.py | 63 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/src/app.py b/src/app.py
index 597743cc8..f01630d71 100644
--- a/src/app.py
+++ b/src/app.py
@@ -849,6 +849,39 @@ def app_upgrade(
                     + "\n     -".join(manually_modified_files_by_app)
                 )
 
+            # If the upgrade didnt fail, update the revision and app files (even if it broke the system, otherwise we end up in a funky intermediate state where the app files don't match the installed version or settings, for example for v1->v2 upgrade marked as "broke the system" for some reason)
+            if not upgrade_failed:
+                now = int(time.time())
+                app_setting(app_instance_name, "update_time", now)
+                app_setting(
+                    app_instance_name,
+                    "current_revision",
+                    manifest.get("remote", {}).get("revision", "?"),
+                )
+
+                # Clean hooks and add new ones
+                hook_remove(app_instance_name)
+                if "hooks" in os.listdir(extracted_app_folder):
+                    for hook in os.listdir(extracted_app_folder + "/hooks"):
+                        hook_add(app_instance_name, extracted_app_folder + "/hooks/" + hook)
+
+                # Replace scripts and manifest and conf (if exists)
+                # Move scripts and manifest to the right place
+                for file_to_copy in APP_FILES_TO_COPY:
+                    rm(f"{app_setting_path}/{file_to_copy}", recursive=True, force=True)
+                    if os.path.exists(os.path.join(extracted_app_folder, file_to_copy)):
+                        cp(
+                            f"{extracted_app_folder}/{file_to_copy}",
+                            f"{app_setting_path}/{file_to_copy}",
+                            recursive=True,
+                        )
+
+                # Clean and set permissions
+                shutil.rmtree(extracted_app_folder)
+                chmod(app_setting_path, 0o600)
+                chmod(f"{app_setting_path}/settings.yml", 0o400)
+                chown(app_setting_path, "root", recursive=True)
+
             # If upgrade failed or broke the system,
             # raise an error and interrupt all other pending upgrades
             if upgrade_failed or broke_the_system:
@@ -899,36 +932,6 @@ def app_upgrade(
                     )
 
             # Otherwise we're good and keep going !
-            now = int(time.time())
-            app_setting(app_instance_name, "update_time", now)
-            app_setting(
-                app_instance_name,
-                "current_revision",
-                manifest.get("remote", {}).get("revision", "?"),
-            )
-
-            # Clean hooks and add new ones
-            hook_remove(app_instance_name)
-            if "hooks" in os.listdir(extracted_app_folder):
-                for hook in os.listdir(extracted_app_folder + "/hooks"):
-                    hook_add(app_instance_name, extracted_app_folder + "/hooks/" + hook)
-
-            # Replace scripts and manifest and conf (if exists)
-            # Move scripts and manifest to the right place
-            for file_to_copy in APP_FILES_TO_COPY:
-                rm(f"{app_setting_path}/{file_to_copy}", recursive=True, force=True)
-                if os.path.exists(os.path.join(extracted_app_folder, file_to_copy)):
-                    cp(
-                        f"{extracted_app_folder}/{file_to_copy}",
-                        f"{app_setting_path}/{file_to_copy}",
-                        recursive=True,
-                    )
-
-            # Clean and set permissions
-            shutil.rmtree(extracted_app_folder)
-            chmod(app_setting_path, 0o600)
-            chmod(f"{app_setting_path}/settings.yml", 0o400)
-            chown(app_setting_path, "root", recursive=True)
 
             # So much win
             logger.success(m18n.n("app_upgraded", app=app_instance_name))

From dbf579b7b4803e3f5478bd0dd3870dd92f10f8a6 Mon Sep 17 00:00:00 2001
From: alexAubin <4533074+alexAubin@users.noreply.github.com>
Date: Sat, 29 Jun 2024 18:31:51 +0000
Subject: [PATCH 19/25] :art: Format Python code with Black

---
 src/app.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/app.py b/src/app.py
index f01630d71..f7e17a269 100644
--- a/src/app.py
+++ b/src/app.py
@@ -863,7 +863,9 @@ def app_upgrade(
                 hook_remove(app_instance_name)
                 if "hooks" in os.listdir(extracted_app_folder):
                     for hook in os.listdir(extracted_app_folder + "/hooks"):
-                        hook_add(app_instance_name, extracted_app_folder + "/hooks/" + hook)
+                        hook_add(
+                            app_instance_name, extracted_app_folder + "/hooks/" + hook
+                        )
 
                 # Replace scripts and manifest and conf (if exists)
                 # Move scripts and manifest to the right place

From ff78f3ada7c118869a7b1d8dfc07e1b6894dcc83 Mon Sep 17 00:00:00 2001
From: OniriCorpe <oniricorpe@disroot.org>
Date: Sat, 29 Jun 2024 20:57:21 +0200
Subject: [PATCH 20/25] automatically ignore the service in diagnosis if it has
 been deactivated with the ynh cli

---
 src/service.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/service.py b/src/service.py
index b0dc82827..0a0c37378 100644
--- a/src/service.py
+++ b/src/service.py
@@ -26,6 +26,7 @@ from glob import glob
 from datetime import datetime
 
 from moulinette import m18n
+from yunohost.utils.diagnosis import diagnosis_ignore, diagnosis_unignore
 from yunohost.utils.error import YunohostError, YunohostValidationError
 from moulinette.utils.process import check_output
 from moulinette.utils.log import getActionLogger
@@ -296,6 +297,9 @@ def service_enable(names):
         names = [names]
     for name in names:
         if _run_service_command("enable", name):
+            services = _get_services()
+            if name in services:
+                diagnosis_unignore({"services": [{"service": name}]})
             logger.success(m18n.n("service_enabled", service=name))
         else:
             raise YunohostError(
@@ -315,6 +319,9 @@ def service_disable(names):
         names = [names]
     for name in names:
         if _run_service_command("disable", name):
+            services = _get_services()
+            if name in services:
+                diagnosis_ignore({"services": [{"service": name}]})
             logger.success(m18n.n("service_disabled", service=name))
         else:
             raise YunohostError(

From eaf00103dd72c01564f245f85f8e0ee68662c3ac Mon Sep 17 00:00:00 2001
From: OniriCorpe <oniricorpe@disroot.org>
Date: Sat, 29 Jun 2024 20:57:59 +0200
Subject: [PATCH 21/25] Revert "automatically ignore the service in diagnosis
 if it has been deactivated with the ynh cli"

This reverts commit ff78f3ada7c118869a7b1d8dfc07e1b6894dcc83.
---
 src/service.py | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/src/service.py b/src/service.py
index 0a0c37378..b0dc82827 100644
--- a/src/service.py
+++ b/src/service.py
@@ -26,7 +26,6 @@ from glob import glob
 from datetime import datetime
 
 from moulinette import m18n
-from yunohost.utils.diagnosis import diagnosis_ignore, diagnosis_unignore
 from yunohost.utils.error import YunohostError, YunohostValidationError
 from moulinette.utils.process import check_output
 from moulinette.utils.log import getActionLogger
@@ -297,9 +296,6 @@ def service_enable(names):
         names = [names]
     for name in names:
         if _run_service_command("enable", name):
-            services = _get_services()
-            if name in services:
-                diagnosis_unignore({"services": [{"service": name}]})
             logger.success(m18n.n("service_enabled", service=name))
         else:
             raise YunohostError(
@@ -319,9 +315,6 @@ def service_disable(names):
         names = [names]
     for name in names:
         if _run_service_command("disable", name):
-            services = _get_services()
-            if name in services:
-                diagnosis_ignore({"services": [{"service": name}]})
             logger.success(m18n.n("service_disabled", service=name))
         else:
             raise YunohostError(

From eee84c5f6670d4e9be1c59516696e3d76347246a Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 29 Jun 2024 21:32:53 +0200
Subject: [PATCH 22/25] helpers2.1: also run _ynh_apply_default_permissions in
 ynh_restore to be consistent (also because the user uid on the new system may
 be different than in the archive etc)

---
 helpers/helpers.v2.1.d/backup | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/helpers/helpers.v2.1.d/backup b/helpers/helpers.v2.1.d/backup
index 0668d3e17..a40c4f1f2 100644
--- a/helpers/helpers.v2.1.d/backup
+++ b/helpers/helpers.v2.1.d/backup
@@ -179,6 +179,8 @@ ynh_restore() {
     else
         mv "$archive_path" "${target}"
     fi
+
+    _ynh_apply_default_permissions "$target"
 }
 
 # Restore all files that were previously backuped in an app backup script

From c2271ab7310025affcbd73ba33c340ad92ec7712 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 29 Jun 2024 23:57:21 +0200
Subject: [PATCH 23/25] Update changelog for 11.2.19

---
 debian/changelog | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/debian/changelog b/debian/changelog
index f3d38c9c7..7a8dfacc9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+yunohost (11.2.19) stable; urgency=low
+
+  - apps: tweaks to be more robust and prevent the stupid flood of 'sh: 0: getcwd() failed: No such file or directory' when running an app upgrade/remove from /var/www/$app, sometimes making it look like the upgrade failed when it didnt (a349fc03)
+  - apps: be more robust when an app upgrade succeeds but for some reason is marked with 'broke the system' ... ending up in inconsistent state between the app settings vs the app scritpts (for example in v1->v2 transitions but not only) (e5b57590)
+  - helpers2.1: Fix getopts error handling ... (3e1c9eba)
+  - helpers2.1: also run _ynh_apply_default_permissions in ynh_restore to be consistent (also because the user uid on the new system may be different than in the archive etc) (eee84c5f)
+
+ -- Alexandre Aubin <alex.aubin@mailoo.org>  Sat, 29 Jun 2024 23:55:52 +0200
+
 yunohost (11.2.18) stable; urgency=low
 
   - helpers2.1: Rework _ynh_apply_default_permissions to hopefully remove the necessity to chown/chmod in the app scripts ([#1883](http://github.com/YunoHost/yunohost/pull/1883))

From a18d5f26f2b42191261d6ab3755ff838d69f8aa0 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sun, 30 Jun 2024 00:21:40 +0200
Subject: [PATCH 24/25] helpers2.1: zgrblg

---
 helpers/helpers.v2.1.d/go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/helpers/helpers.v2.1.d/go b/helpers/helpers.v2.1.d/go
index c0b8e9022..bb272d50c 100644
--- a/helpers/helpers.v2.1.d/go
+++ b/helpers/helpers.v2.1.d/go
@@ -97,7 +97,7 @@ ynh_go_install () {
     test -x /usr/bin/go_goenv && mv /usr/bin/go_goenv /usr/bin/go
 
     # Install the requested version of Go
-    local final_go_version=$("$goenv_latest_dir/bin/goenv-latest" --print "$go_version")
+    local final_go_version=$("$GOENV_INSTALL_DIR/plugins/xxenv-latest/bin/goenv-latest" --print "$go_version")
     ynh_print_info "Installation of Go-$final_go_version"
     goenv install --quiet --skip-existing "$final_go_version" 2>&1
 

From 3f973669fc14de9fd45cbe1b376004b6d422d9fe Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sun, 30 Jun 2024 01:37:56 +0200
Subject: [PATCH 25/25] helpers2.1: fix automigration of phpversion to
 php_version

---
 helpers/helpers.v2.1.d/php     | 12 ------------
 helpers/helpers.v2.1.d/setting | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/helpers/helpers.v2.1.d/php b/helpers/helpers.v2.1.d/php
index 3cca2c8fb..b7165e010 100644
--- a/helpers/helpers.v2.1.d/php
+++ b/helpers/helpers.v2.1.d/php
@@ -3,18 +3,6 @@
 # (this is used in the apt helpers, big meh ...)
 readonly YNH_DEFAULT_PHP_VERSION=7.4
 
-# Legacy: auto-convert phpversion to php_version (for consistency with nodejs_version, ruby_version, ...)
-if [[ -n "${app:-}" ]] && [[ -n "${phpversion:-}" ]]
-then
-    if [[ -z "${php_version:-}" ]]
-    then
-        php_version=$phpversion
-        ynh_app_setting_set --key=php_version --value=$php_version
-    fi
-    ynh_app_setting_delete --key=phpversion
-    unset phpversion
-fi
-
 # Create a dedicated PHP-FPM config
 #
 # usage: ynh_config_add_phpfpm
diff --git a/helpers/helpers.v2.1.d/setting b/helpers/helpers.v2.1.d/setting
index 82528efa5..01480331c 100644
--- a/helpers/helpers.v2.1.d/setting
+++ b/helpers/helpers.v2.1.d/setting
@@ -122,3 +122,18 @@ else:
 EOF
     eval "$xtrace_enable"
 }
+
+# Legacy: auto-convert phpversion to php_version (for consistency with nodejs_version, ruby_version, ...)
+# This has to be here and not in the "php" code file because ynh_app_setting_set/delete need to be defined @_@
+if [[ -n "${app:-}" ]] && [[ -n "${phpversion:-}" ]]
+then
+    if [[ -z "${php_version:-}" ]]
+    then
+        php_version=$phpversion
+        ynh_app_setting_set --key=php_version --value=$php_version
+    fi
+    ynh_app_setting_delete --key=phpversion
+    unset phpversion
+fi
+
+