Audit fixes: batch recompute, negative-baseline percentages, key-ring persistence

Three defects found reviewing the last few commits.

Deriving consumption on ingest made the batch reading endpoint quadratic. A
recompute rewrites a meter's entire consumption series, and POST
/api/v1/readings ran one per reading -- 500 readings for one meter meant 500
full rewrites. IngestByMeterAsync takes renormalize:false and the endpoint
normalizes each touched meter once after the batch.

Percentage change divided by a possibly negative baseline. A net-export meter
going from -100 to -150 exported half again as much and would have been
reported as "+50%", reading as more consumption. A non-positive baseline now
reports no basis rather than a confident lie.

The data-protection key ring had no persistent home outside Docker Compose. The
LXC installer now creates /var/lib/metervault/keys at 0700 -- the app would
otherwise create it under the default umask, leaving a key ring world-readable
-- and the Unraid template maps it, since without that every UI-entered secret
was lost whenever the container was recreated. README documents the variable
and the trust boundary: keys on disk protect against leaked database content,
not against an attacker who already has the host.

Claude-Session: https://claude.ai/code/session_01V6joyergfvVLFEizH1hJLd

README.md

@@ -72,12 +72,21 @@ Configuration is via environment variables (`Section__Key` double-underscore map
 | `MeterVault__ReverseProxyTrust` | `true` to honour `X-Forwarded-User` behind an auth proxy |
 | `MeterVault__EnableLiveIngestion` | `false` to disable the MQTT/HA workers |
 | `MeterVault__SeedReferenceData` | `true` to load the bundled demo dataset on first start (idempotent) |
+| `MeterVault__DataProtectionKeyPath` | Where the key ring for UI-entered connector secrets lives (default `/var/lib/metervault/keys`) |
 
 The REST API is **closed by default**: with no `ApiKeys` configured and `AllowAnonymousApi` off, it
 returns 401. Set at least one API key (or open it explicitly for a trusted network).
 
-Secrets (broker/HA tokens) are **never** stored in the database — endpoint configs hold the *name*
-of an environment variable, resolved at runtime.
+Secrets (broker/HA tokens) are **never** stored in the database as plaintext. Each connector picks
+one of two forms: the *name* of an environment variable, resolved at runtime, or the secret typed
+into the admin UI and encrypted at rest under the data-protection key ring. Either way a `pg_dump`
+or JSON export carries nothing usable.
+
+Keep the key ring on persistent storage outside the app directory — the default
+`/var/lib/metervault/keys` survives an LXC update, and the Compose file mounts a named volume for it.
+Lose it and every UI-entered secret must be re-entered. The key ring is on disk, so this protects
+against leaked database content, not against an attacker who already has the host; that is the same
+trust boundary an environment variable has.
 
 ## Pushing readings (Home Assistant)
 

deploy/install/metervault-install.sh

@@ -23,6 +23,7 @@
 : "${INSTALL_DIR:=/opt/metervault}"
 : "${SOURCE_DIR:=/opt/metervault-src}"
 : "${ENV_FILE:=/etc/metervault/environment}"
+: "${KEYRING_DIR:=/var/lib/metervault/keys}"
 : "${DB_NAME:=metervault}"
 : "${DB_USER:=metervault}"
 
@@ -203,6 +204,13 @@ EOF
   chmod 600 "${ENV_FILE}"
 }
 
