Merge "Block localhost shell tasks in untrusted playbooks" into stable/3.x

36 files changed, 156 insertions, 50 deletions

diff --git a/playbooks/zuul-stream/templates/ansible.cfg.j2 b/playbooks/zuul-stream/templates/ansible.cfg.j2
index 92d6867c9..d07874d4a 100644
--- a/playbooks/zuul-stream/templates/ansible.cfg.j2
+++ b/playbooks/zuul-stream/templates/ansible.cfg.j2
@@ -3,7 +3,7 @@ inventory = {{ ansible_user_dir }}/inventory.yaml
 gathering = smart
 gather_subset = !all
 lookup_plugins = {{ ansible_user_dir }}/src/opendev.org/zuul/zuul/zuul/ansible/{{ zuul_ansible_version }}/lookup
-action_plugins = {{ ansible_user_dir }}/src/opendev.org/zuul/zuul/zuul/ansible/{{ zuul_ansible_version }}/actiongeneral:{{ ansible_user_dir }}/src/opendev.org/zuul/zuul/zuul/ansible/action
+action_plugins = {{ ansible_user_dir }}/src/opendev.org/zuul/zuul/zuul/ansible/{{ zuul_ansible_version }}/actiongeneral:{{ ansible_user_dir }}/src/opendev.org/zuul/zuul/zuul/ansible/{{ zuul_ansible_version }}/action
 callback_plugins = {{ ansible_user_dir }}/src/opendev.org/zuul/zuul/zuul/ansible/{{ zuul_ansible_version }}/callback:/usr/lib/zuul/ansible/{{ zuul_ansible_version }}/lib/python3.5/site-packages/ara/plugins/callbacks
 stdout_callback = zuul_stream
 library = {{ ansible_user_dir }}/src/opendev.org/zuul/zuul/zuul/ansible/{{ zuul_ansible_version }}/library
diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/shell-delegate.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/shell-delegate.yaml
new file mode 100644
index 000000000..7cb81e3b7
--- /dev/null
+++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/shell-delegate.yaml
@@ -0,0 +1,12 @@
+- hosts: all
+  tasks:
+    - name: Normal shell
+      delegate_to: localhost
+      shell: echo 123
+
+    - name: Shell with executable
+      delegate_to: localhost
+      shell: |
+        echo 123
+      args:
+        executable: /bin/bash
diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/shell-localhost.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/shell-localhost.yaml
new file mode 100644
index 000000000..35f7253f1
--- /dev/null
+++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/shell-localhost.yaml
@@ -0,0 +1,10 @@
+- hosts: localhost
+  tasks:
+    - name: Normal shell
+      shell: echo 123
+
+    - name: Shell with executable
+      shell: |
+        echo 123
+      args:
+        executable: /bin/bash
diff --git a/tests/fixtures/config/remote-zuul-stream/git/common-config/playbooks/command-localhost.yaml b/tests/fixtures/config/remote-zuul-stream/git/common-config/playbooks/command-localhost.yaml
new file mode 100644
index 000000000..629f3398c
--- /dev/null
+++ b/tests/fixtures/config/remote-zuul-stream/git/common-config/playbooks/command-localhost.yaml
@@ -0,0 +1,7 @@
+- hosts: localhost
+  tasks:
+    - name: Local shell task with python exception
+      command: echo foo
+      args:
+        chdir: /local-shelltask/somewhere/that/does/not/exist
+      failed_when: false
diff --git a/tests/fixtures/config/remote-zuul-stream/git/common-config/zuul.yaml b/tests/fixtures/config/remote-zuul-stream/git/common-config/zuul.yaml
index a07342e2e..f9ad5fcfe 100644
--- a/tests/fixtures/config/remote-zuul-stream/git/common-config/zuul.yaml
+++ b/tests/fixtures/config/remote-zuul-stream/git/common-config/zuul.yaml
@@ -15,3 +15,7 @@
 - job:
     name: base
     parent: null
