Commit 4e450529 authored by Jürg Billeter's avatar Jürg Billeter
Browse files

utils.py: Change _ensure_real_directory() to not resolve symlinks 

Resolving symlinks during staging causes various issues:

* Split rules may not work properly as the resolved paths will differ
  depending on whether another artifact with a directory symlink has
  been staged in the same root directory or not, e.g., as part of
  compose.

* The order of symlinks in file lists is difficult to get right to
  guarantee consistent and predictable behavior as paths in a file list
  might rely on symlinks in the same file list. See #647 and #817.

* Staging order differences can lead to surprising results. See #390.

* Difficult to properly support absolute symlinks. Absolute symlinks are
  currently converted to relative symlinks, however, this doesn't always
  work. See #606 and #830.

This will require changes in projects that rely on the current behavior.
However, the changes are expected to be small and are often a sign of
buggy element files. E.g., elements that don't fully obey `bindir` or
`sbindir` variables.
parent f2172e3b

buildstream/utils.py

+22 −38
Original line number Diff line number Diff line
@@ -772,32 +772,23 @@ def _resolve_symlinks(path): 
    return os.path.realpath(path)





def _ensure_real_directory(root, destpath):

    # The realpath in the sandbox may refer to a file outside of the

    # sandbox when any of the direcory branches are a symlink to an

    # absolute path.

# _ensure_real_directory()

#

    # This should not happen as we rely on relative_symlink_target() below

    # when staging the actual symlinks which may lead up to this path.

# Ensure `path` is a real directory and there are no symlink components.

#

    destpath_resolved = _resolve_symlinks(destpath)

    if not destpath_resolved.startswith(_resolve_symlinks(root)):

        raise UtilError('Destination path resolves to a path outside ' +

                        'of the staging area\n\n' +

                        '  Destination path: {}\n'.format(destpath) +

                        '  Real path: {}'.format(destpath_resolved))



    # Ensure the real destination path exists before trying to get the mode

    # of the real destination path.

    #

    # It is acceptable that chunks create symlinks inside artifacts which

    # refer to non-existing directories, they will be created on demand here

    # at staging time.

# Symlink components are allowed in `root`.

#

    if not os.path.exists(destpath_resolved):

        os.makedirs(destpath_resolved)



    return destpath_resolved

def _ensure_real_directory(root, path):

    destpath = root

    for name in os.path.split(path):

        destpath = os.path.join(destpath, name)

        try:

            deststat = os.lstat(destpath)

            if not stat.S_ISDIR(deststat.st_mode):

                relpath = destpath[len(root):]

                raise UtilError('Destination is not a directory: {}'.format(relpath))

        except FileNotFoundError:

            os.makedirs(destpath)





# _process_list()
@@ -836,6 +827,10 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result, 
        srcpath = os.path.join(srcdir, path)

        destpath = os.path.join(destdir, path)



        # Ensure that the parent of the destination path exists without symlink

        # components.

        _ensure_real_directory(destdir, os.path.dirname(path))



        # Add to the results the list of files written

        if report_written:

            result.files_written.append(path)
@@ -847,11 +842,6 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result, 
        # The destination directory may not have been created separately

        permissions.extend(_copy_directories(srcdir, destdir, path))



        # Ensure that broken symlinks to directories have their targets

        # created before attempting to stage files across broken

        # symlink boundaries

        _ensure_real_directory(destdir, os.path.dirname(destpath))



        try:

            file_stat = os.lstat(srcpath)

            mode = file_stat.st_mode
@@ -865,13 +855,7 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result, 


        if stat.S_ISDIR(mode):

            # Ensure directory exists in destination

            if not os.path.exists(destpath):

                _ensure_real_directory(destdir, destpath)



            dest_stat = os.lstat(_resolve_symlinks(destpath))

            if not stat.S_ISDIR(dest_stat.st_mode):

                raise UtilError('Destination not a directory. source has {}'

                                ' destination has {}'.format(srcpath, destpath))

            _ensure_real_directory(os.path.dirname(destpath), os.path.basename(path))

            permissions.append((destpath, os.stat(srcpath).st_mode))



        elif stat.S_ISLNK(mode):

tests/integration/compose-symlinks.py

+3 −8
Original line number Diff line number Diff line
@@ -18,7 +18,7 @@ DATA_DIR = os.path.join( 
)





# Test that staging a file inside a directory symlink works as expected.

# Test that staging a file inside a directory symlink fails.

#

# Regression test for https://gitlab.com/BuildStream/buildstream/issues/270

@pytest.mark.datafiles(DATA_DIR)
@@ -34,11 +34,6 @@ def test_compose_symlinks(cli, tmpdir, datafiles): 
    os.symlink(os.path.join('usr', 'sbin'), symlink_file, target_is_directory=True)



    result = cli.run(project=project, args=['build', 'compose-symlinks/compose.bst'])

    result.assert_success()



    result = cli.run(project=project, args=['artifact', 'checkout', 'compose-symlinks/compose.bst',

                                            '--directory', checkout])

    result.assert_success()



    assert set(walk_dir(checkout)) == set(['/sbin', '/usr', '/usr/sbin',

                                           '/usr/sbin/init', '/usr/sbin/dummy'])
 
    assert result.exit_code == -1

    assert 'Destination is not a directory' in result.stderr

tests/integration/symlinks.py

+4 −6
Original line number Diff line number Diff line
@@ -44,7 +44,7 @@ def test_absolute_symlinks_made_relative(cli, tmpdir, datafiles): 


@pytest.mark.datafiles(DATA_DIR)

@pytest.mark.skipif(not HAVE_SANDBOX, reason='Only available with a functioning sandbox')

def test_allow_overlaps_inside_symlink_with_dangling_target(cli, tmpdir, datafiles):

def test_disallow_overlaps_inside_symlink_with_dangling_target(cli, tmpdir, datafiles):

    project = os.path.join(datafiles.dirname, datafiles.basename)

    checkout = os.path.join(cli.directory, 'checkout')

    element_name = 'symlinks/dangling-symlink-overlap.bst'
@@ -53,10 +53,8 @@ def test_allow_overlaps_inside_symlink_with_dangling_target(cli, tmpdir, datafil 
    assert result.exit_code == 0



    result = cli.run(project=project, args=['artifact', 'checkout', element_name, '--directory', checkout])

    assert result.exit_code == 0



    # See the dangling-symlink*.bst elements for details on what we are testing.

    assert_contains(checkout, ['/usr/orgs/orgname/etc/org.conf'])

    assert result.exit_code == -1

    assert 'Destination is not a directory' in result.stderr





@pytest.mark.datafiles(DATA_DIR)
@@ -75,4 +73,4 @@ def test_detect_symlink_overlaps_pointing_outside_sandbox(cli, tmpdir, datafiles 
    # tries to actually write there.

    result = cli.run(project=project, args=['artifact', 'checkout', element_name, '--directory', checkout])

    assert result.exit_code == -1

    assert "Destination path resolves to a path outside of the staging area" in result.stderr
 
    assert 'Destination is not a directory' in result.stderr