From e06bb184390ac64b54989d83db5ae3ae53fa0d7e Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Fri, 10 Jul 2026 13:40:25 -0400 Subject: [PATCH] Fix silent flag in file.directory recurse The silence marker for recurse was assigned before the check_perms walk loop, so the per-file and per-dir change notifications gathered inside the loop repopulated ret["changes"] and overrode it. Move the marker reset to after the walk loop (but before the clean block, so removed entries are preserved) so individual change notifications are suppressed as documented. Fixes #60597 --- changelog/60597.fixed.md | 1 + salt/states/file.py | 6 +- .../unit/states/file/test_directory.py | 123 ++++++++++++++++++ 3 files changed, 127 insertions(+), 3 deletions(-) create mode 100644 changelog/60597.fixed.md diff --git a/changelog/60597.fixed.md b/changelog/60597.fixed.md new file mode 100644 index 000000000000..981fd3f8dc11 --- /dev/null +++ b/changelog/60597.fixed.md @@ -0,0 +1 @@ +Fixed ``silent`` in ``file.directory`` recurse so per-file change notifications are actually suppressed diff --git a/salt/states/file.py b/salt/states/file.py index 96bc5cab9141..8d4c5e48557a 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -4179,9 +4179,6 @@ def directory( file_mode = None dir_mode = None - if "silent" in recurse_set: - ret["changes"] = {"recursion": "Changes silenced"} - check_files = "ignore_files" not in recurse_set check_dirs = "ignore_dirs" not in recurse_set @@ -4230,6 +4227,9 @@ def directory( if not exc.strerror.startswith("Path not found"): errors.append(exc.strerror) + if "silent" in recurse_set: + ret["changes"] = {"recursion": "Changes silenced"} + if clean: keep = _gen_keep_files(name, require, walk_d) log.debug("List of kept files when use file.directory with clean: %s", keep) diff --git a/tests/pytests/unit/states/file/test_directory.py b/tests/pytests/unit/states/file/test_directory.py index 1474e4eee1ae..8ef504c3cd4f 100644 --- a/tests/pytests/unit/states/file/test_directory.py +++ b/tests/pytests/unit/states/file/test_directory.py @@ -1,3 +1,4 @@ +import contextlib import logging import os @@ -347,3 +348,125 @@ def test_directory_test_mode_user_group_not_present(): assert filestate.directory(name, user=user, group=group) == ret assert filestate.directory(name, user=user, group=group) == ret assert filestate.directory(name, user=user, group=group) == ret + + +def _check_perms_populating(*args, **kwargs): + """ + Stand-in for file.check_perms that records a per-path change into the + passed-in ret, mimicking how the real function repopulates ret["changes"] + for every file/dir visited during a recursive run. Handles both the posix + positional call (path, ret, ...) and the Windows keyword call + (path=, ret=, ...). + """ + if kwargs: + path = kwargs["path"] + cur = kwargs["ret"] + else: + path = args[0] + cur = args[1] + cur["changes"][path] = {"mode": "0755"} + if salt.utils.platform.is_windows(): + return cur + return cur, "" + + +def _directory_recurse_patches(name): + """ + Common patches to drive file.directory into the recursive check_perms + loop deterministically: force a pending change so the no-change early + return is skipped, make the target look like an existing directory, and + feed the walk a single child file and child dir. + """ + tchanges = (None, "", {name: {"directory": "new"}}) + return [ + patch.dict( + filestate.__salt__, + {"file.check_perms": MagicMock(side_effect=_check_perms_populating)}, + ), + patch("salt.states.file._check_directory", MagicMock(return_value=tchanges)), + patch( + "salt.states.file._check_directory_win", MagicMock(return_value=tchanges) + ), + patch( + "salt.states.file._depth_limited_walk", + MagicMock(return_value=[(name, ["child_dir"], ["child_file"])]), + ), + patch("salt.utils.win_dacl.get_sid", MagicMock()), + patch( + "salt.utils.win_functions.get_current_user", + MagicMock(return_value="username"), + ), + patch.object(os.path, "isdir", MagicMock(return_value=True)), + patch.object(os.path, "isfile", MagicMock(return_value=False)), + patch.object(os.path, "isabs", MagicMock(return_value=True)), + ] + + +def test_directory_recurse_silent_suppresses_changes_60597(): + """ + Regression test for #60597: with ``silent`` in the recurse set, the + individual per-file/per-dir change notifications produced by the + check_perms loop must be replaced by the single silence marker. + + Production-exact call shape: recurse=["mode", "silent"] (the ``silent`` + flag lives inside ``recurse``, not at the state top level). + """ + name = "/etc/testdir" + if salt.utils.platform.is_windows(): + name = name.replace("/", "\\") + + with contextlib.ExitStack() as stack: + for ctx in _directory_recurse_patches(name): + stack.enter_context(ctx) + # silent is passed inside recurse, alongside mode + ret = filestate.directory(name, recurse=["mode", "silent"]) + + assert ret["changes"] == {"recursion": "Changes silenced"} + + +def test_directory_recurse_without_silent_still_reports_changes_60597(): + """ + Must-not-regress sibling for #60597: without ``silent`` in the recurse + set, the per-file/per-dir changes gathered by the check_perms loop must + still be reported. This passes both with and without the fix; it guards + against the silence marker leaking into the ordinary recurse path. + """ + name = "/etc/testdir" + if salt.utils.platform.is_windows(): + name = name.replace("/", "\\") + + with contextlib.ExitStack() as stack: + for ctx in _directory_recurse_patches(name): + stack.enter_context(ctx) + # no silent this time, only mode + ret = filestate.directory(name, recurse=["mode"]) + + assert os.path.join(name, "child_file") in ret["changes"] + assert os.path.join(name, "child_dir") in ret["changes"] + assert "recursion" not in ret["changes"] + + +def test_directory_recurse_silent_preserves_clean_removed_60597(): + """ + Peripheral coverage for #60597: the silence reset runs after the + check_perms loop but before the ``clean`` block, so entries recorded by + clean (``removed``) must still survive alongside the silence marker. + """ + name = "/etc/testdir" + if salt.utils.platform.is_windows(): + name = name.replace("/", "\\") + removed = [os.path.join(name, "stale")] + + with contextlib.ExitStack() as stack: + for ctx in _directory_recurse_patches(name): + stack.enter_context(ctx) + stack.enter_context( + patch("salt.states.file._gen_keep_files", MagicMock(return_value=set())) + ) + stack.enter_context( + patch("salt.states.file._clean_dir", MagicMock(return_value=removed)) + ) + # silent inside recurse, together with clean=True + ret = filestate.directory(name, recurse=["mode", "silent"], clean=True) + + assert ret["changes"] == {"recursion": "Changes silenced", "removed": removed}