+
+- job:
+    name: command-localhost
+    run: playbooks/command-localhost.yaml
diff --git a/tests/fixtures/config/remote-zuul-stream/git/org_project/playbooks/command.yaml b/tests/fixtures/config/remote-zuul-stream/git/org_project/playbooks/command.yaml
index ec84cf784..ea772d607 100644
--- a/tests/fixtures/config/remote-zuul-stream/git/org_project/playbooks/command.yaml
+++ b/tests/fixtures/config/remote-zuul-stream/git/org_project/playbooks/command.yaml
@@ -97,11 +97,3 @@
       args:
         chdir: /remote-shelltask/somewhere/that/does/not/exist
       failed_when: false
-
-- hosts: localhost
-  tasks:
-    - name: Local shell task with python exception
-      command: echo foo
-      args:
-        chdir: /local-shelltask/somewhere/that/does/not/exist
-      failed_when: false
diff --git a/tests/remote/test_remote_action_modules.py b/tests/remote/test_remote_action_modules.py
index c5818ae9c..23833f174 100644
--- a/tests/remote/test_remote_action_modules.py
+++ b/tests/remote/test_remote_action_modules.py
@@ -18,6 +18,7 @@ import textwrap
 from tests.base import AnsibleZuulTestCase, FIXTURE_DIR
 
 ERROR_ACCESS_OUTSIDE = "Accessing files from outside the working dir"
+ERROR_LOCAL_CODE = "Executing local code is prohibited"
 ERROR_SYNC_TO_OUTSIDE = "Syncing files to outside the working dir"
 ERROR_SYNC_FROM_OUTSIDE = "Syncing files from outside the working dir"
 ERROR_SYNC_RSH = "Using custom synchronize rsh is prohibited"
@@ -175,6 +176,8 @@ class FunctionalActionModulesMixIn:
 
     def test_shell_module(self):
         self._run_job('shell-good', 'SUCCESS')
+        self._run_job('shell-localhost', 'FAILURE', ERROR_LOCAL_CODE)
+        self._run_job('shell-delegate', 'FAILURE', ERROR_LOCAL_CODE)
 
     def test_synchronize_module(self):
         self._run_job('synchronize-good', 'SUCCESS')
diff --git a/tests/remote/test_remote_zuul_stream.py b/tests/remote/test_remote_zuul_stream.py
index 026fd1e7f..9fef472b5 100644
--- a/tests/remote/test_remote_zuul_stream.py
+++ b/tests/remote/test_remote_zuul_stream.py
@@ -32,7 +32,7 @@ class FunctionalZuulStreamMixIn:
         ansible_remote = os.environ.get('ZUUL_REMOTE_IPV4')
         self.assertIsNotNone(ansible_remote)
 
-    def _run_job(self, job_name):
+    def _run_job(self, job_name, create=True):
         # Keep the jobdir around so we can inspect contents if an
         # assert fails. It will be cleaned up anyway as it is contained
         # in a tmp dir which gets cleaned up after the test.
@@ -40,32 +40,40 @@ class FunctionalZuulStreamMixIn:
 
         # Output extra ansible info so we might see errors.
         self.executor_server.verbose = True
-        conf = textwrap.dedent(
-            """
-            - job:
-                name: {job_name}
-                run: playbooks/{job_name}.yaml
-                ansible-version: {version}
-                vars:
-                  test_console_port: {console_port}
-                roles:
-                  - zuul: org/common-config
-                nodeset:
-                  nodes:
-                    - name: compute1
-                      label: whatever
-                    - name: controller
-                      label: whatever
-
-            - project:
-                check:
-                  jobs:
-                    - {job_name}
-            """.format(
-                job_name=job_name,
-                version=self.ansible_version,
-                console_port=self.log_console_port))
+        if create:
+            conf = textwrap.dedent(
+                """
+                - job:
+                    name: {job_name}
+                    run: playbooks/{job_name}.yaml
+                    ansible-version: {version}
+                    vars:
+                      test_console_port: {console_port}
+                    roles:
+                      - zuul: org/common-config
+                    nodeset:
+                      nodes:
+                        - name: compute1
+                          label: whatever
+                        - name: controller
+                          label: whatever
 
+                - project:
+                    check:
+                      jobs:
+                        - {job_name}
+                """.format(
+                    job_name=job_name,
+                    version=self.ansible_version,
+                    console_port=self.log_console_port))
+        else:
+            conf = textwrap.dedent(
+                """
+                - project:
+                    check:
+                      jobs:
+                        - {job_name}
+                """.format(job_name=job_name))
         file_dict = {'zuul.yaml': conf}
         A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
                                            files=file_dict)
