fix(secrets-cli): KEK doctor — fresh-row verdicts, degraded rewrap guard test, vanished-row visibility

2026-07-19 09:33:53 -04:00
parent e60f2d5dad
commit 4576f73c4c
2 changed files with 63 additions and 24 deletions
@@ -34,7 +34,12 @@ public sealed record KekDiagnosis(string SecretName, RowKekStatus Status, string
 /// </summary>
 /// <param name="SessionKekId">The <c>kek_id</c> of the KEK the session is currently using.</param>
 /// <param name="Rows">The per-row verdicts, ordered by secret name.</param>
-public sealed record KekDoctorReport(string SessionKekId, IReadOnlyList<KekDiagnosis> Rows)
+/// <param name="Total">
+/// The number of rows scanned from the store (including tombstones). A value greater than
+/// <see cref="Rows"/> count means some rows vanished between the metadata list and the per-row fetch
+/// (a concurrent hard-absence) and were dropped — the discrepancy makes that visible rather than silent.
+/// </param>
+public sealed record KekDoctorReport(string SessionKekId, IReadOnlyList<KekDiagnosis> Rows, int Total)
 {
    /// <summary>Whether every diagnosed row opens cleanly under the session KEK.</summary>
    public bool Healthy => Rows.All(r => r.Status == RowKekStatus.Ok);
@@ -92,37 +97,42 @@ public sealed class KekDoctor
        {
            ct.ThrowIfCancellationRequested();

-            // A row under a different KEK cannot be opened by this session — report it (with its own
-            // kek_id so the operator knows which key to hunt) WITHOUT attempting a doomed decrypt.
-            if (!string.Equals(meta.KekId, sessionKekId, StringComparison.Ordinal))
-            {
-                rows.Add(new KekDiagnosis(meta.Name.Value, RowKekStatus.WrongKek, meta.KekId));
-                continue;
-            }
-
+            // Fetch the FRESH row and classify from ITS kek_id — never the (possibly stale) list
+            // metadata. A concurrent re-wrap between the list and this fetch would otherwise mislabel a
+            // moved row (a benign WrongKek row read as Corrupt, or a re-homed row read as WrongKek).
            StoredSecret? row = await session.Store.GetAsync(meta.Name, ct).ConfigureAwait(false);
            if (row is null)
            {
-                // Vanished between the list and the fetch (a concurrent hard-absence) — nothing to probe.
+                // Vanished between the list and the fetch (a concurrent hard-absence). Not added to
+                // rows; the Total-vs-Rows.Count gap in the report keeps the drop visible.
                continue;
            }

-            RowKekStatus status;
-            try
-            {
-                // The kek_id matches: probe the actual unwrap/decrypt. Plaintext is discarded at once.
-                _ = session.Cipher.Decrypt(row);
-                status = RowKekStatus.Ok;
-            }
-            catch (SecretDecryptionException)
-            {
-                status = RowKekStatus.Corrupt;
-            }
-
-            rows.Add(new KekDiagnosis(meta.Name.Value, status, row.KekId));
+            rows.Add(Classify(row, sessionKekId, session.Cipher));
        }

-        return new KekDoctorReport(sessionKekId, rows);
+        return new KekDoctorReport(sessionKekId, rows, all.Count);
+    }
+
+    // Classifies a single FRESH row against the session KEK. A row under a different kek_id is
+    // WrongKek (reported with its own id so the operator knows which key to hunt) and is NOT decrypted;
+    // a matching row is decrypt-probed (plaintext discarded at once) → Ok, or Corrupt if it fails closed.
+    private static KekDiagnosis Classify(StoredSecret row, string sessionKekId, ISecretCipher cipher)
+    {
+        if (!string.Equals(row.KekId, sessionKekId, StringComparison.Ordinal))
+        {
+            return new KekDiagnosis(row.Name.Value, RowKekStatus.WrongKek, row.KekId);
+        }
+
+        try
+        {
+            _ = cipher.Decrypt(row);
+            return new KekDiagnosis(row.Name.Value, RowKekStatus.Ok, row.KekId);
+        }
+        catch (SecretDecryptionException)
+        {
+            return new KekDiagnosis(row.Name.Value, RowKekStatus.Corrupt, row.KekId);
+        }
    }

    /// <summary>
@@ -86,11 +86,31 @@ public sealed class KekDoctorTests : IAsyncLifetime, IDisposable
        Assert.True(report.Healthy);
        Assert.Equal(_kekA.KekId, report.SessionKekId);
        Assert.Equal(2, report.Rows.Count);
+        Assert.Equal(2, report.Total); // no rows vanished → Total equals Rows.Count.
        Assert.All(report.Rows, r => Assert.Equal(RowKekStatus.Ok, r.Status));
        // Rows ordered by name: "ldap/b" precedes "sql/a".
        Assert.Equal(["ldap/b", "sql/a"], report.Rows.Select(r => r.SecretName));
    }

+    [Fact]
+    public async Task Diagnose_verdict_reads_fresh_row_kek_not_stale_metadata()
+    {
+        // Seed under A; a first diagnosis on session A reports Ok.
+        await SeedAsync("sql/a", "value-a", _kekA);
+        KekDoctorReport first = await new KekDoctor().DiagnoseAsync(Session(_kekA), CancellationToken.None);
+        Assert.Equal(RowKekStatus.Ok, Assert.Single(first.Rows).Status);
+
+        // Re-seal the SAME row under B (simulates a concurrent re-wrap/re-set moving its KEK). A second
+        // diagnosis on session A must classify from the FRESH row.KekId (B) → WrongKek surfacing B's id,
+        // never a false Corrupt from probing a row the session cannot open.
+        await SeedAsync("sql/a", "value-a", _kekB);
+        KekDoctorReport second = await new KekDoctor().DiagnoseAsync(Session(_kekA), CancellationToken.None);
+
+        KekDiagnosis row = Assert.Single(second.Rows);
+        Assert.Equal(RowKekStatus.WrongKek, row.Status);
+        Assert.Equal(_kekB.KekId, row.RowKekId);
+    }
+
    [Fact]
    public async Task Diagnose_reports_wrong_kek_rows_with_their_kekid()
    {
@@ -149,6 +169,15 @@ public sealed class KekDoctorTests : IAsyncLifetime, IDisposable
        Assert.Contains("key", ex.Message, StringComparison.OrdinalIgnoreCase);
    }

+    [Fact]
+    public async Task RewrapAll_throws_on_degraded_session()
+    {
+        InvalidOperationException ex = await Assert.ThrowsAsync<InvalidOperationException>(
+            () => new KekDoctor().RewrapAllAsync(DegradedSession(), _kekA, CancellationToken.None));
+
+        Assert.Contains("key", ex.Message, StringComparison.OrdinalIgnoreCase);
+    }
+
    [Fact]
    public async Task RewrapAll_moves_wrong_kek_rows_onto_session_kek()
    {