From be0ddf2ecfb38c3cc9a134c0f1fd5ba29c1945e4 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Mon, 15 Dec 2025 14:59:23 -0500 Subject: [PATCH 1/6] user - Warn when an invalid shell is specified Only for BusyBox since that was the context where this initally came up. It may make sense to add this warning more broadly. chsh on Alpine warns but does not error when changing to an invalid shell. Other distros error with an invalid shell. --- lib/ansible/modules/user.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/ansible/modules/user.py b/lib/ansible/modules/user.py index 94fb4be7e33..84811c3728e 100644 --- a/lib/ansible/modules/user.py +++ b/lib/ansible/modules/user.py @@ -3132,6 +3132,13 @@ class BusyBox(User): - remove_user() - modify_user() """ + def _validate_shell(self): + with open("/etc/shells", "rb") as f: + shells = [line.decode().strip() for line in f.readlines() if not line.startswith(b"#")] + + if self.shell not in shells: + self.module.warn(f"'{self.shell}' is not listed as a valid shell on the remote host.") + def _build_password_string(self, current_password=None): """ Build the appropriate password string based on the current password and @@ -3274,6 +3281,8 @@ class BusyBox(User): add_cmd_bin = self.module.get_bin_path('adduser', True) remove_cmd_bin = self.module.get_bin_path('delgroup', True) + self._validate_shell() + # Manage group membership if self.groups: groups = self.get_groups_set() or set() From f78c66e9ba2ca02e11db33ae78a7b7f8aab89191 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Mon, 15 Dec 2025 15:10:00 -0500 Subject: [PATCH 2/6] Add changelog fragment --- changelogs/fragments/86243-user-busybox-shell-warn.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/86243-user-busybox-shell-warn.yml diff --git a/changelogs/fragments/86243-user-busybox-shell-warn.yml b/changelogs/fragments/86243-user-busybox-shell-warn.yml new file mode 100644 index 00000000000..44501686b16 --- /dev/null +++ b/changelogs/fragments/86243-user-busybox-shell-warn.yml @@ -0,0 +1,2 @@ +bugfixes: + - user - On BusyBox systems, warn when an invalid shell is specified (https://github.com/ansible/ansible/pull/86342) From 04b72e815cd7ca6ee1b2f4f5a13dd4485273086a Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Wed, 17 Dec 2025 17:01:27 -0500 Subject: [PATCH 3/6] Return early if shell was not provided as a parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strip blank lines in addition to comments when looking at /etc/shells. Use a nested comprehension instead of the assignment operator so that the module will work with older Python versions. Open the file in text mode since I can’t think of a reason it would be unsafe to do so in this situation. --- lib/ansible/modules/user.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/user.py b/lib/ansible/modules/user.py index 84811c3728e..0dfd84a740a 100644 --- a/lib/ansible/modules/user.py +++ b/lib/ansible/modules/user.py @@ -3133,8 +3133,16 @@ class BusyBox(User): - modify_user() """ def _validate_shell(self): - with open("/etc/shells", "rb") as f: - shells = [line.decode().strip() for line in f.readlines() if not line.startswith(b"#")] + if not self.shell: + return + + with open("/etc/shells", "r") as f: + shells = [ + shell + for shell in (line.strip() for line in f.readlines()) + if shell + and not shell.startswith("#") + ] if self.shell not in shells: self.module.warn(f"'{self.shell}' is not listed as a valid shell on the remote host.") From 24c13ea37550f42dc06e8860840f1d23b03bba5c Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Thu, 5 Feb 2026 15:37:30 -0500 Subject: [PATCH 4/6] Also validate shell when creating the user account --- lib/ansible/modules/user.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ansible/modules/user.py b/lib/ansible/modules/user.py index 0dfd84a740a..66f9009637d 100644 --- a/lib/ansible/modules/user.py +++ b/lib/ansible/modules/user.py @@ -3180,6 +3180,8 @@ class BusyBox(User): def create_user(self): cmd = [self.module.get_bin_path('adduser', True)] + self._validate_shell() + cmd.append('-D') if self.uid is not None: From 994652924f135e549c19313b7cbb8768d425539e Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Fri, 6 Feb 2026 10:23:12 -0500 Subject: [PATCH 5/6] Handle when /etc/shells does not exist --- lib/ansible/modules/user.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/ansible/modules/user.py b/lib/ansible/modules/user.py index 66f9009637d..ebc67896bf7 100644 --- a/lib/ansible/modules/user.py +++ b/lib/ansible/modules/user.py @@ -3136,13 +3136,16 @@ class BusyBox(User): if not self.shell: return - with open("/etc/shells", "r") as f: - shells = [ - shell - for shell in (line.strip() for line in f.readlines()) - if shell - and not shell.startswith("#") - ] + try: + with open("/etc/shells", "r") as f: + shells = [ + shell + for shell in (line.strip() for line in f.readlines()) + if shell + and not shell.startswith("#") + ] + except FileNotFoundError: + return if self.shell not in shells: self.module.warn(f"'{self.shell}' is not listed as a valid shell on the remote host.") From 1d80283efab571fc2cf6664b30300e358f46640c Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Fri, 6 Feb 2026 10:28:24 -0500 Subject: [PATCH 6/6] Iterate over the file object Since the file is being iterated over already, calling readlines() is unnecessary. It also increases memory usage by reading the file into memory and then iterating over that. --- lib/ansible/modules/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/modules/user.py b/lib/ansible/modules/user.py index ebc67896bf7..1f3b0c4e306 100644 --- a/lib/ansible/modules/user.py +++ b/lib/ansible/modules/user.py @@ -3140,7 +3140,7 @@ class BusyBox(User): with open("/etc/shells", "r") as f: shells = [ shell - for shell in (line.strip() for line in f.readlines()) + for shell in (line.strip() for line in f) if shell and not shell.startswith("#") ]