Merge pull request #1214 from YunoHost/more-uniform-tmp-workdir-for-apps

More uniform tmp dir for apps, remove some weird 'admin' ownership
This commit is contained in:
Alexandre Aubin 2021-04-17 01:56:28 +02:00 committed by GitHub
commit 931ed64e1b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 72 additions and 120 deletions

View file

@ -33,6 +33,7 @@ import re
import subprocess import subprocess
import glob import glob
import urllib.parse import urllib.parse
import tempfile
from collections import OrderedDict from collections import OrderedDict
from moulinette import msignals, m18n, msettings from moulinette import msignals, m18n, msettings
@ -57,10 +58,8 @@ from yunohost.log import is_unit_operation, OperationLogger
logger = getActionLogger("yunohost.app") logger = getActionLogger("yunohost.app")
APPS_PATH = "/usr/share/yunohost/apps"
APPS_SETTING_PATH = "/etc/yunohost/apps/" APPS_SETTING_PATH = "/etc/yunohost/apps/"
INSTALL_TMP = "/var/cache/yunohost" APP_TMP_WORKDIRS = "/var/cache/yunohost/app_tmp_work_dirs"
APP_TMP_FOLDER = INSTALL_TMP + "/from_file"
APPS_CATALOG_CACHE = "/var/cache/yunohost/repo" APPS_CATALOG_CACHE = "/var/cache/yunohost/repo"
APPS_CATALOG_CONF = "/etc/yunohost/apps_catalog.yml" APPS_CATALOG_CONF = "/etc/yunohost/apps_catalog.yml"
@ -459,34 +458,12 @@ def app_change_url(operation_logger, app, domain, path):
operation_logger.extra.update({"env": env_dict}) operation_logger.extra.update({"env": env_dict})
operation_logger.start() operation_logger.start()
if os.path.exists(os.path.join(APP_TMP_FOLDER, "scripts")): tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app)
shutil.rmtree(os.path.join(APP_TMP_FOLDER, "scripts")) change_url_script = os.path.join(tmp_workdir_for_app, "scripts/change_url")
shutil.copytree(
os.path.join(APPS_SETTING_PATH, app, "scripts"),
os.path.join(APP_TMP_FOLDER, "scripts"),
)
if os.path.exists(os.path.join(APP_TMP_FOLDER, "conf")):
shutil.rmtree(os.path.join(APP_TMP_FOLDER, "conf"))
shutil.copytree(
os.path.join(APPS_SETTING_PATH, app, "conf"),
os.path.join(APP_TMP_FOLDER, "conf"),
)
# Execute App change_url script # Execute App change_url script
os.system("chown -R admin: %s" % INSTALL_TMP) ret = hook_exec(change_url_script, env=env_dict)[0]
os.system("chmod +x %s" % os.path.join(os.path.join(APP_TMP_FOLDER, "scripts"))) if ret != 0:
os.system(
"chmod +x %s"
% os.path.join(os.path.join(APP_TMP_FOLDER, "scripts", "change_url"))
)
if (
hook_exec(os.path.join(APP_TMP_FOLDER, "scripts/change_url"), env=env_dict)[0]
!= 0
):
msg = "Failed to change '%s' url." % app msg = "Failed to change '%s' url." % app
logger.error(msg) logger.error(msg)
operation_logger.error(msg) operation_logger.error(msg)
@ -496,6 +473,7 @@ def app_change_url(operation_logger, app, domain, path):
app_setting(app, "domain", value=old_domain) app_setting(app, "domain", value=old_domain)
app_setting(app, "path", value=old_path) app_setting(app, "path", value=old_path)
return return
shutil.rmtree(tmp_workdir_for_app)
# this should idealy be done in the change_url script but let's avoid common mistakes # this should idealy be done in the change_url script but let's avoid common mistakes
app_setting(app, "domain", value=domain) app_setting(app, "domain", value=domain)
@ -620,7 +598,7 @@ def app_upgrade(app=[], url=None, file=None, force=False):
_check_manifest_requirements(manifest, app_instance_name=app_instance_name) _check_manifest_requirements(manifest, app_instance_name=app_instance_name)
_assert_system_is_sane_for_app(manifest, "pre") _assert_system_is_sane_for_app(manifest, "pre")
app_setting_path = APPS_SETTING_PATH + "/" + app_instance_name app_setting_path = os.path.join(APPS_SETTING_PATH, app_instance_name)
# Retrieve arguments list for upgrade script # Retrieve arguments list for upgrade script
# TODO: Allow to specify arguments # TODO: Allow to specify arguments
@ -646,9 +624,6 @@ def app_upgrade(app=[], url=None, file=None, force=False):
operation_logger = OperationLogger("app_upgrade", related_to, env=env_dict) operation_logger = OperationLogger("app_upgrade", related_to, env=env_dict)
operation_logger.start() operation_logger.start()
# Execute App upgrade script
os.system("chown -hR admin: %s" % INSTALL_TMP)
# Execute the app upgrade script # Execute the app upgrade script
upgrade_failed = True upgrade_failed = True
try: try:
@ -775,6 +750,12 @@ def app_upgrade(app=[], url=None, file=None, force=False):
% (extracted_app_folder, file_to_copy, app_setting_path) % (extracted_app_folder, file_to_copy, app_setting_path)
) )
# Clean and set permissions
shutil.rmtree(extracted_app_folder)
os.system("chmod 600 %s" % app_setting_path)
os.system("chmod 400 %s/settings.yml" % app_setting_path)
os.system("chown -R root: %s" % app_setting_path)
# So much win # So much win
logger.success(m18n.n("app_upgraded", app=app_instance_name)) logger.success(m18n.n("app_upgraded", app=app_instance_name))
@ -816,10 +797,6 @@ def app_install(
) )
from yunohost.regenconf import manually_modified_files from yunohost.regenconf import manually_modified_files
# Fetch or extract sources
if not os.path.exists(INSTALL_TMP):
os.makedirs(INSTALL_TMP)
def confirm_install(confirm): def confirm_install(confirm):
# Ignore if there's nothing for confirm (good quality app), if --force is used # Ignore if there's nothing for confirm (good quality app), if --force is used
# or if request on the API (confirm already implemented on the API side) # or if request on the API (confirm already implemented on the API side)
@ -953,10 +930,6 @@ def app_install(
} }
_set_app_settings(app_instance_name, app_settings) _set_app_settings(app_instance_name, app_settings)
os.system("chown -R admin: " + extracted_app_folder)
# Execute App install script
os.system("chown -hR admin: %s" % INSTALL_TMP)
# Move scripts and manifest to the right place # Move scripts and manifest to the right place
if os.path.exists(os.path.join(extracted_app_folder, "manifest.json")): if os.path.exists(os.path.join(extracted_app_folder, "manifest.json")):
os.system("cp %s/manifest.json %s" % (extracted_app_folder, app_setting_path)) os.system("cp %s/manifest.json %s" % (extracted_app_folder, app_setting_path))
@ -1131,9 +1104,9 @@ def app_install(
# Clean and set permissions # Clean and set permissions
shutil.rmtree(extracted_app_folder) shutil.rmtree(extracted_app_folder)
os.system("chmod -R 400 %s" % app_setting_path) os.system("chmod 600 %s" % app_setting_path)
os.system("chmod 400 %s/settings.yml" % app_setting_path)
os.system("chown -R root: %s" % app_setting_path) os.system("chown -R root: %s" % app_setting_path)
os.system("chown -R admin: %s/scripts" % app_setting_path)
logger.success(m18n.n("installation_complete")) logger.success(m18n.n("installation_complete"))
@ -1212,13 +1185,7 @@ def app_remove(operation_logger, app):
logger.info(m18n.n("app_start_remove", app=app)) logger.info(m18n.n("app_start_remove", app=app))
app_setting_path = APPS_SETTING_PATH + app app_setting_path = os.path.join(APPS_SETTING_PATH, app)
# TODO: display fail messages from script
try:
shutil.rmtree("/tmp/yunohost_remove")
except Exception:
pass
# Attempt to patch legacy helpers ... # Attempt to patch legacy helpers ...
_patch_legacy_helpers(app_setting_path) _patch_legacy_helpers(app_setting_path)
@ -1228,13 +1195,8 @@ def app_remove(operation_logger, app):
_patch_legacy_php_versions(app_setting_path) _patch_legacy_php_versions(app_setting_path)
manifest = _get_manifest_of_app(app_setting_path) manifest = _get_manifest_of_app(app_setting_path)
tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app)
os.system( remove_script = f"{tmp_workdir_for_app}/scripts/remove"
"cp -a %s /tmp/yunohost_remove && chown -hR admin: /tmp/yunohost_remove"
% app_setting_path
)
os.system("chown -R admin: /tmp/yunohost_remove")
os.system("chmod -R u+rX /tmp/yunohost_remove")
env_dict = {} env_dict = {}
app_id, app_instance_nb = _parse_app_instance_name(app) app_id, app_instance_nb = _parse_app_instance_name(app)
@ -1246,7 +1208,7 @@ def app_remove(operation_logger, app):
operation_logger.flush() operation_logger.flush()
try: try:
ret = hook_exec("/tmp/yunohost_remove/scripts/remove", env=env_dict)[0] ret = hook_exec(remove_script, env=env_dict)[0]
# Here again, calling hook_exec could fail miserably, or get # Here again, calling hook_exec could fail miserably, or get
# manually interrupted (by mistake or because script was stuck) # manually interrupted (by mistake or because script was stuck)
# In that case we still want to proceed with the rest of the # In that case we still want to proceed with the rest of the
@ -1256,6 +1218,8 @@ def app_remove(operation_logger, app):
import traceback import traceback
logger.error(m18n.n("unexpected_error", error="\n" + traceback.format_exc())) logger.error(m18n.n("unexpected_error", error="\n" + traceback.format_exc()))
finally:
shutil.rmtree(tmp_workdir_for_app)
if ret == 0: if ret == 0:
logger.success(m18n.n("app_removed", app=app)) logger.success(m18n.n("app_removed", app=app))
@ -1269,7 +1233,7 @@ def app_remove(operation_logger, app):
if os.path.exists(app_setting_path): if os.path.exists(app_setting_path):
shutil.rmtree(app_setting_path) shutil.rmtree(app_setting_path)
shutil.rmtree("/tmp/yunohost_remove")
hook_remove(app) hook_remove(app)
permission_sync_to_user() permission_sync_to_user()
@ -1717,7 +1681,6 @@ def app_action_run(operation_logger, app, action, args=None):
logger.warning(m18n.n("experimental_feature")) logger.warning(m18n.n("experimental_feature"))
from yunohost.hook import hook_exec from yunohost.hook import hook_exec
import tempfile
# will raise if action doesn't exist # will raise if action doesn't exist
actions = app_action_list(app)["actions"] actions = app_action_list(app)["actions"]
@ -1755,8 +1718,9 @@ def app_action_run(operation_logger, app, action, args=None):
if action_declaration.get("cwd"): if action_declaration.get("cwd"):
cwd = action_declaration["cwd"].replace("$app", app) cwd = action_declaration["cwd"].replace("$app", app)
else: else:
cwd = "/etc/yunohost/apps/" + app cwd = os.path.join(APPS_SETTING_PATH, app)
# FIXME: this should probably be ran in a tmp workdir...
retcode = hook_exec( retcode = hook_exec(
path, path,
env=env_dict, env=env_dict,
@ -1811,6 +1775,7 @@ def app_config_show_panel(operation_logger, app):
"YNH_APP_INSTANCE_NUMBER": str(app_instance_nb), "YNH_APP_INSTANCE_NUMBER": str(app_instance_nb),
} }
# FIXME: this should probably be ran in a tmp workdir...
return_code, parsed_values = hook_exec( return_code, parsed_values = hook_exec(
config_script, args=["show"], env=env, return_format="plain_dict" config_script, args=["show"], env=env, return_format="plain_dict"
) )
@ -1923,6 +1888,7 @@ def app_config_apply(operation_logger, app, args):
"Ignore key '%s' from arguments because it is not in the config", key "Ignore key '%s' from arguments because it is not in the config", key
) )
# FIXME: this should probably be ran in a tmp workdir...
return_code = hook_exec( return_code = hook_exec(
config_script, config_script,
args=["apply"], args=["apply"],
@ -2237,43 +2203,28 @@ def _set_app_settings(app_id, settings):
yaml.safe_dump(settings, f, default_flow_style=False) yaml.safe_dump(settings, f, default_flow_style=False)
def _extract_app_from_file(path, remove=False): def _extract_app_from_file(path):
""" """
Unzip or untar application tarball in APP_TMP_FOLDER, or copy it from a directory Unzip / untar / copy application tarball or directory to a tmp work directory
Keyword arguments: Keyword arguments:
path -- Path of the tarball or directory path -- Path of the tarball or directory
remove -- Remove the tarball after extraction
Returns:
Dict manifest
""" """
logger.debug(m18n.n("extracting")) logger.debug(m18n.n("extracting"))
if os.path.exists(APP_TMP_FOLDER):
shutil.rmtree(APP_TMP_FOLDER)
os.makedirs(APP_TMP_FOLDER)
path = os.path.abspath(path) path = os.path.abspath(path)
extracted_app_folder = _make_tmp_workdir_for_app()
if ".zip" in path: if ".zip" in path:
extract_result = os.system( extract_result = os.system(f"unzip '{path}' -d {extracted_app_folder} > /dev/null 2>&1")
"unzip %s -d %s > /dev/null 2>&1" % (path, APP_TMP_FOLDER)
)
if remove:
os.remove(path)
elif ".tar" in path: elif ".tar" in path:
extract_result = os.system( extract_result = os.system(f"tar -xf '{path}' -C {extracted_app_folder} > /dev/null 2>&1")
"tar -xf %s -C %s > /dev/null 2>&1" % (path, APP_TMP_FOLDER)
)
if remove:
os.remove(path)
elif os.path.isdir(path): elif os.path.isdir(path):
shutil.rmtree(APP_TMP_FOLDER) shutil.rmtree(extracted_app_folder)
if path[-1] != "/": if path[-1] != "/":
path = path + "/" path = path + "/"
extract_result = os.system('cp -a "%s" %s' % (path, APP_TMP_FOLDER)) extract_result = os.system(f"cp -a '{path}' {extracted_app_folder}")
else: else:
extract_result = 1 extract_result = 1
@ -2281,7 +2232,6 @@ def _extract_app_from_file(path, remove=False):
raise YunohostError("app_extraction_failed") raise YunohostError("app_extraction_failed")
try: try:
extracted_app_folder = APP_TMP_FOLDER
if len(os.listdir(extracted_app_folder)) == 1: if len(os.listdir(extracted_app_folder)) == 1:
for folder in os.listdir(extracted_app_folder): for folder in os.listdir(extracted_app_folder):
extracted_app_folder = extracted_app_folder + "/" + folder extracted_app_folder = extracted_app_folder + "/" + folder
@ -2508,24 +2458,11 @@ def _get_git_last_commit_hash(repository, reference="HEAD"):
def _fetch_app_from_git(app): def _fetch_app_from_git(app):
""" """
Unzip or untar application tarball in APP_TMP_FOLDER Unzip or untar application tarball to a tmp directory
Keyword arguments: Keyword arguments:
app -- App_id or git repo URL app -- App_id or git repo URL
Returns:
Dict manifest
""" """
extracted_app_folder = APP_TMP_FOLDER
app_tmp_archive = "{0}.zip".format(extracted_app_folder)
if os.path.exists(extracted_app_folder):
shutil.rmtree(extracted_app_folder)
if os.path.exists(app_tmp_archive):
os.remove(app_tmp_archive)
logger.debug(m18n.n("downloading"))
# Extract URL, branch and revision to download # Extract URL, branch and revision to download
if ("@" in app) or ("http://" in app) or ("https://" in app): if ("@" in app) or ("http://" in app) or ("https://" in app):
@ -2549,6 +2486,10 @@ def _fetch_app_from_git(app):
branch = app_info["git"]["branch"] branch = app_info["git"]["branch"]
revision = str(app_info["git"]["revision"]) revision = str(app_info["git"]["revision"])
extracted_app_folder = _make_tmp_workdir_for_app()
logger.debug(m18n.n("downloading"))
# Download only this commit # Download only this commit
try: try:
# We don't use git clone because, git clone can't download # We don't use git clone because, git clone can't download
@ -3384,6 +3325,23 @@ def _load_apps_catalog():
# ############################### # # ############################### #
# #
def _make_tmp_workdir_for_app(app=None):
# Create parent dir if it doesn't exists yet
if not os.path.exists(APP_TMP_WORKDIRS):
os.makedirs(APP_TMP_WORKDIRS)
# Cleanup old dirs
for dir_ in os.listdir(APP_TMP_WORKDIRS):
shutil.rmtree(os.path.join(APP_TMP_WORKDIRS, dir_))
tmpdir = tempfile.mkdtemp(prefix="app_", dir=APP_TMP_WORKDIRS)
# Copy existing app scripts, conf, ... if an app arg was provided
if app:
os.system(f"cp -a {APPS_SETTING_PATH}/{app}/* {tmpdir}")
return tmpdir
def is_true(arg): def is_true(arg):
""" """

View file

@ -53,6 +53,7 @@ from yunohost.app import (
_patch_legacy_php_versions, _patch_legacy_php_versions,
_patch_legacy_php_versions_in_settings, _patch_legacy_php_versions_in_settings,
LEGACY_PHP_VERSION_REPLACEMENTS, LEGACY_PHP_VERSION_REPLACEMENTS,
_make_tmp_workdir_for_app
) )
from yunohost.hook import ( from yunohost.hook import (
hook_list, hook_list,
@ -644,7 +645,7 @@ class BackupManager:
restore_hooks_dir = os.path.join(self.work_dir, "hooks", "restore") restore_hooks_dir = os.path.join(self.work_dir, "hooks", "restore")
if not os.path.exists(restore_hooks_dir): if not os.path.exists(restore_hooks_dir):
filesystem.mkdir(restore_hooks_dir, mode=0o750, parents=True, uid="admin") filesystem.mkdir(restore_hooks_dir, mode=0o700, parents=True, uid="root")
restore_hooks = hook_list("restore")["hooks"] restore_hooks = hook_list("restore")["hooks"]
@ -705,21 +706,17 @@ class BackupManager:
settings_dir = os.path.join(self.work_dir, "apps", app, "settings") settings_dir = os.path.join(self.work_dir, "apps", app, "settings")
logger.info(m18n.n("app_start_backup", app=app)) logger.info(m18n.n("app_start_backup", app=app))
tmp_folder = tempfile.mkdtemp() tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app)
try: try:
# Prepare backup directory for the app # Prepare backup directory for the app
filesystem.mkdir(tmp_app_bkp_dir, 0o750, True, uid="admin") filesystem.mkdir(tmp_app_bkp_dir, 0o700, True, uid="root")
# Copy the app settings to be able to call _common.sh # Copy the app settings to be able to call _common.sh
shutil.copytree(app_setting_path, settings_dir) shutil.copytree(app_setting_path, settings_dir)
# Copy app backup script in a temporary folder and execute it
app_script = os.path.join(app_setting_path, "scripts/backup")
tmp_script = os.path.join(tmp_folder, "backup")
subprocess.call(["install", "-Dm555", app_script, tmp_script])
hook_exec( hook_exec(
tmp_script, raise_on_error=True, chdir=tmp_app_bkp_dir, env=env_dict f"{tmp_workdir_for_app}/scripts/backup",
raise_on_error=True, chdir=tmp_app_bkp_dir, env=env_dict
)[0] )[0]
self._import_to_list_to_backup(env_dict["YNH_BACKUP_CSV"]) self._import_to_list_to_backup(env_dict["YNH_BACKUP_CSV"])
@ -750,8 +747,7 @@ class BackupManager:
# Remove tmp files in all situations # Remove tmp files in all situations
finally: finally:
if tmp_folder and os.path.exists(tmp_folder): shutil.rmtree(tmp_workdir_for_app)
shutil.rmtree(tmp_folder)
filesystem.rm(env_dict["YNH_BACKUP_CSV"], force=True) filesystem.rm(env_dict["YNH_BACKUP_CSV"], force=True)
# #
@ -1405,13 +1401,11 @@ class RestoreManager:
filesystem.chown(app_scripts_new_path, "root", None, True) filesystem.chown(app_scripts_new_path, "root", None, True)
# Copy the app scripts to a writable temporary folder # Copy the app scripts to a writable temporary folder
# FIXME : use 'install -Dm555' or something similar to what's done tmp_workdir_for_app = _make_tmp_workdir_for_app()
# in the backup method ? copytree(app_scripts_in_archive, tmp_workdir_for_app)
tmp_folder_for_app_restore = tempfile.mkdtemp(prefix="restore") filesystem.chmod(tmp_workdir_for_app, 0o700, 0o700, True)
copytree(app_scripts_in_archive, tmp_folder_for_app_restore) filesystem.chown(tmp_workdir_for_app, "root", None, True)
filesystem.chmod(tmp_folder_for_app_restore, 0o550, 0o550, True) restore_script = os.path.join(tmp_workdir_for_app, "restore")
filesystem.chown(tmp_folder_for_app_restore, "root", None, True)
restore_script = os.path.join(tmp_folder_for_app_restore, "restore")
# Restore permissions # Restore permissions
if not os.path.isfile("%s/permissions.yml" % app_settings_new_path): if not os.path.isfile("%s/permissions.yml" % app_settings_new_path):
@ -1466,7 +1460,7 @@ class RestoreManager:
# Cleanup # Cleanup
shutil.rmtree(app_settings_new_path, ignore_errors=True) shutil.rmtree(app_settings_new_path, ignore_errors=True)
shutil.rmtree(tmp_folder_for_app_restore, ignore_errors=True) shutil.rmtree(tmp_workdir_for_app, ignore_errors=True)
return return
@ -1516,7 +1510,7 @@ class RestoreManager:
failure_message_with_debug_instructions = operation_logger.error(error) failure_message_with_debug_instructions = operation_logger.error(error)
finally: finally:
# Cleaning temporary scripts directory # Cleaning temporary scripts directory
shutil.rmtree(tmp_folder_for_app_restore, ignore_errors=True) shutil.rmtree(tmp_workdir_for_app, ignore_errors=True)
if not restore_failed: if not restore_failed:
self.targets.set_result("apps", app_instance_name, "Success") self.targets.set_result("apps", app_instance_name, "Success")
@ -1872,7 +1866,7 @@ class CopyBackupMethod(BackupMethod):
dest_parent = os.path.dirname(dest) dest_parent = os.path.dirname(dest)
if not os.path.exists(dest_parent): if not os.path.exists(dest_parent):
filesystem.mkdir(dest_parent, 0o750, True, uid="admin") filesystem.mkdir(dest_parent, 0o700, True, uid="admin")
if os.path.isdir(source): if os.path.isdir(source):
shutil.copytree(source, dest) shutil.copytree(source, dest)