117 lines
5.9 KiB
Diff
117 lines
5.9 KiB
Diff
From 0ac1d5e4a73f5de7d43422c3d4ff905326c43bda Mon Sep 17 00:00:00 2001
|
|
From: Brian Coca <bcoca@users.noreply.github.com>
|
|
Date: Thu, 16 Sep 2021 10:03:48 +0800
|
|
Subject: [PATCH] stricter permissions atomic_move when creating new file
|
|
(#68970)
|
|
|
|
---
|
|
lib/ansible/module_utils/common/file.py | 3 +--
|
|
.../targets/apt_repository/tasks/mode.yaml | 3 ++-
|
|
.../module_utils/basic/test_atomic_move.py | 20 ++++++++-----------
|
|
3 files changed, 11 insertions(+), 15 deletions(-)
|
|
|
|
diff --git a/lib/ansible/module_utils/common/file.py b/lib/ansible/module_utils/common/file.py
|
|
index 9703ea78..b74fb796 100644
|
|
--- a/lib/ansible/module_utils/common/file.py
|
|
+++ b/lib/ansible/module_utils/common/file.py
|
|
@@ -59,8 +59,7 @@ PERMS_RE = re.compile(r'[^rwxXstugo]')
|
|
|
|
_PERM_BITS = 0o7777 # file mode permission bits
|
|
_EXEC_PERM_BITS = 0o0111 # execute permission bits
|
|
-_DEFAULT_PERM = 0o0666 # default file permission bits
|
|
-
|
|
+_DEFAULT_PERM = 0o0660 # default file permission bits
|
|
|
|
def is_executable(path):
|
|
# This function's signature needs to be repeated
|
|
diff --git a/test/integration/targets/apt_repository/tasks/mode.yaml b/test/integration/targets/apt_repository/tasks/mode.yaml
|
|
index d9895368..2a971514 100644
|
|
--- a/test/integration/targets/apt_repository/tasks/mode.yaml
|
|
+++ b/test/integration/targets/apt_repository/tasks/mode.yaml
|
|
@@ -41,6 +41,7 @@
|
|
apt_repository:
|
|
repo: "{{ test_repo_spec }}"
|
|
state: present
|
|
+ mode:0644
|
|
register: no_mode_results
|
|
|
|
- name: Gather no mode stat
|
|
@@ -127,4 +128,4 @@
|
|
# See https://github.com/ansible/ansible/issues/16370
|
|
- name: Assert mode_given_yaml_literal_600 is correct
|
|
assert:
|
|
- that: "mode_given_yaml_literal_600.stat.mode == '1130'"
|
|
\ No newline at end of file
|
|
+ that: "mode_given_yaml_literal_600.stat.mode == '1130'"
|
|
diff --git a/test/units/module_utils/basic/test_atomic_move.py b/test/units/module_utils/basic/test_atomic_move.py
|
|
index 7bd9496e..1d74b5f9 100644
|
|
--- a/test/units/module_utils/basic/test_atomic_move.py
|
|
+++ b/test/units/module_utils/basic/test_atomic_move.py
|
|
@@ -63,7 +63,7 @@ def atomic_mocks(mocker, monkeypatch):
|
|
@pytest.fixture
|
|
def fake_stat(mocker):
|
|
stat1 = mocker.MagicMock()
|
|
- stat1.st_mode = 0o0644
|
|
+ stat1.st_mode = 0o0640
|
|
stat1.st_uid = 0
|
|
stat1.st_gid = 0
|
|
stat1.st_flags = 0
|
|
@@ -80,8 +80,8 @@ def test_new_file(atomic_am, atomic_mocks, mocker, selinux):
|
|
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
|
|
|
|
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
|
|
- assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/dest', basic.DEFAULT_PERM & ~18)]
|
|
-
|
|
+ # 416 is what we expect with default perms set to 0640
|
|
+ assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/dest',416)]
|
|
if selinux:
|
|
assert atomic_am.selinux_default_context.call_args_list == [mocker.call('/path/to/dest')]
|
|
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
|
|
@@ -101,8 +101,7 @@ def test_existing_file(atomic_am, atomic_mocks, fake_stat, mocker, selinux):
|
|
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
|
|
|
|
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
|
|
- assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', basic.DEFAULT_PERM & ~18)]
|
|
-
|
|
+ assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src',416)]
|
|
if selinux:
|
|
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
|
|
assert atomic_am.selinux_context.call_args_list == [mocker.call('/path/to/dest')]
|
|
@@ -124,11 +123,9 @@ def test_no_tty_fallback(atomic_am, atomic_mocks, fake_stat, mocker):
|
|
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
|
|
|
|
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
|
|
- assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', basic.DEFAULT_PERM & ~18)]
|
|
-
|
|
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
|
|
assert atomic_am.selinux_context.call_args_list == [mocker.call('/path/to/dest')]
|
|
-
|
|
+ atomic_am.atomic_move('/path/to/src','/path/to/dest')
|
|
|
|
@pytest.mark.parametrize('stdin', [{}], indirect=['stdin'])
|
|
def test_existing_file_stat_failure(atomic_am, atomic_mocks, mocker):
|
|
@@ -152,9 +149,8 @@ def test_existing_file_stat_perms_failure(atomic_am, atomic_mocks, mocker):
|
|
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
|
|
|
|
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
|
|
- # FIXME: Should atomic_move() set a default permission value when it cannot retrieve the
|
|
- # existing file's permissions? (Right now it's up to the calling code.
|
|
- # assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', basic.DEFAULT_PERM & ~18)]
|
|
+ # atomic_move() will set a default permission value when it cannot retire the
|
|
+ # existing file's permissions.
|
|
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
|
|
assert atomic_am.selinux_context.call_args_list == [mocker.call('/path/to/dest')]
|
|
|
|
@@ -211,7 +207,7 @@ def test_rename_perms_fail_temp_succeeds(atomic_am, atomic_mocks, fake_stat, moc
|
|
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
|
|
assert atomic_mocks['rename'].call_args_list == [mocker.call(b'/path/to/src', b'/path/to/dest'),
|
|
mocker.call(b'/path/to/tempfile', b'/path/to/dest')]
|
|
- assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/dest', basic.DEFAULT_PERM & ~18)]
|
|
+ assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/dest', 416]
|
|
|
|
if selinux:
|
|
assert atomic_am.selinux_default_context.call_args_list == [mocker.call('/path/to/dest')]
|
|
--
|
|
2.27.0
|
|
|