Skip to content

ci: Enhanced code checking#3731

Merged
hpohekar merged 66 commits intomainfrom
ci/enhanced_code_checking
Feb 24, 2025
Merged

ci: Enhanced code checking#3731
hpohekar merged 66 commits intomainfrom
ci/enhanced_code_checking

Conversation

@hpohekar
Copy link
Copy Markdown
Collaborator

@hpohekar hpohekar commented Feb 10, 2025

closes #1673

Integrated vulnerability check in pre-commit hooks which will run locally.

There is already work in progress for the deprecated command message, we will use it once it is ready.

Case 1 - check passed.

$ git commit -m "ci: exit handling"
black................................................(no files to check)Skipped
isort................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
codespell................................................................Passed
check yaml...............................................................Passed
pylint...............................................(no files to check)Skipped
Add License Headers..................................(no files to check)Skipped
Check Vulnerabilities Dependencies.......................................Passed
Check Vulnerabilities....................................................Passed
- hook id: check-vulnerabilities
- duration: 20.14s

+===========================================================================================================================================================================================+


DEPRECATED: this command (`check`) has been DEPRECATED, and will be unsupported beyond 01 June 2024.


We highly encourage switching to the new `scan` command which is easier to use, more powerful, and can be set up to mimic the deprecated command if required.


+===========================================================================================================================================================================================+




+===========================================================================================================================================================================================+


DEPRECATED: this command (`check`) has been DEPRECATED, and will be unsupported beyond 01 June 2024.


We highly encourage switching to the new `scan` command which is easier to use, more powerful, and can be set up to mimic the deprecated command if required.


+===========================================================================================================================================================================================+


[main]  INFO    profile include tests: None
[main]  INFO    profile exclude tests: B112,B403,B404,B110,B607,B314,B604,B311,B603,B307,B310,B405,B105,B602
[main]  INFO    cli include tests: None
[main]  INFO    cli exclude tests: None
Safety check performed.
Working... ---------------------------------------- 100% 0:00:10
[json]  INFO    JSON output written to file: info_bandit.json
Bandit check performed.
Advisory files generated successfully.
Dry run... not creating advisories and issues.
Information will be presented on screen.


*******************************************
Total 'safety' advisories detected: 0
Total 'safety' advisories reported: 0
Total 'bandit' advisories detected: 0
Total 'bandit' advisories reported: 0
*******************************************
Total advisories detected: 0
Total advisories reported: 0
*******************************************

[ci/enhanced_code_checking 7cd7c6ca3b] ci: exit handling
 1 file changed, 1 insertion(+), 1 deletion(-)

Case 2 - check failed.

$ pre-commit run --all-files
black....................................................................Passed
isort....................................................................Passed
flake8...................................................................Passed
codespell................................................................Passed
check yaml...............................................................Passed
pylint...................................................................Passed
Add License Headers......................................................Passed
Check Vulnerabilities Dependencies.......................................Passed
Check Vulnerabilities....................................................Failed
- hook id: check-vulnerabilities
- duration: 19.95s
- exit code: 1

+===========================================================================================================================================================================================+


DEPRECATED: this command (`check`) has been DEPRECATED, and will be unsupported beyond 01 June 2024.


We highly encourage switching to the new `scan` command which is easier to use, more powerful, and can be set up to mimic the deprecated command if required.


+===========================================================================================================================================================================================+




+===========================================================================================================================================================================================+


DEPRECATED: this command (`check`) has been DEPRECATED, and will be unsupported beyond 01 June 2024.


We highly encourage switching to the new `scan` command which is easier to use, more powerful, and can be set up to mimic the deprecated command if required.


+===========================================================================================================================================================================================+


[main]  INFO    profile include tests: None
[main]  INFO    profile exclude tests: B310,B604,B405,B602,B404,B110,B307,B112,B311,B403,B603,B314,B105
[main]  INFO    cli include tests: None
[main]  INFO    cli exclude tests: None
Safety check performed.
Working... ---------------------------------------- 100% 0:00:10
[json]  INFO    JSON output written to file: info_bandit.json
Bandit check performed.
Advisory files generated successfully.
Dry run... not creating advisories and issues.
Information will be presented on screen.

===========================================

Bandit [B607:start_process_with_partial_path] on ./src\ansys\fluent\core\fluent_connection.py - Hash: 03762c17822813383a8d90c4ece2ae0f

Starting a process with a partial executable path

#### Code

On file ./src\ansys\fluent\core\fluent_connection.py:

512     def _close_slurm(self):
513         subprocess.run(["scancel", f"{self._slurm_job_id}"])
514

#### CWE - 78
.
.
.
.

#### CWE - 78

For more information see https://cwe.mitre.org/data/definitions/78.html

#### More information

Visit https://bandit.readthedocs.io/en/1.8.2/plugins/b607_start_process_with_partial_path.html to find out more information.


*******************************************
Total 'safety' advisories detected: 0
Total 'safety' advisories reported: 0
Total 'bandit' advisories detected: 7
Total 'bandit' advisories reported: 7
*******************************************
Total advisories detected: 7
Total advisories reported: 7
*******************************************

@hpohekar hpohekar requested a review from RobPasMue February 10, 2025 14:12
@github-actions github-actions Bot added maintenance General maintenance of the repo (libraries, cicd, etc) CI/CD labels Feb 10, 2025
Comment thread .github/workflows/ci.yml Outdated
@RobPasMue
Copy link
Copy Markdown
Member