@@ -141,9 +149,6 @@ class FunctionalZuulStreamMixIn:
             self.assertLogLine(r'compute1 \| failed_in_loop2', text)
             self.assertLogLine(r'compute1 \| ok: Item: failed_in_loop2 '
                                r'Result: 1', text)
-            self.assertLogLine(r'localhost \| .*No such file or directory: .*'
-                               r'\'/local-shelltask/somewhere/'
-                               r'that/does/not/exist\'', text)
             self.assertLogLine(r'compute1 \| .*No such file or directory: .*'
                                r'\'/remote-shelltask/somewhere/'
                                r'that/does/not/exist\'', text)
@@ -160,6 +165,18 @@ class FunctionalZuulStreamMixIn:
                 r'RUN END RESULT_NORMAL: \[untrusted : review.example.com/'
                 r'org/project/playbooks/command.yaml@master]', text)
 
+        # Run a pre-defined job that is defined in a trusted repo to test
+        # localhost tasks.
+        job = self._run_job('command-localhost', create=False)
+        with self.jobLog(job):
+            build = self.history[-1]
+            self.assertEqual(build.result, 'SUCCESS')
+
+            text = self._get_job_output(build)
+            self.assertLogLine(r'localhost \| .*No such file or directory: .*'
+                               r'\'/local-shelltask/somewhere/'
+                               r'that/does/not/exist\'', text)
+
     def test_module_exception(self):
         job = self._run_job('module_failure_exception')
         with self.jobLog(job):
@@ -260,9 +277,6 @@ class TestZuulStream28(AnsibleZuulTestCase, FunctionalZuulStreamMixIn):
             self.assertLogLine(r'compute1 \| failed_in_loop2', text)
             self.assertLogLine(r'compute1 \| ok: Item: failed_in_loop2 '
                                r'Result: 1', text)
-            self.assertLogLine(r'localhost \| .*No such file or directory: .*'
-                               r'\'/local-shelltask/somewhere/'
-                               r'that/does/not/exist\'', text)
             self.assertLogLine(r'compute1 \| .*No such file or directory: .*'
                                r'\'/remote-shelltask/somewhere/'
                                r'that/does/not/exist\'', text)
@@ -281,6 +295,18 @@ class TestZuulStream28(AnsibleZuulTestCase, FunctionalZuulStreamMixIn):
                 r'RUN END RESULT_NORMAL: \[untrusted : review.example.com/'
                 r'org/project/playbooks/command.yaml@master]', text)
 
+        # Run a pre-defined job that is defined in a trusted repo to test
+        # localhost tasks.
+        job = self._run_job('command-localhost', create=False)
+        with self.jobLog(job):
+            build = self.history[-1]
+            self.assertEqual(build.result, 'SUCCESS')
+
+            text = self._get_job_output(build)
+            self.assertLogLine(r'localhost \| .*No such file or directory: .*'
+                               r'\'/local-shelltask/somewhere/'
+                               r'that/does/not/exist\'', text)
+
 
 class TestZuulStream29(TestZuulStream28):
     ansible_version = '2.9'
diff --git a/tools/test-logs.sh b/tools/test-logs.sh
index 2df7eb69a..d71b9b7f9 100755
--- a/tools/test-logs.sh
+++ b/tools/test-logs.sh
@@ -68,7 +68,7 @@ fact_caching = jsonfile
 fact_caching_connection = ~/.cache/facts
 lookup_plugins = ${ZUUL_ANSIBLE}/zuul/ansible/lookup
 callback_plugins = ${ZUUL_ANSIBLE}/zuul/ansible/callback:$ARA_DIR/plugins/callbacks
-action_plugins = ${ZUUL_ANSIBLE}/zuul/ansible/actiongeneral
+action_plugins = ${ZUUL_ANSIBLE}/zuul/ansible/actiongeneral:${ZUUL_ANSIBLE}/zuul/ansible/actiontrusted
 module_utils = ${ZUUL_ANSIBLE}/zuul/ansible/module_utils
 stdout_callback = zuul_stream
 library = ${ZUUL_ANSIBLE}/zuul/ansible/library
diff --git a/zuul/ansible/2.7/action/command.py b/zuul/ansible/2.7/action/command.py
new file mode 120000
index 000000000..56c6b636f
--- /dev/null
+++ b/zuul/ansible/2.7/action/command.py
@@ -0,0 +1 @@
+../../base/action/command.py
\ No newline at end of file
diff --git a/zuul/ansible/2.7/action/command.pyi b/zuul/ansible/2.7/action/command.pyi
new file mode 120000
index 000000000..a003281ca
--- /dev/null
+++ b/zuul/ansible/2.7/action/command.pyi
@@ -0,0 +1 @@
+../../base/action/command.pyi
\ No newline at end of file
diff --git a/zuul/ansible/2.7/actiongeneral/command.py b/zuul/ansible/2.7/actiongeneral/command.py
deleted file mode 120000
index f190db2cc..000000000
--- a/zuul/ansible/2.7/actiongeneral/command.py
+++ /dev/null
@@ -1 +0,0 @@
-../../base/actiongeneral/command.py
\ No newline at end of file
diff --git a/zuul/ansible/2.7/actiongeneral/command.pyi b/zuul/ansible/2.7/actiongeneral/command.pyi
deleted file mode 120000
index 81305dd03..000000000
--- a/zuul/ansible/2.7/actiongeneral/command.pyi
+++ /dev/null
@@ -1 +0,0 @@
-../../base/actiongeneral/command.pyi
\ No newline at end of file
diff --git a/zuul/ansible/2.7/actiontrusted/__init__.py b/zuul/ansible/2.7/actiontrusted/__init__.py
new file mode 120000
index 000000000..e64650426
--- /dev/null
+++ b/zuul/ansible/2.7/actiontrusted/__init__.py
@@ -0,0 +1 @@
+../../base/actiontrusted/__init__.py
\ No newline at end of file
diff --git a/zuul/ansible/2.7/actiontrusted/command.py b/zuul/ansible/2.7/actiontrusted/command.py
new file mode 120000
index 000000000..0eb995eaa
--- /dev/null
+++ b/zuul/ansible/2.7/actiontrusted/command.py
@@ -0,0 +1 @@
+../../base/actiontrusted/command.py
\ No newline at end of file
diff --git a/zuul/ansible/2.7/actiontrusted/command.pyi b/zuul/ansible/2.7/actiontrusted/command.pyi
new file mode 120000
index 000000000..17d0db7f6
--- /dev/null
+++ b/zuul/ansible/2.7/actiontrusted/command.pyi
@@ -0,0 +1 @@
+../../base/actiontrusted/command.pyi
\ No newline at end of file
diff --git a/zuul/ansible/2.8/action/command.py b/zuul/ansible/2.8/action/command.py
new file mode 120000
index 000000000..56c6b636f
--- /dev/null
+++ b/zuul/ansible/2.8/action/command.py
@@ -0,0 +1 @@
+../../base/action/command.py
\ No newline at end of file
diff --git a/zuul/ansible/2.8/action/command.pyi b/zuul/ansible/2.8/action/command.pyi
new file mode 120000
index 000000000..a003281ca
--- /dev/null
+++ b/zuul/ansible/2.8/action/command.pyi
@@ -0,0 +1 @@
+../../base/action/command.pyi
\ No newline at end of file
diff --git a/zuul/ansible/2.8/actiongeneral/command.py b/zuul/ansible/2.8/actiongeneral/command.py
deleted file mode 120000
index f190db2cc..000000000
--- a/zuul/ansible/2.8/actiongeneral/command.py
+++ /dev/null
@@ -1 +0,0 @@
-../../base/actiongeneral/command.py
\ No newline at end of file
diff --git a/zuul/ansible/2.8/actiongeneral/command.pyi b/zuul/ansible/2.8/actiongeneral/command.pyi
deleted file mode 120000
index 81305dd03..000000000
--- a/zuul/ansible/2.8/actiongeneral/command.pyi
+++ /dev/null
@@ -1 +0,0 @@
-../../base/actiongeneral/command.pyi
\ No newline at end of file
diff --git a/zuul/ansible/2.8/actiontrusted/__init__.py b/zuul/ansible/2.8/actiontrusted/__init__.py
new file mode 120000
index 000000000..e64650426
--- /dev/null
+++ b/zuul/ansible/2.8/actiontrusted/__init__.py
@@ -0,0 +1 @@
+../../base/actiontrusted/__init__.py
\ No newline at end of file
diff --git a/zuul/ansible/2.8/actiontrusted/command.py b/zuul/ansible/2.8/actiontrusted/command.py
new file mode 120000
index 000000000..0eb995eaa
--- /dev/null
+++ b/zuul/ansible/2.8/actiontrusted/command.py
@@ -0,0 +1 @@
+../../base/actiontrusted/command.py
\ No newline at end of file
diff --git a/zuul/ansible/2.8/actiontrusted/command.pyi b/zuul/ansible/2.8/actiontrusted/command.pyi
new file mode 120000
index 000000000..17d0db7f6
--- /dev/null
+++ b/zuul/ansible/2.8/actiontrusted/command.pyi
@@ -0,0 +1 @@
+../../base/actiontrusted/command.pyi
\ No newline at end of file
diff --git a/zuul/ansible/2.9/action/command.py b/zuul/ansible/2.9/action/command.py
new file mode 120000
index 000000000..56c6b636f
--- /dev/null
+++ b/zuul/ansible/2.9/action/command.py
@@ -0,0 +1 @@
+../../base/action/command.py
\ No newline at end of file
diff --git a/zuul/ansible/2.9/action/command.pyi b/zuul/ansible/2.9/action/command.pyi
new file mode 120000
index 000000000..a003281ca
--- /dev/null
+++ b/zuul/ansible/2.9/action/command.pyi
@@ -0,0 +1 @@
+../../base/action/command.pyi
\ No newline at end of file
diff --git a/zuul/ansible/2.9/actiongeneral/command.py b/zuul/ansible/2.9/actiongeneral/command.py
deleted file mode 120000
index f190db2cc..000000000
--- a/zuul/ansible/2.9/actiongeneral/command.py
+++ /dev/null
@@ -1 +0,0 @@
-../../base/actiongeneral/command.py
\ No newline at end of file
diff --git a/zuul/ansible/2.9/actiongeneral/command.pyi b/zuul/ansible/2.9/actiongeneral/command.pyi
deleted file mode 120000
index 81305dd03..000000000
--- a/zuul/ansible/2.9/actiongeneral/command.pyi
+++ /dev/null
@@ -1 +0,0 @@
-../../base/actiongeneral/command.pyi
\ No newline at end of file
diff --git a/zuul/ansible/2.9/actiontrusted/__init__.py b/zuul/ansible/2.9/actiontrusted/__init__.py
new file mode 120000
index 000000000..e64650426
--- /dev/null
+++ b/zuul/ansible/2.9/actiontrusted/__init__.py
@@ -0,0 +1 @@
+../../base/actiontrusted/__init__.py
\ No newline at end of file
diff --git a/zuul/ansible/2.9/actiontrusted/command.py b/zuul/ansible/2.9/actiontrusted/command.py
new file mode 120000
index 000000000..0eb995eaa
--- /dev/null
+++ b/zuul/ansible/2.9/actiontrusted/command.py
@@ -0,0 +1 @@
+../../base/actiontrusted/command.py
\ No newline at end of file
diff --git a/zuul/ansible/2.9/actiontrusted/command.pyi b/zuul/ansible/2.9/actiontrusted/command.pyi
new file mode 120000
index 000000000..17d0db7f6
--- /dev/null
+++ b/zuul/ansible/2.9/actiontrusted/command.pyi
@@ -0,0 +1 @@
+../../base/actiontrusted/command.pyi
\ No newline at end of file
diff --git a/zuul/ansible/base/action/command.py b/zuul/ansible/base/action/command.py
new file mode 100644
index 000000000..bf2debc96
--- /dev/null
+++ b/zuul/ansible/base/action/command.py
@@ -0,0 +1,33 @@
+# Copyright 2018 BMW Car IT GmbH
+#
+# This module is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This software is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this software.  If not, see <http://www.gnu.org/licenses/>.
+
+
+from zuul.ansible import paths
+from ansible.errors import AnsibleError
+command = paths._import_ansible_action_plugin("command")
+
+
+class ActionModule(command.ActionModule):
+
+    def run(self, tmp=None, task_vars=None):
+        if paths._is_localhost_task(self):
+            raise AnsibleError("Executing local code is prohibited")
+
+        # we need the zuul_log_id on shell and command tasks
+        host = paths._sanitize_filename(task_vars.get('inventory_hostname'))
+        if self._task.action in ('command', 'shell'):
+            self._task.args['zuul_log_id'] = "%s-%s" % (self._task._uuid, host)
+
+        return super(ActionModule, self).run(tmp, task_vars)
diff --git a/zuul/ansible/base/actiongeneral/command.pyi b/zuul/ansible/base/action/command.pyi
index e69de29bb..e69de29bb 100644
--- a/zuul/ansible/base/actiongeneral/command.pyi
+++ b/zuul/ansible/base/action/command.pyi
diff --git a/zuul/ansible/base/actiontrusted/__init__.py b/zuul/ansible/base/actiontrusted/__init__.py
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/zuul/ansible/base/actiontrusted/__init__.py
diff --git a/zuul/ansible/base/actiongeneral/command.py b/zuul/ansible/base/actiontrusted/command.py
index f9b976ca0..f9b976ca0 100644
--- a/zuul/ansible/base/actiongeneral/command.py
+++ b/zuul/ansible/base/actiontrusted/command.py
diff --git a/zuul/ansible/base/actiontrusted/command.pyi b/zuul/ansible/base/actiontrusted/command.pyi
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/zuul/ansible/base/actiontrusted/command.pyi
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index 2b08d5bc7..4e909491a 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -859,6 +859,7 @@ class AnsibleJob(object):
         self.library_dir = os.path.join(plugin_dir, 'library')
         self.action_dir = os.path.join(plugin_dir, 'action')
         self.action_dir_general = os.path.join(plugin_dir, 'actiongeneral')
