2ee84af1c0
Phase 0 of the ClusterClient→gRPC migration
(docs/plans/2026-07-22-clusterclient-to-grpc-plan.md). Standalone hardening: it
closes a gap that exists today and is a precondition for moving command/control
onto gRPC in later phases.
T0.1 — delete the ManagementActor ClusterClientReceptionist registration.
It was built for an out-of-cluster CLI that was never written: the shipped CLI
speaks HTTP Basic to /management, which asks the actor in-process through
ManagementActorHolder. Nothing in the repo ever sent to /user/management. The
actor still runs there; only the cross-boundary advertisement is gone. Six
documents claimed the CLI used ClusterClient — including the CLI's own README
"Architecture Notes" — and are corrected here rather than left to rot.
T0.2 — record, do not port, the dead integration-routing path.
IntegrationCallRequest is unwired at BOTH ends: RouteIntegrationCallAsync has
zero callers anywhere, and RegisterLocalHandler(Integration, …) appears only in
a test, so production always answers "Integration handler not available". It is
excluded from the gRPC contract (28 of 29 commands migrate) rather than
enshrined on an additive-only wire format, and deleting it during a
transport migration would mix a behavioural change into a change whose whole
value is that behaviour is identical. See
docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md.
T0.3 — preshared-key authentication on SiteStreamService.
The service shipped with no auth at all: plaintext h2c, no interceptor, so
anything that could reach a site node's :8083 could open a live data stream or
read audit rows back via PullAuditEvents/PullSiteCalls. ControlPlaneAuthInterceptor
now gates /sitestream.SiteStreamService/ — modeled on LocalDbSyncAuthInterceptor
(constant-time compare, fail-closed, PermissionDenied) but gating a SET of
service prefixes so phases 1A/1B add services rather than interceptors. LocalDb
sync keeps its own separate key: it authenticates the pair partner, not central,
and collapsing the two would make a site's central-facing key also admit writes
into its database.
Keys are per site (SB-GRPC-PSK-<siteId>), never fleet-wide, so a compromised
site yields only its own. Central attaches them through ControlPlaneCredentials,
which binds CallCredentials to the channel — covering unary and streaming
uniformly, and letting the key resolve asynchronously, which a client
interceptor could not do without blocking. All three central→site channel
creation sites go through it (SiteStreamGrpcClient and both audit pull invokers);
the pull invokers' channel caches are re-keyed by (site, endpoint) because
credentials are per-site and bound to the channel.
Two decisions beyond the plan:
* StartupValidator now requires GrpcPsk on Site nodes. The plan specified only
the runtime gate, but fail-closed with no boot check produces a node that
joins, answers heartbeats and reports healthy while refusing every stream,
audit pull and telemetry ingest — silent and total. Same reasoning as the
existing inbound API-key pepper rule.
* Added Communication:SitePsks as a central-side key map. The plan assumed
central would read the store, seeded via a dev KEK; the docker rig
deliberately boots with no master key, so store-only resolution would leave
it unable to dial its own sites. The store stays primary — it is the only
source that can serve a site added at runtime — with the map covering
key-less hosts and one-off pins. Neither source falling back to
"unauthenticated" is the invariant.
T0.4 — dev keys on both rigs and tests.
34 tests. The seven that matter most exercise a real in-process gRPC stack over
TestServer: the unit tests on either side of the wire would both stay green if
the halves disagreed, and gRPC refuses call credentials on a plaintext channel
by default — the UnsafeUseInsecureChannelCallCredentials opt-in is only provable
by making a real call. They confirm correct key passes on unary AND streaming,
wrong key and no-credentials both get PermissionDenied, and an unresolvable key
fails the call with nothing reaching the service.
OPERATIONAL: a site node upgraded to this build without a key will not boot.
That includes the gitignored deploy/wonder-app-vd03/ overlay.
180 lines
7.4 KiB
C#
180 lines
7.4 KiB
C#
using Microsoft.Extensions.Logging.Abstractions;
|
|
using Microsoft.Extensions.Options;
|
|
using NSubstitute;
|
|
using ZB.MOM.WW.ScadaBridge.Communication;
|
|
using ZB.MOM.WW.Secrets.Abstractions;
|
|
|
|
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
|
|
|
/// <summary>
|
|
/// Central's per-site gRPC preshared key resolution (ClusterClient→gRPC migration, T0.3).
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// The property that matters most here is the negative one: a site whose key cannot be found
|
|
/// must produce a throw, never an unauthenticated channel. Everything else — the two sources,
|
|
/// the caching, the invalidation — exists to make that behaviour usable in practice.
|
|
/// </remarks>
|
|
public class SitePskProviderTests
|
|
{
|
|
private static SitePskProvider Create(
|
|
ISecretResolver resolver, CommunicationOptions? options = null)
|
|
=> new(
|
|
resolver,
|
|
new StaticOptionsMonitor(options ?? new CommunicationOptions()),
|
|
NullLogger<SitePskProvider>.Instance);
|
|
|
|
private sealed class StaticOptionsMonitor(CommunicationOptions value)
|
|
: IOptionsMonitor<CommunicationOptions>
|
|
{
|
|
public CommunicationOptions CurrentValue => value;
|
|
public CommunicationOptions Get(string? name) => value;
|
|
public IDisposable? OnChange(Action<CommunicationOptions, string?> listener) => null;
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ResolvesFromTheSecretStore_UnderTheSiteQualifiedName()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(new SecretName("SB-GRPC-PSK-site-a"), Arg.Any<CancellationToken>())
|
|
.Returns("key-for-a");
|
|
|
|
var psk = await Create(resolver).GetAsync("site-a", CancellationToken.None);
|
|
|
|
Assert.Equal("key-for-a", psk);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ConfiguredMapWins_SoAHostWithNoMasterKeyCanStillDial()
|
|
{
|
|
// The development rig runs with no secrets master key at all — every credential
|
|
// arrives as an environment override. Without this source, the rig could not use the
|
|
// gated control plane and the fail-closed design would be untestable there.
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns("from-the-store");
|
|
|
|
var options = new CommunicationOptions();
|
|
options.SitePsks["site-a"] = "from-config";
|
|
|
|
var psk = await Create(resolver, options).GetAsync("site-a", CancellationToken.None);
|
|
|
|
Assert.Equal("from-config", psk);
|
|
await resolver.DidNotReceive().GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>());
|
|
}
|
|
|
|
[Fact]
|
|
public async Task StoreIsStillConsulted_ForASiteMissingFromTheMap()
|
|
{
|
|
// Sites are added at runtime from the Central UI, so the map can never be complete.
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(new SecretName("SB-GRPC-PSK-site-b"), Arg.Any<CancellationToken>())
|
|
.Returns("key-for-b");
|
|
|
|
var options = new CommunicationOptions();
|
|
options.SitePsks["site-a"] = "from-config";
|
|
|
|
var psk = await Create(resolver, options).GetAsync("site-b", CancellationToken.None);
|
|
|
|
Assert.Equal("key-for-b", psk);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task MissingSecret_Throws_AndNeverYieldsAnEmptyKey()
|
|
{
|
|
// Fail-closed. The alternative — returning "" — would build a channel that presents
|
|
// "Bearer " and gets PermissionDenied anyway, but with a far less diagnosable error.
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns((string?)null);
|
|
|
|
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
|
|
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
|
|
|
// The message must name both sources — either one fixes it.
|
|
Assert.Contains("SB-GRPC-PSK-site-a", ex.Message);
|
|
Assert.Contains("SitePsks", ex.Message);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task EmptySecret_IsTreatedAsMissing()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>()).Returns("");
|
|
|
|
await Assert.ThrowsAsync<InvalidOperationException>(
|
|
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task AFaultingStore_SurfacesAsKeyNotConfigured_NotAsACryptoError()
|
|
{
|
|
// A host with no master key throws from the resolver. The operator's problem is the
|
|
// same either way — "this site has no key" — so the diagnosis must say that.
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns<string?>(_ => throw new InvalidOperationException("no master key configured"));
|
|
|
|
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
|
|
async () => await Create(resolver).GetAsync("site-a", CancellationToken.None));
|
|
|
|
Assert.Contains("SB-GRPC-PSK-site-a", ex.Message);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task SuccessfulResolvesAreCached_SoEveryCallDoesNotHitTheStore()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>()).Returns("k");
|
|
var provider = Create(resolver);
|
|
|
|
await provider.GetAsync("site-a", CancellationToken.None);
|
|
await provider.GetAsync("site-a", CancellationToken.None);
|
|
await provider.GetAsync("site-a", CancellationToken.None);
|
|
|
|
await resolver.Received(1).GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>());
|
|
}
|
|
|
|
[Fact]
|
|
public async Task FailuresAreNotCached_SoASeededKeyIsPickedUpWithoutARestart()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns((string?)null, "seeded-later");
|
|
var provider = Create(resolver);
|
|
|
|
await Assert.ThrowsAsync<InvalidOperationException>(
|
|
async () => await provider.GetAsync("site-a", CancellationToken.None));
|
|
|
|
Assert.Equal("seeded-later", await provider.GetAsync("site-a", CancellationToken.None));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Invalidate_DropsTheCachedKey_SoARotatedKeyIsRead()
|
|
{
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(Arg.Any<SecretName>(), Arg.Any<CancellationToken>())
|
|
.Returns("old", "rotated");
|
|
var provider = Create(resolver);
|
|
|
|
Assert.Equal("old", await provider.GetAsync("site-a", CancellationToken.None));
|
|
provider.Invalidate("site-a");
|
|
|
|
Assert.Equal("rotated", await provider.GetAsync("site-a", CancellationToken.None));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task KeysAreScopedPerSite_SoOneSiteNeverPresentsAnothersKey()
|
|
{
|
|
// The whole reason the design rejected a single fleet-wide key: blast radius.
|
|
var resolver = Substitute.For<ISecretResolver>();
|
|
resolver.GetAsync(new SecretName("SB-GRPC-PSK-site-a"), Arg.Any<CancellationToken>())
|
|
.Returns("key-a");
|
|
resolver.GetAsync(new SecretName("SB-GRPC-PSK-site-b"), Arg.Any<CancellationToken>())
|
|
.Returns("key-b");
|
|
var provider = Create(resolver);
|
|
|
|
Assert.Equal("key-a", await provider.GetAsync("site-a", CancellationToken.None));
|
|
Assert.Equal("key-b", await provider.GetAsync("site-b", CancellationToken.None));
|
|
}
|
|
}
|