+# Key ring for connector secrets typed into the admin UI (SDD §6.4). The app creates this itself if
+# missing, but with the default umask — created here instead so it is 0700 from the start, and so it
+# is visibly outside /opt/metervault, which the updater republishes on every run.
+write_keyring_dir() {
+  install -d -m 0700 "${KEYRING_DIR}"
+}
+
 write_systemd() {
   cat <<'EOF' >/etc/systemd/system/metervault.service
 [Unit]
@@ -246,6 +254,7 @@ main() {
   install_dotnet_sdk
   build_metervault
   write_env
+  write_keyring_dir
   write_systemd
 
   systemctl daemon-reload 2>/dev/null || true

deploy/unraid-template.xml

@@ -23,4 +23,6 @@
   <Config Name="API key" Target="MeterVault__ApiKeys__0" Default="" Mode="" Description="API key for the REST API (X-Api-Key header). Leave blank to leave the API open." Type="Variable" Display="always" Required="false" Mask="true"/>
 
   <Config Name="Reverse-proxy trust" Target="MeterVault__ReverseProxyTrust" Default="false" Mode="" Description="Honour X-Forwarded-User from a trusted auth proxy" Type="Variable" Display="advanced" Required="false">false</Config>
+
+  <Config Name="Secret key ring" Target="/var/lib/metervault/keys" Default="/mnt/user/appdata/metervault/keys" Mode="rw" Description="Encryption keys for connector secrets entered in the web UI. Must persist: without this mapping every stored token is lost when the container is recreated." Type="Path" Display="always" Required="true">/mnt/user/appdata/metervault/keys</Config>
 </Container>

src/App/Api/ApiEndpoints.cs

@@ -36,17 +36,25 @@ public static class ApiEndpoints
             }
 
             int written = 0, updated = 0, rejected = 0, ignored = 0;
+            var touched = new HashSet<int>();
             foreach (var r in readings)
             {
-                switch (await ingestion.IngestByMeterAsync(r.MeterId, r.Time, r.Value, ct))
+                // Normalize once per meter after the batch, not per reading: a recompute rewrites the
+                // meter's whole consumption series, so doing it inside the loop is quadratic.
+                switch (await ingestion.IngestByMeterAsync(r.MeterId, r.Time, r.Value, renormalize: false, ct))
                 {
-                    case IngestionOutcome.Written: written++; break;
-                    case IngestionOutcome.Updated: updated++; break;
+                    case IngestionOutcome.Written: written++; touched.Add(r.MeterId); break;
+                    case IngestionOutcome.Updated: updated++; touched.Add(r.MeterId); break;
                     case IngestionOutcome.RejectedDecrease: rejected++; break;
                     default: ignored++; break; // unknown meter
                 }
             }
 
+            foreach (var meterId in touched)
+            {
+                await ingestion.RenormalizeMeterAsync(meterId, ct);
+            }
+
             return Results.Ok(new IngestResult(written, updated, rejected, ignored));
         }).WithSummary("Ingest one or more readings (idempotent). Lets Home Assistant push.");
 

src/Infrastructure/Dashboard/MeterDetailModels.cs

@@ -45,8 +45,13 @@ public sealed record MeterPeriodView(
 
     public bool HasHistory => Last12Months.Count > 0;
 
+    /// <summary>
+    /// Percentage change is only meaningful against a positive baseline. Dividing by a negative one
+    /// inverts the sign — a net-export meter going from −100 to −150 would report "+50% more used"
+    /// when it exported half as much again — so those report no basis rather than a confident lie.
+    /// </summary>
     private static double? Ratio(double current, double previous) =>
-        Math.Abs(previous) < 1e-9 ? null : (current - previous) / previous;
+        previous <= 1e-9 ? null : (current - previous) / previous;
 }
 
 /// <summary>A meter lifecycle/correction event row.</summary>

src/Infrastructure/Ingestion/IngestionService.cs

@@ -63,8 +63,14 @@ public sealed class IngestionService(
     }
 
     /// <summary>Ingests directly against a meter (REST push, e.g. Home Assistant POST /api/v1/readings).</summary>
+    /// <param name="renormalize">
+    /// False to skip deriving consumption, for callers ingesting a batch into one meter: recomputing
+    /// rewrites the meter's entire series, so doing it per reading is quadratic in batch size. Such a
+    /// caller must recompute the affected meters itself once the batch is in.
+    /// </param>
     public async Task<IngestionOutcome> IngestByMeterAsync(
-        int meterId, DateTimeOffset time, double value, CancellationToken cancellationToken = default)
+        int meterId, DateTimeOffset time, double value, bool renormalize = true,
+        CancellationToken cancellationToken = default)
     {
         var meter = await _db.Meters
             .FirstOrDefaultAsync(m => m.Id == meterId, cancellationToken).ConfigureAwait(false);
@@ -82,10 +88,24 @@ public sealed class IngestionService(
 
         var outcome = await UpsertAsync(meter, utc, value, sourceId: null, cancellationToken).ConfigureAwait(false);
         await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false);
+        if (renormalize)
+        {
             await RenormalizeAsync(meter.Id, outcome, cancellationToken).ConfigureAwait(false);
+        }
+
         return outcome;
     }
 