+        self.action_dir_trusted = os.path.join(plugin_dir, 'actiontrusted')
         self.callback_dir = os.path.join(plugin_dir, 'callback')
         self.lookup_dir = os.path.join(plugin_dir, 'lookup')
         self.filter_dir = os.path.join(plugin_dir, 'filter')
@@ -2049,13 +2050,22 @@ class AnsibleJob(object):
             # 10s to respond
             config.write('timeout = 30\n')
 
-            # We need at least the general action dir as this overwrites the
-            # command action plugin for log streaming.
+            # We need the general action dir to make the zuul_return plugin
+            # available to every job.
             action_dirs = [self.action_dir_general]
             if not trusted:
+                # Untrusted jobs add the action dir which makes sure localhost
+                # modules are restricted where needed. Further the command
+                # plugin needs to be restricted and also inject zuul_log_id
+                # to make log streaming work.
                 action_dirs.append(self.action_dir)
                 config.write('lookup_plugins = %s\n'
                              % self.lookup_dir)
+            else:
+                # Trusted jobs add the actiontrusted dir which adds the
+                # unrestricted command plugin to inject zuul_log_id to make
+                # log streaming work.
+                action_dirs.append(self.action_dir_trusted)
 
             config.write('action_plugins = %s\n'
                          % ':'.join(action_dirs))