diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f57bba..474ce9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## v0.3.5 — 2026-07-13 + +### Changed + +- `delete_snapshot_tree` swallowed the error from its recursive-delete fast path. + That failure is *usually* just "parent already gone" — stock's `finally` winning + the race once our mounts are released, which the by-name sweep then handles. But + if the cause were anything else, this was the only place it was visible, and it + went straight to `/dev/null`. It is now logged before falling through. + +- Annotated the two remaining static-analysis findings as considered-and-accepted + rather than leaving them to be re-litigated: `subprocess` is always called in + list form (no shell, so ZFS dataset names cannot inject), and the partial + `systemctl` path is moot in a script that only runs as root. + ## v0.3.4 — 2026-07-13 ### Changed diff --git a/install.sh b/install.sh index 1a5ead3..5dcd848 100755 --- a/install.sh +++ b/install.sh @@ -18,7 +18,7 @@ set -euo pipefail -VERSION="0.3.4" +VERSION="0.3.5" # The directory containing install.sh is the permanent install location. PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" diff --git a/patch/apply.sh b/patch/apply.sh index 608cc1a..7aae4c3 100755 --- a/patch/apply.sh +++ b/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.4" +VERSION="0.3.5" # 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. diff --git a/patch/create_task.py b/patch/create_task.py index c64285d..8f99bd1 100755 --- a/patch/create_task.py +++ b/patch/create_task.py @@ -52,7 +52,7 @@ import subprocess import sys import time -__version__ = "0.3.4" +__version__ = "0.3.5" _PATCH_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) _STATUS_FILE = os.path.join(_PATCH_DIR, "hook_status.json") @@ -95,8 +95,11 @@ def midclt_call(method, *args): def _middlewared_start_epoch(): """Epoch timestamp of the running middlewared main process, or None.""" try: + # Partial path (S607) is fine here: this runs as root on TrueNAS, so an + # attacker who can poison PATH already has root. Hard-coding a path would + # be less portable (/bin vs /usr/bin) for no security gain. pid = int(subprocess.run( - ["systemctl", "show", "--property=MainPID", "--value", "middlewared"], + ["systemctl", "show", "--property=MainPID", "--value", "middlewared"], # noqa: S607 capture_output=True, text=True, timeout=10, check=True, ).stdout.strip()) if pid <= 0: diff --git a/patch/truecloud_nested.py b/patch/truecloud_nested.py index fbc5b03..18bea3a 100644 --- a/patch/truecloud_nested.py +++ b/patch/truecloud_nested.py @@ -279,7 +279,11 @@ def current_mounts_under(root, mounts_file="/proc/self/mounts"): def _run(cmd): - return subprocess.run(cmd, capture_output=True, text=True, check=False) + # List form, never shell=True: `cmd` is built from our own mount plan, so ZFS + # dataset names cannot inject. Runs as root by definition (it mounts). + return subprocess.run( # noqa: S603 + cmd, capture_output=True, text=True, check=False + ) def apply_plan(mounts, runner=_run, isdir=os.path.isdir): @@ -379,8 +383,16 @@ async def delete_snapshot_tree(middleware, snapshot, logger=None): try: await middleware.call("zfs.snapshot.delete", snapshot, {"recursive": True}) return - except Exception: # noqa: BLE001 - fall through to the explicit sweep - pass + except Exception as e: # noqa: BLE001 - fall through to the explicit sweep + # Usually just "parent already gone" (stock's finally won the race once our + # mounts were released), which the sweep below handles. Log it rather than + # swallow it: if the real cause is something else, this is the only place + # it is visible -- the sweep would report a different, downstream failure. + if logger: + logger.debug( + "truecloud-patch: recursive delete of %s failed (%r); sweeping " + "the tree by name instead", snapshot, e, + ) # The parent may already be gone -- stock's `finally` can win the race once # our mounts are released -- which fails the recursive delete while the diff --git a/recover.sh b/recover.sh index 9b4fd5f..7a25a90 100755 --- a/recover.sh +++ b/recover.sh @@ -17,7 +17,7 @@ # bash /mnt/tank/truenas-truecloud-patch/patch/apply.sh # systemctl restart middlewared -VERSION="0.3.4" +VERSION="0.3.5" PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" diff --git a/uninstall.sh b/uninstall.sh index bb5a7a7..173a256 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -3,7 +3,7 @@ set -euo pipefail -VERSION="0.3.4" +VERSION="0.3.5" PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" _HOOK_COMMENT='TrueCloud provider patch (S3/B2)'