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.

Sign up for GitHub

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

Fix another cornercase with symlink handling #1166

Merged
merged 1 commit into from
Jan 22, 2024
Merged

Fix another cornercase with symlink handling #1166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 45 additions & 21 deletions repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def _mkdir_with_copied_mode(path, mode_from):

def _choose_copy_or_link(symlink, srcdir):
"""
Copy file contents or create a symlink depending on where the pointee resides.
Determine whether to copy file contents or create a symlink depending on where the pointee resides.

:param symlink: The source symlink to follow. This must be an absolute path.
:param srcdir: The root directory that every piece of content must be present in.
Expand Down Expand Up @@ -415,7 +415,7 @@ def _choose_copy_or_link(symlink, srcdir):
# To make comparisons, we need to resolve all symlinks in the directory
# structure leading up to pointee. However, we can't include pointee
# itself otherwise it will resolve to the file that it points to in the
# end.
# end (which would be wrong if pointee_filename is a symlink).
canonical_pointee_dir, pointee_filename = os.path.split(pointee_as_abspath)
canonical_pointee_dir = os.path.realpath(canonical_pointee_dir)

Expand Down Expand Up @@ -454,6 +454,35 @@ def _choose_copy_or_link(symlink, srcdir):
return ('copy', pointee_as_abspath)


def _copy_symlinks(symlinks_to_process, srcdir):
"""
Copy file contents or create a symlink depending on where the pointee resides.

