_symbolic_mode_to_octal- fix raising ValueError for invalid symbolic modes (#80449)

validate the full user and perm strings instead of just first character

fixes unhelpful unarchive error for some invalid modes

4 files changed, 33 insertions, 4 deletions

diff --git a/changelogs/fragments/80449-fix-symbolic-mode-error-msg.yml b/changelogs/fragments/80449-fix-symbolic-mode-error-msg.yml
new file mode 100644
index 0000000000..b760774ef4
--- /dev/null
+++ b/changelogs/fragments/80449-fix-symbolic-mode-error-msg.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - file modules - fix validating invalid symbolic modes.
diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py
index 161d6e79e2..7cb22c6656 100644
--- a/lib/ansible/module_utils/basic.py
+++ b/lib/ansible/module_utils/basic.py
@@ -265,8 +265,8 @@ PASSWD_ARG_RE = re.compile(r'^[-]{0,2}pass[-]?(word|wd)?')
 
 # Used for parsing symbolic file perms
 MODE_OPERATOR_RE = re.compile(r'[+=-]')
-USERS_RE = re.compile(r'[^ugo]')
-PERMS_RE = re.compile(r'[^rwxXstugo]')
+USERS_RE = re.compile(r'^[ugo]+$')
+PERMS_RE = re.compile(r'^[rwxXstugo]*$')
 
 
 #
@@ -1063,14 +1063,14 @@ class AnsibleModule(object):
 
             # Check if there are illegal characters in the user list
             # They can end up in 'users' because they are not split
-            if USERS_RE.match(users):
+            if not USERS_RE.match(users):
                 raise ValueError("bad symbolic permission for mode: %s" % mode)
 
             # Now we have two list of equal length, one contains the requested
             # permissions and one with the corresponding operators.
             for idx, perms in enumerate(permlist):
                 # Check if there are illegal characters in the permissions
-                if PERMS_RE.match(perms):
+                if not PERMS_RE.match(perms):
                     raise ValueError("bad symbolic permission for mode: %s" % mode)
 
                 for user in users:
diff --git a/test/integration/targets/unarchive/tasks/test_mode.yml b/test/integration/targets/unarchive/tasks/test_mode.yml
index c69e3bd2b2..9e8b14c8b2 100644
--- a/test/integration/targets/unarchive/tasks/test_mode.yml
+++ b/test/integration/targets/unarchive/tasks/test_mode.yml
@@ -3,6 +3,29 @@
     path: '{{remote_tmp_dir}}/test-unarchive-tar-gz'
     state: directory
 
+- name: test invalid modes
+  unarchive:
+    src: "{{ remote_tmp_dir }}/test-unarchive.tar.gz"
+    dest: "{{ remote_tmp_dir }}/test-unarchive-tar-gz"
+    remote_src: yes
+    mode: "{{ item }}"
+    list_files: True
+  register: unarchive_mode_errors
+  ignore_errors: yes
+  loop:
+    - u=foo
+    - foo=r
+    - ufoo=r
+    - abc=r
+    - ao=r
+    - oa=r
+
+- assert:
+    that:
+      - item.failed
+      - "'bad symbolic permission for mode: ' + item.item == item.details"
+  loop: "{{ unarchive_mode_errors.results }}"
+
 - name: unarchive and set mode to 0600, directories 0700
   unarchive:
     src: "{{ remote_tmp_dir }}/test-unarchive.tar.gz"
diff --git a/test/units/modules/test_copy.py b/test/units/modules/test_copy.py
index 9de86118ae..beeef6d72c 100644
--- a/test/units/modules/test_copy.py
+++ b/test/units/modules/test_copy.py
@@ -192,6 +192,10 @@ UMASK_DATA = (
 INVALID_DATA = (
     (0o040000, u'a=foo', "bad symbolic permission for mode: a=foo"),
     (0o040000, u'f=rwx', "bad symbolic permission for mode: f=rwx"),
+    (0o100777, u'of=r', "bad symbolic permission for mode: of=r"),
+
+    (0o100777, u'ao=r', "bad symbolic permission for mode: ao=r"),
+    (0o100777, u'oa=r', "bad symbolic permission for mode: oa=r"),
 )