Docker sets environment variable from --env-file to zero-length-string that was meant to be taken if exported only

[ktomk]

This is a bug report I could not find the string " --env-file" with the issue search provided by this project (for opened and closed issues).

Expected behavior

Docker run only takes over exported environment variables values.

Actual behavior

Docker run creates the environment variable when it was not exported.

Steps to reproduce the behavior

Create a environment file that contains a line w/ a variable name but no equality sign , e.g.:

$ cat env.list
# This is a comment
VAR1=value1
VAR2=value2
VAR3

Then execute the run command, print out all variables that start with VAR:

$ docker run --env-file env.list busybox printenv | grep ^VAR
VAR1=value1
VAR2=value2
VAR3=

As the output shows, the variable VAR3 is set to an empty string even though it was not exported on the host:

$ printenv | grep ^VAR | wc -l
0

Output of docker version:

$ docker version
Client:
 Version:	18.03.0-ce
 API version:	1.37
 Go version:	go1.9.4
 Git commit:	0520e24
 Built:	Wed Mar 21 23:10:01 2018
 OS/Arch:	linux/amd64
 Experimental:	false
 Orchestrator:	swarm

Server:
 Engine:
  Version:	18.03.0-ce
  API version:	1.37 (minimum version 1.12)
  Go version:	go1.9.4
  Git commit:	0520e24
  Built:	Wed Mar 21 23:08:31 2018
  OS/Arch:	linux/amd64
  Experimental:	false

Output of docker info:

$ docker info   
Containers: 41
 Running: 0
 Paused: 0
 Stopped: 41
Images: 823
Server Version: 18.03.0-ce
Storage Driver: overlay2
 Backing Filesystem: extfs
 Supports d_type: true
 Native Overlay Diff: true
Logging Driver: json-file
Cgroup Driver: cgroupfs
Plugins:
 Volume: local
 Network: bridge host macvlan null overlay
 Log: awslogs fluentd gcplogs gelf journald json-file logentries splunk syslog
Swarm: inactive
Runtimes: runc
Default Runtime: runc
Init Binary: docker-init
containerd version: cfd04396dc68220d1cecbe686a6cc3aa5ce3667c
runc version: 4fc53a81fb7c994640722ac585fa9ca548971871
init version: 949e6fa
Security Options:
 apparmor
 seccomp
  Profile: default
Kernel Version: 4.13.0-38-generic
Operating System: Ubuntu 16.04.4 LTS
OSType: linux
Architecture: x86_64
CPUs: 4
Total Memory: 15.34GiB
Name: monolith
ID: KTJC:3KLO:VUDV:SY2Y:FWNX:5GSM:7KGI:NJ4X:KQT4:PH75:Y2BJ:OKMT
Docker Root Dir: /var/lib/docker
Debug Mode (client): false
Debug Mode (server): false
Username: vicampotk
Registry: https://index.docker.io/v1/
Labels:
Experimental: false
Insecure Registries:
 127.0.0.0/8
Live Restore Enabled: false

WARNING: No swap limit support

References:

• Set environment variables (-e, --env, --env-file) | docker run | Docker Documentation

[ktomk]

I could spot the cause in the code-base. In components/cli/opts/file.go:63 that lookup--for-if-empty-function emptyFn is os.Getenv (via: components/cli/opts/envfile.go:L21), and that os.Gentenv returns an empty string if the variable is not set for the only return value it has. So the not-set condition gets unnoticed and instead of not importing the variable into the new environment it is created with a zero-length string value. The go docs actually name that case and note on it:

Getenv retrieves the value of the environment variable named by the key. It returns the value, which will be empty if the variable is not present. To distinguish between an empty value and an unset value, use LookupEnv.

(from go docs, highlight and link by me)

As os.LookupEnv has a much better interface for the return values (which can be used to find out if a variable was not set) it looks more appropriate as it would allow to deal with this case.

Additionally while reviewing the code, it looks like it is prone to not detecting zero-length variable names. It should either bail out in such a case (e.g. ErrBadKey on it) or skip that line. POSIX does not even talk about name=value pairs in which the name is a zero-length string (that is nonexistent), so I would really bail out on that one on the parser level and not pass this as an environment variable that could be interpreted as starting with the equal sign to the new environment (as a name shall not contain "=").

docker: Error response from daemon: invalid environment variable: =XXX@@@.

is a follow-up error message I can provoke when tampering with it.

I guess this should be addressed on its own, but didn't wanted to get this finding undocumented while stumbling over it.

[ktomk]

I could provide a patch which addresses both issues, just tested it.

First of all the reason of the original report:

 ./docker run --env-file env.list busybox printenv | grep ^VAR
VAR1=value1
VAR2=value2

And then for the zero-length variable name:

./docker run --env-file test.env busybox printenv | grep ^VAR
./docker: poorly formatted environment: no variable name on line '=XXX@@@'.
See './docker run --help'.

$ cat env.list
VAR1=value1
VAR2=value2
VAR3

$ cat test.env
VAR3
VAR1=value1
VAR2=value2
=XXX@@@