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.

Sign up for GitHub

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

Jump to bottom

Update golang.org/x/crypto and go-mssql dependencies #1479

Merged
merged 1 commit into from
Nov 10, 2022
Merged

Update golang.org/x/crypto and go-mssql dependencies #1479

merged 1 commit into from
Nov 10, 2022

Conversation

gl-johnson
Copy link
Contributor

Desired Outcome

Fix vulnerability in Juxtaposer's golang.org/x/net version by updating upstream dependencies:

  • golang.org/x/crypto
  • go-mssql

Definition of Done

At least 1 todo must be completed in the sections below for the PR to be
merged.

Changelog

  • The CHANGELOG has been updated, or
  • This PR does not include user-facing changes and doesn't require a
    CHANGELOG update

Test coverage

  • This PR includes new unit and integration tests to go with the code
    changes, or
  • The changes in this PR do not require tests

Documentation

  • Docs (e.g. READMEs) were updated in this PR
  • A follow-up issue to update official docs has been filed here: insert issue ID
  • This PR does not require updating any documentation

Behavior

  • This PR changes product behavior and has been reviewed by a PO, or
  • These changes are part of a larger initiative that will be reviewed later, or
  • No behavior was changed with this PR

Security

  • Security architect has reviewed the changes in this PR,
  • These changes are part of a larger initiative with a separate security review, or
  • There are no security aspects to these changes

@gl-johnson gl-johnson force-pushed the fix-net-vulnerability branch from 79f8a4b to 68b960f Compare November 10, 2022 17:03
codeclimate[bot]
codeclimate bot reviewed Nov 10, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@gl-johnson gl-johnson marked this pull request as ready for review November 10, 2022 17:50
@gl-johnson gl-johnson requested a review from a team as a code owner November 10, 2022 17:50
andytinkham
andytinkham previously approved these changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@andytinkham andytinkham left a comment

Choose a reason for hiding this comment

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

This looks good. Snyk can't scan the /go.mod file, so can't say if there are issues there. Should we update that go.mod to point to crypto v0.2.0 at the same time? Approving in case you want to do that as a separate change. Thanks, @gl-johnson!

@gl-johnson gl-johnson force-pushed the fix-net-vulnerability branch from 68b960f to 41d3da1 Compare November 10, 2022 18:41
andytinkham
andytinkham suggested changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@andytinkham andytinkham left a comment

Choose a reason for hiding this comment

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

Thanks! One change needed - in the go.mod, stuff for our libraries needs to keep latest so the auto-updating works. Otherwise, looks good!

go.mod Show resolved Hide resolved
go.mod Outdated
@@ -160,16 +160,16 @@ exclude github.com/emicklei/go-restful v2.9.5+incompatible
replace honnef.co/go/tools => github.com/dominikh/go-tools v0.0.1-2019.2.3

// DO NOT EDIT: CHANGES TO THE BELOW LINE WILL BREAK AUTOMATED RELEASES
replace github.com/cyberark/conjur-api-go => github.com/cyberark/conjur-api-go latest
replace github.com/cyberark/conjur-api-go => github.com/cyberark/conjur-api-go v0.10.1
Copy link
Contributor

Choose a reason for hiding this comment

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

These need to be put back to "latest" after running go mod tidy, so that the build process pulls in the latest version. (also appliaes in line 166, 169, and 172)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woops, good catch. Pushed up the fix!

@gl-johnson gl-johnson force-pushed the fix-net-vulnerability branch from 41d3da1 to 7772644 Compare November 10, 2022 19:09
codeclimate[bot]
codeclimate bot reviewed Nov 10, 2022
View reviewed changes
CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@codeclimate
Copy link

codeclimate bot commented Nov 10, 2022

Code Climate has analyzed commit 7772644 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Style 2

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 70.7% (0.0% change).

View more on Code Climate.

andytinkham
andytinkham approved these changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@andytinkham andytinkham left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @gl-johnson

@gl-johnson gl-johnson merged commit 9a3b482 into main Nov 10, 2022
@gl-johnson gl-johnson deleted the fix-net-vulnerability branch November 10, 2022 20:14
@john-odonnell john-odonnell mentioned this pull request Apr 21, 2023
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeclimate codeclimate[bot] codeclimate[bot] left review comments

@andytinkham andytinkham andytinkham approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gl-johnson @andytinkham