diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainCatalog.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainCatalog.cs index 0c5717b40..5cef4a304 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainCatalog.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainCatalog.cs @@ -67,6 +67,7 @@ public static class HeidenhainCatalog public const string Unsupported = "Unsupported"; public const string InvalidData = "InvalidData"; public const string CommunicationLost = "CommunicationLost"; + public const string ConfigurationError = "ConfigurationError"; } /// Every fixed leaf, value leaves first then the capability strings. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainOptionsParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainOptionsParser.cs index 22e80fa57..925b4735c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainOptionsParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainOptionsParser.cs @@ -4,8 +4,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Heidenhain; /// /// The one parser + validator of the Heidenhain DriverConfig document, shared by the driver factory, -/// the Test-Connect probe, the deploy gate and the AdminUI form, so none of them can disagree about what a -/// config means. +/// the Test-Connect probe, the deploy gate and the AdminUI form/save path, so none of them can disagree about +/// what a config means. /// public static class HeidenhainOptionsParser { @@ -15,6 +15,37 @@ public static class HeidenhainOptionsParser /// Prefix of an environment-variable password reference. public const string EnvPrefix = "env:"; + /// + /// The ONLY secret-store namespace a Heidenhain password may name: secret:heidenhain/<name> (the + /// store's slash-separated, driver-scoped naming, as in galaxy/<inst>/apikey). Scoping is the + /// exfiltration guard: the reference is resolved on the node and sent in cleartext in A_LG to a Host + /// the author chose, so it must never be able to name another subsystem's secret. + /// + public const string SecretNamespace = "heidenhain/"; + + /// + /// The ONLY environment variables a Heidenhain password may name: env:OTOPCUA_HEIDENHAIN_<NAME> + /// (exact upper-case prefix; NAME is [A-Z0-9_]+) — never ConnectionStrings__*, + /// ServerHistorian__ApiKey or any other process variable. + /// + public const string EnvVariablePrefix = "OTOPCUA_HEIDENHAIN_"; + + /// The password fields a Heidenhain DriverConfig/DeviceConfig may carry. + public static readonly IReadOnlyList PasswordFields = ["InspectPassword", "DncPassword", "DataPassword"]; + + /// The kind of a parsed credential reference. + public enum CredentialKind + { + /// Blank — no password. + None, + + /// secret:heidenhain/…. + Secret, + + /// env:OTOPCUA_HEIDENHAIN_…. + Environment, + } + private static readonly JsonSerializerOptions JsonOptions = new() { PropertyNameCaseInsensitive = true, @@ -22,7 +53,10 @@ public static class HeidenhainOptionsParser AllowTrailingCommas = true, }; - /// Deserialises a config document. Throws on unreadable JSON. + /// + /// Deserialises a config document (null sections normalised to defaults). Throws + /// on unreadable JSON. + /// /// The DriverConfig JSON (blank ⇒ defaults). /// The options. public static HeidenhainDriverOptions Parse(string? json) @@ -30,8 +64,8 @@ public static class HeidenhainOptionsParser if (string.IsNullOrWhiteSpace(json)) return new HeidenhainDriverOptions(); try { - return JsonSerializer.Deserialize(json, JsonOptions) - ?? throw new InvalidOperationException("Heidenhain driver config deserialised to null."); + return Normalize(JsonSerializer.Deserialize(json, JsonOptions) + ?? throw new InvalidOperationException("Heidenhain driver config deserialised to null.")); } catch (JsonException ex) { @@ -39,6 +73,30 @@ public static class HeidenhainOptionsParser } } + /// + /// Replaces JSON-null sections ("Access": null, "Polling": null, …) and null list entries + /// with their defaults, so nothing downstream can dereference a null section. + /// + /// The options as deserialised (or constructed). + /// An options instance with every section non-null. + public static HeidenhainDriverOptions Normalize(HeidenhainDriverOptions o) + { + ArgumentNullException.ThrowIfNull(o); + if (o.Access is not null && o.Polling is not null && o.Reconnect is not null && o.Devices is not null + && o.RawTags is not null && o.Devices.All(d => d is not null) && o.RawTags.All(t => t is not null)) + return o; + return new HeidenhainDriverOptions + { + Devices = (o.Devices ?? []).Where(d => d is not null).ToList(), + RawTags = (o.RawTags ?? []).Where(t => t is not null).ToList(), + TimeoutMs = o.TimeoutMs, + Access = o.Access ?? new HeidenhainAccessOptions(), + Polling = o.Polling ?? new HeidenhainPollingOptions(), + Reconnect = o.Reconnect ?? new HeidenhainReconnectOptions(), + ProbeTimeoutSeconds = o.ProbeTimeoutSeconds, + }; + } + /// /// Validates driver-level ranges and every password reference. Device endpoints are validated per device /// by (a driver may be authored before its devices). @@ -48,22 +106,23 @@ public static class HeidenhainOptionsParser public static IReadOnlyList Validate(HeidenhainDriverOptions o) { ArgumentNullException.ThrowIfNull(o); + o = Normalize(o); var errors = new List(); if (o.TimeoutMs is < 100 or > 60000) errors.Add("TimeoutMs must be 100–60000 ms."); - var p = o.Polling ?? new HeidenhainPollingOptions(); + var p = o.Polling; foreach (var (name, v) in new[] { ("StatusMs", p.StatusMs), ("CustomMs", p.CustomMs), ("SlowMs", p.SlowMs) }) if (v is < 100 or > 3_600_000) errors.Add($"Polling.{name} must be 100–3600000 ms."); if (p.MaxAlarms is < 1 or > 256) errors.Add("Polling.MaxAlarms must be 1–256."); - var r = o.Reconnect ?? new HeidenhainReconnectOptions(); + var r = o.Reconnect; if (r.InitialDelayMs is < 100 or > 600_000) errors.Add("Reconnect.InitialDelayMs must be 100–600000 ms."); if (r.MaxDelayMs < r.InitialDelayMs || r.MaxDelayMs > 600_000) errors.Add("Reconnect.MaxDelayMs must be between InitialDelayMs and 600000 ms."); if (o.ProbeTimeoutSeconds is < 1 or > 60) errors.Add("ProbeTimeoutSeconds must be 1–60."); - var a = o.Access ?? new HeidenhainAccessOptions(); + var a = o.Access; CheckRef(errors, "Access.InspectPassword", a.InspectPassword); CheckRef(errors, "Access.DncPassword", a.DncPassword); CheckRef(errors, "Access.DataPassword", a.DataPassword); - foreach (var d in o.Devices ?? []) + foreach (var d in o.Devices) { var label = string.IsNullOrWhiteSpace(d.DeviceName) ? d.Host : d.DeviceName; CheckRef(errors, $"Device '{label}' InspectPassword", d.InspectPassword); @@ -88,24 +147,125 @@ public static class HeidenhainOptionsParser } /// - /// True when is an acceptable password field: blank, secret:<name> or - /// env:<VARIABLE>. A literal password is refused so it never lands in the config database. + /// True when is an acceptable password field: blank, + /// secret:heidenhain/<name> or env:OTOPCUA_HEIDENHAIN_<NAME>. /// /// The field value. - /// Whether it is a reference (or blank). - public static bool IsCredentialReference(string? value) + /// Whether it is a namespaced reference (or blank). + public static bool IsCredentialReference(string? value) => TryParseCredentialReference(value, out _, out _); + + /// + /// Parses a credential reference, enforcing the Heidenhain namespace — the single rule the parser, the + /// resolver (again at resolve time), the probe, the AdminUI save path and the deploy gate all apply. + /// + /// The field value. + /// The reference kind. + /// The full secret name (incl. heidenhain/) or the environment variable name. + /// True for blank or a namespaced reference. + public static bool TryParseCredentialReference(string? value, out CredentialKind kind, out string name) { + kind = CredentialKind.None; + name = ""; if (string.IsNullOrEmpty(value)) return true; - if (value.StartsWith(SecretPrefix, StringComparison.OrdinalIgnoreCase)) - return value.Length > SecretPrefix.Length && !string.IsNullOrWhiteSpace(value[SecretPrefix.Length..]); - if (value.StartsWith(EnvPrefix, StringComparison.OrdinalIgnoreCase)) - return value.Length > EnvPrefix.Length && !string.IsNullOrWhiteSpace(value[EnvPrefix.Length..]); + if (value.StartsWith(SecretPrefix, StringComparison.Ordinal)) + { + var secret = value[SecretPrefix.Length..]; + if (!secret.StartsWith(SecretNamespace, StringComparison.Ordinal)) return false; + var rest = secret[SecretNamespace.Length..]; + if (rest.Length is 0 or > 200) return false; + foreach (var c in rest) + if (!(char.IsAsciiLetterOrDigit(c) || c is '-' or '_' or '.' or '/')) return false; + if (rest.Split('/').Any(seg => seg.Length == 0 || seg is "." or "..")) return false; + kind = CredentialKind.Secret; + name = secret; + return true; + } + + if (value.StartsWith(EnvPrefix, StringComparison.Ordinal)) + { + var variable = value[EnvPrefix.Length..]; + if (!variable.StartsWith(EnvVariablePrefix, StringComparison.Ordinal)) return false; + var rest = variable[EnvVariablePrefix.Length..]; + if (rest.Length is 0 or > 100 || !rest.All(c => char.IsAsciiLetterUpper(c) || char.IsAsciiDigit(c) || c == '_')) + return false; + kind = CredentialKind.Environment; + name = variable; + return true; + } + return false; } + /// + /// Scans a RAW DriverConfig or DeviceConfig document (before it is stored) for password fields that are not + /// namespaced references — at the top level (a device / merged config), under Access, and under every + /// Devices[] entry; keys matched case-insensitively, as the parser binds them. The messages name the + /// field, never the value. Unreadable JSON is reported, since it cannot be checked. + /// + /// The config JSON. + /// The problems; empty when every password field is acceptable. + public static IReadOnlyList DescribeCredentialProblems(string? json) + { + var problems = new List(); + if (string.IsNullOrWhiteSpace(json)) return problems; + JsonDocument doc; + try + { + doc = JsonDocument.Parse(json, new JsonDocumentOptions { CommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = true }); + } + catch (JsonException) + { + problems.Add("Heidenhain config is not valid JSON, so its credential fields cannot be checked."); + return problems; + } + + using (doc) + { + if (doc.RootElement.ValueKind != JsonValueKind.Object) return problems; + ScanObject(doc.RootElement, "", problems); + foreach (var p in doc.RootElement.EnumerateObject()) + { + if (string.Equals(p.Name, "Access", StringComparison.OrdinalIgnoreCase) && p.Value.ValueKind == JsonValueKind.Object) + ScanObject(p.Value, "Access.", problems); + if (string.Equals(p.Name, "Devices", StringComparison.OrdinalIgnoreCase) && p.Value.ValueKind == JsonValueKind.Array) + { + var i = 0; + foreach (var d in p.Value.EnumerateArray()) + { + if (d.ValueKind == JsonValueKind.Object) ScanObject(d, $"Devices[{i}].", problems); + i++; + } + } + } + } + + return problems; + } + + /// The one wording of the credential rule (never includes the value). + /// The field. + /// The message. + public static string CredentialRuleMessage(string field) => + $"{field} must be blank, 'secret:{SecretNamespace}' or 'env:{EnvVariablePrefix}' — a literal " + + "password, or a reference outside the Heidenhain namespace, is refused."; + + private static void ScanObject(JsonElement obj, string prefix, List problems) + { + foreach (var p in obj.EnumerateObject()) + { + if (!PasswordFields.Any(f => string.Equals(f, p.Name, StringComparison.OrdinalIgnoreCase))) continue; + var ok = p.Value.ValueKind switch + { + JsonValueKind.Null => true, + JsonValueKind.String => IsCredentialReference(p.Value.GetString()), + _ => false, + }; + if (!ok) problems.Add(CredentialRuleMessage(prefix + p.Name)); + } + } + private static void CheckRef(List errors, string field, string? value) { - if (!IsCredentialReference(value)) - errors.Add($"{field} must be blank, 'secret:' or 'env:' — a literal password is not stored."); + if (!IsCredentialReference(value)) errors.Add(CredentialRuleMessage(field)); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainCredentials.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainCredentials.cs index 1685b3478..f441dfcb1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainCredentials.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainCredentials.cs @@ -2,10 +2,16 @@ using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.Heidenhain; +/// A password reference that cannot be resolved (or may not be resolved). Never carries the value. +public sealed class HeidenhainCredentialException(string message) : InvalidOperationException(message); + /// /// Resolves password references on demand (each login), so plaintext is never held in options or session -/// state. Fail-closed: a secret: reference with no resolver, an absent secret, an unset environment -/// variable or a literal value throws — the reference text is never sent to the control as a password. +/// state. Fail-closed AT RESOLVE TIME, independently of any earlier validation: only blank, +/// secret:heidenhain/… and env:OTOPCUA_HEIDENHAIN_… are resolved — anything else (a literal, another +/// subsystem's secret, ConnectionStrings__*), a secret: with no store, an absent secret or an unset +/// variable throws . The reference is resolved on this node and sent +/// in cleartext to the configured control, which is why the namespace is enforced here as well. /// internal sealed class HeidenhainCredentials(ISecretResolver? resolver, Func? environment = null) { @@ -14,12 +20,13 @@ internal sealed class HeidenhainCredentials(ISecretResolver? resolver, FuncThe password for a role on a device (device override, then driver-level). public async ValueTask ResolveAsync(HeidenhainDriverOptions options, HeidenhainDeviceOptions device, string role, CancellationToken ct) { + var access = options.Access ?? new HeidenhainAccessOptions(); var reference = role switch { - "INSPECT" => device.InspectPassword ?? options.Access.InspectPassword, - "DNC" => device.DncPassword ?? options.Access.DncPassword, - "DATA" => device.DataPassword ?? options.Access.DataPassword, - _ => throw new InvalidOperationException("Unexpected LSV2 role."), + "INSPECT" => device.InspectPassword ?? access.InspectPassword, + "DNC" => device.DncPassword ?? access.DncPassword, + "DATA" => device.DataPassword ?? access.DataPassword, + _ => throw new HeidenhainCredentialException("Unexpected LSV2 role."), }; return await ResolveReferenceAsync(reference, role, ct).ConfigureAwait(false); } @@ -27,23 +34,23 @@ internal sealed class HeidenhainCredentials(ISecretResolver? resolver, FuncResolves one reference. public async ValueTask ResolveReferenceAsync(string? reference, string role, CancellationToken ct) { - if (string.IsNullOrEmpty(reference)) return ""; - if (!HeidenhainOptionsParser.IsCredentialReference(reference)) - throw new InvalidOperationException($"{role} password must be a secret: or env: reference (fail-closed)."); - if (reference.StartsWith(HeidenhainOptionsParser.EnvPrefix, StringComparison.OrdinalIgnoreCase)) + if (!HeidenhainOptionsParser.TryParseCredentialReference(reference, out var kind, out var name)) + throw new HeidenhainCredentialException(HeidenhainOptionsParser.CredentialRuleMessage($"{role} password") + " (fail-closed)"); + switch (kind) { - var name = reference[HeidenhainOptionsParser.EnvPrefix.Length..].Trim(); - return _environment(name) is { Length: > 0 } value - ? value - : throw new InvalidOperationException($"{role} password environment variable '{name}' is not set (fail-closed)."); + case HeidenhainOptionsParser.CredentialKind.None: + return ""; + case HeidenhainOptionsParser.CredentialKind.Environment: + return _environment(name) is { Length: > 0 } value + ? value + : throw new HeidenhainCredentialException($"{role} password environment variable '{name}' is not set (fail-closed)."); + default: + if (resolver is null) + throw new HeidenhainCredentialException($"{role} password references secret '{name}' but no secret store is available (fail-closed)."); + var resolved = await resolver.GetAsync(new SecretName(name), ct).ConfigureAwait(false); + return !string.IsNullOrEmpty(resolved) + ? resolved + : throw new HeidenhainCredentialException($"{role} password secret '{name}' is absent from the store (fail-closed)."); } - - var secret = reference[HeidenhainOptionsParser.SecretPrefix.Length..].Trim(); - if (resolver is null) - throw new InvalidOperationException($"{role} password references secret '{secret}' but no secret store is available (fail-closed)."); - var resolved = await resolver.GetAsync(new SecretName(secret), ct).ConfigureAwait(false); - return !string.IsNullOrEmpty(resolved) - ? resolved - : throw new InvalidOperationException($"{role} password secret '{secret}' is absent from the store (fail-closed)."); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDeviceRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDeviceRuntime.cs index fe1003583..cdfe9dbed 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDeviceRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDeviceRuntime.cs @@ -66,6 +66,11 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable private DateTime? _lastSuccessUtc; private string? _lastError; private int _consecutiveFailures; + private bool _disposed; + private volatile bool _pollProgress; + + /// Consecutive sessions that ended without one published poll batch (test/diagnostic). + internal int ConsecutiveFailures => Volatile.Read(ref _consecutiveFailures); public HeidenhainDeviceRuntime( HeidenhainDeviceOptions device, @@ -79,7 +84,7 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable ILogger logger) { _device = device; - _options = options; + _options = HeidenhainOptionsParser.Normalize(options); _tags = tags; _passwords = passwords; _publish = publish; @@ -134,7 +139,9 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable { lock (_lifecycle) { - if (_loop is not null) return; + // A Start racing (or following) DisposeAsync must never revive an orphan loop: the driver may be + // iterating a device list it captured just before re-init disposed it. + if (_disposed || _loop is not null) return; _cts = new CancellationTokenSource(); var token = _cts.Token; _loop = Task.Run(() => RunAsync(token), CancellationToken.None); @@ -143,9 +150,59 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable private async Task RunAsync(CancellationToken ct) { + // Nothing may kill this loop silently: every iteration — connect, poll, retire, backoff — is guarded, + // and an unexpected fault becomes a logged retire + backoff, never an unobserved faulted task. while (!ct.IsCancellationRequested) + { + try + { + await RunOnceAsync(ct).ConfigureAwait(false); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + break; + } + catch (Exception ex) + { + _logger.LogError(ex, "Heidenhain {Host} acquisition loop fault; retrying", HostName); + try { RetireAll("Acquisition loop fault: " + ex.GetType().Name); } + catch (Exception inner) { _logger.LogError(inner, "Heidenhain {Host} retire failed", HostName); } + } + + if (ct.IsCancellationRequested) break; + // Backoff resets only when the last session made real progress (published a poll batch): a control + // that accepts login + version and then drops must not be re-dialled every InitialDelayMs. + if (_pollProgress) _consecutiveFailures = 0; + _consecutiveFailures++; + try + { + SetState(HeidenhainConnectionState.Backoff, null); + await Task.Delay(BackoffDelay(_consecutiveFailures), _time, ct).ConfigureAwait(false); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } + catch (Exception ex) + { + _logger.LogError(ex, "Heidenhain {Host} backoff fault", HostName); + try { await Task.Delay(TimeSpan.FromSeconds(1), ct).ConfigureAwait(false); } + catch (OperationCanceledException) { break; } + } + } + } + + /// The reconnect delay for the n-th consecutive failure, clamped to sane bounds (never a hot loop). + internal TimeSpan BackoffDelay(int failures) + { + var r = _options.Reconnect ?? new HeidenhainReconnectOptions(); + var initial = TimeSpan.FromMilliseconds(Math.Clamp(r.InitialDelayMs, 100, 600_000)); + var max = TimeSpan.FromMilliseconds(Math.Clamp(r.MaxDelayMs, (int)initial.TotalMilliseconds, 600_000)); + return ConnectionBackoff.ComputeDelay(initial, failures, max); + } + + private async Task RunOnceAsync(CancellationToken ct) + { { HeidenhainSession? session = null; + _pollProgress = false; try { SetState(HeidenhainConnectionState.Connecting, null); @@ -161,7 +218,6 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable _groupGood.Clear(); } - _consecutiveFailures = 0; SetState(HeidenhainConnectionState.Connected, null); _logger.LogInformation("Heidenhain {Host} connected ({Model})", HostName, session.Identity?.Model); await PollAsync(session, generation, ct).ConfigureAwait(false); @@ -170,7 +226,7 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable } catch (OperationCanceledException) when (ct.IsCancellationRequested) { - break; + throw; } catch (Exception ex) { @@ -188,15 +244,6 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable { session?.Close(); } - - if (ct.IsCancellationRequested) break; - _consecutiveFailures++; - var delay = ConnectionBackoff.ComputeDelay( - TimeSpan.FromMilliseconds(_options.Reconnect.InitialDelayMs), _consecutiveFailures, - TimeSpan.FromMilliseconds(_options.Reconnect.MaxDelayMs)); - SetState(HeidenhainConnectionState.Backoff, null); - try { await Task.Delay(delay, _time, ct).ConfigureAwait(false); } - catch (OperationCanceledException) { break; } } } @@ -204,9 +251,9 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable { var intervals = new Dictionary { - [HeidenhainPollGroup.Status] = TimeSpan.FromMilliseconds(_options.Polling.StatusMs), - [HeidenhainPollGroup.Custom] = TimeSpan.FromMilliseconds(_options.Polling.CustomMs), - [HeidenhainPollGroup.Slow] = TimeSpan.FromMilliseconds(_options.Polling.SlowMs), + [HeidenhainPollGroup.Status] = TimeSpan.FromMilliseconds(Math.Max(100, _options.Polling.StatusMs)), + [HeidenhainPollGroup.Custom] = TimeSpan.FromMilliseconds(Math.Max(100, _options.Polling.CustomMs)), + [HeidenhainPollGroup.Slow] = TimeSpan.FromMilliseconds(Math.Max(100, _options.Polling.SlowMs)), }; var start = _time.GetTimestamp(); var due = _groups.Keys.ToDictionary(g => g, _ => TimeSpan.Zero); @@ -257,6 +304,7 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable _groupGood[group] = good; _lastSuccessUtc = now; + _pollProgress = true; return true; } } @@ -325,6 +373,7 @@ internal sealed class HeidenhainDeviceRuntime : IAsyncDisposable CancellationTokenSource? cts; lock (_lifecycle) { + _disposed = true; loop = _loop; cts = _cts; _loop = null; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDriver.cs index bf95afcc5..65e9ce831 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDriver.cs @@ -42,6 +42,7 @@ public sealed class HeidenhainDriver : IDriver, IReadable, ISubscribable, ITagDi private readonly TimeProvider _time; private readonly SkippedTagLedger _skipped = new(); private readonly object _lifecycleLock = new(); + private readonly object _publishLock = new(); private readonly ConcurrentDictionary _cache = new(StringComparer.Ordinal); private readonly ConcurrentDictionary _subscriptions = new(); private HeidenhainDriverOptions _options; @@ -68,7 +69,7 @@ public sealed class HeidenhainDriver : IDriver, IReadable, ISubscribable, ITagDi { ArgumentNullException.ThrowIfNull(options); ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); - _options = options; + _options = HeidenhainOptionsParser.Normalize(options); _driverInstanceId = driverInstanceId; _logger = logger ?? NullLogger.Instance; _credentials = credentials; @@ -278,13 +279,18 @@ public sealed class HeidenhainDriver : IDriver, IReadable, ISubscribable, ITagDi { ArgumentNullException.ThrowIfNull(fullReferences); var sub = new Subscription(Interlocked.Increment(ref _nextSubscriptionId), fullReferences.ToHashSet(StringComparer.Ordinal)); - _subscriptions[sub.Id] = sub; - // Initial push of the current cache so the subscriber starts from the driver's real state. - var now = _time.GetUtcNow().UtcDateTime; - foreach (var r in sub.References) + // Register + initial push under the publication lock, so no Publish can interleave: either it ran before + // (the push carries its value) or it runs after (it reaches this subscription). The subscriber can never + // receive an older snapshot after a newer one. + lock (_publishLock) { - var snapshot = _cache.TryGetValue(r, out var s) ? s : new DataValueSnapshot(null, HeidenhainStatus.BadNodeIdUnknown, null, now); - Raise(sub, r, snapshot); + _subscriptions[sub.Id] = sub; + var now = _time.GetUtcNow().UtcDateTime; + foreach (var r in sub.References) + { + var snapshot = _cache.TryGetValue(r, out var s) ? s : new DataValueSnapshot(null, HeidenhainStatus.BadNodeIdUnknown, null, now); + Raise(sub, r, snapshot); + } } EnsureStarted(); @@ -298,17 +304,19 @@ public sealed class HeidenhainDriver : IDriver, IReadable, ISubscribable, ITagDi return Task.CompletedTask; } - private void Publish(string rawPath, DataValueSnapshot snapshot) + internal void Publish(string rawPath, DataValueSnapshot snapshot) { - var changed = true; - _cache.AddOrUpdate(rawPath, snapshot, (_, old) => + // Same lock as SubscribeAsync's register + initial push (see there). Lock order is always + // runtime publication → _publishLock; nothing takes them the other way round. + lock (_publishLock) { - changed = old.StatusCode != snapshot.StatusCode || !ValuesEqual(old.Value, snapshot.Value); - return snapshot; - }); - if (!changed) return; - foreach (var sub in _subscriptions.Values) - if (sub.References.Contains(rawPath)) Raise(sub, rawPath, snapshot); + var changed = !_cache.TryGetValue(rawPath, out var old) + || old.StatusCode != snapshot.StatusCode || !ValuesEqual(old.Value, snapshot.Value); + _cache[rawPath] = snapshot; + if (!changed) return; + foreach (var sub in _subscriptions.Values) + if (sub.References.Contains(rawPath)) Raise(sub, rawPath, snapshot); + } } private void Raise(Subscription sub, string rawPath, DataValueSnapshot snapshot) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDriverProbe.cs index 55f0bdbde..44f9c3785 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainDriverProbe.cs @@ -40,8 +40,16 @@ public sealed class HeidenhainDriverProbe(ISecretResolver? secretResolver = null try { string password; + // Resolved only through the namespaced resolver (fail-closed at resolve time): Test Connect can never be + // used to send another subsystem's secret or environment variable to an arbitrary Host. try { password = await new HeidenhainCredentials(secretResolver).ResolveAsync(options, device, "INSPECT", cts.Token).ConfigureAwait(false); } - catch (InvalidOperationException ex) { return new(false, ex.Message, null); } + catch (HeidenhainCredentialException ex) { return new(false, ex.Message, null); } + + try { _ = Lsv2ReadOnlyPolicy.LoginPayload("INSPECT", password); } + catch (Exception ex) when (ex is Lsv2PolicyException or ArgumentException) + { + return new(false, "The INSPECT password cannot be sent in an LSV2 login (NUL, non-Latin-1 or too long).", null); + } var perCall = TimeSpan.FromMilliseconds(Math.Min(options.TimeoutMs, budget.TotalMilliseconds)); await using var client = new Lsv2Client(device.Host, device.Port, perCall); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainSession.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainSession.cs index de24db277..46b255b0d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainSession.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainSession.cs @@ -100,13 +100,42 @@ internal sealed class HeidenhainSession : IAsyncDisposable { try { - await _client.LoginAsync(role, await _passwords(role, ct).ConfigureAwait(false), ct).ConfigureAwait(false); + await LoginWithCredentialAsync(role, ct).ConfigureAwait(false); return HeidenhainStatus.Good; } catch (Lsv2RefusalException e) { return HeidenhainStatus.FromRefusal(e.Code); } + catch (QualityException e) + { + // An unusable optional-role credential degrades only the reads that need that role; INSPECT stays up. + return e.Status; + } + } + + /// + /// Resolves the role's password and logs in. A credential problem — an unresolvable reference, or a + /// password the policy-checked payload cannot carry (NUL, non-Latin-1, > 247 bytes) — becomes + /// (never sent, never a session failure). Refusals and + /// transport failures propagate unchanged. + /// + private async Task LoginWithCredentialAsync(string role, CancellationToken ct) + { + string password; + try { password = await _passwords(role, ct).ConfigureAwait(false); } + catch (Exception ex) when (ex is not OperationCanceledException) + { + throw new QualityException(HeidenhainStatus.BadConfigurationError, HeidenhainCatalog.States.ConfigurationError); + } + + try { _ = Lsv2ReadOnlyPolicy.LoginPayload(role, password); } + catch (Exception ex) when (ex is Lsv2PolicyException or ArgumentException) + { + throw new QualityException(HeidenhainStatus.BadConfigurationError, HeidenhainCatalog.States.ConfigurationError); + } + + await _client.LoginAsync(role, password, ct).ConfigureAwait(false); } /// @@ -269,6 +298,12 @@ internal sealed class HeidenhainSession : IAsyncDisposable _client.Close(); return new Outcome(null, HeidenhainStatus.BadNoCommunication); } + catch (Exception ex) when (ex is Lsv2PolicyException or ArgumentException) + { + // A request the read-only policy refuses is never sent and never ends the session: this tag only. + if (!isCustom) _capabilities[feature] = HeidenhainCatalog.States.ConfigurationError; + return new Outcome(null, HeidenhainStatus.BadConfigurationError); + } } private async Task TimerAsync(string feature, CancellationToken ct) @@ -330,14 +365,7 @@ internal sealed class HeidenhainSession : IAsyncDisposable { await WireAsync(wire, "Login_" + role, async c => { - string password; - try { password = await _passwords(role, c).ConfigureAwait(false); } - catch (Exception ex) when (ex is not OperationCanceledException) - { - throw new QualityException(HeidenhainStatus.BadUnexpectedError, HeidenhainCatalog.States.Unknown); - } - - await _client.LoginAsync(role, password, c).ConfigureAwait(false); + await LoginWithCredentialAsync(role, c).ConfigureAwait(false); return []; }, ct).ConfigureAwait(false); if (dnc) _dncStatus = HeidenhainStatus.Good; @@ -349,6 +377,13 @@ internal sealed class HeidenhainSession : IAsyncDisposable else _dataStatus = HeidenhainStatus.FromRefusal(e.Code); throw; } + catch (QualityException e) + { + // Credential problem: stable for this session (a fixed secret/variable is picked up on reconnect). + if (dnc) _dncStatus = e.Status; + else _dataStatus = e.Status; + throw; + } } private async Task PrepareTelemetryAsync(Dictionary wire, CancellationToken ct) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainStatus.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainStatus.cs index cc2182790..3b4fb4497 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainStatus.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainStatus.cs @@ -29,6 +29,12 @@ public static class HeidenhainStatus /// Any other controller refusal (retried next poll). public const uint BadUnexpectedError = 0x80010000u; + /// + /// An optional-role (DNC/DATA) credential cannot be used — unresolvable reference, or a password the login + /// payload cannot carry (NUL, non-Latin-1, too long). Degrades only the reads that need that role. + /// + public const uint BadConfigurationError = 0x80890000u; + /// A RawPath this driver has no authored tag for. public const uint BadNodeIdUnknown = 0x80340000u; @@ -48,7 +54,7 @@ public static class HeidenhainStatus /// True for the stable refusals that may be suppressed for the rest of a session. /// The status. /// Whether it is stable. - public static bool IsStable(uint status) => status is BadNotSupported or BadUserAccessDenied; + public static bool IsStable(uint status) => status is BadNotSupported or BadUserAccessDenied or BadConfigurationError; /// The capability state for a failure status. /// The status. @@ -59,6 +65,7 @@ public static class HeidenhainStatus BadNotSupported => HeidenhainCatalog.States.Unsupported, BadDecodingError or BadTypeMismatch or BadOutOfRange => HeidenhainCatalog.States.InvalidData, BadNoCommunication => HeidenhainCatalog.States.CommunicationLost, + BadConfigurationError => HeidenhainCatalog.States.ConfigurationError, _ => HeidenhainCatalog.States.Unknown, }; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainConfigTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainConfigTests.cs index a5702b726..5fa229ed3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainConfigTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainConfigTests.cs @@ -117,13 +117,94 @@ public sealed class HeidenhainConfigTests } [Theory] - [InlineData("secret:lsv2-inspect")] - [InlineData("env:LSV2_DNC_PASSWORD")] + [InlineData("secret:heidenhain/inspect")] + [InlineData("secret:heidenhain/line3/cnc-7.inspect")] + [InlineData("env:OTOPCUA_HEIDENHAIN_DNC_PASSWORD")] [InlineData("")] [InlineData(null)] - public void Credential_references_are_accepted(string? value) => + public void Namespaced_credential_references_are_accepted(string? value) => HeidenhainOptionsParser.IsCredentialReference(value).ShouldBeTrue(); + /// + /// Review finding 2: an unscoped reference let a Designer exfiltrate any node secret by pointing Host at their + /// own machine and clicking Test Connect (the value is sent in cleartext in A_LG). + /// + [Theory] + [InlineData("env:ConnectionStrings__ConfigDb")] + [InlineData("env:ServerHistorian__ApiKey")] + [InlineData("env:HEIDENHAIN_PW")] + [InlineData("env:otopcua_heidenhain_pw")] + [InlineData("env:OTOPCUA_HEIDENHAIN_")] + [InlineData("env:OTOPCUA_HEIDENHAIN_A-B")] + [InlineData("secret:galaxy/inst1/apikey")] + [InlineData("secret:opcua/inst/password")] + [InlineData("secret:heidenhain/")] + [InlineData("secret:heidenhain/../galaxy/apikey")] + [InlineData("secret:heidenhain//x")] + [InlineData("secret:HEIDENHAIN/x")] + [InlineData("SECRET:heidenhain/x")] + [InlineData("secret:")] + [InlineData("hunter2")] + public void References_outside_the_heidenhain_namespace_are_refused(string value) + { + HeidenhainOptionsParser.IsCredentialReference(value).ShouldBeFalse(value); + HeidenhainOptionsParser.Validate(new HeidenhainDriverOptions { Access = new() { InspectPassword = value } }).ShouldNotBeEmpty(); + } + + [Theory] + [InlineData("env:ConnectionStrings__ConfigDb")] + [InlineData("secret:galaxy/inst1/apikey")] + public async Task The_resolver_fails_closed_for_out_of_namespace_references_without_consulting_store_or_environment(string reference) + { + var resolver = new FakeResolver(new() { ["galaxy/inst1/apikey"] = "GALAXY-KEY" }); + var envReads = 0; + var c = new HeidenhainCredentials(resolver, n => { envReads++; return "CONFIG-DB-CONNSTR"; }); + var ex = await Should.ThrowAsync(() => c.ResolveReferenceAsync(reference, "INSPECT", TestContext.Current.CancellationToken).AsTask()); + ex.Message.ShouldNotContain("GALAXY-KEY"); + ex.Message.ShouldNotContain("CONFIG-DB-CONNSTR"); + envReads.ShouldBe(0); + resolver.Reads.ShouldBe(0); + } + + [Fact] + public async Task The_probe_never_sends_an_out_of_namespace_credential() + { + var sent = 0; + await using var sim = Simulator.Lsv2Simulator.WithScript(async (conn, ct) => { while (await conn.ReadByteAsync(ct) >= 0) sent++; }); + var json = $$$"""{"Devices":[{"Host":"127.0.0.1","Port":{{{sim.Port}}},"DeviceName":"d"}],"Access":{"InspectPassword":"env:ConnectionStrings__ConfigDb"}}"""; + var result = await new HeidenhainDriverProbe(new FakeResolver(new())).ProbeAsync(json, TimeSpan.FromSeconds(2), TestContext.Current.CancellationToken); + result.Ok.ShouldBeFalse(); + await Task.Delay(100, TestContext.Current.CancellationToken); + sim.Connections.ShouldBe(0); + sent.ShouldBe(0); + } + + [Theory] + [InlineData("""{"Access":null,"Polling":null,"Reconnect":null,"Devices":null,"RawTags":null}""")] + [InlineData("""{"Devices":[null],"RawTags":[null]}""")] + public void Null_sections_normalise_to_defaults(string json) + { + var o = HeidenhainOptionsParser.Parse(json); + o.Access.ShouldNotBeNull(); + o.Polling.MaxAlarms.ShouldBe(64); + o.Reconnect.InitialDelayMs.ShouldBe(1000); + o.Devices.ShouldBeEmpty(); + o.RawTags.ShouldBeEmpty(); + HeidenhainOptionsParser.Validate(o).ShouldBeEmpty(); + Should.NotThrow(() => HeidenhainDriver.ParseOptions(json)); + } + + [Fact] + public void Raw_config_scan_finds_literals_at_every_level_case_insensitively() + { + HeidenhainOptionsParser.DescribeCredentialProblems("""{"Host":"h","dncpassword":"hunter2"}""").Count.ShouldBe(1); + HeidenhainOptionsParser.DescribeCredentialProblems("""{"access":{"InspectPassword":"hunter2"}}""").Count.ShouldBe(1); + HeidenhainOptionsParser.DescribeCredentialProblems("""{"Devices":[{"DataPassword":"x"},{"DataPassword":7}]}""").Count.ShouldBe(2); + HeidenhainOptionsParser.DescribeCredentialProblems("""{"Access":{"InspectPassword":"secret:heidenhain/a"}}""").ShouldBeEmpty(); + string.Join(" ", HeidenhainOptionsParser.DescribeCredentialProblems("""{"Access":{"InspectPassword":"hunter2"}}""")).ShouldNotContain("hunter2"); + HeidenhainOptionsParser.DescribeCredentialProblems("not json").ShouldNotBeEmpty(); + } + [Theory] [InlineData("")] [InlineData("http://host")] @@ -139,25 +220,30 @@ public sealed class HeidenhainConfigTests [Fact] public async Task Credentials_resolve_from_secret_store_and_environment_and_fail_closed() { - var resolver = new FakeResolver(new() { ["lsv2"] = "s3cret" }); - var env = new Dictionary { ["LSV2_PW"] = "envpw" }; + var resolver = new FakeResolver(new() { ["heidenhain/lsv2"] = "s3cret" }); + var env = new Dictionary { ["OTOPCUA_HEIDENHAIN_PW"] = "envpw" }; var c = new HeidenhainCredentials(resolver, n => env.GetValueOrDefault(n)); var ct = TestContext.Current.CancellationToken; - (await c.ResolveReferenceAsync("secret:lsv2", "INSPECT", ct)).ShouldBe("s3cret"); - (await c.ResolveReferenceAsync("env:LSV2_PW", "DNC", ct)).ShouldBe("envpw"); + (await c.ResolveReferenceAsync("secret:heidenhain/lsv2", "INSPECT", ct)).ShouldBe("s3cret"); + (await c.ResolveReferenceAsync("env:OTOPCUA_HEIDENHAIN_PW", "DNC", ct)).ShouldBe("envpw"); (await c.ResolveReferenceAsync(null, "DNC", ct)).ShouldBe(""); - await Should.ThrowAsync(() => c.ResolveReferenceAsync("secret:missing", "DNC", ct).AsTask()); - await Should.ThrowAsync(() => c.ResolveReferenceAsync("env:UNSET", "DNC", ct).AsTask()); - await Should.ThrowAsync(() => c.ResolveReferenceAsync("literal", "DNC", ct).AsTask()); - await Should.ThrowAsync(() => new HeidenhainCredentials(null).ResolveReferenceAsync("secret:lsv2", "DNC", ct).AsTask()); + await Should.ThrowAsync(() => c.ResolveReferenceAsync("secret:heidenhain/missing", "DNC", ct).AsTask()); + await Should.ThrowAsync(() => c.ResolveReferenceAsync("env:OTOPCUA_HEIDENHAIN_UNSET", "DNC", ct).AsTask()); + await Should.ThrowAsync(() => c.ResolveReferenceAsync("literal", "DNC", ct).AsTask()); + await Should.ThrowAsync(() => new HeidenhainCredentials(null).ResolveReferenceAsync("secret:heidenhain/lsv2", "DNC", ct).AsTask()); // device override wins over the driver-level reference - var o = new HeidenhainDriverOptions { Access = new() { InspectPassword = "secret:missing" } }; - (await c.ResolveAsync(o, new HeidenhainDeviceOptions { Host = "h", InspectPassword = "secret:lsv2" }, "INSPECT", ct)).ShouldBe("s3cret"); + var o = new HeidenhainDriverOptions { Access = new() { InspectPassword = "secret:heidenhain/missing" } }; + (await c.ResolveAsync(o, new HeidenhainDeviceOptions { Host = "h", InspectPassword = "secret:heidenhain/lsv2" }, "INSPECT", ct)).ShouldBe("s3cret"); } private sealed class FakeResolver(Dictionary secrets) : ZB.MOM.WW.Secrets.Abstractions.ISecretResolver { - public Task GetAsync(ZB.MOM.WW.Secrets.Abstractions.SecretName name, CancellationToken ct) => - Task.FromResult(secrets.TryGetValue(name.Value, out var v) ? v : null); + public int Reads { get; private set; } + + public Task GetAsync(ZB.MOM.WW.Secrets.Abstractions.SecretName name, CancellationToken ct) + { + Reads++; + return Task.FromResult(secrets.TryGetValue(name.Value, out var v) ? v : null); + } } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainDriverTests.cs index 91ba62b6f..4ef3030e4 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainDriverTests.cs @@ -275,7 +275,7 @@ public sealed class HeidenhainDriverTests { var registry = new DriverFactoryRegistry(); HeidenhainDriverFactoryExtensions.Register(registry); - var json = """{"Devices":[{"Host":"10.0.0.5","DeviceName":"d"}],"Access":{"InspectPassword":"secret:lsv2/inspect"}}"""; + var json = """{"Devices":[{"Host":"10.0.0.5","DeviceName":"d"}],"Access":{"InspectPassword":"secret:heidenhain/inspect"}}"""; var driver = registry.TryGet(DriverTypeNames.Heidenhain)!("hh", json); driver.ShouldBeOfType(); driver.DriverType.ShouldBe(DriverTypeNames.Heidenhain); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainLiveIntegrationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainLiveIntegrationTests.cs index 21897b8df..b0a9c4522 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainLiveIntegrationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainLiveIntegrationTests.cs @@ -12,7 +12,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests; /// /// /// Env: HEIDENHAIN_LSV2_HOST (required), HEIDENHAIN_LSV2_PORT (default 19000), -/// HEIDENHAIN_LSV2_INSPECT_PASSWORD (optional; never logged). +/// OTOPCUA_HEIDENHAIN_INSPECT_PASSWORD (optional; never logged). /// [Trait("Category", "LiveIntegration")] public sealed class HeidenhainLiveIntegrationTests @@ -28,7 +28,7 @@ public sealed class HeidenhainLiveIntegrationTests var ct = TestContext.Current.CancellationToken; await using var client = new Lsv2Client(Host!, Port, TimeSpan.FromSeconds(5)); await client.OpenAsync(ct); - await client.LoginAsync("INSPECT", Environment.GetEnvironmentVariable("HEIDENHAIN_LSV2_INSPECT_PASSWORD") ?? "", ct); + await client.LoginAsync("INSPECT", Environment.GetEnvironmentVariable("OTOPCUA_HEIDENHAIN_INSPECT_PASSWORD") ?? "", ct); var versions = Lsv2Data.Versions(await client.ReadVersionAsync(ct)); TestContext.Current.SendDiagnosticMessage($"model={versions.Model} nc={versions.Nc} plc={versions.Plc}"); versions.Model.ShouldNotBeNullOrWhiteSpace(); @@ -68,7 +68,7 @@ public sealed class HeidenhainLiveIntegrationTests { Devices = [new HeidenhainDeviceOptions { Host = Host!, Port = Port, DeviceName = "dev1" }], RawTags = tags, - Access = new HeidenhainAccessOptions { InspectPassword = Environment.GetEnvironmentVariable("HEIDENHAIN_LSV2_INSPECT_PASSWORD") is { Length: > 0 } ? "env:HEIDENHAIN_LSV2_INSPECT_PASSWORD" : null }, + Access = new HeidenhainAccessOptions { InspectPassword = Environment.GetEnvironmentVariable("OTOPCUA_HEIDENHAIN_INSPECT_PASSWORD") is { Length: > 0 } ? "env:OTOPCUA_HEIDENHAIN_INSPECT_PASSWORD" : null }, }; await using var driver = new HeidenhainDriver(options, "live"); await driver.InitializeAsync("{}", ct); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainReviewFixTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainReviewFixTests.cs new file mode 100644 index 000000000..63dfc37e2 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests/HeidenhainReviewFixTests.cs @@ -0,0 +1,236 @@ +using System.Collections.Concurrent; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Lsv2; +using ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests.Simulator; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Tests; + +/// Regression tests for the independent-review findings 3–7 on feat/803 (each fails on the pre-fix code). +[Trait("Category", "Unit")] +public sealed class HeidenhainReviewFixTests +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private static async Task<(Lsv2Simulator Sim, HeidenhainSession Session)> OpenAsync( + SimulatedControl control, HeidenhainSessionSettings settings, HeidenhainSession.PasswordSource passwords) + { + var sim = Lsv2Simulator.WithControl(control); + var client = new Lsv2Client("127.0.0.1", sim.Port, TimeSpan.FromSeconds(1)); + var session = new HeidenhainSession(client, settings, passwords); + await client.OpenAsync(Ct); + await session.InitializeAsync(Ct); + return (sim, session); + } + + // ---- Finding 6: an unusable OPTIONAL-role credential degrades only the reads that need that role. ---- + + [Theory] + [InlineData("throw")] // unresolvable reference (unset variable / missing secret) + [InlineData("nul")] // a NUL the login payload cannot carry + [InlineData("non-latin1")] // € is not Latin-1 + [InlineData("too-long")] // A_LG payload > 247 bytes + public async Task Bad_dnc_credential_keeps_inspect_telemetry_and_sends_no_dnc_login(string kind) + { + var control = new SimulatedControl(); + control.Runtime[23] = () => SimulatedControl.U16(4); + var settings = new HeidenhainSessionSettings(true, true, false, 64, false, false, false); + var (sim, session) = await OpenAsync(control, settings, (role, _) => role == "DNC" ? BadPassword(kind) : ValueTask.FromResult("")); + await using var _ = sim; + await using var __ = session; + var tags = new[] { TestTags.Def("Status/ExecutionState"), TestTags.Def("Capabilities/DncLogin") }; + var s1 = await session.ReadBatchAsync(tags, Ct); + var s2 = await session.ReadBatchAsync(tags, Ct); + s1[0].Value.ShouldBe(4); + s2[0].Value.ShouldBe(4); + s1[1].Value.ShouldBe("ConfigurationError"); + session.CommunicationLost.ShouldBeFalse(); + control.Logins.Select(l => l.Role).ShouldBe(new[] { "INSPECT" }); + } + + [Theory] + [InlineData("throw")] + [InlineData("nul")] + [InlineData("too-long")] + public async Task Bad_data_credential_is_BadConfigurationError_on_data_bindings_only(string kind) + { + var control = new SimulatedControl(); + control.Runtime[23] = () => SimulatedControl.U16(4); + control.DataPaths["/x"] = SimReply.Ok("S_DP", [0, 0, 0, 3, 0, 0, 0, 42]); + var settings = new HeidenhainSessionSettings(true, false, true, 64, false, false, true); + var (sim, session) = await OpenAsync(control, settings, (role, _) => role == "DATA" ? BadPassword(kind) : ValueTask.FromResult("")); + await using var _ = sim; + await using var __ = session; + var tags = new[] { TestTags.Def("DATA:/x", "Int64"), TestTags.Def("Status/ExecutionState"), TestTags.Def("Capabilities/DataLogin") }; + for (var i = 0; i < 2; i++) + { + var s = await session.ReadBatchAsync(tags, Ct); + s[0].Status.ShouldBe(HeidenhainStatus.BadConfigurationError); + s[1].Value.ShouldBe(4); + s[2].Value.ShouldBe("ConfigurationError"); + } + + session.CommunicationLost.ShouldBeFalse(); + sim.Requests.ShouldNotContain(r => r.Command == "R_DP"); + control.Logins.ShouldNotContain(l => l.Role == "DATA"); + } + + private static ValueTask BadPassword(string kind) => kind switch + { + "throw" => throw new HeidenhainCredentialException("DATA password environment variable 'OTOPCUA_HEIDENHAIN_X' is not set (fail-closed)."), + "nul" => ValueTask.FromResult("a\0b"), + "non-latin1" => ValueTask.FromResult("p€ss"), + _ => ValueTask.FromResult(new string('x', 250)), + }; + + // ---- Finding 3: Start after DisposeAsync must not revive an orphan loop. ---- + + [Fact] + public async Task Start_after_dispose_is_a_no_op() + { + var control = new SimulatedControl(); + control.Runtime[23] = () => SimulatedControl.U16(4); + await using var sim = Lsv2Simulator.WithControl(control); + var runtime = NewRuntime(sim.Port, (_, _) => { }); + await runtime.DisposeAsync(); + runtime.Start(); + await Task.Delay(400, Ct); + sim.Connections.ShouldBe(0); + runtime.State.ShouldBe(HeidenhainConnectionState.Stopped); + } + + // ---- Finding 5: null sections must not NRE, and nothing may kill the loop silently. ---- + + [Fact] + public async Task Null_option_sections_from_a_constructed_options_object_do_not_kill_the_loop() + { + int port; + await using (var probe = Lsv2Simulator.WithScript((_, _) => Task.CompletedTask)) port = probe.Port; + var options = new HeidenhainDriverOptions + { + Devices = [new HeidenhainDeviceOptions { Host = "127.0.0.1", Port = port, DeviceName = "dev1" }], + RawTags = [TestTags.Raw("Status/ExecutionState")], + Access = null!, + Polling = null!, + Reconnect = null!, + TimeoutMs = 300, + }; + var attempts = new ConcurrentQueue(); + await using var d = new HeidenhainDriver(options, "hh", null, new HeidenhainCredentials(null), + (h, p, t) => { attempts.Enqueue(DateTime.UtcNow); return new Lsv2Client(h, p, t); }, null); + await d.InitializeAsync("{}", Ct); + await d.SubscribeAsync([TestTags.Raw("Status/ExecutionState").RawPath], TimeSpan.FromSeconds(1), Ct); + await Task.Delay(3500, Ct); + attempts.Count.ShouldBeGreaterThanOrEqualTo(2); // the loop survived the first failure and retried + d.GetHealth().State.ShouldBe(DriverState.Degraded); + } + + [Fact] + public async Task Null_option_sections_normalise_so_the_control_is_actually_read() + { + var control = new SimulatedControl(); + control.Runtime[23] = () => SimulatedControl.U16(4); + await using var sim = Lsv2Simulator.WithControl(control); + var tag = TestTags.Raw("Status/ExecutionState"); + var options = new HeidenhainDriverOptions + { + Devices = [new HeidenhainDeviceOptions { Host = "127.0.0.1", Port = sim.Port, DeviceName = "dev1" }], + RawTags = [tag], + Access = null!, + Polling = null!, + Reconnect = null!, + }; + await using var d = new HeidenhainDriver(options, "hh", null, new HeidenhainCredentials(null), null, null); + await d.InitializeAsync("{}", Ct); + await d.SubscribeAsync([tag.RawPath], TimeSpan.FromSeconds(1), Ct); + var deadline = DateTime.UtcNow.AddSeconds(5); + while ((await d.ReadAsync([tag.RawPath], Ct))[0].StatusCode != HeidenhainStatus.Good && DateTime.UtcNow < deadline) + await Task.Delay(50, Ct); + (await d.ReadAsync([tag.RawPath], Ct))[0].Value.ShouldBe(4); + } + + // ---- Finding 7: backoff resets only after real progress (a published poll batch). ---- + + [Fact] + public async Task A_control_that_accepts_login_then_drops_is_backed_off_not_hammered() + { + var control = new SimulatedControl(); + control.Runtime[23] = SimReply.Drop; // login + version succeed, the first poll read drops + await using var sim = Lsv2Simulator.WithControl(control); + var runtime = NewRuntime(sim.Port, (_, _) => { }); + await using var _ = runtime; + runtime.Start(); + await Task.Delay(2000, Ct); + // initial 100 ms doubling to 800 ms: ~100+200+400+800 ⇒ ≤ 6 connects in 2 s; pre-fix it reset to + // 100 ms after every login and dialled ~15 times. + sim.Connections.ShouldBeInRange(2, 6); + runtime.ConsecutiveFailures.ShouldBeGreaterThanOrEqualTo(3); + } + + [Fact] + public async Task Backoff_resets_after_a_session_that_published() + { + var reads = 0; + var control = new SimulatedControl(); + control.Runtime[23] = () => Interlocked.Increment(ref reads) % 3 == 0 ? SimReply.Drop() : SimulatedControl.U16(4); + await using var sim = Lsv2Simulator.WithControl(control); + var runtime = NewRuntime(sim.Port, (_, _) => { }); + await using var _ = runtime; + runtime.Start(); + await Task.Delay(1500, Ct); + runtime.ConsecutiveFailures.ShouldBeLessThanOrEqualTo(1); + } + + // ---- Finding 4: the initial subscription push can never deliver an older value after a newer one. ---- + + [Fact] + public async Task Initial_push_and_publish_are_ordered() + { + var options = new HeidenhainDriverOptions + { + Devices = [new HeidenhainDeviceOptions { Host = "127.0.0.1", Port = 1, DeviceName = "dev1" }], + RawTags = [TestTags.Raw("Status/ExecutionState")], + }; + await using var d = new HeidenhainDriver(options, "hh", null, new HeidenhainCredentials(null), null, null); + await d.InitializeAsync("{}", Ct); + var raw = options.RawTags[0].RawPath; + var perSub = new ConcurrentDictionary>(); + d.OnDataChange += (_, e) => + { + if (e.Snapshot.Value is int v) perSub.GetOrAdd(e.SubscriptionHandle, _ => []).Add(v); + }; + using var stop = new CancellationTokenSource(); + var publisher = Task.Run(() => + { + var now = DateTime.UtcNow; + for (var i = 1; i < 20_000 && !stop.IsCancellationRequested; i++) + d.Publish(raw, new DataValueSnapshot(i, HeidenhainStatus.Good, now, now)); + }, Ct); + for (var i = 0; i < 200; i++) + await d.SubscribeAsync([raw], TimeSpan.FromSeconds(1), Ct); + await publisher; + foreach (var (_, values) in perSub) + { + List copy; + lock (values) copy = values.ToList(); + for (var i = 1; i < copy.Count; i++) + copy[i].ShouldBeGreaterThan(copy[i - 1], "a subscriber saw an older value after a newer one"); + } + } + + private static HeidenhainDeviceRuntime NewRuntime(int port, Action publish) + { + var options = new HeidenhainDriverOptions + { + TimeoutMs = 500, + Polling = new HeidenhainPollingOptions { StatusMs = 100, CustomMs = 100, SlowMs = 100 }, + Reconnect = new HeidenhainReconnectOptions { InitialDelayMs = 100, MaxDelayMs = 800 }, + }; + var device = new HeidenhainDeviceOptions { Host = "127.0.0.1", Port = port, DeviceName = "dev1" }; + return new HeidenhainDeviceRuntime(device, options, [TestTags.Def("Status/ExecutionState")], + (_, _) => ValueTask.FromResult(""), publish, (_, _, _) => { }, + (h, p, t) => new Lsv2Client(h, p, t), TimeProvider.System, NullLogger.Instance); + } +}