Commit Graph

86 Commits

Author SHA1 Message Date
Joseph Doherty b34ec10f8b fix(deploymentmanager): bulk site deploy skips up-to-date instances and un-poisons the shared DbContext (arch-review Low P7-5 + P7-6)
P7-6 (behaviour fix, plan decision 1): DeploySiteAsync redeployed EVERY
deployable instance unconditionally. A redeploy is not idempotent at the site —
it resets static attribute overrides and clears native alarm state by design — so
one site-wide push destroyed live operational state on every unchanged instance
in the batch, which is normally most of them. Bulk deploy now pre-filters with
IStaleInstanceProbe (the same seam the bundle importer and Deployments page use;
its memo timing was fixed by the Medium wave's P7-4): an instance whose freshly
computed revision hash still matches its DeployedConfigSnapshot is reported in the
existing additive SkippedInstances list with reason "up-to-date" and never
flattened, staged or sent. The filter is deliberately conservative — every
uncertainty deploys: only an Enabled instance qualifies, a missing snapshot
disqualifies, a prior record that ShouldQuerySiteBeforeRedeploy flags goes through
prepare so the reconcile that HEALS it still runs, and a null/faulting probe means
"indeterminate". New additive MgmtDeploySiteCommand.Force (default false = the new
filtered behaviour) plus CLI `deploy site --force` restores the unconditional push
for the case central cannot see (restored volume, rebuilt node). Composes with the
Medium wave's P7-2: Disabled wins — force suppresses the up-to-date filter only,
never the Disabled skip.

P7-5 (verified, then fixed): the cascade REPRODUCES. All three bulk phases share
one scoped DbContext, and a failed SaveChanges does not untrack the offending
entity — EF re-attempts it on the next save, including MarkDeploymentFailedAsync's
cleanup write. Measured on a real SQLite-backed ScadaBridgeDbContext: a permanent
save failure on instance 1's post-success side effects failed instances 2, 3 and 4
and left all three deployment records stranded InProgress. MarkDeploymentFailedAsync
now detaches the pending changes (new IDeploymentManagerRepository.DetachPendingChanges)
when the cleanup save fails and retries the Failed write once on the clean tracker,
so later instances save cleanly and the record reaches a terminal status. The
DeployFailed audit row moved outside the status-write try (best-effort, must not be
lost because the status write failed).

Tests: 5 new DeploySiteAsyncTests (skip + reason + no site round-trip; --force
deploys; Disabled skipped under both force values; the three not-enough-to-skip
guards), 2 new IntegrationTests over a real EF context (the poisoned-context
cascade, and the narrow EF tracking fact the fix rests on), CLI --force flag
parse test, and a MgmtDeploySite wire-default test. Proof of failure: both P7-6
behaviour tests and both P7-5 assertions fail against the unfixed code.
2026-08-19 16:41:04 -04:00
Joseph Doherty 7ad1872454 fix(management): strict enum parsing for DataType/ElementDataType/AlarmTriggerType (P6-10)
ManagementActor's ParseDataType/ParseElementDataType (bare Enum.TryParse) and
the two AlarmTriggerType parse sites in HandleAddAlarm/HandleUpdateAlarm (raw
Enum.Parse) all admitted ordinals ("2"), undefined values ("99" — a
deploy-time bomb that saves fine and only detonates at deploy), and
comma-joined name lists (bitwise-ORed together regardless of [Flags], e.g.
"String,Int32" silently becoming DateType.DateTime). A typo against the raw
Enum.Parse sites also surfaced as a masked "An internal error occurred"
rather than a curated field-naming error.

Adds ParseEnumStrict<TEnum>, mirroring the existing ParseDeploymentStatus
guard (names only, defined values only, comma-token rejection per the
Medium-wave T8 nuance / AuditQueryParamParsers.ParseEnumListStrict pattern),
and routes all four call sites through it.

Tests: 22 new ManagementActorTests cases per parse site (ordinal, undefined
numeric, comma-joined, and typo all rejected with the curated field-naming
error; every defined name still parses through to the service layer).
Proof of failure recorded in each rejection test's doc comment; verified
live by stashing the ManagementActor.cs fix — exactly the 14 new rejection
tests fail pre-fix (13 acceptance tests pass unchanged), confirming the
defect and the fix.
2026-08-19 16:41:04 -04:00
Joseph Doherty 9a3f006721 fix(management)+fix(cli): neutralize CSV formula injection + align export columns; deadline + Ctrl-C on audit export streaming (arch-review 2026-08-19 P6-7 + P6-8)
P6-7 (CWE-1236): AuditEndpoints CSV export quoted cells on comma/quote/CR/LF
but never neutralized a leading =, +, -, @, TAB, or CR — inbound-API-caller
text landing in Target/ErrorMessage/RequestSummary/ResponseSummary/Extra
could execute as a formula/DDE trigger when opened in Excel. AppendField now
prefixes a literal single quote on free-text columns only (never the
GUID/enum/numeric/date/bool columns the exporter formats itself), applied
before RFC 4180 quoting. The CSV was also missing ExecutionId/
ParentExecutionId/SourceNode that the JSONL export already carried; both are
now 24 columns in the same AuditExportRow order. A second, independently
maintained CSV writer (CentralUI's AuditLogExportService, feeding the Audit
Log page's bulk-export button) had the identical gap and got the identical
fix + column alignment.

P6-8: `audit export` was the one CLI HTTP path with no deadline at all — a
residual gap from the ManagementHttpClient timeout-honesty fix, which made
HttpClient.Timeout infinite and the per-call token the single deadline
everywhere except this streamed GET. RunExportAsync now takes a headers-
received deadline (CancelAfter, reusing ManagementHttpClient.
DefaultConnectTimeout) around the initial GetAsync, an inactivity deadline
around the streaming copy (rearmed on every successful read, so large exports
are bounded by silence, not total size), and an external CancellationToken
wired to Console.CancelKeyPress in AuditCommands so Ctrl-C cancels promptly.
A stalled/faulted stream now produces a friendly error noting the partial
output file instead of hanging or dumping a stack trace.
2026-08-19 15:01:45 -04:00
Joseph Doherty 561582747e fix(centralui): stop debug-stream sessions abandoned mid-start — dispose-vs-connect race (arch-review 2026-08-19 P6-6)
A debug-stream session created after the consumer had already gone away was
leaked permanently. Both consumers awaited StartStreamAsync (up to 30s) and only
then published the session; a circuit teardown / client disconnect inside that
window ran the release path first, found nothing registered and stopped nothing.
The session stayed registered in the singleton DebugStreamService, whose
keepalive renews registered sessions forever (the 5-minute orphan reaper only
collects UNregistered ones), pinning the site gRPC subscription, the
DebugStreamBridgeActor and the dead consumer's object graph until process
restart.

Both call sites now publish the session and re-check liveness under a single
monitor, so exactly one side tears it down and neither can miss it:

- DebugView.Connect/Dispose/Disconnect share a _sessionLock; Dispose sets
  _disposed and takes the session in one step, Connect publishes only if not
  disposed and otherwise stops the session it just created. Dispose can run off
  the renderer's thread, so the Blazor sync context is not relied on. Disconnect
  and the onTerminated callback go through the same take-once helper, which also
  makes a repeated Dispose stop-once instead of stop-again.
- DebugStreamHub uses the per-connection Items monitor: OnDisconnectedAsync marks
  the connection gone before releasing, SubscribeInstance publishes only if
  neither that flag nor ConnectionAborted is set, and UnsubscribeInstance takes
  the id atomically.

DebugStreamService.StartStreamAsync/StopStream become virtual purely as a test
seam (no behaviour change, keepalive/orphan contract untouched) so the race can
be driven deterministically by a gated stub.

Tests: DebugViewConnectDisposeRaceTests (3) + DebugStreamHubSubscribeRaceTests
(3). Verified fail-unfixed: with the two source fixes stashed, both
dispose-mid-start tests report zero StopStream calls and the DebugView
double-dispose test reports two.
2026-08-19 14:53:01 -04:00
Joseph Doherty 64e6ba70e8 fix(management): site-scope ListDataConnections + hoist scope checks above resolve/status oracles (arch-review 2026-08-19 P6-4 + P6-5)
P6-4: HandleListDataConnections took no user at all and returned EVERY
site's connections to any authenticated caller, nullifying the row-98
fail-closed gate on HandleGetDataConnection — Get denied exactly what List
handed over. DataConnectionPublicShape already scrubs secrets, so the leak
was site topology plus a connection-id existence oracle, but it still
defeated scoping. List now takes the AuthenticatedUser: an explicit SiteId
goes through EnforceSiteScope (the ListAreas shape), and an unfiltered list
from a site-scoped non-Administrator caller is narrowed to their permitted
sites (the ListSites shape). GetRequiredRoles is untouched — scoping is not
a role gate, so the frozen 138-command authz matrix is unaffected.

The Central UI Design > Connections page bypasses the actor entirely
(ISiteRepository.GetAllDataConnectionsAsync direct) and IS reachable by a
scoped user: it is gated RequireDesign, and SiteScopeService explicitly does
NOT exempt Designer (register row 86). It now filters sites through
SiteScope.FilterSitesAsync and keeps only their connections, mirroring
EventLogs/AlarmSummary.

P6-5: four handlers resolved their target — and on approve/reject reported
its status — BEFORE the scope check, so an out-of-scope caller could tell a
real id from a fake one, and read a secured write's lifecycle status, from
the error shape alone. The scope check is hoisted above the not-found and
status checks in HandleSubmitSecuredWrite, HandleApproveSecuredWrite,
HandleRejectSecuredWrite and HandleDebugSnapshot, with the unresolvable case
routed through the existing DenyUnresolvedTargetIfScoped so the denial is
identical whether or not the target exists. System-wide callers keep their
ordinary not-found/status answers, and the P6-1 CAS semantics on
approve/reject are unchanged (only the checks above the CAS moved).

Tests: 8 new ManagementService tests + 2 CentralUI bUnit tests, all verified
to FAIL against the unfixed handlers (src reverted, tests re-run) and pass
after — including the unscoped/Administrator control cases proving behaviour
for system-wide callers is unchanged. Full ManagementService.Tests 545 green,
CentralUI.Tests 1073 green.
2026-08-19 14:49:24 -04:00
Joseph Doherty 61a14bccab fix(management): CAS-guard secured-write reject (WHERE Status='Pending') so reject can never falsify an executed two-person write (arch-review 2026-08-19 P6-1 High)
Reject read the row as Pending, then landed a blind full-row UpdateAsync. A
concurrent approve could win its own Pending->Approved CAS in that gap and enter
the multi-second WriteTagAsync relay; the reject's update then stamped
Status/ExecutedAtUtc/ExecutionError to Rejected mid-relay, so the two-person-control
record said Rejected for a write that physically executed at the plant. Works
cross-node (both centrals share one MS SQL).

- ISecuredWriteRepository/SecuredWriteRepository: new TryMarkRejectedAsync,
  mirroring TryMarkApprovedAsync exactly (conditional UPDATE ... WHERE
  Status='Pending', writes only the columns the reject transition owns). No schema
  change, no migration, no rowversion column.
- HandleRejectSecuredWrite: uses the CAS; on loss re-reads the row and returns the
  status that actually won (same UX shape as approve's lost-race error), and emits
  the SecuredWriteReject audit row ONLY on a won CAS.
- Audited the other lifecycle writes: approve/reject/expiry (inline + the
  opportunistic list sweep) now ALL transition only out of Pending, so winning the
  approve CAS gives that leg exclusive ownership of the row and its post-relay
  Executed/Failed update stays safe as an unconditional write. Documented in place.
- Tests: repository-level approve-vs-reject races in both orders (exactly one wins,
  loser false, row reflects the winner only) plus reject-vs-expiry; handler-level
  lost-race reject (conflict error, no write, no audit row) and approve on an
  already-rejected row (no CAS, no relay, no audit).
2026-08-19 13:54:45 -04:00
Joseph Doherty 16056f8eb6 fix(management): ParseDeploymentStatus rejects ordinals — Enum.TryParse accepts "2" as InProgress despite the row-88 names-only contract; same LooksNumeric guard as the audit strict parser (found by the Wave 6 rows-98/99 agent survey) 2026-08-17 06:21:12 -04:00
Joseph Doherty f33f25a10b fix(auth+cli): fail-closed site-scope gate (row 98); case-insensitive audit filter values with a strict server-side check (row 99)
Register row 98 — the site-scope authorization gate was FAIL-OPEN on an
unresolvable target. The resolve-then-check shape was written as
`if (target != null) EnforceSiteScope(...)`, so a null resolve skipped the
check entirely and a site-scoped caller passed the gate for any id in the
system. Authorization now fails CLOSED: a target that cannot be PROVEN to sit
inside the caller's scope is treated as outside it.

The defect turned out to have THREE more instances than the register recorded.
Besides `EnforceSiteScopeForInstance` and `EnforceSiteScopeForIdentifier`, the
same inline shape sat in `HandleGetInstance`, `HandleGetSite` and
`HandleGetDataConnection` — read handlers that resolve their own target and so
never call the helpers. Those were the sharper half of the bug: they answered
success-with-null for a nonexistent id and Unauthorized for an out-of-scope
one, making the pair an id-enumeration oracle over the whole fleet. All five
now route through one shared `DenyUnresolvedTargetIfScoped` arm.

Two properties are load-bearing and pinned by tests:
  - System-wide callers (Administrator, or a Deployer with no site grants) are
    untouched — they return at the guards above the lookup and keep the
    handler's ordinary "not found" / null answer.
  - The denial is indistinguishable from an out-of-scope denial: same
    ManagementUnauthorized shape, and a message that never states whether the
    target exists.

Register row 99 — CLI `audit query --status delivered` failed client-side while
the server would have accepted it: `AcceptOnlyFromAmong` matches ordinally
against the capitalised literals. Same treatment row 88 gave `deploy status` —
the client-side gate is removed, the valid set moves into the option help text,
and the server becomes the single authoritative check.

That required ADDING the server check, not just relying on one: the shared
audit filter parser was LAX, silently dropping an unrecognised value and
leaving the dimension unconstrained — so removing the CLI gate alone would have
turned a loud client-side failure into a plausible, complete-looking page that
did not answer the question asked. New `ParseEnumListStrict` (case-insensitive,
rejects ordinals — `Enum.TryParse` accepts "3", and a filter is a name-based
contract) backs the ManagementService /api/audit/query + /api/audit/export
endpoints, which now return 400 BAD_REQUEST naming the valid set.

Other AcceptOnlyFromAmong sites surveyed in the audit group: `--channel`,
`--kind`, `--status` on BOTH `query` and `export` (6 uses) plus `export
--format` (rejected `--format CSV` though the server lowercases it) — all seven
fixed here. Deliberately left alone: `template folder reorder --direction`
(pure client-side literal, no server-side parse to defer to) and the global
`--format json|table` (CLI-local output selection, never sent to a server).

The CentralUI AuditLogPage drill-in parser and the CentralUI export endpoint it
feeds deliberately KEEP the lax parse — they read a query string a human may
have bookmarked, where a stale fragment must still render the page. Both
contracts now live side by side in AuditQueryParamParsers and are documented as
non-reconcilable.

Docs: Component-ManagementService.md gains a "Site-scope gate: fail-closed on
an unresolvable target" section and a strict-filter note on the audit
endpoints; Component-Security.md records the fail-closed shape under Deployer
site scoping; the CLI README marks the audit filter values case-insensitive and
states the no-silent-drop / no-ordinals contract.

Tests: 5 new ManagementActorTests fail before the row-98 fix (verified by
reverting ManagementActor.cs) and cover scoped+nonexistent -> denied,
system-wide+nonexistent -> unchanged (both Administrator and no-grant Deployer),
scoped+in-scope -> allowed, and the indistinguishability property. 21 CLI tests
fail before the row-99 fix. Four pre-existing tests that pinned the removed
client-side gate were rewritten to pin the new contract.
2026-08-17 06:05:29 -04:00
Joseph Doherty 9f67147e7b refactor(deployments): one paged read of DeploymentRecords + close its three drifts
Deferred-work register rows 37, 86, 87, 88 — the structural duplication and the
three behavioral divergences it had accumulated, fixed together.

Row 37 (structural). `IDeploymentManagerRepository.QueryDeploymentSummariesAsync`
is DELETED. It and `QueryDeploymentListPageAsync` were two paged reads of the same
insert-only table with different contracts — raw offset vs page number, no total vs
total, no instance join vs joined, instance-id-list scoping vs site-id scoping. The
CLI/HTTP `QueryDeployments` handler now goes through `QueryDeploymentListPageAsync`
with a `DeploymentListFilter`, exactly like the Central UI page. The
`DeploymentRecordSummary` projection went with it (`DeploymentListRow` is the single
row shape); its `MaxPageSize` const moved to `DeploymentListPage`, which also gains
`DefaultPageSize`.

Row 86 (Medium, authorization — the headline). `SiteScopeService.ResolveAsync` was
claims-only, so an Administrator carrying `SiteId` claims saw the whole fleet through
the CLI (`ManagementActor.EnforceSiteScope` has always exempted Administrator) and
only their granted sites on the Deployments page. One user, one dataset, two answers.
Administrator = system-wide is the intended semantic on both sides; the UI was the
defect. The role claim is read explicitly with an OrdinalIgnoreCase comparison rather
than via `IsInRole`, which compares claim VALUES ordinally and only resolves when the
identity carries the matching roleType — the two sides must not disagree on a
differently-cased role name. An Administrator's site claims are not collected at all,
so the invariant "system-wide => empty permitted-id set" holds for every caller.
Designer is NOT exempt: the class doc claimed "any Admin/Design user" is system-wide,
which was true of neither side, so the doc was corrected rather than the bypass
widened.

Row 87 (Low). The scoped branch used to resolve the site grant into an instance-id
list by loading EVERY instance in the system, non-transactionally, on every scoped
query; the grant now rides as `SiteIdScope` and is resolved by the query's own join.
An unparseable permitted-site id is DROPPED, never ignored — dropping can only narrow
a grant, discarding it would widen it to fleet-wide. And `PageSize <= 0` no longer
means UNLIMITED down at the repository (`--page-size 0` materialized the whole
retention window): it means "unspecified" and takes the CLI's own default of 50,
capped at 1000. Bounding without telling would be silent truncation, so the handler
now returns a `DeploymentQueryResult` envelope carrying `totalCount` plus the page
coordinates actually applied — with a CLI table projection (mirroring row 57's
`AuditConfigTableProjection`) that unwraps it and appends a "showing N of M" note.

Row 88 (Low, UX). An unrecognised `--status` fell back to "no filter", so a typo like
`--status Sucess` returned every status — a complete-looking answer to a question the
caller never asked. It is now a `ManagementError` naming the valid values. Parsing
stays case-insensitive; a numeric string is rejected too, since `Enum.TryParse`
accepts "7" and yields an undefined value (guarded with `Enum.IsDefined`). Loud
failure over silent full results, per row 46.

Tests: 7 new `SiteScopeServiceTests` pinning Administrator = system-wide (verified
failing 6/7 against the pre-fix service, the Designer pin passing both ways); the
ManagementActor deployment suite migrated onto the consolidated query with new pins
for the status error, case-insensitive parse, numeric rejection, page-size default/
cap/floor, empty-scope-on-garbage-grant, and "reads no instances at all"; a repository
test for site-scope+status composing over a bounded page; and 6 CLI tests for the new
table projection.
2026-08-17 05:15:18 -04:00
Joseph Doherty 41a5ecf14a feat(external-systems): deploy per-system retry policy to sites + CLI/management surface (register row 27)
An external system's MaxRetries/RetryDelay existed on the central entity but never
reached a site and had no management surface, so every ExternalSystem.CachedCall
buffered with the S&F engine default (DefaultMaxRetries 50 x DefaultRetryInterval
30s ~= 25 min to park) whatever central had configured. The sibling
DatabaseConnectionArtifact had always carried its equivalents.

The consumption end was already wired: ExternalSystemClient.CachedCallAsync already
read system.MaxRetries/RetryDelay and passed them to EnqueueAsync with the 0-as-unset
convention. Only the transport and persistence were missing.

Commons
- ExternalSystemArtifact gains MaxRetries/RetryDelay as optional positional params
  (0 / TimeSpan.Zero = unset), matching DatabaseConnectionArtifact rather than going
  nullable, so it maps 1:1 onto the entity and the proto3 scalars.
- Create/UpdateExternalSystemCommand gain nullable MaxRetries/RetryDelay, following
  the existing TimeoutSeconds "omit to preserve" convention. Both additive.

Central
- ManagementActor create maps both; update preserves-if-null.
- ArtifactDeploymentService projects them onto the artifact.

Wire
- site_command.proto ExternalSystemArtifactDto gains max_retries=7 /
  retry_delay=8 (google.protobuf.Duration), mirroring DatabaseConnectionArtifactDto.
  Additive: a pre-existing site decodes them as 0/absent, which IS the unset value,
  so old behaviour is preserved. Vendored C# regenerated via docker/regen-proto.sh
  sitecommand; csproj Protobuf item re-commented, no message types added or removed.

Site
- external_systems gains max_retries / retry_delay_ms via AddColumnIfMissing,
  DEFAULT 0 = unset. Storage upsert, repository SELECT/mapping (new ordinals 6/7
  appended so existing ones are untouched), apply path, and the legacy migrator's
  explicit column list all carry them.

Replicated-table decision: external_systems IS in SiteLocalDbSetup.ReplicatedTables,
so this was checked against the AckTime/native_alarm_state precedent. Real columns
are correct here, not a JSON bag: the table has no general-purpose config column
(auth_configuration and method_definitions are both semantically specific),
timeout_seconds already set the additive-column precedent on this exact table, and
sibling database_connections stores the same two values as plain columns. Verified in
LocalDb source that RegisterReplicated re-reads PRAGMA table_info every boot and its
install script is DROP-then-CREATE, so capture triggers rebuild over the new column
set automatically, and SiteStorageSchema.Apply runs before registration on the same
boot. The only cost is the ReplicatedTable.ComputeDigest change, which the sync
handshake compares fail-closed - a mixed-version pair refuses to sync loudly instead
of diverging silently. Both-nodes-together is already the standing site-pair rule, so
no new operational constraint; recorded in topology-guide.md.

Also fixed: ArtifactDiff.CompareExternalSystem never compared MaxRetries/RetryDelay,
so a bundle import that changed a system's retry budget previewed as "Identical" -
harmless while the values were inert, a live behaviour change now.

No EF migration: the central ExternalSystemDefinitions columns already exist
(InitialSchema). Central UI already had both form fields.

Tests: site storage round-trip + unset-default, site apply path, central artifact
projection, proto round-trip (via the shared sample), management create/preserve,
CLI option parse + alias, and diff reporting. Fail-before verified on the two core
tests (expected 7 got 0 on the read path; expected 9 got 0 on the apply path).

Docs: Component-ExternalSystemGateway, Component-StoreAndForward, Component-CLI,
components/StoreAndForward, components/SiteRuntime, deployment/topology-guide,
CLI README (which was also missing --timeout-seconds entirely).
2026-08-15 12:56:12 -04:00
Joseph Doherty cde4af77fd merge: audit-config query fix + rig live-probe seed (register rows 46, 48) — ValueTuple serialized as {} fixed with AuditLogQueryResult record + ITuple guard; docker/seed-live-probe.sh 2026-08-15 12:00:32 -04:00
Joseph Doherty 09108018f1 fix(management): audit-config query returned {} for every filter — project the repo ValueTuple into a named result record
Deferred-work register row 46. HandleQueryAuditLog returned ICentralUiRepository's
(IReadOnlyList<AuditLogEntry> Entries, int TotalCount) ValueTuple straight to
ManagementActor.SerializeResult. System.Text.Json writes properties only, and a
ValueTuple exposes Item1/Item2 as public FIELDS, so the whole payload serialized to
the literal "{}" — the CLI faithfully printed an empty object no matter how many
AuditLogEntries rows matched, with no error anywhere in the stack.

Fixed at the handler (the layer that leaks the tuple), not the CLI: new Commons record
AuditLogQueryResult(Entries, TotalCount), mirroring the SecuredWriteListResult shape
already used by the other paged management query.

Also guards the whole class of bug: SerializeResult now throws on any ITuple result,
so the next handler that returns a tuple fails loudly instead of silently emitting {}.

Tests (fail-before verified — the query test asserted exactly "{}"):
- QueryAuditLogCommand_ReturnsEntriesAndTotalCount
- SerializeResult_TupleResult_ThrowsInsteadOfEmittingEmptyObject
- SerializeResult_AuditLogQueryResult_EmitsEntriesAndTotalCount

Docs: Component-CLI.md + CLI README document the response shape and why it is a record.
2026-08-15 11:38:28 -04:00
Joseph Doherty b449fe201a fix(transport): curate BundleLockedException on the management surface (register row 51)
ManagementService references Commons but not Transport, so MapFault could not
see BundleLockedException and the per-bundle 3-strike lockout reached the CLI
as "An internal error occurred (CorrelationId=...)" — the operator got a
correlation id instead of the reason, and no hint that further attempts on
that bundle would be refused.

Applies the pattern register row 44 established for BundleUnlockThrottledException:

- Relocate BundleLockedException from Transport/Import/ to
  Commons/Types/Transport/, beside BundleUnlockThrottledException. Both
  consumers (BundleImporter, CentralUI TransportImport) already carried the
  Commons.Types.Transport using, so no call-site edits were needed.
- Add curated catch arms in ManagementActor.HandlePreviewBundle and
  HandleImportBundle that rethrow as ManagementCommandException, so the
  message survives MapFault's disclosure filter.
- Mirror the row-44 throttle fault-mapping test for both handlers.
- Document the relocation in docs/components/Transport.md and
  docs/requirements/Component-Transport.md.

Tests: ManagementService 499 passed (+2 new), Transport 167 passed.
2026-08-15 11:37:36 -04:00
Joseph Doherty 5ab26a4233 fix(transport): enforce the documented per-IP bundle-unlock throttle (row 44)
BundleUnlockRateLimiter was registered (ServiceCollectionExtensions), injected
into BundleImporter, unit-tested, documented on
TransportOptions.MaxUnlockAttemptsPerIpPerHour = 10 and pinned in
appsettings.Central.json — but _unlockRateLimiter was never invoked. A
brute-force passphrase attacker against a stolen bundle was bounded only by the
per-bundle 3-strike lockout, which a re-export side-steps (new ContentHash).

LoadAsync now registers every unlock attempt against the limiter. Ordering is
load-bearing: AFTER the per-bundle lockout check (hammering a locked bundle
spends no hourly budget) and BEFORE the decrypt (so a denial burns no per-bundle
strike — otherwise the throttle would be a way to lock a bundle someone is
legitimately importing). Over budget throws the new
BundleUnlockThrottledException, fail-closed with a clear message on both
surfaces; it lives in Commons because the management surface (Commons-only
references) must catch it to return a curated error rather than MapFault's
generic internal-error text.

Client key plumbing:
- Central UI: new scoped CircuitClientAddressProvider snapshots
  Connection.RemoteIpAddress at circuit establishment — the same constraint and
  the same pattern as CookieAuthenticationStateProvider, since HttpContext is
  null by the time a circuit submits a passphrase.
- CLI/management: additive ManagementEnvelope.ClientIp, stamped by
  ManagementEndpoints and threaded through DispatchCommand to the preview/import
  bundle handlers.
- No attributable address => a single SHARED "<unattributed>" bucket, not a
  bypass: unknown clients get a stricter budget, never none. Per-client
  isolation is only real behind a ForwardedHeaders-trusted proxy, the same
  caveat the login throttle carries (Component-Security.md).

Docs: Component-Transport.md + docs/components/Transport.md record the two-level
ordering, the client-key sourcing, the shared-bucket fallback and a
troubleshooting entry for the new error.

Tests: 6 importer tests (cap enforced; denial != passphrase failure, no strike
burned; per-client isolation; shared unattributed bucket; unencrypted bundles
consume no budget; a locked bundle spends no hourly budget) — 4 of the 6 fail
with the limiter call disabled, i.e. against the dead-code state. Plus
ManagementActor tests pinning the ClientIp forwarding and the curated
throttle error.
2026-08-15 06:01:14 -04:00
Joseph Doherty 48b3c40a7f perf(deploy): flatten-session caching, bulk DeploySiteAsync, paged management queries 2026-08-14 21:14:22 -04:00
Joseph Doherty acbb9eafa5 fix(management): EWS write gate requires username:password credentials 2026-08-10 06:48:15 -04:00
Joseph Doherty 4e633b1e64 feat(management): Transport on UpdateSmtpConfigCommand with EWS shape validation 2026-08-10 06:33:29 -04:00
Joseph Doherty 88638d774a feat(cli,management): close area-move and template-folder CLI parity gaps
Two verified-absent parity gaps between the service layer and the CLI /
ManagementActor command surface, both left as follow-ups by the 2026-05-11
design plans.

(1) area move. AreaService.MoveAreaAsync had existed since the deployment
topology page shipped but was reachable only from the Blazor UI. Adds
MoveAreaCommand(AreaId, NewParentAreaId?) to Commons, a ManagementActor
dispatch arm delegating straight to AreaService.MoveAreaAsync (not-found /
self-parent / descendant-cycle / cross-site / name-collision all surface as
the standard curated ManagementCommandException failure response; the service
writes its own "Move" audit row), and the CLI verb `site area move --id
[--parent-id]`. Omitting --parent-id moves the area to the site root, matching
the command's nullable NewParentAreaId. The command carries the SAME any-of
[Designer, Deployer] gate as CreateArea/UpdateArea/DeleteArea (arch-review C6):
re-parenting is the same structural authoring act, exposed on the same two
surfaces. Placed under the existing `site area` group rather than a new
top-level `area` group, alongside its create/update/delete siblings.

(2) template folder verbs. The five folder management commands have been
handled by ManagementActor since the folder-hierarchy plan, but the promised
CLI surface was never written. Adds `template folder
list|create|rename|move|reorder|delete` mapping 1:1 onto ListTemplateFolders /
CreateTemplateFolder / RenameTemplateFolder / MoveTemplateFolder /
ReorderTemplateFolder / DeleteTemplateFolder. --parent-id is omitted to target
the tree root; --direction takes the lowercase literals up/down, validated at
parse time by AcceptOnlyFromAmong (same case-sensitive contract as the audit
--channel/--kind/--status options).

Follow-on updates: the frozen authorization matrix gains its MoveArea entry
(reflection-driven, so a missing entry would have failed CI); CommandTreeTests
pins both new verb sets plus the omit-parent-id-means-root parse behaviour and
registry round-trips; ManagementActorTests covers the MoveArea role gate and
the delegate-to-service success/root/cycle/not-found paths; the CLI README and
Component-ManagementService.md document the new surface (the latter also gained
the previously-undocumented ReorderTemplateFolder); both plan docs' follow-up
lines are marked done.
2026-08-01 11:15:24 -04:00
Joseph Doherty 63c16d6912 refactor(comm): retire ClusterClient naming after the gRPC cutover
Phase 4 of the ClusterClient -> gRPC migration deleted the Akka transports
but left the naming behind: `ClusterClientSiteAuditClient` was transport-
agnostic and worked unchanged, so it survived the deletion under a name
that now describes a transport the repo no longer has. Same for a scatter
of doc-comments still framing gRPC as "the new transport" beside an Akka
one that is gone.

Renames it to `SiteCommunicationAuditClient` (and its test file) and
rewrites the stale comments to describe the single transport that exists.
Also tightens CLAUDE.md: drops the self-describing directory listing and
the 27-component enumeration in favour of the non-obvious parts only.

Behaviour-neutral: names and prose only. Recorded as the Phase 4
follow-up in docs/plans/2026-07-22-clusterclient-to-grpc-plan.md.
2026-07-27 15:40:01 -04:00
Joseph Doherty 2ee84af1c0 feat(grpc): PSK-authenticate the site gRPC control plane; drop the vestigial management receptionist registration
Phase 0 of the ClusterClient→gRPC migration
(docs/plans/2026-07-22-clusterclient-to-grpc-plan.md). Standalone hardening: it
closes a gap that exists today and is a precondition for moving command/control
onto gRPC in later phases.

T0.1 — delete the ManagementActor ClusterClientReceptionist registration.
It was built for an out-of-cluster CLI that was never written: the shipped CLI
speaks HTTP Basic to /management, which asks the actor in-process through
ManagementActorHolder. Nothing in the repo ever sent to /user/management. The
actor still runs there; only the cross-boundary advertisement is gone. Six
documents claimed the CLI used ClusterClient — including the CLI's own README
"Architecture Notes" — and are corrected here rather than left to rot.

T0.2 — record, do not port, the dead integration-routing path.
IntegrationCallRequest is unwired at BOTH ends: RouteIntegrationCallAsync has
zero callers anywhere, and RegisterLocalHandler(Integration, …) appears only in
a test, so production always answers "Integration handler not available". It is
excluded from the gRPC contract (28 of 29 commands migrate) rather than
enshrined on an additive-only wire format, and deleting it during a
transport migration would mix a behavioural change into a change whose whole
value is that behaviour is identical. See
docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md.

T0.3 — preshared-key authentication on SiteStreamService.
The service shipped with no auth at all: plaintext h2c, no interceptor, so
anything that could reach a site node's :8083 could open a live data stream or
read audit rows back via PullAuditEvents/PullSiteCalls. ControlPlaneAuthInterceptor
now gates /sitestream.SiteStreamService/ — modeled on LocalDbSyncAuthInterceptor
(constant-time compare, fail-closed, PermissionDenied) but gating a SET of
service prefixes so phases 1A/1B add services rather than interceptors. LocalDb
sync keeps its own separate key: it authenticates the pair partner, not central,
and collapsing the two would make a site's central-facing key also admit writes
into its database.

Keys are per site (SB-GRPC-PSK-<siteId>), never fleet-wide, so a compromised
site yields only its own. Central attaches them through ControlPlaneCredentials,
which binds CallCredentials to the channel — covering unary and streaming
uniformly, and letting the key resolve asynchronously, which a client
interceptor could not do without blocking. All three central→site channel
creation sites go through it (SiteStreamGrpcClient and both audit pull invokers);
the pull invokers' channel caches are re-keyed by (site, endpoint) because
credentials are per-site and bound to the channel.

Two decisions beyond the plan:

  * StartupValidator now requires GrpcPsk on Site nodes. The plan specified only
    the runtime gate, but fail-closed with no boot check produces a node that
    joins, answers heartbeats and reports healthy while refusing every stream,
    audit pull and telemetry ingest — silent and total. Same reasoning as the
    existing inbound API-key pepper rule.

  * Added Communication:SitePsks as a central-side key map. The plan assumed
    central would read the store, seeded via a dev KEK; the docker rig
    deliberately boots with no master key, so store-only resolution would leave
    it unable to dial its own sites. The store stays primary — it is the only
    source that can serve a site added at runtime — with the map covering
    key-less hosts and one-off pins. Neither source falling back to
    "unauthenticated" is the invariant.

T0.4 — dev keys on both rigs and tests.
34 tests. The seven that matter most exercise a real in-process gRPC stack over
TestServer: the unit tests on either side of the wire would both stay green if
the halves disagreed, and gRPC refuses call credentials on a plaintext channel
by default — the UnsafeUseInsecureChannelCallCredentials opt-in is only provable
by making a real call. They confirm correct key passes on unary AND streaming,
wrong key and no-credentials both get PermissionDenied, and an unresolvable key
fails the call with nothing reaching the service.

OPERATIONAL: a site node upgraded to this build without a key will not boot.
That includes the gitignored deploy/wonder-app-vd03/ overlay.
2026-07-22 17:51:09 -04:00
Joseph Doherty 2751a6dba9 test(management): freeze scrubber fragment coverage over DataConnection config types + document array-merge limit (plan R2-07 T12) 2026-07-13 11:05:19 -04:00
Joseph Doherty 1fcfa4fb16 fix(security): scope-filter secured-write listing + align spec with enforced site scoping (plan R2-07 T6) 2026-07-13 10:53:16 -04:00
Joseph Doherty 9b9222d223 fix(security): enforce LDAP-mapping site scope on secured-write submit/approve/reject (plan R2-07 T4) 2026-07-13 10:46:08 -04:00
Joseph Doherty e1a692016e fix(security): throttle DebugStreamHub LDAP bind via ManagementAuthenticator + correct coverage doc (plan R2-07 T1) 2026-07-13 10:36:32 -04:00
Joseph Doherty 4e228110ec fix(comm): site delete disposes the site's cached gRPC channels via RemoveSiteAsync (plan R2-02 T13) 2026-07-13 10:22:56 -04:00
Joseph Doherty f480a56737 feat(instance): native-alarm-source-override CSV bulk import (deferred #12) + doc fixes
Closes the operator parity gap the deferred-work register tracked as #12: native
alarm source overrides could only be set one-at-a-time, while attribute overrides
had a CSV bulk path. Adds an all-or-nothing CSV import for native sources.

- Commons: extract the RFC-4180 line splitter into a shared CsvLineSplitter
  (refactor OverrideCsvParser onto it — no behavior change, pinned by its tests);
  new NativeAlarmSourceOverrideCsvParser (header SourceName,Connection,
  SourceReference,Filter; blank = inherited).
- Commons: NativeAlarmSourceOverrideEntry + bulk SetInstanceNativeAlarmSource-
  OverridesCommand (auto-registered via the reflection command registry).
- ManagementService: Deployer-gated handler — flattens once, validates every
  source resolves + is unlocked + no duplicates up front, then upserts the whole
  batch under a single SaveChanges (true all-or-nothing txn). Added to the frozen
  authorization matrix; dispatch-coverage guard passes.
- CLI: `instance native-alarm-source import --instance-id --file` (parity with
  `instance import-overrides`) + README.
- Tests: native parser (Commons), CLI parse/entry mapping, 3 bulk-handler tests
  (happy path single-commit, locked-source reject, unresolved-source reject).

Also corrects a stale XML-doc line in ScriptRuntimeContext (WaitForAttribute
quality-gated mode is shipped, not "planned") and updates the deferred-work
register: marks #7/#13/#15/#16/#20 verified-resolved, #12 as CLI/API-shipped with
only the Central UI upload affordance still pending.

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 08:52:12 -04:00
Joseph Doherty fc918d4679 feat(options): eager startup validation for edge/management options (InboundAPI, ESG, Notification, ManagementService) (arch-review 08 §1.5)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 06:53:30 -04:00
Joseph Doherty d6bad1738e fix(management): single-flight transport bundle commands + drop redundant export copy (arch-review S4)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 06:04:22 -04:00
Joseph Doherty 655834f0b8 feat(management): additive Skip/Take paging on template/instance lists (arch-review P2)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:57:14 -04:00
Joseph Doherty 8221ee797e fix(management): canonicalize role-mapping casing at write path, closing UI/actor case split (arch-review C5)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:52:45 -04:00
Joseph Doherty 83e09fc210 fix(management): honor MgmtDeployArtifactsCommand.SiteId + enforce site scope (arch-review C2)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:51:03 -04:00
Joseph Doherty 5b00c049f8 test(management): dispatch-coverage test — every registered command has a handler (arch-review UA2)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:43:27 -04:00
Joseph Doherty 61df8a28fe test(management): frozen GetRequiredRoles matrix over every registered command (arch-review UA2)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:43:27 -04:00
Joseph Doherty 8d4ffa7ef1 feat(management): secured-write list paging with TotalCount (arch-review P3)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:42:35 -04:00
Joseph Doherty 03295e91bb feat(management): opportunistic expiry sweep on secured-write list (arch-review S2)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:39:29 -04:00
Joseph Doherty 064a7d9415 feat(management): server-side secured-write TTL — stale pending writes expire, never execute (arch-review S2)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:38:19 -04:00
Joseph Doherty 5415e6566f feat(management): Admin-gated actor dispatch for cert-trust commands (arch-review C4)
Made CommunicationService.TrustServerCertAsync/RemoveServerCertAsync/ListServerCertsAsync virtual to support the test seam (mirrors existing virtual WriteTagAsync).

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:29:57 -04:00
Joseph Doherty 722063638b feat(management): actor dispatch for BrowseNode/SearchAddressSpace/VerifyEndpoint — CLI parity (arch-review C4)
Made CommunicationService.BrowseNodeAsync/SearchAddressSpaceAsync/VerifyEndpointAsync virtual to support the test seam (mirrors existing virtual WriteTagAsync).

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:27:48 -04:00
Joseph Doherty 51cff07753 fix(management): reconcile area role gate to Designer|Deployer across actor and all three docs (arch-review C6)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:23:57 -04:00
Joseph Doherty 7af36fd5dc feat(security): wire LoginThrottle into management Basic-Auth and /auth/login|token — password-spray guard (arch-review P1/UA5)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:16:40 -04:00
Joseph Doherty d4d1732a9e fix(management): elide DatabaseConnection ConnectionString from List/Get (arch-review C3)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:15:55 -04:00
Joseph Doherty ecf8ac1b7d fix(management): elide ExternalSystem AuthConfiguration from responses and audit (arch-review C3)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:14:04 -04:00
Joseph Doherty 3a1980cb4c fix(management): elide DataConnection config secrets from responses and audit (arch-review C3)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:12:29 -04:00
Joseph Doherty 7206761cb4 fix(management): 5-min Ask timeout for transport/deploy commands (arch-review S1)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:04:18 -04:00
Joseph Doherty 5cfa85ea91 feat(management): ConfigSecretScrubber for connection-config secret elision
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:04:18 -04:00
Joseph Doherty 6c9de9f7ab feat(management): any-of role gates; restrict ListSecuredWrites to Operator/Verifier/Admin (arch-review UA1)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 05:03:51 -04:00
Joseph Doherty 40088a2123 fix(notifications): validate Twilio AccountSid format at save — closes the un-escaped URI interpolation door
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 04:36:47 -04:00
Joseph Doherty 0ee9975111 feat(notifications): OAuth2 authority/scope management + CLI + UI surfaces
Additive UpdateSmtpConfigCommand params (OAuth2Authority/OAuth2Scope),
preserve-on-null handler application, CLI --oauth2-authority/--oauth2-scope
on smtp update, Central UI text inputs shown only for the OAuth2 auth type,
and the Component-NotificationService doc paragraph with M365 defaults.

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 04:30:28 -04:00
Joseph Doherty eabd2820eb feat(esg): --timeout-seconds on external-system create/update (management command + CLI, additive contract)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 04:20:28 -04:00
Joseph Doherty 899af63b2a docs(inbound-api): DB row is authoritative for method scripts — update broken-save warning + Direct SQL Warning for the revision-checked cache
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
2026-07-10 04:07:44 -04:00