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.

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

Ruff: add and fix T20 #10091

Merged
merged 3 commits into from
Jul 25, 2024
Merged

Ruff: add and fix T20 #10091

merged 3 commits into from
Jul 25, 2024

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented May 2, 2024

all prints replaced with logger.info or logger.debug

https://docs.astral.sh/ruff/rules/#flake8-print-t20

Copy link

dryrunsecurity bot commented May 2, 2024

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security Status Findings
Server-Side Request Forgery Analyzer 0 findings
Configured Codepaths Analyzer 0 findings
IDOR Analyzer 0 findings
Sensitive Files Analyzer 0 findings
SQL Injection Analyzer 0 findings
Authn/Authz Analyzer 3 findings
Secrets Analyzer 0 findings

Note

🟢 Risk threshold not exceeded.

Change Summary (click to expand)

The following is a summary of changes in this pull request made by me, your security buddy 🤖. Note that this summary is auto-generated and not meant to be a definitive list of security issues but rather a helpful summary from a security perspective.

Summary:

The code changes in this pull request cover a wide range of updates and improvements to the Defect Dojo application, with a focus on enhancing the security-related functionality and testing. The changes include improvements to logging, input validation, deduplication of findings, Jira integration, and various test suite enhancements.

From an application security perspective, the key points to highlight are:

  1. Improved logging and error handling: The changes introduce the use of loggers throughout the codebase, which enhances the ability to debug and troubleshoot security-related issues.
  2. Secure data handling: The changes address potential security concerns related to input validation, sensitive data exposure, and secure communication with external systems like Jira.
  3. Robust testing: The updates to the test suite cover a wide range of security-related functionality, including deduplication of findings, file management, and user management, which helps to ensure the overall security of the application.

Overall, the changes in this pull request demonstrate a security-conscious approach to the development and maintenance of the Defect Dojo application. The focus on improving logging, input validation, and comprehensive testing is a positive step towards enhancing the application's security posture.

Files Changed:

  1. dojo/api_v2/views.py: The changes remove a print statement from the destroy() method of the EngagementViewSet class, which is a minor improvement to avoid potential information disclosure.
  2. docker/install_chrome_dependencies.py: The changes improve the logging and dependency management for installing Chrome dependencies in a Docker environment, with a focus on security best practices.
  3. dojo/api_v2/prefetch/schema.py: The changes address potential security concerns related to the prefetch functionality, including excessive data exposure and injection attacks.
  4. dojo/serializers.py: The changes remove a commented-out print statement from the EndpointSerializer class, which is a minor improvement to avoid potential information leakage.
  5. dojo/celery.py: The changes improve the logging of Celery task requests, which can be useful for security monitoring and debugging.
  6. dojo/decorators.py: The changes introduce new decorator functions for logging and rate limiting, which can contribute to the overall security of the application.
  7. dojo/apps.py: The changes remove a commented-out logging statement, and the surrounding code highlights the importance of reviewing the indexing of sensitive information in the Watson search functionality.
  8. dojo/importers/options.py: The changes improve the handling and validation of importer options, which can help to prevent potential security issues.
  9. dojo/management/commands/dupecheck.py: The changes improve the logging and reporting of duplicate entries, which can contribute to the overall data integrity and security of the application.
  10. dojo/management/commands/jira_async_updates.py: The changes introduce logging improvements and highlight the importance of secure integration with external systems like Jira.
  11. dojo/jira_link/helper.py: The changes cover the integration between Defect Dojo and Jira, with a focus on security-related aspects such as input validation, sensitive data handling, and error handling.
  12. dojo/management/commands/migrate_surveys.py: The changes introduce logging improvements and highlight the importance of secure database interactions during the migration process.
  13. dojo/management/commands/print_settings.py: The changes improve the logging of application settings, with a focus on avoiding the exposure of sensitive information.
  14. dojo/management/commands/test_celery_decorator.py: The changes introduce logging improvements and demonstrate the use of decorators, which can be a useful security pattern.
  15. dojo/management/commands/push_to_jira_update.py: The changes improve the logging and highlight the importance of secure communication with external systems like Jira.
  16. dojo/views.py: The changes improve the logging and access control mechanisms for file uploads and downloads, which is a crucial security aspect.
  17. dojo/product/views.py: The changes enhance the product-related functionality and provide more detailed security-related information to users.
  18. tests/Import_scanner_test.py: The changes improve the logging and reporting of the security scanner integration tests, which helps to maintain the overall security of the application.
  19. tests/close_old_findings_dedupe_test.py: The changes improve the logging of the deduplication and closing of old findings, which is

Powered by DryRun Security

@github-actions github-actions bot added the apiv2 label May 3, 2024
@kiblik kiblik marked this pull request as ready for review May 3, 2024 12:25
@kiblik kiblik force-pushed the ruff_t20 branch 2 times, most recently from dfed3e6 to 3c0146f Compare May 22, 2024 20:56
Copy link
Contributor

github-actions bot commented Jun 3, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

github-actions bot commented Jun 3, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link
Contributor

@Maffooch Maffooch left a comment

Choose a reason for hiding this comment

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