:param symlinks_to_process: List of 2-tuples of (src_path, target_path). Each src_path
should be an absolute path to the symlink. target_path is the path to where we
need to create either a link or a copy.
:param srcdir: The root directory that every piece of content must be present in.
:raises ValueError: if the arguments are not correct
"""
for source_linkpath, target_linkpath in symlinks_to_process:
try:
action, source_path = _choose_copy_or_link(source_linkpath, srcdir)
except BrokenSymlinkError as e:
# Skip and report broken symlinks
api.current_logger().warning('{} Will not copy the file!'.format(str(e)))
continue

if action == "copy":
# Note: source_path could be a directory, so '-a' or '-r' must be
# given to cp.
run(['cp', '-a', source_path, target_linkpath])
elif action == 'link':
run(["ln", "-s", source_path, target_linkpath])
else:
# This will not happen unless _copy_or_link() has a bug.
raise RuntimeError("Programming error: _copy_or_link() returned an unknown action:{}".format(action))


def _copy_decouple(srcdir, dstdir):
"""
Copy files inside of `srcdir` to `dstdir` while decoupling symlinks.
Expand All @@ -467,7 +496,6 @@ def _copy_decouple(srcdir, dstdir):
.. warning::
`dstdir` must already exist.
"""
symlinks_to_process = []
for root, directories, files in os.walk(srcdir):
# relative path from srcdir because srcdir is replaced with dstdir for
# the copy.
Expand All @@ -476,11 +504,24 @@ def _copy_decouple(srcdir, dstdir):
# Create all directories with proper permissions for security
# reasons (Putting private data into directories that haven't had their
# permissions set appropriately may leak the private information.)
symlinks_to_process = []
for directory in directories:
source_dirpath = os.path.join(root, directory)
target_dirpath = os.path.join(dstdir, relpath, directory)

# Defer symlinks until later because we may end up having to copy
# the file contents and the directory may not exist yet.
if os.path.islink(source_dirpath):
symlinks_to_process.append((source_dirpath, target_dirpath))
continue

_mkdir_with_copied_mode(target_dirpath, source_dirpath)

# Link or create all directories that were pointed to by symlinks and
# then reset symlinks_to_process for use by files.
_copy_symlinks(symlinks_to_process, srcdir)
symlinks_to_process = []

for filename in files:
source_filepath = os.path.join(root, filename)
target_filepath = os.path.join(dstdir, relpath, filename)
Expand All @@ -494,24 +535,7 @@ def _copy_decouple(srcdir, dstdir):
# Not a symlink so we can copy it now too
run(['cp', '-a', source_filepath, target_filepath])

# Now process all symlinks
for source_linkpath, target_linkpath in symlinks_to_process:
try:
action, source_path = _choose_copy_or_link(source_linkpath, srcdir)
except BrokenSymlinkError as e:
# Skip and report broken symlinks
api.current_logger().warning('{} Will not copy the file!'.format(str(e)))
continue

if action == "copy":
# Note: source_path could be a directory, so '-a' or '-r' must be
# given to cp.
run(['cp', '-a', source_path, target_linkpath])
elif action == 'link':
run(["ln", "-s", source_path, target_linkpath])
else:
# This will not happen unless _copy_or_link() has a bug.
raise RuntimeError("Programming error: _copy_or_link() returned an unknown action:{}".format(action))
_copy_symlinks(symlinks_to_process, srcdir)


def _copy_certificates(context, target_userspace):
Expand Down
128 changes: 128 additions & 0 deletions ...em_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,70 @@ def temp_directory_layout(tmp_path, initial_structure):
},
id="Absolute_symlink_to_a_file_inside_via_a_symlink_to_the_rootdir"
)),
# This should be fixed but not necessarily for this release.
# It makes sure that when we have two separate links to the
# same file outside of /etc/pki, one of the links is copied
# as a real file and the other is made a link to the copy.
# (Right now, the real file is copied in place of both links.)
# (pytest.param(
# {
# 'dir': {
# 'fileA': '/outside/fileC',
# 'fileB': '/outside/fileC',
# },
# 'outside': {
# 'fileC': None,
# },
# },
# {
# 'dir': {
# 'fileA': None,
# 'fileB': '/dir/fileA',
# },
# },
# id="Absolute_two_symlinks_to_the_same_copied_file"
# )),
(pytest.param(
{
'dir': {
'fileA': None,
'link_to_dir': '/dir/inside',
'inside': {
'fileB': None,
},
},
},
{
'dir': {
'fileA': None,
'link_to_dir': '/dir/inside',
'inside': {
'fileB': None,
},
},
},
id="Absolute_symlink_to_a_dir_inside"
)),
(pytest.param(
{
'dir': {
'fileA': None,
'link_to_dir': '/outside',
},
'outside': {
'fileB': None,
},
},
{
'dir': {
'fileA': None,
'link_to_dir': {
'fileB': None,
},
},
},
id="Absolute_symlink_to_a_dir_outside"
)),
(pytest.param(
# This one is very tricky:
# * The user has made /etc/pki a symlink to some other directory that
Expand Down Expand Up @@ -671,6 +735,70 @@ def temp_directory_layout(tmp_path, initial_structure):
},
id="Relative_symlink_to_a_file_inside_via_a_symlink_to_the_rootdir"
)),
# This should be fixed but not necessarily for this release.
# It makes sure that when we have two separate links to the
# same file outside of /etc/pki, one of the links is copied
# as a real file and the other is made a link to the copy.
# (Right now, the real file is copied in place of both links.)
# (pytest.param(
# {
# 'dir': {
# 'fileA': '../outside/fileC',
# 'fileB': '../outside/fileC',
# },
# 'outside': {
# 'fileC': None,
# },
# },
# {
# 'dir': {
# 'fileA': None,
# 'fileB': 'fileA',
# },
# },
# id="Relative_two_symlinks_to_the_same_copied_file"
# )),
(pytest.param(
{
'dir': {
'fileA': None,
'link_to_dir': '../outside',
},
'outside': {
'fileB': None,
},
},
{
'dir': {
'fileA': None,
'link_to_dir': {
'fileB': None,
},
},
},
id="Relative_symlink_to_a_dir_outside"
)),
(pytest.param(
{
'dir': {
'fileA': None,
'link_to_dir': 'inside',
'inside': {
'fileB': None,
},
},
},
{
'dir': {
'fileA': None,
'link_to_dir': 'inside',
'inside': {
'fileB': None,
},
},
},
id="Relative_symlink_to_a_dir_inside"
)),
(pytest.param(
# This one is very tricky:
# * The user has made /etc/pki a symlink to some other directory that
Expand Down
Loading