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 S108 #11192

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Ruff: Add and fix S108 #11192

merged 1 commit into from
Nov 12, 2024

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented Nov 5, 2024

Add S108 https://docs.astral.sh/ruff/rules/hardcoded-temp-file/ and fix it.

  • In unittests, they are compared to files from findings - they are not used for writing of content - so it is safe
  • on_exception_log_kwarg from dojo/decorators.py is used nowhere in the project. It looks like it is a leftover from previous testing.

Copy link

dryrunsecurity bot commented Nov 5, 2024

DryRun Security Summary

The pull request primarily focuses on the removal of the on_exception_log_kwarg decorator function from the dojo/decorators.py file and updates to the Ruff linter configuration in the ruff.toml file, which reduce the risk of sensitive information exposure and improve the overall code quality and security posture of the application.

Expand for full summary

Summary:

The changes in this pull request primarily focus on the removal of the on_exception_log_kwarg decorator function from the dojo/decorators.py file and updates to the Ruff linter configuration in the ruff.toml file.

The removal of the on_exception_log_kwarg decorator function is a positive change from an application security perspective, as it reduces the risk of sensitive information exposure by no longer saving the page source to a temporary file. However, it is important to ensure that the application has appropriate exception handling and logging mechanisms in place to maintain visibility into potential issues that may arise during execution.

The updates to the Ruff linter configuration, such as the addition of the S108 rule to check for the use of temporary paths and the extension of allowed calls for the flake8-boolean-trap plugin, demonstrate a commitment to maintaining code quality and following best practices. While these changes are not directly related to security vulnerabilities, they can indirectly contribute to improving the overall security posture of the application by making the codebase more maintainable and easier to understand.

Files Changed:

  1. dojo/decorators.py:

    • The on_exception_log_kwarg decorator function has been removed. This function was previously used to log exceptions and save the page source to a file in the /tmp directory, which could have potentially exposed sensitive information.
    • The removal of this functionality reduces the risk of inadvertent sensitive information exposure but may also impact the application's ability to debug and investigate issues that arise during execution.
  2. ruff.toml:

    • The select list in the [lint] section has been updated to include the S108 rule, which checks for the use of temporary paths. This rule has been added to the per-file ignores for the unittests/** directory.
    • The [lint.flake8-boolean-trap] section has been added, which extends the allowed calls for the flake8-boolean-trap plugin to include the dojo.utils.get_system_setting function.
    • The [lint.pylint] and [lint.mccabe] sections have been added, setting the maximum number of statements allowed in a function to 234 and the maximum complexity allowed for a function to 70, respectively.
    • These changes demonstrate a commitment to maintaining code quality and following best practices for linting and static code analysis.

Code Analysis

We ran 9 analyzers against 2 files and 1 analyzer had findings. 8 analyzers had no findings.

Analyzer Findings
Authn/Authz Analyzer 1 finding

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

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 46ef075 into DefectDojo:dev Nov 12, 2024
72 checks passed
@kiblik kiblik deleted the ruff_S108 branch November 12, 2024 18:03
Sign up for free to 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.

5 participants