NVIDIA / container-canary

A tool for testing and validating container requirements against versioned manifests
Apache License 2.0
245 stars 15 forks source link

permissions of docker #69

Open Spuxy opened 1 month ago

Spuxy commented 1 month ago

Do we really need to check this permission by docker ps ? And what does it mean by root privileges ? The exec command can return any exit code, which doesnt need to be just privileges.

if _, err := exec.Command("docker", "ps").Output(); err != nil {
    return errors.New("Docker requires root privileges to run")
}
jacobtomlinson commented 1 month ago

I think that error needs updating to say something like You need permission to run the docker command, the check here is just that you are able to use the docker CLI because canary will call this under the hood.

Spuxy commented 1 month ago

Great ! But maybe we could go further and check this better way ? eg. check that the process was executed under uid/guid and we can check, that if he has the group docker as upstream's doc of docker recommend.

This exec.Command runs the sh (or variant of shell) command and the os.Exit can be anything not just permission.

jacobtomlinson commented 1 month ago

I would be very happy to change this check to something more robust. If you have ideas of how things can be done better please feel free to make a Pull Request with your suggestion and ping me for review.