sanitize path symbols in inventory_hostname on file cache plugins (#41420)

* File based cache plugins filenames fix

File based cache plugins will now correctly handle inventory_hostnames
with 'path symbols' in their names. This should allow those using
chroot and jail connection plugins to use file based caches now.

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

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)

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))

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))

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}}'

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}}

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 "$@"