search

scylladb / scylla-cqlsh

A fork of the cqlsh code
Apache License 2.0
16 stars 32 forks source link

build: erase uid/gid information from tar archives #36

Closed avikivity closed 1 year ago

avikivity commented 1 year ago

The default behavior of tar is to record the uid/gid information, and then restore it. This is problematic if the archive is unpacked as root, since it will create files with the uid/gid of the user who created the archive.

If we build under podman, this normally isn't a problem, since podman runs as root. But under docker we run with the same uid as the caller, and of course it's possible to build without a container at all.

The way I encountered it was when I built both with and without podman. The first build, without podman, copied by uid into the archive. The second build extracted the archive, expanding my uid into podman's subuid range. These files can then no longer be deleted by my original user.

The fix is to add a filter that erases all user information from the tar file as they are added.

Note this indicates a dependency problem - these archives should always be created afresh since the build system doesn't track dependencies for real. This problem is not fixed in this patch.

avikivity commented 1 year ago

/cc @syuu1228

fruch commented 1 year ago

@avikivity other places we have the same code doesn't suffer for this ? (scylla-core, scylla-jmx, scylla-tools-java)

avikivity commented 1 year ago

@avikivity other places we have the same code doesn't suffer for this ? (scylla-core, scylla-jmx, scylla-tools-java)

Because of this:

Note this indicates a dependency problem - these archives should always be created afresh since the build system doesn't track dependencies for real. This problem is not fixed in this patch.

When the dependency problem is fixed, either the package is created and extracted with podman, or created and extracted without it. The package will still have the wrong uid, but they won't leak into the filesystem.

The bug should be fixed everywhere (and the dependency problem should be fixed here).