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

Always populating description, adding title for webhook payloads #11159

Merged
merged 4 commits into from
Nov 1, 2024

Conversation

cneill
Copy link
Contributor

@cneill cneill commented Oct 30, 2024

Description

To give the user more information about the events that triggered a webhook, I've slightly modified the logic to ensure that description is always populated with something, and title is included as well in the webhook JSON body. Unfortunately, we aren't always consistent with how we call create_notification:

In the future, we should make these a bit more consistent, but that will be a more involved effort. In the meantime, including the title and ensuring that a description is always populated will at least give the user a better experience.

Relevant fields from some example webhook payloads

{"description": "Event scan_added has occurred.", "title": "Created/Updated 9 findings for test: test: Anchore Grype",  ...}
{"description": "Event test_added has occurred.", "title": "Test created for test: test: Anchore Grype", ...}
{"description": "Event engagement_added has occurred.", "title": "Engagement created for "test": test", ...}
{"description": "Product Type test has been created successfully.", "title": "test", ...}
{"description": "Product test has been created successfully.", "title": "test", ...}
{"description": "Finding "Finding Title" was added by test-user-1", "title": "Addition of Finding Title", ...}

Documentation

The documentation has been updated to reflect these changes.

[sc-8182]

Copy link

DryRun Security Summary

The pull request primarily focuses on improving the documentation and handling of notification webhooks in the DefectDojo application, including updates to example JSON payloads, the addition of new fields, and modifications to the handling of various webhook events, while also considering potential security implications such as sensitive information exposure, input validation, webhook security, secure communication, and access control and auditing.

Expand for full summary

Summary:

The changes in this pull request are primarily focused on improving the documentation and handling of notification webhooks in the DefectDojo application. The changes include updates to the example JSON payloads, the addition of new fields, and modifications to the handling of various webhook events, such as "product_type_added", "engagement_added", and "scan_added".

From an application security perspective, the changes do not appear to introduce any obvious security vulnerabilities. However, there are a few areas that should be reviewed and considered to maintain a secure application:

  1. Sensitive Information Exposure: The example JSON payloads include various URLs and IDs that could potentially expose sensitive information. It's important to ensure that any real-world data shared in documentation or examples does not contain sensitive or personally identifiable information (PII) that could be misused by malicious actors.

  2. Input Validation and Sanitization: While the changes suggest that the application is handling user input securely, it's crucial to verify that all user-supplied data is properly validated and sanitized across the entire application to prevent potential security vulnerabilities, such as injection attacks or cross-site scripting (XSS).

  3. Webhook Security: The implementation of the webhook functionality should be reviewed to ensure that appropriate measures are in place to validate the authenticity of incoming requests, such as verifying webhook signatures or using shared secrets. Additionally, consider implementing rate limiting or other mechanisms to mitigate the risk of denial-of-service attacks.

  4. Secure Communication: Ensure that the communication between the client and the server, as well as any external integrations, is secured using HTTPS to prevent eavesdropping and man-in-the-middle attacks.

  5. Access Control and Auditing: Verify that the notification system enforces proper access controls and that user information (e.g., username, email) is included in the webhook payloads to enable tracking and auditing of the events.

Overall, the changes in this pull request appear to be focused on improving the functionality and documentation of the notification webhooks in DefectDojo. While there are no obvious security concerns, it's important to maintain a vigilant approach to application security and review the implementation in the broader context of the application.

Files Changed:

  1. docs/content/en/integrations/notification_webhooks/product_added.md: The changes remove the null value for the description field and add a new title field with an empty string value. While these changes do not directly impact security, it's important to ensure that any sensitive information is properly sanitized and protected.

  2. docs/content/en/integrations/notification_webhooks/_index.md: The changes provide an overview of the webhook functionality, including details about webhook handling, request headers, and the experimental nature of the feature. These details are relevant from a security perspective to understand the overall context and potential limitations of the implementation.

  3. docs/content/en/integrations/notification_webhooks/product_type_added.md: The changes include the addition of a new product type with an ID of 4. While this information may not be highly sensitive, it's important to consider potential security implications, such as information disclosure or unauthorized access to product-related data.

  4. docs/content/en/integrations/notification_webhooks/engagement_added.md: The changes include the addition of description and title fields to the JSON payload for the engagement_added event. The payload also contains URLs that include internal hostnames and API endpoints, which should be reviewed for potential sensitive information exposure.

  5. dojo/notifications/helper.py: The changes in this file are focused on ensuring that notification messages have a proper title and description, which is a good practice for improving the user experience. However, it's important to review the entire codebase for any potential security vulnerabilities.

  6. docs/content/en/integrations/notification_webhooks/scan_added.md: The changes in this file include the addition of a new title field to the JSON event body and an update to the description of the scan_added_empty event. These changes do not appear to have any direct security implications, but it's important to ensure that the implementation of the notification system is secure.

  7. dojo/templates/notifications/webhooks/subtemplates/base.tpl: The changes in this file involve the addition of a title field and the wrapping of the description and title fields in double quotes. These changes do not appear to introduce any obvious security vulnerabilities, but it's important to review the broader context of the application and ensure that all user

Code Analysis

We ran 9 analyzers against 9 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

@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

@Maffooch Maffooch merged commit 2d27077 into DefectDojo:dev Nov 1, 2024
72 checks passed
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