Multiple fixes for go-getter v2 #361
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The fix for this is to add -- to the arguments of each hg command,
before any user-input. This indicates the end of optional arguments,
only positional arguments are allowed.
Test Results Before Change
```
~> go test ./... -run=TestHg -v
=== RUN TestHgGetter_impl
--- PASS: TestHgGetter_impl (0.00s)
=== RUN TestHgGetter
--- PASS: TestHgGetter (0.60s)
=== RUN TestHgGetter_branch
--- PASS: TestHgGetter_branch (0.96s)
=== RUN TestHgGetter_GetFile
--- PASS: TestHgGetter_GetFile (0.61s)
=== RUN TestHgGetter_HgArgumentsNotAllowed
=== RUN TestHgGetter_HgArgumentsNotAllowed/arguments_allowed_in_destination
get_hg_test.go:144: Expected no err, got: error running /usr/local/bin/hg:
=== RUN TestHgGetter_HgArgumentsNotAllowed/arguments_passed_into_rev_parameter
get_hg_test.go:163: Expected no err, got: /usr/local/bin/hg exited with 1:
=== RUN TestHgGetter_HgArgumentsNotAllowed/arguments_passed_in_the_repository_URL
get_hg_test.go:182: Expected no err, got: /usr/local/bin/hg exited with 255: hg clone: option -U not recognized
alias 'clone' resolves to unknown command 'false'
--- FAIL: TestHgGetter_HgArgumentsNotAllowed (1.02s)
--- FAIL: TestHgGetter_HgArgumentsNotAllowed/arguments_allowed_in_destination (0.15s)
--- FAIL: TestHgGetter_HgArgumentsNotAllowed/arguments_passed_into_rev_parameter (0.56s)
--- FAIL: TestHgGetter_HgArgumentsNotAllowed/arguments_passed_in_the_repository_URL (0.31s)
FAIL
```
Test Results After Change
```
~> go test ./... -run=TestHg -v
=== RUN TestHgGetter_impl
--- PASS: TestHgGetter_impl (0.00s)
=== RUN TestHgGetter
--- PASS: TestHgGetter (0.61s)
=== RUN TestHgGetter_branch
--- PASS: TestHgGetter_branch (0.99s)
=== RUN TestHgGetter_GetFile
--- PASS: TestHgGetter_GetFile (0.61s)
=== RUN TestHgGetter_HgArgumentsNotAllowed
=== RUN TestHgGetter_HgArgumentsNotAllowed/arguments_allowed_in_destination
=== RUN TestHgGetter_HgArgumentsNotAllowed/arguments_passed_into_rev_parameter
=== RUN TestHgGetter_HgArgumentsNotAllowed/arguments_passed_in_the_repository_URL
--- PASS: TestHgGetter_HgArgumentsNotAllowed (1.37s)
--- PASS: TestHgGetter_HgArgumentsNotAllowed/arguments_allowed_in_destination (0.62s)
--- PASS: TestHgGetter_HgArgumentsNotAllowed/arguments_passed_into_rev_parameter (0.61s)
--- PASS: TestHgGetter_HgArgumentsNotAllowed/arguments_passed_in_the_repository_URL (0.15s)
PASS
```
* Prevent arbitrary file read, path traversal via subdirectory extraction Not opt-in or opt-out, just never allowed. Upwards path traversal is not a subdirectory. *Prevent arbitrary file write via `filename` Not opt-in or opt-out, just never allowed. Upwards path traversal is not a filename in a subdirectory.
The fix for this is a new client request option, DisableSymlinks. When set to true, symlinks are disabled. This prevents the client, likely in combination with the GitGetter, from following a symlink when the subdirectory selection from the checked out repo is a symlink. * Add custom symlink copy error * Add DisableSymlinks as client option Setting DisableSymlinks per request works but must be set on all request made by a client. Adding it as a top-level client config option allows for setting DisableSymlinks for all client.Get requests.
* Add XTerraformGetLimit and XTerraformGetDisabled * Add Multiple new options to limit resource consumption: DoNotCheckHeadFirst, HeadFirstTimeout, ReadTimeout, MaxBytes * Add getter client to context for reuse * Add setters/getters for storing configured getter.Client in a context * Update HttpGetter to use ClientFromContext when available; otherwise use a limited client for supporting X-Terraform-Get request * Refactor HttpGetter function to make it clear when a configured getter.Client is required * Add security section to README
Adding timeout to s3Getter
Add missing timeouts to `S3Getter` and `GCSGetter`
Port security patches from v1 into v2
After change
```
> go test -v ./... -run=TestFile
--- PASS: TestFileGetter_dir (0.00s)
=== RUN TestFileGetter_dirSymlink
--- PASS: TestFileGetter_dirSymlink (0.04s)
=== RUN TestFileGetter_GetFile
--- PASS: TestFileGetter_GetFile (0.00s)
=== RUN TestFileGetter_GetFile_Copy
--- PASS: TestFileGetter_GetFile_Copy (0.01s)
=== RUN TestFileGetter_percent2F
--- PASS: TestFileGetter_percent2F (0.06s)
=== RUN TestFileGetter_Mode_notexist
--- PASS: TestFileGetter_Mode_notexist (0.00s)
=== RUN TestFileGetter_Mode_file
--- PASS: TestFileGetter_Mode_file (0.00s)
=== RUN TestFileGetter_Mode_dir
--- PASS: TestFileGetter_Mode_dir (0.00s)
PASS
> go test -v ./... -run=TestGit
=== RUN TestGitGetter_subdirectory_symlink
get_git_test.go:870: initializing git repo in
--- PASS: TestGitGetter_subdirectory_symlink (5.45s)
=== RUN TestGitGetter_subdirectory_traversal
get_git_test.go:870: initializing git repo in
--- PASS: TestGitGetter_subdirectory_traversal (0.27s)
PASS
```
Before Change
```
~> go test ./...
--- FAIL: TestHttpGetter__XTerraformGetConfiguredGettersBypass (0.01s)
--- FAIL: TestHttpGetter__XTerraformGetConfiguredGettersBypass/configured_getter_for_git_protocol_switch (0.00s)
get_http_test.go:742: http://127.0.0.1:52744/start
get_http_test.go:726: making request
get_http_test.go:903: serving start
get_http_test.go:756: expected download not supported for scheme, got: /usr/local/bin/git exited with -1:
--- FAIL: TestHttpGetter__XTerraformGetConfiguredGettersBypass/configured_getter_for_multiple_protocol_switch (0.00s)
get_http_test.go:742: http://127.0.0.1:52746/start
get_http_test.go:726: making request
get_http_test.go:903: serving start
get_http_test.go:756: expected download not supported for scheme, got: /usr/local/bin/git exited with -1:
FAIL
```
After Change
```
~> go test ./...
ok github.com/hashicorp/go-getter/v2 13.908s
? github.com/hashicorp/go-getter/v2/helper/testing [no test files]
ok github.com/hashicorp/go-getter/v2/helper/url (cached)
```
Co-authored-by: Sylvia Moss <[email protected]>
…-test Update test to work on Windows * Fix up test for HttpGetter to handle git failure due to repository not actually existing.
nywilken
changed the title
|
🤔 Not totally sure why the linux test is failing like this at the moment: === Failed
=== FAIL: . (0.00s)
PASS
testing: open /tmp/go-build409413056/b001/_cover_.out: no such file or directory
FAIL ********************/go-getter/v2 4.094s
DONE 178 tests, 4 skipped, 1 failure in 8.704s
gotestsum: Error: failed to open JUnit file: open /tmp/test-results/go-getter/gotestsum-report.xml: no such file or directory |
picatz
approved these changes
May 19, 2022
This was referenced Mar 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Multiple updates to the go-getter v2 library