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

SLA config: allow to use it on all levels #10025

Closed
wants to merge 1 commit into from

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented Apr 24, 2024

⚠️ Note on feature completeness ⚠️

We are narrowing the scope of acceptable enhancements to DefectDojo in preparation for v3. Learn more here:
https://github.com/DefectDojo/django-DefectDojo/blob/master/readme-docs/CONTRIBUTING.md

Description

Describe the feature / bug fix implemented by this PR.
If this is a new parser, the parser guide may be worth (re)reading.

Test results

Ideally you extend the test suite in tests/ and dojo/unittests to cover the changed in this PR.
Alternatively, describe what you have and haven't tested.

Documentation

Please update any documentation when needed in the documentation folder)

Checklist

This checklist is for your information.

  • Make sure to rebase your PR against the very latest dev.
  • Features/Changes should be submitted against the dev.
  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.11 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

Extra information

Please clear everything below when submitting your pull request, it's here purely for your information.

Moderators: Labels currently accepted for PRs:

  • Import Scans (for new scanners/importers)
  • enhancement
  • performance
  • feature
  • bugfix
  • maintenance (a.k.a chores)
  • dependencies
  • New Migration (when the PR introduces a DB migration)
  • settings_changes (when the PR introduces changes or new settings in settings.dist.py)

Contributors: Git Tips

Rebase on dev branch

If the dev branch has changed since you started working on it, please rebase your work after the current dev.

On your working branch mybranch:

git rebase dev mybranch

In case of conflict:

 git mergetool
 git rebase --continue

When everything's fine on your local branch, force push to your myOrigin remote:

git push myOrigin --force-with-lease

To cancel everything:

git rebase --abort

Squashing commits

git rebase -i origin/dev
  • Replace pick by fixup on the commits you want squashed out
  • Replace pick by reword on the first commit if you want to change the commit message
  • Save the file and quit your editor

Force push to your myOrigin remote:

git push myOrigin --force-with-lease

Copy link

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

DryRun Security Status Findings
AppSec Analyzer (beta) 0 findings
Secrets Analyzer 0 findings
Authn/Authz Analyzer 0 findings
Configured Codepaths Analyzer 0 findings
Sensitive Files Analyzer 0 findings

Note

🟢 Risk threshold not exceeded.

Tip

Get answers to your security questions. Add a comment in this PR starting with @DryRunSecurity. For example...

@dryrunsecurity What are common security issues with web application cookies?

Powered by DryRun Security

@github-actions github-actions bot added the New Migration Adding a new migration file. Take care when merging. label Apr 24, 2024
@kiblik kiblik force-pushed the sla_config_other_layers branch from 208fb5c to 4bb07a7 Compare April 25, 2024 09:49
@Maffooch
Copy link
Contributor

@kiblik I can sorta see the use case for SLA config on the engagement level, but am not seeing it on the test and finding level

@kiblik
Copy link
Contributor Author

kiblik commented Apr 30, 2024

@kiblik I can sorta see the use case for SLA config on the engagement level, but am not seeing it on the test and finding level

Thank you, this is a fair question. To be honest, with this PR, I wanted to open a discussion, on how to implement it in the best way.

  • In my use case, engagement level would be enough (e.g. because different environments within one product - grouped to different engagements - need different SLAs).
  • Regarding test level, I can imagine corporate rule: findings from dynamic scanners need to be fixed faster (with higher priority, shorter SLA) because black-box testing can perform anybody but for SAST analysis, you need access to code which is much harder to get.
  • Argument for finding level is hard for me :) but I can imagine that security analysts decide to apply stricter SLA for some findings because of specific circumstances

@Maffooch
Copy link
Contributor

Maffooch commented May 2, 2024

@mtesauro what are your thoughts on this being implemented?

if the decision is to proceed with implementation, I'd say let's start small and add to the engagement only for now. Having an "inheritance" from products would ideal, in a way similar to that of the jira projects do from product to engagement

Copy link
Contributor

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

@kiblik kiblik force-pushed the sla_config_other_layers branch from 4bb07a7 to b747ecc Compare August 15, 2024 14:56
Copy link

dryrunsecurity bot commented Aug 15, 2024

DryRun Security Summary

The code changes in this pull request introduce new fields to the Engagement and Test models in the DefectDojo application, allowing for the setting of custom SLA configurations, including the number of days to remediate findings of different severities, without introducing any obvious security risks.

Expand for full summary

Summary:

The code changes in this pull request introduce new fields to the Engagement and Test models in the DefectDojo application. The key changes are the addition of the sla_configuration field, which allows setting custom SLA configurations for each engagement and test, including the number of days to remediate findings of different severities.

From an application security perspective, these changes do not introduce any obvious security risks. The use of a foreign key field to reference another model is a common and generally secure database design pattern. However, it's important to ensure that the sla_configuration model has appropriate access controls and validation to prevent unauthorized or malicious modifications, and that the application's authorization and access control mechanisms properly restrict access to the sla_configuration model.

Additionally, it's worth considering the handling of null values for the sla_configuration field and implementing appropriate data validation rules to ensure the integrity of the referenced sla_configuration records.

Overall, this code change appears to be a reasonable and useful enhancement to the DefectDojo application, focused on improving the functionality and configurability of the SLA management features.

Files Changed:

  1. dojo/db_migrations/0214_engagement_sla_configuration_and_more.py:

    • This file contains a Django database migration that adds the sla_configuration field to the Engagement and Test models.
    • The sla_configuration field is a foreign key that references the sla_configuration model, and it is set to be nullable with a default value of None.
    • From a security perspective, the use of a foreign key field is generally secure, but it's important to ensure that the sla_configuration model is properly secured and that the application's access control mechanisms are functioning correctly.
  2. dojo/models.py:

    • This file contains the definitions of the Engagement and Test models, which have been updated to include the new sla_configuration field.
    • The addition of these fields allows setting custom SLA configurations for each engagement and test, including the number of days to remediate findings of different severities.
    • These changes provide more flexibility in managing SLAs for findings at the engagement and test level, rather than just at the product level.

Code Analysis

We ran 9 analyzers against 2 files and 0 analyzers had findings. 9 analyzers had no findings.

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

Copy link
Contributor

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

@kiblik kiblik force-pushed the sla_config_other_layers branch from b747ecc to 8395c49 Compare August 26, 2024 20:12
@Maffooch
Copy link
Contributor

It looks like there has not been any activity here for a while. In order to keep the list of pull requests in a manageable state, we are closing this one for now. If we are making a mistake here, please reopen the pull request, and leave us a note 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Migration Adding a new migration file. Take care when merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants