From 4b07ce459a28b058ed0364a5f3f7f5d50cd717ac Mon Sep 17 00:00:00 2001
From: sudolulo <holden@arch.fyi>
Date: Mon, 15 Jun 2026 02:59:57 +0000
Subject: [PATCH] Fix six audit findings: dead code, missing status records,
 missing fallback, unsafe JSON read

create_task.py:
- Remove dead make_client() call before the verify branch; it was called
  unconditionally with host=None/key=None, creating a broken client that was
  immediately discarded or overwritten.
- Remove "verify" from the dispatch dict; it was never reached through dispatch
  (the if/cmd==verify branch above it handled it). Dispatch now only contains
  commands that actually use a client.
- Wrap json.load() in try/except (OSError, JSONDecodeError) so a corrupt or
  partially-written status file produces a useful message instead of a traceback.

sitecustomize.py:
- Call _record_status() on the early-return paths in both _patch_b2 and
  _patch_restic. Without this, if TrueNAS natively supports B2 or the patch
  is already applied, the status file was never written and `verify` always
  reported failure even when everything was fine.
- Add idempotency guard to _record_status(): first call wins; duplicate calls
  for the same module are ignored so the entry count stays accurate.
- Make B2 get_restic_config a @staticmethod. The method never used self; the
  noqa comment was suppressing the evidence of a design mismatch. Removing the
  unused parameter makes the intent explicit.
- Add NamedTuple._replace() fallback after dataclasses.replace() in the restic
  wrapper. If ResticConfig is ever refactored to a NamedTuple, the TypeError
  from dataclasses.replace() would have surfaced as a backup job failure rather
  than a graceful recovery.
---
 patch/create_task.py   | 29 +++++++++++++++++------------
 patch/sitecustomize.py | 20 +++++++++++++++-----
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/patch/create_task.py b/patch/create_task.py
index f9937a8..c26379d 100755
--- a/patch/create_task.py
+++ b/patch/create_task.py
@@ -92,8 +92,14 @@ def cmd_verify(_client, _args):
         print(f"  Expected: {_STATUS_FILE}")
         sys.exit(1)
 
-    with open(_STATUS_FILE, encoding="utf-8") as fh:
-        status = json.load(fh)
+    try:
+        with open(_STATUS_FILE, encoding="utf-8") as fh:
+            status = json.load(fh)
+    except (OSError, json.JSONDecodeError) as exc:
+        print(f"Could not read status file: {exc}")
+        print(f"  Path: {_STATUS_FILE}")
+        print("Try restarting middlewared to regenerate it.")
+        sys.exit(1)
 
     print(f"Hook status (recorded at {status.get('patched_at', 'unknown')})")
     print()
@@ -225,22 +231,21 @@ def main():
                    help="Create the task in a disabled state")
 
     args = p.parse_args()
-    client = make_client(args.host, args.api_key, args.insecure)
 
+    if args.cmd == "verify":
+        cmd_verify(None, args)
+        return
+
+    if not args.host or not args.api_key:
+        p.error("--host and --api-key are required for this command")
+
+    client = make_client(args.host, args.api_key, args.insecure)
     dispatch = {
-        "verify":           cmd_verify,
         "list-credentials": cmd_list_credentials,
         "list-tasks":       cmd_list_tasks,
         "create":           cmd_create,
     }
-
-    if args.cmd == "verify":
-        cmd_verify(None, args)
-    else:
-        if not args.host or not args.api_key:
-            p.error("--host and --api-key are required for this command")
-        client = make_client(args.host, args.api_key, args.insecure)
-        dispatch[args.cmd](client, args)
+    dispatch[args.cmd](client, args)
 
 
 if __name__ == "__main__":
diff --git a/patch/sitecustomize.py b/patch/sitecustomize.py
index 7122667..220053a 100644
--- a/patch/sitecustomize.py
+++ b/patch/sitecustomize.py
@@ -121,9 +121,12 @@ def _patch_b2(module):
 
     if hasattr(cls, "get_restic_config"):
         # A future TrueNAS version already added native B2 restic support.
+        _record_status("middlewared.rclone.remote.b2", ok=True,
+                       detail="native support present; patch not needed")
         return
 
-    def get_restic_config(self, task):  # noqa: ARG001
+    @staticmethod
+    def get_restic_config(task):
         p = task["credentials"]["provider"]
         return "", {
             "B2_ACCOUNT_ID": p["account"],
@@ -138,6 +141,8 @@ def _patch_b2(module):
 
 def _patch_restic(module):
     if getattr(module.get_restic_config, "_truecloud_patched", False):
+        _record_status("middlewared.plugins.cloud_backup.restic", ok=True,
+                       detail="already patched in this process")
         return
 
     import dataclasses
@@ -163,10 +168,13 @@ def _patch_restic(module):
                 m = _broken_url.match(part)
                 if m:
                     cmd[i] = f"{m.group(1)}:{m.group(2)}"
-                    # dataclasses.replace passes all other fields through, so
-                    # new ResticConfig fields added in future TrueNAS versions
-                    # are preserved automatically.
-                    return dataclasses.replace(result, cmd=cmd)
+                    # Prefer dataclasses.replace (passes unknown future fields
+                    # through automatically). Fall back to NamedTuple._replace
+                    # in case ResticConfig is refactored.
+                    try:
+                        return dataclasses.replace(result, cmd=cmd)
+                    except TypeError:
+                        return result._replace(cmd=cmd)
                 break  # -r arg present and already correct
         return result  # URL was fine; return original unchanged
 
@@ -190,6 +198,8 @@ def _record_status(fullname: str, ok: bool, detail: str = "") -> None:
     import os
     import time
 
+    if fullname in _hook_status:
+        return  # idempotent: first call wins
     _hook_status[fullname] = {"ok": ok, "detail": detail}
     if len(_hook_status) < len(_PATCHES):
         return  # wait until all patches have reported