Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove any pipe files before zip/unzipping #1133

Merged
merged 4 commits into from Oct 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions ansible_runner/utils/streaming.py
Expand Up @@ -32,6 +32,11 @@ def stream_dir(source_directory, stream):
permissions |= 0xA000
zip_info.external_attr = permissions << 16
archive.writestr(zip_info, os.readlink(full_path))
elif stat.S_ISFIFO(os.stat(full_path).st_mode):
# skip any pipes, as python hangs when attempting
# to open them.
# i.e. ssh_key_data that was never cleaned up
continue
else:
archive.write(
os.path.join(dirpath, fname), arcname=os.path.join(relpath, fname)
Expand Down Expand Up @@ -81,6 +86,12 @@ def unstream_dir(stream, length, target_directory):
if os.path.exists(out_path):
if is_symlink:
os.remove(out_path)
elif stat.S_ISFIFO(os.stat(out_path).st_mode):
# remove any pipes, as python hangs when attempting
# to open them.
# i.e. ssh_key_data that was never cleaned up
os.remove(out_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one makes more sense to delete as a safety, esp since it will only trigger when the archive contains a file with the same name/path- seems a lot less likely to stomp an arbitrary socket file laying around that might be in use...

continue
elif os.path.isdir(out_path):
# Special case, the important dirs were pre-created so don't try to chmod them
continue
Expand Down
45 changes: 45 additions & 0 deletions test/unit/utils/test_utils.py
Expand Up @@ -4,6 +4,7 @@
import os
import signal
import time
import stat

from pathlib import Path

Expand Down Expand Up @@ -113,6 +114,50 @@ def test_transmit_symlink(tmp_path, symlink_dest, check_content):
assert f.read() == 'hello world'


@pytest.mark.timeout(timeout=3)
def test_stream_dir_no_hang_on_pipe(tmp_path):
# prepare the input private_data_dir directory to zip
pdd = tmp_path / 'timeout_test'
pdd.mkdir()

with open(pdd / 'ordinary_file.txt', 'w') as f:
f.write('hello world')

# make pipe, similar to open_fifo_write
os.mkfifo(pdd / 'my_pipe', stat.S_IRUSR | stat.S_IWUSR)

# zip and stream the data into the in-memory buffer outgoing_buffer
outgoing_buffer = io.BytesIO()
outgoing_buffer.name = 'not_stdout'
stream_dir(pdd, outgoing_buffer)


@pytest.mark.timeout(timeout=3)
def test_unstream_dir_no_hang_on_pipe(tmp_path):
# prepare the input private_data_dir directory to zip
pdd = tmp_path / 'timeout_test_source_dir'
pdd.mkdir()

with open(pdd / 'ordinary_file.txt', 'w') as f:
f.write('hello world')

# zip and stream the data into the in-memory buffer outgoing_buffer
outgoing_buffer = io.BytesIO()
outgoing_buffer.name = 'not_stdout'
stream_dir(pdd, outgoing_buffer)

dest_dir = tmp_path / 'timeout_test_dest'
dest_dir.mkdir()

# We create the pipe in the same location as an archived file to trigger the bug
os.mkfifo(dest_dir / 'ordinary_file.txt', stat.S_IRUSR | stat.S_IWUSR)

outgoing_buffer.seek(0)
first_line = outgoing_buffer.readline()
size_data = json.loads(first_line.strip())
unstream_dir(outgoing_buffer, size_data['zipfile'], dest_dir)


@pytest.mark.parametrize('fperm', [
0o777,
0o666,
Expand Down