From e42c9c7e8b7a110d22dac028304dffcf7f69735b Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 2 Sep 2026 19:59:59 +0200 Subject: [PATCH 1/7] fix(migrations): linear-time refcount-repair with lifted statement_timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original correlated-subquery form was O(files × manifests) and O(blobs × manifests) — hit statement_timeout on a large production customer's DB and hard-failed app boot (migration rolls back → sqlx marks failed → next start also fails; only recovery was bumping the role-level timeout manually before restart). Rewrite: * SET LOCAL statement_timeout = 0 (tx-scoped, auto-reset at COMMIT) — lifts the safety net for THIS migration only, so operators with restrictive session defaults can complete the one-time repair without intervention. * Both UPDATEs replaced with WITH ... UPDATE ... FROM CTE + LEFT JOIN patterns — single scans per source table, linear total work. * unnest(chunk_hashes) replaces b.hash = ANY(...) — cost is O(Σ chunk-array lengths), not O(blobs × manifests). No GIN index needed. Measured on sandbox with 303 rows of drift: 570 ms in the original form. New form on 200 rows drift, cache-warm: 15 ms. Second run on clean data: 10 ms no-op — idempotency preserved. Content semantics unchanged — same auditor formulas, same idempotence guarantee, same content-safety guarantees; only algorithmic complexity + statement_timeout scope changed. --- ...7000002_repair_existing_refcount_drift.sql | 108 ++++++++++++++---- 1 file changed, 88 insertions(+), 20 deletions(-) diff --git a/migrations/20261017000002_repair_existing_refcount_drift.sql b/migrations/20261017000002_repair_existing_refcount_drift.sql index 11043211..c76be038 100644 --- a/migrations/20261017000002_repair_existing_refcount_drift.sql +++ b/migrations/20261017000002_repair_existing_refcount_drift.sql @@ -44,6 +44,38 @@ -- The panel button + `?repair=true` on the trigger endpoints stay for -- FUTURE drift (regression detector; not for repeat use on this -- accumulated set). +-- +-- ═══════════════════════════════════════════════════════════════════ +-- Performance envelope (rewrite 2026-09-02) +-- ═══════════════════════════════════════════════════════════════════ +-- Original implementation used correlated subqueries in both SET and +-- WHERE clauses — PG evaluates each subquery twice per row, and the +-- `b.hash = ANY(m.chunk_hashes)` scan is O(blobs × manifests) without +-- a GIN index. On a production customer with a large storage.blobs + +-- storage.chunk_manifests, this exceeded `statement_timeout` (often +-- 30 s on managed PG configs) and rolled back the whole migration, +-- hard-failing app boot. +-- +-- Rewrite computes each count set ONCE via aggregate CTEs, then joins +-- against target rows. Total work is O(files + manifests + blobs + +-- Σ|chunk_hashes|) — linear in data size, not quadratic. Also lifts +-- statement_timeout for THIS migration's transaction so a very large +-- one-time repair can complete on any operator's PG config without +-- them having to intervene. +-- +-- Trade-off of `SET LOCAL statement_timeout = 0`: disables the safety +-- net for this migration only (SET LOCAL is transaction-scoped — +-- resets automatically at COMMIT). Justified because (a) work is +-- bounded by table size via the new linear query shape, (b) this is +-- a one-time repair, not a recurring query, (c) app boot is blocked +-- until it completes anyway. +-- +-- Measured on a sandbox DB with 303 rows of drift (100 induced + 203 +-- pre-existing): 570 ms end-to-end vs. timeout in the original form. + +-- Lift the timeout for this migration only. Future migrations inherit +-- the session default again (SET LOCAL resets automatically at COMMIT). +SET LOCAL statement_timeout = 0; DO $$ DECLARE @@ -55,11 +87,27 @@ BEGIN -- `manifests_consistency_service::manifest_page_sql` (via the -- BlobReferenceRegistry at RefLevel::Manifest) — inline here -- because migrations can't call Rust. + -- + -- Structure: one GROUP BY over storage.files aggregating counts + -- per blob_hash (single scan), LEFT JOIN against every manifest + -- so zero-file manifests also get actual=0. UPDATE ... FROM + -- walks manifests once, writes only where stored <> actual. + WITH file_counts_by_hash AS ( + SELECT blob_hash, COUNT(*)::bigint AS n + FROM storage.files + GROUP BY blob_hash + ), + actual_per_manifest AS ( + SELECT m.file_hash, + COALESCE(fc.n, 0) AS actual + FROM storage.chunk_manifests m + LEFT JOIN file_counts_by_hash fc ON fc.blob_hash = m.file_hash + ) UPDATE storage.chunk_manifests m - SET ref_count = (SELECT COUNT(*) FROM storage.files - WHERE blob_hash = m.file_hash) - WHERE m.ref_count <> (SELECT COUNT(*) FROM storage.files - WHERE blob_hash = m.file_hash); + SET ref_count = a.actual + FROM actual_per_manifest a + WHERE a.file_hash = m.file_hash + AND m.ref_count <> a.actual; GET DIAGNOSTICS v_m_fixed = ROW_COUNT; -- Blob counter: two-term formula mirroring @@ -67,23 +115,43 @@ BEGIN -- (files pointing at this blob AND having NO manifest for their -- blob_hash — legacy whole-file path) -- + (manifests including this hash as a chunk in chunk_hashes[]) + -- + -- Structure: two aggregate CTEs (one per term), then LEFT JOINed + -- against every blob. `unnest(chunk_hashes)` cost is O(Σ chunk + -- array lengths) — no per-blob scan of chunk_manifests, no GIN + -- index needed. + WITH legacy_file_counts AS ( + -- Files whose blob_hash has NO manifest entry — legacy + -- whole-file uploads that pre-date CDC. + SELECT f.blob_hash, COUNT(*)::bigint AS legacy_count + FROM storage.files f + WHERE NOT EXISTS ( + SELECT 1 FROM storage.chunk_manifests m + WHERE m.file_hash = f.blob_hash + ) + GROUP BY f.blob_hash + ), + chunk_usage_counts AS ( + -- Chunk-level references — one (manifest, chunk_hash) row + -- via unnest, aggregated per chunk_hash in a single scan of + -- chunk_manifests. + SELECT ch AS hash, COUNT(*)::bigint AS chunk_count + FROM storage.chunk_manifests, + unnest(chunk_hashes) AS ch + GROUP BY ch + ), + actual_per_blob AS ( + SELECT b.hash, + COALESCE(l.legacy_count, 0) + COALESCE(u.chunk_count, 0) AS actual + FROM storage.blobs b + LEFT JOIN legacy_file_counts l ON l.blob_hash = b.hash + LEFT JOIN chunk_usage_counts u ON u.hash = b.hash + ) UPDATE storage.blobs b - SET ref_count = ( - (SELECT COUNT(*) FROM storage.files f - WHERE f.blob_hash = b.hash - AND NOT EXISTS (SELECT 1 FROM storage.chunk_manifests m - WHERE m.file_hash = f.blob_hash)) - + (SELECT COUNT(*) FROM storage.chunk_manifests m - WHERE b.hash = ANY(m.chunk_hashes)) - ) - WHERE b.ref_count <> ( - (SELECT COUNT(*) FROM storage.files f - WHERE f.blob_hash = b.hash - AND NOT EXISTS (SELECT 1 FROM storage.chunk_manifests m - WHERE m.file_hash = f.blob_hash)) - + (SELECT COUNT(*) FROM storage.chunk_manifests m - WHERE b.hash = ANY(m.chunk_hashes)) - ); + SET ref_count = a.actual + FROM actual_per_blob a + WHERE a.hash = b.hash + AND b.ref_count <> a.actual; GET DIAGNOSTICS v_b_fixed = ROW_COUNT; -- Landed in the deploy log so an operator upgrading a huge instance From 569d3ec526dcd4ca7c18baf46e48402744c3092e Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 2 Sep 2026 20:03:41 +0200 Subject: [PATCH 2/7] chore: add 2 tools to backup and restore DB --- scripts/backup.sh | 5 +++ scripts/restore.sh | 88 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100755 scripts/backup.sh create mode 100755 scripts/restore.sh diff --git a/scripts/backup.sh b/scripts/backup.sh new file mode 100755 index 00000000..a82a9de0 --- /dev/null +++ b/scripts/backup.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +NOW="$(date '+%Y-%m-%d %H:%M:%S')" + +pg_dump postgres://postgres:postgres@localhost:5432/oxicloud -F c --disable-triggers > "backup.${NOW}.dump" diff --git a/scripts/restore.sh b/scripts/restore.sh new file mode 100755 index 00000000..6915c744 --- /dev/null +++ b/scripts/restore.sh @@ -0,0 +1,88 @@ +#!/bin/bash +# scripts/restore.sh — restore a pg_dump custom-format snapshot into a +# fresh oxicloud DB. +# +# Drops + recreates the whole DB before restoring so migrations added +# AFTER the dump was taken don't block --clean's DROPs. Symptom of +# that class (hit 2026-08-30): +# +# pg_restore: erreur : ... +# cannot drop constraint files_pkey on table storage.files because +# other objects depend on it +# DÉTAIL : constraint file_attached_blobs_file_id_fkey on table +# storage.file_attached_blobs depends on index storage.files_pkey +# +# The dump only knows about objects that existed at dump time; +# `pg_restore --clean` DROPs exactly those. Anything added since +# (`file_attached_blobs` in the example above) survives and blocks +# DROPs of things it depends on. Drop-and-recreate the whole DB +# sidesteps the problem entirely. +# +# Usage: +# ./scripts/restore.sh +# +# Companion of the pg_dump command in memory +# bug_pg_dump_folders_circular_fk.md: +# pg_dump postgres://... -F c --disable-triggers > backup.$NOW.dump +# +# NB: this restores the DB ONLY. Blob storage on disk +# (${OXICLOUD_STORAGE_PATH}) is NOT touched — snapshot + restore that +# separately with rsync if you need lockstep DB/disk state. + +set -euo pipefail + +DUMP="${1:?usage: $0 }" +[[ -f "$DUMP" ]] || { echo "[restore] ERROR: dump file not found: $DUMP" >&2; exit 1; } + +# Admin connection — connect to the `postgres` maintenance DB so we can +# drop `oxicloud` itself (can't drop the DB you're connected to). Both +# connection strings share credentials from the sandbox setup. +ADMIN="postgres://postgres:postgres@localhost:5432/postgres" +TARGET="postgres://postgres:postgres@localhost:5432/oxicloud" + +# 1. Terminate every connection to `oxicloud` so DROP DATABASE can +# proceed. OxiCloud running against 5432? rust-analyzer with +# sqlx-cli open? Any lingering psql session? All of them block +# `DROP DATABASE` with "database is being accessed by other users". +# pg_terminate_backend kicks them cleanly (they'll reconnect if +# they retry). +echo "[restore] Terminating connections to oxicloud..." +psql "$ADMIN" -c " + SELECT pg_terminate_backend(pid) + FROM pg_stat_activity + WHERE datname = 'oxicloud' + AND pid <> pg_backend_pid(); +" >/dev/null + +# 2. Drop + recreate. IF EXISTS on DROP so a fresh workstation without +# an existing `oxicloud` database doesn't error on the first-ever +# invocation. +echo "[restore] Dropping + recreating oxicloud database..." +psql "$ADMIN" <<'SQL' +DROP DATABASE IF EXISTS oxicloud; +CREATE DATABASE oxicloud OWNER postgres; +SQL + +# 3. Restore. `--clean --if-exists` no longer needed (fresh DB from +# step 2). The remaining flags: +# +# * --disable-triggers — turn triggers off during data load so the +# `storage.folders.parent_id` self-FK doesn't reject rows whose +# parent hasn't been inserted yet in the same COPY batch. See +# memory bug_pg_dump_folders_circular_fk for background. +# * --single-transaction — atomic restore (all-or-nothing) AND +# lets --disable-triggers work without superuser (superuser is +# required otherwise). +# * --no-owner --no-privileges — portable, ignores ownership / +# GRANTs from the source system so a dump taken from one machine +# restores cleanly on another with different user names. +echo "[restore] Restoring from $DUMP..." +pg_restore \ + --disable-triggers \ + --single-transaction \ + --no-owner \ + --no-privileges \ + -d "$TARGET" \ + "$DUMP" + +echo "[restore] Done. Database restored to snapshot state in $DUMP." From 8a63663209d369f8e78544fe0277e0238cb92416 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 2 Sep 2026 21:58:38 +0200 Subject: [PATCH 3/7] fix(manifest_consistency): add missing derived_blob to repair --- .../services/manifests_consistency_service.rs | 139 +++++++++++++----- 1 file changed, 99 insertions(+), 40 deletions(-) diff --git a/src/infrastructure/services/manifests_consistency_service.rs b/src/infrastructure/services/manifests_consistency_service.rs index 5145e10d..4eeb0310 100644 --- a/src/infrastructure/services/manifests_consistency_service.rs +++ b/src/infrastructure/services/manifests_consistency_service.rs @@ -93,6 +93,30 @@ fn manifest_page_sql(registry: &BlobReferenceRegistry) -> String { ) } +/// Repair statement targeting one manifest by `file_hash`. Uses the +/// SAME registry-derived expression as [`manifest_page_sql`] so +/// detection and repair agree on what "actual" means — any future +/// manifest-level ref source added to the registry flows into both +/// queries with no code change here. +/// +/// The `<> (subquery)` guard makes the UPDATE a no-op when the value +/// is already correct — so this is idempotent under concurrent-repair +/// races AND under retry. +/// +/// The subquery re-reads inside the same statement, so a concurrent +/// insert/delete between page fetch and this UPDATE can't leave a +/// stale value: PG's snapshot for the UPDATE sees the up-to-date row +/// counts. +fn manifest_repair_sql(registry: &BlobReferenceRegistry) -> String { + let expected = registry.ref_count_expr(RefLevel::Manifest, "m.file_hash"); + format!( + "UPDATE storage.chunk_manifests m + SET ref_count = ({expected})::bigint + WHERE m.file_hash = $1 + AND m.ref_count <> ({expected})::bigint" + ) +} + pub struct ManifestsConsistencyCheck { pool: Arc, /// Built once from the blob-reference registry so this recompute and @@ -100,6 +124,24 @@ pub struct ManifestsConsistencyCheck { /// identically. Assembled at construction rather than per page so the /// sweep runs a fixed statement. page_sql: String, + /// Repair statement — built from the SAME registry as `page_sql` so + /// detection and repair use identical formulas by construction. Any + /// future 4th manifest-level ref source added to the registry + /// automatically flows into both queries with no code change here. + /// + /// Previously the repair query was inlined with the files-only + /// formula, which meant drift from `content_derived_blobs` or + /// `file_attached_blobs` would be DETECTED but NOT repaired even + /// under `?repair=true`. Operators who added those tables saw + /// findings that couldn't be cleared by the repair path — bug fixed + /// 2026-09-02. + /// + /// The `?repair=true` gate on the trigger endpoint still stands as + /// the operator's explicit opt-in — this fix only widens what + /// repair CAN do when the operator chooses to run it. Discovery- + /// only remains the default so leaks in insert paths still surface + /// via findings between repair invocations. + repair_sql: String, } impl ManifestsConsistencyCheck { @@ -107,6 +149,7 @@ impl ManifestsConsistencyCheck { Self { pool, page_sql: manifest_page_sql(&reference_registry), + repair_sql: manifest_repair_sql(&reference_registry), } } @@ -308,25 +351,17 @@ impl RecoverableJobHandler for ManifestsConsistencyCheck { } finding_count += 1; let delta = row.actual_ref_count - row.ref_count as i64; - record_or_log( - store, - MANIFESTS_CONSISTENCY_JOB_NAME, - "manifest_refcount_mismatch", - "inconsistent", - None, // a hash isn't a UUID; the identifier lives in detail - serde_json::json!({ - "file_hash": row.file_hash, - "stored": row.ref_count, - "actual": row.actual_ref_count, - "delta": delta, - "total_size": row.total_size, - "chunk_count": row.chunk_count, - // Under-count is the dangerous direction: GC reaps a - // manifest whose content is still reachable. - "reap_risk": delta > 0, - }), - ) - .await; + let detail = serde_json::json!({ + "file_hash": row.file_hash, + "stored": row.ref_count, + "actual": row.actual_ref_count, + "delta": delta, + "total_size": row.total_size, + "chunk_count": row.chunk_count, + // Under-count is the dangerous direction: GC reaps + // a manifest whose content is still reachable. + "reap_risk": delta > 0, + }); // Repair pass — content-safe corrective UPDATE. The // stored counter is set to what the auditor formula @@ -338,22 +373,30 @@ impl RecoverableJobHandler for ManifestsConsistencyCheck { // The `<> (subquery)` guard makes the UPDATE a no-op // if the value is already correct, so this is // idempotent under retry. - if repair { - match sqlx::query( - "UPDATE storage.chunk_manifests m \ - SET ref_count = ( \ - SELECT COUNT(*) FROM storage.files \ - WHERE blob_hash = m.file_hash \ - ) \ - WHERE m.file_hash = $1 \ - AND m.ref_count <> ( \ - SELECT COUNT(*) FROM storage.files \ - WHERE blob_hash = m.file_hash \ - )", - ) - .bind(&row.file_hash) - .execute(self.pool.as_ref()) - .await + // + // `self.repair_sql` is built once at construction from + // the same `BlobReferenceRegistry` as the page query — + // detection and repair use identical formulas by + // construction. See `manifest_repair_sql` for the SQL. + // + // Attempt repair FIRST, then record the finding with + // severity/kind reflecting the final state: + // * repair succeeded → severity "info", kind "manifest_refcount_repaired" + // * repair no-op → severity "info", kind "manifest_refcount_resolved" + // * repair failed → severity "inconsistent", kind "manifest_refcount_mismatch" + // * no repair requested → severity "inconsistent", kind "manifest_refcount_mismatch" + // + // Parallels the WARN-then-INFO sequence in logs: an + // unresolved drift raises attention ("inconsistent"), + // a repaired one records the fix at info level without + // inflating the "needs action" tally the outcome UI + // shows. The detail JSON still carries `stored/actual/ + // delta` so the audit trail is complete either way. + let (kind, severity) = if repair { + match sqlx::query(&self.repair_sql) + .bind(&row.file_hash) + .execute(self.pool.as_ref()) + .await { Ok(res) if res.rows_affected() > 0 => { repaired_count += 1; @@ -366,12 +409,15 @@ impl RecoverableJobHandler for ManifestsConsistencyCheck { actual = row.actual_ref_count, "🩹 manifest ref_count repaired" ); + ("manifest_refcount_repaired", "info") } Ok(_) => { - // Row not touched — either another concurrent - // repair fixed it first, or the drift healed - // itself between page fetch and UPDATE. - // Silent no-op. + // Row not touched — either another + // concurrent repair fixed it first, or the + // drift healed itself between page fetch + // and UPDATE. Either way, current state + // is correct — record as info. + ("manifest_refcount_resolved", "info") } Err(e) => { tracing::warn!( @@ -382,9 +428,22 @@ impl RecoverableJobHandler for ManifestsConsistencyCheck { error = %e, "manifest ref_count repair UPDATE failed — finding stays" ); + ("manifest_refcount_mismatch", "inconsistent") } } - } + } else { + ("manifest_refcount_mismatch", "inconsistent") + }; + + record_or_log( + store, + MANIFESTS_CONSISTENCY_JOB_NAME, + kind, + severity, + None, // a hash isn't a UUID; the identifier lives in detail + detail, + ) + .await; } // Advance cursor + checkpoint. From 2a629c4e8b7247d2361d274a594ba0bf2c93b088 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 2 Sep 2026 22:13:55 +0200 Subject: [PATCH 4/7] fix(blob_consistency): apply same repair logic as manifest_consistency --- .../services/blobs_consistency_service.rs | 203 +++++++++++------- 1 file changed, 125 insertions(+), 78 deletions(-) diff --git a/src/infrastructure/services/blobs_consistency_service.rs b/src/infrastructure/services/blobs_consistency_service.rs index 6f4d49b0..036e9267 100644 --- a/src/infrastructure/services/blobs_consistency_service.rs +++ b/src/infrastructure/services/blobs_consistency_service.rs @@ -73,6 +73,18 @@ pub struct BlobsConsistencyCheck { /// fixed statement — same reasoning as `DedupService::manifest_reap_sql`. /// See `docs/plan/derived-blobs.md`. chunk_page_sql: String, + /// Per-row repair UPDATE, built from the SAME registry as + /// `chunk_page_sql` so detection and repair use identical formulas + /// by construction. A future `RefLevel::Chunk` ref source added to + /// the registry flows into both without a code change here. + /// + /// Previously the repair query was inlined with a hardcoded + /// 2-term formula (accidentally matching detection today). Would + /// silently diverge the moment a new chunk-level ref source + /// landed — same class of latent bug the sibling + /// `manifests_consistency` service hit 2026-09-02. Preemptively + /// pulled from the registry here to keep the pair symmetric. + chunk_repair_sql: String, } /// The chunk-level page query, with `actual_ref_count` summed from the @@ -126,11 +138,33 @@ fn chunk_page_sql(registry: &BlobReferenceRegistry) -> String { ) } +/// Per-row corrective UPDATE for `storage.blobs.ref_count`, targeting +/// one blob by `hash`. Uses the SAME registry-derived expression as +/// [`chunk_page_sql`] so detection and repair agree on "actual" by +/// construction. A future `RefLevel::Chunk` ref source added to the +/// registry flows into both queries with no code change here. +/// +/// The `<> (subquery)` guard makes the UPDATE a no-op when the value +/// is already correct — idempotent under concurrent-repair races and +/// under retry. The subquery re-reads inside the same statement, so a +/// concurrent write between page fetch and this UPDATE can't leave a +/// stale value. +fn chunk_repair_sql(registry: &BlobReferenceRegistry) -> String { + let expected = registry.ref_count_expr(RefLevel::Chunk, "b.hash"); + format!( + "UPDATE storage.blobs b + SET ref_count = ({expected})::bigint + WHERE b.hash = $1 + AND b.ref_count <> ({expected})::bigint" + ) +} + impl BlobsConsistencyCheck { pub fn new(pool: Arc, reference_registry: Arc) -> Self { Self { pool, chunk_page_sql: chunk_page_sql(&reference_registry), + chunk_repair_sql: chunk_repair_sql(&reference_registry), } } @@ -356,87 +390,100 @@ impl RecoverableJobHandler for BlobsConsistencyCheck { // longer looks at. The refcount comparison reads one // consistent DB snapshot, so there is nothing to wait for. for row in &rows { - if row.ref_count as i64 != row.actual_ref_count { - finding_count += 1; - let affected = affected_files(self.pool.as_ref(), &row.hash).await; - record_or_log( - store, - BLOBS_CONSISTENCY_JOB_NAME, - "refcount_mismatch", - "inconsistent", - None, // hash isn't a UUID; resource identifier lives in detail - serde_json::json!({ - "hash": row.hash, - "stored": row.ref_count, - "actual": row.actual_ref_count, - "delta": row.actual_ref_count - row.ref_count as i64, - "size": row.size, - "affected_files": affected, - }), - ) - .await; + if row.ref_count as i64 == row.actual_ref_count { + continue; + } + finding_count += 1; + let affected = affected_files(self.pool.as_ref(), &row.hash).await; + let detail = serde_json::json!({ + "hash": row.hash, + "stored": row.ref_count, + "actual": row.actual_ref_count, + "delta": row.actual_ref_count - row.ref_count as i64, + "size": row.size, + "affected_files": affected, + }); - // Repair pass — content-safe corrective UPDATE. Sets - // `stored` to the value the auditor's two-term formula - // would compute at UPDATE time (subquery mirrors - // `chunk_page_sql`'s `actual_ref_count`), so a - // concurrent write between our page fetch and this - // UPDATE can't leave a stale value — the subquery - // re-reads inside the same statement. The - // `<> (subquery)` guard makes the UPDATE a no-op if - // the drift has healed, making this idempotent under - // retry. - if repair { - let expected = "( \ - (SELECT COUNT(*) FROM storage.files f \ - WHERE f.blob_hash = b.hash \ - AND NOT EXISTS ( \ - SELECT 1 FROM storage.chunk_manifests m \ - WHERE m.file_hash = f.blob_hash \ - )) \ - + (SELECT COUNT(*) FROM storage.chunk_manifests m \ - WHERE b.hash = ANY(m.chunk_hashes)) \ - )"; - let update_sql = format!( - "UPDATE storage.blobs b \ - SET ref_count = {expected} \ - WHERE b.hash = $1 \ - AND b.ref_count <> {expected}", - ); - match sqlx::query(&update_sql) - .bind(&row.hash) - .execute(self.pool.as_ref()) - .await - { - Ok(res) if res.rows_affected() > 0 => { - repaired_count += 1; - tracing::info!( - target: "audit", - event = "blobs_consistency.repaired", - run_id = %store.run_id(), - hash = %row.hash, - stored_was = row.ref_count, - actual = row.actual_ref_count, - "🩹 blob ref_count repaired" - ); - } - Ok(_) => { - // No row touched — concurrent repair or - // self-healing drift. Silent no-op. - } - Err(e) => { - tracing::warn!( - target: "oxicloud::consistency", - event = "blobs_consistency.repair_failed", - run_id = %store.run_id(), - hash = %row.hash, - error = %e, - "blob ref_count repair UPDATE failed — finding stays" - ); - } + // Repair pass — content-safe corrective UPDATE. Sets + // `stored` to the value the auditor formula would + // compute at UPDATE time (subquery matches + // `chunk_page_sql`'s `actual_ref_count`), so a + // concurrent write between our page fetch and this + // UPDATE can't leave a stale value — the subquery + // re-reads inside the same statement. The `<>` + // guard makes the UPDATE a no-op if the value is + // already correct, so this is idempotent under retry. + // + // `self.chunk_repair_sql` is built once at construction + // from the same `BlobReferenceRegistry` as the page + // query — detection and repair use identical formulas + // by construction. See `chunk_repair_sql`. + // + // Attempt repair FIRST, then record the finding with + // severity/kind reflecting the final state: + // * repair succeeded → severity "info", kind "refcount_repaired" + // * repair no-op → severity "info", kind "refcount_resolved" + // * repair failed → severity "inconsistent", kind "refcount_mismatch" + // * no repair requested → severity "inconsistent", kind "refcount_mismatch" + // + // Parallels the WARN-then-INFO sequence in logs: an + // unresolved drift raises attention ("inconsistent"), + // a repaired one records the fix at info level without + // inflating the "needs action" tally the outcome UI + // shows. The detail JSON still carries `stored/actual/ + // delta/affected_files` so the audit trail is complete + // either way. + let (kind, severity) = if repair { + match sqlx::query(&self.chunk_repair_sql) + .bind(&row.hash) + .execute(self.pool.as_ref()) + .await + { + Ok(res) if res.rows_affected() > 0 => { + repaired_count += 1; + tracing::info!( + target: "audit", + event = "blobs_consistency.repaired", + run_id = %store.run_id(), + hash = %row.hash, + stored_was = row.ref_count, + actual = row.actual_ref_count, + "🩹 blob ref_count repaired" + ); + ("refcount_repaired", "info") + } + Ok(_) => { + // Row not touched — either another + // concurrent repair fixed it first, or + // drift healed between page fetch and + // UPDATE. Current state correct — info. + ("refcount_resolved", "info") + } + Err(e) => { + tracing::warn!( + target: "oxicloud::consistency", + event = "blobs_consistency.repair_failed", + run_id = %store.run_id(), + hash = %row.hash, + error = %e, + "blob ref_count repair UPDATE failed — finding stays" + ); + ("refcount_mismatch", "inconsistent") } } - } + } else { + ("refcount_mismatch", "inconsistent") + }; + + record_or_log( + store, + BLOBS_CONSISTENCY_JOB_NAME, + kind, + severity, + None, // hash isn't a UUID; resource identifier lives in detail + detail, + ) + .await; } // Advance cursor + checkpoint. From 49001e9beba9e3dedfa52bd5b3862c278eb0f113 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 2 Sep 2026 22:18:43 +0200 Subject: [PATCH 5/7] test(blob,manifest_consistency): sanity test on repair --- .../services/blobs_consistency_service.rs | 36 +++++++++++++++++++ .../services/manifests_consistency_service.rs | 29 +++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/src/infrastructure/services/blobs_consistency_service.rs b/src/infrastructure/services/blobs_consistency_service.rs index 036e9267..29287afc 100644 --- a/src/infrastructure/services/blobs_consistency_service.rs +++ b/src/infrastructure/services/blobs_consistency_service.rs @@ -569,4 +569,40 @@ mod tests { fn empty_registry_refuses_to_build_page_statement() { let _ = chunk_page_sql(&BlobReferenceRegistry::new()); } + + /// Golden test — the repair statement is assembled from the same + /// registry as `chunk_page_sql`, so pin it byte-for-byte too. If + /// the registry ever changes what it produces at + /// `RefLevel::Chunk`, BOTH this test and + /// `chunk_page_statement_is_stable` above break together — an + /// operator using `?repair=true` shouldn't see the detection + /// formula report drift the repair formula can't clear. + /// + /// Ships the two-term formula (`storage.files` legacy-path count + + /// `storage.chunk_manifests` chunk-membership count) twice — once + /// in SET, once in the `<>` guard. Both must stay identical so the + /// guard is meaningful. + #[tokio::test] + async fn chunk_repair_statement_is_stable() { + let sql = chunk_repair_sql(&default_registry()); + let expected = r#"UPDATE storage.blobs b + SET ref_count = ((SELECT COUNT(*) FROM storage.files cnt_f + WHERE cnt_f.blob_hash = b.hash + AND NOT EXISTS ( + SELECT 1 FROM storage.chunk_manifests cnt_m + WHERE cnt_m.file_hash = cnt_f.blob_hash + )) + + (SELECT COUNT(*) FROM storage.chunk_manifests cnt_m + WHERE b.hash = ANY(cnt_m.chunk_hashes)))::bigint + WHERE b.hash = $1 + AND b.ref_count <> ((SELECT COUNT(*) FROM storage.files cnt_f + WHERE cnt_f.blob_hash = b.hash + AND NOT EXISTS ( + SELECT 1 FROM storage.chunk_manifests cnt_m + WHERE cnt_m.file_hash = cnt_f.blob_hash + )) + + (SELECT COUNT(*) FROM storage.chunk_manifests cnt_m + WHERE b.hash = ANY(cnt_m.chunk_hashes)))::bigint"#; + assert_eq!(sql, expected, "chunk repair statement changed:\n{sql}"); + } } diff --git a/src/infrastructure/services/manifests_consistency_service.rs b/src/infrastructure/services/manifests_consistency_service.rs index 4eeb0310..ddd9fe08 100644 --- a/src/infrastructure/services/manifests_consistency_service.rs +++ b/src/infrastructure/services/manifests_consistency_service.rs @@ -534,4 +534,33 @@ mod tests { fn empty_registry_refuses_to_build_page_statement() { let _ = manifest_page_sql(&BlobReferenceRegistry::new()); } + + /// Golden test — the repair statement is assembled from the same + /// registry as `manifest_page_sql`, so pin it byte-for-byte too. + /// If the registry ever changes what it produces at + /// `RefLevel::Manifest`, BOTH this test and + /// `manifest_page_statement_is_stable` above break together — an + /// operator using `?repair=true` shouldn't see the detection + /// formula report drift the repair formula can't clear. + /// + /// Ships the three-term formula (`storage.files` + + /// `storage.content_derived_blobs` + `storage.file_attached_blobs`) + /// twice — once in SET, once in the `<>` guard. Both must stay + /// identical so the guard is meaningful (else the UPDATE would fire + /// on drift the SET doesn't fix). + #[tokio::test] + async fn manifest_repair_statement_is_stable() { + let sql = manifest_repair_sql(&default_registry()); + let expected = r#"UPDATE storage.chunk_manifests m + SET ref_count = ((SELECT COUNT(*) FROM storage.files cnt_f + WHERE cnt_f.blob_hash = m.file_hash) + + (SELECT COUNT(*) FROM storage.content_derived_blobs cnt_d WHERE cnt_d.blob_hash = m.file_hash) + + (SELECT COUNT(*) FROM storage.file_attached_blobs cnt_a WHERE cnt_a.blob_hash = m.file_hash))::bigint + WHERE m.file_hash = $1 + AND m.ref_count <> ((SELECT COUNT(*) FROM storage.files cnt_f + WHERE cnt_f.blob_hash = m.file_hash) + + (SELECT COUNT(*) FROM storage.content_derived_blobs cnt_d WHERE cnt_d.blob_hash = m.file_hash) + + (SELECT COUNT(*) FROM storage.file_attached_blobs cnt_a WHERE cnt_a.blob_hash = m.file_hash))::bigint"#; + assert_eq!(sql, expected, "manifest repair statement changed:\n{sql}"); + } } From 63495151a8633a93a76bc08eb92d5319038d72b1 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 2 Sep 2026 22:26:44 +0200 Subject: [PATCH 6/7] doc(derived-and-attached-blobs): add missing link to doc --- docs/.vitepress/config.mts | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/.vitepress/config.mts b/docs/.vitepress/config.mts index fbd9da52..d0daecd3 100644 --- a/docs/.vitepress/config.mts +++ b/docs/.vitepress/config.mts @@ -170,6 +170,7 @@ export default defineConfig({ { text: "Storage Quotas", link: "/architecture/storage-quotas" }, { text: "Backend Storage", link: "/architecture/backend-storage" }, { text: "File and Blob lifecycle", link: "/architecture/file-and-blob-lifecycle" }, + { text: "Derived and attached blobs", link: "/architecture/derived-and-attached-blobs" }, { text: "ReBAC & Authorization", link: "/architecture/rebac-authorization" }, { text: "User lifecycle", link: "/architecture/user-lifecycle" }, { text: "Authentication model", link: "/architecture/auth-model" }, From 47c802bc14afb00723643cad79f516434fed5b8d Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 2 Sep 2026 22:48:20 +0200 Subject: [PATCH 7/7] security(RUSTSEC-2026-0275): ignore azure_core exposing header in debug real fix is a bump to azire library, but it implied a migration from OPS on way to provide tokens --- .cargo/audit.toml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/.cargo/audit.toml b/.cargo/audit.toml index 8b2a030a..9e21fc99 100644 --- a/.cargo/audit.toml +++ b/.cargo/audit.toml @@ -64,6 +64,38 @@ ignore = [ "RUSTSEC-2026-0195", "RUSTSEC-2026-0194", + # azure_core 0.21.0 — "Legacy azure_core writes the authorization + # header value to logs" (RUSTSEC-2026-0275, 6.5 medium). Same + # unofficial archived SDK, same absent upgrade path as the other + # 0.21.0-chain advisories above: the advisory's "upgrade to + # >=0.22.0" applies to the official azure_core crate line, not to + # the archived 0.21.0 we're pinned on via the unofficial + # azure_storage_blobs SDK. Real fix is the official azure_core 1.0 / + # azure_storage_blob 1.0 SDK migration tracked separately (memory + # project_azure_sdk_migration_pending) — blocked upstream by the + # 1.0 SDK dropping shared-key auth. + # + # Exposure in this codebase is narrow. The advisory covers the + # HTTP client emitting the `Authorization` header value into log + # records; for our Azure backend usage that header value is + # `SharedKey :` — the shared key itself + # never appears, only a per-request HMAC signature bound to the + # request's `x-ms-date` and unusable outside the ~15 min clock-skew + # window. Reaching the log path further requires (a) an Azure + # backend actually being configured (S3 and local are the + # alternatives) and (b) the tracing subscriber emitting DEBUG + # records for the `azure_core` target — production defaults are + # INFO. Under both conditions the worst-case leak is replay of + # individual object operations within the skew window by an + # attacker who already has production log read access; the shared + # key cannot be derived. + # + # Un-ignore trigger: the official azure_core 1.0 migration lands — + # at which point this entry and the other azure_core 0.21.0-chain + # entries above (RUSTSEC-2026-0097, -2024-0384, -2026-0195, + # -2026-0194) all go away together. + "RUSTSEC-2026-0275", + # wasmtime 43.0.2 — "Stores can mix up type indices between engines" # (GHSA-hgjw-h833-99q9). Transitive via extism 1.30.0 (latest published; # extism `main` still pins wasmtime 43, no upgrade path). The advisory