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

ARROW-16759: [Go] update testify to get security patch for gopkg.in/yaml.v3 (v7) #13322

Conversation

dominicbarnes
Copy link
Contributor

@dominicbarnes dominicbarnes commented Jun 6, 2022

This PR updates the github.com/stretchr/testify dependency to get a security patch for gopkg.in/yaml.v3 which has a DoS exploit. See stretchr/testify#1192 for more details.

I'm unsure how this project handles security patches for appears to be older versions. I'm here because I have dependencies that rely on v7, so that's what is bringing me here to make this very particular change. It looks like v6.0.0 and v6.0.1 tags exist, so I expect merging this here and tagging v7.0.1 would be the path forward. If not, let me know what would be preferred.

The linked Jira issue also calls out v8.0.0 as having the same vulnerability, but that would need to be addressed in it's own PR.

@github-actions
Copy link

github-actions bot commented Jun 6, 2022

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@dominicbarnes dominicbarnes changed the title go: update testify to get security patch for gopkg.in/yaml.v3 ARROW-16759: update testify to get security patch for gopkg.in/yaml.v3 Jun 6, 2022
@github-actions
Copy link

github-actions bot commented Jun 6, 2022

@github-actions
Copy link

github-actions bot commented Jun 6, 2022

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@dominicbarnes dominicbarnes changed the title ARROW-16759: update testify to get security patch for gopkg.in/yaml.v3 ARROW-16759: [Go] update testify to get security patch for gopkg.in/yaml.v3 Jun 6, 2022
@dominicbarnes dominicbarnes changed the title ARROW-16759: [Go] update testify to get security patch for gopkg.in/yaml.v3 ARROW-16759: [Go] update testify to get security patch for gopkg.in/yaml.v3 (v7) Jun 6, 2022
@zeroshade
Copy link
Member

@dominicbarnes Please run go mod tidy in the directory so that it properly updates the go.sum file also and add that to this PR. You can also change this to merge to master, and we can cherry-pick the change into the other tagged release branches for those patch releases.

As for backporting the change to the previous versions, can you send an email to the arrow dev mailing list to propose patch releases of v6.0.2, v7.0.1, and v8.0.1? Link this PR in the email please. Unfortunately the release process, even for patch releases like this, is still a manual process that needs to get approved by the PMC, so the best route here would be to bring the discussion up there and then we can move forward. Thanks!

@dominicbarnes dominicbarnes changed the base branch from release-7.0.0 to master June 9, 2022 20:23
@dominicbarnes
Copy link
Contributor Author

@zeroshade I've rebased the PR, which seems to have included some JS changes during the process, though once I've completed there are no longer changes to JS.

Once merged, I'll send that email and start the process of getting this patch backported.

@zeroshade
Copy link
Member

@dominicbarnes Perfect, i'll merge this and reply to the email to help start the process of backporting the patch. Thanks!

@zeroshade zeroshade closed this in 6af8b47 Jun 10, 2022
@ursabot
Copy link

ursabot commented Jun 10, 2022

Benchmark runs are scheduled for baseline = 1de30af and contender = 6af8b47. 6af8b47 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.66% ⬆️0.07%] test-mac-arm
[Finished ⬇️0.71% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.6% ⬆️0.26%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 6af8b472 ec2-t3-xlarge-us-east-2
[Finished] 6af8b472 test-mac-arm
[Finished] 6af8b472 ursa-i9-9960x
[Finished] 6af8b472 ursa-thinkcentre-m75q
[Finished] 1de30af0 ec2-t3-xlarge-us-east-2
[Failed] 1de30af0 test-mac-arm
[Finished] 1de30af0 ursa-i9-9960x
[Finished] 1de30af0 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@ursabot
Copy link

ursabot commented Jun 10, 2022

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x

@dominicbarnes dominicbarnes deleted the go-security-patch-testify branch June 10, 2022 16:54
@dominicbarnes
Copy link
Contributor Author

@zeroshade thanks for getting this merged! How can I help getting the fix backported?

@kou
Copy link
Member

kou commented Jul 12, 2022

@dominicbarnes @zeroshade Could you create pull requests against maint-6.0.x, main-7.0.x and maint-8.0.x to release 6.0.2, 7.0.1 and 8.0.1? (I tried it against maint-6.0.x but it causes a conflict.)

We can reuse ARROW-16759 for them. We will add 6.0.2, 7.0.1 and 8.0.1 to the Fixed version/s field.

dominicbarnes added a commit to segment-boneyard/arrow that referenced this pull request Jul 12, 2022
…aml.v3 (v7)

This PR updates the github.com/stretchr/testify dependency to get a security patch for gopkg.in/yaml.v3 which has a DoS exploit. See stretchr/testify#1192 for more details.

I'm unsure how this project handles security patches for appears to be older versions. I'm here because I have dependencies that rely on v7, so that's what is bringing me here to make this very particular change. It looks like v6.0.0 and v6.0.1 tags exist, so I expect merging this here and tagging v7.0.1 would be the path forward. If not, let me know what would be preferred.

The linked Jira issue also calls out v8.0.0 as having the same vulnerability, but that would need to be addressed in it's own PR.

Closes apache#13322 from dominicbarnes/go-security-patch-testify

Authored-by: Dominic Barnes <dominic@dbarnes.info>
Signed-off-by: Matthew Topol <mtopol@factset.com>
dominicbarnes added a commit to segment-boneyard/arrow that referenced this pull request Jul 12, 2022
…aml.v3 (v7)

This PR updates the github.com/stretchr/testify dependency to get a security patch for gopkg.in/yaml.v3 which has a DoS exploit. See stretchr/testify#1192 for more details.

I'm unsure how this project handles security patches for appears to be older versions. I'm here because I have dependencies that rely on v7, so that's what is bringing me here to make this very particular change. It looks like v6.0.0 and v6.0.1 tags exist, so I expect merging this here and tagging v7.0.1 would be the path forward. If not, let me know what would be preferred.

The linked Jira issue also calls out v8.0.0 as having the same vulnerability, but that would need to be addressed in it's own PR.

Closes apache#13322 from dominicbarnes/go-security-patch-testify

Authored-by: Dominic Barnes <dominic@dbarnes.info>
Signed-off-by: Matthew Topol <mtopol@factset.com>
dominicbarnes added a commit to segment-boneyard/arrow that referenced this pull request Jul 12, 2022
…aml.v3 (v7)

This PR updates the github.com/stretchr/testify dependency to get a security patch for gopkg.in/yaml.v3 which has a DoS exploit. See stretchr/testify#1192 for more details.

I'm unsure how this project handles security patches for appears to be older versions. I'm here because I have dependencies that rely on v7, so that's what is bringing me here to make this very particular change. It looks like v6.0.0 and v6.0.1 tags exist, so I expect merging this here and tagging v7.0.1 would be the path forward. If not, let me know what would be preferred.

The linked Jira issue also calls out v8.0.0 as having the same vulnerability, but that would need to be addressed in it's own PR.

Closes apache#13322 from dominicbarnes/go-security-patch-testify

Authored-by: Dominic Barnes <dominic@dbarnes.info>
Signed-off-by: Matthew Topol <mtopol@factset.com>
@dominicbarnes
Copy link
Contributor Author

@kou see the linked PRs above, let me know if this is what you were expecting.

kou pushed a commit that referenced this pull request Jul 13, 2022
)

This PR is a backport of #13322 for v6.

The cherry-pick did create some merge conflicts that needed to be resolved.

Authored-by: Dominic Barnes <dominic@dbarnes.info>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
kou pushed a commit that referenced this pull request Jul 13, 2022
)

This PR is a backport of #13322 for v7.

Authored-by: Dominic Barnes <dominic@dbarnes.info>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
kou pushed a commit that referenced this pull request Jul 13, 2022
)

This PR is a backport of #13322 for v8.

Authored-by: Dominic Barnes <dominic@dbarnes.info>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
dominicbarnes added a commit to segment-boneyard/arrow that referenced this pull request Jul 14, 2022
…aml.v3 (v7)

This PR updates the github.com/stretchr/testify dependency to get a security patch for gopkg.in/yaml.v3 which has a DoS exploit. See stretchr/testify#1192 for more details.

I'm unsure how this project handles security patches for appears to be older versions. I'm here because I have dependencies that rely on v7, so that's what is bringing me here to make this very particular change. It looks like v6.0.0 and v6.0.1 tags exist, so I expect merging this here and tagging v7.0.1 would be the path forward. If not, let me know what would be preferred.

The linked Jira issue also calls out v8.0.0 as having the same vulnerability, but that would need to be addressed in it's own PR.

Closes apache#13322 from dominicbarnes/go-security-patch-testify

Authored-by: Dominic Barnes <dominic@dbarnes.info>
Signed-off-by: Matthew Topol <mtopol@factset.com>
kou pushed a commit that referenced this pull request Jul 14, 2022
…13606)

This PR is a backport of #13322 for v7.0.1 as a follow-up to #13587

cc @kou 

Authored-by: Dominic Barnes <dominic@dbarnes.info>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants