Skip to content

Add a "tag findings" parameter to the import-scan and reimport-scan API endpoints#8707

Merged
cneill merged 32 commits into
DefectDojo:devfrom
FelixHernandez:sc-2329
Jan 5, 2024
Merged

Add a "tag findings" parameter to the import-scan and reimport-scan API endpoints#8707
cneill merged 32 commits into
DefectDojo:devfrom
FelixHernandez:sc-2329

Conversation

@FelixHernandez
Copy link
Copy Markdown
Contributor

Add a "tag findings" parameter to the import-scan and reimport-scan API endpoints

[sc-2329]
Description
This feature adds a new parameter to import-scan and reimport-scan API endpoints the new parameter is apply_tags_to_findings False as default.

@github-actions github-actions Bot added the apiv2 label Sep 21, 2023
Comment thread dojo/api_v2/serializers.py
Comment thread dojo/importers/importer/importer.py Outdated
Comment thread dojo/importers/reimporter/reimporter.py Outdated
Comment on lines +73 to +74
if apply_tags_to_findings:
item.tags.set(tags)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this needed here as well as the reimport_scan function? I think it would be best to remove this instance of adding the tags the findings

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do think it is a good feature to have on re-imports. It provides users the opportunity to apply tags that either were not available or they missed during the initial import.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adding some clarification:

if apply_tags_to_findings:
    item.tags.set(tags)

The above snippet of code is called in two places during the reimport scan process. Once in the process_parsed_findings and then second time in the reimport_scan function after the process_parsed_findings function has run.

Please remove the two lines of code attached to this comment under process_parsed_findings

@Maffooch
Copy link
Copy Markdown
Contributor

It would also be nice to have this option in the UI to maintain parity between API and UI based imports

@mtesauro mtesauro dismissed Maffooch’s stale review October 11, 2023 00:37

UI work will be done as a separate PR so we can merge this one that is done.

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

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

Requesting changes as they were not for any UI work. @FelixHernandez please see the unresolved comment on my previous review. Pasting it here as well

@dryrunsecurity
Copy link
Copy Markdown

dryrunsecurity Bot commented Jan 5, 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

@cneill cneill merged commit 30b2d49 into DefectDojo:dev Jan 5, 2024
@FelixHernandez FelixHernandez deleted the sc-2329 branch February 12, 2024 16:16
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.

6 participants