A heap of tests, and the bugs they found

PortMaster/pylibs/harbourmaster/__init__.py

@@ -34,6 +34,7 @@
     get_dict_list,
     get_path_fs,
     hash_file,
+    json_load_dict,
     json_safe_load,
     json_safe_loads,
     load_pm_signature,

PortMaster/pylibs/harbourmaster/cli.py

@@ -25,6 +25,7 @@
 
 
 LOG_FILE = harbourmaster.HM_TOOLS_DIR / "PortMaster" / "harbourmaster.txt"
+LOG_FILE_HANDLE = None
 if LOG_FILE.parent.is_dir():
     LOG_FILE_HANDLE = logger.add(LOG_FILE, level="DEBUG", backtrace=True, diagnose=True)
 
@@ -271,8 +272,9 @@ def do_portsjson(hm, argv):
         port_info['date_added'], port_info['date_updated'] = (
             ports_info.get('ports', {}).get(port_name, {}).get('date', (default_time, default_time)))
 
-        del port_info['status']
-        del port_info['files']
+        ## Only installed ports have these.
+        port_info.pop('status', None)
+        port_info.pop('files', None)
 
         ports_json['ports'].append(port_info)
 
@@ -468,6 +470,11 @@ def do_fifo_control(hm, argv):
 
 
 def do_device_info(hm, argv):
+    """
+    Device info for the website, every known device and CFW.
+
+    {command} device_info [filename]
+    """
     import json
 
     website_data = {}
@@ -606,7 +613,8 @@ def main(argv):
             logger.remove(0)  # For the default handler, it's actually '0'.
             logger.add(sys.stderr, level="DEBUG")
 
-        if config['no-log']:
+        ## logger.remove(None) would remove every handler.
+        if config['no-log'] and LOG_FILE_HANDLE is not None:
             logger.remove(LOG_FILE_HANDLE)
             LOG_FILE_HANDLE = None
 

PortMaster/pylibs/harbourmaster/console.py

@@ -31,7 +31,12 @@ def in_terminal():
     """
     Are we in a terminal?
     """
-    return os.isatty(sys.stdout.fileno()) and os.isatty(sys.stdin.fileno())
+    try:
+        return os.isatty(sys.stdout.fileno()) and os.isatty(sys.stdin.fileno())
+
+    except (AttributeError, OSError, ValueError):
+        ## No real file behind them (io.UnsupportedOperation is both of the last two).
+        return False
 
 
 def do_cprint_output(file_handle):

PortMaster/pylibs/harbourmaster/harbour.py

@@ -8,7 +8,7 @@
 from loguru import logger
 from .config import HM_PORTS_DIR, HM_SCRIPTS_DIR, HM_SOURCE_DEFAULTS, HM_TESTING, HM_TOOLS_DIR
 from .hardware import device_info
-from .util import Callback, json_safe_load
+from .util import Callback, json_load_dict, json_safe_load
 from .source import HM_SOURCE_APIS
 from .platform import HM_PLATFORMS
 from .hm_info import InfoMixin
