diff --git a/changelogs/fragments/cache_avoid_path_keys.yml b/changelogs/fragments/cache_avoid_path_keys.yml new file mode 100644 index 00000000000..b3ac649c80b --- /dev/null +++ b/changelogs/fragments/cache_avoid_path_keys.yml @@ -0,0 +1,2 @@ +bugfixes: + - cache plugins based on the BaseFileCache class will now sanitize keys to avoid names that could cause issues with the storage path diff --git a/lib/ansible/plugins/cache/__init__.py b/lib/ansible/plugins/cache/__init__.py index ec94fb1e12e..42cf0386313 100644 --- a/lib/ansible/plugins/cache/__init__.py +++ b/lib/ansible/plugins/cache/__init__.py @@ -18,6 +18,7 @@ from __future__ import annotations import copy +import hashlib import os import tempfile import time @@ -40,6 +41,8 @@ display = Display() class BaseCacheModule(AnsiblePlugin): + _PATH_CHARS = frozenset({'/', '..', '<', '>', '|'}) + # Backwards compat only. Just import the global display instead _display = display _persistent = True @@ -91,6 +94,8 @@ class BaseFileCacheModule(BaseCacheModule): self.plugin_name = resource_from_fqcr(self.__module__) self._cache = {} self.validate_cache_connection() + self._sanitized = {} + self._files = {} def _get_cache_connection(self, source): if source: @@ -99,10 +104,23 @@ class BaseFileCacheModule(BaseCacheModule): except TypeError: pass + def _sanitize_key(self, key: str) -> str: + """ + Ensures key name is safe to use on the filesystem + """ + if key not in self._sanitized: + for invalid in self._PATH_CHARS: + if invalid in key: + self._sanitized[key] = hashlib.sha256(key.encode()).hexdigest()[:max(len(key), 12)] + break + else: + self._sanitized[key] = key + return self._sanitized[key] + def validate_cache_connection(self): if not self._cache_dir: - raise AnsibleError("error, '%s' cache plugin requires the 'fact_caching_connection' config option " - "to be set (to a writeable directory path)" % self.plugin_name) + raise AnsibleError(f"'{self.plugin_name!r}' cache plugin requires the 'fact_caching_connection' configuration option " + "to be set (to a writeable directory path)") if not os.path.exists(self._cache_dir): try: @@ -112,16 +130,17 @@ class BaseFileCacheModule(BaseCacheModule): else: for x in (os.R_OK, os.W_OK, os.X_OK): if not os.access(self._cache_dir, x): - raise AnsibleError("error in '%s' cache, configured path (%s) does not have necessary permissions (rwx), disabling plugin" % ( - self.plugin_name, self._cache_dir)) + raise AnsibleError(f"'{self.plugin_name!r}' cache, configured path ({self._cache_dir}) does not have necessary permissions (rwx)," + " disabling plugin") def _get_cache_file_name(self, key: str) -> str: - prefix = self.get_option('_prefix') - if prefix: - cachefile = "%s/%s%s" % (self._cache_dir, prefix, key) - else: - cachefile = "%s/%s" % (self._cache_dir, key) - return cachefile + if key not in self._files: + safe = self._sanitize_key(key) # use key or filesystem safe hash of key + prefix = self.get_option('_prefix') + if not prefix: + prefix = '' + self._files[key] = os.path.join(self._cache_dir, prefix + safe) + return self._files[key] def get(self, key): """ This checks the in memory cache first as the fact was not expired at 'gather time' @@ -155,7 +174,7 @@ class BaseFileCacheModule(BaseCacheModule): self._cache[key] = value cachefile = self._get_cache_file_name(key) - tmpfile_handle, tmpfile_path = tempfile.mkstemp(dir=os.path.dirname(cachefile)) + tmpfile_handle, tmpfile_path = tempfile.mkstemp(dir=self._cache_dir) try: try: self._dump(value, tmpfile_path) diff --git a/test/integration/targets/cache-plugins/cache_plugins/dummy_file_cache.py b/test/integration/targets/cache-plugins/cache_plugins/dummy_file_cache.py new file mode 100644 index 00000000000..7ede3f6745e --- /dev/null +++ b/test/integration/targets/cache-plugins/cache_plugins/dummy_file_cache.py @@ -0,0 +1,49 @@ +from __future__ import annotations + +DOCUMENTATION = """ + name: dummy_file_cache + short_description: dummy file cache + description: see short + options: + _uri: + required: True + description: + - Path in which the cache plugin will save the files + env: + - name: ANSIBLE_CACHE_PLUGIN_CONNECTION + ini: + - key: fact_caching_connection + section: defaults + type: path + _prefix: + description: User defined prefix to use when creating the files + env: + - name: ANSIBLE_CACHE_PLUGIN_PREFIX + ini: + - key: fact_caching_prefix + section: defaults + _timeout: + default: 86400 + description: Expiration timeout for the cache plugin data + env: + - name: ANSIBLE_CACHE_PLUGIN_TIMEOUT + ini: + - key: fact_caching_timeout + section: defaults + type: integer +""" + +from ansible.plugins.cache import BaseFileCacheModule + + +class CacheModule(BaseFileCacheModule): + + _persistent = False + + def _load(self, filepath: str) -> object: + with open(filepath, 'r') as jfile: + return eval(filepath.read()) + + def _dump(self, value: object, filepath: str) -> None: + with open(filepath, 'w') as afile: + afile.write(str(value)) diff --git a/test/integration/targets/cache-plugins/cache_plugins/dummy_file_cache_persistent.py b/test/integration/targets/cache-plugins/cache_plugins/dummy_file_cache_persistent.py new file mode 100644 index 00000000000..a6582777088 --- /dev/null +++ b/test/integration/targets/cache-plugins/cache_plugins/dummy_file_cache_persistent.py @@ -0,0 +1,47 @@ +from __future__ import annotations + +DOCUMENTATION = """ + name: dummy_file_cache + short_description: dummy file cache + description: see short + options: + _uri: + required: True + description: + - Path in which the cache plugin will save the files + env: + - name: ANSIBLE_CACHE_PLUGIN_CONNECTION + ini: + - key: fact_caching_connection + section: defaults + type: path + _prefix: + description: User defined prefix to use when creating the files + env: + - name: ANSIBLE_CACHE_PLUGIN_PREFIX + ini: + - key: fact_caching_prefix + section: defaults + _timeout: + default: 86400 + description: Expiration timeout for the cache plugin data + env: + - name: ANSIBLE_CACHE_PLUGIN_TIMEOUT + ini: + - key: fact_caching_timeout + section: defaults + type: integer +""" + +from ansible.plugins.cache import BaseFileCacheModule + + +class CacheModule(BaseFileCacheModule): + + def _load(self, filepath: str) -> object: + with open(filepath, 'r') as jfile: + return eval(filepath.read()) + + def _dump(self, value: object, filepath: str) -> None: + with open(filepath, 'w') as afile: + afile.write(str(value)) diff --git a/test/integration/targets/cache-plugins/chroot_inventory_config.yml b/test/integration/targets/cache-plugins/chroot_inventory_config.yml new file mode 100644 index 00000000000..8411164633c --- /dev/null +++ b/test/integration/targets/cache-plugins/chroot_inventory_config.yml @@ -0,0 +1,14 @@ +chroots: + hosts: + /my/chroot/host1: + bogusvar: foobarvalue + /my/chroot/host2: +traversal: + hosts: + ..: + ...: + +all: + vars: + ansible_connection: local + ansible_python_interpreter: '{{ansible_playbook_python}}' diff --git a/test/integration/targets/cache-plugins/invalid_hostname_file_caches.yml b/test/integration/targets/cache-plugins/invalid_hostname_file_caches.yml new file mode 100644 index 00000000000..3d4e9a1ae4d --- /dev/null +++ b/test/integration/targets/cache-plugins/invalid_hostname_file_caches.yml @@ -0,0 +1,7 @@ +- hosts: all + gather_facts: false + tasks: + - name: populate cache, will fail if invalid files are used + set_fact: + cacheable: true + testing: 123{{inventory_hostname}} diff --git a/test/integration/targets/cache-plugins/runme.sh b/test/integration/targets/cache-plugins/runme.sh index 65350e1dfb7..bad3b6eda41 100755 --- a/test/integration/targets/cache-plugins/runme.sh +++ b/test/integration/targets/cache-plugins/runme.sh @@ -30,3 +30,17 @@ export ANSIBLE_INVENTORY_CACHE_PLUGIN=dummy_cache ansible-playbook test_inventory_cache.yml "$@" ansible-playbook inspect_inventory_cache.yml -i test.inventoryconfig.yml "$@" + +# test file based cache with 'fun' inventory names +export ANSIBLE_CACHE_PLUGIN=dummy_file_cache ANSIBLE_CACHE_PLUGIN_CONNECTION="${OUTPUT_DIR}/dummy-file-cache" +mkdir -p "${ANSIBLE_CACHE_PLUGIN_CONNECTION}" +ansible-playbook -i chroot_inventory_config.yml invalid_hostname_file_caches.yml "$@" + +# same, but using 'persistent' route +ANSIBLE_CACHE_PLUGIN=dummy_file_cache_persistent ansible-playbook -i chroot_inventory_config.yml invalid_hostname_file_caches.yml "$@" + +# test file based cache with 'fun' inventory names, and a prefix! +export ANSIBLE_CACHE_PLUGIN_PREFIX="YOLO" +ansible-playbook -i chroot_inventory_config.yml invalid_hostname_file_caches.yml "$@" + +ANSIBLE_CACHE_PLUGIN=dummy_file_cache_persistent ansible-playbook -i chroot_inventory_config.yml invalid_hostname_file_caches.yml "$@"