From 533fdc3678ad75838afc9e8c39df3a6bf0c82cd8 Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Fri, 24 Apr 2026 10:26:23 -0400 Subject: [PATCH 1/6] fix: remove trailing comma causing tuple instead of string assignment Fixed volume["name"] assignments in lvm.py and validate.py where a trailing comma was creating a tuple ('snapshot : vg/lv',) instead of a string "snapshot : vg/lv". Signed-off-by: Todd Gill --- module_utils/snapshot_lsr/lvm.py | 2 +- module_utils/snapshot_lsr/validate.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/module_utils/snapshot_lsr/lvm.py b/module_utils/snapshot_lsr/lvm.py index 8f904a15..0d2b2267 100644 --- a/module_utils/snapshot_lsr/lvm.py +++ b/module_utils/snapshot_lsr/lvm.py @@ -129,7 +129,7 @@ def get_json_from_args(module, module_args, vg_include): ) continue volume = {} - volume["name"] = ("snapshot : " + vg_str + "/" + lv["lv_name"],) + volume["name"] = "snapshot : " + vg_str + "/" + lv["lv_name"] volume["vg"] = vg_str volume["lv"] = lv["lv_name"] diff --git a/module_utils/snapshot_lsr/validate.py b/module_utils/snapshot_lsr/validate.py index e7811bcf..6d490002 100644 --- a/module_utils/snapshot_lsr/validate.py +++ b/module_utils/snapshot_lsr/validate.py @@ -79,7 +79,7 @@ def get_json_from_args(module, module_args, vg_include): ) continue volume = {} - volume["name"] = ("snapshot : " + vg_str + "/" + lv["lv_name"],) + volume["name"] = "snapshot : " + vg_str + "/" + lv["lv_name"] volume["vg"] = vg_str volume["lv"] = lv["lv_name"] From a18aa52b821829aec3a7b034b0e761995c853f22 Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Fri, 24 Apr 2026 10:27:51 -0400 Subject: [PATCH 2/6] fix: correct return statement syntax in snapmgr check_mode paths Fixed three locations where return statements were creating sets instead of dicts due to missing key-value syntax. All check_mode returns now properly return {"return_code": rc, "errors": message, "changed": False}. Also initialized rc variable at the start of mgr_snapshot_cmd to ensure it's defined for all code paths. Signed-off-by: Todd Gill --- module_utils/snapshot_lsr/snapmgr.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/module_utils/snapshot_lsr/snapmgr.py b/module_utils/snapshot_lsr/snapmgr.py index 2524c69f..375ea522 100644 --- a/module_utils/snapshot_lsr/snapmgr.py +++ b/module_utils/snapshot_lsr/snapmgr.py @@ -206,6 +206,7 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): logger.info("mgr_snapshot_cmd: %s", snapset_name) changed = False message = "" + rc = SnapshotStatus.SNAPSHOT_OK check_mode = module_args["ansible_check_mode"] rc, message = verify_snapset_source_lvs_exist(module, snapset_json) @@ -240,7 +241,11 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): source_list = mgr_get_source_list_for_create(volume_list) if check_mode: - return {rc, "Would call function manager.create_snapshot_set()", False} + return { + "return_code": rc, + "errors": "Would call function manager.create_snapshot_set()", + "changed": False, + } manager = snap_manager.Manager() @@ -437,9 +442,10 @@ def mgr_extend_cmd(module, module_args, snapset_json): if check_mode: return { - rc, - "Would run function " - + " manager.resize_snapshot_set() with ".join(source_list), + "return_code": rc, + "errors": "Would run function manager.resize_snapshot_set() with " + + ", ".join(source_list), + "changed": False, } # there are no LVs that require an extend operation, return OK. @@ -472,9 +478,10 @@ def mgr_revert_cmd(module_args, snapset_json): if check_mode: return { - rc, - "Would run function " - + " manager.revert_snapshot_set with ".join(snapset_name), + "return_code": rc, + "errors": "Would run function manager.revert_snapshot_set with " + + snapset_name, + "changed": False, } manager = snap_manager.Manager() From bbe5a6e268cbfc31226b865143700b7e521e610b Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Fri, 24 Apr 2026 10:28:22 -0400 Subject: [PATCH 3/6] fix: close file descriptor to prevent resource leak The can_open() function was opening file descriptors with os.open() but never closing them, causing a resource leak. Now properly closes the file descriptor after verifying exclusive access is possible. Signed-off-by: Todd Gill --- tests/library/find_unused_disk.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/library/find_unused_disk.py b/tests/library/find_unused_disk.py index a87bd22a..4f8c90e5 100644 --- a/tests/library/find_unused_disk.py +++ b/tests/library/find_unused_disk.py @@ -117,7 +117,8 @@ def no_holders(disk_path): def can_open(disk_path): """Return true if the device can be opened with exclusive access.""" try: - os.open(disk_path, os.O_EXCL) + fd = os.open(disk_path, os.O_EXCL) + os.close(fd) return True except OSError: return False From 90d0ea77242f155feaeb712a603bbf70c0a5c494 Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Fri, 24 Apr 2026 10:29:35 -0400 Subject: [PATCH 4/6] fix: add null check for snapshot_lvm_snapset_name before endswith() Added null checks in lvm.py and validate.py to prevent AttributeError when snapshot_lvm_snapset_name is None. The code now verifies the value exists before calling .endswith() on it. Signed-off-by: Todd Gill --- module_utils/snapshot_lsr/lvm.py | 4 +++- module_utils/snapshot_lsr/validate.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/module_utils/snapshot_lsr/lvm.py b/module_utils/snapshot_lsr/lvm.py index 0d2b2267..092a4349 100644 --- a/module_utils/snapshot_lsr/lvm.py +++ b/module_utils/snapshot_lsr/lvm.py @@ -96,7 +96,9 @@ def get_json_from_args(module, module_args, vg_include): vg_str = vg["vg_name"] for lv in lv_list: - if lv["lv_name"].endswith(module_args["snapshot_lvm_snapset_name"]): + if module_args["snapshot_lvm_snapset_name"] and lv["lv_name"].endswith( + module_args["snapshot_lvm_snapset_name"] + ): logger.info( "get_json_from_args: already a snapshot for %s", lv["lv_name"] ) diff --git a/module_utils/snapshot_lsr/validate.py b/module_utils/snapshot_lsr/validate.py index 6d490002..b72defac 100644 --- a/module_utils/snapshot_lsr/validate.py +++ b/module_utils/snapshot_lsr/validate.py @@ -46,7 +46,9 @@ def get_json_from_args(module, module_args, vg_include): vg_str = vg["vg_name"] for lv in lv_list: - if lv["lv_name"].endswith(module_args["snapshot_lvm_snapset_name"]): + if module_args["snapshot_lvm_snapset_name"] and lv["lv_name"].endswith( + module_args["snapshot_lvm_snapset_name"] + ): logger.info( "get_json_from_args: already a snapshot for %s", lv["lv_name"] ) From cfe1062aa14fe248b9759c705bbbad46b88cb1b4 Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Fri, 24 Apr 2026 10:30:22 -0400 Subject: [PATCH 5/6] fix: correct typos in user-facing error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed three typos in snapmgr.py error messages: - "emmpty" → "empty" - "snnapshot" → "snapshot" (2 occurrences) Also improved formatting by adding space after colon in error message. Signed-off-by: Todd Gill --- module_utils/snapshot_lsr/snapmgr.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/module_utils/snapshot_lsr/snapmgr.py b/module_utils/snapshot_lsr/snapmgr.py index 375ea522..b4445a41 100644 --- a/module_utils/snapshot_lsr/snapmgr.py +++ b/module_utils/snapshot_lsr/snapmgr.py @@ -450,7 +450,7 @@ def mgr_extend_cmd(module, module_args, snapset_json): # there are no LVs that require an extend operation, return OK. if len(source_list) == 0: - return {"return_code": rc, "errors": "source list emmpty", "changed": changed} + return {"return_code": rc, "errors": "source list empty", "changed": changed} try: manager.resize_snapshot_set(source_list, snapset_name) @@ -519,7 +519,7 @@ def mgr_mount_cmd(module, module_args, snapset_json): if snapshot_set is None or len(snapshot_set) == 0: return { "return_code": SnapshotStatus.ERROR_MOUNT_FAILED, - "errors": "snnapshot not found:" + snapset_name, + "errors": "snapshot not found: " + snapset_name, "changed": False, } @@ -552,7 +552,7 @@ def mgr_umount_cmd(module, module_args, snapset_json): if snapshot_set is None or len(snapshot_set) == 0: return { "return_code": SnapshotStatus.ERROR_MOUNT_FAILED, - "errors": "snnapshot not found:" + snapset_name, + "errors": "snapshot not found: " + snapset_name, "changed": False, } From 419755c7cae0694e0ed8d613e4e868550a8e36cd Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Wed, 6 May 2026 10:39:52 -0400 Subject: [PATCH 6/6] fix: remove redundant variable initialization flagged by CodeQL Removed dead store of rc variable in mgr_snapshot_cmd(). The variable was initialized to SnapshotStatus.SNAPSHOT_OK but immediately overwritten by verify_snapset_source_lvs_exist() before ever being used. In the check_mode path, explicitly use SnapshotStatus.SNAPSHOT_OK instead of the rc variable for clarity, since we know rc must be SNAPSHOT_OK at that point (or we would have returned early on line 213-214). This fixes CodeQL alert py/multiple-definition: 'This assignment to rc is unnecessary as it is redefined before this value is used.' Addresses: https://github.com/linux-system-roles/snapshot/security/code-scanning/95 Signed-off-by: Todd Gill --- module_utils/snapshot_lsr/snapmgr.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/module_utils/snapshot_lsr/snapmgr.py b/module_utils/snapshot_lsr/snapmgr.py index b4445a41..9d5823e2 100644 --- a/module_utils/snapshot_lsr/snapmgr.py +++ b/module_utils/snapshot_lsr/snapmgr.py @@ -206,7 +206,6 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): logger.info("mgr_snapshot_cmd: %s", snapset_name) changed = False message = "" - rc = SnapshotStatus.SNAPSHOT_OK check_mode = module_args["ansible_check_mode"] rc, message = verify_snapset_source_lvs_exist(module, snapset_json) @@ -242,7 +241,7 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): if check_mode: return { - "return_code": rc, + "return_code": SnapshotStatus.SNAPSHOT_OK, "errors": "Would call function manager.create_snapshot_set()", "changed": False, }