Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] unexpected character "-" in variable name since v2.29.3 #12123

Closed
bwaldvogel opened this issue Sep 13, 2024 · 18 comments · Fixed by compose-spec/compose-go#683
Closed

[BUG] unexpected character "-" in variable name since v2.29.3 #12123

bwaldvogel opened this issue Sep 13, 2024 · 18 comments · Fixed by compose-spec/compose-go#683

Comments

@bwaldvogel
Copy link

Description

Environment variables defined via an env_file that have "-" characters in their name get refused after updating docker-compose from v2.29.2 to v2.29.3

Steps To Reproduce

services:
  env_runner:
    image: alpine
    command: env
    env_file:
      - ./myenv.env

myenv.env

foo-bar=test

Expected behaviour (v2.29.2)

$ docker compose up
[+] Running 1/1
 ✔ Container env_runner-1      Created
Attaching to env_runner-1
[…]
env_runner-1  | foo-bar=test
env_runner-1 exited with code 0

Actual behaviour (v2.29.3)

$ docker compose up
failed to read myenv.env: line 1: unexpected character "-" in variable name "foo-bar=test"

Compose Version

Docker Compose version 2.29.3

Docker Environment

Client:
 Version:           27.2.0
 API version:       1.47
 Go version:        go1.23.0
 Git commit:        3ab4256958
 Built:             Thu Aug 29 16:44:26 2024
 OS/Arch:           linux/amd64
 Context:           default

Server:
 Engine:
  Version:          27.2.0
  API version:      1.47 (minimum version 1.24)
  Go version:       go1.23.0
  Git commit:       3ab5c7d003
  Built:            Thu Aug 29 16:44:26 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          v1.7.22
  GitCommit:        7f7fdf5fed64eb6a7caf99b3e12efcf9d60e311c.m
 runc:
  Version:          1.1.14
  GitCommit:        
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Anything else?

No response

@thaJeztah
Copy link
Member

While technically Linux can use env-vars with hyphens as name (any character except nul terminators), it's generally recommended to only use characters from the portable character set; https://pubs.opengroup.org/onlinepubs/7908799/xbd/envvar.html

Screenshot 2024-09-13 at 14 27 14

What I suspect happened here is that the libary used by compose possibly considers them as invalid formatted substitution character, and the part after the hyphen as default; https://tldp.org/LDP/abs/html/parameter-substitution.html

Screenshot 2024-09-13 at 14 24 52

That said, env_file for services should follow the same rules as --env-file on docker run (which only considers = as separator, and performs no substitution/validation otherwise 🤔

@thaJeztah
Copy link
Member

thaJeztah commented Sep 13, 2024

This looks related to this PR;

However the env_file (docker run --env-file) should not do interpolation; is there a way to make that conditional, @ndeloof ?

docker run --rm --env "hello-world=hello-value" alpine printenv hello-world
hello-value

For reference; this is the validation in the OCI runtime (runc); https://github.com/opencontainers/runc/blob/7c2e69f1c4a7df1cf4690ccfc68c0a3857251d12/libcontainer/init_linux.go#L258-L281

// populateProcessEnvironment loads the provided environment variables into the
// current processes's environment.
func populateProcessEnvironment(env []string) error {
	for _, pair := range env {
		p := strings.SplitN(pair, "=", 2)
		if len(p) < 2 {
			return errors.New("invalid environment variable: missing '='")
		}
		name, val := p[0], p[1]
		if name == "" {
			return errors.New("invalid environment variable: name cannot be empty")
		}
		if strings.IndexByte(name, 0) >= 0 {
			return fmt.Errorf("invalid environment variable %q: name contains nul byte (\\x00)", name)
		}
		if strings.IndexByte(val, 0) >= 0 {
			return fmt.Errorf("invalid environment variable %q: value contains nul byte (\\x00)", name)
		}
		if err := os.Setenv(name, val); err != nil {
			return err
		}
	}
	return nil
}

@ndeloof
Copy link
Contributor

ndeloof commented Sep 13, 2024

env-file when used in compose does support interpolation (and a bunch of our users rely on this feature)

@thaJeztah
Copy link
Member

Right, but the reverse also looks to be true; it's rejecting env-vars that should be considered valid 🤔

@ndeloof
Copy link
Contributor

ndeloof commented Sep 16, 2024

Environment variable names used by the utilities in the Shell and Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase letters, digits, and the '_' (underscore) from the characters defined in Portable Character Set and do not begin with a digit. Other characters may be permitted by an implementation;

So - MAY be permitted, and I can guess some will rely on it. To get actual support of this, we will have to make the dotenv parser way more flexible that it is 😓

@thaJeztah
Copy link
Member

thaJeztah commented Sep 16, 2024

Yeah, I recall we once added validation for environment-variables, but then had to revert, for similar reasons;

@bwaldvogel
Copy link
Author

When will the fix be published? I’ve tested docker compose v2.29.7 and I still get the same error.

@bwaldvogel
Copy link
Author

It seems to work now with docker compose v2.29.7. Not sure which other library update did the trick?

@ndeloof
Copy link
Contributor

ndeloof commented Oct 22, 2024

fix took place in compose-go library

@jamessan85
Copy link

I'm still facing this issue, compose version is 2.29.7, is there something else I should update?

@ndeloof
Copy link
Contributor

ndeloof commented Nov 14, 2024

character - is not supported in variable names, as this collides with default value syntax (${FOO-default_value}) and makes parsing non-deterministic

@jamessan85
Copy link

I'm confused, I thought earlier comments said this was fixed, it's been working before the update.

@ndeloof
Copy link
Contributor

ndeloof commented Nov 14, 2024

yes, we tried to propose a fix for this issue, but this triggered more issues as variables with a - in name can't be used: they get parsed as name-default and there's no possible workaround.

@ndeloof
Copy link
Contributor

ndeloof commented Nov 14, 2024

see compose-spec/compose-go#663

@eschrewe
Copy link

For those affected by this issue:

A possible workaround is to remove the respective variables from the .env file and instead apply them via the "environment"

services:
  env_runner:
    image: alpine
    command: env
    environment: 
      foo-bar: test
    env_file:
      - ./myenv.env

@masatana
Copy link

masatana commented Nov 24, 2024

Is it correct to say that it was fixed by compose-spec/compose-go#683 (released in v2.30.0)?

@achinaou
Copy link

achinaou commented Dec 9, 2024

I just upgraded to Docker Compose 2.31.0, and it works.

@jakeaturner
Copy link

Can confirm upgrading Docker Compose solves this. (docker-compose-plugin 2.32.1-1ubuntu.24.04noble)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants