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

Security report mailing and small issue description fix #3930

Merged
merged 6 commits into from
Mar 13, 2025

Conversation

SahilDhillon21
Copy link
Contributor

@SahilDhillon21 SahilDhillon21 commented Mar 13, 2025

User description

Fixes #3862


PR Type

Enhancement, Tests, Bug fix


Description

  • Implemented secure handling of security vulnerability reports.

    • Created encrypted zip files for reports.
    • Sent reports via email without storing in the database.
  • Added a new test to verify email functionality for security reports.

  • Updated HTML templates for security report emails and bug descriptions.

  • Added pyzipper dependency for secure zip file creation.


Changes walkthrough 📝

Relevant files
Enhancement
security_report.html
New HTML template for security report emails                         

website/templates/email/security_report.html

  • Added a new HTML template for security report emails.
  • Included placeholders for domain and password.
  • Styled the email using Tailwind CSS.
  • +30/-0   
    report.html
    Updated bug description field naming                                         

    website/templates/report.html

  • Updated textarea name attribute for bug description.
  • Improved semantic naming for form fields.
  • +1/-1     
    issue.py
    Secure handling and emailing of security vulnerability reports

    website/views/issue.py

  • Implemented secure handling of security vulnerability reports.
  • Created encrypted zip files with screenshots and details.
  • Sent reports via email to the domain or organization.
  • Added fallback and error handling for missing email addresses.
  • +113/-1 
    Tests
    tests.py
    Added test for security report email functionality             

    website/tests.py

  • Added a test for mailing security vulnerability reports.
  • Verified email content and ensured no database storage.
  • +25/-0   
    Dependencies
    pyproject.toml
    Added `pyzipper` dependency                                                           

    pyproject.toml

    • Added pyzipper dependency for secure zip file creation.
    +1/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    3862 - Partially compliant

    Compliant requirements:

    • Create an encrypted zip file for security vulnerability reports.
    • Email the encrypted zip file to the organization.
    • Do not store security vulnerability reports in the Issues model.

    Non-compliant requirements:

    []

    Requires further human verification:

    []

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 PR contains tests
    🔒 Security concerns

    Sensitive information exposure:
    The password for the encrypted zip file is included in the email body. While this is necessary for the recipient to access the file, it could be intercepted if the email is not transmitted securely. Consider additional safeguards, such as sending the password through a separate secure channel.

    ⚡ Recommended focus areas for review

    Possible Issue

    The condition if form.instance.label == "4" or form.instance.label == 4 might be prone to errors if the label type changes or is inconsistent. Consider using a more robust approach, such as converting the label to a consistent type before comparison.

    # Don't save issue if security vulnerability
    if form.instance.label == "4" or form.instance.label == 4:
    Error Handling

    The email sending process does not seem to handle exceptions explicitly. If an error occurs during email sending, it might fail silently or not provide sufficient feedback to the user.

    email = EmailMessage(
        subject=email_subject,
        body=html_body,
        from_email=settings.DEFAULT_FROM_EMAIL,
        to=[dest_email],
    )
    email.content_subtype = "html"
    
    with open(zip_path, "rb") as f:
        email.attach("security_report.zip", f.read(), "application/zip")
    
    email.send(fail_silently=False)

    Copy link
    Contributor

    PR Code Suggestions ✨

    Copy link
    Collaborator

    @DonnieBLT DonnieBLT left a comment

    Choose a reason for hiding this comment

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

    If the user was logged on, let’s include their GitHub sponsors and crypto addresses so they can get a bounty.

    @DonnieBLT
    Copy link
    Collaborator

    Thank you! It looks great and will be a valuable asset to the community

    @SahilDhillon21
    Copy link
    Contributor Author

    Demonstration
    https://github.com/user-attachments/assets/b4daaa0f-dad3-4014-b06c-1592dc9a48b7

    I have made use of mailtrap for testing
    P.S. the bug markdown description gets added properly as there was a name mismatch during normal bug creation

    @SahilDhillon21
    Copy link
    Contributor Author

    Done, please check

    @DonnieBLT DonnieBLT merged commit ec3b725 into OWASP-BLT:main Mar 13, 2025
    10 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    2 participants