IRIS Extension for interactive Webhook execution #801
Conversation
…is-web into feature/ID-10
Feature/id 10
# Conflicts: # docker-compose.dev.yml # docker-compose.yml # source/app/blueprints/case/case_routes.py # source/app/blueprints/case/case_tasks_routes.py # source/app/blueprints/pages/case/case_triggers_routes.py # source/app/blueprints/pages/case/templates/case-nav_actions.html # source/app/blueprints/pages/case/templates/case.html # source/app/blueprints/pages/case/templates/case_triggers.html # source/app/blueprints/pages/case/templates/modal_add_case_task.html # source/app/blueprints/pages/manage/manage_webhooks_routes.py # source/app/blueprints/pages/manage/templates/manage_webhooks.html # source/app/blueprints/pages/manage/templates/modal_upload_webhook.html # source/app/blueprints/pages/manage/templates/modal_webhook.html # source/app/blueprints/rest/api_routes.py # source/app/blueprints/rest/manage/manage_case_templates_routes.py # source/app/blueprints/rest/manage/manage_cases_routes.py # source/app/business/cases.py # source/app/datamgmt/case/case_tasks_db.py # source/app/datamgmt/manage/manage_case_templates_db.py # source/app/datamgmt/manage/manage_cases_db.py # source/app/models/cases.py # source/app/views.py # ui/src/pages/case.taskaction.response.js # ui/src/pages/case.tasks.js # ui/src/pages/case.triggers.js # ui/src/pages/manage.case.templates.js # ui/src/pages/manage.webhooks.js
adding case_template_id column in cases table
Remove Trigger Id , Case Id and Created By from Case Triggers listing
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a comprehensive webhook integration framework into the application. It adds new database models, schemas, and RESTful endpoints for managing webhooks, task and case responses, and their interactions with cases and tasks. The case template system is extended to support actions and triggers linked to webhooks, enabling automated execution of webhooks during case and task lifecycles. The UI is enhanced with new pages and modals for managing webhooks, viewing and executing task actions, and visualizing trigger responses. Supporting backend logic includes validation, asynchronous execution, and result persistence for webhook calls. Numerous templates and JavaScript files are updated or added to support these features. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant FlaskApp
participant DB
participant Webhook
User->>UI: Initiate case creation (select template)
UI->>FlaskApp: POST /api/cases (with case_template_id)
FlaskApp->>DB: Create new case
FlaskApp->>DB: Fetch triggers for case_template_id
alt Triggers exist
loop For each trigger
FlaskApp->>Webhook: POST trigger payload (async)
Webhook-->>FlaskApp: JSON response
FlaskApp->>DB: Save CaseResponse
end
else No triggers
FlaskApp-->>User: Error (no triggers)
end
FlaskApp-->>UI: Success response (case created)
UI-->>User: Show case details
sequenceDiagram
participant User
participant UI
participant FlaskApp
participant DB
participant Webhook
User->>UI: Execute task action (select action)
UI->>FlaskApp: POST /case/jsoneditor (payload, task_id, action_id)
FlaskApp->>DB: Fetch webhook for action_id
FlaskApp->>Webhook: POST action payload
Webhook-->>FlaskApp: JSON response
FlaskApp->>DB: Save TaskResponse
FlaskApp-->>UI: Success response (action result)
UI-->>User: Display action result (JSON viewer)
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 59
🔭 Outside diff range comments (4)
docker-compose.base.yml (1)
29-36:⚠️ Potential issueAvoid overriding
.envvalues by dropping the bare “environment” list or converting it to key-value pairsCompose treats
environment: - POSTGRES_USERas “pass the current shell value (or empty string)”, while
env_fileloads defaults from.env.
If a variable is missing from the host environment, the empty value silently shadows the value coming from.env, yielding blank credentials at container start-up.Either:
- environment: - - POSTGRES_USER - - POSTGRES_PASSWORD - ... - env_file: - - .env + env_file: .env # keep a single source of truthor specify explicit fall-backs:
environment: POSTGRES_USER: ${POSTGRES_USER:-iris} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-changeme}Right now the mix of both sections is brittle and may break local setups or CI pipelines.
source/app/blueprints/rest/api_routes.py (1)
36-41:⚠️ Potential issueSame duplication for
/api/versions– ensure a single decoratorThe twin decorators will likewise crash the app with “endpoint api_version already registered”.
-@rest_api_blueprint.route('/api/versions', methods=['GET']) @rest_api_blueprint.route('/api/versions', methods=['GET'])Delete the redundant line.
source/app/blueprints/rest/manage/manage_case_templates_routes.py (1)
300-305: 🛠️ Refactor suggestionComplete exception handling in the add_case_template route
There are commented-out exception handling blocks, which suggests incomplete implementation.
case_template = CaseTemplate(**case_template_data) db.session.add(case_template) # Commit the template to generate an ID db.session.commit() except Exception as e: print("Error adding case template:", e) db.session.rollback() + # Log the exception + import logging + logging.error(f"Error adding case template: {e}") return response_error("Could not add case template into DB", data=str(e))source/app/models/models.py (1)
255-277: 💡 Verification agent🧩 Analysis chain
Changing
CaseTemplatetable name breaks all FKs & queriesRenaming to
case_templatesssswithout a migration will break existing installations and any code using'case_templates'.
Confirm this is intentional and, if so, accompany it with:
- Alembic migration renaming the table.
- Search/replace across the codebase.
Otherwise revert.
I’ll check for references to both the old and new table names and any migration renames.
🏁 Script executed:
#!/bin/bash echo "=== References to 'case_templates' ===" rg -n "case_templates" || echo "None found" echo echo "=== References to 'case_templatessss' ===" rg -n "case_templatessss" || echo "None found" echo echo "=== 'rename_table' usages (migrations) ===" rg -n "rename_table" || echo "None found"Length of output: 8619
Add Alembic migration or revert accidental table rename
The
CaseTemplatemodel’s__tablename__was changed to'case_templatessss'(source/app/models/models.py:256), but there are no corresponding Alembic migrations to rename the existingcase_templatestable. This will break all foreign‐keys and any raw or ORM queries still targetingcase_templates.Please address by either:
- Writing a new Alembic migration that renames the table from
case_templatestocase_templatessss, and updating any raw SQL/tests referencing the old name.- Or reverting the
__tablename__change back tocase_templatesif the rename was unintended.
🧹 Nitpick comments (52)
source/app/forms.py (1)
114-116: Consider enhancing the webhook form with additional validation.The current implementation only validates that JSON is provided but doesn't perform any structure validation. Consider implementing custom validators to ensure the webhook JSON adheres to expected schema before submission.
class WebhookForm(FlaskForm): - webhook_json = TextAreaField(u'Webhook JSON', validators=[DataRequired()]) + webhook_json = TextAreaField(u'Webhook JSON', validators=[ + DataRequired(), + # Add custom validator for JSON structure + ]) + + def validate_webhook_json(self, field): + """Validate that the JSON has the required fields and structure""" + try: + data = json.loads(field.data) + # Validate required fields + required_fields = ['name', 'url', 'method'] + for req_field in required_fields: + if req_field not in data: + raise ValidationError(f"Missing required field: {req_field}") + except json.JSONDecodeError: + raise ValidationError("Invalid JSON format")docker/webApp/Dockerfile (1)
86-87: Commented-out chmod CommandThis commented-out line appears to be an attempt to consolidate the chmod commands but is not active.
Either remove this commented-out line or uncomment it and remove the individual chmod commands that follow if you intended to use this consolidated approach.
source/app/datamgmt/case/case_tasks_db.py (1)
310-310: Extra Blank LineThere's an extra blank line at line 310 that could be removed.
Consider removing this extra blank line to maintain consistent spacing in the file.
source/app/models/cases.py (2)
23-23: Unused import detectedThe
Tableclass is imported but not used in this file.-# IMPORTS ------------------------------------------------ -from sqlalchemy import BigInteger, Table, CheckConstraint +# IMPORTS ------------------------------------------------ +from sqlalchemy import BigInteger, CheckConstraint🧰 Tools
🪛 Ruff (0.8.2)
23-23:
sqlalchemy.Tableimported but unusedRemove unused import:
sqlalchemy.Table(F401)
46-46: Unused import detectedThe
Baseclass is imported but not used in this file.-from app.models.models import Client, Base +from app.models.models import Client🧰 Tools
🪛 Ruff (0.8.2)
46-46:
app.models.models.Baseimported but unusedRemove unused import:
app.models.models.Base(F401)
source/app/blueprints/pages/manage/templates/modal_upload_webhook.html (1)
51-73: JSON example is helpful but could be improvedThe JSON example provides a good structure, but it contains an inconsistency in the payload_schema structure. The "properties" field is defined as an array but should likely be an object according to JSON Schema standards.
"payload_schema": [ { "type": "object", - "properties": [ + "properties": { { "property": "property name", "type": "type of data" } - ] + } } ],docker/nginx/nginx.conf (1)
29-33: Consider adding integrity hashes for external scriptsWhen loading scripts from external CDNs, consider adding integrity hashes (SRI) in your HTML to ensure the delivered content hasn't been tampered with. While this isn't directly in the nginx.conf, it's an important security consideration for the frontend implementation.
source/app/business/cases.py (2)
25-25: Remove unused imports.The imports
jsonify,current_app, andgfrom Flask are not used in this file.-from flask import jsonify, current_app, g +from flask import current_app🧰 Tools
🪛 Ruff (0.8.2)
25-25:
flask.jsonifyimported but unusedRemove unused import
(F401)
25-25:
flask.current_appimported but unusedRemove unused import
(F401)
25-25:
flask.gimported but unusedRemove unused import
(F401)
122-127: Replace print statement with proper logging.Using a print statement for debugging is not ideal in production code. Consider using Flask's logger instead.
def execute_trigger_with_context(trigger): # Make sure each thread runs within the Flask app context with app.app_context(): - print(f"Trigger executed for case: {trigger}") + log.info(f"Executing trigger for case: {case.case_id}") execute_and_save_trigger(trigger, case.case_id)source/app/blueprints/pages/manage/manage_case_templates_routes.py (2)
136-138: Use clearer placeholder values for webhook IDs.The current placeholder "Webhook Id" might be confused with an actual ID. Consider using a value that clearly indicates it's a placeholder.
{ - "webhook_id": "Webhook Id", - "display_name": "Trigger Name", + "webhook_id": "<enter_webhook_id_here>", + "display_name": "<enter_trigger_name_here>", "input_params": { } }
117-119: Use clearer placeholder values for action webhook IDs.Similar to the triggers, the placeholder "Webhook Id" in actions might be confused with an actual ID.
{ - "webhook_id": "Webhook Id", - "display_name": "Action Name" + "webhook_id": "<enter_webhook_id_here>", + "display_name": "<enter_action_name_here>" }ui/src/pages/case.taskaction.response.js (3)
12-14: Consider adding user-facing error messages.Currently, errors are only logged to the console. Consider showing user-friendly error messages in the UI.
.then(data => { if (data.status && data.data) { const responseData = data.data.body; const jsonCrackEmbed = document.getElementById("jsoncrackIframe"); const jsonViewerContainer = $('#jsonViewerContainer'); jsonViewerContainer.slideDown(); + } else { + // Show user-friendly error message + toastr.error("Failed to load response data. Please try again later."); + console.error("Failed to fetch action response data:", data); + } - } else { - console.error("Failed to fetch action response data:", data); } }) .catch(error => { + toastr.error("Error loading response data. Please try again later."); console.error("Error fetching task action response:", error); });
21-28: Use optional chaining as recommended by static analysis.The code accesses iframe's contentWindow without checking if it exists first.
-if (jsonCrackEmbed && jsonCrackEmbed.contentWindow) { +if (jsonCrackEmbed?.contentWindow) { const options = { theme: "light", direction: "DOWN", }; jsonCrackEmbed.contentWindow.postMessage({ json: JSON.stringify(responseData), options }, "*");🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
1-43: Add a loading indicator during fetch operations.The code doesn't show a loading state while fetching data, which might confuse users if the operation takes time.
function toggleJsonViewer(button) { const responseId = button.getAttribute('data-task'); let id = parseInt(responseId); if (isNaN(id)) { console.error("Invalid task ID:", responseId); return; } + // Show loading indicator + const jsonViewerContainer = $('#jsonViewerContainer'); + jsonViewerContainer.html('<div class="d-flex justify-content-center mt-3"><div class="spinner-border" role="status"><span class="sr-only">Loading...</span></div></div>'); + jsonViewerContainer.slideDown(); fetch(`/case/tasks/action_response/${id}`) .then(response => response.json()) .then(data => { if (data.status && data.data) { const responseData = data.data.body; const jsonCrackEmbed = document.getElementById("jsoncrackIframe"); - const jsonViewerContainer = $('#jsonViewerContainer'); - jsonViewerContainer.slideDown(); + // Replace loading indicator with content + jsonViewerContainer.html('<div class="json-viewer-content"></div><button class="btn btn-sm btn-secondary" data-dismiss="collapse-frame">Close</button>'); if (jsonCrackEmbed?.contentWindow) { const options = { theme: "light", direction: "DOWN", }; jsonCrackEmbed.contentWindow.postMessage({ json: JSON.stringify(responseData), options }, "*"); } else { console.error("jsonCrackEmbed iframe is not available or not loaded."); + jsonViewerContainer.html('<div class="alert alert-danger">Failed to load JSON viewer. Please refresh the page and try again.</div>'); } } else { console.error("Failed to fetch action response data:", data); + jsonViewerContainer.html('<div class="alert alert-danger">Failed to load response data. Please try again later.</div>'); } }) .catch(error => { console.error("Error fetching task action response:", error); + jsonViewerContainer.html('<div class="alert alert-danger">Error loading response data. Please try again later.</div>'); }); // Attach event handler for the close button $('#jsonViewerContainer .btn[data-dismiss="collapse-frame"]').off('click').on('click', function () { $('#jsonViewerContainer').slideUp(); // Hide the container }); }🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
source/app/blueprints/rest/manage/manage_cases_routes.py (1)
289-292: Consider managing thread lifecycle.Starting threads without tracking them can lead to resource issues. Consider using a ThreadPoolExecutor to manage threads or keeping track of threads for proper cleanup.
+# Use a thread pool to manage concurrent execution with a reasonable maximum +thread_pool = ThreadPoolExecutor(max_workers=5) # Start a new thread for each trigger for trigger in triggers: - thread = Thread(target=execute_trigger_with_context, args=(trigger,)) - thread.start() + thread_pool.submit(execute_trigger_with_context, trigger)This approach would require adding back the ThreadPoolExecutor import that was removed earlier.
source/app/blueprints/pages/manage/templates/manage_webhooks.html (3)
27-32: Decide on the Upload Webhook feature.The Upload Webhook button is commented out. Either remove it completely if the feature isn't needed, or uncomment it if the feature is implemented.
If the upload feature is implemented and should be available:
- <!-- <button - class="btn btn-sm btn-dark float-right ml-2" - onclick="fire_upload_webhook();" - > - Upload Webhook - </button> --> + <button + class="btn btn-sm btn-dark float-right ml-2" + onclick="fire_upload_webhook();" + > + Upload Webhook + </button>Or if it's not needed, remove it completely.
46-61: Consider adding more columns to the webhooks table.The current table has minimal information (ID, Name, URL). Consider adding columns for description, status, last used, or action buttons for easier management.
<table aria-describedby="Webhook list" class="table display table table-striped table-hover" style="width: 100%; border-spacing: 0" id="webhooks_table" > <thead> <tr> <th scope="col">#ID</th> <th scope="col">Name</th> <th scope="col">URL</th> + <th scope="col">Description</th> + <th scope="col">Last Used</th> + <th scope="col">Actions</th> </tr> </thead> <tfoot> <tr> <th scope="col">#ID</th> <th scope="col">Name</th> <th scope="col">URL</th> + <th scope="col">Description</th> + <th scope="col">Last Used</th> + <th scope="col">Actions</th> </tr> </tfoot> </table>
1-3: Add title to body element.The
<title>element is empty in the template. Add a descriptive title.{% extends "layouts/default.html" %} {% block title %} Manage Webhooks {% -endblock title %} {% block stylesheets %} {% endblock stylesheets %} {% block +endblock title %} +{% block stylesheets %} +<link rel="stylesheet" href="/static/assets/css/webhooks/webhooks.css"> +{% endblock stylesheets %} {% blockui/src/pages/case.triggers.js (1)
65-82: Remove commented code and improve data rendering.Clean up commented code and enhance the data table with improved handling of response data.
columns: [ { data: "id" }, - // { data: "trigger" }, - // { data: "case" }, { data: "body", render: function (data, type, row) { const task = row.id; + // For non-display types, return raw data + if (type !== 'display') return data; + + // Check if data is valid JSON for display + try { + // Try to parse it to validate + JSON.parse(data); + } catch (e) { + return '<span class="text-danger">Invalid response data</span>'; + } + return ` <button type="button" class="btn btn-primary btn-sm view-response" data-task='${task}' data-response='${data}' > View Response </button> `; }, },source/app/blueprints/pages/manage/templates/modal_upload_case_template.html (1)
109-114: Add validation guidance for webhook identifiers.It would be helpful to include guidance on how to specify the webhook_id in triggers.
"triggers": [ { "webhook_id": "Webhook Id", "display_name": "Trigger Name", - } + } + ], + "_comment": "webhook_id should reference the ID of an existing webhook in the system" ] }source/app/blueprints/pages/case/templates/case_triggers.html (2)
37-41: Commented-out table columns indicate unfinished implementationThere are several commented-out table columns (Trigger Id, Case Id, Created By) which might indicate incomplete implementation or debugging code that was left in the final version.
Either remove these commented sections entirely or implement the columns if they're needed for the complete functionality.
86-87: Bootstrap library loaded from external CDNLoading the Bootstrap bundle from an external CDN (stackpath.bootstrapcdn.com) creates an external dependency that could cause issues if the CDN is unavailable.
Consider hosting the Bootstrap bundle locally to improve reliability and eliminate external dependencies:
-<script src="https://stackpath.bootstrapcdn.com/bootstrap/4.5.2/js/bootstrap.bundle.min.js"></script> +<script src="/static/assets/js/plugin/bootstrap/bootstrap.bundle.min.js"></script>source/app/blueprints/pages/case/templates/case.html (1)
430-435: JavaScript dependencies for new webhook and trigger functionalityThe addition of case.triggers.js and loading of Bootstrap 4.5.2 from an external CDN supports the new webhook-based functionality.
Consider hosting the Bootstrap bundle locally rather than from an external CDN to improve reliability and reduce external dependencies:
-<script src="https://stackpath.bootstrapcdn.com/bootstrap/4.5.2/js/bootstrap.bundle.min.js"></script> +<script src="/static/assets/js/plugin/bootstrap/bootstrap.bundle.min.js"></script>source/app/blueprints/pages/manage/templates/modal_webhook.html (1)
101-153: JSON schema example needs application-specific guidanceThe JSON example provided is generic and doesn't offer specific guidance about how webhooks should be structured in this application.
Enhance the example with:
- Real-world examples specific to this application
- Common webhook patterns for the intended integrations
- Best practices for header authentication and payload structure
Additionally, consider adding inline validation and a schema validator to help users create valid webhook configurations more easily.
source/app/datamgmt/manage/manage_case_response_db.py (1)
83-101: Code style improvements for validation functionThe validation function has nested if statements that could be simplified, as flagged by static analysis.
Simplify the conditionals for better readability:
try: if not update: - # Check required fields for creation - if "case" not in data: + # Check required fields for creation + if "case" not in data: return "Case ID is required." # Check if trigger is provided and valid if "trigger" in data and not isinstance(data["trigger"], int): return "Trigger must be an integer." # Validate the body field if provided - if "body" in data: - if not isinstance(data["body"], dict): + if "body" in data and not isinstance(data["body"], dict): return "Body must be a dictionary." # If all validations pass, return None return None except Exception as e: return str(e)🧰 Tools
🪛 Ruff (0.8.2)
83-85: Use a single
ifstatement instead of nestedifstatements(SIM102)
93-94: Use a single
ifstatement instead of nestedifstatementsCombine
ifstatements usingand(SIM102)
source/app/blueprints/pages/case/case_triggers_routes.py (2)
18-19: Fix redirect parameter inconsistencyThe original request uses
url_redirbut the redirect URL usesredirect=True, which could cause redirect issues.if url_redir: - return redirect(url_for('case_triggers.case_triggers', cid=caseid, redirect=True)) + return redirect(url_for('case_triggers.case_triggers', cid=caseid, url_redir=False))
52-55: Use a proper logger instead of print statementFor better debugging and log management, replace the print statement with a proper logger.
except Exception as e: # Log the exception for debugging (optional: use a logger instead of print) - print(f"Error processing case triggers: {e}") + import logging + logging.error(f"Error processing case triggers: {e}") return jsonify({"success": False, "error": str(e)}), 500source/app/datamgmt/manage/manage_task_response_db.py (3)
18-24: Remove unused importsSeveral imports are not used in this module:
datetimefromdatetimeUnionfromtypingdbfromappTaskResponseSchemafromapp.schema.marshablesfrom datetime import datetime -from typing import List, Optional, Union +from typing import List, Optional -from app import db from app.models.models import TaskResponse from app.models.authorization import User -from app.schema.marshables import TaskResponseSchema🧰 Tools
🪛 Ruff (0.8.2)
18-18:
datetime.datetimeimported but unusedRemove unused import:
datetime.datetime(F401)
19-19:
typing.Unionimported but unusedRemove unused import:
typing.Union(F401)
21-21:
app.dbimported but unusedRemove unused import:
app.db(F401)
24-24:
app.schema.marshables.TaskResponseSchemaimported but unusedRemove unused import:
app.schema.marshables.TaskResponseSchema(F401)
45-56: Remove debugging print statementThere's a debugging
print("in here")statement that should be removed in production code.def get_task_response_by_id(response_id: int) -> TaskResponse: """Get a specific task response by ID. Args: response_id (int): Task response ID. Returns: TaskResponse: Task response object. """ task_response = TaskResponse.query.filter_by(id=response_id).first() - print("in here") return task_response
78-92: Simplify nested if statementsThese nested if statements can be simplified using logical operators.
try: if not update: # Check required fields for creation - if "task" not in data: - return "Task ID is required." + if "task" not in data: + return "Task ID is required." # Check if action is provided and valid if "action" in data and not isinstance(data["action"], int): return "Action must be an integer." # Validate the body field if provided - if "body" in data: - if not isinstance(data["body"], dict): - return "Body must be a dictionary." + if "body" in data and not isinstance(data["body"], dict): + return "Body must be a dictionary."🧰 Tools
🪛 Ruff (0.8.2)
79-81: Use a single
ifstatement instead of nestedifstatements(SIM102)
89-90: Use a single
ifstatement instead of nestedifstatementsCombine
ifstatements usingand(SIM102)
source/app/blueprints/rest/manage/manage_case_templates_routes.py (1)
33-33: Remove unused importsThe imports
AddAssetFormandWebhookFormare not used in this file.-from app.forms import AddAssetForm, WebhookForm, CaseTemplateForm +from app.forms import CaseTemplateForm🧰 Tools
🪛 Ruff (0.8.2)
33-33:
app.forms.AddAssetFormimported but unusedRemove unused import
(F401)
33-33:
app.forms.WebhookFormimported but unusedRemove unused import
(F401)
source/app/blueprints/pages/case/templates/modal_add_case_task.html (1)
229-234: External CDN dependencies may affect reliability.The implementation relies on several external CDN dependencies (jsoneditor, tailwindcss). Consider downloading and serving these libraries locally to improve reliability and offline functionality.
-<script src="https://cdn.jsdelivr.net/npm/@json-editor/json-editor@latest/dist/jsoneditor.min.js"></script> +<script src="/static/assets/js/plugin/jsoneditor/jsoneditor.min.js"></script> -<link href="https://cdn.jsdelivr.net/npm/tailwindcss@latest/dist/tailwind.min.css" rel="stylesheet"> +<link href="/static/assets/css/tailwind.min.css" rel="stylesheet">source/app/datamgmt/manage/manage_cases_db.py (3)
22-24: Remove unused imports to keep the module clean
descandascare imported but never used, and Ruff rightfully flags them.
Leaving dead imports hinders readability and may hide genuine circular-import problems later.-from sqlalchemy import and_, desc, asc +from sqlalchemy import and_🧰 Tools
🪛 Ruff (0.8.2)
23-23:
sqlalchemy.descimported but unusedRemove unused import
(F401)
23-23:
sqlalchemy.ascimported but unusedRemove unused import
(F401)
585-599: Docstring & column names don’t match – could mis-lead maintainersThe docstring speaks of
case_template_id, yet the function parameter iscase_id; theCaseResponsecolumns are calledcaseandtrigger, which are vague and collide with reserved words.Recommend:
- Fix docstring.
- Rename columns to
case_id,trigger_id(requires alembic migration).
611-659: Timeouts and JSON-content check are good – add TLS-verify & logging like the trigger helperSame remarks as for
execute_and_save_trigger()apply here.
Additionally, ifresultsis large, serialising straight into a singleTaskResponserow may hit Postgres’ 1 GB JSONB limit or make querying painful. Consider splitting or truncating.source/app/models/models.py (1)
250-253:update_from_dictis too permissiveBlindly doing
setattr(self, field, value)invites mass-assignment bugs (e.g., someone passescreated_by_user_id).
Whitelist allowed fields or use pydantic / marshmallow for safe loading.ui/src/pages/case.tasks.js (2)
334-336: Remove straydebugger;statementLeaving a
debuggerfreezes browsers in dev-tools mode and leaks into production builds.- debugger;🧰 Tools
🪛 Biome (1.9.4)
[error] 334-334: This is an unexpected use of the debugger statement.
Unsafe fix: Remove debugger statement
(lint/suspicious/noDebugger)
184-239: Handle fetch/network errors consistently & cancel dropdown rebuildIf
fetch(webHooksurl)fails the dropdown is replaced with a disabled item, but the promise chain ignores JSON parse errors (e.g., 204 No Content). Wrapresponse.json()inreturn response.text().then(t=>JSON.parse(t||'{}'))or add anothercatch.Minor, yet improves UX.
source/app/blueprints/pages/manage/manage_webhooks_routes.py (3)
1-10: Remove unused imports to keep module lean and silence Ruff warnings
abort,jsonify, andac_requiresare imported but never used. Removing them tightens the module’s footprint and resolves Ruff F401.-from flask import Blueprint, abort, jsonify +from flask import Blueprint ... -from app.blueprints.access_controls import ac_api_requires, ac_requires, ac_requires_case_identifier +from app.blueprints.access_controls import ac_api_requires, ac_requires_case_identifier🧰 Tools
🪛 Ruff (0.8.2)
3-3:
flask.abortimported but unusedRemove unused import
(F401)
3-3:
flask.jsonifyimported but unusedRemove unused import
(F401)
226-266: Use HTTP-verb semantics & track activity when updating webhooks
update_webhookis registered asPOST, yet it semantically updates an entity. APUT(full replacement) orPATCH(partial) is clearer to API consumers. Additionally, unlikeadd_webhook&delete_webhook, no activity is tracked.-@manage_webhooks_blueprint.route('/manage/webhooks/update/<int:cur_id>', methods=['POST']) +@manage_webhooks_blueprint.route('/manage/webhooks/<int:cur_id>', methods=['PUT']) ... db.session.commit() +# Track update in activity log +track_activity(f"webhook '{webhook.name}' updated", ctx_less=True)
268-283: PreferDELETEverb for deletionsThe route currently accepts
POST. Switching toDELETEbetter matches REST conventions and makes front-end intent explicit.-@manage_webhooks_blueprint.route('/manage/webhooks/delete/<int:webhook_id>', methods=['POST']) +@manage_webhooks_blueprint.route('/manage/webhooks/<int:webhook_id>', methods=['DELETE'])ui/src/pages/manage.webhooks.js (2)
416-465:fileDataleaked to global scope
fileDatais assigned withoutlet/const, creating an implicit global variable and risking collisions.- fileData = e.target.result; + const fileData = e.target.result;
224-268: Duplicate DataTable initialisation may clashThe file initialises both
#case_webhooks_table(viainitializeDataTable) and#webhooks_tabledirectly. If both tables are not present simultaneously, that’s fine; otherwise, IDs collide and memory leaks occur. Consider consolidating or using distinct IDs.source/app/datamgmt/manage/manage_case_templates_db.py (2)
18-21: Remove unused imports to keep the module lean
requestsandjsonschemaare never referenced in this file (Ruff F401).
Deleting them avoids unnecessary dependencies.-import requests -import jsonschema🧰 Tools
🪛 Ruff (0.8.2)
18-18:
requestsimported but unusedRemove unused import:
requests(F401)
19-19:
jsonschemaimported but unusedRemove unused import:
jsonschema(F401)
383-399: Return type annotation misleading
get_triggers_by_case_template_idreturns a list, not aCaseTemplateobject.
Adjusting the signature improves IDE support:-def get_triggers_by_case_template_id(case_template_id) -> CaseTemplate: +from typing import List, Dict + +def get_triggers_by_case_template_id(case_template_id) -> List[Dict]:source/app/blueprints/case/case_routes.py (3)
60-63: Remove unused import to silence lint and shorten load-time
ac_fast_check_current_user_has_case_accessis imported but never used.-from app.iris_engine.access_control.utils import ac_get_all_access_level, ac_fast_check_current_user_has_case_access, \ - ac_fast_check_user_has_case_access +from app.iris_engine.access_control.utils import ac_get_all_access_level, \ + ac_fast_check_user_has_case_access🧰 Tools
🪛 Ruff (0.8.2)
60-60:
app.iris_engine.access_control.utils.ac_fast_check_current_user_has_case_accessimported but unusedRemove unused import:
app.iris_engine.access_control.utils.ac_fast_check_current_user_has_case_access(F401)
228-233: Boolean comparison toTruereduces readabilityRuff E712: use the value directly.
- UserActivity.display_in_ui == True + UserActivity.display_in_ui🧰 Tools
🪛 Ruff (0.8.2)
230-230: Avoid equality comparisons to
True; useif UserActivity.display_in_ui:for truth checksReplace with
UserActivity.display_in_ui(E712)
104-118:setattrfor constants offers no safety and hides typosDirect assignment is simpler and clearer:
- setattr(case, 'case_tags', get_case_tags(caseid)) + case.case_tags = get_case_tags(caseid) ... - setattr(case, 'status_name', CaseStatus(case.status_id).name.replace('_', ' ').title()) + case.status_name = CaseStatus(case.status_id).name.replace('_', ' ').title()🧰 Tools
🪛 Ruff (0.8.2)
104-104: Do not call
setattrwith a constant attribute value. It is not any safer than normal property access.Replace
setattrwith assignment(B010)
117-117: Do not call
setattrwith a constant attribute value. It is not any safer than normal property access.Replace
setattrwith assignment(B010)
source/app/blueprints/case/case_tasks_routes.py (4)
55-61: Prune unused imports to avoid lint noise and wasted memory
Permissionsandac_api_requiresare never referenced. They trigger Ruff F401 and slightly slow module import.-from app.models.authorization import CaseAccessLevel, Permissions +from app.models.authorization import CaseAccessLevel ... -from app.util import ac_api_case_requires, ac_api_requires +from app.util import ac_api_case_requires🧰 Tools
🪛 Ruff (0.8.2)
55-55:
app.models.authorization.Permissionsimported but unusedRemove unused import:
app.models.authorization.Permissions(F401)
60-60:
app.util.ac_api_requiresimported but unusedRemove unused import:
app.util.ac_api_requires(F401)
110-119: Micro-cleanup: compress guard clause
Very minor readability tweak – replace theif/elseblock with a ternary assignment as Ruff suggests.- if not ct: - output = [] - else: - output = ct + output = [] if not ct else ct🧰 Tools
🪛 Ruff (0.8.2)
115-118: Use ternary operator
output = [] if not ct else ctinstead ofif-else-blockReplace
if-else-block withoutput = [] if not ct else ct(SIM108)
240-257: Remove unused variable and simplify body-serialization
eis assigned but never used (Ruff F841). Also, doublejson.dumpswill wrap strings in quotes.- try: - taskActionResponse['body'] = json.dumps(taskActionResponse['body']) - except (TypeError, ValueError) as e: - taskActionResponse['body'] = str(taskActionResponse['body']) + try: + taskActionResponse['body'] = json.dumps( + taskActionResponse['body'], + ensure_ascii=False, + indent=2 + ) + except (TypeError, ValueError): + taskActionResponse['body'] = str(taskActionResponse['body'])🧰 Tools
🪛 Ruff (0.8.2)
255-255: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
138-155: Race-condition potential when multiple clients update the same task status
update_task_status()is called without checking the task’stask_last_update. Two simultaneous requests could overwrite each other silently. Consider optimistic locking (compare-and-swap ontask_last_update) or database-level constraints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
docker-compose.base.yml(1 hunks)docker-compose.yml(1 hunks)docker/nginx/entrypoint.sh(1 hunks)docker/nginx/nginx.conf(1 hunks)docker/webApp/Dockerfile(1 hunks)source/app/blueprints/case/case_routes.py(1 hunks)source/app/blueprints/case/case_tasks_routes.py(1 hunks)source/app/blueprints/pages/case/case_triggers_routes.py(1 hunks)source/app/blueprints/pages/case/templates/case-nav.html(1 hunks)source/app/blueprints/pages/case/templates/case-nav_actions.html(1 hunks)source/app/blueprints/pages/case/templates/case.html(6 hunks)source/app/blueprints/pages/case/templates/case_tasks.html(2 hunks)source/app/blueprints/pages/case/templates/case_triggers.html(1 hunks)source/app/blueprints/pages/case/templates/modal_add_case_task.html(3 hunks)source/app/blueprints/pages/manage/manage_case_templates_routes.py(2 hunks)source/app/blueprints/pages/manage/manage_webhooks_routes.py(1 hunks)source/app/blueprints/pages/manage/templates/manage_webhooks.html(1 hunks)source/app/blueprints/pages/manage/templates/modal_case_template.html(1 hunks)source/app/blueprints/pages/manage/templates/modal_upload_case_template.html(3 hunks)source/app/blueprints/pages/manage/templates/modal_upload_webhook.html(1 hunks)source/app/blueprints/pages/manage/templates/modal_webhook.html(1 hunks)source/app/blueprints/rest/api_routes.py(1 hunks)source/app/blueprints/rest/case/case_tasks_routes.py(3 hunks)source/app/blueprints/rest/manage/manage_case_templates_routes.py(3 hunks)source/app/blueprints/rest/manage/manage_cases_routes.py(3 hunks)source/app/business/cases.py(4 hunks)source/app/datamgmt/case/case_tasks_db.py(2 hunks)source/app/datamgmt/manage/manage_case_response_db.py(1 hunks)source/app/datamgmt/manage/manage_case_templates_db.py(4 hunks)source/app/datamgmt/manage/manage_cases_db.py(3 hunks)source/app/datamgmt/manage/manage_task_response_db.py(1 hunks)source/app/datamgmt/manage/manage_webhooks_db.py(1 hunks)source/app/forms.py(1 hunks)source/app/models/authorization.py(1 hunks)source/app/models/cases.py(6 hunks)source/app/models/models.py(2 hunks)source/app/post_init.py(2 hunks)source/app/schema/marshables.py(3 hunks)source/app/static/assets/css/webhooks/webhooks.css(1 hunks)source/app/templates/includes/sidenav.html(1 hunks)source/app/views.py(4 hunks)source/requirements.txt(1 hunks)ui/src/pages/case.taskaction.response.js(1 hunks)ui/src/pages/case.tasks.js(5 hunks)ui/src/pages/case.triggers.js(1 hunks)ui/src/pages/manage.case.templates.js(1 hunks)ui/src/pages/manage.webhooks.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
source/app/blueprints/rest/api_routes.py (2)
source/app/blueprints/access_controls.py (1)
ac_api_requires(319-337)source/app/blueprints/responses.py (1)
response_success(57-63)
source/app/models/cases.py (1)
source/app/models/models.py (1)
Client(128-140)
source/app/business/cases.py (3)
source/app/datamgmt/manage/manage_case_templates_db.py (1)
get_triggers_by_case_template_id(383-398)source/app/datamgmt/manage/manage_cases_db.py (1)
execute_and_save_trigger(548-582)source/app/business/errors.py (1)
BusinessProcessingError(21-31)
source/app/post_init.py (1)
source/app/configuration.py (1)
Config(266-491)
source/app/datamgmt/manage/manage_case_response_db.py (2)
source/app/models/models.py (1)
CaseResponse(311-333)source/app/schema/marshables.py (1)
CaseResponseSchema(818-834)
source/app/datamgmt/case/case_tasks_db.py (1)
source/app/models/models.py (5)
TaskResponse(287-309)TaskAssignee(768-778)TaskComments(868-876)Comments(839-854)CaseTasks(712-735)
source/app/datamgmt/manage/manage_task_response_db.py (2)
source/app/models/models.py (1)
TaskResponse(287-309)source/app/schema/marshables.py (1)
TaskResponseSchema(800-816)
🪛 Ruff (0.8.2)
source/app/models/cases.py
23-23: sqlalchemy.Table imported but unused
Remove unused import: sqlalchemy.Table
(F401)
46-46: app.models.models.Base imported but unused
Remove unused import: app.models.models.Base
(F401)
source/app/blueprints/rest/manage/manage_cases_routes.py
17-17: typing.Union imported but unused
Remove unused import: typing.Union
(F401)
22-22: concurrent.futures.ThreadPoolExecutor imported but unused
Remove unused import: concurrent.futures.ThreadPoolExecutor
(F401)
23-23: flask.jsonify imported but unused
Remove unused import
(F401)
23-23: flask.g imported but unused
Remove unused import
(F401)
27-27: flask.redirect imported but unused
Remove unused import: flask.redirect
(F401)
28-28: flask.render_template imported but unused
Remove unused import: flask.render_template
(F401)
30-30: flask.url_for imported but unused
Remove unused import: flask.url_for
(F401)
32-32: flask_wtf.FlaskForm imported but unused
Remove unused import: flask_wtf.FlaskForm
(F401)
41-41: app.datamgmt.client.client_db.get_client_list imported but unused
Remove unused import: app.datamgmt.client.client_db.get_client_list
(F401)
44-44: app.datamgmt.manage.manage_attribute_db.get_default_custom_attributes imported but unused
Remove unused import: app.datamgmt.manage.manage_attribute_db.get_default_custom_attributes
(F401)
45-45: app.datamgmt.manage.manage_case_classifications_db.get_case_classifications_list imported but unused
Remove unused import: app.datamgmt.manage.manage_case_classifications_db.get_case_classifications_list
(F401)
46-46: app.datamgmt.manage.manage_case_state_db.get_case_states_list imported but unused
Remove unused import: app.datamgmt.manage.manage_case_state_db.get_case_states_list
(F401)
47-47: app.datamgmt.manage.manage_case_templates_db.get_case_templates_list imported but unused
Remove unused import: app.datamgmt.manage.manage_case_templates_db.get_case_templates_list
(F401)
51-51: app.datamgmt.manage.manage_cases_db.get_case_protagonists imported but unused
Remove unused import: app.datamgmt.manage.manage_cases_db.get_case_protagonists
(F401)
54-54: app.datamgmt.manage.manage_common.get_severities_list imported but unused
Remove unused import: app.datamgmt.manage.manage_common.get_severities_list
(F401)
55-55: app.forms.AddCaseForm imported but unused
Remove unused import: app.forms.AddCaseForm
(F401)
57-57: app.iris_engine.access_control.utils.ac_current_user_has_permission imported but unused
Remove unused import: app.iris_engine.access_control.utils.ac_current_user_has_permission
(F401)
source/app/business/cases.py
25-25: flask.jsonify imported but unused
Remove unused import
(F401)
25-25: flask.current_app imported but unused
Remove unused import
(F401)
25-25: flask.g imported but unused
Remove unused import
(F401)
source/app/datamgmt/manage/manage_case_response_db.py
18-18: datetime.datetime imported but unused
Remove unused import: datetime.datetime
(F401)
19-19: typing.Union imported but unused
Remove unused import: typing.Union
(F401)
21-21: app.db imported but unused
Remove unused import: app.db
(F401)
24-24: app.schema.marshables.CaseResponseSchema imported but unused
Remove unused import: app.schema.marshables.CaseResponseSchema
(F401)
83-85: Use a single if statement instead of nested if statements
(SIM102)
93-94: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
source/app/datamgmt/manage/manage_task_response_db.py
18-18: datetime.datetime imported but unused
Remove unused import: datetime.datetime
(F401)
19-19: typing.Union imported but unused
Remove unused import: typing.Union
(F401)
21-21: app.db imported but unused
Remove unused import: app.db
(F401)
24-24: app.schema.marshables.TaskResponseSchema imported but unused
Remove unused import: app.schema.marshables.TaskResponseSchema
(F401)
79-81: Use a single if statement instead of nested if statements
(SIM102)
89-90: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
source/app/blueprints/rest/manage/manage_case_templates_routes.py
33-33: app.forms.AddAssetForm imported but unused
Remove unused import
(F401)
33-33: app.forms.WebhookForm imported but unused
Remove unused import
(F401)
source/app/blueprints/pages/manage/manage_webhooks_routes.py
3-3: flask.abort imported but unused
Remove unused import
(F401)
3-3: flask.jsonify imported but unused
Remove unused import
(F401)
12-12: app.blueprints.access_controls.ac_requires imported but unused
Remove unused import: app.blueprints.access_controls.ac_requires
(F401)
53-53: Local variable case_id is assigned to but never used
Remove assignment to unused variable case_id
(F841)
source/app/datamgmt/manage/manage_case_templates_db.py
18-18: requests imported but unused
Remove unused import: requests
(F401)
19-19: jsonschema imported but unused
Remove unused import: jsonschema
(F401)
84-85: Use a single if statement instead of nested if statements
(SIM102)
418-419: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
source/app/datamgmt/manage/manage_cases_db.py
23-23: sqlalchemy.desc imported but unused
Remove unused import
(F401)
23-23: sqlalchemy.asc imported but unused
Remove unused import
(F401)
source/app/blueprints/case/case_routes.py
60-60: app.iris_engine.access_control.utils.ac_fast_check_current_user_has_case_access imported but unused
Remove unused import: app.iris_engine.access_control.utils.ac_fast_check_current_user_has_case_access
(F401)
104-104: Do not call setattr with a constant attribute value. It is not any safer than normal property access.
Replace setattr with assignment
(B010)
117-117: Do not call setattr with a constant attribute value. It is not any safer than normal property access.
Replace setattr with assignment
(B010)
173-173: Redefinition of unused socket_summary_onchange from line 157
(F811)
230-230: Avoid equality comparisons to True; use if UserActivity.display_in_ui: for truth checks
Replace with UserActivity.display_in_ui
(E712)
source/app/datamgmt/manage/manage_webhooks_db.py
80-80: Redefinition of unused jsonschema from line 18
Remove definition: jsonschema
(F811)
84-84: Redefinition of unused jsonschema from line 80
Remove definition: jsonschema
(F811)
85-85: Redefinition of unused Optional from line 81
Remove definition: Optional
(F811)
90-91: Use a single if statement instead of nested if statements
(SIM102)
source/app/blueprints/case/case_tasks_routes.py
55-55: app.models.authorization.Permissions imported but unused
Remove unused import: app.models.authorization.Permissions
(F401)
60-60: app.util.ac_api_requires imported but unused
Remove unused import: app.util.ac_api_requires
(F401)
115-118: Use ternary operator output = [] if not ct else ct instead of if-else-block
Replace if-else-block with output = [] if not ct else ct
(SIM108)
207-207: Undefined name get_task_with_assignees
(F821)
224-224: Undefined name get_task_with_assignees
(F821)
255-255: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
280-280: Undefined name get_task_with_assignees
(F821)
323-323: Undefined name get_task_with_assignees
(F821)
🪛 Biome (1.9.4)
ui/src/pages/case.taskaction.response.js
[error] 21-21: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
ui/src/pages/case.triggers.js
[error] 12-12: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 29-29: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
ui/src/pages/case.tasks.js
[error] 334-334: This is an unexpected use of the debugger statement.
Unsafe fix: Remove debugger statement
(lint/suspicious/noDebugger)
🔇 Additional comments (50)
docker/nginx/entrypoint.sh (1)
1-1:✅ Verification successful
Consider the portability implications of changing the shebang line.
Changing from
#!/usr/bin/env bashto#!/bin/bashreduces portability as it assumes bash is installed at a specific location. While this likely works fine in your Docker container,#!/usr/bin/env bashis generally more portable as it uses the environment to locate bash.
🏁 Script executed:
#!/bin/bash # Verify if bash exists at the specified path in the container if [ -f /bin/bash ]; then echo "✅ /bin/bash exists in the container" else echo "❌ /bin/bash doesn't exist in the container - this might cause issues" fiLength of output: 103
No portability issue detected –
/bin/bashexists in this containerThe verification script confirms that
/bin/bashis present, so the script will execute as expected in your Docker environment. If this script is ever run outside of this controlled image or on a different base, consider switching back to:#!/usr/bin/env bashfor maximum portability.
source/app/blueprints/pages/case/templates/case-nav.html (1)
43-47: LGTM! The Triggers navigation item aligns with new webhook functionality.The new navigation item provides a clear access point to the webhook trigger functionality from the case view.
source/app/templates/includes/sidenav.html (1)
136-143: LGTM! Appropriate permission check for webhook management.The new UI entry point for webhook management is correctly secured with the
server_administratorpermission, ensuring only administrators can access this functionality.source/app/views.py (4)
51-51: New Webhooks Management Blueprint ImportedThe code imports a new blueprint for managing webhooks, which aligns with the PR objective of adding webhook-based integration features.
99-99: Case Triggers Blueprint ImportedThe code imports a new blueprint for case triggers functionality, which is part of the webhook integration system being added.
154-154: Webhook Management Blueprint RegistrationThe code registers the webhooks management blueprint with the Flask application, enabling access to the webhook management routes.
181-181: Case Triggers Blueprint RegistrationThe code registers the case triggers blueprint with the Flask application, enabling access to case trigger functionality.
source/app/static/assets/css/webhooks/webhooks.css (3)
1-6: Modal Dialog PositioningThe CSS properly centers modal dialogs using flexbox, providing a consistent UI experience for the new webhook modals.
8-11: Modal Width LimitationThe CSS sets an appropriate maximum width for modal dialogs, preventing them from stretching too wide on large screens.
13-21: Modal Backdrop ConfigurationThe CSS properly configures the modal backdrop to cover the entire viewport with a semi-transparent background, creating visual separation between the modal and page content.
docker/webApp/Dockerfile (1)
90-90: Commented-out ENTRYPOINTThe ENTRYPOINT directive remains commented out, suggesting it's being preserved for reference but not currently used.
source/app/datamgmt/case/case_tasks_db.py (4)
36-36: TaskResponse Model ImportThe import of the TaskResponse model is correctly added to support the new webhook integration functionality.
281-282: Improved Deletion Process DocumentationThe added comments clearly document the deletion process sequence, enhancing code maintainability.
Also applies to: 286-287, 295-296, 298-299, 301-305
301-303: TaskResponse Cleanup ImplementationThe code properly implements deletion of TaskResponse records associated with tasks being deleted, maintaining database referential integrity.
302-302:✅ Verification successful
Verify TaskResponse Reference Field Name
Ensure the TaskResponse model's field name 'task' matches the actual field in the model definition for accurate filtering.
🏁 Script executed:
#!/bin/bash # Confirm the TaskResponse model field name for task reference grep -A 25 "class TaskResponse" source/app/models/models.py | grep "task ="Length of output: 136
TaskResponse Field Name Confirmed
The
TaskResponsemodel definestask = Column(Integer, db.ForeignKey('case_tasks.id'))so using
TaskResponse.task == task_idfor filtering is accurate. No changes required.source/app/post_init.py (2)
140-141: Adding pgcrypto extension supports webhook functionalityThe addition of the pgcrypto PostgreSQL extension is a good enhancement as it provides cryptographic functions that are likely needed for secure webhook implementation, such as generating secure tokens or encrypting sensitive data in webhooks.
154-158: Minor logging improvementsThe updated log messages provide slightly better clarity about the database migration process, which is helpful for troubleshooting.
source/app/models/cases.py (2)
89-89: Case template integration for webhook featureThe addition of
case_template_idis a good design choice. This allows cases to be linked to templates that can define webhook triggers and actions, supporting the new webhook-based integration feature.
101-102: Constructor properly updated with new parameterThe case template ID parameter has been correctly added to the constructor signature and properly assigned to the instance variable.
Also applies to: 120-120
source/app/blueprints/pages/manage/templates/modal_upload_webhook.html (1)
1-120: Well-structured modal for webhook uploadThe modal provides a clean interface for uploading webhook definitions with appropriate guidance:
- Clear explanations about webhook purpose and structure
- Helpful example JSON format that demonstrates the expected structure
- Proper form validation and file type restriction
- Error handling with detailed logs display capability
This is a good UI implementation that supports the webhook integration feature.
docker/nginx/nginx.conf (1)
29-33:❓ Verification inconclusive
CSP header expansion for external resources
The Content-Security-Policy header has been expanded to allow external resources from trusted CDNs and services, which is necessary for the webhook visualization features. However, I recommend adding a verification step to ensure these domains are reliable and secure.
🏁 Script executed:
#!/bin/bash # Verify the reputation and security of the newly added domains echo "Checking reputation of added domains in CSP:" # Check SSL Labs rating for cdn.jsdelivr.net echo "Checking SSL Labs rating for cdn.jsdelivr.net:" curl -s "https://api.ssllabs.com/api/v3/analyze?host=cdn.jsdelivr.net&publish=off&all=done" | grep "grade" # Check SSL Labs rating for jsoncrack.com echo "Checking SSL Labs rating for jsoncrack.com:" curl -s "https://api.ssllabs.com/api/v3/analyze?host=jsoncrack.com&publish=off&all=done" | grep "grade" # Check SSL Labs rating for stackpath.bootstrapcdn.com echo "Checking SSL Labs rating for stackpath.bootstrapcdn.com:" curl -s "https://api.ssllabs.com/api/v3/analyze?host=stackpath.bootstrapcdn.com&publish=off&all=done" | grep "grade"Length of output: 749
Verify SSL grades for CSP domains
The SSL Labs API calls did not return any grades (analysis may still be pending). Please either poll the API until each report’s status is “READY” or manually confirm that the following domains achieve at least an A-grade rating before finalizing the CSP update:
- File: docker/nginx/nginx.conf
Lines: 29–33Domains to verify:
• cdn.jsdelivr.net
• jsoncrack.com
• stackpath.bootstrapcdn.comsource/app/business/cases.py (4)
22-22: Good addition of threading support for asynchronous operations.The addition of the Thread module is necessary for executing triggers asynchronously.
45-45: Good addition of trigger management import.Adding the import for
get_triggers_by_case_template_idsupports the new trigger functionality.
49-49: Correct import for webhook execution.The addition of
execute_and_save_triggerimport supports the webhook execution functionality.
83-88: Good refactoring to extract case template ID early.Extracting the template ID earlier and assigning it directly to the case object improves code clarity.
source/app/blueprints/pages/manage/manage_case_templates_routes.py (4)
103-106: Improved tags formatting.The new format for tags with each tag on its own line improves readability.
111-114: Consistent formatting for task tags.The task tags now follow the same formatting pattern as the case tags, which maintains consistency.
115-120: Good addition of webhook actions for tasks.Adding the actions array with webhook_id and display_name enables task-specific webhook actions.
134-141: Good implementation of triggers at the case level.The addition of triggers with webhook_id, display_name, and input_params provides a clear structure for case-level webhook triggers.
ui/src/pages/case.taskaction.response.js (3)
1-7: Input validation correctly implemented.Good implementation of input validation to check if the task ID is a valid number.
9-11: API endpoint fetch looks good.The API endpoint fetch is properly implemented with error handling.
39-42: Good implementation of clean-up for event handler.The event handler for the close button is properly implemented with
off('click')to prevent duplicate handlers.source/app/blueprints/pages/case/templates/case-nav_actions.html (1)
1-64: Secondary navigation header is well structured.The secondary navigation header with action buttons is well organized with proper spacing and layout.
source/app/blueprints/pages/case/templates/case_triggers.html (1)
14-14:✅ Verification successful
Form reference without clear context
The template includes
{{ form.hidden_tag() }}but there's no clear form definition or explanation of what form is being referenced here.Verify that the form object is being properly passed to the template and that it has the expected fields.
🏁 Script executed:
#!/bin/bash # Check if the form object is being passed to the template grep -r "render_template.*case_triggers\.html.*form" source/app/Length of output: 194
Form context confirmed
Theformobject is correctly passed into the template fromcase_triggers_routes.pyviareturn render_template("case_triggers.html", case=case, form=form)No further action is required here.
source/app/blueprints/pages/case/templates/case.html (2)
61-62: Improved visual representation of case status informationThe reorganization of case status display (severity, customer, SOC ID) improves the UI by grouping related information together.
The layout changes to the case header information provide better organization and visual hierarchy.
238-344: Modal dialogs for report template selectionThe modals for selecting report templates have been reformatted for better readability and consistent indentation.
The improved formatting makes the template code more maintainable while preserving the same functionality.
docker-compose.yml (3)
28-32: Consider security implications of binding ports to all interfaces.The database service is now exposing port 5432 to all network interfaces (
0.0.0.0). This configuration makes your PostgreSQL database accessible from any IP address that can route to your host, which could be a security risk in production environments.For development environments, consider using
127.0.0.1:5432:5432instead to limit connections to localhost only:build: context: docker/db image: iriswebapp_db:v2.4.7 ports: - - "0.0.0.0:5432:5432" + - "127.0.0.1:5432:5432"
38-43: Consider security implications of binding webapp to all interfaces.Similar to the database service, the web application is now exposed on port 8000 to all network interfaces. This could represent a security concern in production environments.
build: context: . dockerfile: docker/webApp/Dockerfile image: iriswebapp_app:v2.4.7 ports: - - "0.0.0.0:8000:8000" + - "127.0.0.1:8000:8000"
28-63:❌ Incorrect review comment
Verify image version downgrade.
All services are now using version
v2.4.7instead of what appears to be a newer version (based on the AI summary). Ensure this change is intentional and that all services are compatible with this version.Run this script to check the git history for the version change:
🏁 Script executed:
#!/bin/bash # Description: Check the git history for changes to the version numbers in docker-compose.yml # Grep for version number changes in previous commits git log -p -- docker-compose.yml | grep -E "(v2.4.[0-9]+)"Length of output: 0
Let’s run a more targeted diff to catch any downgrade in the latest commit:
🏁 Script executed:
#!/bin/bash # Check the most recent commit for version changes in docker-compose.yml git diff HEAD~1 HEAD -- docker-compose.yml | grep -E "^[+-].*image:.*:iriswebapp_.*:v2\.4\.[0-9]+"Length of output: 100
docker-compose.yml: No image version changes detected
I reviewed the diff for
docker-compose.ymland didn’t find any modifications to theimagetags—services remain pinned atv2.4.7, with no downgrade introduced. This review comment can be safely ignored.Likely an incorrect or invalid review comment.
ui/src/pages/manage.case.templates.js (3)
60-67: New actions and triggers autocomplete entries added.The editor now includes autocomplete entries for webhook-related fields (
action,triggers,input_params), properly supporting the new webhook integration feature.
82-93: Improved loading message during trigger execution.The loading message now provides more context with "Executing triggers, please wait..." which improves user experience by setting proper expectations during webhook execution.
105-110: Enhanced error handling for data validation failures.The error handling now provides more detailed feedback when validation fails, displaying specific error details in a list. This improves the debugging experience for template authors.
source/app/blueprints/pages/manage/templates/modal_case_template.html (5)
1-2: Improved template structure with proper blocks.The template now properly uses Jinja block structure, which improves organization and maintainability.
81-85: Documentation for new webhook integration features.The template documentation has been extended to include information about actions and triggers, which is essential for users to understand the new webhook functionality.
119-145: Example JSON updated with webhook integration.The example JSON template has been updated to include the new webhook-related fields (
actions,triggers,webhook_id,display_name,input_params), providing a clear reference for users.
151-190: Added webhooks list for better user visibility.A new section displaying available webhooks has been added, making it easier for users to find and reference webhook IDs when creating templates.
233-239: Added required JavaScript dependencies for webhook functionality.The template now includes necessary JavaScript dependencies for ACE editor, DataTables grouping, and webhook management, properly supporting the new features.
source/app/blueprints/pages/case/templates/modal_add_case_task.html (2)
108-177: New webhook action UI for task execution.A comprehensive webhook action interface has been added to task modals, enabling users to select, configure, and execute webhook actions. The interface includes a dropdown for action selection, a JSON editor for payload configuration, and a table for response history.
169-175: Security concern with external iframe for JSON visualization.The implementation uses an iframe to jsoncrack.com for visualizing JSON responses. This could potentially expose sensitive data to an external service.
Consider implementing a self-hosted solution or validating that no sensitive information is sent to the external service:
- <iframe id="jsoncrackIframe" src="https://jsoncrack.com/widget" frameborder="0" style="height: 50vh; width: 100%;"></iframe> + <!-- Consider a self-hosted alternative or internal JSON viewer component --> + <div id="localJsonViewer" style="height: 50vh; width: 100%;"></div>source/app/datamgmt/manage/manage_webhooks_db.py (1)
144-156: Possible schema/DB mismatch for header-auth tasks
CaseTaskSchematypically expects fields such astask_title; supplying onlyheader_auth_keyandtask_status_idmay raise a validation error that is silently swallowed.
Consider adding required fields or verifying the target schema.
source/requirements.txt
Outdated
| # unfortunately we are relying on a beta version here. I hope a definitive version gets released soon | ||
| graphql-server[flask]==3.0.0b7 | ||
| graphene-sqlalchemy==3.0.0rc1 | ||
| Debugpy |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate debugpy entry.
The second occurrence of Debugpy is redundant and may lead to confusion. Please remove this duplicate line.
- Debugpy📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Debugpy |
source/requirements.txt
Outdated
| @@ -37,9 +37,11 @@ graphene==3.3 | |||
| qrcode[pil]==7.4.2 | |||
| dictdiffer==0.2.0 | |||
| oic==1.7.0 | |||
| Debugpy | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Pin debugpy with an explicit version for reproducibility.
Requirements files should lock down exact versions to prevent unexpected upgrades. Replace the unpinned entry and normalize the package name to lowercase. To discover the latest stable version, you can run:
Suggested diff (substitute <latest-version> with the value returned):
- Debugpy
+ debugpy==<latest-version>🏁 Script executed:
#!/bin/bash
# Fetch latest debugpy version from PyPI
curl -s https://pypi.org/pypi/debugpy/json | python3 -c "import sys, json; print(json.load(sys.stdin)['info']['version'])"Length of output: 143
Pin debugpy to v1.8.14 for reproducible installs
To prevent unexpected upgrades, lock debugpy to the exact, current stable release and use the lowercase package name.
• File: source/requirements.txt
• Line: 40
- Debugpy
+ debugpy==1.8.14📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Debugpy | |
| debugpy==1.8.14 |
|
|
||
| {% block javascripts %} | ||
| {% include 'includes/footer_case.html' %} | ||
| <script> | ||
|
|
||
| const caseTemplateId = {{ case.case_template_id }} | ||
| console.log(caseTemplateId); // Check the value | ||
| </script> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Escape server values when injecting into JavaScript
{{ case.case_template_id }} is rendered verbatim.
If this variable ever becomes non-numeric (e.g., UUID) or contains quotes, it will break the script or open the door to XSS.
Safer pattern:
- const caseTemplateId = {{ case.case_template_id }}
+ const caseTemplateId = {{ case.case_template_id | tojson }};The tojson filter serialises and escapes correctly for inline JS contexts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {% block javascripts %} | |
| {% include 'includes/footer_case.html' %} | |
| <script> | |
| const caseTemplateId = {{ case.case_template_id }} | |
| console.log(caseTemplateId); // Check the value | |
| </script> | |
| {% block javascripts %} | |
| {% include 'includes/footer_case.html' %} | |
| <script> | |
| const caseTemplateId = {{ case.case_template_id | tojson }}; | |
| console.log(caseTemplateId); // Check the value | |
| </script> | |
| {% endblock %} |
| @rest_api_blueprint.route('/api/ping', methods=['GET']) | ||
| # CONTENT ------------------------------------------------ | ||
| @rest_api_blueprint.route('/api/ping', methods=['GET']) | ||
| @ac_api_requires() | ||
| def api_ping(): | ||
| return response_success("pong") |
There was a problem hiding this comment.
Duplicate decorator registers the same endpoint twice – Flask will raise AssertionError
@rest_api_blueprint.route('/api/ping') appears twice for api_ping.
During blueprint registration Flask detects the endpoint name api_ping already in use and aborts the startup.
Fix:
-@rest_api_blueprint.route('/api/ping', methods=['GET'])
# CONTENT ------------------------------------------------
@rest_api_blueprint.route('/api/ping', methods=['GET'])Remove the first (or second) decorator; keep exactly one per route.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @rest_api_blueprint.route('/api/ping', methods=['GET']) | |
| # CONTENT ------------------------------------------------ | |
| @rest_api_blueprint.route('/api/ping', methods=['GET']) | |
| @ac_api_requires() | |
| def api_ping(): | |
| return response_success("pong") | |
| # CONTENT ------------------------------------------------ | |
| @rest_api_blueprint.route('/api/ping', methods=['GET']) | |
| @ac_api_requires() | |
| def api_ping(): | |
| return response_success("pong") |
source/app/models/authorization.py
Outdated
| case_templates_read = 0x100 | ||
| case_templates_write = 0x200 | ||
|
|
||
| webhooks_read = 0x100 | ||
| webhooks_write = 0x200 | ||
|
|
There was a problem hiding this comment.
Permission bit masks collide with existing values
webhooks_read = 0x100 and webhooks_write = 0x200 reuse the exact bits of case_templates_read / case_templates_write.
Any role granted “case template” rights will automatically gain “webhook” rights (and vice-versa), defeating fine-grained access control.
Allocate the next free powers-of-two (after activities_write if it exists) e.g.:
- webhooks_read = 0x100
- webhooks_write = 0x200
+ webhooks_read = 0x1000
+ webhooks_write = 0x2000Remember to run migrations and update any hard-coded masks in fixtures or tests.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case_templates_read = 0x100 | |
| case_templates_write = 0x200 | |
| webhooks_read = 0x100 | |
| webhooks_write = 0x200 | |
| case_templates_read = 0x100 | |
| case_templates_write = 0x200 | |
| webhooks_read = 0x1000 | |
| webhooks_write = 0x2000 |
| def get_action_by_case_template_id_and_task_id(case_template_id, task_id, caseid) -> list: | ||
| """ | ||
| Retrieves the actions array for a given case_template_id and task_id. | ||
|
|
||
| :param case_template_id: The ID of the case template to look up. | ||
| :param task_id: The ID of the task to look up within the case template. | ||
| :return: A list of actions or an empty list if none are found. | ||
| """ | ||
|
|
||
| case_template = CaseTemplate.query.filter_by(id=case_template_id).first() | ||
| case_task = get_task(task_id=task_id) | ||
| if not case_template: | ||
| return [] | ||
|
|
||
| actions = [] | ||
|
|
||
| for task in case_template.tasks: | ||
|
|
||
| if case_task.task_title == task["title"]: | ||
| if 'actions' in task: | ||
|
|
||
| for action in task['actions']: | ||
| actions.append(dict(action)) | ||
|
|
||
| return actions |
There was a problem hiding this comment.
Indentation and nested if can cause a runtime IndentationError and extra complexity
Current snippet mixes 3-space and 4-space indents and nests two ifs that can be collapsed:
- if case_task.task_title == task["title"]:
- if 'actions' in task:
-
- for action in task['actions']:
- actions.append(dict(action))
+ if case_task.task_title == task["title"] and "actions" in task:
+ for action in task["actions"]:
+ actions.append(dict(action))This fixes the indentation and satisfies Ruff SIM102.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_action_by_case_template_id_and_task_id(case_template_id, task_id, caseid) -> list: | |
| """ | |
| Retrieves the actions array for a given case_template_id and task_id. | |
| :param case_template_id: The ID of the case template to look up. | |
| :param task_id: The ID of the task to look up within the case template. | |
| :return: A list of actions or an empty list if none are found. | |
| """ | |
| case_template = CaseTemplate.query.filter_by(id=case_template_id).first() | |
| case_task = get_task(task_id=task_id) | |
| if not case_template: | |
| return [] | |
| actions = [] | |
| for task in case_template.tasks: | |
| if case_task.task_title == task["title"]: | |
| if 'actions' in task: | |
| for action in task['actions']: | |
| actions.append(dict(action)) | |
| return actions | |
| def get_action_by_case_template_id_and_task_id(case_template_id, task_id, caseid) -> list: | |
| """ | |
| Retrieves the actions array for a given case_template_id and task_id. | |
| :param case_template_id: The ID of the case template to look up. | |
| :param task_id: The ID of the task to look up within the case template. | |
| :return: A list of actions or an empty list if none are found. | |
| """ | |
| case_template = CaseTemplate.query.filter_by(id=case_template_id).first() | |
| case_task = get_task(task_id=task_id) | |
| if not case_template: | |
| return [] | |
| actions = [] | |
| for task in case_template.tasks: | |
| if case_task.task_title == task["title"] and "actions" in task: | |
| for action in task["actions"]: | |
| actions.append(dict(action)) | |
| return actions |
🧰 Tools
🪛 Ruff (0.8.2)
418-419: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
| @socket_io.on('clear_buffer') | ||
| @ac_socket_requires(CaseAccessLevel.full_access) | ||
| def socket_summary_onchange(message): | ||
|
|
||
| emit('clear_buffer', message) | ||
|
|
||
|
|
||
| @socket_io.on('join') |
There was a problem hiding this comment.
Function name is re-defined – breaks “change” event listener
socket_summary_onchange is declared at line 157 and again at 173 for the clear_buffer event.
The second definition overwrites the first, leaving the “change” handler unregistered.
-@socket_io.on('clear_buffer')
-@ac_socket_requires(CaseAccessLevel.full_access)
-def socket_summary_onchange(message):
+@socket_io.on('clear_buffer')
+@ac_socket_requires(CaseAccessLevel.full_access)
+def socket_summary_clear_buffer(message):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @socket_io.on('clear_buffer') | |
| @ac_socket_requires(CaseAccessLevel.full_access) | |
| def socket_summary_onchange(message): | |
| emit('clear_buffer', message) | |
| @socket_io.on('join') | |
| @socket_io.on('clear_buffer') | |
| @ac_socket_requires(CaseAccessLevel.full_access) | |
| def socket_summary_clear_buffer(message): | |
| emit('clear_buffer', message) | |
| @socket_io.on('join') |
🧰 Tools
🪛 Ruff (0.8.2)
173-173: Redefinition of unused socket_summary_onchange from line 157
(F811)
| @ac_api_case_requires(CaseAccessLevel.full_access) | ||
| def case_delete_task(cur_id, caseid): | ||
| call_modules_hook('on_preload_task_delete', data=cur_id, caseid=caseid) | ||
| task = get_task_with_assignees(task_id=cur_id, case_id=caseid) | ||
| if not task: | ||
| return response_error("Invalid task ID for this case") | ||
|
|
||
| delete_task(task.id) | ||
|
|
||
| update_tasks_state(caseid=caseid) | ||
|
|
||
| call_modules_hook('on_postload_task_delete', data=cur_id, caseid=caseid) | ||
|
|
||
| track_activity(f"deleted task \"{task.task_title}\"") | ||
|
|
||
| return response_success("Task deleted") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing DB rollback on early returns
If get_task_with_assignees() returns None, the function returns immediately but any previous uncommitted work in the flask request context is kept open, risking a PendingRollbackError later. Add db.session.rollback() before returning on error paths throughout the file.
🧰 Tools
🪛 Ruff (0.8.2)
323-323: Undefined name get_task_with_assignees
(F821)
| # from app.datamgmt.case.case_tasks_db import get_task_with_assignees | ||
| from app.datamgmt.case.case_tasks_db import get_tasks_status | ||
| from app.datamgmt.case.case_tasks_db import get_tasks_with_assignees | ||
| from app.datamgmt.case.case_tasks_db import update_task_assignees | ||
| from app.datamgmt.case.case_tasks_db import update_task_status |
There was a problem hiding this comment.
Missing import causes NameError for every task-level view/update/delete
get_task_with_assignees() is referenced four times in this file (lines 207, 224, 280, 323) but the import is commented out, causing a runtime failure the first time any of those routes is hit.
-# from app.datamgmt.case.case_tasks_db import get_task_with_assignees
+from app.datamgmt.case.case_tasks_db import get_task_with_assigneesWithout this fix, every call path that needs full task information will 500.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # from app.datamgmt.case.case_tasks_db import get_task_with_assignees | |
| from app.datamgmt.case.case_tasks_db import get_tasks_status | |
| from app.datamgmt.case.case_tasks_db import get_tasks_with_assignees | |
| from app.datamgmt.case.case_tasks_db import update_task_assignees | |
| from app.datamgmt.case.case_tasks_db import update_task_status | |
| from app.datamgmt.case.case_tasks_db import get_task_with_assignees | |
| from app.datamgmt.case.case_tasks_db import get_tasks_status | |
| from app.datamgmt.case.case_tasks_db import get_tasks_with_assignees | |
| from app.datamgmt.case.case_tasks_db import update_task_assignees | |
| from app.datamgmt.case.case_tasks_db import update_task_status |
| @case_tasks_blueprint.route('/case/jsoneditor', methods=['POST']) | ||
| @ac_case_requires(CaseAccessLevel.full_access) | ||
| def save_data(caseid, url_redir): | ||
| try: | ||
| data = request.get_json() | ||
|
|
||
| if not isinstance(data, dict): | ||
| raise ValueError("Request payload is not a valid JSON object") | ||
|
|
||
| payload = data.get('payload') | ||
| task_id = data.get('task_id') | ||
| action_id = data.get('action_id') | ||
|
|
||
| if not payload or not task_id or not action_id: | ||
| raise KeyError("Missing one or more required keys: 'payload', 'task_id', 'action_id'") | ||
|
|
||
| action_response = execute_and_save_action(payload, task_id, action_id) | ||
|
|
||
| return jsonify({"status": "success", "message": "Data saved successfully!", "data": action_response}) | ||
| except KeyError as e: | ||
| return jsonify({"status": "error", "message": f"Missing key: {e}"}), 400 | ||
| except ValueError as e: | ||
| return jsonify({"status": "error", "message": str(e)}), 400 | ||
| except Exception as e: | ||
| return jsonify({"status": "error", "message": str(e)}) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
/case/jsoneditor – strengthen validation & HTTP semantics
payloadmay legitimately be an empty dict, butif not payload …rejects it.execute_and_save_action()is called withoutcaseid; double-check its signature.- Generic
except Exceptionreturns HTTP 200; clients cannot distinguish success from failure. - No logging → silent failures.
- if not payload or not task_id or not action_id:
+ if payload is None or task_id is None or action_id is None:
raise KeyError("Missing one or more required keys: 'payload', 'task_id', 'action_id'")
...
- except Exception as e:
- return jsonify({"status": "error", "message": str(e)})
+ except Exception as e:
+ current_app.logger.exception("Unhandled error in save_data")
+ return jsonify({"status": "error", "message": str(e)}), 500Consider passing caseid explicitly if execute_and_save_action requires context.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @case_tasks_blueprint.route('/case/jsoneditor', methods=['POST']) | |
| @ac_case_requires(CaseAccessLevel.full_access) | |
| def save_data(caseid, url_redir): | |
| try: | |
| data = request.get_json() | |
| if not isinstance(data, dict): | |
| raise ValueError("Request payload is not a valid JSON object") | |
| payload = data.get('payload') | |
| task_id = data.get('task_id') | |
| action_id = data.get('action_id') | |
| if not payload or not task_id or not action_id: | |
| raise KeyError("Missing one or more required keys: 'payload', 'task_id', 'action_id'") | |
| action_response = execute_and_save_action(payload, task_id, action_id) | |
| return jsonify({"status": "success", "message": "Data saved successfully!", "data": action_response}) | |
| except KeyError as e: | |
| return jsonify({"status": "error", "message": f"Missing key: {e}"}), 400 | |
| except ValueError as e: | |
| return jsonify({"status": "error", "message": str(e)}), 400 | |
| except Exception as e: | |
| return jsonify({"status": "error", "message": str(e)}) | |
| @case_tasks_blueprint.route('/case/jsoneditor', methods=['POST']) | |
| @ac_case_requires(CaseAccessLevel.full_access) | |
| def save_data(caseid, url_redir): | |
| try: | |
| data = request.get_json() | |
| if not isinstance(data, dict): | |
| raise ValueError("Request payload is not a valid JSON object") | |
| payload = data.get('payload') | |
| task_id = data.get('task_id') | |
| action_id = data.get('action_id') | |
| if payload is None or task_id is None or action_id is None: | |
| raise KeyError("Missing one or more required keys: 'payload', 'task_id', 'action_id'") | |
| action_response = execute_and_save_action(payload, task_id, action_id) | |
| return jsonify({"status": "success", "message": "Data saved successfully!", "data": action_response}) | |
| except KeyError as e: | |
| return jsonify({"status": "error", "message": f"Missing key: {e}"}), 400 | |
| except ValueError as e: | |
| return jsonify({"status": "error", "message": str(e)}), 400 | |
| except Exception as e: | |
| current_app.logger.exception("Unhandled error in save_data") | |
| return jsonify({"status": "error", "message": str(e)}), 500 |
source/app/models/cases.py
Outdated
| @@ -19,8 +19,8 @@ | |||
|
|
|||
| from datetime import datetime | |||
| from flask_login import current_user | |||
| from sqlalchemy import BigInteger | |||
| from sqlalchemy import CheckConstraint | |||
| # IMPORTS ------------------------------------------------ | |||
There was a problem hiding this comment.
We prefer to avoid unnecessary comments such as this one.
source/app/models/cases.py
Outdated
| from sqlalchemy import BigInteger | ||
| from sqlalchemy import CheckConstraint | ||
| # IMPORTS ------------------------------------------------ | ||
| from sqlalchemy import BigInteger, CheckConstraint |
There was a problem hiding this comment.
We try to have one import per line (so two lines instead of one here)
source/app/models/cases.py
Outdated
| @@ -44,7 +43,7 @@ | |||
| from app.datamgmt.states import update_notes_state | |||
| from app.datamgmt.states import update_tasks_state | |||
| from app.datamgmt.states import update_timeline_state | |||
| from app.models.models import Client | |||
| from app.models.models import Client, Base | |||
source/app/models/models.py
Outdated
| @@ -228,8 +228,32 @@ class EvidenceTypes(db.Model): | |||
| created_by = relationship('User') | |||
|
|
|||
|
|
|||
| class Webhook(db.Model): | |||
| __tablename__ = 'webhooksss' | |||
There was a problem hiding this comment.
Why so many s in the table name (webhookss)?
| @@ -87,6 +86,8 @@ class Cases(db.Model): | |||
|
|
|||
| review_status = relationship('ReviewStatus') | |||
|
|
|||
| case_template_id = Column(String(256), nullable=True) | |||
There was a problem hiding this comment.
Since there is a modification in the model, shouldn't there be some migration scripts, so that users with a previous version of DFIR-IRIS are able to migrate?
source/app/business/cases.py
Outdated
| @@ -82,13 +80,12 @@ def cases_exists(identifier): | |||
| def cases_create(request_data): | |||
| # TODO remove caseid doesn't seems to be useful for call_modules_hook => remove argument | |||
| request_data = call_modules_hook('on_preload_case_create', request_data, None) | |||
|
|
|||
| case_template_id = request_data.pop('case_template_id', None) | |||
There was a problem hiding this comment.
This should be done in the _load, with marshmallow.
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) | ||
|
|
||
| def update_from_dict(self, data: dict): |
There was a problem hiding this comment.
Would this work with a load and argument instance, rather than adding method update_from_dict?
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) | ||
|
|
||
| def update_from_dict(self, data: dict): |
There was a problem hiding this comment.
Would this work with a load and argument instance, rather than adding method update_from_dict?
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) | ||
|
|
||
| def update_from_dict(self, data: dict): |
There was a problem hiding this comment.
Does not seem to be called. Method can be removed. The same remark applies down there for the CaseResponse object
We’ve developed a webhook-based integration feature that enables IRIS to send structured requests to external tools and display responses in a visually intuitive format. More details are outlined in the following document:
IRIS_Documentation_Final.docx
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Style
Chores