feat: enhance error handling and configuration validation
Some checks failed
CI / lint-and-test (push) Failing after 27s

- Added a global exception handler to log unhandled exceptions and return a generic 500 JSON response without exposing details to the client.
- Updated the configuration to validate the `DATABASE_URL` format, ensuring it starts with `sqlite://` or `postgresql://`, and log warnings for invalid formats.
- Introduced safe parsing for numeric environment variables (`HTTP_PORT`, `INIT_DATA_MAX_AGE_SECONDS`) with defaults on invalid values, including logging warnings for out-of-range values.
- Enhanced the duty schedule parser to enforce limits on the number of schedule rows and the length of full names and duty strings, raising appropriate errors when exceeded.
- Updated internationalization messages to include generic error responses for import failures and parsing issues, improving user experience.
- Added unit tests to verify the new error handling and configuration validation behaviors.
This commit is contained in:
2026-03-02 23:36:03 +03:00
parent 43386b15fa
commit 7ffa727832
20 changed files with 451 additions and 70 deletions

View File

@@ -46,4 +46,6 @@ Docstrings and code comments must be in English (Google-style docstrings). UI st
- **Branches:** Gitea Flow; changes via Pull Request.
- **Testing:** pytest, 80% coverage target; unit and integration tests.
- **Config:** Environment variables (e.g. `.env`); no hardcoded secrets.
- **Database:** One logical transaction per `session_scope` — a single `commit` at the end of the business operation (e.g. in `run_import`). Repository helpers used inside such a flow (e.g. `get_or_create_user_by_full_name`) accept `commit=False` and let the caller commit once.
- **Error handling:** Do not send `str(exception)` from parsers or DB to the user. Use generic i18n keys (e.g. `import.parse_error_generic`, `import.import_error_generic`) and log the full exception server-side.
- **Cursor:** The project does not version `.cursor/`. You can mirror this file in `.cursor/rules/` locally; [AGENTS.md](AGENTS.md) is the single versioned reference for AI and maintainers.

View File