This one will need a little more TLC. Loggers cannot take more than one argument like print can:

>>> import logging
>>> logger = logging.getLogger(__name__)
>>> logger.error("one", "two", "three")
--- Logging error ---
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/logging/__init__.py", line 1160, in emit
    msg = self.format(record)
          ^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/logging/__init__.py", line 999, in format
    return fmt.format(record)
           ^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/logging/__init__.py", line 703, in format
    record.message = record.getMessage()
                     ^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/logging/__init__.py", line 392, in getMessage
    msg = msg % self.args
          ~~~~^~~~~~~~~~~
TypeError: not all arguments converted during string formatting
Call stack:
  File "<stdin>", line 1, in <module>
Message: 'one'
Arguments: ('two', 'three')

@kiblik
Copy link
Contributor Author

kiblik commented Jun 20, 2024

This one will need a little more TLC. Loggers cannot take more than one argument like print can:

Thanks @Maffooch for this. I can see it only in

  • dojo/decorators.py
  • dojo/utils.py
  • tests/base_test_class.py
  • tests/notes_test.py

I just fixed them. Have you found them somewhere else as well?

@kiblik kiblik requested a review from Maffooch June 20, 2024 15:38
@Maffooch
Copy link
Contributor

There are a few that are commented out that would break if someone were to uncomment them. An example being dojo/api_v2/prefetch/schema.py

Thought it may be more appropriate to just remove. What do you think?

@kiblik
Copy link
Contributor Author

kiblik commented Jun 20, 2024

There are a few that are commented out that would break if someone were to uncomment them. An example being dojo/api_v2/prefetch/schema.py

Thought it may be more appropriate to just remove. What do you think?

Agree. I will remove them

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

github-actions bot commented Jul 3, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

github-actions bot commented Jul 3, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link
Contributor

github-actions bot commented Jul 8, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link

dryrunsecurity bot commented Jul 8, 2024

DryRun Security Summary

The pull request includes a wide range of updates and improvements across various components of the DefectDojo application, focusing on enhancing security, logging, and testing capabilities, with the changes aiming to improve the overall security, reliability, and maintainability of the application.

Expand for full summary

Summary:

The code changes in this pull request cover a wide range of updates and improvements across various components of the DefectDojo application, with a focus on enhancing the application's security, logging, and testing capabilities.

The changes include improvements to the handling of Chrome dependencies in a Docker environment, the management of OpenAPI schema generation, the logging and error handling in various management commands, the integration with the Jira issue tracker, and the testing framework. These changes aim to improve the overall security, reliability, and maintainability of the application.

While the changes do not introduce any obvious security vulnerabilities, it is important to review the broader context and implementation details to ensure that the application's security posture is not compromised. This includes reviewing the handling of user input, the implementation of access controls, the secure storage and transmission of sensitive data, and the overall security practices and testing procedures.

Files Changed:

  1. docker/install_chrome_dependencies.py: This script is designed to find and list the missing packages required for running the Chrome browser within a Docker container. The changes do not introduce any obvious security concerns and follow best practices.

  2. dojo/api_v2/prefetch/schema.py: The changes in this file are focused on ensuring the accuracy and completeness of the OpenAPI schema, which is important for maintaining a secure and well-documented API.

  3. dojo/api_v2/serializers.py: The changes in this file are a routine maintenance update and do not introduce any significant security concerns.

  4. dojo/api_v2/views.py: The changes in this file are minor and do not introduce any obvious security concerns, but the broader application context should be reviewed to ensure the overall security posture.

  5. dojo/celery.py: The changes in this file are focused on improving the logging and debugging capabilities of the Celery tasks, which is a good practice from an application security standpoint.

  6. And several other files related to the DefectDojo application, all of which have been reviewed for potential security implications.

Code Analysis

We ran 7 analyzers against 30 files and 1 analyzer had findings. 6 analyzers had no findings.

Analyzer Findings
Authn/Authz Analyzer 2 findings

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

Copy link
Contributor

github-actions bot commented Jul 8, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

@kiblik kiblik closed this Jul 8, 2024
@kiblik kiblik reopened this Jul 8, 2024
Copy link
Contributor

@cneill cneill left a comment

Choose a reason for hiding this comment

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

A few minor comments

unittests/tools/test_checkmarx_one_parser.py Outdated Show resolved Hide resolved
tests/zap.py Outdated Show resolved Hide resolved
tests/Import_scanner_test.py Outdated Show resolved Hide resolved
tests/Import_scanner_test.py Outdated Show resolved Hide resolved
tests/Import_scanner_test.py Outdated Show resolved Hide resolved
tests/Import_scanner_test.py Outdated Show resolved Hide resolved
tests/Import_scanner_test.py Outdated Show resolved Hide resolved
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

kiblik and others added 3 commits July 13, 2024 00:28
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@kiblik kiblik closed this Jul 12, 2024
@kiblik kiblik reopened this Jul 12, 2024
Copy link
Contributor

@cneill cneill left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

@mtesauro mtesauro merged commit 2b493dc into DefectDojo:dev Jul 25, 2024
236 of 237 checks passed
@kiblik kiblik deleted the ruff_t20 branch July 25, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants