alktls/tasks/coverage-cheap-closes.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
coverage-cheap-closes	Cheap coverage closes — non-Ed25519 pin arms, PEM parse-error arm, fallback seam, escape-hatch methods (U-2, re-baselined)	completed		narrow	low	component	implementation	tests coverage review-001 u2

Description

The coverage inventory (review 001 Part C) — re-baselined 2026-09-12 by a fresh cargo llvm-cov --all-features run after ADR-007/ADR-008 landed (commits 49d4432/ac440f3): the new handshake and impersonation suites already closed two of the original groups, and ADR-008's new verifier opened one new gap. Current per-line ground truth (all line numbers re-verified against the current tree):

1. client.rs:316 — FingerprintPinVerifier:: verify_tls12_signature's non-Ed25519 else-arm (rustls::crypto::verify_tls12_signature) is never executed. (The original group-2 text pointed at the TLS 1.3 twin at 297/316-322 — that TLS 1.3 arm is now covered by the handshake suite's pin suites; only the TLS 1.2 else-arm remains.) Pinned test: an ECDSA-P256 rcgen cert + dss_with_scheme (P256/SHA256) against verify_tls12_signature, assert ok, then a forged sig → err. Mirrors the existing Ed25519 TLS 1.3 routing pin (fingerprint_pin_verifier_routes_ed25519_spki_tls13_signature_ through_raw_key_path, client.rs:472).
2. server.rs:349-366 — VerifyPresentedCertVerifier:: verify_tls12_signature's body (ADR-008's default verifier; did not exist at decomposition time) is entirely uncovered: no TLS 1.2 handshake exists in the suite. Same pop-routing shape as the pin verifier (Ed25519-SPKI → verify_tls13_signature_with_raw_key, else verify_tls12_signature). A direct unit-call test (Ed25519 SPKI + valid sig → ok; wrong-key or mismatched-message dss → err) closes it. This group must be included or the ≥98% acceptance bar is unreachable while the ACME loop stays open (owned by acme-event-loop-test).
3. pem.rs:30 — the Err(e) => Err(io::Error::other(e)) arm of load_private_key (a parse failure, distinct from "no key found") has no test: garbage-but-keyed file (b"-----BEGIN PRIVATE KEY-----\n!!!\n-----END PRIVATE KEY-----\n") exercises it. Verified against rustls-pemfile 2.2.0: malformed base64 in a keyed section is a real Error::Base64Decoding-class Err from private_key — the prescribed input works.
4. client.rs:145-148 — the webpki-roots fallback push loop is covered only nondeterministically (passes vacuously when the platform store is non-empty). Deterministic remediation: a #[cfg(test)]-visible helper taking the "native certs" as a parameter (or an injectable load_native_certs seam) so the empty-platform case is testable without root. This is a load-bearing invariant whose fallback branch has no deterministic test. (rustls_native_certs::CertificateResult has public certs / errors fields, so the seam shape is a plain refactor — see Work item 2.) The tracing::warn! for native-cert errors at client.rs:139 sits in the same uncovered pocket.
5. server.rs:468-475 — AcceptAnyCertVerifier (now the escape-hatch verifier post-ADR-008, moved to server.rs:429) is only partially covered by the impersonation suite's escape-hatch tests: verify_tls12_signature's body and supported_verify_schemes() (468-475) are never called. Two-line test each (call, assert Ok / assert the nine-scheme list), making the no-pop posture explicit in the suite. (The original group-6 range 287-303 is now VerifyPresentedCertVerifier's code — the file shifted under the ADR-008 commit.) Note: the original "coordinate with OQ-TLS-09's resolution" caveat is stale — OQ-TLS-09 is resolved (ADR-008), the escape-hatch posture is permanent and pinned by tests/impersonation_posture.rs.
6. fingerprint.rs:67 — the second disjunct (len() != 33 || [0] != 0x00) matrix case (34-byte bit-string + unused-bits ≠ 0 vs len ≠ 33). Cosmetic.

Already closed by the ADR-007/ADR-008 suites (do not redo): the client resolvers' resolve calls (original group 1 — RawKeyClientCertResolver::resolve and NoClientCertResolver::resolve are exercised by the handshake suites) and RawKeyCertResolver::resolve (original group 5 — exercised end-to-end by tests/handshake_behavior.rs, e.g. raw_key_client_presents_spki_and_server_extracts_fingerprint). FingerprintPinVerifier::verify_tls13_signature's non-Ed25519 arm (original group 2's TLS 1.3 half) is also covered.

Work

1. Write the unit tests (in-module #[cfg(test)] where the items are private-visible, tests/ where public API suffices).
2. Item 4 is the only one touching non-test code: extract a test-visible seam for the fallback loop. Keep the seam #[cfg(test)]-visible or behind a plain pub(crate) fn — do not grow the public API.
3. Run llvm-cov and confirm the ranges close.

Verification

• cargo llvm-cov --all-features shows the six groups covered (client.rs 316/145-148, pem.rs 30, server.rs 349-366/468-475, fingerprint.rs 67)
• The fallback test deterministically exercises the push loop (platform-store-independent)
• cargo test, cargo test --all-features, clippy, fmt green

Acceptance Criteria

• Line coverage ≥ 98% (from 95.81% currently; 95.32% at decomposition) with every load-bearing uncovered group closed
• No public-API growth

References

• docs/reviews/001-implementation-review.md §U-2, Part C (the inventory rows for the closed groups are stale — this task's Description is the current baseline)
• src/client.rs, src/pem.rs, src/server.rs, src/fingerprint.rs
• tests/handshake_behavior.rs (the suites that closed original groups 1 and 5), tests/impersonation_posture.rs (the escape-hatch pins — group 5's verify_tls13_signature coverage comes from here)

Notes

Agent fills this during implementation.

Work notes (2026-09-12, verified against the current tree before and after):

• Line drift since the re-baseline: the two doc-pin commits (9bdc32d, 7713a6e) shifted the ADR-008 ranges. Current ground truth at task start: client.rs TLS 1.2 pin else-arm = 334; server.rs VerifyPresentedCertVerifier::verify_tls12_signature = 414-431; AcceptAnyCertVerifier::verify_tls12_signature = 538-545 (supported_verify_schemes() at 547-549 was already covered).
• Group 1 (client.rs:334): the else-arm needs a parseable X.509 cert — verify_tls12_signature maps the cert through webpki::EndEntityCert. Test: rcgen ECDSA-P256 cert + any_ecdsa_type signer + ECDSA_NISTP256_SHA256 DSS → ok; forged sig → err. (rcgen's KeyPair::generate() is already P-256; its message-signing is pub(crate), so the signer comes from rustls::crypto::aws_lc_rs::sign::any_ecdsa_type.)
• Group 2 (server.rs:414-431): three direct unit-call tests — Ed25519 SPKI → raw-key path ok + forged → err; X.509 → standard path ok + valid-sig-under-different-key → err; Ed25519 sig over a mismatched message → err.
• Group 3 (pem.rs:30): the prescribed garbage-but-keyed input verified against rustls-pemfile 2.2.0 first (returns Err(InvalidCharacter(33))) — works as prescribed.
• Group 4: seam extracted as fill_root_store_from_native(&CertificateResult) (plain private fn, no API growth). Tests construct CertificateResult::default() and push into its public certs / errors fields (probed: construction works despite #[non_exhaustive] — it derives Default and Error's fields are public). Three tests: empty → all webpki-roots anchors (count-checked, deterministic); errors + valid cert → valid cert loads (also covers the client.rs:139 tracing::warn! pocket); valid cert → used verbatim, no fallback.
• Group 5 (server.rs:538-545): verify_tls12_signature asserts unconditionally (pinned with garbage cert + garbage sig → Ok); added the nine-scheme pin for VerifyPresentedCertVerifier:: supported_verify_schemes() (its list delegates to nine_supported_verify_schemes(); AcceptAnyCertVerifier's list was already pinned).
• Group 6 (fingerprint.rs:67): the task's second-disjunct concern was already half-covered, but the existing bad_bit_string_lengths_extract_nothing passed vacuously — its crafted SPKIs declared outer SEQUENCE lengths longer than the actual bytes (0x2b/0x2d/0x2d vs actual 0x29/0x2b/0x2a), so parsing failed at the outer TLV and never reached the bit-string checks. Fixed the three declared lengths and added a tag-matrix test (non-OID tag inside AlgorithmIdentifier; well-formed SPKI with no BIT STRING after the alg-id).
• Coverage leftovers, documented as out of scope: the two panic! arms of the error-path tests were converted to .err().expect(..) (closes client.rs:949 and server.rs:1029 — the config types are deliberately not Debug, so .err() avoids expect_err's T: Debug bound). fingerprint.rs 125/143 (the } else { None } tail arms of expect_sequence / expect_bit_string) remain "uncovered" in llvm-cov output — proven (temporary eprintln + region dump) to be an llvm-cov region attribution artifact: the else arms execute (region counts 19 and
  1. but llvm attributes the region to the read_tlv()? line, not the None literal. The parser is behavior-preservation code; restructuring it for a tooling artifact is not warranted.
• Remaining uncovered lines crate-wide are the ACME event loop (owned by acme-event-loop-test) and the ACME-only task arms.

Summary

Agent fills this on completion.

Closed all six re-baselined U-2 coverage groups. Tests added:

• client.rs: fingerprint_pin_verifier_verify_tls12_signature_routes_x509_through_standard_path (ECDSA-P256 → standard TLS 1.2 path ok + forged → err); empty_platform_store_deterministically_falls_back_to_webpki_roots, native_cert_errors_are_logged_and_valid_certs_still_load, native_certs_fill_the_store_without_the_fallback (deterministic fallback pins over the new fill_root_store_from_native seam) + dss_with_scheme / native_result test helpers.
• server.rs: three VerifyPresentedCertVerifier::verify_tls12_signature tests (Ed25519 raw-key route ok/forged-err, X.509 route ok/wrong-key-err, mismatched-message err), the escape hatch's verify_tls12_signature no-pop pin, and the VerifyPresentedCertVerifier nine-scheme list pin + dss_with_scheme helper.
• pem.rs: load_private_key_parse_error_yields_cert_load_error (garbage-but-keyed input → TlsError::CertLoad).
• fingerprint.rs: corrected the vacuous bad_bit_string_lengths_extract_nothing (wrong outer lengths) so the line-67 disjunct genuinely executes, and added wrong_tag_before_oid_and_missing_bit_string_extract_nothing.

Only non-test change: load_platform_root_cert_store now delegates to a private fill_root_store_from_native(&CertificateResult) seam — no public-API growth.

Verification: cargo llvm-cov --all-features line coverage 98.26% (from 95.90%), all six groups covered; cargo test (80 lib + 13 integration), cargo test --all-features (89 lib + 29 integration), cargo clippy --all-targets -- -D warnings (default and --all-features), cargo fmt --check — all green.