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

Newlines in SARIF parser code blocks #9932

Merged
merged 3 commits into from
May 1, 2024

Conversation

ahmsec
Copy link
Contributor

@ahmsec ahmsec commented Apr 16, 2024

Description

Bug: The SARIF parser doesn't add a newline after the triple backticks for Markdown code blocks. This breaks code blocks on platforms like Jira, since that line is used for language identifiers. See for example https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-and-highlighting-code-blocks#syntax-highlighting.

Fix: This PR simply adds newlines to the Markdown code block delimiters.

Test results
./dc-unittest.sh --profile postgres-redis --test-case unittests.tools.test_sarif_parser.TestSarifParser passes

Documentation

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.

Copy link

dryrunsecurity bot commented Apr 16, 2024

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

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

Note

🟢 Risk threshold not exceeded.

Powered by DryRun Security

@ahmsec ahmsec changed the base branch from dev to bugfix April 16, 2024 18:16
@ahmsec ahmsec closed this Apr 16, 2024
@ahmsec ahmsec deleted the ahmsec/sarif-parser-markdown-fix branch April 16, 2024 18:17
@ahmsec ahmsec restored the ahmsec/sarif-parser-markdown-fix branch April 16, 2024 18:18
@ahmsec ahmsec reopened this Apr 16, 2024
@ahmsec ahmsec changed the base branch from bugfix to dev April 16, 2024 18:18
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.

Looks like the unit tests need to be updated as well

@ahmsec
Copy link
Contributor Author

ahmsec commented Apr 24, 2024

@Maffooch Whoops, I updated the unit tests. Could you approve rerunning the workflow?

@ahmsec ahmsec requested a review from Maffooch April 24, 2024 03:56
@mtesauro
Copy link
Contributor

@ahmsec Tests have been re-kicked

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

Copy link
Contributor

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

@mtesauro
Copy link
Contributor

@ahmsec This can be merged once the merge conflict is resolved.

Copy link
Contributor

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

@mtesauro mtesauro merged commit ab34598 into DefectDojo:dev May 1, 2024
123 checks passed
dogboat pushed a commit to dogboat/django-DefectDojo that referenced this pull request May 6, 2024
* code delimiters on separate lines

* update unit tests
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