diff --git a/changelogs/fragments/dont_choose_unreadable.yml b/changelogs/fragments/dont_choose_unreadable.yml new file mode 100644 index 00000000000..f9c8c7eb597 --- /dev/null +++ b/changelogs/fragments/dont_choose_unreadable.yml @@ -0,0 +1,2 @@ +bugfixes: + - avoid choosing an unreadable ansible.cfg just because it exists. diff --git a/lib/ansible/cli/config.py b/lib/ansible/cli/config.py index 4edc801cdcc..a3f844562da 100644 --- a/lib/ansible/cli/config.py +++ b/lib/ansible/cli/config.py @@ -14,7 +14,7 @@ from ansible.cli import CLI from ansible.cli.arguments import option_helpers as opt_help from ansible.config.manager import ConfigManager, Setting, find_ini_config_file from ansible.errors import AnsibleError, AnsibleOptionsError -from ansible.module_utils._text import to_native, to_text +from ansible.module_utils._text import to_native, to_text, to_bytes from ansible.parsing.yaml.dumper import AnsibleDumper from ansible.utils.color import stringc from ansible.utils.display import Display @@ -79,7 +79,11 @@ class ConfigCLI(CLI): if context.CLIARGS['config_file']: self.config_file = unfrackpath(context.CLIARGS['config_file'], follow=False) - self.config = ConfigManager(self.config_file) + b_config = to_bytes(self.config_file) + if os.path.exists(b_config) and os.access(b_config, os.R_OK): + self.config = ConfigManager(self.config_file) + else: + raise AnsibleOptionsError('The provided configuration file is missing or not accessible: %s' % to_native(self.config_file)) else: self.config = ConfigManager() self.config_file = find_ini_config_file() diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index 15931e36d1e..f76c8bf6ac1 100644 --- a/lib/ansible/config/manager.py +++ b/lib/ansible/config/manager.py @@ -23,9 +23,10 @@ except ImportError: from ansible.config.data import ConfigData from ansible.errors import AnsibleOptionsError, AnsibleError +from ansible.module_utils._text import to_text, to_bytes, to_native +from ansible.module_utils.common._collections_compat import Sequence from ansible.module_utils.six import PY3, string_types from ansible.module_utils.six.moves import configparser -from ansible.module_utils._text import to_text, to_bytes, to_native from ansible.module_utils.parsing.convert_bool import boolean from ansible.parsing.quoting import unquote from ansible.utils import py3compat @@ -76,6 +77,7 @@ def ensure_type(value, value_type, origin=None): :string: Same as 'str' ''' + errmsg = '' basedir = None if origin and os.path.isabs(origin) and os.path.exists(to_bytes(origin)): basedir = origin @@ -96,39 +98,64 @@ def ensure_type(value, value_type, origin=None): elif value_type == 'list': if isinstance(value, string_types): value = [x.strip() for x in value.split(',')] + elif not isinstance(value, Sequence): + errmsg = 'list' elif value_type == 'none': if value == "None": value = None + if value is not None: + errmsg = 'None' + elif value_type == 'path': - value = resolve_path(value, basedir=basedir) + if isinstance(value, string_types): + value = resolve_path(value, basedir=basedir) + else: + errmsg = 'path' elif value_type in ('tmp', 'temppath', 'tmppath'): - value = resolve_path(value, basedir=basedir) - if not os.path.exists(value): - makedirs_safe(value, 0o700) - prefix = 'ansible-local-%s' % os.getpid() - value = tempfile.mkdtemp(prefix=prefix, dir=value) - atexit.register(cleanup_tmp_file, value, warn=True) + if isinstance(value, string_types): + value = resolve_path(value, basedir=basedir) + if not os.path.exists(value): + makedirs_safe(value, 0o700) + prefix = 'ansible-local-%s' % os.getpid() + value = tempfile.mkdtemp(prefix=prefix, dir=value) + atexit.register(cleanup_tmp_file, value, warn=True) + else: + errmsg = 'temppath' elif value_type == 'pathspec': if isinstance(value, string_types): value = value.split(os.pathsep) - value = [resolve_path(x, basedir=basedir) for x in value] + + if isinstance(value, Sequence): + value = [resolve_path(x, basedir=basedir) for x in value] + else: + errmsg = 'pathspec' elif value_type == 'pathlist': if isinstance(value, string_types): value = value.split(',') - value = [resolve_path(x, basedir=basedir) for x in value] + + if isinstance(value, Sequence): + value = [resolve_path(x, basedir=basedir) for x in value] + else: + errmsg = 'pathlist' elif value_type in ('str', 'string'): - value = unquote(to_text(value, errors='surrogate_or_strict')) + if isinstance(value, string_types): + value = unquote(to_text(value, errors='surrogate_or_strict')) + else: + errmsg = 'string' # defaults to string type elif isinstance(value, string_types): value = unquote(value) + if errmsg: + raise ValueError('Invalid type provided for "%s": %s' % (errmsg, to_native(value))) + return to_text(value, errors='surrogate_or_strict', nonstring='passthru') @@ -215,7 +242,8 @@ def find_ini_config_file(warnings=None): potential_paths.append("/etc/ansible/ansible.cfg") for path in potential_paths: - if os.path.exists(to_bytes(path)): + b_path = to_bytes(path) + if os.path.exists(b_path) and os.access(b_path, os.R_OK): break else: path = None @@ -255,9 +283,8 @@ class ConfigManager(object): # consume configuration if self._config_file: - if os.path.exists(to_bytes(self._config_file)): - # initialize parser and read config - self._parse_config_file() + # initialize parser and read config + self._parse_config_file() # update constants self.update_config_data() diff --git a/test/integration/targets/ansible/no-extension b/test/integration/targets/ansible/no-extension new file mode 100644 index 00000000000..61a99f48bbd --- /dev/null +++ b/test/integration/targets/ansible/no-extension @@ -0,0 +1,2 @@ +[defaults] +remote_user = admin diff --git a/test/integration/targets/ansible/runme.sh b/test/integration/targets/ansible/runme.sh index e9554934bad..515058e71eb 100755 --- a/test/integration/targets/ansible/runme.sh +++ b/test/integration/targets/ansible/runme.sh @@ -14,7 +14,7 @@ ANSIBLE_REMOTE_USER=administrator ansible-config dump| grep 'DEFAULT_REMOTE_USER ansible-config list | grep 'DEFAULT_REMOTE_USER' # 'view' command must fail when config file is missing or has an invalid file extension -ansible-config view -c ./ansible-non-existent.cfg 2> err1.txt || grep -Eq '(FileNotFoundError|IOError): \[Errno [0-9]+\] No such file or directory' err1.txt || (cat err*.txt; rm -f err1.txt; exit 1) +ansible-config view -c ./ansible-non-existent.cfg 2> err1.txt || grep -Eq 'ERROR! The provided configuration file is missing or not accessible:' err1.txt || (cat err*.txt; rm -f err1.txt; exit 1) ansible-config view -c ./no-extension 2> err2.txt || grep -q 'Unsupported configuration file extension' err2.txt || (cat err2.txt; rm -f err*.txt; exit 1) rm -f err*.txt diff --git a/test/units/config/manager/test_find_ini_config_file.py b/test/units/config/manager/test_find_ini_config_file.py index 49d931c7c8a..046b89dc5a6 100644 --- a/test/units/config/manager/test_find_ini_config_file.py +++ b/test/units/config/manager/test_find_ini_config_file.py @@ -53,8 +53,15 @@ def setup_existing_files(request, monkeypatch): else: return False + def _os_access(path, access): + if to_text(path) in (request.param[0]): + return True + else: + return False + # Enable user and system dirs so that we know cwd takes precedence monkeypatch.setattr("os.path.exists", _os_path_exists) + monkeypatch.setattr("os.access", _os_access) monkeypatch.setattr("os.getcwd", lambda: os.path.dirname(cfg_dir)) monkeypatch.setattr("os.path.isdir", lambda path: True if to_text(path) == cfg_dir else real_isdir(path))