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

Destinations - vulnerability patching for libraries #44620

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Vee7574
Copy link
Contributor

@Vee7574 Vee7574 commented Aug 23, 2024

What

Updating the version of dependencies that have been flagged with vulnerabilities for these issues:

https://github.com/airbytehq/airbyte-internal-issues/issues/8880
https://github.com/airbytehq/airbyte-internal-issues/issues/8881

How

Reviewed the flagged dependencies and upgraded to higher versions that include fixes for the security issues

Review guide

Review the libraries with the versions being upgraded

User Impact

This should not ideally have any impact on users. Need to test the new connectors for any issues between versions of dependent libraries, before merging to master.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

@Vee7574 Vee7574 requested review from a team as code owners August 23, 2024 23:18
Copy link

vercel bot commented Aug 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Sep 6, 2024 7:26pm

@@ -14,7 +14,7 @@ application {
}

dependencies {
implementation 'org.postgresql:postgresql:42.6.0'
implementation 'org.postgresql:postgresql:42.7.2'
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the vulnerability list source-cockroachdb too ? It should filter on certified destinations i thought

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the change for source-cockroachdb

@johnny-schmidt
Copy link
Contributor

@gisripa , does he need to bump the versions on all the affected connectors? (And/or version the affected CDKs?)

@johnny-schmidt
Copy link
Contributor

@gisripa , does he need to bump the versions on all the affected connectors? (And/or version the affected CDKs?)

For example, if I run the scan manually, I see a critical vulnerability in netty < 4.0. I still see version 3.10.6 in the class path for s3. But if I set useLocalCdk = true, I see that it's picking up netty = 4.1.100

@stephane-airbyte
Copy link
Contributor

@gisripa , does he need to bump the versions on all the affected connectors? (And/or version the affected CDKs?)

For example, if I run the scan manually, I see a critical vulnerability in netty < 4.0. I still see version 3.10.6 in the class path for s3. But if I set useLocalCdk = true, I see that it's picking up netty = 4.1.100

yes, we need to bump the CDK version, and also the connector versions (and update the relevant READMEs

@@ -27,12 +27,12 @@ application {
dependencies {

// csv
implementation 'com.amazonaws:aws-java-sdk-s3:1.11.978'
implementation 'com.amazonaws:aws-java-sdk-s3:1.12.261'
Copy link
Contributor

Choose a reason for hiding this comment

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

how did you get 12.261? That was released over 2 years ago according to https://mvnrepository.com/artifact/com.amazonaws/aws-java-sdk-s3
Last version is 12.770

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the version to 12.770

Copy link
Contributor

Choose a reason for hiding this comment

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

there's other places where we're still using 261, I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the version to 12.770 for the redshift destination also

@stephane-airbyte
Copy link
Contributor

I think it would be nice to use the latest version of each of those library, rather than the 1st version where the current vuln has been fix. Chances are there's another vuln in that version that's fixed in the next and so on. I'd rather directly jump to the latest jar version. I checked 3 libraries, and all had newer version available for months or years.

@Vee7574
Copy link
Contributor Author

Vee7574 commented Sep 6, 2024

Sure, I have upgraded the vulnerable libraries to the latest versions

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.

5 participants