sanitize path symbols in inventory_hostname on file cache plugins by bcoca · Pull Request #41420 · ansible/ansible

bcoca · 2018-06-11T19:38:30Z

SUMMARY

Ensure that we don't error out or allow traversal when the inventory_hostname is or has path symbols.

This is common in environments using the chroot connection plugin and similar.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

cache plugins

ANSIBLE VERSION

>=2.4

ADDITIONAL INFO

Avoids this error when the inventory_hostname='..' or similar 'path related'

[ERROR]: Error while accessing the cache file '/tmp/file-cache/..': [Errno 21] Is a directory: '/tmp/file-cache/..'

and

[ERROR]: Unexpected Exception, this is probably a bug: [Errno 2] No such file or directory: '/tmp/ile-cache/my/chroot/tmpe31z4oif'

When inventory hostname is /my/chroot/hostname

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.

…sible#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. (cherry picked from commit d9d11d6)

webknjaz · 2025-10-27T11:44:10Z

+        """
+        Ensures key name is safe to use on the filesystem
+        """
+        if key not in self._sanitized:


@bcoca could you add test coverage for the else-branch here?

webknjaz · 2025-10-27T11:44:26Z

        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 "


@bcoca could you cover this line with tests?

webknjaz · 2025-10-27T11:44:58Z

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


@bcoca this line never had tests and you changed it. Could you add coverage?

webknjaz · 2025-10-27T11:46:46Z

@mattclay do we have an extablished convention for excluding such dummy files from coverage? This one shows up in Codecov with 0% so we should either measure it or omit. Can we omit it in coveragerc or is there any other preference?

The lack of code coverage collection for this plugin is due to an issue with ansible-test. I'm looking to see how difficult it will be to fix.

I've opened #86100 to fix the code coverage issue.

This file now shows up with 90.91% coverage.

ansibot added WIP affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. labels Jun 11, 2018

bcoca removed the needs_triage Needs a first human triage before being processed. label Jun 11, 2018

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 20, 2018

ansibot added support:community and removed support:core labels Sep 16, 2018

ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 20, 2018

bcoca force-pushed the cache_hostispath branch from 4073785 to bce91aa Compare November 21, 2018 00:49

ansibot added support:core and removed support:community labels Nov 26, 2018

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Dec 4, 2018

ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Dec 12, 2018

ansibot added pre_azp This PR was last tested before migration to Azure Pipelines. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 6, 2020

bcoca changed the title ~~[WIP] fix / in inventory_hostname~~ [WIP] fix / in inventory_hostname on cache plugin Jan 28, 2021

ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Oct 24, 2023

ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jan 28, 2025

bcoca force-pushed the cache_hostispath branch from bce91aa to 8832302 Compare June 4, 2025 17:27

This comment was marked as outdated.

Sign in to view

ansibot removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. ci_verified Changes made in this PR are causing tests to fail. labels Jun 4, 2025

bcoca force-pushed the cache_hostispath branch from 164e97c to bc02487 Compare July 3, 2025 16:44

ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 3, 2025

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 17, 2025

bcoca force-pushed the cache_hostispath branch from bc02487 to b349c29 Compare October 22, 2025 00:41

ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 22, 2025

This comment was marked as outdated.

Sign in to view

ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 22, 2025

This comment was marked as resolved.

Sign in to view

ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 22, 2025

bcoca changed the title ~~[WIP] fix / in inventory_hostname on cache plugin~~ sanitize path symbols in inventory_hostname on file cache plugins (chroot) Oct 23, 2025

bcoca changed the title ~~sanitize path symbols in inventory_hostname on file cache plugins (chroot)~~ sanitize path symbols in inventory_hostname on file cache plugins Oct 23, 2025

File based cache plugins filenames fix

577b8a1

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.

bcoca force-pushed the cache_hostispath branch from 60e1f64 to 577b8a1 Compare October 23, 2025 15:32

bcoca added 3 commits October 23, 2025 11:33

fix charset

f436f61

pipe

bd7b271

sanity cleanup

bfb0001

Akasurde approved these changes Oct 24, 2025

View reviewed changes

bcoca merged commit d9d11d6 into ansible:devel Oct 24, 2025
74 checks passed

bcoca deleted the cache_hostispath branch October 24, 2025 18:57

webknjaz reviewed Oct 27, 2025

View reviewed changes

mattclay mentioned this pull request Oct 30, 2025

ansible-test - Fix integration test code coverage #86100

Merged

ansible locked and limited conversation to collaborators Nov 21, 2025

Navigation Menu

Uh oh!

sanitize path symbols in inventory_hostname on file cache plugins - #41420

sanitize path symbols in inventory_hostname on file cache plugins#41420
bcoca merged 4 commits into
ansible:develfrom
bcoca:cache_hostispath

bcoca commented Jun 11, 2018 •

edited

Loading

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

webknjaz Oct 27, 2025

Uh oh!

webknjaz Oct 27, 2025

Uh oh!

webknjaz Oct 27, 2025

Uh oh!

webknjaz Oct 27, 2025

Uh oh!

mattclay Oct 29, 2025

Uh oh!

mattclay Oct 30, 2025

Uh oh!

mattclay Oct 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

bcoca commented Jun 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFO

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

webknjaz Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

webknjaz Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

webknjaz Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

webknjaz Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

mattclay Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

mattclay Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

mattclay Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bcoca commented Jun 11, 2018 •

edited

Loading