v0.5.1: the update alert could have broken middlewared at startup

middlewared's alert.load() imports every file in alert/source/ with NO try/except:

    def load(self):
        for module in load_modules(.../alert/source):
            for cls in load_classes(module, AlertSource, (ThreadedAlertSource,)):
                ...

and it runs during setup. A module that raises on import therefore takes
middlewared's startup down with it -- exactly the class of failure this project
exists to avoid.

apply.sh now COMPILES the substituted alert source and refuses to write it if it
does not parse. An uninstalled alert is a missing convenience; a broken one is a
broken box.

@PATCH_DIR@ is also substituted with repr() rather than raw, so a repository path
containing a quote or backslash yields a valid Python literal instead of a syntax
error in the installed module.

The alert source no longer mutates sys.path. It loaded tools/release_notes.py via
sys.path.insert(0, ...), which shadows the stdlib for that interpreter -- and
ThreadedAlertSource runs in middlewared's thread pool, so mutating sys.path is a
race. It now loads by file path with importlib.

New tests guard every import-time failure mode: the module compiles, apply.sh
compiles before writing, awkward paths (quotes, backslashes) still produce valid
modules, nothing but imports/constants/classes runs at module scope, every
AlertClass name ends in "AlertClass" (AlertClassMeta raises NameError otherwise),
the alert text placeholders match the args passed, and no git command that writes
to .git is ever used.

152 tests, ruff and shellcheck -S style clean.

CHANGELOG.md

@@ -1,5 +1,33 @@
 # Changelog
 
+## v0.5.1 — 2026-07-13
+
+### Fixed
+
+- **The update alert could have broken middlewared at startup.** middlewared's
+  `alert.load()` imports every file in `alert/source/` with **no try/except**, and
+  it runs during setup — so a module that raises on import takes middlewared down
+  with it. `apply.sh` now **compiles the substituted alert source and refuses to
+  write it** if it does not parse. An uninstalled alert is a missing convenience;
+  a broken one is a broken box.
+
+- **`@PATCH_DIR@` is substituted with `repr()`**, so a repository path containing
+  a quote or a backslash produces a valid Python literal instead of a syntax error
+  in the installed module.
+
+- **The alert source no longer mutates `sys.path`.** It loaded
+  `tools/release_notes.py` via `sys.path.insert(0, …)`, which shadows the stdlib
+  for that interpreter — and `ThreadedAlertSource` runs in middlewared's thread
+  pool, so mutating `sys.path` is a race. It now loads the module by file path with
+  `importlib`.
+
+### Notes
+
+Timing, for the record: `process_alerts` is `@periodic(60)` and
+`alert_source_last_run` is in-memory, so the check runs **within 60 seconds of any
+middlewared restart** (which this patch performs at every boot) and otherwise
+**within 24 hours** of a release.
+
 ## v0.5.0 — 2026-07-13
 
 ### Added

install.sh

@@ -18,7 +18,7 @@
 
 set -euo pipefail
 
-VERSION="0.5.0"
+VERSION="0.5.1"
 
 # The directory containing install.sh is the permanent install location.
 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"

patch/alert_source.py