+    /// <summary>
+    /// Derives consumption for one meter after a batch of readings has been written. The public
+    /// counterpart to skipping <c>renormalize</c> on each individual ingest.
+    /// </summary>
+    public async Task RenormalizeMeterAsync(int meterId, CancellationToken cancellationToken = default)
+    {
+        await _normalization.RecomputeMeterAsync(meterId, batchId: null, cancellationToken).ConfigureAwait(false);
+        await _db.SaveChangesAsync(cancellationToken).ConfigureAwait(false);
+    }
+
     /// <summary>
     /// Derives consumption from the reading just written. Without this a live-ingested reading sits
     /// in <c>reading</c> forever and every derived figure — consumption, generation, cost — stays

tests/Integration.Tests/Ingestion/IngestionServiceTests.cs

@@ -174,6 +174,31 @@ public sealed class IngestionServiceTests(TimescaleFixture fx)
         await CleanupAsync(db, meterId);
     }
 
+    [Fact]
+    public async Task A_batch_can_defer_normalization_and_derive_the_same_series_once_at_the_end()
+    {
+        // Recomputing rewrites a meter's whole consumption series, so the batch endpoint skips it
+        // per reading and does it once. The result must be identical to normalizing as it goes.
+        await using var db = fx.CreateContext();
+        var (meterId, _) = await SetupAsync(db, MeterMode.CumulativeCounter);
+        var service = NewIngestion(db);
+
+        for (var hour = 0; hour < 5; hour++)
+        {
+            await service.IngestByMeterAsync(meterId, T0.AddHours(hour), 1000 + (hour * 10), renormalize: false);
+        }
+
+        Assert.False(await db.Consumption.AnyAsync(c => c.MeterId == meterId));
+
+        await service.RenormalizeMeterAsync(meterId);
+
+        var consumption = await db.Consumption.AsNoTracking().Where(c => c.MeterId == meterId).ToListAsync();
+        Assert.Equal(5, consumption.Count);
+        Assert.Equal(1040d, consumption.Sum(c => c.Amount), 3); // baseline 0 → 1000, then 4 × 10
+
+        await CleanupAsync(db, meterId);
+    }
+
     private static IngestionService NewIngestion(MeterVaultDbContext db) =>
         new(db, new MeterVault.Infrastructure.Normalization.NormalizationService(
             db, MeterVault.Core.Normalization.NormalizationEngine.CreateDefault()));

tests/Integration.Tests/MeterPeriodServiceTests.cs

@@ -77,6 +77,27 @@ public sealed class MeterPeriodServiceTests(TimescaleFixture fx)
         await CleanupAsync(db, meterId);
     }
 
+    [Fact]
+    public async Task A_negative_previous_period_reports_no_basis_rather_than_an_inverted_percentage()
+    {
+        // Net export: -100 -> -150 is half again as much exported, but dividing by a negative
+        // baseline would render it "+50%", which reads as more consumption.
+        await using var db = fx.CreateContext();
+        var meterId = await SetupAsync(db, MeterMode.CumulativeCounter);
+        var today = DateOnly.FromDateTime(DateTime.UtcNow);
+        var thisMonth = new DateOnly(today.Year, today.Month, 1);
+
+        await AddConsumptionAsync(db, meterId, ConsumptionKind.Consumption, thisMonth.AddDays(1), -150);
+        await AddConsumptionAsync(db, meterId, ConsumptionKind.Consumption, thisMonth.AddMonths(-1).AddDays(3), -100);
+
+        var view = await NewService().GetAsync(meterId);
+
+        Assert.NotNull(view);
+        Assert.Null(view!.MonthChange);
+
+        await CleanupAsync(db, meterId);
+    }
+
     private MeterPeriodService NewService()
     {
         var options = Microsoft.Extensions.Options.Options.Create(