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

move to pyproject.toml #1726

Merged
merged 7 commits into from
Oct 24, 2024
Merged

move to pyproject.toml #1726

merged 7 commits into from
Oct 24, 2024

Conversation

oroulet
Copy link
Member

@oroulet oroulet commented Oct 16, 2024

No description provided.

@oroulet oroulet force-pushed the 3.13 branch 6 times, most recently from cc1788d to 959764f Compare October 17, 2024 11:43
rth
rth reviewed Oct 17, 2024
View reviewed changes
pyproject.toml Outdated
"pytz",
"sortedcontainers",
"typing-extensions",
"wait_for2==0.3.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe

Suggested change
"wait_for2==0.3.2",
"wait_for2==0.3.2; python_version < "3.12",

since for Python 3.12+ it will never be imported

opcua-asyncio/asyncua/common/utils.py

Line 153 in add6edd

import wait_for2

cf https://packaging.python.org/en/latest/specifications/dependency-specifiers/#dependency-specifiers

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks

pyproject.toml Outdated

[project.urls]
Homepage = "http://freeopcua.github.io/"

Copy link
Contributor

Choose a reason for hiding this comment

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

Could be nice to add,

Repository = "https://github.com/FreeOpcUa/opcua-asyncio"

So in particular, it would display a link to the github repo on PyPI.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

done

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 23, 2024
View reviewed changes
asyncua/common/connection.py
@@ -170,8 +173,7 @@
if security_policy.host_certificate:
chunk.SecurityHeader.SenderCertificate = security_policy.host_certificate
if security_policy.peer_certificate:
chunk.SecurityHeader.ReceiverCertificateThumbPrint =\
hashlib.sha1(security_policy.peer_certificate).digest()
chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha1(security_policy.peer_certificate).digest()

Check failure

Code scanning / CodeQL

Use of a broken or weak cryptographic hashing algorithm on sensitive data High

Sensitive data (certificate)
Loading
is used in a hashing algorithm (SHA1) that is insecure.

Copilot Autofix AI about 2 months ago

To fix the problem, we need to replace the use of the SHA-1 hashing algorithm with a stronger alternative, such as SHA-256. This change will ensure that the certificate hashing is more secure and resistant to collision attacks.

  • General Fix: Replace the SHA-1 hashing algorithm with SHA-256.
  • Detailed Fix: Modify the line where hashlib.sha1 is used to hashlib.sha256.
  • Specific Changes: Update line 176 in the file asyncua/common/connection.py.
  • Required Changes: Import the hashlib module (already present), and change the hashing function from sha1 to sha256.
Suggested changeset 1
asyncua/common/connection.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/asyncua/common/connection.py b/asyncua/common/connection.py
--- a/asyncua/common/connection.py
+++ b/asyncua/common/connection.py
@@ -175,3 +175,3 @@
             if security_policy.peer_certificate:
-                chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha1(security_policy.peer_certificate).digest()
+                chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha256(security_policy.peer_certificate).digest()
             chunk.MessageHeader.ChannelId = channel_id
EOF
@@ -175,3 +175,3 @@
if security_policy.peer_certificate:
chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha1(security_policy.peer_certificate).digest()
chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha256(security_policy.peer_certificate).digest()
chunk.MessageHeader.ChannelId = channel_id
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@oroulet
Copy link
Member Author

oroulet commented Oct 23, 2024

looks ok @rth ? I am thinking about merging that asap

@rth
Copy link
Contributor

rth commented Oct 24, 2024

The resulting pyproject.toml LGTM. It's nice to be able to move most config there. Thanks @oroulet !

@oroulet oroulet merged commit 5c530ad into master Oct 24, 2024
2 checks passed
@rth
Copy link
Contributor

rth commented Oct 25, 2024

@oroulet I can make a PR to move setup.cfg, pytest.ini and ruff.toml also inside pyproject.toml, unless you were already working on doing that?

@oroulet
Copy link
Member Author

oroulet commented Oct 25, 2024

Feel free to do it. My main goal was to start using uv and doing automated formatting.

@rth rth mentioned this pull request Oct 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rth rth rth left review comments

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
@oroulet @rth