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] Environment variable value cannot contain = #1231

Closed
cfergeau opened this issue Nov 15, 2022 · 3 comments · Fixed by #1232
Closed

[bug] Environment variable value cannot contain = #1231

cfergeau opened this issue Nov 15, 2022 · 3 comments · Fixed by #1232
Labels
area:go Issue related to the Go ecosystem type:bug Something isn't working

Comments

@cfergeau
Copy link
Contributor

Describe the bug
I'm following the steps from https://github.com/slsa-framework/slsa-github-generator/blob/main/internal/builders/go/README.md in order to use SLSA in my project
When building [my project](https://gtihub.com/cfergeau/vfkit/tree/slsa] I want to set CGO_CFLAGS=-mmacosx-version-min=11.0 in the environment.
I added this to my .slsa-goreleaser.yml:

env:
  - CGO_ENABLED=1
  - CGO_CFLAGS=-mmacosx-version-min=11.0

but the 'build dry project' step fails with invalid environment variable: CGO_CFLAGS=-mmacosx-version-min=11.0
https://github.com/cfergeau/vfkit/actions/runs/3469089554/jobs/5795684619

This seems to be caused by this code

es := strings.Split(e, "=")
if len(es) != 2 {
return fmt.Errorf("%w: %s", ErrorInvalidEnvironmentVariable, e)
}
which do not expect the env var line to contain 2 = signs.

To Reproduce
Steps to reproduce the behavior:

  1. Add a env: -CGO_CFLAGS=a=b line to your .slsa-goreleaser-yml
  2. Trigger the slsa github actions workflow

Expected behavior
I think a change such as this one would let me use = in env vars.

diff --git a/internal/builders/go/pkg/config.go b/internal/builders/go/pkg/config.go
index 1df1bf7..eba3e39 100644
--- a/internal/builders/go/pkg/config.go
+++ b/internal/builders/go/pkg/config.go
@@ -167,11 +167,11 @@ func validateVersion(cf *goReleaserConfigFile) error {
 func (r *GoReleaserConfig) setEnvs(cf *goReleaserConfigFile) error {
 	m := make(map[string]string)
 	for _, e := range cf.Env {
-		es := strings.Split(e, "=")
-		if len(es) != 2 {
+		name, value, present := strings.Cut(e, "=")
+		if value == "" || !present {
 			return fmt.Errorf("%w: %s", ErrorInvalidEnvironmentVariable, e)
 		}
-		m[es[0]] = es[1]
+		m[name] = value
 	}
 
 	if len(m) > 0 {
@cfergeau cfergeau added status:triage Issue that has not been triaged type:bug Something isn't working labels Nov 15, 2022
@asraa
Copy link
Collaborator

asraa commented Nov 15, 2022

Thanks! I think your proposed code snippet is correct, and I'm in support of making this change.
@laurentsimon

@laurentsimon
Copy link
Collaborator

+1, good catch and thanks for filing the bug. Do you want to send a PR for this?

@cfergeau
Copy link
Contributor Author

Sure, I'll send a PR!

@ianlewis ianlewis added area:go Issue related to the Go ecosystem and removed status:triage Issue that has not been triaged labels Nov 16, 2022
cfergeau added a commit to cfergeau/slsa-github-generator that referenced this issue Nov 16, 2022
If .slsa-goreleaser.yml contains an env var with multiple '=' signs:
```
env:
  - CGO_ENABLED=1
  - CGO_CFLAGS=-mmacosx-version-min=11.0
```

the 'build dry project' GH workflow step fails with "invalid environment
variable: CGO_CFLAGS=-mmacosx-version-min=11.0"
This is caused by the way the env vars are parsed in GoReleaserConfig:setEnvs
which only allow for a single '=' sign.

This commit fixes this by splitting the env string at the first equal sign,
first part is the env var name and won't contain '=' signs, second part
is its value and can contain any number of '='.

This also adds unit tests for this situation, and for env vars with no
values (`CGO_CFLAGS=`).

This fixes slsa-framework#1231
cfergeau added a commit to cfergeau/slsa-github-generator that referenced this issue Nov 16, 2022
If .slsa-goreleaser.yml contains an env var with multiple '=' signs:
```
env:
  - CGO_ENABLED=1
  - CGO_CFLAGS=-mmacosx-version-min=11.0
```

the 'build dry project' GH workflow step fails with "invalid environment
variable: CGO_CFLAGS=-mmacosx-version-min=11.0"
This is caused by the way the env vars are parsed in GoReleaserConfig:setEnvs
which only allow for a single '=' sign.

This commit fixes this by splitting the env string at the first equal sign,
first part is the env var name and won't contain '=' signs, second part
is its value and can contain any number of '='.

This also adds unit tests for this situation, and for env vars with no
values (`CGO_CFLAGS=`).

This fixes slsa-framework#1231

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
laurentsimon pushed a commit that referenced this issue Nov 16, 2022
If .slsa-goreleaser.yml contains an env var with multiple '=' signs:
```
env:
  - CGO_ENABLED=1
  - CGO_CFLAGS=-mmacosx-version-min=11.0
```

the 'build dry project' GH workflow step fails with "invalid environment
variable: CGO_CFLAGS=-mmacosx-version-min=11.0"
This is caused by the way the env vars are parsed in GoReleaserConfig:setEnvs
which only allow for a single '=' sign.

This commit fixes this by splitting the env string at the first equal sign,
first part is the env var name and won't contain '=' signs, second part
is its value and can contain any number of '='.

This also adds unit tests for this situation, and for env vars with no
values (`CGO_CFLAGS=`).

This fixes #1231

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:go Issue related to the Go ecosystem type:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants