fix: protect admin API endpoints; stop rejecting flood-peak measurements
CI/CD Pipeline - Northern Thailand Ping River Monitor / Test Suite (3.11) (push) Failing after 25s
CI/CD Pipeline - Northern Thailand Ping River Monitor / Build Docker Image (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Integration Test with Services (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Deploy to Staging (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Deploy to Production (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Performance Test (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Code Quality (push) Successful in 12s
Documentation / Validate Documentation (push) Failing after 9s
Documentation / Generate API Documentation (push) Successful in 10s
Documentation / Build Sphinx Documentation (push) Successful in 16s
CI/CD Pipeline - Northern Thailand Ping River Monitor / Cleanup (push) Successful in 0s
Documentation / Documentation Summary (push) Successful in 2s
CI/CD Pipeline - Northern Thailand Ping River Monitor / Test Suite (3.11) (push) Failing after 25s
CI/CD Pipeline - Northern Thailand Ping River Monitor / Build Docker Image (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Integration Test with Services (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Deploy to Staging (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Deploy to Production (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Performance Test (push) Skipped
CI/CD Pipeline - Northern Thailand Ping River Monitor / Code Quality (push) Successful in 12s
Documentation / Validate Documentation (push) Failing after 9s
Documentation / Generate API Documentation (push) Successful in 10s
Documentation / Build Sphinx Documentation (push) Successful in 16s
CI/CD Pipeline - Northern Thailand Ping River Monitor / Cleanup (push) Successful in 0s
Documentation / Documentation Summary (push) Successful in 2s
Security: POST/PUT/DELETE /stations, POST /scrape/trigger and GET /config now require an X-API-Key header matching ADMIN_API_KEY. Secure by default - with no key configured those endpoints return 503 instead of being open. Comparison via secrets.compare_digest. Dashboard and read endpoints stay public. Data: the validator rejected any measurement whose discharge_percent exceeded 200 - which silently deleted the Oct 2024 record-flood peaks (the river genuinely ran at 201-226% of channel capacity). The cap is now 500%, and an out-of-range percent nulls that auxiliary field instead of discarding the whole row (water level and discharge are the data that matter). Surfaced by the user's historical backfill log.
This commit is contained in:
+10
-4
@@ -22,7 +22,9 @@ class DataValidator:
|
|||||||
DISCHARGE_MIN = 0.0 # cms
|
DISCHARGE_MIN = 0.0 # cms
|
||||||
DISCHARGE_MAX = 10000.0 # cms
|
DISCHARGE_MAX = 10000.0 # cms
|
||||||
DISCHARGE_PERCENT_MIN = 0.0
|
DISCHARGE_PERCENT_MIN = 0.0
|
||||||
DISCHARGE_PERCENT_MAX = 200.0 # Allow some overflow
|
# % of channel capacity. Major floods genuinely exceed 200% (Oct 2024 peaked
|
||||||
|
# at 226%); values beyond 500% are treated as data errors.
|
||||||
|
DISCHARGE_PERCENT_MAX = 500.0
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def validate_measurement(cls, measurement: Dict[str, Any]) -> bool:
|
def validate_measurement(cls, measurement: Dict[str, Any]) -> bool:
|
||||||
@@ -59,7 +61,10 @@ class DataValidator:
|
|||||||
logger.warning(f"Discharge out of range: {discharge}")
|
logger.warning(f"Discharge out of range: {discharge}")
|
||||||
return False
|
return False
|
||||||
|
|
||||||
# Validate discharge percent if present
|
# Validate discharge percent if present. This is an auxiliary field:
|
||||||
|
# an implausible value must not cost us the water level and discharge
|
||||||
|
# (rejecting rows here silently deleted the Oct 2024 flood peaks), so
|
||||||
|
# out-of-range percents are nulled and the measurement is kept.
|
||||||
if measurement.get("discharge_percent") is not None:
|
if measurement.get("discharge_percent") is not None:
|
||||||
discharge_percent = float(measurement["discharge_percent"])
|
discharge_percent = float(measurement["discharge_percent"])
|
||||||
if not (
|
if not (
|
||||||
@@ -68,9 +73,10 @@ class DataValidator:
|
|||||||
<= cls.DISCHARGE_PERCENT_MAX
|
<= cls.DISCHARGE_PERCENT_MAX
|
||||||
):
|
):
|
||||||
logger.warning(
|
logger.warning(
|
||||||
f"Discharge percent out of range: {discharge_percent}"
|
f"Discharge percent out of range ({discharge_percent}); "
|
||||||
|
"keeping measurement with discharge_percent=None"
|
||||||
)
|
)
|
||||||
return False
|
measurement["discharge_percent"] = None
|
||||||
|
|
||||||
# Validate station ID
|
# Validate station ID
|
||||||
station_id = measurement["station_id"]
|
station_id = measurement["station_id"]
|
||||||
|
|||||||
+48
-12
@@ -5,27 +5,38 @@ FastAPI web interface for water monitoring system
|
|||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
import os
|
import os
|
||||||
|
import secrets
|
||||||
import time
|
import time
|
||||||
from contextlib import asynccontextmanager
|
from contextlib import asynccontextmanager
|
||||||
from datetime import datetime, timedelta
|
from datetime import datetime, timedelta
|
||||||
from threading import Lock
|
from threading import Lock
|
||||||
from typing import Any, Dict, List
|
from typing import Any, Dict, List, Optional
|
||||||
|
|
||||||
import requests
|
import requests
|
||||||
from fastapi import BackgroundTasks, FastAPI, HTTPException, Query
|
from fastapi import BackgroundTasks, Depends, FastAPI, Header, HTTPException, Query
|
||||||
from fastapi.middleware.cors import CORSMiddleware
|
from fastapi.middleware.cors import CORSMiddleware
|
||||||
from fastapi.responses import HTMLResponse
|
from fastapi.responses import HTMLResponse
|
||||||
from fastapi.staticfiles import StaticFiles
|
from fastapi.staticfiles import StaticFiles
|
||||||
|
|
||||||
from .config import Config
|
from .config import Config
|
||||||
from .health_check import (APIHealthCheck, DatabaseHealthCheck,
|
from .health_check import (
|
||||||
HealthCheckManager, MemoryHealthCheck)
|
APIHealthCheck,
|
||||||
|
DatabaseHealthCheck,
|
||||||
|
HealthCheckManager,
|
||||||
|
MemoryHealthCheck,
|
||||||
|
)
|
||||||
from .logging_config import get_logger, setup_logging
|
from .logging_config import get_logger, setup_logging
|
||||||
from .metrics import get_metrics_collector, increment_counter, set_gauge
|
from .metrics import get_metrics_collector, increment_counter, set_gauge
|
||||||
from .postgres_history import PostgresHistory
|
from .postgres_history import PostgresHistory
|
||||||
from .schemas import (HealthResponse, MeasurementResponse, MetricsResponse,
|
from .schemas import (
|
||||||
ScrapingStatusResponse, StationCreateModel,
|
HealthResponse,
|
||||||
StationResponse, StationUpdateModel)
|
MeasurementResponse,
|
||||||
|
MetricsResponse,
|
||||||
|
ScrapingStatusResponse,
|
||||||
|
StationCreateModel,
|
||||||
|
StationResponse,
|
||||||
|
StationUpdateModel,
|
||||||
|
)
|
||||||
from .thaiwater import ThaiWaterClient
|
from .thaiwater import ThaiWaterClient
|
||||||
from .water_scraper_v3 import EnhancedWaterMonitorScraper
|
from .water_scraper_v3 import EnhancedWaterMonitorScraper
|
||||||
|
|
||||||
@@ -40,6 +51,23 @@ FORECAST_CACHE: Dict[str, tuple] = {}
|
|||||||
FORECAST_CACHE_LOCK = Lock()
|
FORECAST_CACHE_LOCK = Lock()
|
||||||
FORECAST_TTL = 900 # 15 minutes
|
FORECAST_TTL = 900 # 15 minutes
|
||||||
|
|
||||||
|
# Admin API protection. Read/dashboard endpoints stay public; anything that
|
||||||
|
# mutates state or leaks configuration requires the X-API-Key header matching
|
||||||
|
# ADMIN_API_KEY. Secure by default: with no key configured, those endpoints
|
||||||
|
# are disabled entirely rather than open.
|
||||||
|
ADMIN_API_KEY = os.getenv("ADMIN_API_KEY")
|
||||||
|
|
||||||
|
|
||||||
|
def require_admin_key(x_api_key: Optional[str] = Header(None, alias="X-API-Key")):
|
||||||
|
if not ADMIN_API_KEY:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=503,
|
||||||
|
detail="Admin API disabled: ADMIN_API_KEY is not configured on the server",
|
||||||
|
)
|
||||||
|
if not x_api_key or not secrets.compare_digest(x_api_key, ADMIN_API_KEY):
|
||||||
|
raise HTTPException(status_code=401, detail="Invalid or missing X-API-Key")
|
||||||
|
|
||||||
|
|
||||||
# Dashboard HTML is loaded once at import from src/static/dashboard.html.
|
# Dashboard HTML is loaded once at import from src/static/dashboard.html.
|
||||||
_DASHBOARD_HTML_PATH = os.path.join(
|
_DASHBOARD_HTML_PATH = os.path.join(
|
||||||
os.path.dirname(os.path.abspath(__file__)), "static", "dashboard.html"
|
os.path.dirname(os.path.abspath(__file__)), "static", "dashboard.html"
|
||||||
@@ -269,7 +297,11 @@ async def get_stations():
|
|||||||
return stations
|
return stations
|
||||||
|
|
||||||
|
|
||||||
@app.post("/stations", response_model=StationResponse)
|
@app.post(
|
||||||
|
"/stations",
|
||||||
|
response_model=StationResponse,
|
||||||
|
dependencies=[Depends(require_admin_key)],
|
||||||
|
)
|
||||||
async def create_station(station: StationCreateModel):
|
async def create_station(station: StationCreateModel):
|
||||||
"""Create a new monitoring station"""
|
"""Create a new monitoring station"""
|
||||||
increment_counter("api_requests", labels={"endpoint": "create_station"})
|
increment_counter("api_requests", labels={"endpoint": "create_station"})
|
||||||
@@ -319,7 +351,11 @@ async def create_station(station: StationCreateModel):
|
|||||||
raise HTTPException(status_code=500, detail=str(e))
|
raise HTTPException(status_code=500, detail=str(e))
|
||||||
|
|
||||||
|
|
||||||
@app.put("/stations/{station_id}", response_model=StationResponse)
|
@app.put(
|
||||||
|
"/stations/{station_id}",
|
||||||
|
response_model=StationResponse,
|
||||||
|
dependencies=[Depends(require_admin_key)],
|
||||||
|
)
|
||||||
async def update_station(station_id: int, updates: StationUpdateModel):
|
async def update_station(station_id: int, updates: StationUpdateModel):
|
||||||
"""Update an existing monitoring station"""
|
"""Update an existing monitoring station"""
|
||||||
increment_counter("api_requests", labels={"endpoint": "update_station"})
|
increment_counter("api_requests", labels={"endpoint": "update_station"})
|
||||||
@@ -374,7 +410,7 @@ async def update_station(station_id: int, updates: StationUpdateModel):
|
|||||||
raise HTTPException(status_code=500, detail=str(e))
|
raise HTTPException(status_code=500, detail=str(e))
|
||||||
|
|
||||||
|
|
||||||
@app.delete("/stations/{station_id}")
|
@app.delete("/stations/{station_id}", dependencies=[Depends(require_admin_key)])
|
||||||
async def delete_station(station_id: int):
|
async def delete_station(station_id: int):
|
||||||
"""Delete a monitoring station"""
|
"""Delete a monitoring station"""
|
||||||
increment_counter("api_requests", labels={"endpoint": "delete_station"})
|
increment_counter("api_requests", labels={"endpoint": "delete_station"})
|
||||||
@@ -594,7 +630,7 @@ async def get_station_measurements(
|
|||||||
raise HTTPException(status_code=500, detail=str(e))
|
raise HTTPException(status_code=500, detail=str(e))
|
||||||
|
|
||||||
|
|
||||||
@app.post("/scrape/trigger")
|
@app.post("/scrape/trigger", dependencies=[Depends(require_admin_key)])
|
||||||
async def trigger_scraping(background_tasks: BackgroundTasks):
|
async def trigger_scraping(background_tasks: BackgroundTasks):
|
||||||
"""Trigger manual data scraping"""
|
"""Trigger manual data scraping"""
|
||||||
increment_counter("api_requests", labels={"endpoint": "scrape_trigger"})
|
increment_counter("api_requests", labels={"endpoint": "scrape_trigger"})
|
||||||
@@ -654,7 +690,7 @@ async def get_scraping_status():
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@app.get("/config")
|
@app.get("/config", dependencies=[Depends(require_admin_key)])
|
||||||
async def get_config():
|
async def get_config():
|
||||||
"""Get current configuration (sensitive data masked)"""
|
"""Get current configuration (sensitive data masked)"""
|
||||||
increment_counter("api_requests", labels={"endpoint": "config"})
|
increment_counter("api_requests", labels={"endpoint": "config"})
|
||||||
|
|||||||
Reference in New Issue
Block a user