diff --git a/alembic/versions/0032_source_error_type.py b/alembic/versions/0032_source_error_type.py new file mode 100644 index 0000000..e264b35 --- /dev/null +++ b/alembic/versions/0032_source_error_type.py @@ -0,0 +1,41 @@ +"""source.error_type: surface ErrorType taxonomy in FailingSourcesCard + +Revision ID: 0032 +Revises: 0031 +Create Date: 2026-06-02 + +Audit 2026-06-02: the backend computes 13 ErrorType categories (auth_error, +rate_limited, not_found, access_denied, validation_failed, etc.) and +stamps each one on DownloadEvent.metadata, but the Source row only carried +the free-text last_error. Operators couldn't bulk-triage failing sources +("all auth_error → rotate cookies, all rate_limited → just wait") without +opening Logs per row. + +This column receives the last error_type from _update_source_health +and gets cleared on a successful run. Nullable + indexed so the failing- +sources rollup can filter/group cheaply. +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "0032" +down_revision: Union[str, None] = "0031" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "source", + sa.Column("error_type", sa.String(length=32), nullable=True), + ) + op.create_index( + "ix_source_error_type", "source", ["error_type"], + ) + + +def downgrade() -> None: + op.drop_index("ix_source_error_type", table_name="source") + op.drop_column("source", "error_type") diff --git a/backend/app/api/gallery.py b/backend/app/api/gallery.py index 8de2ed8..41b561c 100644 --- a/backend/app/api/gallery.py +++ b/backend/app/api/gallery.py @@ -43,7 +43,6 @@ async def scroll(): "height": i.height, "created_at": i.created_at.isoformat(), "posted_at": i.posted_at.isoformat() if i.posted_at else None, - "effective_date": i.effective_date.isoformat(), "thumbnail_url": i.thumbnail_url, "artist": i.artist, } diff --git a/backend/app/api/tags.py b/backend/app/api/tags.py index e18d94d..9061985 100644 --- a/backend/app/api/tags.py +++ b/backend/app/api/tags.py @@ -245,6 +245,12 @@ async def merge_tag(source_id: int): from ..tasks.ml import apply_allowlist_tags apply_allowlist_tags.delay(tag_id=result.target_id) + # Tag merge invalidates the target's centroid (the merged-in source + # tag's images now contribute to it). Daily list_drifted catches it + # within 24h, but eager recompute closes the suggestion-quality dip + # in the meantime. Audit 2026-06-02. + from ..tasks.ml import recompute_centroid + recompute_centroid.delay(result.target_id) return jsonify( { "target": { diff --git a/backend/app/celery_app.py b/backend/app/celery_app.py index 4051e21..eca48e8 100644 --- a/backend/app/celery_app.py +++ b/backend/app/celery_app.py @@ -130,6 +130,16 @@ def make_celery() -> Celery: "task": "backend.app.tasks.maintenance.prune_import_batches", "schedule": 86400.0, }, + # Audit 2026-06-02 — backfill_thumbnails's docstring claimed + # "periodic Beat" but the entry was never registered, so the + # library got no self-healing thumbnail repair; only the + # manual admin-UI button fired it. Daily cadence is gentle + # (the task is idempotent and only enqueues regen for rows + # whose stored thumbnails are missing or corrupt). + "backfill-thumbnails-daily": { + "task": "backend.app.tasks.thumbnail.backfill_thumbnails", + "schedule": 86400.0, + }, }, timezone="UTC", ) diff --git a/backend/app/models/source.py b/backend/app/models/source.py index 407227e..1bf6c67 100644 --- a/backend/app/models/source.py +++ b/backend/app/models/source.py @@ -26,6 +26,11 @@ class Source(Base): last_checked_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) last_error: Mapped[str | None] = mapped_column(Text, nullable=True) + # alembic 0032: last ErrorType category (auth_error, rate_limited, + # not_found, ...). Lets FailingSourcesCard surface the taxonomy as + # a colored chip so operators can bulk-triage by error class. Set + # by _update_source_health alongside last_error; cleared on 'ok'. + error_type: Mapped[str | None] = mapped_column(String(32), nullable=True, index=True) check_interval_override: Mapped[int | None] = mapped_column(Integer, nullable=True) consecutive_failures: Mapped[int] = mapped_column(Integer, nullable=False, default=0) diff --git a/backend/app/services/artist_service.py b/backend/app/services/artist_service.py index 0947411..1203f51 100644 --- a/backend/app/services/artist_service.py +++ b/backend/app/services/artist_service.py @@ -208,27 +208,32 @@ class ArtistService: ) async def find_or_create(self, name: str) -> tuple[Artist, bool]: - """Return (artist, created). Slug-keyed; idempotent under races.""" + """Return (artist, created). Slug-keyed; idempotent under races. + + Audit 2026-06-02: switched from session.rollback() to a + begin_nested savepoint + IntegrityError recovery so a lost + race doesn't unwind the calling request's surrounding work. + Mirrors importer._get_or_create. + """ cleaned = (name or "").strip() if not cleaned: raise ValueError("artist name must not be empty") slug = slugify(cleaned) - existing = (await self.session.execute( - select(Artist).where(Artist.slug == slug) - )).scalar_one_or_none() + select_existing = select(Artist).where(Artist.slug == slug) + existing = (await self.session.execute(select_existing)).scalar_one_or_none() if existing is not None: return existing, False - artist = Artist(name=cleaned, slug=slug) - self.session.add(artist) + sp = await self.session.begin_nested() try: + artist = Artist(name=cleaned, slug=slug) + self.session.add(artist) await self.session.flush() + await sp.commit() except IntegrityError: - await self.session.rollback() - existing = (await self.session.execute( - select(Artist).where(Artist.slug == slug) - )).scalar_one() + await sp.rollback() + existing = (await self.session.execute(select_existing)).scalar_one() return existing, False await self.session.commit() return artist, True diff --git a/backend/app/services/cleanup_service.py b/backend/app/services/cleanup_service.py index 28b20da..58aab98 100644 --- a/backend/app/services/cleanup_service.py +++ b/backend/app/services/cleanup_service.py @@ -187,10 +187,14 @@ def unlink_image_files( out["thumbnail"] = True except OSError: out["thumbnail"] = False - # Convention thumbs dir — try all extensions; missing OK. + # Convention thumbs dir — try both extensions thumbnailer writes + # (.jpg for opaque, .png for alpha). `.webp` used to be in this + # tuple but the thumbnailer never writes it (operator-flagged in + # the 2026-06-02 audit) — keep the tuple aligned with what + # actually lands on disk. if image.sha256: bucket = image.sha256[:3] - for ext in ("jpg", "png", "webp"): + for ext in ("jpg", "png"): try: (images_root / "thumbs" / bucket / f"{image.sha256}.{ext}").unlink( missing_ok=True, diff --git a/backend/app/services/credential_crypto.py b/backend/app/services/credential_crypto.py index 86a45ce..273e574 100644 --- a/backend/app/services/credential_crypto.py +++ b/backend/app/services/credential_crypto.py @@ -1,40 +1,82 @@ """Fernet-based encryption for credential blobs. The key is a single 32-byte value (urlsafe-base64-encoded; what -Fernet.generate_key produces) stored at a fixed path inside the -images/data root. Created on first boot if absent; mode 0600. No KDF -needed — the file contents are already maximum-entropy random bytes. +Fernet.generate_key produces) stored at /images/secrets/credential_key.b64 +(mode 0600, parent dir 0700). The 2026-06-02 audit caught a silent +key-regeneration path: on a partial disaster restore where the DB was +restored but the secrets dir was lost, the old `_load_or_create_key` +would mint a fresh key with no log, producing a working-looking system +where every authenticated download failed AUTH_ERROR until the operator +re-uploaded every credential by hand. Now the constructor refuses to +auto-generate unless either: -Operator backup procedure must include this file alongside the rest -of /images/ — losing it makes existing encrypted_blob rows -undecryptable (recovery = delete the rows and re-upload). + * the caller explicitly passes `bootstrap_ok=True` (tests, scripts), or + * the env var `CURATOR_BOOTSTRAP_NEW_KEY=1` is set (operator opt-in + during first-time setup). + +Otherwise it raises `MissingCredentialKey` so the app fails fast at +startup and the operator can restore the key file from backup. + +Operator backup procedure must include /images/secrets/ alongside the +rest of /images/ — losing the key file makes existing encrypted_blob +rows undecryptable (recovery = delete the rows and re-upload). """ +import logging import os from pathlib import Path from cryptography.fernet import Fernet, InvalidToken +log = logging.getLogger(__name__) + +_BOOTSTRAP_ENV_VAR = "CURATOR_BOOTSTRAP_NEW_KEY" + class InvalidCredentialBlob(Exception): """Raised when decryption fails (wrong key, tampered blob, …).""" +class MissingCredentialKey(Exception): + """The Fernet key file is missing AND the caller hasn't opted in to + generating a new one. Audit 2026-06-02: prevents silent key + regeneration on partial DB-restored / secrets-lost deployments. + Set CURATOR_BOOTSTRAP_NEW_KEY=1 for first-time setup, or restore the + key file from backup.""" + + class CredentialCrypto: """Fernet encrypt/decrypt with an on-disk key file. - Instantiate with a path; the file is created on first access and - reused thereafter. Tests pass a tmp_path; production calls with + Instantiate with a path; the file is loaded if present, or created + if absent AND the caller has opted in (bootstrap_ok=True or + CURATOR_BOOTSTRAP_NEW_KEY=1 env var). Production sites: `IMAGES_ROOT / "secrets" / "credential_key.b64"`. """ - def __init__(self, key_path: Path): + def __init__(self, key_path: Path, *, bootstrap_ok: bool | None = None): self._key_path = Path(key_path) - self._fernet = Fernet(self._load_or_create_key()) + if bootstrap_ok is None: + bootstrap_ok = os.environ.get(_BOOTSTRAP_ENV_VAR) == "1" + self._fernet = Fernet(self._load_or_create_key(bootstrap_ok)) - def _load_or_create_key(self) -> bytes: + def _load_or_create_key(self, bootstrap_ok: bool) -> bytes: if self._key_path.exists(): return self._key_path.read_bytes() + if not bootstrap_ok: + raise MissingCredentialKey( + f"Fernet key file not found at {self._key_path}. " + f"For first-time setup, set {_BOOTSTRAP_ENV_VAR}=1. " + f"If this is a restored instance, restore the key file " + f"from backup — generating a new one would make every " + f"existing Credential row undecryptable." + ) + log.warning( + "Generating NEW Fernet credential key at %s. Any existing " + "encrypted_blob rows in the DB will be undecryptable — " + "re-upload each credential after this completes.", + self._key_path, + ) parent = self._key_path.parent parent.mkdir(parents=True, exist_ok=True) os.chmod(parent, 0o700) diff --git a/backend/app/services/download_service.py b/backend/app/services/download_service.py index d8c15e9..92b499e 100644 --- a/backend/app/services/download_service.py +++ b/backend/app/services/download_service.py @@ -35,6 +35,7 @@ from .gallery_dl import ( ) from .importer import Importer from .patreon_resolver import resolve_campaign_id +from .platforms import auth_type_for from .scheduler_service import set_platform_cooldown log = logging.getLogger(__name__) @@ -194,7 +195,12 @@ class DownloadService: event_id = ev.id artist = source.artist - if source.platform in ("discord", "pixiv"): + # Drive cookies-vs-token selection from the platform registry's + # auth_type so a new 7th token-platform automatically picks the + # right credential path. The hardcoded tuple here used to drift + # out of sync with credential_service's auth_type_for(). Audit + # 2026-06-02. + if auth_type_for(source.platform) == "token": cookies_path = None auth_token = await self.cred_service.get_token(source.platform) else: @@ -428,9 +434,15 @@ class DownloadService: if status == "ok": source.consecutive_failures = 0 source.last_error = None + # alembic 0032 — clear the failure-class chip on success. + source.error_type = None elif status == "error": source.consecutive_failures = (source.consecutive_failures or 0) + 1 source.last_error = error_message + # alembic 0032 — stamp the failure-class so FailingSourcesCard + # can render a colored chip and operators can bulk-triage + # by error class without opening Logs per row. + source.error_type = error_type if error_type == "rate_limited": await set_platform_cooldown(self.async_session, source.platform) elif status == "skipped": diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index 13614c3..72713c5 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -374,10 +374,19 @@ class Importer: artist = self._resolve_artist(source) post = self._post_for_sidecar(source, artist) sha = _sha256_of(source) - existing = self.session.execute( - select(PostAttachment).where(PostAttachment.sha256 == sha) - ).scalar_one_or_none() - if existing is None: + select_existing = select(PostAttachment).where(PostAttachment.sha256 == sha) + existing = self.session.execute(select_existing).scalar_one_or_none() + if existing is not None: + self.session.commit() + return ImportResult(status="attached") + # Savepoint + IntegrityError recovery — PostAttachment.sha256 is + # UNIQUE, so two workers can both pass the SELECT and only the + # second INSERT fails. Without savepoint, the outer transaction + # poisons and the calling task crashes. attachments.store is + # sha-addressed so both workers race to write the same target + # path; shutil.copy2 + rename is idempotent. Audit 2026-06-02. + sp = self.session.begin_nested() + try: stored = self.attachments.store(source, sha) self.session.add(PostAttachment( post_id=post.id if post else None, @@ -390,6 +399,11 @@ class Importer: size_bytes=source.stat().st_size, )) self.session.flush() + sp.commit() + except IntegrityError: + sp.rollback() + # Lost the race — the other worker's row is canonical. + self.session.execute(select_existing).scalar_one() self.session.commit() return ImportResult(status="attached") diff --git a/backend/app/services/source_service.py b/backend/app/services/source_service.py index 1e2031c..513c9a1 100644 --- a/backend/app/services/source_service.py +++ b/backend/app/services/source_service.py @@ -59,6 +59,7 @@ class SourceRecord: config_overrides: dict | None last_checked_at: str | None last_error: str | None + error_type: str | None check_interval_override: int | None consecutive_failures: int next_check_at: str | None @@ -76,6 +77,7 @@ class SourceRecord: "config_overrides": self.config_overrides, "last_checked_at": self.last_checked_at, "last_error": self.last_error, + "error_type": self.error_type, "check_interval_override": self.check_interval_override, "consecutive_failures": self.consecutive_failures, "next_check_at": self.next_check_at, @@ -144,6 +146,7 @@ class SourceService: config_overrides=source.config_overrides, last_checked_at=source.last_checked_at.isoformat() if source.last_checked_at else None, last_error=source.last_error, + error_type=source.error_type, check_interval_override=source.check_interval_override, consecutive_failures=source.consecutive_failures or 0, next_check_at=nxt.isoformat() if nxt else None, diff --git a/backend/app/services/tag_service.py b/backend/app/services/tag_service.py index da9e499..efb0add 100644 --- a/backend/app/services/tag_service.py +++ b/backend/app/services/tag_service.py @@ -5,6 +5,7 @@ from dataclasses import dataclass from sqlalchemy import and_, case, exists, func, select, text, update from sqlalchemy.dialects.postgresql import insert as pg_insert +from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession from ..models import Tag, TagKind, image_tag @@ -86,9 +87,12 @@ class TagService: f"fandom_id {fandom_id} does not reference a fandom tag" ) - # Upsert via INSERT ... ON CONFLICT DO NOTHING. We can't use the - # uniqueness index name directly (it's a partial coalesce-based - # expression), so we re-select after insert. + # Audit 2026-06-02: race-safe upsert via savepoint + + # IntegrityError recovery. The partial uniqueness index on + # (name, kind, COALESCE(fandom_id, -1)) catches concurrent + # inserts; without the savepoint the outer transaction would + # poison and the calling request crashes. Mirrors + # importer._get_or_create. stmt = ( select(Tag) .where(Tag.name == name) @@ -101,10 +105,16 @@ class TagService: if existing: return existing - new_tag = Tag(name=name, kind=kind, fandom_id=fandom_id) - self.session.add(new_tag) - await self.session.flush() - return new_tag + sp = await self.session.begin_nested() + try: + new_tag = Tag(name=name, kind=kind, fandom_id=fandom_id) + self.session.add(new_tag) + await self.session.flush() + await sp.commit() + return new_tag + except IntegrityError: + await sp.rollback() + return (await self.session.execute(stmt)).scalar_one() async def autocomplete( self, diff --git a/frontend/src/components/settings/QueuesTable.vue b/frontend/src/components/settings/QueuesTable.vue index 2edcfb2..b326101 100644 --- a/frontend/src/components/settings/QueuesTable.vue +++ b/frontend/src/components/settings/QueuesTable.vue @@ -35,7 +35,7 @@ import { computed } from 'vue' const props = defineProps({ queues: { type: Object, default: null }, // store.queues workers: { type: Object, default: null }, // store.workers - recentMinute: { type: Array, default: () => [] }, // store.recentMinute + recentRuns: { type: Array, default: () => [] }, // store.recentRuns compact: { type: Boolean, default: false }, }) @@ -80,7 +80,7 @@ function activeCount(name) { const recentByQueue = computed(() => { const out = {} - for (const r of props.recentMinute) { + for (const r of props.recentRuns) { if (!out[r.queue]) out[r.queue] = { ok: 0, err: 0 } if (r.status === 'ok') out[r.queue].ok++ else if (r.status === 'error' || r.status === 'timeout') out[r.queue].err++ diff --git a/frontend/src/components/settings/SystemActivitySummary.vue b/frontend/src/components/settings/SystemActivitySummary.vue index c494bbd..354f94f 100644 --- a/frontend/src/components/settings/SystemActivitySummary.vue +++ b/frontend/src/components/settings/SystemActivitySummary.vue @@ -24,7 +24,7 @@ @@ -47,7 +47,7 @@ function pollOnce() { if (document.hidden) return store.loadQueues() store.loadWorkers() - store.loadRecentMinute() + store.loadRecentRuns() } onMounted(() => { diff --git a/frontend/src/components/settings/SystemActivityTab.vue b/frontend/src/components/settings/SystemActivityTab.vue index fe9db71..ff2f7a7 100644 --- a/frontend/src/components/settings/SystemActivityTab.vue +++ b/frontend/src/components/settings/SystemActivityTab.vue @@ -14,7 +14,7 @@ @@ -202,7 +202,7 @@ function pollQueues() { if (document.hidden) return store.loadQueues() store.loadWorkers() - store.loadRecentMinute() + store.loadRecentRuns() } function pollFailures() { if (document.hidden) return diff --git a/frontend/src/components/subscriptions/FailingSourcesCard.vue b/frontend/src/components/subscriptions/FailingSourcesCard.vue index 21d8e3d..024a4df 100644 --- a/frontend/src/components/subscriptions/FailingSourcesCard.vue +++ b/frontend/src/components/subscriptions/FailingSourcesCard.vue @@ -25,6 +25,15 @@ {{ s.consecutive_failures }}× failed + + {{ s.error_type }} + {{ s.last_error || 'no error message recorded' }} @@ -66,6 +75,42 @@ const open = ref(true) // Per-row loading flag so the spinner lives on the row whose Logs // button was clicked, not on every row. const logLoadingIds = ref(new Set()) + +// Audit 2026-06-02: surface the ErrorType taxonomy as a colored chip +// next to the consecutive-failures count so operators can bulk-triage +// by error class. Color reflects "what to do next": +// warning (yellow) — auth/cookie issue: operator should rotate +// info (blue) — backend-paced (cooldown / rate limit / timeout) +// error (red) — likely terminal without operator intervention +const ERROR_TYPE_COLOR = { + auth_error: 'warning', + rate_limited: 'info', + timeout: 'info', + network_error: 'info', + not_found: 'error', + access_denied: 'error', + validation_failed: 'error', + unsupported_url: 'error', + http_error: 'error', + unknown_error: 'error', + partial: 'info', + tier_limited: 'info', + no_new_content: 'info', +} +const ERROR_TYPE_HINT = { + auth_error: 'Cookies likely expired — re-upload in Credentials.', + rate_limited: 'Platform-wide cooldown active. Will retry after it expires.', + timeout: 'Subprocess exceeded its time budget. Often retries cleanly.', + network_error: 'Transient network issue. Will retry on next tick.', + not_found: 'URL 404 — creator may have renamed or deleted.', + access_denied: 'Subscription tier may not grant this content.', + validation_failed: 'Downloaded files were quarantined by the validator.', + http_error: 'Generic HTTP error — see Logs.', + unsupported_url: 'gallery-dl does not support this URL pattern.', + unknown_error: 'Could not classify — see Logs.', +} +function errorTypeColor(t) { return ERROR_TYPE_COLOR[t] || 'error' } +function errorTypeHint(t) { return ERROR_TYPE_HINT[t] || '' } async function onViewLogs(s) { if (logLoadingIds.value.has(s.id)) return logLoadingIds.value = new Set(logLoadingIds.value).add(s.id) @@ -115,6 +160,7 @@ async function onViewLogs(s) { } .fc-fail__artist { font-weight: 600; white-space: nowrap; } .fc-fail__count { flex: 0 0 auto; } +.fc-fail__class { flex: 0 0 auto; } .fc-fail__err { color: rgb(var(--v-theme-on-surface-variant)); font-size: 0.8rem; diff --git a/frontend/src/stores/modal.js b/frontend/src/stores/modal.js index 2b89d76..05876d0 100644 --- a/frontend/src/stores/modal.js +++ b/frontend/src/stores/modal.js @@ -155,6 +155,19 @@ export const useModalStore = defineStore('modal', () => { const imageId = currentImageId.value if (!imageId) return const tag = await api.post('/api/tags', { body: { name, kind, fandom_id } }) + // Audit 2026-06-02: a kind='fandom' created here used to be + // invisible to FandomPicker until a full page reload — its load + // gates on fandomCache.length, so a non-empty cache skips the + // refetch and the new fandom never appears. Push it into the + // cache directly so the next open sees it. + if (kind === 'fandom') { + const { useTagStore } = await import('./tags.js') + const tagStore = useTagStore() + tagStore.fandomCache.push({ + id: tag.id, name: tag.name, kind: 'fandom', + fandom_id: null, fandom_name: null, image_count: 0, + }) + } if (currentImageId.value !== imageId) return // navigated away await addExistingTag(tag.id) } diff --git a/frontend/src/stores/systemActivity.js b/frontend/src/stores/systemActivity.js index 7d8252f..44980a6 100644 --- a/frontend/src/stores/systemActivity.js +++ b/frontend/src/stores/systemActivity.js @@ -9,7 +9,7 @@ export const useSystemActivityStore = defineStore('systemActivity', () => { // Live polled state. const queues = ref(null) // { queues: {name: depth|null}, fetched_at } const workers = ref(null) // { workers: {hostname: {...}}, fetched_at } - const recentMinute = ref([]) // last-60s rows (for Overview summary) + const recentRuns = ref([]) // last-60s rows (for Overview summary) const failures = ref(null) // { recent, count_by_type, since } // Paginated runs (Activity tab "All recent activity" pane). @@ -45,14 +45,14 @@ export const useSystemActivityStore = defineStore('systemActivity', () => { } } - async function loadRecentMinute() { + async function loadRecentRuns() { // Used by the Overview summary card: pull last 60s of runs to compute // per-queue ok/err counts. One call covers all queues; UI groups. try { const body = await api.get('/api/system/activity/runs', { params: { limit: 200 }, }) - recentMinute.value = body.runs || [] + recentRuns.value = body.runs || [] } catch (e) { lastError.value = e.message } @@ -106,10 +106,10 @@ export const useSystemActivityStore = defineStore('systemActivity', () => { } return { - queues, workers, recentMinute, failures, summary, + queues, workers, recentRuns, failures, summary, runs, runsCursor, runsHasMore, runsFilter, loading, lastError, - loadQueues, loadWorkers, loadRecentMinute, + loadQueues, loadWorkers, loadRecentRuns, loadRuns, loadFailures, loadSummary, setFilter, } }) diff --git a/tests/conftest.py b/tests/conftest.py index 0afacb0..da8bb9d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,18 +8,25 @@ migration code paths. import os -import pytest -import pytest_asyncio -from sqlalchemy import create_engine -from sqlalchemy.ext.asyncio import ( +# Audit 2026-06-02: CredentialCrypto now refuses to auto-generate a +# Fernet key without explicit opt-in (production safety against silent +# key regeneration on partial restore). The test environment never has +# a pre-seeded key file, so set the bootstrap flag here before any +# create_app() / CredentialCrypto() import path fires. +os.environ.setdefault("CURATOR_BOOTSTRAP_NEW_KEY", "1") + +import pytest # noqa: E402 +import pytest_asyncio # noqa: E402 +from sqlalchemy import create_engine # noqa: E402 +from sqlalchemy.ext.asyncio import ( # noqa: E402 AsyncSession, async_sessionmaker, create_async_engine, ) -from sqlalchemy.orm import sessionmaker +from sqlalchemy.orm import sessionmaker # noqa: E402 -from backend.app import create_app -from backend.app.models import Base +from backend.app import create_app # noqa: E402 +from backend.app.models import Base # noqa: E402 def _async_database_url() -> str: diff --git a/tests/test_credential_crypto.py b/tests/test_credential_crypto.py index 199b838..6274bb5 100644 --- a/tests/test_credential_crypto.py +++ b/tests/test_credential_crypto.py @@ -14,7 +14,7 @@ pytestmark = pytest.mark.integration def test_load_or_create_writes_key_with_mode_0600(tmp_path): key_path = tmp_path / "secrets" / "credential_key.b64" - CredentialCrypto(key_path) + CredentialCrypto(key_path, bootstrap_ok=True) assert key_path.exists() mode = stat.S_IMODE(os.stat(key_path).st_mode) assert mode == 0o600 @@ -25,9 +25,9 @@ def test_load_or_create_writes_key_with_mode_0600(tmp_path): def test_load_existing_key_is_idempotent(tmp_path): key_path = tmp_path / "credential_key.b64" - crypto1 = CredentialCrypto(key_path) + crypto1 = CredentialCrypto(key_path, bootstrap_ok=True) contents_after_first = key_path.read_bytes() - crypto2 = CredentialCrypto(key_path) + crypto2 = CredentialCrypto(key_path, bootstrap_ok=True) contents_after_second = key_path.read_bytes() assert contents_after_first == contents_after_second # And both crypto instances decrypt each other's ciphertext @@ -36,7 +36,7 @@ def test_load_existing_key_is_idempotent(tmp_path): def test_encrypt_decrypt_round_trip(tmp_path): - crypto = CredentialCrypto(tmp_path / "k") + crypto = CredentialCrypto(tmp_path / "k", bootstrap_ok=True) plaintext = "domain.com\tTRUE\t/\tTRUE\t1700000000\tname\tvalue" ct = crypto.encrypt(plaintext) assert isinstance(ct, bytes) @@ -45,8 +45,27 @@ def test_encrypt_decrypt_round_trip(tmp_path): def test_decrypt_with_wrong_key_raises(tmp_path): - crypto_a = CredentialCrypto(tmp_path / "a") - crypto_b = CredentialCrypto(tmp_path / "b") + crypto_a = CredentialCrypto(tmp_path / "a", bootstrap_ok=True) + crypto_b = CredentialCrypto(tmp_path / "b", bootstrap_ok=True) ct = crypto_a.encrypt("secret") with pytest.raises(InvalidCredentialBlob): crypto_b.decrypt(ct) + + +def test_missing_key_without_bootstrap_raises(tmp_path, monkeypatch): + """Audit 2026-06-02: without explicit opt-in, a missing key file + is a fatal startup error — silent regeneration on partial restore + would make every existing Credential row undecryptable.""" + from backend.app.services.credential_crypto import MissingCredentialKey + monkeypatch.delenv("CURATOR_BOOTSTRAP_NEW_KEY", raising=False) + with pytest.raises(MissingCredentialKey): + CredentialCrypto(tmp_path / "absent.b64") + + +def test_missing_key_with_env_var_bootstraps(tmp_path, monkeypatch): + """The env var CURATOR_BOOTSTRAP_NEW_KEY=1 is the operator's + first-time-setup opt-in for auto-creating the key file.""" + monkeypatch.setenv("CURATOR_BOOTSTRAP_NEW_KEY", "1") + key_path = tmp_path / "bootstrap.b64" + CredentialCrypto(key_path) # no bootstrap_ok kwarg — relies on env + assert key_path.exists() diff --git a/tests/test_credential_service.py b/tests/test_credential_service.py index b856761..b9857ec 100644 --- a/tests/test_credential_service.py +++ b/tests/test_credential_service.py @@ -21,7 +21,7 @@ _NETSCAPE = ( @pytest.fixture def crypto(tmp_path): - return CredentialCrypto(tmp_path / "k") + return CredentialCrypto(tmp_path / "k", bootstrap_ok=True) @pytest.mark.asyncio diff --git a/tests/test_download_service.py b/tests/test_download_service.py index 65ea850..cc07f05 100644 --- a/tests/test_download_service.py +++ b/tests/test_download_service.py @@ -130,7 +130,7 @@ async def test_download_source_attaches_written_files( thumbnailer=Thumbnailer(images_root=images_root), settings=sync_settings, ) - crypto = CredentialCrypto(tmp_path / "key.b64") + crypto = CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True) cred_service = CredentialService(db, crypto) svc = DownloadService( @@ -405,7 +405,7 @@ async def test_backfill_decrements_after_run( session=db_sync, images_root=images_root, import_root=images_root, thumbnailer=Thumbnailer(images_root=images_root), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) svc = DownloadService( async_session=db, sync_session=db_sync, gdl=fake_gdl, importer=importer, cred_service=cred_service, @@ -446,7 +446,7 @@ async def test_backfill_auto_resets_on_clean_zero_files( thumbnailer=Thumbnailer(images_root=tmp_path / "images"), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) svc = DownloadService( async_session=db, sync_session=db_sync, gdl=fake_gdl, importer=importer, cred_service=cred_service, @@ -484,7 +484,7 @@ async def test_tick_mode_does_not_touch_backfill_counter( thumbnailer=Thumbnailer(images_root=tmp_path / "images"), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) svc = DownloadService( async_session=db, sync_session=db_sync, gdl=fake_gdl, importer=importer, cred_service=cred_service, @@ -529,7 +529,7 @@ async def test_partial_error_type_maps_to_ok_status( session=db_sync, images_root=images_root, import_root=images_root, thumbnailer=Thumbnailer(images_root=images_root), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) svc = DownloadService( async_session=db, sync_session=db_sync, gdl=fake_gdl, importer=importer, cred_service=cred_service, @@ -582,7 +582,7 @@ async def test_download_enqueues_thumbnail_and_ml_per_attached_image( session=db_sync, images_root=images_root, import_root=images_root, thumbnailer=Thumbnailer(images_root=images_root), settings=sync_settings, ) - cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64")) + cred_service = CredentialService(db, CredentialCrypto(tmp_path / "key.b64", bootstrap_ok=True)) # Capture the IDs that the orchestrator hands off to each Celery task. # The .delay() shim runs inside DownloadService._phase3_persist (lazy