google / google/mcp-security

secops_mcp_get_security_alert_by_id_bug

Open
#265 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.