buildkit icon indicating copy to clipboard operation
buildkit copied to clipboard

[v0.13+] unexpected permissions on COPY'd files

Open tianon opened this issue 1 year ago • 7 comments

When upgrading from BuildKit 0.12 to 0.13, we've got a surprising change in the permission of files that get COPY'd into our images: https://github.com/docker-library/mysql/issues/1069

Previously (on BuildKit 0.12), the permissions of files from COPY config/ /etc/mysql/ was -rw-r--r--, but in 0.13 (and 0.14), the permissions unexpectedly change to -rw-rw-rw-:

$ docker buildx create --name bk-0.12 --driver-opt image=moby/buildkit:v0.12.5
$ docker buildx build --builder bk-0.12 https://github.com/docker-library/mysql.git#319db566ac7fef45c22f3df15ee5e194a7c43259:8.0 --file Dockerfile.debian --load
...
#15 exporting manifest sha256:3a2ad681a06e2d6492f246c29da6f29e28e5c127599bc349340e2e7d080a17b9 done
#15 exporting config sha256:a1e4724c25f314831faf64b92e5518550dbe38b09f5a346c89695c4f8cece893 done
...
$ docker run --rm sha256:a1e4724c25f314831faf64b92e5518550dbe38b09f5a346c89695c4f8cece893 ls -l /etc/mysql/my.cnf
-rw-r--r-- 1 root root 1080 Jun 20 22:31 /etc/mysql/my.cnf
$ docker buildx create --name bk-0.13 --driver-opt image=moby/buildkit:v0.13.2
$ docker buildx build --builder bk-0.13 https://github.com/docker-library/mysql.git#319db566ac7fef45c22f3df15ee5e194a7c43259:8.0 --file Dockerfile.debian --load
...
#16 exporting manifest sha256:54fc97c80362f0f197db2ef97423ca936eda02170c8b6eb3162f84b4132f1384 done
#16 exporting config sha256:471e7a42b881c6b9bc239c1423419ef9270848393c25a7e3083f9f79e8dda9c4 done
...
$ docker run --rm sha256:471e7a42b881c6b9bc239c1423419ef9270848393c25a7e3083f9f79e8dda9c4 ls -l /etc/mysql/my.cnf
-rw-rw-rw- 1 root root 1080 Jun 20 22:34 /etc/mysql/my.cnf
$ docker buildx create --name bk-0.14 --driver-opt image=moby/buildkit:v0.14.1
$ docker buildx build --builder bk-0.14 https://github.com/docker-library/mysql.git#319db566ac7fef45c22f3df15ee5e194a7c43259:8.0 --file Dockerfile.debian --load
...
#16 exporting manifest sha256:801f9f6b8b62bd90cb8f836ef6be6f8e4a1202b56bd4faa794f97b22dfbc49d6 done
#16 exporting config sha256:b470c539d81259b1dc2ee0f0cdd3fb5b692174c187f668b00f8a7ffb553037a1 done
...
$ docker run --rm sha256:b470c539d81259b1dc2ee0f0cdd3fb5b692174c187f668b00f8a7ffb553037a1 ls -l /etc/mysql/my.cnf
-rw-rw-rw- 1 root root 1080 Jun 20 22:36 /etc/mysql/my.cnf

(Obviously, there could be made a simpler/smaller reproducer than this, but this is one I had handy that's already in Git somewhere so it's trivial to let BuildKit do the clone and make certain local permissions aren't a factor.)

tianon avatar Jun 20 '24 22:06 tianon

I think https://github.com/moby/buildkit/pull/2356 was a prior fix for a similar issue, but perhaps it wasn't sufficient or something else in 0.13 lost the fix? (I don't see anything obvious in 0.13's release notes that would explain it, but I'm not sure exactly what to look for since I don't think it was an intentional change :sweat_smile:)

tianon avatar Jun 20 '24 22:06 tianon

I recently was looking at permission issues as well, but there it was the reverse; but there was a difference between buildkit standalone and in docker engine; see

  • https://github.com/moby/buildkit/pull/4936

And most notably; https://github.com/moby/buildkit/pull/4936#issuecomment-2123870982

thaJeztah avatar Jun 20 '24 23:06 thaJeztah

That was added 5+ Years ago though, so not something recent https://github.com/moby/buildkit/commit/7210bf6806999432856be71eccea0b28c4131069

thaJeztah avatar Jun 20 '24 23:06 thaJeztah

Happy to test again if I've done something wrong in my docker buildx creates that would break that (very old) umask line somehow :sweat_smile:

tianon avatar Jun 20 '24 23:06 tianon

d34b2471cd53edd462c2c7097e4de70e26961fa6 is the first bad commit
commit d34b2471cd53edd462c2c7097e4de70e26961fa6
Author: Justin Chadwell <[email protected]>
Date:   Fri Aug 4 12:47:40 2023 +0100

    git: centralize git cli operations

    Move all of the git command line logic into a single object, inspired by
    the object already in buildx.

    The basic implemenation allows for configuring a git cli for a specific
    repository, along with various authorization settings and custom
    binaries. Commands can be run for that repository, and a few helpers are
    provided for accessing data on it - more to come in the future
    hopefully.

    Signed-off-by: Justin Chadwell <[email protected]>

 source/git/source.go            | 248 +++++++++++++++++-----------------------
 source/git/source_test.go       |  21 +++-
 util/gitutil/git_cli.go         | 243 +++++++++++++++++++++++++++++++++++++++
 util/gitutil/git_cli_helpers.go |  44 +++++++
 4 files changed, 411 insertions(+), 145 deletions(-)
 create mode 100644 util/gitutil/git_cli.go
 create mode 100644 util/gitutil/git_cli_helpers.go

https://github.com/moby/buildkit/pull/4106#pullrequestreview-1592606139 🙋

cc @jedevc

tonistiigi avatar Jun 21 '24 06:06 tonistiigi

I would have reported this earlier if the permission behaviour was documented on https://docs.docker.com/reference/dockerfile/#copy.

Request - please document the default chmod permissions for Dockerfile COPY.

grooverdan avatar Jun 28 '24 00:06 grooverdan

/cc @dvdksn on the docs bit ;)

thompson-shaun avatar Jul 01 '24 17:07 thompson-shaun

is 666 a bug then, and the expected file permission for COPY from git context is 600?

dvdksn avatar Jul 02 '24 08:07 dvdksn

is 666 a bug then, and the expected file permission for COPY from git context is 600?

It should be 644 for "regular" files and if the file does have execute bit set/tracked in git, 755 (based on the regular default umask of 0022). Folders copied from the context would also be 755.

yosifkit avatar Jul 02 '24 18:07 yosifkit