secops_mcp_get_security_alert_by_id_bug
- Dominant language
- Python
- Stars
- 528
- Forks
- 141
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 6
Description
`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`):
```python
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:
```diff
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.
Contributor guide
Assessment
This issue has not been assessed yet.