Re: [PATCH v2] cp: Add --keep-directory-symlink option

[Daan De Meyer]

> > When recursively copying files into OS trees, it often happens that
> > some subdirectory of the source directory is a symlink in the target
> > directory. Currently, cp will fail in that scenario with the error:
> >
> > "cannot overwrite non-directory %s with directory %s"
> >
> > However, we'd like cp in this scenario to follow the destination
> > directory symlink and copy the files into the symlinked directory
> > instead. Let's support this by adding a new option
> > --keep-directory-symlink that makes cp follow destination directory
> > symlinks.
> >
> > We name the option --keep-directory-symlink to keep consistent with
> > tar which has the same option with the same effect.
>
> Just checking, this is the same v2 you sent earlier?

Yes it is the same v2. It seems to have been forgotten so I'm resending it.

Cheers,

Daan

On Wed, 21 Feb 2024 at 16:47, Dragan Simic <dsimic@manjaro.org> wrote:

> Hello Daan,
>
> On 2024-02-21 13:37, Daan De Meyer wrote:
> > When recursively copying files into OS trees, it often happens that
> > some subdirectory of the source directory is a symlink in the target
> > directory. Currently, cp will fail in that scenario with the error:
> >
> > "cannot overwrite non-directory %s with directory %s"
> >
> > However, we'd like cp in this scenario to follow the destination
> > directory symlink and copy the files into the symlinked directory
> > instead. Let's support this by adding a new option
> > --keep-directory-symlink that makes cp follow destination directory
> > symlinks.
> >
> > We name the option --keep-directory-symlink to keep consistent with
> > tar which has the same option with the same effect.
>
> Just checking, this is the same v2 you sent earlier?
>
> > ---
> >  doc/coreutils.texi                 |  8 ++++++++
> >  src/copy.c                         |  3 ++-
> >  src/copy.h                         |  3 +++
> >  src/cp.c                           | 12 +++++++++++-
> >  tests/cp/keep-directory-symlink.sh | 27 +++++++++++++++++++++++++++
> >  5 files changed, 51 insertions(+), 2 deletions(-)
> >  create mode 100755 tests/cp/keep-directory-symlink.sh
> >
> > diff --git a/doc/coreutils.texi b/doc/coreutils.texi
> > index d445ea228..07f05b8af 100644
> > --- a/doc/coreutils.texi
> > +++ b/doc/coreutils.texi
> > @@ -10281,6 +10281,14 @@ option is also specified.
> >  @opindex --verbose
> >  Print the name of each file before moving it.
> >
> > +@item --keep-directory-symlink
> > +@opindex --keep-directory-symlink
> > +Follow existing symlinks to directories when copying. Note that this
> > option
> > +should only be used when the contents of the destination directory are
> > trusted
> > +as when this option is enabled, an attacker can place symlinks in the
> > +destination directory to make @command{cp} write to arbitrary
> > directories in the
> > +system.
> > +
> >  @optStripTrailingSlashes
> >
> >  @optBackupSuffix
> > diff --git a/src/copy.c b/src/copy.c
> > index f54253e5b..bbadca293 100644
> > --- a/src/copy.c
> > +++ b/src/copy.c
> > @@ -2311,7 +2311,8 @@ copy_internal (char const *src_name, char const
> > *dst_name,
> >            bool use_lstat
> >              = ((! S_ISREG (src_mode)
> >                  && (! x->copy_as_regular
> > -                    || S_ISDIR (src_mode) || S_ISLNK (src_mode)))
> > +                    || (S_ISDIR (src_mode) &&
> > !x->keep_directory_symlink)
> > +                    || S_ISLNK (src_mode)))
> >                 || x->move_mode || x->symbolic_link || x->hard_link
> >                 || x->backup_type != no_backups
> >                 || x->unlink_dest_before_opening);
> > diff --git a/src/copy.h b/src/copy.h
> > index 3809f8d23..834685c51 100644
> > --- a/src/copy.h
> > +++ b/src/copy.h
> > @@ -256,6 +256,9 @@ struct cp_options
> >    /* If true, display the names of the files before copying them. */
> >    bool verbose;
> >
> > +  /* If true, follow existing symlinks to directories when copying. */
> > +  bool keep_directory_symlink;
> > +
> >    /* If true, display details of how files were copied.  */
> >    bool debug;
> >
> > diff --git a/src/cp.c b/src/cp.c
> > index 04a5cbee3..82c7bd964 100644
> > --- a/src/cp.c
> > +++ b/src/cp.c
> > @@ -68,7 +68,8 @@ enum
> >    REFLINK_OPTION,
> >    SPARSE_OPTION,
> >    STRIP_TRAILING_SLASHES_OPTION,
> > -  UNLINK_DEST_BEFORE_OPENING
> > +  UNLINK_DEST_BEFORE_OPENING,
> > +  KEEP_DIRECTORY_SYMLINK_OPTION
> >  };
> >
> >  /* True if the kernel is SELinux enabled.  */
> > @@ -141,6 +142,7 @@ static struct option const long_opts[] =
> >    {"target-directory", required_argument, nullptr, 't'},
> >    {"update", optional_argument, nullptr, 'u'},
> >    {"verbose", no_argument, nullptr, 'v'},
> > +  {"keep-directory-symlink", no_argument, nullptr,
> > KEEP_DIRECTORY_SYMLINK_OPTION},
> >    {GETOPT_SELINUX_CONTEXT_OPTION_DECL},
> >    {GETOPT_HELP_OPTION_DECL},
> >    {GETOPT_VERSION_OPTION_DECL},
> > @@ -230,6 +232,9 @@ Copy SOURCE to DEST, or multiple SOURCE(s) to
> > DIRECTORY.\n\
> >  "), stdout);
> >        fputs (_("\
> >    -v, --verbose                explain what is being done\n\
> > +"), stdout);
> > +      fputs (_("\
> > +      --keep-directory-symlink  follow existing symlinks to
> > directories\n\
> >  "), stdout);
> >        fputs (_("\
> >    -x, --one-file-system        stay on this file system\n\
> > @@ -859,6 +864,7 @@ cp_option_init (struct cp_options *x)
> >
> >    x->update = false;
> >    x->verbose = false;
> > +  x->keep_directory_symlink = false;
> >
> >    /* By default, refuse to open a dangling destination symlink,
> > because
> >       in general one cannot do that safely, give the current semantics
> > of
> > @@ -1161,6 +1167,10 @@ main (int argc, char **argv)
> >            x.verbose = true;
> >            break;
> >
> > +        case KEEP_DIRECTORY_SYMLINK_OPTION:
> > +          x.keep_directory_symlink = true;
> > +          break;
> > +
> >          case 'x':
> >            x.one_file_system = true;
> >            break;
> > diff --git a/tests/cp/keep-directory-symlink.sh
> > b/tests/cp/keep-directory-symlink.sh
> > new file mode 100755
> > index 000000000..cf3918b79
> > --- /dev/null
> > +++ b/tests/cp/keep-directory-symlink.sh
> > @@ -0,0 +1,27 @@
> > +#!/bin/sh
> > +# Test that cp --keep-directory-symlink follows symlinks.
> > +
> > +# Copyright (C) 2024 Free Software Foundation, Inc.
> > +
> > +# This program is free software: you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation, either version 3 of the License, or
> > +# (at your option) any later version.
> > +
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program.  If not, see
> > <https://www.gnu.org/licenses/>.
> > +
> > +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
> > +print_ver_ cp
> > +
> > +mkdir -p a/b b/d/e || framework_failure_
> > +ln -s b a/d || framework_failure_
> > +
> > +cp -RT --copy-contents b a || fail=1
> > +cp -RT --copy-contents --keep-directory-symlink b a || fail=1
> > +ls a/b/e || fail=1
>