Skip to content

Commit

Permalink
AnsibleModule.set_mode_if_different: handle symlink is in a sticky di…
Browse files Browse the repository at this point in the history
…rectory (ansible#45198)

* file: add symlink is in a sticky directory tests
* file: handle symlink in a sticky directory

Co-Authored-By: Sviatoslav Sydorenko <[email protected]>

* Add changelog and fix unit test
The builtins import was removed since it was unused, but it is now needed.
  • Loading branch information
pilou- authored Dec 4, 2020
1 parent 5226ac5 commit b464d18
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -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)
6 changes: 5 additions & 1 deletion lib/ansible/module_utils/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,11 @@ def set_mode_if_different(self, path, mode, changed, diff=None, expand=True):
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
Expand Down
2 changes: 2 additions & 0 deletions test/integration/targets/file/defaults/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
remote_unprivileged_user: tmp_ansible_test_user
82 changes: 82 additions & 0 deletions test/integration/targets/file/tasks/state_link.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
40 changes: 40 additions & 0 deletions test/units/module_utils/basic/test_set_mode_if_different.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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()

0 comments on commit b464d18

Please sign in to comment.