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

Update vulnerability message checks to include HTTP/2 in Slack and Te… #541

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

kooomix
Copy link
Contributor

@kooomix kooomix commented Dec 17, 2024

User description

…ams workflows


PR Type

Tests


Description

  • Updated test assertions in both Slack and Teams workflows to check for vulnerability messages containing either "http1" or "http2" protocols
  • Modified string validation logic to be more inclusive of different HTTP protocol versions
  • Ensures test coverage for vulnerability notifications across both HTTP/1 and HTTP/2 protocols

Changes walkthrough 📝

Relevant files
Tests
slack_workflows.py
Add HTTP/2 protocol check in Slack vulnerability messages

tests_scripts/workflows/slack_workflows.py

  • Updated vulnerability message check to include both HTTP/1 and HTTP/2
    protocols in message validation
  • +1/-1     
    teams_workflows.py
    Add HTTP/2 protocol check in Teams vulnerability messages

    tests_scripts/workflows/teams_workflows.py

  • Updated vulnerability message check to include both HTTP/1 and HTTP/2
    protocols in message validation
  • +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @kooomix kooomix merged commit 03b1525 into master Dec 17, 2024
    2 checks passed
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Test Coverage
    Verify that test cases exist for both HTTP/1 and HTTP/2 protocols separately to ensure each protocol version is properly tested

    Code Duplication
    Consider extracting the common vulnerability message validation logic into a shared helper method since it's duplicated between Slack and Teams workflows

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Implement case-insensitive string matching for more robust protocol validation

    Add case-insensitive string matching for protocol checks to handle variations like
    'HTTP1', 'Http1', etc. Use lower() method before comparison.

    tests_scripts/workflows/slack_workflows.py [160]

    -if "New Vulnerability found" in message_string and cluster in message_string and ("http1" in message_string or "http2" in message_string) and namespace in message_string:
    +message_string_lower = message_string.lower()
    +if "New Vulnerability found" in message_string and cluster in message_string and ("http1" in message_string_lower or "http2" in message_string_lower) and namespace in message_string:
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion improves robustness by handling different case variations of protocol strings (HTTP1, Http1, etc.), which is a good defensive programming practice for string matching in tests.

    7
    Implement case-insensitive string matching for consistent protocol validation across different messaging platforms

    Add case-insensitive string matching for protocol checks to maintain consistency
    with the Slack workflow implementation and handle protocol string variations.

    tests_scripts/workflows/teams_workflows.py [191]

    -if "New Vulnerability found" in message_string and cluster in message_string and ("http1" in message_string or "http2" in message_string):
    +message_string_lower = message_string.lower()
    +if "New Vulnerability found" in message_string and cluster in message_string and ("http1" in message_string_lower or "http2" in message_string_lower):
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Similar to the Slack workflow suggestion, this improves test reliability by handling case variations in protocol strings, ensuring consistent behavior across both messaging platforms.

    7

    Copy link

    Failed to generate code suggestions for PR

    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.

    1 participant