Skip to content

refactor: Simplify device events, cryptography, YubiHSM, and WebAuthn APIs - #643

Merged
DennisDyallo merged 27 commits into
yubikitfrom
yubikit-isa-integration
Sep 6, 2026
Merged

refactor: Simplify device events, cryptography, YubiHSM, and WebAuthn APIs#643
DennisDyallo merged 27 commits into
yubikitfrom
yubikit-isa-integration

Conversation

@DennisDyallo

@DennisDyallo DennisDyallo commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

What this does

Alpha API cleanup across device events, Native AOT validation, ASN.1 EC handling, YubiHSM/credential naming, and WebAuthn. Driven by an agreed ideal-state artifact with 54 acceptance criteria; all 54 have direct evidence.

Based on yubikit @ c3b7e206. Built as five parallel work quanta in isolated worktrees, each cross-vendor reviewed, then merged with zero conflicts, followed by a simplification pass, a whole-diff audit, and one owner-requested follow-up.

Breaking changes are intentional and unwrapped — this is alpha, and no compatibility shims were added.

Changes by area

Device eventsWatchAsync is now the only public device-change stream. Public DeviceChanges (IObservable) is gone, along with DeviceEventBroadcaster, DeviceEventStream, and two unused interfaces. One internal DeviceEventHub backs delivery, preserving per-watcher bounded buffers, 256-event overflow isolation, lazy subscription, cancellation isolation, and normal completion on disposal. System.Reactive is dropped from the dependency graph entirely.

Native AOT — the hand-maintained nativeAotSdkProjects inventory is replaced by filesystem discovery. The contract now also checks the reverse direction (a host-referenced project that stops matching the SDK layout fails), parses ProjectReference with XDocument instead of a substring match, and forces an entry-type decision for any newly discovered library.

ASN.1 / EC — one shared point-shape validator across all four encoders/decoders. Fixes an IndexOutOfRangeException on empty public points, adds curve-specific coordinate and private-scalar length validation, rejects non-EC OIDs that previously produced a valid-looking id-ecPublicKey encoding, and emits RFC 5915 [1] as an explicitly tagged BIT STRING so keys round-trip through the .NET importer for P-256/384/521.

WebAuthn — adds WebAuthnClientOptions (CredentialPrompt, EnterpriseRpIds, MaxPromptAttempts, default 3). The backend seam is internal, FidoSessionWebAuthnBackend is renamed WebAuthnBackend, the ceremony status streams and WebAuthnStatus hierarchy are removed, and the 1,255-line client is split into partials by concern. A unified interaction model stays deferred; no replacement callback API was introduced.

Secret-parameter naming — the ...Utf8 suffix is dropped from public secret parameters across YubiHSM, Oath, OpenPgp, and Fido2. The UTF-8 contract and caller-ownership/clearing rules move into XML docs; several OpenPgp members had no <param> docs at all and now do.

Notable fixes found along the way

  • PIN/UV token was never zeroed. ClientPin's decrypted token was cloned into PinUvAuthTokenSession and the original abandoned to the GC. Ownership now transfers, so exactly one copy exists and it is zeroed deterministically.
  • ArgumentException.ParamName named nothing. Both ClientPin.ValidatePin and HsmAuthSession reported an internal helper's local name. Now every public entry point reports its own parameter, pinned by tests.
  • Flaky suite root cause. YubiKeyManagerStaticTests were spinning up real PC/SC and HID listeners inside a unit-test project, failing intermittently with discovery worker capacity is saturated. Serialised into the existing discovery-admission collection; the suite is now stable across repeated runs.
  • Three sham guards removed — tests that read as coverage but could not fail.

Verification

Gate Result
dotnet toolchain.cs build 0 errors
dotnet toolchain.cs test 12/12 projects, 2688 tests, 0 failed
dotnet toolchain.cs -- resilience --fast 74 / 0
dotnet toolchain.cs native-aot-contract-qa 10 SDK libraries + 2 excluded tooling

Core 1211/0/3 · Fido2 430/0 · WebAuthn 213/0 · Piv 193/0 · YubiOtp 155/0 · Oath 106/0 · OpenPgp 105/0 · Management 91/0 · YubiHsm 85/0 · SecurityDomain 47/0 · Cli.Shared 43/0 · Cli.Commands 9/0

