Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| a4e920902c | |||
| fdc6b0c2bb |
@@ -104,11 +104,11 @@
|
||||
<PackageVersion Include="ZB.MOM.WW.Auth.AspNetCore" Version="0.1.5" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Audit" Version="0.1.0" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Theme" Version="0.4.1" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets" Version="0.5.1" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets.Abstractions" Version="0.5.1" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets.Ui" Version="0.5.1" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.SqlServer" Version="0.5.1" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.Grpc" Version="0.5.1" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets" Version="0.6.2" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets.Abstractions" Version="0.6.2" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets.Ui" Version="0.6.2" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.SqlServer" Version="0.6.2" />
|
||||
<PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.Grpc" Version="0.6.2" />
|
||||
<PackageVersion Include="ZB.MOM.WW.LocalDb" Version="0.1.3" />
|
||||
<PackageVersion Include="ZB.MOM.WW.LocalDb.Replication" Version="0.1.3" />
|
||||
<PackageVersion Include="ZB.MOM.WW.LocalDb.Contracts" Version="0.1.3" />
|
||||
|
||||
@@ -21,11 +21,13 @@ x-secrets-hub-env: &secrets-hub-env
|
||||
# ciphertext replicates, so a node with a different KEK fails closed on resolve with a
|
||||
# kek_id mismatch that reads like corruption but is a deployment error.
|
||||
ZB_SECRETS_MASTER_KEY: "zZiBWuoaVMbJmGXToLk9Lakw0iJozXoL/7Gxac3GwJ4="
|
||||
# The appsettings default is the relative "scadabridge-secrets.db", which resolves to
|
||||
# /app — inside the image's writable layer, so it is destroyed by any container
|
||||
# recreate and unreachable from the host. /app/data is the node's own mounted volume
|
||||
# (the one LocalDb already uses on sites; added to the central pair for this).
|
||||
Secrets__SqlitePath: "/app/data/scadabridge-secrets.db"
|
||||
# Must be absolute AND outside the content root (/app) — Secrets 0.6.2 validates both
|
||||
# at startup, because a store inside the deployment directory is destroyed by an
|
||||
# ordinary upgrade (this wiped the MxGateway key store on 2026-08-09). /data is the
|
||||
# node's own bind mount, the SAME host directory as /app/data, so the existing store
|
||||
# is found unmoved; only the path used to reach it changes. Do NOT point this back at
|
||||
# /app/data — absolute is not sufficient, and the boot will fail.
|
||||
Secrets__SqlitePath: "/data/scadabridge-secrets.db"
|
||||
Secrets__Replication__Enabled: "true"
|
||||
Secrets__Replication__Mode: "Grpc"
|
||||
# DEV-ONLY shared bearer token — NOT a real secret. Presented by every follower and
|
||||
@@ -116,6 +118,10 @@ services:
|
||||
# scadaproj#4) the scadabridge-secrets.db here is a pre-0.5.0 residue, but the
|
||||
# volume is still needed (inbound-api-keys.sqlite lives on it).
|
||||
- ./central-node-a/data:/app/data
|
||||
# Same host directory, mounted a second time OUTSIDE the content root (/app).
|
||||
# Secrets 0.6.2 refuses a store path under the content root; this bind mount is
|
||||
# not touched by an image rebuild, but the guard is lexical and cannot know that.
|
||||
- ./central-node-a/data:/data
|
||||
- ./central-node-a/logs:/app/logs
|
||||
networks:
|
||||
- scadabridge-net
|
||||
@@ -173,6 +179,10 @@ services:
|
||||
# scadaproj#4) the scadabridge-secrets.db here is a pre-0.5.0 residue, but the
|
||||
# volume is still needed (inbound-api-keys.sqlite lives on it).
|
||||
- ./central-node-b/data:/app/data
|
||||
# Same host directory, mounted a second time OUTSIDE the content root (/app).
|
||||
# Secrets 0.6.2 refuses a store path under the content root; this bind mount is
|
||||
# not touched by an image rebuild, but the guard is lexical and cannot know that.
|
||||
- ./central-node-b/data:/data
|
||||
- ./central-node-b/logs:/app/logs
|
||||
networks:
|
||||
- scadabridge-net
|
||||
@@ -197,6 +207,10 @@ services:
|
||||
volumes:
|
||||
- ./site-a-node-a/appsettings.Site.json:/app/appsettings.Site.json:ro
|
||||
- ./site-a-node-a/data:/app/data
|
||||
# Same host directory, mounted a second time OUTSIDE the content root (/app).
|
||||
# Secrets 0.6.2 refuses a store path under the content root; this bind mount is
|
||||
# not touched by an image rebuild, but the guard is lexical and cannot know that.
|
||||
- ./site-a-node-a/data:/data
|
||||
- ./site-a-node-a/logs:/app/logs
|
||||
networks:
|
||||
- scadabridge-net
|
||||
@@ -221,6 +235,10 @@ services:
|
||||
volumes:
|
||||
- ./site-a-node-b/appsettings.Site.json:/app/appsettings.Site.json:ro
|
||||
- ./site-a-node-b/data:/app/data
|
||||
# Same host directory, mounted a second time OUTSIDE the content root (/app).
|
||||
# Secrets 0.6.2 refuses a store path under the content root; this bind mount is
|
||||
# not touched by an image rebuild, but the guard is lexical and cannot know that.
|
||||
- ./site-a-node-b/data:/data
|
||||
- ./site-a-node-b/logs:/app/logs
|
||||
networks:
|
||||
- scadabridge-net
|
||||
@@ -242,6 +260,10 @@ services:
|
||||
volumes:
|
||||
- ./site-b-node-a/appsettings.Site.json:/app/appsettings.Site.json:ro
|
||||
- ./site-b-node-a/data:/app/data
|
||||
# Same host directory, mounted a second time OUTSIDE the content root (/app).
|
||||
# Secrets 0.6.2 refuses a store path under the content root; this bind mount is
|
||||
# not touched by an image rebuild, but the guard is lexical and cannot know that.
|
||||
- ./site-b-node-a/data:/data
|
||||
- ./site-b-node-a/logs:/app/logs
|
||||
networks:
|
||||
- scadabridge-net
|
||||
@@ -263,6 +285,10 @@ services:
|
||||
volumes:
|
||||
- ./site-b-node-b/appsettings.Site.json:/app/appsettings.Site.json:ro
|
||||
- ./site-b-node-b/data:/app/data
|
||||
# Same host directory, mounted a second time OUTSIDE the content root (/app).
|
||||
# Secrets 0.6.2 refuses a store path under the content root; this bind mount is
|
||||
# not touched by an image rebuild, but the guard is lexical and cannot know that.
|
||||
- ./site-b-node-b/data:/data
|
||||
- ./site-b-node-b/logs:/app/logs
|
||||
networks:
|
||||
- scadabridge-net
|
||||
@@ -284,6 +310,10 @@ services:
|
||||
volumes:
|
||||
- ./site-c-node-a/appsettings.Site.json:/app/appsettings.Site.json:ro
|
||||
- ./site-c-node-a/data:/app/data
|
||||
# Same host directory, mounted a second time OUTSIDE the content root (/app).
|
||||
# Secrets 0.6.2 refuses a store path under the content root; this bind mount is
|
||||
# not touched by an image rebuild, but the guard is lexical and cannot know that.
|
||||
- ./site-c-node-a/data:/data
|
||||
- ./site-c-node-a/logs:/app/logs
|
||||
networks:
|
||||
- scadabridge-net
|
||||
@@ -305,6 +335,10 @@ services:
|
||||
volumes:
|
||||
- ./site-c-node-b/appsettings.Site.json:/app/appsettings.Site.json:ro
|
||||
- ./site-c-node-b/data:/app/data
|
||||
# Same host directory, mounted a second time OUTSIDE the content root (/app).
|
||||
# Secrets 0.6.2 refuses a store path under the content root; this bind mount is
|
||||
# not touched by an image rebuild, but the guard is lexical and cannot know that.
|
||||
- ./site-c-node-b/data:/data
|
||||
- ./site-c-node-b/logs:/app/logs
|
||||
networks:
|
||||
- scadabridge-net
|
||||
|
||||
@@ -148,16 +148,29 @@ The committed default in `appsettings.json` is:
|
||||
|
||||
```json
|
||||
"Secrets": {
|
||||
"SqlitePath": "scadabridge-secrets.db",
|
||||
"MasterKey": { "Source": "Environment", "EnvVarName": "ZB_SECRETS_MASTER_KEY" },
|
||||
"RunMigrationsOnStartup": true,
|
||||
"ResolveCacheTtl": "00:00:30"
|
||||
}
|
||||
```
|
||||
|
||||
This is dev-safe: `Source=Environment` needs no filesystem key, and the SQLite
|
||||
path is relative to the working directory, so local dev and the
|
||||
`WebApplicationFactory<Program>` `Host.Tests` boot cleanly with no external mount.
|
||||
**`SqlitePath` is deliberately absent** (changed with `ZB.MOM.WW.Secrets` 0.6.2, 2026-08-11).
|
||||
That version validates the setting at startup and rejects it unless it is both **absolute** and
|
||||
**outside the application content root** — the two rules are independent, and the path that
|
||||
destroyed the MxGateway key store on 2026-08-09 was absolute. The former committed value
|
||||
(`"scadabridge-secrets.db"`) was relative and now fails the boot, so it was removed rather than
|
||||
replaced: an unset path takes the library's absolute per-user `LocalApplicationData` default,
|
||||
which satisfies both rules everywhere without a mount.
|
||||
|
||||
This is dev-safe: `Source=Environment` needs no filesystem key, and the per-user default is
|
||||
writable, so local dev and the `WebApplicationFactory<Program>` `Host.Tests` boot cleanly with no
|
||||
external mount.
|
||||
|
||||
**Containerized deployments need care.** The rule is lexical, so a path under the content root is
|
||||
rejected even when it is a *bind mount* that an image rebuild never touches. The docker rig hit
|
||||
exactly this: `/app/data/scadabridge-secrets.db` with content root `/app`. The fix was to mount
|
||||
each node's existing host directory a **second** time at `/data` and point `Secrets__SqlitePath`
|
||||
there — same underlying directory, so the store is found unmoved and nothing needs migrating.
|
||||
Local dev and docker-compose environments supply concrete secret values via the
|
||||
**whole-key environment override** (e.g. `ScadaBridge__Database__ConfigurationDb`),
|
||||
which bypasses `${secret:...}` resolution entirely, so the expander is effectively
|
||||
|
||||
+209
@@ -0,0 +1,209 @@
|
||||
# Schema Library (`SharedSchema`) — wiring audit
|
||||
|
||||
**Date:** 2026-08-11
|
||||
**Question:** the Schema Library admin page doesn't seem wired to anything.
|
||||
**Verdict:** the *read* path is complete and careful; the *write* path doesn't exist. One of
|
||||
the three gaps is active silent data loss, not just a missing feature.
|
||||
|
||||
---
|
||||
|
||||
## 1. What IS wired
|
||||
|
||||
The `{"$ref":"lib:Name"}` resolver has four real consumers, plus guarded CRUD:
|
||||
|
||||
| Consumer | Location | Role |
|
||||
|---|---|---|
|
||||
| Deploy-time flattening | `DeploymentManager/FlatteningPipeline.cs:150` | Pre-loads the library once into a name→JSON map; an unresolved ref fails the deploy |
|
||||
| Template validation | `TemplateEngine/Validation/ValidationService.cs:91,149` | Dangling / cyclic / over-depth ref validation |
|
||||
| Inbound API runtime | `InboundAPI/SchemaRefResolver.cs` → `ParameterValidator.cs:35`, `ReturnValueValidator.cs:41` | Resolves refs when validating live request/response bodies; gated on `InboundApiSchema.MightContainRef` so ref-free methods pay nothing |
|
||||
| Central UI value entry | `CentralUI/Components/Shared/ParameterValueForm.razor:166` via `ISchemaLibraryQueryService` | Inlines referenced schemas so value-entry forms render the real shape |
|
||||
|
||||
Supporting cast, all present and correct:
|
||||
|
||||
- Entity `Commons/Entities/Schemas/SharedSchema.cs`, repo `ISharedSchemaRepository`,
|
||||
EF mapping `SharedSchemaConfiguration`, registration in `ConfigurationDatabase/ServiceCollectionExtensions.cs:63`.
|
||||
- CRUD dispatched through `ManagementActor` (`ManagementActor.cs:2818-2884`) — the guard-running
|
||||
path, never a direct repo write from the UI.
|
||||
- Nav entry `NavMenu.razor:35`, page `Pages/Design/SchemaLibrary.razor`, tests
|
||||
`SchemaLibraryPageTests.cs` / `SchemaLibraryHandlerTests.cs`.
|
||||
|
||||
So the library is genuinely consumed at deploy time, at runtime, and in one UI surface.
|
||||
|
||||
---
|
||||
|
||||
## 2. Gap 1 — no UI can produce a `lib:` reference
|
||||
|
||||
`SchemaBuilder` is the **only** schema editor on every authoring surface:
|
||||
|
||||
- `Pages/Design/ApiMethodForm.razor:84,90` (parameters + return)
|
||||
- `Pages/Design/SharedScriptForm.razor:71,76`
|
||||
- `Pages/Design/TemplateScriptDialog.razor:141,146`
|
||||
- `Pages/Design/SchemaLibrary.razor:46` — the library page itself
|
||||
|
||||
Its type picker is `SchemaBuilderModel.PrimitiveTypes` (`SchemaBuilderModel.cs:37-38`):
|
||||
|
||||
```csharp
|
||||
{ "string", "integer", "number", "boolean", "object", "array" }
|
||||
```
|
||||
|
||||
No library-reference option, at root or property level. There is also **no raw-JSON escape hatch
|
||||
for schemas** — the Monaco editor on those forms edits the C# script body, and
|
||||
`ParameterValueForm`'s raw mode edits *values*, not the schema.
|
||||
|
||||
**Consequence:** the Schema Library page creates entries that nothing else in the UI can point at.
|
||||
The only authoring routes for a ref are a direct DB edit or a bundle import.
|
||||
|
||||
---
|
||||
|
||||
## 3. Gap 2 — a ref authored outside the UI is silently destroyed on the next save
|
||||
|
||||
`ParseSchema` inspects only the `type` property (`SchemaBuilderModel.cs:96-100`). A `$ref` node
|
||||
carries no `type`, so the node defaults to `"string"`, and `Serialize` writes that back.
|
||||
|
||||
Verified by running the round-trip through `SchemaBuilderModel` directly (throwaway probe, since
|
||||
removed):
|
||||
|
||||
```
|
||||
{"$ref":"lib:Address"}
|
||||
→ {"type":"string"}
|
||||
|
||||
{"type":"object","properties":{"addr":{"$ref":"lib:Address"},"name":{"type":"string"}},"required":["addr"]}
|
||||
→ {"type":"object","properties":{"addr":{"type":"string"},"name":{"type":"string"}},"required":["addr"]}
|
||||
```
|
||||
|
||||
Both root-level and nested (`SchemaBuilderModel.cs:131-133`) refs collapse.
|
||||
|
||||
**Consequence:** open any ApiMethod / SharedScript / TemplateScript whose definition carries a ref,
|
||||
change anything at all, save — and the contract is silently widened to a bare string. Deploy still
|
||||
passes (the ref is gone, so there is nothing left to dangle) and nothing warns.
|
||||
|
||||
**Sharpest edge:** the Schema Library page edits entries with `SchemaBuilder Mode="value"`, so a
|
||||
library entry that references *another* library entry — which `ManagementActor.cs:2921` explicitly
|
||||
permits — is flattened to `{"type":"string"}` the moment an operator edits it.
|
||||
|
||||
**Reachability — correction.** An earlier draft of this report called this "worse than merely
|
||||
unfinished: the feature is reachable, and touching it destroys data." That overstates it. The
|
||||
corruption requires a `$ref` to exist first, and per §6 **no `$ref` exists anywhere in either
|
||||
environment** — and since there is no UI path (§2) and no CLI path (§7) to author one, the only way
|
||||
to create one today is a hand-edit of the database. So this is a **trap laid for the first ref**,
|
||||
not an active defect. It becomes live the moment §5.2 ships, or the moment someone hand-authors a
|
||||
reference.
|
||||
|
||||
---
|
||||
|
||||
## 4. Gap 3 — library entries don't travel between environments
|
||||
|
||||
`SharedSchema` appears **nowhere** in `src/ZB.MOM.WW.ScadaBridge.Transport/`.
|
||||
|
||||
Contrast its sibling `SharedScript`, which is a first-class Transport artifact across
|
||||
`Serialization/EntityDtos.cs`, `Serialization/EntitySerializer.cs`, `Export/BundleExporter.cs`,
|
||||
`Export/ResolvedExport.cs`, `Export/DependencyResolver.cs`, `Import/ArtifactDiff.cs` and
|
||||
`Import/BundleImporter.cs`.
|
||||
|
||||
**Consequence:** a ref that resolves in dev dangles after import into prod. The import-time script
|
||||
trust gate won't catch it — that gate covers script *bodies*, and this is a schema.
|
||||
|
||||
---
|
||||
|
||||
## 5. Remediation, if the feature is kept
|
||||
|
||||
Three separable slices, in dependency order. **Read §6 first** — on current evidence none of these
|
||||
is urgent, and the prior question is whether the feature should exist at all.
|
||||
|
||||
### 5.1 Close the round-trip trap (small, independent)
|
||||
|
||||
Give `SchemaNode` a passthrough for subtrees the model doesn't understand, so an unrecognised node
|
||||
(`$ref` included) round-trips **verbatim** instead of collapsing to `{"type":"string"}`. Closes
|
||||
Gap 2 outright, needs no new UI, and is a **hard prerequisite for 5.2** — shipping the authoring UI
|
||||
without it would make the corruption reachable for the first time.
|
||||
|
||||
Test to add alongside: a round-trip guard asserting a `$ref` node survives parse→serialize at both
|
||||
root and property level — run it against the current code first and observe it fail, or it proves
|
||||
nothing.
|
||||
|
||||
### 5.2 Make refs authorable (the actual "wire it up")
|
||||
|
||||
Add a library-reference entry to the `SchemaBuilder` type picker, populated from the existing
|
||||
`ISchemaLibraryQueryService` (already registered, `CentralUI/ServiceCollectionExtensions.cs:142`).
|
||||
Emits `{"$ref":"lib:Name"}`; with 5.1 in place it round-trips. This is what actually connects the
|
||||
Schema Library page to the rest of the system.
|
||||
|
||||
### 5.3 Carry `SharedSchema` in Transport bundles (separate, larger)
|
||||
|
||||
DTO + serializer + exporter + dependency resolution (walk definition bodies for `lib:` refs so
|
||||
referenced entries are pulled into the bundle) + differ + importer. Mirrors the `SharedScript`
|
||||
shape throughout. Worth its own pass, and worth deciding whether an unresolved ref on import is a
|
||||
blocker row or an advisory warning — the existing name-resolution severity split (template =
|
||||
advisory, ApiMethod = hard error) is the precedent.
|
||||
|
||||
---
|
||||
|
||||
## 6. Is the feature needed at all?
|
||||
|
||||
The gaps above describe an unfinished feature. The prior question — asked after the audit — is
|
||||
whether it should be finished, and the evidence says not on current usage.
|
||||
|
||||
### 6.1 Usage: zero, in both environments
|
||||
|
||||
Queried live against `scadabridge-mssql` on 2026-08-11:
|
||||
|
||||
| | `ScadaBridgeConfig` (docker rig) | `ScadaBridgeConfig2` (docker-env2) |
|
||||
|---|---|---|
|
||||
| `SharedSchemas` rows | **0** | **0** |
|
||||
| Definitions containing `$ref` | **0** | **0** |
|
||||
| `ApiMethods` | 0 | 0 |
|
||||
| `TemplateScripts` | 31 | 15 |
|
||||
| …of those, carrying *any* parameter definition | 5 | 4 |
|
||||
|
||||
Not only has the library never been used — most scripts don't declare a typed parameter shape at
|
||||
all, which is the precondition for ever wanting a reusable one.
|
||||
|
||||
*(Production `wonder-app-vd03` was deliberately not queried; see §7.)*
|
||||
|
||||
### 6.2 Origin: speculative, not requirement-driven
|
||||
|
||||
It was not built to satisfy a stated need. The schema-editor design doc lists it under deferred
|
||||
future work — reuse of `$ref` across templates *"could be a future template-level schema library"*
|
||||
(`docs/plans/2026-05-12-script-schema-editor.md:117-118`). It was built later as M9/T32. The
|
||||
`Scope` column is likewise documented as being for *future* namespacing — a speculative field on a
|
||||
speculative feature.
|
||||
|
||||
### 6.3 The benefit is authoring-time DRY only
|
||||
|
||||
Resolution is **inlining, not indirection**: `FlatteningPipeline` and `SchemaRefResolver` expand
|
||||
`lib:Name` into the schema body at deploy / validation / runtime. The library therefore adds no
|
||||
runtime capability whatsoever — an inline schema does the entire job. Its single benefit is not
|
||||
retyping a shared shape at authoring time.
|
||||
|
||||
That benefit also carries a cost that *grows with adoption*: there is no reverse index. Editing
|
||||
`lib:Address` silently changes every referencing contract at the next deploy, with no
|
||||
"what references this?" view and no versioning. Tolerable at 2 references, unpleasant at 20. Any
|
||||
decision to finish the feature should budget for an impact-analysis view alongside 5.2.
|
||||
|
||||
### 6.4 Options
|
||||
|
||||
1. **Delete it.** The resolver plumbing (4 consumers), the page, entity, table, repository,
|
||||
ManagementActor handlers and tests are all carried for zero users. Defensible on the evidence.
|
||||
2. **Leave it dormant, fix nothing.** Zero cost today, because per §3 the defect cannot fire
|
||||
without a ref and no ref exists. Revisit when a genuinely repeated payload shape appears.
|
||||
3. **Finish it** (5.1 + 5.2 + 5.3, plus an impact-analysis view per 6.3). Justified only by known
|
||||
upcoming work with a repeated non-trivial shape — e.g. one MES order envelope shared across
|
||||
several inbound methods.
|
||||
|
||||
**Recommendation: option 2, with a hard trigger.** If a `lib:` ref is ever authored, §5.1 must land
|
||||
in the same change or the builder will destroy it. If nothing needs the library within roughly six
|
||||
months, take option 1 rather than leaving permanent unexercised surface in the codebase.
|
||||
|
||||
---
|
||||
|
||||
## 7. Notes
|
||||
|
||||
- Nothing in this report was changed. No fix has been applied; the verification probe used in §3
|
||||
was deleted after use.
|
||||
- The CLI has no schema-library commands either (`src/ZB.MOM.WW.ScadaBridge.CLI/` has no
|
||||
`SharedSchema` reference), so there is no scriptable authoring path as a workaround. This is why
|
||||
§3's trap is currently unreachable: hand-editing the database is the only way to create a ref.
|
||||
- The §6.1 counts are from the two **local docker** environments only. Production
|
||||
(`wonder-app-vd03`) was not queried — that is an operator action, not one to take unasked. If
|
||||
that box carries inbound methods with `$ref` definitions, §3 is live there and the §6.4
|
||||
recommendation changes.
|
||||
@@ -101,6 +101,12 @@ var expanderUsesSharedSqlStore =
|
||||
configuration["ScadaBridge:Node:Role"], "Central", StringComparison.OrdinalIgnoreCase)
|
||||
&& SecretsRegistration.UsesGrpcHub(configuration);
|
||||
|
||||
// Secrets 0.6.2 validates Secrets:SqlitePath as absolute AND outside the content root, finding the
|
||||
// content root from IHostEnvironment — which this throwaway ServiceCollection does not have, and
|
||||
// which does not exist yet anyway (the host is not built until further down). Both the resolution
|
||||
// and the registration therefore live in SecretsRegistration, where a test can enforce them; see
|
||||
// AddPreHostSqliteExpander for why the content-root argument is load-bearing and silent when
|
||||
// dropped, and PreHostSecretsContentRootTests for the regression guard.
|
||||
var expanderServices = new ServiceCollection();
|
||||
if (expanderUsesSharedSqlStore)
|
||||
{
|
||||
@@ -109,7 +115,8 @@ if (expanderUsesSharedSqlStore)
|
||||
}
|
||||
else
|
||||
{
|
||||
expanderServices.AddZbSecrets(configuration, "Secrets");
|
||||
expanderServices.AddPreHostSqliteExpander(
|
||||
configuration, SecretsRegistration.ResolveHostContentRoot());
|
||||
}
|
||||
|
||||
#pragma warning disable ASP0000 // deliberate throwaway container
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using Microsoft.AspNetCore.Routing;
|
||||
using Microsoft.Extensions.Hosting.WindowsServices;
|
||||
using ZB.MOM.WW.Secrets.DependencyInjection;
|
||||
using ZB.MOM.WW.Secrets.Replicator.Grpc.DependencyInjection;
|
||||
using ZB.MOM.WW.Secrets.Replicator.SqlServer.DependencyInjection;
|
||||
@@ -185,6 +186,65 @@ public static class SecretsRegistration
|
||||
&& ResolveReplicationMode(config) == SecretsReplicationMode.Grpc;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the content root the HOST will use, from outside the host — the Layer-A
|
||||
/// <c>${secret:}</c> expander in <c>Program.cs</c> runs before any builder exists, so
|
||||
/// <c>builder.Environment.ContentRootPath</c> is not available to it.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Mirrors the framework's own resolution order. The Windows-Service branch is the one that
|
||||
/// is easy to get wrong: <c>UseWindowsService</c> sets both the current directory and the
|
||||
/// content root to <see cref="AppContext.BaseDirectory"/>, but only once the host is built —
|
||||
/// at expander time the process working directory is still <c>C:\Windows\system32</c>, so
|
||||
/// taking the current directory there would compare the store path against the wrong root.
|
||||
/// </remarks>
|
||||
/// <returns>The absolute content-root path the host will resolve.</returns>
|
||||
public static string ResolveHostContentRoot() =>
|
||||
Environment.GetEnvironmentVariable("ASPNETCORE_CONTENTROOT")
|
||||
?? Environment.GetEnvironmentVariable("DOTNET_CONTENTROOT")
|
||||
?? (WindowsServiceHelpers.IsWindowsService()
|
||||
? AppContext.BaseDirectory
|
||||
: Directory.GetCurrentDirectory());
|
||||
|
||||
/// <summary>
|
||||
/// Composes the SQLite secrets stack for the Layer-A pre-host <c>${secret:}</c> expander.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// This exists as a named method rather than an inline <c>AddZbSecrets</c> call so that the
|
||||
/// content-root argument is covered by a test (<c>PreHostSecretsContentRootTests</c>). The
|
||||
/// argument is load-bearing and its absence is SILENT: the expander composes into a throwaway
|
||||
/// <see cref="IServiceCollection"/> that has no <c>IHostEnvironment</c>, so without it the
|
||||
/// under-content-root rule does not run, the migrator creates the store at the rejected path,
|
||||
/// and the boot then fails once the real host registers <c>IHostEnvironment</c> — leaving an
|
||||
/// empty database behind. That "the file is there, it's just empty" artifact is the confusing
|
||||
/// half of the 2026-08-09 MxGateway outage the guard exists to prevent.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// A correctly configured path shows NO symptom either way, so this cannot be verified by
|
||||
/// observing a clean boot — the exposure is that the guard is not running when the store is
|
||||
/// created, not that any particular path is wrong.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
/// <param name="services">The throwaway expander service collection.</param>
|
||||
/// <param name="config">Application configuration.</param>
|
||||
/// <param name="contentRoot">The host's content root, from <see cref="ResolveHostContentRoot"/>.</param>
|
||||
/// <returns>The same collection, for chaining.</returns>
|
||||
public static IServiceCollection AddPreHostSqliteExpander(
|
||||
this IServiceCollection services,
|
||||
IConfiguration config,
|
||||
string contentRoot)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(services);
|
||||
ArgumentNullException.ThrowIfNull(config);
|
||||
ArgumentException.ThrowIfNullOrWhiteSpace(contentRoot);
|
||||
|
||||
// Do NOT drop the third argument. The 3-argument overload still compiles and silently
|
||||
// reopens the gap described above; the regression would arrive disguised as removing a
|
||||
// redundant parameter.
|
||||
return services.AddZbSecrets(config, SecretsSectionPath, contentRoot);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The one definition of "a Central node in Grpc mode has a usable shared-store connection
|
||||
/// string", shared by <see cref="AddScadaBridgeSecrets"/> and the Layer-A expander in
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"ScadaBridge": {
|
||||
"_secretsClusterPosture": "Central pair KEK/store posture (production): see docs/operations/2026-07-16-secrets-clustered-master-key.md. Committed default is Source=Environment + relative SqlitePath (dev-safe); production uses a File KEK + shared store volume delivered out-of-band. Do NOT hardcode /shared paths here — the pre-host expander migrates the store at every Central boot and would break dev/test.",
|
||||
"_secretsClusterPosture": "Central pair KEK/store posture (production): see docs/operations/2026-07-16-secrets-clustered-master-key.md. Committed default is Source=Environment with SqlitePath UNSET (Secrets 0.6.2 rejects a relative path, and a path inside the content root, at startup — so the library's absolute per-user default is taken in dev); production uses a File KEK + shared store volume delivered out-of-band. Do NOT hardcode /shared paths here — the pre-host expander migrates the store at every Central boot and would break dev/test.",
|
||||
"_nodeName": "Host-018: NodeName stamps SourceNode on AuditLog/Notifications/SiteCalls rows (CLAUDE.md 'Centralized Audit Log' decision) and backs IX_AuditLog_Node_Occurred. Convention: 'central-a'/'central-b' for central nodes, 'node-a'/'node-b' for site nodes. Override per-node in multi-node deployments (the docker per-node configs do this). When left at the default below, single-node dev rows are stamped with 'central-a'; an empty value normalises to a NULL SourceNode.",
|
||||
"Node": {
|
||||
"Role": "Central",
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
}
|
||||
},
|
||||
"Secrets": {
|
||||
"SqlitePath": "scadabridge-secrets.db",
|
||||
"_sqlitePath": "Deliberately unset. ZB.MOM.WW.Secrets 0.6.2 validates Secrets:SqlitePath at startup — it must be ABSOLUTE and must NOT sit inside the content root, because a path inside the deployment directory is destroyed by an ordinary upgrade (this is what wiped the MxGateway API-key store on 2026-08-09). The former value here ('scadabridge-secrets.db') was relative and would now fail the boot. Leaving it unset gives dev the absolute per-user LocalApplicationData default; every real deployment supplies its own absolute path via its environment overlay.",
|
||||
"MasterKey": { "Source": "Environment", "EnvVarName": "ZB_SECRETS_MASTER_KEY" },
|
||||
"RunMigrationsOnStartup": true,
|
||||
"ResolveCacheTtl": "00:00:30",
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using ZB.MOM.WW.ScadaBridge.Host;
|
||||
using ZB.MOM.WW.Secrets.Sqlite;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Guards the content-root argument on the Layer-A pre-host <c>${secret:}</c> expander
|
||||
/// (<see cref="SecretsRegistration.AddPreHostSqliteExpander"/>, called from <c>Program.cs</c>).
|
||||
///
|
||||
/// <para>
|
||||
/// The argument is load-bearing and its absence is SILENT. The expander composes secrets into a
|
||||
/// throwaway <see cref="ServiceCollection"/> with no <c>IHostEnvironment</c>; without an explicit
|
||||
/// content root the under-content-root rule does not run, the migrator creates the store at the
|
||||
/// rejected path, and the boot then fails once the real host registers <c>IHostEnvironment</c>.
|
||||
/// The leftover empty database is the "the file is there, it's just empty" artifact that made the
|
||||
/// 2026-08-09 MxGateway outage read as corruption rather than as a deployment error.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// <b>These tests must resolve and run <see cref="SqliteSecretsStoreMigrator"/>.</b> Resolving
|
||||
/// only the connection factory would make every file assertion here vacuous — that constructor
|
||||
/// never touches the filesystem, so no store is created whether the guard runs or not, and the
|
||||
/// tests would pass for the wrong reason. <see cref="Control_ValidPath_ActuallyCreatesAStore"/>
|
||||
/// exists to prove the harness really does create files.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// <b>Assertion order is deliberate: artifact first, outcome second.</b> Asserting "it threw"
|
||||
/// first masks the real finding — a regression that stops the throw but still creates the store
|
||||
/// would report "no exception was thrown" and never mention the database sitting at the rejected
|
||||
/// path, which IS the defect. Verified by simulating the regression (dropping to the 3-argument
|
||||
/// overload) and confirming these fail on the FILE assertion, not the exception one.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public class PreHostSecretsContentRootTests
|
||||
{
|
||||
private static string NewDir() =>
|
||||
Path.Combine(Path.GetTempPath(), "prehost-secrets-" + Guid.NewGuid().ToString("N"));
|
||||
|
||||
private static IConfiguration Config(string sqlitePath) =>
|
||||
new ConfigurationBuilder().AddInMemoryCollection(new Dictionary<string, string?>
|
||||
{
|
||||
["Secrets:SqlitePath"] = sqlitePath,
|
||||
["Secrets:MasterKey:Source"] = "Environment",
|
||||
["Secrets:MasterKey:EnvVarName"] = "ZB_SECRETS_MASTER_KEY",
|
||||
}).Build();
|
||||
|
||||
/// <summary>
|
||||
/// Mimics the pre-host expander exactly: throwaway collection, no IHostEnvironment,
|
||||
/// register, then run the migrator — which is what creates the database.
|
||||
/// </summary>
|
||||
private static async Task<Exception?> ComposeAndMigrate(string contentRoot, string sqlitePath)
|
||||
{
|
||||
Directory.CreateDirectory(contentRoot);
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
|
||||
return await Record.ExceptionAsync(async () =>
|
||||
{
|
||||
services.AddPreHostSqliteExpander(Config(sqlitePath), contentRoot);
|
||||
await using var provider = services.BuildServiceProvider();
|
||||
await provider.GetRequiredService<SqliteSecretsStoreMigrator>().MigrateAsync(default);
|
||||
});
|
||||
}
|
||||
|
||||
private static void AssertNoStoreAt(string path)
|
||||
{
|
||||
string[] leftovers = new[] { path, path + "-wal", path + "-shm" }
|
||||
.Where(File.Exists)
|
||||
.ToArray();
|
||||
|
||||
Assert.True(
|
||||
leftovers.Length == 0,
|
||||
"A rejected path must not be left holding a store file — an empty database at the "
|
||||
+ "rejected path is exactly the artifact this guard exists to prevent. Found: "
|
||||
+ string.Join(", ", leftovers));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Control_ValidPath_ActuallyCreatesAStore()
|
||||
{
|
||||
// Proves the harness is capable of creating a store, so that the "no file" assertions
|
||||
// in the other two tests are meaningful rather than vacuous.
|
||||
var storeDir = NewDir();
|
||||
Directory.CreateDirectory(storeDir);
|
||||
var path = Path.Combine(storeDir, "secrets.db");
|
||||
|
||||
var thrown = await ComposeAndMigrate(NewDir(), path);
|
||||
|
||||
Assert.Null(thrown);
|
||||
Assert.True(File.Exists(path), "the control must create a store, or the guards prove nothing");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PathInsideContentRoot_IsRejected_AndLeavesNoStore()
|
||||
{
|
||||
var contentRoot = NewDir();
|
||||
var path = Path.Combine(contentRoot, "data", "scadabridge-secrets.db");
|
||||
|
||||
var thrown = await ComposeAndMigrate(contentRoot, path);
|
||||
|
||||
AssertNoStoreAt(path);
|
||||
Assert.NotNull(thrown);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RelativePath_IsRejected_AndLeavesNoStore()
|
||||
{
|
||||
// Unique name: a stale scadabridge-secrets.db from an earlier run under the old relative
|
||||
// default sits in the test output directory and would make this assertion lie.
|
||||
var path = "prehost-probe-" + Guid.NewGuid().ToString("N") + ".db";
|
||||
|
||||
var thrown = await ComposeAndMigrate(NewDir(), path);
|
||||
|
||||
AssertNoStoreAt(path);
|
||||
Assert.NotNull(thrown);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user