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

✨ advance import-scan api to have an additional field to finetune vulnerability parsers #9351

Conversation

manuel-sommer
Copy link
Contributor

see issue #9250

This additional field could help to customize / finetune some scanners directly when parsing the report.

Use cases:

@github-actions github-actions bot added the apiv2 label Jan 17, 2024
Copy link

dryrunsecurity bot commented Jan 17, 2024

Contextual Security Analysis

As DryRun Security performs checks, we’ll summarize them here. You can always dive into the detailed results in the section below for checks.

Status DryRun Security Check
AI-powered Sensitive Function Check
Configured Sensitive Files Check
AI-powered Sensitive Files Check

Chat with your AI-powered Security Buddy by typing @dryrunsecurity followed by your question into a comment.
Example: @dryrunsecurity What are common security issues with web application cookies?

Install and configure more repositories at DryRun Security

@manuel-sommer
Copy link
Contributor Author

manuel-sommer commented Jan 17, 2024

@Maffooch and @kiblik could you take a look here? I don't know how to proceed and need some guidance. Still getting "Bad Request" when doing an import and obviously the parameter is not passed through to the actual scanner parsers.

@manuel-sommer manuel-sommer changed the title 🚧 advance api to have an additional field 🚧 advance import-scan api to have an additional field to finetune vulnerability parsers Jan 17, 2024
@kiblik
Copy link
Contributor

kiblik commented Jan 17, 2024

@Maffooch and @kiblik could you take a look here? I don't know how to proceed and need some guidance. Still getting "Bad Request" when doing an import and obviously the parameter is not passed through to the actual scanner parsers.

What does your request (and response) look like? Do you have also logs from Django?

@manuel-sommer
Copy link
Contributor Author

Lol, my import curl was malformed 😆

@manuel-sommer manuel-sommer marked this pull request as ready for review January 18, 2024 13:49
@manuel-sommer
Copy link
Contributor Author

Could you take a look please @mtesauro ?
Would be nice if we could merge this also fast due to the number of files changed. Then, I don't have merge conflicts too often.

@manuel-sommer
Copy link
Contributor Author

manuel-sommer commented Jan 18, 2024

I would make followup PRs for the mentioned use cases.

@manuel-sommer manuel-sommer changed the title 🚧 advance import-scan api to have an additional field to finetune vulnerability parsers ✨ advance import-scan api to have an additional field to finetune vulnerability parsers Jan 18, 2024
Copy link
Contributor

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

@manuel-sommer
Copy link
Contributor Author

@mtesauro could you take a look at this PR? I would love if this could be a way to finetune specific parsers.
We could extend the unittests in a way that this feature stays limited for parsers and also unittests that the fine tune logic gets documented within the parser markdown files if this is used.

@manuel-sommer
Copy link
Contributor Author

As soon as I get a clear opinion if this has the chance to get merged, I will also resolve the conflicts. What is your opinion @Maffooch ?

@mtesauro mtesauro added the consider-for-3.0 Changes that are under consideration for the DefectDojo 3.0 Design label Jan 31, 2024
@mtesauro
Copy link
Contributor

@mtesauro could you take a look at this PR? I would love if this could be a way to finetune specific parsers. We could extend the unittests in a way that this feature stays limited for parsers and also unittests that the fine tune logic gets documented within the parser markdown files if this is used.

@manuel-sommer From the consensus I've gathered about this PR, the resolution is that:

  1. It's generally seen as a good concept
  2. It's a bit big for right now considering there's 160+ parsers that might need to take this into account.
  3. We like this for a 3.0 feature - I've labelled the PR accordingly

@manuel-sommer
Copy link
Contributor Author

@mtesauro could you take a look at this PR? I would love if this could be a way to finetune specific parsers. We could extend the unittests in a way that this feature stays limited for parsers and also unittests that the fine tune logic gets documented within the parser markdown files if this is used.

@manuel-sommer From the consensus I've gathered about this PR, the resolution is that:

1. It's generally seen as a good concept

2. It's a bit big for right now considering there's 160+ parsers that might need to take this into account.

3. We like this for a 3.0 feature - I've labelled the PR accordingly

@mtesauro: Do you have a rough estimated release date for 3.0? I would like to have this feature available as soon as possible to further be able to advance various parsers. If 3.0 will take a while, can I make this PR ready to merge it earlier?

@manuel-sommer manuel-sommer marked this pull request as draft February 27, 2024 12:04
@mtesauro mtesauro closed this Feb 29, 2024
@manuel-sommer
Copy link
Contributor Author

Hi @mtesauro,
may I ask for the reason why you closed this PR? I would love such a feature to be able to finetune some parsers

@mtesauro
Copy link
Contributor

mtesauro commented Mar 2, 2024

@manuel-sommer

Hi @mtesauro, may I ask for the reason why you closed this PR? I would love such a feature to be able to finetune some parsers

Sure.

I was reviewing PRs in prep for the March minor release and realized this one wasn't going to be in 2.x so I closed it. It's labeled "Consider for 3.0" which is used for PRs that we're not working on for 2.x but want to consider for 3.x so there' no real reason to keep this open till we're ready to start on 3.x.

About 3.x timelines, I have no real idea. Every time I think I'm confident in a timeline we find something else that needs to be updated / adjusted / etc - latest is the upgrade of Django to the 4.2.10 which alters the timeline. So, it's beginning to feel like I just keep giving what I think is the timeline when I'm typing a reply only to have that change a short time after I write down the timeline somewhere.

It's probably safer to use the "Debian" wording for it's releases which is "when its ready". That's about the only accurate thing I can say right now today about the move to 3.x.

HTH

@manuel-sommer
Copy link
Contributor Author

Hi @mtesauro,

may I throw this feature again into consideration earlier than 3.x.x. I guess multiple users would be happy if we could finetune scanners with custom values based on the scanner through api settings. I have already collected multiple use cases. Also, I could prepare a documentation page in advance and collaborate with you to get this on the road (maybe through multiple pull requests to split the work). But it makes only sense to work on it if the maintainers have time to also review and merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apiv2 conflicts-detected consider-for-3.0 Changes that are under consideration for the DefectDojo 3.0 Design parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants