audit-g5: architectural debt — 4 bundles (A/B/C/D) #53

Merged

bvandeusen merged 6 commits from dev into main 2026-06-02 18:07:25 -04:00

Conversation 0 Commits 6 Files Changed 22

+303 -67

6 Commits

Author	SHA1	Message	Date
bvandeusen	9f7261b9c0	fix(audit-g5c): set CURATOR_BOOTSTRAP_NEW_KEY=1 in conftest ... CredentialCrypto's safety check fires on create_app() instantiation because the test environment has no pre-seeded Fernet key file. Set the bootstrap env var before any test imports so the auto-create path is allowed during tests.	2026-06-02 17:55:25 -04:00
bvandeusen	f05aaa707b	fix(audit-g5d): surface ErrorType taxonomy on FailingSourcesCard ... Alembic 0032 adds Source.error_type (varchar(32), indexed). _update_source_health stamps it alongside last_error on status='error' and clears it on 'ok'. SourceRecord/to_dict exposes it. FailingSourcesCard renders a colored chip next to the consecutive- failures count, with a tooltip explaining the suggested operator action. Color reflects intent: - warning (yellow) — operator action needed (auth_error) - info (blue) — backend-paced (rate_limited / timeout / network_error / partial / tier_limited) - error (red) — likely terminal without intervention (not_found / access_denied / validation_failed / unsupported_url / http_error / unknown_error) Audit 2026-06-02: the backend computed 13 ErrorType categories but only the free-text last_error reached the operator. Bulk-triage by class ("all auth_error → rotate cookies", "12 rate_limited → just wait") required opening Logs per row.	2026-06-02 17:54:44 -04:00
bvandeusen	4df98171ab	fix(audit-g5c): refuse silent Fernet key regeneration on partial restore ... Audit 2026-06-02: `_load_or_create_key` silently minted a new Fernet key whenever the key file was missing — no log, no warning. The failure mode the audit flagged: a partial disaster restore where the DB was restored but `/images/secrets/` was lost would produce a working-looking system in which every authenticated download fails AUTH_ERROR until the operator re-uploads every credential by hand. Two opt-ins now needed for auto-creation: 1. Explicit `bootstrap_ok=True` kwarg (tests, scripts), OR 2. `CURATOR_BOOTSTRAP_NEW_KEY=1` env var (operator first-time setup) Otherwise the constructor raises `MissingCredentialKey` so the app fails fast at startup and the operator can restore the key file from backup before encrypted_blob rows go undecryptable. Also: docstring path was wrong (said "images/data root" but actual location is `/images/secrets/credential_key.b64`) — corrected. Tests updated to pass `bootstrap_ok=True` explicitly, and two new tests cover the safety behavior (missing-key-raises, env-var-bootstraps).	2026-06-02 17:04:27 -04:00
bvandeusen	8d75ade1d5	fix(audit-g5b): race-poisoning in three find-or-create sites ... Audit 2026-06-02 flagged three SELECT-then-INSERT sites that lost the race under concurrent writers / recovery-sweep replays, poisoning the outer transaction with an unrecoverable IntegrityError and crashing the calling task. Same shape as the banked 2026-05-26 ImageProvenance incident (see reference_scalar_one_or_none_duplicates memory). Mirrors the importer._get_or_create pattern in all three: savepoint via begin_nested + IntegrityError rollback to that savepoint + re- SELECT to grab the row the other worker committed first. - importer._capture_attachment: PostAttachment.sha256 UNIQUE. attachments.store is sha-addressed so both workers race to write the same on-disk path (shutil.copy2 + rename is idempotent), so no extra cleanup needed. - TagService.find_or_create: partial uniqueness index on (name, kind, COALESCE(fandom_id, -1)). The previous docstring claimed "INSERT ... ON CONFLICT" but the implementation was SELECT-then-INSERT with no recovery. - ArtistService.find_or_create: already had IntegrityError handling but did session.rollback() (unwinds the WHOLE transaction); now uses begin_nested + sp.rollback() so the surrounding request's progress isn't lost.	2026-06-02 17:02:12 -04:00
bvandeusen	75c63e1511	fix(audit-g5a): ruff isort — platforms after patreon_resolver	2026-06-02 16:55:56 -04:00
bvandeusen	98673d4dca	fix(audit-g5a): small architectural cleanups bundle ... Five small G5 findings from the 2026-06-02 audit. Each is local and follows an established FC pattern. - download_service: replace hardcoded ('discord','pixiv') tuple with auth_type_for(platform) == 'token'. A 7th token-platform now picks up the right credential path without touching this site. - /api/tags/<source_id>/merge enqueues recompute_centroid.delay after merge so the target's centroid reflects its new image set immediately. Daily list_drifted catches it within 24h, but eager recompute closes the suggestion-quality dip in the meantime. - backfill_thumbnails added to beat_schedule (daily). The task 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. - modal.createAndAdd pushes a kind='fandom' tag into tagsStore.fandomCache so FandomPicker sees the new fandom on next open. Was: cache-gated load (length===0) skipped refetch, new fandom invisible until full page reload. - cleanup cluster: - Drop .webp from cleanup_service.unlink — thumbnailer only writes .jpg/.png; the third tuple member was dead code. - Drop effective_date from /api/gallery/scroll response — no FE consumer reads it. Service still computes the attribute for timeline ordering; this just trims the JSON. - Rename store.recentMinute → store.recentRuns across the systemActivity store + three consumers (SystemActivitySummary, QueuesTable, SystemActivityTab). The data is the last 200 runs (not actually "last minute"), so the name lied. NOT in this bundle: the duplicate tag-merge endpoint (/api/tags vs /api/admin/tags) is harder — has 1 FE caller and 3 tests on the admin variant; consolidation is its own change.	2026-06-02 16:46:46 -04:00