From b464d18fd1366f497faf0badf97634349109f0db Mon Sep 17 00:00:00 2001 From: Pilou Date: Fri, 4 Dec 2020 20:21:51 +0100 Subject: [PATCH] AnsibleModule.set_mode_if_different: handle symlink is in a sticky directory (#45198) * file: add symlink is in a sticky directory tests * file: handle symlink in a sticky directory Co-Authored-By: Sviatoslav Sydorenko * Add changelog and fix unit test The builtins import was removed since it was unused, but it is now needed. --- ...t_mode_if_different-symlink-sticky-dir.yml | 4 + lib/ansible/module_utils/basic.py | 6 +- .../targets/file/defaults/main.yml | 2 + .../targets/file/tasks/state_link.yml | 82 +++++++++++++++++++ .../basic/test_set_mode_if_different.py | 40 +++++++++ 5 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/set_mode_if_different-symlink-sticky-dir.yml create mode 100644 test/integration/targets/file/defaults/main.yml diff --git a/changelogs/fragments/set_mode_if_different-symlink-sticky-dir.yml b/changelogs/fragments/set_mode_if_different-symlink-sticky-dir.yml new file mode 100644 index 00000000000..ce9d02f6cbd --- /dev/null +++ b/changelogs/fragments/set_mode_if_different-symlink-sticky-dir.yml @@ -0,0 +1,4 @@ +bugfixes: + - > + set_mode_if_different - handle symlink if it is inside a directory with + sticky bit set (https://github.com/ansible/ansible/pull/45198) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 55b44e3e2db..f7cd7a1060a 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1186,7 +1186,11 @@ class AnsibleModule(object): if underlying_stat.st_mode != new_underlying_stat.st_mode: os.chmod(b_path, stat.S_IMODE(underlying_stat.st_mode)) except OSError as e: - if os.path.islink(b_path) and e.errno in (errno.EPERM, errno.EROFS): # Can't set mode on symbolic links + if os.path.islink(b_path) and e.errno in ( + errno.EACCES, # can't access symlink in sticky directory (stat) + errno.EPERM, # can't set mode on symbolic links (chmod) + errno.EROFS, # can't set mode on read-only filesystem + ): pass elif e.errno in (errno.ENOENT, errno.ELOOP): # Can't set mode on broken symbolic links pass diff --git a/test/integration/targets/file/defaults/main.yml b/test/integration/targets/file/defaults/main.yml new file mode 100644 index 00000000000..8e9a5836479 --- /dev/null +++ b/test/integration/targets/file/defaults/main.yml @@ -0,0 +1,2 @@ +--- +remote_unprivileged_user: tmp_ansible_test_user diff --git a/test/integration/targets/file/tasks/state_link.yml b/test/integration/targets/file/tasks/state_link.yml index 15927881d8d..89150adc9f5 100644 --- a/test/integration/targets/file/tasks/state_link.yml +++ b/test/integration/targets/file/tasks/state_link.yml @@ -116,6 +116,88 @@ that: - "file6a_result.changed == false" +# In order for a symlink in a sticky world writable directory to be followed, it must +# either be owned by the follower, +# or the directory and symlink must have the same owner. +- name: symlink in sticky directory + block: + - name: Create remote unprivileged remote user + user: + name: '{{ remote_unprivileged_user }}' + register: user + + - name: Create a local temporary directory + tempfile: + state: directory + register: tempdir + + - name: Set sticky bit + file: + path: '{{ tempdir.path }}' + mode: o=rwXt + + - name: 'Check mode: force creation soft link in sticky directory owned by another user (mode is used)' + file: + src: '{{ user.home }}/nonexistent' + dest: '{{ tempdir.path }}/soft3.txt' + mode: 0640 + state: 'link' + owner: '{{ remote_unprivileged_user }}' + force: true + follow: false + check_mode: true + register: missing_dst_no_follow_enable_force_use_mode1 + + - name: force creation soft link in sticky directory owned by another user (mode is used) + file: + src: '{{ user.home }}/nonexistent' + dest: '{{ tempdir.path }}/soft3.txt' + mode: 0640 + state: 'link' + owner: '{{ remote_unprivileged_user }}' + force: true + follow: false + register: missing_dst_no_follow_enable_force_use_mode2 + + - name: Get stat info for the link + stat: + path: '{{ tempdir.path }}/soft3.txt' + follow: false + register: soft3_result + + - name: 'Idempotence: force creation soft link in sticky directory owned by another user (mode is used)' + file: + src: '{{ user.home }}/nonexistent' + dest: '{{ tempdir.path }}/soft3.txt' + mode: 0640 + state: 'link' + owner: '{{ remote_unprivileged_user }}' + force: yes + follow: false + register: missing_dst_no_follow_enable_force_use_mode3 + always: + - name: Delete remote unprivileged remote user + user: + name: '{{ remote_unprivileged_user }}' + state: absent + + - name: Delete unprivileged user home and tempdir + file: + path: "{{ item }}" + state: absent + loop: + - '{{ tempdir.path }}' + - '{{ user.home }}' + +- name: verify that link was created + assert: + that: + - "missing_dst_no_follow_enable_force_use_mode1 is changed" + - "missing_dst_no_follow_enable_force_use_mode2 is changed" + - "missing_dst_no_follow_enable_force_use_mode3 is not changed" + - "soft3_result['stat'].islnk" + - "soft3_result['stat'].lnk_target == '{{ user.home }}/nonexistent'" + # # Test creating a link to a directory https://github.com/ansible/ansible/issues/1369 # diff --git a/test/units/module_utils/basic/test_set_mode_if_different.py b/test/units/module_utils/basic/test_set_mode_if_different.py index 406b194aead..93fe2467f18 100644 --- a/test/units/module_utils/basic/test_set_mode_if_different.py +++ b/test/units/module_utils/basic/test_set_mode_if_different.py @@ -7,9 +7,16 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import errno import os + from itertools import product +try: + import builtins +except ImportError: + import __builtin__ as builtins + import pytest @@ -141,3 +148,36 @@ def test_missing_lchmod_is_link(am, mock_stats, mocker, monkeypatch, check_mode) mocker.resetall() mocker.stopall() + + +@pytest.mark.parametrize('stdin,', + ({},), + indirect=['stdin']) +def test_missing_lchmod_is_link_in_sticky_dir(am, mock_stats, mocker): + """Some platforms have lchmod (*BSD) others do not (Linux)""" + + am.check_mode = False + original_hasattr = hasattr + + def _hasattr(obj, name): + if obj == os and name == 'lchmod': + return False + return original_hasattr(obj, name) + + mock_lstat = mocker.MagicMock() + mock_lstat.st_mode = 0o777 + + mocker.patch('os.lstat', side_effect=[mock_lstat, mock_lstat]) + mocker.patch.object(builtins, 'hasattr', side_effect=_hasattr) + mocker.patch('os.path.islink', return_value=True) + mocker.patch('os.path.exists', return_value=True) + m_stat = mocker.patch('os.stat', side_effect=OSError(errno.EACCES, 'Permission denied')) + m_chmod = mocker.patch('os.chmod', return_value=None) + + # not changed: can't set mode on symbolic links + assert not am.set_mode_if_different('/path/to/file/no_lchmod', 0o660, False) + m_stat.assert_called_with(b'/path/to/file/no_lchmod') + m_chmod.assert_not_called() + + mocker.resetall() + mocker.stopall()