From 8c16daf766474f9660b4e59e910ab9d6bf8474c0 Mon Sep 17 00:00:00 2001
From: Jon Ellis <jellis@iweb.co.uk>
Date: Sun, 5 Jun 2022 23:15:52 +0100
Subject: [PATCH] Use visudo to validate sudoers rules before use

---
 plugins/modules/system/sudoers.py             | 16 ++++++++
 .../targets/sudoers/tasks/main.yml            | 37 ++++++++++++++++++-
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/plugins/modules/system/sudoers.py b/plugins/modules/system/sudoers.py
index 86d8306c26..775c44260d 100644
--- a/plugins/modules/system/sudoers.py
+++ b/plugins/modules/system/sudoers.py
@@ -109,6 +109,7 @@ EXAMPLES = '''
 '''
 
 import os
+import subprocess
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.common.text.converters import to_native
 
@@ -134,6 +135,8 @@ class Sudoers(object):
         with open(self.file, 'w') as f:
             f.write(self.content())
 
+        os.chmod(self.file, 0o440)
+
     def delete(self):
         if self.check_mode:
             return
@@ -158,11 +161,24 @@ class Sudoers(object):
         runas_str = '({runas})'.format(runas=self.runas) if self.runas is not None else ''
         return "{owner} ALL={runas}{nopasswd} {commands}\n".format(owner=owner, runas=runas_str, nopasswd=nopasswd_str, commands=commands_str)
 
+    def validate(self):
+        # fork to visudo
+        content = bytes(self.content(), 'utf-8')
+        check_command = ['visudo', '-c', '-f', '-']
+
+        check = subprocess.Popen(check_command, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        stdout, stderr = check.communicate(input=content)
+
+        if check.returncode != 0:
+            raise Exception('Failed to validate sudoers rule:\n{stdout}'.format(stdout=stdout))
+
     def run(self):
         if self.state == 'absent' and self.exists():
             self.delete()
             return True
 
+        self.validate()
+
         if self.exists() and self.matches():
             return False
 
diff --git a/tests/integration/targets/sudoers/tasks/main.yml b/tests/integration/targets/sudoers/tasks/main.yml
index 9a632c4de4..cf2c0a417e 100644
--- a/tests/integration/targets/sudoers/tasks/main.yml
+++ b/tests/integration/targets/sudoers/tasks/main.yml
@@ -1,11 +1,15 @@
 ---
 # Initialise environment
 
-- name: Register sudoers.d directory
+- name: Register variables
   set_fact:
     sudoers_path: /etc/sudoers.d
     alt_sudoers_path: /etc/sudoers_alt
 
+- name: Install sudo package
+  ansible.builtin.apt:
+    name: sudo
+
 - name: Ensure sudoers directory exists
   ansible.builtin.file:
     path: "{{ sudoers_path }}"
@@ -29,6 +33,11 @@
     commands: /usr/local/bin/command
   register: rule_1
 
+- name: Stat my-sudo-rule-1 file
+  ansible.builtin.stat:
+    path: "{{ sudoers_path }}/my-sudo-rule-1"
+  register: rule_1_stat
+
 - name: Grab contents of my-sudo-rule-1
   ansible.builtin.slurp:
     src: "{{ sudoers_path }}/my-sudo-rule-1"
@@ -130,8 +139,27 @@
   register: revoke_rule_1_stat
 
 
+- name: Attempt command without full path to executable
+  community.general.sudoers:
+    name: edge-case-1
+    state: present
+    user: alice
+    commands: systemctl
+  ignore_errors: true
+  register: edge_case_1
+
+
 # Run assertions
 
+- name: Check rule 1 file stat
+  ansible.builtin.assert:
+    that:
+      - rule_1_stat.stat.exists
+      - rule_1_stat.stat.isreg
+      - rule_1_stat.stat.mode == '0440'
+      - rule_1_stat.stat.pw_name == 'root'
+      - rule_1_stat.stat.gr_name == 'root'
+
 - name: Check changed status
   ansible.builtin.assert:
     that:
@@ -150,7 +178,12 @@
       - "rule_5_contents['content'] | b64decode == 'alice ALL=NOPASSWD: /usr/local/bin/command\n'"
       - "rule_6_contents['content'] | b64decode == 'alice ALL=(bob)NOPASSWD: /usr/local/bin/command\n'"
 
-- name: Check stats
+- name: Check revocation stat
   ansible.builtin.assert:
     that:
       - not revoke_rule_1_stat.stat.exists
+
+- name: Check edge case responses
+  ansible.builtin.assert:
+    that:
+      - edge_case_1 is failed