Skip to content

Conversation

@FedericoNegri
Copy link
Contributor

@FedericoNegri FedericoNegri commented Jul 8, 2025

Adding vulnerabilities check in the CI + SECURITY.md file as requested by the PyAnsys team in the linked issues.

Bandit flagged two cases of B110 try_except_pass. Reviewed the code and don't see how that can be a security concern. These are not places with any relevant logic.

I think they're acceptable and propose to mark those two instances as # nosec.

@codecov
Copy link

codecov bot commented Jul 8, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.62%. Comparing base (c405273) to head (c61d7cd).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/ansys/hps/client/common/base_resource.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #627   +/-   ##
=======================================
  Coverage   87.62%   87.62%           
=======================================
  Files          65       65           
  Lines        2999     2999           
=======================================
  Hits         2628     2628           
  Misses        371      371           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@FedericoNegri FedericoNegri marked this pull request as ready for review July 8, 2025 09:46
@RobPasMue
Copy link
Member

Hi @FedericoNegri! I think what bandit is trying to tell you is to avoid silently ignoring exceptions. Rather than a security concern this one is more related to best practices - unless it's intended.

Would adding a message to the logger (with debug level?) make sense? I don't know how often and under what circumstances you might end up going down the exception path - but maybe it makes sense to let the users know that happened.

If it doesn't make sense, then let's ignore it. However, I would suggest, in that case, that you only ignored that specific rule and added an explanatory comment - for example:

try:
  # do something
except Exception:
  # Ignoring exceptions because ...
  pass  # nosec B110

@FedericoNegri
Copy link
Contributor Author

@RobPasMue made the nosec marker more specific and added comments

Copy link
Member

@RobPasMue RobPasMue left a comment

Choose a reason for hiding this comment

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

Awesome - thanks @FedericoNegri!

@FedericoNegri FedericoNegri merged commit e6cde77 into main Jul 8, 2025
19 checks passed
@FedericoNegri FedericoNegri deleted the maint/check-vulnerabilities branch July 8, 2025 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Package and maintenance related

Projects

None yet

3 participants