fix(host): distinguish secret-store fault from missing PSK secret (review 2026-10-21 05/U2)
This commit is contained in:
@@ -114,10 +114,9 @@ public sealed class SitePskProvider : ISitePskProvider
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Reads the key from the secrets store, treating a store fault as "not found" rather than
|
/// Reads the key from the secrets store. A store fault (unreachable SQL Server, no master
|
||||||
/// letting it propagate. A host with no master key configured (the development rig) throws
|
/// key) surfaces as <see cref="SecretStoreUnavailableException"/>, distinct from a secret
|
||||||
/// from the resolver; that must produce the same clear "key not configured" error as a
|
/// that is simply missing (null return), and is never cached.
|
||||||
/// missing secret, not an opaque cryptographic one.
|
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private async Task<string?> ResolveFromStoreAsync(string secretName, string siteId, CancellationToken ct)
|
private async Task<string?> ResolveFromStoreAsync(string secretName, string siteId, CancellationToken ct)
|
||||||
{
|
{
|
||||||
@@ -131,10 +130,14 @@ public sealed class SitePskProvider : ISitePskProvider
|
|||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
_logger.LogDebug(ex,
|
// A store fault is NOT "secret missing": telling the operator to seed a secret that
|
||||||
"Secret store lookup for '{SecretName}' (site {SiteId}) faulted; treating as not found.",
|
// exists (while the shared store is merely unreachable) sends them the wrong way.
|
||||||
|
// Surface it as a distinct exception type and keep the cause visible at Warning.
|
||||||
|
_logger.LogWarning(ex,
|
||||||
|
"Secret store lookup for '{SecretName}' (site {SiteId}) faulted; the key could not be "
|
||||||
|
+ "read (this is a store fault, not a missing secret).",
|
||||||
secretName, siteId);
|
secretName, siteId);
|
||||||
return null;
|
throw new SecretStoreUnavailableException(secretName, siteId, ex);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -147,3 +150,29 @@ public sealed class SitePskProvider : ISitePskProvider
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The secrets store could not be read while resolving a site's gRPC preshared key — a store
|
||||||
|
/// fault, as opposed to the secret being absent (review 2026-10-21 05/U2).
|
||||||
|
/// </summary>
|
||||||
|
public sealed class SecretStoreUnavailableException : InvalidOperationException
|
||||||
|
{
|
||||||
|
/// <summary>Initializes the exception.</summary>
|
||||||
|
/// <param name="secretName">The secret that was being read.</param>
|
||||||
|
/// <param name="siteId">The site whose key was being resolved.</param>
|
||||||
|
/// <param name="inner">The underlying store fault.</param>
|
||||||
|
public SecretStoreUnavailableException(string secretName, string siteId, Exception inner)
|
||||||
|
: base($"The secret store could not be read while resolving gRPC preshared key '{secretName}' "
|
||||||
|
+ $"for site '{siteId}' ({inner.GetType().Name}: {inner.Message}); the secret may exist — "
|
||||||
|
+ "this is a store fault, not a missing secret.", inner)
|
||||||
|
{
|
||||||
|
SecretName = secretName;
|
||||||
|
SiteId = siteId;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Gets the secret that was being read.</summary>
|
||||||
|
public string SecretName { get; }
|
||||||
|
|
||||||
|
/// <summary>Gets the site whose key was being resolved.</summary>
|
||||||
|
public string SiteId { get; }
|
||||||
|
}
|
||||||
|
|||||||
@@ -122,21 +122,22 @@ public class SitePskProviderTests
|
|||||||
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Verifies that a faulting store surfaces as a key-not-configured error rather than a crypto error.</summary>
|
/// <summary>Verifies that a faulting store surfaces as a distinct store-unavailable error.</summary>
|
||||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task AFaultingStore_SurfacesAsKeyNotConfigured_NotAsACryptoError()
|
public async Task AFaultingStore_SurfacesAsStoreUnavailable_NotAsMissingSecret()
|
||||||
{
|
{
|
||||||
// A host with no master key throws from the resolver. The operator's problem is the
|
// Review 2026-10-21 05/U2: a store fault must not read as "secret missing".
|
||||||
// same either way — "this site has no key" — so the diagnosis must say that.
|
|
||||||
var resolver = Substitute.For<ISecretResolver>();
|
var resolver = Substitute.For<ISecretResolver>();
|
||||||
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
||||||
.Returns<string?>(_ => throw new InvalidOperationException("no master key configured"));
|
.Returns<string?>(_ => throw new InvalidOperationException("no master key configured"));
|
||||||
|
|
||||||
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
|
var ex = await Assert.ThrowsAsync<SecretStoreUnavailableException>(
|
||||||
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
||||||
|
|
||||||
Assert.Contains("SB-GRPC-PSK-site-a", ex.Message);
|
Assert.Contains("SB-GRPC-PSK-site-a", ex.Message);
|
||||||
|
Assert.Contains("store fault", ex.Message);
|
||||||
|
Assert.IsType<InvalidOperationException>(ex.InnerException);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Verifies that successful resolves are cached so not every call hits the store.</summary>
|
/// <summary>Verifies that successful resolves are cached so not every call hits the store.</summary>
|
||||||
|
|||||||
Reference in New Issue
Block a user