Added cargo test for checking compliance to fedora-crypto-policies#308
Merged
Jakob-Naucke merged 1 commit intoJul 13, 2026
Conversation
Reviewer's GuideAdds a new test that enforces Fedora crypto policy compliance by scanning shipped, non-dev dependencies for unapproved crypto crates and wiring it into the test suite and CI lint workflow. Sequence diagram for new Fedora crypto policy compliance test in CIsequenceDiagram
actor Developer
participant GitHubActions as GitHubActions_lint_workflow
participant Cargo as cargo
participant Test as no_disallowed_crypto_test
Developer->>GitHubActions: push_changes
GitHubActions->>Cargo: cargo test --test no_disallowed_crypto
Cargo->>Test: run_tests
Test->>Test: scan_shipped_dependencies
alt [disallowed crypto crate found]
Test-->>Cargo: test_failure
Cargo-->>GitHubActions: non_zero_exit_status
GitHubActions-->>Developer: report_lint_job_failure
else [no disallowed crypto crate]
Test-->>Cargo: test_success
Cargo-->>GitHubActions: zero_exit_status
GitHubActions-->>Developer: report_lint_job_success
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/no_disallowed_crypto.rs" line_range="155-159" />
<code_context>
+ .iter()
+ .filter(|pkg| !workspace_members.contains(pkg["id"].as_str().unwrap_or_default()))
+ .filter(|pkg| normal_deps.contains(pkg["name"].as_str().unwrap_or_default()))
+ .filter(|pkg| {
+ !ALLOWED_CRYPTO_CRATES.contains(&pkg["name"].as_str().unwrap_or_default())
+ })
+ .filter(|pkg| {
+ let categories = as_lowercase_str_vec(&pkg["categories"]);
+ let keywords = as_lowercase_str_vec(&pkg["keywords"]);
+ categories.iter().any(|c| c == "cryptography")
</code_context>
<issue_to_address>
**suggestion:** Consider how to handle crypto crates that lack `cryptography` category or `crypto` keyword to avoid silently missing non-compliant crates.
Because detection depends on `categories` including `"cryptography"` or `keywords` including `"crypto"`, any crate that implements its own crypto/TLS without these tags will be missed.
To mitigate this:
- Consider adding a blocklist of known non-OpenSSL crypto/TLS crates alongside the allowlist.
- At minimum, document in the test that it relies on crates.io metadata and may miss untagged crypto crates, so the limitation is clear.
This helps avoid tests giving a false sense of coverage when mis-tagged crypto crates are introduced.
Suggested implementation:
```rust
.filter(|pkg| {
let name = pkg["name"].as_str().unwrap_or_default();
let categories = as_lowercase_str_vec(&pkg["categories"]);
let keywords = as_lowercase_str_vec(&pkg["keywords"]);
let looks_like_crypto =
categories.iter().any(|c| c == "cryptography")
|| keywords.iter().any(|k| k.contains("crypto"));
let is_blocklisted_known_crypto_crate =
DISALLOWED_CRYPTO_CRATES.contains(&name);
// NOTE: This test relies on crates.io metadata (categories/keywords) to
// detect crypto usage and may miss crates that implement their own
// crypto/TLS without appropriate tags. The explicit blocklist of known
// non-OpenSSL crypto/TLS crates (DISALLOWED_CRYPTO_CRATES) exists to
// mitigate this, but coverage is necessarily best-effort.
looks_like_crypto || is_blocklisted_known_crypto_crate
})
```
To fully implement the suggestion, you should also:
1. Define the `DISALLOWED_CRYPTO_CRATES` blocklist alongside `ALLOWED_CRYPTO_CRATES`, for example:
```rust
const DISALLOWED_CRYPTO_CRATES: &[&str] = &[
"rustls",
"rustls-pemfile",
"ring",
"boring",
// extend this list as additional non-OpenSSL crypto/TLS crates are introduced
];
```
2. Optionally add a short module- or test-level doc comment at the top of `tests/no_disallowed_crypto.rs` explicitly stating that this test is best-effort, relies on crates.io metadata, and uses a curated blocklist, so readers understand its limitations.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+155
to
+159
| .filter(|pkg| { | ||
| !ALLOWED_CRYPTO_CRATES.contains(&pkg["name"].as_str().unwrap_or_default()) | ||
| }) | ||
| .filter(|pkg| { | ||
| let categories = as_lowercase_str_vec(&pkg["categories"]); |
There was a problem hiding this comment.
suggestion: Consider how to handle crypto crates that lack cryptography category or crypto keyword to avoid silently missing non-compliant crates.
Because detection depends on categories including "cryptography" or keywords including "crypto", any crate that implements its own crypto/TLS without these tags will be missed.
To mitigate this:
- Consider adding a blocklist of known non-OpenSSL crypto/TLS crates alongside the allowlist.
- At minimum, document in the test that it relies on crates.io metadata and may miss untagged crypto crates, so the limitation is clear.
This helps avoid tests giving a false sense of coverage when mis-tagged crypto crates are introduced.
Suggested implementation:
.filter(|pkg| {
let name = pkg["name"].as_str().unwrap_or_default();
let categories = as_lowercase_str_vec(&pkg["categories"]);
let keywords = as_lowercase_str_vec(&pkg["keywords"]);
let looks_like_crypto =
categories.iter().any(|c| c == "cryptography")
|| keywords.iter().any(|k| k.contains("crypto"));
let is_blocklisted_known_crypto_crate =
DISALLOWED_CRYPTO_CRATES.contains(&name);
// NOTE: This test relies on crates.io metadata (categories/keywords) to
// detect crypto usage and may miss crates that implement their own
// crypto/TLS without appropriate tags. The explicit blocklist of known
// non-OpenSSL crypto/TLS crates (DISALLOWED_CRYPTO_CRATES) exists to
// mitigate this, but coverage is necessarily best-effort.
looks_like_crypto || is_blocklisted_known_crypto_crate
})To fully implement the suggestion, you should also:
- Define the
DISALLOWED_CRYPTO_CRATESblocklist alongsideALLOWED_CRYPTO_CRATES, for example:const DISALLOWED_CRYPTO_CRATES: &[&str] = &[ "rustls", "rustls-pemfile", "ring", "boring", // extend this list as additional non-OpenSSL crypto/TLS crates are introduced ];
- Optionally add a short module- or test-level doc comment at the top of
tests/no_disallowed_crypto.rsexplicitly stating that this test is best-effort, relies on crates.io metadata, and uses a curated blocklist, so readers understand its limitations.
SpaceFace02
force-pushed
the
fedora-crypt-policy-compliance-test
branch
from
38ee33a to
bef5d96
Compare
Jakob-Naucke
left a comment
Jakob-Naucke
left a comment
Member
There was a problem hiding this comment.
Indeed, we should get oci-client to use jsonwebtoken-openssl when native-tls is enabled, but it's a bit of a separate issue.
SpaceFace02
force-pushed
the
fedora-crypt-policy-compliance-test
branch
from
bef5d96 to
066d138
Compare
Signed-off-by: Chirag Rao <crao@redhat.com>
SpaceFace02
force-pushed
the
fedora-crypt-policy-compliance-test
branch
from
066d138 to
e4f975f
Compare
Jakob-Naucke
approved these changes
Jul 13, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Jakob-Naucke, SpaceFace02 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Jakob-Naucke
merged commit 73fdc7a
into
trusted-execution-clusters:main
11 of 12 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added a test to check for compliance to fedora-crypt-policies for the final shipped bundle, without test-suite deps and build deps.
the allowlist is subject to change based on inputs.
Summary by Sourcery
Add an automated check ensuring shipped binaries do not depend on non-compliant cryptography crates.
Tests: