Skip to content

fix: #1108 - Replace ecdsa with cryptography - #1114

Merged
tiwarishubham635 merged 2 commits into
sendgrid:mainfrom
dacevedo12:main
Sep 11, 2025
Merged

fix: #1108 - Replace ecdsa with cryptography#1114
tiwarishubham635 merged 2 commits into
sendgrid:mainfrom
dacevedo12:main

Conversation

@dacevedo12

Copy link
Copy Markdown
Contributor

Fixes#1108

Fixes

A short description of what this PR does.

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket.

@tiwarishubham635

Copy link
Copy Markdown
Contributor

Thanks for raising this PR. I can review this.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the ecdsa library with the cryptography library for handling ECDSA signature verification in the SendGrid Python SDK. This change affects the event webhook functionality that validates incoming webhook signatures from SendGrid.

Key changes:

  • Replaces ecdsa dependency with cryptography in package requirements
  • Updates imports and signature verification logic in the EventWebhook class
  • Updates documentation references across README and CONTRIBUTING files

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
setup.pyReplaces ecdsa dependency with cryptography>=45.0.6
sendgrid/helpers/eventwebhook/init.pyUpdates signature verification implementation to use cryptography library
README.rstUpdates dependency documentation reference
README.mdUpdates dependency documentation reference
CONTRIBUTING.mdUpdates development dependency reference

Comment threadsendgrid/helpers/eventwebhook/__init__.py Outdated
Comment threadsendgrid/helpers/eventwebhook/__init__.py Outdated
Comment threadsendgrid/helpers/eventwebhook/__init__.py Outdated
@tiwarishubham635

Copy link
Copy Markdown
Contributor

I see the verify signature test is passing. So it seems to be fine. Can you please do the above mentioned changes so that we can merge it? Thanks!

@dacevedo12

Copy link
Copy Markdown
ContributorAuthor

@tiwarishubham635 Fixed

@dacevedo12

Copy link
Copy Markdown
ContributorAuthor

Hey @tiwarishubham635 , is there anything pending to merge this? One would expect security fixes to be a high priority for Twilio...

@dacevedo12

Copy link
Copy Markdown
ContributorAuthor

@tiwarishubham635@twilio-product-security Hi, is there anything I can do to help speed up merging this security fix?

Kind regards,

@yonatan-shorani

Copy link
Copy Markdown

Hi, any update on this PR?
This change fixes a high-severity security vulnerability, and resolving it is important for us.

Thanks!

@maksym-skorupskyimaksym-skorupskyi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This PR successfully addresses the security vulnerabilities in the ecdsa dependency by migrating to the industry-standard cryptography library. The implementation is clean and maintains backward compatibility.

Minor Suggestion:

Version Constraint: Consider if cryptography>=45.0.6 is too restrictive - you might want to allow a broader range like >=45.0.6,<46 to avoid forcing users to upgrade to very recent versions.

@garikkhgarikkh mentioned this pull request Sep 10, 2025
Comment threadsendgrid/helpers/eventwebhook/__init__.py Outdated
@tiwarishubham635
tiwarishubham635 merged commit 6828852 into sendgrid:mainSep 11, 2025
9 checks passed
@RichardTea

Copy link
Copy Markdown

This is a breaking change as it adds a new client requirement for a Rust toolchain that never existed before.

Please can such things be put behind a Major version change, this broke our build as we now have to explicitly mark this "patch" version as incompatible.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CVE for dependency ecdsa

6 participants

@dacevedo12@tiwarishubham635@yonatan-shorani@RichardTea@maksym-skorupskyi