@@ -5,17 +5,17 @@ All configuration is read from the environment (e.g. `.env` via python-dotenv).
| Variable | Type / format | Default | Description |
|----------|----------------|---------|-------------|
| **BOT_TOKEN** | string | *(empty)* | Telegram bot token from [@BotFather](https://t.me/BotFather). Required for the bot to run; if unset, the entry point exits with a clear message. The server that serves the Mini App API must use the **same** token as the bot; otherwise initData validation returns `hash_mismatch`. |
| **DATABASE_URL** | string (SQLAlchemy URL) | `sqlite:///data/duty_teller.db` | Database connection URL. Example: `sqlite:///data/duty_teller.db`. |
| **DATABASE_URL** | string (SQLAlchemy URL) | `sqlite:///data/duty_teller.db` | Database connection URL. Should start with `sqlite://` or `postgresql://`; a warning is logged at startup if the format is unexpected. Example: `sqlite:///data/duty_teller.db`. |
| **MINI_APP_BASE_URL** | string (URL, no trailing slash) | *(empty)* | Base URL of the miniapp (for documentation and CORS). Trailing slash is stripped. Example: `https://your-domain.com/app`. |
| **HTTP_HOST** | string | `127.0.0.1` | Host to bind the HTTP server to. Use `127.0.0.1` to listen only on localhost; use `0.0.0.0` to accept connections from all interfaces (e.g. when behind a reverse proxy on another machine). |
| **HTTP_PORT** | integer | `8080` | Port for the HTTP server (FastAPI + static webapp). |
| **HTTP_PORT** | integer (165535) | `8080` | Port for the HTTP server (FastAPI + static webapp). Invalid or out-of-range values are clamped; non-numeric values fall back to 8080. |
| **ALLOWED_USERNAMES** | comma-separated list | *(empty)* | **Not used for access.** Kept for reference only. Access to the miniapp is controlled by **roles in the DB** (assigned by an admin via `/set_role`). |
| **ADMIN_USERNAMES** | comma-separated list | *(empty)* | Telegram usernames treated as **admin fallback** when the user has **no role in the DB**. If a user has a role in the DB, only that role applies. Example: `admin1,admin2`. |
| **ALLOWED_PHONES** | comma-separated list | *(empty)* | **Not used for access.** Kept for reference only. |
| **ADMIN_PHONES** | comma-separated list | *(empty)* | Phones treated as **admin fallback** when the user has **no role in the DB** (user sets phone via `/set_phone`). Comparison uses digits only. Example: `+7 999 123-45-67`. |
| **MINI_APP_SKIP_AUTH** | `1`, `true`, or `yes` | *(unset)* | If set, `/api/duties` and `/api/calendar-events` are allowed without Telegram initData. **Dev only — never use in production.** |
| **INIT_DATA_MAX_AGE_SECONDS** | integer | `0` | Reject Telegram initData older than this many seconds. `0` = disabled. Example: `86400` for 24 hours. |
| **CORS_ORIGINS** | comma-separated list | `*` | Allowed origins for CORS. Leave unset or set to `*` for allow-all. Example: `https://your-domain.com`. |
| **MINI_APP_SKIP_AUTH** | `1`, `true`, or `yes` | *(unset)* | If set, `/api/duties` and `/api/calendar-events` are allowed without Telegram initData. **Dev only — never use in production.** The process exits with an error if this is set and **HTTP_HOST** is not localhost (127.0.0.1). |
| **INIT_DATA_MAX_AGE_SECONDS** | integer (≥ 0) | `0` | Reject Telegram initData older than this many seconds. `0` = disabled. Invalid values fall back to 0. Example: `86400` for 24 hours. |
| **CORS_ORIGINS** | comma-separated list | `*` | Allowed origins for CORS. Leave unset or set to `*` for allow-all. **In production**, set an explicit list (e.g. `https://your-domain.com`) instead of `*` to avoid allowing arbitrary origins. Example: `https://your-domain.com`. |
| **EXTERNAL_CALENDAR_ICS_URL** | string (URL) | *(empty)* | URL of a public ICS calendar (e.g. holidays). If set, those days are highlighted on the duty grid; users can tap "i" on a cell to see the event summary. Empty = no external calendar. |
| **DUTY_DISPLAY_TZ** | string (timezone name) | `Europe/Moscow` | Timezone for the pinned duty message in groups. Example: `Europe/Moscow`, `UTC`. |
| **DUTY_PIN_NOTIFY** | `0`, `false`, or `no` to disable | `1` (enabled) | When the pinned duty message is updated on schedule, the bot sends a new message, unpins the old one and pins the new one. If enabled, pinning the new message sends a Telegram notification (“Bot pinned a message”). Set to `0`, `false`, or `no` to pin without notification. The first pin (e.g. when the bot is added to the group or on `/pin_duty`) is always silent. |

View File

@@ -8,7 +8,7 @@ import duty_teller.config as config
from fastapi import Depends, FastAPI, Request
from fastapi.middleware.cors import CORSMiddleware
from fastapi.responses import Response
from fastapi.responses import JSONResponse, Response
from fastapi.staticfiles import StaticFiles
from sqlalchemy.orm import Session
@@ -42,6 +42,16 @@ def _is_valid_calendar_token(token: str) -> bool:
app = FastAPI(title="Duty Teller API")
@app.exception_handler(Exception)
def global_exception_handler(request: Request, exc: Exception) -> JSONResponse:
"""Log unhandled exceptions and return 500 without exposing details to the client."""
log.exception("Unhandled exception: %s", exc)
return JSONResponse(
status_code=500,
content={"detail": "Internal server error"},
)
@app.get("/health", summary="Health check")
def health() -> dict:
"""Return 200 when the app is up. Used by Docker HEALTHCHECK."""
@@ -106,6 +116,18 @@ class NoCacheStaticMiddleware:
app.add_middleware(NoCacheStaticMiddleware)
# Allowed values for config.js to prevent script injection.
_VALID_LANGS = frozenset({"en", "ru"})
_VALID_LOG_LEVELS = frozenset({"debug", "info", "warning", "error"})
def _safe_js_string(value: str, allowed: frozenset[str], default: str) -> str:
"""Return value if it is in allowed set, else default. Prevents injection in config.js."""
if value in allowed:
return value
return default
@app.get(
"/app/config.js",
summary="Mini App config (language, log level)",
@@ -115,8 +137,8 @@ app.add_middleware(NoCacheStaticMiddleware)
)
def app_config_js() -> Response:
"""Return JS assigning window.__DT_LANG and window.__DT_LOG_LEVEL for the webapp. No caching."""
lang = config.DEFAULT_LANGUAGE
log_level = config.LOG_LEVEL_STR.lower()
lang = _safe_js_string(config.DEFAULT_LANGUAGE, _VALID_LANGS, "en")
log_level = _safe_js_string(config.LOG_LEVEL_STR.lower(), _VALID_LOG_LEVELS, "info")
body = f'window.__DT_LANG = "{lang}";\nwindow.__DT_LOG_LEVEL = "{log_level}";'
return Response(
content=body,
@@ -183,10 +205,10 @@ def get_team_calendar_ical(
) -> Response:
"""Return ICS calendar with all duties (event_type duty only). Token validates user."""
if not _is_valid_calendar_token(token):
return Response(status_code=404, content="Not found")
return JSONResponse(status_code=404, content={"detail": "Not found"})
user = get_user_by_calendar_token(session, token)
if user is None:
return Response(status_code=404, content="Not found")
return JSONResponse(status_code=404, content={"detail": "Not found"})
cache_key = ("team_ics",)
ics_bytes, found = ics_calendar_cache.get(cache_key)
if not found:
@@ -224,10 +246,10 @@ def get_personal_calendar_ical(
No Telegram auth; access is by secret token in the URL.
"""
if not _is_valid_calendar_token(token):
return Response(status_code=404, content="Not found")
return JSONResponse(status_code=404, content={"detail": "Not found"})
user = get_user_by_calendar_token(session, token)
if user is None:
return Response(status_code=404, content="Not found")
return JSONResponse(status_code=404, content={"detail": "Not found"})
cache_key = ("personal_ics", user.id)
ics_bytes, found = ics_calendar_cache.get(cache_key)
if not found:

View File

@@ -42,7 +42,12 @@ def _validate_duty_dates(from_date: str, to_date: str, lang: str) -> None:
try:
validate_date_range(from_date, to_date)
except DateRangeValidationError as e:
key = "dates.bad_format" if e.kind == "bad_format" else "dates.from_after_to"
key_map = {
"bad_format": "dates.bad_format",
"from_after_to": "dates.from_after_to",
"range_too_large": "dates.range_too_large",
}
key = key_map.get(e.kind, "dates.bad_format")
raise HTTPException(status_code=400, detail=t(lang, key)) from e
except ValueError as e:
# Backward compatibility if something else raises ValueError.

View File

@@ -1,7 +1,8 @@
"""Load configuration from environment (e.g. .env via python-dotenv).
BOT_TOKEN is not validated on import; call require_bot_token() in the entry point
when running the bot.
when running the bot. Numeric env vars (HTTP_PORT, INIT_DATA_MAX_AGE_SECONDS) use
safe parsing with defaults on invalid values.
"""
import logging
@@ -16,6 +17,11 @@ from duty_teller.i18n.lang import normalize_lang
load_dotenv()
logger = logging.getLogger(__name__)
# Valid port range for HTTP_PORT.
HTTP_PORT_MIN, HTTP_PORT_MAX = 1, 65535
# Project root (parent of duty_teller package). Used for webapp path, etc.
PROJECT_ROOT = Path(__file__).resolve().parent.parent
@@ -55,6 +61,48 @@ def _normalize_log_level(raw: str) -> str:
return "INFO"
def _parse_int_env(
name: str, default: int, min_val: int | None = None, max_val: int | None = None
) -> int:
"""Parse an integer from os.environ; use default on invalid or out-of-range. Log on fallback."""
raw = os.getenv(name)
if raw is None or raw == "":
return default
try:
value = int(raw.strip())
except ValueError:
logger.warning(
"Invalid %s=%r (expected integer); using default %s",
name,
raw,
default,
)
return default
if min_val is not None and value < min_val:
logger.warning(
"%s=%s is below minimum %s; using %s", name, value, min_val, min_val
)
return min_val
if max_val is not None and value > max_val:
logger.warning(
"%s=%s is above maximum %s; using %s", name, value, max_val, max_val
)
return max_val
return value
def _validate_database_url(url: str) -> bool:
"""Return True if URL looks like a supported SQLAlchemy URL (sqlite or postgres)."""
if not url or not isinstance(url, str):
return False
u = url.strip().split("?", 1)[0].lower()
return (
u.startswith("sqlite://")
or u.startswith("postgresql://")
or u.startswith("postgres://")
)
@dataclass(frozen=True)
class Settings:
"""Injectable settings built from environment. Used in tests or when env is overridden."""
@@ -105,20 +153,30 @@ class Settings:
raw_host = (os.getenv("HTTP_HOST") or "127.0.0.1").strip()
http_host = raw_host if raw_host else "127.0.0.1"
bot_username = (os.getenv("BOT_USERNAME", "") or "").strip().lstrip("@").lower()
database_url = os.getenv("DATABASE_URL", "sqlite:///data/duty_teller.db")
if not _validate_database_url(database_url):
logger.warning(
"DATABASE_URL does not look like a supported URL (sqlite:// or postgresql://); "
"DB connection may fail."
)
http_port = _parse_int_env(
"HTTP_PORT", 8080, min_val=HTTP_PORT_MIN, max_val=HTTP_PORT_MAX
)
init_data_max_age = _parse_int_env("INIT_DATA_MAX_AGE_SECONDS", 0, min_val=0)
return cls(
bot_token=bot_token,
database_url=os.getenv("DATABASE_URL", "sqlite:///data/duty_teller.db"),
database_url=database_url,
bot_username=bot_username,
mini_app_base_url=os.getenv("MINI_APP_BASE_URL", "").rstrip("/"),
http_host=http_host,
http_port=int(os.getenv("HTTP_PORT", "8080")),
http_port=http_port,
allowed_usernames=allowed,
admin_usernames=admin,
allowed_phones=allowed_phones,
admin_phones=admin_phones,
mini_app_skip_auth=os.getenv("MINI_APP_SKIP_AUTH", "").strip()
in ("1", "true", "yes"),
init_data_max_age_seconds=int(os.getenv("INIT_DATA_MAX_AGE_SECONDS", "0")),
init_data_max_age_seconds=init_data_max_age,
cors_origins=cors,
external_calendar_ics_url=os.getenv(
"EXTERNAL_CALENDAR_ICS_URL", ""

View File

@@ -7,6 +7,7 @@ from datetime import datetime, timezone
from sqlalchemy.orm import Session
import duty_teller.config as config
from duty_teller.db.schemas import DUTY_EVENT_TYPES
from duty_teller.db.models import (
User,
Duty,
@@ -201,14 +202,19 @@ def get_or_create_user(
return user
def get_or_create_user_by_full_name(session: Session, full_name: str) -> User:
def get_or_create_user_by_full_name(
session: Session, full_name: str, *, commit: bool = True
) -> User:
"""Find user by exact full_name or create one (for duty-schedule import).
New users have telegram_user_id=None and name_manually_edited=True.
When commit=False, caller is responsible for committing (e.g. single commit
per import in run_import).
Args:
session: DB session.
full_name: Exact full name to match or set.
commit: If True, commit immediately. If False, caller commits.
Returns:
User instance (existing or newly created).
@@ -225,8 +231,11 @@ def get_or_create_user_by_full_name(session: Session, full_name: str) -> User:
name_manually_edited=True,
)
session.add(user)
if commit:
session.commit()
session.refresh(user)
else:
session.flush() # Assign id so caller can use user.id before commit
return user
@@ -447,11 +456,13 @@ def insert_duty(
user_id: User id.
start_at: Start time UTC, ISO 8601 with Z (e.g. 2025-01-15T09:00:00Z).
end_at: End time UTC, ISO 8601 with Z.
event_type: One of "duty", "unavailable", "vacation". Default "duty".
event_type: One of "duty", "unavailable", "vacation". Invalid values are stored as "duty".
Returns:
Created Duty instance.
"""
if event_type not in DUTY_EVENT_TYPES:
event_type = "duty"
duty = Duty(
user_id=user_id,
start_at=start_at,

View File

@@ -67,7 +67,8 @@ async def set_phone(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None:
phone = " ".join(args).strip() if args else None
telegram_user_id = update.effective_user.id
def do_set_phone() -> str | None:
def do_set_phone() -> tuple[str, str | None]:
"""Returns (status, display_phone). status is 'error'|'saved'|'cleared'. display_phone for 'saved'."""
with session_scope(config.DATABASE_URL) as session:
full_name = build_full_name(
update.effective_user.first_name, update.effective_user.last_name
@@ -82,16 +83,20 @@ async def set_phone(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None:
)
user = set_user_phone(session, telegram_user_id, phone or None)
if user is None:
return "error"
return ("error", None)
if phone:
return "saved"
return "cleared"
return ("saved", user.phone or config.normalize_phone(phone))
return ("cleared", None)
result = await asyncio.get_running_loop().run_in_executor(None, do_set_phone)
result, display_phone = await asyncio.get_running_loop().run_in_executor(
None, do_set_phone
)
if result == "error":
await update.message.reply_text(t(lang, "set_phone.error"))
elif result == "saved":
await update.message.reply_text(t(lang, "set_phone.saved", phone=phone or ""))
await update.message.reply_text(
t(lang, "set_phone.saved", phone=display_phone or "")
)
else:
await update.message.reply_text(t(lang, "set_phone.cleared"))

View File

@@ -1,6 +1,7 @@
"""Import duty-schedule: /import_duty_schedule (admin only). Two steps: handover time -> JSON file."""
import asyncio
import logging
import duty_teller.config as config
from telegram import Update
@@ -16,6 +17,8 @@ from duty_teller.importers.duty_schedule import (
from duty_teller.services.import_service import run_import
from duty_teller.utils.handover import parse_handover_time
logger = logging.getLogger(__name__)
async def import_duty_schedule_cmd(
update: Update, context: ContextTypes.DEFAULT_TYPE
@@ -80,9 +83,10 @@ async def handle_duty_schedule_document(
try:
result = parse_duty_schedule(raw)
except DutyScheduleParseError as e:
logger.warning("Duty schedule parse error: %s", e, exc_info=True)
context.user_data.pop("awaiting_duty_schedule_file", None)
context.user_data.pop("handover_utc_time", None)
await update.message.reply_text(t(lang, "import.parse_error", error=str(e)))
await update.message.reply_text(t(lang, "import.parse_error_generic"))
return
def run_import_with_scope():
@@ -95,7 +99,8 @@ async def handle_duty_schedule_document(
None, run_import_with_scope
)
except Exception as e:
await update.message.reply_text(t(lang, "import.import_error", error=str(e)))
logger.exception("Import failed: %s", e)
await update.message.reply_text(t(lang, "import.import_error_generic"))
else:
total = num_duty + num_unavailable + num_vacation
unavailable_suffix = (

View File

@@ -72,7 +72,9 @@ MESSAGES: dict[str, dict[str, str]] = {
"import.send_json": "Send the duty-schedule file (JSON).",
"import.need_json": "File must have .json extension.",
"import.parse_error": "File parse error: {error}",
"import.parse_error_generic": "The file could not be parsed. Check the format and try again.",
"import.import_error": "Import error: {error}",
"import.import_error_generic": "Import failed. Please try again or contact an administrator.",
"import.done": (
"Import done: {users} users, {duties} duties{unavailable}{vacation} "
"({total} events total)."
@@ -88,6 +90,7 @@ MESSAGES: dict[str, dict[str, str]] = {
"api.access_denied": "Access denied",
"dates.bad_format": "Parameters from and to must be in YYYY-MM-DD format",
"dates.from_after_to": "from date must not be after to",
"dates.range_too_large": "Date range is too large. Request a shorter period.",
"contact.show": "Contacts",
"contact.back": "Back",
"current_duty.title": "Current Duty",
@@ -160,7 +163,9 @@ MESSAGES: dict[str, dict[str, str]] = {
"import.send_json": "Отправьте файл в формате duty-schedule (JSON).",
"import.need_json": "Нужен файл с расширением .json",
"import.parse_error": "Ошибка разбора файла: {error}",
"import.parse_error_generic": "Не удалось разобрать файл. Проверьте формат и попробуйте снова.",
"import.import_error": "Ошибка импорта: {error}",
"import.import_error_generic": "Импорт не выполнен. Попробуйте снова или обратитесь к администратору.",
"import.done": "Импорт выполнен: {users} пользователей, {duties} дежурств{unavailable}{vacation} (всего {total} событий).",
"import.done_unavailable": ", {count} недоступностей",
"import.done_vacation": ", {count} отпусков",
@@ -171,6 +176,7 @@ MESSAGES: dict[str, dict[str, str]] = {
"api.access_denied": "Доступ запрещён",
"dates.bad_format": "Параметры from и to должны быть в формате YYYY-MM-DD",
"dates.from_after_to": "Дата from не должна быть позже to",
"dates.range_too_large": "Диапазон дат слишком большой. Запросите более короткий период.",
"contact.show": "Контакты",
"contact.back": "Назад",
"current_duty.title": "Текущее дежурство",

View File

@@ -9,6 +9,11 @@ DUTY_MARKERS = frozenset({"б", "Б", "в", "В"})
UNAVAILABLE_MARKER = "Н"
VACATION_MARKER = "О"
# Limits to avoid abuse and unreasonable input.
MAX_SCHEDULE_ROWS = 500
MAX_FULL_NAME_LENGTH = 200
MAX_DUTY_STRING_LENGTH = 10000
@dataclass
class DutyScheduleEntry:
@@ -69,10 +74,24 @@ def parse_duty_schedule(raw_bytes: bytes) -> DutyScheduleResult:
except ValueError as e:
raise DutyScheduleParseError(f"Invalid meta.start_date: {start_str}") from e
# Reject dates outside current year ± 1.
today = date.today()
min_year = today.year - 1
max_year = today.year + 1
if not (min_year <= start_date.year <= max_year):
raise DutyScheduleParseError(
f"meta.start_date year must be between {min_year} and {max_year}"
)
schedule = data.get("schedule")
if not isinstance(schedule, list):
raise DutyScheduleParseError("Missing or invalid 'schedule' (must be array)")
if len(schedule) > MAX_SCHEDULE_ROWS:
raise DutyScheduleParseError(
f"schedule has too many rows (max {MAX_SCHEDULE_ROWS})"
)
max_days = 0
entries: list[DutyScheduleEntry] = []
@@ -85,12 +104,20 @@ def parse_duty_schedule(raw_bytes: bytes) -> DutyScheduleResult:
full_name = name.strip()
if not full_name:
raise DutyScheduleParseError("schedule item 'name' cannot be empty")
if len(full_name) > MAX_FULL_NAME_LENGTH:
raise DutyScheduleParseError(
f"schedule item 'name' must not exceed {MAX_FULL_NAME_LENGTH} characters"
)
duty_str = row.get("duty")
if duty_str is None:
duty_str = ""
if not isinstance(duty_str, str):
raise DutyScheduleParseError("schedule item 'duty' must be string")
if len(duty_str) > MAX_DUTY_STRING_LENGTH:
raise DutyScheduleParseError(
f"schedule item 'duty' must not exceed {MAX_DUTY_STRING_LENGTH} characters"
)
cells = [c.strip() for c in duty_str.split(";")]
max_days = max(max_days, len(cells))
@@ -120,4 +147,9 @@ def parse_duty_schedule(raw_bytes: bytes) -> DutyScheduleResult:
else:
end_date = start_date + timedelta(days=max_days - 1)
if not (min_year <= end_date.year <= max_year):
raise DutyScheduleParseError(
f"Computed end_date year must be between {min_year} and {max_year}"
)
return DutyScheduleResult(start_date=start_date, end_date=end_date, entries=entries)

View File

@@ -3,7 +3,9 @@
import asyncio
import json
import logging
import sys
import threading
import time
import urllib.request
from telegram.ext import ApplicationBuilder
@@ -13,6 +15,9 @@ from duty_teller.config import require_bot_token
from duty_teller.handlers import group_duty_pin, register_handlers
from duty_teller.utils.http_client import safe_urlopen
# Seconds to wait for HTTP server to bind before health check.
_HTTP_STARTUP_WAIT_SEC = 3
async def _resolve_bot_username(application) -> None:
"""If BOT_USERNAME is not set from env, resolve it via get_me()."""
@@ -69,6 +74,25 @@ def _run_uvicorn(web_app, port: int) -> None:
loop.run_until_complete(server.serve())
def _wait_for_http_ready(port: int) -> bool:
"""Return True if /health responds successfully within _HTTP_STARTUP_WAIT_SEC."""
host = config.HTTP_HOST
if host == "0.0.0.0":
host = "127.0.0.1"
url = f"http://{host}:{port}/health"
deadline = time.monotonic() + _HTTP_STARTUP_WAIT_SEC
while time.monotonic() < deadline:
try:
req = urllib.request.Request(url)
with safe_urlopen(req, timeout=2) as resp:
if resp.status == 200:
return True
except Exception as e:
logger.debug("Health check not ready yet: %s", e)
time.sleep(0.5)
return False
def main() -> None:
"""Build the bot and FastAPI, start uvicorn in a thread, run polling."""
require_bot_token()
@@ -85,16 +109,30 @@ def main() -> None:
from duty_teller.api.app import app as web_app
t = threading.Thread(
target=_run_uvicorn,
args=(web_app, config.HTTP_PORT),
daemon=True,
)
t.start()
if config.MINI_APP_SKIP_AUTH:
logger.warning(
"MINI_APP_SKIP_AUTH is set — API auth disabled (insecure); use only for dev"
)
if config.HTTP_HOST not in ("127.0.0.1", "localhost", ""):
print(
"ERROR: MINI_APP_SKIP_AUTH must not be used in production (non-localhost).",
file=sys.stderr,
)
sys.exit(1)
t = threading.Thread(
target=_run_uvicorn,
args=(web_app, config.HTTP_PORT),
daemon=False,
)
t.start()
if not _wait_for_http_ready(config.HTTP_PORT):
logger.error(
"HTTP server did not become ready on port %s within %s s; check port and permissions.",
config.HTTP_PORT,
_HTTP_STARTUP_WAIT_SEC,
)
sys.exit(1)
logger.info("Bot starting (polling)... HTTP API on port %s", config.HTTP_PORT)
app.run_polling(allowed_updates=["message", "my_chat_member"])

View File

@@ -1,5 +1,6 @@
"""Import duty schedule: delete range, insert duties/unavailable/vacation. Accepts session."""
import logging
from datetime import date, timedelta
from sqlalchemy.orm import Session
@@ -14,6 +15,8 @@ from duty_teller.db.repository import (
from duty_teller.importers.duty_schedule import DutyScheduleResult
from duty_teller.utils.dates import day_start_iso, day_end_iso, duty_to_iso
logger = logging.getLogger(__name__)
def _consecutive_date_ranges(dates: list[date]) -> list[tuple[date, date]]:
"""Sort dates and merge consecutive ones into (first, last) ranges. Empty list -> []."""
@@ -53,16 +56,24 @@ def run_import(
Returns:
Tuple (num_users, num_duty, num_unavailable, num_vacation).
"""
logger.info(
"Import started: range %s..%s, %d entries",
result.start_date,
result.end_date,
len(result.entries),
)
from_date_str = result.start_date.isoformat()
to_date_str = result.end_date.isoformat()
num_duty = num_unavailable = num_vacation = 0
# Batch: get all users by full_name, create missing
# Batch: get all users by full_name, create missing (no commit until end)
names = [e.full_name for e in result.entries]
users_map = get_users_by_full_names(session, names)
for name in names:
if name not in users_map:
users_map[name] = get_or_create_user_by_full_name(session, name)
users_map[name] = get_or_create_user_by_full_name(
session, name, commit=False
)
# Delete range per user (no commit)
for entry in result.entries:
@@ -113,4 +124,11 @@ def run_import(
session.bulk_insert_mappings(Duty, duty_rows)
session.commit()
invalidate_duty_related_caches()
logger.info(
"Import done: %d users, %d duty, %d unavailable, %d vacation",
len(result.entries),
num_duty,
num_unavailable,
num_vacation,
)
return (len(result.entries), num_duty, num_unavailable, num_vacation)

View File

@@ -24,10 +24,17 @@ def duty_to_iso(d: date, hour_utc: int, minute_utc: int) -> str:
_ISO_DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$")
# Maximum allowed date range in days (e.g. 731 = 2 years).
MAX_DATE_RANGE_DAYS = 731
class DateRangeValidationError(ValueError):
"""Raised when from_date/to_date validation fails. API uses kind for i18n key."""
def __init__(self, kind: Literal["bad_format", "from_after_to"]) -> None:
def __init__(
self,
kind: Literal["bad_format", "from_after_to", "range_too_large"],
) -> None:
self.kind = kind
super().__init__(kind)
@@ -86,12 +93,20 @@ def parse_iso_date(s: str) -> date | None:
def validate_date_range(from_date: str, to_date: str) -> None:
"""Validate from_date and to_date are YYYY-MM-DD and from_date <= to_date.
"""Validate from_date and to_date are YYYY-MM-DD, from_date <= to_date, and range <= MAX_DATE_RANGE_DAYS.
Raises:
DateRangeValidationError: bad_format if format invalid, from_after_to if from > to.
DateRangeValidationError: bad_format if format invalid, from_after_to if from > to,
range_too_large if (to_date - from_date) > MAX_DATE_RANGE_DAYS.
"""
if not _ISO_DATE_RE.match(from_date or "") or not _ISO_DATE_RE.match(to_date or ""):
raise DateRangeValidationError("bad_format")
if from_date > to_date:
raise DateRangeValidationError("from_after_to")
try:
from_d = date.fromisoformat(from_date)
to_d = date.fromisoformat(to_date)
except ValueError:
raise DateRangeValidationError("bad_format") from None
if (to_d - from_d).days > MAX_DATE_RANGE_DAYS:
raise DateRangeValidationError("range_too_large")

View File

@@ -23,6 +23,21 @@ def test_health(client):
assert r.json() == {"status": "ok"}
def test_unhandled_exception_returns_500_json(client):
"""Global exception handler returns 500 JSON without leaking exception details."""
from unittest.mock import MagicMock
from duty_teller.api.app import global_exception_handler
# Call the registered handler directly: it returns JSON and does not expose str(exc).
request = MagicMock()
exc = RuntimeError("internal failure")
response = global_exception_handler(request, exc)
assert response.status_code == 500
assert response.body.decode() == '{"detail":"Internal server error"}'
assert "internal failure" not in response.body.decode()
def test_health_has_vary_accept_language(client):
"""NoCacheStaticMiddleware adds Vary: Accept-Language to all responses."""
r = client.get("/health")
@@ -60,6 +75,20 @@ def test_app_config_js_returns_lang_from_default_language(client):
assert config.DEFAULT_LANGUAGE in body
@patch("duty_teller.api.app.config.DEFAULT_LANGUAGE", '"; alert(1); "')
@patch("duty_teller.api.app.config.LOG_LEVEL_STR", "DEBUG\x00INJECT")
def test_app_config_js_sanitizes_lang_and_log_level(client):
"""config.js uses whitelist: invalid lang/log_level produce safe defaults, no script injection."""
r = client.get("/app/config.js")
assert r.status_code == 200
body = r.text
# Must be valid JS and not contain the raw malicious strings.
assert 'window.__DT_LANG = "en"' in body or 'window.__DT_LANG = "ru"' in body
assert "alert" not in body
assert "INJECT" not in body
assert "window.__DT_LOG_LEVEL" in body
def test_duties_invalid_date_format(client):
r = client.get("/api/duties", params={"from": "2025-01-01", "to": "invalid"})
assert r.status_code == 400
@@ -74,6 +103,30 @@ def test_duties_from_after_to(client):
assert "from" in detail or "to" in detail or "after" in detail or "позже" in detail
@patch("duty_teller.api.app.config.MINI_APP_SKIP_AUTH", True)
def test_duties_range_too_large_400(client):
"""Date range longer than MAX_DATE_RANGE_DAYS returns 400 with dates.range_too_large message."""
from datetime import date, timedelta
from duty_teller.utils.dates import MAX_DATE_RANGE_DAYS
from_d = date(2020, 1, 1)
to_d = from_d + timedelta(days=MAX_DATE_RANGE_DAYS + 1)
r = client.get(
"/api/duties",
params={"from": from_d.isoformat(), "to": to_d.isoformat()},
)
assert r.status_code == 400
detail = r.json()["detail"]
# EN: "Date range is too large. Request a shorter period." / RU: "Диапазон дат слишком большой..."
assert (
"range" in detail.lower()
or "short" in detail.lower()
or "короткий" in detail
or "большой" in detail
)
@patch("duty_teller.api.dependencies.config.MINI_APP_SKIP_AUTH", False)
def test_duties_403_without_init_data(client):
"""Without X-Telegram-Init-Data and without MINI_APP_SKIP_AUTH → 403 (any client)."""
@@ -309,20 +362,21 @@ def test_duties_200_with_unknown_event_type_mapped_to_duty(client):
def test_calendar_ical_team_404_invalid_token_format(client):
"""GET /api/calendar/ical/team/{token}.ics with invalid token format returns 404 without DB."""
"""GET /api/calendar/ical/team/{token}.ics with invalid token format returns 404 JSON."""
r = client.get("/api/calendar/ical/team/short.ics")
assert r.status_code == 404
assert "not found" in r.text.lower()
assert r.headers.get("content-type", "").startswith("application/json")
assert r.json() == {"detail": "Not found"}
@patch("duty_teller.api.app.get_user_by_calendar_token")
def test_calendar_ical_team_404_unknown_token(mock_get_user, client):
"""GET /api/calendar/ical/team/{token}.ics with unknown token returns 404."""
"""GET /api/calendar/ical/team/{token}.ics with unknown token returns 404 JSON."""
mock_get_user.return_value = None
valid_format_token = "B" * 43
r = client.get(f"/api/calendar/ical/team/{valid_format_token}.ics")
assert r.status_code == 404
assert "not found" in r.text.lower()
assert r.json() == {"detail": "Not found"}
mock_get_user.assert_called_once()
@@ -374,11 +428,10 @@ def test_calendar_ical_team_200_only_duty_and_description(
def test_calendar_ical_404_invalid_token_format(client):
"""GET /api/calendar/ical/{token}.ics with invalid token format returns 404 without DB call."""
# Token format must be base64url, 4050 chars; short or invalid chars → 404
"""GET /api/calendar/ical/{token}.ics with invalid token format returns 404 JSON."""
r = client.get("/api/calendar/ical/short.ics")
assert r.status_code == 404
assert "not found" in r.text.lower()
assert r.json() == {"detail": "Not found"}
r2 = client.get("/api/calendar/ical/" + "x" * 60 + ".ics")
assert r2.status_code == 404
r3 = client.get("/api/calendar/ical/../../../etc/passwd.ics")
@@ -387,13 +440,12 @@ def test_calendar_ical_404_invalid_token_format(client):
@patch("duty_teller.api.app.get_user_by_calendar_token")
def test_calendar_ical_404_unknown_token(mock_get_user, client):
"""GET /api/calendar/ical/{token}.ics with unknown token returns 404."""
"""GET /api/calendar/ical/{token}.ics with unknown token returns 404 JSON."""
mock_get_user.return_value = None
# Use a token that passes format validation (base64url, 4050 chars)
valid_format_token = "A" * 43
r = client.get(f"/api/calendar/ical/{valid_format_token}.ics")
assert r.status_code == 404
assert "not found" in r.text.lower()
assert r.json() == {"detail": "Not found"}
mock_get_user.assert_called_once()

View File

@@ -91,3 +91,29 @@ def test_require_bot_token_does_not_raise_when_set(monkeypatch):
"""require_bot_token() does nothing when BOT_TOKEN is set."""
monkeypatch.setattr(config, "BOT_TOKEN", "123:ABC")
config.require_bot_token()
def test_settings_from_env_invalid_http_port_uses_default(monkeypatch):
"""Invalid HTTP_PORT (non-numeric or out of range) yields default or clamped value."""
monkeypatch.delenv("HTTP_PORT", raising=False)
settings = config.Settings.from_env()
assert 1 <= settings.http_port <= 65535
monkeypatch.setenv("HTTP_PORT", "not-a-number")
settings = config.Settings.from_env()
assert settings.http_port == 8080
monkeypatch.setenv("HTTP_PORT", "0")
settings = config.Settings.from_env()
assert settings.http_port == 1
monkeypatch.setenv("HTTP_PORT", "99999")
settings = config.Settings.from_env()
assert settings.http_port == 65535
def test_settings_from_env_invalid_init_data_max_age_uses_default(monkeypatch):
"""Invalid INIT_DATA_MAX_AGE_SECONDS yields default 0."""
monkeypatch.setenv("INIT_DATA_MAX_AGE_SECONDS", "invalid")
settings = config.Settings.from_env()
assert settings.init_data_max_age_seconds == 0

View File

@@ -1,11 +1,14 @@
"""Tests for duty-schedule JSON parser."""
import json
from datetime import date
import pytest
from duty_teller.importers.duty_schedule import (
DUTY_MARKERS,
MAX_FULL_NAME_LENGTH,
MAX_SCHEDULE_ROWS,
UNAVAILABLE_MARKER,
VACATION_MARKER,
DutyScheduleParseError,
@@ -118,3 +121,38 @@ def test_unavailable_and_vacation_markers():
assert entry.unavailable_dates == [date(2026, 2, 1)]
assert entry.vacation_dates == [date(2026, 2, 2)]
assert entry.duty_dates == [date(2026, 2, 3)]
def test_parse_start_date_year_out_of_range():
"""start_date year must be current ± 1; otherwise DutyScheduleParseError."""
# Use a year far in the past/future so it fails regardless of test run date.
raw_future = b'{"meta": {"start_date": "2030-01-01"}, "schedule": [{"name": "A", "duty": ""}]}'
with pytest.raises(DutyScheduleParseError, match="year|2030"):
parse_duty_schedule(raw_future)
raw_past = b'{"meta": {"start_date": "2019-01-01"}, "schedule": [{"name": "A", "duty": ""}]}'
with pytest.raises(DutyScheduleParseError, match="year|2019"):
parse_duty_schedule(raw_past)
def test_parse_too_many_schedule_rows():
"""More than MAX_SCHEDULE_ROWS rows raises DutyScheduleParseError."""
rows = [{"name": f"User {i}", "duty": ""} for i in range(MAX_SCHEDULE_ROWS + 1)]
today = date.today()
start = today.replace(month=1, day=1)
payload = {"meta": {"start_date": start.isoformat()}, "schedule": rows}
raw = json.dumps(payload).encode("utf-8")
with pytest.raises(DutyScheduleParseError, match="too many|max"):
parse_duty_schedule(raw)
def test_parse_full_name_too_long():
"""full_name longer than MAX_FULL_NAME_LENGTH raises DutyScheduleParseError."""
long_name = "A" * (MAX_FULL_NAME_LENGTH + 1)
today = date.today()
start = today.replace(month=1, day=1)
raw = (
f'{{"meta": {{"start_date": "{start.isoformat()}"}}, '
f'"schedule": [{{"name": "{long_name}", "duty": ""}}]}}'
).encode("utf-8")
with pytest.raises(DutyScheduleParseError, match="exceed|character"):
parse_duty_schedule(raw)

View File

@@ -278,8 +278,8 @@ async def test_handle_duty_schedule_document_non_json_replies_need_json():
@pytest.mark.asyncio
async def test_handle_duty_schedule_document_parse_error_replies_and_clears_user_data():
"""handle_duty_schedule_document: parse_duty_schedule raises DutyScheduleParseError -> reply, clear user_data."""
async def test_handle_duty_schedule_document_parse_error_replies_generic_and_clears_user_data():
"""handle_duty_schedule_document: DutyScheduleParseError -> reply generic message (no str(e)), clear user_data."""
message = MagicMock()
message.document = _make_document()
message.reply_text = AsyncMock()
@@ -299,17 +299,17 @@ async def test_handle_duty_schedule_document_parse_error_replies_and_clears_user
with patch.object(mod, "parse_duty_schedule") as mock_parse:
mock_parse.side_effect = DutyScheduleParseError("Bad JSON")
with patch.object(mod, "t") as mock_t:
mock_t.return_value = "Parse error: Bad JSON"
mock_t.return_value = "The file could not be parsed."
await mod.handle_duty_schedule_document(update, context)
message.reply_text.assert_called_once_with("Parse error: Bad JSON")
mock_t.assert_called_with("en", "import.parse_error", error="Bad JSON")
message.reply_text.assert_called_once_with("The file could not be parsed.")
mock_t.assert_called_with("en", "import.parse_error_generic")
assert "awaiting_duty_schedule_file" not in context.user_data
assert "handover_utc_time" not in context.user_data
@pytest.mark.asyncio
async def test_handle_duty_schedule_document_import_error_replies_and_clears_user_data():
"""handle_duty_schedule_document: run_import in executor raises -> reply import_error, clear user_data."""
async def test_handle_duty_schedule_document_import_error_replies_generic_and_clears_user_data():
"""handle_duty_schedule_document: run_import raises -> reply generic message (no str(e)), clear user_data."""
message = MagicMock()
message.document = _make_document()
message.reply_text = AsyncMock()
@@ -340,10 +340,10 @@ async def test_handle_duty_schedule_document_import_error_replies_and_clears_use
with patch.object(mod, "run_import") as mock_run:
mock_run.side_effect = ValueError("DB error")
with patch.object(mod, "t") as mock_t:
mock_t.return_value = "Import error: DB error"
mock_t.return_value = "Import failed. Please try again."
await mod.handle_duty_schedule_document(update, context)
message.reply_text.assert_called_once_with("Import error: DB error")
mock_t.assert_called_with("en", "import.import_error", error="DB error")
message.reply_text.assert_called_once_with("Import failed. Please try again.")
mock_t.assert_called_with("en", "import.import_error_generic")
assert "awaiting_duty_schedule_file" not in context.user_data
assert "handover_utc_time" not in context.user_data

View File

@@ -24,10 +24,19 @@ def test_main_builds_app_and_starts_thread():
mock_scope.return_value.__exit__.return_value = None
with patch("duty_teller.run.require_bot_token"):
with patch("duty_teller.run.config") as mock_cfg:
mock_cfg.MINI_APP_SKIP_AUTH = False
mock_cfg.HTTP_HOST = "127.0.0.1"
mock_cfg.HTTP_PORT = 8080
with patch("duty_teller.run.ApplicationBuilder", return_value=mock_builder):
with patch("duty_teller.run.register_handlers") as mock_register:
with patch("duty_teller.run.threading.Thread") as mock_thread_class:
with patch("duty_teller.db.session.session_scope", mock_scope):
with patch(
"duty_teller.run._wait_for_http_ready", return_value=True
):
with patch(
"duty_teller.db.session.session_scope", mock_scope
):
mock_thread = MagicMock()
mock_thread_class.return_value = mock_thread
with pytest.raises(KeyboardInterrupt):

View File

@@ -1,6 +1,6 @@
"""Unit tests for utils (dates, user, handover)."""
from datetime import date
from datetime import date, timedelta
import pytest
@@ -62,6 +62,23 @@ def test_validate_date_range_from_after_to():
assert exc_info.value.kind == "from_after_to"
def test_validate_date_range_too_large():
"""Range longer than MAX_DATE_RANGE_DAYS raises range_too_large."""
from duty_teller.utils.dates import MAX_DATE_RANGE_DAYS
# from 2023-01-01 to 2025-06-01 is more than 731 days
with pytest.raises(DateRangeValidationError) as exc_info:
validate_date_range("2023-01-01", "2025-06-01")
assert exc_info.value.kind == "range_too_large"
# Exactly MAX_DATE_RANGE_DAYS + 1 day
from_d = date(2024, 1, 1)
to_d = from_d + timedelta(days=MAX_DATE_RANGE_DAYS + 1)
with pytest.raises(DateRangeValidationError) as exc_info:
validate_date_range(from_d.isoformat(), to_d.isoformat())
assert exc_info.value.kind == "range_too_large"
# --- user ---

View File

@@ -40,6 +40,7 @@ export const MESSAGES = {
"event_type.duty": "Duty",
"event_type.unavailable": "Unavailable",
"event_type.vacation": "Vacation",
"event_type.other": "Other",
"duty.now_on_duty": "On duty now",
"duty.none_this_month": "No duties this month.",
"duty.today": "Today",
@@ -96,6 +97,7 @@ export const MESSAGES = {
"event_type.duty": "Дежурство",
"event_type.unavailable": "Недоступен",
"event_type.vacation": "Отпуск",
"event_type.other": "Другое",
"duty.now_on_duty": "Сейчас дежурит",
"duty.none_this_month": "В этом месяце дежурств нет.",
"duty.today": "Сегодня",
@@ -161,10 +163,30 @@ export function getLang() {
* @param {Record<string, string>} [params] - e.g. { time: '14:00' }
* @returns {string}
*/
/**
* Resolve event_type.* key with fallback: unknown types use event_type.duty or event_type.other.
* @param {'ru'|'en'} lang
* @param {string} key
* @returns {string}
*/
function tEventType(lang, key) {
const dict = MESSAGES[lang] || MESSAGES.en;
const enDict = MESSAGES.en;
let s = dict[key];
if (s === undefined) s = enDict[key];
if (s === undefined) {
s = dict["event_type.other"] || enDict["event_type.other"] || enDict["event_type.duty"] || key;
}
return s;
}
export function t(lang, key, params = {}) {
const dict = MESSAGES[lang] || MESSAGES.en;
let s = dict[key];
if (s === undefined) s = MESSAGES.en[key];
if (s === undefined && key.startsWith("event_type.")) {
return tEventType(lang, key);
}
if (s === undefined) return key;
Object.keys(params).forEach((k) => {
s = s.replace(new RegExp("\\{" + k + "\\}", "g"), params[k]);