From 41e27aaa1cdb91082706c886d568b49eda0628df Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Sat, 4 Jul 2026 16:14:40 -0400 Subject: [PATCH 1/2] Include offending path in file.readdir/rmdir invalid-directory error The "A valid directory was not specified." SaltInvocationError raised by file.readdir and file.rmdir omitted the offending path, making template and highstate failures hard to diagnose. Include the path in the message. Fixes #47707 --- changelog/47707.fixed.md | 1 + salt/modules/file.py | 4 ++-- tests/pytests/unit/modules/file/test_file_rmdir.py | 10 ++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 changelog/47707.fixed.md diff --git a/changelog/47707.fixed.md b/changelog/47707.fixed.md new file mode 100644 index 000000000000..1160b27c0cd2 --- /dev/null +++ b/changelog/47707.fixed.md @@ -0,0 +1 @@ +Include the offending path in the "A valid directory was not specified" error raised by file.readdir and file.rmdir diff --git a/salt/modules/file.py b/salt/modules/file.py index dfb1c66625fe..834cd8437a3e 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -4192,7 +4192,7 @@ def readdir(path): raise SaltInvocationError("Dir path must be absolute.") if not os.path.isdir(path): - raise SaltInvocationError("A valid directory was not specified.") + raise SaltInvocationError(f"A valid directory was not specified: {path}") dirents = [".", ".."] dirents.extend(os.listdir(path)) @@ -4342,7 +4342,7 @@ def rmdir(path, recurse=False, verbose=False, older_than=None): raise SaltInvocationError("File path must be absolute.") if not os.path.isdir(path): - raise SaltInvocationError("A valid directory was not specified.") + raise SaltInvocationError(f"A valid directory was not specified: {path}") if older_than: now = time.time() diff --git a/tests/pytests/unit/modules/file/test_file_rmdir.py b/tests/pytests/unit/modules/file/test_file_rmdir.py index d40a50be50e4..caf6c963e7fc 100644 --- a/tests/pytests/unit/modules/file/test_file_rmdir.py +++ b/tests/pytests/unit/modules/file/test_file_rmdir.py @@ -37,6 +37,16 @@ def test_file_rmdir_not_found_exception(): filemod.rmdir("/tmp/not_there") +def test_file_rmdir_not_found_exception_includes_path(): + with pytest.raises(SaltInvocationError, match="/tmp/not_there"): + filemod.rmdir("/tmp/not_there") + + +def test_file_readdir_not_found_exception_includes_path(): + with pytest.raises(SaltInvocationError, match="/tmp/not_there"): + filemod.readdir("/tmp/not_there") + + def test_file_rmdir_success_return(): with patch("os.rmdir", MagicMock(return_value=True)), patch( "os.path.isdir", MagicMock(return_value=True) From dfc3a87b1c15362802827b660108fd1d45b19e4e Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Sun, 5 Jul 2026 23:21:18 -0400 Subject: [PATCH 2/2] Add direct and inverse regression tests for file.rmdir/readdir invalid-dir errors The direct test calls file.rmdir with the exact argument shape the file.rmdir state uses (recurse=..., verbose=True, older_than=...), since verbose=True normally routes removal failures into the returned dict instead of raising, and proves the invalid-directory error still raises with the offending path in the message. The inverse tests guard against overcorrection: relative paths must still hit the untouched absolute-path check for both rmdir and readdir, and readdir on an existing directory must still return its listing without raising. --- .../unit/modules/file/test_file_rmdir.py | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/pytests/unit/modules/file/test_file_rmdir.py b/tests/pytests/unit/modules/file/test_file_rmdir.py index caf6c963e7fc..22160f90ecb8 100644 --- a/tests/pytests/unit/modules/file/test_file_rmdir.py +++ b/tests/pytests/unit/modules/file/test_file_rmdir.py @@ -47,6 +47,44 @@ def test_file_readdir_not_found_exception_includes_path(): filemod.readdir("/tmp/not_there") +def test_file_rmdir_not_found_includes_path_with_state_args_47707(): + # The file.rmdir state (salt/states/file.py) calls this as + # rmdir(name, recurse=recurse, verbose=True, older_than=older_than). + # verbose=True is the decisive flag: with it, removal failures are + # normally collected into the returned dict's "errors" list instead of + # raised, but an invalid directory must still raise, and the message + # must include the offending path. + with pytest.raises(SaltInvocationError, match="/tmp/not_there"): + filemod.rmdir("/tmp/not_there", recurse=True, verbose=True, older_than=None) + + +def test_file_rmdir_relative_path_error_unchanged_47707(): + """ + Guard against overcorrection: a relative path must still fail the + absolute-path check, not the valid-directory check changed for #47707. + """ + with pytest.raises(SaltInvocationError, match="must be absolute"): + filemod.rmdir("not_absolute") + + +def test_file_readdir_relative_path_error_unchanged_47707(): + """ + Guard against overcorrection: a relative path must still fail readdir's + absolute-path check, not the valid-directory check changed for #47707. + """ + with pytest.raises(SaltInvocationError, match="must be absolute"): + filemod.readdir("not_absolute") + + +def test_file_readdir_valid_directory_47707(tmp_path): + """ + Guard against overcorrection: readdir on an existing directory must + still return the directory listing without raising. + """ + (tmp_path / "afile").write_text("data") + assert filemod.readdir(str(tmp_path)) == [".", "..", "afile"] + + def test_file_rmdir_success_return(): with patch("os.rmdir", MagicMock(return_value=True)), patch( "os.path.isdir", MagicMock(return_value=True)