Repository navigation
refactor(fulcio)!: typed responses, private requests and a client builder - #248
Merged
Merged
Conversation
…lder
- `SigningCertificate` is a `#[non_exhaustive]` enum over the response's
oneof: `EmbeddedSct { chain }` or `DetachedSct { chain,
signed_certificate_timestamp }`, with PEM certificates parsed into
`DerCertificate`s up front. `leaf_certificate()` and
`certificate_chain()` are infallible, because an empty chain is rejected
while parsing.
- `TrustBundle` holds parsed `Vec<Vec<DerCertificate>>` chains; the cache
now stores the raw response and parses it like a fresh one.
- Request types (`CreateSigningCertificateRequest`, `Credentials`,
`PublicKeyRequest`, `PublicKeyData`) are private. `Credentials` no
longer derives `Debug`, which printed the raw OIDC token.
- `FulcioClient::new` and `FulcioClientBuilder::build` return `Result`
instead of panicking; the builder gains `timeout` (was `with_timeout`)
and `user_agent` (default `sigstore-rust/<version>`) and trims trailing
slashes. The built-in `public()` / `staging()` URLs are removed.
- `Error` is `#[non_exhaustive]`. Non-success responses are
`Status { status, message }`, malformed ones `InvalidResponse`, and key
pair failures `Signing`; `Api` and `Certificate` are removed.
BREAKING CHANGE: `SigningCertificate` and `TrustBundle` changed shape;
the request types and `CertificateChain`, `ChainContent` and
`CertificateWithSCT` are no longer public; `FulcioClient::new` and
`build` return `Result`; `with_timeout` is `timeout`;
`FulcioClient::public` and `staging` are removed; `Error` variants
changed.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Wolf Vollprecht <w.vollprecht@gmail.com>
Follow the Rust API guidelines' casing for acronyms. BREAKING CHANGE: `OIDCIssuer` is renamed `OidcIssuer`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Wolf Vollprecht <w.vollprecht@gmail.com>
Callers configure timeouts, proxies, TLS roots and the user agent on their own reqwest::Client and pass it with FulcioClientBuilder::with_http_client. This replaces the builder's own timeout and user_agent options. Without one, a client with a 30-second timeout and a sigstore-rust user agent is used. reqwest is re-exported so callers can build a matching client. BREAKING CHANGE: FulcioClientBuilder::timeout and user_agent are replaced by with_http_client(reqwest::Client); reqwest is part of the public API. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Wolf Vollprecht <w.vollprecht@gmail.com>
This was referenced Sep 24, 2026
jku
approved these changes
Sep 25, 2026
jku
left a comment
jku
left a comment
Member
There was a problem hiding this comment.
I'm approving, this looks good, but have a look at the comment on get_trust_bundle -- should we just get rid of it?
| Some(client) => client, | ||
| None => reqwest::Client::builder() | ||
| .timeout(DEFAULT_TIMEOUT) | ||
| .user_agent(DEFAULT_USER_AGENT) |
Member
There was a problem hiding this comment.
this is a good call -- we should do this in other clients too. This can be very useful on the service side.
For future work, maybe should allow users of the library to override the UA
| /// | ||
| /// With the `cache` feature enabled and a cache configured, this will | ||
| /// cache the trust bundle with the default TTL (24 hours). | ||
| pub async fn get_trust_bundle(&self) -> Result<TrustBundle> { |
Member
There was a problem hiding this comment.
this feels like API we could alternatively just get rid of?
It feels like a debugging tool included in the API -- TrustBundle is not even used by any other part of the API.
Collaborator
Author
There was a problem hiding this comment.
Thanks for the comment, will chekc it out!
Merged
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.
Summary
1.0 API work, PR F in the plan, as two commits.
refactor(fulcio)!: typed responses, private requests and a client builderSigningCertificateis a#[non_exhaustive]enum over the response's oneof:EmbeddedSct { chain }/DetachedSct { chain, signed_certificate_timestamp }DerCertificates up frontleaf_certificate()andcertificate_chain()are infallible, since empty chains, a missing certificate, and both SCT kinds at once are rejected while parsingTrustBundleholds parsedVec<Vec<DerCertificate>>chains. The cache now stores the raw response and parses it like a fresh oneCredentialsno longer derivesDebug, which used to print the raw OIDC tokenFulcioClient::new/buildreturnResultinstead of panicking, and the builder trims trailing slashespublic()/staging()URLs are removed, the same as for Rekor (refactor(rekor)!: remove built-in log URLs and fix stale docs #240), the TSA (refactor(tsa)!: keep the RFC 3161 ASN.1 model private #245) and OIDC (refactor(oidc)!: explicit identity provider configuration #243)Erroris#[non_exhaustive]:Status { status, message }for non-success responses, so callers can tell a 401 from a 5xxInvalidResponsefor malformed bodiesSigningfor key-pair failuresrefactor(fulcio)!: rename OIDCIssuer to OidcIssuer: acronym casing per the API guidelines.refactor(fulcio)!: accept a caller-configured reqwest client:FulcioClientBuilder::with_http_client(reqwest::Client)replaces the builder's owntimeout/with_timeout. Callers configure timeouts, proxies, TLS roots, the user agent and the TLS provider (#220) on their client. Without one, a client with a 30 s timeout and asigstore-rust/<version>UA is used.reqwestis re-exported and is now a public dependency, which was a deliberate decision. The other HTTP clients (Rekor, TSA, OIDC, TUF) will get the same shape.The only file this shares with #247 is
sigstore-sign/src/sign.rs, in a different hunk.Validation
cargo clippy --workspace --all-targets --all-features -- -D warningscargo test -p sigstore-fulcio -p sigstore-sign -p sigstore-conformance --all-features(new tests: SCT oneof parsing, trust-bundle parsing)cargo check -p sigstore-fulcio --no-default-featuresSigned-off-by: Wolf Vollprecht w.vollprecht@gmail.com
🤖 Generated with Claude Code