[Backport main] Bandit audit (#479)

Addon.py

@@ -29,7 +29,12 @@
 from typing import Dict, Set, List, Optional
 from threading import Lock
 from enum import IntEnum, auto
-from xml.etree.ElementTree import ParseError as XmlParseError
+
+# Audited: only the exception class is imported, for catching errors raised by defusedxml,
+# which re-exports this same class. All parsing is done by defusedxml. (added nosec B405)
+from xml.etree.ElementTree import ParseError as XmlParseError  # nosec B405
+
+from defusedxml import DefusedXmlException
 
 try:
     import importlib.metadata as importlib_metadata
@@ -78,6 +83,12 @@
     "web": "Web",
 }
 
+#  The package metadata content types that have a dedicated category in the Addon Manager's
+#  filter list. Every other content type, whether it is the standard "other" type or a type
+#  introduced after this version of the Addon Manager was released, is shown in the "Other"
+#  category.
+CATEGORIZED_CONTENT_TYPES = frozenset(["workbench", "macro", "preferencepack", "bundle"])
+
 
 class Addon:
     """Encapsulates information about a FreeCAD addon"""
@@ -179,6 +190,16 @@ def __init__(
         self.display_name = self.name
         self.url = url.strip()
         self.relative_cache_path = ""
+
+        # A remote location for a zip of this Addon's contents. This is used for Addons that are
+        # not cached in their entirety (typically due to their size). The canonical example here is
+        # the Parts Library.
+        self.zip_url = ""
+
+        # True for Addons that are large enough that downloading all of them for every update is
+        # expensive, so git is used for them whenever it is available. Set by the Addon Index.
+        self.prefer_git = False
+
         self.branch = branch.strip()
         self.branch_display_name = branch.strip()
         self.repo_type = Addon.Kind.WORKBENCH
@@ -325,11 +346,11 @@ def load_metadata_file(self, file: str) -> None:
         if os.path.exists(file):
             try:
                 metadata = MetadataReader.from_file(file)
-            except XmlParseError:
+            except (XmlParseError, DefusedXmlException):
                 fci.Console.PrintWarning(
                     "An invalid or corrupted package.xml file was found in the cache for"
                 )
-                fci.Console.PrintWarning(f" {self.name}... ignoring the bad data.\n")
+                fci.Console.PrintWarning(f" {self.name}… ignoring the bad data.\n")
                 return
             self.set_metadata(metadata)
             self._clean_url()
@@ -344,11 +365,11 @@ def _load_installed_metadata(self) -> None:
         if os.path.isfile(installed_metadata_path):
             try:
                 self.installed_metadata = MetadataReader.from_file(installed_metadata_path)
-            except XmlParseError:
+            except (XmlParseError, DefusedXmlException):
                 fci.Console.PrintWarning(
                     "An invalid or corrupted package.xml file was found in installation of"
                 )
-                fci.Console.PrintWarning(f" {self.name}... ignoring the bad data.\n")
+                fci.Console.PrintWarning(f" {self.name}… ignoring the bad data.\n")
                 return
 
     def set_metadata(self, metadata: Metadata) -> None:
@@ -484,17 +505,21 @@ def contains_macro(self) -> bool:
             return True
         return self.contains_packaged_content("macro")
 
+    def packaged_content_types(self) -> Set[str]:
+        """The content types declared by this package's metadata. Empty for anything that is
+        not a package."""
+        if self.repo_type != Addon.Kind.PACKAGE:
+            return set()
+        if self.metadata is None:
+            fci.Console.PrintLog(
+                f"Addon Manager internal error: lost metadata for package {self.name}\n"
+            )
+            return set()
+        return set(self.metadata.content)
+
     def contains_packaged_content(self, content_type: str):
         """Determine if the package contains content_type"""
-        if self.repo_type == Addon.Kind.PACKAGE:
-            if self.metadata is None:
-                fci.Console.PrintLog(
-                    f"Addon Manager internal error: lost metadata for package {self.name}\n"
-                )
-                return False
-            content = self.metadata.content
-            return content_type in content
-        return False
+        return content_type in self.packaged_content_types()
 
     def contains_preference_pack(self) -> bool:
         """Determine if this package contains a preference pack"""
@@ -505,8 +530,9 @@ def contains_bundle(self) -> bool:
         return self.contains_packaged_content("bundle")
 
     def contains_other(self) -> bool:
-        """Determine if this package contains an "other" content item"""
-        return self.contains_packaged_content("other")
+        """Determine if this package contains an "other" content item, or any content type that
+        this version of the Addon Manager does not have a category for."""
+        return bool(self.packaged_content_types() - CATEGORIZED_CONTENT_TYPES)
 
     def walk_dependency_tree(self, all_repos: Dict[str, "Addon"], deps: Dependencies):
         """Compute the total dependency tree for this repo (recursive)
@@ -688,7 +714,9 @@ def _find_classname_in_file(current_file) -> str:
         return ""
 
     def get_zip_url(self) -> str:
-        if self.url.endswith(".zip"):
+        if self.zip_url:
+            zip_url = self.zip_url
+        elif self.url.endswith(".zip"):
             zip_url = self.url
         else:
             # The ZIP url is based on the location of the main cache file:
@@ -835,7 +863,7 @@ def package_is_installed(package_name: str) -> bool:
         # can do the check by PyPI package name:
         if importlib_metadata is None:
             fci.Console.PrintMessage(
-                f"Cannot check for installation of `{package_name}`... marking it for "
+                f"Cannot check for installation of `{package_name}`… marking it for "
                 "reinstallation to be safe\n"
             )
             return False

AddonCatalog.py

@@ -25,7 +25,12 @@
 import base64
 import datetime
 import os
-from xml.etree.ElementTree import ParseError as XmlParseError
+
+# Audited: only the exception class is imported, for catching errors raised by defusedxml,
+# which re-exports this same class. All parsing is done by defusedxml. (added nosec B405)
+from xml.etree.ElementTree import ParseError as XmlParseError  # nosec B405
+
+from defusedxml import DefusedXmlException
 from dataclasses import dataclass
 import json
 from hashlib import sha256
@@ -81,7 +86,7 @@ class AddonCatalogEntry:
     metadata: Optional[CatalogEntryMetadata] = None  # Generated by the cache system
     last_update_time: str = ""  # Generated by the cache system
     curated: bool = True  # Generated by the cache system
-    sparse_cache: bool = False  # Generated by the cache system
+    sparse_cache: bool = False  # Set by the catalog for Addons too large to cache in full
     relative_cache_path: str = ""  # Generated by the cache system
     git_hash: Optional[str] = None  # Generated by the cache system
     git_tag: Optional[str] = None  # Generated by the cache system
@@ -138,26 +143,27 @@ def instantiate_addon(self, addon_id: str) -> Addon:
             state = Addon.Status.UNCHECKED
         else:
             state = Addon.Status.NOT_INSTALLED
-        if self.sparse_cache:
-            if self.zip_url:
-                url = self.zip_url
-            else:
-                # Technically, this should never happen, but just in case...
-                raise RuntimeError(f"Sparse cache entry {addon_id} has no zip_url")
-        elif self.repository:
-            url = self.repository
-        else:
-            url = self.zip_url
+        if self.sparse_cache and not self.zip_url:
+            # Technically, this should never happen, but just in case...
+            raise RuntimeError(f"Sparse cache entry {addon_id} has no zip_url")
+        url = self.repository or self.zip_url or ""
         if self.git_ref:
             addon = Addon(addon_id, url, state, branch=self.git_ref)
         else:
             addon = Addon(addon_id, url, state)
         addon.relative_cache_path = self.relative_cache_path
+        if self.sparse_cache or not self.repository:
+            # If the cache is sparse, we need a "real" location to get the thing from when installing
+            addon.zip_url = self.zip_url or ""
+        # If it's too big to cache, it's probably too big to want to update by re-downloading the
+        # whole thing. So if the user's machine has git on it, and we know its git repo, then tell
+        # the Addon Manager to try to use git when installing/updating.
+        addon.prefer_git = bool(self.sparse_cache and self.repository)
 
         if self.metadata:
             try:
                 self._load_addon_metadata(addon, self.metadata)
-            except XmlParseError:
+            except (XmlParseError, DefusedXmlException):
                 fci.Console.PrintWarning(
                     "An invalid or corrupted package.xml file was installed "
                     f"for {addon.display_name}\n"
@@ -171,7 +177,7 @@ def instantiate_addon(self, addon_id: str) -> Addon:
             try:
                 package_file = os.path.join(fci.DataPaths().mod_dir, addon_id, "package.xml")
                 addon.installed_metadata = MetadataReader.from_file(package_file)
-            except (FileNotFoundError, XmlParseError, RuntimeError):
+            except (FileNotFoundError, XmlParseError, DefusedXmlException, RuntimeError):
                 pass  # If there was an error, just ignore it, no metadata is not fatal
 
             most_recent_mtime = AddonCatalogEntry.most_recent_mtime(addon_id)

AddonCatalog.schema.json

@@ -56,6 +56,9 @@
           },
           "curated": {
             "type": "boolean"
+          },
+          "sparse_cache": {
+            "type": "boolean"
           }
         },
         "anyOf": [