No integration tests were run — hardware was in use by other work. Integration projects compile.

Known follow-ups (deliberately not in this PR)

  • WebAuthnClient.Registration.cs allocates pinUvAuthParam before Build*ExtensionsCbor can throw, so an extension-pipeline failure leaves a per-request HMAC unzeroed. Wants its own change across both ceremony paths.
  • YubiKeyManagerStaticTests is stable but still non-hermetic; a hermetic fix needs an internal static manager-factory seam.
  • verification/NativeAotVerification/Program.cs is compiled by no build step in the repo; the AOT contract validates it by text matching only.
  • AsnPublicKeyDecoder throws NotSupportedException for an unsupported-but-well-formed curve on decode — reviewed and accepted, but an open taxonomy question.

Review notes

Reviewers may want to look hardest at DeviceEventHub concurrency, the RFC 5915 tagging change, and the PIN token ownership transfer. Each was independently verified cross-vendor, but they are the three places where a subtle mistake would matter most.


Update: merged with yubikit (device-identity / flat device model)

yubikit advanced 28 commits while this branch was in review, including PR #629 (flat device model + device identity) and PR #641 (agent-doc consolidation). Both are now merged in.

Merge policy: our event architecture wins, upstream's device-identity behavior is carried in. Upstream never touched DeviceEventBroadcaster, DeviceEventStream, or either dead interface — it only used them — so the deletions were uncontested. Only ~40 lines of production code genuinely overlapped.

What was carried in from upstream:

  • serial-substitution republishing and DeviceInfo metadata retention in UpdateCache, rewired onto DeviceEventHub
  • the flat device model (YubiKeyDevice, connection slots) intact
  • upstream tests written against the deleted observer harness, ported to WatchAsync/DeviceEventWatcher with every assertion preserved

Merge verification

Because a silently-dropped upstream behavior would not fail any test, the resolution was verified structurally rather than only behaviorally:

Check Result
Symbol probes on YubiKeyDeviceRepository.cs 13/13 — ours and theirs both present in the expected counts
Test-name contract (frozen before resolving) 77 upstream + 68 ours, zero real loss
Attribution audit 16 contested files, every hunk attributed, none unexplained
Auto-merge no-loss proof patch(ours→merge) == patch(base→theirs) and vice versa, byte-exact
Assertion census on ported files DeviceIdentityContractTests 33→33, HotPlugIdentityContractTests 10→10
Mutation proof dropping serialContradiction, DeviceInfo retention, or hub delivery each goes red — the second through the ported upstream tests, proving they are not sham guards

Post-merge gate

12/12 projects, 2757 tests, 0 failed · Core 1261/0/3 · Fido2 449/0 · resilience 75/0 · AOT 10+2 · build 0 errors.

Still needs a human

HotPlugIdentityContractTests is ported and compiles but was never executed — it is hardware-gated and the YubiKeys were in use. Please run it against real hardware before merging.

Replace the hand-maintained nativeAotSdkProjects inventory with discovery
over src/<Module>/src/*.csproj. A newly added module is now held to the
Native AOT contract automatically: it must carry the IsYubiKitSdkLibrary
opt-in and be referenced by the verification host, or native-aot-contract-qa
fails and names the offending project path.

The reported library count is derived from the discovered set rather than a
hard-coded literal, and an empty discovery result is itself a failure so the
per-project checks cannot pass vacuously.
Rename every YubiHSM password parameter to its plain name
(credentialPassword, derivationPassword, currentPassword, newPassword,
password) across the interface, implementation, and module docs. The
UTF-8 encoding contract and the borrowed-buffer ownership contract stay
where they belong, in the XML documentation for each parameter.

Split ICredentialPrompt.cs so CredentialKind and CredentialPromptContext
each live in a correspondingly named file. Namespace, accessibility, and
member bodies are unchanged.
…it tags

The four ASN.1 EC key encoders/decoders each carried their own copy of the
uncompressed-point rules, and two of them were incomplete: AsnPublicKeyDecoder
indexed a zero-length BIT STRING and threw IndexOutOfRangeException instead of
CryptographicException, and neither encoder validated caller-supplied points or
coordinates against the named curve at all.

Move the point-shape rule into AsnUtilities and give it two throwing wrappers
that keep the exception taxonomy intact: malformed encoded key data stays a
CryptographicException, bad caller input to an encoder is an ArgumentException.
Compressed and hybrid points are rejected rather than decompressed, matching the
Rust yubikit crate.

Also fix the private-key encoder to write the optional RFC 5915 publicKey field
as an EXPLICIT [1] BIT STRING. It previously used an implicit tag, which
ECDsa.ImportPkcs8PrivateKey rejects, so any private key carrying its public
point could not be imported by .NET at all. The decoder now reads the explicit
form directly and still accepts the implicit form written by earlier builds.
…ctory

WebAuthnClient took its enterprise RP IDs and credential prompt as loose
optional constructor parameters, and its prompt-attempt cap was a public
const that callers could read but not change. Collect all three on a
WebAuthnClientOptions record instead.

MaxPromptAttempts is validated as it is assigned, so a non-positive limit
fails when the options are built rather than part-way through a ceremony
that has already reached the authenticator.

BREAKING: WebAuthnClient.MaxPromptAttempts, the enterpriseRpIds/prompt
constructor parameters, and the positional CreateWebAuthnClientAsync
compatibility overload are removed. Use WebAuthnClientOptions.
…thnBackend

IWebAuthnBackend exists so the client's ceremony logic can be tested without
an authenticator. It was public, which made a test seam look like an
extension point and put CTAP-shaped request records on the WebAuthn surface.

Internalize the contract (IWebAuthnBackend, the two Backend*Request records,
PinUvAuthMethod, PinUvAuthTokenSession) and the concrete implementation, and
rename FidoSessionWebAuthnBackend to WebAuthnBackend now that it is the only
one. Unit tests already see internals via the repo-wide Directory.Build.targets
rule; DynamicProxyGenAssembly2 is added so NSubstitute can still mock the
interface.

BREAKING: these types are no longer part of the public API.
MakeCredentialStreamAsync and GetAssertionStreamAsync only ever emitted
Processing followed by Finished or Failed, so they duplicated the awaitable
ceremony methods while adding a producer task, a channel, and a second error
path. Both the Rust and Python yubikit clients expose direct ceremony methods
instead, so removing the streams moves this SDK toward canonical behaviour.

No replacement callback is introduced: the unified interaction model is
deferred. Input still comes from pinBytes or WebAuthnClientOptions.CredentialPrompt,
and abandonment still comes from the cancellation token.

Two stream tests covered behaviour with no non-stream equivalent (a backend
CTAP failure surfacing as a typed error, and a PIN-needing ceremony with no
prompt configured); they are converted to direct MakeCredentialAsync tests.
The rest covered stream mechanics only and are deleted with Client/Status/.

BREAKING: MakeCredentialStreamAsync, GetAssertionStreamAsync, WebAuthnStatus
and its subtypes, and StatusChannel are removed.
WebAuthnClient.cs was over 1200 lines and mixed both ceremonies, PIN/UV token
acquisition, validation, and error mapping in one file. Split it into partial
files along those seams. WebAuthnClient.cs keeps construction, disposal, and
the shared zeroing helpers.

Pure move: the multiset of non-blank class-body lines is identical before and
after, and the WebAuthn unit suite reports 199 passing on both sides.
…ders

Two encoder input paths accepted things they should not.

EncodeToPkcs8(ECParameters) treated a half-supplied public point as no point
at all: if exactly one of Q.X and Q.Y was null it omitted the optional RFC 5915
[1] publicKey field and returned a perfectly valid private key encoding that
had quietly dropped the coordinate the caller did supply. Omit [1] only when
both coordinates are null; a half point is now an ArgumentException. This also
brings it in line with the public-key encoder, which already rejected it.

The shared point-shape helper sized coordinates via KeyDefinitions.GetByOid,
which is a general OID resolver rather than an EC-curve one. It also resolves
X25519, Ed25519, AES and Triple DES, several of which have a 32-byte
definition, so P-256 sized coordinates carrying a non-EC algorithm OID passed
validation and were then emitted as an id-ecPublicKey key whose curve parameter
is not a curve. Gate on Oids.IsECDsaCurve instead, and split the resolver into
a decode flavour that throws CryptographicException and an encode flavour that
throws ArgumentException, so a genuinely unsupported curve no longer surfaces
from an encoder as NotSupportedException.

The curve is now also checked at the ECParameters boundary rather than only
inside the point check, because a private key with no public point never
reaches the point check.
DeviceEventBroadcaster and DeviceEventStream became one DeviceEventHub:
Publish fans out into per-watcher bounded channels with TryWrite and never
runs consumer code, so a slow, abandoned, or throwing watcher can no longer
block or interrupt UpdateCache while the monitor holds its publication gate.

WatchAsync is now the only device change stream. DeviceChanges is removed
outright with no shim, which also drops the last System.Reactive reference
and lets IYubiKeyDeviceRepository and IYubiKeyDeviceMonitorService go - both
had a single implementation each.

Per-watcher independence is preserved exactly: lazy subscription, a 256-event
buffer that faults only its own watcher on overflow, cancellation isolation,
and normal completion on repository disposal.

Lifecycle tests that need an in-flight publication use the new
PublishAdmittedForTest seam, since a consumer can no longer hold one. The
monitor's generation, debounce, polling, listener, and retry model is
unchanged; only delivery moved.
…pted token

ClientPin allocates the decrypted PIN/UV auth token and hands it to
WebAuthnBackend, which passed it to PinUvAuthTokenSession. The session
copied it, so disposing the session cleared only the private clone and
left the original plaintext token live in managed memory until GC.

The session now adopts the array instead of copying it, so there is
exactly one live copy and it is zeroed whenever the session is disposed
- which the ceremony does in a finally, on success, failure and
cancellation alike.

Replaces GetAssertion_PinTokenZeroedAfterMethodReturns, which asserted
Assert.True(true) against an authenticator that never needed a token,
with three tests that force token acquisition and inspect the session's
owned buffer after a successful, a failed and a cancelled ceremony, plus
direct ownership tests on the session itself.

Also documents why the pinUvAuthParam copies in WebAuthnBackend are
load-bearing: ExcludeListPreflight reuses one buffer across chunk
probes, so the backend must not zero the caller's array.
Discovery alone left two gaps the previous hand-maintained inventory had
covered. Both are now checked against the verification host instead of a list.

- A library that moves out of the src/<Module>/src/ shape used to drop silently
  out of the per-project loop and pass green with a smaller count. Every
  ProjectReference the host declares under src/ must now resolve to a discovered
  SDK library, which also catches a reference left behind by a deleted module.
- A project reference with no rooted type lets ILC trim the assembly out of the
  host's closure, so the publish verifies nothing for it. The entry-type list is
  now a project -> type map and a discovered library with no mapping fails.

Host references are parsed with XDocument rather than substring-matched: a
commented-out ProjectReference, or a file name appearing only in the host
csproj's header comment, satisfied the old check while referencing nothing.

The Cli.* exclusion becomes a closed two-name set. Any other Cli.* module at the
SDK layout now fails as unclassified rather than being silently skipped, and the
excluded set is printed alongside the count.

The discovered set is unchanged: the same 10 SDK libraries are verified.
The repository-level overflow test started its second watcher after the
fault, so at overflow time exactly one watcher was subscribed. It pinned
"overflow faults" and "publication continues", never isolation - breaking
per-watcher isolation in the hub left it green. It now subscribes the
healthy watcher before the burst and paces publication the way the
hub-level test does, then asserts that watcher received all 511 events and
is still enumerating.

Delivery to a watcher whose channel is already terminated now returns
immediately on a volatile flag. An abandoned enumerator stays in the
publisher's snapshot for the process lifetime, and every publish was
allocating and formatting a fresh overflow exception for it - 688 bytes per
event, inside the monitor's publication gate. WatcherCount is deliberately
left counting terminated-but-registered watchers; it is the diagnostic that
makes that leak visible.

The hardware watch test was disposing its enumerator with a MoveNextAsync
still in flight on the timeout path, which throws NotSupportedException and
would have replaced the assertion message an operator needs. The hot-plug
teardown no longer insists the pump ended in cancellation specifically.

Document on WatchAsync that the caller must dispose the enumerator.
… token buffers

Two paths were guarded only by comments, with an existing test standing in
for coverage that did not exist.

WebAuthnBackend's .ToArray() of the caller's pinUvAuthParam is load-bearing:
ExcludeListPreflight hoists one parameter above its chunk loop and rebuilds a
request around that same buffer for every chunk, so a backend that zeroed the
caller's array would send an all-zero parameter from chunk two onwards. The
one test naming the multi-chunk scenario substitutes IWebAuthnBackend and
never executes WebAuthnBackend, so it stayed green through that mutation.
WebAuthnBackendTests now drives the concrete backend over a substituted
IFidoSession and pins both halves of the contract: the caller's buffer
survives, and the copy the backend put in the CTAP options is cleared.

The MakeCredential token path had no buffer-lifecycle coverage at all.
Deleting the pre-flight tokenSession.Dispose() orphans the decrypted token
minted before pre-flight, and nothing failed. The existing PuatRequired test
drives four acquisitions through that code but hands out new byte[32], which
is already all-zero, and asserts nothing about them. Three ceremony tests now
hand out a distinct sentinel buffer per acquisition and assert every issued
buffer is zeroed, on success, on a backend failure, and across the
PuatRequired retry.

Also hardens two helpers that had sham-guard shapes. TokenBufferAssert.Zeroed
passed vacuously on a zero-length buffer; it now rejects one. The ZeroMemory
helpers in WebAuthnBackend and WebAuthnClient silently skipped when the memory
was not array-backed; they now Debug.Assert instead. They cannot throw because
every call site is a finally block.
The rule is 'only format files you changed', not a restriction on the
subcommand. Also records that formatting a non-project file exits 0
without checking anything, so it must not be reported as a passing gate.
Five quanta were developed in isolation, so a few concepts ended up
expressed more than once in the merged tree. None of these change
behaviour; each removes a copy a reader would otherwise have to hold.

WebAuthn production: `ZeroMemory(ReadOnlyMemory<byte>?)` existed
verbatim, comment and all, in both WebAuthnClient and WebAuthnBackend.
It is now `Util.SensitiveMemory.Zero`, next to the existing Base64Url
helper.

WebAuthn tests: MockFido2Responses is the shared fixture home, but
WebAuthnClientMakeCredentialTests still carried a byte-for-byte copy of
CreateMockMakeCredentialResponse and its three builders, and there were
three separate ways to build a mock GetAssertionResponse. The shared
builder now takes the optional numberOfCredentials / signature / user
that the private copies needed, and the copies are gone.

Core tests: GetNamedCurve and BuildUncompressedPoint each appeared in
two of the Asn*Tests files; both now live in EcTestSupport.
UncompressedPoint deliberately keeps its own implementation rather than
calling AsnUtilities.BuildUncompressedEcPoint, so encoder tests do not
check that helper against itself.
Seven findings from the merged-branch audit, each self-contained.

YubiHsm credential-password validation threw with the internal helper's own
parameter name, so ArgumentException.ParamName reported "password" — a name no
public parameter uses. Callers filtering on it got nothing usable. The public
surface spells the parameter three ways (credentialPassword, currentPassword,
newPassword), so the name is now threaded from each entry point.

AsnPrivateKeyEncoder checked the curve OID and the public point strictly but
wrote the private scalar D straight through. A P-384 curve with a 32-byte D and
a valid 97-byte point produced a well-formed-looking RFC 5915 ECPrivateKey whose
privateKey OCTET STRING contradicted the curve beside it, with no exception. The
ECParameters path now validates D against the curve. The raw-value overload
stays permissive: its EC branch has no production caller, and the only key types
that do reach it (Curve25519) are already checked exactly.

YubiKeyDeviceManager.DisposeAsync cleared the repository before disposing it.
Disposal already clears, so the Clear() call bought nothing and opened a window
in which a publication that outlived the monitor's bounded drain could find a
live-but-emptied repository, diff against an empty cache, and emit Added for
every attached device after DisposeAsync had returned — contradicting the
documented guarantee that no device event escapes a disposed manager.

DeviceEventHub.WatcherCount documented "live subscriptions" while counting every
registered watcher including overflow-terminated ones. The count is right (a
leaked watcher should stay visible); the doc was wrong.

AsnUtilities claimed a CryptographicException-for-malformed-data taxonomy that
the decoders do not follow for unsupported curves: both reject the curve first,
with their own exception types, leaving the CryptographicException branch
unreachable from production. The remark now scopes the taxonomy to point shape
and names curve rejection as each decoder's own concern, and the tests covering
that branch say plainly that no caller reaches it.

Two YubiKeyManagerStaticTests started real HID and PC/SC listeners inside a unit
test, contending for the process-wide four-slot discovery-worker pool and making
unrelated tests fail intermittently with "discovery worker capacity is
saturated". The watcher-independence assertion moves onto the manager seam,
where fake listeners let it also assert the part it previously had to omit — the
surviving watcher keeps receiving. The post-shutdown recreation test drops an
assertion about ambient machine state in favour of one about the facade. The
facade's remaining lifecycle tests have no injection seam by design, so the
class joins the collection that already serializes discovery-pool consumers.

Finally, the codemapper skill still taught IObservable<DeviceEvent>
DeviceChanges, the last reference anywhere to a removed API.
…window

Dispose() emptied the cache but left the HasData flag set, so a disposed
repository reported a cache it no longer held. Reset it alongside the clear,
and correct the HasData doc: it is a cache-validity flag, not a count, so a
scan that found nothing legitimately sets it.

The disposal-window regression test only released its parked publication after
DisposeAsync had returned, by which point both the old and new orderings had
already disposed the repository - it passed either way. Add a teardown hook on
the manager so a test can resume a parked publication at the exact point the
manager is about to dispose the repository, after every other teardown step,
and assert nothing is emitted. Retitle the old test to say what it actually
pins: the post-return path.

Delete the orphaned Repository.Clear(). It had no production callers and its
doc still recommended the shutdown usage that turned out to be unsafe.
…ameters

Finish the naming convention started in YubiHSM Auth. The SDK carried two
spellings for the same kind of UTF-8 secret across sibling modules; every
public secret parameter is now named plainly, and the UTF-8 encoding
contract plus the borrowed-buffer ownership contract live in the XML
documentation for each parameter rather than in the identifier.

Fido2: ClientPin.SetPinAsync, ChangePinAsync, GetPinTokenAsync and
GetPinUvAuthTokenUsingPinAsync take pin, currentPin and newPin.
OpenPgp: IOpenPgpSession/OpenPgpSession VerifyPinAsync, VerifyAdminAsync,
ChangePinAsync, ChangeAdminAsync, SetResetCodeAsync and ResetPinAsync take
pin, currentPin, newPin and resetCode; the public abstract Kdf.Process
takes pin. Oath: IOathSession.DeriveKey takes password.

Several OpenPgp members had no <param> documentation at all, so the UTF-8
contract had been carried solely by the identifier. Those docs are now
written out, along with the caller's ownership and clearing obligation.

Also fixes a latent ArgumentException.ParamName defect surfaced by the
rename. ClientPin.ValidatePin is a shared private helper, so SetPinAsync
and ChangePinAsync reported the helper's own local name instead of the
public parameter the caller supplied. The helper now takes an explicit
paramName, matching HsmAuthSession.ValidateCredentialPassword.

Names and documentation only. No encoding, ownership, padding,
zeroization, or CTAP/OpenPGP wire behaviour changed; no ZeroMemory call or
try/finally was added, removed or moved. Breaking for named arguments
only, with no [Obsolete] aliases, per the alpha posture.
ValidatePin reports the caller's public parameter name, but the existing
ClientPinTests only asserted the exception type, so a wrong nameof would
have gone unnoticed. Assert the exact ParamName for SetPinAsync (newPin),
both ChangePinAsync routes (currentPin, newPin), GetPinTokenAsync (pin)
and GetPinUvAuthTokenUsingPinAsync (pin).

Three existing SetPinAsync tests and both ChangePinAsync tests were
extended in place; GetPinTokenAsync and GetPinUvAuthTokenUsingPinAsync
had no length-validation test at all, so those two are new.
@DennisDyallo DennisDyallo added the v2 label Sep 4, 2026
@DennisDyallo DennisDyallo changed the title Simplify device events, cryptography, YubiHSM, and WebAuthn APIs refactor: Simplify device events, cryptography, YubiHSM, and WebAuthn APIs Sep 4, 2026
Our event architecture wins; upstream's device-identity behaviour is carried in.

- YubiKeyDeviceRepository.UpdateCache keeps our DeviceEventHub fan-out while
  gaining upstream's serial-substitution republish and DeviceInfo retention.
- DeviceChanges/RecordingObserver stay deleted; upstream tests written against
  the observer harness are ported to WatchAsync/DeviceEventWatcher with every
  assertion preserved.
- Upstream's flat device model (YubiKeyDevice, connection slots) arrives intact.

Verified: 12/12 projects, 2757 tests, 0 failed; resilience 75/0; AOT 10+2.
- Rename ListenerEvents_..._PerTransport to _EvictingEvidenceGlobally; upstream
  reworded the doc to say eviction is global but left the old name.
- Strip a stray trailing newline (.editorconfig sets insert_final_newline=false).
- Point hid-listener-rescan-hints evidence at DeviceEventHub.cs, replacing the
  deleted broadcaster/stream files.
- Clarify that UpdateCache orders publication at enqueue, not delivery.
Publication never runs consumer code, but the resumption thread is not
guaranteed: a buffered event can complete MoveNextAsync synchronously on the
caller's thread. The previous text promised background-thread delivery.
AGENTS.md becomes a pointer to CLAUDE.md per upstream's consolidation; the
operational detail it used to carry now lives in TOOLCHAIN.md (verified: total:,
RequiresUserPresence, No tests matched all present there). Our dotnet format
whitespace guidance moves to CLAUDE.md, which is now canonical.
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Coverage and CRAP

module methods CRAP Δ CRAP ≥8 cog>15 coverage Δ cov
Core 1159 8004 +15 139 5 65.2% .
Management 35 130 . 3 0 26.5% .
Piv 191 1723 . 44 3 67.8% .
Fido2 387 2366 -67 43 0 82.6% .
WebAuthn 143 1079 -143 32 1 64.7% +1.0pp
Oath 55 217 . 8 0 74.5% .
YubiOtp 112 264 . 3 0 65.8% .
OpenPgp 156 1011 . 21 0 56.7% .
SecurityDomain 43 315 . 7 0 64.0% .
YubiHsm 50 154 . 3 0 79.7% .
Cli.Commands 188 5261 . 91 2 2.5% .
Cli.Shared 71 617 . 14 2 27.4% .
TOTAL 2590 21141 -194 408 13

CRAP decreased by 194.

Note

CRAP increased in: Core (+15).

/// </summary>
public bool IsRetry { get; init; }

/// <summary>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...

@DennisDyallo

Copy link
Copy Markdown
Collaborator Author

Hardware verification complete — HotPlugIdentityContractTests now has a real hardware run behind it, not just a compile check.

Rig: single authorized USB YubiKey, SN 103, FW 5.8.0.

Control run first (no human interaction, to prove the test is not a sham guard): waited the full 120 s and failed correctly with No Removed event within 120 s. The test genuinely requires the ceremony.

Ceremony run — PASSED (28 s):

[00:00.036] BASELINE    ykphysical:pid:0407  serial=103
[00:25.646] EVENT Removed  serialAtEvent=103  ref=00ca8a98
[00:25.756] REMOVED delivered ref=00ca8a98 retainedSerial=103
[00:27.839] EVENT Added    serialAtEvent=103  ref=0256a4f7
[00:28.019] REINSERTED     serial=103         ref=0256a4f7
watcher pump ended with OperationCanceledException

The distinct object references (00ca8a980256a4f7) confirm both contracts over real hardware: Removed delivers the retained published object with its serial intact, and reinsertion publishes a new object that still correlates as DeviceCorrelation.Same. Events flowed through the merged DeviceEventHub.

This also validates the lazy-subscription fix applied during the port — WatchAsync does not subscribe until the first MoveNextAsync(), so the enumerator is started on the test thread before StartMonitoring. Without it the watcher would have missed the initial scan.

One caveat worth recording: an earlier attempt appeared to pass in 3 s immediately after the rig was re-arranged, because discovery was still settling and residual insertion events satisfied both waits. Valid test, invalid ceremony. Anyone re-running this should let the rig settle first and check the timeline timestamps look human-paced.

@DennisDyallo
DennisDyallo merged commit dbcf775 into yubikit Sep 6, 2026
8 checks passed
@DennisDyallo
DennisDyallo deleted the yubikit-isa-integration branch September 6, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

1 participant