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

Use snakeyaml 2.0 [#16412]. #16415

Merged
merged 4 commits into from
Mar 13, 2023
Merged

Use snakeyaml 2.0 [#16412]. #16415

merged 4 commits into from
Mar 13, 2023

Conversation

atriantafyllos-da
Copy link
Contributor

@atriantafyllos-da atriantafyllos-da commented Feb 28, 2023 •
edited
Loading

#16412

Snakeyaml 1.33, which is knowingly vulnerable, was pulled in by circe-yaml (14.2). Upgrading to 15.0RC1 pulls snakeyaml 2.0 which has not any known vulnerabilities atm.
For more see: #16412

@atriantafyllos-da atriantafyllos-da self-assigned this Feb 28, 2023
tudor-da
tudor-da approved these changes Feb 28, 2023
View reviewed changes
Copy link
Contributor

@tudor-da tudor-da left a comment •
edited
Loading

Choose a reason for hiding this comment

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

🤞 On another note, I guess we need to update in Canton as well as it's using the same circe dep version

@tudor-da
Copy link
Contributor

Ah I was afraid of this. this forced update of snakeyaml breaks binary compatibility (see NoSuchMethod errors in logs)

@atriantafyllos-da
Copy link
Contributor Author

Ah I was afraid of this. this forced update of snakeyaml breaks binary compatibility (see NoSuchMethod errors in logs)

Yes I am trying to see if upgrading io.circe:circe-yaml to 0.14.3-RC3 will solve the problem. @tudor-da @mziolekda wdyt?

@tudor-da
Copy link
Contributor

Ah I was afraid of this. this forced update of snakeyaml breaks binary compatibility (see NoSuchMethod errors in logs)

Yes I am trying to see if upgrading io.circe:circe-yaml to 0.14.3-RC3 will solve the problem. @tudor-da @mziolekda wdyt?

Unfortunately, that one is still relying on snakeyaml 1.33

@tudor-da tudor-da closed this Feb 28, 2023
@tudor-da tudor-da reopened this Feb 28, 2023
@atriantafyllos-da
Copy link
Contributor Author

atriantafyllos-da commented Feb 28, 2023 •
edited
Loading

Ah I was afraid of this. this forced update of snakeyaml breaks binary compatibility (see NoSuchMethod errors in logs)

Yes I am trying to see if upgrading io.circe:circe-yaml to 0.14.3-RC3 will solve the problem. @tudor-da @mziolekda wdyt?

Unfortunately, that one is still relying on snakeyaml 1.33

Is not there a possibility that it will work? If the API has not change dramatically?

@atriantafyllos-da
Copy link
Contributor Author

@mziolekda Can 2.6 be shipped without resolving this issue?

@mziolekda mziolekda mentioned this pull request Feb 28, 2023
Open
@mziolekda
Copy link
Contributor

Unfortunately, it cannot go out without it, we'll wait for the circe upgrade which hopefully will come any hour/day

@atriantafyllos-da atriantafyllos-da force-pushed the snakeyaml-2_0 branch 2 times, most recently from 7396f0e to 3517792 Compare March 3, 2023 09:08
@atriantafyllos-da
Copy link
Contributor Author

Upgraded circe-yaml to 15.0-RC1.

@atriantafyllos-da atriantafyllos-da changed the title Fix snakeyaml to 2.0 [#16412]. Use snakeyaml to 2.0 [#16412]. Mar 13, 2023
@atriantafyllos-da atriantafyllos-da changed the title Use snakeyaml to 2.0 [#16412]. Use snakeyaml 2.0 [#16412]. Mar 13, 2023
@mziolekda
Copy link
Contributor

mziolekda commented Mar 13, 2023 •
edited
Loading

Thanks @atriantafyllos-da, as there is no official version still, I am accepting this PR that introduces version 0.15.0-RC1 of circe-yaml. It is better to be on an unofficial patched release than to be on an official but vulnerable.

@atriantafyllos-da atriantafyllos-da merged commit e58ea28 into main Mar 13, 2023
@atriantafyllos-da atriantafyllos-da deleted the snakeyaml-2_0 branch March 13, 2023 13:51
atriantafyllos-da added a commit that referenced this pull request Mar 13, 2023
@atriantafyllos-da
Upgraded circe-yaml to 15.0-RC1. [#16412]. (#16415)
4210702
@atriantafyllos-da atriantafyllos-da mentioned this pull request Mar 13, 2023
Merged
mziolekda pushed a commit that referenced this pull request Mar 13, 2023
@atriantafyllos-da
Upgraded circe-yaml to 15.0-RC1. [#16412]. (#16415) (#16528)
47fa513
yves-da added a commit that referenced this pull request Jan 16, 2024
@yves-da
Fix canton-3x Bazel rules for 'Moved http health to node and migrated…
2dc23b0 
… health based tests to X nodes #16415'
paulbrauner-da pushed a commit that referenced this pull request Jan 16, 2024
@azure-pipelines @tudor-da
@yves-da @atriantafyllos-da @samuel-williams-da
update canton to 20240115.12269.0.vee57276e/2.9.0-snapshot.20240115.1…
c965857 
…1704.0.v67e3d1e3/3.0.0-snapshot.20240115.12269.0.vee57276e (#18183)

* update canton to 20240115.12269.0.vee57276e/2.9.0-snapshot.20240115.11704.0.v67e3d1e3/3.0.0-snapshot.20240115.12269.0.vee57276e

tell-slack: canton

* Adapt changes after changing of Decode.assertDecodeInfoPackage signature

* Fix canton-3x Bazel rules for 'Moved http health to node and migrated health based tests to X nodes #16415'

* Fixed GetEventsByContractIdResponse misalign in v2.

* Removed unused dependency.

* Removed unused dependency #2.

* Fix daml3-script upgrade querying. Disable the test as more work needed.

---------

Co-authored-by: Azure Pipelines Daml Build <[email protected]>
Co-authored-by: Tudor Voicu <[email protected]>
Co-authored-by: Yves Geissbuehler <[email protected]>
Co-authored-by: Andreas Triantafyllos <[email protected]>
Co-authored-by: Samuel Williams <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mziolekda mziolekda mziolekda approved these changes

@remyhaemmerle-da remyhaemmerle-da Awaiting requested review from remyhaemmerle-da

@tudor-da tudor-da Awaiting requested review from tudor-da

Assignees

@atriantafyllos-da atriantafyllos-da

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@atriantafyllos-da @tudor-da @mziolekda