podman cp can overwrite existing files/directories

[Luap99]

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

using podman cp incorrect can lead to very easy data loss.
Currently it is possible to overwrite an existing file/directory on the host without any warnings.

I noticed this while looking at #14393.
So my question is why is this an option, and why it is not the default? I cannot imagine that somebody actually would want this and if they do it is simple to just remove the file/directory beforehand.

Steps to reproduce the issue:

podman run --name test alpine touch /test1
$ bin/podman cp ^C
$ mkdir test1
$ bin/podman cp test:/test1 .
$ ls -l test1 
-rw-r--r--. 1 pholzing pholzing 0 May 30 17:36 test1

$ podman run --name test alpine mkdir /test1
$ touch test1
$ bin/podman cp test:/test1 .
$ ls -ld test1
drwxr-xr-x. 1 pholzing pholzing 0 May 30 17:39 test1

Describe the results you received:
The original file/directory is gone.

Describe the results you expected:
An error like the one from cp:
cp: cannot overwrite non-directory './test1' with directory 'test/test1'

Additional information you deem important (e.g. issue happens only occasionally):
Looks like we regressed on this one, I already reported this a long time ago: #7790

Also I verified that docker cp works as I would expect and errors correctly in both cases.

[vrothberg]

It screams for regression tests.

[vrothberg]

I think we need to provide a flag for that on the CLI and let it default to the Docker behavior. There is a risk that we may break certain use cases; having a flag will allow for a quick work around.

@rhatdan @baude WDYT?

[Luap99]

An option is fine for me but the default should be to not overwrite.

[rhatdan]

SGTM

[vrothberg]

@nalind, buildah's copy options NoOverwriteDirNonDir blocks dirs from being replaced with files. But we need the same the other way around. Do you want a new option for that or can we extend NoOverwriteDirNonDir?

[nalind]

It needs to be a separate option. This logic backends our handling of COPY and ADD instructions in Dockerfiles, and we definitely don't want to change how they behave.

[nalind]

Correcting myself: we don't set the flag for ADD/COPY, so overloading it wouldn't break them. It's still my preference.

[vrothberg]

-> containers/buildah#4043

[vrothberg]

-> #14526