From f759ceef913677399e13252a22646b372d14cc1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Sat, 12 Sep 2026 15:14:34 +0200 Subject: [PATCH 1/5] fix: the self-updater accepts a registry whose DECLARED validator holds the key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An installation pulling its images from cr.meshweaver.cloud booted, pulled in seconds with its mwi_ instance key, and then never self-updated: the credential was chosen by matching the container registry's HOST against a configured plugin registry, and the fleet's registry deliberately validates at ANOTHER portal (docker_auth forwards the key to memex.meshweaver.cloud/api/instances/token), so the two hosts differ by design and host equality could never succeed. The guard is a credential-disclosure control, so it is not dropped or loosened β€” it gains a second, equally explicit way to qualify. A plugin registry may now be matched by the host this installation DECLARES as the registry's validator (SelfUpdate:RegistryValidationUrl, the registry record's own validationUrl), which is data the fleet already states on RegistrySpec. Both rows of the rule need two statements: something must declare the pairing AND this installation must hold a key for that host. An absent declaration refuses exactly as today, hosts are compared whole through one shared reader, and a value carrying userinfo declares nothing rather than a pairing a human would misread. The consumer cannot learn the pairing by itself β€” RegistrySpec lives on the record of the instance that HOSTS the registry, and reaching across instances would put a network dependency inside the credential path β€” so the operator restates it: SelfUpdate__RegistryValidationUrl, rendered by the chart from selfUpdate.registryValidationUrl or from a record's extraPortalConfig. Falsified both ways: deleting the host check reddens all three negative controls (they assert no request AND no key presented); removing the validator branch reddens the positive. The refusal names hosts and config keys only; the one new Information line, on the paired path only, records which key went where. Refs #4093 β€” this makes the fix POSSIBLE; it does not fix `build` or `pearl`. Both stay refused until SelfUpdate__RegistryValidationUrl is declared on their records in Systemorph/Memex, so the issue stays OPEN past this merge. No auto-close keyword, deliberately. Co-Authored-By: Claude Opus 5 --- .../helm/templates/memex-portal/config.yaml | 10 ++ deploy/helm/values.yaml | 16 ++ .../SelfUpdate/OciTagLister.cs | 110 +++++++++++--- .../SelfUpdate/SelfUpdateHostedService.cs | 12 +- .../Data/Architecture.md | 1 + .../Architecture/ContainerRegistryInMemex.md | 15 +- .../SelfUpdateRegistryCredential.md | 117 +++++++++++++++ ...he-fleet-registry-can-self-update-again.md | 43 ++++++ .../SelfUpdate/SelfUpdateOptions.cs | 65 +++++++++ .../OciTagListerTest.cs | 137 +++++++++++++++++- 10 files changed, 494 insertions(+), 32 deletions(-) create mode 100644 src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md create mode 100644 src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-on-the-fleet-registry-can-self-update-again.md diff --git a/deploy/helm/templates/memex-portal/config.yaml b/deploy/helm/templates/memex-portal/config.yaml index f159ac9995..68fc2d2bf9 100644 --- a/deploy/helm/templates/memex-portal/config.yaml +++ b/deploy/helm/templates/memex-portal/config.yaml @@ -456,6 +456,16 @@ data: {{- if (.Values.selfUpdate).registry }} SelfUpdate__Registry: "{{ .Values.selfUpdate.registry }}" {{- end }} + # 🚨 The portal that VALIDATES this installation's instance key at that registry β€” the registry + # record's own `validationUrl` (MeshWeaver#4093). The fleet registry cr.meshweaver.cloud decides a + # pull by forwarding the caller's key to memex.meshweaver.cloud, so the image registry and the + # plugin registry are DIFFERENT hosts by design, and without this declaration the self-updater + # refuses to present the key it holds β€” an instance boots fine and then never self-updates. + # A string, so an absent value IS inert: no declaration β‡’ the key is presented only to a host that + # is itself a configured plugin registry, exactly as before. Never read as permission, and never + # inferred from name resemblance. It may be set through an overlay's config.memex_portal or a + # record's extraPortalConfig; the values path below is what the instance record renders. + SelfUpdate__RegistryValidationUrl: "{{ (.Values.selfUpdate).registryValidationUrl | default .Values.config.memex_portal.SelfUpdate__RegistryValidationUrl | default "" }}" # ---- Container image mirror (MeshWeaver#3353, src/MeshWeaver.ContainerImages) -------------- # The /v2 pull surface THIS installation serves. Rendered only when containerImages.upstream is # set β€” an absent block reaches no container and the mirror stays OFF (every /v2 route 404s). diff --git a/deploy/helm/values.yaml b/deploy/helm/values.yaml index ffac31d641..867be76ead 100644 --- a/deploy/helm/values.yaml +++ b/deploy/helm/values.yaml @@ -64,6 +64,22 @@ selfUpdate: # 🚨 PAIRED with portal.imagePullSecret below: the images the updater rolls to are named on this # host, so the kubelet needs a credential for it. AcrPull on the node identity covers ACR only. registry: "" + # 🚨 THE PORTAL THAT VALIDATES THIS INSTALLATION'S KEY AT THAT REGISTRY (MeshWeaver#4093) β€” the + # registry record's own `validationUrl`, copied verbatim (a bare host works too; only the host is + # read). Set it whenever `registry` above is a registry that decides a pull by FORWARDING the + # caller's key to a portal, which is the fleet's shape: cr.meshweaver.cloud asks + # https://memex.meshweaver.cloud/api/instances/token whether the key is good, so the image + # registry and the plugin registry are two DIFFERENT hosts by design + # (Doc/Architecture/ContainerRegistryInMemex). Leave it empty when the registry IS the portal that + # serves the catalog β€” a portal serving its own /v2 mirror needs no declaration. + # + # 🚨 It is a GRANT, and an absent one is a refusal, never permission: the self-updater presents + # this installation's instance key ONLY to a host that is a configured pluginCatalog registry, or + # to `registry` when this names that registry's validator AND a pluginCatalog registry is + # configured there. Two explicit statements, both required; name resemblance grants nothing. + # Without it an instance on the fleet registry boots fine and then never self-updates β€” every + # check fails with "no plugin registry is configured on that host". + registryValidationUrl: "" # ---- Container image mirror (the /v2 pull surface THIS installation serves) ----------------- # The read-through OCI mirror (src/MeshWeaver.ContainerImages, MeshWeaver#3353): this installation # proxies container images from an upstream registry with ONE credential it holds, while every diff --git a/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs b/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs index b7beb35efa..d9cf763a40 100644 --- a/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs +++ b/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs @@ -18,9 +18,17 @@ namespace Memex.Portal.Shared.SelfUpdate; /// exactly that key for the registry it installs modules from /// (PluginCatalog:Registries:N:Token / legacy PluginCatalog:RegistryToken, or the /// stored auto-registration credential). So the credential is RESOLVED, never configured a second -/// time: the plugin registry whose host equals the container registry host is the one whose token -/// is presented, through the same the Store uses. No new -/// secret, nothing to rotate twice. +/// time, through the same the Store uses. No new secret, +/// nothing to rotate twice. +/// +/// 🚨 WHICH plugin registry's key is a disclosure decision, and +/// is the control: the container registry's OWN host when a plugin registry is configured there, +/// or the host this installation DECLARES as that registry's validator +/// () when one is configured there β€” and +/// nothing else. No declaration β‡’ refuse. The fleet needs the second case because +/// cr.meshweaver.cloud decides a pull by forwarding the key to +/// memex.meshweaver.cloud, so the two hosts differ BY DESIGN (#4093, +/// Doc/Architecture/SelfUpdateRegistryCredential). /// /// The wire conversation β€” the bearer handshake, the paginated listing, every page /// followed β€” is , the same client @@ -64,49 +72,105 @@ public IObservable> ListTags(string repository) } /// - /// The plugin-registry credential for the registry whose host IS the container registry β€” - /// the reuse this lister exists for. Faults, naming the host, when no configured plugin - /// registry lives there or when the resolved credential is empty: a listing without a - /// credential could only ever be a 401, so the misconfiguration is said once here instead of - /// on every check as a refused handshake. Cold: the fault is raised at subscription, before - /// the client sends a byte. + /// 🚨 The plugin-registry credential this installation may present to the container registry β€” + /// and the DISCLOSURE CONTROL that decides which one, because the answer is an instance key and + /// the wrong answer hands it to whatever host SelfUpdate:Registry happens to name. + /// + /// Exactly two hosts qualify, both by an explicit statement in this installation's own + /// configuration, tried in this order: + /// + /// the container registry's OWN host, when a plugin registry is configured there β€” a + /// portal serving its images from the /v2 mirror it also serves its catalog from; + /// the host this installation DECLARES as that registry's validator + /// (), when a plugin registry is + /// configured THERE β€” the fleet's shape, where cr.meshweaver.cloud forwards the key to + /// memex.meshweaver.cloud to decide the pull (#4093, + /// Doc/Architecture/SelfUpdateRegistryCredential). + /// + /// + /// Nothing else, ever. The two hosts are compared WHOLE and case-insensitively β€” never by + /// suffix, registrable domain or any other resemblance, which is a coincidence of naming rather + /// than a grant β€” and an ABSENT declaration is refused, never read as permission. Both steps + /// need the operator to have configured a plugin registry on the host in question, so the key + /// only ever goes where this installation was already given one for. + /// + /// Faults, naming the hosts (never the key), when no host qualifies or when the resolved + /// credential is empty: a listing without a credential could only ever be a 401, so the + /// misconfiguration is said once here instead of on every check as a refused handshake. Cold: + /// the fault is raised at subscription, before the client sends a byte. /// private IObservable ResolveCredential(string registry) => Observable.Defer(() => { var catalog = hub.ServiceProvider.GetService() ?? new PluginCatalogOptions(); var registries = RegistryTokenResolver.WithLegacyTokens(catalog, catalog.EffectiveRegistries); - var match = registries.FirstOrDefault(r => HostOf(r.Url) is { } host - && string.Equals(host, registry, StringComparison.OrdinalIgnoreCase)); + var declaredValidator = options.RegistryValidatorHost; + + var match = On(registries, registry); + if (match is null && declaredValidator is not null && On(registries, declaredValidator) is { } paired) + { + // 🚨 The new, security-relevant decision, said out loud once per listing β€” and this + // is an Information line, so it LEAVES THE POD for Loki. It therefore carries TWO + // HOSTS and the NAME of a configuration key, and NOTHING derived from the secret: + // not the key, not a prefix or suffix of it, not its length, not a hash of it. + // Do not add one "for diagnostics" β€” a length is a fact about a credential. + // + // 🚨 And the hosts are the PARSED hosts, never `paired.Url`. A configured URL may + // carry userinfo (`https://instance:mwi_…@memex.meshweaver.cloud`), which would put + // the key in Loki in a field nobody thinks of as a credential β€” the #3201 shape, + // where the registry key sat in a pod spec `kubectl describe` printed and is still + // owed a rotation. `declaredValidator` IS this registry's host by construction: it + // is what `On(...)` just matched it by. + logger?.LogInformation( + "[SelfUpdate] presenting the instance key configured under PluginCatalog:Registries " + + "for {PluginRegistryHost} to the container registry {Registry}: " + + "SelfUpdate:RegistryValidationUrl declares that host as the portal which validates " + + "this installation's key there.", + declaredValidator, registry); + match = paired; + } + if (match is null) return Observable.Throw(new InvalidOperationException( $"SelfUpdate:Registry is '{registry}', an OCI registry the self-updater authenticates " + "with this installation's plugin-registry instance key β€” but no plugin registry " + "(PluginCatalog:Registries / PluginCatalog:RegistryUrl) is configured on that host, " - + "so there is no key to present. The mirror an installation pulls images from is the " - + "installation it installs modules from; configure both on the same host, or set " - + "SelfUpdate:Registry back to the upstream Azure Container Registry.")); + + (declaredValidator is null + ? "and no validator is declared for it, so there is no key to present. Either " + + "configure the plugin registry on the same host, or β€” when the registry " + + "validates this installation's key at ANOTHER portal, as cr.meshweaver.cloud " + + "does at memex.meshweaver.cloud β€” declare that portal in " + + "SelfUpdate:RegistryValidationUrl (the registry record's own validationUrl) " + + "and configure it as a plugin registry. " + : $"and the validator it declares, '{declaredValidator}' " + + "(SelfUpdate:RegistryValidationUrl), is not configured as one either, so there " + + "is no key to present. Add that host to PluginCatalog:Registries with the " + + "instance key issued for it. ") + + "Or set SelfUpdate:Registry back to the upstream Azure Container Registry.")); var resolver = hub.ServiceProvider.GetService(); var credential = resolver is null ? Observable.Return(match.Token?.Trim() ?? string.Empty) : resolver.ResolveToken(match); + // 🚨 The HOST, never `match.Url` β€” same reason as the log line above: a configured URL + // may carry userinfo, and this message is logged by the poller on every failed check. + var matchedHost = SelfUpdateOptions.HostOf(match.Url) ?? "(unreadable URL)"; return credential.Select(token => token.Length > 0 ? token : throw new InvalidOperationException( - $"No instance key could be resolved for the plugin registry at {match.Url}, so the " + $"No instance key could be resolved for the plugin registry at {matchedHost}, so the " + $"container registry {registry} cannot be listed. The key arrives as " + "PluginCatalog:Registries:N:Token (or the legacy PluginCatalog:RegistryToken), or " + "through auto-registration with PluginCatalog:BootstrapKey.")); }); - private static string? HostOf(string? url) - { - if (string.IsNullOrWhiteSpace(url)) - return null; - if (!Uri.TryCreate(url.Trim(), UriKind.Absolute, out var uri)) - return null; - return uri.IsDefaultPort ? uri.Host : $"{uri.Host}:{uri.Port}"; - } + /// + /// The configured plugin registry living on β€” whole-host, case + /// insensitive, through the SAME host reader the declaration is parsed with, so the two sides + /// of the comparison can never drift. Null when none does. Pure. + /// + private static PluginRegistryReference? On(IEnumerable registries, string host) => + registries.FirstOrDefault(r => SelfUpdateOptions.HostOf(r.Url) is { } configured + && string.Equals(configured, host, StringComparison.OrdinalIgnoreCase)); } diff --git a/memex/Memex.Portal.Shared/SelfUpdate/SelfUpdateHostedService.cs b/memex/Memex.Portal.Shared/SelfUpdate/SelfUpdateHostedService.cs index f131b0c928..17b2532c0d 100644 --- a/memex/Memex.Portal.Shared/SelfUpdate/SelfUpdateHostedService.cs +++ b/memex/Memex.Portal.Shared/SelfUpdate/SelfUpdateHostedService.cs @@ -99,7 +99,17 @@ public Task StartAsync(CancellationToken cancellationToken) ? _options.SafetyNetCheckInterval.ToString() : "disabled", ShippedReleaseSeed.InstalledPlatformVersion, _options.Registry, _options.PortalRepository, - UsesOciListing ? "an OCI Distribution registry (the mirror, instance-key auth)" : "an Azure Container Registry", + // 🚨 On the OCI path the line says WHERE the instance key may be presented, because that + // is the whole configuration and its absence is invisible otherwise: an install with no + // declared validator boots, serves, and only reveals the gap when the first check fails + // (#4093). Hosts only, never the key. + UsesOciListing + ? _options.RegistryValidatorHost is { } validator + ? $"an OCI Distribution registry (instance-key auth; key validated at {validator})" + : "an OCI Distribution registry (instance-key auth; NO validator declared β€” the key " + + "is presented only if this host is itself a configured plugin registry, else " + + "every check refuses; see SelfUpdate:RegistryValidationUrl)" + : "an Azure Container Registry", _updater.CanPatch, _options.RetryInterval); // 🚨 EVENT-DRIVEN WITH A SAFETY NET, and the event source is deliberately OUTSIDE the diff --git a/src/MeshWeaver.Documentation/Data/Architecture.md b/src/MeshWeaver.Documentation/Data/Architecture.md index c1b57650e2..53345a07b2 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture.md +++ b/src/MeshWeaver.Documentation/Data/Architecture.md @@ -373,6 +373,7 @@ Each theme starts with its introductory page, followed by related architecture t - [Release Support Policy](/Doc/Architecture/SupportPolicy) - [Released Artifact Retention](ReleasedArtifactRetention) β€” retain artifacts for at least 30 days, supported releases for their support lifetime, and every artifact still needed by a published set or consumer - [Self-Update Target Selection](SelfUpdateTargetSelection) β€” candidates are ranked by the CD run number, not the version string; a mislabelled line outranked every sealed set for ever, and an install on a withdrawn tag could never see anything newer +- [The Self-Update Registry Credential](SelfUpdateRegistryCredential) β€” which plugin-registry key may be presented to a container registry: a DECLARED pairing, never host equality or name resemblance; an absent declaration refuses - [The Continuous Delivery Contract](ContinuousDeliveryContract) β€” all-or-nothing publication; verify the image, never the tick - [The Self-Update Schema Wall](SelfUpdateSchemaWall) β€” every schema-bumping release is un-takeable by self-update, the stall is invisible, and a promoted tag is not a deployable tag - [Bake Identity Mismatch](BakeIdentityMismatch) β€” why a green CD can publish a bake no portal adopts, and the one rule that keeps two images of one commit on one address diff --git a/src/MeshWeaver.Documentation/Data/Architecture/ContainerRegistryInMemex.md b/src/MeshWeaver.Documentation/Data/Architecture/ContainerRegistryInMemex.md index 63d2a129fb..676e52337e 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/ContainerRegistryInMemex.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/ContainerRegistryInMemex.md @@ -61,8 +61,11 @@ Icon: +--- + +# The Self-Update Registry Credential + +**An installation on a non-ACR registry lists its platform tags with the `mwi_` instance key it +already holds for a plugin registry. WHICH plugin registry's key is a disclosure decision, and it is +decided by DECLARED PAIRINGS β€” never by host equality, never by name resemblance, and never by "there +is only one key, so use it".** + +The rule, in `OciTagLister.ResolveCredential`: + +| The key is presented to… | …because | +|---|---| +| the container registry's **own host**, when a plugin registry is configured there | the operator already gave this installation a key for that exact host β€” a portal serving its images from the `/v2` mirror it also serves its catalog from | +| the host named by **`SelfUpdate:RegistryValidationUrl`**, when a plugin registry is configured there | the operator declared that portal as the one that *validates* this installation's key at that registry, AND gave this installation a key for it | +| **nothing else, ever** | an absent declaration is a refusal, not permission | + +Both rows need TWO explicit statements. A declaration alone grants nothing (there is still no key for +that host); a key alone grants nothing (nothing says that registry trusts that portal). + +## Why host equality was wrong + +The fleet's registry deliberately does **not** hold credentials. `cr.meshweaver.cloud` decides a pull +by forwarding the caller's password to a portal β€” `docker_auth`'s `ext_auth` hook `POST`s it to +`registry.validationUrl`, by default `https://memex.meshweaver.cloud/api/instances/token`, and grants +`pull` on a 200 ([A Container Registry in Memex](../ContainerRegistryInMemex)). That is what makes ONE +`mwi_` key serve both the plugin catalog and the image pull, exactly as +`RegistrySpec.ValidationUrl` says it does: + +> The registry's own pull is decided by the portal at `ValidationUrl` β€” the same instance-key gate +> the plugin registry uses β€” **so a consumer holds ONE credential for both.** + +So the image registry and the plugin registry are **two different hosts by design**. A rule that +presents the key only to a host that is *itself* a plugin registry can therefore never authenticate +on the fleet's own shape. Measured 2026-09-12 on the first boot of `Deployments/build` +([#4093](https://github.com/Systemorph/MeshWeaver/issues/4093)): the kubelet pulled the image in six +seconds with that very key, and the self-updater in the same pod refused to use it β€” + +``` +[SelfUpdate] check (Startup): check FAILED: InvalidOperationException: SelfUpdate:Registry is +'cr.meshweaver.cloud' … but no plugin registry … is configured on that host +``` + +Every instance provisioned on the fleet registry booted fine and then never self-updated. `Reconcile` +and `Roll` from the control instance still worked, which is why the state is easy to miss: the +instance is healthy, it is merely frozen. + +## Why the obvious fix is the wrong fix + +The guard is a **credential-disclosure control**. `SelfUpdate:Registry` names a host; without the +check, whatever that host happens to be gets handed this installation's instance key. So none of +these is acceptable: + +- **Drop the check** β€” any `SelfUpdate:Registry` value now receives the key. +- **"When exactly one consumer mount carries a key, present it"** β€” the number of mounts an + installation has is not a statement about which registries may hold its key. It is the same rule as + dropping the check for the common case of one mount, which is most of the fleet. +- **Same registrable domain / a suffix match** (`*.meshweaver.cloud`) β€” a coincidence of naming, not + a grant. Whoever can obtain a name under the suffix obtains the key. + +What makes the pairing safe is that the registry **declares** which portal it trusts with the key. +That declaration is already data β€” `RegistrySpec.ValidationUrl` β€” so the fix restates it on the +consuming side rather than inventing a new concept. + +## Where the running portal learns the pairing + +🚨 **It does not learn it by itself, and that is deliberate.** `RegistrySpec` lives on the record of +the instance that **HOSTS** the registry (`Deployments/memex-cloud`), on the control instance. A +CONSUMER's record (`build`, `pearl`) has no `registry` block at all, `HelmValues` renders +`registry.validationUrl` only for the hosting record, and reaching across instances to read it would +put a network dependency inside the credential path β€” a lookup that fails open, or fails the update. + +So the pairing is an **explicit declaration the operator sets**, in the consumer's own configuration: + +| where | what | +|---|---| +| `SelfUpdate:RegistryValidationUrl` | the config key the portal binds (`SelfUpdateOptions.RegistryValidationUrl`) | +| `selfUpdate.registryValidationUrl` in the chart | renders `SelfUpdate__RegistryValidationUrl` into the portal ConfigMap | +| a record's `extraPortalConfig` / an overlay's `config.memex_portal` | renders the same key β€” this is how an existing instance is fixed without a chart change | + +The value is the registry record's `validationUrl`, copied verbatim +(`https://memex.meshweaver.cloud/api/instances/token`); a bare host means the same thing. **Only the +host is ever read**, and it is read whole: a non-default port is part of it, and a value carrying +userinfo (`https://memex.meshweaver.cloud@evil.example`) declares NOTHING rather than a pairing with +the host a human would not have read. An empty value β€” the default β€” declares nothing and refuses. + +## What the refusal says + +The message names the hosts and the key that would declare the pairing. It never names, echoes, +lengths or logs the credential itself. The one new log line, at `Information` on the paired path +only, names the plugin registry, the container registry and the declared validator β€” so "who did this +installation hand its key to" is answerable from Loki, and a pairing nobody declared can never +produce a line. + +## The negative control + +`OciTagListerTest` pins both directions, and the negatives are what prove the guard still guards: + +- **paired** β€” the container registry on its own host, the validator declared: the listing proceeds, + the key presented is the one held for the *validator*, and the only host contacted is the + container registry. +- **the same registry, nothing declared** β€” refused, no request leaves, no key presented. Deleting or + loosening the host comparison turns this green, which is exactly the point. +- **a declared validator this installation holds no key for** β€” refused. A declaration alone is not a + grant. + +## See also + +- [A Container Registry in Memex](../ContainerRegistryInMemex) β€” the registry, `docker_auth`, and the + validation hook that makes one key serve both +- [Self-Update Target Selection](../SelfUpdateTargetSelection) β€” what the listing is then used for +- [Release & Self-Update Strategy](../ReleaseStrategy) diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-on-the-fleet-registry-can-self-update-again.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-on-the-fleet-registry-can-self-update-again.md new file mode 100644 index 0000000000..59a53a5ba5 --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-on-the-fleet-registry-can-self-update-again.md @@ -0,0 +1,43 @@ +--- +Name: An instance on the fleet registry can self-update again +Category: Fix +Description: An installation pulling its images from cr.meshweaver.cloud booted normally and then never updated itself, because the self-updater would only present its instance key to a registry that was also its plugin registry. It now presents it to the portal the registry DECLARES as its validator β€” and still refuses every host nobody declared. +Icon: RefreshCcw +Order: -20260912 +--- + +# An instance on the fleet registry can self-update again + +An installation that pulls its platform images from the fleet's own registry +(`cr.meshweaver.cloud`) started, served, pulled its image in seconds β€” and then never updated itself. +Every check failed the same way, whatever its update policy said: + +``` +[SelfUpdate] check (Startup): check FAILED: SelfUpdate:Registry is 'cr.meshweaver.cloud' … +but no plugin registry … is configured on that host, so there is no key to present. +``` + +The self-updater authenticates with the same `mwi_` instance key the installation already holds for +its plugin registry, and it decided which key to present by matching the **host**. But the fleet's +registry deliberately holds no credentials of its own: it decides a pull by forwarding the caller's +key to a portal β€” `cr.meshweaver.cloud` asks `memex.meshweaver.cloud` whether the key is good. The +image registry and the plugin registry are therefore two **different** hosts by design, and host +matching could never succeed on the shape the fleet actually deploys. + +## What changed + +The pairing is now **declared**, with the registry's own `validationUrl` restated on the consuming +side: `SelfUpdate:RegistryValidationUrl` (chart: `selfUpdate.registryValidationUrl`; on an existing +instance, a record's `extraPortalConfig`). Set it to the registry record's validation URL β€” a bare +host works too β€” and the self-updater presents the key it already holds for that portal. + +**It is a grant, and nothing else is one.** The key still goes only to a host that is a configured +plugin registry, or to a registry whose declared validator is one. An absent declaration **refuses**, +exactly as before β€” it is never read as permission β€” and hosts are compared whole: no suffix, no +"same domain", no "there is only one key so use it". Two statements are needed, both explicit. + +Nothing changes for an installation on ACR, for a portal that serves its own `/v2` mirror, or for any +instance that declares nothing: those behave precisely as they did. + +The rule, the measurement behind it and the tests that keep the guard honest are in +[The Self-Update Registry Credential](/Doc/Architecture/SelfUpdateRegistryCredential). diff --git a/src/MeshWeaver.Hosting/SelfUpdate/SelfUpdateOptions.cs b/src/MeshWeaver.Hosting/SelfUpdate/SelfUpdateOptions.cs index 26d168034d..2cdc5a2a3b 100644 --- a/src/MeshWeaver.Hosting/SelfUpdate/SelfUpdateOptions.cs +++ b/src/MeshWeaver.Hosting/SelfUpdate/SelfUpdateOptions.cs @@ -33,6 +33,71 @@ public record SelfUpdateOptions /// public string RegistryUsername { get; init; } = "instance"; + /// + /// 🚨 The portal that VALIDATES this installation's instance key at β€” + /// the container registry's own RegistrySpec.ValidationUrl, restated on the consuming + /// side. Blank (the default) declares no pairing, and an absent declaration is never + /// permission: the lister then presents the key only to a registry that IS a configured plugin + /// registry, exactly as before. + /// + /// Why a declaration and not an inference. A consuming installation holds ONE + /// mwi_ key, and the fleet's registry deliberately validates it by forwarding it to a + /// portal: cr.meshweaver.cloud's docker_auth asks + /// https://memex.meshweaver.cloud/api/instances/token whether the key is good + /// (Doc/Architecture/ContainerRegistryInMemex). So the image registry and the plugin + /// registry are two DIFFERENT hosts by design, and a rule that presents the key only to a host + /// that is itself a plugin registry can never authenticate there (#4093 β€” every instance + /// provisioned on cr.meshweaver.cloud booted and then never self-updated). What makes + /// the pairing safe is not that the names look alike β€” they must never be compared by suffix or + /// registrable domain, which is a coincidence of naming, not a grant β€” but that the registry + /// DECLARES which portal it trusts with that key, and the operator restates that declaration + /// here. Two explicit statements, both required: this key names the validator, and + /// PluginCatalog:Registries must hold a key for it. + /// + /// A full URL (copy the registry record's validationUrl verbatim) or a bare host; + /// only is ever read from it. The chart renders it from + /// selfUpdate.registryValidationUrl as SelfUpdate__RegistryValidationUrl. + /// + public string RegistryValidationUrl { get; init; } = ""; + + /// + /// The HOST of β€” the ONE extra plugin registry whose + /// instance key may be presented to . null when nothing is + /// declared, when the value names no http(s) host, or when it is blank: every one of those is + /// "no pairing declared", which refuses. Pure; a non-default port is part of the host, the + /// path and scheme are discarded. + /// + public string? RegistryValidatorHost => HostOf(RegistryValidationUrl); + + /// + /// The host[:port] of an http(s) URL, or of a bare host (an operator who states + /// memex.meshweaver.cloud rather than the full validation URL means the same portal). + /// Null for anything else β€” a mailto:, a file path, a blank β€” so a value that cannot be read + /// as a host declares no pairing rather than half of one. Pure. + /// + /// Public because the tag lister reads plugin-registry URLs with it too: ONE host-reading + /// rule on both sides of the comparison, so the two can never drift into disagreeing about + /// what https://memex.meshweaver.cloud:443/ is. + /// + public static string? HostOf(string? value) + { + var text = (value ?? string.Empty).Trim(); + if (text.Length == 0) + return null; + if (!text.Contains("://", StringComparison.Ordinal)) + text = "https://" + text; + if (!Uri.TryCreate(text, UriKind.Absolute, out var uri)) + return null; + if (uri.Scheme is not "http" and not "https" || uri.Host.Length == 0) + return null; + // 🚨 USERINFO IS REFUSED, not stripped. `https://memex.meshweaver.cloud@evil.test` has the + // host `evil.test` and reads to a human as the opposite; a value that can be misread that + // way declares no pairing at all rather than one nobody meant. + if (uri.UserInfo.Length > 0) + return null; + return uri.IsDefaultPort ? uri.Host : $"{uri.Host}:{uri.Port}"; + } + /// /// Whether is an Azure Container Registry host β€” the discriminator /// between the ACR tag lister (proprietary API, Workload Identity) and the OCI Distribution diff --git a/test/Memex.Portal.Shared.Test/OciTagListerTest.cs b/test/Memex.Portal.Shared.Test/OciTagListerTest.cs index 511f63a1a0..1385c908d6 100644 --- a/test/Memex.Portal.Shared.Test/OciTagListerTest.cs +++ b/test/Memex.Portal.Shared.Test/OciTagListerTest.cs @@ -47,6 +47,18 @@ public class OciTagListerTest(ITestOutputHelper output) : MonolithMeshTestBase(o private const string InstanceKey = "mwi_this-installations-own-plugin-registry-key"; private const string Repository = "memex-portal-ai"; + /// + /// The fleet's OTHER shape (#4093): a container registry on its own host, which is NOT a plugin + /// registry and never will be β€” cr.meshweaver.cloud decides a pull by forwarding the + /// caller's key to memex.meshweaver.cloud, so the two hosts differ by design. + /// + private const string RegistryHost = "cr.example.test"; + + /// The validator declaration that pairs with the plugin + /// registry this installation actually holds a key for β€” a registry record's + /// validationUrl, copied verbatim. + private const string DeclaredValidator = MirrorUrl + "/api/instances/token"; + /// Strictly newer than anything this test host can run as, so the roll it drives is /// the ordinary forward roll (the same shape ComboGateRollTest uses) β€” and the highest CD build /// number in the fixture, because lineage orders by ci.N, not by the SemVer prefix. @@ -76,9 +88,9 @@ protected override MeshBuilder ConfigureMesh(MeshBuilder builder) private AccessService Access => Mesh.ServiceProvider.GetRequiredService(); - private OciTagLister Lister(string registry = MirrorHost) => new( + private OciTagLister Lister(string registry = MirrorHost, string validator = "") => new( Mesh, - new SelfUpdateOptions { Registry = registry }, + new SelfUpdateOptions { Registry = registry, RegistryValidationUrl = validator }, Mesh.ServiceProvider.GetService>()); // ══════════════════════════════════════════════════════════════════════════ @@ -128,6 +140,110 @@ public async Task NoPluginRegistryOnThatHost_FaultsNamingTheHost() fault.Message.Should().Contain("PluginCatalog", "the fix is a configuration change, and the message has to say which one"); mirror.Requests.Should().Be(0, "a listing with no credential could only ever be a 401"); + mirror.PresentedSecret.Should().BeNull( + "the guard is a disclosure control: an arbitrary host named in SelfUpdate:Registry must " + + "never be handed this installation's instance key"); + } + + // ══════════════════════════════════════════════════════════════════════════ + // #4093 β€” the registry whose validator is ANOTHER portal + // + // The fleet's registry decides a pull by forwarding the caller's key to a portal + // (cr.meshweaver.cloud β†’ memex.meshweaver.cloud/api/instances/token), so the image registry + // and the plugin registry are DIFFERENT hosts by design. These three pin the whole rule: a + // DECLARED pairing resolves the key, and the two ways of not declaring one still refuse. + // Delete the host check in ResolveCredential and the two negatives below go red. + // ══════════════════════════════════════════════════════════════════════════ + + /// + /// 🚨 THE FIX. The container registry is on its own host and is no plugin registry; the + /// installation DECLARES the portal that validates its key there, and the key it already holds + /// for that portal is what the listing presents. + /// + [Fact(Timeout = 120_000)] + public async Task ADeclaredValidator_PresentsTheKeyHeldForThatPortal_AndListsTheRegistry() + { + var ct = TestContext.Current.CancellationToken; + + var tags = await Lister(RegistryHost, DeclaredValidator) + .ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct); + + tags.Should().Equal(FakeMirror.Tags, + "an instance provisioned on the fleet registry must be able to see what it can roll to"); + mirror.PresentedSecret.Should().Be(InstanceKey, + "the credential is the plugin-registry instance key this installation already holds for " + + "the DECLARED VALIDATOR β€” the registry forwards it there to decide the pull, which is " + + "exactly why the two hosts differ"); + mirror.ServedHosts.Should().OnlyContain(h => h == RegistryHost, + "the listing goes to the container registry; the validator is named in configuration " + + "and never contacted by the lister"); + } + + /// + /// 🚨 NEGATIVE CONTROL, and the one that proves the guard still guards: the SAME registry host + /// as the test above, differing only in that nothing declares the pairing. It must refuse, and + /// the key must not leave. A rule that merely dropped or loosened the host comparison β€” "the + /// sole mount carries a key", "same registrable domain" β€” turns this green. + /// + [Fact(Timeout = 120_000)] + public async Task TheSameRegistry_WithNoDeclaredValidator_IsStillRefused() + { + var ct = TestContext.Current.CancellationToken; + + var fault = await Assert.ThrowsAsync(() => + Lister(RegistryHost).ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); + + fault.Message.Should().Contain(RegistryHost); + fault.Message.Should().Contain("SelfUpdate:RegistryValidationUrl", + "an absent declaration is refused AND the message says what would declare it"); + mirror.Requests.Should().Be(0); + mirror.PresentedSecret.Should().BeNull( + "an undeclared pairing is not permission β€” the key never goes out"); + } + + /// + /// 🚨 NEGATIVE CONTROL: a declaration ALONE grants nothing. The validator named is a host this + /// installation holds no plugin-registry key for, so there is still nothing to present β€” the + /// pairing needs BOTH statements, and the refusal names the host that is missing. + /// + [Fact(Timeout = 120_000)] + public async Task ADeclaredValidatorThisInstallationHoldsNoKeyFor_IsRefused() + { + var ct = TestContext.Current.CancellationToken; + + var fault = await Assert.ThrowsAsync(() => + Lister(RegistryHost, "https://someone-elses-portal.example.test/api/instances/token") + .ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); + + fault.Message.Should().Contain("someone-elses-portal.example.test"); + fault.Message.Should().Contain("PluginCatalog:Registries"); + mirror.Requests.Should().Be(0); + mirror.PresentedSecret.Should().BeNull( + "the key is only ever presented to a host this installation was issued one for"); + } + + /// The declaration is read WHOLE-HOST or not at all β€” never a suffix, a registrable + /// domain or anything else that could make a coincidence of naming look like a grant. + [Fact] + public void TheDeclaredValidatorHost_IsReadWholeOrNotAtAll() + { + new SelfUpdateOptions().RegistryValidatorHost.Should().BeNull( + "nothing declared is never permission"); + new SelfUpdateOptions { RegistryValidationUrl = " " }.RegistryValidatorHost.Should().BeNull(); + new SelfUpdateOptions { RegistryValidationUrl = "https://memex.meshweaver.cloud/api/instances/token" } + .RegistryValidatorHost.Should().Be("memex.meshweaver.cloud", "only the host is read"); + new SelfUpdateOptions { RegistryValidationUrl = "MEMEX.meshweaver.cloud" } + .RegistryValidatorHost.Should().Be("memex.meshweaver.cloud", + "a bare host means the same portal, and host names fold case"); + new SelfUpdateOptions { RegistryValidationUrl = "https://memex.meshweaver.cloud:8443/x" } + .RegistryValidatorHost.Should().Be("memex.meshweaver.cloud:8443", + "a non-default port is part of the host"); + new SelfUpdateOptions { RegistryValidationUrl = "https://memex.meshweaver.cloud@evil.example.test/x" } + .RegistryValidatorHost.Should().BeNull( + "the host there is evil.example.test and it reads to a human as the opposite β€” a " + + "value that can be misread that way declares no pairing at all"); + new SelfUpdateOptions { RegistryValidationUrl = "mailto:ops@example.test" } + .RegistryValidatorHost.Should().BeNull("a value that names no http host declares nothing"); } [Fact] @@ -211,13 +327,22 @@ private sealed class FakeMirror : HttpMessageHandler public int PagesServed; public string? PresentedSecret; + private ImmutableList servedHosts = ImmutableList.Empty; + + /// Every host a request actually reached β€” the listing must go to the CONTAINER + /// registry, never to the portal whose key authenticates it. + public ImmutableList ServedHosts => servedHosts; + protected override Task SendAsync( HttpRequestMessage request, CancellationToken cancellationToken) { Interlocked.Increment(ref Requests); var uri = request.RequestUri!; - if (!string.Equals(uri.Host, MirrorHost, StringComparison.Ordinal)) + // Both fleet shapes on one fake: the portal that serves its own /v2 mirror, and a + // container registry on its own host whose key is validated by that portal. + if (uri.Host is not (MirrorHost or RegistryHost)) return Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound)); + ImmutableInterlocked.Update(ref servedHosts, current => current.Add(uri.Host)); if (uri.AbsolutePath == "/v2/token") { @@ -237,7 +362,7 @@ protected override Task SendAsync( } if (request.Headers.Authorization is not { Scheme: "Bearer" } auth || auth.Parameter != InstanceKey) - return Task.FromResult(Challenge()); + return Task.FromResult(Challenge(uri.Host)); if (uri.AbsolutePath == $"/v2/{Repository}/tags/list") { @@ -257,11 +382,11 @@ protected override Task SendAsync( return Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound)); } - private static HttpResponseMessage Challenge() + private static HttpResponseMessage Challenge(string host) { var response = new HttpResponseMessage(HttpStatusCode.Unauthorized); response.Headers.WwwAuthenticate.Add(new AuthenticationHeaderValue("Bearer", - $"realm=\"{MirrorUrl}/v2/token\",service=\"{MirrorHost}\"")); + $"realm=\"https://{host}/v2/token\",service=\"{host}\"")); return response; } From a86d12a5ac6266a5e179214db4047c670da8e4cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Sat, 12 Sep 2026 15:31:41 +0200 Subject: [PATCH 2/5] fix: validate the registry TARGET before resolving any credential (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on #4094, and it found a regression this change introduced. `SelfUpdate:Registry` reached both the credential match and OciRegistryClient raw, trimmed of '/' only, while plugin-registry URLs were normalized by HostOf. Two defects followed, and the second is a key disclosure: * ASYMMETRY β€” a `https://host` form missed a plugin registry that IS that host. * DISCLOSURE β€” OciRegistryClient builds `https://{registry}/` when the value has no scheme, so `instance:mwi_...@evil.example` is a VALID URI whose host is evil.example. It would have received the Basic credential, and the raw value is interpolated into a dozen of that client's exception messages and into the audit line added here. Host equality could never match such a value, so step 1 always refused it. The declared-validator branch is what made it REACHABLE: it matches on the declared host and never looks at the target. So this is the one thing the design existed to prevent, introduced by the design itself. ListTags now requires the configured value to already BE a bare host[:port] β€” HostOf must parse it AND agree with it β€” before any credential is resolved. The unparseable branch does NOT echo the value, because the userinfo is where a key would be. Host-equality rather than "parses to a host" also keeps this in step with PortalImage/MigrationImage, which interpolate the same value. Two tests pin it; deleting the check reddens both (verified on a genuinely rebuilt binary β€” the first attempt at that experiment did not compile and --no-build reported a false 11/11 on the stale assembly). Also from the review, all three valid: * the doc table read as if the key were sent TO the validator; it is sent to the container registry, and the validator only selects WHICH key. * values.yaml still told operators the mirror host "must equal a pluginCatalog registry's host" β€” the very shape #4093 is. * the What's New entry claimed self-update works again; it adds the setting. Refs #4093 Co-Authored-By: Claude Opus 5 --- deploy/helm/values.yaml | 19 ++++++-- .../SelfUpdate/OciTagLister.cs | 38 ++++++++++++++- .../SelfUpdateRegistryCredential.md | 26 +++++++--- ...hich-portal-validates-its-registry-key.md} | 16 +++++-- .../OciTagListerTest.cs | 47 +++++++++++++++++++ 5 files changed, 130 insertions(+), 16 deletions(-) rename src/MeshWeaver.Documentation/Data/WhatsNew/{2026-09-12-an-instance-on-the-fleet-registry-can-self-update-again.md => 2026-09-12-an-instance-can-be-told-which-portal-validates-its-registry-key.md} (75%) diff --git a/deploy/helm/values.yaml b/deploy/helm/values.yaml index 867be76ead..3334b0d3c0 100644 --- a/deploy/helm/values.yaml +++ b/deploy/helm/values.yaml @@ -56,10 +56,21 @@ selfUpdate: # Container Registry, listed through ACR's API with the workload identity above. A registry # login-server host here β€” `memex.meshweaver.cloud`, the read-through mirror another # installation serves at /v2 β€” makes the self-updater list tags through the OCI Distribution API - # instead, authenticated with THIS installation's plugin-registry instance key (the mirror host - # must equal a pluginCatalog registry's host; no second secret), and roll to images named on - # that host. Every installation EXCEPT the one serving the mirror sets this β€” the mirror instance - # cannot serve the image that boots it (Doc/Architecture/ContainerRegistryInMemex). + # instead, authenticated with THIS installation's plugin-registry instance key (no second + # secret), and roll to images named on that host. Every installation EXCEPT the one serving the + # mirror sets this β€” the mirror instance cannot serve the image that boots it + # (Doc/Architecture/ContainerRegistryInMemex). + # + # 🚨 WHICH key is presented is decided by a PAIRING, and there are exactly two ways to qualify: + # this host IS a pluginCatalog registry (a portal serving its own /v2 mirror), or + # `registryValidationUrl` below declares the portal that validates this installation's key here + # AND that portal is a pluginCatalog registry. The fleet's own registry needs the SECOND β€” do not + # read the first as a requirement to put both on one host, which is what #4093 was. + # + # 🚨 A BARE `host` or `host:port` ONLY β€” never a URL, a path, or a value carrying credentials. + # The same value is interpolated into the portal and migration image references, and a value of + # the form `user:secret@host` would be a valid URI naming a DIFFERENT host as the one to hand the + # instance key to. Anything else is refused at startup of the listing. # # 🚨 PAIRED with portal.imagePullSecret below: the images the updater rolls to are named on this # host, so the kubelet needs a credential for it. AcrPull on the node identity covers ACR only. diff --git a/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs b/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs index d9cf763a40..bc572e76f9 100644 --- a/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs +++ b/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs @@ -61,11 +61,45 @@ public sealed class OciTagLister( /// public IObservable> ListTags(string repository) { - var registry = (options.Registry ?? string.Empty).Trim().TrimEnd('/'); - if (registry.Length == 0) + var configured = (options.Registry ?? string.Empty).Trim().TrimEnd('/'); + if (configured.Length == 0) return Observable.Throw>(new InvalidOperationException( "SelfUpdate:Registry is empty β€” there is no registry to list tags on.")); + // 🚨 THE TARGET IS NORMALIZED AND VALIDATED BEFORE ANY CREDENTIAL IS RESOLVED, and the + // check is that the configured value ALREADY IS a bare `host[:port]`. + // + // Two defects live behind a raw value here, and the second is a key disclosure: + // * ASYMMETRY β€” `ResolveCredential` compares this against plugin-registry hosts parsed by + // `HostOf`, so a `https://host` form would miss a registry that is in fact the same host. + // * DISCLOSURE β€” `OciRegistryClient` builds `https://{registry}/` for a value with no + // scheme, so `instance:mwi_…@evil.example` is a VALID URI whose host is `evil.example`; + // it would receive the Basic credential, and the raw value is interpolated into that + // client's error messages and into this class's audit line. Host equality alone could + // never match such a value, so the declared-validator branch below is what would make it + // reachable β€” this refusal is the reason that branch cannot open a disclosure path. + // + // Requiring host-equality (not merely "parses to a host") also keeps this in step with + // SelfUpdateOptions.PortalImage/MigrationImage, which interpolate the SAME value into an + // image reference and need a bare host for it to be well-formed. + var registry = SelfUpdateOptions.HostOf(configured); + if (registry is null) + // 🚨 The value is NOT echoed: this is exactly the branch a userinfo-bearing value lands + // in, and the userinfo is where a key would be. + return Observable.Throw>(new InvalidOperationException( + "SelfUpdate:Registry does not name an http(s) registry host. It must be a bare " + + "'host' or 'host:port' (for example cr.meshweaver.cloud) β€” never a value carrying " + + "credentials, and never one this platform cannot also use as the registry half of " + + "an image reference. The configured value is not repeated here because a value of " + + "this shape can carry a credential.")); + if (!string.Equals(registry, configured, StringComparison.OrdinalIgnoreCase)) + // Safe to name: HostOf refuses userinfo, so a value that parsed cannot have carried one. + return Observable.Throw>(new InvalidOperationException( + $"SelfUpdate:Registry must be a bare registry host, but it carries a scheme or a path; " + + $"it names the host '{registry}'. Set SelfUpdate:Registry to '{registry}' β€” the same " + + "value is interpolated into the portal and migration image references, which are " + + "malformed with anything else.")); + return new OciRegistryClient( hub, registry, ResolveCredential(registry), options.RegistryUsername, HttpClientName, logger) .ListTags(repository); diff --git a/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md b/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md index 508343d9a6..c649c45613 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md @@ -12,17 +12,31 @@ already holds for a plugin registry. WHICH plugin registry's key is a disclosure decided by DECLARED PAIRINGS β€” never by host equality, never by name resemblance, and never by "there is only one key, so use it".** -The rule, in `OciTagLister.ResolveCredential`: +🚨 **Two different questions, and confusing them is how this page would mislead.** The key is always +**SENT TO** `SelfUpdate:Registry` β€” the container registry β€” because that is the host being listed. +What the rule below decides is **WHICH KEY**: which of this installation's configured +plugin-registry credentials is selected. The validator host is a *credential source*, never a +destination; nothing is ever sent to it by the lister. -| The key is presented to… | …because | -|---|---| -| the container registry's **own host**, when a plugin registry is configured there | the operator already gave this installation a key for that exact host β€” a portal serving its images from the `/v2` mirror it also serves its catalog from | -| the host named by **`SelfUpdate:RegistryValidationUrl`**, when a plugin registry is configured there | the operator declared that portal as the one that *validates* this installation's key at that registry, AND gave this installation a key for it | -| **nothing else, ever** | an absent declaration is a refusal, not permission | +So, in `OciTagLister.ResolveCredential` β€” **selecting** the credential: + +| The key selected is the one held for… | …because | the key is sent to | +|---|---|---| +| the container registry's **own host**, when a plugin registry is configured there | the operator already gave this installation a key for that exact host β€” a portal serving its images from the `/v2` mirror it also serves its catalog from | the container registry | +| the host named by **`SelfUpdate:RegistryValidationUrl`**, when a plugin registry is configured there | the operator declared that portal as the one that *validates* this installation's key at that registry, AND gave this installation a key for it | the container registry β€” **still**, never the validator | +| **nothing else, ever** | an absent declaration is a refusal, not permission | nothing is sent | Both rows need TWO explicit statements. A declaration alone grants nothing (there is still no key for that host); a key alone grants nothing (nothing says that registry trusts that portal). +**And the destination itself is validated first.** `SelfUpdate:Registry` must already be a bare +`host[:port]`: `OciRegistryClient` builds `https://{registry}/` for a value with no scheme, so +`instance:mwi_…@evil.example` would be a valid URI whose host is `evil.example` and would receive the +Basic credential. Host equality could never match such a value, so it is the declared-validator row +that would otherwise make it reachable β€” the target check is what keeps that row from opening a +disclosure path. A value that does not parse to an http(s) host is refused **without being echoed**, +because the userinfo is where a key would be. + ## Why host equality was wrong The fleet's registry deliberately does **not** hold credentials. `cr.meshweaver.cloud` decides a pull diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-on-the-fleet-registry-can-self-update-again.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-can-be-told-which-portal-validates-its-registry-key.md similarity index 75% rename from src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-on-the-fleet-registry-can-self-update-again.md rename to src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-can-be-told-which-portal-validates-its-registry-key.md index 59a53a5ba5..370e347b64 100644 --- a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-on-the-fleet-registry-can-self-update-again.md +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-can-be-told-which-portal-validates-its-registry-key.md @@ -1,16 +1,20 @@ --- -Name: An instance on the fleet registry can self-update again +Name: An instance can be told which portal validates its registry key Category: Fix Description: An installation pulling its images from cr.meshweaver.cloud booted normally and then never updated itself, because the self-updater would only present its instance key to a registry that was also its plugin registry. It now presents it to the portal the registry DECLARES as its validator β€” and still refuses every host nobody declared. Icon: RefreshCcw Order: -20260912 --- -# An instance on the fleet registry can self-update again +# An instance can be told which portal validates its registry key + +> **This adds the setting; it does not switch anything on.** An affected instance keeps refusing +> until an operator declares the pairing on it, and an instance that declares nothing behaves +> exactly as before. An installation that pulls its platform images from the fleet's own registry -(`cr.meshweaver.cloud`) started, served, pulled its image in seconds β€” and then never updated itself. -Every check failed the same way, whatever its update policy said: +(`cr.meshweaver.cloud`) starts, serves, pulls its image in seconds β€” and then never updates itself. +Every check fails the same way, whatever its update policy says: ``` [SelfUpdate] check (Startup): check FAILED: SelfUpdate:Registry is 'cr.meshweaver.cloud' … @@ -31,6 +35,10 @@ side: `SelfUpdate:RegistryValidationUrl` (chart: `selfUpdate.registryValidationU instance, a record's `extraPortalConfig`). Set it to the registry record's validation URL β€” a bare host works too β€” and the self-updater presents the key it already holds for that portal. +`SelfUpdate:Registry` must also be a bare `host` or `host:port`: the same value is used as the +registry half of an image reference, and a value shaped like `user:secret@host` would be a valid URI +naming a different host as the one to hand the instance key to. + **It is a grant, and nothing else is one.** The key still goes only to a host that is a configured plugin registry, or to a registry whose declared validator is one. An absent declaration **refuses**, exactly as before β€” it is never read as permission β€” and hosts are compared whole: no suffix, no diff --git a/test/Memex.Portal.Shared.Test/OciTagListerTest.cs b/test/Memex.Portal.Shared.Test/OciTagListerTest.cs index 1385c908d6..a915234dc1 100644 --- a/test/Memex.Portal.Shared.Test/OciTagListerTest.cs +++ b/test/Memex.Portal.Shared.Test/OciTagListerTest.cs @@ -222,6 +222,53 @@ public async Task ADeclaredValidatorThisInstallationHoldsNoKeyFor_IsRefused() "the key is only ever presented to a host this installation was issued one for"); } + /// + /// 🚨 NEGATIVE CONTROL for the TARGET, not the declaration (Copilot review on #4094). A + /// `SelfUpdate:Registry` carrying userinfo is a valid URI whose host is the LAST one β€” + /// `OciRegistryClient` would build `https://instance:mwi_…@evil…/` and hand it the Basic + /// credential β€” and the raw value is interpolated into that client's errors and this class's + /// audit line. Host equality could never match such a value, so it is the declared-validator + /// branch that would make it reachable; this refusal is what stops that branch opening a + /// disclosure path. The refusal must ALSO not echo the value, which is where the key would be. + /// + [Fact(Timeout = 120_000)] + public async Task ARegistryTargetCarryingUserinfo_IsRefused_AndNeverEchoed() + { + var ct = TestContext.Current.CancellationToken; + const string smuggled = "instance:mwi_smuggled-in-the-registry-value"; + + var fault = await Assert.ThrowsAsync(() => + Lister($"{smuggled}@evil.example.test", DeclaredValidator) + .ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); + + fault.Message.Should().NotContain(smuggled, + "the refusal must not echo a value of this shape β€” the userinfo is where a key would be"); + fault.Message.Should().NotContain("mwi_"); + fault.Message.Should().Contain("SelfUpdate:Registry"); + mirror.Requests.Should().Be(0); + mirror.PresentedSecret.Should().BeNull( + "a declared validator must never make an unparseable target reachable"); + } + + /// + /// The same normalization closes an ASYMMETRY: the target was compared raw against plugin + /// hosts parsed by HostOf, so a URL form missed a registry that IS the same host. It is + /// refused rather than silently accepted, because the identical value is interpolated into the + /// portal and migration image references, which a URL form makes malformed. + /// + [Fact(Timeout = 120_000)] + public async Task AUrlFormRegistryTarget_IsRefused_NamingTheBareHostToUse() + { + var ct = TestContext.Current.CancellationToken; + + var fault = await Assert.ThrowsAsync(() => + Lister($"https://{MirrorHost}").ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); + + fault.Message.Should().Contain(MirrorHost, "the message names the value to set instead"); + fault.Message.Should().Contain("bare registry host"); + mirror.Requests.Should().Be(0); + } + /// The declaration is read WHOLE-HOST or not at all β€” never a suffix, a registrable /// domain or anything else that could make a coincidence of naming look like a grant. [Fact] From 229605fe9655e27008ea71edcb0c18d5a527d00b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Sat, 12 Sep 2026 15:42:04 +0200 Subject: [PATCH 3/5] test: make the disclosure assertion capable of failing, and name the expansion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coordinator asked whether the userinfo negative asserts DISCLOSURE or only message text. It asserted both β€” and checking the claim showed one of the two was VACUOUS, which is the more useful answer. PresentedSecret is assigned only inside the /v2/token branch, reached solely for the two hosts the fake serves. A request to evil.example.test was 404'd at the host check before any secret was recorded, so "PresentedSecret is null" held whatever the code did β€” an assertion that could not fail for exactly the host the negatives exist to catch. Only Requests == 0 carried weight. Proven rather than reasoned: with the target guard deleted, the isolated assertion still PASSED. The cause is the protocol β€” the OCI client presents Basic only AFTER a 401 challenge, and a fake that answers 404 can never observe a leak. An unknown host now behaves like a HOSTILE REGISTRY: it challenges, serves /v2/token, and records whatever secret it is handed, before the host check. The same experiment now fails with "found 2 item(s)" β€” Basic at the token realm and the Bearer on the listing. So the key really would have gone to evil.example.test, and the control now sees it. Every negative asserts CredentialsSeen. Doc page gains what a security page owes its reader: * the TRUST EXPANSION, stated plainly β€” before, the key could reach only a host with a configured plugin registry; after, any host some configured plugin registry is declared to validate β€” with the three bounds that hold it, each load-bearing rather than defence in depth. * the ATTACK attached to the bare-host requirement, so nobody later simplifies it back to HostOf(registry) and reopens the hole: the fix introduced the vulnerability it exists to prevent. * both false passes, because the reusable lesson is that a negative control must be proven to have RUN, not merely to be green. Refs #4093 Co-Authored-By: Claude Opus 5 --- .../SelfUpdateRegistryCredential.md | 57 ++++++++++++++++--- .../OciTagListerTest.cs | 48 ++++++++++++---- 2 files changed, 87 insertions(+), 18 deletions(-) diff --git a/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md b/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md index c649c45613..2902cf8529 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md @@ -29,13 +29,41 @@ So, in `OciTagLister.ResolveCredential` β€” **selecting** the credential: Both rows need TWO explicit statements. A declaration alone grants nothing (there is still no key for that host); a key alone grants nothing (nothing says that registry trusts that portal). -**And the destination itself is validated first.** `SelfUpdate:Registry` must already be a bare -`host[:port]`: `OciRegistryClient` builds `https://{registry}/` for a value with no scheme, so -`instance:mwi_…@evil.example` would be a valid URI whose host is `evil.example` and would receive the -Basic credential. Host equality could never match such a value, so it is the declared-validator row -that would otherwise make it reachable β€” the target check is what keeps that row from opening a -disclosure path. A value that does not parse to an http(s) host is refused **without being echoed**, -because the userinfo is where a key would be. +## 🚨 This IS a trust expansion β€” say so + +**Before this change, the instance key could reach only a host that had a plugin registry configured +on it. After it, the key can reach ANY host, provided some configured plugin registry is declared to +be that host's validator.** That is the feature working as designed β€” the fleet's registry is exactly +such a host β€” and it is fail-closed. But it widens who can receive a credential, and three things +bound it: + +1. **An explicit declaration.** `SelfUpdate:RegistryValidationUrl` must name the validator. Absent, + nothing changes; absence is never permission, and no resemblance of names substitutes for it. +2. **A bare-host target.** `SelfUpdate:Registry` must already be `host[:port]` (below). +3. **A plugin registry actually configured at the declared validator.** A declaration for a host this + installation holds no key for grants nothing β€” there is no key to select. + +Remove any one of the three and the expansion becomes unbounded. They are not defence in depth; each +one is load-bearing. + +### The destination is validated first, and here is the attack that requires it + +**`SelfUpdate:Registry` must already be a bare `host[:port]`.** `OciRegistryClient` builds +`https://{registry}/` for a value with no scheme, so `instance:mwi_…@evil.example` is a *valid* URI +whose host is `evil.example` β€” it receives the Basic credential, and the raw value is interpolated +into a dozen of that client's error messages besides. + +🚨 **The fix introduced the vulnerability it exists to prevent.** Host equality could never match +`instance:mwi_…@evil.example`, so the old code always refused it. The declared-validator rule matches +on the *declared* host and never looks at the *target* β€” so the new trust path made a previously +unreachable value reachable. Measured, not argued: with the target check deleted, the test fixture's +attacker host received **two** credentials (Basic at the token realm, then the Bearer). + +That is why the requirement is host-EQUALITY and not merely "parses to a host". Do not simplify it +back to `HostOf(registry)`: normalizing silently would accept a URL form here while +`PortalImage`/`MigrationImage` β€” which interpolate the same value β€” stayed malformed, and the +constraint would have lost the reason it exists. A value that does not parse to an http(s) host is +refused **without being echoed**, because the userinfo is where a key would be. ## Why host equality was wrong @@ -122,6 +150,21 @@ produce a line. loosening the host comparison turns this green, which is exactly the point. - **a declared validator this installation holds no key for** β€” refused. A declaration alone is not a grant. +- **a target carrying userinfo** β€” refused, and the refusal does not echo the value. + +🚨 **A negative control must be proven to have RUN, not merely to be green.** Both traps were hit +while writing these, and both produced a confident pass: + +- A falsification patch that **did not compile**, run with `--no-build`, reported `11/11 passed` off + the stale assembly β€” "I did not check" wearing the costume of "I checked and it was fine". +- The disclosure assertion was at first **vacuous**: the fake answered an unknown host with `404`, + and the OCI client only presents Basic *after* a `401` challenge, so no credential could ever be + observed leaking and the assertion passed by construction. The fake now makes an unknown host + behave like a **hostile registry** β€” it challenges, serves `/v2/token`, and records whatever + secret it is handed. That is what turns "no credential was seen" from a tautology into a test. + +So each negative here was checked by deleting the guard, rebuilding for real, and confirming it goes +red for the *right* reason. ## See also diff --git a/test/Memex.Portal.Shared.Test/OciTagListerTest.cs b/test/Memex.Portal.Shared.Test/OciTagListerTest.cs index a915234dc1..46e305d95d 100644 --- a/test/Memex.Portal.Shared.Test/OciTagListerTest.cs +++ b/test/Memex.Portal.Shared.Test/OciTagListerTest.cs @@ -140,7 +140,7 @@ public async Task NoPluginRegistryOnThatHost_FaultsNamingTheHost() fault.Message.Should().Contain("PluginCatalog", "the fix is a configuration change, and the message has to say which one"); mirror.Requests.Should().Be(0, "a listing with no credential could only ever be a 401"); - mirror.PresentedSecret.Should().BeNull( + mirror.CredentialsSeen.Should().BeEmpty( "the guard is a disclosure control: an arbitrary host named in SelfUpdate:Registry must " + "never be handed this installation's instance key"); } @@ -197,7 +197,7 @@ public async Task TheSameRegistry_WithNoDeclaredValidator_IsStillRefused() fault.Message.Should().Contain("SelfUpdate:RegistryValidationUrl", "an absent declaration is refused AND the message says what would declare it"); mirror.Requests.Should().Be(0); - mirror.PresentedSecret.Should().BeNull( + mirror.CredentialsSeen.Should().BeEmpty( "an undeclared pairing is not permission β€” the key never goes out"); } @@ -218,7 +218,7 @@ public async Task ADeclaredValidatorThisInstallationHoldsNoKeyFor_IsRefused() fault.Message.Should().Contain("someone-elses-portal.example.test"); fault.Message.Should().Contain("PluginCatalog:Registries"); mirror.Requests.Should().Be(0); - mirror.PresentedSecret.Should().BeNull( + mirror.CredentialsSeen.Should().BeEmpty( "the key is only ever presented to a host this installation was issued one for"); } @@ -246,8 +246,10 @@ public async Task ARegistryTargetCarryingUserinfo_IsRefused_AndNeverEchoed() fault.Message.Should().NotContain("mwi_"); fault.Message.Should().Contain("SelfUpdate:Registry"); mirror.Requests.Should().Be(0); - mirror.PresentedSecret.Should().BeNull( - "a declared validator must never make an unparseable target reachable"); + mirror.CredentialsSeen.Should().BeEmpty( + "a declared validator must never make an unparseable target reachable β€” and this " + + "assertion is recorded before the fake's host check, so it can see a credential sent " + + "to evil.example.test, which PresentedSecret by construction could not"); } /// @@ -267,6 +269,7 @@ public async Task AUrlFormRegistryTarget_IsRefused_NamingTheBareHostToUse() fault.Message.Should().Contain(MirrorHost, "the message names the value to set instead"); fault.Message.Should().Contain("bare registry host"); mirror.Requests.Should().Be(0); + mirror.CredentialsSeen.Should().BeEmpty(); } /// The declaration is read WHOLE-HOST or not at all β€” never a suffix, a registrable @@ -375,21 +378,44 @@ private sealed class FakeMirror : HttpMessageHandler public string? PresentedSecret; private ImmutableList servedHosts = ImmutableList.Empty; + private ImmutableList credentialsSeen = ImmutableList.Empty; /// Every host a request actually reached β€” the listing must go to the CONTAINER /// registry, never to the portal whose key authenticates it. public ImmutableList ServedHosts => servedHosts; + /// + /// 🚨 Every host that was sent ANY credential, recorded BEFORE the host check β€” which is + /// what makes the disclosure assertions non-vacuous. is only + /// assigned inside the /v2/token branch, reached solely for the two hosts this fake + /// serves, so on its own it would read null even if the key HAD been sent to + /// evil.example.test β€” an assertion that cannot fail for exactly the host the + /// negatives exist to catch. This one can. + /// + public ImmutableList CredentialsSeen => credentialsSeen; + protected override Task SendAsync( HttpRequestMessage request, CancellationToken cancellationToken) { Interlocked.Increment(ref Requests); var uri = request.RequestUri!; - // Both fleet shapes on one fake: the portal that serves its own /v2 mirror, and a - // container registry on its own host whose key is validated by that portal. - if (uri.Host is not (MirrorHost or RegistryHost)) - return Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound)); - ImmutableInterlocked.Update(ref servedHosts, current => current.Add(uri.Host)); + if (request.Headers.Authorization is { } offered) + ImmutableInterlocked.Update(ref credentialsSeen, + current => current.Add($"{offered.Scheme} to {uri.Host}")); + + // 🚨 AN UNKNOWN HOST BEHAVES LIKE A HOSTILE REGISTRY, NOT LIKE A 404. + // + // This is what makes the disclosure assertions real. The client presents Basic only + // AFTER a 401 Bearer challenge, so a fake that answered 404 here could never observe a + // leaked credential β€” and a negative asserting "no credential was seen" would pass by + // construction whatever the code did. (Measured: with the target guard deleted, that + // shape of fixture reported a clean PASS.) A real attacker-controlled host issues the + // challenge, so this one does too: it challenges, it serves /v2/token, and it records + // whatever secret is handed over. Only the tags listing stays restricted to the two + // hosts this fixture legitimately serves. + var known = uri.Host is MirrorHost or RegistryHost; + if (known) + ImmutableInterlocked.Update(ref servedHosts, current => current.Add(uri.Host)); if (uri.AbsolutePath == "/v2/token") { @@ -411,7 +437,7 @@ protected override Task SendAsync( if (request.Headers.Authorization is not { Scheme: "Bearer" } auth || auth.Parameter != InstanceKey) return Task.FromResult(Challenge(uri.Host)); - if (uri.AbsolutePath == $"/v2/{Repository}/tags/list") + if (known && uri.AbsolutePath == $"/v2/{Repository}/tags/list") { Interlocked.Increment(ref PagesServed); var query = System.Web.HttpUtility.ParseQueryString(uri.Query); From 56f2022601c2795646727d13f3ef6ec618633d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Sat, 12 Sep 2026 15:47:05 +0200 Subject: [PATCH 4/5] docs: audit every negative for vacuity, and lead with the measurement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A vacuous negative is a CLASS, and the userinfo case gave up its mechanism: a fake that refuses early can never observe what it was built to catch. So each negative in this change was audited against that one question β€” not "does it assert the right property", which is the question that missed it, but "could this assertion FAIL?" β€” by deleting the guard it protects, stripping the test to the assertion under audit, and measuring what reported. TWO OF FIVE shared the shape, and the hostile-host fixture fixed both: no plugin registry on that host -> 2 credentials WAS VACUOUS (404'd early) same registry, nothing declared -> 3 credentials sound (served host) declared validator, no key held -> 3 credentials sound (served host) target carrying userinfo -> 2 credentials WAS VACUOUS (the trigger) URL-form target -> 4 requests sound; blocks the HostOf-without-equality simplification specifically "no plugin registry on that host" is the one worth noting: it predates this PR and had the identical defect β€” an unknown host was 404'd before a credential could be observed, so its disclosure assertion had never been able to fail. It only became a real control when the fake learned to challenge. The two non-fake negatives cannot take this shape: one asserts values from a pure function with no fake in the path, the other drives a host the fake does serve and asserts a throw, which an empty-list regression could not satisfy. The doc page now LEADS with the measurement β€” with the target check removed the instance key reaches an arbitrary host, twice β€” because that is what makes the bare-host requirement non-negotiable. The trust-expansion prose supports it rather than leading. The same audit is owed across the wider suite. That is its own piece of work and is deliberately not folded in here. Refs #4093 Co-Authored-By: Claude Opus 5 --- .../SelfUpdateRegistryCredential.md | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md b/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md index 2902cf8529..e35e665b8e 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md @@ -29,7 +29,18 @@ So, in `OciTagLister.ResolveCredential` β€” **selecting** the credential: Both rows need TWO explicit statements. A declaration alone grants nothing (there is still no key for that host); a key alone grants nothing (nothing says that registry trusts that portal). -## 🚨 This IS a trust expansion β€” say so +## 🚨 Measured: with the target check removed, the instance key reaches an arbitrary host β€” twice + +Not an argument, a measurement. Delete the bare-host requirement, point `SelfUpdate:Registry` at +`instance:mwi_…@evil.example.test`, declare any validator this installation holds a key for, and the +test fixture's attacker host receives **two credentials**: the Basic credential at the token realm it +names, then the Bearer on the listing. `OciRegistryClient` builds `https://{registry}/` for a value +with no scheme, so that string is a *valid* URI whose host is `evil.example.test`. + +**That is why the bare-host requirement is not negotiable, and why it is host-EQUALITY rather than +"parses to a host".** Everything below explains the shape; this is the reason it exists. + +## This IS a trust expansion β€” say so **Before this change, the instance key could reach only a host that had a plugin registry configured on it. After it, the key can reach ANY host, provided some configured plugin registry is declared to @@ -163,8 +174,28 @@ while writing these, and both produced a confident pass: behave like a **hostile registry** β€” it challenges, serves `/v2/token`, and records whatever secret it is handed. That is what turns "no credential was seen" from a tautology into a test. -So each negative here was checked by deleting the guard, rebuilding for real, and confirming it goes -red for the *right* reason. +### The vacuity audit β€” ask "could this assertion FAIL?", not "does it assert the right property" + +A vacuous negative is a **class**, and its mechanism is now known: *a fake that refuses early can +never observe what it was built to catch.* So every negative here was audited against that one +question, by deleting the guard it protects, stripping the test to the assertion under audit, and +measuring what it reported. **Two of the five shared the shape**, and the hostile-host fixture fixed +both: + +| negative | guard deleted β‡’ the assertion reports | was it vacuous? | +|---|---|---| +| no plugin registry on that host | **2 credentials** at `other.example.test` | **YES** β€” an unknown host was 404'd before any credential could be observed. Same shape as the userinfo one; fixed by the same change | +| the same registry, nothing declared | **3 credentials** | no β€” the target is a host the fake serves, so the challenge always happened | +| a declared validator holding no key | **3 credentials** | no β€” same reason | +| a target carrying userinfo | **2 credentials** at `evil.example.test` | **YES** β€” the case that started this audit | +| a URL-form target | **4 requests** under the `HostOf`-without-equality simplification | no β€” and it is precisely what blocks that simplification | + +The two non-fake negatives cannot take this shape at all: the validator-host test asserts values +returned by a pure function with no fake in the path, and the refused-key test drives a host the fake +*does* serve and asserts that an exception is thrown, which an empty-list regression could not +satisfy. + +**The same audit is owed across the wider suite; it is not part of this change.** ## See also From adbb963eb417df7ef6631d55db5f07da6fa17800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= <6334612+rbuergi@users.noreply.github.com> Date: Sat, 12 Sep 2026 21:15:22 +0200 Subject: [PATCH 5/5] fix: present the DURABLE key to a declared validator, and never log the registry value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2 of #4094 (hand-run), in the reviewer's order: 1. The self-updater's boot line logged SelfUpdate:Registry verbatim at Information β€” a `user:mwi_…@evil` value shipped the key to Loki at boot, before the lister's non-echoing refusal ever ran. It now logs the HOST HostOf reads, or "(unreadable β€” see SelfUpdate:Registry)". No other read of Registry / RegistryValidationUrl reaches a log, message, node or report. 2. The declared-validator branch presented whatever ResolveToken yields; for an auto-registered instance (PluginCatalog:BootstrapKey, no Token) that is the stored key EXCHANGED for an mwa_ JWT β€” and the declared validator IS /api/instances/token, which refuses a token by shape, so only build/pearl (raw Token) were fixed. RegistryTokenResolver.ResolveDurableKey presents the configured token, else the stored key, with no exchange; ResolveToken shares its one ConfiguredToken rule and differs only in exchanging the STORED key. 3. A plugin registry whose URL carries credentials is refused NAMING THE CAUSE and the fix (Registries:N:Token), URL not echoed β€” never diagnosed as "no plugin registry is configured on that host". 4. A declaration that is set but unreadable is refused as MALFORMED, never folded into "not declared"; SelfUpdateOptions.RegistryValidatorDeclared is the predicate, and the boot line has three states. 5. A bare host:443 is accepted: "bare" is a textual test (no scheme, no path), not equality with the normalized host, and the refusal message is true. 6. FakeMirror.ReachedHosts records every host reached, unconditionally, before the fake decides what it serves; ServedHosts (known hosts only) is gone. 7. A two-registry positive pins that the key presented is the one held for the DECLARED validator, not the first configured registry's. 8. RegistryUpdateReconciler.SameRegistry reads through SelfUpdateOptions.HostOf β€” one registry-host rule for both subsystems; the theory pins the cases. 9. What's New icon: ArrowSync (Fluent), not RefreshCcw (Lucide). 10. The dead HostOf(match.Url) fallback is gone; the matched host is carried. 11. The render-time derivation of the pairing is filed as #4123, not done. Every negative asserts the disclosure FIRST through Record.ExceptionAsync, so a falsification reports what leaked. Every falsification was built under -warnaserror before it ran (the first attempt did not compile and was voided, not replayed): each fired β€” the auto-registered shape reports one exchange under ResolveToken, the two-registry positive reports A's key under registries.First(), the third-host injection is seen by all three paired positives, the boot line shows the smuggled value when reverted. Verified: MeshWeaver.Hosting, Memex.Portal.Shared, Memex.Portal.Shared.Test each 0 Error(s) on -c Release -warnaserror; lister + SameRegistry 32/32; Memex.Portal.Shared.Test 1397/1397; MeshWeaver.Documentation.Test 432/432 with the doc guards executed by name. Refs #4093 β€” deliberately not Fixes: the declaration on build/pearl is a config-repo change and the issue closes on it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BRLdhAiX5DjdZC2WUfPg9E --- .../SelfUpdate/OciTagLister.cs | 134 +++++- .../SelfUpdate/SelfUpdateHostedService.cs | 23 +- .../SelfUpdateRegistryCredential.md | 111 ++++- ...which-portal-validates-its-registry-key.md | 11 +- .../SelfUpdate/SelfUpdateOptions.cs | 27 +- .../InstanceAutoRegistrationService.cs | 40 +- .../RegistryUpdateReconciler.cs | 24 +- .../OciTagListerTest.cs | 447 ++++++++++++++++-- .../RegistryUpdateReconcilerTest.cs | 11 +- 9 files changed, 716 insertions(+), 112 deletions(-) diff --git a/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs b/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs index bc572e76f9..6b95620f6e 100644 --- a/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs +++ b/memex/Memex.Portal.Shared/SelfUpdate/OciTagLister.cs @@ -79,7 +79,7 @@ public IObservable> ListTags(string repository) // never match such a value, so the declared-validator branch below is what would make it // reachable β€” this refusal is the reason that branch cannot open a disclosure path. // - // Requiring host-equality (not merely "parses to a host") also keeps this in step with + // Requiring a bare host (not merely "parses to a host") also keeps this in step with // SelfUpdateOptions.PortalImage/MigrationImage, which interpolate the SAME value into an // image reference and need a bare host for it to be well-formed. var registry = SelfUpdateOptions.HostOf(configured); @@ -92,7 +92,13 @@ public IObservable> ListTags(string repository) + "credentials, and never one this platform cannot also use as the registry half of " + "an image reference. The configured value is not repeated here because a value of " + "this shape can carry a credential.")); - if (!string.Equals(registry, configured, StringComparison.OrdinalIgnoreCase)) + // 🚨 "Bare" is a TEXTUAL test on the configured value, NOT equality with the host HostOf + // read from it: HostOf drops a scheme-default port, so `cr.example.test:443` β€” a legal + // `host:port` the documentation promises β€” would fail an equality test and be refused with + // a message claiming it carries a scheme or a path when it carries neither (#4094 review). + // A value that parsed (so: no userinfo) and carries no scheme and no path IS a bare host, + // whatever port it names; the client and the messages then use the normalized host. + if (!IsBareHost(configured)) // Safe to name: HostOf refuses userinfo, so a value that parsed cannot have carried one. return Observable.Throw>(new InvalidOperationException( $"SelfUpdate:Registry must be a bare registry host, but it carries a scheme or a path; " @@ -105,6 +111,15 @@ public IObservable> ListTags(string repository) .ListTags(repository); } + /// + /// Whether a value that could read is ALSO nothing but + /// host[:port]: no scheme, no path, no query, no fragment. Userinfo is not tested here + /// because HostOf already refused it. Pure. + /// + private static bool IsBareHost(string configured) => + !configured.Contains("://", StringComparison.Ordinal) + && configured.IndexOfAny(['/', '\\', '?', '#']) < 0; + /// /// 🚨 The plugin-registry credential this installation may present to the container registry β€” /// and the DISCLOSURE CONTROL that decides which one, because the answer is an instance key and @@ -128,20 +143,46 @@ public IObservable> ListTags(string repository) /// need the operator to have configured a plugin registry on the host in question, so the key /// only ever goes where this installation was already given one for. /// - /// Faults, naming the hosts (never the key), when no host qualifies or when the resolved - /// credential is empty: a listing without a credential could only ever be a 401, so the - /// misconfiguration is said once here instead of on every check as a refused handshake. Cold: - /// the fault is raised at subscription, before the client sends a byte. + /// The SHAPE of the credential follows the host: the registry's own host gets what the + /// Store presents ( β€” a short-lived token when + /// the stored key was exchanged for one); a declared validator gets the DURABLE key + /// (), because on the fleet's shape the + /// validator IS the key-to-token exchange and refuses a token by design. + /// + /// Faults, naming the hosts (never the key), when the declaration is set but unreadable, + /// when no host qualifies, when the only registry on a qualifying host carries its credential + /// inside its URL, or when the resolved credential is empty: a listing without a credential + /// could only ever be a 401, so the misconfiguration is said once here instead of on every + /// check as a refused handshake. Cold: the fault is raised at subscription, before the client + /// sends a byte. /// private IObservable ResolveCredential(string registry) => Observable.Defer(() => { + // 🚨 A DECLARED-BUT-UNREADABLE validator is refused as MALFORMED, first and on its own β€” + // never folded into "nothing declared". `RegistryValidatorHost` is null for both, and + // treating the two alike diagnosed a typo'd scheme or a userinfo-bearing declaration + // as an ABSENT one: the boot line said "NO validator declared" and the refusal told the + // operator to set the key they had already set (#4094 review). The value is not echoed β€” + // a userinfo-bearing value lands exactly here, and the userinfo is where a key would be. + if (options.RegistryValidatorDeclared && options.RegistryValidatorHost is null) + return Observable.Throw(new InvalidOperationException( + "SelfUpdate:RegistryValidationUrl is set but does not name an http(s) host, so it " + + "declares no validator and the listing refuses. Set it to the registry record's " + + "validationUrl verbatim (for example https://memex.meshweaver.cloud/api/instances/token) " + + "or to a bare host β€” never a value carrying credentials. The configured value is " + + "not repeated here because a value of this shape can carry a credential.")); + var catalog = hub.ServiceProvider.GetService() ?? new PluginCatalogOptions(); var registries = RegistryTokenResolver.WithLegacyTokens(catalog, catalog.EffectiveRegistries); var declaredValidator = options.RegistryValidatorHost; var match = On(registries, registry); - if (match is null && declaredValidator is not null && On(registries, declaredValidator) is { } paired) + // The HOST the match was made on β€” the parsed host, never `match.Url`, which may carry + // userinfo; every message and log line below names this and nothing else. + var matchedHost = registry; + var paired = false; + if (match is null && declaredValidator is not null && On(registries, declaredValidator) is { } viaValidator) { // 🚨 The new, security-relevant decision, said out loud once per listing β€” and this // is an Information line, so it LEAVES THE POD for Loki. It therefore carries TWO @@ -149,22 +190,48 @@ private IObservable ResolveCredential(string registry) => // not the key, not a prefix or suffix of it, not its length, not a hash of it. // Do not add one "for diagnostics" β€” a length is a fact about a credential. // - // 🚨 And the hosts are the PARSED hosts, never `paired.Url`. A configured URL may - // carry userinfo (`https://instance:mwi_…@memex.meshweaver.cloud`), which would put - // the key in Loki in a field nobody thinks of as a credential β€” the #3201 shape, - // where the registry key sat in a pod spec `kubectl describe` printed and is still - // owed a rotation. `declaredValidator` IS this registry's host by construction: it - // is what `On(...)` just matched it by. + // 🚨 And the hosts are the PARSED hosts, never `viaValidator.Url`. A configured URL + // may carry userinfo (`https://instance:mwi_…@memex.meshweaver.cloud`), which would + // put the key in Loki in a field nobody thinks of as a credential β€” the #3201 + // shape, where the registry key sat in a pod spec `kubectl describe` printed and is + // still owed a rotation. `declaredValidator` IS this registry's host by + // construction: it is what `On(...)` just matched it by. logger?.LogInformation( "[SelfUpdate] presenting the instance key configured under PluginCatalog:Registries " + "for {PluginRegistryHost} to the container registry {Registry}: " + "SelfUpdate:RegistryValidationUrl declares that host as the portal which validates " - + "this installation's key there.", + + "this installation's key there. The durable key is presented, unexchanged β€” the " + + "validator IS the key-to-token exchange and refuses a token.", declaredValidator, registry); - match = paired; + match = viaValidator; + matchedHost = declaredValidator; + paired = true; } if (match is null) + { + // 🚨 A plugin registry that EXISTS on the host but whose URL carries credentials is + // named as such, never diagnosed as "no plugin registry is configured on that host". + // HostOf refuses userinfo (so `https://memex.meshweaver.cloud@evil.example` can never + // be read as memex), which also means a registry configured as + // `https://instance:mwi_…@host` stops matching on BOTH branches β€” the pre-#4094 + // reader tolerated it. The registry is real and the catalog is already talking to + // it; what is wrong is WHERE the credential sits, and the message says so. The URL + // is not echoed: the credential is in it. + var credentialInUrl = registries.FirstOrDefault(r => + CarriesCredentialsFor(r.Url, registry) + || (declaredValidator is not null && CarriesCredentialsFor(r.Url, declaredValidator))); + if (credentialInUrl is not null) + return Observable.Throw(new InvalidOperationException( + $"SelfUpdate:Registry is '{registry}', an OCI registry the self-updater authenticates " + + "with this installation's plugin-registry instance key. A plugin registry IS configured " + + "for that host or for its declared validator, but its PluginCatalog:Registries URL " + + "carries credentials (user:secret@host), and a URL of that shape is never read as " + + "naming a host β€” the credential is where a human misreads the host. Move the key " + + "to PluginCatalog:Registries:N:Token and set the URL to the bare " + + "https://host; the registry then qualifies. The URL is not repeated here because " + + "the credential is in it.")); + return Observable.Throw(new InvalidOperationException( $"SelfUpdate:Registry is '{registry}', an OCI registry the self-updater authenticates " + "with this installation's plugin-registry instance key β€” but no plugin registry " @@ -181,15 +248,31 @@ private IObservable ResolveCredential(string registry) => + "is no key to present. Add that host to PluginCatalog:Registries with the " + "instance key issued for it. ") + "Or set SelfUpdate:Registry back to the upstream Azure Container Registry.")); + } + // 🚨 WHICH SHAPE of the credential depends on WHO validates it, and the two branches + // differ here on purpose: + // * the registry's OWN host is a portal serving its /v2 mirror, whose token endpoint + // runs the same authenticator the plugin registry does and accepts BOTH the durable + // key and the short-lived token β€” so it is handed what the Store presents + // (`ResolveToken`: the configured token, or the stored key EXCHANGED for a JWT); + // * a DECLARED validator is, on the fleet's shape, the key-to-token exchange itself + // (`/api/instances/token`), and that endpoint refuses a token by design β€” a token may + // never mint its successor. An auto-registered installation (PluginCatalog:BootstrapKey, + // no configured Token) resolving through `ResolveToken` would therefore present an + // `mwa_` JWT and be refused on EVERY check, while only an installation with a raw + // Token configured would ever work (#4094 review). So this branch presents the + // DURABLE key with no exchange β€” the one shape every validator that accepts the + // instance key accepts. var resolver = hub.ServiceProvider.GetService(); var credential = resolver is null ? Observable.Return(match.Token?.Trim() ?? string.Empty) - : resolver.ResolveToken(match); + : paired + ? resolver.ResolveDurableKey(match) + : resolver.ResolveToken(match); - // 🚨 The HOST, never `match.Url` β€” same reason as the log line above: a configured URL - // may carry userinfo, and this message is logged by the poller on every failed check. - var matchedHost = SelfUpdateOptions.HostOf(match.Url) ?? "(unreadable URL)"; + // 🚨 `matchedHost`, never `match.Url` β€” same reason as the log line above: a configured + // URL may carry userinfo, and this message is logged by the poller on every failed check. return credential.Select(token => token.Length > 0 ? token : throw new InvalidOperationException( @@ -199,6 +282,19 @@ private IObservable ResolveCredential(string registry) => + "through auto-registration with PluginCatalog:BootstrapKey.")); }); + /// + /// Whether a configured plugin-registry URL names BUT carries + /// credentials (userinfo) β€” the one shape refuses for a + /// registry that nevertheless exists and is in use. Serves the DIAGNOSIS only: nothing is ever + /// matched, selected or logged through this reader. Pure. + /// + private static bool CarriesCredentialsFor(string? url, string host) => + Uri.TryCreate((url ?? string.Empty).Trim(), UriKind.Absolute, out var uri) + && uri.UserInfo.Length > 0 + && uri.Scheme is "http" or "https" + && string.Equals(uri.IsDefaultPort ? uri.Host : $"{uri.Host}:{uri.Port}", host, + StringComparison.OrdinalIgnoreCase); + /// /// The configured plugin registry living on β€” whole-host, case /// insensitive, through the SAME host reader the declaration is parsed with, so the two sides diff --git a/memex/Memex.Portal.Shared/SelfUpdate/SelfUpdateHostedService.cs b/memex/Memex.Portal.Shared/SelfUpdate/SelfUpdateHostedService.cs index 17b2532c0d..480641cf89 100644 --- a/memex/Memex.Portal.Shared/SelfUpdate/SelfUpdateHostedService.cs +++ b/memex/Memex.Portal.Shared/SelfUpdate/SelfUpdateHostedService.cs @@ -98,17 +98,30 @@ public Task StartAsync(CancellationToken cancellationToken) _options.SafetyNetCheckInterval > TimeSpan.Zero ? _options.SafetyNetCheckInterval.ToString() : "disabled", - ShippedReleaseSeed.InstalledPlatformVersion, _options.Registry, _options.PortalRepository, + ShippedReleaseSeed.InstalledPlatformVersion, + // 🚨 The HOST read from SelfUpdate:Registry, never the configured value. This is an + // Information line, so it leaves the pod for Loki at boot β€” before the OCI lister's + // refusal (which deliberately does not echo the value) has ever run β€” and a value of + // the shape `instance:mwi_…@evil.example` would have shipped the key to Loki here on + // every start (#4094 review). A value HostOf cannot read is named as unreadable, not + // printed; the lister's first check then says what is wrong with it. + SelfUpdateOptions.HostOf(_options.Registry) ?? "(unreadable β€” see SelfUpdate:Registry)", + _options.PortalRepository, // 🚨 On the OCI path the line says WHERE the instance key may be presented, because that // is the whole configuration and its absence is invisible otherwise: an install with no // declared validator boots, serves, and only reveals the gap when the first check fails - // (#4093). Hosts only, never the key. + // (#4093). Hosts only, never the key β€” and THREE states, not two: a declaration that is + // set but unreadable is a misconfiguration to name, never "NO validator declared". UsesOciListing ? _options.RegistryValidatorHost is { } validator ? $"an OCI Distribution registry (instance-key auth; key validated at {validator})" - : "an OCI Distribution registry (instance-key auth; NO validator declared β€” the key " - + "is presented only if this host is itself a configured plugin registry, else " - + "every check refuses; see SelfUpdate:RegistryValidationUrl)" + : _options.RegistryValidatorDeclared + ? "an OCI Distribution registry (instance-key auth; SelfUpdate:RegistryValidationUrl " + + "is SET but does not name an http(s) host β€” every check refuses until it does; " + + "the value is not repeated here)" + : "an OCI Distribution registry (instance-key auth; NO validator declared β€” the key " + + "is presented only if this host is itself a configured plugin registry, else " + + "every check refuses; see SelfUpdate:RegistryValidationUrl)" : "an Azure Container Registry", _updater.CanPatch, _options.RetryInterval); diff --git a/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md b/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md index e35e665b8e..bfdc0a8dd6 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/SelfUpdateRegistryCredential.md @@ -29,6 +29,23 @@ So, in `OciTagLister.ResolveCredential` β€” **selecting** the credential: Both rows need TWO explicit statements. A declaration alone grants nothing (there is still no key for that host); a key alone grants nothing (nothing says that registry trusts that portal). +### The SHAPE of the credential follows the row + +🚨 **A declared validator is handed the DURABLE `mwi_` key, never an exchanged token.** The Store's +resolver (`RegistryTokenResolver.ResolveToken`) presents a configured `Token` as configured, but a +STORED key β€” the one an auto-registered installation holds after `PluginCatalog:BootstrapKey`, the +zero-touch shape `InstanceProvisioningPlan` documents β€” it first EXCHANGES for a short-lived `mwa_` +token at `/api/instances/token`. On the fleet's shape the declared validator **is** that exchange +endpoint, and it refuses a token by design (a token may never mint its successor; +`InstanceTokenEndpoints`). So a lister resolving through `ResolveToken` would present `mwa_…` to +`cr.meshweaver.cloud`, be refused on every check, and fix only the installations with a raw `Token` +configured β€” `build` and `pearl` β€” while the auto-registered ones stayed frozen (review of #4094). +The second row therefore resolves through `ResolveDurableKey`: the configured token, else the stored +key decrypted, **no exchange**. The first row is unchanged β€” a portal's own `/v2` mirror runs the same +authenticator the plugin registry does and accepts both shapes. `OciTagListerTest` pins it with the +auto-registered shape: the presented secret is the stored `mwi_` key and the exchange route is never +called; resolving through `ResolveToken` reports one exchange and a refused listing. + ## 🚨 Measured: with the target check removed, the instance key reaches an arbitrary host β€” twice Not an argument, a measurement. Delete the bare-host requirement, point `SelfUpdate:Registry` at @@ -70,12 +87,26 @@ on the *declared* host and never looks at the *target* β€” so the new trust path unreachable value reachable. Measured, not argued: with the target check deleted, the test fixture's attacker host received **two** credentials (Basic at the token realm, then the Bearer). -That is why the requirement is host-EQUALITY and not merely "parses to a host". Do not simplify it +That is why the requirement is a BARE HOST and not merely "parses to a host". Do not simplify it back to `HostOf(registry)`: normalizing silently would accept a URL form here while `PortalImage`/`MigrationImage` β€” which interpolate the same value β€” stayed malformed, and the constraint would have lost the reason it exists. A value that does not parse to an http(s) host is refused **without being echoed**, because the userinfo is where a key would be. +🚨 **"Bare" is a textual test on the configured value, not equality with the parsed host.** `HostOf` +drops a scheme-default port, so a check written as `HostOf(value) == value` refused `cr.example.test:443` +β€” a legal `host:port` this page and the chart promise β€” with a message claiming it "carries a scheme +or a path" (review of #4094). The check is now: no scheme, no path, no query, no fragment (userinfo +was refused a line earlier); the client and the credential match then use the normalized host. + +`SelfUpdateOptions.HostOf` is the platform's ONE registry-host rule, not this feature's. It used to +have a twin β€” `RegistryUpdateReconciler.SameRegistry` decided whether a `ModulePublished` broadcast +names a configured registry with its own reader, with different port semantics (`http://x:443` and +`https://x` were the same registry there and different hosts here). `SameRegistry` now reads through +`HostOf`, so the two subsystems agree: a scheme-default port is not part of the host, any other port +is, and a value that names no http(s) host β€” a bare non-URL, a `mailto:`, a URL carrying userinfo β€” +names no registry anywhere rather than matching a second copy of the same unreadable string. + ## Why host equality was wrong The fleet's registry deliberately does **not** hold credentials. `cr.meshweaver.cloud` decides a pull @@ -142,6 +173,28 @@ host is ever read**, and it is read whole: a non-default port is part of it, and userinfo (`https://memex.meshweaver.cloud@evil.example`) declares NOTHING rather than a pairing with the host a human would not have read. An empty value β€” the default β€” declares nothing and refuses. +🚨 **"Declared" and "readable" are two questions, and the refusal says which one failed.** A value +that is SET but names no http(s) host β€” userinfo, a typo'd scheme, a `mailto:` β€” is refused as +**malformed** (`SelfUpdate:RegistryValidationUrl is set but does not name an http(s) host`, value not +echoed), never folded into "nothing declared": collapsing the two told the operator to declare the +key they had already set, a fail-closed fallback forging a correct-looking bug (review of #4094). The +boot line has the same three states: validated at `{host}`, SET but unreadable, NO validator +declared. `SelfUpdateOptions.RegistryValidatorDeclared` is the predicate; `RegistryValidatorHost` is +the reading. + +### The alternative: derive it on the control instance β€” recorded, not done + +The pairing could be **derived at render time** instead of hand-copied: `HelmValues` already derives +`selfUpdate.registry` from the image host, and the hosting record whose `registry.host` equals it +carries the `validationUrl`. One declaration on the registry's own record, no per-consumer copies, +still no network in the update path β€” the consumer-side key stays the wire; only who writes it +changes. It would also give `PluginBundleClient.DownloadArtifact`, which today presents the same key +to whatever host the catalog advertises with no declaration at all, the same rule as the self-updater +β€” one key/host pair currently lives under two trust rules. That is a real alternative to this page's +design, and it is deliberately NOT part of #4094: it moves where a trust rule lives. +[#4123](https://github.com/Systemorph/MeshWeaver/issues/4123) carries it, ordered after the +config-repo declaration that closes #4093. + ## What the refusal says The message names the hosts and the key that would declare the pairing. It never names, echoes, @@ -150,6 +203,24 @@ only, names the plugin registry, the container registry and the declared validat installation hand its key to" is answerable from Loki, and a pairing nobody declared can never produce a line. +Two diagnoses exist so that a wrong one is never given: + +- **A plugin registry that EXISTS on the host, but whose URL carries credentials** + (`https://instance:mwi_…@memex.meshweaver.cloud`). `HostOf` refuses userinfo, so such a registry + matches on neither row β€” the pre-#4094 reader tolerated it β€” and without its own diagnosis the + refusal would claim "no plugin registry is configured on that host" about a registry the catalog is + already talking to. The message names the host, says the URL carries credentials, and names the fix + (`PluginCatalog:Registries:N:Token`, bare `https://host`); the URL is not echoed. Refuse-and-name + was chosen over matching the host: userinfo in a plugin-registry URL is never honoured as a + credential by the catalog either (the HTTP client drops it), the catalog logs `registry.Url` + verbatim on several pre-existing paths, and no record in the fleet uses the shape β€” so the right + answer is to say where the credential belongs, not to read past it. +- **The boot line names the HOST read from `SelfUpdate:Registry`, never the configured value.** It + is an `Information` line, written on every start BEFORE the lister's non-echoing refusal has ever + run β€” so the pre-review line, which logged `_options.Registry` verbatim, would have shipped + `instance:mwi_…@evil.example` to Loki at boot. A value `HostOf` cannot read is named as + `(unreadable β€” see SelfUpdate:Registry)`. `OciTagListerTest` pins it against a captured logger. + ## The negative control `OciTagListerTest` pins both directions, and the negatives are what prove the guard still guards: @@ -162,6 +233,11 @@ produce a line. - **a declared validator this installation holds no key for** β€” refused. A declaration alone is not a grant. - **a target carrying userinfo** β€” refused, and the refusal does not echo the value. +- **a declaration that is set but unreadable** β€” refused as malformed, never as absent, value not + echoed. +- **a plugin registry whose URL carries credentials** β€” refused naming the cause and the fix, never + as "no registry configured"; URL not echoed. +- **the boot line** β€” names the host, never the configured value, and names the third validator state. 🚨 **A negative control must be proven to have RUN, not merely to be green.** Both traps were hit while writing these, and both produced a confident pass: @@ -182,18 +258,29 @@ question, by deleting the guard it protects, stripping the test to the assertion measuring what it reported. **Two of the five shared the shape**, and the hostile-host fixture fixed both: -| negative | guard deleted β‡’ the assertion reports | was it vacuous? | +Every negative asserts the DISCLOSURE first (`CredentialsSeen`, recorded before the fake's host +check) and the message second, through `Record.ExceptionAsync` rather than `Assert.ThrowsAsync` β€” so +a falsification reports *what leaked*, not "no exception was thrown". Re-run in the review round of +#4094, each patch built under `-warnaserror` before it ran (a patch that did not compile is VOID, and +the runner refuses it rather than replaying the stale assembly β€” the first attempt was exactly that): + +| negative | guard deleted β‡’ the assertion reports | vacuous? | |---|---|---| -| no plugin registry on that host | **2 credentials** at `other.example.test` | **YES** β€” an unknown host was 404'd before any credential could be observed. Same shape as the userinfo one; fixed by the same change | -| the same registry, nothing declared | **3 credentials** | no β€” the target is a host the fake serves, so the challenge always happened | -| a declared validator holding no key | **3 credentials** | no β€” same reason | -| a target carrying userinfo | **2 credentials** at `evil.example.test` | **YES** β€” the case that started this audit | -| a URL-form target | **4 requests** under the `HostOf`-without-equality simplification | no β€” and it is precisely what blocks that simplification | - -The two non-fake negatives cannot take this shape at all: the validator-host test asserts values -returned by a pure function with no fake in the path, and the refused-key test drives a host the fake -*does* serve and asserts that an exception is thrown, which an empty-list regression could not -satisfy. +| no plugin registry on that host | **2 credentials** at `other.example.test` | was β€” an unknown host was 404'd before any credential could be observed; fixed by the hostile-host fake | +| the same registry, nothing declared | **3 credentials** | no | +| a declared validator holding no key | **3 credentials** | no | +| a target carrying userinfo | **2 credentials** at `evil.example.test` | was β€” the case that started the audit | +| a URL-form target (3 shapes) | the `no plugin registry` message for the raw value; under the equality form of the check, `host:443` refused with an untrue message | no | +| a declaration set but unreadable (3 shapes) | the old **"no validator is declared"** diagnosis | no | +| a plugin registry whose URL carries credentials (2 rows) | the old **"no plugin registry is configured"** diagnosis; **3 credentials** when the host guard goes | no | +| the boot line | the line with **`instance:mwi_…@evil.example.test`** in it; the two-state line saying **"NO validator declared"** | no | +| a refused key faults, never an empty list | `No exception was thrown` under a `.Catch(empty)` | no | +| a third host reached (the positives' `ReachedHosts`) | `Expected all items to match` with `third.example.test` in the list | **was** β€” `ServedHosts` was appended only for KNOWN hosts, so an un-credentialed request to a third host was invisible by construction; every host is now recorded before the fake decides what it serves | +| the auto-registered shape presents the durable key | **`ExchangeRequests` 1, expected 0** under `ResolveToken` | no | +| the key held for the DECLARED validator, two registries | **`mwi_issued-by-a…`** presented instead of B's under `registries.First()` | no β€” and with one registry configured this assertion could not exist | + +The pure-function negatives (the validator-host reader, declared-vs-readable) have no fake in the +path and cannot take this shape. **The same audit is owed across the wider suite; it is not part of this change.** diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-can-be-told-which-portal-validates-its-registry-key.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-can-be-told-which-portal-validates-its-registry-key.md index 370e347b64..17753e5d64 100644 --- a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-can-be-told-which-portal-validates-its-registry-key.md +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-12-an-instance-can-be-told-which-portal-validates-its-registry-key.md @@ -2,7 +2,7 @@ Name: An instance can be told which portal validates its registry key Category: Fix Description: An installation pulling its images from cr.meshweaver.cloud booted normally and then never updated itself, because the self-updater would only present its instance key to a registry that was also its plugin registry. It now presents it to the portal the registry DECLARES as its validator β€” and still refuses every host nobody declared. -Icon: RefreshCcw +Icon: ArrowSync Order: -20260912 --- @@ -35,9 +35,16 @@ side: `SelfUpdate:RegistryValidationUrl` (chart: `selfUpdate.registryValidationU instance, a record's `extraPortalConfig`). Set it to the registry record's validation URL β€” a bare host works too β€” and the self-updater presents the key it already holds for that portal. +The key presented to a declared validator is the durable instance key itself β€” never the short-lived +token the plugin catalog otherwise exchanges it for β€” so an instance that registered itself at first +boot (`PluginCatalog:BootstrapKey`, no token configured anywhere) is covered exactly like one with a +configured token: the validator is the exchange endpoint, and it refuses a token by design. + `SelfUpdate:Registry` must also be a bare `host` or `host:port`: the same value is used as the registry half of an image reference, and a value shaped like `user:secret@host` would be a valid URI -naming a different host as the one to hand the instance key to. +naming a different host as the one to hand the instance key to. A declaration that is set but does +not name a host, and a plugin registry whose URL carries its credential, are each refused with a +message that says so β€” never diagnosed as "nothing declared" or "no registry configured". **It is a grant, and nothing else is one.** The key still goes only to a host that is a configured plugin registry, or to a registry whose declared validator is one. An absent declaration **refuses**, diff --git a/src/MeshWeaver.Hosting/SelfUpdate/SelfUpdateOptions.cs b/src/MeshWeaver.Hosting/SelfUpdate/SelfUpdateOptions.cs index 2cdc5a2a3b..3d39c6a9e2 100644 --- a/src/MeshWeaver.Hosting/SelfUpdate/SelfUpdateOptions.cs +++ b/src/MeshWeaver.Hosting/SelfUpdate/SelfUpdateOptions.cs @@ -60,12 +60,24 @@ public record SelfUpdateOptions /// public string RegistryValidationUrl { get; init; } = ""; + /// + /// Whether the operator SET at all β€” a separate question + /// from whether it could be read (), and the two must stay + /// separate: collapsing "declared but unreadable" into "not declared" diagnoses a typo'd scheme + /// or a userinfo-bearing value as an ABSENT declaration, and the refusal then tells the operator + /// to set the key they already set β€” a fail-closed fallback forging a correct-looking bug. A + /// declared value that yields no host is refused as MALFORMED, naming the key and never the + /// value. Pure. + /// + public bool RegistryValidatorDeclared => !string.IsNullOrWhiteSpace(RegistryValidationUrl); + /// /// The HOST of β€” the ONE extra plugin registry whose /// instance key may be presented to . null when nothing is - /// declared, when the value names no http(s) host, or when it is blank: every one of those is - /// "no pairing declared", which refuses. Pure; a non-default port is part of the host, the - /// path and scheme are discarded. + /// declared OR when the declared value names no http(s) host; the two are told apart by + /// , and a caller that refuses on null must + /// consult it to say WHICH of the two it is refusing on. Pure; a non-default port is part of + /// the host, the path and scheme are discarded. /// public string? RegistryValidatorHost => HostOf(RegistryValidationUrl); @@ -75,9 +87,12 @@ public record SelfUpdateOptions /// Null for anything else β€” a mailto:, a file path, a blank β€” so a value that cannot be read /// as a host declares no pairing rather than half of one. Pure. /// - /// Public because the tag lister reads plugin-registry URLs with it too: ONE host-reading - /// rule on both sides of the comparison, so the two can never drift into disagreeing about - /// what https://memex.meshweaver.cloud:443/ is. + /// Public because it is THE registry-host rule of the platform, not this record's: the tag + /// lister reads plugin-registry URLs with it, and RegistryUpdateReconciler.SameRegistry + /// decides whether a broadcast names a configured registry with it β€” ONE reader on every side + /// of every comparison, so no two subsystems can drift into disagreeing about what + /// https://memex.meshweaver.cloud:443/ is. The port rule is therefore fleet-wide: a + /// scheme-default port is not part of the host, any other port is. /// public static string? HostOf(string? value) { diff --git a/src/MeshWeaver.PluginCatalog/InstanceAutoRegistrationService.cs b/src/MeshWeaver.PluginCatalog/InstanceAutoRegistrationService.cs index b1188198f2..74dba9f1cb 100644 --- a/src/MeshWeaver.PluginCatalog/InstanceAutoRegistrationService.cs +++ b/src/MeshWeaver.PluginCatalog/InstanceAutoRegistrationService.cs @@ -106,16 +106,38 @@ public sealed class RegistryTokenResolver(IMessageHub hub, ILogger tokens = new(StringComparer.OrdinalIgnoreCase); - /// The effective credential for β€” a JWT when the registry - /// issues one, else the durable key. Cold; emits once. - public IObservable ResolveToken(PluginRegistryReference registry) - { - if (!string.IsNullOrWhiteSpace(registry.Token)) - return Observable.Return(registry.Token.Trim()); + /// The effective credential for β€” a configured token as + /// configured, never exchanged; else the stored key EXCHANGED for a JWT when the registry issues + /// one, else that key itself. Cold; emits once. + public IObservable ResolveToken(PluginRegistryReference registry) => + ConfiguredToken(registry) is { } configured + ? Observable.Return(configured) + : StoredCredential(registry) + .SelectMany(raw => raw.Length == 0 ? Observable.Return("") : Exchange(registry.Url, raw)); - return StoredCredential(registry) - .SelectMany(raw => raw.Length == 0 ? Observable.Return("") : Exchange(registry.Url, raw)); - } + /// + /// The DURABLE credential this installation holds for β€” the + /// configured Token, else the stored auto-registration key decrypted β€” with NO exchange. + /// Empty when it holds neither. Cold; emits once. + /// + /// 🚨 For the ONE caller whose counterpart IS the exchange endpoint. A container registry + /// that decides a pull by forwarding the presented secret to the portal's keyβ†’token exchange + /// (cr.meshweaver.cloud β†’ /api/instances/token, SelfUpdate:RegistryValidationUrl) + /// must be handed the mwi_ key itself: that endpoint refuses a token by design β€” a token + /// may never mint its successor β€” so the mwa_ JWT yields for an + /// auto-registered installation is a 401 there on every attempt (#4093, review of #4094). The + /// two methods share ONE rule for "which durable credential" (, + /// then the store) and differ only in whether the STORED key is exchanged. + /// + public IObservable ResolveDurableKey(PluginRegistryReference registry) => + ConfiguredToken(registry) is { } configured + ? Observable.Return(configured) + : StoredCredential(registry); + + /// An explicitly configured token wins over the store, on every path, as configured β€” + /// it is presented as-is whether or not it is an instance key. Null when none is configured. + private static string? ConfiguredToken(PluginRegistryReference registry) => + string.IsNullOrWhiteSpace(registry.Token) ? null : registry.Token.Trim(); /// /// The durable mwi_ key exchanged for a token at {registry}/api/instances/token, diff --git a/src/MeshWeaver.PluginCatalog/RegistryUpdateReconciler.cs b/src/MeshWeaver.PluginCatalog/RegistryUpdateReconciler.cs index 657100f2d6..6029db7c92 100644 --- a/src/MeshWeaver.PluginCatalog/RegistryUpdateReconciler.cs +++ b/src/MeshWeaver.PluginCatalog/RegistryUpdateReconciler.cs @@ -6,6 +6,7 @@ using System.Reactive.Subjects; using MeshWeaver.Graph; using MeshWeaver.Graph.Configuration; +using MeshWeaver.Hosting.SelfUpdate; using MeshWeaver.Mesh; using MeshWeaver.Mesh.Security; using MeshWeaver.Mesh.Services; @@ -884,19 +885,18 @@ private IObservable WriteLedger() /// /// Whether a broadcast's names a configured registry: - /// by HOST when both parse as absolute URLs β€” a registry announces itself by its public URL, - /// which a consumer may have configured with a different scheme or a trailing slash β€” with the - /// port compared only when one side names one explicitly (a scheme's default port is not a - /// statement about the registry); else by the trimmed, case-insensitive URL. Pure. + /// by HOST β€” a registry announces itself by its public URL, which a consumer may have + /// configured with a different scheme or a trailing slash β€” read on both sides through + /// , the platform's ONE registry-host rule (the + /// self-updater selects its credential with the same reader, #4094). A scheme-default port is + /// not part of the host; any other port is; a value that names no http(s) host β€” a bare + /// non-URL, a mailto:, a URL carrying userinfo β€” names no registry and matches nothing, + /// rather than matching a second copy of the same unreadable string. Pure. /// - internal static bool SameRegistry(string? configured, string? announced) - { - if (Uri.TryCreate(Key(configured), UriKind.Absolute, out var a) - && Uri.TryCreate(Key(announced), UriKind.Absolute, out var b)) - return string.Equals(a.Host, b.Host, StringComparison.OrdinalIgnoreCase) - && ((a.IsDefaultPort && b.IsDefaultPort) || a.Port == b.Port); - return string.Equals(Key(configured), Key(announced), StringComparison.OrdinalIgnoreCase); - } + internal static bool SameRegistry(string? configured, string? announced) => + SelfUpdateOptions.HostOf(configured) is { } a + && SelfUpdateOptions.HostOf(announced) is { } b + && string.Equals(a, b, StringComparison.OrdinalIgnoreCase); private static string EffectiveRef(string? gitRef) => string.IsNullOrWhiteSpace(gitRef) ? "HEAD" : gitRef; diff --git a/test/Memex.Portal.Shared.Test/OciTagListerTest.cs b/test/Memex.Portal.Shared.Test/OciTagListerTest.cs index 46e305d95d..1388b57885 100644 --- a/test/Memex.Portal.Shared.Test/OciTagListerTest.cs +++ b/test/Memex.Portal.Shared.Test/OciTagListerTest.cs @@ -14,6 +14,7 @@ using System.Threading.Tasks; using Memex.Portal.Shared.SelfUpdate; using MeshWeaver.Fixture; +using MeshWeaver.Graph; using MeshWeaver.Hosting.Monolith.TestBase; using MeshWeaver.Hosting.SelfUpdate; using MeshWeaver.Mesh; @@ -59,6 +60,23 @@ public class OciTagListerTest(ITestOutputHelper output) : MonolithMeshTestBase(o /// validationUrl, copied verbatim. private const string DeclaredValidator = MirrorUrl + "/api/instances/token"; + /// The key an AUTO-REGISTERED installation holds: issued at first boot against + /// PluginCatalog:BootstrapKey, stored at Admin/PluginRegistryCredential/…, and + /// configured NOWHERE β€” the zero-touch shape InstanceProvisioningPlan documents. Distinct + /// from so a presented secret proves WHERE it came from. + private const string StoredKey = "mwi_issued-at-first-boot-and-stored-in-the-admin-partition"; + + /// What the plugin registry's exchange route mints for a stored key β€” the short-lived + /// token RegistryTokenResolver.ResolveToken would present, which the fleet registry's + /// validator refuses BY DESIGN (a token may never mint its successor). + private const string ExchangedToken = "mwa_short-lived-token-the-validator-refuses"; + + /// A SECOND plugin registry this installation holds a key for, so that "the key held for + /// the DECLARED validator" and "some other registry's key" are different values. + private const string OtherRegistryHost = "a.example.test"; + private const string OtherRegistryUrl = "https://" + OtherRegistryHost; + private const string OtherRegistryKey = "mwi_issued-by-a-example-test-and-not-the-validator"; + /// Strictly newer than anything this test host can run as, so the roll it drives is /// the ordinary forward roll (the same shape ComboGateRollTest uses) β€” and the highest CD build /// number in the fixture, because lineage orders by ci.N, not by the SemVer prefix. @@ -68,9 +86,34 @@ public class OciTagListerTest(ITestOutputHelper output) : MonolithMeshTestBase(o private readonly FakeMirror mirror = new(); + /// + /// This installation's plugin-catalog configuration β€” the singleton the lister resolves. The + /// default is the fleet's common shape (ONE registry, the mirror host, its key configured); a + /// test that needs another shape assigns Registries before it lists, and the lister reads + /// the instance at listing time, so no test depends on when the container first resolved it. + /// + private readonly PluginCatalogOptions catalog = new() + { + Registries = + [ + new PluginRegistryReference { Name = "Plugins", Url = MirrorUrl, Token = InstanceKey }, + ], + }; + protected override MeshBuilder ConfigureMesh(MeshBuilder builder) => base.ConfigureMesh(builder) .AddUpdatePolicyType() + // The credential STORE's node type, and only that β€” the same definition + // AddPluginCatalog registers, without the catalog's hosted services (the boot reconcile + // and the auto-registration pass would reach the fake at startup and count as requests + // the negatives assert are zero). The resolver reads it; the auto-registered test writes it. + .AddMeshNodes(new MeshNode(PluginRegistryCredentials.NodeType) + { + Name = "Plugin Registry Credential", + IsSatelliteType = false, + HubConfiguration = config => config + .AddMeshDataSource(s => s.WithContentType()), + }) .ConfigureServices(s => s .AddSingleton(new FakeMirrorClientFactory(mirror)) // The resolver the Store uses β€” real, so "the same credential" is measured on the @@ -78,13 +121,7 @@ protected override MeshBuilder ConfigureMesh(MeshBuilder builder) .AddSingleton() // The plugin registry this installation installs modules from IS the mirror host, // and the key it holds for it is the credential the lister must present. - .AddSingleton(new PluginCatalogOptions - { - Registries = - [ - new PluginRegistryReference { Name = "Plugins", Url = MirrorUrl, Token = InstanceKey }, - ], - })); + .AddSingleton(catalog)); private AccessService Access => Mesh.ServiceProvider.GetRequiredService(); @@ -133,16 +170,18 @@ public async Task NoPluginRegistryOnThatHost_FaultsNamingTheHost() { var ct = TestContext.Current.CancellationToken; - var fault = await Assert.ThrowsAsync(() => + var fault = await Record.ExceptionAsync(() => Lister("other.example.test").ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); - fault.Message.Should().Contain("other.example.test"); - fault.Message.Should().Contain("PluginCatalog", - "the fix is a configuration change, and the message has to say which one"); - mirror.Requests.Should().Be(0, "a listing with no credential could only ever be a 401"); + // 🚨 Disclosure FIRST, so a falsification reports WHAT LEAKED rather than "no exception". mirror.CredentialsSeen.Should().BeEmpty( "the guard is a disclosure control: an arbitrary host named in SelfUpdate:Registry must " + "never be handed this installation's instance key"); + mirror.Requests.Should().Be(0, "a listing with no credential could only ever be a 401"); + var message = fault.Should().BeOfType().Which.Message; + message.Should().Contain("other.example.test"); + message.Should().Contain("PluginCatalog", + "the fix is a configuration change, and the message has to say which one"); } // ══════════════════════════════════════════════════════════════════════════ @@ -174,9 +213,83 @@ public async Task ADeclaredValidator_PresentsTheKeyHeldForThatPortal_AndListsThe "the credential is the plugin-registry instance key this installation already holds for " + "the DECLARED VALIDATOR β€” the registry forwards it there to decide the pull, which is " + "exactly why the two hosts differ"); - mirror.ServedHosts.Should().OnlyContain(h => h == RegistryHost, + mirror.ReachedHosts.Should().OnlyContain(h => h == RegistryHost, "the listing goes to the container registry; the validator is named in configuration " - + "and never contacted by the lister"); + + "and never contacted by the lister β€” and this is EVERY host reached, recorded before " + + "the fake decides what it serves, so a request to a third host cannot hide"); + } + + /// + /// 🚨 THE FIX, for the installation the fix was FOR (#4094 review). A zero-touch consumer β€” + /// PluginCatalog:BootstrapKey, no configured Token β€” holds its mwi_ key only in the + /// store, and the Store's resolver EXCHANGES that key for a short-lived mwa_ token before + /// presenting it. The fleet registry decides a pull by forwarding the presented secret to the + /// declared validator, which IS the exchange endpoint and refuses a token by design β€” so the + /// lister must present the durable key itself, unexchanged. A lister that resolved through + /// ResolveToken here presents , is refused on every check, and + /// fixes only the installations with a raw Token configured. + /// + [Fact(Timeout = 120_000)] + public async Task AnAutoRegisteredInstallation_PresentsItsStoredDurableKey_NeverAnExchangedToken() + { + var ct = TestContext.Current.CancellationToken; + // The zero-touch shape: the registry is configured, its key is NOT β€” it lives in the store. + catalog.Registries = [new PluginRegistryReference { Name = "Plugins", Url = MirrorUrl }]; + await StoreCredential(MirrorUrl, StoredKey); + // The registry's validator knows the key it issued, and nothing else β€” in particular not a + // token minted FROM it, which `InstanceTokenEndpoints` refuses by shape. + mirror.AcceptedSecrets = [StoredKey]; + + IReadOnlyList? tags = null; + var fault = await Record.ExceptionAsync(async () => + tags = await Lister(RegistryHost, DeclaredValidator) + .ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); + + // 🚨 The MECHANISM first, so a regression reports what was presented rather than only the + // 401 it earns: a lister resolving through ResolveToken exchanges the stored key (one + // exchange request) and presents the mwa_ token, which the registry's validator refuses. + mirror.ExchangeRequests.Should().Be(0, + "the declared validator IS the key-to-token exchange; exchanging first would hand the " + + "registry a token its validator refuses by design, on every check"); + mirror.PresentedSecret.Should().Be(StoredKey, + "the credential is the DURABLE mwi_ key from the store β€” configured nowhere, so it can " + + "only have come from there β€” and never the mwa_ token the Store would present"); + mirror.PresentedSecret.Should().StartWith("mwi_"); + fault.Should().BeNull("the listing must succeed for an auto-registered instance"); + tags.Should().Equal(FakeMirror.Tags, + "an auto-registered instance on the fleet registry must be able to see what it can roll to"); + mirror.ReachedHosts.Should().OnlyContain(h => h == RegistryHost, + "the key goes to the container registry only; the store is read in-process"); + } + + /// + /// 🚨 SELECTION, not merely resolution (#4094 review). With ONE registry configured nothing + /// distinguishes "the key held for the DECLARED validator" from "some registry's key", so a + /// lister that took the first configured registry would pass the positive above. Two + /// registries, two keys, the validator declared as the SECOND: the key presented must be B's. + /// + [Fact(Timeout = 120_000)] + public async Task ADeclaredValidator_SelectsTheKeyHeldForThatPortal_NotAnotherRegistrys() + { + var ct = TestContext.Current.CancellationToken; + catalog.Registries = + [ + new PluginRegistryReference { Name = "A", Url = OtherRegistryUrl, Token = OtherRegistryKey }, + new PluginRegistryReference { Name = "B", Url = MirrorUrl, Token = InstanceKey }, + ]; + // The fake accepts BOTH keys on purpose: a wrong selection must report as the wrong SECRET, + // not as a refusal the reader could attribute to the fixture. + mirror.AcceptedSecrets = [OtherRegistryKey, InstanceKey]; + + var tags = await Lister(RegistryHost, DeclaredValidator) + .ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct); + + tags.Should().Equal(FakeMirror.Tags); + mirror.PresentedSecret.Should().Be(InstanceKey, + "the key presented is the one held for the DECLARED validator (B), selected by host β€” " + + "never the first configured registry's, never 'a key this installation happens to hold'"); + mirror.PresentedSecret.Should().NotBe(OtherRegistryKey); + mirror.ReachedHosts.Should().OnlyContain(h => h == RegistryHost); } /// @@ -190,15 +303,16 @@ public async Task TheSameRegistry_WithNoDeclaredValidator_IsStillRefused() { var ct = TestContext.Current.CancellationToken; - var fault = await Assert.ThrowsAsync(() => + var fault = await Record.ExceptionAsync(() => Lister(RegistryHost).ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); - fault.Message.Should().Contain(RegistryHost); - fault.Message.Should().Contain("SelfUpdate:RegistryValidationUrl", - "an absent declaration is refused AND the message says what would declare it"); - mirror.Requests.Should().Be(0); mirror.CredentialsSeen.Should().BeEmpty( "an undeclared pairing is not permission β€” the key never goes out"); + mirror.Requests.Should().Be(0); + var message = fault.Should().BeOfType().Which.Message; + message.Should().Contain(RegistryHost); + message.Should().Contain("SelfUpdate:RegistryValidationUrl", + "an absent declaration is refused AND the message says what would declare it"); } /// @@ -211,15 +325,85 @@ public async Task ADeclaredValidatorThisInstallationHoldsNoKeyFor_IsRefused() { var ct = TestContext.Current.CancellationToken; - var fault = await Assert.ThrowsAsync(() => + var fault = await Record.ExceptionAsync(() => Lister(RegistryHost, "https://someone-elses-portal.example.test/api/instances/token") .ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); - fault.Message.Should().Contain("someone-elses-portal.example.test"); - fault.Message.Should().Contain("PluginCatalog:Registries"); - mirror.Requests.Should().Be(0); mirror.CredentialsSeen.Should().BeEmpty( "the key is only ever presented to a host this installation was issued one for"); + mirror.Requests.Should().Be(0); + var message = fault.Should().BeOfType().Which.Message; + message.Should().Contain("someone-elses-portal.example.test"); + message.Should().Contain("PluginCatalog:Registries"); + } + + /// + /// 🚨 NEGATIVE CONTROL for the DIAGNOSIS (#4094 review). A declaration that is SET but names no + /// http(s) host β€” a userinfo-bearing value, a typo'd scheme β€” used to read as ABSENT: the + /// refusal told the operator to declare the key they had already set. It is refused as + /// MALFORMED, the message names the key and the shape, and the value is never echoed, because + /// the userinfo is where a key would be. + /// + [Theory(Timeout = 120_000)] + [InlineData("https://memex.example.test@evil.example.test/api/instances/token")] + [InlineData("mailto:ops@example.test")] + [InlineData("htps://memex.example.test/api/instances/token")] + public async Task ADeclaredButUnreadableValidator_IsRefusedAsMalformed_NeverAsAbsent(string declared) + { + var ct = TestContext.Current.CancellationToken; + + var fault = await Record.ExceptionAsync(() => + Lister(RegistryHost, declared).ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); + + mirror.CredentialsSeen.Should().BeEmpty(); + mirror.Requests.Should().Be(0); + var message = fault.Should().BeOfType().Which.Message; + message.Should().Contain("SelfUpdate:RegistryValidationUrl is set but does not name an http(s) host", + "a set-but-unreadable declaration is a misconfiguration to NAME, never 'nothing declared'"); + message.Should().NotContain("no validator is declared", + "telling the operator to set a key they already set is a fail-closed fallback forging a " + + "correct-looking bug"); + message.Should().NotContain("evil.example.test", "the value is not echoed"); + message.Should().NotContain("ops@example.test"); + } + + /// + /// 🚨 NEGATIVE CONTROL for the OTHER diagnosis (#4094 review): a plugin registry that EXISTS on + /// the qualifying host but whose URL carries credentials (https://instance:mwi_…@host). + /// HostOf refuses userinfo, so such a registry stops matching on BOTH branches β€” the + /// pre-#4094 reader tolerated it β€” and without this control the refusal would claim "no plugin + /// registry is configured on that host" about a registry the catalog is talking to. The cause + /// is named, the fix is named, and the URL is not echoed: the credential is in it. + /// + [Theory(Timeout = 120_000)] + [InlineData(MirrorHost, "")] // the registry's OWN host + [InlineData(RegistryHost, DeclaredValidator)] // the declared validator β€” the fleet's shape + public async Task APluginRegistryWhoseUrlCarriesCredentials_IsRefusedNamingTheCause_NeverAsAbsent( + string registry, string validator) + { + var ct = TestContext.Current.CancellationToken; + catalog.Registries = + [ + new PluginRegistryReference + { + Name = "Plugins", Url = $"https://instance:{InstanceKey}@{MirrorHost}", Token = InstanceKey, + }, + ]; + + var fault = await Record.ExceptionAsync(() => + Lister(registry, validator).ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); + + mirror.CredentialsSeen.Should().BeEmpty( + "a registry whose URL is not read as naming a host is not a credential source either"); + mirror.Requests.Should().Be(0); + var message = fault.Should().BeOfType().Which.Message; + message.Should().Contain("carries credentials", + "the registry exists; what is wrong is where its credential sits, and the message says so"); + message.Should().Contain("PluginCatalog:Registries:N:Token", "the fix is named"); + message.Should().NotContain("no plugin registry", + "'no registry configured' must never be the diagnosis for a registry that exists"); + message.Should().NotContain(InstanceKey); + message.Should().NotContain("mwi_"); } /// @@ -237,19 +421,20 @@ public async Task ARegistryTargetCarryingUserinfo_IsRefused_AndNeverEchoed() var ct = TestContext.Current.CancellationToken; const string smuggled = "instance:mwi_smuggled-in-the-registry-value"; - var fault = await Assert.ThrowsAsync(() => + var fault = await Record.ExceptionAsync(() => Lister($"{smuggled}@evil.example.test", DeclaredValidator) .ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); - fault.Message.Should().NotContain(smuggled, - "the refusal must not echo a value of this shape β€” the userinfo is where a key would be"); - fault.Message.Should().NotContain("mwi_"); - fault.Message.Should().Contain("SelfUpdate:Registry"); - mirror.Requests.Should().Be(0); mirror.CredentialsSeen.Should().BeEmpty( "a declared validator must never make an unparseable target reachable β€” and this " + "assertion is recorded before the fake's host check, so it can see a credential sent " + "to evil.example.test, which PresentedSecret by construction could not"); + mirror.Requests.Should().Be(0); + var message = fault.Should().BeOfType().Which.Message; + message.Should().NotContain(smuggled, + "the refusal must not echo a value of this shape β€” the userinfo is where a key would be"); + message.Should().NotContain("mwi_"); + message.Should().Contain("SelfUpdate:Registry"); } /// @@ -258,18 +443,44 @@ public async Task ARegistryTargetCarryingUserinfo_IsRefused_AndNeverEchoed() /// refused rather than silently accepted, because the identical value is interpolated into the /// portal and migration image references, which a URL form makes malformed. /// - [Fact(Timeout = 120_000)] - public async Task AUrlFormRegistryTarget_IsRefused_NamingTheBareHostToUse() + [Theory(Timeout = 120_000)] + [InlineData("https://" + MirrorHost)] + [InlineData(MirrorHost + "/v2")] + [InlineData("https://" + MirrorHost + ":443/v2/")] + public async Task AUrlFormRegistryTarget_IsRefused_NamingTheBareHostToUse(string target) { var ct = TestContext.Current.CancellationToken; - var fault = await Assert.ThrowsAsync(() => - Lister($"https://{MirrorHost}").ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); + var fault = await Record.ExceptionAsync(() => + Lister(target).ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct)); - fault.Message.Should().Contain(MirrorHost, "the message names the value to set instead"); - fault.Message.Should().Contain("bare registry host"); - mirror.Requests.Should().Be(0); + mirror.Requests.Should().Be(0, "the refusal is decided before a byte is sent"); mirror.CredentialsSeen.Should().BeEmpty(); + var message = fault.Should().BeOfType().Which.Message; + message.Should().Contain(MirrorHost, "the message names the value to set instead"); + message.Should().Contain("bare registry host"); + message.Should().Contain("carries a scheme or a path", "and the message must be TRUE of the value"); + } + + /// + /// The documentation promises host or host:port, and 443 is a port (#4094 review). + /// HostOf drops a scheme-default port, so a bare-host check written as equality with the + /// parsed host refused host:443 β€” with a message claiming it "carries a scheme or a + /// path", which it does not. The check is textual on the configured value; the client and the + /// credential match then use the normalized host, which is what the plugin registry is + /// configured under. + /// + [Fact(Timeout = 120_000)] + public async Task ABareHostNamingTheDefaultPort_IsAccepted_AndListed() + { + var ct = TestContext.Current.CancellationToken; + + var tags = await Lister($"{MirrorHost}:443").ListTags(Repository).FirstAsync().Timeout(Budget).Await(ct); + + tags.Should().Equal(FakeMirror.Tags, "'host:port' is the documented shape, whatever the port"); + mirror.PresentedSecret.Should().Be(InstanceKey, + "the plugin registry configured as https://host is the same host as host:443"); + mirror.ReachedHosts.Should().OnlyContain(h => h == MirrorHost); } /// The declaration is read WHOLE-HOST or not at all β€” never a suffix, a registrable @@ -296,6 +507,23 @@ public void TheDeclaredValidatorHost_IsReadWholeOrNotAtAll() .RegistryValidatorHost.Should().BeNull("a value that names no http host declares nothing"); } + /// "Declared" and "readable" are two questions (#4094 review): a set-but-unreadable + /// value is DECLARED and yields no host, and a caller refusing on the null host must be able to + /// say which of the two it is refusing on. + [Fact] + public void ADeclaration_IsToldApartFromAReadableOne() + { + new SelfUpdateOptions().RegistryValidatorDeclared.Should().BeFalse(); + new SelfUpdateOptions { RegistryValidationUrl = " " }.RegistryValidatorDeclared.Should().BeFalse( + "whitespace is not a declaration"); + var malformed = new SelfUpdateOptions { RegistryValidationUrl = "https://memex.example.test@evil.example.test/x" }; + malformed.RegistryValidatorDeclared.Should().BeTrue("the operator SET it"); + malformed.RegistryValidatorHost.Should().BeNull("and it names no host β€” malformed, not absent"); + var declared = new SelfUpdateOptions { RegistryValidationUrl = DeclaredValidator }; + declared.RegistryValidatorDeclared.Should().BeTrue(); + declared.RegistryValidatorHost.Should().Be(MirrorHost); + } + [Fact] public void AnAzureContainerRegistryHost_KeepsTheAcrLister() { @@ -355,6 +583,56 @@ public async Task AMirrorInstall_RollsFromTheMirrorListing_AndNeverCallsAcr() mirror.PresentedSecret.Should().Be(InstanceKey); } + /// + /// 🚨 NEGATIVE CONTROL for the BOOT LINE (#4094 review, the serious one). The poller's startup + /// line is Information β€” it leaves the pod for Loki on every start, BEFORE the lister's + /// refusal (which deliberately does not echo the value) has ever run. Logging + /// SelfUpdate:Registry verbatim there shipped a value of the shape + /// instance:mwi_…@evil.example, key included, to Loki at boot. The line names the HOST + /// read from the value, or says it is unreadable β€” and it names the THIRD validator state (set + /// but unreadable) as such, never as "NO validator declared". + /// + [Fact(Timeout = 120_000)] + public async Task TheBootLine_NamesTheRegistryHost_NeverTheConfiguredValue() + { + const string smuggled = "instance:mwi_smuggled-in-the-registry-value"; + var log = new CapturingLogger(); + var options = new SelfUpdateOptions + { + Registry = $"{smuggled}@evil.example.test", + RegistryValidationUrl = "https://memex.example.test@evil.example.test/api/instances/token", + DefaultPolicy = UpdatePolicyKind.Continuous, + DefaultPattern = "*-ci*", + }; + var service = new SeamedSelfUpdateService( + Mesh, new AcrMustNotBeCalled(), new RecordingUpdater(), options, log, + new AlwaysAvailable(Mesh, new ConfigurationBuilder().Build()), + new OciTagLister(Mesh, options, Mesh.ServiceProvider.GetService>())); + + await service.StartAsync(CancellationToken.None); + try + { + var boot = log.Lines.Should().ContainSingle(l => l.Contains("[SelfUpdate] starting"), + "the startup line is written synchronously in StartAsync").Which; + boot.Should().NotContain(smuggled, + "the configured value is never logged β€” this line ships to Loki at boot, before any refusal"); + boot.Should().NotContain("mwi_"); + boot.Should().Contain("(unreadable", "an unreadable value is named as such, not printed"); + boot.Should().Contain("is SET but does not name an http(s) host", + "a declaration that is set but unreadable is the THIRD state, named as a misconfiguration"); + boot.Should().NotContain("NO validator declared", + "which is what it used to say, sending the operator to set a key they had already set"); + } + finally + { + await service.StopAsync(CancellationToken.None); + } + + log.Lines.Should().NotContain(l => l.Contains("mwi_"), + "no line this service wrote while it ran carries anything shaped like the key"); + mirror.CredentialsSeen.Should().BeEmpty(); + } + // ══════════════════════════════════════════════════════════════════════════ // The fake mirror // ══════════════════════════════════════════════════════════════════════════ @@ -374,15 +652,30 @@ private sealed class FakeMirror : HttpMessageHandler public bool RefuseKey; public int Requests; public int TokenRequests; + public int ExchangeRequests; public int PagesServed; public string? PresentedSecret; - private ImmutableList servedHosts = ImmutableList.Empty; + /// + /// The secrets the registry's token realm accepts β€” what its validator would answer 200 for. + /// The fleet registry forwards the presented secret to /api/instances/token, which + /// accepts a durable mwi_ key it knows and REFUSES a token by shape, so a minted + /// is never in this set however it got there. + /// + public ImmutableHashSet AcceptedSecrets = [InstanceKey]; + + private ImmutableList reachedHosts = ImmutableList.Empty; private ImmutableList credentialsSeen = ImmutableList.Empty; - /// Every host a request actually reached β€” the listing must go to the CONTAINER - /// registry, never to the portal whose key authenticates it. - public ImmutableList ServedHosts => servedHosts; + /// + /// 🚨 EVERY host a request reached, recorded UNCONDITIONALLY before the fake decides what it + /// serves. It used to be appended only for the hosts the fake knows, so a positive asserting + /// "only the container registry was contacted" could not see an un-credentialed request to a + /// third host β€” the same vacuity class as the credential one below, on the other assertion + /// (#4094 review). The listing must go to the CONTAINER registry, never to the portal whose + /// key authenticates it. + /// + public ImmutableList ReachedHosts => reachedHosts; /// /// 🚨 Every host that was sent ANY credential, recorded BEFORE the host check β€” which is @@ -399,10 +692,25 @@ protected override Task SendAsync( { Interlocked.Increment(ref Requests); var uri = request.RequestUri!; + ImmutableInterlocked.Update(ref reachedHosts, current => current.Add(uri.Host)); if (request.Headers.Authorization is { } offered) ImmutableInterlocked.Update(ref credentialsSeen, current => current.Add($"{offered.Scheme} to {uri.Host}")); + // The plugin registry's keyβ†’token exchange (SyncTokenPayloads.Route): a durable key is + // exchanged for a short-lived token, exactly as RegistryTokenResolver.ResolveToken does + // for a STORED key. Served so that a lister which exchanges before presenting is SEEN to + // β€” it then presents the token, which the registry's realm below refuses, as the fleet's + // validator would. A lister that must not exchange leaves ExchangeRequests at zero. + if (uri.AbsolutePath == SyncTokenPayloads.Route && request.Method == HttpMethod.Post) + { + Interlocked.Increment(ref ExchangeRequests); + return Task.FromResult(request.Headers.Authorization is { Scheme: "Bearer", Parameter: { } key } + && key.StartsWith("mwi_", StringComparison.Ordinal) + ? Json($$"""{"accessToken":"{{ExchangedToken}}","tokenType":"Bearer","expiresIn":300,"scope":[]}""") + : new HttpResponseMessage(HttpStatusCode.Unauthorized)); + } + // 🚨 AN UNKNOWN HOST BEHAVES LIKE A HOSTILE REGISTRY, NOT LIKE A 404. // // This is what makes the disclosure assertions real. The client presents Basic only @@ -414,8 +722,6 @@ protected override Task SendAsync( // whatever secret is handed over. Only the tags listing stays restricted to the two // hosts this fixture legitimately serves. var known = uri.Host is MirrorHost or RegistryHost; - if (known) - ImmutableInterlocked.Update(ref servedHosts, current => current.Add(uri.Host)); if (uri.AbsolutePath == "/v2/token") { @@ -428,13 +734,14 @@ protected override Task SendAsync( secret = pair[(pair.IndexOf(':') + 1)..]; } PresentedSecret = secret; - if (RefuseKey || secret != InstanceKey) + if (RefuseKey || secret is null || !AcceptedSecrets.Contains(secret)) return Task.FromResult(new HttpResponseMessage(HttpStatusCode.Unauthorized)); // The mirror mints nothing: the bearer IS the caller's key. - return Task.FromResult(Json($$"""{"token":"{{InstanceKey}}"}""")); + return Task.FromResult(Json($$"""{"token":"{{secret}}"}""")); } - if (request.Headers.Authorization is not { Scheme: "Bearer" } auth || auth.Parameter != InstanceKey) + if (request.Headers.Authorization is not { Scheme: "Bearer", Parameter: { } bearer } + || !AcceptedSecrets.Contains(bearer)) return Task.FromResult(Challenge(uri.Host)); if (known && uri.AbsolutePath == $"/v2/{Repository}/tags/list") @@ -529,6 +836,56 @@ private sealed class SeamedSelfUpdateService( protected override ComboVerificationGate? ResolveComboGate() => null; } + /// Records every formatted line β€” instance state, owned by the test. + private sealed class CapturingLogger : ILogger + { + private ImmutableList lines = ImmutableList.Empty; + + public ImmutableList Lines => lines; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => ImmutableInterlocked.Update(ref lines, current => current.Add(formatter(state, exception))); + } + + /// + /// What auto-registration leaves behind: the issued key at + /// Admin/PluginRegistryCredential/{host-slug}, which RegistryTokenResolver reads as + /// System. Stored in the clear here β€” the protector passes a value without the enc: tag + /// through unchanged, and what this test measures is which SHAPE is presented, not the envelope. + /// + private Task StoreCredential(string registryUrl, string key) + { + var meshService = Mesh.ServiceProvider.GetRequiredService(); + var path = PluginRegistryCredentials.Path(registryUrl); + var node = new MeshNode(path.Split('/').Last(), PluginRegistryCredentials.Namespace) + { + NodeType = PluginRegistryCredentials.NodeType, + Name = "Registry credential (auto-registered)", + State = MeshNodeState.Active, + Content = new PluginRegistryCredential + { + RegistryUrl = registryUrl, + InstanceId = "this-installation", + ProtectedKey = key, + RegisteredAt = DateTimeOffset.UtcNow, + }, + }; + return Observable.Create(observer => + { + using (Access.ImpersonateAsSystem()) + return (IDisposable)meshService.CreateNode(node).Subscribe(observer); + }) + .FirstAsync() + .Timeout(Budget) + .Await(TestContext.Current.CancellationToken); + } + private Task Seed() { var meshService = Mesh.ServiceProvider.GetRequiredService(); diff --git a/test/Memex.Portal.Shared.Test/RegistryUpdateReconcilerTest.cs b/test/Memex.Portal.Shared.Test/RegistryUpdateReconcilerTest.cs index 01adb3243e..32ec050fd4 100644 --- a/test/Memex.Portal.Shared.Test/RegistryUpdateReconcilerTest.cs +++ b/test/Memex.Portal.Shared.Test/RegistryUpdateReconcilerTest.cs @@ -160,14 +160,21 @@ await InboxDeliveries() } /// The host-based match: a consumer configured with a trailing slash or another scheme - /// still recognises its registry; a different host never does. + /// still recognises its registry; a different host never does. Since #4094 the reader is the + /// platform's ONE registry-host rule (SelfUpdateOptions.HostOf, shared with the + /// self-updater's credential selection), so a value that names no http(s) host names no + /// registry β€” two copies of an unreadable string no longer "match", and a URL carrying + /// userinfo matches nothing rather than the host a human would not have read. [Theory] [InlineData("https://memex.meshweaver.cloud", "https://memex.meshweaver.cloud/", true)] [InlineData("https://memex.meshweaver.cloud", "http://memex.meshweaver.cloud", true)] [InlineData("https://MEMEX.meshweaver.cloud/", "https://memex.meshweaver.cloud", true)] + [InlineData("https://memex.meshweaver.cloud:443", "https://memex.meshweaver.cloud", true)] [InlineData("https://memex.meshweaver.cloud", "https://memex.systemorph.com", false)] [InlineData("http://registry:8080", "http://registry:9090", false)] - [InlineData("not a url", "not a url/", true)] + [InlineData("http://registry:8080", "http://registry", false)] + [InlineData("not a url", "not a url/", false)] + [InlineData("https://memex.meshweaver.cloud@evil.example.test", "https://evil.example.test", false)] public void SameRegistry_MatchesByHost(string configured, string announced, bool expected) => Assert.Equal(expected, RegistryUpdateReconciler.SameRegistry(configured, announced));