nextflow-io / nextflow

A DSL for data-driven computational pipelines
http://nextflow.io
Apache License 2.0
2.79k stars 633 forks source link

envWhitelist not working with `docker.sudo = true` #5309

Open fntlnz opened 2 months ago

fntlnz commented 2 months ago

Bug report

Expected behavior and actual behavior

When running nextflow with docker.sudo=true and any env var in envWhitelist I expect the env vars to be available inside the process containers, however they are not, see additional context for more details.

Steps to reproduce the problem

Open your nextflow config and write

docker {
    enabled = true
    sudo = true
    envWhitelist = 'AVARTHATSHOULDTOTALLYBETHERE'
}

Now run this pipeline

#!/usr/bin/env nextflow

process testenv {
    container 'ubuntu'

    output:
    path 'out'

    """
    echo "IS IT THERE \${AVARTHATSHOULDTOTALLYBETHERE}" > out
    """
}

workflow {
    testenv | view
}
export AVARTHATSHOULDTOTALLYBETHERE="Hello, I'm here"
nextflow run ./test.nf

Program output

ERROR ~ Error executing process > 'testenv'

Caused by:
  Process `testenv` terminated with an error exit status (1)

Command executed:

  echo "IS IT THERE ${AVARTHATSHOULDTOTALLYBETHERE}" > out

Command exit status:
  1

Command output:
  (empty)

Command error:
  .command.sh: line 2: AVARTHATSHOULDTOTALLYBETHERE: unbound variable

Work dir:
  /tmp/test-nf/work/f6/98c929c696d8b31f6f12f7422a90c4

Tip: when you have fixed the problem you can continue the execution adding the option `-resume` to the run command line

 -- Check '.nextflow.log' file for details

Environment

Additional context

As a best practice, when you install the Docker daemon the installation process suggests you to not allow access to /var/run/docker.sock to your otherwise unprivileged user on the machine. This is because when a user gets added to the docker group it essentially becomes root since things like this can be done docker run -it --privileged --net=host --pid=host debian nsenter -t 1 -n -m -p.

Following this best practice, one can configure Nextflow to ask for privileges when running docker in this way:

docker {
    enabled = true
    sudo = true
    envWhitelist = 'AVARTHATSHOULDTOTALLYBETHERE'
}

This works well indeed, however when used in combination with docker.envWhitelist. Why? Because envWhitelist will share the environment variables with the shell that is starting the sudo command here but the final docker run command does not get the env vars in its /proc/self/environ because they are not automatically passed down by sudo. A possible solution to this problem is to run with sudo -E, however a safer approach would be to forward only the whitelisted env vars.

fntlnz commented 2 months ago

This is the nfx_launch command that it created

nxf_launch() {
    sudo docker run -i --cpu-shares 1024 -e "NXF_TASK_WORKDIR" -e "AVARTHATSHOULDTOTALLYBETHERE" -v /tmp/test-nf/work/aa/de499a28fb8da7677d285fc3941f97:/tmp/test-nf/work/aa/de499a28fb8da7677d285fc3941f97 -w "$NXF_TASK_WORKDIR" --name $NXF_BOXID ubuntu /bin/bash -ue /tmp/test-nf/work/aa/de499a28fb8da7677d285fc3941f97/.command.sh
}

as you can see the sudo command does not pass down the needed env vars.

pditommaso commented 2 months ago

this seems to be a docker limitation?

fntlnz commented 2 months ago

@pditommaso no we just have to compose the nfx_launch script in this way when using sudo

nxf_launch() {
sudo NXF_TASK_WORKDIR="$NXF_TASK_WORKDIR" AVARTHATSHOULDTOTALLYBETHERE="$AVARTHATSHOULDTOTALLYBETHERE" docker run -i --cpu-shares 1024 -e "NXF_TASK_WORKDIR" -e "AVARTHATSHOULDTOTALLYBETHERE" -v /tmp/test-nf/work/aa/de499a28fb8da7677d285fc3941f97:/tmp/test-nf/work/aa/de499a28fb8da7677d285fc3941f97 -w "$NXF_TASK_WORKDIR" --name "$NXF_BOXID" ubuntu /bin/bash -ue /tmp/test-nf/work/aa/de499a28fb8da7677d285fc3941f97/.command.sh
}

instead of this

nxf_launch() {
    sudo docker run -i --cpu-shares 1024 -e "NXF_TASK_WORKDIR" -e "AVARTHATSHOULDTOTALLYBETHERE" -v /tmp/test-nf/work/aa/de499a28fb8da7677d285fc3941f97:/tmp/test-nf/work/aa/de499a28fb8da7677d285fc3941f97 -w "$NXF_TASK_WORKDIR" --name $NXF_BOXID ubuntu /bin/bash -ue /tmp/test-nf/work/aa/de499a28fb8da7677d285fc3941f97/.command.sh
}

and of course do the same thing in any other place where we do the same thing with sudo and the docker cli.

pditommaso commented 2 months ago

sorry don't get the difference

pditommaso commented 2 months ago

Got it, the variable must be passed before the docker command. Likely it can be extended as general case