test: Wave 17 - insta snapshot tests for all key-type crates#41
Merged
EffortlessSteven merged 3 commits intomainfrom Mar 1, 2026
Merged
test: Wave 17 - insta snapshot tests for all key-type crates#41EffortlessSteven merged 3 commits intomainfrom
EffortlessSteven merged 3 commits intomainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (25)
📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoAdd insta snapshot tests for all key-type crates (Wave 17)
WalkthroughsDescription• Add insta snapshot tests for all 6 key-type crates • Snapshot PEM shapes, DER lengths, JWK structures across RSA, ECDSA, Ed25519 • Snapshot HMAC secret lengths and token metadata with redacted sensitive data • Add insta and serde dev-dependencies to all key-type crates Diagramflowchart LR
RSA["RSA<br/>5 snapshots"]
ECDSA["ECDSA<br/>6 snapshots"]
ED25519["Ed25519<br/>5 snapshots"]
HMAC["HMAC<br/>3 snapshots"]
TOKEN["Token<br/>4 snapshots"]
X509["X509<br/>4 snapshots"]
RSA -- "PEM shapes, DER lengths, JWK, KID" --> INSTA["Insta Snapshots"]
ECDSA -- "PEM shapes, DER lengths, JWK, KID" --> INSTA
ED25519 -- "PEM shapes, DER lengths, JWK, KID" --> INSTA
HMAC -- "Secret lengths, JWK, KID" --> INSTA
TOKEN -- "Token shapes, auth headers" --> INSTA
X509 -- "Cert/key PEM, DER, metadata" --> INSTA
INSTA -- "Redacted crypto material" --> DETECT["Detect unintended changes"]
File Changes1. crates/uselesskey-rsa/tests/snapshots_rsa.rs
|
Code Review by Qodo
1. Token prefix leaks secrets
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+26
to
+35
| let shape = TokenShape { | ||
| kind: "api_key", | ||
| len: value.len(), | ||
| prefix: value.chars().take(8).collect(), | ||
| value: value.to_string(), | ||
| }; | ||
|
|
||
| insta::assert_yaml_snapshot!("token_api_key_shape", shape, { | ||
| ".value" => "[REDACTED]", | ||
| }); |
There was a problem hiding this comment.
1. Token prefix leaks secrets 📘 Rule violation ⛨ Security
The new token snapshot tests include a prefix derived from the real token value and commit it into .snap files, leaking token material via repo artifacts/test output. This violates the requirement to avoid leaking secret tokens in debug/log-like outputs.
Agent Prompt
## Issue description
Token snapshot tests currently serialize and snapshot a `prefix` derived from the real token string. Even though `.value` is redacted, the committed `.snap` output still contains part of the secret token.
## Issue Context
The compliance policy forbids leaking token/key material via debug/log-like outputs; insta snapshots are committed artifacts and will appear in diffs/CI output.
## Fix Focus Areas
- crates/uselesskey-token/tests/snapshots_token.rs[26-35]
- crates/uselesskey-token/tests/snapshots_token.rs[44-53]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+20
to
+27
| fn pem_shape(pem: &str) -> PemShape { | ||
| let lines: Vec<&str> = pem.lines().collect(); | ||
| PemShape { | ||
| first_line: lines.first().unwrap_or(&"").to_string(), | ||
| last_line: lines.last().unwrap_or(&"").to_string(), | ||
| line_count: lines.len(), | ||
| total_len: pem.len(), | ||
| } |
There was a problem hiding this comment.
2. Pem headers committed snapshots 📘 Rule violation ⛨ Security
The new PEM-shape snapshots commit PEM boundary markers like -----BEGIN PRIVATE KEY----- and -----BEGIN CERTIFICATE----- into .snap files, which are secret-shaped blobs likely to trigger secret scanners. This violates the policy to not commit PEM/DER/token-shaped material into the repository.
Agent Prompt
## Issue description
PEM-shape snapshots currently commit PEM boundary markers (e.g., `-----BEGIN PRIVATE KEY-----`) into insta `.snap` files, creating secret-shaped repository blobs.
## Issue Context
Even without base64 bodies, PEM boundary markers commonly trigger secret scanning/push-protection and violate the repository policy to avoid committing PEM/DER/token-shaped artifacts.
## Fix Focus Areas
- crates/uselesskey-ecdsa/tests/snapshots_ecdsa.rs[20-52]
- crates/uselesskey-rsa/tests/snapshots_rsa.rs[20-51]
- crates/uselesskey-ed25519/tests/snapshots_ed25519.rs[20-51]
- crates/uselesskey-x509/tests/snapshots_x509.rs[20-46]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Pull request overview
Adds deterministic insta snapshot tests across key/token/x509 crates to catch unintended format/metadata changes without snapshotting secret material.
Changes:
- Introduces new snapshot test files for
uselesskey-{rsa,ecdsa,ed25519,hmac,token,x509}. - Adds corresponding
.snapgolden files under each crate’stests/snapshots/. - Adds
insta+serde(derive)test-time dependencies to relevant crates.
Reviewed changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/uselesskey-x509/tests/snapshots_x509.rs | Adds PEM/DER/metadata snapshot tests for X509 outputs |
| crates/uselesskey-x509/tests/snapshots/snapshots_x509__x509_private_key_pem_shape.snap | Snapshot for X509 private key PEM shape |
| crates/uselesskey-x509/tests/snapshots/snapshots_x509__x509_der_lengths.snap | Snapshot for X509 DER length metadata |
| crates/uselesskey-x509/tests/snapshots/snapshots_x509__x509_cert_pem_shape.snap | Snapshot for X509 cert PEM shape |
| crates/uselesskey-x509/tests/snapshots/snapshots_x509__x509_cert_metadata.snap | Snapshot for parsed X509 certificate metadata |
| crates/uselesskey-x509/Cargo.toml | Adds insta/serde dev deps for snapshot tests |
| crates/uselesskey-token/tests/snapshots_token.rs | Adds token shape + auth header snapshot tests |
| crates/uselesskey-token/tests/snapshots/snapshots_token__token_oauth_shape.snap | Snapshot for OAuth token shape metadata |
| crates/uselesskey-token/tests/snapshots/snapshots_token__token_bearer_shape.snap | Snapshot for bearer token shape metadata |
| crates/uselesskey-token/tests/snapshots/snapshots_token__token_authorization_headers.snap | Snapshot for authorization header shapes/lengths |
| crates/uselesskey-token/tests/snapshots/snapshots_token__token_api_key_shape.snap | Snapshot for API key token shape metadata |
| crates/uselesskey-token/Cargo.toml | Adds insta/serde for token snapshot tests |
| crates/uselesskey-rsa/tests/snapshots_rsa.rs | Adds RSA PEM/DER/JWK/KID snapshot tests |
| crates/uselesskey-rsa/tests/snapshots/snapshots_rsa__rsa_der_lengths.snap | Snapshot for RSA DER lengths by key size |
| crates/uselesskey-rsa/tests/snapshots/snapshots_rsa__rsa_2048_public_pem_shape.snap | Snapshot for RSA public key PEM shape |
| crates/uselesskey-rsa/tests/snapshots/snapshots_rsa__rsa_2048_private_pem_shape.snap | Snapshot for RSA private key PEM shape |
| crates/uselesskey-rsa/tests/snapshots/snapshots_rsa__jwk_snapshots__rsa_kid_format.snap | Snapshot for RSA KID format metadata |
| crates/uselesskey-rsa/tests/snapshots/snapshots_rsa__jwk_snapshots__rsa_2048_public_jwk.snap | Snapshot for RSA public JWK structure (redacted fields) |
| crates/uselesskey-rsa/Cargo.toml | Adds insta/serde dev deps for RSA snapshot tests |
| crates/uselesskey-hmac/tests/snapshots_hmac.rs | Adds HMAC secret length + JWK/KID snapshot tests |
| crates/uselesskey-hmac/tests/snapshots/snapshots_hmac__jwk_snapshots__hmac_kid_format.snap | Snapshot for HMAC KID format across algs |
| crates/uselesskey-hmac/tests/snapshots/snapshots_hmac__jwk_snapshots__hmac_hs256_jwk.snap | Snapshot for HMAC HS256 JWK structure (redacted fields) |
| crates/uselesskey-hmac/tests/snapshots/snapshots_hmac__hmac_secret_lengths.snap | Snapshot for HMAC secret byte lengths |
| crates/uselesskey-hmac/Cargo.toml | Adds insta/serde(_json) dev deps for HMAC snapshots |
| crates/uselesskey-ed25519/tests/snapshots_ed25519.rs | Adds Ed25519 PEM/DER/JWK/KID snapshot tests |
| crates/uselesskey-ed25519/tests/snapshots/snapshots_ed25519__jwk_snapshots__ed25519_public_jwk.snap | Snapshot for Ed25519 public JWK structure (redacted fields) |
| crates/uselesskey-ed25519/tests/snapshots/snapshots_ed25519__jwk_snapshots__ed25519_kid_format.snap | Snapshot for Ed25519 KID format metadata |
| crates/uselesskey-ed25519/tests/snapshots/snapshots_ed25519__ed25519_public_pem_shape.snap | Snapshot for Ed25519 public PEM shape |
| crates/uselesskey-ed25519/tests/snapshots/snapshots_ed25519__ed25519_private_pem_shape.snap | Snapshot for Ed25519 private PEM shape |
| crates/uselesskey-ed25519/tests/snapshots/snapshots_ed25519__ed25519_der_lengths.snap | Snapshot for Ed25519 DER lengths |
| crates/uselesskey-ed25519/Cargo.toml | Adds insta/serde dev deps for Ed25519 snapshots |
| crates/uselesskey-ecdsa/tests/snapshots_ecdsa.rs | Adds ECDSA P-256/P-384 PEM/DER/JWK/KID snapshot tests |
| crates/uselesskey-ecdsa/tests/snapshots/snapshots_ecdsa__jwk_snapshots__ecdsa_p384_public_jwk.snap | Snapshot for ECDSA P-384 public JWK structure (redacted fields) |
| crates/uselesskey-ecdsa/tests/snapshots/snapshots_ecdsa__jwk_snapshots__ecdsa_p256_public_jwk.snap | Snapshot for ECDSA P-256 public JWK structure (redacted fields) |
| crates/uselesskey-ecdsa/tests/snapshots/snapshots_ecdsa__jwk_snapshots__ecdsa_kid_format.snap | Snapshot for ECDSA KID format metadata |
| crates/uselesskey-ecdsa/tests/snapshots/snapshots_ecdsa__ecdsa_p384_private_pem_shape.snap | Snapshot for ECDSA P-384 private PEM shape |
| crates/uselesskey-ecdsa/tests/snapshots/snapshots_ecdsa__ecdsa_p256_private_pem_shape.snap | Snapshot for ECDSA P-256 private PEM shape |
| crates/uselesskey-ecdsa/tests/snapshots/snapshots_ecdsa__ecdsa_der_lengths.snap | Snapshot for ECDSA DER lengths by curve |
| crates/uselesskey-ecdsa/Cargo.toml | Adds insta/serde dev deps for ECDSA snapshots |
Comments suppressed due to low confidence (5)
crates/uselesskey-token/tests/snapshots_token.rs:1
- The snapshots redact
.value, but still persist the first 8 characters of the token in.prefix(and that prefix is committed in the.snapfiles). This conflicts with “Token values are always redacted” and can leak sensitive material; consider replacingprefixwith non-sensitive metadata (e.g.,starts_with_uk_test: bool,prefix_len: usize, or a redacted/constant placeholder) so no token characters are stored in snapshots.
crates/uselesskey-token/tests/snapshots_token.rs:1 - The snapshots redact
.value, but still persist the first 8 characters of the token in.prefix(and that prefix is committed in the.snapfiles). This conflicts with “Token values are always redacted” and can leak sensitive material; consider replacingprefixwith non-sensitive metadata (e.g.,starts_with_uk_test: bool,prefix_len: usize, or a redacted/constant placeholder) so no token characters are stored in snapshots.
crates/uselesskey-x509/tests/snapshots_x509.rs:1 is_self_signedis derived fromsubject == issuer, which only checks name equality and does not actually verify the certificate is self-signed (signature validation). Either rename the field to reflect what it measures (e.g.,subject_eq_issuer) or compute self-signed status by verifying the signature against the cert’s own public key.
crates/uselesskey-rsa/tests/snapshots_rsa.rs:72- The
algorithmfield is being set to"RSA-2048"/"RSA-4096"(key size), and thealgtuple element ("RS256","RS512") isn’t actually used to configure the keypair (RsaSpec::new(bits)ignores it). To avoid confusion, consider renamingalgorithmto something likekey_size/key_type, and derive it directly frombits(eliminating thealgstring + match).
let cases: Vec<DerLengths> = [
(2048, "RS256", "snap-rsa-2048"),
(4096, "RS512", "snap-rsa-4096"),
]
.into_iter()
.map(|(bits, alg, label)| {
let kp = fx.rsa(label, RsaSpec::new(bits));
DerLengths {
algorithm: match alg {
"RS256" => "RSA-2048",
_ => "RSA-4096",
},
private_key_der_len: kp.private_key_pkcs8_der().len(),
public_key_der_len: kp.public_key_spki_der().len(),
}
})
crates/uselesskey-token/Cargo.toml:25
instaandserde(derive)appear to be introduced for snapshot tests; if they’re not required by the library’s public code, prefer adding them under[dev-dependencies]to avoid increasing downstream compile time and dependency surface for normal consumers.
base64.workspace = true
serde_json.workspace = true
proptest.workspace = true
insta.workspace = true
serde = { workspace = true, features = ["derive"] }
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
EffortlessSteven
force-pushed
the
wave-17/snapshot-tests
branch
from
15d6d2b to
1ac4490
Compare
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
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.
Summary
Adds deterministic snapshot tests using insta for all 6 key-type crates:
Pattern
All snapshot tests follow the established pattern from existing adapter crate snapshots:
Testing
Determinism impact: None
Policy impact: None (no blob additions)