@@ -125,11 +125,8 @@ def __init__(self, config, *, tools_dir=None, ports_dir=None, scripts_dir=None,
         with self.callback.enable_messages():
             self.callback.message(_("Loading..."))
 
-            if not self.cfg_file.is_file():
-                self.cfg_data = self.DEFAULT_CONFIG.copy()
-            else:
-                with open(self.cfg_file, 'r') as fh:
-                    self.cfg_data = json.load(fh)
+            ## Missing or broken (cut off by a power off while saving): start afresh.
+            self.cfg_data = json_load_dict(self.cfg_file, self.DEFAULT_CONFIG)
 
             if self.cfg_data.get('first-run', True) or not self.cfg_dir.is_dir():
                 self.cfg_dir.mkdir(0o755, parents=True, exist_ok=True)

PortMaster/pylibs/harbourmaster/hm_info.py

@@ -70,7 +70,9 @@ def load_info_fetch_remote(self, local_file, remote_url, remote_size, sha_source
             if sha_local != sha_source:
                 self.callback.message(_("Download validation failed."))
                 logger.error(f"File validation failed {sha_local} vs {sha_source}")
+                ## Keep the old file: this went on to copy the deleted download over it.
                 temp_file.unlink()
+                return False
 
             if temp_file != local_file:
                 shutil.copy2(temp_file, local_file)
@@ -107,9 +109,11 @@ def load_info(self, force_load=False):
             with open(self.runtimes_file, 'r') as fh:
                 self.runtimes_info = json_safe_load(fh)
 
-            self.list_runtimes()
+            ## A cut-off or corrupt runtimes.json crashed every start.
+            if isinstance(self.runtimes_info, dict):
+                self.list_runtimes()
 
-        if self.runtimes_info is None:
+        if not isinstance(self.runtimes_info, dict):
             self.runtimes_info = {}
 
         # These are the base files, and empty versions that are safe.

PortMaster/pylibs/harbourmaster/hm_install.py

@@ -12,7 +12,7 @@
 from .console import cprint
 from .config import HM_ACCEPTABLE_NON_BASH_TOP_LEVEL_FILES
 from .util import HarbourException, add_dict_list_unique, add_list_unique, add_pm_signature, get_dict_list, get_path_fs, hash_file, name_cleaner, runtime_nicename
-from .info import port_info_load, port_info_merge
+from .info import is_safe_port_item, port_info_load, port_info_merge
 from .source import raw_download
 from .captain import check_port
 
@@ -584,11 +584,15 @@ def uninstall_port(self, port_name):
 
         ports_dir = self.ports_dir
 
+        for item in all_port_items:
+            if not is_safe_port_item(item):
+                logger.error(f"Not removing {item!r} for {port_name}: it is outside the ports folder.")
+
         uninstall_items = [
             item
             for item in all_port_items
             # Only delete files/scripts with only 1 owner.
-            if len(get_dict_list(all_items, item)) == 1]
+            if len(get_dict_list(all_items, item)) == 1 and is_safe_port_item(item)]
 
         self.platform.port_uninstall(port_name, port_info, all_port_items)
 

PortMaster/pylibs/harbourmaster/hm_ports.py

@@ -46,7 +46,8 @@ def _get_pm_signature(self, file_name):
 
                 if len(port_owners) > 0:
                     add_pm_signature(file_name, [port_owners[0], other_name])
-                    return (other_name, file_name.name, port_owners[0])
+                    ## (file_name, original_file_name, port_name), as the other returns.
+                    return (file_name.name, other_name, port_owners[0])
 
             return (file_name.name, file_name.name, None)
 
@@ -74,17 +75,18 @@ def _load_port_info(self, port_file):
                 logger.error(f"Unable to load {port_file}, missing 'name' and 'items' keys.")
                 return None
 
+            ## ports_info.json's items are "script.sh": "port.zip" (a list if shared), in the script's own case.
             for item in port_info['items']:
-                port_temp = ports_info['items'].get(item.casefold(), None)
-                if isinstance(port_temp, str):
+                port_owners = get_dict_list(ports_info['items'], item)
+                if len(port_owners) > 0:
                     break
             else:
                 # Couldn't find the port.
                 logger.error(f"Unable to load {port_file}, unknown items.")
                 return None
 
             changed = True
-            port_info['name'] = name_cleaner(port_temp[0])
+            port_info['name'] = name_cleaner(port_owners[0])
 
         # Force the port_info['name'] to be lowercase/casefolded.
         if port_info['name'] != name_cleaner(port_info['name']):
@@ -179,7 +181,9 @@ def load_ports(self):
 
         all_ports = {}
         ports_files = {}
+        ## original name -> renamed file, and back.
         file_renames = {}
+        file_originals = {}
 
         ports_info = self.ports_info()
         self._PORT_INFO_CACHE.clear()
@@ -374,6 +378,7 @@ def load_ports(self):
                 if pm_signature[0] != pm_signature[1]:
                     # We atleast know the file is renamed.
                     file_renames[pm_signature[1]] = pm_signature[0]
+                    file_originals[pm_signature[0]] = pm_signature[1]
 
                 if pm_signature[2] is None:
                     # Unknown port?
@@ -407,7 +412,7 @@ def load_ports(self):
 
             elif len(port_owners) == 0:
                 if unknown_file.casefold().endswith('.sh'):
-                    re_name = file_renames.get(unknown_file, None)
+                    re_name = file_originals.get(unknown_file, None)
                     if re_name is not None:
                         port_owners = get_dict_list(ports_info['items'], re_name)
 
@@ -511,6 +516,10 @@ def load_ports(self):
                 file_names = get_dict_list(port_info['files'], port_file)
 
                 for file_name in list(file_names):
+                    ## A new port's port.json is written below.
+                    if port_file == 'port.json' and port_name in ports_files:
+                        continue
+
                     if not self._ports_dir_exists(file_name):
                         remove_dict_list(port_info['files'], port_file, file_name)
                         port_info['changed'] = True

PortMaster/pylibs/harbourmaster/hm_runtimes.py

@@ -25,6 +25,11 @@ def list_runtimes(self):
                 runtime_data['remote']['aarch64'] = temp
                 changed = True
 
+            local_info = runtime_data.get('local')
+            if not isinstance(local_info, dict) or (local_info.get('status') == 'Not Installed' and runtime_file.is_file()):
+                ## Never checked, or copied in since (by hand, autoinstall): check it now.
+                runtime_data.pop('local', None)
+
             if 'local' not in runtime_data:
                 changed = True
                 runtime_file = (self.libs_dir / runtime_name)
@@ -66,7 +71,8 @@ def list_runtimes(self):
                     }
 
             else:
