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

Allow loading a custom threats file #68

Merged
merged 1 commit into from
Feb 14, 2020

Conversation

nineinchnick
Copy link
Collaborator

Allow loading a custom threats file by setting a new TM.threatsFile attribute.

Added throwing an exception when an attribute is already set, instead of silently ignoring the new value, which could lead to surprising errors.

Added missing unit tests.

@ghost
Copy link

ghost commented Feb 13, 2020

DeepCode's analysis on #2583a4 found:

  • 0 critical issues. ⚠️ 0 warnings and 1 minor issue. ✔️ 1 issue were fixed.

💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues.


☺️ If you want to provide feedback on our bot, here is how to contact us.

Copy link
Collaborator

@izar izar left a comment

Choose a reason for hiding this comment

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

This is great, thank you so much (incidentally do you have a Twitter handle i can add when I post about the latest additions you made?) !
I am wondering if for completeness the name of the threat file added should be part of the report.

@nineinchnick
Copy link
Collaborator Author

My Twitter handle is the same as my username here: https://twitter.com/n1neinchnick

Adding threats filename might be a good idea but on the other hand it does already copy selected threat properties. The default template could include threat IDs. The file itself can change after generating the report so I'm not sure how useful that would be, other than just saying a custom one was used instead of the default.

BTW I got more PRs queued up, just need to get them in one by one to avoid conflicts.

@izar izar merged commit 79eb52f into OWASP:master Feb 14, 2020
@nineinchnick nineinchnick deleted the custom-threats branch March 12, 2020 17:28
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.

2 participants