v0.3.4: one implementation of apply/revert (patch/mw_patch.py)

The "strip the TRUECLOUD_PATCH block" logic existed twice -- in apply.sh's heredoc
and in an inline heredoc in uninstall.sh -- and the uninstall copy was the untested
one. That is precisely how the two could have drifted apart, with apply.sh
reverting one set of files and uninstall.sh another.

Both now call patch/mw_patch.py. 17 new tests cover it, including that
revert_nested never touches restic.py: that file carries a TRUECLOUD_PATCH block
too, but it belongs to the providers module, and removing it would silently break
B2 backups.

apply.sh imports it fail-safe -- on ImportError the backend patch is skipped and
middlewared starts stock, which is this script's whole design principle. The
import uses sys.path.append, never insert(0): prepending would give patch/
precedence over the stdlib for that interpreter, so a future patch/json.py would
shadow the real json module and break the boot.

Also: the README's create_task.py example still taught `--password <secret>`, which
is how a security fix quietly fails to land. It now shows --password-stdin.

132 tests, ruff and shellcheck clean.

CHANGELOG.md

@@ -1,5 +1,29 @@
 # Changelog
 
+## v0.3.4 — 2026-07-13
+
+### Changed
+
+- **One implementation of apply/revert (`patch/mw_patch.py`).** The "strip the
+  `TRUECLOUD_PATCH` block" logic existed twice — in `apply.sh`'s heredoc and in an
+  inline heredoc in `uninstall.sh` — and the uninstall copy was the untested one.
+  That is exactly how the two could have drifted apart, with `apply.sh` reverting
+  one set of files and `uninstall.sh` another. Both now call the same tested
+  module (17 new tests, including that `revert_nested` never touches `restic.py`,
+  which belongs to the providers module and whose removal would silently break B2
+  backups).
+
+  `apply.sh` imports it fail-safe: if it cannot, the backend patch is skipped and
+  middlewared starts stock, which is this script's whole design principle. The
+  import uses `sys.path.append`, never `insert(0)` — prepending would give
+  `patch/` precedence over the stdlib for that interpreter, so a future
+  `patch/json.py` would shadow the real `json` and break the boot.
+
+### Docs
+
+- The README's `create_task.py` example still taught `--password <secret>`, which
+  is how a security fix quietly fails to land. It now shows `--password-stdin`.
+
 ## v0.3.3 — 2026-07-13
 
 ### Security

README.md

@@ -376,18 +376,25 @@ host address or API key:
 # List your cloud credentials to find the right ID
 python3 /mnt/tank/truenas-truecloud-patch/patch/create_task.py list-credentials
 
-# Create a task with a B2 credential (id=3)
+# Create a task with a B2 credential (id=3).
+# The restic repo password is read from stdin, so it never lands in your shell
+# history — nor in any process's argv, where `ps` would expose it.
+printf '%s' 'restic-repo-password' | \
 python3 /mnt/tank/truenas-truecloud-patch/patch/create_task.py create \
     --name "tank-to-b2" \
     --path /mnt/tank/data \
     --credential 3 \
     --bucket my-bucket \
     --folder backups/tank \
