Vault and Azure: the backend-said-no branches are untested (52% and 46% branch coverage)

[kirill-abblix]

Measurement

Line coverage flatters these two packages; branch coverage does not. From the merged solution run (3556 tests, all green):

package	lines	branches
Abblix.Oidc.Server.Vault	91.9%	51.9% (80/154)
Abblix.Oidc.Server.Azure	78.5%	45.9% (45/98)

Every other production project sits between 71% and 89% on branches. These two are the outliers, and they are the two that talk to an external custodian over the network.

A line is counted as covered when it runs once. A line like if (properties.Enabled != true) therefore reads as covered while only the true side has ever been taken. That is the whole gap here: the happy path runs, the answers-we-do-not-like do not.

Reproduce with dotnet test Abblix.Oidc.slnx -c Debug --coverage --coverage-output-format cobertura, then read condition-coverage per line rather than the line hits.

Where the untaken branches are

Almost all of them sit in the two classes that speak to the backend:

• Vault/TransitCustodian.cs - 62 of the package's 74 missed branches
• Azure/KeyVaultClient.cs - 48 of the package's 53

The rest is a handful in ApiResponse.cs, KeyValueStore.cs, Transport.cs, BlobKeyRingStore.cs and ServiceCollectionExtensions.cs.

They fall into two kinds, and the second kind is the one that matters.

Algorithm mapping switches, one arm walked

• TransitCustodian.cs:120 and :135 - the algorithm and hash-algorithm switches
• TransitCustodian.cs:254 - the RSA key-family check
• KeyVaultClient.cs:120 and :165 - MapSignatureAlgorithm and MapEncryptionAlgorithm

Cheap to close with a theory over the algorithm names. No backend needed. Worth doing because a wrong arm here means signing with an algorithm the caller did not ask for, and the mistake is invisible until a verifier elsewhere rejects the result.

Guards on an answer we do not like

• KeyVaultClient.cs:197 - if (properties.Enabled != true). Disabling a key in Key Vault is how an operator revokes a compromised key. The branch that honours that has never been taken. If it is wrong, revocation silently does not revoke, and nothing in the suite would say so.
• KeyVaultClient.cs:261 - if (separator <= 0 || separator == keyId.Length - 1), the malformed key-identifier guard.
• KeyVaultClient.cs:103 - the credential-configuration check.
• The remaining untaken branches in ApiResponse.cs, Transport.cs and KeyValueStore.cs are the backend-said-no paths.

Proposal

• Cover the mapping switches with theories. Mechanical, no infrastructure, closes the bulk of the count.
• Cover the guards with a stub custodian that answers badly: a disabled key, a malformed identifier, a rejected credential, a transport failure. These are the branches an operator's real actions reach, so a test here defends behaviour rather than a number.
• Verify the disabled-key path against a live backend. There is an OpenBao container in the loop already and Azurite for the Azure side; a unit test with a stub proves the branch is wired, a live one proves the backend actually reports what the branch expects. Both, not either.
• Exclude generated sources from the measurement - obj/**/LoggerMessage.g.cs contributes untaken branches in both packages and nothing a human can act on. Same measurement noise as Mvc adapter coverage: opt-in endpoints are never walked end to end #274.

Why this is not "raise the number"

Both packages exist to keep a private key inside an HSM or KMS. The interesting behaviour of such a client is not what it does when the custodian cooperates - it is what it does when the custodian refuses, returns a disabled key, or cannot be reached. That is precisely the half that is currently untested.

Acceptance

• Branch coverage in both packages in line with the rest of the solution.
• The disabled-key path covered by a unit test and confirmed against a live backend.
• Every algorithm arm of every mapping switch exercised.
• Generated sources no longer counted.

[kirill-abblix]

Progress note from #276, which covers the custodian half of this issue.

Reading the source corrected part of this issue's framing. The paths I assumed were dark are in fact covered: a rejected ciphertext becoming null, a 403 throwing, an unsupported algorithm refused before any call. The branch counter was pointing at something else.

What was genuinely untested, and is now covered:

• A disabled Key Vault version being skipped - the revocation path.
• A version reporting no creation time - the rotation-ordering guard.
• Vault throwing on 429, 503 and 500, not only on 403.
• A Transit key type this store cannot publish (ed25519, symmetric).
• Vault's signing algorithm table, arm by arm, asserted against the wire.

Each was verified by mutation rather than by watching it pass: the production source was broken, the test went red, the source was restored.

One thing that could not be done

Azure's mapping table cannot be asserted at the wire the way Vault's can. KeyVaultClient reaches the vault through the Azure SDK, and by the time the injected HttpMessageHandler sees the request its content is already gone, so the algorithm actually sent is not observable. Confirmed empirically: the sign call arrives as POST /keys/{name}/{version}/sign with a null Content.

Options, none of them free:

• Make the two mappers internal and assert them directly. Changes production visibility for a test, though the file already has an internal member for a different reason.
• Assert through the SDK's own policy pipeline rather than the transport, which means constructing the client differently in tests than in production - a seam that then goes untested itself.
• Leave it. The arms are a literal one-to-one table between two enums with identical member names, so a transposition is possible but visible on review, unlike the Vault case where the two vocabularies genuinely differ.

Left alone for now. Azure's branch figure is therefore unmoved by #276; Vault went from 51.9% to 76.4%.

[kirill-abblix]

Re-measured, and neither package is an outlier any more

On develop, unioning per-line condition coverage across the ten reports:

package	lines	branches	this issue recorded
Abblix.Jwt.Vault	99.2%	76.4% (113/148)	51.9%
Abblix.Jwt.Azure	91.4%	81.5% (75/92)	45.9%

(Both moved to the Abblix.Jwt.* family since this was written.) Against the rest of the solution, Azure is now above average on branches and Vault is mid-pack. The lowest branch coverage in the solution today is Abblix.Oidc.Server.MinimalApi at 63.0%, which has no issue of its own.

The branch this issue was most worried about is closed. KeyVaultClient.cs if (properties.Enabled != true) - disabling a key being how an operator revokes a compromised one - now has both arms taken. So do the malformed key-identifier guard and the credential-configuration check.

The algorithm-mapping switches are also both fully walked: MapSignatureAlgorithm and its Vault counterpart are covered by theories over all nine JWS algorithms, and MapEncryptionAlgorithm reads 100% (6/6).

What was still open

Neither store had a test for refusing ECDH-ES. Both must refuse it, because neither backend exposes a key-agreement primitive - Key Vault has none and Transit has none - and a silent fallback would derive a shared secret the peer did not derive, surfacing later as an undecryptable token with nothing pointing back. Added to both.

The Azure key-management mapping also gained a theory over all three algorithms it maps rather than the one the server asks for by default. Worth having, since the arm chosen decides the RSA padding to unwrap with and a wrong arm returns bytes that are not the content encryption key. It did not move the number: that switch was already at 100%, and I took the line reference from this issue instead of from my own fresh measurement.

Still open, recorded

Abblix.Jwt.Azure/ServiceCollectionExtensions.cs has 17 dark lines (the blob key-ring registration overloads) - a larger gap than anything left in the client itself, and not what this issue is about. TransitCustodian.ParseKeyId and KeyValueStore each keep one untaken arm on a malformed-input guard.

Suggestion

The measurement that motivated this issue no longer supports it. If the branch-coverage concern is still live, it points at Abblix.Oidc.Server.MinimalApi (63.0%) and Abblix.Oidc.Server.AspNetCore (68.3%) rather than at these two.