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

Backport fix from https://github.com/hashicorp/go-getter/pull/497 to v2 #498

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

nywilken
Copy link
Member

@nywilken nywilken commented Jun 28, 2024

Recreate git config during update to prevent git config alteration

Related to: #497

@nywilken nywilken changed the base branch from main to v2 June 28, 2024 20:21
@nywilken nywilken changed the title nywilken.backport git update from v1 Backport fix from https://github.com/hashicorp/go-getter/pull/497 to v2 Jun 28, 2024
@nywilken nywilken force-pushed the nywilken.backport-git-update-from-v1 branch from 93b729e to b6203bd Compare June 28, 2024 20:23
The Go module for v2 is 1.19, bumping the base docker image to match the
minimum version ensures go-getter can be compiled and executed on the
container. This change resolves the failing acceptance test for Samba.

```
Run docker exec -i gogetter bash -c "env ACC_SMB_TEST=1 go test -v ./... -run=TestSmb_"
  docker exec -i gogetter bash -c "env ACC_SMB_TEST=1 go test -v ./... -run=TestSmb_"
  shell: /usr/bin/bash -e {0}
  env:
    TEST_RESULTS_PATH: /tmp/test-results
Error: ./get_git.go:366:16: undefined: os.ReadDir
Error: ./get_git_test.go:886:9: undefined: os.WriteFile
Error: ./get_git_test.go:904:22: undefined: os.ReadFile
note: module requires Go 1.19
FAIL	github.com/hashicorp/go-getter/v2 [build failed]
?   	github.com/hashicorp/go-getter/v2/helper/testing	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/go-getter/v2/helper/url	0.006s [no tests to run]
FAIL
Error: Process completed with exit code 2.

```
Copy link
Contributor

@dduzgun-security dduzgun-security left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the backport 🙌

Should we also backport the fix we did here too (maybe in another PR)

Dockerfile Show resolved Hide resolved
@nywilken
Copy link
Member Author

nywilken commented Jul 1, 2024

Thanks a lot for the backport 🙌

Should we also backport the fix we did here too (maybe in another PR)

When presented with this PR in Slack @sylviamoss and @mcollao-hc validated that v2 is not susceptible to the vulnerability because v2 does not have a function called findRemoteDefaultBranch nor does it execute a command similar to exec.CommandContext(ctx, "git", "ls-remote", "--symref", "--", u.String(), "HEAD")

Please advise if your testing is showing different results.

@dduzgun-security
Copy link
Contributor

👋 @nywilken, this looks good thanks a lot for taking care of the backport

@nywilken nywilken merged commit dc7f6bc into v2 Jul 23, 2024
22 checks passed
@nywilken nywilken deleted the nywilken.backport-git-update-from-v1 branch July 23, 2024 20:57
@nywilken
Copy link
Member Author

👋 @nywilken, this looks good thanks a lot for taking care of the backport

I work on getting this released by EoD tomorrow the latest. Thanks for review and gentle nudge.

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

Successfully merging this pull request may close these issues.

None yet

3 participants