Commit Graph
4 Commits
Author SHA1 Message Date
Marc Schier f5ad124891 Additional fuzzing plus fixes for every defect the campaigns found (#4459)
# Description

Brings the stack's fuzz testing to parity with the fork
(`aio-opcua-sdk`), then fixes every defect the resulting campaigns
exposed.

The fork's fuzzing was used as the behavioural baseline: all **28
fork-local callbacks** and **13 OneFuzz registrations** are now
accounted for by an explicit mapping, not just by a larger target count.
The work extends the existing SharpFuzz/NUnit infrastructure rather than
introducing another framework.

### Coverage

- **84 continuous libFuzzer targets** and **81 AFL adapters** across
Encoders, Certificates, Network and PubSub.
- **PubSub area integrated** (Corpus/Tools/Tests siblings, solution
entries) — it previously had host source only and was absent from the
solution and replay infrastructure.
- Segmented binary adapters, binary-to-JSON and XML-to-JSON conversions,
standalone certificate loading, both chain-parser variants, CRL encoding
and canonical TBS round-trips.
- The 12 original fork message inputs are retained byte-identically
alongside richer deterministic generators; the three historical
crash/slow/timeout reproducers are unchanged (verified identical by
SHA-256).

### Production fixes found by fuzzing

These are real robustness and data-integrity bugs, each with a retained
regression input:

- `UadpBinaryReader.Advance` accepted attacker-controlled lengths on the
receive path. Declared payload sizes are now authoritative bounded
sections, with negative-count rejection and overflow-safe reassembly
arithmetic.
- PubSub JSON decoding leaked `InvalidOperationException` for invalid
UTF-8.
- `JsonEncoder.WriteExtensionObject` leaked
`System.Text.Json.JsonReaderException`.
- `JsonEncoder` silently emitted a lossy `{}` for an `ExtensionObject`
whose TypeId could not be represented in JSON.
- `JsonDecoder` wrapped a bodyless `ExtensionObject` envelope as a raw
JSON body instead of preserving `ExtensionObjectEncoding.None`.
- `Node.Encode` lost the null/empty collection distinction.
- Binary/XML Variant matrix dimensions are prevalidated rather than
relying on a caught `ArgumentException`.
- `X509CRL.ReadTime` pre-validates strict RFC 5280 whole-second Zulu
time forms.
- `Utils.ParseCertificateChainBlob` disposes correctly on partial-chain
failure.
- **`nodeid.dict` was unloadable by libFuzzer** (empty token, unescaped
quote). libFuzzer rejects the whole file on one bad line, so four parser
targets silently executed **zero** inputs. Fixed and now guarded by
dictionary validation.

### Infrastructure

- `fuzzing/fuzz-targets.json` manifest recording adapters, corpora,
dictionaries, oracles and fork-behaviour mappings, with
`validate-fuzz-manifest.ps1` and **27 negative controls**.
- Strict host/tools discovery and replay: `--list`, `--replay`, non-zero
exits on unknown targets, missing or empty corpora, and failed callbacks
(playback previously logged failures and returned success).
- A real process watchdog for the timeout/slow regressions — NUnit
`CancelAfter` cannot interrupt a synchronous fuzz callback.
- Runner scripts made non-destructive (unique work directories, no
generic `bin`/findings deletion) and corrected to the native driver's
single-`--target_arg` apphost contract.
- Opt-in `OneFuzz=true` class-library publication producing a
relocatable, **uninstrumented** drop, validated by exact callback
discovery, dependency closure, corpora and dictionaries.
- CI: fuzz replay matrix plus a public drop gate, triggered by corpus-,
dictionary- and script-only changes.

### Verification

| Gate | Result |
|---|---|
| Bounded campaigns, all 84 targets (1,000 inputs, seed 1) | 84/84,
84,000 executions, zero findings |
| OneFuzz drop | 84/84 published callbacks replayed, zero skips |
| Fuzz replay net10.0 / net8.0 / net9.0 | 9,470 passed each, 0 failed |
| Fuzz replay net48 | 8,996 passed, 0 failed (Network/PubSub are
modern-only) |
| `Opc.Ua.Types.Tests` / `Opc.Ua.Core.Tests` | 9,800 and 4,442 passed, 0
failed |
| Retained regression assets | 132 `crash-*` inputs replayed by every
target each run |

### Known gaps (deliberately not closed)

- **Fidelity is not enforced against the pipeline crash corpus.** The
differential binary-to-JSON targets assert both robustness (no unhandled
exception, expected of arbitrary bytes) and fidelity (a decoded value
re-encodes equivalently, a property of well formed values). The pipeline
supplies 22,293 blobs collected for the targets that existed before this
PR, none of which asserted fidelity; 522 of them violate it, and that
count did not move across two rounds of genuine fixes because each fix
only lets the mutator reach the next representability gap. Rather than
weaken the assertion, the two contracts are separated: fidelity failures
raise `EncodingFidelityException`; curated assets under `Assets/Repo`,
the curated `Testcases` corpus and every continuous libFuzzer campaign
stay strictly enforced, while externally supplied assets are gated on
robustness only and their fidelity findings are reported with counts and
bounded base64 reproducers.
- **Internal MSFT OneFuzz canary has not been run.** It needs an
owner-supplied ownership profile, pipeline authorisation and corpus
snapshot; none were available. The drop builds and validates locally and
records `serviceCanaryPerformed: false`. A build or upload is not a
successful worker run.
- **Legacy JSON wire forms are a partial mapping.** The fork accepts its
four original `TypeId`/`Body` messages; this stack rejects that retired
envelope (measured JSON-only control: fork `JsonEncoder` 630/1356 lines
vs 0/1007 here). Recorded as an explicit unresolved mapping rather than
an exemption. `LegacyNonReversible` is a RawData-derived profile and is
not counted as independent coverage.

## Related Issues

_No tracking issue was supplied for this work; happy to open one and
link it if maintainers prefer._

- Fixes #github-issue-number, ...

## 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.
- [x] 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.
- [x] 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.

> Note on the checklist: the fuzz replay matrix, `Opc.Ua.Types.Tests`
and `Opc.Ua.Core.Tests` were run locally on **net48** and **net10.0**
(plus net8.0/net9.0) and all passed. CI and CodeQL results are not yet
available for this branch, so those boxes are left unchecked. The CLA
box is for the author to confirm.
2026-09-10 05:56:31 +02:00
Marc SchierandCopilot ea0f266b66 Move build and test to the Azure DevOps managed pool and replace Codecov with an in-pipeline coverage gate (#4122)
# Description

Moves the all-target-framework build, the cross-platform test matrix and
the Native AoT run from GitHub Actions onto the Azure DevOps
`netstandard` Managed DevOps Pool, and replaces Codecov with a coverage
gate that runs inside the pipeline.

## 1. CI backend switch

One conceptual switch decides who owns that work. It is checked into
source in **two places that must be flipped together**:

| File | Setting | Default |
| --- | --- | --- |
| `azure-pipelines.yml` | `parameters.ciBuildBackend` | `ado` |
| `.github/workflows/buildandtest.yml` | `env.CI_BUILD_BACKEND` | `ado`
|

Setting both to `actions` restores the previous behaviour exactly.

Azure Pipelines gains what it was missing to take the work over:

- A **Linux all-TFM `sln.yml` leg**, replacing `build-all-tfm-linux`.
`sln.yml` gets a `configurations` parameter so this leg builds `Release`
only instead of doubling the managed-pool job count.
- A **Linux agent in the `net10.0` `testfastpr` matrix**, replacing the
ubuntu `build-and-test` job.
- **Native AoT and Code Coverage now gate pull requests** instead of
running only on Schedule/Manual. The coverage stage drops its
`testdebug` dependency, which is Schedule/Manual-only and would
otherwise have skipped coverage on every PR.
- On a PR, Windows builds the .NET Framework TFMs and Linux the modern
ones — mirroring the split the GitHub Actions jobs used — so all seven
TFMs stay covered without fanning every TFM across both pools.

Two things are deliberately **not** covered by the switch:

- **macOS** stays on GitHub-hosted runners, because Managed DevOps Pools
provide no macOS image.
- **`master378` and `develop/*`** keep running the GitHub Actions jobs,
since Azure Pipelines only builds `master`/`main` from this file.

GitHub Actions does not expose the `env` context to job-level `if:`
conditions, so the switch is resolved once in the existing `discover`
job and re-exported as job outputs.

A new **`build-and-test summary`** job gives branch protection one
stable check to require, instead of the generated matrix job names. It
treats an intentionally skipped job as success but still fails on a real
failure or cancellation.

The `paths:` allow-list was moved off the `pull_request` trigger and
into `discover`. A workflow filtered out by `paths` never reports its
checks, and a required check that never reports leaves a PR permanently
*"Expected — waiting for status to be reported"*. That would have made
the routine "ratchet the coverage floor" PR (which touches only
`coverage-thresholds.json` and docs) unmergeable.

## 2. Coverage gate replacing Codecov

Codecov is removed entirely — `codecov.yml`, both uploads, the badge,
and the token usage. `.azurepipelines/check-coverage.ps1` runs at the
end of the `Code Coverage` stage against the Cobertura report
ReportGenerator already merges, driven by `coverage-thresholds.json`:

| Check | Behaviour |
| --- | --- |
| Project floor | **Blocking** — total line and branch rates against an
absolute floor. |
| Patch coverage | **Blocking on PRs** — changed lines must reach
`patch.target`, tolerating `patch.threshold` percentage points (the same
semantics `codecov/patch` had). |
| Baseline delta | **Advisory** — reports the change against the
recorded baseline, never fails. |

The project rate is recomputed from the per-line data rather than read
off the Cobertura root element, so the `ignore` globs carried over from
`codecov.yml` apply to the project floor as well as to the changed-lines
gate — otherwise samples and generated code would count towards one gate
but not the other.

`testcc.yml` gains an explicit `checkout: self` with `fetchDepth: 0`,
because the default shallow clone cannot diff against the base branch.

## Follow-up actions for maintainers

Two items cannot be automated from source:

1. **Enable the `/azp run` gate.** In the Azure DevOps portal: pipeline
→ **More actions** → **Triggers** → **Pull request validation** →
**Require a team member's comment before building a pull request**,
scoped to *Only on pull requests from non-team members*. This makes
Copilot-authored and outside-contributor PRs wait for a maintainer's
`/azp run`, mirroring the approval GitHub Actions already requires. The
documentation added here describes the workflow and the
private-org-membership gotcha; **the setting itself still has to be
switched on.**
2. **Update branch protection and secrets.** Drop the `codecov/project`
and `codecov/patch` required checks, require `build-and-test summary`
and the ADO coverage check instead, and remove `CODECOV_TOKEN` from the
GitHub repository secrets and the ADO variable group.

Also note the thresholds in `coverage-thresholds.json` are seeded from
the last Codecov reading (73.61 %) with headroom, because the
in-pipeline number had not been observed yet. The first full `Code
Coverage` run on master should confirm it and the floors should be
tightened then — the gate prints the exact figure it computes.

## Related Issues

_No tracking issue; this is CI infrastructure work requested directly._

## Checklist

- [x] 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.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- No product code changed, so there are no unit tests to add. The new
logic was verified with purpose-built harnesses instead:
- All 8 branch/event/backend combinations of the `discover` switch,
confirming `master378` and `develop/*` are untouched and that `actions`
restores the old behaviour.
- 12 scenarios for `check-coverage.ps1`: the project floor, the patch
gate, ignore globs applying to *both* gates, three path-normalization
shapes, and the advisory delta.
- 14 path-relevance cases confirming the in-job filter matches the
retired `paths:` allow-list exactly.
- End-to-end against a **real** ReportGenerator report: correctly
resolved `src/Opc.Ua.PubSub/DependencyInjection/PubSubBuilder.cs`, found
exactly the four modified tracked lines and reported 50 % patch coverage
(2 covered, 2 uncovered).
- [x] I have added all necessary documentation.
- New **Continuous integration** section in `docs/DeveloperGuide.md`, a
CI note in `CONTRIBUTING.md`, and links from `docs/README.md`. Also
corrected the CI matrix description, which claimed the PR gate ran net48
+ **net8.0** — it runs net48 + **net10.0**.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- `dotnet build src/Opc.Ua.Server` → 0 warnings, 0 errors. No product
code changed.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- Not applicable — no product code changed. `Opc.Ua.Server.Tests` was
run on net10.0 during development (3480 passed; the single
`ServerFluentApiHostingTests` failure is pre-existing and reproduces on
master).
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [x] I have addressed **all** PR feedback received.
- A review pass over this branch flagged the `paths:`-filter deadlock
described above; it is fixed in `1926e240e`.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 181ba7ea-c72f-42d7-a737-00a3faadaa17
Copilot-Session: 26aa55a2-5731-4a4c-b61c-df2f7a785eca
2026-07-31 21:06:25 +00:00
Marc Schier 8538f531e7 Update paths for the consolidated repository layout 2026-07-16 06:26:05 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00