-    --password "restic-repo-password" \
+    --password-stdin \
     --cache-path /mnt/tank/.restic-cache \
     --keep-last 14
 ```
 
+Omit `--password-stdin` and you'll be prompted for the password instead. `--password
+<secret>` still works but warns: that password is the encryption key for the whole
+repository, and a CLI argument persists in your shell history forever.
+
 > **Always pass `--cache-path`.** Without it TrueNAS runs restic with `--no-cache`,
 > which re-fetches all repo metadata from the provider every run — glacially slow
 > on large repos. Point it at a writable dir on a pool with free space.

install.sh

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

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.3.3"
+VERSION="0.3.4"
 
 # 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.
@@ -468,65 +468,24 @@ if _tc_nested is not None:
 """
 
 
-def patch_file(path, block):
-    with open(path, encoding="utf-8") as fh:
-        content = fh.read()
-    marker = "\n# TRUECLOUD_PATCH"
-    idx = content.find(marker)
-    base = content[:idx] if idx != -1 else content
-    with open(path, "w", encoding="utf-8") as fh:
-        fh.write(base.rstrip("\n") + "\n" + block)
+# Single implementation of the block apply/revert logic (patch/mw_patch.py), so
+# uninstall.sh and apply.sh cannot drift apart. Fail-safe: if it cannot be
+# imported, skip the backend patch entirely -- middlewared then starts stock,
+# which is the whole design principle of this script.
+# APPEND, never insert(0): this dir would otherwise take precedence over the
+# stdlib for this interpreter, so a future patch/json.py (say) would shadow the
+# real json module and break the boot. Appending fails safe -- worst case our
+# import misses and the backend patch is skipped.
+sys.path.append(os.path.dirname(nested_src))
+try:
+    from mw_patch import patch_file, revert_nested
+except ImportError as _e:
+    print(f'WARNING: cannot import patch/mw_patch.py ({_e}) — skipping backend patch.')
+    print('WARNING: middlewared will start with stock (unpatched) modules.')
+    sys.exit(1)
 
-
-def unpatch_file(path):
-    """Strip our appended block, restoring the stock file. True if it was patched."""
-    try:
-        with open(path, encoding="utf-8") as fh:
-            content = fh.read()
-    except OSError:
-        return False
-    idx = content.find("\n# TRUECLOUD_PATCH")
-    if idx == -1:
-        return False
-    try:
-        with open(path, "w", encoding="utf-8") as fh:
-            fh.write(content[:idx].rstrip("\n") + "\n")
-    except OSError:
-        return False
-    return True
-
-
-def revert_nested(cloud_dir, sync_path):
-    """Undo the nested patch. Returns the names of what was actually reverted.
-
-    Skipping the patch is NOT enough to disable the feature. The overlay persists
-    for the whole boot, so an earlier run this boot may already have written the
-    patched files -- and middlewared re-imports them on the restart that
-    install.sh performs. Without this, `install.sh --disable-nested-snapshots`
-    would report "disabled" while the feature kept running until the next reboot.
-    """
-    reverted = []
-
-    # Remove the module FIRST. Every injected block is guarded by
-    # `if _tc_nested is not None`, so once it is gone they all no-op even if a
-    # later step here fails -- the guard is restored no matter what.
-    try:
-        os.unlink(os.path.join(cloud_dir, '_truecloud_nested.py'))
-        reverted.append('_truecloud_nested.py')
-    except OSError:
-        pass
-
-    # NB: restic.py also carries a TRUECLOUD_PATCH block, but that belongs to the
-    # providers module. Only these three are ours to revert.
-    for name, path in (
-        ('crud.py', os.path.join(cloud_dir, 'crud.py')),
-        ('sync.py', sync_path),
-        ('snapshot.py', os.path.join(cloud_dir, 'snapshot.py')),
-    ):
-        if unpatch_file(path):
-            reverted.append(name)
-
-    return reverted
+# .../middlewared/plugins/cloud -> .../middlewared
+mw_dir = os.path.dirname(os.path.dirname(cloud_dir))
 
 b2_ok = restic_ok = False
 nested_ok = False
@@ -582,7 +541,7 @@ if not nested_needed:
         nested_detail = 'not needed'
         print('INFO: Nested module skipped.')
 
-    reverted = revert_nested(cloud_dir, sync_path)
+    reverted = revert_nested(mw_dir)
     if reverted:
         print('OK: Reverted a previously-applied nested patch (' + ', '.join(reverted) + ').')
         print('    The stock nesting guard is restored once middlewared restarts.')

patch/create_task.py

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

patch/mw_patch.py

@@ -0,0 +1,137 @@
+#!/usr/bin/env python3
+"""Apply and revert truecloud-patch's blocks in middlewared's modules.
+
+Every patch this project makes to a middlewared module is an appended block that
+begins with the MARKER line. That makes patching idempotent (truncate at the
+marker, re-append) and reverting exact (truncate at the marker, stop).
+
+This is the single implementation of that. It used to live in two places --
+apply.sh's heredoc and an inline heredoc in uninstall.sh -- and the uninstall copy
+was the untested one.
+
+    python3 mw_patch.py revert-all       # remove every block + the nested module
+    python3 mw_patch.py revert-nested    # remove only the nested module's blocks
+"""
+
+from __future__ import annotations
+
+import os
+import sys
+
+MARKER = "\n# TRUECLOUD_PATCH"
+
+#: Modules the providers module (B2/S3) patches.
+PROVIDER_RELPATHS = [
+    ("rclone", "remote", "b2.py"),
+    ("plugins", "cloud_backup", "restic.py"),
+]
+
+#: Modules the nested-snapshot module patches. Order matters on revert -- see
+#: revert(): the loadable module goes first.
+NESTED_RELPATHS = [
+    ("plugins", "cloud", "crud.py"),
+    ("plugins", "cloud_backup", "sync.py"),
+    ("plugins", "cloud", "snapshot.py"),
+]
+
+#: The importable module the nested blocks depend on.
+NESTED_MODULE = ("plugins", "cloud", "_truecloud_nested.py")
+
+
+def patch_file(path, block):
+    """Append `block`, replacing any block we appended before. Idempotent."""
+    with open(path, encoding="utf-8") as fh:
+        content = fh.read()
+    idx = content.find(MARKER)
+    base = content[:idx] if idx != -1 else content
+    with open(path, "w", encoding="utf-8") as fh:
+        fh.write(base.rstrip("\n") + "\n" + block)
+
+
+def unpatch_file(path):
+    """Strip our appended block, restoring the stock file. True if it was patched."""
+    try:
+        with open(path, encoding="utf-8") as fh:
+            content = fh.read()
+    except OSError:
+        return False
+    idx = content.find(MARKER)
+    if idx == -1:
+        return False
+    try:
+        with open(path, "w", encoding="utf-8") as fh:
+            fh.write(content[:idx].rstrip("\n") + "\n")
+    except OSError:
+        return False
+    return True
+
+
+def revert(mw_dir, relpaths, module_relpath=None):
+    """Remove our blocks from `relpaths`, and the module at `module_relpath`.
+
+    The module is deleted FIRST. Every injected block is guarded by
+    `if _tc_nested is not None`, so once the module is gone the blocks all no-op
+    even if a later unpatch fails -- the stock guard comes back regardless.
+
+    Returns the names of what was actually reverted.
+    """
+    reverted = []
+
+    if module_relpath:
+        try:
+            os.unlink(os.path.join(mw_dir, *module_relpath))
+            reverted.append(module_relpath[-1])
+        except OSError:
+            pass
+
+    for rel in relpaths:
+        if unpatch_file(os.path.join(mw_dir, *rel)):
+            reverted.append(rel[-1])
+
+    return reverted
+
+
+def revert_nested(mw_dir):
+    """Undo the nested-snapshot patch only. Leaves the providers patch alone.
+
+    restic.py also carries a block, but it belongs to the providers module --
+    reverting it would silently break B2 backups.
+    """
+    return revert(mw_dir, NESTED_RELPATHS, NESTED_MODULE)
+
+
+def revert_all(mw_dir):
+    """Undo every patch this project applies."""
+    return revert(mw_dir, NESTED_RELPATHS + PROVIDER_RELPATHS, NESTED_MODULE)
+
+
+def find_middlewared_dir():
+    """Directory of the installed `middlewared` package, or None."""
+    try:
+        import middlewared
+    except ImportError:
+        return None
+    return os.path.dirname(os.path.abspath(middlewared.__file__))
+
+
+def main(argv):
+    if len(argv) < 2 or argv[1] not in ("revert-all", "revert-nested"):
+        print(__doc__, file=sys.stderr)
+        return 2
+
+    mw_dir = find_middlewared_dir()
+    if mw_dir is None:
+        print("  middlewared not importable — nothing to revert.")
+        return 0
+
+    fn = revert_all if argv[1] == "revert-all" else revert_nested
+    reverted = fn(mw_dir)
+    if reverted:
+        print("  Reverted: " + ", ".join(reverted))
+    else:
+        print("  Nothing to revert (overlay already removed, or never patched).")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv))

recover.sh

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

tests/test_apply_blocks.py

@@ -277,42 +277,22 @@ class TestOptIn:
         running until the next reboot.
         """
         src = heredoc_source()
-        assert "def unpatch_file(" in src
-        assert "def revert_nested(" in src
-        # The revert must run on every not-needed path (opt-out, superseded).
+        # The implementation lives in patch/mw_patch.py (see test_mw_patch.py);
+        # apply.sh must import and actually call it.
+        assert "from mw_patch import patch_file, revert_nested" in src
         gate = src.index("if not nested_needed:")
         revert = src.index("reverted = revert_nested(")
         patch = src.index("patch_file(crud_py, CRUD_BLOCK)")
         assert gate < revert < patch, "revert belongs in the not-needed branch"
 
-    def test_revert_removes_the_module_before_unpatching_files(self):
-        # Every injected block is guarded by `if _tc_nested is not None`, so
-        # deleting the module first means the guard is restored even if a later
-        # unpatch step fails.
+    def test_import_failure_skips_the_patch_rather_than_crashing(self):
+        # apply.sh runs at PREINIT. If mw_patch.py cannot be imported it must
+        # degrade to "middlewared starts stock", never take the boot down.
         src = heredoc_source()
-        body = src[src.index("def revert_nested("):src.index("def patch_file(") if
-                   src.index("def patch_file(") > src.index("def revert_nested(") else len(src)]
-        body = src[src.index("def revert_nested("):]
-        body = body[:body.index("\n\n\n")] if "\n\n\n" in body else body
-        assert body.index("_truecloud_nested.py") < body.index("crud.py")
-
-    def test_revert_never_touches_the_providers_patch(self):
-        # restic.py also carries a TRUECLOUD_PATCH block, but it belongs to the
-        # providers module. Reverting it would silently break B2 backups.
-        src = heredoc_source()
-        body = src[src.index("def revert_nested("):]
-        body = body[:body.index("return reverted")]
-        # Comments legitimately *mention* restic.py to explain why it is excluded;
-        # what matters is that no code line touches it.
-        code = "\n".join(
-            ln for ln in body.splitlines() if not ln.lstrip().startswith("#")
-        )
-        assert "restic" not in code
-        assert "b2.py" not in code
-        # It must only ever revert these three, plus the module itself.
-        assert "crud.py" in code
-        assert "sync_path" in code
-        assert "snapshot.py" in code
+        i = src.index("from mw_patch import")
+        tail = src[i:i + 400]
+        assert "except ImportError" in tail
+        assert "skipping backend patch" in tail
 
 
 def test_guard_is_relaxed_only_after_traversal_is_installed():

tests/test_mw_patch.py

@@ -0,0 +1,160 @@
+"""Tests for mw_patch — the single implementation of apply/revert.
+
+apply.sh and uninstall.sh both go through this. It used to be duplicated in an
+untested shell heredoc, which is exactly how the two could have drifted apart:
+apply.sh reverting one set of files and uninstall.sh another.
+"""
+
+import os
+import sys
+
+import pytest
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "patch"))
+
+from mw_patch import (  # noqa: E402
+    MARKER,
+    NESTED_MODULE,
+    NESTED_RELPATHS,
+    PROVIDER_RELPATHS,
+    patch_file,
+    revert_all,
+    revert_nested,
+    unpatch_file,
+)
+
+STOCK = "import os\n\n\ndef stock():\n    return 1\n"
+BLOCK = "\n# TRUECLOUD_PATCH\ninjected = 1\n"
+
+
+def build_mw(root):
+    """A fake middlewared tree with every file this project touches."""
+    mw = os.path.join(root, "middlewared")
+    for rel in NESTED_RELPATHS + PROVIDER_RELPATHS:
+        path = os.path.join(mw, *rel)
+        os.makedirs(os.path.dirname(path), exist_ok=True)
+        with open(path, "w", encoding="utf-8") as fh:
+            fh.write(STOCK)
+    with open(os.path.join(mw, *NESTED_MODULE), "w", encoding="utf-8") as fh:
+        fh.write("# module\n")
+    return mw
+
+
+def read(mw, rel):
+    with open(os.path.join(mw, *rel), encoding="utf-8") as fh:
+        return fh.read()
+
+
+class TestPatchFile:
+    def test_appends_the_block(self, tmp_path):
+        p = tmp_path / "m.py"
+        p.write_text(STOCK)
+        patch_file(str(p), BLOCK)
+        assert MARKER in p.read_text()
+        assert p.read_text().startswith("import os")
+
+    def test_is_idempotent(self, tmp_path):
+        # apply.sh runs on EVERY boot. Without truncate-then-append, repeated runs
+        # would stack duplicate copies of the block into a middlewared module.
+        p = tmp_path / "m.py"
+        p.write_text(STOCK)
+        for _ in range(5):
+            patch_file(str(p), BLOCK)
+        assert p.read_text().count("# TRUECLOUD_PATCH") == 1
+        assert p.read_text().count("injected = 1") == 1
+
+    def test_round_trips_back_to_stock(self, tmp_path):
+        p = tmp_path / "m.py"
+        p.write_text(STOCK)
+        patch_file(str(p), BLOCK)
+        assert unpatch_file(str(p)) is True
+        assert p.read_text() == STOCK
+
+
+class TestUnpatchFile:
+    def test_returns_false_on_an_unpatched_file(self, tmp_path):
+        p = tmp_path / "m.py"
+        p.write_text(STOCK)
+        assert unpatch_file(str(p)) is False
+        assert p.read_text() == STOCK
+
+    def test_returns_false_on_a_missing_file(self, tmp_path):
+        assert unpatch_file(str(tmp_path / "nope.py")) is False
+
+
+class TestRevertNested:
+    def test_reverts_only_the_nested_files(self, tmp_path):
+        mw = build_mw(str(tmp_path))
+        for rel in NESTED_RELPATHS + PROVIDER_RELPATHS:
+            patch_file(os.path.join(mw, *rel), BLOCK)
+
+        reverted = revert_nested(mw)
+
+        for rel in NESTED_RELPATHS:
+            assert read(mw, rel) == STOCK, f"{rel[-1]} should be stock"
+            assert rel[-1] in reverted
+
+    def test_never_touches_the_providers_patch(self, tmp_path):
+        # restic.py carries a TRUECLOUD_PATCH block too, but it belongs to the
+        # providers module. Reverting it would silently break B2 backups.
+        mw = build_mw(str(tmp_path))
+        for rel in NESTED_RELPATHS + PROVIDER_RELPATHS:
+            patch_file(os.path.join(mw, *rel), BLOCK)
+
+        revert_nested(mw)
+
+        for rel in PROVIDER_RELPATHS:
+            assert MARKER in read(mw, rel), f"{rel[-1]} must keep its providers block"
+
+    def test_removes_the_module(self, tmp_path):
+        mw = build_mw(str(tmp_path))
+        assert os.path.exists(os.path.join(mw, *NESTED_MODULE))
+        reverted = revert_nested(mw)
+        assert not os.path.exists(os.path.join(mw, *NESTED_MODULE))
+        assert "_truecloud_nested.py" in reverted
+
+    def test_module_is_removed_before_the_files_are_unpatched(self, tmp_path):
+        # Every injected block is guarded by `if _tc_nested is not None`, so once
+        # the module is gone they all no-op — the stock guard is restored even if
+        # a later unpatch fails.
+        mw = build_mw(str(tmp_path))
+        for rel in NESTED_RELPATHS:
+            patch_file(os.path.join(mw, *rel), BLOCK)
+        reverted = revert_nested(mw)
+        assert reverted[0] == "_truecloud_nested.py"
+
+    def test_is_idempotent(self, tmp_path):
+        mw = build_mw(str(tmp_path))
+        for rel in NESTED_RELPATHS:
+            patch_file(os.path.join(mw, *rel), BLOCK)
+        revert_nested(mw)
+        assert revert_nested(mw) == []
+
+
+class TestRevertAll:
+    def test_reverts_providers_and_nested(self, tmp_path):
+        mw = build_mw(str(tmp_path))
+        for rel in NESTED_RELPATHS + PROVIDER_RELPATHS:
+            patch_file(os.path.join(mw, *rel), BLOCK)
+
+        revert_all(mw)
+
+        for rel in NESTED_RELPATHS + PROVIDER_RELPATHS:
+            assert read(mw, rel) == STOCK, f"{rel[-1]} should be stock"
+        assert not os.path.exists(os.path.join(mw, *NESTED_MODULE))
+
+    def test_is_a_noop_on_a_stock_tree(self, tmp_path):
+        mw = build_mw(str(tmp_path))
+        os.unlink(os.path.join(mw, *NESTED_MODULE))
+        assert revert_all(mw) == []
+
+
+class TestTargetsAreDisjoint:
+    def test_no_file_is_in_both_module_lists(self):
+        # If restic.py ever appeared in NESTED_RELPATHS, revert_nested would break
+        # B2 backups.
+        assert not set(NESTED_RELPATHS) & set(PROVIDER_RELPATHS)
+
+    @pytest.mark.parametrize("rel", PROVIDER_RELPATHS)
+    def test_provider_targets_are_not_nested_targets(self, rel):
+        assert rel not in NESTED_RELPATHS

uninstall.sh

@@ -3,7 +3,7 @@
 
 set -euo pipefail
 
-VERSION="0.3.3"
+VERSION="0.3.4"
 
 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"
 _HOOK_COMMENT='TrueCloud provider patch (S3/B2)'
@@ -98,54 +98,11 @@ echo ""
 # would then remove the boot hook and report success while leaving every patch
 # applied. Strip our appended blocks explicitly.
 
+# Same implementation apply.sh uses (patch/mw_patch.py) — a second shell copy of
+# this would be the untested one.
 echo "Reverting any file-level patches ..."
-python3 - <<'PYEOF'
-import os
-
-try:
-    import middlewared
-except ImportError:
-    print("  middlewared not importable — nothing to revert.")
-    raise SystemExit(0)
-
-mw = os.path.dirname(os.path.abspath(middlewared.__file__))
-targets = [
-    os.path.join(mw, "rclone", "remote", "b2.py"),
-    os.path.join(mw, "plugins", "cloud_backup", "restic.py"),
-    os.path.join(mw, "plugins", "cloud_backup", "sync.py"),
-    os.path.join(mw, "plugins", "cloud", "crud.py"),
-    os.path.join(mw, "plugins", "cloud", "snapshot.py"),
-]
-
-reverted = []
-for path in targets:
-    try:
-        with open(path, encoding="utf-8") as fh:
-            content = fh.read()
-    except OSError:
-        continue
-    idx = content.find("\n# TRUECLOUD_PATCH")
-    if idx == -1:
-        continue
-    try:
-        with open(path, "w", encoding="utf-8") as fh:
-            fh.write(content[:idx].rstrip("\n") + "\n")
-        reverted.append(os.path.basename(path))
-    except OSError as e:
-        print(f"  WARNING: could not revert {path}: {e}")
-
-nested = os.path.join(mw, "plugins", "cloud", "_truecloud_nested.py")
-try:
-    os.unlink(nested)
-    reverted.append("_truecloud_nested.py")
-except OSError:
-    pass
-
-if reverted:
-    print("  Reverted: " + ", ".join(reverted))
-else:
-    print("  Nothing to revert (overlay already removed, or never patched).")
-PYEOF
+python3 "$PATCH_DIR/patch/mw_patch.py" revert-all || \
+    echo "  WARNING: could not revert file-level patches."
 echo ""
 
 # ── Unmount nested-snapshot staging trees ─────────────────────────────────────