I would encourage your team to give it a local pass to the tool before enabling it on CI/CD. I'm sure Bandit will detect many issues and it will populate your advisories with a lot of draft ones... See https://actions.docs.ansys.com/version/stable/vulnerability-actions/index.html#check-vulnerabilities-action on how to run the tool locally

@hpohekar
Copy link
Copy Markdown
Collaborator Author

I would encourage your team to give it a local pass to the tool before enabling it on CI/CD. I'm sure Bandit will detect many issues and it will populate your advisories with a lot of draft ones... See https://actions.docs.ansys.com/version/stable/vulnerability-actions/index.html#check-vulnerabilities-action on how to run the tool locally

@RobPasMue Sure. Thanks a lot.

Copy link
Copy Markdown
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.

Blocking PR until local resolutions have been performed by @hpohekar to avoid creation of unnecessary advisories

@hpohekar hpohekar marked this pull request as draft February 11, 2025 08:00
hpohekar and others added 2 commits February 11, 2025 13:31
* ci: Add workflow for examples [skip tests]

* chore: adding changelog file 3730.maintenance.md [dependabot-skip]

---------

Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>
)

* refactor: Update docstring and check file extension in Mesh class

* chore: adding changelog file 3727.miscellaneous.md [dependabot-skip]

* refactor: Update docstring and check file extension in Mesh class

* ci: Add workflow for examples [skip tests] (#3730)

* ci: Add workflow for examples [skip tests]

* chore: adding changelog file 3730.maintenance.md [dependabot-skip]

---------

Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>

* ci: Error fix

* ci: Error fix

* ci: Error fix

---------

Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>
@github-actions github-actions Bot added the documentation Documentation related (improving, adding, etc) label Feb 11, 2025
@github-actions github-actions Bot removed the documentation Documentation related (improving, adding, etc) label Feb 11, 2025
Comment thread src/ansys/fluent/core/workflow.py
Comment thread src/ansys/fluent/core/workflow.py
@hpohekar hpohekar requested a review from RobPasMue February 20, 2025 12:34
@hpohekar hpohekar marked this pull request as ready for review February 20, 2025 12:34
@hpohekar hpohekar marked this pull request as draft February 20, 2025 13:26
Comment thread .pre-commit-config.yaml
Comment thread .pre-commit-config.yaml
Comment thread pyproject.toml
@seanpearsonuk
Copy link
Copy Markdown
Collaborator

DEPENDENCY_CHECK_TOKEN = your GitHub PAT.

Any need to be concerned about this?

@hpohekar
Copy link
Copy Markdown
Collaborator Author

DEPENDENCY_CHECK_TOKEN = your GitHub PAT.

Any need to be concerned about this?

@seanpearsonuk No. I have already discussed this with @RobPasMue.

@seanpearsonuk
Copy link
Copy Markdown
Collaborator

DEPENDENCY_CHECK_TOKEN = your GitHub PAT.
Any need to be concerned about this?

@seanpearsonuk No. I have already discussed this with @RobPasMue.

OK

@mkundu1
Copy link
Copy Markdown
Contributor

mkundu1 commented Feb 21, 2025

DEPENDENCY_CHECK_TOKEN = your GitHub PAT.
Any need to be concerned about this?

@seanpearsonuk No. I have already discussed this with @RobPasMue.

OK

@hpohekar In check_vulnerabilities.py, the github integration seems to be used only for creatings issues or security advisories in the github repo. Maybe we can skip those for local run and remove or avoid the github integration?

A separate concern is to maintain a copy of a file from another repo in our repo, but we can address it later (may require some change in the original file in the other repo).

Comment thread .pre-commit-config.yaml Outdated
@hpohekar
Copy link
Copy Markdown
Collaborator Author

DEPENDENCY_CHECK_TOKEN = your GitHub PAT.
Any need to be concerned about this?

@seanpearsonuk No. I have already discussed this with @RobPasMue.

OK

@hpohekar In check_vulnerabilities.py, the github integration seems to be used only for creatings issues or security advisories in the github repo. Maybe we can skip those for local run and remove or avoid the github integration?

A separate concern is to maintain a copy of a file from another repo in our repo, but we can address it later (may require some change in the original file in the other repo).

@mkundu1 Done. Thank you.

@RobPasMue
Copy link
Copy Markdown
Member

@mkundu1 @seanpearsonuk @hpohekar - as I posted here #3731 (comment), it is important that these checks are not only performed on pre-commit hooks but also on GitHub actions since they will be the ones reporting the vulnerabilities (if any) to our internal dashboard and on GitHub (privately, of course - users won't see them). Please address this request whenever you have the chance

@hpohekar
Copy link
Copy Markdown
Collaborator Author

hpohekar commented Feb 25, 2025

@mkundu1 @seanpearsonuk @hpohekar - as I posted here #3731 (comment), it is important that these checks are not only performed on pre-commit hooks but also on GitHub actions since they will be the ones reporting the vulnerabilities (if any) to our internal dashboard and on GitHub (privately, of course - users won't see them). Please address this request whenever you have the chance

@RobPasMue @seanpearsonuk @mkundu1

We have removed this from local pre-commit hook in this PR. It was giving warnings for common Python packages like 'pillow' 'zipp' 'virtualenv' 'urllib3' etc.

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

Labels

documentation Documentation related (improving, adding, etc) maintenance General maintenance of the repo (libraries, cicd, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhanced code checking

6 participants