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

Files rescan and pivot #121

Open
wants to merge 15 commits into
base: old/develop
Choose a base branch
from
Open

Conversation

khulnasoft-bot
Copy link
Collaborator

@khulnasoft-bot khulnasoft-bot commented Oct 5, 2024

User description

(Please add to the PR name the issue/s that this PR would close if merged by using a Github keyword. Example: <feature name>. Closes #999. If your PR is made by a single commit, please add that clause in the commit too. This is all required to automate the closure of related issues.)

Description

Please include a summary of the change and link to the related issue.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist

  • I have read and understood the rules about how to Contribute to this project
  • The pull request is for the branch develop
  • A new plugin (analyzer, connector, visualizer, playbook, pivot or ingestor) was added or changed, in which case:
    • I strictly followed the documentation "How to create a Plugin"
    • Usage file was updated.
    • Advanced-Usage was updated (in case the plugin provides additional optional configuration).
    • I have dumped the configuration from Django Admin using the dumpplugin command and added it in the project as a data migration. ("How to share a plugin with the community")
    • If a File analyzer was added and it supports a mimetype which is not already supported, you added a sample of that type inside the archive test_files.zip and you added the default tests for that mimetype in test_classes.py.
    • If you created a new analyzer and it is free (does not require any API key), please add it in the FREE_TO_USE_ANALYZERS playbook by following this guide.
    • Check if it could make sense to add that analyzer/connector to other freely available playbooks.
    • I have provided the resulting raw JSON of a finished analysis and a screenshot of the results.
    • If the plugin interacts with an external service, I have created an attribute called precisely url that contains this information. This is required for Health Checks.
    • If the plugin requires mocked testing, _monkeypatch() was used in its class to apply the necessary decorators.
    • I have added that raw JSON sample to the MockUpResponse of the _monkeypatch() method. This serves us to provide a valid sample for testing.
  • If external libraries/packages with restrictive licenses were used, they were added in the Legal Notice section.
  • Linters (Black, Flake, Isort) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.
  • I have added tests for the feature/bug I solved (see tests folder). All the tests (new and old ones) gave 0 errors.
  • If changes were made to an existing model/serializer/view, the docs were updated and regenerated (check CONTRIBUTE.md).
  • If the GUI has been modified:
    • I have a provided a screenshot of the result in the PR.
    • I have created new frontend tests for the new component or updated existing ones.
  • After you had submitted the PR, if DeepSource, Django Doctors or other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.

Important Rules

  • If you miss to compile the Checklist properly, your PR won't be reviewed by the maintainers.
  • Everytime you make changes to the PR and you think the work is done, you should explicitly ask for a review. After being reviewed and received a "change request", you should explicitly ask for a review again once you have made the requested changes.

PR Type

enhancement, tests


Description

  • Implemented a new rescan feature in the Job viewset, allowing jobs to be rescanned with updated configurations.
  • Enhanced the frontend to support the rescan feature, including UI updates and API integration.
  • Added comprehensive tests for the rescan feature, covering various scenarios and permissions.
  • Introduced a .prettierignore file to exclude coverage artifacts from formatting checks.

Changes walkthrough 📝

Relevant files
Enhancement
7 files
job.py
Add `is_sample` field to Job serializer                                   

api_app/serializers/job.py

  • Added is_sample field to the Job serializer.
+1/-0     
views.py
Implement rescan action in Job viewset                                     

api_app/views.py

  • Imported ScanMode from api_app.choices.
  • Added rescan action to Job viewset.
  • Updated permissions to include rescan action.
  • +32/-1   
    utils.js
    Include `is_sample` property in job node                                 

    frontend/src/components/investigations/flow/utils.js

    • Added is_sample property to job node.
    +1/-0     
    CustomJobNode.jsx
    Update pivot button for sample analysis                                   

    frontend/src/components/investigations/flow/CustomJobNode.jsx

  • Updated pivot button URL to handle isSample parameter.
  • Added tooltip warning for sample analysis.
  • +5/-2     
    JobActionBar.jsx
    Refactor retry logic to use rescanJob                                       

    frontend/src/components/jobs/result/bar/JobActionBar.jsx

    • Refactored retry logic to use rescanJob function.
    +5/-29   
    jobApi.jsx
    Add rescanJob function for job rescanning                               

    frontend/src/components/jobs/result/jobApi.jsx

    • Added rescanJob function to handle job rescanning.
    +27/-0   
    ScanForm.jsx
    Handle isSample parameter and refactor observable selection

    frontend/src/components/scan/ScanForm.jsx

  • Added logic to handle isSample parameter in URL.
  • Refactored observable type selection logic.
  • +24/-25 
    Tests
    5 files
    test_api.py
    Add tests for Job rescan feature                                                 

    tests/api_app/test_api.py

  • Added multiple tests for the new rescan feature.
  • Included tests for permissions and different job types.
  • +213/-0 
    observables.test.js
    Extend invalid domain test cases                                                 

    frontend/tests/utils/observables.test.js

    • Added new invalid domain test case.
    +9/-6     
    InvestigationFlow.test.jsx
    Add tests for sample job nodes in InvestigationFlow           

    frontend/tests/components/investigations/flow/InvestigationFlow.test.jsx

  • Added tests for is_sample property in job nodes.
  • Verified UI elements for sample jobs.
  • +50/-6   
    JobActionBar.test.jsx
    Update tests for rescanJob API calls                                         

    frontend/tests/components/jobs/result/utils/JobActionBar.test.jsx

    • Updated tests to verify rescanJob API calls.
    +18/-100
    ScanForm.advanced.test.jsx
    Test handling of isSample parameter in ScanForm                   

    frontend/tests/components/scan/ScanForm/ScanForm.advanced.test.jsx

    • Added test for handling isSample parameter in ScanForm.
    +16/-0   
    Configuration changes
    1 files
    .prettierignore
    Add .prettierignore file for coverage artifacts                   

    frontend/.prettierignore

    • Created .prettierignore file to ignore coverage artifacts.
    +2/-0     

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

    Summary by Sourcery

    Implement a 'rescan' feature for jobs, allowing users to re-analyze jobs with the same configuration. Update the frontend to support this feature and add corresponding tests to ensure functionality.

    New Features:

    • Introduce a 'rescan' feature for jobs, allowing users to initiate a new analysis based on the configuration of an existing job.

    Enhancements:

    • Refactor the job rescan functionality to handle both observable and sample jobs, ensuring appropriate serializers are used based on job type.
    • Update the frontend to support the new 'rescan' feature, including changes to the JobActionBar and ScanForm components.

    Tests:

    • Add new test cases for the job rescan functionality, covering scenarios for both observable and sample jobs.
    • Update existing frontend tests to accommodate changes in the job rescan feature and ensure proper UI behavior.

    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    @khulnasoft-bot khulnasoft-bot marked this pull request as ready for review October 5, 2024 20:38
    Copy link

    sourcery-ai bot commented Oct 5, 2024

    Reviewer's Guide by Sourcery

    This pull request implements a rescan functionality for jobs in the ThreatMatrix project. It adds a new API endpoint for rescanning jobs, updates the frontend to support this feature, and includes corresponding test cases. The changes span across backend (Django) and frontend (React) components, with modifications to API views, serializers, React components, and test files.

    Sequence diagram for job rescan process

    sequenceDiagram
        actor User
        participant Frontend
        participant Backend
        participant Database
    
        User->>Frontend: Click 'Rescan' button
        Frontend->>Backend: POST /api/jobs/{jobId}/rescan
        Backend->>Database: Retrieve existing job details
        alt Job is observable
            Backend->>Database: Create new observable job
        else Job is sample
            Backend->>Database: Create new sample job
        end
        Database-->>Backend: Return new job ID
        Backend-->>Frontend: Respond with new job ID
        Frontend-->>User: Display success message with new job ID
    
    Loading

    Updated class diagram for Job and related serializers

    classDiagram
        class Job {
            +String tlp
            +String observable_name
            +String observable_classification
            +String status
            +DateTime finished_analysis_time
            +Map runtime_configuration
            +Boolean is_sample
            +File file
            +String file_name
            +PlaybookConfig playbook_requested
            +Set analyzers_requested
            +Set connectors_requested
        }
        class ObservableAnalysisSerializer {
            +validate()
            +save()
        }
        class FileJobSerializer {
            +validate()
            +save()
        }
        Job <|-- ObservableAnalysisSerializer
        Job <|-- FileJobSerializer
        Job : +rescan()
    
    Loading

    File-Level Changes

    Change Details Files
    Implemented rescan functionality for jobs
    • Added a new 'rescan' action in the JobViewSet
    • Created a new API endpoint for rescanning jobs
    • Updated the JobActionsBar component to use the new rescan endpoint
    • Modified the ScanForm component to handle rescan requests for both observables and samples
    • Updated the InvestigationFlow component to support rescanning of sample jobs
    api_app/views.py
    frontend/src/components/jobs/result/bar/JobActionBar.jsx
    frontend/src/components/scan/ScanForm.jsx
    frontend/src/components/investigations/flow/CustomJobNode.jsx
    Added and updated test cases for the new rescan functionality
    • Created new test cases for the rescan API endpoint
    • Updated existing test cases in JobActionBar and InvestigationFlow components
    • Added new test cases for the ScanForm component to cover rescan scenarios
    tests/api_app/test_api.py
    frontend/tests/components/jobs/result/utils/JobActionBar.test.jsx
    frontend/tests/components/investigations/flow/InvestigationFlow.test.jsx
    frontend/tests/components/scan/ScanForm/ScanForm.advanced.test.jsx
    Refactored and improved existing code
    • Extracted the selectObservableType function in ScanForm for better code organization
    • Updated the JobActionsBar component to use the new rescanJob function
    • Modified the CustomJobNode component to handle both observable and sample rescans
    frontend/src/components/scan/ScanForm.jsx
    frontend/src/components/jobs/result/bar/JobActionBar.jsx
    frontend/src/components/investigations/flow/CustomJobNode.jsx
    Updated API and serializer to include 'is_sample' field
    • Added 'is_sample' field to the JobSerializer
    • Updated the InvestigationFlow component to use the 'is_sample' field
    api_app/serializers/job.py
    frontend/src/components/investigations/flow/utils.js

    Tips and commands

    Interacting with Sourcery

    • Trigger a new review: Comment @sourcery-ai review on the pull request.
    • Continue discussions: Reply directly to Sourcery's review comments.
    • Generate a GitHub issue from a review comment: Ask Sourcery to create an
      issue from a review comment by replying to it.
    • Generate a pull request title: Write @sourcery-ai anywhere in the pull
      request title to generate a title at any time.
    • Generate a pull request summary: Write @sourcery-ai summary anywhere in
      the pull request body to generate a PR summary at any time. You can also use
      this command to specify where the summary should be inserted.

    Customizing Your Experience

    Access your dashboard to:

    • Enable or disable review features such as the Sourcery-generated pull request
      summary, the reviewer's guide, and others.
    • Change the review language.
    • Add, remove or edit custom review instructions.
    • Adjust other review settings.

    Getting Help

    Copy link

    deepsource-io bot commented Oct 5, 2024

    Here's the code health analysis summary for commits cf422ba..a37911d. View details on DeepSource ↗.

    Analysis Summary

    AnalyzerStatusSummaryLink
    DeepSource Python LogoPython❌ Failure
    ❗ 29 occurences introduced
    🎯 19 occurences resolved
    View Check ↗
    DeepSource Docker LogoDocker✅ SuccessView Check ↗

    💡 If you’re a repository administrator, you can configure the quality gates from the settings.

    @codiumai-pr-agent-free codiumai-pr-agent-free bot added enhancement New feature or request tests labels Oct 5, 2024
    Copy link

    @sourcery-ai sourcery-ai bot left a comment

    Choose a reason for hiding this comment

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

    We have skipped reviewing this pull request. It seems to have been created by a bot (hey, khulnasoft-bot!). We assume it knows what it's doing!

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Possible Bug
    The rescan action in the Job viewset doesn't handle the case where the job doesn't exist. It should return a 404 error if the job is not found.

    Error Handling
    The handleRetry function doesn't handle potential errors from the rescanJob API call. It should include error handling to provide feedback to the user if the rescan fails.

    Code Duplication
    The selectObservableType function contains logic that is similar to the existing onClick handler. Consider refactoring to avoid duplication and improve maintainability.

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Oct 5, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Security
    ✅ Add a permission check before allowing job rescan

    Consider adding a check to ensure that the user has permission to rescan the job
    before proceeding with the rescan operation. This can help prevent unauthorized
    access to job data.

    api_app/views.py [545-548]

     @action(detail=True, methods=["post"])
     def rescan(self, request, pk=None):
         logger.info(f"rescan request for job: {pk}")
         existing_job: Job = self.get_object()
    +    if not request.user.has_perm('api_app.rescan_job', existing_job):
    +        raise PermissionDenied("You do not have permission to rescan this job.")

    [Suggestion has been applied]

    Suggestion importance[1-10]: 8

    Why: Adding a permission check before allowing a job rescan is crucial for security, ensuring that only authorized users can perform this action. This suggestion addresses a potential security vulnerability.

    8
    Error handling
    ✅ Add error handling for job serializer operations

    Consider adding error handling for the job serializer creation and saving process.
    This can help provide more informative error messages if something goes wrong during
    the rescan process.

    api_app/views.py [570-571]

    -job_serializer.is_valid(raise_exception=True)
    -new_job = job_serializer.save(send_task=True)
    +try:
    +    job_serializer.is_valid(raise_exception=True)
    +    new_job = job_serializer.save(send_task=True)
    +except serializers.ValidationError as e:
    +    logger.error(f"Validation error during job rescan: {e}")
    +    raise ValidationError(f"Failed to create new job: {e}")
    +except Exception as e:
    +    logger.error(f"Unexpected error during job rescan: {e}")
    +    raise ValidationError("An unexpected error occurred while creating the new job.")

    [Suggestion has been applied]

    Suggestion importance[1-10]: 7

    Why: Adding error handling for the job serializer operations can provide more informative error messages and improve the robustness of the rescan process, making it easier to diagnose and fix issues.

    7
    Enhancement
    Use a more specific exception type for better error handling

    Consider using a more specific exception type instead of ValidationError when
    checking the job status. For example, you could create a custom exception like
    JobRunningError to provide more clarity about the specific error condition.

    api_app/views.py [540-541]

     if job.status not in Job.Status.final_statuses():
    -    raise ValidationError({"detail": "Job is running"})
    +    raise JobRunningError("Cannot rescan a job that is currently running")
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion to use a more specific exception type like JobRunningError can improve error clarity and maintainability, making it easier to understand the specific error condition when a job is running.

    6
    Best practice
    Add a check to prevent rescanning of very old jobs

    Consider adding a check to ensure that the job is not too old to be rescanned. This
    can prevent unnecessary resource usage on very old jobs that may no longer be
    relevant.

    api_app/views.py [545-548]

     @action(detail=True, methods=["post"])
     def rescan(self, request, pk=None):
         logger.info(f"rescan request for job: {pk}")
         existing_job: Job = self.get_object()
    +    if (timezone.now() - existing_job.finished_analysis_time).days > 30:
    +        raise ValidationError("This job is too old to be rescanned.")
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggestion to prevent rescanning of very old jobs is a reasonable best practice to avoid unnecessary resource usage. However, its importance depends on the specific use case and requirements of the application.

    5

    💡 Need additional feedback ? start a PR chat

    Signed-off-by: KhulnaSoft bot <43526132+khulnasoft-bot@users.noreply.github.com>
    Comment on lines +545 to +548
    @action(detail=True, methods=["post"])
    def rescan(self, request, pk=None):
    logger.info(f"rescan request for job: {pk}")
    existing_job: Job = self.get_object()
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Suggestion: Add a permission check before allowing job rescan [Security, importance: 8]

    Suggested change
    @action(detail=True, methods=["post"])
    def rescan(self, request, pk=None):
    logger.info(f"rescan request for job: {pk}")
    existing_job: Job = self.get_object()
    @action(detail=True, methods=["post"])
    def rescan(self, request, pk=None):
    logger.info(f"rescan request for job: {pk}")
    existing_job: Job = self.get_object()
    if not request.user.has_perm('api_app.rescan_job', existing_job):
    raise PermissionDenied("You do not have permission to rescan this job.")

    Comment on lines +545 to +548
    @action(detail=True, methods=["post"])
    def rescan(self, request, pk=None):
    logger.info(f"rescan request for job: {pk}")
    existing_job: Job = self.get_object()
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Suggestion: Add a permission check before allowing job rescan [Security, importance: 8]

    Suggested change
    @action(detail=True, methods=["post"])
    def rescan(self, request, pk=None):
    logger.info(f"rescan request for job: {pk}")
    existing_job: Job = self.get_object()
    @action(detail=True, methods=["post"])
    def rescan(self, request, pk=None):
    logger.info(f"rescan request for job: {pk}")
    existing_job: Job = self.get_object()
    if not request.user.has_perm('api_app.rescan_job', existing_job):
    raise PermissionDenied("You do not have permission to rescan this job.")

    Comment on lines +570 to +571
    job_serializer.is_valid(raise_exception=True)
    new_job = job_serializer.save(send_task=True)
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Suggestion: Add error handling for job serializer operations [Error handling, importance: 7]

    Suggested change
    job_serializer.is_valid(raise_exception=True)
    new_job = job_serializer.save(send_task=True)
    try:
    job_serializer.is_valid(raise_exception=True)
    new_job = job_serializer.save(send_task=True)
    except serializers.ValidationError as e:
    logger.error(f"Validation error during job rescan: {e}")
    raise ValidationError(f"Failed to create new job: {e}")
    except Exception as e:
    logger.error(f"Unexpected error during job rescan: {e}")
    raise ValidationError("An unexpected error occurred while creating the new job.")

    Comment on lines +570 to +571
    job_serializer.is_valid(raise_exception=True)
    new_job = job_serializer.save(send_task=True)
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Suggestion: Add error handling for job serializer operations [Error handling, importance: 7]

    Suggested change
    job_serializer.is_valid(raise_exception=True)
    new_job = job_serializer.save(send_task=True)
    try:
    job_serializer.is_valid(raise_exception=True)
    new_job = job_serializer.save(send_task=True)
    except serializers.ValidationError as e:
    logger.error(f"Validation error during job rescan: {e}")
    raise ValidationError(f"Failed to create new job: {e}")
    except Exception as e:
    logger.error(f"Unexpected error during job rescan: {e}")
    raise ValidationError("An unexpected error occurred while creating the new job.")

    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.

    2 participants