From 5e50284693cb5531eb4265a0ab94b35be89457f6 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 17 May 2022 11:41:47 -0400 Subject: [PATCH] Restrict role loading .. to role ... (#77683) * Ansible will now error out if you try to use the `*_from` to load files from outside the role being loaded by `{import,include}_role` --- changelogs/fragments/restrict_role_files_to_role.yml | 2 ++ lib/ansible/playbook/role/__init__.py | 7 ++++++- test/integration/targets/roles/no_outside.yml | 7 +++++++ test/integration/targets/roles/runme.sh | 9 +++++++++ test/integration/targets/roles/tasks/dummy.yml | 1 + 5 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/restrict_role_files_to_role.yml create mode 100644 test/integration/targets/roles/no_outside.yml create mode 100644 test/integration/targets/roles/tasks/dummy.yml diff --git a/changelogs/fragments/restrict_role_files_to_role.yml b/changelogs/fragments/restrict_role_files_to_role.yml new file mode 100644 index 00000000000..7b26610486b --- /dev/null +++ b/changelogs/fragments/restrict_role_files_to_role.yml @@ -0,0 +1,2 @@ +bugfixes: + - roles, fixed issue with roles loading paths not contained in the role itself when using the `_from` options. diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index 7e3511fc40a..58788953370 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -36,9 +36,9 @@ from ansible.playbook.role.metadata import RoleMetadata from ansible.playbook.taggable import Taggable from ansible.plugins.loader import add_all_plugin_dirs from ansible.utils.collection_loader import AnsibleCollectionConfig +from ansible.utils.path import is_subpath from ansible.utils.vars import combine_vars - __all__ = ['Role', 'hash_params'] # TODO: this should be a utility function, but can't be a member of @@ -397,6 +397,11 @@ class Role(Base, Conditional, Taggable, CollectionSearch): found_files = self._loader.find_vars_files(file_path, _main, extensions, allow_dir) if found_files: for found in found_files: + + if not is_subpath(found, file_path): + raise AnsibleParserError("Failed loading '%s' for role (%s) as it is not inside the expected role path: '%s'" % + (to_text(found), self._role_name, to_text(file_path))) + new_data = self._loader.load_from_file(found) if new_data: if data is not None and isinstance(new_data, Mapping): diff --git a/test/integration/targets/roles/no_outside.yml b/test/integration/targets/roles/no_outside.yml new file mode 100644 index 00000000000..cf6fe103c21 --- /dev/null +++ b/test/integration/targets/roles/no_outside.yml @@ -0,0 +1,7 @@ +- hosts: testhost + gather_facts: false + tasks: + - name: role attempts to load file from outside itself + include_role: + name: a + tasks_from: "{{ playbook_dir }}/tasks/dummy.yml" diff --git a/test/integration/targets/roles/runme.sh b/test/integration/targets/roles/runme.sh index 5f11c1fca61..bb98a932920 100755 --- a/test/integration/targets/roles/runme.sh +++ b/test/integration/targets/roles/runme.sh @@ -17,3 +17,12 @@ set -eux # ensure role data is merged correctly ansible-playbook data_integrity.yml -i ../../inventory "$@" + +# ensure role fails when trying to load 'non role' in _from +ansible-playbook no_outside.yml -i ../../inventory "$@" > role_outside_output.log 2>&1 || true +if grep "as it is not inside the expected role path" role_outside_output.log >/dev/null; then + echo "Test passed (playbook failed with expected output, output not shown)." +else + echo "Test failed, expected output from playbook failure is missing, output not shown)." + exit 1 +fi diff --git a/test/integration/targets/roles/tasks/dummy.yml b/test/integration/targets/roles/tasks/dummy.yml new file mode 100644 index 00000000000..b168b7ab84f --- /dev/null +++ b/test/integration/targets/roles/tasks/dummy.yml @@ -0,0 +1 @@ +- debug: msg='this should not run'