From ab1d29b20300711802f3168b5073c5f23761dd07 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Tue, 17 Feb 2026 21:59:41 +0100 Subject: [PATCH 1/2] streamlined --- .dockerignore | 1 + .env.example | 5 + .github/workflows/ci.yml | 15 ++ .github/workflows/scheduled-probes.yml | 54 ++++++ .gitignore | 2 + CONTRIBUTING.md | 24 +++ README.md | 17 +- alembic/versions/.gitkeep | 1 - app/api/routers/runs.py | 3 + app/api/routers/scholars.py | 4 + app/api/schemas.py | 4 + app/services/ingestion.py | 123 ++++++++++++- app/services/runs.py | 67 +++++-- app/services/scheduler.py | 7 + app/services/scholars.py | 87 ++++++++- app/settings.py | 20 ++ docker-compose.yml | 5 + frontend/src/features/runs/index.ts | 8 + frontend/src/pages/RunDetailPage.vue | 63 +++++++ scripts/check_no_generated_artifacts.sh | 42 +++++ tests/integration/test_fixture_probe_runs.py | 182 +++++++++++++++++++ tests/unit/test_runs_summary.py | 58 ++++++ tests/unit/test_scholar_search_safety.py | 83 +++++++++ 23 files changed, 853 insertions(+), 22 deletions(-) create mode 100644 .github/workflows/scheduled-probes.yml create mode 100644 CONTRIBUTING.md delete mode 100644 alembic/versions/.gitkeep create mode 100755 scripts/check_no_generated_artifacts.sh create mode 100644 tests/integration/test_fixture_probe_runs.py create mode 100644 tests/unit/test_runs_summary.py diff --git a/.dockerignore b/.dockerignore index 28eee55..d600e0e 100644 --- a/.dockerignore +++ b/.dockerignore @@ -14,4 +14,5 @@ __pycache__ htmlcov frontend/node_modules frontend/dist +frontend/.vite planning diff --git a/.env.example b/.env.example index 6b3a78b..83a3a3e 100644 --- a/.env.example +++ b/.env.example @@ -35,6 +35,9 @@ INGESTION_NETWORK_ERROR_RETRIES=1 INGESTION_RETRY_BACKOFF_SECONDS=1.0 INGESTION_MAX_PAGES_PER_SCHOLAR=30 INGESTION_PAGE_SIZE=100 +INGESTION_ALERT_BLOCKED_FAILURE_THRESHOLD=1 +INGESTION_ALERT_NETWORK_FAILURE_THRESHOLD=2 +INGESTION_ALERT_RETRY_SCHEDULED_THRESHOLD=3 INGESTION_CONTINUATION_QUEUE_ENABLED=1 INGESTION_CONTINUATION_BASE_DELAY_SECONDS=120 INGESTION_CONTINUATION_MAX_DELAY_SECONDS=3600 @@ -50,6 +53,8 @@ SCHOLAR_NAME_SEARCH_MIN_INTERVAL_SECONDS=8.0 SCHOLAR_NAME_SEARCH_INTERVAL_JITTER_SECONDS=2.0 SCHOLAR_NAME_SEARCH_COOLDOWN_BLOCK_THRESHOLD=1 SCHOLAR_NAME_SEARCH_COOLDOWN_SECONDS=1800 +SCHOLAR_NAME_SEARCH_ALERT_RETRY_COUNT_THRESHOLD=2 +SCHOLAR_NAME_SEARCH_ALERT_COOLDOWN_REJECTIONS_THRESHOLD=3 BOOTSTRAP_ADMIN_ON_START=0 BOOTSTRAP_ADMIN_EMAIL= diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9a943af..758e5dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,8 +7,20 @@ on: - main jobs: + repo-hygiene: + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Enforce no generated artifacts + run: ./scripts/check_no_generated_artifacts.sh + test: runs-on: ubuntu-latest + needs: + - repo-hygiene services: postgres: image: postgres:15 @@ -54,6 +66,8 @@ jobs: frontend-quality: runs-on: ubuntu-latest + needs: + - repo-hygiene steps: - name: Checkout @@ -91,6 +105,7 @@ jobs: docker-publish: runs-on: ubuntu-latest needs: + - repo-hygiene - test - frontend-quality if: github.event_name == 'push' && github.ref == 'refs/heads/main' diff --git a/.github/workflows/scheduled-probes.yml b/.github/workflows/scheduled-probes.yml new file mode 100644 index 0000000..1673db3 --- /dev/null +++ b/.github/workflows/scheduled-probes.yml @@ -0,0 +1,54 @@ +name: Scheduled Probes + +on: + workflow_dispatch: + schedule: + - cron: "20 6 * * *" + +jobs: + fixture-probes: + runs-on: ubuntu-latest + services: + postgres: + image: postgres:15 + env: + POSTGRES_DB: scholar + POSTGRES_USER: scholar + POSTGRES_PASSWORD: scholar + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U scholar -d scholar" + --health-interval 5s + --health-timeout 5s + --health-retries 20 + + env: + DATABASE_URL: postgresql+asyncpg://scholar:scholar@localhost:5432/scholar + SCHOLAR_IMAGE_UPLOAD_DIR: /tmp/scholarr_uploads/scholar_images + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Setup uv + uses: astral-sh/setup-uv@v4 + + - name: Install dependencies + run: uv sync --extra dev + + - name: Fixture-backed parser unit probes + run: >- + uv run pytest + tests/unit/test_scholar_parser.py + tests/unit/test_scholar_search_safety.py + + - name: Fixture-backed integration probe + run: >- + uv run pytest -m integration + tests/integration/test_fixture_probe_runs.py diff --git a/.gitignore b/.gitignore index ef21cff..faab882 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,8 @@ htmlcov/ .uv-cache/ .env *.log +.DS_Store planning/ frontend/node_modules/ frontend/dist/ +frontend/.vite/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..ab44797 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,24 @@ +# Contributing + +## Scope +This project favors small, reviewable pull requests that keep runtime behavior clear and operationally safe. + +## Essential-File Policy +Commit only source-of-truth files required to build, run, test, or document the app. + +Do not commit generated or local-only artifacts, including: +- `__pycache__/`, `*.pyc`, `.pytest_cache/`, `.mypy_cache/`, `.ruff_cache/` +- frontend build/install outputs like `frontend/dist/`, `frontend/node_modules/`, `frontend/.vite/` +- coverage outputs (`.coverage`, `htmlcov/`) +- packaging/build leftovers (`*.egg-info/`, `build/`, `dist/`) +- local probe/scratch material (`planning/`) + +CI enforces this with `scripts/check_no_generated_artifacts.sh`. + +## Merge Checklist +- [ ] Changes are minimal, purposeful, and remove obsolete/dead code in touched areas. +- [ ] Backend tests pass (`uv run pytest tests/unit` and integration scope as needed). +- [ ] Frontend checks pass (`npm run typecheck`, `npm run test:run`, `npm run build`). +- [ ] API/behavior docs are updated when env vars, endpoints, or payloads change. +- [ ] `README.md`, `.env.example`, and deployment notes stay aligned. +- [ ] `scripts/check_no_generated_artifacts.sh` passes locally. diff --git a/README.md b/README.md index a757257..75dd587 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,8 @@ docker compose -f docker-compose.yml -f docker-compose.dev.yml down - `.env.example`: full env variable template. - `Dockerfile`: multi-stage build (frontend + backend runtime). - `README.md`: deployment and operations reference. +- `CONTRIBUTING.md`: contribution policy and merge checklist. +- `scripts/check_no_generated_artifacts.sh`: tracked-artifact guard used by CI. ## Environment Variables (Complete Reference) @@ -141,6 +143,9 @@ Notes: | `INGESTION_RETRY_BACKOFF_SECONDS` | `1.0` | float >= 0 | deploy, dev | Backoff delay for retry attempts. | | `INGESTION_MAX_PAGES_PER_SCHOLAR` | `30` | integer >= 1 | deploy, dev | Upper bound of pages fetched per scholar run. | | `INGESTION_PAGE_SIZE` | `100` | integer >= 1 | deploy, dev | Requested Scholar page size. | +| `INGESTION_ALERT_BLOCKED_FAILURE_THRESHOLD` | `1` | integer >= 1 | deploy, dev | Trigger blocked/captcha scrape alert flag when this many blocked failures occur in a run. | +| `INGESTION_ALERT_NETWORK_FAILURE_THRESHOLD` | `2` | integer >= 1 | deploy, dev | Trigger network scrape alert flag when this many network failures occur in a run. | +| `INGESTION_ALERT_RETRY_SCHEDULED_THRESHOLD` | `3` | integer >= 1 | deploy, dev | Trigger retry alert flag when scheduled retry count reaches this threshold in a run. | | `INGESTION_CONTINUATION_QUEUE_ENABLED` | `1` | boolean | deploy, dev | Enable continuation queue for long runs. | | `INGESTION_CONTINUATION_BASE_DELAY_SECONDS` | `120` | integer >= 0 | deploy, dev | Initial delay before retrying continuation queue items. | | `INGESTION_CONTINUATION_MAX_DELAY_SECONDS` | `3600` | integer >= 0 | deploy, dev | Maximum continuation retry delay. | @@ -150,7 +155,7 @@ Notes: | Variable | Default | Options | Scope | Description | | --- | --- | --- | --- | --- | -| `SCHOLAR_IMAGE_UPLOAD_DIR` | `/var/lib/scholarr/uploads` | writable absolute path | deploy, dev | Storage path for uploaded scholar images. | +| `SCHOLAR_IMAGE_UPLOAD_DIR` | `/var/lib/scholarr/uploads` | writable absolute path | deploy, dev | Storage path for uploaded scholar images. Use a persistent mounted path in production. | | `SCHOLAR_IMAGE_UPLOAD_MAX_BYTES` | `2000000` | integer >= 1 | deploy, dev | Max uploaded image size in bytes. | | `SCHOLAR_NAME_SEARCH_ENABLED` | `1` | boolean | deploy, dev | Enable name-search helper endpoint. | | `SCHOLAR_NAME_SEARCH_CACHE_TTL_SECONDS` | `21600` | integer >= 1 | deploy, dev | Cache TTL for successful name-search responses. | @@ -160,6 +165,8 @@ Notes: | `SCHOLAR_NAME_SEARCH_INTERVAL_JITTER_SECONDS` | `2.0` | float >= 0 | deploy, dev | Added jitter to reduce request burst patterns. | | `SCHOLAR_NAME_SEARCH_COOLDOWN_BLOCK_THRESHOLD` | `1` | integer >= 1 | deploy, dev | Consecutive blocked responses before cooldown starts. | | `SCHOLAR_NAME_SEARCH_COOLDOWN_SECONDS` | `1800` | integer >= 1 | deploy, dev | Cooldown duration after repeated blocked responses. | +| `SCHOLAR_NAME_SEARCH_ALERT_RETRY_COUNT_THRESHOLD` | `2` | integer >= 1 | deploy, dev | Emit retry-threshold observability warning when name-search retry count reaches this value. | +| `SCHOLAR_NAME_SEARCH_ALERT_COOLDOWN_REJECTIONS_THRESHOLD` | `3` | integer >= 1 | deploy, dev | Emit cooldown-threshold observability alert after this many requests are rejected during active cooldown. | ### Startup Bootstrap and DB Wait @@ -197,6 +204,14 @@ Contract drift: python3 scripts/check_frontend_api_contract.py ``` +Repository hygiene: + +```bash +./scripts/check_no_generated_artifacts.sh +``` + +Scheduled fixture probes run in GitHub Actions via `.github/workflows/scheduled-probes.yml`. + ## API Contract - Base path: `/api/v1` diff --git a/alembic/versions/.gitkeep b/alembic/versions/.gitkeep deleted file mode 100644 index 8b13789..0000000 --- a/alembic/versions/.gitkeep +++ /dev/null @@ -1 +0,0 @@ - diff --git a/app/api/routers/runs.py b/app/api/routers/runs.py index e2eab8c..e095cb7 100644 --- a/app/api/routers/runs.py +++ b/app/api/routers/runs.py @@ -382,6 +382,9 @@ async def run_manual( auto_queue_continuations=settings.ingestion_continuation_queue_enabled, queue_delay_seconds=settings.ingestion_continuation_base_delay_seconds, idempotency_key=idempotency_key, + alert_blocked_failure_threshold=settings.ingestion_alert_blocked_failure_threshold, + alert_network_failure_threshold=settings.ingestion_alert_network_failure_threshold, + alert_retry_scheduled_threshold=settings.ingestion_alert_retry_scheduled_threshold, ) except ingestion_service.RunAlreadyInProgressError as exc: await db_session.rollback() diff --git a/app/api/routers/scholars.py b/app/api/routers/scholars.py index bc7b8a3..7ce8fc5 100644 --- a/app/api/routers/scholars.py +++ b/app/api/routers/scholars.py @@ -167,6 +167,10 @@ async def search_scholars( interval_jitter_seconds=settings.scholar_name_search_interval_jitter_seconds, cooldown_block_threshold=settings.scholar_name_search_cooldown_block_threshold, cooldown_seconds=settings.scholar_name_search_cooldown_seconds, + retry_alert_threshold=settings.scholar_name_search_alert_retry_count_threshold, + cooldown_rejection_alert_threshold=( + settings.scholar_name_search_alert_cooldown_rejections_threshold + ), ) except scholar_service.ScholarServiceError as exc: raise ApiException( diff --git a/app/api/schemas.py b/app/api/schemas.py index e5264db..13c093a 100644 --- a/app/api/schemas.py +++ b/app/api/schemas.py @@ -219,6 +219,10 @@ class RunSummaryData(BaseModel): partial_count: int failed_state_counts: dict[str, int] = Field(default_factory=dict) failed_reason_counts: dict[str, int] = Field(default_factory=dict) + scrape_failure_counts: dict[str, int] = Field(default_factory=dict) + retry_counts: dict[str, int] = Field(default_factory=dict) + alert_thresholds: dict[str, int] = Field(default_factory=dict) + alert_flags: dict[str, bool] = Field(default_factory=dict) model_config = ConfigDict(extra="forbid") diff --git a/app/services/ingestion.py b/app/services/ingestion.py index 986f460..5d50811 100644 --- a/app/services/ingestion.py +++ b/app/services/ingestion.py @@ -40,6 +40,11 @@ FAILED_STATES = { ParseState.NETWORK_ERROR.value, "ingestion_error", } +FAILURE_BUCKET_BLOCKED = "blocked_or_captcha" +FAILURE_BUCKET_NETWORK = "network_error" +FAILURE_BUCKET_LAYOUT = "layout_changed" +FAILURE_BUCKET_INGESTION = "ingestion_error" +FAILURE_BUCKET_OTHER = "other_failure" RUN_LOCK_NAMESPACE = 8217 RESUMABLE_PARTIAL_REASONS = { "max_pages_reached", @@ -83,6 +88,28 @@ class RunAlreadyInProgressError(RuntimeError): """Raised when a run lock for a user is already held by another process.""" +def _int_or_default(value: Any, default: int = 0) -> int: + try: + return int(value) + except (TypeError, ValueError): + return default + + +def _classify_failure_bucket(*, state: str, state_reason: str) -> str: + reason = state_reason.strip().lower() + normalized_state = state.strip().lower() + + if normalized_state == ParseState.BLOCKED_OR_CAPTCHA.value or reason.startswith("blocked_"): + return FAILURE_BUCKET_BLOCKED + if normalized_state == ParseState.NETWORK_ERROR.value or reason.startswith("network_"): + return FAILURE_BUCKET_NETWORK + if normalized_state == ParseState.LAYOUT_CHANGED.value: + return FAILURE_BUCKET_LAYOUT + if normalized_state == "ingestion_error": + return FAILURE_BUCKET_INGESTION + return FAILURE_BUCKET_OTHER + + class ScholarIngestionService: def __init__(self, *, source: ScholarSource) -> None: self._source = source @@ -103,6 +130,9 @@ class ScholarIngestionService: auto_queue_continuations: bool = True, queue_delay_seconds: int = 60, idempotency_key: str | None = None, + alert_blocked_failure_threshold: int = 1, + alert_network_failure_threshold: int = 2, + alert_retry_scheduled_threshold: int = 3, ) -> RunExecutionSummary: lock_acquired = await self._try_acquire_user_lock( db_session, @@ -161,6 +191,9 @@ class ScholarIngestionService: "max_pages_per_scholar": max_pages_per_scholar, "page_size": page_size, "idempotency_key": idempotency_key, + "alert_blocked_failure_threshold": alert_blocked_failure_threshold, + "alert_network_failure_threshold": alert_network_failure_threshold, + "alert_retry_scheduled_threshold": alert_retry_scheduled_threshold, }, ) @@ -416,17 +449,87 @@ class ScholarIngestionService: failed_state_counts: dict[str, int] = {} failed_reason_counts: dict[str, int] = {} + scrape_failure_counts: dict[str, int] = {} + retries_scheduled_count = 0 + scholars_with_retries_count = 0 + retry_exhausted_count = 0 for entry in scholar_results: - if str(entry.get("outcome", "")) != "failed": + attempt_count = max(0, _int_or_default(entry.get("attempt_count"), 0)) + retries_for_entry = max(0, attempt_count - 1) + if retries_for_entry > 0: + retries_scheduled_count += retries_for_entry + scholars_with_retries_count += 1 + + outcome = str(entry.get("outcome", "")) + if outcome != "failed": continue - state = str(entry.get("state", "")) + + state = str(entry.get("state", "")).strip() if state not in FAILED_STATES: continue failed_state_counts[state] = failed_state_counts.get(state, 0) + 1 + reason = str(entry.get("state_reason", "")).strip() if reason: failed_reason_counts[reason] = failed_reason_counts.get(reason, 0) + 1 + failure_bucket = _classify_failure_bucket(state=state, state_reason=reason) + scrape_failure_counts[failure_bucket] = ( + scrape_failure_counts.get(failure_bucket, 0) + 1 + ) + + if ( + state == ParseState.NETWORK_ERROR.value + and retries_for_entry > 0 + ): + retry_exhausted_count += 1 + + blocked_failure_count = int(scrape_failure_counts.get(FAILURE_BUCKET_BLOCKED, 0)) + network_failure_count = int(scrape_failure_counts.get(FAILURE_BUCKET_NETWORK, 0)) + + bounded_blocked_threshold = max(1, int(alert_blocked_failure_threshold)) + bounded_network_threshold = max(1, int(alert_network_failure_threshold)) + bounded_retry_threshold = max(1, int(alert_retry_scheduled_threshold)) + alert_flags = { + "blocked_failure_threshold_exceeded": blocked_failure_count >= bounded_blocked_threshold, + "network_failure_threshold_exceeded": network_failure_count >= bounded_network_threshold, + "retry_scheduled_threshold_exceeded": retries_scheduled_count >= bounded_retry_threshold, + } + + if alert_flags["blocked_failure_threshold_exceeded"]: + logger.warning( + "ingestion.alert_blocked_failure_threshold_exceeded", + extra={ + "event": "ingestion.alert_blocked_failure_threshold_exceeded", + "user_id": user_id, + "crawl_run_id": run.id, + "blocked_failure_count": blocked_failure_count, + "threshold": bounded_blocked_threshold, + }, + ) + if alert_flags["network_failure_threshold_exceeded"]: + logger.warning( + "ingestion.alert_network_failure_threshold_exceeded", + extra={ + "event": "ingestion.alert_network_failure_threshold_exceeded", + "user_id": user_id, + "crawl_run_id": run.id, + "network_failure_count": network_failure_count, + "threshold": bounded_network_threshold, + }, + ) + if alert_flags["retry_scheduled_threshold_exceeded"]: + logger.warning( + "ingestion.alert_retry_scheduled_threshold_exceeded", + extra={ + "event": "ingestion.alert_retry_scheduled_threshold_exceeded", + "user_id": user_id, + "crawl_run_id": run.id, + "retries_scheduled_count": retries_scheduled_count, + "threshold": bounded_retry_threshold, + }, + ) + run.end_dt = datetime.now(timezone.utc) run.status = self._resolve_run_status( scholar_count=len(scholars), @@ -442,6 +545,18 @@ class ScholarIngestionService: "partial_count": partial_count, "failed_state_counts": failed_state_counts, "failed_reason_counts": failed_reason_counts, + "scrape_failure_counts": scrape_failure_counts, + "retry_counts": { + "retries_scheduled_count": retries_scheduled_count, + "scholars_with_retries_count": scholars_with_retries_count, + "retry_exhausted_count": retry_exhausted_count, + }, + "alert_thresholds": { + "blocked_failure_threshold": bounded_blocked_threshold, + "network_failure_threshold": bounded_network_threshold, + "retry_scheduled_threshold": bounded_retry_threshold, + }, + "alert_flags": alert_flags, }, "meta": { "idempotency_key": idempotency_key, @@ -463,6 +578,10 @@ class ScholarIngestionService: "failed_count": failed_count, "partial_count": partial_count, "new_publication_count": run.new_pub_count, + "blocked_failure_count": blocked_failure_count, + "network_failure_count": network_failure_count, + "retries_scheduled_count": retries_scheduled_count, + "alert_flags": alert_flags, }, ) diff --git a/app/services/runs.py b/app/services/runs.py index 1e65cfe..368815f 100644 --- a/app/services/runs.py +++ b/app/services/runs.py @@ -67,26 +67,65 @@ def _summary_dict(error_log: object) -> dict[str, Any]: return summary +def _summary_int_dict(summary: dict[str, Any], key: str) -> dict[str, int]: + value = summary.get(key) + if not isinstance(value, dict): + return {} + return { + str(item_key): _safe_int(item_value, 0) + for item_key, item_value in value.items() + if isinstance(item_key, str) + } + + +def _summary_bool_dict(summary: dict[str, Any], key: str) -> dict[str, bool]: + value = summary.get(key) + if not isinstance(value, dict): + return {} + return { + str(item_key): bool(item_value) + for item_key, item_value in value.items() + if isinstance(item_key, str) + } + + def extract_run_summary(error_log: object) -> dict[str, Any]: summary = _summary_dict(error_log) return { "succeeded_count": _safe_int(summary.get("succeeded_count", 0)), "failed_count": _safe_int(summary.get("failed_count", 0)), "partial_count": _safe_int(summary.get("partial_count", 0)), - "failed_state_counts": { - str(key): _safe_int(value, 0) - for key, value in summary.get("failed_state_counts", {}).items() - if isinstance(key, str) - } - if isinstance(summary.get("failed_state_counts"), dict) - else {}, - "failed_reason_counts": { - str(key): _safe_int(value, 0) - for key, value in summary.get("failed_reason_counts", {}).items() - if isinstance(key, str) - } - if isinstance(summary.get("failed_reason_counts"), dict) - else {}, + "failed_state_counts": _summary_int_dict(summary, "failed_state_counts"), + "failed_reason_counts": _summary_int_dict(summary, "failed_reason_counts"), + "scrape_failure_counts": _summary_int_dict(summary, "scrape_failure_counts"), + "retry_counts": { + "retries_scheduled_count": _safe_int( + ( + summary.get("retry_counts", {}).get("retries_scheduled_count") + if isinstance(summary.get("retry_counts"), dict) + else 0 + ), + 0, + ), + "scholars_with_retries_count": _safe_int( + ( + summary.get("retry_counts", {}).get("scholars_with_retries_count") + if isinstance(summary.get("retry_counts"), dict) + else 0 + ), + 0, + ), + "retry_exhausted_count": _safe_int( + ( + summary.get("retry_counts", {}).get("retry_exhausted_count") + if isinstance(summary.get("retry_counts"), dict) + else 0 + ), + 0, + ), + }, + "alert_thresholds": _summary_int_dict(summary, "alert_thresholds"), + "alert_flags": _summary_bool_dict(summary, "alert_flags"), } diff --git a/app/services/scheduler.py b/app/services/scheduler.py index 40dbc6f..fef4ddb 100644 --- a/app/services/scheduler.py +++ b/app/services/scheduler.py @@ -20,6 +20,7 @@ from app.db.session import get_session_factory from app.services import continuation_queue as queue_service from app.services.ingestion import RunAlreadyInProgressError, ScholarIngestionService from app.services.scholar_source import LiveScholarSource +from app.settings import settings logger = logging.getLogger(__name__) @@ -195,6 +196,9 @@ class SchedulerService: page_size=self._page_size, auto_queue_continuations=self._continuation_queue_enabled, queue_delay_seconds=self._continuation_base_delay_seconds, + alert_blocked_failure_threshold=settings.ingestion_alert_blocked_failure_threshold, + alert_network_failure_threshold=settings.ingestion_alert_network_failure_threshold, + alert_retry_scheduled_threshold=settings.ingestion_alert_retry_scheduled_threshold, ) except RunAlreadyInProgressError: await session.rollback() @@ -326,6 +330,9 @@ class SchedulerService: }, auto_queue_continuations=self._continuation_queue_enabled, queue_delay_seconds=self._continuation_base_delay_seconds, + alert_blocked_failure_threshold=settings.ingestion_alert_blocked_failure_threshold, + alert_network_failure_threshold=settings.ingestion_alert_network_failure_threshold, + alert_retry_scheduled_threshold=settings.ingestion_alert_retry_scheduled_threshold, ) except RunAlreadyInProgressError: await session.rollback() diff --git a/app/services/scholars.py b/app/services/scholars.py index d5f9569..5bb88c1 100644 --- a/app/services/scholars.py +++ b/app/services/scholars.py @@ -38,6 +38,8 @@ DEFAULT_AUTHOR_SEARCH_COOLDOWN_BLOCK_THRESHOLD = 1 DEFAULT_AUTHOR_SEARCH_COOLDOWN_SECONDS = 1800 DEFAULT_AUTHOR_SEARCH_MIN_INTERVAL_SECONDS = 3.0 DEFAULT_AUTHOR_SEARCH_INTERVAL_JITTER_SECONDS = 1.0 +DEFAULT_AUTHOR_SEARCH_RETRY_ALERT_THRESHOLD = 2 +DEFAULT_AUTHOR_SEARCH_COOLDOWN_REJECTION_ALERT_THRESHOLD = 3 ALLOWED_IMAGE_UPLOAD_CONTENT_TYPES = { "image/jpeg": ".jpg", "image/png": ".png", @@ -101,6 +103,8 @@ _AUTHOR_SEARCH_CACHE: OrderedDict[str, _AuthorSearchCacheEntry] = OrderedDict() _AUTHOR_SEARCH_LAST_LIVE_REQUEST_MONOTONIC = 0.0 _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC: datetime | None = None _AUTHOR_SEARCH_CONSECUTIVE_BLOCKED_COUNT = 0 +_AUTHOR_SEARCH_COOLDOWN_REJECTION_COUNT = 0 +_AUTHOR_SEARCH_COOLDOWN_ALERT_EMITTED = False class ScholarServiceError(ValueError): @@ -281,10 +285,14 @@ def _reset_author_search_runtime_state_for_tests() -> None: global _AUTHOR_SEARCH_LAST_LIVE_REQUEST_MONOTONIC global _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC global _AUTHOR_SEARCH_CONSECUTIVE_BLOCKED_COUNT + global _AUTHOR_SEARCH_COOLDOWN_REJECTION_COUNT + global _AUTHOR_SEARCH_COOLDOWN_ALERT_EMITTED _AUTHOR_SEARCH_CACHE.clear() _AUTHOR_SEARCH_LAST_LIVE_REQUEST_MONOTONIC = 0.0 _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC = None _AUTHOR_SEARCH_CONSECUTIVE_BLOCKED_COUNT = 0 + _AUTHOR_SEARCH_COOLDOWN_REJECTION_COUNT = 0 + _AUTHOR_SEARCH_COOLDOWN_ALERT_EMITTED = False async def list_scholars_for_user( @@ -379,10 +387,14 @@ async def search_author_candidates( interval_jitter_seconds: float = DEFAULT_AUTHOR_SEARCH_INTERVAL_JITTER_SECONDS, cooldown_block_threshold: int = DEFAULT_AUTHOR_SEARCH_COOLDOWN_BLOCK_THRESHOLD, cooldown_seconds: int = DEFAULT_AUTHOR_SEARCH_COOLDOWN_SECONDS, + retry_alert_threshold: int = DEFAULT_AUTHOR_SEARCH_RETRY_ALERT_THRESHOLD, + cooldown_rejection_alert_threshold: int = DEFAULT_AUTHOR_SEARCH_COOLDOWN_REJECTION_ALERT_THRESHOLD, ) -> ParsedAuthorSearchPage: global _AUTHOR_SEARCH_LAST_LIVE_REQUEST_MONOTONIC global _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC global _AUTHOR_SEARCH_CONSECUTIVE_BLOCKED_COUNT + global _AUTHOR_SEARCH_COOLDOWN_REJECTION_COUNT + global _AUTHOR_SEARCH_COOLDOWN_ALERT_EMITTED normalized_query = query.strip() if len(normalized_query) < 2: @@ -408,8 +420,47 @@ async def search_author_candidates( now_utc = datetime.now(timezone.utc) now_monotonic = time.monotonic() + if ( + _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC is not None + and now_utc >= _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC + ): + logger.info( + "scholar_search.cooldown_expired", + extra={ + "event": "scholar_search.cooldown_expired", + "cooldown_until_utc": _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC.isoformat(), + }, + ) + _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC = None + _AUTHOR_SEARCH_COOLDOWN_REJECTION_COUNT = 0 + _AUTHOR_SEARCH_COOLDOWN_ALERT_EMITTED = False + cooldown_remaining_seconds = _author_search_cooldown_remaining_seconds(now_utc) if cooldown_remaining_seconds > 0: + _AUTHOR_SEARCH_COOLDOWN_REJECTION_COUNT += 1 + bounded_cooldown_rejection_alert_threshold = max( + 1, + int(cooldown_rejection_alert_threshold), + ) + if ( + _AUTHOR_SEARCH_COOLDOWN_REJECTION_COUNT + >= bounded_cooldown_rejection_alert_threshold + and not _AUTHOR_SEARCH_COOLDOWN_ALERT_EMITTED + ): + logger.error( + "scholar_search.cooldown_rejection_threshold_exceeded", + extra={ + "event": "scholar_search.cooldown_rejection_threshold_exceeded", + "query": normalized_query, + "cooldown_rejection_count": _AUTHOR_SEARCH_COOLDOWN_REJECTION_COUNT, + "threshold": bounded_cooldown_rejection_alert_threshold, + "cooldown_until_utc": _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC.isoformat() + if _AUTHOR_SEARCH_COOLDOWN_UNTIL_UTC + else None, + }, + ) + _AUTHOR_SEARCH_COOLDOWN_ALERT_EMITTED = True + logger.warning( "scholar_search.cooldown_active", extra={ @@ -421,12 +472,15 @@ async def search_author_candidates( else None, }, ) + warning_codes = [ + "author_search_cooldown_active", + f"author_search_cooldown_remaining_{cooldown_remaining_seconds}s", + ] + if _AUTHOR_SEARCH_COOLDOWN_ALERT_EMITTED: + warning_codes.append("author_search_cooldown_alert_threshold_exceeded") return _policy_blocked_author_search_result( reason=SEARCH_COOLDOWN_REASON, - warning_codes=[ - "author_search_cooldown_active", - f"author_search_cooldown_remaining_{cooldown_remaining_seconds}s", - ], + warning_codes=warning_codes, limit=bounded_limit, ) @@ -472,6 +526,7 @@ async def search_author_candidates( max_attempts = max(1, int(network_error_retries) + 1) parsed: ParsedAuthorSearchPage | None = None retry_warnings: list[str] = [] + retry_scheduled_count = 0 for attempt_index in range(max_attempts): fetch_result = await source.fetch_author_search_html(normalized_query, start=0) @@ -480,6 +535,7 @@ async def search_author_candidates( break retry_warnings.append("network_retry_scheduled_for_author_search") + retry_scheduled_count += 1 sleep_seconds = max(float(retry_backoff_seconds), 0.0) * (2**attempt_index) if sleep_seconds > 0: await asyncio.sleep(sleep_seconds) @@ -494,6 +550,27 @@ async def search_author_candidates( warnings=_merge_warnings(parsed.warnings, retry_warnings), ) + bounded_retry_alert_threshold = max(1, int(retry_alert_threshold)) + if retry_scheduled_count >= bounded_retry_alert_threshold: + logger.warning( + "scholar_search.retry_threshold_exceeded", + extra={ + "event": "scholar_search.retry_threshold_exceeded", + "query": normalized_query, + "retry_scheduled_count": retry_scheduled_count, + "threshold": bounded_retry_alert_threshold, + "final_state": merged_parsed.state.value, + "final_state_reason": merged_parsed.state_reason, + }, + ) + merged_parsed = replace( + merged_parsed, + warnings=_merge_warnings( + merged_parsed.warnings, + [f"author_search_retry_threshold_exceeded_{retry_scheduled_count}"], + ), + ) + if _is_author_search_block_state(merged_parsed): _AUTHOR_SEARCH_CONSECUTIVE_BLOCKED_COUNT += 1 logger.warning( @@ -510,6 +587,8 @@ async def search_author_candidates( seconds=max(60, int(cooldown_seconds)) ) _AUTHOR_SEARCH_CONSECUTIVE_BLOCKED_COUNT = 0 + _AUTHOR_SEARCH_COOLDOWN_REJECTION_COUNT = 0 + _AUTHOR_SEARCH_COOLDOWN_ALERT_EMITTED = False merged_parsed = replace( merged_parsed, warnings=_merge_warnings( diff --git a/app/settings.py b/app/settings.py index 7e24550..67931c3 100644 --- a/app/settings.py +++ b/app/settings.py @@ -62,6 +62,18 @@ class Settings: 30, ) ingestion_page_size: int = _env_int("INGESTION_PAGE_SIZE", 100) + ingestion_alert_blocked_failure_threshold: int = _env_int( + "INGESTION_ALERT_BLOCKED_FAILURE_THRESHOLD", + 1, + ) + ingestion_alert_network_failure_threshold: int = _env_int( + "INGESTION_ALERT_NETWORK_FAILURE_THRESHOLD", + 2, + ) + ingestion_alert_retry_scheduled_threshold: int = _env_int( + "INGESTION_ALERT_RETRY_SCHEDULED_THRESHOLD", + 3, + ) ingestion_continuation_queue_enabled: bool = _env_bool( "INGESTION_CONTINUATION_QUEUE_ENABLED", True, @@ -118,6 +130,14 @@ class Settings: "SCHOLAR_NAME_SEARCH_COOLDOWN_SECONDS", 1800, ) + scholar_name_search_alert_retry_count_threshold: int = _env_int( + "SCHOLAR_NAME_SEARCH_ALERT_RETRY_COUNT_THRESHOLD", + 2, + ) + scholar_name_search_alert_cooldown_rejections_threshold: int = _env_int( + "SCHOLAR_NAME_SEARCH_ALERT_COOLDOWN_REJECTIONS_THRESHOLD", + 3, + ) settings = Settings() diff --git a/docker-compose.yml b/docker-compose.yml index 764ce81..a2d9a5c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -44,6 +44,9 @@ services: INGESTION_RETRY_BACKOFF_SECONDS: ${INGESTION_RETRY_BACKOFF_SECONDS:-1.0} INGESTION_MAX_PAGES_PER_SCHOLAR: ${INGESTION_MAX_PAGES_PER_SCHOLAR:-30} INGESTION_PAGE_SIZE: ${INGESTION_PAGE_SIZE:-100} + INGESTION_ALERT_BLOCKED_FAILURE_THRESHOLD: ${INGESTION_ALERT_BLOCKED_FAILURE_THRESHOLD:-1} + INGESTION_ALERT_NETWORK_FAILURE_THRESHOLD: ${INGESTION_ALERT_NETWORK_FAILURE_THRESHOLD:-2} + INGESTION_ALERT_RETRY_SCHEDULED_THRESHOLD: ${INGESTION_ALERT_RETRY_SCHEDULED_THRESHOLD:-3} INGESTION_CONTINUATION_QUEUE_ENABLED: ${INGESTION_CONTINUATION_QUEUE_ENABLED:-1} INGESTION_CONTINUATION_BASE_DELAY_SECONDS: ${INGESTION_CONTINUATION_BASE_DELAY_SECONDS:-120} INGESTION_CONTINUATION_MAX_DELAY_SECONDS: ${INGESTION_CONTINUATION_MAX_DELAY_SECONDS:-3600} @@ -59,6 +62,8 @@ services: SCHOLAR_NAME_SEARCH_INTERVAL_JITTER_SECONDS: ${SCHOLAR_NAME_SEARCH_INTERVAL_JITTER_SECONDS:-2.0} SCHOLAR_NAME_SEARCH_COOLDOWN_BLOCK_THRESHOLD: ${SCHOLAR_NAME_SEARCH_COOLDOWN_BLOCK_THRESHOLD:-1} SCHOLAR_NAME_SEARCH_COOLDOWN_SECONDS: ${SCHOLAR_NAME_SEARCH_COOLDOWN_SECONDS:-1800} + SCHOLAR_NAME_SEARCH_ALERT_RETRY_COUNT_THRESHOLD: ${SCHOLAR_NAME_SEARCH_ALERT_RETRY_COUNT_THRESHOLD:-2} + SCHOLAR_NAME_SEARCH_ALERT_COOLDOWN_REJECTIONS_THRESHOLD: ${SCHOLAR_NAME_SEARCH_ALERT_COOLDOWN_REJECTIONS_THRESHOLD:-3} BOOTSTRAP_ADMIN_ON_START: ${BOOTSTRAP_ADMIN_ON_START:-0} BOOTSTRAP_ADMIN_EMAIL: ${BOOTSTRAP_ADMIN_EMAIL:-} BOOTSTRAP_ADMIN_PASSWORD: ${BOOTSTRAP_ADMIN_PASSWORD:-} diff --git a/frontend/src/features/runs/index.ts b/frontend/src/features/runs/index.ts index c8c47e8..4fe4add 100644 --- a/frontend/src/features/runs/index.ts +++ b/frontend/src/features/runs/index.ts @@ -18,6 +18,14 @@ export interface RunSummary { partial_count: number; failed_state_counts: Record; failed_reason_counts: Record; + scrape_failure_counts: Record; + retry_counts: { + retries_scheduled_count: number; + scholars_with_retries_count: number; + retry_exhausted_count: number; + }; + alert_thresholds: Record; + alert_flags: Record; } export interface RunScholarResult { diff --git a/frontend/src/pages/RunDetailPage.vue b/frontend/src/pages/RunDetailPage.vue index d858ee3..1cade16 100644 --- a/frontend/src/pages/RunDetailPage.vue +++ b/frontend/src/pages/RunDetailPage.vue @@ -21,6 +21,35 @@ const errorMessage = ref(null); const errorRequestId = ref(null); const runId = computed(() => Number(route.params.id)); +const defaultRetryCounts = { + retries_scheduled_count: 0, + scholars_with_retries_count: 0, + retry_exhausted_count: 0, +}; + +function sortedCountEntries(record: Record | null | undefined): Array<[string, number]> { + if (!record) { + return []; + } + return Object.entries(record) + .filter(([, value]) => Number.isFinite(value) && value > 0) + .sort((a, b) => (b[1] - a[1]) || a[0].localeCompare(b[0])); +} + +const failureBucketEntries = computed(() => + detail.value ? sortedCountEntries(detail.value.summary.scrape_failure_counts) : [], +); + +const alertFlagEntries = computed(() => { + if (!detail.value) { + return []; + } + return Object.entries(detail.value.summary.alert_flags ?? {}) + .filter(([, enabled]) => Boolean(enabled)) + .sort((a, b) => a[0].localeCompare(b[0])); +}); + +const retryCounts = computed(() => detail.value?.summary.retry_counts ?? defaultRetryCounts); function formatDate(value: string | null): string { if (!value) { @@ -96,6 +125,40 @@ onMounted(() => { Outcome summary: {{ detail.summary.succeeded_count }} succeeded, {{ detail.summary.partial_count }} partial, {{ detail.summary.failed_count }} failed.

+
+
+

Retries scheduled

+

{{ retryCounts.retries_scheduled_count }}

+
+
+

Scholars with retries

+

{{ retryCounts.scholars_with_retries_count }}

+
+
+

Retry exhausted

+

{{ retryCounts.retry_exhausted_count }}

+
+
+
+
+

Scrape failure buckets

+

n/a

+
    +
  • + {{ name }}: {{ count }} +
  • +
+
+
+

Active alerts

+

none

+
    +
  • + {{ name }} +
  • +
+
+
diff --git a/scripts/check_no_generated_artifacts.sh b/scripts/check_no_generated_artifacts.sh new file mode 100755 index 0000000..e2ce3ae --- /dev/null +++ b/scripts/check_no_generated_artifacts.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +set -euo pipefail + +blocked_regexes=( + '(^|/)__pycache__/' + '\\.py[co]$' + '(^|/)\\.pytest_cache(/|$)' + '(^|/)\\.mypy_cache(/|$)' + '(^|/)\\.ruff_cache(/|$)' + '(^|/)\\.coverage$' + '(^|/)htmlcov(/|$)' + '(^|/)(dist|build)(/|$)' + '(^|/)frontend/(dist|node_modules|\\.vite)(/|$)' + '(^|/)[^/]+\\.egg-info(/|$)' + '(^|/)\\.DS_Store$' + '(^|/)planning(/|$)' +) + +tracked_files=() +while IFS= read -r path; do + tracked_files+=("$path") +done < <(git ls-files) + +offenders=() +for path in "${tracked_files[@]}"; do + for regex in "${blocked_regexes[@]}"; do + if [[ "$path" =~ $regex ]]; then + offenders+=("$path") + break + fi + done +done + +if (( ${#offenders[@]} > 0 )); then + { + echo "Tracked generated artifacts detected (remove from git tracking):" + printf ' - %s\n' "${offenders[@]}" + } >&2 + exit 1 +fi + +echo "Generated artifact guard passed (no tracked cache/build/probe files)." diff --git a/tests/integration/test_fixture_probe_runs.py b/tests/integration/test_fixture_probe_runs.py new file mode 100644 index 0000000..02743ce --- /dev/null +++ b/tests/integration/test_fixture_probe_runs.py @@ -0,0 +1,182 @@ +from __future__ import annotations + +from collections import defaultdict +from pathlib import Path + +import pytest +from sqlalchemy import text +from sqlalchemy.ext.asyncio import AsyncSession + +from app.db.models import RunStatus, RunTriggerType +from app.services.ingestion import ScholarIngestionService +from app.services.scholar_source import FetchResult +from tests.integration.helpers import insert_user + +REGRESSION_FIXTURE_DIR = Path("tests/fixtures/scholar/regression") + + +def _fixture(name: str) -> str: + return (REGRESSION_FIXTURE_DIR / name).read_text(encoding="utf-8") + + +def _profile_fetch(*, scholar_id: str, body: str) -> FetchResult: + return FetchResult( + requested_url=f"https://scholar.google.com/citations?hl=en&user={scholar_id}", + status_code=200, + final_url=f"https://scholar.google.com/citations?hl=en&user={scholar_id}", + body=body, + error=None, + ) + + +def _blocked_fetch(*, scholar_id: str, body: str) -> FetchResult: + return FetchResult( + requested_url=f"https://scholar.google.com/citations?hl=en&user={scholar_id}", + status_code=200, + final_url=( + "https://accounts.google.com/v3/signin/identifier" + "?continue=https%3A%2F%2Fscholar.google.com%2Fcitations" + ), + body=body, + error=None, + ) + + +def _network_timeout_fetch(*, scholar_id: str) -> FetchResult: + return FetchResult( + requested_url=f"https://scholar.google.com/citations?hl=en&user={scholar_id}", + status_code=None, + final_url=None, + body="", + error="timed out", + ) + + +class FixtureScholarSource: + def __init__(self, responses: dict[tuple[str, int], list[FetchResult]]) -> None: + self._responses = responses + self._calls: defaultdict[tuple[str, int], int] = defaultdict(int) + + async def fetch_profile_page_html( + self, + scholar_id: str, + *, + cstart: int, + pagesize: int, + ) -> FetchResult: + _ = pagesize + key = (scholar_id, int(cstart)) + fetches = self._responses.get(key) + if not fetches: + return _network_timeout_fetch(scholar_id=scholar_id) + + call_index = self._calls[key] + self._calls[key] += 1 + return fetches[min(call_index, len(fetches) - 1)] + + async def fetch_profile_html(self, scholar_id: str) -> FetchResult: + return await self.fetch_profile_page_html(scholar_id, cstart=0, pagesize=100) + + +@pytest.mark.integration +@pytest.mark.db +@pytest.mark.asyncio +async def test_fixture_probe_run_emits_failure_and_retry_summary_metrics( + db_session: AsyncSession, +) -> None: + user_id = await insert_user( + db_session, + email="fixture-probe@example.com", + password="fixture-probe-password", + ) + + scholar_ids = { + "ok": "abcDEF123456", + "blocked": "A1B2C3D4E5F6", + "retry": "RSTUVWX12345", + } + + for label in ("ok", "blocked", "retry"): + await db_session.execute( + text( + """ + INSERT INTO scholar_profiles (user_id, scholar_id, display_name, is_enabled) + VALUES (:user_id, :scholar_id, :display_name, true) + """ + ), + { + "user_id": user_id, + "scholar_id": scholar_ids[label], + "display_name": f"fixture-{label}", + }, + ) + await db_session.commit() + + source = FixtureScholarSource( + { + (scholar_ids["ok"], 0): [ + _profile_fetch( + scholar_id=scholar_ids["ok"], + body=_fixture("profile_P1RwlvoAAAAJ.html"), + ) + ], + (scholar_ids["blocked"], 0): [ + _blocked_fetch( + scholar_id=scholar_ids["blocked"], + body=_fixture("profile_AAAAAAAAAAAA.html"), + ) + ], + (scholar_ids["retry"], 0): [ + _network_timeout_fetch(scholar_id=scholar_ids["retry"]), + _profile_fetch( + scholar_id=scholar_ids["retry"], + body=_fixture("profile_LZ5D_p4AAAAJ.html"), + ), + ], + } + ) + ingestion = ScholarIngestionService(source=source) + + summary = await ingestion.run_for_user( + db_session, + user_id=user_id, + trigger_type=RunTriggerType.MANUAL, + request_delay_seconds=0, + network_error_retries=1, + retry_backoff_seconds=0.0, + max_pages_per_scholar=1, + page_size=100, + auto_queue_continuations=False, + alert_blocked_failure_threshold=1, + alert_network_failure_threshold=1, + alert_retry_scheduled_threshold=1, + ) + + assert summary.status == RunStatus.PARTIAL_FAILURE + assert summary.scholar_count == 3 + assert summary.succeeded_count == 2 + assert summary.failed_count == 1 + assert summary.partial_count == 0 + + run_result = await db_session.execute( + text("SELECT error_log FROM crawl_runs WHERE id = :run_id"), + {"run_id": summary.crawl_run_id}, + ) + error_log = run_result.scalar_one() + run_summary = error_log["summary"] + + assert run_summary["failed_state_counts"]["blocked_or_captcha"] == 1 + assert run_summary["failed_reason_counts"]["blocked_accounts_redirect"] == 1 + assert run_summary["scrape_failure_counts"]["blocked_or_captcha"] == 1 + + assert run_summary["retry_counts"]["retries_scheduled_count"] == 1 + assert run_summary["retry_counts"]["scholars_with_retries_count"] == 1 + assert run_summary["retry_counts"]["retry_exhausted_count"] == 0 + + assert run_summary["alert_thresholds"]["blocked_failure_threshold"] == 1 + assert run_summary["alert_thresholds"]["network_failure_threshold"] == 1 + assert run_summary["alert_thresholds"]["retry_scheduled_threshold"] == 1 + + assert run_summary["alert_flags"]["blocked_failure_threshold_exceeded"] is True + assert run_summary["alert_flags"]["network_failure_threshold_exceeded"] is False + assert run_summary["alert_flags"]["retry_scheduled_threshold_exceeded"] is True diff --git a/tests/unit/test_runs_summary.py b/tests/unit/test_runs_summary.py new file mode 100644 index 0000000..025f335 --- /dev/null +++ b/tests/unit/test_runs_summary.py @@ -0,0 +1,58 @@ +from __future__ import annotations + +from app.services.runs import extract_run_summary + + +def test_extract_run_summary_includes_extended_metrics() -> None: + error_log = { + "summary": { + "succeeded_count": 3, + "failed_count": 1, + "partial_count": 2, + "failed_state_counts": {"network_error": 1}, + "failed_reason_counts": {"network_timeout": 1}, + "scrape_failure_counts": {"network_error": 1}, + "retry_counts": { + "retries_scheduled_count": 4, + "scholars_with_retries_count": 2, + "retry_exhausted_count": 1, + }, + "alert_thresholds": { + "blocked_failure_threshold": 1, + "network_failure_threshold": 2, + "retry_scheduled_threshold": 3, + }, + "alert_flags": { + "blocked_failure_threshold_exceeded": False, + "network_failure_threshold_exceeded": True, + "retry_scheduled_threshold_exceeded": True, + }, + } + } + + summary = extract_run_summary(error_log) + + assert summary["succeeded_count"] == 3 + assert summary["failed_count"] == 1 + assert summary["partial_count"] == 2 + assert summary["failed_state_counts"] == {"network_error": 1} + assert summary["failed_reason_counts"] == {"network_timeout": 1} + assert summary["scrape_failure_counts"] == {"network_error": 1} + assert summary["retry_counts"]["retries_scheduled_count"] == 4 + assert summary["retry_counts"]["scholars_with_retries_count"] == 2 + assert summary["retry_counts"]["retry_exhausted_count"] == 1 + assert summary["alert_thresholds"]["retry_scheduled_threshold"] == 3 + assert summary["alert_flags"]["network_failure_threshold_exceeded"] is True + + +def test_extract_run_summary_defaults_extended_metrics() -> None: + summary = extract_run_summary({}) + + assert summary["scrape_failure_counts"] == {} + assert summary["retry_counts"] == { + "retries_scheduled_count": 0, + "scholars_with_retries_count": 0, + "retry_exhausted_count": 0, + } + assert summary["alert_thresholds"] == {} + assert summary["alert_flags"] == {} diff --git a/tests/unit/test_scholar_search_safety.py b/tests/unit/test_scholar_search_safety.py index c9cc8c4..bf3dedc 100644 --- a/tests/unit/test_scholar_search_safety.py +++ b/tests/unit/test_scholar_search_safety.py @@ -63,6 +63,16 @@ def _blocked_author_search_fetch() -> FetchResult: ) +def _network_timeout_fetch() -> FetchResult: + return FetchResult( + requested_url="https://scholar.google.com/citations?hl=en&view_op=search_authors&mauthors=ada", + status_code=None, + final_url=None, + body="", + error="timed out", + ) + + @pytest.mark.asyncio async def test_search_author_candidates_serves_cached_response_for_same_query() -> None: source = StubScholarSource([_ok_author_search_fetch()]) @@ -150,6 +160,79 @@ async def test_search_author_candidates_trips_cooldown_after_blocked_responses() assert "author_search_cooldown_active" in second.warnings +@pytest.mark.asyncio +async def test_search_author_candidates_emits_cooldown_alert_warning_after_threshold() -> None: + source = StubScholarSource([_blocked_author_search_fetch()]) + + _ = await scholar_service.search_author_candidates( + source=source, + query="Blocked Query", + limit=10, + network_error_retries=0, + retry_backoff_seconds=0.0, + search_enabled=True, + cache_ttl_seconds=0, + blocked_cache_ttl_seconds=0, + cache_max_entries=64, + min_interval_seconds=0.0, + interval_jitter_seconds=0.0, + cooldown_block_threshold=1, + cooldown_seconds=300, + cooldown_rejection_alert_threshold=1, + ) + second = await scholar_service.search_author_candidates( + source=source, + query="Blocked Query", + limit=10, + network_error_retries=0, + retry_backoff_seconds=0.0, + search_enabled=True, + cache_ttl_seconds=0, + blocked_cache_ttl_seconds=0, + cache_max_entries=64, + min_interval_seconds=0.0, + interval_jitter_seconds=0.0, + cooldown_block_threshold=1, + cooldown_seconds=300, + cooldown_rejection_alert_threshold=1, + ) + + assert second.state == ParseState.BLOCKED_OR_CAPTCHA + assert "author_search_cooldown_alert_threshold_exceeded" in second.warnings + + +@pytest.mark.asyncio +async def test_search_author_candidates_adds_retry_threshold_warning() -> None: + source = StubScholarSource( + [ + _network_timeout_fetch(), + _network_timeout_fetch(), + _ok_author_search_fetch(), + ] + ) + + result = await scholar_service.search_author_candidates( + source=source, + query="Ada Lovelace", + limit=10, + network_error_retries=2, + retry_backoff_seconds=0.0, + search_enabled=True, + cache_ttl_seconds=0, + blocked_cache_ttl_seconds=0, + cache_max_entries=64, + min_interval_seconds=0.0, + interval_jitter_seconds=0.0, + cooldown_block_threshold=3, + cooldown_seconds=300, + retry_alert_threshold=2, + ) + + assert result.state == ParseState.OK + assert "author_search_retry_threshold_exceeded_2" in result.warnings + assert source.calls == 3 + + @pytest.mark.asyncio async def test_search_author_candidates_can_be_disabled_by_configuration() -> None: source = StubScholarSource([_ok_author_search_fetch()]) -- 2.49.1 From ae2ca8f149951285128364449cc7ca3e5f1dd2b9 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Thu, 19 Feb 2026 15:43:20 +0100 Subject: [PATCH 2/2] feat: production-harden app and finalize merge-ready UX/theme baseline - complete tokenized theme preset system and admin style guide visibility - refine dashboard layout/scroll behavior and shared async/request UI states - add user nav-visibility settings across API, migration, and frontend stores - harden security headers/CSP handling and related test coverage - enforce CI repository hygiene + frontend contract/theme/build quality gates - update docs/env references and make migration smoke test head-aware --- .env.example | 15 + .github/workflows/ci.yml | 4 + README.md | 25 +- ...19_0008_user_settings_nav_visible_pages.py | 51 ++ app/api/routers/settings.py | 17 +- app/api/schemas.py | 2 + app/db/models.py | 7 + app/http/middleware.py | 120 ++++ app/main.py | 26 +- app/services/user_settings.py | 66 +- app/settings.py | 67 +- frontend/package.json | 1 + frontend/scripts/check_theme_tokens.mjs | 84 +++ frontend/src/app/AppShell.vue | 14 +- frontend/src/app/providers.ts | 8 + frontend/src/components/layout/AppHeader.vue | 28 +- frontend/src/components/layout/AppNav.vue | 35 +- frontend/src/components/layout/AppPage.vue | 31 +- .../components/patterns/AsyncStateGate.vue | 34 + .../components/patterns/QueueHealthBadge.vue | 15 +- .../patterns/RequestStateAlerts.vue | 44 ++ .../components/patterns/RunStatusBadge.vue | 10 +- frontend/src/components/ui/AppAlert.vue | 8 +- frontend/src/components/ui/AppBadge.vue | 10 +- frontend/src/components/ui/AppButton.vue | 10 +- frontend/src/components/ui/AppCard.vue | 2 +- frontend/src/components/ui/AppCheckbox.vue | 4 +- frontend/src/components/ui/AppEmptyState.vue | 6 +- frontend/src/components/ui/AppHelpHint.vue | 209 ++++++ frontend/src/components/ui/AppInput.vue | 2 +- frontend/src/components/ui/AppModal.vue | 6 +- frontend/src/components/ui/AppSelect.vue | 2 +- frontend/src/components/ui/AppSkeleton.vue | 2 +- frontend/src/components/ui/AppTable.vue | 10 +- frontend/src/env.d.ts | 5 + .../scholars/components/ScholarAvatar.vue | 67 ++ frontend/src/features/settings/index.ts | 1 + frontend/src/pages/AdminUsersPage.vue | 225 +++++-- frontend/src/pages/DashboardPage.vue | 331 ++++++---- frontend/src/pages/LoginPage.vue | 32 +- frontend/src/pages/PublicationsPage.vue | 349 +++++++--- frontend/src/pages/RunDetailPage.vue | 76 ++- frontend/src/pages/RunsPage.vue | 66 +- frontend/src/pages/ScholarsPage.vue | 595 ++++++++---------- frontend/src/pages/SettingsPage.vue | 301 +++++++-- frontend/src/pages/StyleGuidePage.vue | 292 ++++++++- frontend/src/stores/auth.ts | 5 + frontend/src/stores/theme.ts | 72 ++- frontend/src/stores/user_settings.ts | 87 +++ frontend/src/styles.css | 147 ++++- frontend/src/theme/THEME_PHASE0_INVENTORY.md | 89 +++ frontend/src/theme/presets.test.ts | 69 ++ frontend/src/theme/presets.ts | 274 ++++++++ frontend/src/theme/presets/dune.js | 314 +++++++++ frontend/src/theme/presets/graphite.json | 313 +++++++++ frontend/src/theme/presets/lilac.js | 313 +++++++++ frontend/src/theme/presets/oatmeal.js | 314 +++++++++ frontend/src/theme/presets/parchment.js | 313 +++++++++ frontend/src/theme/presets/scholarly.json | 313 +++++++++ frontend/src/theme/presets/tide.json | 313 +++++++++ frontend/tailwind.config.js | 112 +++- tests/integration/test_api_v1.py | 9 + tests/integration/test_migrations.py | 19 +- tests/smoke/test_migration_schema.py | 11 +- tests/unit/test_security_headers.py | 36 ++ tests/unit/test_user_settings.py | 70 +++ 66 files changed, 5655 insertions(+), 853 deletions(-) create mode 100644 alembic/versions/20260219_0008_user_settings_nav_visible_pages.py create mode 100644 frontend/scripts/check_theme_tokens.mjs create mode 100644 frontend/src/components/patterns/AsyncStateGate.vue create mode 100644 frontend/src/components/patterns/RequestStateAlerts.vue create mode 100644 frontend/src/components/ui/AppHelpHint.vue create mode 100644 frontend/src/features/scholars/components/ScholarAvatar.vue create mode 100644 frontend/src/stores/user_settings.ts create mode 100644 frontend/src/theme/THEME_PHASE0_INVENTORY.md create mode 100644 frontend/src/theme/presets.test.ts create mode 100644 frontend/src/theme/presets.ts create mode 100644 frontend/src/theme/presets/dune.js create mode 100644 frontend/src/theme/presets/graphite.json create mode 100644 frontend/src/theme/presets/lilac.js create mode 100644 frontend/src/theme/presets/oatmeal.js create mode 100644 frontend/src/theme/presets/parchment.js create mode 100644 frontend/src/theme/presets/scholarly.json create mode 100644 frontend/src/theme/presets/tide.json create mode 100644 tests/unit/test_security_headers.py create mode 100644 tests/unit/test_user_settings.py diff --git a/.env.example b/.env.example index 83a3a3e..5107a14 100644 --- a/.env.example +++ b/.env.example @@ -21,6 +21,21 @@ MIGRATE_ON_START=1 SESSION_SECRET_KEY=replace-with-a-long-random-secret-at-least-32-characters SESSION_COOKIE_SECURE=1 +SECURITY_HEADERS_ENABLED=1 +SECURITY_X_CONTENT_TYPE_OPTIONS=nosniff +SECURITY_X_FRAME_OPTIONS=DENY +SECURITY_REFERRER_POLICY=strict-origin-when-cross-origin +SECURITY_PERMISSIONS_POLICY=accelerometer=(), autoplay=(), camera=(), display-capture=(), geolocation=(), gyroscope=(), microphone=(), payment=(), usb=() +SECURITY_CROSS_ORIGIN_OPENER_POLICY=same-origin +SECURITY_CROSS_ORIGIN_RESOURCE_POLICY=same-origin +SECURITY_CSP_ENABLED=1 +SECURITY_CSP_POLICY=default-src 'self'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'; img-src 'self' data: https:; script-src 'self'; style-src 'self' 'unsafe-inline'; font-src 'self' data:; connect-src 'self'; object-src 'none' +SECURITY_CSP_DOCS_POLICY=default-src 'self'; img-src 'self' data: https:; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; font-src 'self' data:; connect-src 'self' https:; object-src 'none'; frame-ancestors 'none' +SECURITY_CSP_REPORT_ONLY=0 +SECURITY_STRICT_TRANSPORT_SECURITY_ENABLED=0 +SECURITY_STRICT_TRANSPORT_SECURITY_MAX_AGE=31536000 +SECURITY_STRICT_TRANSPORT_SECURITY_INCLUDE_SUBDOMAINS=1 +SECURITY_STRICT_TRANSPORT_SECURITY_PRELOAD=0 LOGIN_RATE_LIMIT_ATTEMPTS=5 LOGIN_RATE_LIMIT_WINDOW_SECONDS=60 LOG_LEVEL=INFO diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 758e5dd..4ff6965 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,6 +90,10 @@ jobs: working-directory: frontend run: npm install + - name: Frontend theme token policy + working-directory: frontend + run: npm run check:theme-tokens + - name: Frontend typecheck working-directory: frontend run: npm run typecheck diff --git a/README.md b/README.md index 75dd587..9ee3ccb 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,8 @@ docker compose -f docker-compose.yml -f docker-compose.dev.yml down Notes: - Boolean envs accept: `1/0`, `true/false`, `yes/no`, `on/off`. -- Values shown are defaults from `.env.example`. +- Values shown are deployment defaults from `.env.example` and `docker-compose.yml`. +- Some internal app fallbacks may differ for local test/dev safety when env vars are omitted. - `deploy` means used in regular deployment. - `dev` means used in local dev workflow. @@ -121,6 +122,26 @@ Notes: | `LOGIN_RATE_LIMIT_ATTEMPTS` | `5` | integer >= 1 | deploy, dev | Login attempts allowed per window. | | `LOGIN_RATE_LIMIT_WINDOW_SECONDS` | `60` | integer >= 1 | deploy, dev | Login rate limit window in seconds. | +### HTTP Security Headers and CSP + +| Variable | Default | Options | Scope | Description | +| --- | --- | --- | --- | --- | +| `SECURITY_HEADERS_ENABLED` | `1` | boolean | deploy, dev | Global switch for response security headers middleware. | +| `SECURITY_X_CONTENT_TYPE_OPTIONS` | `nosniff` | header value | deploy, dev | `X-Content-Type-Options` response header value. | +| `SECURITY_X_FRAME_OPTIONS` | `DENY` | header value | deploy, dev | `X-Frame-Options` response header value. | +| `SECURITY_REFERRER_POLICY` | `strict-origin-when-cross-origin` | header value | deploy, dev | `Referrer-Policy` response header value. | +| `SECURITY_PERMISSIONS_POLICY` | `accelerometer=(), autoplay=(), camera=(), display-capture=(), geolocation=(), gyroscope=(), microphone=(), payment=(), usb=()` | permissions policy string | deploy, dev | `Permissions-Policy` response header value. | +| `SECURITY_CROSS_ORIGIN_OPENER_POLICY` | `same-origin` | header value | deploy, dev | `Cross-Origin-Opener-Policy` response header value. | +| `SECURITY_CROSS_ORIGIN_RESOURCE_POLICY` | `same-origin` | header value | deploy, dev | `Cross-Origin-Resource-Policy` response header value. | +| `SECURITY_CSP_ENABLED` | `1` | boolean | deploy, dev | Enable Content Security Policy headers. | +| `SECURITY_CSP_POLICY` | strict SPA/API default | CSP policy string | deploy, dev | CSP applied to app/API routes (excluding docs paths). | +| `SECURITY_CSP_DOCS_POLICY` | relaxed docs default | CSP policy string | deploy, dev | CSP override for `/docs` and `/redoc` to keep Swagger/ReDoc usable. | +| `SECURITY_CSP_REPORT_ONLY` | `0` | boolean | deploy, dev | Emit CSP in report-only mode instead of enforcement mode. | +| `SECURITY_STRICT_TRANSPORT_SECURITY_ENABLED` | `0` | boolean | deploy, dev | Enable `Strict-Transport-Security` header. | +| `SECURITY_STRICT_TRANSPORT_SECURITY_MAX_AGE` | `31536000` | integer >= 0 | deploy, dev | `max-age` for HSTS header. | +| `SECURITY_STRICT_TRANSPORT_SECURITY_INCLUDE_SUBDOMAINS` | `1` | boolean | deploy, dev | Add `includeSubDomains` directive to HSTS header. | +| `SECURITY_STRICT_TRANSPORT_SECURITY_PRELOAD` | `0` | boolean | deploy, dev | Add `preload` directive to HSTS header. | + ### Logging | Variable | Default | Options | Scope | Description | @@ -155,7 +176,7 @@ Notes: | Variable | Default | Options | Scope | Description | | --- | --- | --- | --- | --- | -| `SCHOLAR_IMAGE_UPLOAD_DIR` | `/var/lib/scholarr/uploads` | writable absolute path | deploy, dev | Storage path for uploaded scholar images. Use a persistent mounted path in production. | +| `SCHOLAR_IMAGE_UPLOAD_DIR` | `/var/lib/scholarr/uploads` | writable absolute path | deploy, dev | Storage path for uploaded scholar images in compose deployments. App fallback without env is `/tmp/scholarr_uploads/scholar_images`. | | `SCHOLAR_IMAGE_UPLOAD_MAX_BYTES` | `2000000` | integer >= 1 | deploy, dev | Max uploaded image size in bytes. | | `SCHOLAR_NAME_SEARCH_ENABLED` | `1` | boolean | deploy, dev | Enable name-search helper endpoint. | | `SCHOLAR_NAME_SEARCH_CACHE_TTL_SECONDS` | `21600` | integer >= 1 | deploy, dev | Cache TTL for successful name-search responses. | diff --git a/alembic/versions/20260219_0008_user_settings_nav_visible_pages.py b/alembic/versions/20260219_0008_user_settings_nav_visible_pages.py new file mode 100644 index 0000000..df481fd --- /dev/null +++ b/alembic/versions/20260219_0008_user_settings_nav_visible_pages.py @@ -0,0 +1,51 @@ +"""Add per-user nav visibility settings. + +Revision ID: 20260219_0008 +Revises: 20260217_0007 +Create Date: 2026-02-19 14:58:00.000000 +""" + +from collections.abc import Sequence + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +# revision identifiers, used by Alembic. +revision: str = "20260219_0008" +down_revision: str | Sequence[str] | None = "20260217_0007" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +TABLE_NAME = "user_settings" +DEFAULT_NAV_VISIBLE_PAGES_SQL = ( + "'[\"dashboard\",\"scholars\",\"publications\",\"settings\",\"style-guide\",\"runs\",\"users\"]'::jsonb" +) + + +def upgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + columns = {column["name"] for column in inspector.get_columns(TABLE_NAME)} + + if "nav_visible_pages" not in columns: + op.add_column( + TABLE_NAME, + sa.Column( + "nav_visible_pages", + postgresql.JSONB(astext_type=sa.Text()), + nullable=False, + server_default=sa.text(DEFAULT_NAV_VISIBLE_PAGES_SQL), + ), + ) + + +def downgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + columns = {column["name"] for column in inspector.get_columns(TABLE_NAME)} + + if "nav_visible_pages" in columns: + op.drop_column(TABLE_NAME, "nav_visible_pages") diff --git a/app/api/routers/settings.py b/app/api/routers/settings.py index 7659591..082cfe1 100644 --- a/app/api/routers/settings.py +++ b/app/api/routers/settings.py @@ -23,6 +23,7 @@ def _serialize_settings(settings) -> dict[str, object]: "auto_run_enabled": bool(settings.auto_run_enabled), "run_interval_minutes": int(settings.run_interval_minutes), "request_delay_seconds": int(settings.request_delay_seconds), + "nav_visible_pages": list(settings.nav_visible_pages or []), } @@ -55,6 +56,11 @@ async def update_settings( db_session: AsyncSession = Depends(get_db_session), current_user: User = Depends(get_api_current_user), ): + settings = await user_settings_service.get_or_create_settings( + db_session, + user_id=current_user.id, + ) + try: parsed_interval = user_settings_service.parse_run_interval_minutes( str(payload.run_interval_minutes) @@ -62,6 +68,11 @@ async def update_settings( parsed_delay = user_settings_service.parse_request_delay_seconds( str(payload.request_delay_seconds) ) + parsed_nav_visible_pages = user_settings_service.parse_nav_visible_pages( + payload.nav_visible_pages + if payload.nav_visible_pages is not None + else list(settings.nav_visible_pages or user_settings_service.DEFAULT_NAV_VISIBLE_PAGES) + ) except user_settings_service.UserSettingsServiceError as exc: raise ApiException( status_code=400, @@ -69,16 +80,13 @@ async def update_settings( message=str(exc), ) from exc - settings = await user_settings_service.get_or_create_settings( - db_session, - user_id=current_user.id, - ) updated = await user_settings_service.update_settings( db_session, settings=settings, auto_run_enabled=bool(payload.auto_run_enabled), run_interval_minutes=parsed_interval, request_delay_seconds=parsed_delay, + nav_visible_pages=parsed_nav_visible_pages, ) logger.info( "api.settings.updated", @@ -88,6 +96,7 @@ async def update_settings( "auto_run_enabled": updated.auto_run_enabled, "run_interval_minutes": updated.run_interval_minutes, "request_delay_seconds": updated.request_delay_seconds, + "nav_visible_pages": updated.nav_visible_pages, }, ) return success_payload( diff --git a/app/api/schemas.py b/app/api/schemas.py index 13c093a..73a3452 100644 --- a/app/api/schemas.py +++ b/app/api/schemas.py @@ -433,6 +433,7 @@ class SettingsData(BaseModel): auto_run_enabled: bool run_interval_minutes: int request_delay_seconds: int + nav_visible_pages: list[str] model_config = ConfigDict(extra="forbid") @@ -448,6 +449,7 @@ class SettingsUpdateRequest(BaseModel): auto_run_enabled: bool run_interval_minutes: int request_delay_seconds: int + nav_visible_pages: list[str] | None = None model_config = ConfigDict(extra="forbid") diff --git a/app/db/models.py b/app/db/models.py index 68c3e72..d98cfe9 100644 --- a/app/db/models.py +++ b/app/db/models.py @@ -98,6 +98,13 @@ class UserSetting(Base): request_delay_seconds: Mapped[int] = mapped_column( Integer, nullable=False, server_default=text("10") ) + nav_visible_pages: Mapped[list[str]] = mapped_column( + JSONB, + nullable=False, + server_default=text( + '\'["dashboard","scholars","publications","settings","style-guide","runs","users"]\'::jsonb' + ), + ) created_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), nullable=False, server_default=func.now() ) diff --git a/app/http/middleware.py b/app/http/middleware.py index 9d85458..49a40a4 100644 --- a/app/http/middleware.py +++ b/app/http/middleware.py @@ -11,6 +11,32 @@ from starlette.responses import Response from app.logging_context import set_request_id REQUEST_ID_HEADER = "X-Request-ID" +DEFAULT_PERMISSIONS_POLICY = ( + "accelerometer=(), autoplay=(), camera=(), display-capture=(), " + "geolocation=(), gyroscope=(), microphone=(), payment=(), usb=()" +) +DEFAULT_CSP_POLICY = ( + "default-src 'self'; " + "base-uri 'self'; " + "form-action 'self'; " + "frame-ancestors 'none'; " + "img-src 'self' data: https:; " + "script-src 'self'; " + "style-src 'self' 'unsafe-inline'; " + "font-src 'self' data:; " + "connect-src 'self'; " + "object-src 'none'" +) +DEFAULT_CSP_DOCS_POLICY = ( + "default-src 'self'; " + "img-src 'self' data: https:; " + "script-src 'self' 'unsafe-inline'; " + "style-src 'self' 'unsafe-inline'; " + "font-src 'self' data:; " + "connect-src 'self' https:; " + "object-src 'none'; " + "frame-ancestors 'none'" +) logger = logging.getLogger(__name__) @@ -80,6 +106,100 @@ class RequestLoggingMiddleware(BaseHTTPMiddleware): return any(path.startswith(prefix) for prefix in self._skip_paths) +class SecurityHeadersMiddleware(BaseHTTPMiddleware): + def __init__( + self, + app, + *, + enabled: bool = True, + x_content_type_options: str = "nosniff", + x_frame_options: str = "DENY", + referrer_policy: str = "strict-origin-when-cross-origin", + permissions_policy: str = DEFAULT_PERMISSIONS_POLICY, + cross_origin_opener_policy: str = "same-origin", + cross_origin_resource_policy: str = "same-origin", + content_security_policy_enabled: bool = True, + content_security_policy: str = DEFAULT_CSP_POLICY, + content_security_policy_docs: str = DEFAULT_CSP_DOCS_POLICY, + content_security_policy_report_only: bool = False, + strict_transport_security_enabled: bool = False, + strict_transport_security_max_age: int = 31_536_000, + strict_transport_security_include_subdomains: bool = True, + strict_transport_security_preload: bool = False, + ) -> None: + super().__init__(app) + self._enabled = enabled + self._x_content_type_options = x_content_type_options.strip() + self._x_frame_options = x_frame_options.strip() + self._referrer_policy = referrer_policy.strip() + self._permissions_policy = permissions_policy.strip() + self._cross_origin_opener_policy = cross_origin_opener_policy.strip() + self._cross_origin_resource_policy = cross_origin_resource_policy.strip() + self._csp_enabled = content_security_policy_enabled + self._csp_policy = content_security_policy.strip() + self._csp_docs_policy = content_security_policy_docs.strip() + self._csp_report_only = content_security_policy_report_only + self._hsts_enabled = strict_transport_security_enabled + self._hsts_max_age = max(0, strict_transport_security_max_age) + self._hsts_include_subdomains = strict_transport_security_include_subdomains + self._hsts_preload = strict_transport_security_preload + + async def dispatch(self, request: Request, call_next) -> Response: + response = await call_next(request) + if not self._enabled: + return response + + if self._x_content_type_options: + response.headers.setdefault("X-Content-Type-Options", self._x_content_type_options) + if self._x_frame_options: + response.headers.setdefault("X-Frame-Options", self._x_frame_options) + if self._referrer_policy: + response.headers.setdefault("Referrer-Policy", self._referrer_policy) + if self._permissions_policy: + response.headers.setdefault("Permissions-Policy", self._permissions_policy) + if self._cross_origin_opener_policy: + response.headers.setdefault( + "Cross-Origin-Opener-Policy", + self._cross_origin_opener_policy, + ) + if self._cross_origin_resource_policy: + response.headers.setdefault( + "Cross-Origin-Resource-Policy", + self._cross_origin_resource_policy, + ) + + csp_policy = self._csp_policy_for_path(request.url.path) + if self._csp_enabled and csp_policy: + csp_header = ( + "Content-Security-Policy-Report-Only" + if self._csp_report_only + else "Content-Security-Policy" + ) + response.headers.setdefault(csp_header, csp_policy) + + hsts = self._strict_transport_security_value() + if hsts: + response.headers.setdefault("Strict-Transport-Security", hsts) + + return response + + def _csp_policy_for_path(self, path: str) -> str: + if path.startswith("/docs") or path.startswith("/redoc"): + return self._csp_docs_policy or self._csp_policy + return self._csp_policy + + def _strict_transport_security_value(self) -> str: + if not self._hsts_enabled: + return "" + + directives = [f"max-age={self._hsts_max_age}"] + if self._hsts_include_subdomains: + directives.append("includeSubDomains") + if self._hsts_preload: + directives.append("preload") + return "; ".join(directives) + + def parse_skip_paths(raw_value: str) -> tuple[str, ...]: parts = [part.strip() for part in raw_value.split(",")] return tuple(part for part in parts if part) diff --git a/app/main.py b/app/main.py index 44d5cc7..7fbef93 100644 --- a/app/main.py +++ b/app/main.py @@ -13,7 +13,11 @@ from app.api.router import router as api_router from app.api.runtime_deps import get_ingestion_service, get_scholar_source from app.db.session import check_database from app.db.session import close_engine -from app.http.middleware import RequestLoggingMiddleware, parse_skip_paths +from app.http.middleware import ( + RequestLoggingMiddleware, + SecurityHeadersMiddleware, + parse_skip_paths, +) from app.logging_config import configure_logging, parse_redact_fields from app.security.csrf import CSRFMiddleware from app.services.scheduler import SchedulerService @@ -63,6 +67,26 @@ app.add_middleware( log_requests=settings.log_requests, skip_paths=parse_skip_paths(settings.log_request_skip_paths), ) +app.add_middleware( + SecurityHeadersMiddleware, + enabled=settings.security_headers_enabled, + x_content_type_options=settings.security_x_content_type_options, + x_frame_options=settings.security_x_frame_options, + referrer_policy=settings.security_referrer_policy, + permissions_policy=settings.security_permissions_policy, + cross_origin_opener_policy=settings.security_cross_origin_opener_policy, + cross_origin_resource_policy=settings.security_cross_origin_resource_policy, + content_security_policy_enabled=settings.security_csp_enabled, + content_security_policy=settings.security_csp_policy, + content_security_policy_docs=settings.security_csp_docs_policy, + content_security_policy_report_only=settings.security_csp_report_only, + strict_transport_security_enabled=settings.security_strict_transport_security_enabled, + strict_transport_security_max_age=settings.security_strict_transport_security_max_age, + strict_transport_security_include_subdomains=( + settings.security_strict_transport_security_include_subdomains + ), + strict_transport_security_preload=settings.security_strict_transport_security_preload, +) app.include_router(api_router) diff --git a/app/services/user_settings.py b/app/services/user_settings.py index 92aff32..d3314e7 100644 --- a/app/services/user_settings.py +++ b/app/services/user_settings.py @@ -10,13 +10,38 @@ class UserSettingsServiceError(ValueError): """Raised for expected settings-validation failures.""" +NAV_PAGE_DASHBOARD = "dashboard" +NAV_PAGE_SCHOLARS = "scholars" +NAV_PAGE_PUBLICATIONS = "publications" +NAV_PAGE_SETTINGS = "settings" +NAV_PAGE_STYLE_GUIDE = "style-guide" +NAV_PAGE_RUNS = "runs" +NAV_PAGE_USERS = "users" + +ALLOWED_NAV_PAGES = ( + NAV_PAGE_DASHBOARD, + NAV_PAGE_SCHOLARS, + NAV_PAGE_PUBLICATIONS, + NAV_PAGE_SETTINGS, + NAV_PAGE_STYLE_GUIDE, + NAV_PAGE_RUNS, + NAV_PAGE_USERS, +) +REQUIRED_NAV_PAGES = ( + NAV_PAGE_DASHBOARD, + NAV_PAGE_SCHOLARS, + NAV_PAGE_SETTINGS, +) +DEFAULT_NAV_VISIBLE_PAGES = list(ALLOWED_NAV_PAGES) + + def parse_run_interval_minutes(value: str) -> int: try: parsed = int(value) except ValueError as exc: - raise UserSettingsServiceError("Run interval must be a whole number.") from exc + raise UserSettingsServiceError("Check interval must be a whole number.") from exc if parsed < 15: - raise UserSettingsServiceError("Run interval must be at least 15 minutes.") + raise UserSettingsServiceError("Check interval must be at least 15 minutes.") return parsed @@ -25,11 +50,41 @@ def parse_request_delay_seconds(value: str) -> int: parsed = int(value) except ValueError as exc: raise UserSettingsServiceError("Request delay must be a whole number.") from exc - if parsed < 1: - raise UserSettingsServiceError("Request delay must be at least 1 second.") + if parsed < 2: + raise UserSettingsServiceError("Request delay must be at least 2 seconds.") return parsed +def parse_nav_visible_pages(value: object) -> list[str]: + if not isinstance(value, list): + raise UserSettingsServiceError("Navigation visibility must be a list of page ids.") + + deduped: list[str] = [] + seen: set[str] = set() + + for raw_page in value: + if not isinstance(raw_page, str): + raise UserSettingsServiceError("Navigation visibility entries must be strings.") + + page_id = raw_page.strip() + if page_id not in ALLOWED_NAV_PAGES: + raise UserSettingsServiceError(f"Unsupported navigation page id: {page_id}") + + if page_id in seen: + continue + + seen.add(page_id) + deduped.append(page_id) + + missing_required = [page for page in REQUIRED_NAV_PAGES if page not in seen] + if missing_required: + raise UserSettingsServiceError( + "Dashboard, Scholars, and Settings must remain visible." + ) + + return deduped + + async def get_or_create_settings( db_session: AsyncSession, *, @@ -56,11 +111,12 @@ async def update_settings( auto_run_enabled: bool, run_interval_minutes: int, request_delay_seconds: int, + nav_visible_pages: list[str], ) -> UserSetting: settings.auto_run_enabled = auto_run_enabled settings.run_interval_minutes = run_interval_minutes settings.request_delay_seconds = request_delay_seconds + settings.nav_visible_pages = nav_visible_pages await db_session.commit() await db_session.refresh(settings) return settings - diff --git a/app/settings.py b/app/settings.py index 67931c3..48b875b 100644 --- a/app/settings.py +++ b/app/settings.py @@ -1,6 +1,33 @@ from dataclasses import dataclass import os +DEFAULT_SECURITY_PERMISSIONS_POLICY = ( + "accelerometer=(), autoplay=(), camera=(), display-capture=(), " + "geolocation=(), gyroscope=(), microphone=(), payment=(), usb=()" +) +DEFAULT_SECURITY_CSP_POLICY = ( + "default-src 'self'; " + "base-uri 'self'; " + "form-action 'self'; " + "frame-ancestors 'none'; " + "img-src 'self' data: https:; " + "script-src 'self'; " + "style-src 'self' 'unsafe-inline'; " + "font-src 'self' data:; " + "connect-src 'self'; " + "object-src 'none'" +) +DEFAULT_SECURITY_CSP_DOCS_POLICY = ( + "default-src 'self'; " + "img-src 'self' data: https:; " + "script-src 'self' 'unsafe-inline'; " + "style-src 'self' 'unsafe-inline'; " + "font-src 'self' data:; " + "connect-src 'self' https:; " + "object-src 'none'; " + "frame-ancestors 'none'" +) + def _env_bool(name: str, default: bool) -> bool: value = os.getenv(name) @@ -39,6 +66,42 @@ class Settings: ) session_secret_key: str = os.getenv("SESSION_SECRET_KEY", "dev-insecure-session-key") session_cookie_secure: bool = _env_bool("SESSION_COOKIE_SECURE", False) + security_headers_enabled: bool = _env_bool("SECURITY_HEADERS_ENABLED", True) + security_x_content_type_options: str = _env_str("SECURITY_X_CONTENT_TYPE_OPTIONS", "nosniff") + security_x_frame_options: str = _env_str("SECURITY_X_FRAME_OPTIONS", "DENY") + security_referrer_policy: str = _env_str("SECURITY_REFERRER_POLICY", "strict-origin-when-cross-origin") + security_permissions_policy: str = _env_str( + "SECURITY_PERMISSIONS_POLICY", + DEFAULT_SECURITY_PERMISSIONS_POLICY, + ) + security_cross_origin_opener_policy: str = _env_str( + "SECURITY_CROSS_ORIGIN_OPENER_POLICY", + "same-origin", + ) + security_cross_origin_resource_policy: str = _env_str( + "SECURITY_CROSS_ORIGIN_RESOURCE_POLICY", + "same-origin", + ) + security_csp_enabled: bool = _env_bool("SECURITY_CSP_ENABLED", True) + security_csp_policy: str = _env_str("SECURITY_CSP_POLICY", DEFAULT_SECURITY_CSP_POLICY) + security_csp_docs_policy: str = _env_str("SECURITY_CSP_DOCS_POLICY", DEFAULT_SECURITY_CSP_DOCS_POLICY) + security_csp_report_only: bool = _env_bool("SECURITY_CSP_REPORT_ONLY", False) + security_strict_transport_security_enabled: bool = _env_bool( + "SECURITY_STRICT_TRANSPORT_SECURITY_ENABLED", + False, + ) + security_strict_transport_security_max_age: int = _env_int( + "SECURITY_STRICT_TRANSPORT_SECURITY_MAX_AGE", + 31_536_000, + ) + security_strict_transport_security_include_subdomains: bool = _env_bool( + "SECURITY_STRICT_TRANSPORT_SECURITY_INCLUDE_SUBDOMAINS", + True, + ) + security_strict_transport_security_preload: bool = _env_bool( + "SECURITY_STRICT_TRANSPORT_SECURITY_PRELOAD", + False, + ) login_rate_limit_attempts: int = _env_int("LOGIN_RATE_LIMIT_ATTEMPTS", 5) login_rate_limit_window_seconds: int = _env_int( "LOGIN_RATE_LIMIT_WINDOW_SECONDS", @@ -116,11 +179,11 @@ class Settings: ) scholar_name_search_min_interval_seconds: float = _env_float( "SCHOLAR_NAME_SEARCH_MIN_INTERVAL_SECONDS", - 3.0, + 8.0, ) scholar_name_search_interval_jitter_seconds: float = _env_float( "SCHOLAR_NAME_SEARCH_INTERVAL_JITTER_SECONDS", - 1.0, + 2.0, ) scholar_name_search_cooldown_block_threshold: int = _env_int( "SCHOLAR_NAME_SEARCH_COOLDOWN_BLOCK_THRESHOLD", diff --git a/frontend/package.json b/frontend/package.json index 2b5f7d3..f9c9a02 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -8,6 +8,7 @@ "build": "vue-tsc --noEmit && vite build", "preview": "vite preview --host 0.0.0.0 --port 4173", "typecheck": "vue-tsc --noEmit", + "check:theme-tokens": "node ./scripts/check_theme_tokens.mjs", "test": "vitest", "test:run": "vitest run" }, diff --git a/frontend/scripts/check_theme_tokens.mjs b/frontend/scripts/check_theme_tokens.mjs new file mode 100644 index 0000000..7fb50a6 --- /dev/null +++ b/frontend/scripts/check_theme_tokens.mjs @@ -0,0 +1,84 @@ +import fs from "node:fs"; +import path from "node:path"; + +const projectRoot = path.resolve(path.dirname(new URL(import.meta.url).pathname), ".."); +const sourceRoot = path.join(projectRoot, "src"); +const allowedExtensions = new Set([".vue", ".css"]); + +const checks = [ + { + name: "non-token palette utility", + regex: /\b(?:bg|text|border|ring|from|to|via)-(?:zinc|gray|slate|neutral|stone|white|black)(?:[-/][A-Za-z0-9_\-[\]./%]+)?\b/g, + }, + { + name: "dark mode utility variant", + regex: /\bdark:[^\s"'`<>}]*/g, + }, + { + name: "inline hex color", + regex: /#[0-9a-fA-F]{3,8}\b/g, + }, +]; + +function listFilesRecursively(dirPath) { + const entries = fs.readdirSync(dirPath, { withFileTypes: true }); + const files = []; + + for (const entry of entries) { + const nextPath = path.join(dirPath, entry.name); + if (entry.isDirectory()) { + files.push(...listFilesRecursively(nextPath)); + continue; + } + + if (allowedExtensions.has(path.extname(nextPath))) { + files.push(nextPath); + } + } + + return files; +} + +function relativePath(filePath) { + return path.relative(projectRoot, filePath).replaceAll(path.sep, "/"); +} + +function run() { + const violations = []; + const files = listFilesRecursively(sourceRoot); + + for (const filePath of files) { + const content = fs.readFileSync(filePath, "utf8"); + const lines = content.split(/\r?\n/); + + for (let index = 0; index < lines.length; index += 1) { + const line = lines[index]; + + for (const check of checks) { + check.regex.lastIndex = 0; + let match = check.regex.exec(line); + while (match) { + violations.push({ + filePath: relativePath(filePath), + line: index + 1, + check: check.name, + value: match[0], + }); + match = check.regex.exec(line); + } + } + } + } + + if (violations.length > 0) { + console.error("Theme token policy violations found:\n"); + for (const violation of violations) { + console.error(`${violation.filePath}:${violation.line} [${violation.check}] ${violation.value}`); + } + process.exit(1); + } + + console.log(`Theme token policy passed for ${files.length} files.`); +} + +run(); diff --git a/frontend/src/app/AppShell.vue b/frontend/src/app/AppShell.vue index 5bee674..4d5d104 100644 --- a/frontend/src/app/AppShell.vue +++ b/frontend/src/app/AppShell.vue @@ -12,23 +12,27 @@ const showChrome = computed(() => auth.isAuthenticated);