-                if not runtime_file.is_file():
+                ## Only when it changes: this runs on every start.
+                if not runtime_file.is_file() and runtime_data['local'].get('status') != 'Not Installed':
                     changed = True
                     runtime_data['local'] = {
                         'status': 'Not Installed',

PortMaster/pylibs/harbourmaster/info.py

@@ -95,6 +95,20 @@ def _clean_attr_types(attr, source_name):
         attr[key] = bool(attr.get(key, False))
 
 
+def is_safe_port_item(item):
+    """
+    A port's item (a script or folder in the ports folder) is a plain relative name:
+    uninstalling deletes it, so `..` in it would delete things outside the ports folder.
+    """
+    if not isinstance(item, str) or item.strip() in ("", ".", "./"):
+        return False
+
+    if item.startswith(('/', '\\')):
+        return False
+
+    return not any(part == '..' for part in item.replace('\\', '/').split('/'))
+
+
 def port_info_load(raw_info, source_name=None, do_default=False):
     if isinstance(raw_info, pathlib.PurePath):
         source_name = str(raw_info)
@@ -251,58 +265,22 @@ def port_info_load(raw_info, source_name=None, do_default=False):
         if attr in info:
             port_info[attr] = info[attr]
 
-    if isinstance(port_info['items'], list):
-        i = 0
-        while i < len(port_info['items']):
-            item = port_info['items'][i]
-            if item.startswith('/'):
-                logger.error(f"port_info['items'] contains bad name {item!r}")
-                del port_info['items'][i]
-                continue
-
-            if item.startswith('../'):
-                logger.error(f"port_info['items'] contains bad name {item!r}")
-                del port_info['items'][i]
-                continue
-
-            if '/../' in item:
-                logger.error(f"port_info['items'] contains bad name {item!r}")
-                del port_info['items'][i]
-                continue
-
-            if item == "":
-                logger.error(f"port_info['items'] contains bad name {item!r}")
-                del port_info['items'][i]
-
-            i += 1
-
-    if isinstance(port_info['items_opt'], list):
-        i = 0
-        while i < len(port_info['items_opt']):
-            item = port_info['items_opt'][i]
-            if item.startswith('/'):
-                logger.error(f"port_info['items_opt'] contains bad name {item}")
-                del port_info['items_opt'][i]
-                continue
-
-            if item.startswith('../'):
-                logger.error(f"port_info['items_opt'] contains bad name {item}")
-                del port_info['items_opt'][i]
-                continue
-
-            if '/../' in item:
-                logger.error(f"port_info['items_opt'] contains bad name {item}")
-                del port_info['items_opt'][i]
-                continue
-
-            if item == "":
-                logger.error(f"port_info['items'] contains bad name {item!r}")
-                del port_info['items_opt'][i]
-
-            i += 1
-
-        if port_info['items_opt'] == []:
-            port_info['items_opt'] = None
+    if port_info['items_opt'] is not None and not isinstance(port_info['items_opt'], list):
+        logger.warning(f"port_info['items_opt'] is not a list in {source_name!r}: {port_info['items_opt']!r}")
+        port_info['items_opt'] = None
+
+    for key in ('items', 'items_opt'):
+        if not isinstance(port_info[key], list):
+            continue
+
+        for item in port_info[key]:
+            if not is_safe_port_item(item):
+                logger.error(f"port_info[{key!r}] contains bad name {item!r}")
+
+        port_info[key] = [item for item in port_info[key] if is_safe_port_item(item)]
+
+    if port_info['items_opt'] == []:
+        port_info['items_opt'] = None
 
     if isinstance(port_info['attr'].get('genres', None), list):
         genres = port_info['attr']['genres']

PortMaster/pylibs/harbourmaster/platform/base.py

@@ -68,10 +68,24 @@ def gamelist_backup(self):
 
         gamelist_xml = self.gamelist_file()
 
-        if gamelist_xml in (None, SPECIAL_GAMELIST_CODE):
+        if gamelist_xml is None:
             yield gamelist_xml
             return
 
+        if gamelist_xml is SPECIAL_GAMELIST_CODE:
+            ## The CFW's own catalogue (muOS, TrimUI): nothing to back up, but a failure
+            ## mustn't fail the install either.
+            try:
+                yield gamelist_xml
+
+            except BaseException as err:
+                if isinstance(err, (CancelEvent, KeyboardInterrupt, SystemExit, GeneratorExit)):
+                    raise
+
+                logger.exception(f"Updating the frontend's catalogue failed: {err}")
+
+            return
+
         gamelist_bak = gamelist_xml.with_name(gamelist_xml.name + '.bak')
 
         broken = False

PortMaster/pylibs/harbourmaster/platform/muos.py

@@ -42,6 +42,13 @@ def gamelist_add(self, gameinfo_file):
         INFO_PREVIEW_DIR = INFO_CATALOG / "preview"
         INFO_TEXT_DIR    = INFO_CATALOG / "text"
 
+        ## Not this muOS's layout: leave it be.
+        if not INFO_CATALOG.is_dir():
+            return
+
+        for info_dir in (INFO_BOX_DIR, INFO_PREVIEW_DIR, INFO_TEXT_DIR):
+            info_dir.mkdir(exist_ok=True)
+
         with self.gamelist_backup() as gamelist_xml:
             if gamelist_xml is None:
                 return

PortMaster/pylibs/harbourmaster/platform/retrodeck.py

@@ -82,6 +82,10 @@ def get_rdconfig(self):
             if not rdconfig_file.is_file():
                 rdconfig_file=(Path.home() / ".var/app/net.retrodeck.retrodeck/config/retrodeck/retrodeck.cfg")
 
+            if not rdconfig_file.is_file():
+                logger.error("Unable to find RetroDECK's retrodeck.json or retrodeck.cfg.")
+                return None
+
             with open(rdconfig_file, 'r') as fh:
                 for line in fh:
                     line = line.strip()