@@ -20,11 +20,11 @@ the way a `git fetch` from middlewared (running as root) would.
 """
 
 import datetime
+import importlib.util
 import logging
 import os
 import re
 import subprocess
-import sys
 import urllib.request
 
 from middlewared.alert.base import (
@@ -102,11 +102,10 @@ class TrueCloudPatchUpdateAlertSource(ThreadedAlertSource):
         if not latest:
             return None
 
-        sys.path.insert(0, os.path.join(PATCH_DIR, "tools"))
+        rn = self._release_notes()
-        try:
+        if rn is None:
-            from release_notes import significance, version_tuple
+            return None
-        finally:
+        significance, version_tuple = rn.significance, rn.version_tuple
-            sys.path.pop(0)
 
         if version_tuple(latest) <= version_tuple(current):
             return None
@@ -136,6 +135,22 @@ class TrueCloudPatchUpdateAlertSource(ThreadedAlertSource):
         )
         return Alert(klass, args, key=[current, latest])
 
+    def _release_notes(self):
+        """Load tools/release_notes.py by path.
+
+        NOT via sys.path: prepending would shadow the stdlib for this interpreter,
+        and this runs in middlewared's thread pool, so mutating sys.path is a race.
+        """
+        path = os.path.join(PATCH_DIR, "tools", "release_notes.py")
+        try:
+            spec = importlib.util.spec_from_file_location("_tc_release_notes", path)
+            mod = importlib.util.module_from_spec(spec)
+            spec.loader.exec_module(mod)
+            return mod
+        except Exception:
+            logger.debug("could not load release_notes", exc_info=True)
+            return None
+
     def _installed_version(self):
         """The version of the patch actually checked out here."""
         try:

patch/apply.sh

@@ -32,7 +32,7 @@
 # Derive PATCH_DIR from this script's location (parent of the patch/ directory).
 PATCH_DIR="$(cd "$(dirname "$0")/.." && pwd)"
 LOG="$PATCH_DIR/apply.log"
-VERSION="0.5.0"
+VERSION="0.5.1"
 
 # Rotate log at 512 KB to avoid unbounded growth on a system volume.
 # Keep two prior generations (.1 and .2) so the last three boots are always available.
@@ -602,15 +602,26 @@ else:
     try:
         with open(alert_src, encoding='utf-8') as fh:
             _body = fh.read()
-        # PATCH_DIR is baked in: the alert source must find the repo it belongs to.
+
+        # repr() so ANY path becomes a valid Python literal -- a directory
+        # containing a quote or backslash would otherwise produce a syntax error.
+        _body = _body.replace('"@PATCH_DIR@"', repr(_patch_dir))
+
+        # COMPILE BEFORE WRITING. middlewared's alert.load() imports every file in
+        # alert/source/ with NO try/except, and it runs at startup -- a module that
+        # raises on import takes middlewared's setup down with it. An uninstalled
+        # alert is a missing convenience; a broken one is a broken box.
+        compile(_body, alert_dst, 'exec')
+
         with open(alert_dst, 'w', encoding='utf-8') as fh:
-            fh.write(_body.replace('@PATCH_DIR@', _patch_dir))
+            fh.write(_body)
         alert_ok = True
         alert_detail = 'update alert installed'
         print(f'OK: Installed update alert → {alert_dst}')
     except Exception as e:
         alert_detail = f'not applied: {e}'
         print(f'WARNING: could not install update alert: {e}')
+        print('WARNING: no update alert; everything else is unaffected.')
 
 patches = {
     'providers': {

patch/create_task.py

@@ -52,7 +52,7 @@ import subprocess
 import sys
 import time
 
-__version__ = "0.5.0"
+__version__ = "0.5.1"
 
 _PATCH_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 _STATUS_FILE = os.path.join(_PATCH_DIR, "hook_status.json")

recover.sh

@@ -17,7 +17,7 @@
 #   bash /mnt/tank/truenas-truecloud-patch/patch/apply.sh
 #   systemctl restart middlewared
 
-VERSION="0.5.0"
+VERSION="0.5.1"
 
 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"
 

tests/test_alert_source.py

@@ -0,0 +1,132 @@
+"""Tests for the update-available alert source.
+
+This file is imported by middlewared's `alert.load()`, which runs at STARTUP and
+has **no try/except**:
+
+    def load(self):
+        for module in load_modules(.../alert/source):
+            for cls in load_classes(module, AlertSource, (ThreadedAlertSource,)):
+                source = cls(self.middleware)
+                if source.name in ALERT_SOURCES:
+                    raise RuntimeError(...)
+
+So a module that raises on import takes middlewared's setup down with it. These
+tests guard the realistic ways that could happen.
+"""
+
+import ast
+import os
+import re
+
+import pytest
+
+ALERT_SRC = os.path.join(os.path.dirname(__file__), "..", "patch", "alert_source.py")
+APPLY_SH = os.path.join(os.path.dirname(__file__), "..", "patch", "apply.sh")
+
+
+def source():
+    with open(ALERT_SRC, encoding="utf-8") as fh:
+        return fh.read()
+
+
+def tree():
+    return ast.parse(source())
+
+
+class TestCannotBreakMiddlewaredAtImport:
+    def test_it_compiles(self):
+        compile(source(), "alert_source.py", "exec")
+
+    def test_apply_sh_compiles_it_before_writing_it(self):
+        # The substituted file is what middlewared imports. If it does not compile,
+        # installing it would break startup — so apply.sh must refuse to write it.
+        with open(APPLY_SH, encoding="utf-8") as fh:
+            sh = fh.read()
+        i = sh.index("alert_dst = os.path.join(mw_dir, 'alert', 'source'")
+        block = sh[i:i + 1600]
+        assert "compile(_body, alert_dst, 'exec')" in block
+        assert block.index("compile(_body") < block.index("open(alert_dst, 'w'")
+
+    def test_patch_dir_is_substituted_with_repr(self):
+        # A directory containing a quote or backslash would otherwise produce a
+        # syntax error in the installed module.
+        with open(APPLY_SH, encoding="utf-8") as fh:
+            sh = fh.read()
+        assert "_body.replace('\"@PATCH_DIR@\"', repr(_patch_dir))" in sh
+
+    @pytest.mark.parametrize("path", [
+        "/mnt/tank/patch",
+        '/mnt/we"ird/patch',      # a quote in the path
+        "/mnt/back\\slash/patch",  # a backslash
+    ])
+    def test_substituted_module_compiles_for_awkward_paths(self, path):
+        body = source().replace('"@PATCH_DIR@"', repr(path))
+        compile(body, "alert_source.py", "exec")  # must not raise
+
+    def test_no_io_at_module_import_time(self):
+        # Anything at module scope runs during alert.load(). Only imports,
+        # constants and class definitions are allowed.
+        allowed = (ast.Import, ast.ImportFrom, ast.Assign, ast.AnnAssign,
+                   ast.ClassDef, ast.FunctionDef, ast.Expr)
+        for node in tree().body:
+            assert isinstance(node, allowed), f"module-level {type(node).__name__}"
+            if isinstance(node, ast.Expr):
+                assert isinstance(node.value, ast.Constant), "only the docstring"
+
+
+class TestAlertClassNaming:
+    """middlewared's AlertClassMeta raises NameError unless the name ends in
+    'AlertClass' — at import, inside alert.load(), which has no try/except."""
+
+    def alert_classes(self):
+        return [n for n in tree().body
+                if isinstance(n, ast.ClassDef)
+                and any(getattr(b, "id", "") == "AlertClass" for b in n.bases)]
+
+    def test_there_are_alert_classes(self):
+        assert self.alert_classes()
+
+    def test_every_alert_class_name_ends_in_AlertClass(self):
+        for cls in self.alert_classes():
+            assert cls.name.endswith("AlertClass"), (
+                f"{cls.name}: AlertClassMeta raises NameError on this"
+            )
+
+    def test_every_alert_class_defines_the_required_attrs(self):
+        # category/level/title are NotImplemented on the base; a missing one shows
+        # up as a broken alert rather than an error.
+        for cls in self.alert_classes():
+            names = {t.id for n in cls.body if isinstance(n, ast.Assign)
+                     for t in n.targets if isinstance(t, ast.Name)}
+            assert {"category", "level", "title", "text"} <= names, cls.name
+
+    def test_alert_text_placeholders_match_the_args_we_pass(self):
+        src = source()
+        placeholders = set(re.findall(r"%\((\w+)\)s", src))
+        # These are the keys built in _check().
+        assert placeholders <= {"current", "latest", "summary", "dir"}
+
+
+class TestNoSysPathMutation:
+    def test_release_notes_is_loaded_by_path_not_sys_path(self):
+        # sys.path.insert(0, ...) would shadow the stdlib for this interpreter, and
+        # ThreadedAlertSource runs in a thread pool — mutating sys.path is a race.
+        #
+        # Check for actual MUTATION, not the string: the docstring legitimately
+        # mentions sys.path to explain why it is avoided.
+        src = source()
+        assert "sys.path.insert" not in src
+        assert "sys.path.append" not in src
+        assert not re.search(r"^import sys$", src, re.M), "sys is not needed"
+        assert "spec_from_file_location" in src
+
+
+class TestNeverWritesToGit:
+    def test_only_read_only_git_commands(self):
+        # A `git fetch` from middlewared (running as root) would leave root-owned
+        # objects in .git and break every later non-root git command — which is
+        # exactly the breakage this project already hit once.
+        src = source()
+        for forbidden in ("fetch", "pull", "checkout", "clone", "reset"):
+            assert f'"{forbidden}"' not in src, f"git {forbidden} writes to .git"
+        assert '"ls-remote"' in src

uninstall.sh

@@ -3,7 +3,7 @@
 
 set -euo pipefail
 
-VERSION="0.5.0"
+VERSION="0.5.1"
 
 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"
 _HOOK_COMMENT='TrueCloud provider patch (S3/B2)'

update.sh

@@ -19,7 +19,7 @@
 
 set -euo pipefail
 
-VERSION="0.5.0"
+VERSION="0.5.1"
 
 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"
 _PREV_FILE="$PATCH_DIR/.update_previous"