Fragile ISO datetime parsing antipattern .replace("Z", "+00:00") causes time shift and validation errors
- Dominant language
- Python
- Stars
- 528
- Forks
- 141
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 6
Description
## Problem Statement
Across several tool modules in `server/secops/secops_mcp/tools/`, ISO-8601 datetime strings are parsed using the ad-hoc pattern:
```python
start_dt = datetime.fromisoformat(start_time.replace("Z", "+00:00"))
```
This pattern has several functional and correctness issues:
1. **Silent Time-Shift with Non-UTC Offsets**:
If a user or LLM agent supplies an ISO timestamp with an explicit non-UTC offset (for example `"2025-01-20T12:00:00-05:00"` which is 17:00:00 UTC):
- `datetime.fromisoformat()` produces a timezone-aware datetime with `tzinfo=UTC-5`.
- When passed to Chronicle SDK methods (e.g. `list_detections`, `test_rule`), the SDK formats the datetime using:
```python
extra_params["startTime"] = start_time.strftime("%Y-%m-%dT%H:%M:%S.%fZ")
```
- `strftime` formats the **local wall-clock hours** (`12:00:00`) and attaches a literal `Z`, sending `"2025-01-20T12:00:00.000000Z"` to Chronicle API instead of the correct `"2025-01-20T17:00:00.000000Z"`.
- Without `.astimezone(timezone.utc)`, this results in a silent time shift in API queries.
2. **Brittle String Replacement**:
- A global `.replace("Z", "+00:00")` replaces any uppercase `Z` in the entire string.
- It fails on lowercase `'z'` (e.g. `"2025-01-20T00:00:00z"`), raising an uncaught `ValueError: Invalid isoformat string`.
- In Python 3.11+, `fromisoformat()` natively parses trailing `"Z"` without needing `.replace()`.
3. **Naive vs. Aware Inconsistencies**:
- Strings without an explicit timezone (e.g. `"2025-01-20T00:00:00"`) produce naive datetimes (`tzinfo=None`), which can cause `TypeError: can't compare offset-naive and offset-aware datetimes` if mixed with aware datetimes.
---
## Occurrences in the Codebase
The antipattern is currently used in the following locations:
- **`server/secops/secops_mcp/tools/log_ingestion.py`** (Lines 122, 124) in `ingest_log`:
```python
ingestion_params['log_entry_time'] = datetime.fromisoformat(log_entry_time.replace('Z', '+00:00'))
ingestion_params['collection_time'] = datetime.fromisoformat(collection_time.replace('Z', '+00:00'))
```
- **`server/secops/secops_mcp/tools/curated_rules_management.py`** (Lines 321, 322) in `list_curated_rule_detections`:
```python
start_dt = datetime.fromisoformat(start_time.replace("Z", "+00:00"))
end_dt = datetime.fromisoformat(end_time.replace("Z", "+00:00"))
```
- **`server/secops/secops_mcp/tools/security_rules.py`** (Lines 943, 948) in `test_rule`:
```python
start_dt = datetime.fromisoformat(start_time.replace("Z", "+00:00"))
end_dt = datetime.fromisoformat(end_time.replace("Z", "+00:00"))
```
- **`server/secops/secops_mcp/tools/security_rules.py`** (Lines 1227, 1232) in `create_retrohunt`:
```python
start_dt = datetime.fromisoformat(start_time.replace("Z", "+00:00"))
end_dt = datetime.fromisoformat(end_time.replace("Z", "+00:00"))
```
- **`server/secops/secops_mcp/tools/security_rules.py`** (Lines 321, 326) in PR #283 (`get_rule_detections`).
---
## Proposed Fix
1. Add a shared, robust datetime parser in `server/secops/secops_mcp/utils.py`:
```python
def parse_iso_datetime(time_str: str) -> datetime:
"""Parses an ISO 8601 string and returns a UTC timezone-aware datetime."""
if time_str.endswith(("z", "Z")):
time_str = time_str[:-1] + "+00:00"
dt = datetime.fromisoformat(time_str)
if dt.tzinfo is None:
dt = dt.replace(tzinfo=timezone.utc)
return dt.astimezone(timezone.utc)
```
2. Refactor existing tools in `log_ingestion.py`, `curated_rules_management.py`, and `security_rules.py` to use `parse_iso_datetime` (or `parse_time_range`).
3. Add unit tests covering:
- Trailing `"Z"` and `"z"`
- Positive and negative timezone offsets (`"+02:00"`, `"-05:00"`) ensuring proper normalization to UTC
- Naive ISO timestamps defaulting to UTC
- Invalid formats raising `ValueError` cleanly
Contributor guide
Research direction
Start by reading the datetime handling in server/secops/secops_mcp/tools/log_ingestion.py, curated_rules_management.py, and security_rules.py, then inspect server/secops/secops_mcp/utils.py as the proposed shared location. Trace the listed tool entry points and add focused unit coverage for trailing Z/z, timezone offsets, naive timestamps, and invalid formats; done means all inputs normalize correctly and invalid values raise ValueError cleanly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 70/100