Merge branch 'dev' into dev

debian/changelog

@@ -1,3 +1,23 @@
+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))
+  - 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)

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

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

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
 

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)"

helpers/helpers.v2.1.d/mongodb

@@ -265,20 +265,24 @@ ynh_install_mongo() {
 	fi
 
 	if [[ "$install_package" = true ]]; then
-        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 \
-    fi
+        --repo="deb http://repo.mongodb.org/apt/debian $mongo_debian_release/mongodb-org/$mongo_version main" \
-    mongodb_servicename=mongod
+        --package="mongodb-org mongodb-org-server mongodb-org-tools mongodb-mongosh" \
+        --key="https://www.mongodb.org/static/pgp/server-$mongo_version.asc"
+  fi
+
+  mongodb_servicename=mongod
 
     # Make sure MongoDB is started and enabled
-    systemctl enable $mongodb_servicename --quiet
+  systemctl enable $mongodb_servicename --quiet
-    systemctl daemon-reload --quiet
+  systemctl daemon-reload --quiet
-    ynh_systemctl --service=$mongodb_servicename --action=restart --wait_until="aiting for connections" --log_path="/var/log/mongodb/$mongodb_servicename.log"
+  ynh_systemctl --service=$mongodb_servicename --action=restart --wait_until="aiting for connections" --log_path="/var/log/mongodb/$mongodb_servicename.log"
 
     # Integrate MongoDB service in YunoHost
-    yunohost service add $mongodb_servicename --description="MongoDB daemon" --log="/var/log/mongodb/$mongodb_servicename.log"
+  yunohost service add $mongodb_servicename --description="MongoDB daemon" --log="/var/log/mongodb/$mongodb_servicename.log"
 
     # Store mongo_version into the config of this app
-    ynh_app_setting_set --key=mongo_version --value=$mongo_version
+  ynh_app_setting_set --key=mongo_version --value=$mongo_version
 }
 
 # Remove MongoDB

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

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
+
+

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
 }

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."
@@ -226,41 +226,61 @@ 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')
+# Apply sane permissions for files installed by ynh_setup_source and ynh_config_add.
-# on file/folders handled with ynh_setup_source and ynh_config_add
 #
 # [internal]
 #
-# Having a file others-readable or a folder others-executable(=enterable)
+# * Anything below $install_dir is chown $app:$app and chmod o-rwx,g-w
-# is a security risk comparable to "chmod 777"
+# * The rest is considered as system configuration and chown root, chmod 400
-#
-# 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.
 #
 _ynh_apply_default_permissions() {
     local target=$1
 
-    chmod o-rwx $target
+    is_in_dir() {
-    chmod g-w $target
+        # Returns false if parent is empty
-    chown -R root:root $target
+        [ -n "$2" ] || return 1
-    if ynh_system_user_exists --username=$app; then
+        local child=$(realpath "$1" 2>/dev/null)
-        chown $app:$app $target
+        local parent=$(realpath "$2" 2>/dev/null)
+        [[ "${child}" =~ ^$parent ]]
+    }
+
+    # 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:-}" || is_in_dir "$target" "/etc/$app")
+        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
+            # 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)
+            chmod -R u=rwX,g=r-X,o=--- "$target"
+            chown -R "$app:$group" "$target"
+            return
+        fi
     fi
 
-    # Crons should be owned by root
+    # Other files are considered system
-    # Also we don't want systemd conf, nginx conf or others stuff to be owned by the app,
+    chmod 400 "$target"
-    # otherwise they could self-edit their own systemd conf and escalate privilege
+    chown root:root "$target"
-    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
 }
 
 int_to_bool() {

src/app.py

@@ -849,6 +849,41 @@ 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 +934,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))

src/service.py

@@ -688,13 +688,20 @@ 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

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, ...