Skip to content

secops_mcp_get_security_alert_by_id_bug #265

@barnabys-drew

Description

@barnabys-drew

get_security_alert_by_id docstring promises a formatted summary but returns raw json.dumps; alert_id typed optional with no validation

secops_mcp 0.1.3. secops_mcp/tools/security_alerts.py::get_security_alert_by_id advertises a "formatted string summarizing... rule name, creation time, status, severity, and associated case ID" but the implementation is return json.dumps(response) — the entire raw alert object (10+ KB with include_detections=True) as an escaped JSON string. Agents that allocate context budget based on the docstring will be surprised.

Same function takes alert_id: Optional[str] = None with no validation. Calling without it passes None to the SDK and surfaces a confusing Error retrieving security alert for None: .... do_update_security_alert has the identical defect and is a destructive operation, so the missing validation is also a defense-in-depth gap.

Current code (security_alerts.py:152–205):

async def get_security_alert_by_id(
    project_id: Optional[str] = None,
    customer_id: Optional[str] = None,
    region: Optional[str] = None,
    alert_id: Optional[str] = None,        # <-- should be required
    include_detections: bool = True
) -> str:
    """...formatted string summarizing the retrieved security alerts..."""
    try:
        chronicle = get_chronicle_client(project_id, customer_id, region)
        response = chronicle.get_alert(alert_id, include_detections)
    except Exception as e:
        return f'Error retrieving security alert for {alert_id}: {str(e)}'
    return json.dumps(response)            # <-- not a summary

Fix:

 async def get_security_alert_by_id(
+    alert_id: str,
     project_id: Optional[str] = None,
     customer_id: Optional[str] = None,
     region: Optional[str] = None,
-    alert_id: Optional[str] = None,
     include_detections: bool = True,
-) -> str:
+) -> Dict[str, Any]:
     """Get a security alert by ID directly from Chronicle SIEM.
-    ...formatted string summarizing... rule name, creation time, ...
+    Returns the Chronicle alert object as a structured dict including
+    rule metadata, detection fields, outcomes, and (when include_detections=True)
+    collection elements with event samples.
     """
+    if not alert_id:
+        return {"error": "alert_id is required"}
     try:
         chronicle = get_chronicle_client(project_id, customer_id, region)
-        response = chronicle.get_alert(alert_id, include_detections)
+        return chronicle.get_alert(alert_id, include_detections)
     except Exception as e:
-        return f'Error retrieving security alert for {alert_id}: {str(e)}'
-    return json.dumps(response)
+        logger.error(f"Error retrieving alert {alert_id}: {e}", exc_info=True)
+        return {"error": f"Error retrieving security alert for {alert_id}: {str(e)}"}

Apply the same alert_id validation to do_update_security_alert (line 208), which has the identical signature and mutates state.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions