Skip to content

chore(ci): harden docs API and test coverage - #12

Merged
thieman merged 3 commits into
mainfrom
thieman/fix-docs-ci-api-hardening
Jun 25, 2026
Merged

chore(ci): harden docs API and test coverage#12
thieman merged 3 commits into
mainfrom
thieman/fix-docs-ci-api-hardening

Conversation

@thieman

@thieman thieman commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add FIPS denied-lint CI coverage and rustdoc warning validation; cfg-gate non-FIPS-only key exchange helpers to keep FIPS clippy clean.
  • Move FIPS instructions into public docs, document/encode Windows-target-only build policy, remove unused WinRT feature, and align cipher-suite preference docs with provider order.
  • Re-export TLS13_CHACHA20_POLY1305_SHA256 through cipher_suite, add API/doctest coverage, and ignore the live crates.io interoperability test by default.

Fixes #9

Validation

  • cargo fmt -- --check
  • cargo check --target x86_64-pc-windows-msvc
  • cargo check --target x86_64-pc-windows-msvc --no-default-features
  • cargo check --features fips --target x86_64-pc-windows-msvc
  • cargo check --all-features --target x86_64-pc-windows-msvc
  • cargo clippy --target x86_64-pc-windows-msvc -- -D warnings
  • cargo clippy --target x86_64-pc-windows-msvc --no-default-features -- -D warnings
  • cargo clippy --features fips --target x86_64-pc-windows-msvc -- -D warnings
  • cargo clippy --all-features --target x86_64-pc-windows-msvc -- -D warnings
  • RUSTDOCFLAGS='-D warnings' cargo doc --no-deps --target x86_64-pc-windows-msvc
  • cargo check on macOS host now fails with the intended Windows-target policy message.

Not run locally

  • Windows runtime tests (cargo test, cargo test --doc) because this workstation cannot execute Windows test binaries; cargo check --test api --target x86_64-pc-windows-msvc also cannot complete locally because aws-lc-sys needs Windows SDK headers (windows.h) when dev-dependencies are compiled for the Windows target.

@thieman thieman changed the title Harden docs, CI, and API exports chore(ci): harden docs API and test coverage Jun 23, 2026
Comment thread README.md Outdated

```bash
cargo test test_to_internet -- --ignored
```

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This isn't necessary, we can assume test runners have network access. Simplify the tests and remove this part of the docs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Removed the ignored-test docs and restored the crates.io interoperability test to the default test set in 8f49924.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Direct answer: this section was not necessary. I added it because I assumed default tests should avoid live network dependencies, but in this repo/CI we can assume network access for test runners. I removed the docs and restored the test to the default suite in 8f49924.

Comment thread src/lib.rs
/// * `TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256`
/// * `TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256`
/// * `TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384`
/// * `TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why'd you move this around? It seems like it was already ordered correctly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Reverted the RSA TLS 1.2 cipher-suite doc ordering change in 8f49924.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Direct answer: I moved it because my review notes said the prose preference order differed from the actual ALL_CIPHER_SUITES array order. Re-checking it here, that churn is not needed for this PR and it distracts from the docs/CI/API scope. I reverted the ordering change in 8f49924.

Comment thread src/lib.rs Outdated
//! provider.cipher_suites[0].suite(),
//! CipherSuite::TLS13_CHACHA20_POLY1305_SHA256
//! );
//! ```

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What's this comment block for?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Removed that rustdoc example block in 8f49924. The API export is still covered by the dedicated tests/api.rs test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Direct answer: that block was meant to make the new ChaCha20 public re-export show up in rustdoc/doctest coverage. It is overkill in module docs because tests/api.rs already exercises constructing a custom provider with TLS13_CHACHA20_POLY1305_SHA256. I removed the block in 8f49924.

Comment thread Cargo.toml
[target.'cfg(windows)'.dependencies]
windows = { version = "0.61.3", features = [
"Win32_Security_Cryptography",
"Win32_System_WinRT",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What is this and are you sure it was unused?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Reverted the Cargo.toml dependency reshuffle and WinRT feature removal in 8f49924. That change was not necessary for this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Direct answer: I moved windows to a target-specific dependency and removed Win32_System_WinRT because local grep showed no source references to WinRT APIs and I was trying to reduce non-Windows dependency exposure. That was too broad for this PR, and I did not prove the feature was unnecessary across generated bindings/platform behavior. I reverted the Cargo.toml change in 8f49924.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Investigated more deeply and removed it in 17b2971. Evidence: repo search has no WinRT, System_WinRT, or Win32_System references outside the old Cargo feature; after removal, cargo tree -e features -i windows --target x86_64-pc-windows-msvc no longer contains Win32_System or Win32_System_WinRT; and the Windows-target validation set passes (cargo check default/no-default/fips, clippy default/no-default/fips with -D warnings, and denied-warning rustdoc). So this feature was unused by the crate and safe to remove.

Comment thread src/kx.rs Outdated
// This product includes software developed at Datadog (https://www.datadoghq.com/)
// Copyright 2026 Datadog, Inc.

#[cfg(not(feature = "fips"))]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Are all these config gates necessary? How was this working before?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Removed the extra cfg(not(feature = "fips")) gates in 8f49924. To keep --features fips clippy clean without those gates, fips_provider() now uses kx::default_kx_groups() and then filters to FIPS-approved groups.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[GPT 5.5] Direct answer: no, all those gates were not necessary. It worked before because the default KX helper code was compiled under --features fips but not used, producing dead-code warnings only when we added FIPS clippy with -D warnings. Instead of cfg-gating the helper code everywhere, I changed fips_provider() to call kx::default_kx_groups() and then filter to FIPS groups, so the existing helper path remains used and FIPS clippy stays clean. That replacement is in 8f49924.

@thieman
thieman marked this pull request as ready for review June 24, 2026 20:11
@thieman
thieman requested a review from a team as a code owner June 24, 2026 20:11
@thieman
thieman merged commit 465e6ce into main Jun 25, 2026
10 checks passed
@thieman
thieman deleted the thieman/fix-docs-ci-api-hardening branch June 25, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden docs, CI, API exports, and test reliability

2 participants