Skip to content

Commit 8da35a4

Browse files
committed
fix: address remaining issues
- Fix path traversal vulnerability with strict regex validation - Implement unified inclusive date boundaries across components - Remove problematic session-level skip check in history manager - Add date range validation in CLI - Update test expectations for new boundary semantics - Remove complex date conversion logic from main.py
1 parent 4fb6aa5 commit 8da35a4

6 files changed

Lines changed: 135 additions & 59 deletions

File tree

src/claude_monitor/cli/main.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ def _run_table_view(
386386

387387
try:
388388
# Parse date filters early so they can be used for both current and historical data
389-
from datetime import datetime, timedelta
389+
from datetime import datetime
390390

391391
from claude_monitor.utils.time_utils import TimezoneHandler
392392

@@ -401,17 +401,21 @@ def _parse_date(date_str: Optional[str]):
401401
except ValueError:
402402
continue
403403
print_themed(
404-
f"Invalid date format: {date_str}. Use YYYY-MM-DD.", style="warning"
404+
f"Invalid date format: {date_str}. Use one of: YYYY-MM-DD, YYYY.MM.DD, YYYY/MM/DD.",
405+
style="warning",
405406
)
406407
return None
407408

408409
start_dt = _parse_date(getattr(args, "start_date", None))
409410
end_dt = _parse_date(getattr(args, "end_date", None))
410411

411-
# Note: end_dt is already inclusive in history_manager
412-
end_dt_inclusive = (
413-
end_dt + timedelta(days=1) if end_dt else None
414-
) # For aggregator (needs exclusive end)
412+
# Validate date range
413+
if start_dt and end_dt and start_dt > end_dt:
414+
print_themed(
415+
f"Error: start_date ({getattr(args, 'start_date', None)}) must be on or before end_date ({getattr(args, 'end_date', None)})",
416+
style="error",
417+
)
418+
return
415419

416420
# Create aggregator with appropriate mode
417421
aggregator = UsageAggregator(
@@ -425,7 +429,7 @@ def _parse_date(date_str: Optional[str]):
425429

426430
# Get aggregated data with date filters
427431
logger.info(f"Loading {view_mode} usage data...")
428-
aggregated_data = aggregator.aggregate(start_dt, end_dt_inclusive)
432+
aggregated_data = aggregator.aggregate(start_dt, end_dt)
429433

430434
# Initialize history manager for daily and monthly views
431435
history_mode = getattr(args, "history", "auto")
@@ -475,9 +479,7 @@ def _parse_date(date_str: Optional[str]):
475479
aggregation_mode="daily",
476480
timezone=args.timezone,
477481
)
478-
current_daily = daily_aggregator.aggregate(
479-
start_dt, end_dt_inclusive
480-
)
482+
current_daily = daily_aggregator.aggregate(start_dt, end_dt)
481483

482484
# Load historical daily data
483485
daily_historical = history_manager.load_historical_daily_data(

src/claude_monitor/core/settings.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,13 @@ def _get_system_time_format() -> str:
139139
)
140140

141141
start_date: Optional[str] = Field(
142-
default=None, description="Start date for filtering data (YYYY-MM-DD format)"
142+
default=None,
143+
description="Start date for filtering data (formats: YYYY-MM-DD, YYYY.MM.DD, YYYY/MM/DD)",
143144
)
144145

145146
end_date: Optional[str] = Field(
146-
default=None, description="End date for filtering data (YYYY-MM-DD format)"
147+
default=None,
148+
description="End date for filtering data (formats: YYYY-MM-DD, YYYY.MM.DD, YYYY/MM/DD)",
147149
)
148150

149151
custom_limit_tokens: Optional[int] = Field(

src/claude_monitor/data/aggregator.py

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import logging
88
from collections import defaultdict
99
from dataclasses import dataclass, field
10-
from datetime import datetime
10+
from datetime import datetime, timedelta
1111
from typing import Any, Callable, Dict, List, Optional
1212

1313
from claude_monitor.core.models import SessionBlock, UsageEntry, normalize_model_name
@@ -105,7 +105,9 @@ def __init__(
105105
self.data_path = data_path
106106
self.aggregation_mode = aggregation_mode
107107
self.timezone = timezone
108-
self.timezone_handler = TimezoneHandler()
108+
# Initialize handler with the user-selected timezone so subsequent
109+
# conversions and localizations use it consistently.
110+
self.timezone_handler = TimezoneHandler(timezone)
109111

110112
def _aggregate_by_period(
111113
self,
@@ -121,23 +123,48 @@ def _aggregate_by_period(
121123
entries: List of usage entries
122124
period_key_func: Function to extract period key from timestamp
123125
period_type: Type of period ('date' or 'month')
124-
start_date: Optional start date filter
125-
end_date: Optional end date filter
126+
start_date: Optional start date filter (inclusive)
127+
end_date: Optional end date filter (inclusive - includes the whole day)
126128
127129
Returns:
128130
List of aggregated data dictionaries
131+
132+
Note:
133+
Both start_date and end_date are inclusive. If end_date is provided,
134+
all entries from that entire day are included (up to 23:59:59.999999).
129135
"""
130136
period_data: Dict[str, AggregatedPeriod] = {}
131137

138+
# Normalize filter boundaries into the configured timezone for
139+
# consistent, intuitive "whole-day inclusive" semantics.
140+
norm_start = (
141+
self.timezone_handler.to_timezone(start_date, self.timezone)
142+
if start_date
143+
else None
144+
)
145+
norm_end = (
146+
self.timezone_handler.to_timezone(end_date, self.timezone)
147+
if end_date
148+
else None
149+
)
150+
132151
for entry in entries:
133-
# Apply date filters
134-
if start_date and entry.timestamp < start_date:
135-
continue
136-
if end_date and entry.timestamp > end_date:
137-
continue
152+
# Convert entry timestamp to the configured timezone for filtering
153+
# and period-key extraction.
154+
ts_local = self.timezone_handler.to_timezone(entry.timestamp, self.timezone)
138155

139-
# Get period key
140-
period_key = period_key_func(entry.timestamp)
156+
# Apply date filters (inclusive boundaries in local timezone)
157+
if norm_start and ts_local < norm_start:
158+
continue
159+
# For end_date, include all entries up to the end of that day.
160+
# Exclude entries >= next day's midnight in local timezone.
161+
if norm_end:
162+
next_day = norm_end + timedelta(days=1)
163+
if ts_local >= next_day:
164+
continue
165+
166+
# Get period key using local time
167+
period_key = period_key_func(ts_local)
141168

142169
# Get or create period aggregate
143170
if period_key not in period_data:

src/claude_monitor/data/history_manager.py

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import logging
1111
from datetime import datetime, timedelta
1212
from pathlib import Path
13-
from typing import Any, Dict, List, Optional, Set
13+
from typing import Any, Dict, List, Optional
1414

1515
logger = logging.getLogger(__name__)
1616

@@ -29,8 +29,7 @@ def __init__(self, data_dir: Optional[Path] = None):
2929
self.daily_dir = self.data_dir / "daily"
3030
self.daily_dir.mkdir(parents=True, exist_ok=True)
3131

32-
# Keep track of which dates have been saved this session to avoid duplicates
33-
self._saved_dates: Set[str] = set()
32+
# Session-level saved-date tracking removed to avoid short-circuiting logic
3433

3534
def _get_daily_file_path(self, date_str: str) -> Path:
3635
"""Get the file path for a specific date's data.
@@ -84,10 +83,6 @@ def save_daily_data(
8483
logger.warning("Daily data missing 'date' field, skipping")
8584
continue
8685

87-
# Skip if already saved in this session (unless overwrite)
88-
if not overwrite and date_str in self._saved_dates:
89-
continue
90-
9186
file_path = self._get_daily_file_path(date_str)
9287

9388
# Check if file exists and whether to overwrite
@@ -97,9 +92,8 @@ def save_daily_data(
9792
with open(file_path, "r", encoding="utf-8") as f:
9893
existing_data = json.load(f)
9994

100-
# If the data is identical, skip
95+
# If the data is identical, skip writing
10196
if existing_data == day_data:
102-
self._saved_dates.add(date_str)
10397
continue
10498

10599
# Compare total information to decide which data to keep
@@ -130,7 +124,7 @@ def save_daily_data(
130124
and existing_entries >= new_entries
131125
and existing_cost >= new_cost
132126
):
133-
self._saved_dates.add(date_str)
127+
# Keep existing file; no write needed
134128
continue
135129

136130
# Otherwise, save the new data (it has more information)
@@ -145,7 +139,6 @@ def save_daily_data(
145139
json.dump(day_data, f, indent=2, default=str, ensure_ascii=False)
146140
temp_file.replace(file_path)
147141

148-
self._saved_dates.add(date_str)
149142
saved_count += 1
150143
logger.debug(f"Saved historical data for {date_str}")
151144

@@ -165,15 +158,17 @@ def load_historical_daily_data(
165158
) -> List[Dict[str, Any]]:
166159
"""Load historical daily data within the specified range.
167160
168-
Both start_date and end_date are inclusive when specified.
169-
170161
Args:
171-
start_date: Start date for data retrieval
172-
end_date: End date for data retrieval
162+
start_date: Start date for data retrieval (inclusive)
163+
end_date: End date for data retrieval (inclusive)
173164
days_back: Alternative to date range - get last N days of data
174165
175166
Returns:
176167
List of historical daily data dictionaries
168+
169+
Note:
170+
Both start_date and end_date are inclusive. For example, specifying
171+
end_date as 2024-12-15 will include the file for 2024-12-15.
177172
"""
178173
historical_data = []
179174

src/tests/test_aggregator.py

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,8 @@ def test_aggregate_daily_with_date_filter(
349349
) -> None:
350350
"""Test daily aggregation with date filters."""
351351
start_date = datetime(2024, 1, 15, tzinfo=timezone.utc)
352-
end_date = datetime(
353-
2024, 1, 31, 23, 59, 59, tzinfo=timezone.utc
354-
) # Include the whole day
352+
# end_date is inclusive - pass Jan 31 to include all of Jan 31
353+
end_date = datetime(2024, 1, 31, tzinfo=timezone.utc)
355354

356355
result = aggregator.aggregate_daily(sample_entries, start_date, end_date)
357356

@@ -644,7 +643,8 @@ def test_aggregate_daily_with_date_filters(
644643

645644
# Filter for days 3-7 (Jan 3 to Jan 7)
646645
start_date = datetime(2024, 1, 3, tzinfo=timezone.utc)
647-
end_date = datetime(2024, 1, 8, tzinfo=timezone.utc) # End is exclusive
646+
# end_date is inclusive - to get Jan 3-7, pass Jan 7
647+
end_date = datetime(2024, 1, 7, tzinfo=timezone.utc)
648648

649649
result = aggregator.aggregate_daily(entries, start_date, end_date)
650650

@@ -729,7 +729,8 @@ def test_aggregate_with_date_filters(
729729

730730
# Test with date filters
731731
start_date = datetime(2024, 1, 2, tzinfo=timezone.utc)
732-
end_date = datetime(2024, 1, 4, tzinfo=timezone.utc)
732+
# end_date is inclusive - to get Jan 2-3, pass Jan 3
733+
end_date = datetime(2024, 1, 3, tzinfo=timezone.utc)
733734

734735
result = aggregator.aggregate(start_date=start_date, end_date=end_date)
735736

@@ -741,3 +742,49 @@ def test_aggregate_with_date_filters(
741742
# Test without filters - should return all
742743
result_all = aggregator.aggregate()
743744
assert len(result_all) == 5
745+
746+
def test_timezone_grouping_and_filters(self, tmp_path) -> None:
747+
"""Entries should be grouped and filtered using the selected timezone."""
748+
from claude_monitor.core.models import UsageEntry
749+
750+
# Two entries around the UTC day boundary
751+
e1 = UsageEntry(
752+
timestamp=datetime(2023, 12, 31, 23, 30, tzinfo=timezone.utc),
753+
input_tokens=100,
754+
output_tokens=50,
755+
cache_creation_tokens=0,
756+
cache_read_tokens=0,
757+
cost_usd=0.001,
758+
model="m",
759+
message_id="a",
760+
request_id="a",
761+
)
762+
e2 = UsageEntry(
763+
timestamp=datetime(2024, 1, 1, 0, 30, tzinfo=timezone.utc),
764+
input_tokens=200,
765+
output_tokens=100,
766+
cache_creation_tokens=0,
767+
cache_read_tokens=0,
768+
cost_usd=0.002,
769+
model="m",
770+
message_id="b",
771+
request_id="b",
772+
)
773+
774+
entries = [e1, e2]
775+
776+
# Under UTC they should fall into different dates (2023-12-31 and 2024-01-01)
777+
agg_utc = UsageAggregator(data_path=str(tmp_path), timezone="UTC")
778+
res_utc = agg_utc.aggregate_daily(entries)
779+
assert len(res_utc) == 2
780+
assert res_utc[0]["date"] == "2023-12-31"
781+
assert res_utc[1]["date"] == "2024-01-01"
782+
783+
# Under America/New_York (UTC-5) both timestamps belong to 2023-12-31
784+
agg_est = UsageAggregator(data_path=str(tmp_path), timezone="America/New_York")
785+
res_est = agg_est.aggregate_daily(entries)
786+
assert len(res_est) == 1
787+
assert res_est[0]["date"] == "2023-12-31"
788+
# Validate totals
789+
assert res_est[0]["input_tokens"] == 300
790+
assert res_est[0]["output_tokens"] == 150

src/tests/test_history_manager.py

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ def test_initialization(
3131
assert history_manager.data_dir == temp_dir
3232
assert history_manager.daily_dir == temp_dir / "daily"
3333
assert history_manager.daily_dir.exists()
34-
assert history_manager._saved_dates == set()
3534

3635
def test_get_daily_file_path(self, history_manager: HistoryManager) -> None:
3736
"""Test file path generation for daily data."""
@@ -82,30 +81,40 @@ def test_save_daily_data(self, history_manager: HistoryManager) -> None:
8281
def test_save_daily_data_no_overwrite(
8382
self, history_manager: HistoryManager
8483
) -> None:
85-
"""Test that save_daily_data doesn't overwrite by default."""
84+
"""Test that save_daily_data doesn't overwrite when existing data has more information."""
8685
daily_data = [
8786
{
8887
"date": "2024-12-15",
89-
"input_tokens": 1000,
90-
"output_tokens": 500,
91-
"total_cost": 0.015,
88+
"input_tokens": 2000,
89+
"output_tokens": 1000,
90+
"total_cost": 0.030,
91+
"entries_count": 10,
9292
}
9393
]
9494

95-
# Save first time
95+
# Save first time with more data
9696
saved_count = history_manager.save_daily_data(daily_data)
9797
assert saved_count == 1
9898

99-
# Modify data and try to save again
100-
daily_data[0]["input_tokens"] = 2000
101-
saved_count = history_manager.save_daily_data(daily_data, overwrite=False)
99+
# Try to save again with less data
100+
less_data = [
101+
{
102+
"date": "2024-12-15",
103+
"input_tokens": 1000,
104+
"output_tokens": 500,
105+
"total_cost": 0.015,
106+
"entries_count": 5,
107+
}
108+
]
109+
saved_count = history_manager.save_daily_data(less_data, overwrite=False)
102110
assert saved_count == 0 # Should not save again
103111

104112
# Verify original data is preserved
105113
file_path = history_manager._get_daily_file_path("2024-12-15")
106114
with open(file_path, "r") as f:
107115
saved_data = json.load(f)
108-
assert saved_data["input_tokens"] == 1000
116+
assert saved_data["input_tokens"] == 2000
117+
assert saved_data["output_tokens"] == 1000
109118

110119
def test_save_daily_data_with_overwrite(
111120
self, history_manager: HistoryManager
@@ -368,9 +377,6 @@ def test_save_daily_data_with_existing_better_data(
368377
]
369378
history_manager.save_daily_data(initial_data)
370379

371-
# Clear saved dates to allow checking existing file
372-
history_manager._saved_dates.clear()
373-
374380
# Try to save data with fewer total tokens
375381
new_data = [
376382
{
@@ -411,9 +417,6 @@ def test_save_daily_data_updates_with_more_info(
411417
]
412418
history_manager.save_daily_data(initial_data)
413419

414-
# Clear saved dates
415-
history_manager._saved_dates.clear()
416-
417420
# Save new data with more total tokens
418421
new_data = [
419422
{

0 commit comments

Comments
 (0)