diff --git a/__init__.py b/__init__.py index f04a20e..f030f9d 100644 --- a/__init__.py +++ b/__init__.py @@ -1,5 +1,6 @@ __version__ = "1.2.30" +# 1.2.30 hotfix: Samba/CIFS Apply detaches only changed targets and preserves unchanged busy mounts # 1.2.30: SFTP resolves managed mountpoint templates and disabled per-user mountpoints before Apply # 1.2.29: c_menu flushes each rendered ANSI frame before blocking for keyboard input # 1.2.28: Samba/CIFS batch failures expose their concrete exception; non-empty managed targets use a typed error with target path diff --git a/sftp/AGENTS.md b/sftp/AGENTS.md index c82bc88..957ea58 100644 --- a/sftp/AGENTS.md +++ b/sftp/AGENTS.md @@ -14,6 +14,6 @@ - `sambaPoint.ensureMountpoint()` must re-raise its original failure and must never continue with an uninitialized `cifs_path`. - Parser Apply is a synchronization operation: mountpoints removed from config, moved to another real path, or changed between bind and Samba must be removed before desired mountpoints are ensured. - SSHD config must use the real user home returned by the system and must create `.ssh` there instead of falling back to `/home/`. -- Samba/CIFS Apply is a transaction: configuration changes may be queued, but no physical CIFS unmount or mountpoint-directory cleanup may occur while a batch is active. At finalization, read the final managed CIFS set from `/etc/fstab`, unmount all managed loopback CIFS mounts first, remove only obsolete queued directories that are not targets in that final set, verify every final unmounted target is empty and secure it as root-owned/non-writable, then reload Samba, close only the affected managed `sftp_mount_*` service connections with `smbcontrol smbd close-share` (fallback to full smbd restart if targeted close fails), run exactly one `systemctl daemon-reload`, and remount every managed CIFS entry still present in `/etc/fstab`. Samba reload alone does not change parameters of established service connections, so the targeted close is required for live RO/RW changes. This preserves a target removed and recreated in the same batch (for example RO/RW or real-path changes) without disconnecting unrelated Samba shares. +- Samba/CIFS Apply is a transaction: configuration changes may be queued, but no physical CIFS unmount or mountpoint-directory cleanup may occur while a batch is active. At finalization, derive the affected targets from queued remove/recreate targets, newly mounted shares and queued share closes, then unmount only those affected managed loopback CIFS mounts. Unchanged mounted targets must stay connected, even when Samba is reloaded, so an unrelated busy CWD cannot fail Apply. Read the final managed CIFS set from `/etc/fstab`, remove only obsolete queued directories that are not targets in that final set, verify every affected final unmounted target is empty and secure it as root-owned/non-writable, then reload Samba, close only the affected managed `sftp_mount_*` service connections with `smbcontrol smbd close-share` (fallback to full smbd restart if targeted close fails), run exactly one `systemctl daemon-reload`, and remount only affected CIFS entries still present in `/etc/fstab`. Samba reload alone does not change parameters of established service connections, so the targeted close is required for live RO/RW changes. This preserves a target removed and recreated in the same batch (for example RO/RW or real-path changes) without disconnecting unrelated Samba shares or unchanged SFTP mountpoints. - Never mount a managed CIFS share over a non-empty underlying target: Linux would hide that data under the mount. During user deletion, recursive jail cleanup is allowed only after explicit confirmation and after proving from `/proc/self/mountinfo` that neither the jail nor any descendant is still mounted; do not rely on `os.path.ismount()` for this destructive guard because same-filesystem bind mounts are not reliably detected. If mountinfo cannot be read, fail closed and preserve the jail. - Never run `daemon-reload` while a managed loopback CIFS mount is reconnecting after an `smbd` restart; `systemd-fstab-generator` may block in the CIFS kernel client for about 90 seconds. diff --git a/sftp/sambaPoint.py b/sftp/sambaPoint.py index d52caa7..f9f1c3e 100644 --- a/sftp/sambaPoint.py +++ b/sftp/sambaPoint.py @@ -536,6 +536,13 @@ def _isManagedCIFSSource(source:str)->bool: or source.startswith("//localhost/sftp_mount_") ) + @staticmethod + def _managedCIFSShareName(source:str)->str|None: + """Vrátí jméno spravovaného share ze zdrojové CIFS cesty.""" + if not smbHelp._isManagedCIFSSource(source): + return None + return source.rstrip("/").rsplit("/", 1)[-1] + @staticmethod def getMountedManagedCIFS()->list[tuple[str, str]]: """Vrátí (source, target) všech aktivních spravovaných loopback CIFS mountů.""" @@ -579,9 +586,38 @@ def getConfiguredManagedCIFS()->list[tuple[str, str]]: return mounts @staticmethod - def unmountAllManagedCIFS()->None: - """Odmountuje všechny aktivní spravované CIFS mounty před změnou Samba služby.""" - for source, target in reversed(smbHelp.getMountedManagedCIFS()): + def getBatchAffectedCIFSTargets( + mounted_mounts:list[tuple[str, str]], + configured_mounts:list[tuple[str, str]] + )->set[str]: + """Odvodí fyzické CIFS targety změněné v aktuální dávce. + + toRemove pokrývá odstranění a recreate. toMount pokrývá nové pointy. + toCloseShares zachová identitu změněného share i při pre-delete záloze, + která záměrně odebere target z toRemove. + """ + affected_targets = set(smbHelp.toRemove) + affected_shares = set(smbHelp.toCloseShares) + for source in smbHelp.toMount: + share_name = smbHelp._managedCIFSShareName(source) + if share_name: + affected_shares.add(share_name) + + for source, target in [*mounted_mounts, *configured_mounts]: + share_name = smbHelp._managedCIFSShareName(source) + if target in affected_targets or share_name in affected_shares: + affected_targets.add(target) + return affected_targets + + @staticmethod + def unmountManagedCIFS( + mounts:list[tuple[str, str]], + targets:set[str] + )->None: + """Odmountuje pouze spravované CIFS targety změněné v aktuální dávce.""" + for source, target in reversed(mounts): + if target not in targets: + continue log.info(f"Unmounting managed CIFS mount {source} from {target} before Samba reload.") try: subprocess.run( @@ -690,9 +726,9 @@ def removeQueuedMountpointDirectories(preserve_targets:set[str]|None=None)->bool def finalizeMountpointChanges()->bool: """Dokončí Samba/CIFS změny v bezpečném pořadí jako jednu transakci. - Po úpravách smb.conf a /etc/fstab odmountuje všechny stále aktivní - spravované loopback CIFS mounty, načte novou Samba konfiguraci, - provede jediný daemon-reload a připojí výsledný stav z /etc/fstab. + Po úpravách smb.conf a /etc/fstab odmountuje pouze targety dotčené + aktuální dávkou. Nezměněné loopback CIFS mounty zůstávají připojené, + takže Apply nového pointu neselže na jejich otevřeném CWD. """ smbHelp.lastError = None pending = smbHelp.requireSambaRestart or bool(smbHelp.toMount) or bool(smbHelp.toRemove) @@ -701,15 +737,34 @@ def finalizeMountpointChanges()->bool: return True try: + mounted_mounts = smbHelp.getMountedManagedCIFS() configured_mounts = smbHelp.getConfiguredManagedCIFS() configured_targets = {target for _, target in configured_mounts} + affected_targets = smbHelp.getBatchAffectedCIFSTargets( + mounted_mounts, + configured_mounts + ) + affected_configured_mounts = [ + item for item in configured_mounts if item[1] in affected_targets + ] + affected_configured_targets = { + target for _, target in affected_configured_mounts + } + unchanged_mounted_count = sum( + 1 for _, target in mounted_mounts if target not in affected_targets + ) + log.info( + "Samba/CIFS batch affects " + f"{len(affected_targets)} target(s); " + f"{unchanged_mounted_count} unchanged mounted target(s) stay connected." + ) - smbHelp.unmountAllManagedCIFS() + smbHelp.unmountManagedCIFS(mounted_mounts, affected_targets) if not smbHelp.removeQueuedMountpointDirectories(configured_targets): raise RuntimeError("Failed to remove one or more obsolete mountpoint directories.") - smbHelp.prepareConfiguredMountpointDirectories(configured_targets) + smbHelp.prepareConfiguredMountpointDirectories(affected_configured_targets) if smbHelp.requireSambaRestart: if not reloadSambaService(): @@ -722,7 +777,7 @@ def finalizeMountpointChanges()->bool: if not smbHelp.reloadSystemdDaemon(): raise RuntimeError("Failed to reload systemd after CIFS fstab changes.") - smbHelp.mountConfiguredManagedCIFS(configured_mounts) + smbHelp.mountConfiguredManagedCIFS(affected_configured_mounts) log.info("Samba/CIFS mountpoint transaction completed successfully.") return True except Exception as e: diff --git a/tests/test_sftp_rw_reconcile.py b/tests/test_sftp_rw_reconcile.py index b47364e..c133957 100644 --- a/tests/test_sftp_rw_reconcile.py +++ b/tests/test_sftp_rw_reconcile.py @@ -96,14 +96,59 @@ def test_post_remove_does_not_cleanup_inside_active_batch(self): def test_finalize_unmounts_before_cleanup_and_preserves_final_targets(self): events = [] configured = [("//127.0.0.1/sftp_mount_alice_docs", "/jail/alice/docs")] + mounted = list(configured) samba_module.smbHelp.requireSambaRestart = True samba_module.smbHelp.toRemove.extend(["/jail/alice/docs", "/jail/alice/obsolete"]) def cleanup(preserve): events.append(("cleanup", set(preserve))) return True - with patch.object(samba_module.smbHelp, "getConfiguredManagedCIFS", return_value=configured), patch.object(samba_module.smbHelp, "unmountAllManagedCIFS", side_effect=lambda: events.append(("unmount", None))), patch.object(samba_module.smbHelp, "removeQueuedMountpointDirectories", side_effect=cleanup), patch.object(samba_module.smbHelp, "prepareConfiguredMountpointDirectories", side_effect=lambda targets: events.append(("prepare", set(targets)))), patch.object(samba_module, "reloadSambaService", side_effect=lambda: events.append(("samba", None)) or True), patch.object(samba_module.smbHelp, "closeQueuedSambaShares", side_effect=lambda: events.append(("close", None)) or True), patch.object(samba_module.smbHelp, "reloadSystemdDaemon", side_effect=lambda: events.append(("systemd", None)) or True), patch.object(samba_module.smbHelp, "mountConfiguredManagedCIFS", side_effect=lambda mounts: events.append(("mount", mounts))): + with patch.object(samba_module.smbHelp, "getMountedManagedCIFS", return_value=mounted), patch.object(samba_module.smbHelp, "getConfiguredManagedCIFS", return_value=configured), patch.object(samba_module.smbHelp, "unmountManagedCIFS", side_effect=lambda mounts, targets: events.append(("unmount", mounts, set(targets)))), patch.object(samba_module.smbHelp, "removeQueuedMountpointDirectories", side_effect=cleanup), patch.object(samba_module.smbHelp, "prepareConfiguredMountpointDirectories", side_effect=lambda targets: events.append(("prepare", set(targets)))), patch.object(samba_module, "reloadSambaService", side_effect=lambda: events.append(("samba", None)) or True), patch.object(samba_module.smbHelp, "closeQueuedSambaShares", side_effect=lambda: events.append(("close", None)) or True), patch.object(samba_module.smbHelp, "reloadSystemdDaemon", side_effect=lambda: events.append(("systemd", None)) or True), patch.object(samba_module.smbHelp, "mountConfiguredManagedCIFS", side_effect=lambda mounts: events.append(("mount", mounts))): self.assertTrue(samba_module.smbHelp.finalizeMountpointChanges()) - self.assertEqual(events, [("unmount", None), ("cleanup", {"/jail/alice/docs"}), ("prepare", {"/jail/alice/docs"}), ("samba", None), ("close", None), ("systemd", None), ("mount", configured)]) + self.assertEqual(events, [("unmount", mounted, {"/jail/alice/docs", "/jail/alice/obsolete"}), ("cleanup", {"/jail/alice/docs"}), ("prepare", {"/jail/alice/docs"}), ("samba", None), ("close", None), ("systemd", None), ("mount", configured)]) + + def test_finalize_add_mount_keeps_unchanged_mounted_target(self): + events = [] + mounted = [ + ("//127.0.0.1/sftp_mount_alice_tmp", "/jail/alice/tmp"), + ] + new_mount = ( + "//127.0.0.1/sftp_mount_alice_var", + "/jail/alice/var", + ) + configured = [*mounted, new_mount] + samba_module.smbHelp.requireSambaRestart = True + samba_module.smbHelp.toMount.append(new_mount[0]) + + with patch.object(samba_module.smbHelp, "getMountedManagedCIFS", return_value=mounted), patch.object(samba_module.smbHelp, "getConfiguredManagedCIFS", return_value=configured), patch.object(samba_module.smbHelp, "unmountManagedCIFS", side_effect=lambda mounts, targets: events.append(("unmount", mounts, set(targets)))), patch.object(samba_module.smbHelp, "removeQueuedMountpointDirectories", side_effect=lambda preserve: events.append(("cleanup", set(preserve))) or True), patch.object(samba_module.smbHelp, "prepareConfiguredMountpointDirectories", side_effect=lambda targets: events.append(("prepare", set(targets)))), patch.object(samba_module, "reloadSambaService", side_effect=lambda: events.append(("samba", None)) or True), patch.object(samba_module.smbHelp, "closeQueuedSambaShares", side_effect=lambda: events.append(("close", None)) or True), patch.object(samba_module.smbHelp, "reloadSystemdDaemon", side_effect=lambda: events.append(("systemd", None)) or True), patch.object(samba_module.smbHelp, "mountConfiguredManagedCIFS", side_effect=lambda mounts: events.append(("mount", mounts))): + self.assertTrue(samba_module.smbHelp.finalizeMountpointChanges()) + + self.assertEqual(events, [ + ("unmount", mounted, {"/jail/alice/var"}), + ("cleanup", {"/jail/alice/tmp", "/jail/alice/var"}), + ("prepare", {"/jail/alice/var"}), + ("samba", None), + ("close", None), + ("systemd", None), + ("mount", [new_mount]), + ]) + + def test_unmount_managed_cifs_skips_unchanged_target(self): + mounts = [ + ("//127.0.0.1/sftp_mount_alice_tmp", "/jail/alice/tmp"), + ("//127.0.0.1/sftp_mount_alice_var", "/jail/alice/var"), + ] + proc = types.SimpleNamespace(returncode=0, stdout=b"", stderr=b"") + with patch.object(samba_module.subprocess, "run", return_value=proc) as run: + samba_module.smbHelp.unmountManagedCIFS( + mounts, + {"/jail/alice/var"}, + ) + run.assert_called_once_with( + ["umount", "/jail/alice/var"], + stdout=samba_module.subprocess.PIPE, + stderr=samba_module.subprocess.PIPE, + check=True, + ) def test_remove_queue_keeps_target_recreated_in_same_batch(self): with tempfile.TemporaryDirectory() as tmp: @@ -165,7 +210,8 @@ def test_finalize_exposes_concrete_transaction_error(self): (target / "stale.txt").write_text("stale", encoding="utf-8") configured = [("//127.0.0.1/sftp_mount_alice_docs", str(target))] samba_module.smbHelp.requireSambaRestart = True - with patch.object(samba_module.smbHelp, "getConfiguredManagedCIFS", return_value=configured), patch.object(samba_module.smbHelp, "unmountAllManagedCIFS"), patch.object(samba_module.smbHelp, "removeQueuedMountpointDirectories", return_value=True): + samba_module.smbHelp.toRemove.append(str(target)) + with patch.object(samba_module.smbHelp, "getMountedManagedCIFS", return_value=configured), patch.object(samba_module.smbHelp, "getConfiguredManagedCIFS", return_value=configured), patch.object(samba_module.smbHelp, "unmountManagedCIFS"), patch.object(samba_module.smbHelp, "removeQueuedMountpointDirectories", return_value=True): self.assertFalse(samba_module.smbHelp.finalizeMountpointChanges()) self.assertIsInstance(samba_module.smbHelp.lastError, samba_module.ManagedCIFSTargetNotEmptyError) self.assertEqual(samba_module.smbHelp.lastError.target, str(target)) @@ -173,7 +219,7 @@ def test_finalize_exposes_concrete_transaction_error(self): def test_close_share_failure_falls_back_to_full_samba_restart(self): configured = [("//127.0.0.1/sftp_mount_alice_docs", "/jail/alice/docs")] samba_module.smbHelp.requireSambaRestart = True - with patch.object(samba_module.smbHelp, "getConfiguredManagedCIFS", return_value=configured), patch.object(samba_module.smbHelp, "unmountAllManagedCIFS"), patch.object(samba_module.smbHelp, "removeQueuedMountpointDirectories", return_value=True), patch.object(samba_module.smbHelp, "prepareConfiguredMountpointDirectories"), patch.object(samba_module, "reloadSambaService", return_value=True), patch.object(samba_module.smbHelp, "closeQueuedSambaShares", return_value=False), patch.object(samba_module, "restartSambaService", return_value=True) as restart, patch.object(samba_module.smbHelp, "reloadSystemdDaemon", return_value=True), patch.object(samba_module.smbHelp, "mountConfiguredManagedCIFS"): + with patch.object(samba_module.smbHelp, "getMountedManagedCIFS", return_value=configured), patch.object(samba_module.smbHelp, "getConfiguredManagedCIFS", return_value=configured), patch.object(samba_module.smbHelp, "unmountManagedCIFS"), patch.object(samba_module.smbHelp, "removeQueuedMountpointDirectories", return_value=True), patch.object(samba_module.smbHelp, "prepareConfiguredMountpointDirectories"), patch.object(samba_module, "reloadSambaService", return_value=True), patch.object(samba_module.smbHelp, "closeQueuedSambaShares", return_value=False), patch.object(samba_module, "restartSambaService", return_value=True) as restart, patch.object(samba_module.smbHelp, "reloadSystemdDaemon", return_value=True), patch.object(samba_module.smbHelp, "mountConfiguredManagedCIFS"): self.assertTrue(samba_module.smbHelp.finalizeMountpointChanges()) restart.assert_called_once_with()