2 Commits

Author	SHA1	Message	Date
romanett	9b034f8291	Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700) ... # Description Nullable reference types are now enabled once, in `common.props`, and every project in the repository builds warning-clean with them. **Configuration** - `<Nullable>enable</Nullable>` lives in `common.props`. All per-project `<Nullable>` settings are removed: 178 project files (enable, annotations, and the `disable` in the three Aot test projects). - 419 per-file `#nullable enable`/`restore` directives and the 4 `#nullable disable` directives are removed. - Kept on purpose: - The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`, `PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the project setting in generated code, so the directive is required there. - `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against the 1.5.378 packages and does not import the repository props. - `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now describe the central setting. **Migration** (the 43 unset and 13 annotations-only projects, roughly 10k diagnostics) - **Production code: annotated at the declarations.** Covers `tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the LDS sample. - Members, parameters and returns are nullable where null actually flows. - Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`, `IsMethodTypeDesign`). - `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take `params object?[]`. `Template.AddReplacement` takes `string?` (rendering already skips null replacements). - `IRegisteredServerStore.RegisterAsync` returns `Task<RegistrationEntry?>`: it returns null when a registration is removed. - Dereferences backed by an invariant (for example, the validated model) use `!`, so runtime behaviour is unchanged. - **Behavioural changes in production code** (small, deliberate): - `NodeSetToModelDesign` throws `InvalidOperationException` when the file is not a NodeSet. Before, it threw a NullReferenceException on the next line. - `ModelDesignValidator` starts its lookup tables empty instead of null. A redundant `if (m_nodes == null) return;` guard is gone. - **Test code: minimal annotations.** - `?` on values that can be null, `!` where the test asserts non-null. - `= null!` for fixture members assigned in SetUp. - `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the `IEncodeable` constraint. - A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`) folded into the adjacent `IsNull`/`Count` checks. **Notes for reviewers** - The diff is large (~1,500 files) but almost entirely mechanical: `?`, `!` and directive removal. The non-annotation changes are listed above. - Known follow-up, not done here: `IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay non-nullable even though the validator returns null before it loads a dictionary. Making them nullable would ripple through every generator call site. ## Related Issues - Fixes #4439 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. The "no new warnings" box covers net10.0 and net48: full non-incremental builds of `UA.slnx`, `tools/SourceGeneration.slnx`, `tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0 warnings and 0 errors under the repository's warnings-as-errors settings. net9.0 and net8.0 also had no nullable diagnostics, but they were not rebuilt after the final fixes. Tests have not been run locally yet. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-08 08:48:13 +02:00
romanett	f96ca6df0b	Enforce trust-material changes on live channels regardless of origin (#4357) ... ## Proposed changes When a CRL (or any other trust-material change) reaches the server, connected clients whose certificates it revokes should be disconnected immediately — not at their next security-token renewal (default lifetime 1 h). The `ApplyChanges` push path already ran the OPC 10000-12 §7.10.9 sweep, but three gaps kept a revoked client connected: 1. **The push path never invalidated the validator.** `TrustList` writes certificates/CRLs directly to the stores; `CertificateManager.NotifyTrustListChanged` (the only thing that drops the cached validation cores) was never called. Freshness relied on the directory store's `(name, length, mtime)` + 30 s cache heuristic, which non-directory `ICertificateStore` implementations don't even guarantee. Worse: with `SecurityConfiguration.UseValidatedCertificates = true`, the thumbprint fast path returned **before the revocation check**, so a pushed CRL was never enforced at all for a previously-seen client — not at renewal, not even on a fresh channel. 2. **Out-of-band changes had no server-side consumer.** `CertificateChangeKind.TrustListUpdated`/`CrlUpdated` (raised by `WriteTrustListAsync`, `ITrustListTransaction` commits, HA shared-store sync) were only observed by the client-side `ManagedSession`; enforcement waited for the token renewal. 3. The legacy non-transactional `TrustList` (immediate apply, `applyChangesRequired = false`) ran no effects at all. ### Changes - `NotifyTrustListChanged` is now part of `ICertificateTrustListManager` (public on `CertificateManager`). The `ApplyChanges` deferred apply calls it for every committed scope after the response boundary and **before** the §7.10.9 sweep, so the sweep provably observes the committed stores. The legacy `TrustList` gets the same via an opt-in `SetTrustListChangeNotifier`. - The `UseValidatedCertificates` fast path in `CertificateValidationCore` moved after the issuer walk: revocation is always evaluated; a clean cache hit still skips the expensive chain build (the performance intent is preserved, the security hole is not). - New internal `CertificateChangePump<TState>`, extracted from `ClientChannelManagerCertRotation` (which now runs on it, behavior unchanged), coalesces `CertificateChanges` events. `ConfigurationNodeManager` subscribes a scope-union pump so **any** `TrustListUpdated`/`CrlUpdated` — whatever wrote the stores — triggers the same `IPushConfigurationTrustListEffectHandler` fan-out. Self-raised notifications are suppressed, so a push still runs exactly one sweep. Session semantics are unchanged: peer revocation cuts only the SecureChannel; only user-token-group changes close Sessions (+Subscriptions). - The per-group store identifiers preserve the configured `StoreType` instead of re-inferring it from the path (fixes pushing through a different store implementation than the validator reads for custom providers, e.g. HA shared key-value stores). - Integrated the caller-owned `CertificateStoreIdentifier.OpenStore` contract from #4382: `TrustList` lazily holds its trusted/issuer store instances for its lifetime, `ConfigurationNodeManager` owns its rejected-store instance, and both node managers dispose the handlers they own. This replaces the former interim identifier-cache cleanup while keeping the shutdown leak gate clean. ### Behavior notes - A server that previously kept revoked clients connected until token renewal now cuts them as soon as an out-of-band CRL lands. Re-validation failures fail open (a channel whose trust cannot be determined stays open), and closed-channel counts are logged. - Hosts using `UseValidatedCertificates` for throughput keep the chain-build saving but lose a revocation-check skip they should never have had. - A bare file copied into a store directory raises no event and is still only picked up at the next validation (bounded by the 30 s directory cache for new channels). ### Tests - `PushTest.PushedCrlDisconnectsRevokedClientAsync` — end-to-end: a client with a CA-issued certificate connects, a CRL revoking it is pushed via `CloseAndUpdate` + `ApplyChanges`, the server cuts its channel right after the response and refuses its reconnects; the unaffected admin client stays connected; trust material is restored afterwards. - `ConfigurationNodeManagerTrustListEffectsTests` — live-validator enforcement after a pushed CRL, out-of-band pump dispatch per scope, scope→effect mapping, store-type preservation. - `CertificateValidationCoreTests.ValidatedCertificateFastPathDoesNotSkipRevocationAsync` — fails without the fast-path fix. - `CertificateChangePumpTests` — coalescing, drain-until-empty, error resilience, dispose semantics. - `TrustListTests` — legacy notifier flags for `CloseAndUpdate`/`AddCertificate`/`RemoveCertificate`. Suites run locally (net10.0): GDS PushTest 314/314 (leak detector clean), Server.Tests configuration/trust/push 416/416, Core.Tests certificate/channel 556/556, Client.Tests 2161/2161; net48 build clean. ## Types of changes - [x] Bugfix (non-breaking change which fixes an issue) - [x] Enhancement (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation Update (if none of the other choices apply) ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf). - [x] I ran tests locally with my changes. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added necessary documentation (if appropriate). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Marc <marcschier@hotmail.com> Copilot-Session: d043dc7d-5c08-4084-96be-1f64e5466525	2026-09-02 12:12:55 +00:00