From b84745c4a2331b99b9ac0d1fc40a6ab7f49eeca4 Mon Sep 17 00:00:00 2001
From: David Shrewsbury <dshrewsb@redhat.com>
Date: Wed, 16 Feb 2022 11:13:24 -0500
Subject: [PATCH] Less permissive bwrap options

---
 ansible_runner/config/runner.py | 15 +++++++++-
 test/unit/config/test_runner.py | 52 +++++++++++++++++++++++----------
 2 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/ansible_runner/config/runner.py b/ansible_runner/config/runner.py
index 11f6d353b..69f0015c1 100644
--- a/ansible_runner/config/runner.py
+++ b/ansible_runner/config/runner.py
@@ -334,7 +334,20 @@ def wrap_args_for_sandbox(self, args):
         '''
         cwd = os.path.realpath(self.cwd)
         self.process_isolation_path_actual = self.build_process_isolation_temp_dir()
-        new_args = [self.process_isolation_executable or 'bwrap', '--die-with-parent', '--unshare-pid', '--dev-bind', '/', '/', '--proc', '/proc']
+        new_args = [self.process_isolation_executable or 'bwrap']
+
+        new_args.extend([
+            '--die-with-parent',
+            '--unshare-pid',
+            '--dev', '/dev',
+            '--proc', '/proc',
+            '--dir', '/tmp',
+            '--ro-bind', '/bin', '/bin',
+            '--ro-bind', '/etc', '/etc',
+            '--ro-bind', '/usr', '/usr',
+            '--ro-bind', '/opt', '/opt',
+            '--symlink', 'usr/lib64', '/lib64',
+        ])
 
         for path in sorted(set(self.process_isolation_hide_paths or [])):
             if not os.path.exists(path):
diff --git a/test/unit/config/test_runner.py b/test/unit/config/test_runner.py
index 4886d2956..3248df68e 100644
--- a/test/unit/config/test_runner.py
+++ b/test/unit/config/test_runner.py
@@ -563,8 +563,14 @@ def test_bwrap_process_isolation_defaults(mocker):
         'bwrap',
         '--die-with-parent',
         '--unshare-pid',
-        '--dev-bind', '/', '/',
+        '--dev', '/dev',
         '--proc', '/proc',
+        '--dir', '/tmp',
+        '--ro-bind', '/bin', '/bin',
+        '--ro-bind', '/etc', '/etc',
+        '--ro-bind', '/usr', '/usr',
+        '--ro-bind', '/opt', '/opt',
+        '--symlink', 'usr/lib64', '/lib64',
         '--bind', '/', '/',
         '--chdir', '/project',
         'ansible-playbook', '-i', '/inventory', 'main.yaml',
@@ -610,8 +616,14 @@ def isfile(self, path):
         'bwrap',
         '--die-with-parent',
         '--unshare-pid',
-        '--dev-bind', '/', '/',
+        '--dev', '/dev',
         '--proc', '/proc',
+        '--dir', '/tmp',
+        '--ro-bind', '/bin', '/bin',
+        '--ro-bind', '/etc', '/etc',
+        '--ro-bind', '/usr', '/usr',
+        '--ro-bind', '/opt', '/opt',
+        '--symlink', 'usr/lib64', '/lib64',
         '--bind', '/', '/',
         '--chdir', os.path.realpath(rc.directory_isolation_path),
         'ansible-playbook', '-i', '/inventory', 'main.yaml',
@@ -628,7 +640,7 @@ def test_process_isolation_settings(mocker, tmp_path):
     rc.playbook = 'main.yaml'
     rc.command = 'ansible-playbook'
     rc.process_isolation = True
-    rc.process_isolation_executable = 'not_bwrap'
+    rc.process_isolation_executable = 'no_bwrap'
     rc.process_isolation_hide_paths = ['/home', '/var']
     rc.process_isolation_show_paths = ['/usr']
     rc.process_isolation_ro_paths = ['/venv']
@@ -638,35 +650,43 @@ def test_process_isolation_settings(mocker, tmp_path):
 
     rc.prepare()
     print(rc.command)
-    assert rc.command[0:8] == [
-        'not_bwrap',
+    expected = [
+        'no_bwrap',
         '--die-with-parent',
         '--unshare-pid',
-        '--dev-bind', '/', '/',
+        '--dev', '/dev',
         '--proc', '/proc',
+        '--dir', '/tmp',
+        '--ro-bind', '/bin', '/bin',
+        '--ro-bind', '/etc', '/etc',
+        '--ro-bind', '/usr', '/usr',
+        '--ro-bind', '/opt', '/opt',
+        '--symlink', 'usr/lib64', '/lib64',
     ]
+    index = len(expected)
+    assert rc.command[0:index] == expected
 
     # hide /home
-    assert rc.command[8] == '--bind'
-    assert 'ansible_runner_pi' in rc.command[9]
-    assert rc.command[10] == os.path.realpath('/home')  # needed for Mac
+    assert rc.command[index] == '--bind'
+    assert 'ansible_runner_pi' in rc.command[index + 1]
+    assert rc.command[index + 2] == os.path.realpath('/home')  # needed for Mac
 
     # hide /var
-    assert rc.command[11] == '--bind'
-    assert 'ansible_runner_pi' in rc.command[12]
-    assert rc.command[13] == '/var' or rc.command[13] == '/private/var'
+    assert rc.command[index + 3] == '--bind'
+    assert 'ansible_runner_pi' in rc.command[index + 4]
+    assert rc.command[index + 5] in ('/var', '/private/var')
 
     # read-only bind
-    assert rc.command[14:17] == ['--ro-bind', '/venv', '/venv']
+    assert rc.command[index + 6:index + 9] == ['--ro-bind', '/venv', '/venv']
 
     # root bind
-    assert rc.command[17:20] == ['--bind', '/', '/']
+    assert rc.command[index + 9:index + 12] == ['--bind', '/', '/']
 
     # show /usr
-    assert rc.command[20:23] == ['--bind', '/usr', '/usr']
+    assert rc.command[index + 12:index + 15] == ['--bind', '/usr', '/usr']
 
     # chdir and ansible-playbook command
-    assert rc.command[23:] == ['--chdir', '/project', 'ansible-playbook', '-i', '/inventory', 'main.yaml']
+    assert rc.command[index + 15:] == ['--chdir', '/project', 'ansible-playbook', '-i', '/inventory', 'main.yaml']
 
 
 def test_container_volume_mounting_with_Z(mocker, tmp_path):