Skip to content

feat(hosting): instance keys rotate through the operator — AdoptKeyHash + hosting-kv-rotate (#2802) - #2803

Merged
rbuergi merged 1 commit into
mainfrom
feat/hosting-rotate-registry-key
Aug 30, 2026
Merged

rbuergi merged 1 commit into
mainfrom
feat/hosting-rotate-registry-key

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

The platform half of #2802. The Hosting plugin's RotateRegistryKey verb follows in MeshWeaver.Plugins once this is on main (cross-repo halves land platform first).

Why

Two live mwi_ instance keys need rotating. MeshWeaverInstanceService.ReissueKey exists with no caller, and the only way to land a new value in a running portal was a terminal that would have to see the key — which is how today's exposure happened in the first place (an audit selected .value).

The design: the raw key exists in exactly two places, ever

where what it holds
the operator job's memory the raw key, until unset
Key Vault the raw key
the job log / the mesh / this repo the hash only

hosting-kv-rotate — the one operator script allowed to overwrite a vault secret (hosting-kv-ensure never will: the master key it guards seals stored credentials, which is why this is a separate verb and kv-ensure must never grow a --force). Mints mwi_ + base64url(32 random bytes) — the InstanceKeys scheme exactly — sets it in the vault from a variable that is never echoed, discards it, reports ::hosting:: key_hash=<sha256>, then waits for the synced Kubernetes Secret to carry the new value, compared by hash (DeploymentAKS → "KeyVault CSI env timing": a restart that reads the old synced Secret puts the old key into the new pods) and refuses with a message naming the SPC object if it never does. Dry-run aware; shellcheck clean.

IInstanceKeyRegistry (contract assembly) + MeshWeaverInstanceService.AdoptKeyHash — the registry-side half: the instance takes the new hash and issued-at, a fresh index entry points at it, and the previous index entry is deleted so the old key stops authenticating the moment adoption lands (ReissueKey left that "to be cleaned up separately"; a lifecycle verb must not). Refuses anything that is not 64-char lowercase hex — a raw key arriving here by mistake is precisely what must never happen. On the contract assembly so the Hosting plugin's in-mesh control plane can resolve it from the hub without referencing the portal host.

Verified

InstanceKeyAdoptionTest — 2/2 on the real Monolith mesh: register → adopt by instance id through the contract → new hash + later KeyIssuedAt on the node, new index entry resolves to the instance, old index entry gone; plus the shape refusal. Index nodes are read as System through GetMeshNode, the way the registry's own tests read fresh nodes (the first draft point-read them and hit the absent-node NotFound — fixed by reading the way the house rules say, not by widening a wait).

Already done outside the repo

The hosting-operator identity had a Key Vault Secrets Officer role assignment that was inert — the Systemorph vault uses access policies (enableRbacAuthorization=false). It now holds get/set/list as a policy, verified by read-back. Recording it here because it is the precondition the script depends on and it lives nowhere else yet.

🤖 Generated with Claude Code

…sh + hosting-kv-rotate (#2802)

The platform half of MeshWeaver#2802. Two live mwi_ instance keys need
rotating, and there was no clean path: MeshWeaverInstanceService.ReissueKey
existed with NO caller, and the only way to land a new value in a running
portal was a terminal that would have to see the key.

- deploy/aks/operator/bin/hosting-kv-rotate: the ONE operator script allowed
  to overwrite a vault secret (kv-ensure never will — the master key it guards
  seals stored credentials, which is why this is a separate verb and kv-ensure
  must never grow a --force). Mints mwi_ + base64url(32 random bytes) — the
  InstanceKeys scheme exactly — sets it in Key Vault from a variable that is
  never echoed, unsets it, and reports ONLY `::hosting:: key_hash=<sha256>`.
  Then waits for the synced Kubernetes Secret to carry the new value,
  compared by HASH of the decoded value (DeploymentAKS → "KeyVault CSI env
  timing": a restart that reads the old synced Secret puts the OLD key into
  the new pods), and refuses with a message naming the SecretProviderClass
  object if it never does. Dry-run aware. shellcheck clean.

- IInstanceKeyRegistry (MeshWeaver.Mesh.Contract/Security): the registry-side
  contract, on the contract assembly so the Hosting plugin's in-mesh control
  plane can resolve it from the hub's ServiceProvider without referencing the
  portal host. Hashes only, by instance id.

- MeshWeaverInstanceService.AdoptKeyHash: a ReissueKey that takes the hash
  instead of minting the key. The instance node takes KeyHash/KeyIssuedAt, a
  fresh index entry points at it, and the PREVIOUS index entry is DELETED —
  ReissueKey left that "to be cleaned up separately"; a rotation driven by a
  lifecycle verb must not leave the old key authenticating. Refuses anything
  that is not a 64-char lowercase hex (a raw key sent here by mistake is
  exactly the thing that must never arrive). Idempotent for an adopted hash.

Tests (Memex.Portal.Shared.Test/InstanceKeyAdoptionTest, 2/2 on the real
Monolith mesh): register → adopt by instance id through the contract →
instance carries the new hash and a later KeyIssuedAt, the new index entry
resolves to the instance, the old index entry is GONE; and the shape
refusal. Index nodes are read as System through GetMeshNode, the way the
registry's own tests read fresh nodes.

The Hosting plugin's RotateRegistryKey verb, which drives both halves, is
the Plugins half and follows once this is on main (cross-repo halves land
platform first).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements the platform-side support for rotating MeshWeaver instance registry keys via the Hosting operator, ensuring the raw mwi_ key only exists in the operator job’s memory and in Key Vault while the registry adopts only the SHA-256 hash and retires the previous index entry.

Changes:

  • Added IInstanceKeyRegistry (contract) for adopting a rotated key by hash.
  • Implemented MeshWeaverInstanceService.AdoptKeyHash and an instance-id based adapter for in-mesh callers.
  • Added the hosting-kv-rotate operator script plus a Monolith integration test and a What’s New entry.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/Memex.Portal.Shared.Test/InstanceKeyAdoptionTest.cs Adds an end-to-end Monolith test covering hash adoption + old-index retirement behavior.
src/MeshWeaver.Mesh.Contract/Security/IInstanceKeyRegistry.cs Introduces a contract surface for adopting a rotated instance key by hash.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-instance-keys-rotate-through-the-hosting-operator.md Documents the new rotation split (operator mints key, registry adopts hash).
memex/Memex.Portal.Shared/Authentication/MeshWeaverInstanceService.cs Implements hash adoption and contract adapter logic in the registry host.
deploy/aks/operator/bin/hosting-kv-rotate Adds the operator-side rotation script that writes the new secret to Key Vault and reports only the hash.
Suppressed comments (1)

memex/Memex.Portal.Shared/Authentication/MeshWeaverInstanceService.cs:503

  • DeleteIndex currently switches access context manually and then swallows all delete failures. That can leave the old index entry in place (so the old key may keep authenticating) while AdoptKeyHash still reports success. Use the sealed impersonation boundary (RunAsSystem) and only treat the specific "Node not found" case as ignorable; propagate any other failure.
            var disposable = accessService.SwitchAccessContext(
                new AccessContext { ObjectId = WellKnownUsers.System, Name = "system-security" });
            return nodeFactory.DeleteNode(indexPath)
                .Select(_ => Unit.Default)
                .Catch<Unit, Exception>(ex =>
                {
                    // Absent already: not a failure. Anything else IS one — a stale entry that
                    // still authenticates is the hole this method exists to close.
                    logger.LogDebug(ex, "No index entry to remove at {Path}", indexPath);
                    return Observable.Return(Unit.Default);
                })

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +464 to +480
return Observable.Using(
() => accessService.ImpersonateAsSystem(),
_ => meshService.Query(request))
.Take(1)
.Timeout(TimeSpan.FromSeconds(10))
.SelectMany(results =>
{
// The index nodes share the NodeType and use the hash prefix as id, so an instance
// record is the hit whose id IS the instance id and whose namespace is not the index.
var node = results.FirstOrDefault(n =>
string.Equals(n.Id, instanceId, StringComparison.Ordinal)
&& !string.Equals(n.Namespace, MeshWeaverInstanceNodeType.IndexNamespace, StringComparison.Ordinal));
return node?.Path is null
? Observable.Throw<Unit>(new InvalidOperationException(
$"no MeshWeaverInstance is registered as '{instanceId}' — nothing to rotate"))
: AdoptKeyHash(node.Path, keyHash);
});
@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

444 tests  ±0   444 ✅ ±0   5m 10s ⏱️ +46s
  6 suites ±0     0 💤 ±0 
  6 files   ±0     0 ❌ ±0 

Results for commit 2a36150. ± Comparison against base commit a7cc0a7.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

1 178 tests  ±0   985 ✅ ±0   5m 38s ⏱️ +10s
    5 suites ±0   193 💤 ±0 
    5 files   ±0     0 ❌ ±0 

Results for commit 2a36150. ± Comparison against base commit a7cc0a7.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

990 tests  +2   990 ✅ +2   5m 38s ⏱️ -27s
  5 suites ±0     0 💤 ±0 
  5 files   ±0     0 ❌ ±0 

Results for commit 2a36150. ± Comparison against base commit a7cc0a7.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

2 184 tests  ±0   2 180 ✅ ±0   6m 46s ⏱️ +21s
   10 suites ±0       4 💤 ±0 
   10 files   ±0       0 ❌ ±0 

Results for commit 2a36150. ± Comparison against base commit a7cc0a7.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

624 tests  ±0   624 ✅ ±0   6m 4s ⏱️ +3s
  6 suites ±0     0 💤 ±0 
  6 files   ±0     0 ❌ ±0 

Results for commit 2a36150. ± Comparison against base commit a7cc0a7.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

   10 files  ±0     10 suites  ±0   6m 1s ⏱️ +21s
3 084 tests ±0  3 084 ✅ ±0  0 💤 ±0  0 ❌ ±0 
3 750 runs  +1  3 750 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 2a36150. ± Comparison against base commit a7cc0a7.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

   42 files  ±0     42 suites  ±0   35m 18s ⏱️ + 1m 13s
8 504 tests +2  8 307 ✅ +2  197 💤 ±0  0 ❌ ±0 
9 170 runs  +3  8 973 ✅ +3  197 💤 ±0  0 ❌ ±0 

Results for commit 2a36150. ± Comparison against base commit a7cc0a7.

@rbuergi
rbuergi added this pull request to the merge queue Aug 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 30, 2026
@rbuergi

rbuergi commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Ejected from the merge queue at 17:00Z (failed_checks) — not this PR's diff. Its entry (gh-readonly-queue/main/pr-2803-afb5a5ec9) failed shard 3 on:

MeshWeaver.Hosting.Orleans.Test.StreamAttachTransientRetryTest.TransientAttachFailure_IsRetried_UpToTheBudget [FAIL]

That is #2793, and #2800 fixes it. This PR touches MeshWeaver.Mesh.Contract/Security, Memex.Portal.Shared/Authentication and an operator shell script; it cannot reach an Orleans stream attach.

Why the queue currently cannot land anything

Both settle-by-silence tests false-RED on a queue-entry runner, and each is unfixed on the branch the other builds on:

entry failed on fixed by
pr-2803 (this) StreamAttachTransientRetryTest #2800
pr-2800 PodHubClaimLifetimeTest.AClaimThatCannotLand_WhereNoGrainCanBeHosted_StopsAtTheInitialBudget_AtDebug (expected 6, found 1) nothing yet

The queue builds each entry on the ones ahead of it, so #2800 alone still meets the unfixed pod-hub test and ejects before this one is ever built. They have to be in the same entry, so I am folding the pod-hub fix into #2800 rather than opening a second PR that would eject it.

Re-queuing this once #2800 lands. No change needed here.

rbuergi added a commit that referenced this pull request Aug 30, 2026
…t a pause (#2793 sibling)

The SECOND settle-by-silence poll, found by the merge queue itself:
PodHubClaimLifetimeTest.AClaimThatCannotLand_WhereNoGrainCanBeHosted_
StopsAtTheInitialBudget_AtDebug failed the queue entry for #2800 with
"Expected value to be 6 … but found 1" — the identical shape, and the
identical value, as StreamAttachTransientRetryTest in #2793.

Its WaitUntilSettled polled factory.AttachCalls every 25 ms and called two
equal readings "settled". The claim's RetryWhen hops through the thread-pool
scheduler between attempts; on a loaded runner that hop exceeds 25 ms, so the
poll reads the counter MID-HOP and returns 1 — which is exactly the value
unfixed code produces. A false RED that spells the regression's own signature
is worse than no test, and the file's own doc comment even said "there is no
completion signal to await for a claim that ends".

There is one now. OrleansRoutingService.PodHubClaimSettled(address) is the
sibling of #2800's AttachSettled: a per-address AsyncSubject<Unit>, an
INSTANCE field on the routing service (never static), armed BEFORE the claim
is subscribed so it cannot terminate unobserved, and completed in BOTH
terminal arms of the claim's Subscribe — it landed, or it hit the one
terminal that is impossibility rather than a budget (a process that cannot
host a grain can never win the claim). A claim that is still retrying never
completes it, which is the honest answer for a lifetime with no give-up:
that case keeps WaitForAttempts, which was already a positive signal. The
subject is removed with the registration.

Verified:
- 5/5 green, and in 1 s rather than the poll's seconds-per-test.
- MUTATION-PROVED, not merely observed green: collapsing the budget
  (`attempt >= PodHubAttachRetries` → `attempt >= 0`) makes the test FAIL
  with "found 1", so the new wait still detects the regression it exists for.
  Restored and re-verified after the probe.

Folded into this PR rather than opened as its own, because the queue builds
each entry on the ones ahead of it: #2800 alone still meets the unfixed
pod-hub test and ejects (16:48Z), while #2803 alone meets the unfixed stream
attach and ejected too (17:00Z). Neither sibling can land without the other,
so they belong in ONE entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi
rbuergi added this pull request to the merge queue Aug 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 30, 2026
rbuergi added a commit that referenced this pull request Aug 30, 2026
…ecting releases on them (#2777, #2700)

Since the merge queue went live every entry builds on a loaded runner BY
CONSTRUCTION, and three releases have now been ejected by three different
tests whose assertion is a bound rather than a condition:

  #2800  PodHubClaimLifetimeTest        (fixed, merged)
  #2803  StreamAttachTransientRetryTest (fixed by #2800)
  #2803  DataContextInitFaultedTest     ← this commit, ejected 17:32Z

1. SelfUpdateAvailabilityGateTest (#2777)

   NeverRolled_WithTheGateUnavailable_StillHolds waited up to 30s for a HELD
   marker to land on Admin/UpdatePolicy. The service is event-driven, so "the
   hold is not there yet" and "the first check has not run yet" are
   indistinguishable from outside — and on a loaded shard it is the second.
   The issue's own diagnosis, adopted verbatim: make the first poll
   deterministic rather than widen the bound.

   SelfUpdateHostedService now publishes ChecksReported — one emission per
   check that has been evaluated AND reported, an INSTANCE Subject (never
   static), completed in StopAsync so a subscriber on a stopped service is
   released rather than left hanging. The tests await the first emission, then
   assert on the state it produced. Production never subscribes; nothing about
   what a check does or when it runs changes.

   The member is protected internal and Hosting.Monolith.Test is not in
   Memex.Portal.Shared's InternalsVisibleTo list, so the test's own subclass
   forwards it (`Evaluations`) — cheaper than widening the production
   assembly's friend list for one file.

2. DataContextInitFaultedTest.FaultedInit_ReachesTerminalFailedState_AndAnswersFast

   🚨 ONE bound was doing TWO jobs that want OPPOSITE values. The test asserts
   liveness ("is a faulted init answered at all" — wants a generous bound, so
   a timeout means NEVER) and latency (the AndAnswersFast in its name — wants
   a tight one). A single 15s wait conflated them: a loaded runner pushed the
   rejection past it, and the assertion read the resulting TimeoutException as
   "the gate is shut" — the defect's own signature, for a defect that was not
   there. It ejected #2803, a PR touching only Mesh.Contract/Security and an
   operator script, which cannot reach a DataContext.

   Merely widening to 60s would have DELETED the latency half in silence: the
   test would still pass if the answer took 55s, which is precisely what its
   name guards. So they are separated — a generous 60s wait (a timeout now
   means never), plus an elapsed-time assertion of its own against a threshold
   DERIVED FROM THE MECHANISM rather than a feeling of "fast": the hub's own
   30s request budget, which is what a request that FELL THROUGH the faulted
   arm waits out. A slow answer and an absent one now fail with different
   messages, because they are different defects.
   (The split is owed to the reviewing session, who caught that my first pass
   would have quietly dropped the latency half.)

Verified: Memex.Portal.Shared, Hosting.Monolith.Test and Data.Test each
-c Release -warnaserror → 0 Error(s); SelfUpdateAvailabilityGateTest 6/6 and
FaultedInit_ReachesTerminalFailedState 1/1 green.

🚨 Trap worth recording: `--filter "FullyQualifiedName~DataContextInitWatchdogTest"`
matched NOTHING and exited 0 — the file is DataContextInitWatchdogTest.cs but
the CLASS is DataContextInitFaultedTest. A filter that matches nothing is a
silent pass; filter on the method name when the class name is inferred from a
filename.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi
rbuergi added this pull request to the merge queue Aug 30, 2026
Merged via the queue into main with commit 38ce5b7 Aug 30, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants